Re: pg_auth_members.grantor is bunk

On Mon, Aug 1, 2022 at 3:51 PM Robert Haas <robertmhaas@gmail.com> wrote:
> On Mon, Aug 1, 2022 at 1:38 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
> >> I think the latter --- the cfbot thinks the July CF is no longer relevant,
> > but Jacob hasn't yet moved your patches forward.  You could wait for
> > him to do that, or do it yourself.
>
> Done. New patches attached.

Well, CI isn't happy with this, and for good reason:

 ALTER GROUP regress_priv_group2 ADD USER regress_priv_user2; -- duplicate
-NOTICE:  role "regress_priv_user2" has already been granted
membership in role "regress_priv_group2" by role "rhaas"
+NOTICE:  role "regress_priv_user2" has already been granted
membership in role "regress_priv_group2" by role "postgres"

The problem here is that I revised the error message to include the
name of the grantor, since that's now a part of the identity of the
grant. It would be misleading to say, as we did previously...

NOTICE:  role "regress_priv_user2" is already a member of role
"regress_priv_group2"

...because them being in the group isn't relevant so much as them
being in the group by means of the same grantor. However, I suspect
that I can't persuade all of you that we should hard-code the name of
the bootstrap superuser as "rhaas", so this test case needs some
alteration. I found, however, that the original intent of the test
case couldn't be preserved with the patch as written, because when you
grant membership in one role to another role as the superuser or a
CREATEROLE user, the grant is attributed to the bootstrap superuser,
whose name is variable, as this test failure shows. Therefore, to fix
the test, I needed to use ALTER GROUP as a non-CREATEROLE user, some
user created as part of the test, for the results to be stable. But
that was impossible, because even though "GRANT user_name TO
group_name" requires *either* CREATEROLE *or* ADMIN OPTION on the
group, the equivalent command "ALTER GROUP group_name ADD USER
user_name" requires specifically CREATEROLE.

I debated whether to fix that inconsistency or just remove this test
case and eventually came down on the side of fixing the inconsistency,
so the attached version does it that way.

--
Robert Haas
EDB: http://www.enterprisedb.com