[Tarantool-patches] [PATCH] sql: remove grants associated with the table
Chris Sosnin
k.sosnin at tarantool.org
Wed Dec 11 12:50:08 MSK 2019
Thank you for the review!
See fixes for your comments below:
> Sequences also can have grants. And sequence.drop removes them.
> I propose you to drop sequence grants in a second commit.
For this purpose I rewrote the function so it can be used not only for space
objects but for sequences too. Diff:
+/**
+ * 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);
+ for (uint8_t token = 0; token < BOX_USER_MAX; ++token) {
+ if (!access[token].granted)
+ continue;
+ 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);
+ }
+ VdbeComment((v, "Remove %s grants", object_type));
+ sqlReleaseTempRange(parser, key_reg, 4);
+}
And it is used like this:
+ vdbe_emit_revoke_object(parse_context, "space", space->def->id,
+ space->access);
> 1. Seems like the space is not changed here, so can be made
> 'const struct space *'.
Not needed anymore.
> 2. Lets assert that v is never NULL. By the time this
> function is called, Vdbe is always created.
Done:
+ struct Vdbe *v = sqlGetVdbe(parser);
+ assert(v != NULL);
> 3. The indentation is broken, some calls fit one line,
> and there are dangling whitespaces right below 'Add' on
> each line.
Done, you can see it in the first diff above.
> 4. Box.cfg{} is already called by that time, so you
> don't need to call it again.
Done.
> 5. Lets add a minimal format. Just one field type/name.
> Because generally SQL does not work with spaces not
> having a format.
Done. Also added sequence grants in tests:
+test_space = box.schema.create_space('T', {format = {{'i', 'integer'}}})
+seq = box.schema.sequence.create('S')
+ind = test_space:create_index('I', {sequence = 'S'})
+box.schema.user.grant('test_user1', 'write', 'sequence', 'S')
New version of the patch below:
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 | 47 +++++++++++++++++++++++
src/box/user.cc | 7 ++++
src/box/user.h | 4 ++
test/sql/gh-4546-sql-drop-grants.result | 40 +++++++++++++++++++
test/sql/gh-4546-sql-drop-grants.test.lua | 16 ++++++++
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..af937de3b 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,42 @@ 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);
+ for (uint8_t token = 0; token < BOX_USER_MAX; ++token) {
+ if (!access[token].granted)
+ continue;
+ 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);
+ }
+ 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 +1575,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
@@ -1565,6 +1608,10 @@ sql_code_drop_table(struct Parse *parse_context, struct space *space,
sqlVdbeAddOp2(v, OP_Integer, space_id, space_id_reg);
sqlVdbeAddOp1(v, OP_CheckViewReferences, space_id_reg);
if (space->sequence != NULL) {
+ /* Delete entries from _priv */
+ vdbe_emit_revoke_object(parse_context, "sequence",
+ space->sequence->def->id,
+ space->sequence->access);
/* Delete entry from _sequence_data. */
int sequence_id_reg = ++parse_context->nMem;
sqlVdbeAddOp2(v, OP_Integer, space->sequence->def->id,
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..ca95ed0f9
--- /dev/null
+++ b/test/sql/gh-4546-sql-drop-grants.result
@@ -0,0 +1,40 @@
+test_run = require('test_run').new()
+---
+...
+engine = test_run:get_cfg('engine')
+---
+...
+box.execute('pragma sql_default_engine=\''..engine..'\'')
+---
+- row_count: 0
+...
+-- If we drop the table with sql, all associated
+-- grants must be deleted so we don't recieve an error
+box.schema.user.create('test_user1')
+---
+...
+box.schema.user.create('test_user2')
+---
+...
+test_space = box.schema.create_space('T', {format = {{'i', 'integer'}}})
+---
+...
+seq = box.schema.sequence.create('S')
+---
+...
+ind = test_space:create_index('I', {sequence = 'S'})
+---
+...
+box.schema.user.grant('test_user1', 'write', 'sequence', 'S')
+---
+...
+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
+...
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..5ac4b5701
--- /dev/null
+++ b/test/sql/gh-4546-sql-drop-grants.test.lua
@@ -0,0 +1,16 @@
+test_run = require('test_run').new()
+engine = test_run:get_cfg('engine')
+box.execute('pragma sql_default_engine=\''..engine..'\'')
+
+-- If we drop the table with sql, all associated
+-- grants must be deleted so we don't recieve an error
+
+box.schema.user.create('test_user1')
+box.schema.user.create('test_user2')
+test_space = box.schema.create_space('T', {format = {{'i', 'integer'}}})
+seq = box.schema.sequence.create('S')
+ind = test_space:create_index('I', {sequence = 'S'})
+box.schema.user.grant('test_user1', 'write', 'sequence', 'S')
+box.schema.user.grant('test_user1', 'read', 'space', 'T')
+box.schema.user.grant('test_user2', 'write', 'space', 'T')
+box.execute([[DROP TABLE T;]])
--
2.21.0 (Apple Git-122.2)
More information about the Tarantool-patches
mailing list