[tarantool-patches] Re: [PATCH] Add entities user, role to access control.
Konstantin Osipov
kostja at tarantool.org
Thu Jul 26 23:15:52 MSK 2018
* Serge Petrenko <sergepetrenko at tarantool.org> [18/07/12 20:54]:
> + /* Do not allow changes for system users and roles. */
> + struct credentials *cr = effective_user();
> + if ((uid <= (uint32_t) BOX_SYSTEM_USER_ID_MAX || uid == SUPER) &&
> + cr->uid != ADMIN) {
> + struct user *current_user = user_find_xc(cr->uid);
> + tnt_raise(AccessDeniedError, "alter", "user or role",
> + old_user->def->name, current_user->def->name);
> + }
It should be possible to change these system objects, provided the
person knows what they are doing, please remove this check, esp.
since it's outside the scope of this bug fix.
> case SC_ROLE:
> {
> - struct user *role = user_by_id(priv->object_id);
> - if (role == NULL || role->def->type != SC_ROLE) {
> - tnt_raise(ClientError, ER_NO_SUCH_ROLE,
> - role ? role->def->name :
> - int2str(priv->object_id));
> - }
> - /*
> - * Only the creator of the role can grant or revoke it.
> - * Everyone can grant 'PUBLIC' role.
> - */
> - if (role->def->owner != grantor->def->uid &&
> - grantor->def->uid != ADMIN &&
> - (role->def->uid != PUBLIC || priv->access != PRIV_X)) {
> + if (priv->object_id != 0) {
> + struct user *role = user_by_id(priv->object_id);
> + if (role == NULL || role->def->type != SC_ROLE) {
> + tnt_raise(ClientError, ER_NO_SUCH_ROLE,
> + role ? role->def->name :
> + int2str(priv->object_id));
> + }
> + /*
> + * Only the creator of the role can grant or revoke it.
> + * Everyone can grant 'PUBLIC' role.
> + */
> + if (role->def->owner != grantor->def->uid &&
> + grantor->def->uid != ADMIN &&
> + (role->def->uid != PUBLIC || priv->access != PRIV_X)) {
> + tnt_raise(AccessDeniedError,
> + priv_name(priv_type),
> + schema_object_name(SC_ROLE), name,
> + grantor->def->name);
> + }
> + /* Not necessary to do during revoke, but who cares. */
> + role_check(grantee, role);
> + } else if (grantor->def->uid != ADMIN) {
> + /* only admin may grant privileges on an entire entity. */
> tnt_raise(AccessDeniedError,
> priv_name(priv_type),
> schema_object_name(SC_ROLE), name,
> grantor->def->name);
> }
> - /* Not necessary to do during revoke, but who cares. */
> - role_check(grantee, role);
Why no 'break;'?
Instead of changing indent level you could have added a break.
> + case SC_USER:
> + {
> + /*
> + * user ID 0 is shared between user 'guest' and granting
> + * privileges upon whole entity user. This is not a problem,
> + * since we don't want to grant privileges on any system user,
> + * including 'guest'.
> + */
> + if(priv->object_id == 0) {
> + access = entity_access.user;
> + break;
> + }
Hm, does it mean that now it's not possible to grant some
privileges to 'guest'? Looks like we should be using a different
identifier for entity-level grants then?
Are you testing this at all? (I found no test for this).
--
Konstantin Osipov, Moscow, Russia, +7 903 626 22 32
http://tarantool.io - www.twitter.com/kostja_osipov
More information about the Tarantool-patches
mailing list