From ee84e28607cd4534470c07e595572293b02d81bf Mon Sep 17 00:00:00 2001
From: Vladimir Davydov <vdavydov@tarantool.org>
Date: Wed, 19 Jul 2023 11:22:59 +0300
Subject: [PATCH] schema: drop entity object types

SC_ENTITY_FOO is used instead of SC_FOO when a privilege is granted to
an entire object class, not an individual object (object id is set to ''
in the _priv system space). Introduction of this new concept made the
access checking code rather confusing, especially the part converting
entity types to object types and back, and complicated addition of new
schema object types.

Actually, there's no point in maintaining separate schema object types
for entities. Instead, we can simply add a flag to the priv_def struct
saying that the object id stored in the struct is meaningless and the
privilege should be applied to an entire object class. This simplifies
the code quite a bit and makes introduction of new schema object types
must easier.

Needed for #8803

NO_DOC=refactoring
NO_TEST=refactoring
NO_CHANGELOG=refactoring
---
 src/box/alter.cc     | 44 ++++++++++-------------
 src/box/schema.h     |  5 ---
 src/box/schema_def.c | 32 -----------------
 src/box/schema_def.h | 36 +++++--------------
 src/box/user.cc      | 86 ++++++++++++++------------------------------
 src/box/user_def.h   |  5 +++
 6 files changed, 59 insertions(+), 149 deletions(-)

diff --git a/src/box/alter.cc b/src/box/alter.cc
index 76bbcaa3ce..5e3bb4c675 100644
--- a/src/box/alter.cc
+++ b/src/box/alter.cc
@@ -3751,12 +3751,13 @@ priv_def_create_from_tuple(struct priv_def *priv, struct tuple *tuple)
 	case MP_STR:
 		if (mp_decode_strl(&data) == 0) {
 			/* Entity-wide privilege. */
+			priv->is_entity_access = true;
 			priv->object_id = 0;
-			priv->object_type = schema_entity_type(priv->object_type);
 			break;
 		}
 		FALLTHROUGH;
 	default:
+		priv->is_entity_access = false;
 		if (tuple_field_u32(tuple,
 		    BOX_PRIV_FIELD_OBJECT_ID, &(priv->object_id)) != 0)
 			return -1;
