[PATCH 1/2] box: remove compatibility mode for privileges
Serge Petrenko
sergepetrenko at tarantool.org
Tue Oct 30 16:32:00 MSK 2018
Before version 1.7.7 there were no CREATE or ALTER privileges.
READ+WRITE permitted object creation and altering.
In 1.10 CREATE and ALTER privileges were introduced together with a
compatibility mode in access_check_ddl() which assumed user had CREATE
and ALTER if it had READ and WRITE on a respective object.
Now its time for us to remove this compatibility mode.
Part of #3539
---
src/box/alter.cc | 55 +++++++++++++++-------------------------
test/box/access.result | 40 +++++++++++++++++++++++------
test/box/access.test.lua | 18 +++++++++----
test/sql/iproto.result | 6 +++++
test/sql/iproto.test.lua | 2 ++
5 files changed, 74 insertions(+), 47 deletions(-)
diff --git a/src/box/alter.cc b/src/box/alter.cc
index a6bb5a0f0..6d2c59bbc 100644
--- a/src/box/alter.cc
+++ b/src/box/alter.cc
@@ -65,23 +65,10 @@
static void
access_check_ddl(const char *name, uint32_t object_id, uint32_t owner_uid,
- enum schema_object_type type, enum priv_type priv_type,
- bool is_17_compat_mode)
+ enum schema_object_type type, enum priv_type priv_type)
{
struct credentials *cr = effective_user();
user_access_t has_access = cr->universal_access;
- /*
- * XXX: pre 1.7.7 there was no specific 'CREATE' or
- * 'ALTER' ACL, instead, read and write access on universe
- * was used to allow create/alter.
- * For backward compatibility, if a user has read and write
- * access on the universe, grant it CREATE access
- * automatically.
- * The legacy fix does not affect sequences since they
- * were added in 1.7.7 only.
- */
- if (is_17_compat_mode && has_access & PRIV_R && has_access & PRIV_W)
- has_access |= PRIV_C | PRIV_A;
user_access_t access = ((PRIV_U | (user_access_t) priv_type) &
~has_access);
@@ -1678,7 +1665,7 @@ on_replace_dd_space(struct trigger * /* trigger */, void *event)
auto def_guard =
make_scoped_guard([=] { space_def_delete(def); });
access_check_ddl(def->name, def->id, def->uid, SC_SPACE,
- PRIV_C, true);
+ PRIV_C);
RLIST_HEAD(empty_list);
struct space *space = space_new_xc(def, &empty_list);
/**
@@ -1740,7 +1727,7 @@ on_replace_dd_space(struct trigger * /* trigger */, void *event)
}
} else if (new_tuple == NULL) { /* DELETE */
access_check_ddl(old_space->def->name, old_space->def->id,
- old_space->def->uid, SC_SPACE, PRIV_D, true);
+ old_space->def->uid, SC_SPACE, PRIV_D);
/* Verify that the space is empty (has no indexes) */
if (old_space->index_count) {
tnt_raise(ClientError, ER_DROP_SPACE,
@@ -1818,7 +1805,7 @@ on_replace_dd_space(struct trigger * /* trigger */, void *event)
auto def_guard =
make_scoped_guard([=] { space_def_delete(def); });
access_check_ddl(def->name, def->id, def->uid, SC_SPACE,
- PRIV_A, true);
+ PRIV_A);
if (def->id != space_id(old_space))
tnt_raise(ClientError, ER_ALTER_SPACE,
space_name(old_space),
@@ -1953,7 +1940,7 @@ on_replace_dd_index(struct trigger * /* trigger */, void *event)
if (old_tuple && new_tuple)
priv_type = PRIV_A;
access_check_ddl(old_space->def->name, old_space->def->id,
- old_space->def->uid, SC_SPACE, priv_type, true);
+ old_space->def->uid, SC_SPACE, priv_type);
struct index *old_index = space_index(old_space, iid);
/*
@@ -2361,7 +2348,7 @@ on_replace_dd_user(struct trigger * /* trigger */, void *event)
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->uid, user->owner, user->type,
- PRIV_C, true);
+ PRIV_C);
auto def_guard = make_scoped_guard([=] { free(user); });
(void) user_cache_replace(user);
def_guard.is_active = false;
@@ -2371,7 +2358,7 @@ on_replace_dd_user(struct trigger * /* trigger */, void *event)
} else if (new_tuple == NULL) { /* DELETE */
access_check_ddl(old_user->def->name, old_user->def->uid,
old_user->def->owner, old_user->def->type,
- PRIV_D, true);
+ PRIV_D);
/* Can't drop guest or super user */
if (uid <= (uint32_t) BOX_SYSTEM_USER_ID_MAX || uid == SUPER) {
tnt_raise(ClientError, ER_DROP_USER,
@@ -2398,7 +2385,7 @@ on_replace_dd_user(struct trigger * /* trigger */, void *event)
*/
struct user_def *user = user_def_new_from_tuple(new_tuple);
access_check_ddl(user->name, user->uid, user->uid,
- old_user->def->type, PRIV_A, true);
+ old_user->def->type, PRIV_A);
auto def_guard = make_scoped_guard([=] { free(user); });
struct trigger *on_commit =
txn_alter_trigger_new(user_cache_alter_user, NULL);
@@ -2501,7 +2488,7 @@ on_replace_dd_func(struct trigger * /* trigger */, void *event)
if (new_tuple != NULL && old_func == NULL) { /* INSERT */
struct func_def *def = func_def_new_from_tuple(new_tuple);
access_check_ddl(def->name, def->fid, def->uid, SC_FUNCTION,
- PRIV_C, true);
+ PRIV_C);
auto def_guard = make_scoped_guard([=] { free(def); });
func_cache_replace(def);
def_guard.is_active = false;
@@ -2516,7 +2503,7 @@ on_replace_dd_func(struct trigger * /* trigger */, void *event)
* who created it or a superuser.
*/
access_check_ddl(old_func->def->name, fid, uid, SC_FUNCTION,
- PRIV_D, true);
+ PRIV_D);
/* Can only delete func if it has no grants. */
if (schema_find_grants("function", old_func->def->fid)) {
tnt_raise(ClientError, ER_DROP_FUNCTION,
@@ -2530,7 +2517,7 @@ on_replace_dd_func(struct trigger * /* trigger */, void *event)
struct func_def *def = func_def_new_from_tuple(new_tuple);
auto def_guard = make_scoped_guard([=] { free(def); });
access_check_ddl(def->name, def->fid, def->uid, SC_FUNCTION,
- PRIV_A, true);
+ PRIV_A);
struct trigger *on_commit =
txn_alter_trigger_new(func_cache_replace_func, NULL);
txn_on_commit(txn, on_commit);
@@ -2688,7 +2675,7 @@ on_replace_dd_collation(struct trigger * /* trigger */, void *event)
assert(old_coll_id != NULL);
access_check_ddl(old_coll_id->name, old_coll_id->id,
old_coll_id->owner_id, SC_COLLATION,
- PRIV_D, false);
+ PRIV_D);
/*
* Set on_commit/on_rollback triggers after
* deletion from the cache to make trigger logic
@@ -2704,7 +2691,7 @@ on_replace_dd_collation(struct trigger * /* trigger */, void *event)
struct coll_id_def new_def;
coll_id_def_new_from_tuple(new_tuple, &new_def);
access_check_ddl(new_def.name, new_def.id, new_def.owner_id,
- SC_COLLATION, PRIV_C, false);
+ SC_COLLATION, PRIV_C);
struct coll_id *new_coll_id = coll_id_new(&new_def);
if (new_coll_id == NULL)
diag_raise();
@@ -2789,7 +2776,7 @@ priv_def_check(struct priv_def *priv, enum priv_type priv_type)
}
const char *name = schema_find_name(priv->object_type, priv->object_id);
access_check_ddl(name, priv->object_id, grantor->def->uid,
- priv->object_type, priv_type, false);
+ priv->object_type, priv_type);
switch (priv->object_type) {
case SC_UNIVERSE:
if (grantor->def->uid != ADMIN) {
@@ -3250,7 +3237,7 @@ on_replace_dd_sequence(struct trigger * /* trigger */, void *event)
ER_CREATE_SEQUENCE);
assert(sequence_by_id(new_def->id) == NULL);
access_check_ddl(new_def->name, new_def->id, new_def->uid,
- SC_SEQUENCE, PRIV_C, false);
+ SC_SEQUENCE, PRIV_C);
sequence_cache_replace(new_def);
alter->new_def = new_def;
} else if (old_tuple != NULL && new_tuple == NULL) { /* DELETE */
@@ -3259,7 +3246,7 @@ on_replace_dd_sequence(struct trigger * /* trigger */, void *event)
struct sequence *seq = sequence_by_id(id);
assert(seq != NULL);
access_check_ddl(seq->def->name, seq->def->id, seq->def->uid,
- SC_SEQUENCE, PRIV_D, false);
+ SC_SEQUENCE, PRIV_D);
if (space_has_data(BOX_SEQUENCE_DATA_ID, 0, id))
tnt_raise(ClientError, ER_DROP_SEQUENCE,
seq->def->name, "the sequence has data");
@@ -3276,7 +3263,7 @@ on_replace_dd_sequence(struct trigger * /* trigger */, void *event)
struct sequence *seq = sequence_by_id(new_def->id);
assert(seq != NULL);
access_check_ddl(seq->def->name, seq->def->id, seq->def->uid,
- SC_SEQUENCE, PRIV_A, false);
+ SC_SEQUENCE, PRIV_A);
alter->old_def = seq->def;
alter->new_def = new_def;
}
@@ -3357,20 +3344,20 @@ on_replace_dd_space_sequence(struct trigger * /* trigger */, void *event)
/* Check we have the correct access type on the sequence. * */
if (is_generated || !stmt->new_tuple) {
access_check_ddl(seq->def->name, seq->def->id, seq->def->uid,
- SC_SEQUENCE, priv_type, false);
+ SC_SEQUENCE, priv_type);
} else {
/*
* In case user wants to attach an existing sequence,
* check that it has read and write access.
*/
access_check_ddl(seq->def->name, seq->def->id, seq->def->uid,
- SC_SEQUENCE, PRIV_R, false);
+ SC_SEQUENCE, PRIV_R);
access_check_ddl(seq->def->name, seq->def->id, seq->def->uid,
- SC_SEQUENCE, PRIV_W, false);
+ SC_SEQUENCE, PRIV_W);
}
/** Check we have alter access on space. */
access_check_ddl(space->def->name, space->def->id, space->def->uid,
- SC_SPACE, PRIV_A, false);
+ SC_SPACE, PRIV_A);
struct trigger *on_commit =
txn_alter_trigger_new(on_commit_dd_space_sequence, space);
diff --git a/test/box/access.result b/test/box/access.result
index 8a022d032..9c190240f 100644
--- a/test/box/access.result
+++ b/test/box/access.result
@@ -104,7 +104,10 @@ test_run:cmd("setopt delimiter ''");
box.schema.user.create('rich')
---
...
-box.schema.user.grant('rich', 'read,write', 'universe')
+box.schema.user.grant('rich', 'read,write', 'space', '_func')
+---
+...
+box.schema.user.grant('rich', 'create', 'function')
---
...
session.su('rich')
@@ -130,15 +133,18 @@ box.space['_user']:delete{uid}
---
- error: 'Failed to drop user or role ''rich'': the user has objects'
...
-box.schema.user.revoke('rich', 'read,write', 'universe')
+box.schema.user.revoke('rich', 'public')
---
...
-box.schema.user.revoke('rich', 'public')
+box.schema.user.revoke('rich', 'read,write', 'space', '_func')
---
...
box.schema.user.revoke('rich', 'alter', 'user', 'rich')
---
...
+box.schema.user.revoke('rich', 'create', 'function')
+---
+...
box.schema.user.disable("rich")
---
...
@@ -345,7 +351,13 @@ session = box.session
box.schema.user.create('uniuser')
---
...
-box.schema.user.grant('uniuser', 'read, write, execute', 'universe')
+box.schema.user.grant('uniuser', 'create', 'space')
+---
+...
+box.schema.user.grant('uniuser', 'write', 'space', '_schema')
+---
+...
+box.schema.user.grant('uniuser', 'write', 'space', '_space')
---
...
session.su('uniuser')
@@ -583,7 +595,22 @@ session = nil
box.schema.user.create('twostep')
---
...
-box.schema.user.grant('twostep', 'read,write,execute', 'universe')
+box.schema.user.grant('twostep', 'create', 'space')
+---
+...
+box.schema.user.grant('twostep', 'create', 'function')
+---
+...
+box.schema.user.grant('twostep', 'write', 'space', '_schema')
+---
+...
+box.schema.user.grant('twostep', 'write', 'space', '_space')
+---
+...
+box.schema.user.grant('twostep', 'write', 'space', '_index')
+---
+...
+box.schema.user.grant('twostep', 'read,write', 'space', '_func')
---
...
box.session.su('twostep')
@@ -601,9 +628,6 @@ box.schema.func.create('test')
box.session.su('admin')
---
...
-box.schema.user.revoke('twostep', 'execute,read,write', 'universe')
----
-...
box.schema.user.create('twostep_client')
---
...
diff --git a/test/box/access.test.lua b/test/box/access.test.lua
index f7c18b106..4baeb2ef6 100644
--- a/test/box/access.test.lua
+++ b/test/box/access.test.lua
@@ -50,7 +50,8 @@ end;
usermax();
test_run:cmd("setopt delimiter ''");
box.schema.user.create('rich')
-box.schema.user.grant('rich', 'read,write', 'universe')
+box.schema.user.grant('rich', 'read,write', 'space', '_func')
+box.schema.user.grant('rich', 'create', 'function')
session.su('rich')
uid = session.uid()
box.schema.func.create('dummy')
@@ -58,9 +59,10 @@ session.su('admin')
box.space['_user']:delete{uid}
box.schema.func.drop('dummy')
box.space['_user']:delete{uid}
-box.schema.user.revoke('rich', 'read,write', 'universe')
box.schema.user.revoke('rich', 'public')
+box.schema.user.revoke('rich', 'read,write', 'space', '_func')
box.schema.user.revoke('rich', 'alter', 'user', 'rich')
+box.schema.user.revoke('rich', 'create', 'function')
box.schema.user.disable("rich")
-- test double disable is a no op
box.schema.user.disable("rich")
@@ -154,7 +156,9 @@ box.schema.user.drop('testus')
-- ------------------------------------------------------------
session = box.session
box.schema.user.create('uniuser')
-box.schema.user.grant('uniuser', 'read, write, execute', 'universe')
+box.schema.user.grant('uniuser', 'create', 'space')
+box.schema.user.grant('uniuser', 'write', 'space', '_schema')
+box.schema.user.grant('uniuser', 'write', 'space', '_space')
session.su('uniuser')
us = box.schema.space.create('uniuser_space')
session.su('admin')
@@ -241,13 +245,17 @@ session = nil
-- admin can't manage grants on not owned objects
-- -----------------------------------------------------------
box.schema.user.create('twostep')
-box.schema.user.grant('twostep', 'read,write,execute', 'universe')
+box.schema.user.grant('twostep', 'create', 'space')
+box.schema.user.grant('twostep', 'create', 'function')
+box.schema.user.grant('twostep', 'write', 'space', '_schema')
+box.schema.user.grant('twostep', 'write', 'space', '_space')
+box.schema.user.grant('twostep', 'write', 'space', '_index')
+box.schema.user.grant('twostep', 'read,write', 'space', '_func')
box.session.su('twostep')
twostep = box.schema.space.create('twostep')
index2 = twostep:create_index('primary')
box.schema.func.create('test')
box.session.su('admin')
-box.schema.user.revoke('twostep', 'execute,read,write', 'universe')
box.schema.user.create('twostep_client')
box.schema.user.grant('twostep_client', 'execute', 'function', 'test')
box.schema.user.drop('twostep')
diff --git a/test/sql/iproto.result b/test/sql/iproto.result
index af474bcf5..ee34f6fc1 100644
--- a/test/sql/iproto.result
+++ b/test/sql/iproto.result
@@ -37,6 +37,9 @@ box.sql.execute('select * from test')
box.schema.user.grant('guest','read,write,execute', 'universe')
---
...
+box.schema.user.grant('guest', 'create', 'space')
+---
+...
cn = remote.connect(box.cfg.listen)
---
...
@@ -583,6 +586,9 @@ box.sql.execute('drop table test')
box.schema.user.revoke('guest', 'read,write,execute', 'universe')
---
...
+box.schema.user.revoke('guest', 'create', 'space')
+---
+...
space = nil
---
...
diff --git a/test/sql/iproto.test.lua b/test/sql/iproto.test.lua
index 220331b40..b06559075 100644
--- a/test/sql/iproto.test.lua
+++ b/test/sql/iproto.test.lua
@@ -10,6 +10,7 @@ space:replace{4, 5, '6'}
space:replace{7, 8.5, '9'}
box.sql.execute('select * from test')
box.schema.user.grant('guest','read,write,execute', 'universe')
+box.schema.user.grant('guest', 'create', 'space')
cn = remote.connect(box.cfg.listen)
cn:ping()
@@ -202,6 +203,7 @@ cn:close()
box.sql.execute('drop table test')
box.schema.user.revoke('guest', 'read,write,execute', 'universe')
+box.schema.user.revoke('guest', 'create', 'space')
space = nil
-- Cleanup xlog
--
2.17.1 (Apple Git-112)
More information about the Tarantool-patches
mailing list