[tarantool-patches] [PATCH 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            | 42 +++++++++++++++++++++++++++++++++++++++---
 src/box/lua/schema.lua      | 29 +++++++++++++++++++----------
 src/box/schema.h            |  8 ++++++++
 src/box/user.cc             | 22 +++++++++++++++++++++-
 test/box/access.result      | 20 ++++++++++----------
 test/box/access.test.lua    | 15 +++++----------
 test/box/access_misc.result |  2 +-
 test/box/role.result        | 25 +++++++++++++++++++++++--
 test/box/role.test.lua      | 12 ++++++++++--
 9 files changed, 136 insertions(+), 39 deletions(-)

diff --git a/src/box/alter.cc b/src/box/alter.cc
index 42136b7df..436827a6d 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,
@@ -2670,6 +2670,38 @@ 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_USER:
+	{
+		struct user *user = NULL;
+		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 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,
+				  grantor->def->name);
+		}
 	}
 	default:
 		break;
@@ -2690,7 +2722,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..bad35b680 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,19 +1847,24 @@ 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)
-        end
+        else
+            box.error(box.error.NO_SUCH_USER, object_name)
+	end
     end
 
     box.error(box.error.UNKNOWN_SCHEMA_OBJECT, object_type)
@@ -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.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/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..7ef2adac5 100644
--- a/test/box/access.result
+++ b/test/box/access.result
@@ -878,8 +878,6 @@ box.schema.user.create('test')
 box.schema.user.grant('test', 'read', 'space', '_collation')
 ---
 ...
---box.schema.user.grant('test', 'write', 'space', '_collation')
--- FIXME: granting create on 'collation' only doesn't work
 box.schema.user.grant('test', 'create', 'universe')
 ---
 ...
@@ -1427,16 +1425,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..9b7510e64 100644
--- a/test/box/access.test.lua
+++ b/test/box/access.test.lua
@@ -341,8 +341,7 @@ c:close()
 session = box.session
 box.schema.user.create('test')
 box.schema.user.grant('test', 'read', 'space', '_collation')
---box.schema.user.grant('test', 'write', 'space', '_collation')
--- FIXME: granting create on 'collation' only doesn't work
+
 box.schema.user.grant('test', 'create', 'universe')
 session.su('test')
 box.internal.collation.create('test', 'ICU', 'ru_RU')
@@ -538,14 +537,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)
 ---
diff --git a/test/box/role.result b/test/box/role.result
index 806cea90b..243c7bc6c 100644
--- a/test/box/role.result
+++ b/test/box/role.result
@@ -214,7 +214,22 @@ box.schema.role.drop('test')
 box.schema.user.grant('grantee', 'liaison')
 ---
 ...
-box.schema.user.grant('test', 'read,write,create', 'universe')
+box.schema.user.grant('test', 'read,write', 'space', '_priv')
+---
+...
+box.schema.user.grant('test', 'write', 'space', '_schema')
+---
+...
+box.schema.user.grant('test', 'create', 'space')
+---
+...
+box.schema.user.grant('test', 'read,write', 'space', '_space')
+---
+...
+box.schema.user.grant('test', 'write', 'space', '_index')
+---
+...
+box.schema.user.grant('test', 'read', 'space', '_user')
 ---
 ...
 box.session.su('test')
@@ -635,7 +650,13 @@ box.schema.user.create('user')
 box.schema.user.create('grantee')
 ---
 ...
-box.schema.user.grant('user', 'read,write,execute,create', 'universe')
+box.schema.user.grant('user', 'read,write', 'space', '_user')
+---
+...
+box.schema.user.grant('user', 'read,write', 'space', '_priv')
+---
+...
+box.schema.user.grant('user', 'create', 'role')
 ---
 ...
 box.session.su('user')
diff --git a/test/box/role.test.lua b/test/box/role.test.lua
index e97339f49..9845f4c4c 100644
--- a/test/box/role.test.lua
+++ b/test/box/role.test.lua
@@ -69,7 +69,13 @@ box.schema.role.revoke('test', 'liaison')
 box.schema.role.drop('test')
 
 box.schema.user.grant('grantee', 'liaison')
-box.schema.user.grant('test', 'read,write,create', 'universe')
+
+box.schema.user.grant('test', 'read,write', 'space', '_priv')
+box.schema.user.grant('test', 'write', 'space', '_schema')
+box.schema.user.grant('test', 'create', 'space')
+box.schema.user.grant('test', 'read,write', 'space', '_space')
+box.schema.user.grant('test', 'write', 'space', '_index')
+box.schema.user.grant('test', 'read', 'space', '_user')
 box.session.su('test')
 s = box.schema.space.create('test')
 _ = s:create_index('i1')
@@ -248,7 +254,9 @@ box.schema.role.drop("role10")
 box.schema.user.create('user')
 box.schema.user.create('grantee')
 
-box.schema.user.grant('user', 'read,write,execute,create', 'universe')
+box.schema.user.grant('user', 'read,write', 'space', '_user')
+box.schema.user.grant('user', 'read,write', 'space', '_priv')
+box.schema.user.grant('user', 'create', 'role')
 box.session.su('user')
 box.schema.role.create('role')
 box.session.su('admin')
-- 
2.15.2 (Apple Git-101.1)