Jeff Davis wrote:
> GRANT "postgres" TO "test_role" GRANTED BY "";
>
> We either need to rethink the way we handle grantor information and when it's valid.
> Or we need to at least allow dump/restore to work as expected when a dropped role
> granted privileges to other users.
I've been staring at this for a while. Upon first reading it, I thought
that it would be simply a matter of adding pg_shdepend entries for the
pg_auth_members rows. This starts sounding suspicious the moment you
consider that there will be one pg_shdepend entry for each role granted,
that is, a lot.
The second problem with this idea is that it's not at all possible,
because pg_shdepend entries can reference an object by OID, but
pg_auth_members rows don't have OIDs. So the most we could do is add
entries for pg_authid.
So I'm currently considering the following alternatives:
1. do nothing at all with pg_shdepend. Upon role deletion, seqscan
pg_auth_members and reject the drop altogether if there is a role
granted to another which mentions the to-be-dropped role ID as grantor.
This is easiest in terms of code (it's even mentioned in the comments in
DropRole).
2. record one pg_shdepend entry for each role that has granted something
to each role (unless the grantor is the same role being granted, in
which case we needn't record anything). So if role A grants Z and X to
C, and role B grants Y and W to C, C now has access to W, Y, X and Z and
there are two pg_shdepend entries:
C -> A
C -> B
So dropping a role would be disallowed automatically without any code
changes, with the checkSharedDependencies() call that's already in
DropRole. Adding a role membership would require a bit more work,
because we'd first need to check that there's not already a pg_shdepend
entry for that combination. Removing a role membership also becomes
more work; we need to check that no other grant depends on the same
grantor before removing the entry.
Note that I'm considering that this alternative requires adding a
GRANTOR symbol to the SharedDependencyType, which probably rules this
out for backpatching.
Comments? I'm leaning towards implementing (2). The patch for
pg_dumpall would also be needed.
--
Alvaro Herrera http://www.CommandPrompt.com/
The PostgreSQL Company - Command Prompt, Inc.
