Re: Missing warning on revokes with grant options

On Mon, May 15, 2023 at 11:23:22PM -0400, Joseph Koshakow wrote:
> Reading through the docs [0], I'm not actually sure if the REVOKE
> in the second example should succeed or not. At first it says:
>
>> A user can only revoke privileges that were granted directly by that
>> user. If, for example, user A has granted a privilege with grant
>> option to user B, and user B has in turn granted it to user C, then
>> user A cannot revoke the privilege directly from C.
>
> Which seems pretty clear that you can only revoke privileges that you
> directly granted. However later on it says:
>
>> As long as some privilege is available, the command will proceed, but
>>it will revoke only those privileges for which the user has grant
>> options.
> ...
>> while the other forms will issue a warning if grant options for any
>> of the privileges specifically named in the command are not held.
>
> Which seems to imply that you can revoke a privilege as long as you
> have a grant option on that privilege.

I believe the "can only revoke privileges that were granted directly by
that user" rule still applies. However, I can see how the section about
non-owners attempting to revoke privileges might cause confusion about
this. The text in question has been around since 2004 (4b2dafc) and might
be worth revisiting.

IMO the most confusing part is that the warnings won't appear if you have
the grant option on the privilege in question but aren't the grantor. My
(possibly naive) expectation would be that you'd see warnings when a
privilege cannot be revoked because you are not the grantor.

> Either way I think the REVOKE should either fail and emit a warning
> OR succeed and emit no warning.

The thread for the aforementioned change [0] mentions the standard quite a
bit, which might explain the current behavior.

> I wasn't able to locate where the check for
>> A user can only revoke privileges that were granted directly by that
>> user.
> is in the code, but we should probably just add a warning there.

І'm not certain, but I suspect the calls to aclupdate() in
merge_acl_with_grant() take care of this because the grantors will never
match.

[0] https://postgr.es/m/20040511091816.E9887CF519E%40www.postgresql.com

--
Nathan Bossart
Amazon Web Services: https://aws.amazon.com