* Serge Petrenko <sergepetrenko@xxxxxxxxxxxxx> [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.
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. */
+ 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;
+ }
* 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)
