Re: pg_auth_members.grantor is bunk - Mailing list pgsql-hackers

From David G. Johnston
Subject Re: pg_auth_members.grantor is bunk
Date
Msg-id CAKFQuwbJcWQCMVf_aBrSSqp13uJEqm=tuJvL82X13t9=wYm-Xg@mail.gmail.com
Whole thread Raw
In response to Re: pg_auth_members.grantor is bunk  (Robert Haas <robertmhaas@gmail.com>)
Responses Re: pg_auth_members.grantor is bunk
List pgsql-hackers
On Thu, Jul 28, 2022 at 12:09 PM Robert Haas <robertmhaas@gmail.com> wrote:
On Tue, Jul 26, 2022 at 12:46 PM Robert Haas <robertmhaas@gmail.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.

pgsql-hackers by date:

Previous
From: Tom Lane
Date:
Subject: Re: Inconvenience of pg_read_binary_file()
Next
From: Jacob Champion
Date:
Subject: [Commitfest 2022-07] Patch Triage: Needs Review, Part 1