This is an automated email from the ASF dual-hosted git repository.
collado pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/polaris.git
The following commit(s) were added to refs/heads/main by this push:
new ae1004bd72 Remove PRINCIPAL_ROLE_MANAGE_GRANTS_FOR_GRANTEE requirement
from REVOKE_CATALOG_ROLE_FROM_PRINCIPAL_ROLE (#3906)
ae1004bd72 is described below
commit ae1004bd72080eca297210e4bcc1f4ad23f81a0c
Author: Michael Collado <[email protected]>
AuthorDate: Tue Mar 10 14:05:08 2026 -0700
Remove PRINCIPAL_ROLE_MANAGE_GRANTS_FOR_GRANTEE requirement from
REVOKE_CATALOG_ROLE_FROM_PRINCIPAL_ROLE (#3906)
* Remove PRINCIPAL_ROLE_MANAGE_GRANTS_FOR_GRANTEE requirement from
REVOKE_CATALOG_ROLE_FROM_PRINCIPAL_ROLE
This change simplifies the authorization model for revoking catalog roles
from principal roles by removing the requirement to have privileges on the
grantee (principal role). After this change, only privileges on the
securable
(catalog role) are required, making revoke symmetric with the assign
operation.
This allows catalog administrators to fully manage catalog role assignments
without requiring elevated privileges on principal roles.
Changes:
- PolarisAuthorizableOperation: Remove
PRINCIPAL_ROLE_MANAGE_GRANTS_FOR_GRANTEE
from REVOKE_CATALOG_ROLE_FROM_PRINCIPAL_ROLE operation definition
- PolarisAdminServiceAuthzTest: Update test to validate only
CATALOG_ROLE_MANAGE_GRANTS_ON_SECURABLE privilege is required
- PolarisManagementServiceIntegrationTest: Update integration test to expect
catalog admin revoke to succeed and handle re-granting for service admin
test
Co-Authored-By: Claude Opus 4.6 <[email protected]>
* Add CHANGELOG entry for REVOKE_CATALOG_ROLE_FROM_PRINCIPAL_ROLE
authorization change
Co-Authored-By: Claude Opus 4.6 <[email protected]>
* Fix test name collisions in ManagementServiceIntegrationTest
Multiple tests were using the same entity name hints ("mypr33" and
"principal1"),
causing 409 CONFLICT errors when tests run in parallel. This change makes
each
test use unique entity names based on the test function name.
Fixed tests:
- testCatalogAdminGrantAndRevokeCatalogRoles
- testServiceAdminCanTransferCatalogAdmin
- testCatalogAdminGrantAndRevokeCatalogRolesFromWrongCatalog
- testTableManageAccessCanGrantAndRevokeFromCatalogRoles
Co-Authored-By: Claude Opus 4.6 <[email protected]>
* Fixed name collisions in integration test
---------
Co-authored-by: Claude Opus 4.6 <[email protected]>
---
CHANGELOG.md | 1 +
.../PolarisManagementServiceIntegrationTest.java | 65 ++++++++++++----------
.../core/auth/PolarisAuthorizableOperation.java | 3 +-
.../admin/PolarisAdminServiceAuthzTest.java | 13 +----
4 files changed, 41 insertions(+), 41 deletions(-)
diff --git a/CHANGELOG.md b/CHANGELOG.md
index c525dca1a5..5fa87f5ec3 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -43,6 +43,7 @@ request adding CHANGELOG notes for breaking (!) changes and
possibly other secti
- The `PolarisMetricsReporter.reportMetric()` method signature has been
extended to include a `receivedTimestamp` parameter of type `java.time.Instant`.
- The `ExternalCatalogFactory.createCatalog()` and `createGenericCatalog()`
method signatures have been extended to include a `catalogProperties` parameter
of type `Map<String, String>` for passing through proxy and timeout settings to
federated catalog HTTP clients.
- Metrics reporting now requires the `TABLE_READ_DATA` privilege on the target
table for read (scan) metrics and `TABLE_WRITE_DATA` for write (commit) metrics.
+- The `REVOKE_CATALOG_ROLE_FROM_PRINCIPAL_ROLE` operation no longer requires
the `PRINCIPAL_ROLE_MANAGE_GRANTS_FOR_GRANTEE` privilege. Only
`CATALOG_ROLE_MANAGE_GRANTS_ON_SECURABLE` on the catalog role is now required,
making revoke symmetric with assign. This allows catalog administrators to
fully manage catalog role assignments without requiring elevated privileges on
principal roles.
### New Features
diff --git
a/integration-tests/src/main/java/org/apache/polaris/service/it/test/PolarisManagementServiceIntegrationTest.java
b/integration-tests/src/main/java/org/apache/polaris/service/it/test/PolarisManagementServiceIntegrationTest.java
index cd3f7f15d8..822fd5c04d 100644
---
a/integration-tests/src/main/java/org/apache/polaris/service/it/test/PolarisManagementServiceIntegrationTest.java
+++
b/integration-tests/src/main/java/org/apache/polaris/service/it/test/PolarisManagementServiceIntegrationTest.java
@@ -1724,7 +1724,7 @@ public class PolarisManagementServiceIntegrationTest {
public void testCatalogAdminGrantAndRevokeCatalogRoles() {
// Create a PrincipalRole and a new catalog. Grant the catalog_admin role
to the new principal
// role
- String principalRoleName = client.newEntityName("mypr33");
+ String principalRoleName =
client.newEntityName("mypr_catalog_admin_grant_revoke");
managementApi.createPrincipalRole(principalRoleName);
String catalogName = client.newEntityName("myuniquetestcatalog");
@@ -1743,7 +1743,7 @@ public class PolarisManagementServiceIntegrationTest {
managementApi.grantCatalogRoleToPrincipalRole(principalRoleName,
catalogName, catalogAdminRole);
PrincipalWithCredentials catalogAdminPrincipal =
- managementApi.createPrincipal(client.newEntityName("principal1"));
+
managementApi.createPrincipal(client.newEntityName("principal_catalog_admin_grant_revoke"));
managementApi.assignPrincipalRole(
catalogAdminPrincipal.getPrincipal().getName(), principalRoleName);
@@ -1752,12 +1752,12 @@ public class PolarisManagementServiceIntegrationTest {
// Create a second principal role. Use the catalog admin principal to list
principal roles and
// grant a catalog role to the new principal role
- String principalRoleName2 = "mypr2";
+ String principalRoleName2 =
client.newEntityName("mypr2_catalog_admin_grant_revoke");
PrincipalRole principalRole2 = new PrincipalRole(principalRoleName2);
managementApi.createPrincipalRole(principalRole2);
// create a catalog role and grant it manage_content privilege
- String catalogRoleName = "mycr1";
+ String catalogRoleName =
client.newEntityName("mycr1_catalog_admin_grant_revoke");
client.managementApi(catalogAdminToken).createCatalogRole(catalogName,
catalogRoleName);
CatalogPrivilege privilege = CatalogPrivilege.CATALOG_MANAGE_CONTENT;
@@ -1774,8 +1774,8 @@ public class PolarisManagementServiceIntegrationTest {
.grantCatalogRoleToPrincipalRole(
principalRoleName2, catalogName, new CatalogRole(catalogRoleName));
- // But the catalog admin cannot revoke the role because it requires
- // PRINCIPAL_ROLE_MANAGE_GRANTS_FOR_GRANTEE
+ // The catalog admin can now revoke the role since it has
+ // CATALOG_ROLE_MANAGE_GRANTS_ON_SECURABLE
try (Response response =
client
.managementApi(catalogAdminToken)
@@ -1787,11 +1787,17 @@ public class PolarisManagementServiceIntegrationTest {
+ "/"
+ catalogRoleName)
.delete()) {
- assertThat(response).returns(FORBIDDEN.getStatusCode(),
Response::getStatus);
+ assertThat(response).returns(Response.Status.NO_CONTENT.getStatusCode(),
Response::getStatus);
}
- // The service admin can revoke the role because it has the
- // PRINCIPAL_ROLE_MANAGE_GRANTS_FOR_GRANTEE privilege
+ // Re-grant the role to test that service admin can also revoke
+ client
+ .managementApi(catalogAdminToken)
+ .grantCatalogRoleToPrincipalRole(
+ principalRoleName2, catalogName, new CatalogRole(catalogRoleName));
+
+ // The service admin can also revoke the role (has
CATALOG_ROLE_MANAGE_GRANTS_ON_SECURABLE
+ // via catalog-scoped privileges)
try (Response response =
managementApi
.request(
@@ -1828,7 +1834,7 @@ public class PolarisManagementServiceIntegrationTest {
public void testServiceAdminCanTransferCatalogAdmin() {
// Create a PrincipalRole and a new catalog. Grant the catalog_admin role
to the new principal
// role
- String principalRoleName = client.newEntityName("mypr33");
+ String principalRoleName =
client.newEntityName("mypr_service_admin_transfer");
PrincipalRole principalRole1 = new PrincipalRole(principalRoleName);
managementApi.createPrincipalRole(principalRole1);
@@ -1848,14 +1854,14 @@ public class PolarisManagementServiceIntegrationTest {
managementApi.grantCatalogRoleToPrincipalRole(principalRoleName,
catalogName, catalogAdminRole);
PrincipalWithCredentials catalogAdminPrincipal =
- managementApi.createPrincipal(client.newEntityName("principal1"));
+
managementApi.createPrincipal(client.newEntityName("principal_service_admin_transfer"));
managementApi.assignPrincipalRole(
catalogAdminPrincipal.getPrincipal().getName(),
principalRole1.getName());
String catalogAdminToken = client.obtainToken(catalogAdminPrincipal);
- // service_admin revokes the catalog_admin privilege from its principal
role
+ // service_admin revokes the catalog_admin privilege from its own
principal role
try {
try (Response response =
managementApi
@@ -1868,10 +1874,11 @@ public class PolarisManagementServiceIntegrationTest {
.returns(Response.Status.NO_CONTENT.getStatusCode(),
Response::getStatus);
}
- // the service_admin can not revoke the catalog_admin privilege from the
new principal role
+ // After revoking its own catalog_admin, service_admin can no longer
revoke the
+ // catalog_admin privilege from the new principal role because it no
longer has
+ // CATALOG_ROLE_MANAGE_GRANTS_ON_SECURABLE (which comes from having
catalog_admin)
try (Response response =
- client
- .managementApi(catalogAdminToken)
+ managementApi
.request(
"v1/principal-roles/"
+ principalRoleName
@@ -1896,7 +1903,7 @@ public class PolarisManagementServiceIntegrationTest {
public void testCatalogAdminGrantAndRevokeCatalogRolesFromWrongCatalog() {
// Create a PrincipalRole and a new catalog. Grant the catalog_admin role
to the new principal
// role
- String principalRoleName = client.newEntityName("mypr33");
+ String principalRoleName = client.newEntityName("mypr_wrong_catalog");
PrincipalRole principalRole1 = new PrincipalRole(principalRoleName);
managementApi.createPrincipalRole(principalRole1);
@@ -1923,7 +1930,7 @@ public class PolarisManagementServiceIntegrationTest {
managementApi.createCatalog(catalog2);
// create a catalog role *in the second catalog* and grant it
manage_content privilege
- String catalogRoleName = "mycr1";
+ String catalogRoleName = client.newEntityName("mycr1_wrong_catalog");
managementApi.createCatalogRole(catalogName2, catalogRoleName);
// Get the catalog admin role from the *first* catalog and grant that role
to the principal role
@@ -1934,7 +1941,7 @@ public class PolarisManagementServiceIntegrationTest {
// Create a principal and grant the principal role to it
PrincipalWithCredentials catalogAdminPrincipal =
- managementApi.createPrincipal(client.newEntityName("principal1"));
+
managementApi.createPrincipal(client.newEntityName("principal_wrong_catalog"));
managementApi.assignPrincipalRole(
catalogAdminPrincipal.getPrincipal().getName(),
principalRole1.getName());
@@ -1959,7 +1966,7 @@ public class PolarisManagementServiceIntegrationTest {
@Test
public void testTableManageAccessCanGrantAndRevokeFromCatalogRoles() {
// Create a PrincipalRole and a new catalog.
- String principalRoleName = client.newEntityName("mypr33");
+ String principalRoleName = client.newEntityName("mypr_table_manage");
PrincipalRole principalRole1 = new PrincipalRole(principalRoleName);
managementApi.createPrincipalRole(principalRole1);
@@ -1975,7 +1982,8 @@ public class PolarisManagementServiceIntegrationTest {
managementApi.createCatalog(catalog);
// create a valid target CatalogRole in this catalog
- managementApi.createCatalogRole(catalogName, "target_catalog_role");
+ String targetCatalogRoleName = client.newEntityName("target_catalog_role");
+ managementApi.createCatalogRole(catalogName, targetCatalogRoleName);
// create a second catalog
String catalogName2 = client.newEntityName("anothertablemanagecatalog");
@@ -1989,17 +1997,18 @@ public class PolarisManagementServiceIntegrationTest {
managementApi.createCatalog(catalog2);
// create an *invalid* target CatalogRole in second catalog
- managementApi.createCatalogRole(catalogName2,
"invalid_target_catalog_role");
+ String invalidTargetCatalogRoleName =
client.newEntityName("invalid_target_catalog_role");
+ managementApi.createCatalogRole(catalogName2,
invalidTargetCatalogRoleName);
// create the namespace "c" in *both* namespaces
- String namespaceName = "c";
+ String namespaceName = client.newEntityName("c");
catalogApi.createNamespace(catalogName, namespaceName);
catalogApi.createNamespace(catalogName2, namespaceName);
// create a catalog role *in the first catalog* and grant it
manage_content privilege at the
// namespace level
// grant that role to the PrincipalRole
- String catalogRoleName = "ns_manage_access_role";
+ String catalogRoleName = client.newEntityName("ns_manage_access_role");
managementApi.createCatalogRole(catalogName, catalogRoleName);
managementApi.addGrant(
catalogName,
@@ -2028,7 +2037,7 @@ public class PolarisManagementServiceIntegrationTest {
.managementApi(manageAccessUserToken)
.addGrant(
catalogName,
- "target_catalog_role",
+ targetCatalogRoleName,
new NamespaceGrant(
List.of(namespaceName),
NamespacePrivilege.TABLE_CREATE,
@@ -2044,7 +2053,7 @@ public class PolarisManagementServiceIntegrationTest {
.managementApi(manageAccessUserToken)
.request("v1/principal-roles/" + principalRoleName +
"/catalog-roles/" + catalogName)
.put(
- Entity.json(new GrantCatalogRoleRequest(new
CatalogRole("target_catalog_role"))))) {
+ Entity.json(new GrantCatalogRoleRequest(new
CatalogRole(targetCatalogRoleName))))) {
assertThat(response).returns(FORBIDDEN.getStatusCode(),
Response::getStatus);
}
@@ -2054,7 +2063,7 @@ public class PolarisManagementServiceIntegrationTest {
.managementApi(manageAccessUserToken)
.request(
"v1/catalogs/{cat}/catalog-roles/{role}/grants",
- Map.of("cat", catalogName, "role", "target_catalog_role"))
+ Map.of("cat", catalogName, "role", targetCatalogRoleName))
.put(
Entity.json(
new CatalogGrant(
@@ -2070,7 +2079,7 @@ public class PolarisManagementServiceIntegrationTest {
.managementApi(manageAccessUserToken)
.request(
"v1/catalogs/{cat}/catalog-roles/{role}/grants",
- Map.of("cat", catalogName2, "role",
"invalid_target_catalog_role"))
+ Map.of("cat", catalogName2, "role",
invalidTargetCatalogRoleName))
.put(
Entity.json(
new NamespaceGrant(
@@ -2086,7 +2095,7 @@ public class PolarisManagementServiceIntegrationTest {
.managementApi(manageAccessUserToken)
.request(
"v1/catalogs/{cat}/catalog-roles/{role}/grants",
- Map.of("cat", catalogName2, "role",
"invalid_target_catalog_role"))
+ Map.of("cat", catalogName2, "role",
invalidTargetCatalogRoleName))
.put(
Entity.json(
new CatalogGrant(
diff --git
a/polaris-core/src/main/java/org/apache/polaris/core/auth/PolarisAuthorizableOperation.java
b/polaris-core/src/main/java/org/apache/polaris/core/auth/PolarisAuthorizableOperation.java
index ee373a2325..318481709b 100644
---
a/polaris-core/src/main/java/org/apache/polaris/core/auth/PolarisAuthorizableOperation.java
+++
b/polaris-core/src/main/java/org/apache/polaris/core/auth/PolarisAuthorizableOperation.java
@@ -173,8 +173,7 @@ public enum PolarisAuthorizableOperation {
LIST_ASSIGNEE_PRINCIPALS_FOR_PRINCIPAL_ROLE(PRINCIPAL_ROLE_LIST_GRANTS),
LIST_CATALOG_ROLES_FOR_PRINCIPAL_ROLE(PRINCIPAL_ROLE_LIST_GRANTS),
ASSIGN_CATALOG_ROLE_TO_PRINCIPAL_ROLE(CATALOG_ROLE_MANAGE_GRANTS_ON_SECURABLE),
- REVOKE_CATALOG_ROLE_FROM_PRINCIPAL_ROLE(
- CATALOG_ROLE_MANAGE_GRANTS_ON_SECURABLE,
PRINCIPAL_ROLE_MANAGE_GRANTS_FOR_GRANTEE),
+
REVOKE_CATALOG_ROLE_FROM_PRINCIPAL_ROLE(CATALOG_ROLE_MANAGE_GRANTS_ON_SECURABLE),
LIST_CATALOG_ROLES(CATALOG_ROLE_LIST),
CREATE_CATALOG_ROLE(CATALOG_ROLE_CREATE),
GET_CATALOG_ROLE(CATALOG_ROLE_READ_PROPERTIES),
diff --git
a/runtime/service/src/test/java/org/apache/polaris/service/admin/PolarisAdminServiceAuthzTest.java
b/runtime/service/src/test/java/org/apache/polaris/service/admin/PolarisAdminServiceAuthzTest.java
index 6c9fe81724..33545e1852 100644
---
a/runtime/service/src/test/java/org/apache/polaris/service/admin/PolarisAdminServiceAuthzTest.java
+++
b/runtime/service/src/test/java/org/apache/polaris/service/admin/PolarisAdminServiceAuthzTest.java
@@ -563,9 +563,7 @@ public class PolarisAdminServiceAuthzTest extends
PolarisAuthzTestBase {
@TestFactory
Stream<DynamicNode> testRevokeCatalogRoleFromPrincipalRolePrivileges() {
- // Revoke requires privileges both on the "securable" (CatalogRole) as
well as the "grantee"
- // (PrincipalRole); neither CATALOG_MANAGE_ACCESS nor
SERVICE_MANAGE_ACCESS alone are
- // sufficient.
+ // Revoke only requires privileges on the securable (CatalogRole).
return authzTestsBuilder("revokeCatalogRoleFromPrincipalRole")
.action(
() ->
@@ -580,14 +578,7 @@ public class PolarisAdminServiceAuthzTest extends
PolarisAuthzTestBase {
privilege ->
adminService.revokePrivilegeOnRootContainerFromPrincipalRole(
PRINCIPAL_ROLE1, privilege))
- .shouldPassWith(
- PolarisPrivilege.CATALOG_MANAGE_ACCESS,
PolarisPrivilege.SERVICE_MANAGE_ACCESS)
- .shouldPassWith(
- PolarisPrivilege.CATALOG_MANAGE_ACCESS,
- PolarisPrivilege.PRINCIPAL_ROLE_MANAGE_GRANTS_FOR_GRANTEE)
- .shouldPassWith(
- PolarisPrivilege.CATALOG_ROLE_MANAGE_GRANTS_ON_SECURABLE,
- PolarisPrivilege.PRINCIPAL_ROLE_MANAGE_GRANTS_FOR_GRANTEE)
+
.shouldPassWith(PolarisPrivilege.CATALOG_ROLE_MANAGE_GRANTS_ON_SECURABLE)
.createTests();
}
