You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@polaris.apache.org by co...@apache.org on 2026/03/10 21:05:14 UTC
(polaris) branch main updated: Remove PRINCIPAL_ROLE_MANAGE_GRANTS_FOR_GRANTEE requirement from REVOKE_CATALOG_ROLE_FROM_PRINCIPAL_ROLE (#3906)
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 <40...@users.noreply.github.com>
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 <no...@anthropic.com>
* Add CHANGELOG entry for REVOKE_CATALOG_ROLE_FROM_PRINCIPAL_ROLE authorization change
Co-Authored-By: Claude Opus 4.6 <no...@anthropic.com>
* 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 <no...@anthropic.com>
* Fixed name collisions in integration test
---------
Co-authored-by: Claude Opus 4.6 <no...@anthropic.com>
---
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();
}