From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from localhost (localhost [127.0.0.1]) by turing.freelists.org (Avenir Technologies Mail Multiplex) with ESMTP id 6F8772834A for ; Tue, 31 Jul 2018 10:56:26 -0400 (EDT) Received: from turing.freelists.org ([127.0.0.1]) by localhost (turing.freelists.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id tKsdsnEFdIe8 for ; Tue, 31 Jul 2018 10:56:26 -0400 (EDT) Received: from smtpng2.m.smailru.net (smtpng2.m.smailru.net [94.100.179.3]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by turing.freelists.org (Avenir Technologies Mail Multiplex) with ESMTPS id ADACB226D2 for ; Tue, 31 Jul 2018 10:56:24 -0400 (EDT) Date: Tue, 31 Jul 2018 17:56:21 +0300 From: Konstantin Osipov Subject: [tarantool-patches] Re: [PATCH] Introduce separate entity object types for entity privileges. Message-ID: <20180731145621.GC13269@chai> References: <56C246C1-B584-4D45-B120-5794757E3F54@tarantool.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <56C246C1-B584-4D45-B120-5794757E3F54@tarantool.org> Sender: tarantool-patches-bounce@freelists.org Errors-to: tarantool-patches-bounce@freelists.org Reply-To: tarantool-patches@freelists.org List-help: List-unsubscribe: List-software: Ecartis version 1.0.0 List-Id: tarantool-patches List-subscribe: List-owner: List-post: List-archive: To: Sergey Petrenko Cc: tarantool-patches@freelists.org * Sergey Petrenko [18/07/31 17:20]: Hi, I chatted with @locker and we agreed to: - keep new entities in the enum - alter index key parts to store scalar in the object id column - use '*' (asterisk) to represent entity in the object id - use the new values of enum in priv_def - keep the old syntax for granting access to all entities (box.schema.user.grant('read', 'space') Sorry for asking you to rewrite this piece again. > 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_USER, etc. > Also now granting privileges on an entity (e.g. space) may be done in 2 ways: > the old one: box.schema.user.grant('user', 'privilege', 'space') > the new one: box.schema.user.grant('user', 'privilege', 'all spaces') > The same applies to all entities. > > Closes: #3574 > Prerequisite: #3524 > --- > https://github.com/tarantool/tarantool/tree/sergepetrenko/gh-3574-whole-entity-types > https://github.com/tarantool/tarantool/issues/3574 > > src/box/lua/schema.lua | 87 ++++++++++++++++++++++++++++++++++++++------------ > src/box/schema.cc | 16 ++++++---- > src/box/schema.h | 23 +++++++------ > src/box/schema_def.c | 22 ++++++++----- > src/box/schema_def.h | 18 +++++++---- > src/box/user.cc | 27 +++++++++------- > test/box/access.result | 8 ++--- > 7 files changed, 134 insertions(+), 67 deletions(-) > > diff --git a/src/box/lua/schema.lua b/src/box/lua/schema.lua > index b9b8c9004..eb984b1de 100644 > --- a/src/box/lua/schema.lua > +++ b/src/box/lua/schema.lua > @@ -1731,14 +1731,21 @@ local priv_object_combo = { > ["universe"] = box.priv.ALL, > -- sic: we allow to grant 'execute' on space. This is a legacy > -- bug, please fix it in 2.0 > - ["space"] = bit.bxor(box.priv.ALL, box.priv.S, > - box.priv.REVOKE, box.priv.GRANT), > - ["sequence"] = bit.bor(box.priv.R, box.priv.W, box.priv.U, > - box.priv.C, box.priv.A, box.priv.D), > - ["function"] = bit.bor(box.priv.X, box.priv.U, > - box.priv.C, box.priv.D), > - ["role"] = bit.bor(box.priv.X, box.priv.U, > - box.priv.C, box.priv.D), > + ["space"] = bit.bxor(box.priv.ALL, box.priv.S, box.priv.C, > + box.priv.REVOKE, box.priv.GRANT), > + ["all spaces"] = bit.bxor(box.priv.ALL, box.priv.S, > + box.priv.REVOKE, box.priv.GRANT), > + ["sequence"] = bit.bor(box.priv.R, box.priv.W, box.priv.U, > + box.priv.A, box.priv.D), > + ["all sequences"] = bit.bor(box.priv.R, box.priv.W, > + box.priv.C, box.priv.A, box.priv.D), > + ["function"] = bit.bor(box.priv.X, box.priv.U, > + box.priv.D), > + ["all functions"] = bit.bor(box.priv.X, > + box.priv.C, box.priv.D), > + ["role"] = bit.bor(box.priv.X, box.priv.U, > + box.priv.D), > + ["all roles"] = bit.bor(box.priv.C, box.priv.D), > } > > -- > @@ -1808,10 +1815,34 @@ local function object_resolve(object_type, object_name) > end > return 0 > end > - if object_type == 'space' then > - if object_name == nil or object_name == 0 then > - return 0 > + if object_type == 'all spaces' then > + if object_name ~= nil and object_name ~= 0 then > + box.error(box.error.ILLEGAL_PARAMS, "no object name allowed") > + end > + return 0 > + end > + if object_type == 'all sequences' then > + if object_name ~= nil and object_name ~= 0 then > + box.error(box.error.ILLEGAL_PARAMS, "no object name allowed") > end > + return 0 > + end > + if object_type == 'all functions' then > + if object_name ~= nil and object_name ~= 0 then > + box.error(box.error.ILLEGAL_PARAMS, "no object name allowed") > + end > + return 0 > + end > + if object_type == 'all roles' then > + if object_name ~= nil and object_name ~= 0 then > + box.error(box.error.ILLEGAL_PARAMS, "no object name allowed") > + end > + return 0 > + end > + if object_type == 'space' then > + if object_name == nil then > + return nil > + end > local space = box.space[object_name] > if space == nil then > box.error(box.error.NO_SUCH_SPACE, object_name) > @@ -1819,9 +1850,9 @@ 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 > - end > + if object_name == nil then > + return nil > + end > local _vfunc = box.space[box.schema.VFUNC_ID] > local func > if type(object_name) == 'string' then > @@ -1836,16 +1867,19 @@ 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 > - end > - local seq = sequence_resolve(object_name) > + if object_name == nil then > + return nil > + end > + local seq = sequence_resolve(object_name) > if seq == nil then > box.error(box.error.NO_SUCH_SEQUENCE, object_name) > end > return seq > end > if object_type == 'role' then > + if object_name == nil then > + return nil > + end > local _vuser = box.space[box.schema.VUSER_ID] > local role > if type(object_name) == 'string' then > @@ -1864,7 +1898,9 @@ 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_type == 'all spaces' or > + object_type == 'all sequences' or object_type == 'all functions' or > + object_type == 'all roles' or object_type == 'all users' then > return "" > end > local space > @@ -2079,9 +2115,14 @@ local function grant(uid, name, privilege, object_type, > object_name = privilege > privilege = 'execute' > end > - local privilege_hex = privilege_check(privilege, object_type) > > local oid = object_resolve(object_type, object_name) > + -- allow for old syntax to grant privileges on an entity > + if oid == nil then > + oid = 0 > + object_type = 'all ' .. object_type .. 's' > + end > + local privilege_hex = privilege_check(privilege, object_type) > options = options or {} > if options.grantor == nil then > options.grantor = session.euid() > @@ -2122,9 +2163,13 @@ local function revoke(uid, name, privilege, object_type, object_name, options) > object_name = privilege > privilege = 'execute' > end > - local privilege_hex = privilege_check(privilege, object_type) > options = options or {} > local oid = object_resolve(object_type, object_name) > + if oid == nil then > + oid = 0 > + object_type = 'all ' .. object_type .. 's' > + end > + local privilege_hex = privilege_check(privilege, object_type) > local _priv = box.space[box.schema.PRIV_ID] > local _vpriv = box.space[box.schema.VPRIV_ID] > local tuple = _vpriv:get{uid, object_type, oid} > diff --git a/src/box/schema.cc b/src/box/schema.cc > index 433f52c08..b81d2cd8a 100644 > --- a/src/box/schema.cc > +++ b/src/box/schema.cc > @@ -536,10 +536,18 @@ schema_find_name(enum schema_object_type type, uint32_t object_id) > switch (type) { > case SC_UNIVERSE: > return ""; > + case SC_ENTITY_SPACE: > + return "SPACE"; > + case SC_ENTITY_FUNCTION: > + return "FUNCTION"; > + case SC_ENTITY_SEQUENCE: > + return "SEQUENCE"; > + case SC_ENTITY_ROLE: > + return "ROLE"; > + case SC_ENTITY_USER: > + return "USER"; > case SC_SPACE: > { > - if (object_id == 0) > - return "SPACE"; > struct space *space = space_by_id(object_id); > if (space == NULL) > break; > @@ -547,8 +555,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 +562,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..caec2e00d 100644 > --- a/src/box/schema_def.c > +++ b/src/box/schema_def.c > @@ -31,14 +31,20 @@ > #include "schema_def.h" > > static const char *object_type_strs[] = { > - /* [SC_UKNNOWN] = */ "unknown", > - /* [SC_UNIVERSE] = */ "universe", > - /* [SC_SPACE] = */ "space", > - /* [SC_FUNCTION] = */ "function", > - /* [SC_USER] = */ "user", > - /* [SC_ROLE] = */ "role", > - /* [SC_SEQUENCE] = */ "sequence", > - /* [SC_COLLATION] = */ "collation", > + /* [SC_UKNNOWN] = */ "unknown", > + /* [SC_UNIVERSE] = */ "universe", > + /* [SC_SPACE] = */ "space", > + /* [SC_ENTITY_SPACE] = */ "all spaces", > + /* [SC_FUNCTION] = */ "function", > + /* [SC_ENTITY_FUNCTION] = */ "all functions", > + /* [SC_USER] = */ "user", > + /* [SC_ENTITY_USER] = */ "all users", > + /* [SC_ROLE] = */ "role", > + /* [SC_ENTITY_ROLE] = */ "all roles", > + /* [SC_SEQUENCE] = */ "sequence", > + /* [SC_ENTITY_SEQUENCE] = */ "all sequences", > + /* [SC_COLLATION] = */ "collation", > + /* [SC_ENTITY_COLLATION] = */ "all collations", > }; > > enum schema_object_type > diff --git a/src/box/schema_def.h b/src/box/schema_def.h > index 2edb8d37f..0c917094c 100644 > --- a/src/box/schema_def.h > +++ b/src/box/schema_def.h > @@ -223,12 +223,18 @@ enum schema_object_type { > SC_UNKNOWN = 0, > SC_UNIVERSE = 1, > SC_SPACE = 2, > - SC_FUNCTION = 3, > - SC_USER = 4, > - SC_ROLE = 5, > - SC_SEQUENCE = 6, > - SC_COLLATION = 7, > - schema_object_type_MAX = 8 > + SC_ENTITY_SPACE = 3, > + SC_FUNCTION = 4, > + SC_ENTITY_FUNCTION = 5, > + SC_USER = 6, > + SC_ENTITY_USER = 7, > + SC_ROLE = 8, > + SC_ENTITY_ROLE = 9, > + SC_SEQUENCE = 10, > + SC_ENTITY_SEQUENCE = 11, > + SC_COLLATION = 12, > + SC_ENTITY_COLLATION = 13, > + schema_object_type_MAX = 13 > }; > > enum schema_object_type > 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/access.result b/test/box/access.result > index f4669a4a3..30fcb6455 100644 > --- a/test/box/access.result > +++ b/test/box/access.result > @@ -1733,19 +1733,19 @@ box.session.su('admin') > -- prerequisite gh-945 > box.schema.user.grant("guest", "alter", "function") > --- > -- error: Unsupported function privilege 'alter' > +- error: Unsupported all functions privilege 'alter' > ... > box.schema.user.grant("guest", "execute", "sequence") > --- > -- error: Unsupported sequence privilege 'execute' > +- error: Unsupported all sequences privilege 'execute' > ... > box.schema.user.grant("guest", "read,execute", "sequence") > --- > -- error: Unsupported sequence privilege 'read,execute' > +- error: Unsupported all sequences privilege 'read,execute' > ... > box.schema.user.grant("guest", "read,write,execute", "role") > --- > -- error: Unsupported role privilege 'read,write,execute' > +- error: Unsupported all roles privilege 'read,write,execute' > ... > -- Check entities DML > box.schema.user.create("tester", { password = '123' }) > -- > 2.15.2 (Apple Git-101.1) -- Konstantin Osipov, Moscow, Russia, +7 903 626 22 32 http://tarantool.io - www.twitter.com/kostja_osipov