On Mon, Jun 19, 2023 at 02:55:34PM -0700, Nathan Bossart wrote:
> In v4 of the patch set, I moved the skip_privs flag refactoring to 0001. I
> intend to commit this tomorrow unless there is additional feedback.
Fine by me. 0001 looks OK seen from here.
> These object_ownercheck() calls were removed because they were redundant,
> as owners have all privileges by default. Privileges can be revoked from
> the owner, so an extra ownership check would effectively bypass the
> relation's ACL in that case. I looked around and didn't see any other
> examples of a combined ownership and ACL check like we were doing for
> MAINTAIN. The only thing that gives me pause is that the docs call out
> ownership as sufficient for some maintenance commands. With these patches,
> that's only true as long as no one revokes privileges from the owner. IMO
> we should update the docs and leave out the ownership checks since MAINTAIN
> is now a grantable privilege like any other. WDYT?
TBH, I have a mixed feeling about this line of reasoning because
MAINTAIN is much broader and less specific than TRUNCATE, for
instance, being spawned across so much more operations. As you say,
owners of a relation have the MAINTAIN right by default, but they
would not be able to run any maintenance operations if somebody has
revoked their MAINTAIN right to do so, even if they are the owners of
the so-said relation. Perhaps that's OK in the long run, still I have
mixed feeling about whether that's the best decision we can take here,
especially because MAINTAIN impacts VACUUM, ANALYZE, CLUSTER, REFRESH
MATVIEW, REINDEX and LOCK. Some users may find that surprising as they
used to have more control over these operations as owners of the
relations worked on.
--
Michael