maria-developers team mailing list archive

[Sergei Golubchik <serg@xxxxxxxxxxx>]

Hi, Anel!

On Oct 21, Anel Husakovic wrote:
> revision-id: 6729bf5c031 (mariadb-10.1.43-112-g6729bf5c031)
> parent(s): ad4b70562bb
> author: Anel Husakovic <anel@xxxxxxxxxxx>
> committer: Anel Husakovic <anel@xxxxxxxxxxx>
> 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<char*>(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@xxxxxxxxxxx