[PATCH v2 2/4] Add entities user, role to access control.

[Serge Petrenko <sergepetrenko@tarantool.org>]

Previously the only existing entities in access control were space,
funciton and sequence. Added user and role entities, so it is now
possible to create users or roles without create privilege on universe.
Also added all the needed checks and modified tests accordingly.

Closes #3524
Prerequisite #3530
---
 src/box/alter.cc            | 33 ++++++++++++++++++++++++++++-----
 src/box/lua/schema.lua      | 27 ++++++++++++++++++---------
 src/box/schema.cc           |  2 ++
 src/box/schema.h            |  8 ++++++++
 src/box/schema_def.c        |  4 ++++
 src/box/schema_def.h        |  2 ++
 src/box/user.cc             | 22 +++++++++++++++++++++-
 test/box/access.result      | 18 ++++++++++--------
 test/box/access.test.lua    | 12 ++++--------
 test/box/access_misc.result |  2 +-
 10 files changed, 98 insertions(+), 32 deletions(-)

diff --git a/src/box/alter.cc b/src/box/alter.cc
index 8eba21a51..8cf0f44bd 100644
--- a/src/box/alter.cc
+++ b/src/box/alter.cc
@@ -2170,7 +2170,7 @@ on_replace_dd_user(struct trigger * /* trigger */, void *event)
 	struct user *old_user = user_by_id(uid);
 	if (new_tuple != NULL && old_user == NULL) { /* INSERT */
 		struct user_def *user = user_def_new_from_tuple(new_tuple);
-		access_check_ddl(user->name, user->owner, SC_USER, PRIV_C, true);
+		access_check_ddl(user->name, user->owner, user->type, PRIV_C, true);
 		auto def_guard = make_scoped_guard([=] { free(user); });
 		(void) user_cache_replace(user);
 		def_guard.is_active = false;
@@ -2179,7 +2179,7 @@ on_replace_dd_user(struct trigger * /* trigger */, void *event)
 		txn_on_rollback(txn, on_rollback);
 	} else if (new_tuple == NULL) { /* DELETE */
 		access_check_ddl(old_user->def->name, old_user->def->owner,
-				 SC_USER, PRIV_D, true);
+				 old_user->def->type, PRIV_D, true);
 		/* Can't drop guest or super user */
 		if (uid <= (uint32_t) BOX_SYSTEM_USER_ID_MAX || uid == SUPER) {
 			tnt_raise(ClientError, ER_DROP_USER,
@@ -2205,7 +2205,7 @@ on_replace_dd_user(struct trigger * /* trigger */, void *event)
 		 * correct.
 		 */
 		struct user_def *user = user_def_new_from_tuple(new_tuple);
-		access_check_ddl(user->name, user->uid, SC_USER, PRIV_A,
+		access_check_ddl(user->name, user->uid, old_user->def->type, PRIV_A,
 				 true);
 		auto def_guard = make_scoped_guard([=] { free(user); });
 		struct trigger *on_commit =
@@ -2672,11 +2672,30 @@ priv_def_check(struct priv_def *priv, enum priv_type priv_type)
 		role_check(grantee, role);
 		break;
 	}
+	case SC_USER:
+	{
+		struct user *user = user_by_id(priv->object_id);
+		if (user == NULL || user->def->type != SC_USER) {
+			tnt_raise(ClientError, ER_NO_SUCH_USER,
+				  user ? user->def->name :
+				  int2str(priv->object_id));
+		}
+		if (user->def->owner != grantor->def->uid &&
+		    grantor->def->uid != ADMIN) {
+			tnt_raise(AccessDeniedError,
+				  priv_name(priv_type),
+				  schema_object_name(SC_USER), name,
+				  grantor->def->name);
+		}
+		break;
+	}
 	case SC_ENTITY_SPACE:
 	case SC_ENTITY_FUNCTION:
 	case SC_ENTITY_SEQUENCE:
+	case SC_ENTITY_ROLE:
+	case SC_ENTITY_USER:
 	{
-		/* Only admin may grant privileges on an entire entity. */
+		/* Only amdin 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,
@@ -2702,7 +2721,11 @@ grant_or_revoke(struct priv_def *priv)
 	struct user *grantee = user_by_id(priv->grantee_id);
 	if (grantee == NULL)
 		return;
-	if (priv->object_type == SC_ROLE) {
+	/*
+	 * Grant a role to a user only when privilege type is 'execute'
+	 * and the role is specified.
+	 */
+	if (priv->object_type == SC_ROLE && !(priv->access & ~PRIV_X)) {
 		struct user *role = user_by_id(priv->object_id);
 		if (role == NULL || role->def->type != SC_ROLE)
 			return;
diff --git a/src/box/lua/schema.lua b/src/box/lua/schema.lua
index d347fd1e6..6fc0cc7f6 100644
--- a/src/box/lua/schema.lua
+++ b/src/box/lua/schema.lua
@@ -1739,6 +1739,8 @@ local priv_object_combo = {
                            box.priv.C, box.priv.D),
     ["role"]     = bit.bor(box.priv.X, box.priv.U,
                            box.priv.C, box.priv.D),
+    ["user"]     = bit.bor(box.priv.C, box.priv.A,
+                           box.priv.D),
 }
 
 --
@@ -1845,18 +1847,23 @@ local function object_resolve(object_type, object_name)
         end
         return seq
     end
-    if object_type == 'role' then
+    if object_type == 'role' or object_type == 'user' then
+        if object_name == '' then
+            return ''
+        end
         local _vuser = box.space[box.schema.VUSER_ID]
-        local role
+        local role_or_user
         if type(object_name) == 'string' then
-            role = _vuser.index.name:get{object_name}
+            role_or_user = _vuser.index.name:get{object_name}
         else
-            role = _vuser:get{object_name}
+            role_or_user = _vuser:get{object_name}
         end
-        if role and role[4] == 'role' then
-            return role[1]
-        else
+        if role_or_user and role_or_user[4] == object_type then
+            return role_or_user[1]
+        elseif object_type == 'role' then
             box.error(box.error.NO_SUCH_ROLE, object_name)
+        else
+            box.error(box.error.NO_SUCH_USER, object_name)
         end
     end
 
@@ -2111,7 +2118,8 @@ local function grant(uid, name, privilege, object_type,
     if privilege_hex ~= old_privilege then
         _priv:replace{options.grantor, uid, object_type, oid, privilege_hex}
     elseif not options.if_not_exists then
-            if object_type == 'role' then
+            if object_type == 'role' and object_name ~= '' and
+               privilege == 'execute' then
                 box.error(box.error.ROLE_GRANTED, name, object_name)
             else
                 box.error(box.error.PRIV_GRANTED, name, privilege,
@@ -2145,7 +2153,8 @@ local function revoke(uid, name, privilege, object_type, object_name, options)
         if options.if_exists then
             return
         end
-        if object_type == 'role' then
+        if object_type == 'role' and object_name ~= '' and
+           privilege == 'execute' then
             box.error(box.error.ROLE_NOT_GRANTED, name, object_name)
         else
             box.error(box.error.PRIV_NOT_GRANTED, name, privilege,
diff --git a/src/box/schema.cc b/src/box/schema.cc
index aaa63a083..4502ca6dc 100644
--- a/src/box/schema.cc
+++ b/src/box/schema.cc
@@ -538,6 +538,8 @@ schema_find_name(enum schema_object_type type, uint32_t object_id)
 	case SC_ENTITY_SPACE:
 	case SC_ENTITY_FUNCTION:
 	case SC_ENTITY_SEQUENCE:
+	case SC_ENTITY_ROLE:
+	case SC_ENTITY_USER:
 		return "";
 	case SC_SPACE:
 		{
diff --git a/src/box/schema.h b/src/box/schema.h
index f1735ff34..c343650de 100644
--- a/src/box/schema.h
+++ b/src/box/schema.h
@@ -240,6 +240,8 @@ struct on_access_denied_ctx {
 struct entity_access {
        struct access space[BOX_USER_MAX];
        struct access function[BOX_USER_MAX];
+       struct access user[BOX_USER_MAX];
+       struct access role[BOX_USER_MAX];
        struct access sequence[BOX_USER_MAX];
 };
 
@@ -257,6 +259,12 @@ entity_access_get(enum schema_object_type type)
 	case SC_FUNCTION:
 	case SC_ENTITY_FUNCTION:
 		return entity_access.function;
+	case SC_USER:
+	case SC_ENTITY_USER:
+		return entity_access.user;
+	case SC_ROLE:
+	case SC_ENTITY_ROLE:
+		return entity_access.role;
 	case SC_SEQUENCE:
 	case SC_ENTITY_SEQUENCE:
 		return entity_access.sequence;
diff --git a/src/box/schema_def.c b/src/box/schema_def.c
index 9091af054..199b26183 100644
--- a/src/box/schema_def.c
+++ b/src/box/schema_def.c
@@ -49,6 +49,10 @@ schema_entity_type(enum schema_object_type type)
 		return SC_ENTITY_SPACE;
 	case SC_FUNCTION:
 		return SC_ENTITY_FUNCTION;
+	case SC_USER:
+		return SC_ENTITY_USER;
+	case SC_ROLE:
+		return SC_ENTITY_ROLE;
 	case SC_SEQUENCE:
 		return SC_ENTITY_SEQUENCE;
 	case SC_COLLATION:
diff --git a/src/box/schema_def.h b/src/box/schema_def.h
index d2fc39b76..c0444cd11 100644
--- a/src/box/schema_def.h
+++ b/src/box/schema_def.h
@@ -235,6 +235,8 @@ enum schema_object_type {
 	schema_object_type_MAX = 8,
 	SC_ENTITY_SPACE,
 	SC_ENTITY_FUNCTION,
+	SC_ENTITY_ROLE,
+	SC_ENTITY_USER,
 	SC_ENTITY_SEQUENCE,
 	SC_ENTITY_COLLATION
 };
diff --git a/src/box/user.cc b/src/box/user.cc
index eec785652..b4fb65a59 100644
--- a/src/box/user.cc
+++ b/src/box/user.cc
@@ -217,6 +217,16 @@ access_find(struct priv_def *priv)
 		access = entity_access.function;
 		break;
 	}
+	case SC_ENTITY_USER:
+	{
+		access = entity_access.user;
+		break;
+	}
+	case SC_ENTITY_ROLE:
+	{
+		access = entity_access.role;
+		break;
+	}
 	case SC_ENTITY_SEQUENCE:
 	{
 		access = entity_access.sequence;
@@ -236,6 +246,16 @@ access_find(struct priv_def *priv)
 			access = func->access;
 		break;
 	}
+	case SC_USER:
+	{
+		/* No grants on a single object user yet. */
+		break;
+	}
+	case SC_ROLE:
+	{
+		/* No grants on a single object role yet. */
+		break;
+	}
 	case SC_SEQUENCE:
 	{
 		struct sequence *seq = sequence_by_id(priv->object_id);
@@ -318,7 +338,7 @@ user_reload_privs(struct user *user)
 			 * Skip role grants, we're only
 			 * interested in real objects.
 			 */
-			if (priv.object_type != SC_ROLE)
+			if (priv.object_type != SC_ROLE || !(priv.access & PRIV_X))
 				user_grant_priv(user, &priv);
 		}
 	}
diff --git a/test/box/access.result b/test/box/access.result
index 599500633..377a8fa66 100644
--- a/test/box/access.result
+++ b/test/box/access.result
@@ -1427,16 +1427,18 @@ box.session.su("admin")
 box.schema.user.grant('tester', 'write', 'universe')
 ---
 ...
--- no entity user currently, so have to grant create
--- on universe in order to create a user.
-box.schema.user.grant('tester', 'create', 'universe')
+box.schema.user.grant('tester', 'create', 'user')
+---
+...
+box.schema.user.grant('tester', 'create', 'space')
+---
+...
+box.schema.user.grant('tester', 'create', 'function')
+---
+...
+box.schema.user.grant('tester', 'create' , 'sequence')
 ---
 ...
--- this should work instead:
---box.schema.user.grant('tester', 'create', 'user')
---box.schema.user.grant('tester', 'create', 'space')
---box.schema.user.grant('tester', 'create', 'function')
---box.schema.user.grant('tester', 'create' , 'sequence')
 box.schema.user.grant('tester', 'read', 'space', '_sequence')
 ---
 ...
diff --git a/test/box/access.test.lua b/test/box/access.test.lua
index 9ae0e1114..c90fe0a1a 100644
--- a/test/box/access.test.lua
+++ b/test/box/access.test.lua
@@ -538,14 +538,10 @@ box.session.su("admin")
 -- tables from ddl
 --
 box.schema.user.grant('tester', 'write', 'universe')
--- no entity user currently, so have to grant create
--- on universe in order to create a user.
-box.schema.user.grant('tester', 'create', 'universe')
--- this should work instead:
---box.schema.user.grant('tester', 'create', 'user')
---box.schema.user.grant('tester', 'create', 'space')
---box.schema.user.grant('tester', 'create', 'function')
---box.schema.user.grant('tester', 'create' , 'sequence')
+box.schema.user.grant('tester', 'create', 'user')
+box.schema.user.grant('tester', 'create', 'space')
+box.schema.user.grant('tester', 'create', 'function')
+box.schema.user.grant('tester', 'create' , 'sequence')
 box.schema.user.grant('tester', 'read', 'space', '_sequence')
 box.session.su("tester")
 -- successful create
diff --git a/test/box/access_misc.result b/test/box/access_misc.result
index 3061f1181..c1809d69a 100644
--- a/test/box/access_misc.result
+++ b/test/box/access_misc.result
@@ -361,7 +361,7 @@ testuser_uid = session.uid()
 ...
 _ = box.space._user:delete(2)
 ---
-- error: Drop access to user 'public' is denied for user 'testuser'
+- error: Drop access to role 'public' is denied for user 'testuser'
 ...
 box.space._user:select(1)
 ---
-- 
2.15.2 (Apple Git-101.1)