From 5e5113447410a0b2913a722a77c6f8d21351d2cb Mon Sep 17 00:00:00 2001 From: Michael Collado Date: Thu, 10 Apr 2025 14:58:37 -0700 Subject: [PATCH 01/10] Add support for federated principal and role with block for manual role assignment --- ...larisManagementServiceIntegrationTest.java | 49 +++++- .../polaris/core/entity/PolarisEntity.java | 6 + .../core/entity/PolarisEntityConstants.java | 2 + .../polaris/core/entity/PrincipalEntity.java | 9 + .../core/entity/PrincipalRoleEntity.java | 23 ++- .../quarkus/admin/ManagementServiceTest.java | 161 ++++++++++++++++++ .../service/admin/PolarisAdminService.java | 24 ++- spec/polaris-management-service.yml | 8 + 8 files changed, 270 insertions(+), 12 deletions(-) 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 1b3899c31f..17984912cd 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 @@ -18,6 +18,8 @@ */ package org.apache.polaris.service.it.test; +import static javax.ws.rs.core.Response.Status.BAD_REQUEST; +import static javax.ws.rs.core.Response.Status.CREATED; import static javax.ws.rs.core.Response.Status.FORBIDDEN; import static org.apache.polaris.service.it.env.PolarisClient.polarisClient; import static org.apache.polaris.service.it.test.PolarisApplicationIntegrationTest.PRINCIPAL_ROLE_ALL; @@ -872,6 +874,42 @@ public void testCreatePrincipalAndRotateCredentials() { // rotation that makes the old secret fall off retention. } + @Test + public void testCreateFederatedPrincipalFails() { + // Create a federated Principal + Principal federatedPrincipal = + new Principal(client.newEntityName("federatedPrincipal"), "abc", true, Map.of(), 0L, 0L, 1); + + // Attempt to create the federated Principal using the managementApi + try (Response createPResponse = + managementApi + .request("v1/principals") + .post(Entity.json(new CreatePrincipalRequest(federatedPrincipal, false)))) { + assertThat(createPResponse).returns(BAD_REQUEST.getStatusCode(), Response::getStatus); + } + } + + @Test + public void testCreateFederatedPrincipalRoleSucceeds() { + // Create a federated Principal Role + PrincipalRole federatedPrincipalRole = + new PrincipalRole( + client.newEntityName("federatedRole"), + true, + Map.of(), + Instant.now().toEpochMilli(), + Instant.now().toEpochMilli(), + 1); + + // Attempt to create the federated Principal using the managementApi + try (Response createResponse = + managementApi + .request("v1/principal-roles") + .post(Entity.json(new CreatePrincipalRoleRequest(federatedPrincipalRole)))) { + assertThat(createResponse).returns(CREATED.getStatusCode(), Response::getStatus); + } + } + @Test public void testCreateListUpdateAndDeletePrincipal() { Principal principal = @@ -1023,7 +1061,7 @@ public void testGetPrincipalWithInvalidName() { public void testCreateListUpdateAndDeletePrincipalRole() { PrincipalRole principalRole = new PrincipalRole( - client.newEntityName("myprincipalrole"), Map.of("custom-tag", "foo"), 0L, 0L, 1); + client.newEntityName("myprincipalrole"), false, Map.of("custom-tag", "foo"), 0L, 0L, 1); managementApi.createPrincipalRole(principalRole); // Second attempt to create the same entity should fail with CONFLICT. @@ -1115,7 +1153,7 @@ public void testCreateListUpdateAndDeletePrincipalRole() { public void testCreatePrincipalRoleInvalidName() { String goodName = RandomStringUtils.random(MAX_IDENTIFIER_LENGTH, true, true); PrincipalRole principalRole = - new PrincipalRole(goodName, Map.of("custom-tag", "good_principal_role"), 0L, 0L, 1); + new PrincipalRole(goodName, false, Map.of("custom-tag", "good_principal_role"), 0L, 0L, 1); managementApi.createPrincipalRole(principalRole); String longInvalidName = RandomStringUtils.random(MAX_IDENTIFIER_LENGTH + 1, true, true); @@ -1131,7 +1169,12 @@ public void testCreatePrincipalRoleInvalidName() { for (String invalidPrincipalRoleName : invalidPrincipalRoleNames) { principalRole = new PrincipalRole( - invalidPrincipalRoleName, Map.of("custom-tag", "bad_principal_role"), 0L, 0L, 1); + invalidPrincipalRoleName, + false, + Map.of("custom-tag", "bad_principal_role"), + 0L, + 0L, + 1); try (Response response = managementApi diff --git a/polaris-core/src/main/java/org/apache/polaris/core/entity/PolarisEntity.java b/polaris-core/src/main/java/org/apache/polaris/core/entity/PolarisEntity.java index 363c1d3ce4..a085273b6c 100644 --- a/polaris-core/src/main/java/org/apache/polaris/core/entity/PolarisEntity.java +++ b/polaris-core/src/main/java/org/apache/polaris/core/entity/PolarisEntity.java @@ -182,6 +182,12 @@ public static List toNameAndIdList(List entit .orElse(null); } + public static boolean isFederated(PolarisBaseEntity entity) { + return Optional.ofNullable(entity.getInternalPropertiesAsMap()) + .map(map -> Boolean.parseBoolean(map.get(PolarisEntityConstants.FEDERATED_ENTITY))) + .orElse(false); + } + public PolarisEntity(@Nonnull PolarisBaseEntity sourceEntity) { super( sourceEntity.getCatalogId(), diff --git a/polaris-core/src/main/java/org/apache/polaris/core/entity/PolarisEntityConstants.java b/polaris-core/src/main/java/org/apache/polaris/core/entity/PolarisEntityConstants.java index 4452a90200..15cf1b7315 100644 --- a/polaris-core/src/main/java/org/apache/polaris/core/entity/PolarisEntityConstants.java +++ b/polaris-core/src/main/java/org/apache/polaris/core/entity/PolarisEntityConstants.java @@ -60,6 +60,8 @@ public class PolarisEntityConstants { public static final String PRINCIPAL_CREDENTIAL_ROTATION_REQUIRED_STATE = "CREDENTIAL_ROTATION_REQUIRED"; + public static final String FEDERATED_ENTITY = "federated"; + /** * Name format of storage integration for polaris entity: {@code * POLARIS__}. This name format gives us flexibility to switch to use diff --git a/polaris-core/src/main/java/org/apache/polaris/core/entity/PrincipalEntity.java b/polaris-core/src/main/java/org/apache/polaris/core/entity/PrincipalEntity.java index ba0cfe3f53..63c7f3b9e3 100644 --- a/polaris-core/src/main/java/org/apache/polaris/core/entity/PrincipalEntity.java +++ b/polaris-core/src/main/java/org/apache/polaris/core/entity/PrincipalEntity.java @@ -36,6 +36,7 @@ public static PrincipalEntity of(PolarisBaseEntity sourceEntity) { public static PrincipalEntity fromPrincipal(Principal principal) { return new Builder() .setName(principal.getName()) + .setFederated(principal.getFederated()) .setProperties(principal.getProperties()) .setClientId(principal.getClientId()) .build(); @@ -45,6 +46,7 @@ public Principal asPrincipal() { return new Principal( getName(), getClientId(), + PolarisEntity.isFederated(this), getPropertiesAsMap(), getCreateTimestamp(), getLastUpdateTimestamp(), @@ -78,6 +80,13 @@ public Builder setCredentialRotationRequiredState() { return this; } + public Builder setFederated(Boolean isFederated) { + if (isFederated != null && isFederated) { + internalProperties.put(PolarisEntityConstants.FEDERATED_ENTITY, "true"); + } + return this; + } + @Override public PrincipalEntity build() { return new PrincipalEntity(buildBase()); diff --git a/polaris-core/src/main/java/org/apache/polaris/core/entity/PrincipalRoleEntity.java b/polaris-core/src/main/java/org/apache/polaris/core/entity/PrincipalRoleEntity.java index 26f13cc8f2..81d46c268a 100644 --- a/polaris-core/src/main/java/org/apache/polaris/core/entity/PrincipalRoleEntity.java +++ b/polaris-core/src/main/java/org/apache/polaris/core/entity/PrincipalRoleEntity.java @@ -38,19 +38,19 @@ public static PrincipalRoleEntity of(PolarisBaseEntity sourceEntity) { public static PrincipalRoleEntity fromPrincipalRole(PrincipalRole principalRole) { return new Builder() .setName(principalRole.getName()) + .setFederated(principalRole.getFederated()) .setProperties(principalRole.getProperties()) .build(); } public PrincipalRole asPrincipalRole() { - PrincipalRole principalRole = - new PrincipalRole( - getName(), - getPropertiesAsMap(), - getCreateTimestamp(), - getLastUpdateTimestamp(), - getEntityVersion()); - return principalRole; + return new PrincipalRole( + getName(), + PolarisEntity.isFederated(this), + getPropertiesAsMap(), + getCreateTimestamp(), + getLastUpdateTimestamp(), + getEntityVersion()); } public static class Builder extends PolarisEntity.BaseBuilder { @@ -65,6 +65,13 @@ public Builder(PrincipalRoleEntity original) { super(original); } + public Builder setFederated(Boolean isFederated) { + if (isFederated != null && isFederated) { + internalProperties.put(PolarisEntityConstants.FEDERATED_ENTITY, "true"); + } + return this; + } + @Override public PrincipalRoleEntity build() { return new PrincipalRoleEntity(buildBase()); diff --git a/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/admin/ManagementServiceTest.java b/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/admin/ManagementServiceTest.java index c07c80bd79..6c05626825 100644 --- a/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/admin/ManagementServiceTest.java +++ b/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/admin/ManagementServiceTest.java @@ -22,9 +22,14 @@ import static org.assertj.core.api.Assertions.assertThatThrownBy; import jakarta.ws.rs.core.Response; +import jakarta.ws.rs.core.SecurityContext; +import java.security.Principal; import java.time.Clock; +import java.time.Instant; import java.util.List; import java.util.Map; +import java.util.Set; +import org.apache.iceberg.exceptions.ValidationException; import org.apache.polaris.core.PolarisCallContext; import org.apache.polaris.core.admin.model.AwsStorageConfigInfo; import org.apache.polaris.core.admin.model.Catalog; @@ -34,8 +39,18 @@ import org.apache.polaris.core.admin.model.PolarisCatalog; import org.apache.polaris.core.admin.model.StorageConfigInfo; import org.apache.polaris.core.admin.model.UpdateCatalogRequest; +import org.apache.polaris.core.auth.AuthenticatedPolarisPrincipal; +import org.apache.polaris.core.auth.PolarisAuthorizerImpl; import org.apache.polaris.core.context.CallContext; +import org.apache.polaris.core.context.RealmContext; +import org.apache.polaris.core.entity.PrincipalEntity; +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.EntityResult; import org.apache.polaris.service.TestServices; +import org.apache.polaris.service.admin.PolarisAdminService; +import org.apache.polaris.service.config.DefaultConfigurationStore; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.mockito.Mockito; @@ -158,4 +173,150 @@ public void testUpdateCatalogWithDisallowedStorageConfig() { .isInstanceOf(IllegalArgumentException.class) .hasMessage("Unsupported storage type: FILE"); } + + private PolarisMetaStoreManager setupMetaStoreManager() { + MetaStoreManagerFactory metaStoreManagerFactory = services.metaStoreManagerFactory(); + RealmContext realmContext = services.realmContext(); + return metaStoreManagerFactory.getOrCreateMetaStoreManager(realmContext); + } + + private PolarisCallContext setupCallContext(PolarisMetaStoreManager metaStoreManager) { + MetaStoreManagerFactory metaStoreManagerFactory = services.metaStoreManagerFactory(); + RealmContext realmContext = services.realmContext(); + return new PolarisCallContext( + metaStoreManagerFactory.getOrCreateSessionSupplier(realmContext).get(), + services.polarisDiagnostics()); + } + + private PolarisAdminService setupPolarisAdminService( + PolarisMetaStoreManager metaStoreManager, PolarisCallContext callContext) { + RealmContext realmContext = services.realmContext(); + return new PolarisAdminService( + CallContext.of(realmContext, callContext), + services.entityManagerFactory().getOrCreateEntityManager(realmContext), + metaStoreManager, + new SecurityContext() { + @Override + public Principal getUserPrincipal() { + return new AuthenticatedPolarisPrincipal( + new PrincipalEntity.Builder().setName("root").build(), Set.of("service_admin")); + } + + @Override + public boolean isUserInRole(String role) { + return true; + } + + @Override + public boolean isSecure() { + return false; + } + + @Override + public String getAuthenticationScheme() { + return ""; + } + }, + new PolarisAuthorizerImpl(new DefaultConfigurationStore(Map.of()))); + } + + private PrincipalEntity createPrincipal( + PolarisMetaStoreManager metaStoreManager, + PolarisCallContext callContext, + String name, + boolean isFederated) { + return new PrincipalEntity.Builder() + .setFederated(isFederated) + .setName(name) + .setCreateTimestamp(Instant.now().toEpochMilli()) + .setId(metaStoreManager.generateNewEntityId(callContext).getId()) + .build(); + } + + private PrincipalRoleEntity createRole( + PolarisMetaStoreManager metaStoreManager, + PolarisCallContext callContext, + String name, + boolean isFederated) { + return new PrincipalRoleEntity.Builder() + .setId(metaStoreManager.generateNewEntityId(callContext).getId()) + .setName(name) + .setFederated(isFederated) + .setProperties(Map.of()) + .setCreateTimestamp(Instant.now().toEpochMilli()) + .setLastUpdateTimestamp(Instant.now().toEpochMilli()) + .build(); + } + + @Test + public void testCannotAddFederatedPrincipalToNonFederatedRole() { + PolarisMetaStoreManager metaStoreManager = setupMetaStoreManager(); + PolarisCallContext callContext = setupCallContext(metaStoreManager); + PolarisAdminService polarisAdminService = + setupPolarisAdminService(metaStoreManager, callContext); + + PrincipalEntity federatedPrincipal = + createPrincipal(metaStoreManager, callContext, "federated_id", true); + metaStoreManager.createPrincipal(callContext, federatedPrincipal); + + PrincipalRoleEntity nonFederatedRole = + createRole(metaStoreManager, callContext, "non_federated_role", false); + EntityResult result = + metaStoreManager.createEntityIfNotExists(callContext, null, nonFederatedRole); + assertThat(result.isSuccess()).isTrue(); + + assertThatThrownBy( + () -> + polarisAdminService.assignPrincipalRole( + federatedPrincipal.getName(), nonFederatedRole.getName())) + .isInstanceOf(ValidationException.class); + } + + @Test + public void testCannotAddNonFederatedPrincipalToFederatedRole() { + PolarisMetaStoreManager metaStoreManager = setupMetaStoreManager(); + PolarisCallContext callContext = setupCallContext(metaStoreManager); + PolarisAdminService polarisAdminService = + setupPolarisAdminService(metaStoreManager, callContext); + + PrincipalEntity nonFederatedPrincipal = + createPrincipal(metaStoreManager, callContext, "non_federated_id", false); + metaStoreManager.createPrincipal(callContext, nonFederatedPrincipal); + + PrincipalRoleEntity federatedRole = + createRole(metaStoreManager, callContext, "federated_role", true); + EntityResult result = + metaStoreManager.createEntityIfNotExists(callContext, null, federatedRole); + assertThat(result.isSuccess()).isTrue(); + + assertThatThrownBy( + () -> + polarisAdminService.assignPrincipalRole( + nonFederatedPrincipal.getName(), federatedRole.getName())) + .isInstanceOf(ValidationException.class); + } + + @Test + public void testCannotAddFederatedPrincipalToFederatedRole() { + PolarisMetaStoreManager metaStoreManager = setupMetaStoreManager(); + PolarisCallContext callContext = setupCallContext(metaStoreManager); + PolarisAdminService polarisAdminService = + setupPolarisAdminService(metaStoreManager, callContext); + + PrincipalEntity federatedPrincipal = + createPrincipal(metaStoreManager, callContext, "federated_principal", true); + metaStoreManager.createPrincipal(callContext, federatedPrincipal); + + PrincipalRoleEntity federatedRole = + createRole(metaStoreManager, callContext, "federated_role", true); + EntityResult result = + metaStoreManager.createEntityIfNotExists(callContext, null, federatedRole); + assertThat(result.isSuccess()).isTrue(); + + assertThatThrownBy( + () -> + polarisAdminService.assignPrincipalRole( + federatedPrincipal.getName(), federatedRole.getName())) + .isInstanceOf(ValidationException.class); + } } diff --git a/service/common/src/main/java/org/apache/polaris/service/admin/PolarisAdminService.java b/service/common/src/main/java/org/apache/polaris/service/admin/PolarisAdminService.java index c51c9a8ebc..619e0f46ef 100644 --- a/service/common/src/main/java/org/apache/polaris/service/admin/PolarisAdminService.java +++ b/service/common/src/main/java/org/apache/polaris/service/admin/PolarisAdminService.java @@ -910,6 +910,9 @@ public PrincipalWithCredentials createPrincipal(PolarisEntity entity) { PolarisAuthorizableOperation op = PolarisAuthorizableOperation.CREATE_PRINCIPAL; authorizeBasicRootOperationOrThrow(op); + if (PolarisEntity.isFederated(entity)) { + throw new ValidationException("Cannot create a federated principal"); + } checkArgument(entity.getId() == -1, "Entity to be created must have no ID assigned"); CreatePrincipalResult principalResult = @@ -972,6 +975,10 @@ public void deletePrincipal(String name) { findPrincipalByName(name) .orElseThrow(() -> new NotFoundException("Principal %s not found", name)); + if (PolarisEntity.isFederated(currentPrincipalEntity)) { + throw new ValidationException( + "Cannot update a federated principal: %s", currentPrincipalEntity.getName()); + } if (currentPrincipalEntity.getEntityVersion() != updateRequest.getCurrentEntityVersion()) { throw new CommitFailedException( "Failed to update Principal; currentEntityVersion '%s', expected '%s'", @@ -1005,6 +1012,10 @@ public void deletePrincipal(String name) { findPrincipalByName(principalName) .orElseThrow(() -> new NotFoundException("Principal %s not found", principalName)); + if (PolarisEntity.isFederated(currentPrincipalEntity)) { + throw new ValidationException( + "Cannot rotate/reset credentials for a federated principal: %s", principalName); + } PolarisPrincipalSecrets currentSecrets = metaStoreManager .loadPrincipalSecrets(getCurrentPolarisContext(), currentPrincipalEntity.getClientId()) @@ -1331,11 +1342,16 @@ public boolean assignPrincipalRole(String principalName, String principalRoleNam PolarisEntity principalEntity = findPrincipalByName(principalName) .orElseThrow(() -> new NotFoundException("Principal %s not found", principalName)); + if (PolarisEntity.isFederated(principalEntity)) { + throw new ValidationException("Cannot assign a role to a federated principal"); + } PolarisEntity principalRoleEntity = findPrincipalRoleByName(principalRoleName) .orElseThrow( () -> new NotFoundException("PrincipalRole %s not found", principalRoleName)); - + if (PolarisEntity.isFederated(principalRoleEntity)) { + throw new ValidationException("Cannot assign a federated role to a principal"); + } return metaStoreManager .grantUsageOnRoleToGrantee( getCurrentPolarisContext(), null, principalRoleEntity, principalEntity) @@ -1349,10 +1365,16 @@ public boolean revokePrincipalRole(String principalName, String principalRoleNam PolarisEntity principalEntity = findPrincipalByName(principalName) .orElseThrow(() -> new NotFoundException("Principal %s not found", principalName)); + if (PolarisEntity.isFederated(principalEntity)) { + throw new ValidationException("Cannot revoke a role from a federated principal"); + } PolarisEntity principalRoleEntity = findPrincipalRoleByName(principalRoleName) .orElseThrow( () -> new NotFoundException("PrincipalRole %s not found", principalRoleName)); + if (PolarisEntity.isFederated(principalRoleEntity)) { + throw new ValidationException("Cannot revoke a federated role from a principal"); + } return metaStoreManager .revokeUsageOnRoleFromGrantee( getCurrentPolarisContext(), null, principalRoleEntity, principalEntity) diff --git a/spec/polaris-management-service.yml b/spec/polaris-management-service.yml index 795dd61ffc..2f0490e789 100644 --- a/spec/polaris-management-service.yml +++ b/spec/polaris-management-service.yml @@ -1101,6 +1101,10 @@ components: clientId: type: string description: The output-only OAuth clientId associated with this principal if applicable + federated: + type: boolean + description: Whether the principal is a federated identity (that is, managed by an external identity provider) + default: false properties: type: object additionalProperties: @@ -1163,6 +1167,10 @@ components: maxLength: 256 pattern: '^(?!\s*[s|S][y|Y][s|S][t|T][e|E][m|M]\$).*$' description: The name of the role + federated: + type: boolean + description: Whether the principal role is a federated role (that is, managed by an external identity provider) + default: false properties: type: object additionalProperties: From 98d33563e75ebc91373fdf4fd3247b72d79f6b55 Mon Sep 17 00:00:00 2001 From: Michael Collado Date: Wed, 16 Apr 2025 15:31:31 -0700 Subject: [PATCH 02/10] Update spec to distinguish federated and non-federated entities --- api/management-model/build.gradle.kts | 2 + .../polaris/service/it/env/ManagementApi.java | 3 +- ...larisManagementServiceIntegrationTest.java | 18 +++-- .../polaris/core/entity/PolarisEntity.java | 6 -- .../core/entity/PolarisEntityConstants.java | 2 - .../polaris/core/entity/PrincipalEntity.java | 23 +++++- .../core/entity/PrincipalRoleEntity.java | 5 +- .../table/federated/FederatedEntities.java | 35 ++++++++ .../quarkus/admin/ManagementServiceTest.java | 80 +++++-------------- .../service/admin/PolarisAdminService.java | 16 ++-- .../service/admin/PolarisServiceImpl.java | 12 +-- .../polaris/service/config/Serializers.java | 21 +++++ spec/polaris-management-service.yml | 44 +++++++--- 13 files changed, 163 insertions(+), 104 deletions(-) create mode 100644 polaris-core/src/main/java/org/apache/polaris/core/entity/table/federated/FederatedEntities.java diff --git a/api/management-model/build.gradle.kts b/api/management-model/build.gradle.kts index ec78abe5d5..f1c6258587 100644 --- a/api/management-model/build.gradle.kts +++ b/api/management-model/build.gradle.kts @@ -57,6 +57,8 @@ openApiGenerate { additionalProperties.put("apiNameSuffix", "Api") additionalProperties.put("metricsPrefix", "polaris") serverVariables = mapOf("basePath" to "api/v1") + openapiNormalizer.put("REFACTOR_ALLOF_WITH_PROPERTIES_ONLY", "true") + openapiNormalizer.put("REF_AS_PARENT_IN_ALLOF", "true") } listOf("sourcesJar", "compileJava").forEach { task -> diff --git a/integration-tests/src/main/java/org/apache/polaris/service/it/env/ManagementApi.java b/integration-tests/src/main/java/org/apache/polaris/service/it/env/ManagementApi.java index 83ec020388..fc4298757f 100644 --- a/integration-tests/src/main/java/org/apache/polaris/service/it/env/ManagementApi.java +++ b/integration-tests/src/main/java/org/apache/polaris/service/it/env/ManagementApi.java @@ -29,6 +29,7 @@ import java.net.URI; import java.util.List; import java.util.Map; +import org.apache.polaris.core.admin.model.BasePrincipal; import org.apache.polaris.core.admin.model.Catalog; import org.apache.polaris.core.admin.model.CatalogGrant; import org.apache.polaris.core.admin.model.CatalogPrivilege; @@ -222,7 +223,7 @@ public List listCatalogRoles(String catalogName) { } } - public List listPrincipals() { + public List listPrincipals() { try (Response response = request("v1/principals").get()) { assertThat(response.getStatus()).isEqualTo(OK.getStatusCode()); return response.readEntity(Principals.class).getPrincipals(); 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 17984912cd..46442f99cd 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 @@ -18,7 +18,6 @@ */ package org.apache.polaris.service.it.test; -import static javax.ws.rs.core.Response.Status.BAD_REQUEST; import static javax.ws.rs.core.Response.Status.CREATED; import static javax.ws.rs.core.Response.Status.FORBIDDEN; import static org.apache.polaris.service.it.env.PolarisClient.polarisClient; @@ -28,6 +27,7 @@ import com.auth0.jwt.JWT; import com.auth0.jwt.JWTCreator; import com.auth0.jwt.algorithms.Algorithm; +import com.fasterxml.jackson.core.JsonProcessingException; import com.fasterxml.jackson.databind.JsonNode; import com.fasterxml.jackson.databind.ObjectMapper; import com.fasterxml.jackson.databind.node.ObjectNode; @@ -57,6 +57,7 @@ import org.apache.polaris.core.admin.model.CreatePrincipalRequest; import org.apache.polaris.core.admin.model.CreatePrincipalRoleRequest; import org.apache.polaris.core.admin.model.ExternalCatalog; +import org.apache.polaris.core.admin.model.FederatedPrincipal; import org.apache.polaris.core.admin.model.GcpStorageConfigInfo; import org.apache.polaris.core.admin.model.GrantCatalogRoleRequest; import org.apache.polaris.core.admin.model.GrantResource; @@ -875,17 +876,22 @@ public void testCreatePrincipalAndRotateCredentials() { } @Test - public void testCreateFederatedPrincipalFails() { + public void testCreateFederatedPrincipalFails() throws JsonProcessingException { // Create a federated Principal - Principal federatedPrincipal = - new Principal(client.newEntityName("federatedPrincipal"), "abc", true, Map.of(), 0L, 0L, 1); + FederatedPrincipal federatedPrincipal = + new FederatedPrincipal( + true, client.newEntityName("federatedPrincipal"), Map.of(), 0L, 0L, 1); // Attempt to create the federated Principal using the managementApi try (Response createPResponse = managementApi .request("v1/principals") - .post(Entity.json(new CreatePrincipalRequest(federatedPrincipal, false)))) { - assertThat(createPResponse).returns(BAD_REQUEST.getStatusCode(), Response::getStatus); + .post( + Entity.json( + "{\"principal\":" + + new ObjectMapper().writeValueAsString(federatedPrincipal) + + "}"))) { + assertThat(createPResponse).returns(CREATED.getStatusCode(), Response::getStatus); } } diff --git a/polaris-core/src/main/java/org/apache/polaris/core/entity/PolarisEntity.java b/polaris-core/src/main/java/org/apache/polaris/core/entity/PolarisEntity.java index a085273b6c..363c1d3ce4 100644 --- a/polaris-core/src/main/java/org/apache/polaris/core/entity/PolarisEntity.java +++ b/polaris-core/src/main/java/org/apache/polaris/core/entity/PolarisEntity.java @@ -182,12 +182,6 @@ public static List toNameAndIdList(List entit .orElse(null); } - public static boolean isFederated(PolarisBaseEntity entity) { - return Optional.ofNullable(entity.getInternalPropertiesAsMap()) - .map(map -> Boolean.parseBoolean(map.get(PolarisEntityConstants.FEDERATED_ENTITY))) - .orElse(false); - } - public PolarisEntity(@Nonnull PolarisBaseEntity sourceEntity) { super( sourceEntity.getCatalogId(), diff --git a/polaris-core/src/main/java/org/apache/polaris/core/entity/PolarisEntityConstants.java b/polaris-core/src/main/java/org/apache/polaris/core/entity/PolarisEntityConstants.java index 15cf1b7315..4452a90200 100644 --- a/polaris-core/src/main/java/org/apache/polaris/core/entity/PolarisEntityConstants.java +++ b/polaris-core/src/main/java/org/apache/polaris/core/entity/PolarisEntityConstants.java @@ -60,8 +60,6 @@ public class PolarisEntityConstants { public static final String PRINCIPAL_CREDENTIAL_ROTATION_REQUIRED_STATE = "CREDENTIAL_ROTATION_REQUIRED"; - public static final String FEDERATED_ENTITY = "federated"; - /** * Name format of storage integration for polaris entity: {@code * POLARIS__}. This name format gives us flexibility to switch to use diff --git a/polaris-core/src/main/java/org/apache/polaris/core/entity/PrincipalEntity.java b/polaris-core/src/main/java/org/apache/polaris/core/entity/PrincipalEntity.java index 63c7f3b9e3..8bb4344045 100644 --- a/polaris-core/src/main/java/org/apache/polaris/core/entity/PrincipalEntity.java +++ b/polaris-core/src/main/java/org/apache/polaris/core/entity/PrincipalEntity.java @@ -18,7 +18,10 @@ */ package org.apache.polaris.core.entity; +import org.apache.polaris.core.admin.model.BasePrincipal; +import org.apache.polaris.core.admin.model.FederatedPrincipal; import org.apache.polaris.core.admin.model.Principal; +import org.apache.polaris.core.entity.table.federated.FederatedEntities; /** Wrapper for translating between the REST Principal object and the base PolarisEntity type. */ public class PrincipalEntity extends PolarisEntity { @@ -36,17 +39,29 @@ public static PrincipalEntity of(PolarisBaseEntity sourceEntity) { public static PrincipalEntity fromPrincipal(Principal principal) { return new Builder() .setName(principal.getName()) - .setFederated(principal.getFederated()) .setProperties(principal.getProperties()) .setClientId(principal.getClientId()) .build(); } + public BasePrincipal asBasePrincipal() { + if (FederatedEntities.isFederated(this)) { + return new FederatedPrincipal( + true, + getName(), + getPropertiesAsMap(), + getCreateTimestamp(), + getLastUpdateTimestamp(), + getEntityVersion()); + } else { + return asPrincipal(); + } + } + public Principal asPrincipal() { return new Principal( - getName(), getClientId(), - PolarisEntity.isFederated(this), + getName(), getPropertiesAsMap(), getCreateTimestamp(), getLastUpdateTimestamp(), @@ -82,7 +97,7 @@ public Builder setCredentialRotationRequiredState() { public Builder setFederated(Boolean isFederated) { if (isFederated != null && isFederated) { - internalProperties.put(PolarisEntityConstants.FEDERATED_ENTITY, "true"); + internalProperties.put(FederatedEntities.FEDERATED_ENTITY, "true"); } return this; } diff --git a/polaris-core/src/main/java/org/apache/polaris/core/entity/PrincipalRoleEntity.java b/polaris-core/src/main/java/org/apache/polaris/core/entity/PrincipalRoleEntity.java index 81d46c268a..96a759794c 100644 --- a/polaris-core/src/main/java/org/apache/polaris/core/entity/PrincipalRoleEntity.java +++ b/polaris-core/src/main/java/org/apache/polaris/core/entity/PrincipalRoleEntity.java @@ -19,6 +19,7 @@ package org.apache.polaris.core.entity; import org.apache.polaris.core.admin.model.PrincipalRole; +import org.apache.polaris.core.entity.table.federated.FederatedEntities; /** * Wrapper for translating between the REST PrincipalRole object and the base PolarisEntity type. @@ -46,7 +47,7 @@ public static PrincipalRoleEntity fromPrincipalRole(PrincipalRole principalRole) public PrincipalRole asPrincipalRole() { return new PrincipalRole( getName(), - PolarisEntity.isFederated(this), + FederatedEntities.isFederated(this), getPropertiesAsMap(), getCreateTimestamp(), getLastUpdateTimestamp(), @@ -67,7 +68,7 @@ public Builder(PrincipalRoleEntity original) { public Builder setFederated(Boolean isFederated) { if (isFederated != null && isFederated) { - internalProperties.put(PolarisEntityConstants.FEDERATED_ENTITY, "true"); + internalProperties.put(FederatedEntities.FEDERATED_ENTITY, "true"); } return this; } diff --git a/polaris-core/src/main/java/org/apache/polaris/core/entity/table/federated/FederatedEntities.java b/polaris-core/src/main/java/org/apache/polaris/core/entity/table/federated/FederatedEntities.java new file mode 100644 index 0000000000..e1c2d91a14 --- /dev/null +++ b/polaris-core/src/main/java/org/apache/polaris/core/entity/table/federated/FederatedEntities.java @@ -0,0 +1,35 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.polaris.core.entity.table.federated; + +import java.util.Optional; +import org.apache.polaris.core.entity.PolarisBaseEntity; + +public final class FederatedEntities { + + public static final String FEDERATED_ENTITY = "federated"; + + public static boolean isFederated(PolarisBaseEntity entity) { + return Optional.ofNullable(entity.getInternalPropertiesAsMap()) + .map(map -> Boolean.parseBoolean(map.get(FEDERATED_ENTITY))) + .orElse(false); + } + + private FederatedEntities() {} +} diff --git a/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/admin/ManagementServiceTest.java b/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/admin/ManagementServiceTest.java index 6c05626825..e76a0ccd07 100644 --- a/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/admin/ManagementServiceTest.java +++ b/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/admin/ManagementServiceTest.java @@ -53,6 +53,8 @@ import org.apache.polaris.service.config.DefaultConfigurationStore; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.MethodSource; import org.mockito.Mockito; public class ManagementServiceTest { @@ -248,75 +250,37 @@ private PrincipalRoleEntity createRole( .build(); } - @Test - public void testCannotAddFederatedPrincipalToNonFederatedRole() { - PolarisMetaStoreManager metaStoreManager = setupMetaStoreManager(); - PolarisCallContext callContext = setupCallContext(metaStoreManager); - PolarisAdminService polarisAdminService = - setupPolarisAdminService(metaStoreManager, callContext); - - PrincipalEntity federatedPrincipal = - createPrincipal(metaStoreManager, callContext, "federated_id", true); - metaStoreManager.createPrincipal(callContext, federatedPrincipal); - - PrincipalRoleEntity nonFederatedRole = - createRole(metaStoreManager, callContext, "non_federated_role", false); - EntityResult result = - metaStoreManager.createEntityIfNotExists(callContext, null, nonFederatedRole); - assertThat(result.isSuccess()).isTrue(); - - assertThatThrownBy( - () -> - polarisAdminService.assignPrincipalRole( - federatedPrincipal.getName(), nonFederatedRole.getName())) - .isInstanceOf(ValidationException.class); - } - - @Test - public void testCannotAddNonFederatedPrincipalToFederatedRole() { - PolarisMetaStoreManager metaStoreManager = setupMetaStoreManager(); - PolarisCallContext callContext = setupCallContext(metaStoreManager); - PolarisAdminService polarisAdminService = - setupPolarisAdminService(metaStoreManager, callContext); - - PrincipalEntity nonFederatedPrincipal = - createPrincipal(metaStoreManager, callContext, "non_federated_id", false); - metaStoreManager.createPrincipal(callContext, nonFederatedPrincipal); - - PrincipalRoleEntity federatedRole = - createRole(metaStoreManager, callContext, "federated_role", true); - EntityResult result = - metaStoreManager.createEntityIfNotExists(callContext, null, federatedRole); - assertThat(result.isSuccess()).isTrue(); - - assertThatThrownBy( - () -> - polarisAdminService.assignPrincipalRole( - nonFederatedPrincipal.getName(), federatedRole.getName())) - .isInstanceOf(ValidationException.class); + public static Object[][] federatedIdentityArugments() { + return new Object[][] { + {true, false}, + {false, true}, + {true, true}, + }; } - @Test - public void testCannotAddFederatedPrincipalToFederatedRole() { + @ParameterizedTest + @MethodSource("federatedIdentityArugments") + public void testCannotAssignFederatedEntities( + boolean isFederatedPrincipal, boolean isFederatedRole) { PolarisMetaStoreManager metaStoreManager = setupMetaStoreManager(); PolarisCallContext callContext = setupCallContext(metaStoreManager); PolarisAdminService polarisAdminService = setupPolarisAdminService(metaStoreManager, callContext); - PrincipalEntity federatedPrincipal = - createPrincipal(metaStoreManager, callContext, "federated_principal", true); - metaStoreManager.createPrincipal(callContext, federatedPrincipal); + String principalPrefix = isFederatedPrincipal ? "federated_" : ""; + PrincipalEntity principal = + createPrincipal( + metaStoreManager, callContext, principalPrefix + "principal_id", isFederatedPrincipal); + metaStoreManager.createPrincipal(callContext, principal); - PrincipalRoleEntity federatedRole = - createRole(metaStoreManager, callContext, "federated_role", true); - EntityResult result = - metaStoreManager.createEntityIfNotExists(callContext, null, federatedRole); + String rolePrefix = isFederatedRole ? "federated_" : ""; + PrincipalRoleEntity role = + createRole(metaStoreManager, callContext, rolePrefix + "role_id", isFederatedRole); + EntityResult result = metaStoreManager.createEntityIfNotExists(callContext, null, role); assertThat(result.isSuccess()).isTrue(); assertThatThrownBy( - () -> - polarisAdminService.assignPrincipalRole( - federatedPrincipal.getName(), federatedRole.getName())) + () -> polarisAdminService.assignPrincipalRole(principal.getName(), role.getName())) .isInstanceOf(ValidationException.class); } } diff --git a/service/common/src/main/java/org/apache/polaris/service/admin/PolarisAdminService.java b/service/common/src/main/java/org/apache/polaris/service/admin/PolarisAdminService.java index 619e0f46ef..3e7bf92263 100644 --- a/service/common/src/main/java/org/apache/polaris/service/admin/PolarisAdminService.java +++ b/service/common/src/main/java/org/apache/polaris/service/admin/PolarisAdminService.java @@ -87,6 +87,7 @@ import org.apache.polaris.core.entity.PrincipalEntity; import org.apache.polaris.core.entity.PrincipalRoleEntity; import org.apache.polaris.core.entity.table.IcebergTableLikeEntity; +import org.apache.polaris.core.entity.table.federated.FederatedEntities; import org.apache.polaris.core.persistence.PolarisEntityManager; import org.apache.polaris.core.persistence.PolarisMetaStoreManager; import org.apache.polaris.core.persistence.PolarisResolvedPathWrapper; @@ -910,7 +911,8 @@ public PrincipalWithCredentials createPrincipal(PolarisEntity entity) { PolarisAuthorizableOperation op = PolarisAuthorizableOperation.CREATE_PRINCIPAL; authorizeBasicRootOperationOrThrow(op); - if (PolarisEntity.isFederated(entity)) { + // the API should prevent this from happening + if (FederatedEntities.isFederated(entity)) { throw new ValidationException("Cannot create a federated principal"); } checkArgument(entity.getId() == -1, "Entity to be created must have no ID assigned"); @@ -975,7 +977,7 @@ public void deletePrincipal(String name) { findPrincipalByName(name) .orElseThrow(() -> new NotFoundException("Principal %s not found", name)); - if (PolarisEntity.isFederated(currentPrincipalEntity)) { + if (FederatedEntities.isFederated(currentPrincipalEntity)) { throw new ValidationException( "Cannot update a federated principal: %s", currentPrincipalEntity.getName()); } @@ -1012,7 +1014,7 @@ public void deletePrincipal(String name) { findPrincipalByName(principalName) .orElseThrow(() -> new NotFoundException("Principal %s not found", principalName)); - if (PolarisEntity.isFederated(currentPrincipalEntity)) { + if (FederatedEntities.isFederated(currentPrincipalEntity)) { throw new ValidationException( "Cannot rotate/reset credentials for a federated principal: %s", principalName); } @@ -1342,14 +1344,14 @@ public boolean assignPrincipalRole(String principalName, String principalRoleNam PolarisEntity principalEntity = findPrincipalByName(principalName) .orElseThrow(() -> new NotFoundException("Principal %s not found", principalName)); - if (PolarisEntity.isFederated(principalEntity)) { + if (FederatedEntities.isFederated(principalEntity)) { throw new ValidationException("Cannot assign a role to a federated principal"); } PolarisEntity principalRoleEntity = findPrincipalRoleByName(principalRoleName) .orElseThrow( () -> new NotFoundException("PrincipalRole %s not found", principalRoleName)); - if (PolarisEntity.isFederated(principalRoleEntity)) { + if (FederatedEntities.isFederated(principalRoleEntity)) { throw new ValidationException("Cannot assign a federated role to a principal"); } return metaStoreManager @@ -1365,14 +1367,14 @@ public boolean revokePrincipalRole(String principalName, String principalRoleNam PolarisEntity principalEntity = findPrincipalByName(principalName) .orElseThrow(() -> new NotFoundException("Principal %s not found", principalName)); - if (PolarisEntity.isFederated(principalEntity)) { + if (FederatedEntities.isFederated(principalEntity)) { throw new ValidationException("Cannot revoke a role from a federated principal"); } PolarisEntity principalRoleEntity = findPrincipalRoleByName(principalRoleName) .orElseThrow( () -> new NotFoundException("PrincipalRole %s not found", principalRoleName)); - if (PolarisEntity.isFederated(principalRoleEntity)) { + if (FederatedEntities.isFederated(principalRoleEntity)) { throw new ValidationException("Cannot revoke a federated role from a principal"); } return metaStoreManager diff --git a/service/common/src/main/java/org/apache/polaris/service/admin/PolarisServiceImpl.java b/service/common/src/main/java/org/apache/polaris/service/admin/PolarisServiceImpl.java index e4e351a99c..f5e07f90d0 100644 --- a/service/common/src/main/java/org/apache/polaris/service/admin/PolarisServiceImpl.java +++ b/service/common/src/main/java/org/apache/polaris/service/admin/PolarisServiceImpl.java @@ -29,6 +29,7 @@ import org.apache.iceberg.exceptions.NotAuthorizedException; import org.apache.polaris.core.PolarisCallContext; import org.apache.polaris.core.admin.model.AddGrantRequest; +import org.apache.polaris.core.admin.model.BasePrincipal; import org.apache.polaris.core.admin.model.Catalog; import org.apache.polaris.core.admin.model.CatalogGrant; import org.apache.polaris.core.admin.model.CatalogRole; @@ -44,7 +45,6 @@ import org.apache.polaris.core.admin.model.GrantResource; import org.apache.polaris.core.admin.model.GrantResources; import org.apache.polaris.core.admin.model.NamespaceGrant; -import org.apache.polaris.core.admin.model.Principal; import org.apache.polaris.core.admin.model.PrincipalRole; import org.apache.polaris.core.admin.model.PrincipalRoles; import org.apache.polaris.core.admin.model.PrincipalWithCredentials; @@ -270,7 +270,7 @@ public Response deletePrincipal( public Response getPrincipal( String principalName, RealmContext realmContext, SecurityContext securityContext) { PolarisAdminService adminService = newAdminService(realmContext, securityContext); - return Response.ok(adminService.getPrincipal(principalName).asPrincipal()).build(); + return Response.ok(adminService.getPrincipal(principalName).asBasePrincipal()).build(); } /** From PolarisPrincipalsApiService */ @@ -297,10 +297,10 @@ public Response rotateCredentials( @Override public Response listPrincipals(RealmContext realmContext, SecurityContext securityContext) { PolarisAdminService adminService = newAdminService(realmContext, securityContext); - List principalList = + List principalList = adminService.listPrincipals().stream() .map(PrincipalEntity::new) - .map(PrincipalEntity::asPrincipal) + .map(PrincipalEntity::asBasePrincipal) .toList(); Principals principals = new Principals(principalList); LOGGER.debug("listPrincipals returning: {}", principals); @@ -534,10 +534,10 @@ public Response revokeCatalogRoleFromPrincipalRole( public Response listAssigneePrincipalsForPrincipalRole( String principalRoleName, RealmContext realmContext, SecurityContext securityContext) { PolarisAdminService adminService = newAdminService(realmContext, securityContext); - List principalList = + List principalList = adminService.listAssigneePrincipalsForPrincipalRole(principalRoleName).stream() .map(PrincipalEntity::new) - .map(PrincipalEntity::asPrincipal) + .map(PrincipalEntity::asBasePrincipal) .toList(); Principals principals = new Principals(principalList); LOGGER.debug("listAssigneePrincipalsForPrincipalRole returning: {}", principals); diff --git a/service/common/src/main/java/org/apache/polaris/service/config/Serializers.java b/service/common/src/main/java/org/apache/polaris/service/config/Serializers.java index 6f6701f58e..81f0c671e5 100644 --- a/service/common/src/main/java/org/apache/polaris/service/config/Serializers.java +++ b/service/common/src/main/java/org/apache/polaris/service/config/Serializers.java @@ -28,12 +28,14 @@ import com.fasterxml.jackson.databind.node.ObjectNode; import java.io.IOException; import org.apache.polaris.core.admin.model.AddGrantRequest; +import org.apache.polaris.core.admin.model.BasePrincipal; import org.apache.polaris.core.admin.model.Catalog; import org.apache.polaris.core.admin.model.CatalogRole; import org.apache.polaris.core.admin.model.CreateCatalogRequest; import org.apache.polaris.core.admin.model.CreateCatalogRoleRequest; import org.apache.polaris.core.admin.model.CreatePrincipalRequest; import org.apache.polaris.core.admin.model.CreatePrincipalRoleRequest; +import org.apache.polaris.core.admin.model.FederatedPrincipal; import org.apache.polaris.core.admin.model.GrantCatalogRoleRequest; import org.apache.polaris.core.admin.model.GrantPrincipalRoleRequest; import org.apache.polaris.core.admin.model.GrantResource; @@ -58,6 +60,25 @@ public static void registerSerializers(ObjectMapper mapper) { GrantCatalogRoleRequest.class, new GrantCatalogRoleRequestDeserializer()); module.addDeserializer(AddGrantRequest.class, new AddGrantRequestDeserializer()); module.addDeserializer(RevokeGrantRequest.class, new RevokeGrantRequestDeserializer()); + module.addDeserializer( + BasePrincipal.class, + new JsonDeserializer() { + @Override + public BasePrincipal deserialize( + JsonParser jsonParser, DeserializationContext deserializationContext) + throws IOException { + TreeNode jsonNode = jsonParser.readValueAsTree(); + + if (jsonNode.isObject() + && jsonNode.get("federated") != null + && ((ObjectNode) jsonNode).get("federated").asBoolean()) { + return deserializationContext.readTreeAsValue( + (JsonNode) jsonNode, FederatedPrincipal.class); + } else { + return deserializationContext.readTreeAsValue((JsonNode) jsonNode, Principal.class); + } + } + }); mapper.registerModule(module); } diff --git a/spec/polaris-management-service.yml b/spec/polaris-management-service.yml index 2f0490e789..0a0a24517a 100644 --- a/spec/polaris-management-service.yml +++ b/spec/polaris-management-service.yml @@ -187,7 +187,9 @@ paths: content: application/json: schema: - $ref: "#/components/schemas/Principal" + oneOf: + - $ref: "#/components/schemas/Principal" + - $ref: "#/components/schemas/FederatedPrincipal" 403: description: "The caller does not have permission to get principal details" 404: @@ -443,7 +445,9 @@ paths: content: application/json: schema: - $ref: "#/components/schemas/Principals" + oneOf: + - $ref: "#/components/schemas/Principal" + - $ref: "#/components/schemas/FederatedPrincipal" 403: description: "The caller does not have permission to list principals" 404: @@ -1057,7 +1061,7 @@ components: principals: type: array items: - $ref: "#/components/schemas/Principal" + $ref: "#/components/schemas/BasePrincipal" required: - principals @@ -1089,8 +1093,7 @@ components: type: boolean description: If true, the initial credentials can only be used to call rotateCredentials - Principal: - description: A Polaris principal. + BasePrincipal: type: object properties: name: @@ -1098,13 +1101,6 @@ components: minLength: 1 maxLength: 256 pattern: '^(?!\s*[s|S][y|Y][s|S][t|T][e|E][m|M]\$).*$' - clientId: - type: string - description: The output-only OAuth clientId associated with this principal if applicable - federated: - type: boolean - description: Whether the principal is a federated identity (that is, managed by an external identity provider) - default: false properties: type: object additionalProperties: @@ -1121,6 +1117,30 @@ components: required: - name + Principal: + description: A Polaris principal. + type: object + allOf: + - $ref: '#/components/schemas/BasePrincipal' + - type: object + properties: + clientId: + type: string + description: The output-only OAuth clientId associated with this principal if applicable + + FederatedPrincipal: + description: A federated Polaris principal. + type: object + allOf: + - $ref: '#/components/schemas/BasePrincipal' + - type: object + properties: + federated: + type: boolean + description: Whether the principal is a federated identity (that is, managed by an external identity provider) + enum: + - true + UpdatePrincipalRequest: description: Updates to apply to a Principal type: object From 75351f88ba703310f087111cae400c0ba34b6507 Mon Sep 17 00:00:00 2001 From: Michael Collado Date: Wed, 30 Apr 2025 21:59:33 -0700 Subject: [PATCH 03/10] Changed builder to allow setting federated status twice --- .../org/apache/polaris/core/entity/PrincipalRoleEntity.java | 2 ++ 1 file changed, 2 insertions(+) diff --git a/polaris-core/src/main/java/org/apache/polaris/core/entity/PrincipalRoleEntity.java b/polaris-core/src/main/java/org/apache/polaris/core/entity/PrincipalRoleEntity.java index 96a759794c..3eb2d00506 100644 --- a/polaris-core/src/main/java/org/apache/polaris/core/entity/PrincipalRoleEntity.java +++ b/polaris-core/src/main/java/org/apache/polaris/core/entity/PrincipalRoleEntity.java @@ -69,6 +69,8 @@ public Builder(PrincipalRoleEntity original) { public Builder setFederated(Boolean isFederated) { if (isFederated != null && isFederated) { internalProperties.put(FederatedEntities.FEDERATED_ENTITY, "true"); + } else { + internalProperties.remove(FederatedEntities.FEDERATED_ENTITY); } return this; } From 2197bb8c62341aea446fb11aea2ac53f073d4fbb Mon Sep 17 00:00:00 2001 From: Michael Collado Date: Mon, 5 May 2025 10:32:57 -0700 Subject: [PATCH 04/10] Revert spec changes - add 'federated' property back to Principal entity --- .../polaris/service/it/env/ManagementApi.java | 3 +- ...larisManagementServiceIntegrationTest.java | 18 +++----- .../polaris/core/entity/PrincipalEntity.java | 20 ++------- .../service/admin/PolarisServiceImpl.java | 12 ++--- .../polaris/service/config/Serializers.java | 21 --------- spec/polaris-management-service.yml | 44 +++++-------------- 6 files changed, 28 insertions(+), 90 deletions(-) diff --git a/integration-tests/src/main/java/org/apache/polaris/service/it/env/ManagementApi.java b/integration-tests/src/main/java/org/apache/polaris/service/it/env/ManagementApi.java index fc4298757f..83ec020388 100644 --- a/integration-tests/src/main/java/org/apache/polaris/service/it/env/ManagementApi.java +++ b/integration-tests/src/main/java/org/apache/polaris/service/it/env/ManagementApi.java @@ -29,7 +29,6 @@ import java.net.URI; import java.util.List; import java.util.Map; -import org.apache.polaris.core.admin.model.BasePrincipal; import org.apache.polaris.core.admin.model.Catalog; import org.apache.polaris.core.admin.model.CatalogGrant; import org.apache.polaris.core.admin.model.CatalogPrivilege; @@ -223,7 +222,7 @@ public List listCatalogRoles(String catalogName) { } } - public List listPrincipals() { + public List listPrincipals() { try (Response response = request("v1/principals").get()) { assertThat(response.getStatus()).isEqualTo(OK.getStatusCode()); return response.readEntity(Principals.class).getPrincipals(); 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 46442f99cd..17984912cd 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 @@ -18,6 +18,7 @@ */ package org.apache.polaris.service.it.test; +import static javax.ws.rs.core.Response.Status.BAD_REQUEST; import static javax.ws.rs.core.Response.Status.CREATED; import static javax.ws.rs.core.Response.Status.FORBIDDEN; import static org.apache.polaris.service.it.env.PolarisClient.polarisClient; @@ -27,7 +28,6 @@ import com.auth0.jwt.JWT; import com.auth0.jwt.JWTCreator; import com.auth0.jwt.algorithms.Algorithm; -import com.fasterxml.jackson.core.JsonProcessingException; import com.fasterxml.jackson.databind.JsonNode; import com.fasterxml.jackson.databind.ObjectMapper; import com.fasterxml.jackson.databind.node.ObjectNode; @@ -57,7 +57,6 @@ import org.apache.polaris.core.admin.model.CreatePrincipalRequest; import org.apache.polaris.core.admin.model.CreatePrincipalRoleRequest; import org.apache.polaris.core.admin.model.ExternalCatalog; -import org.apache.polaris.core.admin.model.FederatedPrincipal; import org.apache.polaris.core.admin.model.GcpStorageConfigInfo; import org.apache.polaris.core.admin.model.GrantCatalogRoleRequest; import org.apache.polaris.core.admin.model.GrantResource; @@ -876,22 +875,17 @@ public void testCreatePrincipalAndRotateCredentials() { } @Test - public void testCreateFederatedPrincipalFails() throws JsonProcessingException { + public void testCreateFederatedPrincipalFails() { // Create a federated Principal - FederatedPrincipal federatedPrincipal = - new FederatedPrincipal( - true, client.newEntityName("federatedPrincipal"), Map.of(), 0L, 0L, 1); + Principal federatedPrincipal = + new Principal(client.newEntityName("federatedPrincipal"), "abc", true, Map.of(), 0L, 0L, 1); // Attempt to create the federated Principal using the managementApi try (Response createPResponse = managementApi .request("v1/principals") - .post( - Entity.json( - "{\"principal\":" - + new ObjectMapper().writeValueAsString(federatedPrincipal) - + "}"))) { - assertThat(createPResponse).returns(CREATED.getStatusCode(), Response::getStatus); + .post(Entity.json(new CreatePrincipalRequest(federatedPrincipal, false)))) { + assertThat(createPResponse).returns(BAD_REQUEST.getStatusCode(), Response::getStatus); } } diff --git a/polaris-core/src/main/java/org/apache/polaris/core/entity/PrincipalEntity.java b/polaris-core/src/main/java/org/apache/polaris/core/entity/PrincipalEntity.java index 8bb4344045..df988a8afe 100644 --- a/polaris-core/src/main/java/org/apache/polaris/core/entity/PrincipalEntity.java +++ b/polaris-core/src/main/java/org/apache/polaris/core/entity/PrincipalEntity.java @@ -18,8 +18,6 @@ */ package org.apache.polaris.core.entity; -import org.apache.polaris.core.admin.model.BasePrincipal; -import org.apache.polaris.core.admin.model.FederatedPrincipal; import org.apache.polaris.core.admin.model.Principal; import org.apache.polaris.core.entity.table.federated.FederatedEntities; @@ -39,29 +37,17 @@ public static PrincipalEntity of(PolarisBaseEntity sourceEntity) { public static PrincipalEntity fromPrincipal(Principal principal) { return new Builder() .setName(principal.getName()) + .setFederated(principal.getFederated()) .setProperties(principal.getProperties()) .setClientId(principal.getClientId()) .build(); } - public BasePrincipal asBasePrincipal() { - if (FederatedEntities.isFederated(this)) { - return new FederatedPrincipal( - true, - getName(), - getPropertiesAsMap(), - getCreateTimestamp(), - getLastUpdateTimestamp(), - getEntityVersion()); - } else { - return asPrincipal(); - } - } - public Principal asPrincipal() { return new Principal( - getClientId(), getName(), + getClientId(), + FederatedEntities.isFederated(this), getPropertiesAsMap(), getCreateTimestamp(), getLastUpdateTimestamp(), diff --git a/service/common/src/main/java/org/apache/polaris/service/admin/PolarisServiceImpl.java b/service/common/src/main/java/org/apache/polaris/service/admin/PolarisServiceImpl.java index f5e07f90d0..e4e351a99c 100644 --- a/service/common/src/main/java/org/apache/polaris/service/admin/PolarisServiceImpl.java +++ b/service/common/src/main/java/org/apache/polaris/service/admin/PolarisServiceImpl.java @@ -29,7 +29,6 @@ import org.apache.iceberg.exceptions.NotAuthorizedException; import org.apache.polaris.core.PolarisCallContext; import org.apache.polaris.core.admin.model.AddGrantRequest; -import org.apache.polaris.core.admin.model.BasePrincipal; import org.apache.polaris.core.admin.model.Catalog; import org.apache.polaris.core.admin.model.CatalogGrant; import org.apache.polaris.core.admin.model.CatalogRole; @@ -45,6 +44,7 @@ import org.apache.polaris.core.admin.model.GrantResource; import org.apache.polaris.core.admin.model.GrantResources; import org.apache.polaris.core.admin.model.NamespaceGrant; +import org.apache.polaris.core.admin.model.Principal; import org.apache.polaris.core.admin.model.PrincipalRole; import org.apache.polaris.core.admin.model.PrincipalRoles; import org.apache.polaris.core.admin.model.PrincipalWithCredentials; @@ -270,7 +270,7 @@ public Response deletePrincipal( public Response getPrincipal( String principalName, RealmContext realmContext, SecurityContext securityContext) { PolarisAdminService adminService = newAdminService(realmContext, securityContext); - return Response.ok(adminService.getPrincipal(principalName).asBasePrincipal()).build(); + return Response.ok(adminService.getPrincipal(principalName).asPrincipal()).build(); } /** From PolarisPrincipalsApiService */ @@ -297,10 +297,10 @@ public Response rotateCredentials( @Override public Response listPrincipals(RealmContext realmContext, SecurityContext securityContext) { PolarisAdminService adminService = newAdminService(realmContext, securityContext); - List principalList = + List principalList = adminService.listPrincipals().stream() .map(PrincipalEntity::new) - .map(PrincipalEntity::asBasePrincipal) + .map(PrincipalEntity::asPrincipal) .toList(); Principals principals = new Principals(principalList); LOGGER.debug("listPrincipals returning: {}", principals); @@ -534,10 +534,10 @@ public Response revokeCatalogRoleFromPrincipalRole( public Response listAssigneePrincipalsForPrincipalRole( String principalRoleName, RealmContext realmContext, SecurityContext securityContext) { PolarisAdminService adminService = newAdminService(realmContext, securityContext); - List principalList = + List principalList = adminService.listAssigneePrincipalsForPrincipalRole(principalRoleName).stream() .map(PrincipalEntity::new) - .map(PrincipalEntity::asBasePrincipal) + .map(PrincipalEntity::asPrincipal) .toList(); Principals principals = new Principals(principalList); LOGGER.debug("listAssigneePrincipalsForPrincipalRole returning: {}", principals); diff --git a/service/common/src/main/java/org/apache/polaris/service/config/Serializers.java b/service/common/src/main/java/org/apache/polaris/service/config/Serializers.java index 81f0c671e5..6f6701f58e 100644 --- a/service/common/src/main/java/org/apache/polaris/service/config/Serializers.java +++ b/service/common/src/main/java/org/apache/polaris/service/config/Serializers.java @@ -28,14 +28,12 @@ import com.fasterxml.jackson.databind.node.ObjectNode; import java.io.IOException; import org.apache.polaris.core.admin.model.AddGrantRequest; -import org.apache.polaris.core.admin.model.BasePrincipal; import org.apache.polaris.core.admin.model.Catalog; import org.apache.polaris.core.admin.model.CatalogRole; import org.apache.polaris.core.admin.model.CreateCatalogRequest; import org.apache.polaris.core.admin.model.CreateCatalogRoleRequest; import org.apache.polaris.core.admin.model.CreatePrincipalRequest; import org.apache.polaris.core.admin.model.CreatePrincipalRoleRequest; -import org.apache.polaris.core.admin.model.FederatedPrincipal; import org.apache.polaris.core.admin.model.GrantCatalogRoleRequest; import org.apache.polaris.core.admin.model.GrantPrincipalRoleRequest; import org.apache.polaris.core.admin.model.GrantResource; @@ -60,25 +58,6 @@ public static void registerSerializers(ObjectMapper mapper) { GrantCatalogRoleRequest.class, new GrantCatalogRoleRequestDeserializer()); module.addDeserializer(AddGrantRequest.class, new AddGrantRequestDeserializer()); module.addDeserializer(RevokeGrantRequest.class, new RevokeGrantRequestDeserializer()); - module.addDeserializer( - BasePrincipal.class, - new JsonDeserializer() { - @Override - public BasePrincipal deserialize( - JsonParser jsonParser, DeserializationContext deserializationContext) - throws IOException { - TreeNode jsonNode = jsonParser.readValueAsTree(); - - if (jsonNode.isObject() - && jsonNode.get("federated") != null - && ((ObjectNode) jsonNode).get("federated").asBoolean()) { - return deserializationContext.readTreeAsValue( - (JsonNode) jsonNode, FederatedPrincipal.class); - } else { - return deserializationContext.readTreeAsValue((JsonNode) jsonNode, Principal.class); - } - } - }); mapper.registerModule(module); } diff --git a/spec/polaris-management-service.yml b/spec/polaris-management-service.yml index 0a0a24517a..2f0490e789 100644 --- a/spec/polaris-management-service.yml +++ b/spec/polaris-management-service.yml @@ -187,9 +187,7 @@ paths: content: application/json: schema: - oneOf: - - $ref: "#/components/schemas/Principal" - - $ref: "#/components/schemas/FederatedPrincipal" + $ref: "#/components/schemas/Principal" 403: description: "The caller does not have permission to get principal details" 404: @@ -445,9 +443,7 @@ paths: content: application/json: schema: - oneOf: - - $ref: "#/components/schemas/Principal" - - $ref: "#/components/schemas/FederatedPrincipal" + $ref: "#/components/schemas/Principals" 403: description: "The caller does not have permission to list principals" 404: @@ -1061,7 +1057,7 @@ components: principals: type: array items: - $ref: "#/components/schemas/BasePrincipal" + $ref: "#/components/schemas/Principal" required: - principals @@ -1093,7 +1089,8 @@ components: type: boolean description: If true, the initial credentials can only be used to call rotateCredentials - BasePrincipal: + Principal: + description: A Polaris principal. type: object properties: name: @@ -1101,6 +1098,13 @@ components: minLength: 1 maxLength: 256 pattern: '^(?!\s*[s|S][y|Y][s|S][t|T][e|E][m|M]\$).*$' + clientId: + type: string + description: The output-only OAuth clientId associated with this principal if applicable + federated: + type: boolean + description: Whether the principal is a federated identity (that is, managed by an external identity provider) + default: false properties: type: object additionalProperties: @@ -1117,30 +1121,6 @@ components: required: - name - Principal: - description: A Polaris principal. - type: object - allOf: - - $ref: '#/components/schemas/BasePrincipal' - - type: object - properties: - clientId: - type: string - description: The output-only OAuth clientId associated with this principal if applicable - - FederatedPrincipal: - description: A federated Polaris principal. - type: object - allOf: - - $ref: '#/components/schemas/BasePrincipal' - - type: object - properties: - federated: - type: boolean - description: Whether the principal is a federated identity (that is, managed by an external identity provider) - enum: - - true - UpdatePrincipalRequest: description: Updates to apply to a Principal type: object From c7d10ef4fcb7217484baef97b0e2d7348d044033 Mon Sep 17 00:00:00 2001 From: Michael Collado Date: Mon, 5 May 2025 10:58:36 -0700 Subject: [PATCH 05/10] Fixed builder to remove federated property --- .../java/org/apache/polaris/core/entity/PrincipalEntity.java | 2 ++ 1 file changed, 2 insertions(+) diff --git a/polaris-core/src/main/java/org/apache/polaris/core/entity/PrincipalEntity.java b/polaris-core/src/main/java/org/apache/polaris/core/entity/PrincipalEntity.java index df988a8afe..372a9bb341 100644 --- a/polaris-core/src/main/java/org/apache/polaris/core/entity/PrincipalEntity.java +++ b/polaris-core/src/main/java/org/apache/polaris/core/entity/PrincipalEntity.java @@ -84,6 +84,8 @@ public Builder setCredentialRotationRequiredState() { public Builder setFederated(Boolean isFederated) { if (isFederated != null && isFederated) { internalProperties.put(FederatedEntities.FEDERATED_ENTITY, "true"); + } else { + internalProperties.remove(FederatedEntities.FEDERATED_ENTITY); } return this; } From 257cc370329d6dfc456080957502d120f2d59b8d Mon Sep 17 00:00:00 2001 From: Michael Collado Date: Tue, 6 May 2025 09:31:33 -0700 Subject: [PATCH 06/10] Removed unnecessary openapi config flags --- api/iceberg-service/build.gradle.kts | 1 - api/management-model/build.gradle.kts | 2 -- api/polaris-catalog-service/build.gradle.kts | 1 - 3 files changed, 4 deletions(-) diff --git a/api/iceberg-service/build.gradle.kts b/api/iceberg-service/build.gradle.kts index d1fdf0cc78..f8e348c42f 100644 --- a/api/iceberg-service/build.gradle.kts +++ b/api/iceberg-service/build.gradle.kts @@ -66,7 +66,6 @@ openApiGenerate { configOptions.put("useBeanValidation", "false") configOptions.put("sourceFolder", "src/main/java") configOptions.put("useJakartaEe", "true") - openapiNormalizer.put("REFACTOR_ALLOF_WITH_PROPERTIES_ONLY", "true") additionalProperties.put("apiNamePrefix", "IcebergRest") additionalProperties.put("apiNameSuffix", "") additionalProperties.put("metricsPrefix", "polaris") diff --git a/api/management-model/build.gradle.kts b/api/management-model/build.gradle.kts index f1c6258587..ec78abe5d5 100644 --- a/api/management-model/build.gradle.kts +++ b/api/management-model/build.gradle.kts @@ -57,8 +57,6 @@ openApiGenerate { additionalProperties.put("apiNameSuffix", "Api") additionalProperties.put("metricsPrefix", "polaris") serverVariables = mapOf("basePath" to "api/v1") - openapiNormalizer.put("REFACTOR_ALLOF_WITH_PROPERTIES_ONLY", "true") - openapiNormalizer.put("REF_AS_PARENT_IN_ALLOF", "true") } listOf("sourcesJar", "compileJava").forEach { task -> diff --git a/api/polaris-catalog-service/build.gradle.kts b/api/polaris-catalog-service/build.gradle.kts index d8f2cbff4e..7b4ea7454c 100644 --- a/api/polaris-catalog-service/build.gradle.kts +++ b/api/polaris-catalog-service/build.gradle.kts @@ -96,7 +96,6 @@ openApiGenerate { configOptions.put("generateBuilders", "true") configOptions.put("generateConstructorWithAllArgs", "true") configOptions.put("openApiNullable", "false") - openapiNormalizer.put("REFACTOR_ALLOF_WITH_PROPERTIES_ONLY", "true") additionalProperties.put("apiNamePrefix", "PolarisCatalog") additionalProperties.put("apiNameSuffix", "") additionalProperties.put("metricsPrefix", "polaris") From bcbe4ee9227f9bf9f0a1ef4fcc65e3f596c037d4 Mon Sep 17 00:00:00 2001 From: Michael Collado Date: Wed, 7 May 2025 11:56:46 -0700 Subject: [PATCH 07/10] Fix compilation issue in test --- .../polaris/service/quarkus/admin/ManagementServiceTest.java | 2 ++ 1 file changed, 2 insertions(+) diff --git a/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/admin/ManagementServiceTest.java b/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/admin/ManagementServiceTest.java index e76a0ccd07..f7d0759768 100644 --- a/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/admin/ManagementServiceTest.java +++ b/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/admin/ManagementServiceTest.java @@ -48,6 +48,7 @@ import org.apache.polaris.core.persistence.MetaStoreManagerFactory; import org.apache.polaris.core.persistence.PolarisMetaStoreManager; import org.apache.polaris.core.persistence.dao.entity.EntityResult; +import org.apache.polaris.core.secrets.UnsafeInMemorySecretsManager; import org.apache.polaris.service.TestServices; import org.apache.polaris.service.admin.PolarisAdminService; import org.apache.polaris.service.config.DefaultConfigurationStore; @@ -197,6 +198,7 @@ private PolarisAdminService setupPolarisAdminService( CallContext.of(realmContext, callContext), services.entityManagerFactory().getOrCreateEntityManager(realmContext), metaStoreManager, + new UnsafeInMemorySecretsManager(), new SecurityContext() { @Override public Principal getUserPrincipal() { From d5a88b825b50ff57f7a7f332343ffcb255743fb1 Mon Sep 17 00:00:00 2001 From: Michael Collado Date: Mon, 12 May 2025 10:56:52 -0700 Subject: [PATCH 08/10] Remove federated flag from principal entity --- .../PolarisManagementServiceIntegrationTest.java | 16 ---------------- .../polaris/core/entity/PrincipalEntity.java | 2 -- spec/polaris-management-service.yml | 4 ---- 3 files changed, 22 deletions(-) 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 17984912cd..948b1e195d 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 @@ -18,7 +18,6 @@ */ package org.apache.polaris.service.it.test; -import static javax.ws.rs.core.Response.Status.BAD_REQUEST; import static javax.ws.rs.core.Response.Status.CREATED; import static javax.ws.rs.core.Response.Status.FORBIDDEN; import static org.apache.polaris.service.it.env.PolarisClient.polarisClient; @@ -874,21 +873,6 @@ public void testCreatePrincipalAndRotateCredentials() { // rotation that makes the old secret fall off retention. } - @Test - public void testCreateFederatedPrincipalFails() { - // Create a federated Principal - Principal federatedPrincipal = - new Principal(client.newEntityName("federatedPrincipal"), "abc", true, Map.of(), 0L, 0L, 1); - - // Attempt to create the federated Principal using the managementApi - try (Response createPResponse = - managementApi - .request("v1/principals") - .post(Entity.json(new CreatePrincipalRequest(federatedPrincipal, false)))) { - assertThat(createPResponse).returns(BAD_REQUEST.getStatusCode(), Response::getStatus); - } - } - @Test public void testCreateFederatedPrincipalRoleSucceeds() { // Create a federated Principal Role diff --git a/polaris-core/src/main/java/org/apache/polaris/core/entity/PrincipalEntity.java b/polaris-core/src/main/java/org/apache/polaris/core/entity/PrincipalEntity.java index 372a9bb341..ed3d5c83e0 100644 --- a/polaris-core/src/main/java/org/apache/polaris/core/entity/PrincipalEntity.java +++ b/polaris-core/src/main/java/org/apache/polaris/core/entity/PrincipalEntity.java @@ -37,7 +37,6 @@ public static PrincipalEntity of(PolarisBaseEntity sourceEntity) { public static PrincipalEntity fromPrincipal(Principal principal) { return new Builder() .setName(principal.getName()) - .setFederated(principal.getFederated()) .setProperties(principal.getProperties()) .setClientId(principal.getClientId()) .build(); @@ -47,7 +46,6 @@ public Principal asPrincipal() { return new Principal( getName(), getClientId(), - FederatedEntities.isFederated(this), getPropertiesAsMap(), getCreateTimestamp(), getLastUpdateTimestamp(), diff --git a/spec/polaris-management-service.yml b/spec/polaris-management-service.yml index 2f0490e789..0f0a4fc0b0 100644 --- a/spec/polaris-management-service.yml +++ b/spec/polaris-management-service.yml @@ -1101,10 +1101,6 @@ components: clientId: type: string description: The output-only OAuth clientId associated with this principal if applicable - federated: - type: boolean - description: Whether the principal is a federated identity (that is, managed by an external identity provider) - default: false properties: type: object additionalProperties: From 60e18eaa6654828e867e5d13599ff5fec3d039de Mon Sep 17 00:00:00 2001 From: Michael Collado Date: Wed, 14 May 2025 09:37:48 -0700 Subject: [PATCH 09/10] Fixed builder oversight --- .../polaris/core/entity/PrincipalEntity.java | 10 ------ .../quarkus/admin/ManagementServiceTest.java | 31 +++---------------- 2 files changed, 5 insertions(+), 36 deletions(-) diff --git a/polaris-core/src/main/java/org/apache/polaris/core/entity/PrincipalEntity.java b/polaris-core/src/main/java/org/apache/polaris/core/entity/PrincipalEntity.java index ed3d5c83e0..ba0cfe3f53 100644 --- a/polaris-core/src/main/java/org/apache/polaris/core/entity/PrincipalEntity.java +++ b/polaris-core/src/main/java/org/apache/polaris/core/entity/PrincipalEntity.java @@ -19,7 +19,6 @@ package org.apache.polaris.core.entity; import org.apache.polaris.core.admin.model.Principal; -import org.apache.polaris.core.entity.table.federated.FederatedEntities; /** Wrapper for translating between the REST Principal object and the base PolarisEntity type. */ public class PrincipalEntity extends PolarisEntity { @@ -79,15 +78,6 @@ public Builder setCredentialRotationRequiredState() { return this; } - public Builder setFederated(Boolean isFederated) { - if (isFederated != null && isFederated) { - internalProperties.put(FederatedEntities.FEDERATED_ENTITY, "true"); - } else { - internalProperties.remove(FederatedEntities.FEDERATED_ENTITY); - } - return this; - } - @Override public PrincipalEntity build() { return new PrincipalEntity(buildBase()); diff --git a/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/admin/ManagementServiceTest.java b/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/admin/ManagementServiceTest.java index f7d0759768..618c9b8fff 100644 --- a/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/admin/ManagementServiceTest.java +++ b/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/admin/ManagementServiceTest.java @@ -54,8 +54,6 @@ import org.apache.polaris.service.config.DefaultConfigurationStore; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; -import org.junit.jupiter.params.ParameterizedTest; -import org.junit.jupiter.params.provider.MethodSource; import org.mockito.Mockito; public class ManagementServiceTest { @@ -225,12 +223,8 @@ public String getAuthenticationScheme() { } private PrincipalEntity createPrincipal( - PolarisMetaStoreManager metaStoreManager, - PolarisCallContext callContext, - String name, - boolean isFederated) { + PolarisMetaStoreManager metaStoreManager, PolarisCallContext callContext, String name) { return new PrincipalEntity.Builder() - .setFederated(isFederated) .setName(name) .setCreateTimestamp(Instant.now().toEpochMilli()) .setId(metaStoreManager.generateNewEntityId(callContext).getId()) @@ -252,32 +246,17 @@ private PrincipalRoleEntity createRole( .build(); } - public static Object[][] federatedIdentityArugments() { - return new Object[][] { - {true, false}, - {false, true}, - {true, true}, - }; - } - - @ParameterizedTest - @MethodSource("federatedIdentityArugments") - public void testCannotAssignFederatedEntities( - boolean isFederatedPrincipal, boolean isFederatedRole) { + @Test + public void testCannotAssignFederatedEntities() { PolarisMetaStoreManager metaStoreManager = setupMetaStoreManager(); PolarisCallContext callContext = setupCallContext(metaStoreManager); PolarisAdminService polarisAdminService = setupPolarisAdminService(metaStoreManager, callContext); - String principalPrefix = isFederatedPrincipal ? "federated_" : ""; - PrincipalEntity principal = - createPrincipal( - metaStoreManager, callContext, principalPrefix + "principal_id", isFederatedPrincipal); + PrincipalEntity principal = createPrincipal(metaStoreManager, callContext, "principal_id"); metaStoreManager.createPrincipal(callContext, principal); - String rolePrefix = isFederatedRole ? "federated_" : ""; - PrincipalRoleEntity role = - createRole(metaStoreManager, callContext, rolePrefix + "role_id", isFederatedRole); + PrincipalRoleEntity role = createRole(metaStoreManager, callContext, "federated_role_id", true); EntityResult result = metaStoreManager.createEntityIfNotExists(callContext, null, role); assertThat(result.isSuccess()).isTrue(); From 9509dff8ca42e81046d4003d3da52403ebc76f44 Mon Sep 17 00:00:00 2001 From: Michael Collado Date: Fri, 16 May 2025 12:04:41 -0700 Subject: [PATCH 10/10] Fix compilation failures and rebase on main --- .../PolarisManagementServiceIntegrationTest.java | 14 ++++++++++++-- .../quarkus/admin/ManagementServiceTest.java | 14 +++++++++++++- 2 files changed, 25 insertions(+), 3 deletions(-) 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 948b1e195d..d0f18bf5ab 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 @@ -2181,7 +2181,12 @@ public void testCreateAndUpdatePrincipalRoleWithReservedProperties() { PrincipalRole badPrincipalRole = new PrincipalRole( - client.newEntityName("myprincipalrole"), Map.of("polaris.reserved", "foo"), 0L, 0L, 1); + client.newEntityName("myprincipalrole"), + false, + Map.of("polaris.reserved", "foo"), + 0L, + 0L, + 1); try (Response response = managementApi .request("v1/principal-roles") @@ -2192,7 +2197,12 @@ public void testCreateAndUpdatePrincipalRoleWithReservedProperties() { PrincipalRole goodPrincipalRole = new PrincipalRole( - client.newEntityName("myprincipalrole"), Map.of("not.reserved", "foo"), 0L, 0L, 1); + client.newEntityName("myprincipalrole"), + false, + Map.of("not.reserved", "foo"), + 0L, + 0L, + 1); try (Response response = managementApi .request("v1/principal-roles") diff --git a/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/admin/ManagementServiceTest.java b/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/admin/ManagementServiceTest.java index 618c9b8fff..ff42c384a7 100644 --- a/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/admin/ManagementServiceTest.java +++ b/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/admin/ManagementServiceTest.java @@ -52,6 +52,7 @@ import org.apache.polaris.service.TestServices; import org.apache.polaris.service.admin.PolarisAdminService; import org.apache.polaris.service.config.DefaultConfigurationStore; +import org.apache.polaris.service.config.ReservedProperties; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.mockito.Mockito; @@ -219,7 +220,18 @@ public String getAuthenticationScheme() { return ""; } }, - new PolarisAuthorizerImpl(new DefaultConfigurationStore(Map.of()))); + new PolarisAuthorizerImpl(new DefaultConfigurationStore(Map.of())), + new ReservedProperties() { + @Override + public List prefixes() { + return List.of(); + } + + @Override + public Set allowlist() { + return Set.of(); + } + }); } private PrincipalEntity createPrincipal(