On Wed, Mar 12, 2025 at 12:10:30PM +0530, Ashutosh Sharma wrote:
> I think moving the check to the second pass won´t work in this case.
> The reason is that we rely on entries in the pg_auth_members table. By
> the time the check occurs in the second pass, the first pass will have
> already removed all entries related to the roles being dropped from
> pg_auth_members and incremented the command counter. As a result, when
> check_drop_role_dependency() is called in the second pass, it won´t
> find any entries in the table and won't be able to detect any ADMIN
> privilege-related dependencies.
Oh, good catch.
> So, we need to explore alternative approaches to handle this better.
> I´ll spend some more time today to investigate other possibilities for
> addressing this issue.
I think this approach has other problems. For example, even if a role has
admin directly on the dropped role, we'll block DROP ROLE if it also has
admin indirectly:
postgres=# create role a createrole;
CREATE ROLE
postgres=# set role a;
SET
postgres=> create role b createrole;
CREATE ROLE
postgres=> set role b;
SET
postgres=> create role c admin a;
CREATE ROLE
postgres=> reset role;
RESET
postgres=# drop role b;
ERROR: role "b" cannot be dropped because some objects depend on it
DETAIL: role a inherits ADMIN privileges on role c through role b
There are also other ways besides DROP ROLE that roles can end up without
any admins:
postgres=# create role a createrole;
CREATE ROLE
postgres=# set role a;
SET
postgres=> create role b createrole;
CREATE ROLE
postgres=> set role b;
SET
postgres=> create role c;
CREATE ROLE
postgres=> reset role;
RESET
postgres=# revoke c from b;
REVOKE ROLE
I wonder if we should adjust the requirements here to be "an operation
cannot result in a role with 0 admins." That does mean you might lose
indirect admin privileges, but I'm not sure that's sustainable, anyway.
For example, if a role has 2 admins, should we block REVOKE from one of the
admins because another role inherited its privileges? If nothing else, it
sounds like a complicated user experience.
If we just want to make sure that roles don't end up without admins, I
think we could just collect the set of roles losing an admin during DROP
ROLE, REVOKE, etc., and then once all removals have completed, we verify
that each of the collected roles has an admin.
--
nathan
