From 0c3651ad54d9332f00efb22452633462e304a7b5 Mon Sep 17 00:00:00 2001 From: "arun.suri" Date: Mon, 28 Jul 2025 18:46:24 +0530 Subject: [PATCH 01/29] initial commit --- ...olarisEclipseLinkMetaStoreSessionImpl.java | 6 ++ .../jdbc/JdbcBasePersistenceImpl.java | 64 +++++++++++++++++++ .../core/auth/PolarisSecretsManager.java | 14 ++++ .../core/entity/PolarisPrincipalSecrets.java | 15 ++++- .../AtomicOperationMetaStoreManager.java | 53 +++++++++++++++ .../persistence/IntegrationPersistence.java | 23 +++++++ .../TransactionWorkspaceMetaStoreManager.java | 15 +++++ .../TransactionalMetaStoreManagerImpl.java | 16 +++++ .../TreeMapTransactionalPersistenceImpl.java | 16 +++++ .../service/admin/PolarisServiceImpl.java | 15 ++++- spec/polaris-management-service.yml | 52 +++++++++++++++ 11 files changed, 286 insertions(+), 3 deletions(-) diff --git a/persistence/eclipselink/src/main/java/org/apache/polaris/extension/persistence/impl/eclipselink/PolarisEclipseLinkMetaStoreSessionImpl.java b/persistence/eclipselink/src/main/java/org/apache/polaris/extension/persistence/impl/eclipselink/PolarisEclipseLinkMetaStoreSessionImpl.java index 75c4c4ad4f..df6233f124 100644 --- a/persistence/eclipselink/src/main/java/org/apache/polaris/extension/persistence/impl/eclipselink/PolarisEclipseLinkMetaStoreSessionImpl.java +++ b/persistence/eclipselink/src/main/java/org/apache/polaris/extension/persistence/impl/eclipselink/PolarisEclipseLinkMetaStoreSessionImpl.java @@ -747,4 +747,10 @@ Optional> hasOverlappingSiblings( @Nonnull PolarisCallContext callContext, T entity) { return Optional.empty(); } + + @Nullable + @Override + public PolarisPrincipalSecrets resetPrincipalSecrets(@Nonnull PolarisCallContext callCtx, @Nonnull String clientId, long principalId, boolean reset, @Nonnull String oldSecretHash, String customClientId, String customClientSecret) { + return null; + } } diff --git a/persistence/relational-jdbc/src/main/java/org/apache/polaris/persistence/relational/jdbc/JdbcBasePersistenceImpl.java b/persistence/relational-jdbc/src/main/java/org/apache/polaris/persistence/relational/jdbc/JdbcBasePersistenceImpl.java index 71f1d2e7aa..a0168c49cd 100644 --- a/persistence/relational-jdbc/src/main/java/org/apache/polaris/persistence/relational/jdbc/JdbcBasePersistenceImpl.java +++ b/persistence/relational-jdbc/src/main/java/org/apache/polaris/persistence/relational/jdbc/JdbcBasePersistenceImpl.java @@ -773,6 +773,70 @@ public PolarisPrincipalSecrets generateNewPrincipalSecrets( return principalSecrets; } + + @Nullable + @Override + public PolarisPrincipalSecrets resetPrincipalSecrets( + @Nonnull PolarisCallContext callCtx, + @Nonnull String clientId, + long principalId, + boolean reset, + @Nonnull String oldSecretHash, + String customClientId, + String customClientSecret) { + PolarisPrincipalSecrets principalSecrets = loadPrincipalSecrets(callCtx, clientId); + + // should be found + callCtx + .getDiagServices() + .checkNotNull( + principalSecrets, + "cannot_find_secrets", + "client_id={} principalId={}", + clientId, + principalId); + + // ensure principal id is matching + callCtx + .getDiagServices() + .check( + principalId == principalSecrets.getPrincipalId(), + "principal_id_mismatch", + "expectedId={} id={}", + principalId, + principalSecrets.getPrincipalId()); + + principalSecrets.setPrincipalClientId(customClientId); + principalSecrets.resetSecrets(customClientSecret); + + Map params = Map.of("principal_client_id", clientId, "realm_id", realmId); + try { + ModelPrincipalAuthenticationData modelPrincipalAuthenticationData = + ModelPrincipalAuthenticationData.fromPrincipalAuthenticationData(principalSecrets); + datasourceOperations.executeUpdate( + QueryGenerator.generateUpdateQuery( + ModelPrincipalAuthenticationData.ALL_COLUMNS, + ModelPrincipalAuthenticationData.TABLE_NAME, + modelPrincipalAuthenticationData + .toMap(datasourceOperations.getDatabaseType()) + .values() + .stream() + .toList(), + params)); + } catch (SQLException e) { + LOGGER.error( + "Failed to reset PrincipalSecrets for clientId: {}, due to {}", + clientId, + e.getMessage(), + e); + throw new RuntimeException( + String.format("Failed to reset PrincipalSecrets for clientId: %s", clientId), e); + } + + // return those + return principalSecrets; + } + @Nullable @Override public PolarisPrincipalSecrets rotatePrincipalSecrets( diff --git a/polaris-core/src/main/java/org/apache/polaris/core/auth/PolarisSecretsManager.java b/polaris-core/src/main/java/org/apache/polaris/core/auth/PolarisSecretsManager.java index 553feacf7a..b44455668c 100644 --- a/polaris-core/src/main/java/org/apache/polaris/core/auth/PolarisSecretsManager.java +++ b/polaris-core/src/main/java/org/apache/polaris/core/auth/PolarisSecretsManager.java @@ -54,4 +54,18 @@ PrincipalSecretsResult rotatePrincipalSecrets( long principalId, boolean reset, @Nonnull String oldSecretHash); + + @Nonnull + PrincipalSecretsResult resetPrincipalSecrets( + @Nonnull PolarisCallContext callCtx, + @Nonnull String clientId, + long principalId, + boolean reset, + @Nonnull String oldSecretHash, + String customClientId, + String customClientSecret); + + } + + diff --git a/polaris-core/src/main/java/org/apache/polaris/core/entity/PolarisPrincipalSecrets.java b/polaris-core/src/main/java/org/apache/polaris/core/entity/PolarisPrincipalSecrets.java index dc98d46f6a..907f1f4db1 100644 --- a/polaris-core/src/main/java/org/apache/polaris/core/entity/PolarisPrincipalSecrets.java +++ b/polaris-core/src/main/java/org/apache/polaris/core/entity/PolarisPrincipalSecrets.java @@ -36,7 +36,7 @@ public class PolarisPrincipalSecrets { private final long principalId; // the client id for that principal - private final String principalClientId; + private String principalClientId; // the main secret hash for that principal private String mainSecret; @@ -147,6 +147,19 @@ public void rotateSecrets(String newSecondaryHash) { this.mainSecretHash = hashSecret(mainSecret); } + /** Reset the main secrets */ + public void resetSecrets(String customClientSecret) { + this.mainSecret = customClientSecret; + this.mainSecretHash = hashSecret(mainSecret); + + this.secondarySecret = null; + this.secondarySecretHash = hashSecret(mainSecret); + } + + public void setPrincipalClientId(String customClientId) { + this.principalClientId = customClientId; + } + public long getPrincipalId() { return principalId; } diff --git a/polaris-core/src/main/java/org/apache/polaris/core/persistence/AtomicOperationMetaStoreManager.java b/polaris-core/src/main/java/org/apache/polaris/core/persistence/AtomicOperationMetaStoreManager.java index f6f04143f0..628907a01d 100644 --- a/polaris-core/src/main/java/org/apache/polaris/core/persistence/AtomicOperationMetaStoreManager.java +++ b/polaris-core/src/main/java/org/apache/polaris/core/persistence/AtomicOperationMetaStoreManager.java @@ -912,6 +912,59 @@ private void revokeGrantRecord( : new PrincipalSecretsResult(secrets); } + @Override + public @Nonnull PrincipalSecretsResult resetPrincipalSecrets( + @Nonnull PolarisCallContext callCtx, + @Nonnull String clientId, + long principalId, + boolean reset, + @Nonnull String oldSecretHash, + String customClientId, + String customClientSecret) { + // get metastore we should be using + BasePersistence ms = callCtx.getMetaStore(); + + // if not found, the principal must have been dropped + EntityResult loadEntityResult = + loadEntity( + callCtx, PolarisEntityConstants.getNullId(), principalId, PolarisEntityType.PRINCIPAL); + if (loadEntityResult.getReturnStatus() != BaseResult.ReturnStatus.SUCCESS) { + return new PrincipalSecretsResult(BaseResult.ReturnStatus.ENTITY_NOT_FOUND, null); + } + + PolarisBaseEntity principal = loadEntityResult.getEntity(); + Map internalProps = + PolarisObjectMapperUtil.deserializeProperties( + principal.getInternalProperties() == null ? "{}" : principal.getInternalProperties()); + + boolean doReset = + reset + || internalProps.get( + PolarisEntityConstants.PRINCIPAL_CREDENTIAL_ROTATION_REQUIRED_STATE) + != null; + PolarisPrincipalSecrets secrets = + ((IntegrationPersistence) ms) + .resetPrincipalSecrets(callCtx, clientId, principalId, doReset, oldSecretHash,customClientId, customClientSecret); + + PolarisBaseEntity.Builder principalBuilder = new PolarisBaseEntity.Builder(principal); + if (reset + && !internalProps.containsKey( + PolarisEntityConstants.PRINCIPAL_CREDENTIAL_ROTATION_REQUIRED_STATE) + && customClientId != null + && customClientSecret != null) { + internalProps.put( + PolarisEntityConstants.PRINCIPAL_CREDENTIAL_ROTATION_REQUIRED_STATE, "true"); + principalBuilder.internalProperties( + PolarisObjectMapperUtil.serializeProperties(internalProps)); + principalBuilder.entityVersion(principal.getEntityVersion() + 1); + ms.writeEntity(callCtx, principalBuilder.build(), true, principal); + } + + return (secrets == null) + ? new PrincipalSecretsResult(BaseResult.ReturnStatus.ENTITY_NOT_FOUND, null) + : new PrincipalSecretsResult(secrets); + } + /** {@inheritDoc} */ @Override public @Nonnull EntityResult createEntityIfNotExists( diff --git a/polaris-core/src/main/java/org/apache/polaris/core/persistence/IntegrationPersistence.java b/polaris-core/src/main/java/org/apache/polaris/core/persistence/IntegrationPersistence.java index c9374182b6..d3e636c5e0 100644 --- a/polaris-core/src/main/java/org/apache/polaris/core/persistence/IntegrationPersistence.java +++ b/polaris-core/src/main/java/org/apache/polaris/core/persistence/IntegrationPersistence.java @@ -81,6 +81,29 @@ PolarisPrincipalSecrets rotatePrincipalSecrets( boolean reset, @Nonnull String oldSecretHash); + /** + * Reset the secrets of a principal entity, i.e. make the specified secrets as main and secondary + * and assign a new client id + * + * @param callCtx call context + * @param clientId principal client id + * @param principalId principal id + * @param reset true if the principal secrets should be disabled and replaced with a one-time + * password + * @param oldSecretHash the principal secret's old main secret hash + * @param customClientId the principal secret's old main secret hash + * @param customClientSecret the principal secret's old main secret hash + */ + @Nullable + PolarisPrincipalSecrets resetPrincipalSecrets( + @Nonnull PolarisCallContext callCtx, + @Nonnull String clientId, + long principalId, + boolean reset, + @Nonnull String oldSecretHash, + String customClientId, + String customClientSecret); + /** * When dropping a principal, we also need to drop the secrets of that principal * diff --git a/polaris-core/src/main/java/org/apache/polaris/core/persistence/TransactionWorkspaceMetaStoreManager.java b/polaris-core/src/main/java/org/apache/polaris/core/persistence/TransactionWorkspaceMetaStoreManager.java index 3d9f3c0523..929d5fbdda 100644 --- a/polaris-core/src/main/java/org/apache/polaris/core/persistence/TransactionWorkspaceMetaStoreManager.java +++ b/polaris-core/src/main/java/org/apache/polaris/core/persistence/TransactionWorkspaceMetaStoreManager.java @@ -172,6 +172,21 @@ public PrincipalSecretsResult rotatePrincipalSecrets( return null; } + @Override + public PrincipalSecretsResult resetPrincipalSecrets( + @Nonnull PolarisCallContext callCtx, + @Nonnull String clientId, + long principalId, + boolean reset, + @Nonnull String oldSecretHash, + String customClientId, + String customClientSecret) { + callCtx + .getDiagServices() + .fail("illegal_method_in_transaction_workspace", "resetPrincipalSecrets"); + return null; + } + @Override public CreateCatalogResult createCatalog( @Nonnull PolarisCallContext callCtx, diff --git a/polaris-core/src/main/java/org/apache/polaris/core/persistence/transactional/TransactionalMetaStoreManagerImpl.java b/polaris-core/src/main/java/org/apache/polaris/core/persistence/transactional/TransactionalMetaStoreManagerImpl.java index 97af650b08..39cb5f461c 100644 --- a/polaris-core/src/main/java/org/apache/polaris/core/persistence/transactional/TransactionalMetaStoreManagerImpl.java +++ b/polaris-core/src/main/java/org/apache/polaris/core/persistence/transactional/TransactionalMetaStoreManagerImpl.java @@ -977,6 +977,22 @@ private void bootstrapPolarisService( : new PrincipalSecretsResult(secrets); } + @Override + public @Nonnull PrincipalSecretsResult resetPrincipalSecrets( + @Nonnull PolarisCallContext callCtx, + @Nonnull String clientId, + long principalId, + boolean reset, + @Nonnull String oldSecretHash, + String customClientId, + String customClientSecret) { + // get metastore we should be using + callCtx + .getDiagServices() + .fail("illegal_method_in_transaction_workspace", "resetPrincipalSecrets"); + return null; + } + /** {@inheritDoc} */ @Override public @Nonnull CreateCatalogResult createCatalog( diff --git a/polaris-core/src/main/java/org/apache/polaris/core/persistence/transactional/TreeMapTransactionalPersistenceImpl.java b/polaris-core/src/main/java/org/apache/polaris/core/persistence/transactional/TreeMapTransactionalPersistenceImpl.java index 651800621a..bcb1644add 100644 --- a/polaris-core/src/main/java/org/apache/polaris/core/persistence/transactional/TreeMapTransactionalPersistenceImpl.java +++ b/polaris-core/src/main/java/org/apache/polaris/core/persistence/transactional/TreeMapTransactionalPersistenceImpl.java @@ -485,6 +485,22 @@ public int lookupEntityGrantRecordsVersionInCurrentTxn( return principalSecrets; } + @Override + public @Nonnull PolarisPrincipalSecrets resetPrincipalSecrets( + @Nonnull PolarisCallContext callCtx, + @Nonnull String clientId, + long principalId, + boolean reset, + @Nonnull String oldSecretHash, + String customClientId, + String customClientSecret) { + // get metastore we should be using + callCtx + .getDiagServices() + .fail("illegal_method_in_transaction_workspace", "resetPrincipalSecrets"); + return null; + } + /** {@inheritDoc} */ @Override public void deletePrincipalSecretsInCurrentTxn( 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 c455e9c991..a24a8fa34c 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 @@ -54,6 +54,7 @@ import org.apache.polaris.core.admin.model.PrincipalRoles; import org.apache.polaris.core.admin.model.PrincipalWithCredentials; import org.apache.polaris.core.admin.model.Principals; +import org.apache.polaris.core.admin.model.ResetPrincipalRequest; import org.apache.polaris.core.admin.model.RevokeGrantRequest; import org.apache.polaris.core.admin.model.StorageConfigInfo; import org.apache.polaris.core.admin.model.TableGrant; @@ -160,7 +161,7 @@ private static Response toResponse(BaseResult result, Response.Status successSta /** From PolarisCatalogsApiService */ @Override public Response createCatalog( - CreateCatalogRequest request, RealmContext realmContext, SecurityContext securityContext) { + CreateCatalogRequest request, RealmContext realmContext, SecurityContext securityContext) { PolarisAdminService adminService = newAdminService(realmContext, securityContext); Catalog catalog = request.getCatalog(); validateStorageConfig(catalog.getStorageConfigInfo()); @@ -275,7 +276,7 @@ public Response listCatalogs(RealmContext realmContext, SecurityContext security /** From PolarisPrincipalsApiService */ @Override public Response createPrincipal( - CreatePrincipalRequest request, RealmContext realmContext, SecurityContext securityContext) { + CreatePrincipalRequest request, RealmContext realmContext, SecurityContext securityContext) { PolarisAdminService adminService = newAdminService(realmContext, securityContext); PrincipalEntity principal = new PrincipalEntity.Builder() @@ -293,6 +294,16 @@ public Response createPrincipal( return Response.status(Response.Status.CREATED).entity(createdPrincipal).build(); } + @Override + public Response resetCredentials( + String principalName, + ResetPrincipalRequest resetPrincipalRequest, + RealmContext realmContext, + SecurityContext securityContext) { + PolarisAdminService adminService = newAdminService(realmContext, securityContext); + return Response.ok(adminService.resetCredentials(principalName, resetPrincipalRequest)).build(); + } + /** From PolarisPrincipalsApiService */ @Override public Response deletePrincipal( diff --git a/spec/polaris-management-service.yml b/spec/polaris-management-service.yml index 9ec981bc40..ab0f1ff417 100644 --- a/spec/polaris-management-service.yml +++ b/spec/polaris-management-service.yml @@ -199,6 +199,42 @@ paths: 404: description: "The catalog or principal does not exist" + /principals/{principalName}/reset: + parameters: + - name: principalName + in: path + description: The principal's name + required: true + schema: + type: string + minLength: 1 + maxLength: 256 + pattern: '^(?!\s*[s|S][y|Y][s|S][t|T][e|E][m|M]\$).*$' + post: + operationId: resetCredentials + description: > + Reset a principal's credentials to a new set. By default, the system generates random credentials unless + explicitly allowed to accept user-provided credentials via configuration. + This API is *not* idempotent and will return the newly created credentials. + requestBody: + required: false + content: + application/json: + schema: + $ref: '#/components/schemas/ResetPrincipalRequest' + responses: + 200: + description: The principal details along with the newly reset credentials + content: + application/json: + schema: + $ref: "#/components/schemas/PrincipalWithCredentials" + 403: + description: The caller does not have permission to reset credentials + 404: + description: The principal does not exist + + put: operationId: updatePrincipal description: Update an existing principal @@ -1248,6 +1284,22 @@ components: - currentEntityVersion - properties + ResetPrincipalRequest: + type: object + properties: + clientId: + type: string + description: Optional client ID to set for the principal. + minLength: 16 + maxLength: 16 + pattern: '^[0-9a-f]{16}$' + clientSecret: + type: string + description: Optional client secret to set for the principal. + minLength: 32 + maxLength: 32 + pattern: '^[0-9a-f]{32}$' + PrincipalRoles: type: object properties: From 03455e0133661565e80451dd1c7a3bff42764e43 Mon Sep 17 00:00:00 2001 From: "arun.suri" Date: Mon, 28 Jul 2025 18:52:31 +0530 Subject: [PATCH 02/29] formatting --- .../jdbc/JdbcBasePersistenceImpl.java | 73 +++++++++---------- 1 file changed, 36 insertions(+), 37 deletions(-) diff --git a/persistence/relational-jdbc/src/main/java/org/apache/polaris/persistence/relational/jdbc/JdbcBasePersistenceImpl.java b/persistence/relational-jdbc/src/main/java/org/apache/polaris/persistence/relational/jdbc/JdbcBasePersistenceImpl.java index a0168c49cd..e43619755e 100644 --- a/persistence/relational-jdbc/src/main/java/org/apache/polaris/persistence/relational/jdbc/JdbcBasePersistenceImpl.java +++ b/persistence/relational-jdbc/src/main/java/org/apache/polaris/persistence/relational/jdbc/JdbcBasePersistenceImpl.java @@ -773,38 +773,37 @@ public PolarisPrincipalSecrets generateNewPrincipalSecrets( return principalSecrets; } - @Nullable @Override public PolarisPrincipalSecrets resetPrincipalSecrets( - @Nonnull PolarisCallContext callCtx, - @Nonnull String clientId, - long principalId, - boolean reset, - @Nonnull String oldSecretHash, - String customClientId, - String customClientSecret) { + @Nonnull PolarisCallContext callCtx, + @Nonnull String clientId, + long principalId, + boolean reset, + @Nonnull String oldSecretHash, + String customClientId, + String customClientSecret) { PolarisPrincipalSecrets principalSecrets = loadPrincipalSecrets(callCtx, clientId); // should be found callCtx - .getDiagServices() - .checkNotNull( - principalSecrets, - "cannot_find_secrets", - "client_id={} principalId={}", - clientId, - principalId); + .getDiagServices() + .checkNotNull( + principalSecrets, + "cannot_find_secrets", + "client_id={} principalId={}", + clientId, + principalId); // ensure principal id is matching callCtx - .getDiagServices() - .check( - principalId == principalSecrets.getPrincipalId(), - "principal_id_mismatch", - "expectedId={} id={}", - principalId, - principalSecrets.getPrincipalId()); + .getDiagServices() + .check( + principalId == principalSecrets.getPrincipalId(), + "principal_id_mismatch", + "expectedId={} id={}", + principalId, + principalSecrets.getPrincipalId()); principalSecrets.setPrincipalClientId(customClientId); principalSecrets.resetSecrets(customClientSecret); @@ -812,25 +811,25 @@ public PolarisPrincipalSecrets resetPrincipalSecrets( Map params = Map.of("principal_client_id", clientId, "realm_id", realmId); try { ModelPrincipalAuthenticationData modelPrincipalAuthenticationData = - ModelPrincipalAuthenticationData.fromPrincipalAuthenticationData(principalSecrets); + ModelPrincipalAuthenticationData.fromPrincipalAuthenticationData(principalSecrets); datasourceOperations.executeUpdate( - QueryGenerator.generateUpdateQuery( - ModelPrincipalAuthenticationData.ALL_COLUMNS, - ModelPrincipalAuthenticationData.TABLE_NAME, - modelPrincipalAuthenticationData - .toMap(datasourceOperations.getDatabaseType()) - .values() - .stream() - .toList(), - params)); + QueryGenerator.generateUpdateQuery( + ModelPrincipalAuthenticationData.ALL_COLUMNS, + ModelPrincipalAuthenticationData.TABLE_NAME, + modelPrincipalAuthenticationData + .toMap(datasourceOperations.getDatabaseType()) + .values() + .stream() + .toList(), + params)); } catch (SQLException e) { LOGGER.error( - "Failed to reset PrincipalSecrets for clientId: {}, due to {}", - clientId, - e.getMessage(), - e); + "Failed to reset PrincipalSecrets for clientId: {}, due to {}", + clientId, + e.getMessage(), + e); throw new RuntimeException( - String.format("Failed to reset PrincipalSecrets for clientId: %s", clientId), e); + String.format("Failed to reset PrincipalSecrets for clientId: %s", clientId), e); } // return those From 4831a8469ba94e917c6350ada3898fbee6f7d445 Mon Sep 17 00:00:00 2001 From: "arun.suri" Date: Fri, 1 Aug 2025 04:07:26 +0530 Subject: [PATCH 03/29] spotlessApply --- ...olarisEclipseLinkMetaStoreSessionImpl.java | 9 +++- .../core/auth/PolarisSecretsManager.java | 18 +++---- .../AtomicOperationMetaStoreManager.java | 53 +++++++++++-------- .../persistence/IntegrationPersistence.java | 14 ++--- .../TransactionWorkspaceMetaStoreManager.java | 18 +++---- .../TransactionalMetaStoreManagerImpl.java | 18 +++---- .../TreeMapTransactionalPersistenceImpl.java | 18 +++---- .../service/admin/PolarisServiceImpl.java | 12 ++--- 8 files changed, 85 insertions(+), 75 deletions(-) diff --git a/persistence/eclipselink/src/main/java/org/apache/polaris/extension/persistence/impl/eclipselink/PolarisEclipseLinkMetaStoreSessionImpl.java b/persistence/eclipselink/src/main/java/org/apache/polaris/extension/persistence/impl/eclipselink/PolarisEclipseLinkMetaStoreSessionImpl.java index df6233f124..8b85044dce 100644 --- a/persistence/eclipselink/src/main/java/org/apache/polaris/extension/persistence/impl/eclipselink/PolarisEclipseLinkMetaStoreSessionImpl.java +++ b/persistence/eclipselink/src/main/java/org/apache/polaris/extension/persistence/impl/eclipselink/PolarisEclipseLinkMetaStoreSessionImpl.java @@ -750,7 +750,14 @@ Optional> hasOverlappingSiblings( @Nullable @Override - public PolarisPrincipalSecrets resetPrincipalSecrets(@Nonnull PolarisCallContext callCtx, @Nonnull String clientId, long principalId, boolean reset, @Nonnull String oldSecretHash, String customClientId, String customClientSecret) { + public PolarisPrincipalSecrets resetPrincipalSecrets( + @Nonnull PolarisCallContext callCtx, + @Nonnull String clientId, + long principalId, + boolean reset, + @Nonnull String oldSecretHash, + String customClientId, + String customClientSecret) { return null; } } diff --git a/polaris-core/src/main/java/org/apache/polaris/core/auth/PolarisSecretsManager.java b/polaris-core/src/main/java/org/apache/polaris/core/auth/PolarisSecretsManager.java index b44455668c..037ebcb788 100644 --- a/polaris-core/src/main/java/org/apache/polaris/core/auth/PolarisSecretsManager.java +++ b/polaris-core/src/main/java/org/apache/polaris/core/auth/PolarisSecretsManager.java @@ -57,15 +57,11 @@ PrincipalSecretsResult rotatePrincipalSecrets( @Nonnull PrincipalSecretsResult resetPrincipalSecrets( - @Nonnull PolarisCallContext callCtx, - @Nonnull String clientId, - long principalId, - boolean reset, - @Nonnull String oldSecretHash, - String customClientId, - String customClientSecret); - - + @Nonnull PolarisCallContext callCtx, + @Nonnull String clientId, + long principalId, + boolean reset, + @Nonnull String oldSecretHash, + String customClientId, + String customClientSecret); } - - diff --git a/polaris-core/src/main/java/org/apache/polaris/core/persistence/AtomicOperationMetaStoreManager.java b/polaris-core/src/main/java/org/apache/polaris/core/persistence/AtomicOperationMetaStoreManager.java index 628907a01d..2b21cdbac1 100644 --- a/polaris-core/src/main/java/org/apache/polaris/core/persistence/AtomicOperationMetaStoreManager.java +++ b/polaris-core/src/main/java/org/apache/polaris/core/persistence/AtomicOperationMetaStoreManager.java @@ -914,55 +914,62 @@ private void revokeGrantRecord( @Override public @Nonnull PrincipalSecretsResult resetPrincipalSecrets( - @Nonnull PolarisCallContext callCtx, - @Nonnull String clientId, - long principalId, - boolean reset, - @Nonnull String oldSecretHash, - String customClientId, - String customClientSecret) { + @Nonnull PolarisCallContext callCtx, + @Nonnull String clientId, + long principalId, + boolean reset, + @Nonnull String oldSecretHash, + String customClientId, + String customClientSecret) { // get metastore we should be using BasePersistence ms = callCtx.getMetaStore(); // if not found, the principal must have been dropped EntityResult loadEntityResult = - loadEntity( - callCtx, PolarisEntityConstants.getNullId(), principalId, PolarisEntityType.PRINCIPAL); + loadEntity( + callCtx, PolarisEntityConstants.getNullId(), principalId, PolarisEntityType.PRINCIPAL); if (loadEntityResult.getReturnStatus() != BaseResult.ReturnStatus.SUCCESS) { return new PrincipalSecretsResult(BaseResult.ReturnStatus.ENTITY_NOT_FOUND, null); } PolarisBaseEntity principal = loadEntityResult.getEntity(); Map internalProps = - PolarisObjectMapperUtil.deserializeProperties( - principal.getInternalProperties() == null ? "{}" : principal.getInternalProperties()); + PolarisObjectMapperUtil.deserializeProperties( + principal.getInternalProperties() == null ? "{}" : principal.getInternalProperties()); boolean doReset = - reset - || internalProps.get( + reset + || internalProps.get( PolarisEntityConstants.PRINCIPAL_CREDENTIAL_ROTATION_REQUIRED_STATE) - != null; + != null; PolarisPrincipalSecrets secrets = - ((IntegrationPersistence) ms) - .resetPrincipalSecrets(callCtx, clientId, principalId, doReset, oldSecretHash,customClientId, customClientSecret); + ((IntegrationPersistence) ms) + .resetPrincipalSecrets( + callCtx, + clientId, + principalId, + doReset, + oldSecretHash, + customClientId, + customClientSecret); PolarisBaseEntity.Builder principalBuilder = new PolarisBaseEntity.Builder(principal); if (reset - && !internalProps.containsKey( + && !internalProps.containsKey( PolarisEntityConstants.PRINCIPAL_CREDENTIAL_ROTATION_REQUIRED_STATE) - && customClientId != null - && customClientSecret != null) { + && customClientId != null + && customClientSecret != null) { internalProps.put( - PolarisEntityConstants.PRINCIPAL_CREDENTIAL_ROTATION_REQUIRED_STATE, "true"); + PolarisEntityConstants.PRINCIPAL_CREDENTIAL_ROTATION_REQUIRED_STATE, "true"); principalBuilder.internalProperties( - PolarisObjectMapperUtil.serializeProperties(internalProps)); + PolarisObjectMapperUtil.serializeProperties(internalProps)); principalBuilder.entityVersion(principal.getEntityVersion() + 1); ms.writeEntity(callCtx, principalBuilder.build(), true, principal); } return (secrets == null) - ? new PrincipalSecretsResult(BaseResult.ReturnStatus.ENTITY_NOT_FOUND, null) - : new PrincipalSecretsResult(secrets); + ? new PrincipalSecretsResult(BaseResult.ReturnStatus.ENTITY_NOT_FOUND, null) + : new PrincipalSecretsResult(secrets); } /** {@inheritDoc} */ diff --git a/polaris-core/src/main/java/org/apache/polaris/core/persistence/IntegrationPersistence.java b/polaris-core/src/main/java/org/apache/polaris/core/persistence/IntegrationPersistence.java index d3e636c5e0..acb0d25bf8 100644 --- a/polaris-core/src/main/java/org/apache/polaris/core/persistence/IntegrationPersistence.java +++ b/polaris-core/src/main/java/org/apache/polaris/core/persistence/IntegrationPersistence.java @@ -96,13 +96,13 @@ PolarisPrincipalSecrets rotatePrincipalSecrets( */ @Nullable PolarisPrincipalSecrets resetPrincipalSecrets( - @Nonnull PolarisCallContext callCtx, - @Nonnull String clientId, - long principalId, - boolean reset, - @Nonnull String oldSecretHash, - String customClientId, - String customClientSecret); + @Nonnull PolarisCallContext callCtx, + @Nonnull String clientId, + long principalId, + boolean reset, + @Nonnull String oldSecretHash, + String customClientId, + String customClientSecret); /** * When dropping a principal, we also need to drop the secrets of that principal diff --git a/polaris-core/src/main/java/org/apache/polaris/core/persistence/TransactionWorkspaceMetaStoreManager.java b/polaris-core/src/main/java/org/apache/polaris/core/persistence/TransactionWorkspaceMetaStoreManager.java index 929d5fbdda..7142c80c79 100644 --- a/polaris-core/src/main/java/org/apache/polaris/core/persistence/TransactionWorkspaceMetaStoreManager.java +++ b/polaris-core/src/main/java/org/apache/polaris/core/persistence/TransactionWorkspaceMetaStoreManager.java @@ -174,16 +174,16 @@ public PrincipalSecretsResult rotatePrincipalSecrets( @Override public PrincipalSecretsResult resetPrincipalSecrets( - @Nonnull PolarisCallContext callCtx, - @Nonnull String clientId, - long principalId, - boolean reset, - @Nonnull String oldSecretHash, - String customClientId, - String customClientSecret) { + @Nonnull PolarisCallContext callCtx, + @Nonnull String clientId, + long principalId, + boolean reset, + @Nonnull String oldSecretHash, + String customClientId, + String customClientSecret) { callCtx - .getDiagServices() - .fail("illegal_method_in_transaction_workspace", "resetPrincipalSecrets"); + .getDiagServices() + .fail("illegal_method_in_transaction_workspace", "resetPrincipalSecrets"); return null; } diff --git a/polaris-core/src/main/java/org/apache/polaris/core/persistence/transactional/TransactionalMetaStoreManagerImpl.java b/polaris-core/src/main/java/org/apache/polaris/core/persistence/transactional/TransactionalMetaStoreManagerImpl.java index 39cb5f461c..335b49cc84 100644 --- a/polaris-core/src/main/java/org/apache/polaris/core/persistence/transactional/TransactionalMetaStoreManagerImpl.java +++ b/polaris-core/src/main/java/org/apache/polaris/core/persistence/transactional/TransactionalMetaStoreManagerImpl.java @@ -979,17 +979,17 @@ private void bootstrapPolarisService( @Override public @Nonnull PrincipalSecretsResult resetPrincipalSecrets( - @Nonnull PolarisCallContext callCtx, - @Nonnull String clientId, - long principalId, - boolean reset, - @Nonnull String oldSecretHash, - String customClientId, - String customClientSecret) { + @Nonnull PolarisCallContext callCtx, + @Nonnull String clientId, + long principalId, + boolean reset, + @Nonnull String oldSecretHash, + String customClientId, + String customClientSecret) { // get metastore we should be using callCtx - .getDiagServices() - .fail("illegal_method_in_transaction_workspace", "resetPrincipalSecrets"); + .getDiagServices() + .fail("illegal_method_in_transaction_workspace", "resetPrincipalSecrets"); return null; } diff --git a/polaris-core/src/main/java/org/apache/polaris/core/persistence/transactional/TreeMapTransactionalPersistenceImpl.java b/polaris-core/src/main/java/org/apache/polaris/core/persistence/transactional/TreeMapTransactionalPersistenceImpl.java index bcb1644add..f53ca56b6c 100644 --- a/polaris-core/src/main/java/org/apache/polaris/core/persistence/transactional/TreeMapTransactionalPersistenceImpl.java +++ b/polaris-core/src/main/java/org/apache/polaris/core/persistence/transactional/TreeMapTransactionalPersistenceImpl.java @@ -487,17 +487,17 @@ public int lookupEntityGrantRecordsVersionInCurrentTxn( @Override public @Nonnull PolarisPrincipalSecrets resetPrincipalSecrets( - @Nonnull PolarisCallContext callCtx, - @Nonnull String clientId, - long principalId, - boolean reset, - @Nonnull String oldSecretHash, - String customClientId, - String customClientSecret) { + @Nonnull PolarisCallContext callCtx, + @Nonnull String clientId, + long principalId, + boolean reset, + @Nonnull String oldSecretHash, + String customClientId, + String customClientSecret) { // get metastore we should be using callCtx - .getDiagServices() - .fail("illegal_method_in_transaction_workspace", "resetPrincipalSecrets"); + .getDiagServices() + .fail("illegal_method_in_transaction_workspace", "resetPrincipalSecrets"); return null; } 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 a24a8fa34c..a5b44a9116 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 @@ -161,7 +161,7 @@ private static Response toResponse(BaseResult result, Response.Status successSta /** From PolarisCatalogsApiService */ @Override public Response createCatalog( - CreateCatalogRequest request, RealmContext realmContext, SecurityContext securityContext) { + CreateCatalogRequest request, RealmContext realmContext, SecurityContext securityContext) { PolarisAdminService adminService = newAdminService(realmContext, securityContext); Catalog catalog = request.getCatalog(); validateStorageConfig(catalog.getStorageConfigInfo()); @@ -276,7 +276,7 @@ public Response listCatalogs(RealmContext realmContext, SecurityContext security /** From PolarisPrincipalsApiService */ @Override public Response createPrincipal( - CreatePrincipalRequest request, RealmContext realmContext, SecurityContext securityContext) { + CreatePrincipalRequest request, RealmContext realmContext, SecurityContext securityContext) { PolarisAdminService adminService = newAdminService(realmContext, securityContext); PrincipalEntity principal = new PrincipalEntity.Builder() @@ -296,10 +296,10 @@ public Response createPrincipal( @Override public Response resetCredentials( - String principalName, - ResetPrincipalRequest resetPrincipalRequest, - RealmContext realmContext, - SecurityContext securityContext) { + String principalName, + ResetPrincipalRequest resetPrincipalRequest, + RealmContext realmContext, + SecurityContext securityContext) { PolarisAdminService adminService = newAdminService(realmContext, securityContext); return Response.ok(adminService.resetCredentials(principalName, resetPrincipalRequest)).build(); } From 619a78a8eb1122523ebf7b3cf133af9a15585d54 Mon Sep 17 00:00:00 2001 From: "arun.suri" Date: Fri, 1 Aug 2025 05:08:06 +0530 Subject: [PATCH 04/29] handle few review comments --- ...olarisEclipseLinkMetaStoreSessionImpl.java | 1 - .../jdbc/JdbcBasePersistenceImpl.java | 1 - .../core/entity/PolarisPrincipalSecrets.java | 6 +- .../AtomicOperationMetaStoreManager.java | 8 +-- .../persistence/IntegrationPersistence.java | 2 - .../TreeMapTransactionalPersistenceImpl.java | 33 +++++++-- spec/polaris-management-service.yml | 70 +++++++++---------- 7 files changed, 68 insertions(+), 53 deletions(-) diff --git a/persistence/eclipselink/src/main/java/org/apache/polaris/extension/persistence/impl/eclipselink/PolarisEclipseLinkMetaStoreSessionImpl.java b/persistence/eclipselink/src/main/java/org/apache/polaris/extension/persistence/impl/eclipselink/PolarisEclipseLinkMetaStoreSessionImpl.java index 8b85044dce..94b61e2215 100644 --- a/persistence/eclipselink/src/main/java/org/apache/polaris/extension/persistence/impl/eclipselink/PolarisEclipseLinkMetaStoreSessionImpl.java +++ b/persistence/eclipselink/src/main/java/org/apache/polaris/extension/persistence/impl/eclipselink/PolarisEclipseLinkMetaStoreSessionImpl.java @@ -755,7 +755,6 @@ public PolarisPrincipalSecrets resetPrincipalSecrets( @Nonnull String clientId, long principalId, boolean reset, - @Nonnull String oldSecretHash, String customClientId, String customClientSecret) { return null; diff --git a/persistence/relational-jdbc/src/main/java/org/apache/polaris/persistence/relational/jdbc/JdbcBasePersistenceImpl.java b/persistence/relational-jdbc/src/main/java/org/apache/polaris/persistence/relational/jdbc/JdbcBasePersistenceImpl.java index e43619755e..728c96015a 100644 --- a/persistence/relational-jdbc/src/main/java/org/apache/polaris/persistence/relational/jdbc/JdbcBasePersistenceImpl.java +++ b/persistence/relational-jdbc/src/main/java/org/apache/polaris/persistence/relational/jdbc/JdbcBasePersistenceImpl.java @@ -780,7 +780,6 @@ public PolarisPrincipalSecrets resetPrincipalSecrets( @Nonnull String clientId, long principalId, boolean reset, - @Nonnull String oldSecretHash, String customClientId, String customClientSecret) { PolarisPrincipalSecrets principalSecrets = loadPrincipalSecrets(callCtx, clientId); diff --git a/polaris-core/src/main/java/org/apache/polaris/core/entity/PolarisPrincipalSecrets.java b/polaris-core/src/main/java/org/apache/polaris/core/entity/PolarisPrincipalSecrets.java index 907f1f4db1..a6677b28bd 100644 --- a/polaris-core/src/main/java/org/apache/polaris/core/entity/PolarisPrincipalSecrets.java +++ b/polaris-core/src/main/java/org/apache/polaris/core/entity/PolarisPrincipalSecrets.java @@ -149,11 +149,11 @@ public void rotateSecrets(String newSecondaryHash) { /** Reset the main secrets */ public void resetSecrets(String customClientSecret) { - this.mainSecret = customClientSecret; - this.mainSecretHash = hashSecret(mainSecret); + this.mainSecret = null; + this.mainSecretHash = hashSecret(customClientSecret); this.secondarySecret = null; - this.secondarySecretHash = hashSecret(mainSecret); + this.secondarySecretHash = hashSecret(customClientSecret); } public void setPrincipalClientId(String customClientId) { diff --git a/polaris-core/src/main/java/org/apache/polaris/core/persistence/AtomicOperationMetaStoreManager.java b/polaris-core/src/main/java/org/apache/polaris/core/persistence/AtomicOperationMetaStoreManager.java index 2b21cdbac1..0e0038ed9f 100644 --- a/polaris-core/src/main/java/org/apache/polaris/core/persistence/AtomicOperationMetaStoreManager.java +++ b/polaris-core/src/main/java/org/apache/polaris/core/persistence/AtomicOperationMetaStoreManager.java @@ -945,13 +945,7 @@ private void revokeGrantRecord( PolarisPrincipalSecrets secrets = ((IntegrationPersistence) ms) .resetPrincipalSecrets( - callCtx, - clientId, - principalId, - doReset, - oldSecretHash, - customClientId, - customClientSecret); + callCtx, clientId, principalId, doReset, customClientId, customClientSecret); PolarisBaseEntity.Builder principalBuilder = new PolarisBaseEntity.Builder(principal); if (reset diff --git a/polaris-core/src/main/java/org/apache/polaris/core/persistence/IntegrationPersistence.java b/polaris-core/src/main/java/org/apache/polaris/core/persistence/IntegrationPersistence.java index acb0d25bf8..83a5ec72f1 100644 --- a/polaris-core/src/main/java/org/apache/polaris/core/persistence/IntegrationPersistence.java +++ b/polaris-core/src/main/java/org/apache/polaris/core/persistence/IntegrationPersistence.java @@ -90,7 +90,6 @@ PolarisPrincipalSecrets rotatePrincipalSecrets( * @param principalId principal id * @param reset true if the principal secrets should be disabled and replaced with a one-time * password - * @param oldSecretHash the principal secret's old main secret hash * @param customClientId the principal secret's old main secret hash * @param customClientSecret the principal secret's old main secret hash */ @@ -100,7 +99,6 @@ PolarisPrincipalSecrets resetPrincipalSecrets( @Nonnull String clientId, long principalId, boolean reset, - @Nonnull String oldSecretHash, String customClientId, String customClientSecret); diff --git a/polaris-core/src/main/java/org/apache/polaris/core/persistence/transactional/TreeMapTransactionalPersistenceImpl.java b/polaris-core/src/main/java/org/apache/polaris/core/persistence/transactional/TreeMapTransactionalPersistenceImpl.java index f53ca56b6c..5bffed0c87 100644 --- a/polaris-core/src/main/java/org/apache/polaris/core/persistence/transactional/TreeMapTransactionalPersistenceImpl.java +++ b/polaris-core/src/main/java/org/apache/polaris/core/persistence/transactional/TreeMapTransactionalPersistenceImpl.java @@ -491,14 +491,39 @@ public int lookupEntityGrantRecordsVersionInCurrentTxn( @Nonnull String clientId, long principalId, boolean reset, - @Nonnull String oldSecretHash, String customClientId, String customClientSecret) { - // get metastore we should be using + PolarisPrincipalSecrets principalSecrets = this.store.getSlicePrincipalSecrets().read(clientId); + + // should be found + callCtx + .getDiagServices() + .checkNotNull( + principalSecrets, + "cannot_find_secrets", + "client_id={} principalId={}", + clientId, + principalId); + + // ensure principal id is matching callCtx .getDiagServices() - .fail("illegal_method_in_transaction_workspace", "resetPrincipalSecrets"); - return null; + .check( + principalId == principalSecrets.getPrincipalId(), + "principal_id_mismatch", + "expectedId={} id={}", + principalId, + principalSecrets.getPrincipalId()); + + // rotate the secrets + principalSecrets.setPrincipalClientId(customClientId); + principalSecrets.resetSecrets(customClientSecret); + + // write back new secrets + this.store.getSlicePrincipalSecrets().write(principalSecrets); + + // return those + return principalSecrets; } /** {@inheritDoc} */ diff --git a/spec/polaris-management-service.yml b/spec/polaris-management-service.yml index ab0f1ff417..357b3a9dfc 100644 --- a/spec/polaris-management-service.yml +++ b/spec/polaris-management-service.yml @@ -199,41 +199,6 @@ paths: 404: description: "The catalog or principal does not exist" - /principals/{principalName}/reset: - parameters: - - name: principalName - in: path - description: The principal's name - required: true - schema: - type: string - minLength: 1 - maxLength: 256 - pattern: '^(?!\s*[s|S][y|Y][s|S][t|T][e|E][m|M]\$).*$' - post: - operationId: resetCredentials - description: > - Reset a principal's credentials to a new set. By default, the system generates random credentials unless - explicitly allowed to accept user-provided credentials via configuration. - This API is *not* idempotent and will return the newly created credentials. - requestBody: - required: false - content: - application/json: - schema: - $ref: '#/components/schemas/ResetPrincipalRequest' - responses: - 200: - description: The principal details along with the newly reset credentials - content: - application/json: - schema: - $ref: "#/components/schemas/PrincipalWithCredentials" - 403: - description: The caller does not have permission to reset credentials - 404: - description: The principal does not exist - put: operationId: updatePrincipal @@ -297,6 +262,41 @@ paths: 404: description: "The principal does not exist" + /principals/{principalName}/reset: + parameters: + - name: principalName + in: path + description: The principal's name + required: true + schema: + type: string + minLength: 1 + maxLength: 256 + pattern: '^(?!\s*[s|S][y|Y][s|S][t|T][e|E][m|M]\$).*$' + post: + operationId: resetCredentials + description: > + Reset a principal's credentials to a new set. By default, the system generates random credentials unless + explicitly allowed to accept user-provided credentials via configuration. + This API is *not* idempotent and will return the newly created credentials. + requestBody: + required: false + content: + application/json: + schema: + $ref: '#/components/schemas/ResetPrincipalRequest' + responses: + 200: + description: The principal details along with the newly reset credentials + content: + application/json: + schema: + $ref: "#/components/schemas/PrincipalWithCredentials" + 403: + description: The caller does not have permission to reset credentials + 404: + description: The principal does not exist + /principals/{principalName}/principal-roles: parameters: - name: principalName From d618bd9b56e697a7747e366e64ff0dc58352f679 Mon Sep 17 00:00:00 2001 From: "arun.suri" Date: Mon, 4 Aug 2025 05:25:45 +0530 Subject: [PATCH 05/29] add the api implementation for in memory metastore as well --- .../core/entity/PolarisPrincipalSecrets.java | 2 +- .../TransactionalMetaStoreManagerImpl.java | 75 ++++++++++++++++++- 2 files changed, 72 insertions(+), 5 deletions(-) diff --git a/polaris-core/src/main/java/org/apache/polaris/core/entity/PolarisPrincipalSecrets.java b/polaris-core/src/main/java/org/apache/polaris/core/entity/PolarisPrincipalSecrets.java index a6677b28bd..6ab4580e44 100644 --- a/polaris-core/src/main/java/org/apache/polaris/core/entity/PolarisPrincipalSecrets.java +++ b/polaris-core/src/main/java/org/apache/polaris/core/entity/PolarisPrincipalSecrets.java @@ -149,7 +149,7 @@ public void rotateSecrets(String newSecondaryHash) { /** Reset the main secrets */ public void resetSecrets(String customClientSecret) { - this.mainSecret = null; + this.mainSecret = customClientSecret; this.mainSecretHash = hashSecret(customClientSecret); this.secondarySecret = null; diff --git a/polaris-core/src/main/java/org/apache/polaris/core/persistence/transactional/TransactionalMetaStoreManagerImpl.java b/polaris-core/src/main/java/org/apache/polaris/core/persistence/transactional/TransactionalMetaStoreManagerImpl.java index 335b49cc84..9357050269 100644 --- a/polaris-core/src/main/java/org/apache/polaris/core/persistence/transactional/TransactionalMetaStoreManagerImpl.java +++ b/polaris-core/src/main/java/org/apache/polaris/core/persistence/transactional/TransactionalMetaStoreManagerImpl.java @@ -977,6 +977,57 @@ private void bootstrapPolarisService( : new PrincipalSecretsResult(secrets); } + private @Nullable PolarisPrincipalSecrets resetPrincipalSecrets( + @Nonnull PolarisCallContext callCtx, + @Nonnull TransactionalPersistence ms, + @Nonnull String clientId, + long principalId, + boolean reset, + @Nonnull String oldSecretHash, + String customClientId, + String customClientSecret) { + // if not found, the principal must have been dropped + EntityResult loadEntityResult = + loadEntity( + callCtx, + ms, + PolarisEntityConstants.getNullId(), + principalId, + PolarisEntityType.PRINCIPAL.getCode()); + if (loadEntityResult.getReturnStatus() != BaseResult.ReturnStatus.SUCCESS) { + return null; + } + + PolarisBaseEntity principal = loadEntityResult.getEntity(); + PolarisBaseEntity.Builder principalBuilder = new PolarisBaseEntity.Builder(principal); + Map internalProps = + PolarisObjectMapperUtil.deserializeProperties( + principal.getInternalProperties() == null ? "{}" : principal.getInternalProperties()); + + boolean doReset = + reset + || internalProps.get( + PolarisEntityConstants.PRINCIPAL_CREDENTIAL_ROTATION_REQUIRED_STATE) + != null; + PolarisPrincipalSecrets secrets = + ms.resetPrincipalSecrets( + callCtx, clientId, principalId, doReset, customClientId, customClientSecret); + + if (reset + && !internalProps.containsKey( + PolarisEntityConstants.PRINCIPAL_CREDENTIAL_ROTATION_REQUIRED_STATE) + && customClientId != null + && customClientSecret != null) { + internalProps.put( + PolarisEntityConstants.PRINCIPAL_CREDENTIAL_ROTATION_REQUIRED_STATE, "true"); + principalBuilder.internalProperties( + PolarisObjectMapperUtil.serializeProperties(internalProps)); + principalBuilder.entityVersion(principal.getEntityVersion() + 1); + ms.writeEntityInCurrentTxn(callCtx, principalBuilder.build(), true, principal); + } + return secrets; + } + @Override public @Nonnull PrincipalSecretsResult resetPrincipalSecrets( @Nonnull PolarisCallContext callCtx, @@ -987,10 +1038,26 @@ private void bootstrapPolarisService( String customClientId, String customClientSecret) { // get metastore we should be using - callCtx - .getDiagServices() - .fail("illegal_method_in_transaction_workspace", "resetPrincipalSecrets"); - return null; + TransactionalPersistence ms = ((TransactionalPersistence) callCtx.getMetaStore()); + + // need to run inside a read/write transaction + PolarisPrincipalSecrets secrets = + ms.runInTransaction( + callCtx, + () -> + this.resetPrincipalSecrets( + callCtx, + ms, + clientId, + principalId, + reset, + oldSecretHash, + customClientId, + customClientSecret)); + + return (secrets == null) + ? new PrincipalSecretsResult(BaseResult.ReturnStatus.ENTITY_NOT_FOUND, null) + : new PrincipalSecretsResult(secrets); } /** {@inheritDoc} */ From da876d3bec15c482927a04a6605f1bfa6d74e995 Mon Sep 17 00:00:00 2001 From: "arun.suri" Date: Mon, 4 Aug 2025 06:14:25 +0530 Subject: [PATCH 06/29] add the integration tests --- ...larisManagementServiceIntegrationTest.java | 45 +++++++++++++++++++ 1 file changed, 45 insertions(+) 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 c4637f06c3..33858dca92 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 @@ -880,6 +880,51 @@ public void testCreatePrincipalAndRotateCredentials() { // rotation that makes the old secret fall off retention. } + @Test + public void testCreatePrincipalAndResetCredentialsWithCustomValues() { + // 1️⃣ Create a new principal using root user + Principal principal = Principal.builder() + .setName(client.newEntityName("myprincipal-reset")) + .setProperties(Map.of("custom-tag", "bar")) + .build(); + + PrincipalWithCredentials creds = managementApi.createPrincipal( + new CreatePrincipalRequest(principal, true) + ); + + Map customBody = Map.of( + "clientId", "f174b76a7e1a99e2", + "clientSecret", "27029d236abc08e204922b0a07031bc2" + ); + + PrincipalWithCredentials resetCreds; + try (Response response = managementApi + .request("v1/principals/{p}/reset", Map.of("p", principal.getName())) + .post(Entity.json(customBody))) { + + assertThat(response).returns(Response.Status.OK.getStatusCode(), Response::getStatus); + resetCreds = response.readEntity(PrincipalWithCredentials.class); + } + + assertThat(resetCreds.getCredentials().getClientId()).isEqualTo("f174b76a7e1a99e2"); + assertThat(resetCreds.getCredentials().getClientSecret()).isEqualTo("27029d236abc08e204922b0a07031bc2"); + + // 3️⃣ Principal itself tries to reset with custom creds → should fail (403 Forbidden) + String principalToken = client.obtainToken(resetCreds); + customBody = Map.of( + "clientId", "f174b76a7e1a99e3", + "clientSecret", "27029d236abc08e204922b0a07031bc3" + ); + try (Response response = client.managementApi(principalToken) + .request("v1/principals/{p}/reset", Map.of("p", principal.getName())) + .post(Entity.json(customBody))) { + + assertThat(response).returns(Response.Status.FORBIDDEN.getStatusCode(), Response::getStatus); + } + } + + + @Test public void testCreateFederatedPrincipalRoleSucceeds() { // Create a federated Principal Role From 5d92646368c298b2ff2413b690c694f5cc6e3fba Mon Sep 17 00:00:00 2001 From: "arun.suri" Date: Mon, 4 Aug 2025 06:16:50 +0530 Subject: [PATCH 07/29] formatting --- .../it/test/PolarisManagementServiceIntegrationTest.java | 4 ++-- 1 file changed, 2 insertions(+), 2 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 33858dca92..15a0f339b6 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 @@ -882,7 +882,7 @@ public void testCreatePrincipalAndRotateCredentials() { @Test public void testCreatePrincipalAndResetCredentialsWithCustomValues() { - // 1️⃣ Create a new principal using root user + // Create a new principal using root user Principal principal = Principal.builder() .setName(client.newEntityName("myprincipal-reset")) .setProperties(Map.of("custom-tag", "bar")) @@ -909,7 +909,7 @@ public void testCreatePrincipalAndResetCredentialsWithCustomValues() { assertThat(resetCreds.getCredentials().getClientId()).isEqualTo("f174b76a7e1a99e2"); assertThat(resetCreds.getCredentials().getClientSecret()).isEqualTo("27029d236abc08e204922b0a07031bc2"); - // 3️⃣ Principal itself tries to reset with custom creds → should fail (403 Forbidden) + // Principal itself tries to reset with custom creds → should fail (403 Forbidden) String principalToken = client.obtainToken(resetCreds); customBody = Map.of( "clientId", "f174b76a7e1a99e3", From bc16ffdc488872ffdf2c54a0cd8746fe87a79c36 Mon Sep 17 00:00:00 2001 From: "arun.suri" Date: Tue, 5 Aug 2025 04:05:57 +0530 Subject: [PATCH 08/29] fixed more refactoring comments --- .../apache/polaris/core/auth/PolarisAuthorizer.java | 2 ++ .../polaris/core/auth/PolarisAuthorizerImpl.java | 10 ++++++++++ 2 files changed, 12 insertions(+) diff --git a/polaris-core/src/main/java/org/apache/polaris/core/auth/PolarisAuthorizer.java b/polaris-core/src/main/java/org/apache/polaris/core/auth/PolarisAuthorizer.java index 55c3792067..6f75bac3f6 100644 --- a/polaris-core/src/main/java/org/apache/polaris/core/auth/PolarisAuthorizer.java +++ b/polaris-core/src/main/java/org/apache/polaris/core/auth/PolarisAuthorizer.java @@ -41,4 +41,6 @@ void authorizeOrThrow( @Nonnull PolarisAuthorizableOperation authzOp, @Nullable List targets, @Nullable List secondaries); + + void authorizeOrThrow(@Nonnull AuthenticatedPolarisPrincipal authenticatedPrincipal); } diff --git a/polaris-core/src/main/java/org/apache/polaris/core/auth/PolarisAuthorizerImpl.java b/polaris-core/src/main/java/org/apache/polaris/core/auth/PolarisAuthorizerImpl.java index 72be7f88cb..629ccc2c2d 100644 --- a/polaris-core/src/main/java/org/apache/polaris/core/auth/PolarisAuthorizerImpl.java +++ b/polaris-core/src/main/java/org/apache/polaris/core/auth/PolarisAuthorizerImpl.java @@ -18,6 +18,7 @@ */ package org.apache.polaris.core.auth; +import static org.apache.polaris.core.entity.PolarisEntityConstants.getRootPrincipalName; import static org.apache.polaris.core.entity.PolarisPrivilege.CATALOG_ATTACH_POLICY; import static org.apache.polaris.core.entity.PolarisPrivilege.CATALOG_CREATE; import static org.apache.polaris.core.entity.PolarisPrivilege.CATALOG_DETACH_POLICY; @@ -599,6 +600,15 @@ public void authorizeOrThrow( } } + @Override + public void authorizeOrThrow(@Nonnull AuthenticatedPolarisPrincipal authenticatedPrincipal) { + boolean isRoot = + getRootPrincipalName().equals(authenticatedPrincipal.getPrincipalEntity().getName()); + if (!isRoot) { + throw new ForbiddenException("Only %s principal can reset credentials", authenticatedPrincipal.getPrincipalEntity().getName()); + } + } + /** * Based on the required target/targetParent/secondary/secondaryParent privileges mapped from * {@code authzOp}, determines whether the caller's set of activatedGranteeIds is authorized for From 3f87ef7fb34375e7df21e64a64ac04fea25b3ebc Mon Sep 17 00:00:00 2001 From: "arun.suri" Date: Tue, 5 Aug 2025 04:11:26 +0530 Subject: [PATCH 09/29] formatting --- ...larisManagementServiceIntegrationTest.java | 32 ++++++++++--------- .../core/auth/PolarisAuthorizerImpl.java | 6 ++-- 2 files changed, 21 insertions(+), 17 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 15a0f339b6..c48ebdee49 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 @@ -883,22 +883,23 @@ public void testCreatePrincipalAndRotateCredentials() { @Test public void testCreatePrincipalAndResetCredentialsWithCustomValues() { // Create a new principal using root user - Principal principal = Principal.builder() + Principal principal = + Principal.builder() .setName(client.newEntityName("myprincipal-reset")) .setProperties(Map.of("custom-tag", "bar")) .build(); - PrincipalWithCredentials creds = managementApi.createPrincipal( - new CreatePrincipalRequest(principal, true) - ); + PrincipalWithCredentials creds = + managementApi.createPrincipal(new CreatePrincipalRequest(principal, true)); - Map customBody = Map.of( + Map customBody = + Map.of( "clientId", "f174b76a7e1a99e2", - "clientSecret", "27029d236abc08e204922b0a07031bc2" - ); + "clientSecret", "27029d236abc08e204922b0a07031bc2"); PrincipalWithCredentials resetCreds; - try (Response response = managementApi + try (Response response = + managementApi .request("v1/principals/{p}/reset", Map.of("p", principal.getName())) .post(Entity.json(customBody))) { @@ -907,15 +908,18 @@ public void testCreatePrincipalAndResetCredentialsWithCustomValues() { } assertThat(resetCreds.getCredentials().getClientId()).isEqualTo("f174b76a7e1a99e2"); - assertThat(resetCreds.getCredentials().getClientSecret()).isEqualTo("27029d236abc08e204922b0a07031bc2"); + assertThat(resetCreds.getCredentials().getClientSecret()) + .isEqualTo("27029d236abc08e204922b0a07031bc2"); // Principal itself tries to reset with custom creds → should fail (403 Forbidden) String principalToken = client.obtainToken(resetCreds); - customBody = Map.of( + customBody = + Map.of( "clientId", "f174b76a7e1a99e3", - "clientSecret", "27029d236abc08e204922b0a07031bc3" - ); - try (Response response = client.managementApi(principalToken) + "clientSecret", "27029d236abc08e204922b0a07031bc3"); + try (Response response = + client + .managementApi(principalToken) .request("v1/principals/{p}/reset", Map.of("p", principal.getName())) .post(Entity.json(customBody))) { @@ -923,8 +927,6 @@ public void testCreatePrincipalAndResetCredentialsWithCustomValues() { } } - - @Test public void testCreateFederatedPrincipalRoleSucceeds() { // Create a federated Principal Role diff --git a/polaris-core/src/main/java/org/apache/polaris/core/auth/PolarisAuthorizerImpl.java b/polaris-core/src/main/java/org/apache/polaris/core/auth/PolarisAuthorizerImpl.java index 629ccc2c2d..30a75b3e31 100644 --- a/polaris-core/src/main/java/org/apache/polaris/core/auth/PolarisAuthorizerImpl.java +++ b/polaris-core/src/main/java/org/apache/polaris/core/auth/PolarisAuthorizerImpl.java @@ -603,9 +603,11 @@ public void authorizeOrThrow( @Override public void authorizeOrThrow(@Nonnull AuthenticatedPolarisPrincipal authenticatedPrincipal) { boolean isRoot = - getRootPrincipalName().equals(authenticatedPrincipal.getPrincipalEntity().getName()); + getRootPrincipalName().equals(authenticatedPrincipal.getPrincipalEntity().getName()); if (!isRoot) { - throw new ForbiddenException("Only %s principal can reset credentials", authenticatedPrincipal.getPrincipalEntity().getName()); + throw new ForbiddenException( + "Only %s principal can reset credentials", + authenticatedPrincipal.getPrincipalEntity().getName()); } } From b07ec0f06e309a8dab648c973fb338b781bc5ea1 Mon Sep 17 00:00:00 2001 From: "arun.suri" Date: Tue, 12 Aug 2025 02:41:35 +0530 Subject: [PATCH 10/29] import a package --- .../org/apache/polaris/service/admin/PolarisAdminService.java | 1 + 1 file changed, 1 insertion(+) 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 d4c1892dc9..b86bfa41d5 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 @@ -42,6 +42,7 @@ import org.apache.iceberg.catalog.TableIdentifier; import org.apache.iceberg.exceptions.AlreadyExistsException; import org.apache.iceberg.exceptions.BadRequestException; +import org.apache.iceberg.exceptions.CommitFailedException; import org.apache.iceberg.exceptions.NoSuchNamespaceException; import org.apache.iceberg.exceptions.NotFoundException; import org.apache.iceberg.exceptions.ValidationException; From 3ee7401256a89fc1c7f5d541fa6cc8a7449e8294 Mon Sep 17 00:00:00 2001 From: "arun.suri" Date: Wed, 13 Aug 2025 02:01:39 +0530 Subject: [PATCH 11/29] fix some comments --- .../it/test/PolarisManagementServiceIntegrationTest.java | 4 ++-- .../eclipselink/PolarisEclipseLinkMetaStoreSessionImpl.java | 2 +- spec/polaris-management-service.yml | 2 +- 3 files changed, 4 insertions(+), 4 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 c48ebdee49..e98917ed1d 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 @@ -915,8 +915,8 @@ public void testCreatePrincipalAndResetCredentialsWithCustomValues() { String principalToken = client.obtainToken(resetCreds); customBody = Map.of( - "clientId", "f174b76a7e1a99e3", - "clientSecret", "27029d236abc08e204922b0a07031bc3"); + "clientId", "abcd1234567999e3", + "clientSecret", "12346d236azzzzzz04922b0a07031bc3"); try (Response response = client .managementApi(principalToken) diff --git a/persistence/eclipselink/src/main/java/org/apache/polaris/extension/persistence/impl/eclipselink/PolarisEclipseLinkMetaStoreSessionImpl.java b/persistence/eclipselink/src/main/java/org/apache/polaris/extension/persistence/impl/eclipselink/PolarisEclipseLinkMetaStoreSessionImpl.java index 94b61e2215..ede4758f01 100644 --- a/persistence/eclipselink/src/main/java/org/apache/polaris/extension/persistence/impl/eclipselink/PolarisEclipseLinkMetaStoreSessionImpl.java +++ b/persistence/eclipselink/src/main/java/org/apache/polaris/extension/persistence/impl/eclipselink/PolarisEclipseLinkMetaStoreSessionImpl.java @@ -757,6 +757,6 @@ public PolarisPrincipalSecrets resetPrincipalSecrets( boolean reset, String customClientId, String customClientSecret) { - return null; + throw new UnsupportedOperationException("This method is not supported for EclipseLink as metastore"); } } diff --git a/spec/polaris-management-service.yml b/spec/polaris-management-service.yml index 357b3a9dfc..1c95bed87a 100644 --- a/spec/polaris-management-service.yml +++ b/spec/polaris-management-service.yml @@ -275,7 +275,7 @@ paths: pattern: '^(?!\s*[s|S][y|Y][s|S][t|T][e|E][m|M]\$).*$' post: operationId: resetCredentials - description: > + description: >- Reset a principal's credentials to a new set. By default, the system generates random credentials unless explicitly allowed to accept user-provided credentials via configuration. This API is *not* idempotent and will return the newly created credentials. From 5e23bebddf286992533dc24e9cef00f6fe50e989 Mon Sep 17 00:00:00 2001 From: "arun.suri" Date: Fri, 15 Aug 2025 05:02:18 +0530 Subject: [PATCH 12/29] handle comments --- ...larisManagementServiceIntegrationTest.java | 4 ++-- ...olarisEclipseLinkMetaStoreSessionImpl.java | 4 ++-- .../jdbc/JdbcBasePersistenceImpl.java | 10 +++++--- .../polaris/core/auth/PolarisAuthorizer.java | 2 -- .../core/auth/PolarisSecretsManager.java | 1 - .../core/entity/PolarisPrincipalSecrets.java | 13 ---------- .../AtomicOperationMetaStoreManager.java | 24 ++++--------------- .../persistence/IntegrationPersistence.java | 3 --- .../TransactionWorkspaceMetaStoreManager.java | 1 - .../TransactionalMetaStoreManagerImpl.java | 14 ++++------- .../TreeMapTransactionalPersistenceImpl.java | 12 ++++++---- 11 files changed, 27 insertions(+), 61 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 e98917ed1d..d50b6ab35c 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 @@ -915,8 +915,8 @@ public void testCreatePrincipalAndResetCredentialsWithCustomValues() { String principalToken = client.obtainToken(resetCreds); customBody = Map.of( - "clientId", "abcd1234567999e3", - "clientSecret", "12346d236azzzzzz04922b0a07031bc3"); + "clientId", "g174b76a7e1a99e3", + "clientSecret", "37029d236abc08e204922b0a07031bc3"); try (Response response = client .managementApi(principalToken) diff --git a/persistence/eclipselink/src/main/java/org/apache/polaris/extension/persistence/impl/eclipselink/PolarisEclipseLinkMetaStoreSessionImpl.java b/persistence/eclipselink/src/main/java/org/apache/polaris/extension/persistence/impl/eclipselink/PolarisEclipseLinkMetaStoreSessionImpl.java index ede4758f01..8cb56dc365 100644 --- a/persistence/eclipselink/src/main/java/org/apache/polaris/extension/persistence/impl/eclipselink/PolarisEclipseLinkMetaStoreSessionImpl.java +++ b/persistence/eclipselink/src/main/java/org/apache/polaris/extension/persistence/impl/eclipselink/PolarisEclipseLinkMetaStoreSessionImpl.java @@ -754,9 +754,9 @@ public PolarisPrincipalSecrets resetPrincipalSecrets( @Nonnull PolarisCallContext callCtx, @Nonnull String clientId, long principalId, - boolean reset, String customClientId, String customClientSecret) { - throw new UnsupportedOperationException("This method is not supported for EclipseLink as metastore"); + throw new UnsupportedOperationException( + "This method is not supported for EclipseLink as metastore"); } } diff --git a/persistence/relational-jdbc/src/main/java/org/apache/polaris/persistence/relational/jdbc/JdbcBasePersistenceImpl.java b/persistence/relational-jdbc/src/main/java/org/apache/polaris/persistence/relational/jdbc/JdbcBasePersistenceImpl.java index 728c96015a..19f9071625 100644 --- a/persistence/relational-jdbc/src/main/java/org/apache/polaris/persistence/relational/jdbc/JdbcBasePersistenceImpl.java +++ b/persistence/relational-jdbc/src/main/java/org/apache/polaris/persistence/relational/jdbc/JdbcBasePersistenceImpl.java @@ -779,7 +779,6 @@ public PolarisPrincipalSecrets resetPrincipalSecrets( @Nonnull PolarisCallContext callCtx, @Nonnull String clientId, long principalId, - boolean reset, String customClientId, String customClientSecret) { PolarisPrincipalSecrets principalSecrets = loadPrincipalSecrets(callCtx, clientId); @@ -804,8 +803,13 @@ public PolarisPrincipalSecrets resetPrincipalSecrets( principalId, principalSecrets.getPrincipalId()); - principalSecrets.setPrincipalClientId(customClientId); - principalSecrets.resetSecrets(customClientSecret); + if (customClientId != null && customClientSecret != null) { + principalSecrets = + new PolarisPrincipalSecrets( + principalSecrets.getPrincipalId(), customClientId, customClientSecret, null); + } else { + principalSecrets.rotateSecrets(principalSecrets.getMainSecretHash()); + } Map params = Map.of("principal_client_id", clientId, "realm_id", realmId); try { diff --git a/polaris-core/src/main/java/org/apache/polaris/core/auth/PolarisAuthorizer.java b/polaris-core/src/main/java/org/apache/polaris/core/auth/PolarisAuthorizer.java index 6f75bac3f6..55c3792067 100644 --- a/polaris-core/src/main/java/org/apache/polaris/core/auth/PolarisAuthorizer.java +++ b/polaris-core/src/main/java/org/apache/polaris/core/auth/PolarisAuthorizer.java @@ -41,6 +41,4 @@ void authorizeOrThrow( @Nonnull PolarisAuthorizableOperation authzOp, @Nullable List targets, @Nullable List secondaries); - - void authorizeOrThrow(@Nonnull AuthenticatedPolarisPrincipal authenticatedPrincipal); } diff --git a/polaris-core/src/main/java/org/apache/polaris/core/auth/PolarisSecretsManager.java b/polaris-core/src/main/java/org/apache/polaris/core/auth/PolarisSecretsManager.java index 037ebcb788..04154c6616 100644 --- a/polaris-core/src/main/java/org/apache/polaris/core/auth/PolarisSecretsManager.java +++ b/polaris-core/src/main/java/org/apache/polaris/core/auth/PolarisSecretsManager.java @@ -60,7 +60,6 @@ PrincipalSecretsResult resetPrincipalSecrets( @Nonnull PolarisCallContext callCtx, @Nonnull String clientId, long principalId, - boolean reset, @Nonnull String oldSecretHash, String customClientId, String customClientSecret); diff --git a/polaris-core/src/main/java/org/apache/polaris/core/entity/PolarisPrincipalSecrets.java b/polaris-core/src/main/java/org/apache/polaris/core/entity/PolarisPrincipalSecrets.java index 6ab4580e44..4903c51aba 100644 --- a/polaris-core/src/main/java/org/apache/polaris/core/entity/PolarisPrincipalSecrets.java +++ b/polaris-core/src/main/java/org/apache/polaris/core/entity/PolarisPrincipalSecrets.java @@ -147,19 +147,6 @@ public void rotateSecrets(String newSecondaryHash) { this.mainSecretHash = hashSecret(mainSecret); } - /** Reset the main secrets */ - public void resetSecrets(String customClientSecret) { - this.mainSecret = customClientSecret; - this.mainSecretHash = hashSecret(customClientSecret); - - this.secondarySecret = null; - this.secondarySecretHash = hashSecret(customClientSecret); - } - - public void setPrincipalClientId(String customClientId) { - this.principalClientId = customClientId; - } - public long getPrincipalId() { return principalId; } diff --git a/polaris-core/src/main/java/org/apache/polaris/core/persistence/AtomicOperationMetaStoreManager.java b/polaris-core/src/main/java/org/apache/polaris/core/persistence/AtomicOperationMetaStoreManager.java index 0e0038ed9f..fcf0f49305 100644 --- a/polaris-core/src/main/java/org/apache/polaris/core/persistence/AtomicOperationMetaStoreManager.java +++ b/polaris-core/src/main/java/org/apache/polaris/core/persistence/AtomicOperationMetaStoreManager.java @@ -917,7 +917,6 @@ private void revokeGrantRecord( @Nonnull PolarisCallContext callCtx, @Nonnull String clientId, long principalId, - boolean reset, @Nonnull String oldSecretHash, String customClientId, String customClientSecret) { @@ -937,30 +936,15 @@ private void revokeGrantRecord( PolarisObjectMapperUtil.deserializeProperties( principal.getInternalProperties() == null ? "{}" : principal.getInternalProperties()); - boolean doReset = - reset - || internalProps.get( - PolarisEntityConstants.PRINCIPAL_CREDENTIAL_ROTATION_REQUIRED_STATE) - != null; PolarisPrincipalSecrets secrets = ((IntegrationPersistence) ms) .resetPrincipalSecrets( - callCtx, clientId, principalId, doReset, customClientId, customClientSecret); + callCtx, clientId, principalId, customClientId, customClientSecret); PolarisBaseEntity.Builder principalBuilder = new PolarisBaseEntity.Builder(principal); - if (reset - && !internalProps.containsKey( - PolarisEntityConstants.PRINCIPAL_CREDENTIAL_ROTATION_REQUIRED_STATE) - && customClientId != null - && customClientSecret != null) { - internalProps.put( - PolarisEntityConstants.PRINCIPAL_CREDENTIAL_ROTATION_REQUIRED_STATE, "true"); - principalBuilder.internalProperties( - PolarisObjectMapperUtil.serializeProperties(internalProps)); - principalBuilder.entityVersion(principal.getEntityVersion() + 1); - ms.writeEntity(callCtx, principalBuilder.build(), true, principal); - } - + principalBuilder.internalProperties(PolarisObjectMapperUtil.serializeProperties(internalProps)); + principalBuilder.entityVersion(principal.getEntityVersion() + 1); + ms.writeEntity(callCtx, principalBuilder.build(), true, principal); return (secrets == null) ? new PrincipalSecretsResult(BaseResult.ReturnStatus.ENTITY_NOT_FOUND, null) : new PrincipalSecretsResult(secrets); diff --git a/polaris-core/src/main/java/org/apache/polaris/core/persistence/IntegrationPersistence.java b/polaris-core/src/main/java/org/apache/polaris/core/persistence/IntegrationPersistence.java index 83a5ec72f1..e0c0813ca2 100644 --- a/polaris-core/src/main/java/org/apache/polaris/core/persistence/IntegrationPersistence.java +++ b/polaris-core/src/main/java/org/apache/polaris/core/persistence/IntegrationPersistence.java @@ -88,8 +88,6 @@ PolarisPrincipalSecrets rotatePrincipalSecrets( * @param callCtx call context * @param clientId principal client id * @param principalId principal id - * @param reset true if the principal secrets should be disabled and replaced with a one-time - * password * @param customClientId the principal secret's old main secret hash * @param customClientSecret the principal secret's old main secret hash */ @@ -98,7 +96,6 @@ PolarisPrincipalSecrets resetPrincipalSecrets( @Nonnull PolarisCallContext callCtx, @Nonnull String clientId, long principalId, - boolean reset, String customClientId, String customClientSecret); diff --git a/polaris-core/src/main/java/org/apache/polaris/core/persistence/TransactionWorkspaceMetaStoreManager.java b/polaris-core/src/main/java/org/apache/polaris/core/persistence/TransactionWorkspaceMetaStoreManager.java index 7142c80c79..f748c8e9c1 100644 --- a/polaris-core/src/main/java/org/apache/polaris/core/persistence/TransactionWorkspaceMetaStoreManager.java +++ b/polaris-core/src/main/java/org/apache/polaris/core/persistence/TransactionWorkspaceMetaStoreManager.java @@ -177,7 +177,6 @@ public PrincipalSecretsResult resetPrincipalSecrets( @Nonnull PolarisCallContext callCtx, @Nonnull String clientId, long principalId, - boolean reset, @Nonnull String oldSecretHash, String customClientId, String customClientSecret) { diff --git a/polaris-core/src/main/java/org/apache/polaris/core/persistence/transactional/TransactionalMetaStoreManagerImpl.java b/polaris-core/src/main/java/org/apache/polaris/core/persistence/transactional/TransactionalMetaStoreManagerImpl.java index 9357050269..2aed8f9390 100644 --- a/polaris-core/src/main/java/org/apache/polaris/core/persistence/transactional/TransactionalMetaStoreManagerImpl.java +++ b/polaris-core/src/main/java/org/apache/polaris/core/persistence/transactional/TransactionalMetaStoreManagerImpl.java @@ -982,7 +982,6 @@ private void bootstrapPolarisService( @Nonnull TransactionalPersistence ms, @Nonnull String clientId, long principalId, - boolean reset, @Nonnull String oldSecretHash, String customClientId, String customClientSecret) { @@ -1005,16 +1004,13 @@ private void bootstrapPolarisService( principal.getInternalProperties() == null ? "{}" : principal.getInternalProperties()); boolean doReset = - reset - || internalProps.get( - PolarisEntityConstants.PRINCIPAL_CREDENTIAL_ROTATION_REQUIRED_STATE) - != null; + internalProps.get(PolarisEntityConstants.PRINCIPAL_CREDENTIAL_ROTATION_REQUIRED_STATE) + != null; PolarisPrincipalSecrets secrets = ms.resetPrincipalSecrets( - callCtx, clientId, principalId, doReset, customClientId, customClientSecret); + callCtx, clientId, principalId, customClientId, customClientSecret); - if (reset - && !internalProps.containsKey( + if (!internalProps.containsKey( PolarisEntityConstants.PRINCIPAL_CREDENTIAL_ROTATION_REQUIRED_STATE) && customClientId != null && customClientSecret != null) { @@ -1033,7 +1029,6 @@ private void bootstrapPolarisService( @Nonnull PolarisCallContext callCtx, @Nonnull String clientId, long principalId, - boolean reset, @Nonnull String oldSecretHash, String customClientId, String customClientSecret) { @@ -1050,7 +1045,6 @@ private void bootstrapPolarisService( ms, clientId, principalId, - reset, oldSecretHash, customClientId, customClientSecret)); diff --git a/polaris-core/src/main/java/org/apache/polaris/core/persistence/transactional/TreeMapTransactionalPersistenceImpl.java b/polaris-core/src/main/java/org/apache/polaris/core/persistence/transactional/TreeMapTransactionalPersistenceImpl.java index 5bffed0c87..1d30a9f267 100644 --- a/polaris-core/src/main/java/org/apache/polaris/core/persistence/transactional/TreeMapTransactionalPersistenceImpl.java +++ b/polaris-core/src/main/java/org/apache/polaris/core/persistence/transactional/TreeMapTransactionalPersistenceImpl.java @@ -490,7 +490,6 @@ public int lookupEntityGrantRecordsVersionInCurrentTxn( @Nonnull PolarisCallContext callCtx, @Nonnull String clientId, long principalId, - boolean reset, String customClientId, String customClientSecret) { PolarisPrincipalSecrets principalSecrets = this.store.getSlicePrincipalSecrets().read(clientId); @@ -515,9 +514,14 @@ public int lookupEntityGrantRecordsVersionInCurrentTxn( principalId, principalSecrets.getPrincipalId()); - // rotate the secrets - principalSecrets.setPrincipalClientId(customClientId); - principalSecrets.resetSecrets(customClientSecret); + // reset the secrets + if (customClientId != null && customClientSecret != null) { + principalSecrets = + new PolarisPrincipalSecrets( + principalSecrets.getPrincipalId(), customClientId, customClientSecret, null); + } else { + principalSecrets.rotateSecrets(principalSecrets.getMainSecretHash()); + } // write back new secrets this.store.getSlicePrincipalSecrets().write(principalSecrets); From a62123117ab0920313f0c13a5ab35bcc20c39758 Mon Sep 17 00:00:00 2001 From: "arun.suri" Date: Fri, 15 Aug 2025 05:38:00 +0530 Subject: [PATCH 13/29] :polaris-core:spotlessApply --- .../TransactionalMetaStoreManagerImpl.java | 18 +++--------------- 1 file changed, 3 insertions(+), 15 deletions(-) diff --git a/polaris-core/src/main/java/org/apache/polaris/core/persistence/transactional/TransactionalMetaStoreManagerImpl.java b/polaris-core/src/main/java/org/apache/polaris/core/persistence/transactional/TransactionalMetaStoreManagerImpl.java index 2aed8f9390..1455ff7efd 100644 --- a/polaris-core/src/main/java/org/apache/polaris/core/persistence/transactional/TransactionalMetaStoreManagerImpl.java +++ b/polaris-core/src/main/java/org/apache/polaris/core/persistence/transactional/TransactionalMetaStoreManagerImpl.java @@ -1003,24 +1003,12 @@ private void bootstrapPolarisService( PolarisObjectMapperUtil.deserializeProperties( principal.getInternalProperties() == null ? "{}" : principal.getInternalProperties()); - boolean doReset = - internalProps.get(PolarisEntityConstants.PRINCIPAL_CREDENTIAL_ROTATION_REQUIRED_STATE) - != null; PolarisPrincipalSecrets secrets = ms.resetPrincipalSecrets( callCtx, clientId, principalId, customClientId, customClientSecret); - - if (!internalProps.containsKey( - PolarisEntityConstants.PRINCIPAL_CREDENTIAL_ROTATION_REQUIRED_STATE) - && customClientId != null - && customClientSecret != null) { - internalProps.put( - PolarisEntityConstants.PRINCIPAL_CREDENTIAL_ROTATION_REQUIRED_STATE, "true"); - principalBuilder.internalProperties( - PolarisObjectMapperUtil.serializeProperties(internalProps)); - principalBuilder.entityVersion(principal.getEntityVersion() + 1); - ms.writeEntityInCurrentTxn(callCtx, principalBuilder.build(), true, principal); - } + principalBuilder.internalProperties(PolarisObjectMapperUtil.serializeProperties(internalProps)); + principalBuilder.entityVersion(principal.getEntityVersion() + 1); + ms.writeEntityInCurrentTxn(callCtx, principalBuilder.build(), true, principal); return secrets; } From ac4d8b3d34b0af461d353bd8052343347fb455bf Mon Sep 17 00:00:00 2001 From: "arun.suri" Date: Fri, 15 Aug 2025 06:18:43 +0530 Subject: [PATCH 14/29] some more refactor --- .../service/admin/PolarisServiceImpl.java | 26 ++++++++++++++++++- spec/polaris-management-service.yml | 14 +++++----- 2 files changed, 31 insertions(+), 9 deletions(-) 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 a5b44a9116..a1546e73ba 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 @@ -171,6 +171,18 @@ public Response createCatalog( return Response.status(Response.Status.CREATED).build(); } + private void validateResetClientId(String clientId) { + if (!clientId.matches("^[0-9a-f]{16}$")) { + throw new IllegalArgumentException("Invalid resetClientId format"); + } + } + + private void validateResetClientSecret(String clientSecret) { + if (!clientSecret.matches("^[0-9a-f]{32}$")) { + throw new IllegalArgumentException("Invalid resetClientSecret format"); + } + } + private void validateStorageConfig(StorageConfigInfo storageConfigInfo) { List allowedStorageTypes = realmConfig.getConfig(FeatureConfiguration.SUPPORTED_CATALOG_STORAGE_TYPES); @@ -300,8 +312,20 @@ public Response resetCredentials( ResetPrincipalRequest resetPrincipalRequest, RealmContext realmContext, SecurityContext securityContext) { + ResetPrincipalRequest safeResetPrincipalRequest = + (resetPrincipalRequest != null) + ? resetPrincipalRequest + : new ResetPrincipalRequest(null, null); + + if (safeResetPrincipalRequest.getClientId() != null) { + validateResetClientId(safeResetPrincipalRequest.getClientId()); + } + if (safeResetPrincipalRequest.getClientSecret() != null) { + validateResetClientSecret(safeResetPrincipalRequest.getClientSecret()); + } PolarisAdminService adminService = newAdminService(realmContext, securityContext); - return Response.ok(adminService.resetCredentials(principalName, resetPrincipalRequest)).build(); + return Response.ok(adminService.resetCredentials(principalName, safeResetPrincipalRequest)) + .build(); } /** From PolarisPrincipalsApiService */ diff --git a/spec/polaris-management-service.yml b/spec/polaris-management-service.yml index 1c95bed87a..aa87a9b663 100644 --- a/spec/polaris-management-service.yml +++ b/spec/polaris-management-service.yml @@ -1289,16 +1289,14 @@ components: properties: clientId: type: string - description: Optional client ID to set for the principal. - minLength: 16 - maxLength: 16 - pattern: '^[0-9a-f]{16}$' + description: > + Optional client ID to set for the principal. + Must be a valid client ID previously generated by this service. clientSecret: type: string - description: Optional client secret to set for the principal. - minLength: 32 - maxLength: 32 - pattern: '^[0-9a-f]{32}$' + description: > + Optional client secret to set for the principal. + Must match the secret issued for the given client ID. PrincipalRoles: type: object From d4d98d562effbbfcf79dcee7290beb619b50fb71 Mon Sep 17 00:00:00 2001 From: "arun.suri" Date: Fri, 15 Aug 2025 06:31:07 +0530 Subject: [PATCH 15/29] fix test --- .../it/test/PolarisManagementServiceIntegrationTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 d50b6ab35c..93ca8d5dff 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 @@ -915,7 +915,7 @@ public void testCreatePrincipalAndResetCredentialsWithCustomValues() { String principalToken = client.obtainToken(resetCreds); customBody = Map.of( - "clientId", "g174b76a7e1a99e3", + "clientId", "a174b76a7e1a99e3", "clientSecret", "37029d236abc08e204922b0a07031bc3"); try (Response response = client From add9284c29d145420ede43faedff7ae4c7943afd Mon Sep 17 00:00:00 2001 From: "arun.suri" Date: Fri, 15 Aug 2025 07:23:18 +0530 Subject: [PATCH 16/29] refactor --- .../PolarisEclipseLinkMetaStoreSessionImpl.java | 3 ++- .../relational/jdbc/JdbcBasePersistenceImpl.java | 5 +++-- .../polaris/core/entity/PolarisPrincipalSecrets.java | 2 +- .../persistence/AtomicOperationMetaStoreManager.java | 10 ++++++++-- .../core/persistence/IntegrationPersistence.java | 4 +++- .../TransactionalMetaStoreManagerImpl.java | 11 +++++++++-- .../TreeMapTransactionalPersistenceImpl.java | 5 +++-- 7 files changed, 29 insertions(+), 11 deletions(-) diff --git a/persistence/eclipselink/src/main/java/org/apache/polaris/extension/persistence/impl/eclipselink/PolarisEclipseLinkMetaStoreSessionImpl.java b/persistence/eclipselink/src/main/java/org/apache/polaris/extension/persistence/impl/eclipselink/PolarisEclipseLinkMetaStoreSessionImpl.java index 8cb56dc365..c5a71d8724 100644 --- a/persistence/eclipselink/src/main/java/org/apache/polaris/extension/persistence/impl/eclipselink/PolarisEclipseLinkMetaStoreSessionImpl.java +++ b/persistence/eclipselink/src/main/java/org/apache/polaris/extension/persistence/impl/eclipselink/PolarisEclipseLinkMetaStoreSessionImpl.java @@ -755,7 +755,8 @@ public PolarisPrincipalSecrets resetPrincipalSecrets( @Nonnull String clientId, long principalId, String customClientId, - String customClientSecret) { + String customClientSecret, + boolean customReset) { throw new UnsupportedOperationException( "This method is not supported for EclipseLink as metastore"); } diff --git a/persistence/relational-jdbc/src/main/java/org/apache/polaris/persistence/relational/jdbc/JdbcBasePersistenceImpl.java b/persistence/relational-jdbc/src/main/java/org/apache/polaris/persistence/relational/jdbc/JdbcBasePersistenceImpl.java index 19f9071625..9de118d31c 100644 --- a/persistence/relational-jdbc/src/main/java/org/apache/polaris/persistence/relational/jdbc/JdbcBasePersistenceImpl.java +++ b/persistence/relational-jdbc/src/main/java/org/apache/polaris/persistence/relational/jdbc/JdbcBasePersistenceImpl.java @@ -780,7 +780,8 @@ public PolarisPrincipalSecrets resetPrincipalSecrets( @Nonnull String clientId, long principalId, String customClientId, - String customClientSecret) { + String customClientSecret, + boolean customReset) { PolarisPrincipalSecrets principalSecrets = loadPrincipalSecrets(callCtx, clientId); // should be found @@ -803,7 +804,7 @@ public PolarisPrincipalSecrets resetPrincipalSecrets( principalId, principalSecrets.getPrincipalId()); - if (customClientId != null && customClientSecret != null) { + if (customReset) { principalSecrets = new PolarisPrincipalSecrets( principalSecrets.getPrincipalId(), customClientId, customClientSecret, null); diff --git a/polaris-core/src/main/java/org/apache/polaris/core/entity/PolarisPrincipalSecrets.java b/polaris-core/src/main/java/org/apache/polaris/core/entity/PolarisPrincipalSecrets.java index 4903c51aba..dc98d46f6a 100644 --- a/polaris-core/src/main/java/org/apache/polaris/core/entity/PolarisPrincipalSecrets.java +++ b/polaris-core/src/main/java/org/apache/polaris/core/entity/PolarisPrincipalSecrets.java @@ -36,7 +36,7 @@ public class PolarisPrincipalSecrets { private final long principalId; // the client id for that principal - private String principalClientId; + private final String principalClientId; // the main secret hash for that principal private String mainSecret; diff --git a/polaris-core/src/main/java/org/apache/polaris/core/persistence/AtomicOperationMetaStoreManager.java b/polaris-core/src/main/java/org/apache/polaris/core/persistence/AtomicOperationMetaStoreManager.java index fcf0f49305..9246f176aa 100644 --- a/polaris-core/src/main/java/org/apache/polaris/core/persistence/AtomicOperationMetaStoreManager.java +++ b/polaris-core/src/main/java/org/apache/polaris/core/persistence/AtomicOperationMetaStoreManager.java @@ -936,14 +936,20 @@ private void revokeGrantRecord( PolarisObjectMapperUtil.deserializeProperties( principal.getInternalProperties() == null ? "{}" : principal.getInternalProperties()); + boolean customReset = customClientId != null && customClientSecret != null; PolarisPrincipalSecrets secrets = ((IntegrationPersistence) ms) .resetPrincipalSecrets( - callCtx, clientId, principalId, customClientId, customClientSecret); + callCtx, clientId, principalId, customClientId, customClientSecret, customReset); PolarisBaseEntity.Builder principalBuilder = new PolarisBaseEntity.Builder(principal); principalBuilder.internalProperties(PolarisObjectMapperUtil.serializeProperties(internalProps)); - principalBuilder.entityVersion(principal.getEntityVersion() + 1); + // To avoid incrementing entity version twice + if (customReset) { + principalBuilder.entityVersion(principal.getEntityVersion()); + } else { + principalBuilder.entityVersion(principal.getEntityVersion() + 1); + } ms.writeEntity(callCtx, principalBuilder.build(), true, principal); return (secrets == null) ? new PrincipalSecretsResult(BaseResult.ReturnStatus.ENTITY_NOT_FOUND, null) diff --git a/polaris-core/src/main/java/org/apache/polaris/core/persistence/IntegrationPersistence.java b/polaris-core/src/main/java/org/apache/polaris/core/persistence/IntegrationPersistence.java index e0c0813ca2..56f78b1803 100644 --- a/polaris-core/src/main/java/org/apache/polaris/core/persistence/IntegrationPersistence.java +++ b/polaris-core/src/main/java/org/apache/polaris/core/persistence/IntegrationPersistence.java @@ -90,6 +90,7 @@ PolarisPrincipalSecrets rotatePrincipalSecrets( * @param principalId principal id * @param customClientId the principal secret's old main secret hash * @param customClientSecret the principal secret's old main secret hash + * @param customReset */ @Nullable PolarisPrincipalSecrets resetPrincipalSecrets( @@ -97,7 +98,8 @@ PolarisPrincipalSecrets resetPrincipalSecrets( @Nonnull String clientId, long principalId, String customClientId, - String customClientSecret); + String customClientSecret, + boolean customReset); /** * When dropping a principal, we also need to drop the secrets of that principal diff --git a/polaris-core/src/main/java/org/apache/polaris/core/persistence/transactional/TransactionalMetaStoreManagerImpl.java b/polaris-core/src/main/java/org/apache/polaris/core/persistence/transactional/TransactionalMetaStoreManagerImpl.java index 1455ff7efd..eb0bbe7d5a 100644 --- a/polaris-core/src/main/java/org/apache/polaris/core/persistence/transactional/TransactionalMetaStoreManagerImpl.java +++ b/polaris-core/src/main/java/org/apache/polaris/core/persistence/transactional/TransactionalMetaStoreManagerImpl.java @@ -1003,11 +1003,18 @@ private void bootstrapPolarisService( PolarisObjectMapperUtil.deserializeProperties( principal.getInternalProperties() == null ? "{}" : principal.getInternalProperties()); + boolean customReset = customClientId != null && customClientSecret != null; + PolarisPrincipalSecrets secrets = ms.resetPrincipalSecrets( - callCtx, clientId, principalId, customClientId, customClientSecret); + callCtx, clientId, principalId, customClientId, customClientSecret, customReset); principalBuilder.internalProperties(PolarisObjectMapperUtil.serializeProperties(internalProps)); - principalBuilder.entityVersion(principal.getEntityVersion() + 1); + // To avoid incrementing entity version twice + if (customReset) { + principalBuilder.entityVersion(principal.getEntityVersion()); + } else { + principalBuilder.entityVersion(principal.getEntityVersion() + 1); + } ms.writeEntityInCurrentTxn(callCtx, principalBuilder.build(), true, principal); return secrets; } diff --git a/polaris-core/src/main/java/org/apache/polaris/core/persistence/transactional/TreeMapTransactionalPersistenceImpl.java b/polaris-core/src/main/java/org/apache/polaris/core/persistence/transactional/TreeMapTransactionalPersistenceImpl.java index 1d30a9f267..1f5626f70d 100644 --- a/polaris-core/src/main/java/org/apache/polaris/core/persistence/transactional/TreeMapTransactionalPersistenceImpl.java +++ b/polaris-core/src/main/java/org/apache/polaris/core/persistence/transactional/TreeMapTransactionalPersistenceImpl.java @@ -491,7 +491,8 @@ public int lookupEntityGrantRecordsVersionInCurrentTxn( @Nonnull String clientId, long principalId, String customClientId, - String customClientSecret) { + String customClientSecret, + boolean customReset) { PolarisPrincipalSecrets principalSecrets = this.store.getSlicePrincipalSecrets().read(clientId); // should be found @@ -515,7 +516,7 @@ public int lookupEntityGrantRecordsVersionInCurrentTxn( principalSecrets.getPrincipalId()); // reset the secrets - if (customClientId != null && customClientSecret != null) { + if (customReset) { principalSecrets = new PolarisPrincipalSecrets( principalSecrets.getPrincipalId(), customClientId, customClientSecret, null); From 3ae1f79d0de3b81fb3280e3e65f6c0ebd54802fd Mon Sep 17 00:00:00 2001 From: "arun.suri" Date: Wed, 20 Aug 2025 01:58:25 +0530 Subject: [PATCH 17/29] comments --- .../PolarisEclipseLinkMetaStoreSessionImpl.java | 3 +-- .../relational/jdbc/JdbcBasePersistenceImpl.java | 5 ++--- .../persistence/AtomicOperationMetaStoreManager.java | 8 +++----- .../core/persistence/IntegrationPersistence.java | 4 +--- .../TransactionalMetaStoreManagerImpl.java | 2 +- .../TreeMapTransactionalPersistenceImpl.java | 5 ++--- .../polaris/service/admin/PolarisServiceImpl.java | 12 ++++++------ spec/polaris-management-service.yml | 1 - 8 files changed, 16 insertions(+), 24 deletions(-) diff --git a/persistence/eclipselink/src/main/java/org/apache/polaris/extension/persistence/impl/eclipselink/PolarisEclipseLinkMetaStoreSessionImpl.java b/persistence/eclipselink/src/main/java/org/apache/polaris/extension/persistence/impl/eclipselink/PolarisEclipseLinkMetaStoreSessionImpl.java index c5a71d8724..8cb56dc365 100644 --- a/persistence/eclipselink/src/main/java/org/apache/polaris/extension/persistence/impl/eclipselink/PolarisEclipseLinkMetaStoreSessionImpl.java +++ b/persistence/eclipselink/src/main/java/org/apache/polaris/extension/persistence/impl/eclipselink/PolarisEclipseLinkMetaStoreSessionImpl.java @@ -755,8 +755,7 @@ public PolarisPrincipalSecrets resetPrincipalSecrets( @Nonnull String clientId, long principalId, String customClientId, - String customClientSecret, - boolean customReset) { + String customClientSecret) { throw new UnsupportedOperationException( "This method is not supported for EclipseLink as metastore"); } diff --git a/persistence/relational-jdbc/src/main/java/org/apache/polaris/persistence/relational/jdbc/JdbcBasePersistenceImpl.java b/persistence/relational-jdbc/src/main/java/org/apache/polaris/persistence/relational/jdbc/JdbcBasePersistenceImpl.java index 9de118d31c..19f9071625 100644 --- a/persistence/relational-jdbc/src/main/java/org/apache/polaris/persistence/relational/jdbc/JdbcBasePersistenceImpl.java +++ b/persistence/relational-jdbc/src/main/java/org/apache/polaris/persistence/relational/jdbc/JdbcBasePersistenceImpl.java @@ -780,8 +780,7 @@ public PolarisPrincipalSecrets resetPrincipalSecrets( @Nonnull String clientId, long principalId, String customClientId, - String customClientSecret, - boolean customReset) { + String customClientSecret) { PolarisPrincipalSecrets principalSecrets = loadPrincipalSecrets(callCtx, clientId); // should be found @@ -804,7 +803,7 @@ public PolarisPrincipalSecrets resetPrincipalSecrets( principalId, principalSecrets.getPrincipalId()); - if (customReset) { + if (customClientId != null && customClientSecret != null) { principalSecrets = new PolarisPrincipalSecrets( principalSecrets.getPrincipalId(), customClientId, customClientSecret, null); diff --git a/polaris-core/src/main/java/org/apache/polaris/core/persistence/AtomicOperationMetaStoreManager.java b/polaris-core/src/main/java/org/apache/polaris/core/persistence/AtomicOperationMetaStoreManager.java index 9246f176aa..adf4225656 100644 --- a/polaris-core/src/main/java/org/apache/polaris/core/persistence/AtomicOperationMetaStoreManager.java +++ b/polaris-core/src/main/java/org/apache/polaris/core/persistence/AtomicOperationMetaStoreManager.java @@ -932,15 +932,13 @@ private void revokeGrantRecord( } PolarisBaseEntity principal = loadEntityResult.getEntity(); - Map internalProps = - PolarisObjectMapperUtil.deserializeProperties( - principal.getInternalProperties() == null ? "{}" : principal.getInternalProperties()); + Map internalProps = principal.getInternalPropertiesAsMap(); boolean customReset = customClientId != null && customClientSecret != null; PolarisPrincipalSecrets secrets = ((IntegrationPersistence) ms) .resetPrincipalSecrets( - callCtx, clientId, principalId, customClientId, customClientSecret, customReset); + callCtx, clientId, principalId, customClientId, customClientSecret); PolarisBaseEntity.Builder principalBuilder = new PolarisBaseEntity.Builder(principal); principalBuilder.internalProperties(PolarisObjectMapperUtil.serializeProperties(internalProps)); @@ -949,8 +947,8 @@ private void revokeGrantRecord( principalBuilder.entityVersion(principal.getEntityVersion()); } else { principalBuilder.entityVersion(principal.getEntityVersion() + 1); + ms.writeEntity(callCtx, principalBuilder.build(), true, principal); } - ms.writeEntity(callCtx, principalBuilder.build(), true, principal); return (secrets == null) ? new PrincipalSecretsResult(BaseResult.ReturnStatus.ENTITY_NOT_FOUND, null) : new PrincipalSecretsResult(secrets); diff --git a/polaris-core/src/main/java/org/apache/polaris/core/persistence/IntegrationPersistence.java b/polaris-core/src/main/java/org/apache/polaris/core/persistence/IntegrationPersistence.java index 56f78b1803..e0c0813ca2 100644 --- a/polaris-core/src/main/java/org/apache/polaris/core/persistence/IntegrationPersistence.java +++ b/polaris-core/src/main/java/org/apache/polaris/core/persistence/IntegrationPersistence.java @@ -90,7 +90,6 @@ PolarisPrincipalSecrets rotatePrincipalSecrets( * @param principalId principal id * @param customClientId the principal secret's old main secret hash * @param customClientSecret the principal secret's old main secret hash - * @param customReset */ @Nullable PolarisPrincipalSecrets resetPrincipalSecrets( @@ -98,8 +97,7 @@ PolarisPrincipalSecrets resetPrincipalSecrets( @Nonnull String clientId, long principalId, String customClientId, - String customClientSecret, - boolean customReset); + String customClientSecret); /** * When dropping a principal, we also need to drop the secrets of that principal diff --git a/polaris-core/src/main/java/org/apache/polaris/core/persistence/transactional/TransactionalMetaStoreManagerImpl.java b/polaris-core/src/main/java/org/apache/polaris/core/persistence/transactional/TransactionalMetaStoreManagerImpl.java index eb0bbe7d5a..e95cdc4863 100644 --- a/polaris-core/src/main/java/org/apache/polaris/core/persistence/transactional/TransactionalMetaStoreManagerImpl.java +++ b/polaris-core/src/main/java/org/apache/polaris/core/persistence/transactional/TransactionalMetaStoreManagerImpl.java @@ -1007,7 +1007,7 @@ private void bootstrapPolarisService( PolarisPrincipalSecrets secrets = ms.resetPrincipalSecrets( - callCtx, clientId, principalId, customClientId, customClientSecret, customReset); + callCtx, clientId, principalId, customClientId, customClientSecret); principalBuilder.internalProperties(PolarisObjectMapperUtil.serializeProperties(internalProps)); // To avoid incrementing entity version twice if (customReset) { diff --git a/polaris-core/src/main/java/org/apache/polaris/core/persistence/transactional/TreeMapTransactionalPersistenceImpl.java b/polaris-core/src/main/java/org/apache/polaris/core/persistence/transactional/TreeMapTransactionalPersistenceImpl.java index 1f5626f70d..1d30a9f267 100644 --- a/polaris-core/src/main/java/org/apache/polaris/core/persistence/transactional/TreeMapTransactionalPersistenceImpl.java +++ b/polaris-core/src/main/java/org/apache/polaris/core/persistence/transactional/TreeMapTransactionalPersistenceImpl.java @@ -491,8 +491,7 @@ public int lookupEntityGrantRecordsVersionInCurrentTxn( @Nonnull String clientId, long principalId, String customClientId, - String customClientSecret, - boolean customReset) { + String customClientSecret) { PolarisPrincipalSecrets principalSecrets = this.store.getSlicePrincipalSecrets().read(clientId); // should be found @@ -516,7 +515,7 @@ public int lookupEntityGrantRecordsVersionInCurrentTxn( principalSecrets.getPrincipalId()); // reset the secrets - if (customReset) { + if (customClientId != null && customClientSecret != null) { principalSecrets = new PolarisPrincipalSecrets( principalSecrets.getPrincipalId(), customClientId, customClientSecret, null); 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 a1546e73ba..636c044976 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 @@ -171,15 +171,15 @@ public Response createCatalog( return Response.status(Response.Status.CREATED).build(); } - private void validateResetClientId(String clientId) { + private void validateClientId(String clientId) { if (!clientId.matches("^[0-9a-f]{16}$")) { - throw new IllegalArgumentException("Invalid resetClientId format"); + throw new IllegalArgumentException("Invalid clientId format"); } } - private void validateResetClientSecret(String clientSecret) { + private void validateClientSecret(String clientSecret) { if (!clientSecret.matches("^[0-9a-f]{32}$")) { - throw new IllegalArgumentException("Invalid resetClientSecret format"); + throw new IllegalArgumentException("Invalid clientSecret format"); } } @@ -318,10 +318,10 @@ public Response resetCredentials( : new ResetPrincipalRequest(null, null); if (safeResetPrincipalRequest.getClientId() != null) { - validateResetClientId(safeResetPrincipalRequest.getClientId()); + validateClientId(safeResetPrincipalRequest.getClientId()); } if (safeResetPrincipalRequest.getClientSecret() != null) { - validateResetClientSecret(safeResetPrincipalRequest.getClientSecret()); + validateClientSecret(safeResetPrincipalRequest.getClientSecret()); } PolarisAdminService adminService = newAdminService(realmContext, securityContext); return Response.ok(adminService.resetCredentials(principalName, safeResetPrincipalRequest)) diff --git a/spec/polaris-management-service.yml b/spec/polaris-management-service.yml index aa87a9b663..a4fed562d2 100644 --- a/spec/polaris-management-service.yml +++ b/spec/polaris-management-service.yml @@ -199,7 +199,6 @@ paths: 404: description: "The catalog or principal does not exist" - put: operationId: updatePrincipal description: Update an existing principal From a4623b64f2dc332ad5dd7bb25d8df8efe0dcfd9c Mon Sep 17 00:00:00 2001 From: "arun.suri" Date: Wed, 20 Aug 2025 02:36:47 +0530 Subject: [PATCH 18/29] comments --- .../transactional/TransactionalMetaStoreManagerImpl.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/polaris-core/src/main/java/org/apache/polaris/core/persistence/transactional/TransactionalMetaStoreManagerImpl.java b/polaris-core/src/main/java/org/apache/polaris/core/persistence/transactional/TransactionalMetaStoreManagerImpl.java index e95cdc4863..2a1c0fcd99 100644 --- a/polaris-core/src/main/java/org/apache/polaris/core/persistence/transactional/TransactionalMetaStoreManagerImpl.java +++ b/polaris-core/src/main/java/org/apache/polaris/core/persistence/transactional/TransactionalMetaStoreManagerImpl.java @@ -1014,8 +1014,8 @@ private void bootstrapPolarisService( principalBuilder.entityVersion(principal.getEntityVersion()); } else { principalBuilder.entityVersion(principal.getEntityVersion() + 1); + ms.writeEntityInCurrentTxn(callCtx, principalBuilder.build(), true, principal); } - ms.writeEntityInCurrentTxn(callCtx, principalBuilder.build(), true, principal); return secrets; } From bb5ad2942fd5d08d5fe3808443044e0703a796f3 Mon Sep 17 00:00:00 2001 From: "arun.suri" Date: Wed, 20 Aug 2025 03:31:07 +0530 Subject: [PATCH 19/29] rebase --- .../core/auth/PolarisAuthorizerImpl.java | 20 +++-- .../service/admin/PolarisAdminService.java | 75 +++++++++++++++++-- 2 files changed, 79 insertions(+), 16 deletions(-) diff --git a/polaris-core/src/main/java/org/apache/polaris/core/auth/PolarisAuthorizerImpl.java b/polaris-core/src/main/java/org/apache/polaris/core/auth/PolarisAuthorizerImpl.java index 30a75b3e31..5091f4b82d 100644 --- a/polaris-core/src/main/java/org/apache/polaris/core/auth/PolarisAuthorizerImpl.java +++ b/polaris-core/src/main/java/org/apache/polaris/core/auth/PolarisAuthorizerImpl.java @@ -582,6 +582,7 @@ public void authorizeOrThrow( boolean enforceCredentialRotationRequiredState = realmConfig.getConfig( FeatureConfiguration.ENFORCE_PRINCIPAL_CREDENTIAL_ROTATION_REQUIRED_CHECKING); + boolean isRoot = getRootPrincipalName().equals(polarisPrincipal.getName()); if (enforceCredentialRotationRequiredState && polarisPrincipal .getProperties() @@ -590,6 +591,14 @@ public void authorizeOrThrow( throw new ForbiddenException( "Principal '%s' is not authorized for op %s due to PRINCIPAL_CREDENTIAL_ROTATION_REQUIRED_STATE", polarisPrincipal.getName(), authzOp); + } else if (authzOp == PolarisAuthorizableOperation.RESET_CREDENTIALS) { + if (!isRoot) { + throw new ForbiddenException("Only Root principal(service-admin) can perform %s", authzOp); + } + LOGGER + .atDebug() + .addKeyValue("principalName", polarisPrincipal.getName()) + .log("Root principal allowed to reset credentials"); } else if (!isAuthorized(polarisPrincipal, activatedEntities, authzOp, targets, secondaries)) { throw new ForbiddenException( "Principal '%s' with activated PrincipalRoles '%s' and activated grants via '%s' is not authorized for op %s", @@ -600,17 +609,6 @@ public void authorizeOrThrow( } } - @Override - public void authorizeOrThrow(@Nonnull AuthenticatedPolarisPrincipal authenticatedPrincipal) { - boolean isRoot = - getRootPrincipalName().equals(authenticatedPrincipal.getPrincipalEntity().getName()); - if (!isRoot) { - throw new ForbiddenException( - "Only %s principal can reset credentials", - authenticatedPrincipal.getPrincipalEntity().getName()); - } - } - /** * Based on the required target/targetParent/secondary/secondaryParent privileges mapped from * {@code authzOp}, determines whether the caller's set of activatedGranteeIds is authorized for 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 b86bfa41d5..f23e93cc1f 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 @@ -67,6 +67,7 @@ import org.apache.polaris.core.admin.model.PrincipalRole; import org.apache.polaris.core.admin.model.PrincipalWithCredentials; import org.apache.polaris.core.admin.model.PrincipalWithCredentialsCredentials; +import org.apache.polaris.core.admin.model.ResetPrincipalRequest; import org.apache.polaris.core.admin.model.TableGrant; import org.apache.polaris.core.admin.model.TablePrivilege; import org.apache.polaris.core.admin.model.UpdateCatalogRequest; @@ -252,8 +253,7 @@ private void authorizeBasicTopLevelEntityOperationOrThrow( // be extracted into an EnumSet and/or pushed down into PolarisAuthorizer. if (topLevelEntityWrapper.getResolvedLeafEntity().getEntity().getId() == polarisPrincipal.getId() - && (op.equals(PolarisAuthorizableOperation.ROTATE_CREDENTIALS) - || op.equals(PolarisAuthorizableOperation.RESET_CREDENTIALS))) { + && (op.equals(PolarisAuthorizableOperation.ROTATE_CREDENTIALS))) { LOGGER .atDebug() .addKeyValue("principalName", topLevelEntityName) @@ -1064,6 +1064,69 @@ public void deletePrincipal(String name) { return returnedEntity; } + private @Nonnull PrincipalWithCredentials resetCredentialsHelper( + String principalName, String customClientId, String customClientSecret) { + PrincipalEntity currentPrincipalEntity = + findPrincipalByName(principalName) + .orElseThrow(() -> new NotFoundException("Principal %s not found", principalName)); + + if (FederatedEntities.isFederated(currentPrincipalEntity)) { + throw new ValidationException( + "Cannot reset credentials for a federated principal: %s", principalName); + } + PolarisPrincipalSecrets currentSecrets = + metaStoreManager + .loadPrincipalSecrets(getCurrentPolarisContext(), currentPrincipalEntity.getClientId()) + .getPrincipalSecrets(); + if (currentSecrets == null) { + throw new IllegalArgumentException( + String.format("Failed to load current secrets for principal '%s'", principalName)); + } + PolarisPrincipalSecrets newSecrets = + metaStoreManager + .resetPrincipalSecrets( + getCurrentPolarisContext(), + currentPrincipalEntity.getClientId(), + currentPrincipalEntity.getId(), + currentSecrets.getMainSecretHash(), + customClientId, + customClientSecret) + .getPrincipalSecrets(); + if (newSecrets == null) { + throw new IllegalStateException( + String.format("Failed to %s secrets for principal '%s'", "reset", principalName)); + } + PolarisEntity newPrincipal = + PolarisEntity.of( + metaStoreManager.loadEntity( + getCurrentPolarisContext(), + 0L, + currentPrincipalEntity.getId(), + currentPrincipalEntity.getType())); + + PrincipalEntity newPrincipalEntity = PrincipalEntity.of(newPrincipal); + if (customClientId != null && customClientSecret != null) { + PrincipalEntity.Builder updateBuilder = new PrincipalEntity.Builder(newPrincipalEntity); + updateBuilder.setClientId(newSecrets.getPrincipalClientId()); + PrincipalEntity updatedNewPrincipalEntity = updateBuilder.build(); + updatedNewPrincipalEntity = + Optional.ofNullable( + PrincipalEntity.of( + PolarisEntity.of( + metaStoreManager.updateEntityPropertiesIfNotChanged( + getCurrentPolarisContext(), null, updatedNewPrincipalEntity)))) + .orElseThrow( + () -> + new CommitFailedException( + "Concurrent modification on Principal '%s'; retry later", principalName)); + newPrincipalEntity = updatedNewPrincipalEntity; + } + return new PrincipalWithCredentials( + newPrincipalEntity.asPrincipal(), + new PrincipalWithCredentialsCredentials( + newSecrets.getPrincipalClientId(), newSecrets.getMainSecret())); + } + private @Nonnull PrincipalWithCredentials rotateOrResetCredentialsHelper( String principalName, boolean shouldReset) { PrincipalEntity currentPrincipalEntity = @@ -1117,11 +1180,13 @@ public void deletePrincipal(String name) { return rotateOrResetCredentialsHelper(principalName, false); } - public @Nonnull PrincipalWithCredentials resetCredentials(String principalName) { + public @Nonnull PrincipalWithCredentials resetCredentials( + String principalName, ResetPrincipalRequest resetPrincipalRequest) { PolarisAuthorizableOperation op = PolarisAuthorizableOperation.RESET_CREDENTIALS; authorizeBasicTopLevelEntityOperationOrThrow(op, principalName, PolarisEntityType.PRINCIPAL); - - return rotateOrResetCredentialsHelper(principalName, true); + var customClientId = resetPrincipalRequest.getClientId(); + var customClientSecret = resetPrincipalRequest.getClientSecret(); + return resetCredentialsHelper(principalName, customClientId, customClientSecret); } public List listPrincipals() { From 95d60b5fe9710ccc54cde08875e5bbcc48166abd Mon Sep 17 00:00:00 2001 From: "arun.suri" Date: Wed, 20 Aug 2025 05:32:54 +0530 Subject: [PATCH 20/29] make changes to PolarisPrincipalSecrets --- .../polaris/service/it/env/ManagementApi.java | 14 ++++++++++++++ ...olarisManagementServiceIntegrationTest.java | 4 ++++ .../jdbc/JdbcBasePersistenceImpl.java | 8 +------- .../core/entity/PolarisPrincipalSecrets.java | 16 ++++++++++++++++ .../AtomicOperationMetaStoreManager.java | 15 ++++++--------- .../TransactionalMetaStoreManagerImpl.java | 18 ++++++------------ .../TreeMapTransactionalPersistenceImpl.java | 8 +------- .../service/admin/PolarisAdminService.java | 2 +- 8 files changed, 49 insertions(+), 36 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 9bd1ccded1..3e76d92d72 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 @@ -80,6 +80,20 @@ public PrincipalWithCredentials createPrincipal(CreatePrincipalRequest request) } } + /** + * Retrieves a Principal by name via the management API. + * + * @param principalName the name of the principal to fetch + * @return the Principal object + */ + public Principal getPrincipal(String principalName) { + try (Response response = + request("v1/principals/{principalName}", Map.of("principalName", principalName)).get()) { + assertThat(response).returns(Response.Status.OK.getStatusCode(), Response::getStatus); + return response.readEntity(Principal.class); + } + } + public void createPrincipalRole(String name) { createPrincipalRole(new PrincipalRole(name)); } 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 93ca8d5dff..9bd1e04a06 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 @@ -911,6 +911,10 @@ public void testCreatePrincipalAndResetCredentialsWithCustomValues() { assertThat(resetCreds.getCredentials().getClientSecret()) .isEqualTo("27029d236abc08e204922b0a07031bc2"); + // Validate that the principal entity itself is updated in sync with credentials + Principal updatedPrincipal = managementApi.getPrincipal(principal.getName()); + assertThat(updatedPrincipal.getClientId()).isEqualTo("f174b76a7e1a99e2"); + // Principal itself tries to reset with custom creds → should fail (403 Forbidden) String principalToken = client.obtainToken(resetCreds); customBody = diff --git a/persistence/relational-jdbc/src/main/java/org/apache/polaris/persistence/relational/jdbc/JdbcBasePersistenceImpl.java b/persistence/relational-jdbc/src/main/java/org/apache/polaris/persistence/relational/jdbc/JdbcBasePersistenceImpl.java index 19f9071625..66dd5f8dc2 100644 --- a/persistence/relational-jdbc/src/main/java/org/apache/polaris/persistence/relational/jdbc/JdbcBasePersistenceImpl.java +++ b/persistence/relational-jdbc/src/main/java/org/apache/polaris/persistence/relational/jdbc/JdbcBasePersistenceImpl.java @@ -803,13 +803,7 @@ public PolarisPrincipalSecrets resetPrincipalSecrets( principalId, principalSecrets.getPrincipalId()); - if (customClientId != null && customClientSecret != null) { - principalSecrets = - new PolarisPrincipalSecrets( - principalSecrets.getPrincipalId(), customClientId, customClientSecret, null); - } else { - principalSecrets.rotateSecrets(principalSecrets.getMainSecretHash()); - } + principalSecrets = principalSecrets.resetSecrets(customClientId, customClientSecret); Map params = Map.of("principal_client_id", clientId, "realm_id", realmId); try { diff --git a/polaris-core/src/main/java/org/apache/polaris/core/entity/PolarisPrincipalSecrets.java b/polaris-core/src/main/java/org/apache/polaris/core/entity/PolarisPrincipalSecrets.java index dc98d46f6a..9b930e9c99 100644 --- a/polaris-core/src/main/java/org/apache/polaris/core/entity/PolarisPrincipalSecrets.java +++ b/polaris-core/src/main/java/org/apache/polaris/core/entity/PolarisPrincipalSecrets.java @@ -20,6 +20,7 @@ import com.fasterxml.jackson.annotation.JsonCreator; import com.fasterxml.jackson.annotation.JsonProperty; +import jakarta.annotation.Nullable; import java.security.SecureRandom; import org.apache.commons.codec.digest.DigestUtils; @@ -147,6 +148,21 @@ public void rotateSecrets(String newSecondaryHash) { this.mainSecretHash = hashSecret(mainSecret); } + public PolarisPrincipalSecrets resetSecrets( + @Nullable String newClientId, @Nullable String newSecret) { + String finalClientId = (newClientId != null) ? newClientId : this.principalClientId; + String finalSecret = (newSecret != null) ? newSecret : this.generateRandomHexString(32); + String finalSecondarySecret = this.secondarySecret; // keep existing secondary secret + return new PolarisPrincipalSecrets( + this.principalId, + finalClientId, + finalSecret, + finalSecondarySecret, + this.secretSalt, // reuse existing salt + null, // recompute mainSecretHash + this.secondarySecretHash); + } + public long getPrincipalId() { return principalId; } diff --git a/polaris-core/src/main/java/org/apache/polaris/core/persistence/AtomicOperationMetaStoreManager.java b/polaris-core/src/main/java/org/apache/polaris/core/persistence/AtomicOperationMetaStoreManager.java index adf4225656..cabd0bf3e3 100644 --- a/polaris-core/src/main/java/org/apache/polaris/core/persistence/AtomicOperationMetaStoreManager.java +++ b/polaris-core/src/main/java/org/apache/polaris/core/persistence/AtomicOperationMetaStoreManager.java @@ -932,21 +932,18 @@ private void revokeGrantRecord( } PolarisBaseEntity principal = loadEntityResult.getEntity(); - Map internalProps = principal.getInternalPropertiesAsMap(); - - boolean customReset = customClientId != null && customClientSecret != null; PolarisPrincipalSecrets secrets = ((IntegrationPersistence) ms) .resetPrincipalSecrets( callCtx, clientId, principalId, customClientId, customClientSecret); PolarisBaseEntity.Builder principalBuilder = new PolarisBaseEntity.Builder(principal); - principalBuilder.internalProperties(PolarisObjectMapperUtil.serializeProperties(internalProps)); - // To avoid incrementing entity version twice - if (customReset) { - principalBuilder.entityVersion(principal.getEntityVersion()); - } else { - principalBuilder.entityVersion(principal.getEntityVersion() + 1); + var newEntityVersion = + (customClientId != null) ? principal.getEntityVersion() : principal.getEntityVersion() + 1; + + principalBuilder.entityVersion(newEntityVersion); + // Only write if version changed + if (customClientId == null) { ms.writeEntity(callCtx, principalBuilder.build(), true, principal); } return (secrets == null) diff --git a/polaris-core/src/main/java/org/apache/polaris/core/persistence/transactional/TransactionalMetaStoreManagerImpl.java b/polaris-core/src/main/java/org/apache/polaris/core/persistence/transactional/TransactionalMetaStoreManagerImpl.java index 2a1c0fcd99..a8ae99a0bb 100644 --- a/polaris-core/src/main/java/org/apache/polaris/core/persistence/transactional/TransactionalMetaStoreManagerImpl.java +++ b/polaris-core/src/main/java/org/apache/polaris/core/persistence/transactional/TransactionalMetaStoreManagerImpl.java @@ -999,22 +999,16 @@ private void bootstrapPolarisService( PolarisBaseEntity principal = loadEntityResult.getEntity(); PolarisBaseEntity.Builder principalBuilder = new PolarisBaseEntity.Builder(principal); - Map internalProps = - PolarisObjectMapperUtil.deserializeProperties( - principal.getInternalProperties() == null ? "{}" : principal.getInternalProperties()); - - boolean customReset = customClientId != null && customClientSecret != null; - PolarisPrincipalSecrets secrets = ms.resetPrincipalSecrets( callCtx, clientId, principalId, customClientId, customClientSecret); - principalBuilder.internalProperties(PolarisObjectMapperUtil.serializeProperties(internalProps)); // To avoid incrementing entity version twice - if (customReset) { - principalBuilder.entityVersion(principal.getEntityVersion()); - } else { - principalBuilder.entityVersion(principal.getEntityVersion() + 1); - ms.writeEntityInCurrentTxn(callCtx, principalBuilder.build(), true, principal); + var newEntityVersion = + (customClientId != null) ? principal.getEntityVersion() : principal.getEntityVersion() + 1; + principalBuilder.entityVersion(newEntityVersion); + // Only write if version changed + if (customClientId == null) { + ms.writeEntity(callCtx, principalBuilder.build(), true, principal); } return secrets; } diff --git a/polaris-core/src/main/java/org/apache/polaris/core/persistence/transactional/TreeMapTransactionalPersistenceImpl.java b/polaris-core/src/main/java/org/apache/polaris/core/persistence/transactional/TreeMapTransactionalPersistenceImpl.java index 1d30a9f267..962a3e029e 100644 --- a/polaris-core/src/main/java/org/apache/polaris/core/persistence/transactional/TreeMapTransactionalPersistenceImpl.java +++ b/polaris-core/src/main/java/org/apache/polaris/core/persistence/transactional/TreeMapTransactionalPersistenceImpl.java @@ -515,13 +515,7 @@ public int lookupEntityGrantRecordsVersionInCurrentTxn( principalSecrets.getPrincipalId()); // reset the secrets - if (customClientId != null && customClientSecret != null) { - principalSecrets = - new PolarisPrincipalSecrets( - principalSecrets.getPrincipalId(), customClientId, customClientSecret, null); - } else { - principalSecrets.rotateSecrets(principalSecrets.getMainSecretHash()); - } + principalSecrets = principalSecrets.resetSecrets(customClientId, customClientSecret); // write back new secrets this.store.getSlicePrincipalSecrets().write(principalSecrets); 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 f23e93cc1f..6d7aba6081 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 @@ -1105,7 +1105,7 @@ public void deletePrincipal(String name) { currentPrincipalEntity.getType())); PrincipalEntity newPrincipalEntity = PrincipalEntity.of(newPrincipal); - if (customClientId != null && customClientSecret != null) { + if (customClientId != null) { PrincipalEntity.Builder updateBuilder = new PrincipalEntity.Builder(newPrincipalEntity); updateBuilder.setClientId(newSecrets.getPrincipalClientId()); PrincipalEntity updatedNewPrincipalEntity = updateBuilder.build(); From f0ab60dab9f22152696e62c8539edac7cb131e86 Mon Sep 17 00:00:00 2001 From: "arun.suri" Date: Wed, 20 Aug 2025 05:55:20 +0530 Subject: [PATCH 21/29] make changes to APi doc --- spec/polaris-management-service.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/polaris-management-service.yml b/spec/polaris-management-service.yml index a4fed562d2..322f1473b5 100644 --- a/spec/polaris-management-service.yml +++ b/spec/polaris-management-service.yml @@ -1295,7 +1295,7 @@ components: type: string description: > Optional client secret to set for the principal. - Must match the secret issued for the given client ID. + Polaris service implementations may impose extra requirements on what is accepted as a secret (special chars, length, etc.) PrincipalRoles: type: object From 5f0b4f90e8a8dc8209fecff36065c5c74de8baca Mon Sep 17 00:00:00 2001 From: "arun.suri" Date: Fri, 22 Aug 2025 05:20:10 +0530 Subject: [PATCH 22/29] handle idempotency --- .../core/auth/PolarisSecretsManager.java | 5 +- .../AtomicOperationMetaStoreManager.java | 11 +++- .../TransactionWorkspaceMetaStoreManager.java | 7 ++- .../TransactionalMetaStoreManagerImpl.java | 16 ++++-- .../service/admin/PolarisAdminService.java | 50 ++++++++----------- 5 files changed, 52 insertions(+), 37 deletions(-) diff --git a/polaris-core/src/main/java/org/apache/polaris/core/auth/PolarisSecretsManager.java b/polaris-core/src/main/java/org/apache/polaris/core/auth/PolarisSecretsManager.java index 04154c6616..91270ffed5 100644 --- a/polaris-core/src/main/java/org/apache/polaris/core/auth/PolarisSecretsManager.java +++ b/polaris-core/src/main/java/org/apache/polaris/core/auth/PolarisSecretsManager.java @@ -60,7 +60,10 @@ PrincipalSecretsResult resetPrincipalSecrets( @Nonnull PolarisCallContext callCtx, @Nonnull String clientId, long principalId, - @Nonnull String oldSecretHash, String customClientId, String customClientSecret); + + @Nonnull + void deletePrincipalSecrets( + @Nonnull PolarisCallContext callCtx, @Nonnull String clientId, long principalId); } diff --git a/polaris-core/src/main/java/org/apache/polaris/core/persistence/AtomicOperationMetaStoreManager.java b/polaris-core/src/main/java/org/apache/polaris/core/persistence/AtomicOperationMetaStoreManager.java index cabd0bf3e3..4d75a1449c 100644 --- a/polaris-core/src/main/java/org/apache/polaris/core/persistence/AtomicOperationMetaStoreManager.java +++ b/polaris-core/src/main/java/org/apache/polaris/core/persistence/AtomicOperationMetaStoreManager.java @@ -857,6 +857,16 @@ private void revokeGrantRecord( : new PrincipalSecretsResult(secrets); } + /** {@inheritDoc} */ + @Override + public @Nonnull void deletePrincipalSecrets( + @Nonnull PolarisCallContext callCtx, @Nonnull String clientId, long principalId) { + // get metastore we should be using + BasePersistence ms = callCtx.getMetaStore(); + ((IntegrationPersistence) ms).deletePrincipalSecrets(callCtx, clientId, principalId); + + } + /** {@inheritDoc} */ @Override public @Nonnull PrincipalSecretsResult rotatePrincipalSecrets( @@ -917,7 +927,6 @@ private void revokeGrantRecord( @Nonnull PolarisCallContext callCtx, @Nonnull String clientId, long principalId, - @Nonnull String oldSecretHash, String customClientId, String customClientSecret) { // get metastore we should be using diff --git a/polaris-core/src/main/java/org/apache/polaris/core/persistence/TransactionWorkspaceMetaStoreManager.java b/polaris-core/src/main/java/org/apache/polaris/core/persistence/TransactionWorkspaceMetaStoreManager.java index f748c8e9c1..a303c77ba9 100644 --- a/polaris-core/src/main/java/org/apache/polaris/core/persistence/TransactionWorkspaceMetaStoreManager.java +++ b/polaris-core/src/main/java/org/apache/polaris/core/persistence/TransactionWorkspaceMetaStoreManager.java @@ -161,6 +161,12 @@ public PrincipalSecretsResult loadPrincipalSecrets( return null; } + @Override + public void deletePrincipalSecrets( + @Nonnull PolarisCallContext callCtx, @Nonnull String clientId, long principalId) { + diagnostics.fail("illegal_method_in_transaction_workspace", "loadPrincipalSecrets"); + } + @Override public PrincipalSecretsResult rotatePrincipalSecrets( @Nonnull PolarisCallContext callCtx, @@ -177,7 +183,6 @@ public PrincipalSecretsResult resetPrincipalSecrets( @Nonnull PolarisCallContext callCtx, @Nonnull String clientId, long principalId, - @Nonnull String oldSecretHash, String customClientId, String customClientSecret) { callCtx diff --git a/polaris-core/src/main/java/org/apache/polaris/core/persistence/transactional/TransactionalMetaStoreManagerImpl.java b/polaris-core/src/main/java/org/apache/polaris/core/persistence/transactional/TransactionalMetaStoreManagerImpl.java index a8ae99a0bb..0e920effa0 100644 --- a/polaris-core/src/main/java/org/apache/polaris/core/persistence/transactional/TransactionalMetaStoreManagerImpl.java +++ b/polaris-core/src/main/java/org/apache/polaris/core/persistence/transactional/TransactionalMetaStoreManagerImpl.java @@ -902,6 +902,17 @@ private void bootstrapPolarisService( : new PrincipalSecretsResult(secrets); } + /** {@inheritDoc} */ + @Override + public @Nonnull void deletePrincipalSecrets( + @Nonnull PolarisCallContext callCtx, @Nonnull String clientId, long principalId) { + // get metastore we should be using + TransactionalPersistence ms = ((TransactionalPersistence) callCtx.getMetaStore()); + + // need to run inside a read/write transaction + ms.deletePrincipalSecretsInCurrentTxn(callCtx, clientId, principalId); + } + /** See {@link #} */ private @Nullable PolarisPrincipalSecrets rotatePrincipalSecrets( @Nonnull PolarisCallContext callCtx, @@ -982,7 +993,6 @@ private void bootstrapPolarisService( @Nonnull TransactionalPersistence ms, @Nonnull String clientId, long principalId, - @Nonnull String oldSecretHash, String customClientId, String customClientSecret) { // if not found, the principal must have been dropped @@ -1018,7 +1028,6 @@ private void bootstrapPolarisService( @Nonnull PolarisCallContext callCtx, @Nonnull String clientId, long principalId, - @Nonnull String oldSecretHash, String customClientId, String customClientSecret) { // get metastore we should be using @@ -1034,8 +1043,7 @@ private void bootstrapPolarisService( ms, clientId, principalId, - oldSecretHash, - customClientId, + customClientId, customClientSecret)); return (secrets == null) 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 6d7aba6081..bad1817644 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 @@ -42,7 +42,6 @@ import org.apache.iceberg.catalog.TableIdentifier; import org.apache.iceberg.exceptions.AlreadyExistsException; import org.apache.iceberg.exceptions.BadRequestException; -import org.apache.iceberg.exceptions.CommitFailedException; import org.apache.iceberg.exceptions.NoSuchNamespaceException; import org.apache.iceberg.exceptions.NotFoundException; import org.apache.iceberg.exceptions.ValidationException; @@ -1078,49 +1077,40 @@ public void deletePrincipal(String name) { metaStoreManager .loadPrincipalSecrets(getCurrentPolarisContext(), currentPrincipalEntity.getClientId()) .getPrincipalSecrets(); - if (currentSecrets == null) { - throw new IllegalArgumentException( - String.format("Failed to load current secrets for principal '%s'", principalName)); + if (currentSecrets != null) { + metaStoreManager.deletePrincipalSecrets(getCurrentPolarisContext(), currentPrincipalEntity.getClientId(), currentPrincipalEntity.getId()); + } + PrincipalEntity newPrincipalEntity = currentPrincipalEntity; + if (customClientId != null) { + PrincipalEntity.Builder updateBuilder = new PrincipalEntity.Builder(newPrincipalEntity); + updateBuilder.setClientId(customClientId); + PrincipalEntity updatedNewPrincipalEntity = updateBuilder.build(); + newPrincipalEntity = + Optional.ofNullable( + PrincipalEntity.of( + PolarisEntity.of( + metaStoreManager.updateEntityPropertiesIfNotChanged( + getCurrentPolarisContext(), null, updatedNewPrincipalEntity)))) + .orElseThrow( + () -> + new CommitConflictException( + "Concurrent modification on Principal '%s'; retry later", principalName)); } + PolarisPrincipalSecrets newSecrets = metaStoreManager .resetPrincipalSecrets( getCurrentPolarisContext(), currentPrincipalEntity.getClientId(), currentPrincipalEntity.getId(), - currentSecrets.getMainSecretHash(), - customClientId, + customClientId, customClientSecret) .getPrincipalSecrets(); if (newSecrets == null) { throw new IllegalStateException( String.format("Failed to %s secrets for principal '%s'", "reset", principalName)); } - PolarisEntity newPrincipal = - PolarisEntity.of( - metaStoreManager.loadEntity( - getCurrentPolarisContext(), - 0L, - currentPrincipalEntity.getId(), - currentPrincipalEntity.getType())); - PrincipalEntity newPrincipalEntity = PrincipalEntity.of(newPrincipal); - if (customClientId != null) { - PrincipalEntity.Builder updateBuilder = new PrincipalEntity.Builder(newPrincipalEntity); - updateBuilder.setClientId(newSecrets.getPrincipalClientId()); - PrincipalEntity updatedNewPrincipalEntity = updateBuilder.build(); - updatedNewPrincipalEntity = - Optional.ofNullable( - PrincipalEntity.of( - PolarisEntity.of( - metaStoreManager.updateEntityPropertiesIfNotChanged( - getCurrentPolarisContext(), null, updatedNewPrincipalEntity)))) - .orElseThrow( - () -> - new CommitFailedException( - "Concurrent modification on Principal '%s'; retry later", principalName)); - newPrincipalEntity = updatedNewPrincipalEntity; - } return new PrincipalWithCredentials( newPrincipalEntity.asPrincipal(), new PrincipalWithCredentialsCredentials( From e347b9bd13ea34759280bb965bdd8cc11dcd1f8a Mon Sep 17 00:00:00 2001 From: "arun.suri" Date: Sat, 23 Aug 2025 04:18:35 +0530 Subject: [PATCH 23/29] handle idempotency --- .../jdbc/JdbcBasePersistenceImpl.java | 35 +++++-------------- .../core/auth/PolarisSecretsManager.java | 2 +- .../core/entity/PolarisPrincipalSecrets.java | 27 +++++++------- .../AtomicOperationMetaStoreManager.java | 23 +----------- .../TransactionWorkspaceMetaStoreManager.java | 2 +- .../TransactionalMetaStoreManagerImpl.java | 31 ++-------------- .../TreeMapTransactionalPersistenceImpl.java | 34 ++++-------------- .../service/admin/PolarisAdminService.java | 25 +++++++------ 8 files changed, 47 insertions(+), 132 deletions(-) diff --git a/persistence/relational-jdbc/src/main/java/org/apache/polaris/persistence/relational/jdbc/JdbcBasePersistenceImpl.java b/persistence/relational-jdbc/src/main/java/org/apache/polaris/persistence/relational/jdbc/JdbcBasePersistenceImpl.java index 66dd5f8dc2..444d9345b7 100644 --- a/persistence/relational-jdbc/src/main/java/org/apache/polaris/persistence/relational/jdbc/JdbcBasePersistenceImpl.java +++ b/persistence/relational-jdbc/src/main/java/org/apache/polaris/persistence/relational/jdbc/JdbcBasePersistenceImpl.java @@ -781,36 +781,19 @@ public PolarisPrincipalSecrets resetPrincipalSecrets( long principalId, String customClientId, String customClientSecret) { - PolarisPrincipalSecrets principalSecrets = loadPrincipalSecrets(callCtx, clientId); - - // should be found - callCtx - .getDiagServices() - .checkNotNull( - principalSecrets, - "cannot_find_secrets", - "client_id={} principalId={}", - clientId, - principalId); - - // ensure principal id is matching - callCtx - .getDiagServices() - .check( - principalId == principalSecrets.getPrincipalId(), - "principal_id_mismatch", - "expectedId={} id={}", - principalId, - principalSecrets.getPrincipalId()); - principalSecrets = principalSecrets.resetSecrets(customClientId, customClientSecret); - - Map params = Map.of("principal_client_id", clientId, "realm_id", realmId); + PolarisPrincipalSecrets principalSecrets; + if (customClientId == null) { + principalSecrets = new PolarisPrincipalSecrets(principalId, clientId, customClientSecret); + } else { + principalSecrets = + new PolarisPrincipalSecrets(principalId, customClientId, customClientSecret); + } try { ModelPrincipalAuthenticationData modelPrincipalAuthenticationData = ModelPrincipalAuthenticationData.fromPrincipalAuthenticationData(principalSecrets); datasourceOperations.executeUpdate( - QueryGenerator.generateUpdateQuery( + QueryGenerator.generateInsertQuery( ModelPrincipalAuthenticationData.ALL_COLUMNS, ModelPrincipalAuthenticationData.TABLE_NAME, modelPrincipalAuthenticationData @@ -818,7 +801,7 @@ public PolarisPrincipalSecrets resetPrincipalSecrets( .values() .stream() .toList(), - params)); + realmId)); } catch (SQLException e) { LOGGER.error( "Failed to reset PrincipalSecrets for clientId: {}, due to {}", diff --git a/polaris-core/src/main/java/org/apache/polaris/core/auth/PolarisSecretsManager.java b/polaris-core/src/main/java/org/apache/polaris/core/auth/PolarisSecretsManager.java index 91270ffed5..04524515eb 100644 --- a/polaris-core/src/main/java/org/apache/polaris/core/auth/PolarisSecretsManager.java +++ b/polaris-core/src/main/java/org/apache/polaris/core/auth/PolarisSecretsManager.java @@ -65,5 +65,5 @@ PrincipalSecretsResult resetPrincipalSecrets( @Nonnull void deletePrincipalSecrets( - @Nonnull PolarisCallContext callCtx, @Nonnull String clientId, long principalId); + @Nonnull PolarisCallContext callCtx, @Nonnull String clientId, long principalId); } diff --git a/polaris-core/src/main/java/org/apache/polaris/core/entity/PolarisPrincipalSecrets.java b/polaris-core/src/main/java/org/apache/polaris/core/entity/PolarisPrincipalSecrets.java index 9b930e9c99..d3ab4a4310 100644 --- a/polaris-core/src/main/java/org/apache/polaris/core/entity/PolarisPrincipalSecrets.java +++ b/polaris-core/src/main/java/org/apache/polaris/core/entity/PolarisPrincipalSecrets.java @@ -139,6 +139,18 @@ public PolarisPrincipalSecrets(long principalId) { this.secondarySecretHash = hashSecret(secondarySecret); } + public PolarisPrincipalSecrets( + long principalId, @Nullable String newClientId, @Nullable String newSecret) { + this.principalId = principalId; + this.principalClientId = newClientId; + this.mainSecret = (newSecret != null) ? newSecret : this.generateRandomHexString(32); + this.secondarySecret = this.generateRandomHexString(32); + + this.secretSalt = this.generateRandomHexString(16); + this.mainSecretHash = hashSecret(mainSecret); + this.secondarySecretHash = hashSecret(secondarySecret); + } + /** Rotate the main secrets */ public void rotateSecrets(String newSecondaryHash) { this.secondarySecret = null; @@ -148,21 +160,6 @@ public void rotateSecrets(String newSecondaryHash) { this.mainSecretHash = hashSecret(mainSecret); } - public PolarisPrincipalSecrets resetSecrets( - @Nullable String newClientId, @Nullable String newSecret) { - String finalClientId = (newClientId != null) ? newClientId : this.principalClientId; - String finalSecret = (newSecret != null) ? newSecret : this.generateRandomHexString(32); - String finalSecondarySecret = this.secondarySecret; // keep existing secondary secret - return new PolarisPrincipalSecrets( - this.principalId, - finalClientId, - finalSecret, - finalSecondarySecret, - this.secretSalt, // reuse existing salt - null, // recompute mainSecretHash - this.secondarySecretHash); - } - public long getPrincipalId() { return principalId; } diff --git a/polaris-core/src/main/java/org/apache/polaris/core/persistence/AtomicOperationMetaStoreManager.java b/polaris-core/src/main/java/org/apache/polaris/core/persistence/AtomicOperationMetaStoreManager.java index 4d75a1449c..ddf385c699 100644 --- a/polaris-core/src/main/java/org/apache/polaris/core/persistence/AtomicOperationMetaStoreManager.java +++ b/polaris-core/src/main/java/org/apache/polaris/core/persistence/AtomicOperationMetaStoreManager.java @@ -860,11 +860,10 @@ private void revokeGrantRecord( /** {@inheritDoc} */ @Override public @Nonnull void deletePrincipalSecrets( - @Nonnull PolarisCallContext callCtx, @Nonnull String clientId, long principalId) { + @Nonnull PolarisCallContext callCtx, @Nonnull String clientId, long principalId) { // get metastore we should be using BasePersistence ms = callCtx.getMetaStore(); ((IntegrationPersistence) ms).deletePrincipalSecrets(callCtx, clientId, principalId); - } /** {@inheritDoc} */ @@ -931,30 +930,10 @@ private void revokeGrantRecord( String customClientSecret) { // get metastore we should be using BasePersistence ms = callCtx.getMetaStore(); - - // if not found, the principal must have been dropped - EntityResult loadEntityResult = - loadEntity( - callCtx, PolarisEntityConstants.getNullId(), principalId, PolarisEntityType.PRINCIPAL); - if (loadEntityResult.getReturnStatus() != BaseResult.ReturnStatus.SUCCESS) { - return new PrincipalSecretsResult(BaseResult.ReturnStatus.ENTITY_NOT_FOUND, null); - } - - PolarisBaseEntity principal = loadEntityResult.getEntity(); PolarisPrincipalSecrets secrets = ((IntegrationPersistence) ms) .resetPrincipalSecrets( callCtx, clientId, principalId, customClientId, customClientSecret); - - PolarisBaseEntity.Builder principalBuilder = new PolarisBaseEntity.Builder(principal); - var newEntityVersion = - (customClientId != null) ? principal.getEntityVersion() : principal.getEntityVersion() + 1; - - principalBuilder.entityVersion(newEntityVersion); - // Only write if version changed - if (customClientId == null) { - ms.writeEntity(callCtx, principalBuilder.build(), true, principal); - } return (secrets == null) ? new PrincipalSecretsResult(BaseResult.ReturnStatus.ENTITY_NOT_FOUND, null) : new PrincipalSecretsResult(secrets); diff --git a/polaris-core/src/main/java/org/apache/polaris/core/persistence/TransactionWorkspaceMetaStoreManager.java b/polaris-core/src/main/java/org/apache/polaris/core/persistence/TransactionWorkspaceMetaStoreManager.java index a303c77ba9..5b9aae95c1 100644 --- a/polaris-core/src/main/java/org/apache/polaris/core/persistence/TransactionWorkspaceMetaStoreManager.java +++ b/polaris-core/src/main/java/org/apache/polaris/core/persistence/TransactionWorkspaceMetaStoreManager.java @@ -163,7 +163,7 @@ public PrincipalSecretsResult loadPrincipalSecrets( @Override public void deletePrincipalSecrets( - @Nonnull PolarisCallContext callCtx, @Nonnull String clientId, long principalId) { + @Nonnull PolarisCallContext callCtx, @Nonnull String clientId, long principalId) { diagnostics.fail("illegal_method_in_transaction_workspace", "loadPrincipalSecrets"); } diff --git a/polaris-core/src/main/java/org/apache/polaris/core/persistence/transactional/TransactionalMetaStoreManagerImpl.java b/polaris-core/src/main/java/org/apache/polaris/core/persistence/transactional/TransactionalMetaStoreManagerImpl.java index 0e920effa0..8560e06d62 100644 --- a/polaris-core/src/main/java/org/apache/polaris/core/persistence/transactional/TransactionalMetaStoreManagerImpl.java +++ b/polaris-core/src/main/java/org/apache/polaris/core/persistence/transactional/TransactionalMetaStoreManagerImpl.java @@ -905,7 +905,7 @@ private void bootstrapPolarisService( /** {@inheritDoc} */ @Override public @Nonnull void deletePrincipalSecrets( - @Nonnull PolarisCallContext callCtx, @Nonnull String clientId, long principalId) { + @Nonnull PolarisCallContext callCtx, @Nonnull String clientId, long principalId) { // get metastore we should be using TransactionalPersistence ms = ((TransactionalPersistence) callCtx.getMetaStore()); @@ -995,31 +995,9 @@ private void bootstrapPolarisService( long principalId, String customClientId, String customClientSecret) { - // if not found, the principal must have been dropped - EntityResult loadEntityResult = - loadEntity( - callCtx, - ms, - PolarisEntityConstants.getNullId(), - principalId, - PolarisEntityType.PRINCIPAL.getCode()); - if (loadEntityResult.getReturnStatus() != BaseResult.ReturnStatus.SUCCESS) { - return null; - } - - PolarisBaseEntity principal = loadEntityResult.getEntity(); - PolarisBaseEntity.Builder principalBuilder = new PolarisBaseEntity.Builder(principal); PolarisPrincipalSecrets secrets = ms.resetPrincipalSecrets( callCtx, clientId, principalId, customClientId, customClientSecret); - // To avoid incrementing entity version twice - var newEntityVersion = - (customClientId != null) ? principal.getEntityVersion() : principal.getEntityVersion() + 1; - principalBuilder.entityVersion(newEntityVersion); - // Only write if version changed - if (customClientId == null) { - ms.writeEntity(callCtx, principalBuilder.build(), true, principal); - } return secrets; } @@ -1039,12 +1017,7 @@ private void bootstrapPolarisService( callCtx, () -> this.resetPrincipalSecrets( - callCtx, - ms, - clientId, - principalId, - customClientId, - customClientSecret)); + callCtx, ms, clientId, principalId, customClientId, customClientSecret)); return (secrets == null) ? new PrincipalSecretsResult(BaseResult.ReturnStatus.ENTITY_NOT_FOUND, null) diff --git a/polaris-core/src/main/java/org/apache/polaris/core/persistence/transactional/TreeMapTransactionalPersistenceImpl.java b/polaris-core/src/main/java/org/apache/polaris/core/persistence/transactional/TreeMapTransactionalPersistenceImpl.java index 962a3e029e..5fd0745d32 100644 --- a/polaris-core/src/main/java/org/apache/polaris/core/persistence/transactional/TreeMapTransactionalPersistenceImpl.java +++ b/polaris-core/src/main/java/org/apache/polaris/core/persistence/transactional/TreeMapTransactionalPersistenceImpl.java @@ -492,35 +492,15 @@ public int lookupEntityGrantRecordsVersionInCurrentTxn( long principalId, String customClientId, String customClientSecret) { - PolarisPrincipalSecrets principalSecrets = this.store.getSlicePrincipalSecrets().read(clientId); - - // should be found - callCtx - .getDiagServices() - .checkNotNull( - principalSecrets, - "cannot_find_secrets", - "client_id={} principalId={}", - clientId, - principalId); - - // ensure principal id is matching - callCtx - .getDiagServices() - .check( - principalId == principalSecrets.getPrincipalId(), - "principal_id_mismatch", - "expectedId={} id={}", - principalId, - principalSecrets.getPrincipalId()); - - // reset the secrets - principalSecrets = principalSecrets.resetSecrets(customClientId, customClientSecret); + PolarisPrincipalSecrets principalSecrets; + if (customClientId == null) { + principalSecrets = new PolarisPrincipalSecrets(principalId, clientId, customClientSecret); + } else { + principalSecrets = + new PolarisPrincipalSecrets(principalId, customClientId, customClientSecret); + } - // write back new secrets this.store.getSlicePrincipalSecrets().write(principalSecrets); - - // return those return principalSecrets; } 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 bad1817644..59e2acba53 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 @@ -1078,7 +1078,10 @@ public void deletePrincipal(String name) { .loadPrincipalSecrets(getCurrentPolarisContext(), currentPrincipalEntity.getClientId()) .getPrincipalSecrets(); if (currentSecrets != null) { - metaStoreManager.deletePrincipalSecrets(getCurrentPolarisContext(), currentPrincipalEntity.getClientId(), currentPrincipalEntity.getId()); + metaStoreManager.deletePrincipalSecrets( + getCurrentPolarisContext(), + currentPrincipalEntity.getClientId(), + currentPrincipalEntity.getId()); } PrincipalEntity newPrincipalEntity = currentPrincipalEntity; if (customClientId != null) { @@ -1086,15 +1089,15 @@ public void deletePrincipal(String name) { updateBuilder.setClientId(customClientId); PrincipalEntity updatedNewPrincipalEntity = updateBuilder.build(); newPrincipalEntity = - Optional.ofNullable( - PrincipalEntity.of( - PolarisEntity.of( - metaStoreManager.updateEntityPropertiesIfNotChanged( - getCurrentPolarisContext(), null, updatedNewPrincipalEntity)))) - .orElseThrow( - () -> - new CommitConflictException( - "Concurrent modification on Principal '%s'; retry later", principalName)); + Optional.ofNullable( + PrincipalEntity.of( + PolarisEntity.of( + metaStoreManager.updateEntityPropertiesIfNotChanged( + getCurrentPolarisContext(), null, updatedNewPrincipalEntity)))) + .orElseThrow( + () -> + new CommitConflictException( + "Concurrent modification on Principal '%s'; retry later", principalName)); } PolarisPrincipalSecrets newSecrets = @@ -1103,7 +1106,7 @@ public void deletePrincipal(String name) { getCurrentPolarisContext(), currentPrincipalEntity.getClientId(), currentPrincipalEntity.getId(), - customClientId, + customClientId, customClientSecret) .getPrincipalSecrets(); if (newSecrets == null) { From fedaa7b6e7bede5c98be9021939fa743c29e6ace Mon Sep 17 00:00:00 2001 From: "arun.suri" Date: Sat, 23 Aug 2025 04:41:23 +0530 Subject: [PATCH 24/29] handle if principal exists --- .../persistence/AtomicOperationMetaStoreManager.java | 8 ++++++++ .../TransactionalMetaStoreManagerImpl.java | 11 +++++++++++ .../TreeMapTransactionalPersistenceImpl.java | 4 +++- .../polaris/service/admin/PolarisAdminService.java | 4 ++++ 4 files changed, 26 insertions(+), 1 deletion(-) diff --git a/polaris-core/src/main/java/org/apache/polaris/core/persistence/AtomicOperationMetaStoreManager.java b/polaris-core/src/main/java/org/apache/polaris/core/persistence/AtomicOperationMetaStoreManager.java index ddf385c699..7b17f102b7 100644 --- a/polaris-core/src/main/java/org/apache/polaris/core/persistence/AtomicOperationMetaStoreManager.java +++ b/polaris-core/src/main/java/org/apache/polaris/core/persistence/AtomicOperationMetaStoreManager.java @@ -930,6 +930,14 @@ private void revokeGrantRecord( String customClientSecret) { // get metastore we should be using BasePersistence ms = callCtx.getMetaStore(); + // if not found, the principal must have been dropped + EntityResult loadEntityResult = + loadEntity( + callCtx, PolarisEntityConstants.getNullId(), principalId, PolarisEntityType.PRINCIPAL); + if (loadEntityResult.getReturnStatus() != BaseResult.ReturnStatus.SUCCESS) { + return new PrincipalSecretsResult(BaseResult.ReturnStatus.ENTITY_NOT_FOUND, null); + } + PolarisPrincipalSecrets secrets = ((IntegrationPersistence) ms) .resetPrincipalSecrets( diff --git a/polaris-core/src/main/java/org/apache/polaris/core/persistence/transactional/TransactionalMetaStoreManagerImpl.java b/polaris-core/src/main/java/org/apache/polaris/core/persistence/transactional/TransactionalMetaStoreManagerImpl.java index 8560e06d62..5eb5ce89d8 100644 --- a/polaris-core/src/main/java/org/apache/polaris/core/persistence/transactional/TransactionalMetaStoreManagerImpl.java +++ b/polaris-core/src/main/java/org/apache/polaris/core/persistence/transactional/TransactionalMetaStoreManagerImpl.java @@ -1010,6 +1010,17 @@ private void bootstrapPolarisService( String customClientSecret) { // get metastore we should be using TransactionalPersistence ms = ((TransactionalPersistence) callCtx.getMetaStore()); + // if not found, the principal must have been dropped + EntityResult loadEntityResult = + loadEntity( + callCtx, + ms, + PolarisEntityConstants.getNullId(), + principalId, + PolarisEntityType.PRINCIPAL.getCode()); + if (loadEntityResult.getReturnStatus() != BaseResult.ReturnStatus.SUCCESS) { + return null; + } // need to run inside a read/write transaction PolarisPrincipalSecrets secrets = diff --git a/polaris-core/src/main/java/org/apache/polaris/core/persistence/transactional/TreeMapTransactionalPersistenceImpl.java b/polaris-core/src/main/java/org/apache/polaris/core/persistence/transactional/TreeMapTransactionalPersistenceImpl.java index 5fd0745d32..dff5345a00 100644 --- a/polaris-core/src/main/java/org/apache/polaris/core/persistence/transactional/TreeMapTransactionalPersistenceImpl.java +++ b/polaris-core/src/main/java/org/apache/polaris/core/persistence/transactional/TreeMapTransactionalPersistenceImpl.java @@ -499,8 +499,10 @@ public int lookupEntityGrantRecordsVersionInCurrentTxn( principalSecrets = new PolarisPrincipalSecrets(principalId, customClientId, customClientSecret); } - + // write back new secrets this.store.getSlicePrincipalSecrets().write(principalSecrets); + + // return principal creds return principalSecrets; } 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 59e2acba53..fb4a8c38de 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 @@ -1077,6 +1077,7 @@ public void deletePrincipal(String name) { metaStoreManager .loadPrincipalSecrets(getCurrentPolarisContext(), currentPrincipalEntity.getClientId()) .getPrincipalSecrets(); + // delete the existing creds if present if (currentSecrets != null) { metaStoreManager.deletePrincipalSecrets( getCurrentPolarisContext(), @@ -1084,6 +1085,7 @@ public void deletePrincipal(String name) { currentPrincipalEntity.getId()); } PrincipalEntity newPrincipalEntity = currentPrincipalEntity; + // update the clientId tied to the principal entity if (customClientId != null) { PrincipalEntity.Builder updateBuilder = new PrincipalEntity.Builder(newPrincipalEntity); updateBuilder.setClientId(customClientId); @@ -1100,6 +1102,7 @@ public void deletePrincipal(String name) { "Concurrent modification on Principal '%s'; retry later", principalName)); } + // generate new secrets PolarisPrincipalSecrets newSecrets = metaStoreManager .resetPrincipalSecrets( @@ -1109,6 +1112,7 @@ public void deletePrincipal(String name) { customClientId, customClientSecret) .getPrincipalSecrets(); + if (newSecrets == null) { throw new IllegalStateException( String.format("Failed to %s secrets for principal '%s'", "reset", principalName)); From 9560fc97bcccd712702471ae9b4627843ef6f717 Mon Sep 17 00:00:00 2001 From: "arun.suri" Date: Sat, 23 Aug 2025 05:04:02 +0530 Subject: [PATCH 25/29] fix test --- .../transactional/TransactionalMetaStoreManagerImpl.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/polaris-core/src/main/java/org/apache/polaris/core/persistence/transactional/TransactionalMetaStoreManagerImpl.java b/polaris-core/src/main/java/org/apache/polaris/core/persistence/transactional/TransactionalMetaStoreManagerImpl.java index 5eb5ce89d8..587aa780d4 100644 --- a/polaris-core/src/main/java/org/apache/polaris/core/persistence/transactional/TransactionalMetaStoreManagerImpl.java +++ b/polaris-core/src/main/java/org/apache/polaris/core/persistence/transactional/TransactionalMetaStoreManagerImpl.java @@ -910,7 +910,7 @@ private void bootstrapPolarisService( TransactionalPersistence ms = ((TransactionalPersistence) callCtx.getMetaStore()); // need to run inside a read/write transaction - ms.deletePrincipalSecretsInCurrentTxn(callCtx, clientId, principalId); + ms.deletePrincipalSecrets(callCtx, clientId, principalId); } /** See {@link #} */ From 3c1cb9f38ffd3a2d90d93fac9ee352556eae46b5 Mon Sep 17 00:00:00 2001 From: "arun.suri" Date: Mon, 25 Aug 2025 17:43:25 +0530 Subject: [PATCH 26/29] handle comments --- ...olarisEclipseLinkMetaStoreSessionImpl.java | 2 +- .../jdbc/JdbcBasePersistenceImpl.java | 2 +- .../core/auth/PolarisSecretsManager.java | 25 +++++++++++++++++++ .../AtomicOperationMetaStoreManager.java | 2 +- .../persistence/IntegrationPersistence.java | 16 ++++++------ .../TransactionalMetaStoreManagerImpl.java | 10 +++----- .../TreeMapTransactionalPersistenceImpl.java | 2 +- 7 files changed, 41 insertions(+), 18 deletions(-) diff --git a/persistence/eclipselink/src/main/java/org/apache/polaris/extension/persistence/impl/eclipselink/PolarisEclipseLinkMetaStoreSessionImpl.java b/persistence/eclipselink/src/main/java/org/apache/polaris/extension/persistence/impl/eclipselink/PolarisEclipseLinkMetaStoreSessionImpl.java index 8cb56dc365..392c8d8877 100644 --- a/persistence/eclipselink/src/main/java/org/apache/polaris/extension/persistence/impl/eclipselink/PolarisEclipseLinkMetaStoreSessionImpl.java +++ b/persistence/eclipselink/src/main/java/org/apache/polaris/extension/persistence/impl/eclipselink/PolarisEclipseLinkMetaStoreSessionImpl.java @@ -750,7 +750,7 @@ Optional> hasOverlappingSiblings( @Nullable @Override - public PolarisPrincipalSecrets resetPrincipalSecrets( + public PolarisPrincipalSecrets storePrincipalSecrets( @Nonnull PolarisCallContext callCtx, @Nonnull String clientId, long principalId, diff --git a/persistence/relational-jdbc/src/main/java/org/apache/polaris/persistence/relational/jdbc/JdbcBasePersistenceImpl.java b/persistence/relational-jdbc/src/main/java/org/apache/polaris/persistence/relational/jdbc/JdbcBasePersistenceImpl.java index 444d9345b7..90cc12e225 100644 --- a/persistence/relational-jdbc/src/main/java/org/apache/polaris/persistence/relational/jdbc/JdbcBasePersistenceImpl.java +++ b/persistence/relational-jdbc/src/main/java/org/apache/polaris/persistence/relational/jdbc/JdbcBasePersistenceImpl.java @@ -775,7 +775,7 @@ public PolarisPrincipalSecrets generateNewPrincipalSecrets( @Nullable @Override - public PolarisPrincipalSecrets resetPrincipalSecrets( + public PolarisPrincipalSecrets storePrincipalSecrets( @Nonnull PolarisCallContext callCtx, @Nonnull String clientId, long principalId, diff --git a/polaris-core/src/main/java/org/apache/polaris/core/auth/PolarisSecretsManager.java b/polaris-core/src/main/java/org/apache/polaris/core/auth/PolarisSecretsManager.java index 04524515eb..1b9029d243 100644 --- a/polaris-core/src/main/java/org/apache/polaris/core/auth/PolarisSecretsManager.java +++ b/polaris-core/src/main/java/org/apache/polaris/core/auth/PolarisSecretsManager.java @@ -55,6 +55,22 @@ PrincipalSecretsResult rotatePrincipalSecrets( boolean reset, @Nonnull String oldSecretHash); + /** + * Reset the secrets of a principal entity. + * + *

This operation makes the specified secrets (either provided by the caller or newly + * generated) the active credentials for the principal. It effectively overwrites any previous + * secrets and sets the provided values as the new client id/secret for the principal. + * + * @param callCtx call context + * @param clientId current principal client id + * @param principalId id of the principal + * @param customClientId optional new client id to assign (may be {@code null} if + * system-generated) + * @param customClientSecret optional new client secret to assign (may be {@code null} if + * system-generated) + * @return the secrets associated with the principal, including the updated client id and secret + */ @Nonnull PrincipalSecretsResult resetPrincipalSecrets( @Nonnull PolarisCallContext callCtx, @@ -63,6 +79,15 @@ PrincipalSecretsResult resetPrincipalSecrets( String customClientId, String customClientSecret); + /** + * Permanently delete the secrets of a principal. + * + *

This operation removes all stored secrets associated with the given principal + * + * @param callCtx call context + * @param clientId principal client id + * @param principalId id of the principal whose secrets should be deleted + */ @Nonnull void deletePrincipalSecrets( @Nonnull PolarisCallContext callCtx, @Nonnull String clientId, long principalId); diff --git a/polaris-core/src/main/java/org/apache/polaris/core/persistence/AtomicOperationMetaStoreManager.java b/polaris-core/src/main/java/org/apache/polaris/core/persistence/AtomicOperationMetaStoreManager.java index 7b17f102b7..b5ef86a45e 100644 --- a/polaris-core/src/main/java/org/apache/polaris/core/persistence/AtomicOperationMetaStoreManager.java +++ b/polaris-core/src/main/java/org/apache/polaris/core/persistence/AtomicOperationMetaStoreManager.java @@ -940,7 +940,7 @@ private void revokeGrantRecord( PolarisPrincipalSecrets secrets = ((IntegrationPersistence) ms) - .resetPrincipalSecrets( + .storePrincipalSecrets( callCtx, clientId, principalId, customClientId, customClientSecret); return (secrets == null) ? new PrincipalSecretsResult(BaseResult.ReturnStatus.ENTITY_NOT_FOUND, null) diff --git a/polaris-core/src/main/java/org/apache/polaris/core/persistence/IntegrationPersistence.java b/polaris-core/src/main/java/org/apache/polaris/core/persistence/IntegrationPersistence.java index e0c0813ca2..b5dacdc358 100644 --- a/polaris-core/src/main/java/org/apache/polaris/core/persistence/IntegrationPersistence.java +++ b/polaris-core/src/main/java/org/apache/polaris/core/persistence/IntegrationPersistence.java @@ -82,17 +82,19 @@ PolarisPrincipalSecrets rotatePrincipalSecrets( @Nonnull String oldSecretHash); /** - * Reset the secrets of a principal entity, i.e. make the specified secrets as main and secondary - * and assign a new client id + * Store the secrets of a principal entity. + * + *

This method creates and persists new credentials for the given client ID. The credentials + * are expected not to already exist for the given client ID. * * @param callCtx call context - * @param clientId principal client id - * @param principalId principal id - * @param customClientId the principal secret's old main secret hash - * @param customClientSecret the principal secret's old main secret hash + * @param clientId the principal client id + * @param principalId the principal id + * @param customClientSecret the secret for the principal + * @return the stored principal secrets */ @Nullable - PolarisPrincipalSecrets resetPrincipalSecrets( + PolarisPrincipalSecrets storePrincipalSecrets( @Nonnull PolarisCallContext callCtx, @Nonnull String clientId, long principalId, diff --git a/polaris-core/src/main/java/org/apache/polaris/core/persistence/transactional/TransactionalMetaStoreManagerImpl.java b/polaris-core/src/main/java/org/apache/polaris/core/persistence/transactional/TransactionalMetaStoreManagerImpl.java index 587aa780d4..3d3b7910f5 100644 --- a/polaris-core/src/main/java/org/apache/polaris/core/persistence/transactional/TransactionalMetaStoreManagerImpl.java +++ b/polaris-core/src/main/java/org/apache/polaris/core/persistence/transactional/TransactionalMetaStoreManagerImpl.java @@ -996,7 +996,7 @@ private void bootstrapPolarisService( String customClientId, String customClientSecret) { PolarisPrincipalSecrets secrets = - ms.resetPrincipalSecrets( + ms.storePrincipalSecrets( callCtx, clientId, principalId, customClientId, customClientSecret); return secrets; } @@ -1013,13 +1013,9 @@ private void bootstrapPolarisService( // if not found, the principal must have been dropped EntityResult loadEntityResult = loadEntity( - callCtx, - ms, - PolarisEntityConstants.getNullId(), - principalId, - PolarisEntityType.PRINCIPAL.getCode()); + callCtx, PolarisEntityConstants.getNullId(), principalId, PolarisEntityType.PRINCIPAL); if (loadEntityResult.getReturnStatus() != BaseResult.ReturnStatus.SUCCESS) { - return null; + return new PrincipalSecretsResult(BaseResult.ReturnStatus.ENTITY_NOT_FOUND, null); } // need to run inside a read/write transaction diff --git a/polaris-core/src/main/java/org/apache/polaris/core/persistence/transactional/TreeMapTransactionalPersistenceImpl.java b/polaris-core/src/main/java/org/apache/polaris/core/persistence/transactional/TreeMapTransactionalPersistenceImpl.java index dff5345a00..74ff870cf6 100644 --- a/polaris-core/src/main/java/org/apache/polaris/core/persistence/transactional/TreeMapTransactionalPersistenceImpl.java +++ b/polaris-core/src/main/java/org/apache/polaris/core/persistence/transactional/TreeMapTransactionalPersistenceImpl.java @@ -486,7 +486,7 @@ public int lookupEntityGrantRecordsVersionInCurrentTxn( } @Override - public @Nonnull PolarisPrincipalSecrets resetPrincipalSecrets( + public @Nonnull PolarisPrincipalSecrets storePrincipalSecrets( @Nonnull PolarisCallContext callCtx, @Nonnull String clientId, long principalId, From 1b3ca195444606955ba3f7d5d440fd19c5c42164 Mon Sep 17 00:00:00 2001 From: "arun.suri" Date: Mon, 25 Aug 2025 18:40:32 +0530 Subject: [PATCH 27/29] handle comments --- .../PolarisEclipseLinkMetaStoreSessionImpl.java | 3 +-- .../jdbc/JdbcBasePersistenceImpl.java | 17 +++++------------ .../core/auth/PolarisSecretsManager.java | 8 +++----- .../core/entity/PolarisPrincipalSecrets.java | 3 +-- .../AtomicOperationMetaStoreManager.java | 6 ++---- .../persistence/IntegrationPersistence.java | 5 ++--- .../TransactionWorkspaceMetaStoreManager.java | 3 +-- .../TransactionalMetaStoreManagerImpl.java | 13 ++++++++----- .../TreeMapTransactionalPersistenceImpl.java | 13 ++++--------- .../service/admin/PolarisAdminService.java | 6 ++++-- 10 files changed, 31 insertions(+), 46 deletions(-) diff --git a/persistence/eclipselink/src/main/java/org/apache/polaris/extension/persistence/impl/eclipselink/PolarisEclipseLinkMetaStoreSessionImpl.java b/persistence/eclipselink/src/main/java/org/apache/polaris/extension/persistence/impl/eclipselink/PolarisEclipseLinkMetaStoreSessionImpl.java index 392c8d8877..ccaf16cf0c 100644 --- a/persistence/eclipselink/src/main/java/org/apache/polaris/extension/persistence/impl/eclipselink/PolarisEclipseLinkMetaStoreSessionImpl.java +++ b/persistence/eclipselink/src/main/java/org/apache/polaris/extension/persistence/impl/eclipselink/PolarisEclipseLinkMetaStoreSessionImpl.java @@ -752,9 +752,8 @@ Optional> hasOverlappingSiblings( @Override public PolarisPrincipalSecrets storePrincipalSecrets( @Nonnull PolarisCallContext callCtx, - @Nonnull String clientId, long principalId, - String customClientId, + @Nonnull String resolvedClientId, String customClientSecret) { throw new UnsupportedOperationException( "This method is not supported for EclipseLink as metastore"); diff --git a/persistence/relational-jdbc/src/main/java/org/apache/polaris/persistence/relational/jdbc/JdbcBasePersistenceImpl.java b/persistence/relational-jdbc/src/main/java/org/apache/polaris/persistence/relational/jdbc/JdbcBasePersistenceImpl.java index 90cc12e225..e52c143b61 100644 --- a/persistence/relational-jdbc/src/main/java/org/apache/polaris/persistence/relational/jdbc/JdbcBasePersistenceImpl.java +++ b/persistence/relational-jdbc/src/main/java/org/apache/polaris/persistence/relational/jdbc/JdbcBasePersistenceImpl.java @@ -777,18 +777,11 @@ public PolarisPrincipalSecrets generateNewPrincipalSecrets( @Override public PolarisPrincipalSecrets storePrincipalSecrets( @Nonnull PolarisCallContext callCtx, - @Nonnull String clientId, long principalId, - String customClientId, + @Nonnull String resolvedClientId, String customClientSecret) { - - PolarisPrincipalSecrets principalSecrets; - if (customClientId == null) { - principalSecrets = new PolarisPrincipalSecrets(principalId, clientId, customClientSecret); - } else { - principalSecrets = - new PolarisPrincipalSecrets(principalId, customClientId, customClientSecret); - } + PolarisPrincipalSecrets principalSecrets = + new PolarisPrincipalSecrets(principalId, resolvedClientId, customClientSecret); try { ModelPrincipalAuthenticationData modelPrincipalAuthenticationData = ModelPrincipalAuthenticationData.fromPrincipalAuthenticationData(principalSecrets); @@ -805,11 +798,11 @@ public PolarisPrincipalSecrets storePrincipalSecrets( } catch (SQLException e) { LOGGER.error( "Failed to reset PrincipalSecrets for clientId: {}, due to {}", - clientId, + resolvedClientId, e.getMessage(), e); throw new RuntimeException( - String.format("Failed to reset PrincipalSecrets for clientId: %s", clientId), e); + String.format("Failed to reset PrincipalSecrets for clientId: %s", resolvedClientId), e); } // return those diff --git a/polaris-core/src/main/java/org/apache/polaris/core/auth/PolarisSecretsManager.java b/polaris-core/src/main/java/org/apache/polaris/core/auth/PolarisSecretsManager.java index 1b9029d243..b60c318e13 100644 --- a/polaris-core/src/main/java/org/apache/polaris/core/auth/PolarisSecretsManager.java +++ b/polaris-core/src/main/java/org/apache/polaris/core/auth/PolarisSecretsManager.java @@ -21,6 +21,7 @@ import jakarta.annotation.Nonnull; import org.apache.polaris.core.PolarisCallContext; import org.apache.polaris.core.persistence.dao.entity.PrincipalSecretsResult; +import software.amazon.awssdk.annotations.NotNull; /** Manages secrets for Polaris principals. */ public interface PolarisSecretsManager { @@ -63,10 +64,8 @@ PrincipalSecretsResult rotatePrincipalSecrets( * secrets and sets the provided values as the new client id/secret for the principal. * * @param callCtx call context - * @param clientId current principal client id * @param principalId id of the principal - * @param customClientId optional new client id to assign (may be {@code null} if - * system-generated) + * @param resolvedClientId current principal client id * @param customClientSecret optional new client secret to assign (may be {@code null} if * system-generated) * @return the secrets associated with the principal, including the updated client id and secret @@ -74,9 +73,8 @@ PrincipalSecretsResult rotatePrincipalSecrets( @Nonnull PrincipalSecretsResult resetPrincipalSecrets( @Nonnull PolarisCallContext callCtx, - @Nonnull String clientId, long principalId, - String customClientId, + @NotNull String resolvedClientId, String customClientSecret); /** diff --git a/polaris-core/src/main/java/org/apache/polaris/core/entity/PolarisPrincipalSecrets.java b/polaris-core/src/main/java/org/apache/polaris/core/entity/PolarisPrincipalSecrets.java index d3ab4a4310..61d48e8875 100644 --- a/polaris-core/src/main/java/org/apache/polaris/core/entity/PolarisPrincipalSecrets.java +++ b/polaris-core/src/main/java/org/apache/polaris/core/entity/PolarisPrincipalSecrets.java @@ -139,8 +139,7 @@ public PolarisPrincipalSecrets(long principalId) { this.secondarySecretHash = hashSecret(secondarySecret); } - public PolarisPrincipalSecrets( - long principalId, @Nullable String newClientId, @Nullable String newSecret) { + public PolarisPrincipalSecrets(long principalId, String newClientId, @Nullable String newSecret) { this.principalId = principalId; this.principalClientId = newClientId; this.mainSecret = (newSecret != null) ? newSecret : this.generateRandomHexString(32); diff --git a/polaris-core/src/main/java/org/apache/polaris/core/persistence/AtomicOperationMetaStoreManager.java b/polaris-core/src/main/java/org/apache/polaris/core/persistence/AtomicOperationMetaStoreManager.java index b5ef86a45e..68d3035728 100644 --- a/polaris-core/src/main/java/org/apache/polaris/core/persistence/AtomicOperationMetaStoreManager.java +++ b/polaris-core/src/main/java/org/apache/polaris/core/persistence/AtomicOperationMetaStoreManager.java @@ -924,9 +924,8 @@ private void revokeGrantRecord( @Override public @Nonnull PrincipalSecretsResult resetPrincipalSecrets( @Nonnull PolarisCallContext callCtx, - @Nonnull String clientId, long principalId, - String customClientId, + @Nonnull String resolvedClientId, String customClientSecret) { // get metastore we should be using BasePersistence ms = callCtx.getMetaStore(); @@ -940,8 +939,7 @@ private void revokeGrantRecord( PolarisPrincipalSecrets secrets = ((IntegrationPersistence) ms) - .storePrincipalSecrets( - callCtx, clientId, principalId, customClientId, customClientSecret); + .storePrincipalSecrets(callCtx, principalId, resolvedClientId, customClientSecret); return (secrets == null) ? new PrincipalSecretsResult(BaseResult.ReturnStatus.ENTITY_NOT_FOUND, null) : new PrincipalSecretsResult(secrets); diff --git a/polaris-core/src/main/java/org/apache/polaris/core/persistence/IntegrationPersistence.java b/polaris-core/src/main/java/org/apache/polaris/core/persistence/IntegrationPersistence.java index b5dacdc358..0d9eae6134 100644 --- a/polaris-core/src/main/java/org/apache/polaris/core/persistence/IntegrationPersistence.java +++ b/polaris-core/src/main/java/org/apache/polaris/core/persistence/IntegrationPersistence.java @@ -88,17 +88,16 @@ PolarisPrincipalSecrets rotatePrincipalSecrets( * are expected not to already exist for the given client ID. * * @param callCtx call context - * @param clientId the principal client id * @param principalId the principal id + * @param resolvedClientId * @param customClientSecret the secret for the principal * @return the stored principal secrets */ @Nullable PolarisPrincipalSecrets storePrincipalSecrets( @Nonnull PolarisCallContext callCtx, - @Nonnull String clientId, long principalId, - String customClientId, + @Nonnull String resolvedClientId, String customClientSecret); /** diff --git a/polaris-core/src/main/java/org/apache/polaris/core/persistence/TransactionWorkspaceMetaStoreManager.java b/polaris-core/src/main/java/org/apache/polaris/core/persistence/TransactionWorkspaceMetaStoreManager.java index 5b9aae95c1..3a9f6e1e69 100644 --- a/polaris-core/src/main/java/org/apache/polaris/core/persistence/TransactionWorkspaceMetaStoreManager.java +++ b/polaris-core/src/main/java/org/apache/polaris/core/persistence/TransactionWorkspaceMetaStoreManager.java @@ -181,9 +181,8 @@ public PrincipalSecretsResult rotatePrincipalSecrets( @Override public PrincipalSecretsResult resetPrincipalSecrets( @Nonnull PolarisCallContext callCtx, - @Nonnull String clientId, long principalId, - String customClientId, + @Nonnull String resolvedClientId, String customClientSecret) { callCtx .getDiagServices() diff --git a/polaris-core/src/main/java/org/apache/polaris/core/persistence/transactional/TransactionalMetaStoreManagerImpl.java b/polaris-core/src/main/java/org/apache/polaris/core/persistence/transactional/TransactionalMetaStoreManagerImpl.java index 3d3b7910f5..d313705ff8 100644 --- a/polaris-core/src/main/java/org/apache/polaris/core/persistence/transactional/TransactionalMetaStoreManagerImpl.java +++ b/polaris-core/src/main/java/org/apache/polaris/core/persistence/transactional/TransactionalMetaStoreManagerImpl.java @@ -996,17 +996,15 @@ private void bootstrapPolarisService( String customClientId, String customClientSecret) { PolarisPrincipalSecrets secrets = - ms.storePrincipalSecrets( - callCtx, clientId, principalId, customClientId, customClientSecret); + ms.storePrincipalSecrets(callCtx, principalId, customClientId, customClientSecret); return secrets; } @Override public @Nonnull PrincipalSecretsResult resetPrincipalSecrets( @Nonnull PolarisCallContext callCtx, - @Nonnull String clientId, long principalId, - String customClientId, + @Nonnull String resolvedClientId, String customClientSecret) { // get metastore we should be using TransactionalPersistence ms = ((TransactionalPersistence) callCtx.getMetaStore()); @@ -1024,7 +1022,12 @@ private void bootstrapPolarisService( callCtx, () -> this.resetPrincipalSecrets( - callCtx, ms, clientId, principalId, customClientId, customClientSecret)); + callCtx, + ms, + resolvedClientId, + principalId, + resolvedClientId, + customClientSecret)); return (secrets == null) ? new PrincipalSecretsResult(BaseResult.ReturnStatus.ENTITY_NOT_FOUND, null) diff --git a/polaris-core/src/main/java/org/apache/polaris/core/persistence/transactional/TreeMapTransactionalPersistenceImpl.java b/polaris-core/src/main/java/org/apache/polaris/core/persistence/transactional/TreeMapTransactionalPersistenceImpl.java index 74ff870cf6..a4bf1d9461 100644 --- a/polaris-core/src/main/java/org/apache/polaris/core/persistence/transactional/TreeMapTransactionalPersistenceImpl.java +++ b/polaris-core/src/main/java/org/apache/polaris/core/persistence/transactional/TreeMapTransactionalPersistenceImpl.java @@ -488,17 +488,12 @@ public int lookupEntityGrantRecordsVersionInCurrentTxn( @Override public @Nonnull PolarisPrincipalSecrets storePrincipalSecrets( @Nonnull PolarisCallContext callCtx, - @Nonnull String clientId, long principalId, - String customClientId, + @Nonnull String resolvedClientId, String customClientSecret) { - PolarisPrincipalSecrets principalSecrets; - if (customClientId == null) { - principalSecrets = new PolarisPrincipalSecrets(principalId, clientId, customClientSecret); - } else { - principalSecrets = - new PolarisPrincipalSecrets(principalId, customClientId, customClientSecret); - } + PolarisPrincipalSecrets principalSecrets = + new PolarisPrincipalSecrets(principalId, resolvedClientId, customClientSecret); + // write back new secrets this.store.getSlicePrincipalSecrets().write(principalSecrets); 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 fb4a8c38de..59f8bb15a1 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 @@ -1102,14 +1102,16 @@ public void deletePrincipal(String name) { "Concurrent modification on Principal '%s'; retry later", principalName)); } + String resolvedClientId = + (customClientId != null) ? customClientId : currentPrincipalEntity.getClientId(); + // generate new secrets PolarisPrincipalSecrets newSecrets = metaStoreManager .resetPrincipalSecrets( getCurrentPolarisContext(), - currentPrincipalEntity.getClientId(), currentPrincipalEntity.getId(), - customClientId, + resolvedClientId, customClientSecret) .getPrincipalSecrets(); From e6a040749538f6919fa531935f636e7715a38574 Mon Sep 17 00:00:00 2001 From: "arun.suri" Date: Thu, 28 Aug 2025 00:30:40 +0530 Subject: [PATCH 28/29] add a FF --- .../apache/polaris/core/config/FeatureConfiguration.java | 9 +++++++++ .../defaults/src/main/resources/application.properties | 1 + .../polaris/service/admin/PolarisAdminService.java | 2 ++ 3 files changed, 12 insertions(+) diff --git a/polaris-core/src/main/java/org/apache/polaris/core/config/FeatureConfiguration.java b/polaris-core/src/main/java/org/apache/polaris/core/config/FeatureConfiguration.java index 9eba940a95..ad91ad0240 100644 --- a/polaris-core/src/main/java/org/apache/polaris/core/config/FeatureConfiguration.java +++ b/polaris-core/src/main/java/org/apache/polaris/core/config/FeatureConfiguration.java @@ -367,4 +367,13 @@ public static void enforceFeatureEnabledOrThrow( + "it is still possible to enforce the uniqueness of table locations within a catalog.") .defaultValue(false) .buildFeatureConfiguration(); + + public static final FeatureConfiguration ENABLE_CREDENTIAL_RESET = + PolarisConfiguration.builder() + .key("ENABLE_CREDENTIAL_RESET") + .description( + "Flag to enable or disable the API to reset principal credentials. " + + "Defaults to enabled, but service providers may want to disable it.") + .defaultValue(true) + .buildFeatureConfiguration(); } diff --git a/runtime/defaults/src/main/resources/application.properties b/runtime/defaults/src/main/resources/application.properties index 794ea13375..cac4e78a66 100644 --- a/runtime/defaults/src/main/resources/application.properties +++ b/runtime/defaults/src/main/resources/application.properties @@ -116,6 +116,7 @@ polaris.features."SUPPORTED_CATALOG_STORAGE_TYPES"=["S3","GCS","AZURE"] # polaris.features."ENABLE_CATALOG_FEDERATION"=true polaris.features."SUPPORTED_CATALOG_CONNECTION_TYPES"=["ICEBERG_REST"] polaris.features."SUPPORTED_EXTERNAL_CATALOG_AUTHENTICATION_TYPES"=["OAUTH", "BEARER"] +polaris.features."ENABLE_CREDENTIAL_RESET"=true # realm overrides # polaris.features.realm-overrides."my-realm"."SKIP_CREDENTIAL_SUBSCOPING_INDIRECTION"=true 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 59f8bb15a1..4a7737711a 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 @@ -1181,6 +1181,8 @@ public void deletePrincipal(String name) { public @Nonnull PrincipalWithCredentials resetCredentials( String principalName, ResetPrincipalRequest resetPrincipalRequest) { + FeatureConfiguration.enforceFeatureEnabledOrThrow( + realmConfig, FeatureConfiguration.ENABLE_CREDENTIAL_RESET); PolarisAuthorizableOperation op = PolarisAuthorizableOperation.RESET_CREDENTIALS; authorizeBasicTopLevelEntityOperationOrThrow(op, principalName, PolarisEntityType.PRINCIPAL); var customClientId = resetPrincipalRequest.getClientId(); From c7501f1863ca65586edd8a42c4e2c948239f0077 Mon Sep 17 00:00:00 2001 From: "arun.suri" Date: Thu, 28 Aug 2025 11:25:28 +0530 Subject: [PATCH 29/29] rebase --- CHANGELOG.md | 2 ++ runtime/defaults/src/main/resources/application.properties | 1 - 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index f0ffb54e07..9722a6355d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -88,6 +88,8 @@ automatic storage credential refresh per table on the client side. Java client v The endpoint path is always returned when using vended credentials, but clients must enable the refresh-credentials flag for the desired storage provider. +- Added a Management API endpoint to reset principal credentials, controlled by the `ENABLE_CREDENTIAL_RESET` (default: true) feature flag. + ### Changes - Polaris Management API clients must be prepared to deal with new attributes in `AwsStorageConfigInfo` objects. diff --git a/runtime/defaults/src/main/resources/application.properties b/runtime/defaults/src/main/resources/application.properties index cac4e78a66..794ea13375 100644 --- a/runtime/defaults/src/main/resources/application.properties +++ b/runtime/defaults/src/main/resources/application.properties @@ -116,7 +116,6 @@ polaris.features."SUPPORTED_CATALOG_STORAGE_TYPES"=["S3","GCS","AZURE"] # polaris.features."ENABLE_CATALOG_FEDERATION"=true polaris.features."SUPPORTED_CATALOG_CONNECTION_TYPES"=["ICEBERG_REST"] polaris.features."SUPPORTED_EXTERNAL_CATALOG_AUTHENTICATION_TYPES"=["OAUTH", "BEARER"] -polaris.features."ENABLE_CREDENTIAL_RESET"=true # realm overrides # polaris.features.realm-overrides."my-realm"."SKIP_CREDENTIAL_SUBSCOPING_INDIRECTION"=true