Re: Orphaned users in PG16 and above can only be managed by Superusers

Hi Nathan,

Thanks for the review comment.

On Mon, Mar 10, 2025 at 8:31 PM Nathan Bossart <nathandbossart(at)gmail(dot)com> wrote:
>
> On Mon, Mar 10, 2025 at 11:15:04AM +0530, Ashutosh Sharma wrote:
> > On Fri, Mar 7, 2025 at 10:55 PM Nathan Bossart <nathandbossart(at)gmail(dot)com> wrote:
> >> I noticed that much of this code is lifted from DropRole(), and the new
> >> check_drop_role_dependency() function is only used by DropRole() right
> >> before it does the exact same scans. Couldn't we put the new dependency
> >> detection in those existing scans in DropRole()?
> >
> > It can be done, but mixing the code that checks for the drop role
> > dependency with the code that removes entries for the role being
> > dropped from pg_auth_members could reduce clarity and precision. This
> > is more of a sanity check which I felt was necessary before we proceed
> > with actually dropping the role, starting with the deletion of drop
> > role entries from the system catalogs. I’m aware there’s some code
> > duplication, but I think it should be fine.
>
> Looking closer, we probably need to move this check to the second pass,
> anyway:
>
> 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=# DROP ROLE b, c;
> ERROR: role "b" cannot be dropped because some objects depend on it
> DETAIL: role a inherits ADMIN privileges on role c through role b
> postgres=# DROP ROLE c, b;
> DROP ROLE
>
> The first DROP ROLE should probably succeed, if for no other reason than
> individually dropping role c followed by role b would succeed.
>

I initially thought this was fine because the sequence of dropping
roles matters. In the previous case (the first drop role statement),
the referenced role is dropped first, followed by the dependent role,
which causes the statement to fail. However, in the second statement,
the order is reversed, with the dependent role being dropped first,
allowing the referenced role to be dropped afterward, which makes the
statement succeed.

That said, I've realized this doesn't apply to the grantor role. If
both the grantor and grantee roles are dropped in a single statement,
the drop command succeeds regardless of the order. So, we need to
ensure that the same behavior is maintained here as well. I’ll address
this in the next patch version.

--
With Regards,
Ashutosh Sharma.