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