Re: pg_auth_members.grantor is bunk

On Tue, Sep 6, 2022 at 7:26 PM Jeff Davis <pgsql(at)j-davis(dot)com> wrote:
> There's at least one other difference: if you specify "GRANTED BY su1"
> for a table grant, it still selects the table owner as the grantor;
> whereas if you specify "GRANTED BY su1" for a role grant, it selects
> "su1".

Right. Personally, I'm inclined to view that as a defect in the
"GRANTED BY whoever" implementation for other object types, and I
think it should be resolved by making other object types error out if
the user explicitly mentioned in the "GRANTED BY" clause isn't a valid
grantor. It also seems possible to view it as a defect in the new
implementation, and argue that inference should always be performed
starting at the named user. I find that a POLA violation, but someone
could disagree.

Parenthetically, I think we should also fix GRANTED BY for other
object types so that it actually works, but that is a bit of headache
because it doesn't seem like that code is relying as heavily on common
infrastructure as some things, so I believe it's actually a fair
amount of work to make that happen.

> In other words, try to issue the grant normally if at all possible, and
> play the superuser card as a last resort. I believe that will lead to
> the fewest surprising cases, and make them easiest to explain, because
> superuser-ness doesn't influence the outcome in as many cases.

It seems to me that this policy would reverse select_best_grantor()'s
decision about whether we should prefer to rely on superuser
privileges or on privileges actually granted to the current user. I
think either behavior is defensible, but the existing precedent is to
prefer relying on superuser privileges. Like you, I found that a bit
weird when I realized that's what it was doing, but it does have some
advantages. In particular, it means that the privileges granted by a
superuser don't depend on any other grants, which is something that a
user might value.

Now that is not to say that we couldn't decide that
select_best_grantor() got it wrong and choose to break backward
compatibility in order to fix it ... but I'm not even convinced that
the alternative behavior you propose is clearly better, let alone that
it's enough better to justify changing things. However, I don't
personally have a strong preference about it one way or the other; if
there's a strong consensus to change it, so be it.

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