Re: [Maria-developers] 6729bf5c031: MDEV-22313: Neither SHOW CREATE USER nor SHOW GRANTS prints a user's default role

Hi, Anel! On Oct 21, Anel Husakovic wrote:

revision-id: 6729bf5c031 (mariadb-10.1.43-112-g6729bf5c031) parent(s): ad4b70562bb author: Anel Husakovic committer: Anel Husakovic timestamp: 2020-04-22 20:48:55 +0200 message:

MDEV-22313: Neither SHOW CREATE USER nor SHOW GRANTS prints a user's default role

diff --git a/mysql-test/t/grant5.test b/mysql-test/t/grant5.test index 649bba7d1ca..5eefd9b9185 100644 --- a/mysql-test/t/grant5.test +++ b/mysql-test/t/grant5.test @@ -33,3 +33,16 @@ REVOKE EXECUTE ON PROCEDURE sp FROM u; --error ER_TABLE_NOT_LOCKED REVOKE PROCESS ON *.* FROM u; DROP TABLE t1; + +# +# MDEV-22313: Neither SHOW CREATE USER nor SHOW GRANTS prints a user's default role +# +CREATE ROLE test_role; +CREATE USER test_user; +GRANT test_role TO test_user; +SET DEFAULT ROLE test_role FOR test_user; +SHOW GRANTS FOR test_user; +SET DEFAULT ROLE NONE for test_user; +SHOW GRANTS FOR test_user;

add a test for SHOW GRANTS without FOR username

+DROP USER test_user; +DROP ROLE test_role; diff --git a/sql/sql_acl.cc b/sql/sql_acl.cc index b6a6f806e50..90dd552ec43 100644 --- a/sql/sql_acl.cc @@ -7891,6 +7893,10 @@ bool mysql_show_grants(THD *thd, LEX_USER *lex_user) if (show_role_grants(thd, username, hostname, acl_user, buff, sizeof(buff))) goto end;

+ /* Show default role to acl_user */ + if (show_default_role(thd, username, hostname, acl_user, buff, sizeof(buff))) + goto end;

Please, print SET DEFAULT ROLE after all grants for this user.

+ /* Add first global access grants */ if (show_global_privileges(thd, acl_user, FALSE, buff, sizeof(buff))) goto end; @@ -7963,6 +7969,42 @@ static ROLE_GRANT_PAIR *find_role_grant_pair(const LEX_STRING *u, my_hash_search(&acl_roles_mappings, (uchar*)pair_key.ptr(), key_length); }

+static bool show_default_role(THD *thd, const char *username, + const char *hostname, ACL_USER_BASE *acl_entry,

why did you cast acl_entry (which you know is ACL_USER) to the base class ACL_USER_BASE? why did you pass the username if you don't use it?

+ char *buff, size_t buffsize) +{ + Protocol *protocol= thd->protocol; + LEX_STRING host= {const_cast(hostname), strlen(hostname)};

This doesn't make much sense. You only create this LEX_STRING to use it for String::append(LEX_STRING *). But you can directly use String::append(const char*) and you won't need any LEX_STRING here. Furthermore, you can print acl_entry->host.hostname just as you print acl_entry->user, so you don't need the above hostname argument at all.

+ String def_role(buff,sizeof(buff),system_charset_info); + def_role.length(0);

you don't use def_role anywhere.

+ LEX_STRING def_rolename= ((ACL_USER *)acl_entry)->default_rolename; + if (def_rolename.length) + { + String def_str(buff,sizeof(buff),system_charset_info); + def_str.length(0); + def_str.append("SET DEFAULT ROLE ");

prefer to use STRING_WITH_LEN() for string literals. E.g. here it should be def_str.append(STRING_WITH_LEN("SET DEFAULT ROLE "));

+ def_str.append(def_rolename.str, def_rolename.length, + system_charset_info);

here you can do def_str.append(&def_rolename);

+ def_str.append(" FOR '"); + def_str.append(acl_entry->user.str, acl_entry->user.length, + system_charset_info); + if (!(acl_entry->flags & IS_ROLE))

better put an assert here not an if

+ { + def_str.append(STRING_WITH_LEN("'@'")); + def_str.append(&host); + } + def_str.append('\''); + protocol->prepare_for_resend(); + protocol->store(def_str.ptr(),def_str.length(),def_str.charset()); + if (protocol->write()) + { + return TRUE; + } + } + return FALSE; +} + static bool show_role_grants(THD *thd, const char *username, const char *hostname, ACL_USER_BASE *acl_entry, char *buff, size_t buffsize)

Regards, Sergei VP of MariaDB Server Engineering and security@mariadb.org