[tarantool-patches] Re: [PATCH 3/3] Introduce privileges for object groups
Konstantin Osipov
kostja at tarantool.org
Fri Jun 8 20:26:45 MSK 2018
* Georgy Kirichenko <georgy at tarantool.org> [18/06/08 12:11]:
> Allow define access privileges for all spaces, functions and sequences.
> Read and write privileges are supported for spaces, execute privilege
> for sequences. Privilege granting and revoking might be done through old api
> without object identification:
> box.schema.user.grant("guest", "read", "space")
The patch looks good, I don't understand though why we need it - I
never asked for one.
Does it have a documentation request?
A few minor comments below.
>
> Prerequisite #945
> ---
> src/box/alter.cc | 18 ++++++---
> src/box/call.c | 2 +
> src/box/lua/schema.lua | 9 +++++
> src/box/schema.cc | 8 ++++
> src/box/schema.h | 26 +++++++++++++
> src/box/sequence.c | 1 +
> src/box/space.c | 2 +
> src/box/sysview_index.c | 11 ++++++
> src/box/user.cc | 12 ++++++
> test/box/access.result | 77 +++++++++++++++++++++++++++++++++++++
> test/box/access.test.lua | 29 ++++++++++++++
> test/box/lua/identifier.lua | 1 -
> 12 files changed, 189 insertions(+), 7 deletions(-)
>
> diff --git a/src/box/call.c b/src/box/call.c
> index 6388e1e68..8a43db130 100644
> --- a/src/box/call.c
> +++ b/src/box/call.c
> @@ -71,6 +71,8 @@ access_check_func(const char *name, uint32_t name_len, struct func **funcp)
> return 0;
> }
> user_access_t access = PRIV_X | PRIV_U;
> + /* Check access for all functions. */
> + access &= ~get_entity_access(SC_FUNCTION)[credentials->auth_token].effective;
1) Are you sure get_entity_access() is inlined? This is a hot
path I would try to make it as fast as possible.
2) the name should be entity_access_get() or entity_access_find().
> @@ -295,6 +298,10 @@ vfunc_filter(struct space *source, struct tuple *tuple)
> */
> if ((PRIV_WRDA | PRIV_X) & cr->universal_access)
> return true;
> + /* Allow access for a user with function privileges. */
> + if ((PRIV_WRDA | PRIV_X) &
> + get_entity_access(SC_FUNCTION)[cr->auth_token].effective)
> + return true;
Perhaps we should have a variable for PRIV_WRDA | PRIV_X
Please add a test case checking that granting entity access via
role works.
We also need a test case that revoking entity access works. The
same is for revoking entity access granted via role.
Thank you for working on it,
--
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