[tarantool-patches] Re: [PATCH 1/3] box: Add privilleges constants to lua

[Vladislav Shpilevoy <v.shpilevoy@tarantool.org>]

Hello. Please, do not change older upgrade functions in
upgrade.lua. They are not executed on new versions, and
so _priv is not updated.

Because of such updates now 1.8.2 and 1.8.4 upgrade to
2.1.0 does not work.

On 08/06/2018 12:06, Georgy Kirichenko wrote:
> From: imarkov <imarkov@tarantool.org>
> 
> Add lua bindings of PRIV_XXX constants.
> 
> This patch helps to avoid using numerical constants of privilleges
> in schema.lua code.
> 
> Relates #945
> ---
>   src/box/lua/schema.lua       | 67 ++++++++++++++++++++++++++++--------
>   src/box/lua/upgrade.lua      | 40 +++++++++++----------
>   test/box/misc.result         |  1 +
>   test/engine/iterator.result  |  2 +-
>   test/engine/savepoint.result | 12 +++----
>   5 files changed, 82 insertions(+), 40 deletions(-)
> 
> diff --git a/src/box/lua/schema.lua b/src/box/lua/schema.lua
> index d6d39170f..43c7d4e6b 100644
> --- a/src/box/lua/schema.lua
> +++ b/src/box/lua/schema.lua
> @@ -101,8 +101,47 @@ ffi.cdef[[
>   
>       void password_prepare(const char *password, int len,
>                             char *out, int out_len);
> +
> +    enum priv_type {
> +        PRIV_R = 1,
> +        PRIV_W = 2,
> +        PRIV_X = 4,
> +        PRIV_S = 8,
> +        PRIV_U = 16,
> +        PRIV_C = 32,
> +        PRIV_D = 64,
> +        PRIV_A = 128,
> +        PRIV_REFERENCE = 256,
> +        PRIV_TRIGGER = 512,
> +        PRIV_INSERT = 1024,
> +        PRIV_UPDATE = 2048,
> +        PRIV_DELETE = 4096,
> +        PRIV_GRANT = 8192,
> +        PRIV_REVOKE = 16384,
> +        PRIV_ALL  = 4294967295
> +    };
> +
>   ]]
>   
> +box.priv = {
> +    ["R"] = builtin.PRIV_R,
> +    ["W"] = builtin.PRIV_W,
> +    ["X"] = builtin.PRIV_X,
> +    ["S"] = builtin.PRIV_S,
> +    ["U"] = builtin.PRIV_U,
> +    ["C"] = builtin.PRIV_C,
> +    ["D"] = builtin.PRIV_D,
> +    ["A"] = builtin.PRIV_A,
> +    ["REFERENCE"] = builtin.PRIV_REFERENCE,
> +    ["TRIGGER"] = builtin.PRIV_TRIGGER,
> +    ["INSERT"] = builtin.PRIV_INSERT,
> +    ["UPDATE"] = builtin.PRIV_UPDATE,
> +    ["DELETE"] = builtin.PRIV_DELETE,
> +    ["GRANT"]= builtin.PRIV_GRANT,
> +    ["REVOKE"] = builtin.PRIV_REVOKE,
> +    ["ALL"] = builtin.PRIV_ALL
> +}
> +
>   local function user_or_role_resolve(user)
>       local _vuser = box.space[box.schema.VUSER_ID]
>       local tuple
> @@ -1687,7 +1726,7 @@ end
>   
>   local function checked_privilege(privilege, object_type)
>       local priv_hex = privilege_resolve(privilege)
> -    if object_type == 'role' and priv_hex ~= 4 then
> +    if object_type == 'role' and priv_hex ~= box.priv.X then
>           box.error(box.error.UNSUPPORTED_ROLE_PRIV, privilege)
>       end
>       return priv_hex
> @@ -1695,43 +1734,43 @@ end
>   
>   local function privilege_name(privilege)
>       local names = {}
> -    if bit.band(privilege, 1) ~= 0 then
> +    if bit.band(privilege, box.priv.R) ~= 0 then
>           table.insert(names, "read")
>       end
> -    if bit.band(privilege, 2) ~= 0 then
> +    if bit.band(privilege, box.priv.W) ~= 0 then
>           table.insert(names, "write")
>       end
> -    if bit.band(privilege, 4) ~= 0 then
> +    if bit.band(privilege, box.priv.X) ~= 0 then
>           table.insert(names, "execute")
>       end
> -    if bit.band(privilege, 8) ~= 0 then
> +    if bit.band(privilege, box.priv.S) ~= 0 then
>           table.insert(names, "session")
>       end
> -    if bit.band(privilege, 16) ~= 0 then
> +    if bit.band(privilege, box.priv.U) ~= 0 then
>           table.insert(names, "usage")
>       end
> -    if bit.band(privilege, 32) ~= 0 then
> +    if bit.band(privilege, box.priv.C) ~= 0 then
>           table.insert(names, "create")
>       end
> -    if bit.band(privilege, 64) ~= 0 then
> +    if bit.band(privilege, box.priv.D) ~= 0 then
>           table.insert(names, "drop")
>       end
> -    if bit.band(privilege, 128) ~= 0 then
> +    if bit.band(privilege, box.priv.A) ~= 0 then
>           table.insert(names, "alter")
>       end
> -    if bit.band(privilege, 256) ~= 0 then
> +    if bit.band(privilege, box.priv.REFERENCE) ~= 0 then
>           table.insert(names, "reference")
>       end
> -    if bit.band(privilege, 512) ~= 0 then
> +    if bit.band(privilege, box.priv.TRIGGER) ~= 0 then
>           table.insert(names, "trigger")
>       end
> -    if bit.band(privilege, 1024) ~= 0 then
> +    if bit.band(privilege, box.priv.INSERT) ~= 0 then
>           table.insert(names, "insert")
>       end
> -    if bit.band(privilege, 2048) ~= 0 then
> +    if bit.band(privilege, box.priv.UPDATE) ~= 0 then
>           table.insert(names, "update")
>       end
> -    if bit.band(privilege, 4096) ~= 0 then
> +    if bit.band(privilege, box.priv.DELETE) ~= 0 then
>           table.insert(names, "delete")
>       end
>       return table.concat(names, ",")
> diff --git a/src/box/lua/upgrade.lua b/src/box/lua/upgrade.lua
> index 589161944..0293f6ef8 100644
> --- a/src/box/lua/upgrade.lua
> +++ b/src/box/lua/upgrade.lua
> @@ -211,11 +211,11 @@ local function initial()
>       log.info("create role public")
>       _user:insert{PUBLIC, ADMIN, 'public', 'role'}
>       log.info("grant read,write,execute on universe to admin")
> -    _priv:insert{ADMIN, ADMIN, 'universe', 0, 7}
> +    _priv:insert{ADMIN, ADMIN, 'universe', 0, box.priv.R + box.priv.W + box.priv.X}
>   
>       -- grant role 'public' to 'guest'
>       log.info("grant role public to guest")
> -    _priv:insert{ADMIN, GUEST, 'role', PUBLIC, 4}
> +    _priv:insert{ADMIN, GUEST, 'role', PUBLIC, box.priv.X}
>   
>       log.info("set max_id to box.schema.SYSTEM_ID_MAX")
>       _schema:insert{'max_id', box.schema.SYSTEM_ID_MAX}
> @@ -406,7 +406,7 @@ local function create_sysview(source_id, target_id)
>       -- public can read system views
>       if box.space._priv.index.primary:count({PUBLIC, 'space', target_id}) == 0 then
>           log.info("grant read access to 'public' role for %s view", def[3])
> -        box.space._priv:insert({1, PUBLIC, 'space', target_id, 1})
> +        box.space._priv:insert({1, PUBLIC, 'space', target_id, box.priv.R})
>       end
>   end
>   
> @@ -416,16 +416,17 @@ local function upgrade_users_to_1_6_8()
>           local RPL_ID = box.space._user:auto_increment{ADMIN, 'replication', 'role'}[1]
>           -- replication can read the entire universe
>           log.info("grant read on universe to replication")
> -        box.space._priv:replace{1, RPL_ID, 'universe', 0, 1}
> +        box.space._priv:replace{1, RPL_ID, 'universe', 0, box.priv.R}
>           -- replication can append to '_cluster' system space
>           log.info("grant write on space _cluster to replication")
> -        box.space._priv:replace{1, RPL_ID, 'space', box.space._cluster.id, 2}
> +        box.space._priv:replace{1, RPL_ID, 'space', box.space._cluster.id, box.priv.W}
>       end
>   
>       if box.space._priv.index.primary:count({ADMIN, 'universe', 0}) == 0 then
>           -- grant admin access to the universe
>           log.info("grant all on universe to admin")
> -        box.space._priv:insert{ADMIN, ADMIN, 'universe', 0, 7}
> +        box.space._priv:insert{ADMIN, ADMIN, 'universe', 0, box.priv.R +
> +                                                        box.priv.W + box.priv.X}
>       end
>   
>       if box.space._func.index.name:count("box.schema.user.info") == 0 then
> @@ -435,7 +436,7 @@ local function upgrade_users_to_1_6_8()
>   
>           -- grant 'public' role access to 'box.schema.user.info' function
>           log.info('grant execute on function "box.schema.user.info" to public')
> -        box.space._priv:replace{ADMIN, PUBLIC, 'function', 1, 4}
> +        box.space._priv:replace{ADMIN, PUBLIC, 'function', 1, box.priv.X}
>       end
>   end
>   
> @@ -555,7 +556,7 @@ local function create_truncate_space()
>       box.space._index:insert{_truncate.id, 0, 'primary', 'tree', {unique = true}, {{0, 'unsigned'}}}
>   
>       local _priv = box.space[box.schema.PRIV_ID]
> -    _priv:insert{ADMIN, PUBLIC, 'space', _truncate.id, 2}
> +    _priv:insert{ADMIN, PUBLIC, 'space', _truncate.id, box.priv.W}
>   end
>   
>   local function update_existing_users_to_1_7_5()
> @@ -809,20 +810,20 @@ local function initial_1_7_5()
>       -- Create grants
>       --
>       log.info("grant read,write,execute on universe to admin")
> -    _priv:insert{ADMIN, ADMIN, 'universe', 0, 7}
> +    _priv:insert{ADMIN, ADMIN, 'universe', 0, box.priv.R + box.priv.W + box.priv.X}
>   
>       -- grant role 'public' to 'guest'
>       log.info("grant role public to guest")
> -    _priv:insert{ADMIN, GUEST, 'role', PUBLIC, 4}
> +    _priv:insert{ADMIN, GUEST, 'role', PUBLIC, box.priv.X}
>   
>       -- replication can read the entire universe
>       log.info("grant read on universe to replication")
> -    _priv:replace{ADMIN, REPLICATION, 'universe', 0, 1}
> +    _priv:replace{ADMIN, REPLICATION, 'universe', 0, box.priv.R}
>       -- replication can append to '_cluster' system space
>       log.info("grant write on space _cluster to replication")
> -    _priv:replace{ADMIN, REPLICATION, 'space', _cluster.id, 2}
> +    _priv:replace{ADMIN, REPLICATION, 'space', _cluster.id, box.priv.W}
>   
> -    _priv:insert{ADMIN, PUBLIC, 'space', _truncate.id, 2}
> +    _priv:insert{ADMIN, PUBLIC, 'space', _truncate.id, box.priv.W}
>   
>       -- create "box.schema.user.info" function
>       log.info('create function "box.schema.user.info" with setuid')
> @@ -830,7 +831,7 @@ local function initial_1_7_5()
>   
>       -- grant 'public' role access to 'box.schema.user.info' function
>       log.info('grant execute on function "box.schema.user.info" to public')
> -    _priv:replace{ADMIN, PUBLIC, 'function', 1, 4}
> +    _priv:replace{ADMIN, PUBLIC, 'function', 1, box.priv.X}
>   
>       log.info("set max_id to box.schema.SYSTEM_ID_MAX")
>       _schema:insert{'max_id', box.schema.SYSTEM_ID_MAX}
> @@ -904,7 +905,7 @@ local function create_collation_space()
>       box.space._collation:replace{2, "unicode_ci", ADMIN, "ICU", "", {strength='primary'}}
>   
>       local _priv = box.space[box.schema.PRIV_ID]
> -    _priv:insert{ADMIN, PUBLIC, 'space', _collation.id, 2}
> +    _priv:insert{ADMIN, PUBLIC, 'space', _collation.id, box.priv.W}
>   end
>   
>   local function upgrade_to_1_7_6()
> @@ -924,7 +925,8 @@ local function upgrade_to_1_7_7()
>       --
>       for _, v in _user:pairs() do
>           if v[4] ~= "role" then
> -            _priv:upsert({ADMIN, v[1], "universe", 0, 24}, {{"|", 5, 24}})
> +            _priv:upsert({ADMIN, v[1], "universe", 0, box.priv.S + box.priv.U},
> +                                                {{"|", 5, box.priv.S + box.priv.U}})
>           end
>       end
>       --
> @@ -935,14 +937,14 @@ local function upgrade_to_1_7_7()
>       --
>       for _, v in _priv.index.object:pairs{'universe'} do
>           if bit.band(v[5], 1) ~= 0 and bit.band(v[5], 2) ~= 0 then
> -            _priv:update({v[2], v[3], v[4]}, {{ "|", 5, 32}})
> +            _priv:update({v[2], v[3], v[4]}, {{ "|", 5, box.priv.C}})
>           end
>       end
>       -- grant admin all new privileges (session, usage, grant option,
>       -- create, alter, drop and anything that might come up in the future
>       --
> -    _priv:upsert({ADMIN, ADMIN, 'universe', 0, 4294967295},
> -                 {{ "|", 5, 4294967295}})
> +    _priv:upsert({ADMIN, ADMIN, 'universe', 0, box.priv.ALL},
> +                 {{ "|", 5, box.priv.ALL}})
>       --
>       -- create role 'super' and grant it all privileges on universe
>       --
> diff --git a/test/box/misc.result b/test/box/misc.result
> index 8f94f5513..c6e4917bf 100644
> --- a/test/box/misc.result
> +++ b/test/box/misc.result
> @@ -68,6 +68,7 @@ t
>     - info
>     - internal
>     - once
> +  - priv
>     - rollback
>     - rollback_to_savepoint
>     - runtime
> diff --git a/test/engine/iterator.result b/test/engine/iterator.result
> index ae14c4320..1bde10eaf 100644
> --- a/test/engine/iterator.result
> +++ b/test/engine/iterator.result
> @@ -4211,7 +4211,7 @@ s:replace{35}
>   ...
>   state, value = gen(param,state)
>   ---
> -- error: 'builtin/box/schema.lua:993: usage: next(param, state)'
> +- error: 'builtin/box/schema.lua:1032: usage: next(param, state)'
>   ...
>   value
>   ---
> diff --git a/test/engine/savepoint.result b/test/engine/savepoint.result
> index dc2ad7986..a62a2e135 100644
> --- a/test/engine/savepoint.result
> +++ b/test/engine/savepoint.result
> @@ -14,7 +14,7 @@ s1 = box.savepoint()
>   ...
>   box.rollback_to_savepoint(s1)
>   ---
> -- error: 'builtin/box/schema.lua:301: Usage: box.rollback_to_savepoint(savepoint)'
> +- error: 'builtin/box/schema.lua:340: Usage: box.rollback_to_savepoint(savepoint)'
>   ...
>   box.begin() s1 = box.savepoint()
>   ---
> @@ -323,27 +323,27 @@ test_run:cmd("setopt delimiter ''");
>   ok1, errmsg1
>   ---
>   - false
> -- 'builtin/box/schema.lua:301: Usage: box.rollback_to_savepoint(savepoint)'
> +- 'builtin/box/schema.lua:340: Usage: box.rollback_to_savepoint(savepoint)'
>   ...
>   ok2, errmsg2
>   ---
>   - false
> -- 'builtin/box/schema.lua:301: Usage: box.rollback_to_savepoint(savepoint)'
> +- 'builtin/box/schema.lua:340: Usage: box.rollback_to_savepoint(savepoint)'
>   ...
>   ok3, errmsg3
>   ---
>   - false
> -- 'builtin/box/schema.lua:301: Usage: box.rollback_to_savepoint(savepoint)'
> +- 'builtin/box/schema.lua:340: Usage: box.rollback_to_savepoint(savepoint)'
>   ...
>   ok4, errmsg4
>   ---
>   - false
> -- 'builtin/box/schema.lua:301: Usage: box.rollback_to_savepoint(savepoint)'
> +- 'builtin/box/schema.lua:340: Usage: box.rollback_to_savepoint(savepoint)'
>   ...
>   ok5, errmsg5
>   ---
>   - false
> -- 'builtin/box/schema.lua:301: Usage: box.rollback_to_savepoint(savepoint)'
> +- 'builtin/box/schema.lua:340: Usage: box.rollback_to_savepoint(savepoint)'
>   ...
>   s:select{}
>   ---
>