[tarantool-patches] Re: [PATCH v2 3/4] Add single object privilege checks to access_check_ddl.
Sergey Petrenko
sergepetrenko at tarantool.org
Mon Jul 30 11:37:01 MSK 2018
Hi, I rebased this patch on top of the new one regarding #3524, and fixed
your comments. Please see 2 my comments and the new diff below.
> 26 июля 2018 г., в 23:37, Konstantin Osipov <kostja at tarantool.org> написал(а):
>
> * Serge Petrenko <sergepetrenko at tarantool.org> [18/07/17 21:31]:
>
> This patch is generally LGTM, but it's based on entity access, so
> I can't push it, since I requested some changes in entity access
> patch. A few questions below.
>
>> access_check_ddl() didn't check for single object privileges, e.g. user
>> with alter access on a space couldn't create an index in this space. It
>> would only succeed if it had alter on entire entity space.
>> Fix this by adding single object privilege checks to access_check_ddl and
>> adding access cache to struct user, to hold other users' privileges on it.
>>
>> Also checking for single object privilege made it possible to grant
>> every user alter privilege on itself, so that a user may change its own
>> password (previously it was possible because of a hack). Removed the
>> hack, and added grant alter to itself upon user creation.
>> Modified tests accordingly, and added a couple of test cases.
>
> What hack did you remove? The problem I have with self-granting
> 'alter' is that old deployments will break, since they don't have
> a self-grant. Maybe we leave the hack in place for a while, begin
> self-granting 'alter', and only remove the hack in 2.0? We
> shouldn't forget to grant everyone an 'alter' on self in 2.0 upgrade script.
> But to begin with, I can't find where this hack was in the code,
> and where you removed it.
Here’s the hack I was talking about: in function on_replace_dd_user in alter.cc
- access_check_ddl(user->name, user->uid, SC_USER, PRIV_A,
- true);
We pass user as its own owner, which enables it to change its own password/username.
I leave the hack in place for now, and also add grant of alter on itself upon user creation:
+ access_check_ddl(user->name, user->uid, user->uid, SC_USER,
+ PRIV_A, true);
schema.lua: function box.schema.user.create():
+ -- grant user 'alter' on itself, so it can
+ -- change its password or username.
+ box.schema.user.grant(uid, 'alter', 'user', uid)
>
>> Closes #3530
>> ---
>> src/box/alter.cc | 123 ++++++++++++++++++++++----------
>> src/box/lua/schema.lua | 5 +-
>> src/box/user.cc | 10 ++-
>> src/box/user.h | 2 +
>> test/box/access.result | 182 +++++++++++++++++++++++++++++++++++++++++++++++
>> test/box/access.test.lua | 53 ++++++++++++++
>> test/box/role.result | 9 +++
>> test/box/sequence.result | 3 +
>> 8 files changed, 347 insertions(+), 40 deletions(-)
>>
>> diff --git a/src/box/alter.cc b/src/box/alter.cc
>> index 6293dcc50..54a09664b 100644
>> --- a/src/box/alter.cc
>> +++ b/src/box/alter.cc
>> @@ -62,7 +62,8 @@
>> /* {{{ Auxiliary functions and methods. */
>>
>> static void
>> -access_check_ddl(const char *name, uint32_t owner_uid,
>> +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)
>> @@ -103,7 +104,48 @@ access_check_ddl(const char *name, uint32_t owner_uid,
>> */
>> if (access == 0 || (is_owner && !(access & (PRIV_U | PRIV_C))))
>> return; /* Access granted. */
>> -
>> + /*
>> + * You can't grant CREATE privilege to a non-existing object.
>> + * USAGE can be granted only globally.
>> + */
>> + if (!(access & (PRIV_U | PRIV_C))) {
>> + /* Check for privileges on a single object. */
>
> Please avoid this deep nesting, use early returns from the
> function for example, or create a separate function,
> access_check_ddl_object()?
Fixed. Added a jump to error case.
>
>> + switch (type) {
>> + case SC_SPACE:
>> + {
>> + struct space *space = space_by_id(object_id);
>> + if (space)
>> + access &= ~space->access[cr->auth_token].effective;
>> + break;
>> + }
>> + case SC_FUNCTION:
>> + {
>> + struct func *func = func_by_id(object_id);
>> + if (func)
>> + access &= ~func->access[cr->auth_token].effective;
>> + break;
>> + }
>> + case SC_USER:
>> + case SC_ROLE:
>> + {
>> + struct user *user_or_role = user_by_id(object_id);
>> + if (user_or_role)
>> + access &= ~user_or_role->access[cr->auth_token].effective;
>
>> + break;
>> + }
>> + case SC_SEQUENCE:
>> + {
>> + struct sequence *seq = sequence_by_id(object_id);
>> + if (seq)
>> + access &= ~seq->access[cr->auth_token].effective;
>> + break;
>> + }
>> + default:
>> + break;
>> + }
>
> OK.
>
>> * check that it has read and write access.
>> */
>> - access_check_ddl(seq->def->name, seq->def->uid, SC_SEQUENCE,
>> - PRIV_R, false);
>> - access_check_ddl(seq->def->name, seq->def->uid, SC_SEQUENCE,
>> - PRIV_W, false);
>> + access_check_ddl(seq->def->name, seq->def->id, seq->def->uid,
>> + SC_SEQUENCE, PRIV_R, false);
>> + access_check_ddl(seq->def->name, seq->def->id, seq->def->uid,
>> + SC_SEQUENCE, PRIV_W, false);
>> }
>> /** Check we have alter access on space. */
>> - access_check_ddl(space->def->name, space->def->uid, SC_SPACE, PRIV_A,
>> - false);
>> + access_check_ddl(space->def->name, space->def->id, space->def->uid,
>> + SC_SPACE, PRIV_A, false);
>>
>> struct trigger *on_commit =
>> txn_alter_trigger_new(on_commit_dd_space_sequence, space);
>> diff --git a/src/box/lua/schema.lua b/src/box/lua/schema.lua
>> index 4b7a14411..a098e44fe 100644
>> --- a/src/box/lua/schema.lua
>> +++ b/src/box/lua/schema.lua
>> @@ -1740,7 +1740,7 @@ local priv_object_combo = {
>> ["role"] = bit.bor(box.priv.X, box.priv.U,
>> box.priv.C, box.priv.D),
>> ["user"] = bit.bor(box.priv.C, box.priv.U,
>> - box.priv.D),
>> + box.priv.A, box.priv.D),
>> }
>> -- grant role 'public' to the user
>> box.schema.user.grant(uid, 'public')
>> + -- grant user 'alter' on itself, so it can
>> + -- change its password or username.
>> + box.schema.user.grant(uid, 'alter', 'user', uid)
>
> --
> Konstantin Osipov, Moscow, Russia, +7 903 626 22 32
> http://tarantool.io - www.twitter.com/kostja_osipov
src/box/alter.cc | 121 +++++++++++++++++++++---------
src/box/lua/schema.lua | 3 +
src/box/user.cc | 8 +-
src/box/user.h | 2 +
test/box/access.result | 186 +++++++++++++++++++++++++++++++++++++++++++++++
test/box/access.test.lua | 57 +++++++++++++++
test/box/role.result | 9 +++
test/box/sequence.result | 3 +
8 files changed, 351 insertions(+), 38 deletions(-)
diff --git a/src/box/alter.cc b/src/box/alter.cc
index d4545f31c..35e4d0999 100644
--- a/src/box/alter.cc
+++ b/src/box/alter.cc
@@ -62,7 +62,8 @@
/* {{{ Auxiliary functions and methods. */
static void
-access_check_ddl(const char *name, uint32_t owner_uid,
+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)
@@ -103,9 +104,51 @@ access_check_ddl(const char *name, uint32_t owner_uid,
*/
if (access == 0 || (is_owner && !(access & (PRIV_U | PRIV_C))))
return; /* Access granted. */
+ /*
+ * You can't grant CREATE privilege to a non-existing object.
+ * USAGE can be granted only globally.
+ */
+ if (access & (PRIV_U | PRIV_C))
+ goto error;
+ /* Check for privileges on a single object. */
+ switch (type) {
+ case SC_SPACE:
+ {
+ struct space *space = space_by_id(object_id);
+ if (space)
+ access &= ~space->access[cr->auth_token].effective;
+ break;
+ }
+ case SC_FUNCTION:
+ {
+ struct func *func = func_by_id(object_id);
+ if (func)
+ access &= ~func->access[cr->auth_token].effective;
+ break;
+ }
+ case SC_USER:
+ case SC_ROLE:
+ {
+ struct user *user_or_role = user_by_id(object_id);
+ if (user_or_role)
+ access &= ~user_or_role->access[cr->auth_token].effective;
+ break;
+ }
+ case SC_SEQUENCE:
+ {
+ struct sequence *seq = sequence_by_id(object_id);
+ if (seq)
+ access &= ~seq->access[cr->auth_token].effective;
+ break;
+ }
+ default:
+ break;
+ }
+ if (access == 0)
+ return; /* Access granted. */
/* Create a meaningful error message. */
- struct user *user = user_find_xc(cr->uid);
+error: struct user *user = user_find_xc(cr->uid);
const char *object_name;
const char *pname;
if (access & PRIV_U) {
@@ -1590,7 +1633,8 @@ on_replace_dd_space(struct trigger * /* trigger */, void *event)
struct space_def *def =
space_def_new_from_tuple(new_tuple, ER_CREATE_SPACE,
region);
- access_check_ddl(def->name, def->uid, SC_SPACE, PRIV_C, true);
+ access_check_ddl(def->name, def->id, def->uid, SC_SPACE,
+ PRIV_C, true);
auto def_guard =
make_scoped_guard([=] { space_def_delete(def); });
RLIST_HEAD(empty_list);
@@ -1623,8 +1667,8 @@ on_replace_dd_space(struct trigger * /* trigger */, void *event)
txn_alter_trigger_new(on_create_space_rollback, space);
txn_on_rollback(txn, on_rollback);
} else if (new_tuple == NULL) { /* DELETE */
- access_check_ddl(old_space->def->name, old_space->def->uid,
- SC_SPACE, PRIV_D, true);
+ access_check_ddl(old_space->def->name, old_space->def->id,
+ old_space->def->uid, SC_SPACE, PRIV_D, true);
/* Verify that the space is empty (has no indexes) */
if (old_space->index_count) {
tnt_raise(ClientError, ER_DROP_SPACE,
@@ -1669,7 +1713,8 @@ on_replace_dd_space(struct trigger * /* trigger */, void *event)
struct space_def *def =
space_def_new_from_tuple(new_tuple, ER_ALTER_SPACE,
region);
- access_check_ddl(def->name, def->uid, SC_SPACE, PRIV_A, true);
+ access_check_ddl(def->name, def->id, def->uid, SC_SPACE,
+ PRIV_A, true);
auto def_guard =
make_scoped_guard([=] { space_def_delete(def); });
if (def->id != space_id(old_space))
@@ -1774,8 +1819,8 @@ on_replace_dd_index(struct trigger * /* trigger */, void *event)
enum priv_type priv_type = new_tuple ? PRIV_C : PRIV_D;
if (old_tuple && new_tuple)
priv_type = PRIV_A;
- access_check_ddl(old_space->def->name, old_space->def->uid, SC_SPACE,
- priv_type, true);
+ access_check_ddl(old_space->def->name, old_space->def->id,
+ old_space->def->uid, SC_SPACE, priv_type, true);
struct index *old_index = space_index(old_space, iid);
/*
@@ -2170,7 +2215,8 @@ 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, user->type, PRIV_C, true);
+ access_check_ddl(user->name, user->uid, 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;
@@ -2178,7 +2224,8 @@ on_replace_dd_user(struct trigger * /* trigger */, void *event)
txn_alter_trigger_new(user_cache_remove_user, NULL);
txn_on_rollback(txn, on_rollback);
} else if (new_tuple == NULL) { /* DELETE */
- access_check_ddl(old_user->def->name, old_user->def->owner,
+ access_check_ddl(old_user->def->name, old_user->def->uid,
+ old_user->def->owner,
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) {
@@ -2205,8 +2252,8 @@ on_replace_dd_user(struct trigger * /* trigger */, void *event)
* correct.
*/
struct user_def *user = user_def_new_from_tuple(new_tuple);
- access_check_ddl(user->name, user->uid, SC_USER, PRIV_A,
- true);
+ access_check_ddl(user->name, user->uid, user->uid, SC_USER,
+ PRIV_A, true);
auto def_guard = make_scoped_guard([=] { free(user); });
struct trigger *on_commit =
txn_alter_trigger_new(user_cache_alter_user, NULL);
@@ -2308,7 +2355,8 @@ on_replace_dd_func(struct trigger * /* trigger */, void *event)
struct func *old_func = func_by_id(fid);
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->uid, SC_FUNCTION, PRIV_C, true);
+ access_check_ddl(def->name, def->fid, def->uid, SC_FUNCTION,
+ PRIV_C, true);
auto def_guard = make_scoped_guard([=] { free(def); });
func_cache_replace(def);
def_guard.is_active = false;
@@ -2322,7 +2370,7 @@ on_replace_dd_func(struct trigger * /* trigger */, void *event)
* Can only delete func if you're the one
* who created it or a superuser.
*/
- access_check_ddl(old_func->def->name, uid, SC_FUNCTION,
+ access_check_ddl(old_func->def->name, fid, uid, SC_FUNCTION,
PRIV_D, true);
/* Can only delete func if it has no grants. */
if (schema_find_grants("function", old_func->def->fid)) {
@@ -2336,8 +2384,8 @@ on_replace_dd_func(struct trigger * /* trigger */, void *event)
} else { /* UPDATE, REPLACE */
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->uid, SC_FUNCTION, PRIV_A,
- true);
+ access_check_ddl(def->name, def->fid, def->uid,
+ SC_FUNCTION, PRIV_A, true);
struct trigger *on_commit =
txn_alter_trigger_new(func_cache_replace_func, NULL);
txn_on_commit(txn, on_commit);
@@ -2493,8 +2541,9 @@ on_replace_dd_collation(struct trigger * /* trigger */, void *event)
BOX_COLLATION_FIELD_ID);
struct coll_id *old_coll_id = coll_by_id(old_id);
assert(old_coll_id != NULL);
- access_check_ddl(old_coll_id->name, old_coll_id->owner_id,
- SC_COLLATION, PRIV_D, false);
+ access_check_ddl(old_coll_id->name, old_id,
+ old_coll_id->owner_id, SC_COLLATION, PRIV_D,
+ false);
/*
* Set on_commit/on_rollback triggers after
* deletion from the cache to make trigger logic
@@ -2509,8 +2558,8 @@ on_replace_dd_collation(struct trigger * /* trigger */, void *event)
/* INSERT */
struct coll_id_def new_def;
coll_id_def_new_from_tuple(new_tuple, &new_def);
- access_check_ddl(new_def.name, new_def.owner_id, SC_COLLATION,
- PRIV_C, false);
+ access_check_ddl(new_def.name, new_def.id, new_def.owner_id,
+ SC_COLLATION, PRIV_C, false);
struct coll_id *new_coll_id = coll_id_new(&new_def);
if (new_coll_id == NULL)
diag_raise();
@@ -2569,8 +2618,8 @@ priv_def_check(struct priv_def *priv, enum priv_type priv_type)
int2str(priv->grantee_id));
}
const char *name = schema_find_name(priv->object_type, priv->object_id);
- access_check_ddl(name, grantor->def->uid, priv->object_type, priv_type,
- false);
+ access_check_ddl(name, priv->object_id, grantor->def->uid,
+ priv->object_type, priv_type, false);
switch (priv->object_type) {
case SC_UNIVERSE:
if (grantor->def->uid != ADMIN) {
@@ -3075,8 +3124,8 @@ on_replace_dd_sequence(struct trigger * /* trigger */, void *event)
new_def = sequence_def_new_from_tuple(new_tuple,
ER_CREATE_SEQUENCE);
assert(sequence_by_id(new_def->id) == NULL);
- access_check_ddl(new_def->name, new_def->uid, SC_SEQUENCE,
- PRIV_C, false);
+ access_check_ddl(new_def->name, new_def->id, new_def->uid,
+ SC_SEQUENCE, PRIV_C, false);
sequence_cache_replace(new_def);
alter->new_def = new_def;
} else if (old_tuple != NULL && new_tuple == NULL) { /* DELETE */
@@ -3084,8 +3133,8 @@ on_replace_dd_sequence(struct trigger * /* trigger */, void *event)
BOX_SEQUENCE_DATA_FIELD_ID);
struct sequence *seq = sequence_by_id(id);
assert(seq != NULL);
- access_check_ddl(seq->def->name, seq->def->uid, SC_SEQUENCE,
- PRIV_D, false);
+ access_check_ddl(seq->def->name, seq->def->id, seq->def->uid,
+ SC_SEQUENCE, PRIV_D, false);
if (space_has_data(BOX_SEQUENCE_DATA_ID, 0, id))
tnt_raise(ClientError, ER_DROP_SEQUENCE,
seq->def->name, "the sequence has data");
@@ -3101,8 +3150,8 @@ on_replace_dd_sequence(struct trigger * /* trigger */, void *event)
ER_ALTER_SEQUENCE);
struct sequence *seq = sequence_by_id(new_def->id);
assert(seq != NULL);
- access_check_ddl(seq->def->name, seq->def->uid, SC_SEQUENCE,
- PRIV_A, false);
+ access_check_ddl(seq->def->name, seq->def->id, seq->def->uid,
+ SC_SEQUENCE, PRIV_A, false);
alter->old_def = seq->def;
alter->new_def = new_def;
}
@@ -3182,21 +3231,21 @@ 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->uid, SC_SEQUENCE,
- priv_type, false);
+ access_check_ddl(seq->def->name, seq->def->id, seq->def->uid,
+ SC_SEQUENCE, priv_type, false);
} 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->uid, SC_SEQUENCE,
- PRIV_R, false);
- access_check_ddl(seq->def->name, seq->def->uid, SC_SEQUENCE,
- PRIV_W, false);
+ access_check_ddl(seq->def->name, seq->def->id, seq->def->uid,
+ SC_SEQUENCE, PRIV_R, false);
+ access_check_ddl(seq->def->name, seq->def->id, seq->def->uid,
+ SC_SEQUENCE, PRIV_W, false);
}
/** Check we have alter access on space. */
- access_check_ddl(space->def->name, space->def->uid, SC_SPACE, PRIV_A,
- false);
+ access_check_ddl(space->def->name, space->def->id, space->def->uid,
+ SC_SPACE, PRIV_A, false);
struct trigger *on_commit =
txn_alter_trigger_new(on_commit_dd_space_sequence, space);
diff --git a/src/box/lua/schema.lua b/src/box/lua/schema.lua
index 28f68cbda..350da210a 100644
--- a/src/box/lua/schema.lua
+++ b/src/box/lua/schema.lua
@@ -2103,6 +2103,9 @@ box.schema.user.create = function(name, opts)
uid = _user:auto_increment{session.euid(), name, 'user', auth_mech_list}[1]
-- grant role 'public' to the user
box.schema.user.grant(uid, 'public')
+ -- grant user 'alter' on itself, so it can
+ -- change its password or username.
+ box.schema.user.grant(uid, 'alter', 'user', uid)
-- we have to grant global privileges from setuid function, since
-- only admin has the ownership over universe and we don't have
-- grant option
diff --git a/src/box/user.cc b/src/box/user.cc
index b4fb65a59..83d07f7b3 100644
--- a/src/box/user.cc
+++ b/src/box/user.cc
@@ -248,12 +248,16 @@ access_find(struct priv_def *priv)
}
case SC_USER:
{
- /* No grants on a single object user yet. */
+ struct user *user = user_by_id(priv->object_id);
+ if (user)
+ access = user->access;
break;
}
case SC_ROLE:
{
- /* No grants on a single object role yet. */
+ struct user *role = user_by_id(priv->object_id);
+ if (role)
+ access = role->access;
break;
}
case SC_SEQUENCE:
diff --git a/src/box/user.h b/src/box/user.h
index 07c4dc504..069d9b77e 100644
--- a/src/box/user.h
+++ b/src/box/user.h
@@ -88,6 +88,8 @@ struct user
bool is_dirty;
/** Memory pool for privs */
struct region pool;
+ /** Cached runtime access imformation. */
+ struct access access[BOX_USER_MAX];
};
/** Find user by id. */
diff --git a/test/box/access.result b/test/box/access.result
index 7acd6fa43..819d8c471 100644
--- a/test/box/access.result
+++ b/test/box/access.result
@@ -136,6 +136,9 @@ box.schema.user.revoke('rich', 'read,write', 'universe')
box.schema.user.revoke('rich', 'public')
---
...
+box.schema.user.revoke('rich', 'alter', 'user', 'rich')
+---
+...
box.schema.user.disable("rich")
---
...
@@ -501,6 +504,7 @@ box.space._priv:select{id}
---
- - [1, 32, 'role', 2, 4]
- [1, 32, 'universe', 0, 27]
+ - [1, 32, 'user', 32, 128]
...
box.schema.user.grant('user', 'read', 'universe')
---
@@ -510,6 +514,7 @@ box.space._priv:select{id}
---
- - [1, 32, 'role', 2, 4]
- [1, 32, 'universe', 0, 27]
+ - [1, 32, 'user', 32, 128]
...
box.schema.user.revoke('user', 'write', 'universe')
---
@@ -518,6 +523,7 @@ box.space._priv:select{id}
---
- - [1, 32, 'role', 2, 4]
- [1, 32, 'universe', 0, 25]
+ - [1, 32, 'user', 32, 128]
...
box.schema.user.revoke('user', 'read', 'universe')
---
@@ -526,6 +532,7 @@ box.space._priv:select{id}
---
- - [1, 32, 'role', 2, 4]
- [1, 32, 'universe', 0, 24]
+ - [1, 32, 'user', 32, 128]
...
box.schema.user.grant('user', 'write', 'universe')
---
@@ -534,6 +541,7 @@ box.space._priv:select{id}
---
- - [1, 32, 'role', 2, 4]
- [1, 32, 'universe', 0, 26]
+ - [1, 32, 'user', 32, 128]
...
box.schema.user.grant('user', 'read', 'universe')
---
@@ -542,6 +550,7 @@ box.space._priv:select{id}
---
- - [1, 32, 'role', 2, 4]
- [1, 32, 'universe', 0, 27]
+ - [1, 32, 'user', 32, 128]
...
box.schema.user.drop('user')
---
@@ -965,6 +974,9 @@ box.schema.user.info('test_user')
- - session,usage
- universe
-
+ - - alter
+ - user
+ - test_user
...
box.schema.role.info('test_role')
---
@@ -995,6 +1007,9 @@ box.schema.user.info('test_user')
- - session,usage
- universe
-
+ - - alter
+ - user
+ - test_user
...
box.schema.role.info('test_role')
---
@@ -1860,3 +1875,174 @@ box.session.su('admin')
box.schema.user.drop('tester')
---
...
+--
+-- test case for 3530: do not ignore single object privileges in
+-- access_check_ddl.
+--
+box.schema.user.create("test")
+---
+...
+_ = box.schema.space.create("space1")
+---
+...
+box.schema.user.grant("test", "read", "space", "space1")
+---
+...
+box.schema.user.grant("test", "write", "space", "_index")
+---
+...
+box.session.su("test")
+---
+...
+box.space.space1:create_index("pk")
+---
+- error: Create access to space 'space1' is denied for user 'test'
+...
+box.session.su("admin")
+---
+...
+box.space.space1.index[0] == nil
+---
+- true
+...
+-- fixme: cannot grant create on a single space
+-- this is because when checking for create
+-- access_check_ddl ignores space privileges,
+-- assuming that there is no space yet.
+box.schema.user.grant("test", "create", "space")
+---
+...
+box.session.su("test")
+---
+...
+_ = box.space.space1:create_index("pk")
+---
+...
+box.space.space1:insert{5}
+---
+- error: Write access to space 'space1' is denied for user 'test'
+...
+box.session.su("admin")
+---
+...
+box.space.space1.index[0] ~= nil
+---
+- true
+...
+box.space.space1:select{}
+---
+- []
+...
+box.schema.user.grant("test", "write", "space", "space1")
+---
+...
+box.session.su("test")
+---
+...
+box.space.space1:insert{5}
+---
+- [5]
+...
+box.session.su("admin")
+---
+...
+box.space.space1:select{}
+---
+- - [5]
+...
+box.schema.user.drop("test")
+---
+...
+box.space.space1:drop()
+---
+...
+--
+-- test that it is possible to grant privileges on a single user.
+box.schema.user.create("user1")
+---
+...
+box.schema.user.create("user2")
+---
+...
+box.schema.user.create("user3")
+---
+...
+box.schema.user.grant("user1", "write", "space", "_user")
+---
+...
+box.schema.user.grant("user1", "read", "space", "_user")
+---
+...
+box.space._user:select{}
+---
+- - [0, 1, 'guest', 'user', {'chap-sha1': 'vhvewKp0tNyweZQ+cFKAlsyphfg='}]
+ - [1, 1, 'admin', 'user', {}]
+ - [2, 1, 'public', 'role', {}]
+ - [3, 1, 'replication', 'role', {}]
+ - [31, 1, 'super', 'role', {}]
+ - [32, 1, 'user1', 'user', {}]
+ - [33, 1, 'user2', 'user', {}]
+ - [34, 1, 'user3', 'user', {}]
+...
+box.session.su("user1")
+---
+...
+-- can alter itself, but can't alter others without privileges.
+box.schema.user.passwd("user1", "abcd")
+---
+...
+box.schema.user.passwd("user2", "abcd")
+---
+- error: Alter access to user 'user2' is denied for user 'user1'
+...
+box.session.su("admin")
+---
+...
+box.space._user:select{}
+---
+- - [0, 1, 'guest', 'user', {'chap-sha1': 'vhvewKp0tNyweZQ+cFKAlsyphfg='}]
+ - [1, 1, 'admin', 'user', {}]
+ - [2, 1, 'public', 'role', {}]
+ - [3, 1, 'replication', 'role', {}]
+ - [31, 1, 'super', 'role', {}]
+ - [32, 1, 'user1', 'user', {'chap-sha1': 'oVTFJWXp5/lL/Aih/nAmJO2O/9o='}]
+ - [33, 1, 'user2', 'user', {}]
+ - [34, 1, 'user3', 'user', {}]
+...
+box.schema.user.grant("user1", "alter", "user", "user2")
+---
+...
+box.session.su("user1")
+---
+...
+box.schema.user.passwd("user2", "abcd")
+---
+...
+-- still fails
+box.schema.user.passwd("user3", "qewr")
+---
+- error: Alter access to user 'user3' is denied for user 'user1'
+...
+box.session.su("admin")
+---
+...
+box.space._user:select{}
+---
+- - [0, 1, 'guest', 'user', {'chap-sha1': 'vhvewKp0tNyweZQ+cFKAlsyphfg='}]
+ - [1, 1, 'admin', 'user', {}]
+ - [2, 1, 'public', 'role', {}]
+ - [3, 1, 'replication', 'role', {}]
+ - [31, 1, 'super', 'role', {}]
+ - [32, 1, 'user1', 'user', {'chap-sha1': 'oVTFJWXp5/lL/Aih/nAmJO2O/9o='}]
+ - [33, 1, 'user2', 'user', {'chap-sha1': 'oVTFJWXp5/lL/Aih/nAmJO2O/9o='}]
+ - [34, 1, 'user3', 'user', {}]
+...
+box.schema.user.drop("user1")
+---
+...
+box.schema.user.drop("user2")
+---
+...
+box.schema.user.drop("user3")
+---
+...
diff --git a/test/box/access.test.lua b/test/box/access.test.lua
index 9b7510e64..991ddf6ba 100644
--- a/test/box/access.test.lua
+++ b/test/box/access.test.lua
@@ -60,6 +60,7 @@ 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', 'alter', 'user', 'rich')
box.schema.user.disable("rich")
-- test double disable is a no op
box.schema.user.disable("rich")
@@ -726,3 +727,59 @@ _ = box.schema.sequence.create('test_sequence')
box.session.su('admin')
box.schema.user.drop('tester')
+
+--
+-- test case for 3530: do not ignore single object privileges in
+-- access_check_ddl.
+--
+box.schema.user.create("test")
+_ = box.schema.space.create("space1")
+box.schema.user.grant("test", "read", "space", "space1")
+box.schema.user.grant("test", "write", "space", "_index")
+box.session.su("test")
+box.space.space1:create_index("pk")
+box.session.su("admin")
+box.space.space1.index[0] == nil
+-- fixme: cannot grant create on a single space
+-- this is because when checking for create
+-- access_check_ddl ignores space privileges,
+-- assuming that there is no space yet.
+box.schema.user.grant("test", "create", "space")
+box.session.su("test")
+_ = box.space.space1:create_index("pk")
+box.space.space1:insert{5}
+box.session.su("admin")
+box.space.space1.index[0] ~= nil
+box.space.space1:select{}
+box.schema.user.grant("test", "write", "space", "space1")
+box.session.su("test")
+box.space.space1:insert{5}
+box.session.su("admin")
+box.space.space1:select{}
+box.schema.user.drop("test")
+box.space.space1:drop()
+
+--
+-- test that it is possible to grant privileges on a single user.
+box.schema.user.create("user1")
+box.schema.user.create("user2")
+box.schema.user.create("user3")
+box.schema.user.grant("user1", "write", "space", "_user")
+box.schema.user.grant("user1", "read", "space", "_user")
+box.space._user:select{}
+box.session.su("user1")
+-- can alter itself, but can't alter others without privileges.
+box.schema.user.passwd("user1", "abcd")
+box.schema.user.passwd("user2", "abcd")
+box.session.su("admin")
+box.space._user:select{}
+box.schema.user.grant("user1", "alter", "user", "user2")
+box.session.su("user1")
+box.schema.user.passwd("user2", "abcd")
+-- still fails
+box.schema.user.passwd("user3", "qewr")
+box.session.su("admin")
+box.space._user:select{}
+box.schema.user.drop("user1")
+box.schema.user.drop("user2")
+box.schema.user.drop("user3")
diff --git a/test/box/role.result b/test/box/role.result
index 243c7bc6c..5666f7ef7 100644
--- a/test/box/role.result
+++ b/test/box/role.result
@@ -49,6 +49,9 @@ box.schema.user.info('tester')
- - session,usage
- universe
-
+ - - alter
+ - user
+ - tester
...
box.schema.user.grant('tester', 'execute', 'role', 'iddqd')
---
@@ -64,6 +67,9 @@ box.schema.user.info('tester')
- - session,usage
- universe
-
+ - - alter
+ - user
+ - tester
...
-- test granting user to a user
box.schema.user.grant('tester', 'execute', 'role', 'tester')
@@ -956,6 +962,9 @@ box.schema.user.info('test_user')
- - session,usage
- universe
-
+ - - alter
+ - user
+ - test_user
...
box.schema.role.info('test_user')
---
diff --git a/test/box/sequence.result b/test/box/sequence.result
index a2a1a60ea..b3907659f 100644
--- a/test/box/sequence.result
+++ b/test/box/sequence.result
@@ -1362,6 +1362,9 @@ box.schema.user.info()
- - session,usage
- universe
-
+ - - alter
+ - user
+ - user
...
sq:set(100) -- ok
---
--
2.15.2 (Apple Git-101.1)
More information about the Tarantool-patches
mailing list