Date: Wed, 14 Jun 2023 21:10:44 -0700
From: Nathan Bossart <nathandbossart@gmail.com>
To: Michael Paquier <michael@paquier.xyz>
Cc: Jeff Davis <pgsql@j-davis.com>, Ted Yu <yuzhihong@gmail.com>,
	Pavel Luzanov <p.luzanov@postgrespro.ru>,
	Justin Pryzby <pryzby@telsasoft.com>, pgsql-hackers@postgresql.org
Subject: Re: allow granting CLUSTER, REFRESH MATERIALIZED VIEW, and REINDEX
Message-ID: <20230615041044.GA736001@nathanxps13>
References: <7d2a8b72e23c8236281c6e00ae790327f965a8e5.camel@j-davis.com>
 <20230113203334.GA2206335@nathanxps13>
 <e25c84fafae4eb08d8a6e83207678096bac102d6.camel@j-davis.com>
 <20230113225626.GA2380176@nathanxps13>
 <20230113231339.GA2422750@nathanxps13>
 <20230613211246.GA219055@nathanxps13>
 <ZIj4v1CwqlDVJZfB@paquier.xyz>
 <20230613235442.GA222795@nathanxps13>
 <20230614181711.GA488295@nathanxps13>
 <ZIpfack6qJUHaw+g@paquier.xyz>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <ZIpfack6qJUHaw+g@paquier.xyz>
Archived-At: 
 <https://www.postgresql.org/message-id/20230615041044.GA736001%40nathanxps13>
Precedence: bulk

On Thu, Jun 15, 2023 at 09:46:33AM +0900, Michael Paquier wrote:
> The result after 0001 is applied is that a couple of
> object_ownercheck() calls that existed before ff9618e are removed from
> some ACL checks in the REINDEX, CLUSTER and VACUUM paths.  Is that OK
> for shared relations and shouldn't cluster_is_permitted_for_relation()
> include that?  vacuum_is_permitted_for_relation() is consistent on
> this side.

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?

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