maria-developers team mailing list archive

[Sergei Golubchik <serg@xxxxxxxxxxx>]

Hi, Vicentiu,

On Sep 12, Vicentiu Ciorbaru wrote:
> revision-id: f5a8327d016 (mariadb-10.3.36-26-gf5a8327d016)
> parent(s): fa75057b238
> author: Vicențiu Ciorbaru
> committer: Vicențiu Ciorbaru
> timestamp: 2022-09-05 16:40:10 +0300
> message:
> 
> MDEV-29458: Role grant commands do not propagate all grants
> 
> diff --git a/sql/sql_acl.cc b/sql/sql_acl.cc
> index ba457083b75..3806fb31aa6 100644
> --- a/sql/sql_acl.cc
> +++ b/sql/sql_acl.cc
> @@ -2741,7 +2741,9 @@ static void acl_update_role(const char *rolename, ulong privileges)
>  {
>    ACL_ROLE *role= find_acl_role(rolename);
>    if (role)
> +  {
>      role->initial_role_access= role->access= privileges;
> +  }
>  }

I wish you put this and the tab-to-spaces change in a separate commit.
(even better - tab-to-spaces in a separate commit and this hunk - nowhere :)

> @@ -9720,6 +9725,14 @@ static int handle_grant_struct(enum enum_acl_lists struct_no, bool drop,
>          remove_ptr_from_dynarray(&grantee->role_grants, acl_role);
>        }
>  
> +      /* Remove all of the role_grants from this user. */
> +      while (acl_role->role_grants.elements)
> +        delete_dynamic_element(&acl_role->role_grants,
> +                               acl_role->role_grants.elements - 1);

this is a weird way of deleting all elements in the array. What's wrong
with

    delete_dynamic(&acl_role->role_grants);

?

> +
> +      /* all grants must be revoked from this role by now. propagate this */
> +      propagate_role_grants(acl_role, PRIVS_TO_MERGE::ALL);
> +
>        my_hash_delete(&acl_roles, (uchar*) acl_role);
>        DBUG_RETURN(1);
>      }
> 
Regards,
Sergei
VP of MariaDB Server Engineering
and security@xxxxxxxxxxx