Re: pg_auth_members.grantor is bunk

On Mon, 2022-08-22 at 11:47 -0400, Robert Haas wrote:
> On Thu, Aug 18, 2022 at 1:26 PM Robert Haas <robertmhaas(at)gmail(dot)com>
> wrote:
> > On Wed, Aug 10, 2022 at 4:28 PM Robert Haas <robertmhaas(at)gmail(dot)com>
> > wrote:
> > > Well, CI isn't happy with this, and for good reason:
> >
> > CI is happier with this version, so I've committed 0001. If no
> > major
> > problems emerge, I'll proceed with 0002 as well.
>
> Done.

It's still on the CF, so I took a look.

There's still some weirdness around superusers:

1. "GRANTED BY current_user" differs from not specifying "GRANTED BY"
at all.

a. With GRANTED BY current_user, weird because current_user is a
superuser:

CREATE USER su1 SUPERUSER;
CREATE ROLE u1;
CREATE ROLE u2;
\c - su1
GRANT u2 TO u1 GRANTED BY current_user;
ERROR: grantor must have ADMIN OPTION on "u2"

b. Without GRANTED BY:

CREATE USER su1 SUPERUSER;
CREATE ROLE u1;
CREATE ROLE u2;
\c - su1
GRANT u2 TO u1;
-- grantor is bootstrap superuser

2. Grantor can depend on the path to get there:

a. Already superuser:

CREATE USER su1 SUPERUSER;
CREATE ROLE u1;
CREATE ROLE u2;
GRANT u2 TO su1 WITH ADMIN OPTION;
\c - su1
GRANT u2 TO u1;
-- grantor is bootstrap superuser

b. Becomes superuser after GRANT:

CREATE USER su1;
CREATE ROLE u1;
CREATE ROLE u2;
GRANT u2 TO su1 WITH ADMIN OPTION;
\c - su1
GRANT u2 TO u1;
\c - bootstrap_superuser
ALTER ROLE su1 SUPERUSER;
-- grantor is su1

3. Another case where "GRANTED BY current_user" differs from no
"GRANTED BY" at all, with slightly different consequences:

a. GRANTED BY current_user, throws error:

 CREATE USER su1 SUPERUSER;
CREATE ROLE u1;
CREATE ROLE u2;
GRANT u2 TO su1 WITH ADMIN OPTION;
\c - su1
GRANT u2 TO u1 GRANTED BY current_user;
-- grantor is su1
\c - bootstrap_superuser
REVOKE ADMIN OPTION FOR u2 FROM su1;
ERROR: dependent privileges exist

b. No GRANTED BY, no error:

CREATE USER su1 SUPERUSER;
CREATE ROLE u1;
CREATE ROLE u2;
GRANT u2 TO su1 WITH ADMIN OPTION;
\c - su1
GRANT u2 TO u1;
-- grantor is bootstrap superuser
\c - boostrap_superuser
REVOKE ADMIN OPTION FOR u2 FROM su1;

We seem to be trying very hard to satisfy two things that seem
impossible to satisfy:

i. "ALTER ROLE ... NOSUPERUSER" must always succeed, and probably
execute quickly, too.
ii. We want to maintain catalog invariants that are based, in part,
on roles having superuser privileges or not.

The hacks we are using to try to make this work are just that: hacks.
And it's all to satisfy a fairly rare case: removing superuser
privileges and expecting the catalogs to be consistent.

I think we'd be better off without these hacks. I'm not sure exactly
how, but the benefit doesn't seem to be worth the cost. Some
alternative ideas:

* Have a "safe" version of removing superuser that can error or
cascade, and an "unsafe" version that always succeeds but might leave
inconsistent catalogs.
* Ignore the problems with removing superuser, but issue a WARNING
* Superusers would auto-grant themselves the privileges that a normal
user would need to do something before doing it. For instance, if a
superuser did "GRANT u2 TO u1", it would first automatically issue a
"GRANT u2 TO current_user WITH ADMIN OPTION GRANTED BY
bootstrap_superuser", then do the grant normally. If the superuser
privileges are removed, then the catalogs would still be consistent.
This is a new idea and I didn't think it through very carefully, but
might be an interesting approach.

Also, it would be nice to have REASSIGN OWNED work with grants, perhaps
by adding a "WITH[OUT] GRANT" or something.

Regards,
Jeff Davis