[Tarantool-patches] [PATCH 1/2] sql: remove grants associated with the table

[Chris Sosnin]

Dropping table with sql removes everything
associated with it but grants, which is
inconsistent. Generating code for it fixes this bug.

Closes #4546
---
branch: https://github.com/tarantool/tarantool/tree/ksosnin/gh-4546-sql-drop-grants
issue: https://github.com/tarantool/tarantool/issues/4546

 src/box/sql/build.c                       | 46 +++++++++++++++++++++++
 src/box/user.cc                           |  7 ++++
 src/box/user.h                            |  4 ++
 test/sql/gh-4546-sql-drop-grants.result   | 38 +++++++++++++++++++
 test/sql/gh-4546-sql-drop-grants.test.lua | 19 ++++++++++
 5 files changed, 114 insertions(+)
 create mode 100644 test/sql/gh-4546-sql-drop-grants.result
 create mode 100644 test/sql/gh-4546-sql-drop-grants.test.lua

diff --git a/src/box/sql/build.c b/src/box/sql/build.c
index 51cd7ce63..f1433645a 100644
--- a/src/box/sql/build.c
+++ b/src/box/sql/build.c
@@ -56,6 +56,7 @@
 #include "box/schema.h"
 #include "box/tuple_format.h"
 #include "box/coll_id_cache.h"
+#include "box/user.h"
 
 void
 sql_finish_coding(struct Parse *parse_context)
@@ -1523,6 +1524,45 @@ vdbe_emit_ck_constraint_drop(struct Parse *parser, const char *ck_name,
 	sqlReleaseTempRange(parser, key_reg, 3);
 }
 
+/**
+ * Generate VDBE program to revoke all
+ * privileges associated with the given object.
+ *
+ * @param parser Parsing context.
+ * @param object_type Object type.
+ * @param object_id Object id.
+ * @param access Access array associated with an object.
+ */
+static void
+vdbe_emit_revoke_object(struct Parse *parser, const char *object_type,
+			uint32_t object_id, struct access *access)
+{
+	struct Vdbe *v = sqlGetVdbe(parser);
+	assert(v != NULL);
+	/*
+	 * Get uid of users through access array
+	 * and generate code to delete corresponding
+	 * entries from _priv
+	 */
+	int key_reg = sqlGetTempRange(parser, 4);
+	bool had_grants = false;
+	for (uint8_t token = 0; token < BOX_USER_MAX; ++token) {
+		if (!access[token].granted)
+			continue;
+		had_grants = true;
+		const struct user *user = user_find_by_token(token);
+		sqlVdbeAddOp2(v, OP_Integer, user->def->uid, key_reg);
+		sqlVdbeAddOp4(v, OP_String8, 0, key_reg + 1, 0,
+			      object_type, P4_STATIC);
+		sqlVdbeAddOp2(v, OP_Integer, object_id, key_reg + 2);
+		sqlVdbeAddOp3(v, OP_MakeRecord, key_reg, 3, key_reg + 3);
+		sqlVdbeAddOp2(v, OP_SDelete, BOX_PRIV_ID, key_reg + 3);
+	}
+	if (had_grants)
+		VdbeComment((v, "Remove %s grants", object_type));
+	sqlReleaseTempRange(parser, key_reg, 4);
+}
+
 /**
  * Generate code to drop a table.
  * This routine includes dropping triggers, sequences,
@@ -1538,6 +1578,12 @@ sql_code_drop_table(struct Parse *parse_context, struct space *space,
 {
 	struct Vdbe *v = sqlGetVdbe(parse_context);
 	assert(v != NULL);
+	/*
+	 * Remove all grants associated with
+	 * with the table being dropped.
+	 */
+	vdbe_emit_revoke_object(parse_context, "space", space->def->id,
+				space->access);
 	/*
 	 * Drop all triggers associated with the table being
 	 * dropped. Code is generated to remove entries from
diff --git a/src/box/user.cc b/src/box/user.cc
index a012cb196..fe0555886 100644
--- a/src/box/user.cc
+++ b/src/box/user.cc
@@ -517,6 +517,13 @@ user_find(uint32_t uid)
 	return user;
 }
 
+/* Find a user by authentication token. */
+struct user *
+user_find_by_token(uint8_t auth_token)
+{
+    return &users[auth_token];
+}
+
 /** Find user by name. */
 struct user *
 user_find_by_name(const char *name, uint32_t len)
diff --git a/src/box/user.h b/src/box/user.h
index 545401d5c..ccfc59346 100644
--- a/src/box/user.h
+++ b/src/box/user.h
@@ -112,6 +112,10 @@ user_find_by_name(const char *name, uint32_t len);
 struct user *
 user_find(uint32_t uid);
 
+/* Find a user by authentication token. */
+struct user *
+user_find_by_token(uint8_t auth_token);
+
 /** Create a cache of user's privileges in @a cr. */
 void
 credentials_create(struct credentials *cr, struct user *user);
diff --git a/test/sql/gh-4546-sql-drop-grants.result b/test/sql/gh-4546-sql-drop-grants.result
new file mode 100644
index 000000000..1079d2ab4
--- /dev/null
+++ b/test/sql/gh-4546-sql-drop-grants.result
@@ -0,0 +1,38 @@
+test_run = require('test_run').new()
+---
+...
+engine = test_run:get_cfg('engine')
+---
+...
+--
+-- gh-4546: DROP TABLE must delete all privileges given on that
+-- table to any user.
+--
+box.schema.user.create('test_user1')
+---
+...
+box.schema.user.create('test_user2')
+---
+...
+test_space = box.schema.create_space('T', {		\
+	engine = engine,				\
+	format = {{'i', 'integer'}}			\
+})
+---
+...
+box.schema.user.grant('test_user1', 'read', 'space', 'T')
+---
+...
+box.schema.user.grant('test_user2', 'write', 'space', 'T')
+---
+...
+box.execute([[DROP TABLE T;]])
+---
+- row_count: 1
+...
+box.schema.user.drop('test_user1')
+---
+...
+box.schema.user.drop('test_user2')
+---
+...
diff --git a/test/sql/gh-4546-sql-drop-grants.test.lua b/test/sql/gh-4546-sql-drop-grants.test.lua
new file mode 100644
index 000000000..09cfeda19
--- /dev/null
+++ b/test/sql/gh-4546-sql-drop-grants.test.lua
@@ -0,0 +1,19 @@
+test_run = require('test_run').new()
+engine = test_run:get_cfg('engine')
+
+--
+-- gh-4546: DROP TABLE must delete all privileges given on that
+-- table to any user.
+--
+
+box.schema.user.create('test_user1')
+box.schema.user.create('test_user2')
+test_space = box.schema.create_space('T', {		\
+	engine = engine,				\
+	format = {{'i', 'integer'}}			\
+})
+box.schema.user.grant('test_user1', 'read', 'space', 'T')
+box.schema.user.grant('test_user2', 'write', 'space', 'T')
+box.execute([[DROP TABLE T;]])
+box.schema.user.drop('test_user1')
+box.schema.user.drop('test_user2')
-- 
2.21.0 (Apple Git-122.2)