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@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