Re: pg_auth_members.grantor is bunk

On Thu, Jul 28, 2022 at 12:09 PM Robert Haas <robertmhaas(at)gmail(dot)com> wrote:

> On Tue, Jul 26, 2022 at 12:46 PM Robert Haas <robertmhaas(at)gmail(dot)com>
> wrote:
> > I believe that these patches are mostly complete, but I think that
> > dumpRoleMembership() probably needs some more work. I don't know what
> > exactly, but there's nothing to cause it to dump the role grants in an
> > order that will create dependent grants after the things that they
> > depend on, which seems essential.
>
> OK, so I fixed that, and also updated the documentation a bit more. I
> think these patches are basically done, and I'd like to get them
> committed before too much more time goes by, because I have other
> things that depend on this which I also want to get done for this
> release. Anybody object?
>
> I'm hoping not, because, while this is a behavior change, the current
> state of play in this area is just terrible. To my knowledge, this is
> the only place in the system where we allow a dangling OID reference
> in a catalog table to persist after the object to which it refers has
> been dropped. I believe it's also the object type where multiple
> grants by different grantors aren't tracked separately, and where the
> grantor need not themselves have the permission being granted. It
> doesn't really look like any of these things were intentional behavior
> so much as just ... nobody ever bothered to write the code to make it
> work properly. I'm hoping the fact that I have now done that will be
> viewed as a good thing, but maybe that won't turn out to be the case.
>
>
I suggest changing \du memberof to output something like this:

select r.rolname,
array(
select format('%s:%s/%s',
b.rolname,
case when m.admin_option then 'admin' else 'member' end,
g.rolname)
from pg_catalog.pg_auth_members m
join pg_catalog.pg_roles b on (m.roleid = b.oid)
join pg_catalog.pg_roles g on (m.grantor = g.oid)
where m.member = r.oid
) as memberof
from pg_catalog.pg_roles r where r.rolname !~ '^pg_';

rolname | memberof
---------+------------------------------------
vagrant | {}
o | {}
a | {o:admin/p,o:admin/vagrant}
x | {o:admin/a,p:member/vagrant}
b | {o:admin/a}
p | {o:admin/vagrant}
y | {x:member/vagrant}
q | {}
r | {q:admin/vagrant}
s | {}
t | {q:admin/vagrant,s:member/vagrant}

(needs sorting, tried to model it after ACL - column privileges
specifically)

=> \dp mytable
Access privileges
Schema | Name | Type | Access privileges | Column privileges |
Policies
--------+---------+-------+-----------------------+-----------------------+----------
public | mytable | table | miriam=arwdDxt/miriam+| col1: +|
| | | =r/miriam +| miriam_rw=rw/miriam |
| | | admin=arw/miriam | |
(1 row)

If we aren't dead set on having \du and \dg be aliases for each other I'd
rather redesign \dg (or add a new meta-command) to be a group-centric view
of this exact same data instead of user-centric one. Namely it has a
"members" column instead of "memberof" and have it output, one line per
member:

user=[admin|member]/grantor

I looked over the rest of the patch and played with the circularity a bit,
which motivated the expanded info in \du, and the confirmation that two
separate admin grants that are not circular can exist.

I don't have any meaningful insight as to breaking things with these
changes but I am strongly in favor of tightening this up and formalizing it.

David J.