maria-developers team mailing list archive

[Sergei Golubchik <serg@xxxxxxxxxxx>]

Hi, Anel!

Looks good, thanks!
A couple of small comments below.

On Oct 23, Anel Husakovic wrote:
> revision-id: 581857bd999 (mariadb-10.1.43-314-g581857bd999)

let's fix it in 10.2 and up, we're doing the very last 10.1 release, so
better be extra-super-conservative about it.

> parent(s): 43ec9370b32
> author: Anel Husakovic <anel@xxxxxxxxxxx>
> committer: Anel Husakovic <anel@xxxxxxxxxxx>
> timestamp: 2020-10-23 00:20:53 +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 74a69952124..bf97a930097 100644
> --- a/mysql-test/t/grant5.test
> +++ b/mysql-test/t/grant5.test
> @@ -52,6 +52,23 @@ disconnect u1;
>  drop user u1@localhost;
>  drop database mysqltest1;
>  
> +#
> +# 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;
> +SET DEFAULT ROLE test_role;

oops, sorry, my mistake.
Please, add `SET ROLE test_role`
before SHOW GRANTS.
The goal is to see that SET DEFAULT ROLE is shown *after* all grants
to root@localhost, but before any grants to test_role.

> +SHOW GRANTS;
> +SET DEFAULT ROLE NONE;
> +SHOW GRANTS;
> +DROP USER test_user;
> +DROP ROLE test_role;
> +
>  #
>  # End of 10.1 tests
>  #
> diff --git a/sql/sql_acl.cc b/sql/sql_acl.cc
> index cf0b1d87bd7..6c852976061 100644
> --- a/sql/sql_acl.cc
> +++ b/sql/sql_acl.cc
> @@ -8001,15 +8007,44 @@ 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 *hostname,
> +                              ACL_USER *acl_entry, char *buff, size_t buffsize)
> +{
> +  Protocol *protocol= thd->protocol;
> +  LEX_STRING def_rolename= acl_entry->default_rolename;
> +
> +  if (def_rolename.length)
> +  {
> +    String def_str(buff, buffsize, system_charset_info);
> +    def_str.length(0);
> +    def_str.append(STRING_WITH_LEN("SET DEFAULT ROLE "));
> +    def_str.append(&def_rolename);
> +    def_str.append(" FOR '");
> +    def_str.append(&acl_entry->user);
> +    DBUG_ASSERT(!(acl_entry->flags & IS_ROLE));
> +    def_str.append(STRING_WITH_LEN("'@'"));
> +    def_str.append(acl_entry->host.hostname, acl_entry->hostname_length,
> +                   system_charset_info);
> +    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;

looks good. The hostname argument seems to be unused now, doesn't it?

> +}
> +
> -static bool show_role_grants(THD *thd, const char *username,
> -                             const char *hostname, ACL_USER_BASE *acl_entry,
> +static bool show_role_grants(THD *thd, const char *hostname,
> +                             ACL_USER_BASE *acl_entry,
>                               char *buff, size_t buffsize)
>  {
>    uint counter;
>    Protocol *protocol= thd->protocol;
>    LEX_STRING host= {const_cast<char*>(hostname), strlen(hostname)};
>  
> -  String grant(buff,sizeof(buff),system_charset_info);
> +  String grant(buff, buffsize, system_charset_info);

ouch, indeed. thanks!

>    for (counter= 0; counter < acl_entry->role_grants.elements; counter++)
>    {
>      grant.length(0);

Regards,
Sergei
VP of MariaDB Server Engineering
and security@xxxxxxxxxxx