Tarantool development patches archive
 help / color / mirror / Atom feed
From: Serge Petrenko <sergepetrenko@tarantool.org>
To: Vladimir Davydov <vdavydov.dev@gmail.com>
Cc: kostja@tarantool.org, tarantool-patches@freelists.org
Subject: Re: [PATCH v2 1/4] Introduce separate entity object types for entity privileges.
Date: Wed, 22 Aug 2018 18:42:07 +0300	[thread overview]
Message-ID: <34DD46B3-6FB8-423F-9FEC-AB47211C2FF8@tarantool.org> (raw)
In-Reply-To: <6d03b3dad54d482d419fa4e61c35f37676a04da0.1534944662.git.sergepetrenko@tarantool.org>

Sorry, forgot one minor change in priv_def_check(). No need to check for object_id 0
here anymore.

Here’s everything that’s changed.

@@ -2583,10 +2623,8 @@ priv_def_check(struct priv_def *priv, enum priv_type priv_type)
                break;
        case SC_SPACE:
        {
-               struct space *space = NULL;
-               if (priv->object_id != 0)
-                       space = space_cache_find_xc(priv->object_id);
-               if ((space == NULL || space->def->uid != grantor->def->uid) &&
+               struct space *space = space_cache_find_xc(priv->object_id);
+               if (space->def->uid != grantor->def->uid &&
                    grantor->def->uid != ADMIN) {
                        tnt_raise(AccessDeniedError,
                                  priv_name(priv_type),
@@ -2597,10 +2635,8 @@ priv_def_check(struct priv_def *priv, enum priv_type priv_type)
        }
        case SC_FUNCTION:
        {
-               struct func *func = NULL;
-               if (priv->object_id != 0)
-                       func = func_cache_find(priv->object_id);
-               if ((func == NULL || func->def->uid != grantor->def->uid) &&
+               struct func *func = func_cache_find(priv->object_id);
+               if (func->def->uid != grantor->def->uid &&
                    grantor->def->uid != ADMIN) {
                        tnt_raise(AccessDeniedError,
                                  priv_name(priv_type),
@@ -2611,10 +2647,8 @@ priv_def_check(struct priv_def *priv, enum priv_type priv_type)
        }
        case SC_SEQUENCE:
        {
-               struct sequence *seq = NULL;
-               if (priv->object_id != 0)
-                       seq = sequence_cache_find(priv->object_id);
-               if ((seq == NULL || seq->def->uid != grantor->def->uid) &&
+               struct sequence *seq = sequence_cache_find(priv->object_id);
+               if (seq->def->uid != grantor->def->uid &&
                    grantor->def->uid != ADMIN) {
                        tnt_raise(AccessDeniedError,
                                  priv_name(priv_type),


> 22 авг. 2018 г., в 16:39, Serge Petrenko <sergepetrenko@tarantool.org> написал(а):
> 
> When granting or revoking a privilege on an entire entity, id 0 was used
> to indicate the fact that we don't grant a privilege on a single object,
> but on a whole entity. This caused confusion, because for entity USER,
> for example, id 0 is a valid object id (user 'guest' uses it).
> Any non-zero id dedicated to this cause obviously may be confused as well.
> Fix this by creating separate schema_object_types for entities:
> SC_ENTITY_SPACE, SC_ENTITY_SEQUENCE, etc.
> 
> Closes: #3574
> Prerequisite: #3524
> ---
> src/box/alter.cc             |  39 ++++++++++++++++++++++++++++-
> src/box/bootstrap.snap       | Bin 1540 -> 1556 bytes
> src/box/lua/schema.lua       |  58 ++++++++++++++++++++++++++-----------------
> src/box/lua/upgrade.lua      |  23 +++++++++++++++++
> src/box/schema.cc            |   9 +++----
> src/box/schema.h             |  23 +++++++++--------
> src/box/schema_def.c         |  17 +++++++++++++
> src/box/schema_def.h         |  16 +++++++++++-
> src/box/user.cc              |  27 +++++++++++---------
> test/box-py/bootstrap.result |  14 +++++------
> test/box/access.result       |   6 ++---
> test/box/access_misc.result  |   8 +++---
> test/box/alter.result        |   8 +++---
> test/xlog/upgrade.result     |  14 +++++------
> 14 files changed, 184 insertions(+), 78 deletions(-)
> 
> diff --git a/src/box/alter.cc b/src/box/alter.cc
> index 3007a131d..8eba21a51 100644
> --- a/src/box/alter.cc
> +++ b/src/box/alter.cc
> @@ -2537,10 +2537,35 @@ priv_def_create_from_tuple(struct priv_def *priv, struct tuple *tuple)
> {
> 	priv->grantor_id = tuple_field_u32_xc(tuple, BOX_PRIV_FIELD_ID);
> 	priv->grantee_id = tuple_field_u32_xc(tuple, BOX_PRIV_FIELD_UID);
> +
> 	const char *object_type =
> 		tuple_field_cstr_xc(tuple, BOX_PRIV_FIELD_OBJECT_TYPE);
> -	priv->object_id = tuple_field_u32_xc(tuple, BOX_PRIV_FIELD_OBJECT_ID);
> 	priv->object_type = schema_object_type(object_type);
> +
> +	const char *data = tuple_field(tuple, BOX_PRIV_FIELD_OBJECT_ID);
> +	if (data == NULL) {
> +		tnt_raise(ClientError, ER_NO_SUCH_FIELD,
> +			  BOX_PRIV_FIELD_OBJECT_ID + TUPLE_INDEX_BASE);
> +	}
> +	/*
> +	 * When granting or revoking privileges on a whole entity
> +	 * we pass empty string ('') to object_id to indicate
> +	 * grant on every object of that entity.
> +	 * So check for that first.
> +	 */
> +	switch (mp_typeof(*data)) {
> +	case MP_STR:
> +		if (mp_decode_strl(&data) == 0) {
> +			/* Entity-wide privilege. */
> +			priv->object_id = 0;
> +			priv->object_type = schema_entity_type(priv->object_type);
> +			break;
> +		}
> +		FALLTHROUGH;
> +	default:
> +		priv->object_id = tuple_field_u32_xc(tuple,
> +						     BOX_PRIV_FIELD_OBJECT_ID);
> +	}
> 	if (priv->object_type == SC_UNKNOWN) {
> 		tnt_raise(ClientError, ER_UNKNOWN_SCHEMA_OBJECT,
> 			  object_type);
> @@ -2645,6 +2670,18 @@ priv_def_check(struct priv_def *priv, enum priv_type priv_type)
> 		}
> 		/* Not necessary to do during revoke, but who cares. */
> 		role_check(grantee, role);
> +		break;
> +	}
> +	case SC_ENTITY_SPACE:
> +	case SC_ENTITY_FUNCTION:
> +	case SC_ENTITY_SEQUENCE:
> +	{
> +		/* Only admin may grant privileges on an entire entity. */
> +		if (grantor->def->uid != ADMIN) {
> +			tnt_raise(AccessDeniedError, priv_name(priv_type),
> +				  schema_object_name(priv->object_type), name,
> +				  grantor->def->name);
> +		}
> 	}
> 	default:
> 		break;
> diff --git a/src/box/bootstrap.snap b/src/box/bootstrap.snap
> index b610828c9c9ae9a22acdd8c150c16c6838b7a273..44992b050e9d73d69608fb92782d1e965f6cdc51 100644
> GIT binary patch
> delta 1551
> zcmV+q2JrcW43rFz7=JM>GB!9ZXE|m$WMeloGhzx!ZgX^DZewLSAT%>EG&o@}VJ$c{
> zGcheRV>UP~VPZ03EipATVq!NoIX7cCWeQe9Y;R+0Iv{&}3JTS_3%bn(nE=k-$Bkd5
> z0000004TLD{QyvfEC41w*h<hAaRLBeJb=_J{}F0FAgJ*mVt>V;#?6&k&og$Aul<dv
> zv3Dv(N-{sS_S_R$=If$P=IFJ-Qc5dSF3)wMv;i7#PyNg>UnEmX0mA^f0L=io5sT!g
> zD{Fnf#vxPKYgy*n?1AMVIy{=p9gia^hC=L0fWLuz8rC_P&bxEnm$jbL+0jvRI_+sG
> zru|Ih;m)d9Ab%fU2!MAhTHBbGce$H&nf~2*&yFmHLT-w8E2;yCj1eht_F<%$wYjrA
> z--_qmirxZNlbP>7cRjpY(I5*1%V7?t>8y7vDuOB;TVebwm5d3mRG{g<l`_3ftd)rc
> zn_;zHClt(+{hgbmxU+pKw$>&pI!j<g$w35)6B;Hdw}0vc|76#A%XVkpWtdu!!EHY`
> zQ473V(OEa~Z9g}|)Sgs&h}w*uq0W{Rc(<Zvf8ImXW-LrCDFM%iVB<2)IG&t`=^X0p
> z%Yh$fD8!Boc(<bFh|kp4F)S=%TMBr$q8k55lCM#${Y=DS+Bfbz&A~d;aQkK7b>5ZV
> zdG3dP6Mx6^(8sb6!1;Xg8sDAa&WZ-$y>W_Zn~dpXc#Nf8o1_@R#?P@Ion`uUK3>En
> z4|O&qz`GSKF%8FiNe275855I3zK&RX`$!^s><!J0t0shXfpC3jd1!TLaj@ww*s37t
> z-HLiP9v};~La9)x)67Zcgesj%8+J-DBoj%4cYiCoL(1@BuzjT$lI`Wf$ik??2xi9d
> ztc3#01eTBGMdTtHiCXRru~^$q5*v~bieRnCyA@ptRojk|H*sWb>UR9^o~tSri06BO
> z0Q-*j>z9Q*4+(#G7_m1GyjxLQ$D%fO4}ENn1MgPUw1qv^wQ}ZNwi<Z1qOu?JnRn@f
> zM}I7Goln@sxA7u>0uPF;){6CdrBo=>OXh`5;7UxHP%|pfgijbup=MGrPps{8(Quhz
> zWw^YXTN9%N1NEZh;%hNtVMQxeEKqt?SWV_GYBSbz?d=0?f`WpY*c$IvG*qAOKeIM_
> zQh2wbDYx3&2cp<tnmn~GC&WCKzkjS};(teS=fpg|%u;9X?K_pwnFhnDwIsY-(GRlN
> ztgP{o#>Rx0;YE$dsj=v%(gx?x*@*;&XWUfcndq})KR0uozbQ*?y?v(<-mNH&3>7#_
> z#&nJefg`BVSY%8M%m4s@04M+v21hAoRSyz?&^U^NFbZH021E!rh)xs$fZ#w_L4OfM
> zEf@qq49vQeA))6t$8JsxhygJm2AP2;6jR1Ytzq>yuq)+j_YBuXSm^>oOc44oRR2+r
> zfKn$8@9xdDJsnXgj8fvJX-OOEnOz2N82f7Vr6^tAcX*WFIPM{0Ntj!pLMS~2mv!oE
> zRrM)J%oMN{r_N{V(s7_ah4$kjihs$6<OnC-Xy|vNdloebHTnit@EUCcD|n6OjV)em
> zPC$*u2HNnREZ$rv)082si4@Jx$$z3_Ifq*d0kp^Vv;;5qa1a|JkDvz!Y7ZzgZLtTG
> znKqel_G}Nk_kmTD8UY2Zw|;97AY4LmR2{?0eUDLb#Sq+kdJS#-4;^$U>3_%%=)mhn
> z2W|IQc+vMG<^-d?>WOsJ?wF5DwFGHVZ;5{eV6!P9un202v<JSzI-4XFWp2pfJ=}&9
> z|B3!R7$^i;Ur3=7lANu}q6eAjK|>w!Z8R4d19B91M{)$#!E>qp7jG6HO>MyhU^@4T
> z^f(MLmIX$FanP|HqB(=3bAPgI29$n(?koqvkkh_+Jdt&9kemV%zz>2&MdX*2g^(0B
> zb3fr-G#O-Ux`l`P)LF58aj(U8%fMRlqE2MOof#G*F;Vmq(Um!gfT;c#$##_N2vs0b
> zGn`~?u8)RY;W>4D58f)EMzh}yokR<1H8AM^(7KO=Ygngs1QB1+=m<%o!R6Hut?jEW
> B)aL*I
> 
> delta 1534
> zcmV<a1p)e$41^4j7=JJ=G%zh^V>dT4F*z~{Np5p=VQyn(Iv_b=V_`LAF*YqTV`gS8
> zG-hTvEnzrjFfCy?W;Qf4VP!dGGGq!?Lu_wjYdRo%F*+bOeF_TIx(m9^1&9F7A|rpb
> zr2qf`001bpFZ}>e{VM<lJ=RLl7I6XqUp$Ch%_7lYJ_Lj_@PFoXP~+we(A76EO7~<&
> z)F3;RA|;uhw(`4WgTRw>I&<{eU?~Ai$VOI~v629dx2JaIm@kx4N&&zCxB$xlBK}6v
> zM#}nreKVfmYggvF>;bcooE<&pjzw|&ydgCd;B)Xi4C|Up=iRyONm;Mu=<J|49rmyj
> z!~XSg?^aVP(0>n31mId~$~K1OP2OJJrGIzc!y|v*5Q}mxHFpG|K|%#jeQ^A;E^l>*
> z3*Bp}NefgxX1+gm^KmUTgDMc1Jsb?P)oZC4!mL!?OgGcvU!7uNP_0u4V%4c@$$Tde
> z<JJuBKo=+bJ1@oWHsew%rA$(CR4!bhCq$aiEJ3j}Cx2ttc)M<~yh-n@9Ru2aUZxgY
> zOU+j=F>OCDy|bcHe0;hLW1g*=6kJQqvOo9n=`!@rT1v3PA=sEaGtUmrgF1V*+HwF$
> z%o|ck2Ck)MDdN~U>lpNk)Rh9)Qd8p>$FU`TvVVQ}JNw3+pEy`&7|(v!cb#wKcU}Xc
> zztr*E^M9!-1a3aZxQyRo-mQ)X;JxvQVS5a1E-Z#;*X8(mp!hlb(b=Wn_TxWHa?e&p
> z0<NVdiD5jvlVdHR&M*6U`SOA@wvN<F<4PlXstt_|t42g+g<yeLeOP%|b(q;~x1wEl
> zEj90~M>ms_NyVg8C=MtXRLX=hZO}2jlujiL*MCw|r<8%iFyl-urCG}bk_A!)lFN(Z
> z*9rxd2`ZdQ%ZOz}8m(9>QmHeYBvm9q6rtA9wbTrW=uAh+mpZa8Z9M*WuhEnWw8OPP
> z0Ken?`duE+M1Fc0sWuO;rRJ>TPnWxUKDEZdwbU$UUQcC(VtJFR2G>&aT`%TX-lUJ$
> zzkkQk8h`!+Ak42Gb5O~I9X_GL4xhR(RGq@`<8YEmeGLBa%Ed{YaV#4wH7X6(wXrob
> zRx(mCOfa|?Bo}qm;#z9jPz5?Ii>k-mKV61Bw$iw6MNm)>lUj2vHAVFK{#e#!kBV!l
> ziDGG`ab1f2WXaFUg5t{}`TM8d6EBXt_J3uuRg$)9X&lOg&M*+4wU)S+njl=^i<EJZ
> zX2ppwLyQ}LBXiMF<cgb*y<$jG_{C2oetkYX_VcpV`IoM<QW}RMaV<4zXt3a^V}T&3
> z(OhIq3CsWhfB+}}AqGb&W<?JYfWSD4qc93!7zRWLI0_9I0RX{)ke~>l77Rj$ynhqj
> zxeG%W18Ui|uoin^FOJH&6BE><l!R4e54%@5i^3tJ2=RjuhoRy}$qP!PWxP;CvBe2+
> zB`AQ$m7GU4)NS@llAFhZ>1-;h#Us!zr2@yPWOP}$xk=>TS*Ej6Q8ZgBx&}sXh}Wym
> z;Yvs=Xm6s&oTIkS5VXc28)t^2+kc4qR4r1#;*gdpU~^0s$pG8pm?G)gVGer7p5*j@
> zrQSOro_cGm&fR}<@b8h^=iXW!%4reE=RqJFW<g443Z&4$8`^`Azzc12c=p(Su(w)T
> z1)jmCmtK6UOn@l~=HxoFllhK1;>sqt`1BcS0HC@As3ba#z^Ko9Pi8avYk$Oa!!S#&
> zSx3_!A8xyrpfT#3Y0zk_Hi}Y>p%$pI61F+5M3N&%2xKJAC;$CDD}<6?=$#5uoTZ5Z
> zCNj}EgB|fKv=$L@ag>MEJ_76DnN*W1Z`O`MZ9ptwx|K1yxMV1V1(Ltx%(4y7obk4E
> zW?=^8GIQ=kv4A0`wS7N1*>`Z!opukPx!_g@c{8|RJi<`!QS!z#<&+X#nnV8V6uo^%
> zFL^|hfwSZ-+|LAZ&d5i?Zt>-$tMH_1UnT!Yb{@SussOv@WfJ50JyfLCQd9Bw_N-dh
> k2=V>Mm|$S01}gnuSl5<E4okR>OyKJnJxL~`9Mur5?adb1g8%>k
> 
> diff --git a/src/box/lua/schema.lua b/src/box/lua/schema.lua
> index b9b8c9004..d347fd1e6 100644
> --- a/src/box/lua/schema.lua
> +++ b/src/box/lua/schema.lua
> @@ -1801,16 +1801,16 @@ local function privilege_name(privilege)
> end
> 
> local function object_resolve(object_type, object_name)
> +    if object_name ~= nil and type(object_name) ~= 'string'
> +            and type(object_name) ~= 'number' then
> +        box.error(box.error.ILLEGAL_PARAMS, "wrong object name type")
> +    end
>     if object_type == 'universe' then
> -        if object_name ~= nil and type(object_name) ~= 'string'
> -                and type(object_name) ~= 'number' then
> -            box.error(box.error.ILLEGAL_PARAMS, "wrong object name type")
> -        end
>         return 0
>     end
>     if object_type == 'space' then
> -        if object_name == nil or object_name == 0 then
> -            return 0
> +        if object_name == '' then
> +            return ''
>         end
>         local space = box.space[object_name]
>         if  space == nil then
> @@ -1819,8 +1819,8 @@ local function object_resolve(object_type, object_name)
>         return space.id
>     end
>     if object_type == 'function' then
> -        if object_name == nil or object_name == 0 then
> -            return 0
> +        if object_name == '' then
> +            return ''
>         end
>         local _vfunc = box.space[box.schema.VFUNC_ID]
>         local func
> @@ -1836,8 +1836,8 @@ local function object_resolve(object_type, object_name)
>         end
>     end
>     if object_type == 'sequence' then
> -        if object_name == nil or object_name == 0 then
> -            return 0
> +        if object_name == '' then
> +            return ''
>         end
>         local seq = sequence_resolve(object_name)
>         if seq == nil then
> @@ -1864,7 +1864,7 @@ local function object_resolve(object_type, object_name)
> end
> 
> local function object_name(object_type, object_id)
> -    if object_type == 'universe' then
> +    if object_type == 'universe' or object_id == '' then
>         return ""
>     end
>     local space
> @@ -2072,12 +2072,18 @@ local function grant(uid, name, privilege, object_type,
>                      object_name, options)
>     -- From user point of view, role is the same thing
>     -- as a privilege. Allow syntax grant(user, role).
> -    if object_name == nil and object_type == nil then
> -        -- sic: avoid recursion, to not bother with roles
> -        -- named 'execute'
> -        object_type = 'role'
> -        object_name = privilege
> -        privilege = 'execute'
> +    if object_name == nil then
> +        if object_type == nil then
> +            -- sic: avoid recursion, to not bother with roles
> +            -- named 'execute'
> +            object_type = 'role'
> +            object_name = privilege
> +            privilege = 'execute'
> +        else
> +            -- Allow syntax grant(user, priv, entity)
> +            -- for entity grants.
> +            object_name = ''
> +        end
>     end
>     local privilege_hex = privilege_check(privilege, object_type)
> 
> @@ -2117,10 +2123,16 @@ end
> local function revoke(uid, name, privilege, object_type, object_name, options)
>     -- From user point of view, role is the same thing
>     -- as a privilege. Allow syntax revoke(user, role).
> -    if object_name == nil and object_type == nil then
> -        object_type = 'role'
> -        object_name = privilege
> -        privilege = 'execute'
> +    if object_name == nil then
> +        if object_type == nil then
> +            object_type = 'role'
> +            object_name = privilege
> +            privilege = 'execute'
> +        else
> +            -- Allow syntax revoke(user, privilege, entity)
> +            -- to revoke entity privileges.
> +            object_name = ''
> +        end
>     end
>     local privilege_hex = privilege_check(privilege, object_type)
>     options = options or {}
> @@ -2192,8 +2204,8 @@ local function drop(uid, opts)
>     local privs = _vpriv.index.primary:select{uid}
> 
>     for k, tuple in pairs(privs) do
> -	-- we need an additional box.session.su() here, because of
> -	-- unnecessary check for privilege PRIV_REVOKE in priv_def_check()
> +        -- we need an additional box.session.su() here, because of
> +        -- unnecessary check for privilege PRIV_REVOKE in priv_def_check()
>         box.session.su("admin", revoke, uid, uid, tuple[5], tuple[3], tuple[4])
>     end
>     box.space[box.schema.USER_ID]:delete{uid}
> diff --git a/src/box/lua/upgrade.lua b/src/box/lua/upgrade.lua
> index 0293f6ef8..ec3826399 100644
> --- a/src/box/lua/upgrade.lua
> +++ b/src/box/lua/upgrade.lua
> @@ -964,6 +964,28 @@ local function upgrade_to_1_10_0()
>     create_vsequence_space()
> end
> 
> +--------------------------------------------------------------------------------
> +--- Tarantool 1.10.2
> +--------------------------------------------------------------------------------
> +local function upgrade_priv_to_1_10_2()
> +    local _priv = box.space._priv
> +    local _vpriv = box.space._vpriv
> +    local format = _priv:format()
> +
> +    format[4].type = 'scalar'
> +    _priv:format(format)
> +    format = _vpriv:format()
> +    format[4].type = 'scalar'
> +    _vpriv:format(format)
> +    _priv.index.primary:alter{parts={2, 'unsigned', 3, 'string', 4, 'scalar'}}
> +    _vpriv.index.primary:alter{parts={2, 'unsigned', 3, 'string', 4, 'scalar'}}
> +    _priv.index.object:alter{parts={3, 'string', 4, 'scalar'}}
> +    _vpriv.index.object:alter{parts={3, 'string', 4, 'scalar'}}
> +end
> +
> +local function upgrade_to_1_10_2()
> +    upgrade_priv_to_1_10_2()
> +end
> 
> local function get_version()
>     local version = box.space._schema:get{'version'}
> @@ -991,6 +1013,7 @@ local function upgrade(options)
>         {version = mkversion(1, 7, 6), func = upgrade_to_1_7_6, auto = false},
>         {version = mkversion(1, 7, 7), func = upgrade_to_1_7_7, auto = true},
>         {version = mkversion(1, 10, 0), func = upgrade_to_1_10_0, auto = true},
> +        {version = mkversion(1, 10, 2), func = upgrade_to_1_10_2, auto = true},
>     }
> 
>     for _, handler in ipairs(handlers) do
> diff --git a/src/box/schema.cc b/src/box/schema.cc
> index 433f52c08..aaa63a083 100644
> --- a/src/box/schema.cc
> +++ b/src/box/schema.cc
> @@ -535,11 +535,12 @@ schema_find_name(enum schema_object_type type, uint32_t object_id)
> {
> 	switch (type) {
> 	case SC_UNIVERSE:
> +	case SC_ENTITY_SPACE:
> +	case SC_ENTITY_FUNCTION:
> +	case SC_ENTITY_SEQUENCE:
> 		return "";
> 	case SC_SPACE:
> 		{
> -			if (object_id == 0)
> -				return "SPACE";
> 			struct space *space = space_by_id(object_id);
> 			if (space == NULL)
> 				break;
> @@ -547,8 +548,6 @@ schema_find_name(enum schema_object_type type, uint32_t object_id)
> 		}
> 	case SC_FUNCTION:
> 		{
> -			if (object_id == 0)
> -				return "FUNCTION";
> 			struct func *func = func_by_id(object_id);
> 			if (func == NULL)
> 				break;
> @@ -556,8 +555,6 @@ schema_find_name(enum schema_object_type type, uint32_t object_id)
> 		}
> 	case SC_SEQUENCE:
> 		{
> -			if (object_id == 0)
> -				return "SEQUENCE";
> 			struct sequence *seq = sequence_by_id(object_id);
> 			if (seq == NULL)
> 				break;
> diff --git a/src/box/schema.h b/src/box/schema.h
> index 0822262d0..f1735ff34 100644
> --- a/src/box/schema.h
> +++ b/src/box/schema.h
> @@ -250,16 +250,19 @@ static inline
> struct access *
> entity_access_get(enum schema_object_type type)
> {
> -       switch (type) {
> -       case SC_SPACE:
> -               return entity_access.space;
> -       case SC_FUNCTION:
> -               return entity_access.function;
> -       case SC_SEQUENCE:
> -               return entity_access.sequence;
> -       default:
> -               return NULL;
> -       }
> +	switch (type) {
> +	case SC_SPACE:
> +	case SC_ENTITY_SPACE:
> +		return entity_access.space;
> +	case SC_FUNCTION:
> +	case SC_ENTITY_FUNCTION:
> +		return entity_access.function;
> +	case SC_SEQUENCE:
> +	case SC_ENTITY_SEQUENCE:
> +		return entity_access.sequence;
> +	default:
> +		return NULL;
> +	}
> }
> 
> #endif /* INCLUDES_TARANTOOL_BOX_SCHEMA_H */
> diff --git a/src/box/schema_def.c b/src/box/schema_def.c
> index 97c074ab2..9091af054 100644
> --- a/src/box/schema_def.c
> +++ b/src/box/schema_def.c
> @@ -41,6 +41,23 @@ static const char *object_type_strs[] = {
> 	/* [SC_COLLATION]       = */ "collation",
> };
> 
> +enum schema_object_type
> +schema_entity_type(enum schema_object_type type)
> +{
> +	switch (type) {
> +	case SC_SPACE:
> +		return SC_ENTITY_SPACE;
> +	case SC_FUNCTION:
> +		return SC_ENTITY_FUNCTION;
> +	case SC_SEQUENCE:
> +		return SC_ENTITY_SEQUENCE;
> +	case SC_COLLATION:
> +		return SC_ENTITY_COLLATION;
> +	default:
> +		return SC_UNKNOWN;
> +	}
> +}
> +
> enum schema_object_type
> schema_object_type(const char *name)
> {
> diff --git a/src/box/schema_def.h b/src/box/schema_def.h
> index 2edb8d37f..d2fc39b76 100644
> --- a/src/box/schema_def.h
> +++ b/src/box/schema_def.h
> @@ -228,9 +228,23 @@ enum schema_object_type {
> 	SC_ROLE = 5,
> 	SC_SEQUENCE = 6,
> 	SC_COLLATION = 7,
> -	schema_object_type_MAX = 8
> +	/*
> +	 * All object types are supposed to be above this point,
> +	 * all entity types - below.
> +	 */
> +	schema_object_type_MAX = 8,
> +	SC_ENTITY_SPACE,
> +	SC_ENTITY_FUNCTION,
> +	SC_ENTITY_SEQUENCE,
> +	SC_ENTITY_COLLATION
> };
> 
> +/**
> + * Given a object type, return an entity type it belongs to.
> + */
> +enum schema_object_type
> +schema_entity_type(enum schema_object_type type);
> +
> enum schema_object_type
> schema_object_type(const char *name);
> 
> diff --git a/src/box/user.cc b/src/box/user.cc
> index fbf06566a..eec785652 100644
> --- a/src/box/user.cc
> +++ b/src/box/user.cc
> @@ -207,12 +207,23 @@ access_find(struct priv_def *priv)
> 		access = universe.access;
> 		break;
> 	}
> +	case SC_ENTITY_SPACE:
> +	{
> +		access = entity_access.space;
> +		break;
> +	}
> +	case SC_ENTITY_FUNCTION:
> +	{
> +		access = entity_access.function;
> +		break;
> +	}
> +	case SC_ENTITY_SEQUENCE:
> +	{
> +		access = entity_access.sequence;
> +		break;
> +	}
> 	case SC_SPACE:
> 	{
> -		if (priv->object_id == 0) {
> -			access = entity_access.space;
> -			break;
> -		}
> 		struct space *space = space_by_id(priv->object_id);
> 		if (space)
> 			access = space->access;
> @@ -220,10 +231,6 @@ access_find(struct priv_def *priv)
> 	}
> 	case SC_FUNCTION:
> 	{
> -		if (priv->object_id == 0) {
> -			access = entity_access.function;
> -			break;
> -		}
> 		struct func *func = func_by_id(priv->object_id);
> 		if (func)
> 			access = func->access;
> @@ -231,10 +238,6 @@ access_find(struct priv_def *priv)
> 	}
> 	case SC_SEQUENCE:
> 	{
> -		if (priv->object_id == 0) {
> -			access = entity_access.sequence;
> -			break;
> -		}
> 		struct sequence *seq = sequence_by_id(priv->object_id);
> 		if (seq)
> 			access = seq->access;
> diff --git a/test/box-py/bootstrap.result b/test/box-py/bootstrap.result
> index 16c2027cf..cf8242de5 100644
> --- a/test/box-py/bootstrap.result
> +++ b/test/box-py/bootstrap.result
> @@ -5,7 +5,7 @@ box.space._schema:select{}
> ---
> - - ['cluster', '<cluster uuid>']
>   - ['max_id', 511]
> -  - ['version', 1, 10, 0]
> +  - ['version', 1, 10, 2]
> ...
> box.space._cluster:select{}
> ---
> @@ -58,10 +58,10 @@ box.space._space:select{}
>         'type': 'string'}, {'name': 'auth', 'type': 'map'}]]
>   - [312, 1, '_priv', 'memtx', 0, {}, [{'name': 'grantor', 'type': 'unsigned'}, {
>         'name': 'grantee', 'type': 'unsigned'}, {'name': 'object_type', 'type': 'string'},
> -      {'name': 'object_id', 'type': 'unsigned'}, {'name': 'privilege', 'type': 'unsigned'}]]
> +      {'name': 'object_id', 'type': 'scalar'}, {'name': 'privilege', 'type': 'unsigned'}]]
>   - [313, 1, '_vpriv', 'sysview', 0, {}, [{'name': 'grantor', 'type': 'unsigned'},
>       {'name': 'grantee', 'type': 'unsigned'}, {'name': 'object_type', 'type': 'string'},
> -      {'name': 'object_id', 'type': 'unsigned'}, {'name': 'privilege', 'type': 'unsigned'}]]
> +      {'name': 'object_id', 'type': 'scalar'}, {'name': 'privilege', 'type': 'unsigned'}]]
>   - [320, 1, '_cluster', 'memtx', 0, {}, [{'name': 'id', 'type': 'unsigned'}, {'name': 'uuid',
>         'type': 'string'}]]
>   - [330, 1, '_truncate', 'memtx', 0, {}, [{'name': 'id', 'type': 'unsigned'}, {'name': 'count',
> @@ -104,13 +104,13 @@ box.space._index:select{}
>   - [305, 1, 'owner', 'tree', {'unique': false}, [[1, 'unsigned']]]
>   - [305, 2, 'name', 'tree', {'unique': true}, [[2, 'string']]]
>   - [312, 0, 'primary', 'tree', {'unique': true}, [[1, 'unsigned'], [2, 'string'],
> -      [3, 'unsigned']]]
> +      [3, 'scalar']]]
>   - [312, 1, 'owner', 'tree', {'unique': false}, [[0, 'unsigned']]]
> -  - [312, 2, 'object', 'tree', {'unique': false}, [[2, 'string'], [3, 'unsigned']]]
> +  - [312, 2, 'object', 'tree', {'unique': false}, [[2, 'string'], [3, 'scalar']]]
>   - [313, 0, 'primary', 'tree', {'unique': true}, [[1, 'unsigned'], [2, 'string'],
> -      [3, 'unsigned']]]
> +      [3, 'scalar']]]
>   - [313, 1, 'owner', 'tree', {'unique': false}, [[0, 'unsigned']]]
> -  - [313, 2, 'object', 'tree', {'unique': false}, [[2, 'string'], [3, 'unsigned']]]
> +  - [313, 2, 'object', 'tree', {'unique': false}, [[2, 'string'], [3, 'scalar']]]
>   - [320, 0, 'primary', 'tree', {'unique': true}, [[0, 'unsigned']]]
>   - [320, 1, 'uuid', 'tree', {'unique': true}, [[1, 'string']]]
>   - [330, 0, 'primary', 'tree', {'unique': true}, [[0, 'unsigned']]]
> diff --git a/test/box/access.result b/test/box/access.result
> index f4669a4a3..599500633 100644
> --- a/test/box/access.result
> +++ b/test/box/access.result
> @@ -504,7 +504,7 @@ box.space._priv:select{id}
> ...
> box.schema.user.grant('user', 'read', 'universe')
> ---
> -- error: User 'user' already has read access on universe 'nil'
> +- error: User 'user' already has read access on universe ''
> ...
> box.space._priv:select{id}
> ---
> @@ -690,7 +690,7 @@ box.schema.user.grant('guest', 'read,write,execute', 'universe')
> ...
> box.schema.user.grant('guest', 'read,write,execute', 'universe')
> ---
> -- error: User 'guest' already has read,write,execute access on universe 'nil'
> +- error: User 'guest' already has read,write,execute access on universe ''
> ...
> box.schema.user.grant('guest', 'read,write,execute', 'universe', '', { if_not_exists = true })
> ---
> @@ -703,7 +703,7 @@ box.schema.user.revoke('guest', 'usage,session', 'universe')
> ...
> box.schema.user.revoke('guest', 'read,write,execute', 'universe')
> ---
> -- error: User 'guest' does not have read,write,execute access on universe 'nil'
> +- error: User 'guest' does not have read,write,execute access on universe ''
> ...
> box.schema.user.revoke('guest', 'read,write,execute', 'universe', '', { if_exists = true })
> ---
> diff --git a/test/box/access_misc.result b/test/box/access_misc.result
> index 2d87fa2d5..3061f1181 100644
> --- a/test/box/access_misc.result
> +++ b/test/box/access_misc.result
> @@ -436,7 +436,7 @@ box.schema.user.revoke('testuser', 'usage,session', 'universe')
> ...
> box.schema.user.revoke('testuser', 'read, write, execute', 'universe')
> ---
> -- error: User 'testuser' does not have read, write, execute access on universe 'nil'
> +- error: User 'testuser' does not have read, write, execute access on universe ''
> ...
> box.schema.user.revoke('testuser', 'create', 'universe')
> ---
> @@ -797,10 +797,10 @@ box.space._space:select()
>         'type': 'string'}, {'name': 'auth', 'type': 'map'}]]
>   - [312, 1, '_priv', 'memtx', 0, {}, [{'name': 'grantor', 'type': 'unsigned'}, {
>         'name': 'grantee', 'type': 'unsigned'}, {'name': 'object_type', 'type': 'string'},
> -      {'name': 'object_id', 'type': 'unsigned'}, {'name': 'privilege', 'type': 'unsigned'}]]
> +      {'name': 'object_id', 'type': 'scalar'}, {'name': 'privilege', 'type': 'unsigned'}]]
>   - [313, 1, '_vpriv', 'sysview', 0, {}, [{'name': 'grantor', 'type': 'unsigned'},
>       {'name': 'grantee', 'type': 'unsigned'}, {'name': 'object_type', 'type': 'string'},
> -      {'name': 'object_id', 'type': 'unsigned'}, {'name': 'privilege', 'type': 'unsigned'}]]
> +      {'name': 'object_id', 'type': 'scalar'}, {'name': 'privilege', 'type': 'unsigned'}]]
>   - [320, 1, '_cluster', 'memtx', 0, {}, [{'name': 'id', 'type': 'unsigned'}, {'name': 'uuid',
>         'type': 'string'}]]
>   - [330, 1, '_truncate', 'memtx', 0, {}, [{'name': 'id', 'type': 'unsigned'}, {'name': 'count',
> @@ -853,7 +853,7 @@ box.schema.user.grant('tester', 'read', 'universe')
> -- error: the privilege is not granted
> box.schema.user.revoke('tester', 'create', 'universe')
> ---
> -- error: User 'tester' does not have create access on universe 'nil'
> +- error: User 'tester' does not have create access on universe ''
> ...
> -- no error: if_exists clause
> box.schema.user.revoke('tester', 'create', 'universe', nil, { if_exists = true })
> diff --git a/test/box/alter.result b/test/box/alter.result
> index eb7014d8b..36bdb5fd3 100644
> --- a/test/box/alter.result
> +++ b/test/box/alter.result
> @@ -214,13 +214,13 @@ _index:select{}
>   - [305, 1, 'owner', 'tree', {'unique': false}, [[1, 'unsigned']]]
>   - [305, 2, 'name', 'tree', {'unique': true}, [[2, 'string']]]
>   - [312, 0, 'primary', 'tree', {'unique': true}, [[1, 'unsigned'], [2, 'string'],
> -      [3, 'unsigned']]]
> +      [3, 'scalar']]]
>   - [312, 1, 'owner', 'tree', {'unique': false}, [[0, 'unsigned']]]
> -  - [312, 2, 'object', 'tree', {'unique': false}, [[2, 'string'], [3, 'unsigned']]]
> +  - [312, 2, 'object', 'tree', {'unique': false}, [[2, 'string'], [3, 'scalar']]]
>   - [313, 0, 'primary', 'tree', {'unique': true}, [[1, 'unsigned'], [2, 'string'],
> -      [3, 'unsigned']]]
> +      [3, 'scalar']]]
>   - [313, 1, 'owner', 'tree', {'unique': false}, [[0, 'unsigned']]]
> -  - [313, 2, 'object', 'tree', {'unique': false}, [[2, 'string'], [3, 'unsigned']]]
> +  - [313, 2, 'object', 'tree', {'unique': false}, [[2, 'string'], [3, 'scalar']]]
>   - [320, 0, 'primary', 'tree', {'unique': true}, [[0, 'unsigned']]]
>   - [320, 1, 'uuid', 'tree', {'unique': true}, [[1, 'string']]]
>   - [330, 0, 'primary', 'tree', {'unique': true}, [[0, 'unsigned']]]
> diff --git a/test/xlog/upgrade.result b/test/xlog/upgrade.result
> index f02996bba..76467baf1 100644
> --- a/test/xlog/upgrade.result
> +++ b/test/xlog/upgrade.result
> @@ -36,7 +36,7 @@ box.space._schema:select()
> ---
> - - ['cluster', '<server_uuid>']
>   - ['max_id', 513]
> -  - ['version', 1, 10, 0]
> +  - ['version', 1, 10, 2]
> ...
> box.space._space:select()
> ---
> @@ -85,10 +85,10 @@ box.space._space:select()
>         'type': 'string'}, {'name': 'auth', 'type': 'map'}]]
>   - [312, 1, '_priv', 'memtx', 0, {}, [{'name': 'grantor', 'type': 'unsigned'}, {
>         'name': 'grantee', 'type': 'unsigned'}, {'name': 'object_type', 'type': 'string'},
> -      {'name': 'object_id', 'type': 'unsigned'}, {'name': 'privilege', 'type': 'unsigned'}]]
> +      {'name': 'object_id', 'type': 'scalar'}, {'name': 'privilege', 'type': 'unsigned'}]]
>   - [313, 1, '_vpriv', 'sysview', 0, {}, [{'name': 'grantor', 'type': 'unsigned'},
>       {'name': 'grantee', 'type': 'unsigned'}, {'name': 'object_type', 'type': 'string'},
> -      {'name': 'object_id', 'type': 'unsigned'}, {'name': 'privilege', 'type': 'unsigned'}]]
> +      {'name': 'object_id', 'type': 'scalar'}, {'name': 'privilege', 'type': 'unsigned'}]]
>   - [320, 1, '_cluster', 'memtx', 0, {}, [{'name': 'id', 'type': 'unsigned'}, {'name': 'uuid',
>         'type': 'string'}]]
>   - [330, 1, '_truncate', 'memtx', 0, {}, [{'name': 'id', 'type': 'unsigned'}, {'name': 'count',
> @@ -134,13 +134,13 @@ box.space._index:select()
>   - [305, 1, 'owner', 'tree', {'unique': false}, [[1, 'unsigned']]]
>   - [305, 2, 'name', 'tree', {'unique': true}, [[2, 'string']]]
>   - [312, 0, 'primary', 'tree', {'unique': true}, [[1, 'unsigned'], [2, 'string'],
> -      [3, 'unsigned']]]
> +      [3, 'scalar']]]
>   - [312, 1, 'owner', 'tree', {'unique': false}, [[0, 'unsigned']]]
> -  - [312, 2, 'object', 'tree', {'unique': false}, [[2, 'string'], [3, 'unsigned']]]
> +  - [312, 2, 'object', 'tree', {'unique': false}, [[2, 'string'], [3, 'scalar']]]
>   - [313, 0, 'primary', 'tree', {'unique': true}, [[1, 'unsigned'], [2, 'string'],
> -      [3, 'unsigned']]]
> +      [3, 'scalar']]]
>   - [313, 1, 'owner', 'tree', {'unique': false}, [[0, 'unsigned']]]
> -  - [313, 2, 'object', 'tree', {'unique': false}, [[2, 'string'], [3, 'unsigned']]]
> +  - [313, 2, 'object', 'tree', {'unique': false}, [[2, 'string'], [3, 'scalar']]]
>   - [320, 0, 'primary', 'tree', {'unique': true}, [[0, 'unsigned']]]
>   - [320, 1, 'uuid', 'tree', {'unique': true}, [[1, 'string']]]
>   - [330, 0, 'primary', 'tree', {'unique': true}, [[0, 'unsigned']]]
> -- 
> 2.15.2 (Apple Git-101.1)
> 

  reply	other threads:[~2018-08-22 15:42 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-08-22 13:39 [PATCH v2 0/4] Finish implementation of privileges Serge Petrenko
2018-08-22 13:39 ` [PATCH v2 1/4] Introduce separate entity object types for entity privileges Serge Petrenko
2018-08-22 15:42   ` Serge Petrenko [this message]
2018-08-22 16:22   ` Vladimir Davydov
2018-08-22 13:39 ` [PATCH v2 2/4] Add entities user, role to access control Serge Petrenko
2018-08-22 16:36   ` Vladimir Davydov
2018-08-22 13:39 ` [PATCH v2 3/4] Add single object privilege checks to access_check_ddl Serge Petrenko
2018-08-22 16:47   ` Vladimir Davydov
2018-08-23  7:51     ` Serge Petrenko
2018-08-23  8:57   ` Vladimir Davydov
2018-08-22 13:39 ` [PATCH v2 4/4] Add a privilege upgrade script and update tests Serge Petrenko
2018-08-22 16:48   ` Vladimir Davydov
2018-08-23  7:54     ` Serge Petrenko

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=34DD46B3-6FB8-423F-9FEC-AB47211C2FF8@tarantool.org \
    --to=sergepetrenko@tarantool.org \
    --cc=kostja@tarantool.org \
    --cc=tarantool-patches@freelists.org \
    --cc=vdavydov.dev@gmail.com \
    --subject='Re: [PATCH v2 1/4] Introduce separate entity object types for entity privileges.' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox