On Thu, Jul 28, 2022 at 12:09 PM Robert Haas <robertmh...@gmail.com> wrote:
> On Tue, Jul 26, 2022 at 12:46 PM Robert Haas <robertmh...@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.