Thread: Superuser can't revoke role granted by non-superuser

On Mon, 27 Jan 2025 at 13:49, Alexander Kukushkin <cyberdemn@gmail.com> wrote:
>
> Hi,
>
> Here is a self-contained example with 17.2, however I assume that 16 and master will exhibit similar behaviour.
>
> postgres=# create user a with createrole;
> CREATE ROLE
> postgres=# create user b with createrole;
> CREATE ROLE
> postgres=# set role a;
> SET
> postgres=> create user aa;
> CREATE ROLE
> postgres=> set role b;
> SET
> postgres=> create user bb;
> CREATE ROLE
> postgres=> grant bb to aa;
> GRANT ROLE
> postgres=> \drg
>                List of role grants
>  Role name | Member of |   Options    | Grantor
> -----------+-----------+--------------+----------
>  a         | aa        | ADMIN        | postgres
>  aa        | bb        | INHERIT, SET | b
>  b         | bb        | ADMIN        | postgres
> (3 rows)
>
> postgres=> reset role;
> RESET
> postgres=# revoke bb from aa;
> WARNING:  role "aa" has not been granted membership in role "bb" by role "postgres"
> REVOKE ROLE
> postgres=# \drg
>                List of role grants
>  Role name | Member of |   Options    | Grantor
> -----------+-----------+--------------+----------
>  a         | aa        | ADMIN        | postgres
>  aa        | bb        | INHERIT, SET | b
>  b         | bb        | ADMIN        | postgres
> (3 rows)
>
> IMO, superusers should be able to revoke privileges it didn't grant.
>
> Regards,
> --
> Alexander Kukushkin

Reproduced this at cf5eb37 (and not on its parent f026c16)
There was some huge refactoring around user.c and particularly
`check_role_grantor` function. I'm trying to comprehend.

-- 
Best regards,
Kirill Reshke

On Mon, 27 Jan 2025 at 14:23, Alexander Kukushkin <cyberdemn@gmail.com> wrote:
>
>
> On Mon, 27 Jan 2025 at 10:20, Kirill Reshke <reshkekirill@gmail.com> wrote:
>>
>> Reproduced this at cf5eb37 (and not on its parent f026c16)
>> There was some huge refactoring around user.c and particularly
>> `check_role_grantor` function. I'm trying to comprehend.
>
>
> I think the fix should look like:
> diff --git a/src/backend/commands/user.c b/src/backend/commands/user.c
> index 02824c32a49..29948d692b6 100644
> --- a/src/backend/commands/user.c
> +++ b/src/backend/commands/user.c
> @@ -2342,7 +2342,8 @@ plan_single_revoke(CatCList *memlist, RevokeRoleGrantAction *actions,
>                 authmem_form = (Form_pg_auth_members) GETSTRUCT(authmem_tuple);
>
>                 if (authmem_form->member == member &&
> -                       authmem_form->grantor == grantor)
> +                       (authmem_form->grantor == grantor ||
> +                        grantor == BOOTSTRAP_SUPERUSERID))
>                 {
>                         if ((popt->specified & GRANT_ROLE_SPECIFIED_INHERIT) != 0)
>                         {
>
I doubt this is a correct fix. The difference between cf5eb37 &
f026c16 behaviour is in who granted membership in role 'bb' to role
'aa'. In the case of f026c16 the role is 'b', while after it is
bootstrap superuser. Is this correct? If yes, why should we consider
BOOTSTRAP_SUPERUSERID in this if statement? Maybe there are some other
cases from which this will not guard?

-- 
Best regards,
Kirill Reshke

Alexander Kukushkin <cyberdemn@gmail.com> writes:
> E.g. superuser is supposed to use "revoke bb from aa granted by b"

Exactly.  This is not a bug, you just did not correctly name the
privilege you're trying to revoke.

If memory serves, the default behavior for a superuser is that
we'll revoke the privilege as granted by the object owner, since
that's the most common case.  For everyone else, the default
assumption about "granted by" is "yourself".  Some edge cases
about this might have changed in v16, not sure.

            regards, tom lane