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 2bbd3830c..21c37ab80 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 @@ -124,7 +124,9 @@ public static void setup(PolarisApiEndpoints endpoints, ClientCredentials creden @AfterAll public static void close() throws Exception { - client.close(); + if (client != null) { + client.close(); + } } @AfterEach @@ -1728,7 +1730,7 @@ public void testCatalogAdminGrantAndRevokeCatalogRoles() { .managementApi(catalogAdminToken) .request( "v1/principal-roles/" - + principalRoleName + + principalRoleName2 + "/catalog-roles/" + catalogName + "/" @@ -1743,7 +1745,7 @@ public void testCatalogAdminGrantAndRevokeCatalogRoles() { managementApi .request( "v1/principal-roles/" - + principalRoleName + + principalRoleName2 + "/catalog-roles/" + catalogName + "/" @@ -1751,6 +1753,24 @@ public void testCatalogAdminGrantAndRevokeCatalogRoles() { .delete()) { assertThat(response).returns(Response.Status.NO_CONTENT.getStatusCode(), Response::getStatus); } + + // trigger an internal error by using "principalRoleName" instead of "principalRoleName2" + try (Response response = + managementApi + .request( + "v1/principal-roles/" + + principalRoleName + + "/catalog-roles/" + + catalogName + + "/" + + catalogRoleName) + .delete()) { + assertThat(response) + .returns(Response.Status.BAD_REQUEST.getStatusCode(), Response::getStatus); + assertThat(response.hasEntity()).isTrue(); + ErrorResponse errorResponse = response.readEntity(ErrorResponse.class); + assertThat(errorResponse.message()).contains("Operation failed: GRANT_NOT_FOUND"); + } } @Test diff --git a/runtime/service/src/main/java/org/apache/polaris/service/admin/PolarisAdminService.java b/runtime/service/src/main/java/org/apache/polaris/service/admin/PolarisAdminService.java index bfb77ac26..dca65497f 100644 --- a/runtime/service/src/main/java/org/apache/polaris/service/admin/PolarisAdminService.java +++ b/runtime/service/src/main/java/org/apache/polaris/service/admin/PolarisAdminService.java @@ -104,6 +104,7 @@ import org.apache.polaris.core.persistence.dao.entity.DropEntityResult; import org.apache.polaris.core.persistence.dao.entity.EntityResult; import org.apache.polaris.core.persistence.dao.entity.LoadGrantsResult; +import org.apache.polaris.core.persistence.dao.entity.PrivilegeResult; import org.apache.polaris.core.persistence.pagination.PageToken; import org.apache.polaris.core.persistence.resolver.PolarisResolutionManifest; import org.apache.polaris.core.persistence.resolver.ResolutionManifestFactory; @@ -1400,7 +1401,7 @@ public List listCatalogRoles(String catalogName) { .toList(); } - public boolean assignPrincipalRole(String principalName, String principalRoleName) { + public PrivilegeResult assignPrincipalRole(String principalName, String principalRoleName) { PolarisAuthorizableOperation op = PolarisAuthorizableOperation.ASSIGN_PRINCIPAL_ROLE; authorizeGrantOnPrincipalRoleToPrincipalOperationOrThrow(op, principalRoleName, principalName); @@ -1417,13 +1418,11 @@ public boolean assignPrincipalRole(String principalName, String principalRoleNam if (FederatedEntities.isFederated(principalRoleEntity)) { throw new ValidationException("Cannot assign a federated role to a principal"); } - return metaStoreManager - .grantUsageOnRoleToGrantee( - getCurrentPolarisContext(), null, principalRoleEntity, principalEntity) - .isSuccess(); + return metaStoreManager.grantUsageOnRoleToGrantee( + getCurrentPolarisContext(), null, principalRoleEntity, principalEntity); } - public boolean revokePrincipalRole(String principalName, String principalRoleName) { + public PrivilegeResult revokePrincipalRole(String principalName, String principalRoleName) { PolarisAuthorizableOperation op = PolarisAuthorizableOperation.REVOKE_PRINCIPAL_ROLE; authorizeGrantOnPrincipalRoleToPrincipalOperationOrThrow(op, principalRoleName, principalName); @@ -1440,10 +1439,8 @@ public boolean revokePrincipalRole(String principalName, String principalRoleNam if (FederatedEntities.isFederated(principalRoleEntity)) { throw new ValidationException("Cannot revoke a federated role from a principal"); } - return metaStoreManager - .revokeUsageOnRoleFromGrantee( - getCurrentPolarisContext(), null, principalRoleEntity, principalEntity) - .isSuccess(); + return metaStoreManager.revokeUsageOnRoleFromGrantee( + getCurrentPolarisContext(), null, principalRoleEntity, principalEntity); } public List listPrincipalRolesAssigned(String principalName) { @@ -1459,7 +1456,7 @@ public List listPrincipalRolesAssigned(String principalName) { return buildEntitiesFromGrantResults(grantList, false, PolarisEntityType.PRINCIPAL_ROLE, null); } - public boolean assignCatalogRoleToPrincipalRole( + public PrivilegeResult assignCatalogRoleToPrincipalRole( String principalRoleName, String catalogName, String catalogRoleName) { PolarisAuthorizableOperation op = PolarisAuthorizableOperation.ASSIGN_CATALOG_ROLE_TO_PRINCIPAL_ROLE; @@ -1477,13 +1474,11 @@ public boolean assignCatalogRoleToPrincipalRole( findCatalogRoleByName(catalogName, catalogRoleName) .orElseThrow(() -> new NotFoundException("CatalogRole %s not found", catalogRoleName)); - return metaStoreManager - .grantUsageOnRoleToGrantee( - getCurrentPolarisContext(), catalogEntity, catalogRoleEntity, principalRoleEntity) - .isSuccess(); + return metaStoreManager.grantUsageOnRoleToGrantee( + getCurrentPolarisContext(), catalogEntity, catalogRoleEntity, principalRoleEntity); } - public boolean revokeCatalogRoleFromPrincipalRole( + public PrivilegeResult revokeCatalogRoleFromPrincipalRole( String principalRoleName, String catalogName, String catalogRoleName) { PolarisAuthorizableOperation op = PolarisAuthorizableOperation.REVOKE_CATALOG_ROLE_FROM_PRINCIPAL_ROLE; @@ -1500,10 +1495,8 @@ public boolean revokeCatalogRoleFromPrincipalRole( PolarisEntity catalogRoleEntity = findCatalogRoleByName(catalogName, catalogRoleName) .orElseThrow(() -> new NotFoundException("CatalogRole %s not found", catalogRoleName)); - return metaStoreManager - .revokeUsageOnRoleFromGrantee( - getCurrentPolarisContext(), catalogEntity, catalogRoleEntity, principalRoleEntity) - .isSuccess(); + return metaStoreManager.revokeUsageOnRoleFromGrantee( + getCurrentPolarisContext(), catalogEntity, catalogRoleEntity, principalRoleEntity); } public List listAssigneePrincipalsForPrincipalRole(String principalRoleName) { @@ -1579,7 +1572,7 @@ public List listCatalogRolesForPrincipalRole( } /** Adds a grant on the root container of this realm to {@code principalRoleName}. */ - public boolean grantPrivilegeOnRootContainerToPrincipalRole( + public PrivilegeResult grantPrivilegeOnRootContainerToPrincipalRole( String principalRoleName, PolarisPrivilege privilege) { PolarisAuthorizableOperation op = PolarisAuthorizableOperation.ADD_ROOT_GRANT_TO_PRINCIPAL_ROLE; authorizeGrantOnRootContainerToPrincipalRoleOperationOrThrow(op, principalRoleName); @@ -1591,14 +1584,12 @@ public boolean grantPrivilegeOnRootContainerToPrincipalRole( .orElseThrow( () -> new NotFoundException("PrincipalRole %s not found", principalRoleName)); - return metaStoreManager - .grantPrivilegeOnSecurableToRole( - getCurrentPolarisContext(), principalRoleEntity, null, rootContainerEntity, privilege) - .isSuccess(); + return metaStoreManager.grantPrivilegeOnSecurableToRole( + getCurrentPolarisContext(), principalRoleEntity, null, rootContainerEntity, privilege); } /** Revokes a grant on the root container of this realm from {@code principalRoleName}. */ - public boolean revokePrivilegeOnRootContainerFromPrincipalRole( + public PrivilegeResult revokePrivilegeOnRootContainerFromPrincipalRole( String principalRoleName, PolarisPrivilege privilege) { PolarisAuthorizableOperation op = PolarisAuthorizableOperation.REVOKE_ROOT_GRANT_FROM_PRINCIPAL_ROLE; @@ -1611,17 +1602,15 @@ public boolean revokePrivilegeOnRootContainerFromPrincipalRole( .orElseThrow( () -> new NotFoundException("PrincipalRole %s not found", principalRoleName)); - return metaStoreManager - .revokePrivilegeOnSecurableFromRole( - getCurrentPolarisContext(), principalRoleEntity, null, rootContainerEntity, privilege) - .isSuccess(); + return metaStoreManager.revokePrivilegeOnSecurableFromRole( + getCurrentPolarisContext(), principalRoleEntity, null, rootContainerEntity, privilege); } /** * Adds a catalog-level grant on {@code catalogName} to {@code catalogRoleName} which resides * within the same catalog on which it is being granted the privilege. */ - public boolean grantPrivilegeOnCatalogToRole( + public PrivilegeResult grantPrivilegeOnCatalogToRole( String catalogName, String catalogRoleName, PolarisPrivilege privilege) { PolarisAuthorizableOperation op = PolarisAuthorizableOperation.ADD_CATALOG_GRANT_TO_CATALOG_ROLE; @@ -1635,18 +1624,16 @@ public boolean grantPrivilegeOnCatalogToRole( findCatalogRoleByName(catalogName, catalogRoleName) .orElseThrow(() -> new NotFoundException("CatalogRole %s not found", catalogRoleName)); - return metaStoreManager - .grantPrivilegeOnSecurableToRole( - getCurrentPolarisContext(), - catalogRoleEntity, - PolarisEntity.toCoreList(List.of(catalogEntity)), - catalogEntity, - privilege) - .isSuccess(); + return metaStoreManager.grantPrivilegeOnSecurableToRole( + getCurrentPolarisContext(), + catalogRoleEntity, + PolarisEntity.toCoreList(List.of(catalogEntity)), + catalogEntity, + privilege); } /** Removes a catalog-level grant on {@code catalogName} from {@code catalogRoleName}. */ - public boolean revokePrivilegeOnCatalogFromRole( + public PrivilegeResult revokePrivilegeOnCatalogFromRole( String catalogName, String catalogRoleName, PolarisPrivilege privilege) { PolarisAuthorizableOperation op = PolarisAuthorizableOperation.REVOKE_CATALOG_GRANT_FROM_CATALOG_ROLE; @@ -1659,18 +1646,16 @@ public boolean revokePrivilegeOnCatalogFromRole( findCatalogRoleByName(catalogName, catalogRoleName) .orElseThrow(() -> new NotFoundException("CatalogRole %s not found", catalogRoleName)); - return metaStoreManager - .revokePrivilegeOnSecurableFromRole( - getCurrentPolarisContext(), - catalogRoleEntity, - PolarisEntity.toCoreList(List.of(catalogEntity)), - catalogEntity, - privilege) - .isSuccess(); + return metaStoreManager.revokePrivilegeOnSecurableFromRole( + getCurrentPolarisContext(), + catalogRoleEntity, + PolarisEntity.toCoreList(List.of(catalogEntity)), + catalogEntity, + privilege); } /** Adds a namespace-level grant on {@code namespace} to {@code catalogRoleName}. */ - public boolean grantPrivilegeOnNamespaceToRole( + public PrivilegeResult grantPrivilegeOnNamespaceToRole( String catalogName, String catalogRoleName, Namespace namespace, PolarisPrivilege privilege) { PolarisAuthorizableOperation op = PolarisAuthorizableOperation.ADD_NAMESPACE_GRANT_TO_CATALOG_ROLE; @@ -1688,18 +1673,16 @@ public boolean grantPrivilegeOnNamespaceToRole( List catalogPath = resolvedPathWrapper.getRawParentPath(); PolarisEntity namespaceEntity = resolvedPathWrapper.getRawLeafEntity(); - return metaStoreManager - .grantPrivilegeOnSecurableToRole( - getCurrentPolarisContext(), - catalogRoleEntity, - PolarisEntity.toCoreList(catalogPath), - namespaceEntity, - privilege) - .isSuccess(); + return metaStoreManager.grantPrivilegeOnSecurableToRole( + getCurrentPolarisContext(), + catalogRoleEntity, + PolarisEntity.toCoreList(catalogPath), + namespaceEntity, + privilege); } /** Removes a namespace-level grant on {@code namespace} from {@code catalogRoleName}. */ - public boolean revokePrivilegeOnNamespaceFromRole( + public PrivilegeResult revokePrivilegeOnNamespaceFromRole( String catalogName, String catalogRoleName, Namespace namespace, PolarisPrivilege privilege) { PolarisAuthorizableOperation op = PolarisAuthorizableOperation.REVOKE_NAMESPACE_GRANT_FROM_CATALOG_ROLE; @@ -1717,17 +1700,15 @@ public boolean revokePrivilegeOnNamespaceFromRole( List catalogPath = resolvedPathWrapper.getRawParentPath(); PolarisEntity namespaceEntity = resolvedPathWrapper.getRawLeafEntity(); - return metaStoreManager - .revokePrivilegeOnSecurableFromRole( - getCurrentPolarisContext(), - catalogRoleEntity, - PolarisEntity.toCoreList(catalogPath), - namespaceEntity, - privilege) - .isSuccess(); + return metaStoreManager.revokePrivilegeOnSecurableFromRole( + getCurrentPolarisContext(), + catalogRoleEntity, + PolarisEntity.toCoreList(catalogPath), + namespaceEntity, + privilege); } - public boolean grantPrivilegeOnTableToRole( + public PrivilegeResult grantPrivilegeOnTableToRole( String catalogName, String catalogRoleName, TableIdentifier identifier, @@ -1749,7 +1730,7 @@ public boolean grantPrivilegeOnTableToRole( privilege); } - public boolean revokePrivilegeOnTableFromRole( + public PrivilegeResult revokePrivilegeOnTableFromRole( String catalogName, String catalogRoleName, TableIdentifier identifier, @@ -1772,7 +1753,7 @@ public boolean revokePrivilegeOnTableFromRole( privilege); } - public boolean grantPrivilegeOnViewToRole( + public PrivilegeResult grantPrivilegeOnViewToRole( String catalogName, String catalogRoleName, TableIdentifier identifier, @@ -1790,7 +1771,7 @@ public boolean grantPrivilegeOnViewToRole( privilege); } - public boolean revokePrivilegeOnViewFromRole( + public PrivilegeResult revokePrivilegeOnViewFromRole( String catalogName, String catalogRoleName, TableIdentifier identifier, @@ -1809,7 +1790,7 @@ public boolean revokePrivilegeOnViewFromRole( privilege); } - public boolean grantPrivilegeOnPolicyToRole( + public PrivilegeResult grantPrivilegeOnPolicyToRole( String catalogName, String catalogRoleName, PolicyIdentifier identifier, @@ -1821,7 +1802,7 @@ public boolean grantPrivilegeOnPolicyToRole( return grantPrivilegeOnPolicyEntityToRole(catalogName, catalogRoleName, identifier, privilege); } - public boolean revokePrivilegeOnPolicyFromRole( + public PrivilegeResult revokePrivilegeOnPolicyFromRole( String catalogName, String catalogRoleName, PolicyIdentifier identifier, @@ -1998,7 +1979,7 @@ public List listGrantsForCatalogRole(String catalogName, String c } /** Adds a table-level or view-level grant on {@code identifier} to {@code catalogRoleName}. */ - private boolean grantPrivilegeOnTableLikeToRole( + private PrivilegeResult grantPrivilegeOnTableLikeToRole( String catalogName, String catalogRoleName, TableIdentifier identifier, @@ -2021,20 +2002,18 @@ private boolean grantPrivilegeOnTableLikeToRole( List catalogPath = resolvedPathWrapper.getRawParentPath(); PolarisEntity tableLikeEntity = resolvedPathWrapper.getRawLeafEntity(); - return metaStoreManager - .grantPrivilegeOnSecurableToRole( - getCurrentPolarisContext(), - catalogRoleEntity, - PolarisEntity.toCoreList(catalogPath), - tableLikeEntity, - privilege) - .isSuccess(); + return metaStoreManager.grantPrivilegeOnSecurableToRole( + getCurrentPolarisContext(), + catalogRoleEntity, + PolarisEntity.toCoreList(catalogPath), + tableLikeEntity, + privilege); } /** * Removes a table-level or view-level grant on {@code identifier} from {@code catalogRoleName}. */ - private boolean revokePrivilegeOnTableLikeFromRole( + private PrivilegeResult revokePrivilegeOnTableLikeFromRole( String catalogName, String catalogRoleName, TableIdentifier identifier, @@ -2057,17 +2036,15 @@ private boolean revokePrivilegeOnTableLikeFromRole( List catalogPath = resolvedPathWrapper.getRawParentPath(); PolarisEntity tableLikeEntity = resolvedPathWrapper.getRawLeafEntity(); - return metaStoreManager - .revokePrivilegeOnSecurableFromRole( - getCurrentPolarisContext(), - catalogRoleEntity, - PolarisEntity.toCoreList(catalogPath), - tableLikeEntity, - privilege) - .isSuccess(); + return metaStoreManager.revokePrivilegeOnSecurableFromRole( + getCurrentPolarisContext(), + catalogRoleEntity, + PolarisEntity.toCoreList(catalogPath), + tableLikeEntity, + privilege); } - private boolean grantPrivilegeOnPolicyEntityToRole( + private PrivilegeResult grantPrivilegeOnPolicyEntityToRole( String catalogName, String catalogRoleName, PolicyIdentifier identifier, @@ -2087,17 +2064,15 @@ private boolean grantPrivilegeOnPolicyEntityToRole( List catalogPath = resolvedPathWrapper.getRawParentPath(); PolarisEntity policyEntity = resolvedPathWrapper.getRawLeafEntity(); - return metaStoreManager - .grantPrivilegeOnSecurableToRole( - getCurrentPolarisContext(), - catalogRoleEntity, - PolarisEntity.toCoreList(catalogPath), - policyEntity, - privilege) - .isSuccess(); + return metaStoreManager.grantPrivilegeOnSecurableToRole( + getCurrentPolarisContext(), + catalogRoleEntity, + PolarisEntity.toCoreList(catalogPath), + policyEntity, + privilege); } - private boolean revokePrivilegeOnPolicyEntityFromRole( + private PrivilegeResult revokePrivilegeOnPolicyEntityFromRole( String catalogName, String catalogRoleName, PolicyIdentifier identifier, @@ -2117,13 +2092,11 @@ private boolean revokePrivilegeOnPolicyEntityFromRole( List catalogPath = resolvedPathWrapper.getRawParentPath(); PolarisEntity policyEntity = resolvedPathWrapper.getRawLeafEntity(); - return metaStoreManager - .revokePrivilegeOnSecurableFromRole( - getCurrentPolarisContext(), - catalogRoleEntity, - PolarisEntity.toCoreList(catalogPath), - policyEntity, - privilege) - .isSuccess(); + return metaStoreManager.revokePrivilegeOnSecurableFromRole( + getCurrentPolarisContext(), + catalogRoleEntity, + PolarisEntity.toCoreList(catalogPath), + policyEntity, + privilege); } } diff --git a/runtime/service/src/main/java/org/apache/polaris/service/admin/PolarisServiceImpl.java b/runtime/service/src/main/java/org/apache/polaris/service/admin/PolarisServiceImpl.java index 5318c00b8..1c57a37f7 100644 --- a/runtime/service/src/main/java/org/apache/polaris/service/admin/PolarisServiceImpl.java +++ b/runtime/service/src/main/java/org/apache/polaris/service/admin/PolarisServiceImpl.java @@ -20,6 +20,7 @@ import jakarta.enterprise.context.RequestScoped; import jakarta.inject.Inject; +import jakarta.ws.rs.core.MediaType; import jakarta.ws.rs.core.Response; import jakarta.ws.rs.core.SecurityContext; import java.util.List; @@ -27,6 +28,7 @@ import org.apache.iceberg.catalog.Namespace; import org.apache.iceberg.catalog.TableIdentifier; import org.apache.iceberg.exceptions.NotAuthorizedException; +import org.apache.iceberg.rest.responses.ErrorResponse; import org.apache.polaris.core.admin.model.AddGrantRequest; import org.apache.polaris.core.admin.model.AuthenticationParameters; import org.apache.polaris.core.admin.model.Catalog; @@ -71,6 +73,8 @@ import org.apache.polaris.core.entity.PrincipalRoleEntity; import org.apache.polaris.core.persistence.MetaStoreManagerFactory; import org.apache.polaris.core.persistence.PolarisMetaStoreManager; +import org.apache.polaris.core.persistence.dao.entity.BaseResult; +import org.apache.polaris.core.persistence.dao.entity.PrivilegeResult; import org.apache.polaris.core.persistence.resolver.ResolutionManifestFactory; import org.apache.polaris.core.secrets.UserSecretsManager; import org.apache.polaris.core.secrets.UserSecretsManagerFactory; @@ -134,6 +138,22 @@ private PolarisAdminService newAdminService( reservedProperties); } + private static Response toResponse(BaseResult result, Response.Status successStatus) { + if (!result.isSuccess()) { + ErrorResponse icebergErrorResponse = + ErrorResponse.builder() + .responseCode(Response.Status.BAD_REQUEST.getStatusCode()) + .withType(result.getReturnStatus().toString()) + .withMessage("Operation failed: " + result.getReturnStatus().toString()) + .build(); + return Response.status(Response.Status.BAD_REQUEST) + .type(MediaType.APPLICATION_JSON_TYPE) + .entity(icebergErrorResponse) + .build(); + } + return Response.status(successStatus).build(); + } + /** From PolarisCatalogsApiService */ @Override public Response createCatalog( @@ -457,8 +477,9 @@ public Response assignPrincipalRole( request.getPrincipalRole().getName(), principalName); PolarisAdminService adminService = newAdminService(realmContext, securityContext); - adminService.assignPrincipalRole(principalName, request.getPrincipalRole().getName()); - return Response.status(Response.Status.CREATED).build(); + PrivilegeResult result = + adminService.assignPrincipalRole(principalName, request.getPrincipalRole().getName()); + return toResponse(result, Response.Status.CREATED); } /** From PolarisPrincipalsApiService */ @@ -470,8 +491,8 @@ public Response revokePrincipalRole( SecurityContext securityContext) { LOGGER.info("Revoking principalRole {} from principal {}", principalRoleName, principalName); PolarisAdminService adminService = newAdminService(realmContext, securityContext); - adminService.revokePrincipalRole(principalName, principalRoleName); - return Response.status(Response.Status.NO_CONTENT).build(); + PrivilegeResult result = adminService.revokePrincipalRole(principalName, principalRoleName); + return toResponse(result, Response.Status.NO_CONTENT); } /** From PolarisPrincipalsApiService */ @@ -503,9 +524,10 @@ public Response assignCatalogRoleToPrincipalRole( catalogName, principalRoleName); PolarisAdminService adminService = newAdminService(realmContext, securityContext); - adminService.assignCatalogRoleToPrincipalRole( - principalRoleName, catalogName, request.getCatalogRole().getName()); - return Response.status(Response.Status.CREATED).build(); + PrivilegeResult result = + adminService.assignCatalogRoleToPrincipalRole( + principalRoleName, catalogName, request.getCatalogRole().getName()); + return toResponse(result, Response.Status.CREATED); } /** From PolarisPrincipalRolesApiService */ @@ -522,9 +544,10 @@ public Response revokeCatalogRoleFromPrincipalRole( catalogName, principalRoleName); PolarisAdminService adminService = newAdminService(realmContext, securityContext); - adminService.revokeCatalogRoleFromPrincipalRole( - principalRoleName, catalogName, catalogRoleName); - return Response.status(Response.Status.NO_CONTENT).build(); + PrivilegeResult result = + adminService.revokeCatalogRoleFromPrincipalRole( + principalRoleName, catalogName, catalogRoleName); + return toResponse(result, Response.Status.NO_CONTENT); } /** From PolarisPrincipalRolesApiService */ @@ -574,6 +597,7 @@ public Response addGrantToCatalogRole( catalogRoleName, catalogName); PolarisAdminService adminService = newAdminService(realmContext, securityContext); + PrivilegeResult result; switch (grantRequest.getGrant()) { // The per-securable-type Privilege enums must be exact String match for a subset of all // PolarisPrivilege values. @@ -583,11 +607,12 @@ public Response addGrantToCatalogRole( PolarisPrivilege.valueOf(viewGrant.getPrivilege().toString()); String viewName = viewGrant.getViewName(); String[] namespaceParts = viewGrant.getNamespace().toArray(new String[0]); - adminService.grantPrivilegeOnViewToRole( - catalogName, - catalogRoleName, - TableIdentifier.of(Namespace.of(namespaceParts), viewName), - privilege); + result = + adminService.grantPrivilegeOnViewToRole( + catalogName, + catalogRoleName, + TableIdentifier.of(Namespace.of(namespaceParts), viewName), + privilege); break; } case TableGrant tableGrant: @@ -596,11 +621,12 @@ public Response addGrantToCatalogRole( PolarisPrivilege.valueOf(tableGrant.getPrivilege().toString()); String tableName = tableGrant.getTableName(); String[] namespaceParts = tableGrant.getNamespace().toArray(new String[0]); - adminService.grantPrivilegeOnTableToRole( - catalogName, - catalogRoleName, - TableIdentifier.of(Namespace.of(namespaceParts), tableName), - privilege); + result = + adminService.grantPrivilegeOnTableToRole( + catalogName, + catalogRoleName, + TableIdentifier.of(Namespace.of(namespaceParts), tableName), + privilege); break; } case NamespaceGrant namespaceGrant: @@ -608,15 +634,17 @@ public Response addGrantToCatalogRole( PolarisPrivilege privilege = PolarisPrivilege.valueOf(namespaceGrant.getPrivilege().toString()); String[] namespaceParts = namespaceGrant.getNamespace().toArray(new String[0]); - adminService.grantPrivilegeOnNamespaceToRole( - catalogName, catalogRoleName, Namespace.of(namespaceParts), privilege); + result = + adminService.grantPrivilegeOnNamespaceToRole( + catalogName, catalogRoleName, Namespace.of(namespaceParts), privilege); break; } case CatalogGrant catalogGrant: { PolarisPrivilege privilege = PolarisPrivilege.valueOf(catalogGrant.getPrivilege().toString()); - adminService.grantPrivilegeOnCatalogToRole(catalogName, catalogRoleName, privilege); + result = + adminService.grantPrivilegeOnCatalogToRole(catalogName, catalogRoleName, privilege); break; } case PolicyGrant policyGrant: @@ -625,11 +653,12 @@ public Response addGrantToCatalogRole( PolarisPrivilege.valueOf(policyGrant.getPrivilege().toString()); String policyName = policyGrant.getPolicyName(); String[] namespaceParts = policyGrant.getNamespace().toArray(new String[0]); - adminService.grantPrivilegeOnPolicyToRole( - catalogName, - catalogRoleName, - new PolicyIdentifier(Namespace.of(namespaceParts), policyName), - privilege); + result = + adminService.grantPrivilegeOnPolicyToRole( + catalogName, + catalogRoleName, + new PolicyIdentifier(Namespace.of(namespaceParts), policyName), + privilege); break; } default: @@ -640,7 +669,7 @@ public Response addGrantToCatalogRole( .log("Don't know how to handle privilege grant: {}", grantRequest); return Response.status(Response.Status.BAD_REQUEST).build(); } - return Response.status(Response.Status.CREATED).build(); + return toResponse(result, Response.Status.CREATED); } /** From PolarisCatalogsApiService */ @@ -663,6 +692,7 @@ public Response revokeGrantFromCatalogRole( } PolarisAdminService adminService = newAdminService(realmContext, securityContext); + PrivilegeResult result; switch (grantRequest.getGrant()) { // The per-securable-type Privilege enums must be exact String match for a subset of all // PolarisPrivilege values. @@ -672,11 +702,12 @@ public Response revokeGrantFromCatalogRole( PolarisPrivilege.valueOf(viewGrant.getPrivilege().toString()); String viewName = viewGrant.getViewName(); String[] namespaceParts = viewGrant.getNamespace().toArray(new String[0]); - adminService.revokePrivilegeOnViewFromRole( - catalogName, - catalogRoleName, - TableIdentifier.of(Namespace.of(namespaceParts), viewName), - privilege); + result = + adminService.revokePrivilegeOnViewFromRole( + catalogName, + catalogRoleName, + TableIdentifier.of(Namespace.of(namespaceParts), viewName), + privilege); break; } case TableGrant tableGrant: @@ -685,11 +716,12 @@ public Response revokeGrantFromCatalogRole( PolarisPrivilege.valueOf(tableGrant.getPrivilege().toString()); String tableName = tableGrant.getTableName(); String[] namespaceParts = tableGrant.getNamespace().toArray(new String[0]); - adminService.revokePrivilegeOnTableFromRole( - catalogName, - catalogRoleName, - TableIdentifier.of(Namespace.of(namespaceParts), tableName), - privilege); + result = + adminService.revokePrivilegeOnTableFromRole( + catalogName, + catalogRoleName, + TableIdentifier.of(Namespace.of(namespaceParts), tableName), + privilege); break; } case NamespaceGrant namespaceGrant: @@ -697,15 +729,18 @@ public Response revokeGrantFromCatalogRole( PolarisPrivilege privilege = PolarisPrivilege.valueOf(namespaceGrant.getPrivilege().toString()); String[] namespaceParts = namespaceGrant.getNamespace().toArray(new String[0]); - adminService.revokePrivilegeOnNamespaceFromRole( - catalogName, catalogRoleName, Namespace.of(namespaceParts), privilege); + result = + adminService.revokePrivilegeOnNamespaceFromRole( + catalogName, catalogRoleName, Namespace.of(namespaceParts), privilege); break; } case CatalogGrant catalogGrant: { PolarisPrivilege privilege = PolarisPrivilege.valueOf(catalogGrant.getPrivilege().toString()); - adminService.revokePrivilegeOnCatalogFromRole(catalogName, catalogRoleName, privilege); + result = + adminService.revokePrivilegeOnCatalogFromRole( + catalogName, catalogRoleName, privilege); break; } case PolicyGrant policyGrant: @@ -714,11 +749,12 @@ public Response revokeGrantFromCatalogRole( PolarisPrivilege.valueOf(policyGrant.getPrivilege().toString()); String policyName = policyGrant.getPolicyName(); String[] namespaceParts = policyGrant.getNamespace().toArray(new String[0]); - adminService.revokePrivilegeOnPolicyFromRole( - catalogName, - catalogRoleName, - new PolicyIdentifier(Namespace.of(namespaceParts), policyName), - privilege); + result = + adminService.revokePrivilegeOnPolicyFromRole( + catalogName, + catalogRoleName, + new PolicyIdentifier(Namespace.of(namespaceParts), policyName), + privilege); break; } default: @@ -729,7 +765,7 @@ public Response revokeGrantFromCatalogRole( .log("Don't know how to handle privilege revocation: {}", grantRequest); return Response.status(Response.Status.BAD_REQUEST).build(); } - return Response.status(Response.Status.CREATED).build(); + return toResponse(result, Response.Status.CREATED); } /** From PolarisCatalogsApiService */ 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 8e6b92f84..e6d256eca 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 @@ -35,7 +35,7 @@ import org.apache.polaris.core.entity.PolarisPrivilege; import org.apache.polaris.core.entity.PrincipalEntity; import org.apache.polaris.core.entity.PrincipalRoleEntity; -import org.assertj.core.api.Assertions; +import org.apache.polaris.core.persistence.dao.entity.PrivilegeResult; import org.junit.jupiter.api.Test; @QuarkusTest @@ -62,10 +62,10 @@ private void doTestSufficientPrivileges( List sufficientPrivileges, Runnable action, Runnable cleanupAction, - Function grantAction, - Function revokeAction) { + Function grantAction, + Function revokeAction) { doTestSufficientPrivilegeSets( - sufficientPrivileges.stream().map(priv -> Set.of(priv)).toList(), + sufficientPrivileges.stream().map(Set::of).toList(), action, cleanupAction, PRINCIPAL_NAME, @@ -76,8 +76,8 @@ private void doTestSufficientPrivileges( private void doTestInsufficientPrivileges( List insufficientPrivileges, Runnable action, - Function grantAction, - Function revokeAction) { + Function grantAction, + Function revokeAction) { doTestInsufficientPrivileges( insufficientPrivileges, PRINCIPAL_NAME, action, grantAction, revokeAction); } @@ -127,10 +127,9 @@ public void testListCatalogsInsufficientPrivileges() { @Test public void testCreateCatalogSufficientPrivileges() { // Cleanup with PRINCIPAL_ROLE2 - Assertions.assertThat( - adminService.grantPrivilegeOnRootContainerToPrincipalRole( - PRINCIPAL_ROLE2, PolarisPrivilege.CATALOG_DROP)) - .isTrue(); + assertSuccess( + adminService.grantPrivilegeOnRootContainerToPrincipalRole( + PRINCIPAL_ROLE2, PolarisPrivilege.CATALOG_DROP)); final CatalogEntity newCatalog = new CatalogEntity.Builder().setName("new_catalog").build(); final CreateCatalogRequest createRequest = new CreateCatalogRequest(newCatalog.asCatalog()); @@ -282,10 +281,9 @@ public void testUpdateCatalogInsufficientPrivileges() { @Test public void testDeleteCatalogSufficientPrivileges() { // Cleanup with PRINCIPAL_ROLE2 - Assertions.assertThat( - adminService.grantPrivilegeOnRootContainerToPrincipalRole( - PRINCIPAL_ROLE2, PolarisPrivilege.CATALOG_CREATE)) - .isTrue(); + assertSuccess( + adminService.grantPrivilegeOnRootContainerToPrincipalRole( + PRINCIPAL_ROLE2, PolarisPrivilege.CATALOG_CREATE)); final CatalogEntity newCatalog = new CatalogEntity.Builder().setName("new_catalog").build(); final CreateCatalogRequest createRequest = new CreateCatalogRequest(newCatalog.asCatalog()); adminService.createCatalog(createRequest); @@ -375,10 +373,9 @@ public void testListPrincipalsInsufficientPrivileges() { @Test public void testCreatePrincipalSufficientPrivileges() { // Cleanup with PRINCIPAL_ROLE2 - Assertions.assertThat( - adminService.grantPrivilegeOnRootContainerToPrincipalRole( - PRINCIPAL_ROLE2, PolarisPrivilege.PRINCIPAL_DROP)) - .isTrue(); + assertSuccess( + adminService.grantPrivilegeOnRootContainerToPrincipalRole( + PRINCIPAL_ROLE2, PolarisPrivilege.PRINCIPAL_DROP)); final PrincipalEntity newPrincipal = new PrincipalEntity.Builder().setName("new_principal").build(); @@ -521,10 +518,9 @@ public void testUpdatePrincipalInsufficientPrivileges() { @Test public void testDeletePrincipalSufficientPrivileges() { // Cleanup with PRINCIPAL_ROLE2 - Assertions.assertThat( - adminService.grantPrivilegeOnRootContainerToPrincipalRole( - PRINCIPAL_ROLE2, PolarisPrivilege.PRINCIPAL_CREATE)) - .isTrue(); + assertSuccess( + adminService.grantPrivilegeOnRootContainerToPrincipalRole( + PRINCIPAL_ROLE2, PolarisPrivilege.PRINCIPAL_CREATE)); final PrincipalEntity newPrincipal = new PrincipalEntity.Builder().setName("new_principal").build(); adminService.createPrincipal(newPrincipal); @@ -611,10 +607,9 @@ public void testListPrincipalRolesInsufficientPrivileges() { @Test public void testCreatePrincipalRoleSufficientPrivileges() { // Cleanup with PRINCIPAL_ROLE2 - Assertions.assertThat( - adminService.grantPrivilegeOnRootContainerToPrincipalRole( - PRINCIPAL_ROLE2, PolarisPrivilege.PRINCIPAL_ROLE_DROP)) - .isTrue(); + assertSuccess( + adminService.grantPrivilegeOnRootContainerToPrincipalRole( + PRINCIPAL_ROLE2, PolarisPrivilege.PRINCIPAL_ROLE_DROP)); final PrincipalRoleEntity newPrincipalRole = new PrincipalRoleEntity.Builder().setName("new_principal_role").build(); @@ -759,10 +754,9 @@ public void testUpdatePrincipalRoleInsufficientPrivileges() { @Test public void testDeletePrincipalRoleSufficientPrivileges() { // Cleanup with PRINCIPAL_ROLE2 - Assertions.assertThat( - adminService.grantPrivilegeOnRootContainerToPrincipalRole( - PRINCIPAL_ROLE2, PolarisPrivilege.PRINCIPAL_ROLE_CREATE)) - .isTrue(); + assertSuccess( + adminService.grantPrivilegeOnRootContainerToPrincipalRole( + PRINCIPAL_ROLE2, PolarisPrivilege.PRINCIPAL_ROLE_CREATE)); final PrincipalRoleEntity newPrincipalRole = new PrincipalRoleEntity.Builder().setName("new_principal_role").build(); adminService.createPrincipalRole(newPrincipalRole); @@ -852,10 +846,9 @@ public void testListCatalogRolesInsufficientPrivileges() { @Test public void testCreateCatalogRoleSufficientPrivileges() { // Cleanup with CATALOG_ROLE2 - Assertions.assertThat( - adminService.grantPrivilegeOnCatalogToRole( - CATALOG_NAME, CATALOG_ROLE2, PolarisPrivilege.CATALOG_ROLE_DROP)) - .isTrue(); + assertSuccess( + adminService.grantPrivilegeOnCatalogToRole( + CATALOG_NAME, CATALOG_ROLE2, PolarisPrivilege.CATALOG_ROLE_DROP)); final CatalogRoleEntity newCatalogRole = new CatalogRoleEntity.Builder().setName("new_catalog_role").build(); @@ -1005,10 +998,9 @@ public void testUpdateCatalogRoleInsufficientPrivileges() { @Test public void testDeleteCatalogRoleSufficientPrivileges() { // Cleanup with CATALOG_ROLE2 - Assertions.assertThat( - adminService.grantPrivilegeOnCatalogToRole( - CATALOG_NAME, CATALOG_ROLE2, PolarisPrivilege.CATALOG_ROLE_CREATE)) - .isTrue(); + assertSuccess( + adminService.grantPrivilegeOnCatalogToRole( + CATALOG_NAME, CATALOG_ROLE2, PolarisPrivilege.CATALOG_ROLE_CREATE)); final CatalogRoleEntity newCatalogRole = new CatalogRoleEntity.Builder().setName("new_catalog_role").build(); adminService.createCatalogRole(CATALOG_NAME, newCatalogRole); diff --git a/runtime/service/src/test/java/org/apache/polaris/service/admin/PolarisAdminServiceTest.java b/runtime/service/src/test/java/org/apache/polaris/service/admin/PolarisAdminServiceTest.java index c5744a8de..24a2476bf 100644 --- a/runtime/service/src/test/java/org/apache/polaris/service/admin/PolarisAdminServiceTest.java +++ b/runtime/service/src/test/java/org/apache/polaris/service/admin/PolarisAdminServiceTest.java @@ -18,7 +18,6 @@ */ package org.apache.polaris.service.admin; -import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatThrownBy; import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.eq; @@ -39,12 +38,14 @@ import org.apache.polaris.core.entity.PolarisPrivilege; import org.apache.polaris.core.persistence.PolarisMetaStoreManager; import org.apache.polaris.core.persistence.PolarisResolvedPathWrapper; +import org.apache.polaris.core.persistence.dao.entity.BaseResult; import org.apache.polaris.core.persistence.dao.entity.PrivilegeResult; import org.apache.polaris.core.persistence.resolver.PolarisResolutionManifest; import org.apache.polaris.core.persistence.resolver.ResolutionManifestFactory; import org.apache.polaris.core.persistence.resolver.ResolverStatus; import org.apache.polaris.core.secrets.UserSecretsManager; import org.apache.polaris.service.config.ReservedProperties; +import org.assertj.core.api.Assertions; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.mockito.Mock; @@ -85,6 +86,10 @@ void setUp() throws Exception { reservedProperties); } + protected static void assertSuccess(BaseResult result) { + Assertions.assertThat(result.isSuccess()).isTrue(); + } + @Test void testGrantPrivilegeOnNamespaceToRole() throws Exception { String catalogName = "test-catalog"; @@ -99,11 +104,11 @@ void testGrantPrivilegeOnNamespaceToRole() throws Exception { when(metaStoreManager.grantPrivilegeOnSecurableToRole(any(), any(), any(), any(), any())) .thenReturn(successResult); - boolean result = + PrivilegeResult result = adminService.grantPrivilegeOnNamespaceToRole( catalogName, catalogRoleName, namespace, privilege); - assertThat(result).isTrue(); + assertSuccess(result); } @Test @@ -180,11 +185,11 @@ void testRevokePrivilegeOnNamespaceFromRole() throws Exception { when(metaStoreManager.revokePrivilegeOnSecurableFromRole(any(), any(), any(), any(), any())) .thenReturn(successResult); - boolean result = + PrivilegeResult result = adminService.revokePrivilegeOnNamespaceFromRole( catalogName, catalogRoleName, namespace, privilege); - assertThat(result).isTrue(); + assertSuccess(result); } @Test diff --git a/runtime/service/src/test/java/org/apache/polaris/service/admin/PolarisAuthzTestBase.java b/runtime/service/src/test/java/org/apache/polaris/service/admin/PolarisAuthzTestBase.java index 5e2b349d1..e057dcc49 100644 --- a/runtime/service/src/test/java/org/apache/polaris/service/admin/PolarisAuthzTestBase.java +++ b/runtime/service/src/test/java/org/apache/polaris/service/admin/PolarisAuthzTestBase.java @@ -66,7 +66,9 @@ import org.apache.polaris.core.entity.PrincipalRoleEntity; import org.apache.polaris.core.persistence.MetaStoreManagerFactory; import org.apache.polaris.core.persistence.PolarisMetaStoreManager; +import org.apache.polaris.core.persistence.dao.entity.BaseResult; import org.apache.polaris.core.persistence.dao.entity.EntityResult; +import org.apache.polaris.core.persistence.dao.entity.PrivilegeResult; import org.apache.polaris.core.persistence.resolver.PolarisResolutionManifest; import org.apache.polaris.core.persistence.resolver.ResolutionManifestFactory; import org.apache.polaris.core.persistence.resolver.ResolverFactory; @@ -288,15 +290,21 @@ public void before(TestInfo testInfo) { adminService.createCatalogRole( CATALOG_NAME, new CatalogRoleEntity.Builder().setName(CATALOG_ROLE_SHARED).build()); - adminService.assignPrincipalRole(PRINCIPAL_NAME, PRINCIPAL_ROLE1); - adminService.assignPrincipalRole(PRINCIPAL_NAME, PRINCIPAL_ROLE2); - - adminService.assignCatalogRoleToPrincipalRole(PRINCIPAL_ROLE1, CATALOG_NAME, CATALOG_ROLE1); - adminService.assignCatalogRoleToPrincipalRole(PRINCIPAL_ROLE2, CATALOG_NAME, CATALOG_ROLE2); - adminService.assignCatalogRoleToPrincipalRole( - PRINCIPAL_ROLE1, CATALOG_NAME, CATALOG_ROLE_SHARED); - adminService.assignCatalogRoleToPrincipalRole( - PRINCIPAL_ROLE2, CATALOG_NAME, CATALOG_ROLE_SHARED); + assertSuccess(adminService.assignPrincipalRole(PRINCIPAL_NAME, PRINCIPAL_ROLE1)); + assertSuccess(adminService.assignPrincipalRole(PRINCIPAL_NAME, PRINCIPAL_ROLE2)); + + assertSuccess( + adminService.assignCatalogRoleToPrincipalRole( + PRINCIPAL_ROLE1, CATALOG_NAME, CATALOG_ROLE1)); + assertSuccess( + adminService.assignCatalogRoleToPrincipalRole( + PRINCIPAL_ROLE2, CATALOG_NAME, CATALOG_ROLE2)); + assertSuccess( + adminService.assignCatalogRoleToPrincipalRole( + PRINCIPAL_ROLE1, CATALOG_NAME, CATALOG_ROLE_SHARED)); + assertSuccess( + adminService.assignCatalogRoleToPrincipalRole( + PRINCIPAL_ROLE2, CATALOG_NAME, CATALOG_ROLE_SHARED)); // Do some shared setup with non-authz-aware baseCatalog. baseCatalog.createNamespace(NS1); @@ -368,6 +376,10 @@ public void after() { } } + protected static void assertSuccess(BaseResult result) { + Assertions.assertThat(result.isSuccess()).isTrue(); + } + protected @Nonnull SecurityContext securityContext( AuthenticatedPolarisPrincipal p, Set roles) { SecurityContext securityContext = Mockito.mock(SecurityContext.class); @@ -522,12 +534,12 @@ protected void doTestSufficientPrivilegeSets( Runnable action, Runnable cleanupAction, String principalName, - Function grantAction, - Function revokeAction) { + Function grantAction, + Function revokeAction) { for (Set privilegeSet : sufficientPrivileges) { for (PolarisPrivilege privilege : privilegeSet) { // Grant the single privilege at a catalog level to cascade to all objects. - Assertions.assertThat(grantAction.apply(privilege)).isTrue(); + assertSuccess(grantAction.apply(privilege)); } // Should run without issues. @@ -556,7 +568,7 @@ protected void doTestSufficientPrivilegeSets( // Knockout testing - Revoke single privileges and the same action should throw // NotAuthorizedException. for (PolarisPrivilege privilege : privilegeSet) { - Assertions.assertThat(revokeAction.apply(privilege)).isTrue(); + assertSuccess(revokeAction.apply(privilege)); try { Assertions.assertThatThrownBy(() -> action.run()) @@ -572,13 +584,13 @@ protected void doTestSufficientPrivilegeSets( } // Grant the single privilege at a catalog level to cascade to all objects. - Assertions.assertThat(grantAction.apply(privilege)).isTrue(); + assertSuccess(grantAction.apply(privilege)); } } // Now remove all the privileges for (PolarisPrivilege privilege : privilegeSet) { - Assertions.assertThat(revokeAction.apply(privilege)).isTrue(); + assertSuccess(revokeAction.apply(privilege)); } try { Assertions.assertThatThrownBy(() -> action.run()) @@ -602,10 +614,10 @@ protected void doTestInsufficientPrivileges( List insufficientPrivileges, String principalName, Runnable action, - Function grantAction, - Function revokeAction) { + Function grantAction, + Function revokeAction) { doTestInsufficientPrivilegeSets( - insufficientPrivileges.stream().map(priv -> Set.of(priv)).toList(), + insufficientPrivileges.stream().map(Set::of).toList(), principalName, action, grantAction, @@ -620,16 +632,16 @@ protected void doTestInsufficientPrivilegeSets( List> insufficientPrivilegeSets, String principalName, Runnable action, - Function grantAction, - Function revokeAction) { + Function grantAction, + Function revokeAction) { for (Set privilegeSet : insufficientPrivilegeSets) { for (PolarisPrivilege privilege : privilegeSet) { // Grant the single privilege at a catalog level to cascade to all objects. - Assertions.assertThat(grantAction.apply(privilege)).isTrue(); + assertSuccess(grantAction.apply(privilege)); // Should be insufficient try { - Assertions.assertThatThrownBy(() -> action.run()) + Assertions.assertThatThrownBy(action::run) .isInstanceOf(ForbiddenException.class) .hasMessageContaining(principalName) .hasMessageContaining("is not authorized"); @@ -640,7 +652,7 @@ protected void doTestInsufficientPrivilegeSets( // Revoking only matters in case there are some multi-privilege actions being tested with // only granting individual privileges in isolation. - Assertions.assertThat(revokeAction.apply(privilege)).isTrue(); + assertSuccess(revokeAction.apply(privilege)); } } } diff --git a/runtime/service/src/test/java/org/apache/polaris/service/catalog/IcebergCatalogHandlerAuthzTest.java b/runtime/service/src/test/java/org/apache/polaris/service/catalog/IcebergCatalogHandlerAuthzTest.java index ea963784b..ed7befc5c 100644 --- a/runtime/service/src/test/java/org/apache/polaris/service/catalog/IcebergCatalogHandlerAuthzTest.java +++ b/runtime/service/src/test/java/org/apache/polaris/service/catalog/IcebergCatalogHandlerAuthzTest.java @@ -121,10 +121,7 @@ private IcebergCatalogHandler newWrapper( private void doTestSufficientPrivileges( List sufficientPrivileges, Runnable action, Runnable cleanupAction) { doTestSufficientPrivilegeSets( - sufficientPrivileges.stream().map(priv -> Set.of(priv)).toList(), - action, - cleanupAction, - PRINCIPAL_NAME); + sufficientPrivileges.stream().map(Set::of).toList(), action, cleanupAction, PRINCIPAL_NAME); } /** @@ -301,10 +298,9 @@ public void testInsufficientPermissionsPriorToSecretRotation() { @Test public void testListNamespacesCatalogLevelWithPrincipalRoleActivation() { // Grant catalog-level privilege to CATALOG_ROLE1 - Assertions.assertThat( - adminService.grantPrivilegeOnCatalogToRole( - CATALOG_NAME, CATALOG_ROLE1, PolarisPrivilege.NAMESPACE_LIST)) - .isTrue(); + assertSuccess( + adminService.grantPrivilegeOnCatalogToRole( + CATALOG_NAME, CATALOG_ROLE1, PolarisPrivilege.NAMESPACE_LIST)); Assertions.assertThat(newWrapper().listNamespaces(Namespace.of()).namespaces()) .containsAll(List.of(NS1, NS2)); @@ -320,10 +316,9 @@ public void testListNamespacesCatalogLevelWithPrincipalRoleActivation() { .hasMessageContaining("is not authorized"); // If we revoke, then it should fail again even with all principal roles activated. - Assertions.assertThat( - adminService.revokePrivilegeOnCatalogFromRole( - CATALOG_NAME, CATALOG_ROLE1, PolarisPrivilege.NAMESPACE_LIST)) - .isTrue(); + assertSuccess( + adminService.revokePrivilegeOnCatalogFromRole( + CATALOG_NAME, CATALOG_ROLE1, PolarisPrivilege.NAMESPACE_LIST)); Assertions.assertThatThrownBy(() -> newWrapper().listNamespaces(Namespace.of())) .isInstanceOf(ForbiddenException.class); } @@ -331,10 +326,9 @@ public void testListNamespacesCatalogLevelWithPrincipalRoleActivation() { @Test public void testListNamespacesChildOnly() { // Grant only NS1-level privilege to CATALOG_ROLE1 - Assertions.assertThat( - adminService.grantPrivilegeOnNamespaceToRole( - CATALOG_NAME, CATALOG_ROLE1, NS1, PolarisPrivilege.NAMESPACE_LIST)) - .isTrue(); + assertSuccess( + adminService.grantPrivilegeOnNamespaceToRole( + CATALOG_NAME, CATALOG_ROLE1, NS1, PolarisPrivilege.NAMESPACE_LIST)); // Listing directly on NS1 succeeds Assertions.assertThat(newWrapper().listNamespaces(NS1).namespaces()) @@ -355,10 +349,9 @@ public void testListNamespacesChildOnly() { @Test public void testCreateNamespaceAllSufficientPrivileges() { - Assertions.assertThat( - adminService.grantPrivilegeOnCatalogToRole( - CATALOG_NAME, CATALOG_ROLE2, PolarisPrivilege.NAMESPACE_DROP)) - .isTrue(); + assertSuccess( + adminService.grantPrivilegeOnCatalogToRole( + CATALOG_NAME, CATALOG_ROLE2, PolarisPrivilege.NAMESPACE_DROP)); // Use PRINCIPAL_ROLE1 for privilege-testing, PRINCIPAL_ROLE2 for cleanup. doTestSufficientPrivileges( @@ -452,10 +445,9 @@ public void testNamespaceExistsInsufficientPermissions() { @Test public void testDropNamespaceSufficientPrivileges() { - Assertions.assertThat( - adminService.grantPrivilegeOnCatalogToRole( - CATALOG_NAME, CATALOG_ROLE2, PolarisPrivilege.NAMESPACE_CREATE)) - .isTrue(); + assertSuccess( + adminService.grantPrivilegeOnCatalogToRole( + CATALOG_NAME, CATALOG_ROLE2, PolarisPrivilege.NAMESPACE_CREATE)); // Use PRINCIPAL_ROLE1 for privilege-testing, PRINCIPAL_ROLE2 for cleanup. doTestSufficientPrivileges( @@ -547,14 +539,12 @@ public void testListTablesInsufficientPermissions() { @Test public void testCreateTableDirectAllSufficientPrivileges() { - Assertions.assertThat( - adminService.grantPrivilegeOnCatalogToRole( - CATALOG_NAME, CATALOG_ROLE2, PolarisPrivilege.TABLE_DROP)) - .isTrue(); - Assertions.assertThat( - adminService.grantPrivilegeOnCatalogToRole( - CATALOG_NAME, CATALOG_ROLE2, PolarisPrivilege.TABLE_WRITE_DATA)) - .isTrue(); + assertSuccess( + adminService.grantPrivilegeOnCatalogToRole( + CATALOG_NAME, CATALOG_ROLE2, PolarisPrivilege.TABLE_DROP)); + assertSuccess( + adminService.grantPrivilegeOnCatalogToRole( + CATALOG_NAME, CATALOG_ROLE2, PolarisPrivilege.TABLE_WRITE_DATA)); final TableIdentifier newtable = TableIdentifier.of(NS2, "newtable"); final CreateTableRequest createRequest = @@ -596,14 +586,12 @@ public void testCreateTableDirectInsufficientPermissions() { @Test public void testCreateTableDirectWithWriteDelegationAllSufficientPrivileges() { - Assertions.assertThat( - adminService.grantPrivilegeOnCatalogToRole( - CATALOG_NAME, CATALOG_ROLE2, PolarisPrivilege.TABLE_DROP)) - .isTrue(); - Assertions.assertThat( - adminService.grantPrivilegeOnCatalogToRole( - CATALOG_NAME, CATALOG_ROLE2, PolarisPrivilege.TABLE_WRITE_DATA)) - .isTrue(); + assertSuccess( + adminService.grantPrivilegeOnCatalogToRole( + CATALOG_NAME, CATALOG_ROLE2, PolarisPrivilege.TABLE_DROP)); + assertSuccess( + adminService.grantPrivilegeOnCatalogToRole( + CATALOG_NAME, CATALOG_ROLE2, PolarisPrivilege.TABLE_WRITE_DATA)); final TableIdentifier newtable = TableIdentifier.of(NS2, "newtable"); final CreateTableRequest createDirectWithWriteDelegationRequest = @@ -651,10 +639,9 @@ public void testCreateTableDirectWithWriteDelegationInsufficientPermissions() { @Test public void testCreateTableStagedAllSufficientPrivileges() { - Assertions.assertThat( - adminService.grantPrivilegeOnCatalogToRole( - CATALOG_NAME, CATALOG_ROLE2, PolarisPrivilege.TABLE_DROP)) - .isTrue(); + assertSuccess( + adminService.grantPrivilegeOnCatalogToRole( + CATALOG_NAME, CATALOG_ROLE2, PolarisPrivilege.TABLE_DROP)); final CreateTableRequest createStagedRequest = CreateTableRequest.builder() @@ -702,10 +689,9 @@ public void testCreateTableStagedInsufficientPermissions() { @Test public void testCreateTableStagedWithWriteDelegationAllSufficientPrivileges() { - Assertions.assertThat( - adminService.grantPrivilegeOnCatalogToRole( - CATALOG_NAME, CATALOG_ROLE2, PolarisPrivilege.TABLE_DROP)) - .isTrue(); + assertSuccess( + adminService.grantPrivilegeOnCatalogToRole( + CATALOG_NAME, CATALOG_ROLE2, PolarisPrivilege.TABLE_DROP)); final CreateTableRequest createStagedWithWriteDelegationRequest = CreateTableRequest.builder() @@ -755,14 +741,12 @@ public void testCreateTableStagedWithWriteDelegationInsufficientPermissions() { @Test public void testRegisterTableAllSufficientPrivileges() { - Assertions.assertThat( - adminService.grantPrivilegeOnCatalogToRole( - CATALOG_NAME, CATALOG_ROLE2, PolarisPrivilege.TABLE_DROP)) - .isTrue(); - Assertions.assertThat( - adminService.grantPrivilegeOnCatalogToRole( - CATALOG_NAME, CATALOG_ROLE2, PolarisPrivilege.TABLE_READ_PROPERTIES)) - .isTrue(); + assertSuccess( + adminService.grantPrivilegeOnCatalogToRole( + CATALOG_NAME, CATALOG_ROLE2, PolarisPrivilege.TABLE_DROP)); + assertSuccess( + adminService.grantPrivilegeOnCatalogToRole( + CATALOG_NAME, CATALOG_ROLE2, PolarisPrivilege.TABLE_READ_PROPERTIES)); // To get a handy metadata file we can use one from another table. // to avoid overlapping directories, drop the original table and recreate it via registerTable @@ -798,10 +782,9 @@ public String metadataLocation() { @Test public void testRegisterTableInsufficientPermissions() { - Assertions.assertThat( - adminService.grantPrivilegeOnCatalogToRole( - CATALOG_NAME, CATALOG_ROLE2, PolarisPrivilege.TABLE_READ_PROPERTIES)) - .isTrue(); + assertSuccess( + adminService.grantPrivilegeOnCatalogToRole( + CATALOG_NAME, CATALOG_ROLE2, PolarisPrivilege.TABLE_READ_PROPERTIES)); // To get a handy metadata file we can use one from another table. final String metadataLocation = newWrapper().loadTable(TABLE_NS1_1, "all").metadataLocation(); @@ -1069,10 +1052,9 @@ public void testUpdateTableForStagedCreateInsufficientPermissions() { @Test public void testDropTableWithoutPurgeAllSufficientPrivileges() { - Assertions.assertThat( - adminService.grantPrivilegeOnCatalogToRole( - CATALOG_NAME, CATALOG_ROLE2, PolarisPrivilege.TABLE_CREATE)) - .isTrue(); + assertSuccess( + adminService.grantPrivilegeOnCatalogToRole( + CATALOG_NAME, CATALOG_ROLE2, PolarisPrivilege.TABLE_CREATE)); final CreateTableRequest createRequest = CreateTableRequest.builder().withName(TABLE_NS1_1.name()).withSchema(SCHEMA).build(); @@ -1111,10 +1093,9 @@ public void testDropTableWithoutPurgeInsufficientPermissions() { @Test public void testDropTableWithPurgeAllSufficientPrivileges() { - Assertions.assertThat( - adminService.grantPrivilegeOnCatalogToRole( - CATALOG_NAME, CATALOG_ROLE2, PolarisPrivilege.TABLE_CREATE)) - .isTrue(); + assertSuccess( + adminService.grantPrivilegeOnCatalogToRole( + CATALOG_NAME, CATALOG_ROLE2, PolarisPrivilege.TABLE_CREATE)); final CreateTableRequest createRequest = CreateTableRequest.builder().withName(TABLE_NS1_1.name()).withSchema(SCHEMA).build(); @@ -1236,14 +1217,12 @@ public void testRenameTablePrivilegesOnWrongSourceOrDestination() { RenameTableRequest.builder().withSource(dstTable).withDestination(srcTable).build(); // Minimum privileges should succeed -- drop on src, create on dst parent. - Assertions.assertThat( - adminService.grantPrivilegeOnTableToRole( - CATALOG_NAME, CATALOG_ROLE1, srcTable, PolarisPrivilege.TABLE_DROP)) - .isTrue(); - Assertions.assertThat( - adminService.grantPrivilegeOnNamespaceToRole( - CATALOG_NAME, CATALOG_ROLE1, dstTable.namespace(), PolarisPrivilege.TABLE_CREATE)) - .isTrue(); + assertSuccess( + adminService.grantPrivilegeOnTableToRole( + CATALOG_NAME, CATALOG_ROLE1, srcTable, PolarisPrivilege.TABLE_DROP)); + assertSuccess( + adminService.grantPrivilegeOnNamespaceToRole( + CATALOG_NAME, CATALOG_ROLE1, dstTable.namespace(), PolarisPrivilege.TABLE_CREATE)); // Initial rename should succeed newWrapper().renameTable(rename1); @@ -1253,30 +1232,27 @@ public void testRenameTablePrivilegesOnWrongSourceOrDestination() { .isInstanceOf(ForbiddenException.class); // Now grant TABLE_DROP on dst - Assertions.assertThat( - adminService.grantPrivilegeOnTableToRole( - CATALOG_NAME, CATALOG_ROLE1, dstTable, PolarisPrivilege.TABLE_DROP)) - .isTrue(); + assertSuccess( + adminService.grantPrivilegeOnTableToRole( + CATALOG_NAME, CATALOG_ROLE1, dstTable, PolarisPrivilege.TABLE_DROP)); // Still not enough without TABLE_CREATE at source Assertions.assertThatThrownBy(() -> newWrapper().renameTable(rename2)) .isInstanceOf(ForbiddenException.class); // Even grant CATALOG_MANAGE_CONTENT under all of NS1 - Assertions.assertThat( - adminService.grantPrivilegeOnNamespaceToRole( - CATALOG_NAME, CATALOG_ROLE1, NS1, PolarisPrivilege.CATALOG_MANAGE_CONTENT)) - .isTrue(); + assertSuccess( + adminService.grantPrivilegeOnNamespaceToRole( + CATALOG_NAME, CATALOG_ROLE1, NS1, PolarisPrivilege.CATALOG_MANAGE_CONTENT)); // Still not enough to rename back to src since src was NS2. Assertions.assertThatThrownBy(() -> newWrapper().renameTable(rename2)) .isInstanceOf(ForbiddenException.class); // Finally, grant TABLE_CREATE on NS2 and it should succeed to rename back to src. - Assertions.assertThat( - adminService.grantPrivilegeOnNamespaceToRole( - CATALOG_NAME, CATALOG_ROLE1, NS2, PolarisPrivilege.TABLE_CREATE)) - .isTrue(); + assertSuccess( + adminService.grantPrivilegeOnNamespaceToRole( + CATALOG_NAME, CATALOG_ROLE1, NS2, PolarisPrivilege.TABLE_CREATE)); newWrapper().renameTable(rename2); } @@ -1336,42 +1312,37 @@ public void testCommitTransactionMixedPermissions() { UpdateTableRequest.create(TABLE_NS2_1, List.of(), List.of()))); // Grant TABLE_CREATE for all of NS1 - Assertions.assertThat( - adminService.grantPrivilegeOnNamespaceToRole( - CATALOG_NAME, CATALOG_ROLE1, NS1, PolarisPrivilege.TABLE_CREATE)) - .isTrue(); + assertSuccess( + adminService.grantPrivilegeOnNamespaceToRole( + CATALOG_NAME, CATALOG_ROLE1, NS1, PolarisPrivilege.TABLE_CREATE)); Assertions.assertThatThrownBy(() -> newWrapper().commitTransaction(req)) .isInstanceOf(ForbiddenException.class); // Grant TABLE_FULL_METADATA directly on TABLE_NS1_1 - Assertions.assertThat( - adminService.grantPrivilegeOnTableToRole( - CATALOG_NAME, CATALOG_ROLE1, TABLE_NS1_1, PolarisPrivilege.TABLE_FULL_METADATA)) - .isTrue(); + assertSuccess( + adminService.grantPrivilegeOnTableToRole( + CATALOG_NAME, CATALOG_ROLE1, TABLE_NS1_1, PolarisPrivilege.TABLE_FULL_METADATA)); Assertions.assertThatThrownBy(() -> newWrapper().commitTransaction(req)) .isInstanceOf(ForbiddenException.class); // Grant TABLE_WRITE_PROPERTIES on NS1A namespace - Assertions.assertThat( - adminService.grantPrivilegeOnNamespaceToRole( - CATALOG_NAME, CATALOG_ROLE1, NS1A, PolarisPrivilege.TABLE_WRITE_PROPERTIES)) - .isTrue(); + assertSuccess( + adminService.grantPrivilegeOnNamespaceToRole( + CATALOG_NAME, CATALOG_ROLE1, NS1A, PolarisPrivilege.TABLE_WRITE_PROPERTIES)); Assertions.assertThatThrownBy(() -> newWrapper().commitTransaction(req)) .isInstanceOf(ForbiddenException.class); // Grant TABLE_WRITE_DATA directly on TABLE_NS1B_1 - Assertions.assertThat( - adminService.grantPrivilegeOnTableToRole( - CATALOG_NAME, CATALOG_ROLE1, TABLE_NS1B_1, PolarisPrivilege.TABLE_WRITE_DATA)) - .isTrue(); + assertSuccess( + adminService.grantPrivilegeOnTableToRole( + CATALOG_NAME, CATALOG_ROLE1, TABLE_NS1B_1, PolarisPrivilege.TABLE_WRITE_DATA)); Assertions.assertThatThrownBy(() -> newWrapper().commitTransaction(req)) .isInstanceOf(ForbiddenException.class); // Grant TABLE_WRITE_PROPERTIES directly on TABLE_NS2_1 - Assertions.assertThat( - adminService.grantPrivilegeOnTableToRole( - CATALOG_NAME, CATALOG_ROLE1, TABLE_NS2_1, PolarisPrivilege.TABLE_WRITE_PROPERTIES)) - .isTrue(); + assertSuccess( + adminService.grantPrivilegeOnTableToRole( + CATALOG_NAME, CATALOG_ROLE1, TABLE_NS2_1, PolarisPrivilege.TABLE_WRITE_PROPERTIES)); Assertions.assertThatThrownBy(() -> newWrapper().commitTransaction(req)) .isInstanceOf(ForbiddenException.class); @@ -1379,10 +1350,9 @@ public void testCommitTransactionMixedPermissions() { // TODO: If we end up having fine-grained differentiation between updateForStagedCreate // and update, then this one should only be TABLE_CREATE on the *parent* of this last table // and the table shouldn't exist. - Assertions.assertThat( - adminService.grantPrivilegeOnTableToRole( - CATALOG_NAME, CATALOG_ROLE1, TABLE_NS2_1, PolarisPrivilege.TABLE_CREATE)) - .isTrue(); + assertSuccess( + adminService.grantPrivilegeOnTableToRole( + CATALOG_NAME, CATALOG_ROLE1, TABLE_NS2_1, PolarisPrivilege.TABLE_CREATE)); newWrapper().commitTransaction(req); } @@ -1412,10 +1382,9 @@ public void testListViewsInsufficientPermissions() { @Test public void testCreateViewAllSufficientPrivileges() { - Assertions.assertThat( - adminService.grantPrivilegeOnCatalogToRole( - CATALOG_NAME, CATALOG_ROLE2, PolarisPrivilege.VIEW_DROP)) - .isTrue(); + assertSuccess( + adminService.grantPrivilegeOnCatalogToRole( + CATALOG_NAME, CATALOG_ROLE2, PolarisPrivilege.VIEW_DROP)); final TableIdentifier newview = TableIdentifier.of(NS2, "newview"); final CreateViewRequest createRequest = @@ -1533,10 +1502,9 @@ public void testUpdateViewInsufficientPermissions() { @Test public void testDropViewAllSufficientPrivileges() { - Assertions.assertThat( - adminService.grantPrivilegeOnCatalogToRole( - CATALOG_NAME, CATALOG_ROLE2, PolarisPrivilege.VIEW_CREATE)) - .isTrue(); + assertSuccess( + adminService.grantPrivilegeOnCatalogToRole( + CATALOG_NAME, CATALOG_ROLE2, PolarisPrivilege.VIEW_CREATE)); final CreateViewRequest createRequest = ImmutableCreateViewRequest.builder() @@ -1663,14 +1631,12 @@ public void testRenameViewPrivilegesOnWrongSourceOrDestination() { RenameTableRequest.builder().withSource(dstView).withDestination(srcView).build(); // Minimum privileges should succeed -- drop on src, create on dst parent. - Assertions.assertThat( - adminService.grantPrivilegeOnViewToRole( - CATALOG_NAME, CATALOG_ROLE1, srcView, PolarisPrivilege.VIEW_DROP)) - .isTrue(); - Assertions.assertThat( - adminService.grantPrivilegeOnNamespaceToRole( - CATALOG_NAME, CATALOG_ROLE1, dstView.namespace(), PolarisPrivilege.VIEW_CREATE)) - .isTrue(); + assertSuccess( + adminService.grantPrivilegeOnViewToRole( + CATALOG_NAME, CATALOG_ROLE1, srcView, PolarisPrivilege.VIEW_DROP)); + assertSuccess( + adminService.grantPrivilegeOnNamespaceToRole( + CATALOG_NAME, CATALOG_ROLE1, dstView.namespace(), PolarisPrivilege.VIEW_CREATE)); // Initial rename should succeed newWrapper().renameView(rename1); @@ -1680,30 +1646,27 @@ public void testRenameViewPrivilegesOnWrongSourceOrDestination() { .isInstanceOf(ForbiddenException.class); // Now grant VIEW_DROP on dst - Assertions.assertThat( - adminService.grantPrivilegeOnViewToRole( - CATALOG_NAME, CATALOG_ROLE1, dstView, PolarisPrivilege.VIEW_DROP)) - .isTrue(); + assertSuccess( + adminService.grantPrivilegeOnViewToRole( + CATALOG_NAME, CATALOG_ROLE1, dstView, PolarisPrivilege.VIEW_DROP)); // Still not enough without VIEW_CREATE at source Assertions.assertThatThrownBy(() -> newWrapper().renameView(rename2)) .isInstanceOf(ForbiddenException.class); // Even grant CATALOG_MANAGE_CONTENT under all of NS1 - Assertions.assertThat( - adminService.grantPrivilegeOnNamespaceToRole( - CATALOG_NAME, CATALOG_ROLE1, NS1, PolarisPrivilege.CATALOG_MANAGE_CONTENT)) - .isTrue(); + assertSuccess( + adminService.grantPrivilegeOnNamespaceToRole( + CATALOG_NAME, CATALOG_ROLE1, NS1, PolarisPrivilege.CATALOG_MANAGE_CONTENT)); // Still not enough to rename back to src since src was NS2. Assertions.assertThatThrownBy(() -> newWrapper().renameView(rename2)) .isInstanceOf(ForbiddenException.class); // Finally, grant VIEW_CREATE on NS2 and it should succeed to rename back to src. - Assertions.assertThat( - adminService.grantPrivilegeOnNamespaceToRole( - CATALOG_NAME, CATALOG_ROLE1, NS2, PolarisPrivilege.VIEW_CREATE)) - .isTrue(); + assertSuccess( + adminService.grantPrivilegeOnNamespaceToRole( + CATALOG_NAME, CATALOG_ROLE1, NS2, PolarisPrivilege.VIEW_CREATE)); newWrapper().renameView(rename2); } @@ -1734,14 +1697,12 @@ public void testSendNotificationSufficientPrivileges() { adminService.assignPrincipalRole(PRINCIPAL_NAME, PRINCIPAL_ROLE1); adminService.assignCatalogRoleToPrincipalRole(PRINCIPAL_ROLE1, externalCatalog, CATALOG_ROLE1); adminService.assignCatalogRoleToPrincipalRole(PRINCIPAL_ROLE2, externalCatalog, CATALOG_ROLE2); - Assertions.assertThat( - adminService.grantPrivilegeOnCatalogToRole( - externalCatalog, CATALOG_ROLE2, PolarisPrivilege.TABLE_DROP)) - .isTrue(); - Assertions.assertThat( - adminService.grantPrivilegeOnCatalogToRole( - externalCatalog, CATALOG_ROLE2, PolarisPrivilege.NAMESPACE_DROP)) - .isTrue(); + assertSuccess( + adminService.grantPrivilegeOnCatalogToRole( + externalCatalog, CATALOG_ROLE2, PolarisPrivilege.TABLE_DROP)); + assertSuccess( + adminService.grantPrivilegeOnCatalogToRole( + externalCatalog, CATALOG_ROLE2, PolarisPrivilege.NAMESPACE_DROP)); Namespace namespace = Namespace.of("extns1", "extns2"); TableIdentifier table = TableIdentifier.of(namespace, "tbl1"); diff --git a/runtime/service/src/test/java/org/apache/polaris/service/catalog/PolarisGenericTableCatalogHandlerAuthzTest.java b/runtime/service/src/test/java/org/apache/polaris/service/catalog/PolarisGenericTableCatalogHandlerAuthzTest.java index e434f9c7d..715d68140 100644 --- a/runtime/service/src/test/java/org/apache/polaris/service/catalog/PolarisGenericTableCatalogHandlerAuthzTest.java +++ b/runtime/service/src/test/java/org/apache/polaris/service/catalog/PolarisGenericTableCatalogHandlerAuthzTest.java @@ -28,7 +28,6 @@ import org.apache.polaris.core.entity.PolarisPrivilege; import org.apache.polaris.service.admin.PolarisAuthzTestBase; import org.apache.polaris.service.catalog.generic.GenericTableCatalogHandler; -import org.assertj.core.api.Assertions; import org.junit.jupiter.api.Test; @QuarkusTest @@ -73,10 +72,7 @@ private GenericTableCatalogHandler newWrapper( private void doTestSufficientPrivileges( List sufficientPrivileges, Runnable action, Runnable cleanupAction) { doTestSufficientPrivilegeSets( - sufficientPrivileges.stream().map(priv -> Set.of(priv)).toList(), - action, - cleanupAction, - PRINCIPAL_NAME); + sufficientPrivileges.stream().map(Set::of).toList(), action, cleanupAction, PRINCIPAL_NAME); } /** @@ -169,14 +165,12 @@ public void testListGenericTablesInsufficientPermissions() { @Test public void testCreateGenericTableAllSufficientPrivileges() { - Assertions.assertThat( - adminService.grantPrivilegeOnCatalogToRole( - CATALOG_NAME, CATALOG_ROLE2, PolarisPrivilege.TABLE_DROP)) - .isTrue(); - Assertions.assertThat( - adminService.grantPrivilegeOnCatalogToRole( - CATALOG_NAME, CATALOG_ROLE2, PolarisPrivilege.TABLE_WRITE_DATA)) - .isTrue(); + assertSuccess( + adminService.grantPrivilegeOnCatalogToRole( + CATALOG_NAME, CATALOG_ROLE2, PolarisPrivilege.TABLE_DROP)); + assertSuccess( + adminService.grantPrivilegeOnCatalogToRole( + CATALOG_NAME, CATALOG_ROLE2, PolarisPrivilege.TABLE_WRITE_DATA)); final TableIdentifier newtable = TableIdentifier.of(NS2, "newtable"); @@ -242,10 +236,9 @@ public void testLoadTableInsufficientPermissions() { @Test public void testDropGenericTableAllSufficientPrivileges() { - Assertions.assertThat( - adminService.grantPrivilegeOnCatalogToRole( - CATALOG_NAME, CATALOG_ROLE2, PolarisPrivilege.TABLE_CREATE)) - .isTrue(); + assertSuccess( + adminService.grantPrivilegeOnCatalogToRole( + CATALOG_NAME, CATALOG_ROLE2, PolarisPrivilege.TABLE_CREATE)); doTestSufficientPrivileges( List.of( diff --git a/runtime/service/src/test/java/org/apache/polaris/service/catalog/PolicyCatalogHandlerAuthzTest.java b/runtime/service/src/test/java/org/apache/polaris/service/catalog/PolicyCatalogHandlerAuthzTest.java index cd7c4125c..b955a6ec5 100644 --- a/runtime/service/src/test/java/org/apache/polaris/service/catalog/PolicyCatalogHandlerAuthzTest.java +++ b/runtime/service/src/test/java/org/apache/polaris/service/catalog/PolicyCatalogHandlerAuthzTest.java @@ -35,7 +35,6 @@ import org.apache.polaris.service.types.PolicyAttachmentTarget; import org.apache.polaris.service.types.PolicyIdentifier; import org.apache.polaris.service.types.UpdatePolicyRequest; -import org.assertj.core.api.Assertions; import org.junit.jupiter.api.Test; @QuarkusTest @@ -78,10 +77,7 @@ private PolicyCatalogHandler newWrapper(Set activatedPrincipalRoles, Str private void doTestSufficientPrivileges( List sufficientPrivileges, Runnable action, Runnable cleanupAction) { doTestSufficientPrivilegeSets( - sufficientPrivileges.stream().map(priv -> Set.of(priv)).toList(), - action, - cleanupAction, - PRINCIPAL_NAME); + sufficientPrivileges.stream().map(Set::of).toList(), action, cleanupAction, PRINCIPAL_NAME); } /** @@ -199,10 +195,9 @@ public void testListPoliciesInsufficientPrivileges() { @Test public void testCreatePolicyAllSufficientPrivileges() { - Assertions.assertThat( - adminService.grantPrivilegeOnCatalogToRole( - CATALOG_NAME, CATALOG_ROLE2, PolarisPrivilege.POLICY_DROP)) - .isTrue(); + assertSuccess( + adminService.grantPrivilegeOnCatalogToRole( + CATALOG_NAME, CATALOG_ROLE2, PolarisPrivilege.POLICY_DROP)); final PolicyIdentifier newPolicy = new PolicyIdentifier(NS2, "newPolicy"); final CreatePolicyRequest createPolicyRequest = @@ -303,10 +298,9 @@ public void testUpdatePolicyInsufficientPrivileges() { @Test public void testDropPolicyAllSufficientPrivileges() { - Assertions.assertThat( - adminService.grantPrivilegeOnCatalogToRole( - CATALOG_NAME, CATALOG_ROLE2, PolarisPrivilege.POLICY_CREATE)) - .isTrue(); + assertSuccess( + adminService.grantPrivilegeOnCatalogToRole( + CATALOG_NAME, CATALOG_ROLE2, PolarisPrivilege.POLICY_CREATE)); final CreatePolicyRequest createPolicyRequest = CreatePolicyRequest.builder() @@ -341,14 +335,12 @@ public void testDropPolicyInsufficientPrivileges() { @Test public void testAttachPolicyToCatalogSufficientPrivileges() { - Assertions.assertThat( - adminService.grantPrivilegeOnCatalogToRole( - CATALOG_NAME, CATALOG_ROLE2, PolarisPrivilege.POLICY_DETACH)) - .isTrue(); - Assertions.assertThat( - adminService.grantPrivilegeOnCatalogToRole( - CATALOG_NAME, CATALOG_ROLE2, PolarisPrivilege.CATALOG_DETACH_POLICY)) - .isTrue(); + assertSuccess( + adminService.grantPrivilegeOnCatalogToRole( + CATALOG_NAME, CATALOG_ROLE2, PolarisPrivilege.POLICY_DETACH)); + assertSuccess( + adminService.grantPrivilegeOnCatalogToRole( + CATALOG_NAME, CATALOG_ROLE2, PolarisPrivilege.CATALOG_DETACH_POLICY)); PolicyAttachmentTarget namespaceTarget = PolicyAttachmentTarget.builder().setType(PolicyAttachmentTarget.TypeEnum.CATALOG).build(); AttachPolicyRequest attachPolicyRequest = @@ -384,14 +376,12 @@ public void testAttachPolicyToCatalogInsufficientPrivileges() { @Test public void testAttachPolicyToNamespaceSufficientPrivileges() { - Assertions.assertThat( - adminService.grantPrivilegeOnCatalogToRole( - CATALOG_NAME, CATALOG_ROLE2, PolarisPrivilege.POLICY_DETACH)) - .isTrue(); - Assertions.assertThat( - adminService.grantPrivilegeOnCatalogToRole( - CATALOG_NAME, CATALOG_ROLE2, PolarisPrivilege.NAMESPACE_DETACH_POLICY)) - .isTrue(); + assertSuccess( + adminService.grantPrivilegeOnCatalogToRole( + CATALOG_NAME, CATALOG_ROLE2, PolarisPrivilege.POLICY_DETACH)); + assertSuccess( + adminService.grantPrivilegeOnCatalogToRole( + CATALOG_NAME, CATALOG_ROLE2, PolarisPrivilege.NAMESPACE_DETACH_POLICY)); PolicyAttachmentTarget namespaceTarget = PolicyAttachmentTarget.builder() @@ -433,14 +423,12 @@ public void testAttachPolicyToNamespaceInsufficientPrivileges() { @Test public void testAttachPolicyToTableSufficientPrivileges() { - Assertions.assertThat( - adminService.grantPrivilegeOnCatalogToRole( - CATALOG_NAME, CATALOG_ROLE2, PolarisPrivilege.POLICY_DETACH)) - .isTrue(); - Assertions.assertThat( - adminService.grantPrivilegeOnCatalogToRole( - CATALOG_NAME, CATALOG_ROLE2, PolarisPrivilege.TABLE_DETACH_POLICY)) - .isTrue(); + assertSuccess( + adminService.grantPrivilegeOnCatalogToRole( + CATALOG_NAME, CATALOG_ROLE2, PolarisPrivilege.POLICY_DETACH)); + assertSuccess( + adminService.grantPrivilegeOnCatalogToRole( + CATALOG_NAME, CATALOG_ROLE2, PolarisPrivilege.TABLE_DETACH_POLICY)); PolicyAttachmentTarget tableTarget = PolicyAttachmentTarget.builder() @@ -482,22 +470,18 @@ public void testAttachPolicyToTableInsufficientPrivileges() { @Test public void testDetachPolicyFromCatalogSufficientPrivileges() { - Assertions.assertThat( - adminService.grantPrivilegeOnCatalogToRole( - CATALOG_NAME, CATALOG_ROLE2, PolarisPrivilege.POLICY_ATTACH)) - .isTrue(); - Assertions.assertThat( - adminService.grantPrivilegeOnCatalogToRole( - CATALOG_NAME, CATALOG_ROLE2, PolarisPrivilege.CATALOG_ATTACH_POLICY)) - .isTrue(); - Assertions.assertThat( - adminService.grantPrivilegeOnCatalogToRole( - CATALOG_NAME, CATALOG_ROLE2, PolarisPrivilege.POLICY_DETACH)) - .isTrue(); - Assertions.assertThat( - adminService.grantPrivilegeOnCatalogToRole( - CATALOG_NAME, CATALOG_ROLE2, PolarisPrivilege.CATALOG_DETACH_POLICY)) - .isTrue(); + assertSuccess( + adminService.grantPrivilegeOnCatalogToRole( + CATALOG_NAME, CATALOG_ROLE2, PolarisPrivilege.POLICY_ATTACH)); + assertSuccess( + adminService.grantPrivilegeOnCatalogToRole( + CATALOG_NAME, CATALOG_ROLE2, PolarisPrivilege.CATALOG_ATTACH_POLICY)); + assertSuccess( + adminService.grantPrivilegeOnCatalogToRole( + CATALOG_NAME, CATALOG_ROLE2, PolarisPrivilege.POLICY_DETACH)); + assertSuccess( + adminService.grantPrivilegeOnCatalogToRole( + CATALOG_NAME, CATALOG_ROLE2, PolarisPrivilege.CATALOG_DETACH_POLICY)); PolicyAttachmentTarget catalogTarget = PolicyAttachmentTarget.builder().setType(PolicyAttachmentTarget.TypeEnum.CATALOG).build(); AttachPolicyRequest attachPolicyRequest = @@ -522,22 +506,18 @@ public void testDetachPolicyFromCatalogSufficientPrivileges() { @Test public void testDetachPolicyFromCatalogInsufficientPrivileges() { - Assertions.assertThat( - adminService.grantPrivilegeOnCatalogToRole( - CATALOG_NAME, CATALOG_ROLE2, PolarisPrivilege.POLICY_ATTACH)) - .isTrue(); - Assertions.assertThat( - adminService.grantPrivilegeOnCatalogToRole( - CATALOG_NAME, CATALOG_ROLE2, PolarisPrivilege.CATALOG_ATTACH_POLICY)) - .isTrue(); - Assertions.assertThat( - adminService.grantPrivilegeOnCatalogToRole( - CATALOG_NAME, CATALOG_ROLE2, PolarisPrivilege.POLICY_DETACH)) - .isTrue(); - Assertions.assertThat( - adminService.grantPrivilegeOnCatalogToRole( - CATALOG_NAME, CATALOG_ROLE2, PolarisPrivilege.CATALOG_DETACH_POLICY)) - .isTrue(); + assertSuccess( + adminService.grantPrivilegeOnCatalogToRole( + CATALOG_NAME, CATALOG_ROLE2, PolarisPrivilege.POLICY_ATTACH)); + assertSuccess( + adminService.grantPrivilegeOnCatalogToRole( + CATALOG_NAME, CATALOG_ROLE2, PolarisPrivilege.CATALOG_ATTACH_POLICY)); + assertSuccess( + adminService.grantPrivilegeOnCatalogToRole( + CATALOG_NAME, CATALOG_ROLE2, PolarisPrivilege.POLICY_DETACH)); + assertSuccess( + adminService.grantPrivilegeOnCatalogToRole( + CATALOG_NAME, CATALOG_ROLE2, PolarisPrivilege.CATALOG_DETACH_POLICY)); PolicyAttachmentTarget catalogTarget = PolicyAttachmentTarget.builder().setType(PolicyAttachmentTarget.TypeEnum.CATALOG).build(); @@ -561,22 +541,18 @@ public void testDetachPolicyFromCatalogInsufficientPrivileges() { @Test public void testDetachPolicyFromNamespaceSufficientPrivileges() { - Assertions.assertThat( - adminService.grantPrivilegeOnCatalogToRole( - CATALOG_NAME, CATALOG_ROLE2, PolarisPrivilege.POLICY_ATTACH)) - .isTrue(); - Assertions.assertThat( - adminService.grantPrivilegeOnCatalogToRole( - CATALOG_NAME, CATALOG_ROLE2, PolarisPrivilege.NAMESPACE_ATTACH_POLICY)) - .isTrue(); - Assertions.assertThat( - adminService.grantPrivilegeOnCatalogToRole( - CATALOG_NAME, CATALOG_ROLE2, PolarisPrivilege.POLICY_DETACH)) - .isTrue(); - Assertions.assertThat( - adminService.grantPrivilegeOnCatalogToRole( - CATALOG_NAME, CATALOG_ROLE2, PolarisPrivilege.NAMESPACE_DETACH_POLICY)) - .isTrue(); + assertSuccess( + adminService.grantPrivilegeOnCatalogToRole( + CATALOG_NAME, CATALOG_ROLE2, PolarisPrivilege.POLICY_ATTACH)); + assertSuccess( + adminService.grantPrivilegeOnCatalogToRole( + CATALOG_NAME, CATALOG_ROLE2, PolarisPrivilege.NAMESPACE_ATTACH_POLICY)); + assertSuccess( + adminService.grantPrivilegeOnCatalogToRole( + CATALOG_NAME, CATALOG_ROLE2, PolarisPrivilege.POLICY_DETACH)); + assertSuccess( + adminService.grantPrivilegeOnCatalogToRole( + CATALOG_NAME, CATALOG_ROLE2, PolarisPrivilege.NAMESPACE_DETACH_POLICY)); PolicyAttachmentTarget namespaceTarget = PolicyAttachmentTarget.builder() @@ -605,22 +581,18 @@ public void testDetachPolicyFromNamespaceSufficientPrivileges() { @Test public void testDetachPolicyFromNamespaceInsufficientPrivilege() { - Assertions.assertThat( - adminService.grantPrivilegeOnCatalogToRole( - CATALOG_NAME, CATALOG_ROLE2, PolarisPrivilege.POLICY_ATTACH)) - .isTrue(); - Assertions.assertThat( - adminService.grantPrivilegeOnCatalogToRole( - CATALOG_NAME, CATALOG_ROLE2, PolarisPrivilege.NAMESPACE_ATTACH_POLICY)) - .isTrue(); - Assertions.assertThat( - adminService.grantPrivilegeOnCatalogToRole( - CATALOG_NAME, CATALOG_ROLE2, PolarisPrivilege.POLICY_DETACH)) - .isTrue(); - Assertions.assertThat( - adminService.grantPrivilegeOnCatalogToRole( - CATALOG_NAME, CATALOG_ROLE2, PolarisPrivilege.NAMESPACE_DETACH_POLICY)) - .isTrue(); + assertSuccess( + adminService.grantPrivilegeOnCatalogToRole( + CATALOG_NAME, CATALOG_ROLE2, PolarisPrivilege.POLICY_ATTACH)); + assertSuccess( + adminService.grantPrivilegeOnCatalogToRole( + CATALOG_NAME, CATALOG_ROLE2, PolarisPrivilege.NAMESPACE_ATTACH_POLICY)); + assertSuccess( + adminService.grantPrivilegeOnCatalogToRole( + CATALOG_NAME, CATALOG_ROLE2, PolarisPrivilege.POLICY_DETACH)); + assertSuccess( + adminService.grantPrivilegeOnCatalogToRole( + CATALOG_NAME, CATALOG_ROLE2, PolarisPrivilege.NAMESPACE_DETACH_POLICY)); PolicyAttachmentTarget namespaceTarget = PolicyAttachmentTarget.builder() @@ -647,22 +619,18 @@ public void testDetachPolicyFromNamespaceInsufficientPrivilege() { @Test public void testDetachPolicyFromTableSufficientPrivileges() { - Assertions.assertThat( - adminService.grantPrivilegeOnCatalogToRole( - CATALOG_NAME, CATALOG_ROLE2, PolarisPrivilege.POLICY_ATTACH)) - .isTrue(); - Assertions.assertThat( - adminService.grantPrivilegeOnCatalogToRole( - CATALOG_NAME, CATALOG_ROLE2, PolarisPrivilege.TABLE_ATTACH_POLICY)) - .isTrue(); - Assertions.assertThat( - adminService.grantPrivilegeOnCatalogToRole( - CATALOG_NAME, CATALOG_ROLE2, PolarisPrivilege.POLICY_DETACH)) - .isTrue(); - Assertions.assertThat( - adminService.grantPrivilegeOnCatalogToRole( - CATALOG_NAME, CATALOG_ROLE2, PolarisPrivilege.TABLE_DETACH_POLICY)) - .isTrue(); + assertSuccess( + adminService.grantPrivilegeOnCatalogToRole( + CATALOG_NAME, CATALOG_ROLE2, PolarisPrivilege.POLICY_ATTACH)); + assertSuccess( + adminService.grantPrivilegeOnCatalogToRole( + CATALOG_NAME, CATALOG_ROLE2, PolarisPrivilege.TABLE_ATTACH_POLICY)); + assertSuccess( + adminService.grantPrivilegeOnCatalogToRole( + CATALOG_NAME, CATALOG_ROLE2, PolarisPrivilege.POLICY_DETACH)); + assertSuccess( + adminService.grantPrivilegeOnCatalogToRole( + CATALOG_NAME, CATALOG_ROLE2, PolarisPrivilege.TABLE_DETACH_POLICY)); PolicyAttachmentTarget tableTarget = PolicyAttachmentTarget.builder() @@ -691,22 +659,18 @@ public void testDetachPolicyFromTableSufficientPrivileges() { @Test public void testDetachFromPolicyInsufficientPrivileges() { - Assertions.assertThat( - adminService.grantPrivilegeOnCatalogToRole( - CATALOG_NAME, CATALOG_ROLE2, PolarisPrivilege.POLICY_ATTACH)) - .isTrue(); - Assertions.assertThat( - adminService.grantPrivilegeOnCatalogToRole( - CATALOG_NAME, CATALOG_ROLE2, PolarisPrivilege.TABLE_ATTACH_POLICY)) - .isTrue(); - Assertions.assertThat( - adminService.grantPrivilegeOnCatalogToRole( - CATALOG_NAME, CATALOG_ROLE2, PolarisPrivilege.POLICY_DETACH)) - .isTrue(); - Assertions.assertThat( - adminService.grantPrivilegeOnCatalogToRole( - CATALOG_NAME, CATALOG_ROLE2, PolarisPrivilege.TABLE_DETACH_POLICY)) - .isTrue(); + assertSuccess( + adminService.grantPrivilegeOnCatalogToRole( + CATALOG_NAME, CATALOG_ROLE2, PolarisPrivilege.POLICY_ATTACH)); + assertSuccess( + adminService.grantPrivilegeOnCatalogToRole( + CATALOG_NAME, CATALOG_ROLE2, PolarisPrivilege.TABLE_ATTACH_POLICY)); + assertSuccess( + adminService.grantPrivilegeOnCatalogToRole( + CATALOG_NAME, CATALOG_ROLE2, PolarisPrivilege.POLICY_DETACH)); + assertSuccess( + adminService.grantPrivilegeOnCatalogToRole( + CATALOG_NAME, CATALOG_ROLE2, PolarisPrivilege.TABLE_DETACH_POLICY)); PolicyAttachmentTarget tableTarget = PolicyAttachmentTarget.builder()