Thanks. It looks safe to me. On Thu, Mar 12, 2026 at 8:15 AM Dmitri Bourlatchkov <[email protected]> wrote:
> Hi All, > > Following up on this discussion, I opened PR [3991] to remove a couple of > unused PolarisAuthorizableOperation values. Please take a look. > > [3991] https://github.com/apache/polaris/pull/3991 > > Thanks, > Dmitri. > > On Mon, Mar 2, 2026 at 12:48 PM Dmitri Bourlatchkov <[email protected]> > wrote: > > > Hi Michael, > > > > Making catalog-level grant/revote permissions symmetrical makes sense to > > me. > > > > Re: PR 3906, the change looks good to me. I believe it still deserves > > mention in the CHANGELOG. > > > > I have a couple of side question, though: while browsing related code, I > > found that the following existing operations are not used in Apache > Polaris: > > * REVOKE_PRINCIPAL_GRANT_FROM_PRINCIPAL_ROLE > > * REVOKE_PRINCIPAL_ROLE_GRANT_FROM_PRINCIPAL_ROLE > > > > The operations also reference the > PRINCIPAL_ROLE_MANAGE_GRANTS_FOR_GRANTEE > > permission, but due to lack of their usage, reasoning about this change > is > > a bit complicated in the current codebase. > > > > Would it make sense to remove the unused operations (in a separate PR, of > > course)? > > > > The only remaining usage of PRINCIPAL_ROLE_MANAGE_GRANTS_FOR_GRANTEE > > appears to be in REVOKE_ROOT_GRANT_FROM_PRINCIPAL_ROLE, which is invoked > by > > PolarisAdminService.revokePrivilegeOnRootContainerFromPrincipalRole()... > > yet it does not appear to be called anywhere. WDYT? > > > > Is the REVOKE_ROOT_GRANT_FROM_PRINCIPAL_ROLE permission relevant anymore? > > > > Thanks, > > Dmitri. > > > > On Mon, Mar 2, 2026 at 12:19 PM Michael Collado <[email protected]> > > wrote: > > > >> Hey folks > >> > >> I wanted to raise awareness of a small change in the privilege model in > >> #3906. Currently, the catalog_admin role in a given catalog has > privileges > >> to grant a Catalog Role to any Principal Role. However, the > catalog_admin > >> role by itself is not enough to revoke that Catalog Role. Instead, the > >> privilege model requires the user has both the catalog_admin role and > also > >> the privilege to manage grants for principal roles (typically, the > >> service_admin). In effect, this means that the service_admin role has to > >> have catalog_admin privileges on every catalog or catalog roles can't be > >> revoked once they were granted. > >> > >> The change in my PR removes the requirement to manage grants on > >> the principale role so that the grant and revoke actions are symmetrical > >> and require the same privilege - CATALOG_MANAGE_ACCESS on the target > >> catalog. > >> > >> Unless there are objections, I'd like to merge this PR in the next > couple > >> of days. Please let me know if there are any concerns. > >> > >> https://github.com/apache/polaris/pull/3906 > >> > >> Mike > >> > > >
