diff --git a/extension/persistence/eclipselink/src/main/java/org/apache/polaris/extension/persistence/impl/eclipselink/PolarisEclipseLinkMetaStoreSessionImpl.java b/extension/persistence/eclipselink/src/main/java/org/apache/polaris/extension/persistence/impl/eclipselink/PolarisEclipseLinkMetaStoreSessionImpl.java index 071d7fbc3..534d32a0b 100644 --- a/extension/persistence/eclipselink/src/main/java/org/apache/polaris/extension/persistence/impl/eclipselink/PolarisEclipseLinkMetaStoreSessionImpl.java +++ b/extension/persistence/eclipselink/src/main/java/org/apache/polaris/extension/persistence/impl/eclipselink/PolarisEclipseLinkMetaStoreSessionImpl.java @@ -672,8 +672,8 @@ public int lookupEntityGrantRecordsVersion( @NotNull PolarisCallContext callCtx, @NotNull String clientId, long principalId, - @NotNull String mainSecretToRotate, - boolean reset) { + boolean reset, + @NotNull String oldSecretHash) { // load the existing secrets PolarisPrincipalSecrets principalSecrets = @@ -701,9 +701,9 @@ public int lookupEntityGrantRecordsVersion( principalSecrets.getPrincipalId()); // rotate the secrets - principalSecrets.rotateSecrets(mainSecretToRotate); + principalSecrets.rotateSecrets(oldSecretHash); if (reset) { - principalSecrets.rotateSecrets(principalSecrets.getMainSecret()); + principalSecrets.rotateSecrets(principalSecrets.getMainSecretHash()); } // write back new secrets diff --git a/extension/persistence/eclipselink/src/test/java/org/apache/polaris/extension/persistence/impl/eclipselink/PolarisEclipseLinkMetaStoreManagerTest.java b/extension/persistence/eclipselink/src/test/java/org/apache/polaris/extension/persistence/impl/eclipselink/PolarisEclipseLinkMetaStoreManagerTest.java index 000a909e9..be2ff5bca 100644 --- a/extension/persistence/eclipselink/src/test/java/org/apache/polaris/extension/persistence/impl/eclipselink/PolarisEclipseLinkMetaStoreManagerTest.java +++ b/extension/persistence/eclipselink/src/test/java/org/apache/polaris/extension/persistence/impl/eclipselink/PolarisEclipseLinkMetaStoreManagerTest.java @@ -18,19 +18,25 @@ */ package org.apache.polaris.extension.persistence.impl.eclipselink; +import static jakarta.persistence.Persistence.createEntityManagerFactory; import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertNotNull; import static org.junit.jupiter.api.Assertions.assertTrue; import java.time.ZoneId; +import java.util.UUID; import java.util.stream.Stream; import org.apache.polaris.core.PolarisCallContext; import org.apache.polaris.core.PolarisConfigurationStore; import org.apache.polaris.core.PolarisDefaultDiagServiceImpl; import org.apache.polaris.core.PolarisDiagnostics; +import org.apache.polaris.core.entity.PolarisPrincipalSecrets; import org.apache.polaris.core.persistence.BasePolarisMetaStoreManagerTest; import org.apache.polaris.core.persistence.PolarisMetaStoreManagerImpl; import org.apache.polaris.core.persistence.PolarisTestMetaStoreManager; +import org.apache.polaris.core.persistence.models.ModelPrincipalSecrets; +import org.junit.jupiter.api.Assertions; +import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.ExtensionContext; import org.junit.jupiter.params.ParameterizedTest; import org.junit.jupiter.params.provider.Arguments; @@ -80,6 +86,77 @@ void testCreateStoreSession(String confFile, boolean success) { } } + @Test + void testRotateLegacyPrincipalSecret() { + + PolarisEclipseLinkMetaStoreSessionImpl.clearEntityManagerFactories(); + PolarisDiagnostics diagServices = new PolarisDefaultDiagServiceImpl(); + + var key = "client-id-" + UUID.randomUUID(); + ModelPrincipalSecrets model = + ModelPrincipalSecrets.builder() + .principalId(Math.abs(key.hashCode())) + .principalClientId(key) + .mainSecret("secret!") + .build(); + Assertions.assertNotNull(model.getMainSecret()); + Assertions.assertNull(model.getMainSecretHash()); + + try (var emf = createEntityManagerFactory("polaris")) { + var entityManager = emf.createEntityManager(); + + // Persist the original model: + entityManager.getTransaction().begin(); + entityManager.persist(model); + entityManager.getTransaction().commit(); + + // Retrieve the model + entityManager.clear(); + ModelPrincipalSecrets retrievedModel = entityManager.find(ModelPrincipalSecrets.class, key); + + // Verify the retrieved entity still has no hash + Assertions.assertNotNull(retrievedModel); + Assertions.assertNotNull(retrievedModel.getMainSecret()); + Assertions.assertNull(retrievedModel.getMainSecretHash()); + + // Now read using PolarisEclipseLinkStore + PolarisEclipseLinkStore store = new PolarisEclipseLinkStore(diagServices); + store.initialize(entityManager); + PolarisPrincipalSecrets principalSecrets = + ModelPrincipalSecrets.toPrincipalSecrets( + store.lookupPrincipalSecrets(entityManager, key)); + + // The principalSecrets should have both a main secret and a hashed secret + Assertions.assertNotNull(principalSecrets); + Assertions.assertNotNull(principalSecrets.getMainSecret()); + Assertions.assertNotNull(principalSecrets.getMainSecretHash()); + Assertions.assertNull(principalSecrets.getSecondarySecret()); + + // Rotate: + String originalSecret = principalSecrets.getMainSecret(); + String originalHash = principalSecrets.getMainSecretHash(); + principalSecrets.rotateSecrets(principalSecrets.getMainSecretHash()); + Assertions.assertNotEquals(originalHash, principalSecrets.getMainSecretHash()); + Assertions.assertEquals(originalHash, principalSecrets.getSecondarySecretHash()); + Assertions.assertEquals(null, principalSecrets.getSecondarySecret()); + + // Persist the rotated credential: + store.deletePrincipalSecrets(entityManager, key); + store.writePrincipalSecrets(entityManager, principalSecrets); + + // Reload the model: + var reloadedModel = store.lookupPrincipalSecrets(entityManager, key); + + // The old plaintext secret is gone: + Assertions.assertNull(reloadedModel.getMainSecret()); + Assertions.assertNull(reloadedModel.getSecondarySecret()); + + // Confirm the old secret still works via hash: + var reloadedSecrets = ModelPrincipalSecrets.toPrincipalSecrets(reloadedModel); + Assertions.assertTrue(reloadedSecrets.matchesSecret(originalSecret)); + } + } + private static class CreateStoreSessionArgs implements ArgumentsProvider { @Override public Stream provideArguments(ExtensionContext extensionContext) { 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 195c668f6..dc98d46f6 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 @@ -21,6 +21,7 @@ import com.fasterxml.jackson.annotation.JsonCreator; import com.fasterxml.jackson.annotation.JsonProperty; import java.security.SecureRandom; +import org.apache.commons.codec.digest.DigestUtils; /** * Simple class to represent the secrets used to authenticate a catalog principal, These secrets are @@ -37,12 +38,20 @@ public class PolarisPrincipalSecrets { // the client id for that principal private final String principalClientId; - // the main secret for that principal + // the main secret hash for that principal private String mainSecret; // the secondary secret for that principal private String secondarySecret; + // Hash of mainSecret + private String mainSecretHash; + + // Hash of secondarySecret + private String secondarySecretHash; + + private String secretSalt; + /** * Generate a secure random string * @@ -64,16 +73,48 @@ private String generateRandomHexString(int stringLength) { return sb.toString(); } + private String hashSecret(String secret) { + return DigestUtils.sha256Hex(secret + ":" + secretSalt); + } + @JsonCreator public PolarisPrincipalSecrets( @JsonProperty("principalId") long principalId, @JsonProperty("principalClientId") String principalClientId, @JsonProperty("mainSecret") String mainSecret, - @JsonProperty("secondarySecret") String secondarySecret) { + @JsonProperty("secondarySecret") String secondarySecret, + @JsonProperty("secretSalt") String secretSalt, + @JsonProperty("mainSecretHash") String mainSecretHash, + @JsonProperty("secondarySecretHash") String secondarySecretHash) { + this.principalId = principalId; + this.principalClientId = principalClientId; + this.mainSecret = mainSecret; + this.secondarySecret = secondarySecret; + + this.secretSalt = secretSalt; + if (this.secretSalt == null) { + this.secretSalt = generateRandomHexString(16); + } + this.mainSecretHash = mainSecretHash; + if (this.mainSecretHash == null) { + this.mainSecretHash = hashSecret(mainSecret); + } + this.secondarySecretHash = secondarySecretHash; + if (this.secondarySecretHash == null) { + this.secondarySecretHash = hashSecret(secondarySecret); + } + } + + public PolarisPrincipalSecrets( + long principalId, String principalClientId, String mainSecret, String secondarySecret) { this.principalId = principalId; this.principalClientId = principalClientId; this.mainSecret = mainSecret; this.secondarySecret = secondarySecret; + + this.secretSalt = generateRandomHexString(16); + this.mainSecretHash = hashSecret(mainSecret); + this.secondarySecretHash = hashSecret(secondarySecret); } public PolarisPrincipalSecrets(PolarisPrincipalSecrets principalSecrets) { @@ -81,6 +122,9 @@ public PolarisPrincipalSecrets(PolarisPrincipalSecrets principalSecrets) { this.principalClientId = principalSecrets.getPrincipalClientId(); this.mainSecret = principalSecrets.getMainSecret(); this.secondarySecret = principalSecrets.getSecondarySecret(); + this.secretSalt = principalSecrets.getSecretSalt(); + this.mainSecretHash = principalSecrets.getMainSecretHash(); + this.secondarySecretHash = principalSecrets.getSecondarySecretHash(); } public PolarisPrincipalSecrets(long principalId) { @@ -88,16 +132,19 @@ public PolarisPrincipalSecrets(long principalId) { this.principalClientId = this.generateRandomHexString(16); this.mainSecret = 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 - * - * @param mainSecretToRotate the main secrets to rotate - */ - public void rotateSecrets(String mainSecretToRotate) { - this.secondarySecret = mainSecretToRotate; + /** Rotate the main secrets */ + public void rotateSecrets(String newSecondaryHash) { + this.secondarySecret = null; + this.secondarySecretHash = newSecondaryHash; + this.mainSecret = this.generateRandomHexString(32); + this.mainSecretHash = hashSecret(mainSecret); } public long getPrincipalId() { @@ -108,6 +155,12 @@ public String getPrincipalClientId() { return principalClientId; } + public boolean matchesSecret(String potentialSecret) { + String potentialSecretHash = hashSecret(potentialSecret); + return potentialSecretHash.equals(this.mainSecretHash) + || potentialSecretHash.equals(this.secondarySecretHash); + } + public String getMainSecret() { return mainSecret; } @@ -115,4 +168,16 @@ public String getMainSecret() { public String getSecondarySecret() { return secondarySecret; } + + public String getMainSecretHash() { + return mainSecretHash; + } + + public String getSecondarySecretHash() { + return secondarySecretHash; + } + + public String getSecretSalt() { + return secretSalt; + } } diff --git a/polaris-core/src/main/java/org/apache/polaris/core/persistence/LocalPolarisMetaStoreManagerFactory.java b/polaris-core/src/main/java/org/apache/polaris/core/persistence/LocalPolarisMetaStoreManagerFactory.java index 4453e4f14..095853cff 100644 --- a/polaris-core/src/main/java/org/apache/polaris/core/persistence/LocalPolarisMetaStoreManagerFactory.java +++ b/polaris-core/src/main/java/org/apache/polaris/core/persistence/LocalPolarisMetaStoreManagerFactory.java @@ -204,8 +204,8 @@ public void setStorageIntegrationProvider(PolarisStorageIntegrationProvider stor polarisContext, secrets.getPrincipalClientId(), secrets.getPrincipalId(), - secrets.getMainSecret(), - false); + false, + secrets.getMainSecretHash()); return rotatedSecrets; } diff --git a/polaris-core/src/main/java/org/apache/polaris/core/persistence/PolarisMetaStoreManager.java b/polaris-core/src/main/java/org/apache/polaris/core/persistence/PolarisMetaStoreManager.java index 9033795d8..697cb69e5 100644 --- a/polaris-core/src/main/java/org/apache/polaris/core/persistence/PolarisMetaStoreManager.java +++ b/polaris-core/src/main/java/org/apache/polaris/core/persistence/PolarisMetaStoreManager.java @@ -547,10 +547,10 @@ PrincipalSecretsResult loadPrincipalSecrets( * @param callCtx call context * @param clientId principal client id * @param principalId id of the principal - * @param mainSecret main secret for the principal * @param reset true if the principal's secrets should be disabled and replaced with a one-time * password. if the principal's secret is already a one-time password, this flag is * automatically true + * @param oldSecretHash main secret hash for the principal * @return the secrets associated to that principal amd the id of the principal */ @NotNull @@ -558,8 +558,8 @@ PrincipalSecretsResult rotatePrincipalSecrets( @NotNull PolarisCallContext callCtx, @NotNull String clientId, long principalId, - @NotNull String mainSecret, - boolean reset); + boolean reset, + @NotNull String oldSecretHash); /** the return the result of a create-catalog method */ class CreateCatalogResult extends BaseResult { diff --git a/polaris-core/src/main/java/org/apache/polaris/core/persistence/PolarisMetaStoreManagerImpl.java b/polaris-core/src/main/java/org/apache/polaris/core/persistence/PolarisMetaStoreManagerImpl.java index b05129fa3..c3fc3a81e 100644 --- a/polaris-core/src/main/java/org/apache/polaris/core/persistence/PolarisMetaStoreManagerImpl.java +++ b/polaris-core/src/main/java/org/apache/polaris/core/persistence/PolarisMetaStoreManagerImpl.java @@ -954,7 +954,7 @@ public Map deserializeProperties(PolarisCallContext callCtx, Str return new CreatePrincipalResult(ReturnStatus.ENTITY_ALREADY_EXISTS, null); } - // generate new secretes for this principal + // generate new secrets for this principal PolarisPrincipalSecrets principalSecrets = ms.generateNewPrincipalSecrets(callCtx, principal.getName(), principal.getId()); @@ -1012,8 +1012,8 @@ public Map deserializeProperties(PolarisCallContext callCtx, Str @NotNull PolarisMetaStoreSession ms, @NotNull String clientId, long principalId, - @NotNull String masterSecret, - boolean reset) { + boolean reset, + @NotNull String oldSecretHash) { // if not found, the principal must have been dropped EntityResult loadEntityResult = loadEntity(callCtx, ms, PolarisEntityConstants.getNullId(), principalId); @@ -1033,7 +1033,7 @@ public Map deserializeProperties(PolarisCallContext callCtx, Str PolarisEntityConstants.PRINCIPAL_CREDENTIAL_ROTATION_REQUIRED_STATE) != null; PolarisPrincipalSecrets secrets = - ms.rotatePrincipalSecrets(callCtx, clientId, principalId, masterSecret, doReset); + ms.rotatePrincipalSecrets(callCtx, clientId, principalId, doReset, oldSecretHash); if (reset && !internalProps.containsKey( @@ -1061,8 +1061,8 @@ public Map deserializeProperties(PolarisCallContext callCtx, Str @NotNull PolarisCallContext callCtx, @NotNull String clientId, long principalId, - @NotNull String mainSecret, - boolean reset) { + boolean reset, + @NotNull String oldSecretHash) { // get metastore we should be using PolarisMetaStoreSession ms = callCtx.getMetaStore(); @@ -1071,7 +1071,8 @@ public Map deserializeProperties(PolarisCallContext callCtx, Str ms.runInTransaction( callCtx, () -> - this.rotatePrincipalSecrets(callCtx, ms, clientId, principalId, mainSecret, reset)); + this.rotatePrincipalSecrets( + callCtx, ms, clientId, principalId, reset, oldSecretHash)); return (secrets == null) ? new PrincipalSecretsResult(ReturnStatus.ENTITY_NOT_FOUND, null) diff --git a/polaris-core/src/main/java/org/apache/polaris/core/persistence/PolarisMetaStoreSession.java b/polaris-core/src/main/java/org/apache/polaris/core/persistence/PolarisMetaStoreSession.java index ff5bcbf3e..448cb2353 100644 --- a/polaris-core/src/main/java/org/apache/polaris/core/persistence/PolarisMetaStoreSession.java +++ b/polaris-core/src/main/java/org/apache/polaris/core/persistence/PolarisMetaStoreSession.java @@ -444,17 +444,17 @@ PolarisPrincipalSecrets generateNewPrincipalSecrets( * @param callCtx call context * @param clientId principal client id * @param principalId principal id - * @param mainSecretToRotate main secret for comparison with the current entity version * @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 */ @Nullable PolarisPrincipalSecrets rotatePrincipalSecrets( @NotNull PolarisCallContext callCtx, @NotNull String clientId, long principalId, - @NotNull String mainSecretToRotate, - boolean reset); + boolean reset, + @NotNull String oldSecretHash); /** * 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/PolarisTreeMapMetaStoreSessionImpl.java b/polaris-core/src/main/java/org/apache/polaris/core/persistence/PolarisTreeMapMetaStoreSessionImpl.java index 096bce49e..bcf1a97f5 100644 --- a/polaris-core/src/main/java/org/apache/polaris/core/persistence/PolarisTreeMapMetaStoreSessionImpl.java +++ b/polaris-core/src/main/java/org/apache/polaris/core/persistence/PolarisTreeMapMetaStoreSessionImpl.java @@ -474,8 +474,8 @@ public int lookupEntityGrantRecordsVersion( @NotNull PolarisCallContext callCtx, @NotNull String clientId, long principalId, - @NotNull String mainSecretToRotate, - boolean reset) { + boolean reset, + @NotNull String oldSecretHash) { // load the existing secrets PolarisPrincipalSecrets principalSecrets = this.store.getSlicePrincipalSecrets().read(clientId); @@ -501,9 +501,9 @@ public int lookupEntityGrantRecordsVersion( principalSecrets.getPrincipalId()); // rotate the secrets - principalSecrets.rotateSecrets(mainSecretToRotate); + principalSecrets.rotateSecrets(oldSecretHash); if (reset) { - principalSecrets.rotateSecrets(principalSecrets.getMainSecret()); + principalSecrets.rotateSecrets(principalSecrets.getMainSecretHash()); } // write back new 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 4d7d656ad..e2ae84146 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 @@ -134,8 +134,8 @@ public PrincipalSecretsResult rotatePrincipalSecrets( @NotNull PolarisCallContext callCtx, @NotNull String clientId, long principalId, - @NotNull String mainSecret, - boolean reset) { + boolean reset, + @NotNull String oldSecretHash) { callCtx .getDiagServices() .fail("illegal_method_in_transaction_workspace", "rotatePrincipalSecrets"); diff --git a/polaris-core/src/main/java/org/apache/polaris/core/persistence/models/ModelPrincipalSecrets.java b/polaris-core/src/main/java/org/apache/polaris/core/persistence/models/ModelPrincipalSecrets.java index 349d54395..2a4cf7a55 100644 --- a/polaris-core/src/main/java/org/apache/polaris/core/persistence/models/ModelPrincipalSecrets.java +++ b/polaris-core/src/main/java/org/apache/polaris/core/persistence/models/ModelPrincipalSecrets.java @@ -43,6 +43,14 @@ public class ModelPrincipalSecrets { // the secondary secret for that principal private String secondarySecret; + // Hash of mainSecret + private String mainSecretHash; + + // Hash of secondarySecret + private String secondarySecretHash; + + private String secretSalt; + // Used for Optimistic Locking to handle concurrent reads and updates @Version private long version; @@ -62,6 +70,18 @@ public String getSecondarySecret() { return secondarySecret; } + public String getSecretSalt() { + return secretSalt; + } + + public String getMainSecretHash() { + return mainSecretHash; + } + + public String getSecondarySecretHash() { + return secondarySecretHash; + } + public static Builder builder() { return new Builder(); } @@ -93,6 +113,21 @@ public Builder secondarySecret(String secondarySecret) { return this; } + public Builder secretSalt(String secretSalt) { + principalSecrets.secretSalt = secretSalt; + return this; + } + + public Builder mainSecretHash(String mainSecretHash) { + principalSecrets.mainSecretHash = mainSecretHash; + return this; + } + + public Builder secondarySecretHash(String secondarySecretHash) { + principalSecrets.secondarySecretHash = secondarySecretHash; + return this; + } + public ModelPrincipalSecrets build() { return principalSecrets; } @@ -104,8 +139,9 @@ public static ModelPrincipalSecrets fromPrincipalSecrets(PolarisPrincipalSecrets return ModelPrincipalSecrets.builder() .principalId(record.getPrincipalId()) .principalClientId(record.getPrincipalClientId()) - .mainSecret(record.getMainSecret()) - .secondarySecret(record.getSecondarySecret()) + .secretSalt(record.getSecretSalt()) + .mainSecretHash(record.getMainSecretHash()) + .secondarySecretHash(record.getSecondarySecretHash()) .build(); } @@ -116,7 +152,10 @@ public static PolarisPrincipalSecrets toPrincipalSecrets(ModelPrincipalSecrets m model.getPrincipalId(), model.getPrincipalClientId(), model.getMainSecret(), - model.getSecondarySecret()); + model.getSecondarySecret(), + model.getSecretSalt(), + model.getMainSecretHash(), + model.getSecondarySecretHash()); } public void update(PolarisPrincipalSecrets principalSecrets) { @@ -124,7 +163,18 @@ public void update(PolarisPrincipalSecrets principalSecrets) { this.principalId = principalSecrets.getPrincipalId(); this.principalClientId = principalSecrets.getPrincipalClientId(); + this.secretSalt = principalSecrets.getSecretSalt(); this.mainSecret = principalSecrets.getMainSecret(); this.secondarySecret = principalSecrets.getSecondarySecret(); + this.mainSecretHash = principalSecrets.getMainSecretHash(); + this.secondarySecretHash = principalSecrets.getSecondarySecretHash(); + + // Once a hash is stored, do not keep the original secret + if (this.mainSecretHash != null) { + this.mainSecret = null; + } + if (this.secondarySecretHash != null) { + this.secondarySecret = null; + } } } diff --git a/polaris-core/src/testFixtures/java/org/apache/polaris/core/persistence/PolarisTestMetaStoreManager.java b/polaris-core/src/testFixtures/java/org/apache/polaris/core/persistence/PolarisTestMetaStoreManager.java index 893d22927..fe44f38e8 100644 --- a/polaris-core/src/testFixtures/java/org/apache/polaris/core/persistence/PolarisTestMetaStoreManager.java +++ b/polaris-core/src/testFixtures/java/org/apache/polaris/core/persistence/PolarisTestMetaStoreManager.java @@ -402,9 +402,9 @@ PolarisBaseEntity createPrincipal(String name) { Assertions.assertThat(reloadSecrets.getPrincipalId()).isEqualTo(secrets.getPrincipalId()); Assertions.assertThat(reloadSecrets.getPrincipalClientId()) .isEqualTo(secrets.getPrincipalClientId()); - Assertions.assertThat(reloadSecrets.getMainSecret()).isEqualTo(secrets.getMainSecret()); - Assertions.assertThat(reloadSecrets.getSecondarySecret()) - .isEqualTo(secrets.getSecondarySecret()); + Assertions.assertThat(reloadSecrets.getMainSecretHash()).isEqualTo(secrets.getMainSecretHash()); + Assertions.assertThat(reloadSecrets.getSecondarySecretHash()) + .isEqualTo(secrets.getSecondarySecretHash()); Map internalProperties = PolarisObjectMapperUtil.deserializeProperties( @@ -428,8 +428,8 @@ PolarisBaseEntity createPrincipal(String name) { Assertions.assertThat(newSecrets.getPrincipalId()).isEqualTo(secrets.getPrincipalId()); Assertions.assertThat(newSecrets.getPrincipalClientId()) .isEqualTo(secrets.getPrincipalClientId()); - Assertions.assertThat(newSecrets.getMainSecret()).isEqualTo(secrets.getMainSecret()); - Assertions.assertThat(newSecrets.getMainSecret()).isEqualTo(secrets.getMainSecret()); + Assertions.assertThat(newSecrets.getMainSecretHash()).isEqualTo(secrets.getMainSecretHash()); + Assertions.assertThat(newSecrets.getMainSecretHash()).isEqualTo(secrets.getMainSecretHash()); } secrets = @@ -438,8 +438,8 @@ PolarisBaseEntity createPrincipal(String name) { this.polarisCallContext, clientId, principalEntity.getId(), - secrets.getMainSecret(), - false) + false, + secrets.getMainSecretHash()) .getPrincipalSecrets(); Assertions.assertThat(secrets.getMainSecret()).isNotEqualTo(reloadSecrets.getMainSecret()); @@ -457,9 +457,17 @@ PolarisBaseEntity createPrincipal(String name) { // rotate the secrets, twice! polarisMetaStoreManager.rotatePrincipalSecrets( - this.polarisCallContext, clientId, principalEntity.getId(), secrets.getMainSecret(), false); + this.polarisCallContext, + clientId, + principalEntity.getId(), + false, + secrets.getMainSecretHash()); polarisMetaStoreManager.rotatePrincipalSecrets( - this.polarisCallContext, clientId, principalEntity.getId(), secrets.getMainSecret(), false); + this.polarisCallContext, + clientId, + principalEntity.getId(), + false, + secrets.getMainSecretHash()); // reload and check that now the main should be secondary reloadSecrets = @@ -470,16 +478,17 @@ PolarisBaseEntity createPrincipal(String name) { Assertions.assertThat(reloadSecrets.getPrincipalId()).isEqualTo(secrets.getPrincipalId()); Assertions.assertThat(reloadSecrets.getPrincipalClientId()) .isEqualTo(secrets.getPrincipalClientId()); - Assertions.assertThat(reloadSecrets.getSecondarySecret()).isEqualTo(secrets.getMainSecret()); - String newMainSecret = reloadSecrets.getMainSecret(); + Assertions.assertThat(reloadSecrets.getSecondarySecretHash()) + .isEqualTo(secrets.getMainSecretHash()); + String newMainSecretHash = reloadSecrets.getMainSecretHash(); // reset - the previous main secret is no longer one of the secrets polarisMetaStoreManager.rotatePrincipalSecrets( this.polarisCallContext, clientId, principalEntity.getId(), - reloadSecrets.getMainSecret(), - true); + true, + reloadSecrets.getMainSecretHash()); reloadSecrets = polarisMetaStoreManager .loadPrincipalSecrets(this.polarisCallContext, clientId) @@ -488,8 +497,8 @@ PolarisBaseEntity createPrincipal(String name) { Assertions.assertThat(reloadSecrets.getPrincipalId()).isEqualTo(secrets.getPrincipalId()); Assertions.assertThat(reloadSecrets.getPrincipalClientId()) .isEqualTo(secrets.getPrincipalClientId()); - Assertions.assertThat(reloadSecrets.getMainSecret()).isNotEqualTo(newMainSecret); - Assertions.assertThat(reloadSecrets.getSecondarySecret()).isNotEqualTo(newMainSecret); + Assertions.assertThat(reloadSecrets.getMainSecretHash()).isNotEqualTo(newMainSecretHash); + Assertions.assertThat(reloadSecrets.getSecondarySecretHash()).isNotEqualTo(newMainSecretHash); PolarisBaseEntity newPrincipal = polarisMetaStoreManager @@ -509,8 +518,8 @@ PolarisBaseEntity createPrincipal(String name) { this.polarisCallContext, clientId, principalEntity.getId(), - reloadSecrets.getMainSecret(), - true); + true, + reloadSecrets.getMainSecretHash()); PolarisPrincipalSecrets postResetCredentials = polarisMetaStoreManager .loadPrincipalSecrets(this.polarisCallContext, clientId) @@ -520,10 +529,10 @@ PolarisBaseEntity createPrincipal(String name) { .isEqualTo(reloadSecrets.getPrincipalId()); Assertions.assertThat(postResetCredentials.getPrincipalClientId()) .isEqualTo(reloadSecrets.getPrincipalClientId()); - Assertions.assertThat(postResetCredentials.getMainSecret()) - .isNotEqualTo(reloadSecrets.getMainSecret()); - Assertions.assertThat(postResetCredentials.getSecondarySecret()) - .isNotEqualTo(reloadSecrets.getSecondarySecret()); + Assertions.assertThat(postResetCredentials.getMainSecretHash()) + .isNotEqualTo(reloadSecrets.getMainSecretHash()); + Assertions.assertThat(postResetCredentials.getSecondarySecretHash()) + .isNotEqualTo(reloadSecrets.getSecondarySecretHash()); PolarisBaseEntity finalPrincipal = polarisMetaStoreManager diff --git a/polaris-service/src/main/java/org/apache/polaris/service/admin/PolarisAdminService.java b/polaris-service/src/main/java/org/apache/polaris/service/admin/PolarisAdminService.java index 9a2dae2b9..236279d4f 100644 --- a/polaris-service/src/main/java/org/apache/polaris/service/admin/PolarisAdminService.java +++ b/polaris-service/src/main/java/org/apache/polaris/service/admin/PolarisAdminService.java @@ -854,8 +854,8 @@ public void deletePrincipal(String name) { getCurrentPolarisContext(), currentPrincipalEntity.getClientId(), currentPrincipalEntity.getId(), - currentSecrets.getMainSecret(), - shouldReset) + shouldReset, + currentSecrets.getMainSecretHash()) .getPrincipalSecrets(); if (newSecrets == null) { throw new IllegalStateException( diff --git a/polaris-service/src/main/java/org/apache/polaris/service/auth/TokenBroker.java b/polaris-service/src/main/java/org/apache/polaris/service/auth/TokenBroker.java index 3d281730e..492718701 100644 --- a/polaris-service/src/main/java/org/apache/polaris/service/auth/TokenBroker.java +++ b/polaris-service/src/main/java/org/apache/polaris/service/auth/TokenBroker.java @@ -51,8 +51,7 @@ TokenResponse generateFromToken( if (!principalSecrets.isSuccess()) { return Optional.empty(); } - if (!principalSecrets.getPrincipalSecrets().getMainSecret().equals(clientSecret) - && !principalSecrets.getPrincipalSecrets().getSecondarySecret().equals(clientSecret)) { + if (!principalSecrets.getPrincipalSecrets().matchesSecret(clientSecret)) { return Optional.empty(); } PolarisMetaStoreManager.EntityResult result = diff --git a/polaris-service/src/test/java/org/apache/polaris/service/admin/PolarisAuthzTestBase.java b/polaris-service/src/test/java/org/apache/polaris/service/admin/PolarisAuthzTestBase.java index f0a6d50df..6fc24a2b2 100644 --- a/polaris-service/src/test/java/org/apache/polaris/service/admin/PolarisAuthzTestBase.java +++ b/polaris-service/src/test/java/org/apache/polaris/service/admin/PolarisAuthzTestBase.java @@ -325,8 +325,8 @@ public void after() { callContext.getPolarisCallContext(), credentials.getClientId(), lookupEntity.getEntity().getId(), - credentials.getClientSecret(), - false); + false, + credentials.getClientSecret()); // This should actually be the secret's hash return new PrincipalEntity( PolarisEntity.of(