@@ -3799,18 +3800,10 @@ priv_def_check(struct priv_def *priv, enum priv_type priv_type)
 	const char *name = "";
 	struct access *object = NULL;
 	switch (priv->object_type) {
-	case SC_UNIVERSE:
-		if (grantor->def->uid != ADMIN) {
-			diag_set(AccessDeniedError,
-				  priv_name(priv_type),
-				  schema_object_name(SC_UNIVERSE),
-				  name,
-				  grantor->def->name);
-			return -1;
-		}
-		break;
 	case SC_SPACE:
 	{
+		if (priv->is_entity_access)
+			break;
 		struct space *space = space_cache_find(priv->object_id);
 		if (space == NULL)
 			return -1;
@@ -3828,6 +3821,8 @@ priv_def_check(struct priv_def *priv, enum priv_type priv_type)
 	}
 	case SC_FUNCTION:
 	{
+		if (priv->is_entity_access)
+			break;
 		struct func *func = func_by_id(priv->object_id);
 		if (func == NULL) {
 			diag_set(ClientError, ER_NO_SUCH_FUNCTION, int2str(priv->object_id));
@@ -3847,6 +3842,8 @@ priv_def_check(struct priv_def *priv, enum priv_type priv_type)
 	}
 	case SC_SEQUENCE:
 	{
+		if (priv->is_entity_access)
+			break;
 		struct sequence *seq = sequence_by_id(priv->object_id);
 		if (seq == NULL) {
 			diag_set(ClientError, ER_NO_SUCH_SEQUENCE, int2str(priv->object_id));
@@ -3866,6 +3863,8 @@ priv_def_check(struct priv_def *priv, enum priv_type priv_type)
 	}
 	case SC_ROLE:
 	{
+		if (priv->is_entity_access)
+			break;
 		struct user *role = user_by_id(priv->object_id);
 		if (role == NULL || role->def->type != SC_ROLE) {
 			diag_set(ClientError, ER_NO_SUCH_ROLE,
@@ -3895,6 +3894,8 @@ priv_def_check(struct priv_def *priv, enum priv_type priv_type)
 	}
 	case SC_USER:
 	{
+		if (priv->is_entity_access)
+			break;
 		struct user *user = user_by_id(priv->object_id);
 		if (user == NULL || user->def->type != SC_USER) {
 			diag_set(ClientError, ER_NO_SUCH_USER,
@@ -3914,23 +3915,16 @@ priv_def_check(struct priv_def *priv, enum priv_type priv_type)
 		}
 		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. */
-		if (grantor->def->uid != ADMIN) {
-			diag_set(AccessDeniedError, priv_name(priv_type),
-				 schema_entity_name(priv->object_type), name,
-				  grantor->def->name);
-			return -1;
-		}
-	}
 	default:
 		break;
 	}
+	/* Only admin may grant privileges on an entire entity. */
+	if (object == NULL && grantor->def->uid != ADMIN) {
+		diag_set(AccessDeniedError, priv_name(priv_type),
+			 schema_object_name(priv->object_type), name,
+			 grantor->def->name);
+		return -1;
+	}
 	if (access_check_ddl(name, grantor->def->uid, object,
 			     priv->object_type, priv_type) != 0)
 		return -1;
diff --git a/src/box/schema.h b/src/box/schema.h
index 95ee598e98..5e16a7d40a 100644
--- a/src/box/schema.h
+++ b/src/box/schema.h
@@ -177,19 +177,14 @@ entity_access_get(enum schema_object_type type)
 {
 	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_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;
 	default:
 		return NULL;
diff --git a/src/box/schema_def.c b/src/box/schema_def.c
index f028efe4a8..cbfad479ac 100644
--- a/src/box/schema_def.c
+++ b/src/box/schema_def.c
@@ -46,32 +46,6 @@ static const char *object_type_strs[] = {
 	/* [SC_COLLATION]       = */ "collation",
 };
 
-/** Given object type @type, return corresponding entity type. */
-static enum schema_object_type
-schema_object_type_to_entity(enum schema_object_type type)
-{
-	assert(type >= SC_SPACE);
-	assert((int) type < (int) schema_object_type_MAX);
-	return type + schema_object_type_MAX - 1;
-}
-
-/** Given entity type @type, return corresponding object type. */
-static enum schema_object_type
-schema_entity_type_to_object(enum schema_object_type type)
-{
-	assert((int) type > (int) schema_object_type_MAX);
-	assert((int) type < (int) schema_entity_type_MAX);
-	return (type % (schema_object_type_MAX)) + 1;
-}
-
-enum schema_object_type
-schema_entity_type(enum schema_object_type type)
-{
-	if (type <= SC_UNIVERSE || type >= schema_object_type_MAX)
-		return SC_UNKNOWN;
-	return schema_object_type_to_entity(type);
-}
-
 enum schema_object_type
 schema_object_type(const char *name)
 {
@@ -91,9 +65,3 @@ schema_object_name(enum schema_object_type type)
 	assert((int) type < (int) schema_object_type_MAX);
 	return object_type_strs[type];
 }
-
-const char *
-schema_entity_name(enum schema_object_type type)
-{
-	return object_type_strs[schema_entity_type_to_object(type)];
-}
diff --git a/src/box/schema_def.h b/src/box/schema_def.h
index cb8faca2dd..60d2340015 100644
--- a/src/box/schema_def.h
+++ b/src/box/schema_def.h
@@ -302,25 +302,14 @@ enum {
  */
 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,
-	/*
-	 * All object types are supposed to be above this point,
-	 * all entity types - below.
-	 */
-	schema_object_type_MAX = 8,
-	SC_ENTITY_SPACE,
-	SC_ENTITY_FUNCTION,
-	SC_ENTITY_USER,
-	SC_ENTITY_ROLE,
-	SC_ENTITY_SEQUENCE,
-	SC_ENTITY_COLLATION,
-	schema_entity_type_MAX = 15
+	SC_UNIVERSE,
+	SC_SPACE,
+	SC_FUNCTION,
+	SC_USER,
+	SC_ROLE,
+	SC_SEQUENCE,
+	SC_COLLATION,
+	schema_object_type_MAX,
 };
 
 /** SQL Storage engine. */
@@ -332,21 +321,12 @@ enum sql_storage_engine {
 
 extern const char *sql_storage_engine_strs[];
 
-/**
- * Given a object type, return an entity type it belongs to.
- */
-enum schema_object_type
-schema_entity_type(enum schema_object_type type);
-
 enum schema_object_type
 schema_object_type(const char *name);
 
 const char *
 schema_object_name(enum schema_object_type type);
 
-const char *
-schema_entity_name(enum schema_object_type type);
-
 /**
  * Check that the space id corresponds to a system space, which means that is
  * has a special meaning for tarantool and has predefined insert/remove
diff --git a/src/box/user.cc b/src/box/user.cc
index e5b33598da..5510f90217 100644
--- a/src/box/user.cc
+++ b/src/box/user.cc
@@ -208,83 +208,52 @@ user_grant_priv(struct user *user, struct priv_def *def)
 }
 
 /**
- * Find the corresponding access structure
- * given object type and object id.
+ * Find the corresponding access structure for the given privilege.
  */
 static struct access *
-access_find(enum schema_object_type object_type, uint32_t object_id)
+access_find(const struct priv_def *priv)
 {
-	struct access *access = NULL;
-	switch (object_type) {
+	switch (priv->object_type) {
 	case SC_UNIVERSE:
-	{
-		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_USER:
-	{
-		access = entity_access.user;
-		break;
-	}
-	case SC_ENTITY_ROLE:
-	{
-		access = entity_access.role;
-		break;
-	}
-	case SC_ENTITY_SEQUENCE:
-	{
-		access = entity_access.sequence;
-		break;
-	}
+		return universe.access;
 	case SC_SPACE:
 	{
-		struct space *space = space_by_id(object_id);
-		if (space)
-			access = space->access;
-		break;
+		if (priv->is_entity_access)
+			return entity_access.space;
+		struct space *space = space_by_id(priv->object_id);
+		return space != NULL ? space->access : NULL;
 	}
 	case SC_FUNCTION:
 	{
-		struct func *func = func_by_id(object_id);
-		if (func)
-			access = func->access;
-		break;
+		if (priv->is_entity_access)
+			return entity_access.function;
+		struct func *func = func_by_id(priv->object_id);
+		return func != NULL ? func->access : NULL;
 	}
 	case SC_USER:
 	{
-		struct user *user = user_by_id(object_id);
-		if (user)
-			access = user->access;
-		break;
+		if (priv->is_entity_access)
+			return entity_access.user;
+		struct user *user = user_by_id(priv->object_id);
+		return user != NULL ? user->access : NULL;
 	}
 	case SC_ROLE:
 	{
-		struct user *role = user_by_id(object_id);
-		if (role)
-			access = role->access;
-		break;
+		if (priv->is_entity_access)
+			return entity_access.role;
+		struct user *role = user_by_id(priv->object_id);
+		return role != NULL ? role->access : NULL;
 	}
 	case SC_SEQUENCE:
 	{
-		struct sequence *seq = sequence_by_id(object_id);
-		if (seq)
-			access = seq->access;
-		break;
+		if (priv->is_entity_access)
+			return entity_access.sequence;
+		struct sequence *seq = sequence_by_id(priv->object_id);
+		return seq != NULL ? seq->access : NULL;
 	}
 	default:
-		break;
+		return NULL;
 	}
-	return access;
 }
 
 
@@ -299,8 +268,7 @@ user_set_effective_access(struct user *user)
 	privset_ifirst(&user->privs, &it);
 	struct priv_def *priv;
 	while ((priv = privset_inext(&it)) != NULL) {
-		struct access *object = access_find(priv->object_type,
-						    priv->object_id);
+		struct access *object = access_find(priv);
 		 /* Protect against a concurrent drop. */
 		if (object == NULL)
 			continue;
@@ -759,7 +727,7 @@ role_revoke(struct user *grantee, struct user *role)
 int
 priv_grant(struct user *grantee, struct priv_def *priv)
 {
-	struct access *object = access_find(priv->object_type, priv->object_id);
+	struct access *object = access_find(priv);
 	if (object == NULL)
 		return 0;
 	if (grantee->auth_token == ADMIN && priv->object_type == SC_UNIVERSE &&
diff --git a/src/box/user_def.h b/src/box/user_def.h
index 7acc525b6a..d4b3a76383 100644
--- a/src/box/user_def.h
+++ b/src/box/user_def.h
@@ -88,6 +88,11 @@ struct priv_def {
 	uint32_t grantee_id;
 	/* Object id - is only defined for object type */
 	uint32_t object_id;
+	/**
+	 * If this flag is set, the object id is unused and the privilege
+	 * should be applied to the whole object class.
+	 */
+	bool is_entity_access;
 	/* Object type - function, space, universe */
 	enum schema_object_type object_type;
 	/**
-- 
GitLab