From: Konstantin Osipov <kostja@tarantool.org>
To: Sergey Petrenko <sergepetrenko@tarantool.org>
Cc: tarantool-patches@freelists.org
Subject: [tarantool-patches] Re: [PATCH] Introduce separate entity object types for entity privileges.
Date: Tue, 31 Jul 2018 17:56:21 +0300 [thread overview]
Message-ID: <20180731145621.GC13269@chai> (raw)
In-Reply-To: <56C246C1-B584-4D45-B120-5794757E3F54@tarantool.org>
* Sergey Petrenko <sergepetrenko@tarantool.org> [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/tree/sergepetrenko/gh-3574-whole-entity-types>
> https://github.com/tarantool/tarantool/issues/3574 <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
prev parent reply other threads:[~2018-07-31 14:56 UTC|newest]
Thread overview: 2+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-07-31 14:19 [tarantool-patches] " Sergey Petrenko
2018-07-31 14:56 ` Konstantin Osipov [this message]
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=20180731145621.GC13269@chai \
--to=kostja@tarantool.org \
--cc=sergepetrenko@tarantool.org \
--cc=tarantool-patches@freelists.org \
--subject='[tarantool-patches] Re: [PATCH] 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