[Tarantool-patches] [PATCH] box: user.grant error should be versatile
Maria Khaydich
maria.khaydich at tarantool.org
Thu Feb 20 18:48:13 MSK 2020
Thank you for the review. The fix is done.
----------------------------------------------------------------------
Error message on granted privileges was not flexible and
did not distinguish between universal or any other privileges
leaving either 'nil' or simply '' at the end.
Closes #714
---
Issue:
https://github.com/tarantool/tarantool/issues/714
Branch:
https://github.com/tarantool/tarantool/tree/eljashm/gh-714-box-schema-user-grant-invalid-error
src/box/errcode.h | 2 +-
src/box/lua/schema.lua | 3 +++
test/box/access.result | 30 ++++++++++++++++++++++++++++--
test/box/access.test.lua | 14 ++++++++++++++
4 files changed, 46 insertions(+), 3 deletions(-)
diff --git a/src/box/errcode.h b/src/box/errcode.h
index 6f6e28c6c..cac8447e2 100644
--- a/src/box/errcode.h
+++ b/src/box/errcode.h
@@ -141,7 +141,7 @@ struct errcode_record {
/* 86 */_(ER_SESSION_CLOSED, "Session is closed") \
/* 87 */_(ER_ROLE_LOOP, "Granting role '%s' to role '%s' would create a loop") \
/* 88 */_(ER_GRANT, "Incorrect grant arguments: %s") \
- /* 89 */_(ER_PRIV_GRANTED, "User '%s' already has %s access on %s '%s'") \
+ /* 89 */_(ER_PRIV_GRANTED, "User '%s' already has %s access on %s%s") \
/* 90 */_(ER_ROLE_GRANTED, "User '%s' already has role '%s'") \
/* 91 */_(ER_PRIV_NOT_GRANTED, "User '%s' does not have %s access on %s '%s'") \
/* 92 */_(ER_ROLE_NOT_GRANTED, "User '%s' does not have role '%s'") \
diff --git a/src/box/lua/schema.lua b/src/box/lua/schema.lua
index 50c96a335..f537c3cec 100644
--- a/src/box/lua/schema.lua
+++ b/src/box/lua/schema.lua
@@ -2408,6 +2408,9 @@ local function grant(uid, name, privilege, object_type,
privilege == 'execute' then
box.error(box.error.ROLE_GRANTED, name, object_name)
else
+ if object_type ~= 'universe' then
+ object_name = string.format(" '%s'", object_name)
+ end
box.error(box.error.PRIV_GRANTED, name, privilege,
object_type, object_name)
end
diff --git a/test/box/access.result b/test/box/access.result
index 9554991ad..be8b1c521 100644
--- a/test/box/access.result
+++ b/test/box/access.result
@@ -532,7 +532,7 @@ box.space._priv:select{id}
...
box.schema.user.grant('user', 'read', 'universe')
---
-- error: User 'user' already has read access on universe ''
+- error: User 'user' already has read access on universe
...
box.space._priv:select{id}
---
@@ -738,7 +738,7 @@ box.schema.user.grant('guest', 'read,write,execute', 'universe')
...
box.schema.user.grant('guest', 'read,write,execute', 'universe')
---
-- error: User 'guest' already has read,write,execute access on universe ''
+- error: User 'guest' already has read,write,execute access on universe
...
box.schema.user.grant('guest', 'read,write,execute', 'universe', '', { if_not_exists = true })
---
@@ -2108,3 +2108,29 @@ box.space._priv:delete{1, 'universe', 0}
---
- error: 'Incorrect grant arguments: can''t revoke universe from the admin user'
...
+--
+-- gh-714: box.schema.user.grant error should be versatile,
+-- i.e. error on universally granted privileges shouldn't
+-- include any redundant details and/or symbols
+--
+box.schema.user.grant('guest', 'read,write,execute', 'universe')
+---
+...
+box.schema.user.grant('guest', 'read,write,execute', 'universe')
+---
+- error: User 'guest' already has read,write,execute access on universe
+...
+-- expected behavior of grant() error shouldn't change otherwise
+sp = box.schema.create_space('not_universe')
+---
+...
+box.schema.user.grant('guest', 'read,write,execute', 'space', 'not_universe')
+---
+...
+box.schema.user.grant('guest', 'read,write,execute', 'space', 'not_universe')
+---
+- error: User 'guest' already has read,write,execute access on space 'not_universe'
+...
+sp:drop()
+---
+...
diff --git a/test/box/access.test.lua b/test/box/access.test.lua
index 759827721..46373b71a 100644
--- a/test/box/access.test.lua
+++ b/test/box/access.test.lua
@@ -806,3 +806,17 @@ box.schema.user.drop("user3")
-- instance could not bootstrap nor recovery.
--
box.space._priv:delete{1, 'universe', 0}
+
+--
+-- gh-714: box.schema.user.grant error should be versatile,
+-- i.e. error on universally granted privileges shouldn't
+-- include any redundant details and/or symbols
+--
+box.schema.user.grant('guest', 'read,write,execute', 'universe')
+box.schema.user.grant('guest', 'read,write,execute', 'universe')
+
+-- expected behavior of grant() error shouldn't change otherwise
+sp = box.schema.create_space('not_universe')
+box.schema.user.grant('guest', 'read,write,execute', 'space', 'not_universe')
+box.schema.user.grant('guest', 'read,write,execute', 'space', 'not_universe')
+sp:drop()
--
2.24.0
>Четверг, 20 февраля 2020, 13:52 +03:00 от Igor Munkin <imun at tarantool.org>:
>
>Masha,
>
>Thanks for the patch. It LGTM except the one minor comment below.
>
>On 19.02.20, Maria Khaydich wrote:
>>
>> Error message on granted privileges was not flexible and
>> did not distinguish between universal or any other priviliges
>> leaving either 'nil' or simply '' at the end.
>>
>> Closes #714
>> ----------------------------------------------------------------------
>> Issue:
>> https://github.com/tarantool/tarantool/issues/714
>> Branch:
>> https://github.com/tarantool/tarantool/tree/eljashm/gh-714-box-schema-user-grant-invalid-error
>>
>> src/box/errcode.h | 2 +-
>> src/box/lua/schema.lua | 3 +++
>> test/box/access.result | 30 ++++++++++++++++++++++++++++--
>> test/box/access.test.lua | 14 ++++++++++++++
>> 4 files changed, 46 insertions(+), 3 deletions(-)
>>
>
><snipped>
>
>> diff --git a/src/box/lua/schema.lua b/src/box/lua/schema.lua
>> index 50c96a335..228f8798a 100644
>> --- a/src/box/lua/schema.lua
>> +++ b/src/box/lua/schema.lua
>> @@ -2408,6 +2408,9 @@ local function grant(uid, name, privilege, object_type,
>> privilege == 'execute' then
>> box.error(box.error.ROLE_GRANTED, name, object_name)
>> else
>> + if object_type ~= 'universe' then
>> + object_name = ' \''..object_name..'\''
>
>Minor: the way you build the resulting error string seems to be
>inconvenient to me. Please consider the following and choose the one
>you guess fits more:
>| object_name = " '" .. object_name .. "'"
>or
>| object name = string.format(" '%s'", object_name)
>
>> + end
>> box.error(box.error.PRIV_GRANTED, name, privilege,
>> object_type, object_name)
>> end
>>
>
><snipped>
>
>> --
>> 2.24.0
>>
>> --
>> Maria Khaydich
>
>--
>Best regards,
>IM
--
Maria Khaydich
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.tarantool.org/pipermail/tarantool-patches/attachments/20200220/ef48acd5/attachment.html>
More information about the Tarantool-patches
mailing list