Tarantool development patches archive
 help / color / mirror / Atom feed
* [tarantool-patches] [PATCH] Introduce separate entity object types for entity privileges.
@ 2018-07-31 14:19 Sergey Petrenko
  2018-07-31 14:56 ` [tarantool-patches] " Konstantin Osipov
  0 siblings, 1 reply; 3+ messages in thread
From: Sergey Petrenko @ 2018-07-31 14:19 UTC (permalink / raw)
  To: tarantool-patches, Konstantin Osipov

[-- Attachment #1: Type: text/plain, Size: 12701 bytes --]


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)

[-- Attachment #2: Type: text/html, Size: 111633 bytes --]

^ permalink raw reply	[flat|nested] 3+ messages in thread

* [tarantool-patches] Re: [PATCH] Introduce separate entity object types for entity privileges.
  2018-07-31 14:19 [tarantool-patches] [PATCH] Introduce separate entity object types for entity privileges Sergey Petrenko
@ 2018-07-31 14:56 ` Konstantin Osipov
  0 siblings, 0 replies; 3+ messages in thread
From: Konstantin Osipov @ 2018-07-31 14:56 UTC (permalink / raw)
  To: Sergey Petrenko; +Cc: tarantool-patches

* 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

^ permalink raw reply	[flat|nested] 3+ messages in thread

* [tarantool-patches] [PATCH] Introduce separate entity object types for entity privileges.
@ 2018-07-27 15:26 Serge Petrenko
  0 siblings, 0 replies; 3+ messages in thread
From: Serge Petrenko @ 2018-07-27 15:26 UTC (permalink / raw)
  To: tarantool-patches, kostja; +Cc: Serge Petrenko

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)

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2018-07-31 14:56 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-07-31 14:19 [tarantool-patches] [PATCH] Introduce separate entity object types for entity privileges Sergey Petrenko
2018-07-31 14:56 ` [tarantool-patches] " Konstantin Osipov
  -- strict thread matches above, loose matches on Subject: below --
2018-07-27 15:26 [tarantool-patches] " Serge Petrenko

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