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 f0a307e61..bbee4e378 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 @@ -20,12 +20,12 @@ import static jakarta.persistence.Persistence.createEntityManagerFactory; import static org.apache.polaris.core.persistence.PrincipalSecretsGenerator.RANDOM_SECRETS; +import static org.assertj.core.api.Assertions.assertThat; 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; @@ -91,17 +91,41 @@ void testCreateStoreSession(String confFile, boolean success) { 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()); + var newSecrets = new PolarisPrincipalSecrets(42L); + assertThat(newSecrets) + .extracting( + PolarisPrincipalSecrets::getMainSecret, + PolarisPrincipalSecrets::getSecondarySecret, + PolarisPrincipalSecrets::getMainSecretHash, + PolarisPrincipalSecrets::getSecondarySecretHash, + PolarisPrincipalSecrets::getSecretSalt) + .doesNotContainNull() + .allMatch(x -> !x.toString().isEmpty()); + + ModelPrincipalSecrets model = ModelPrincipalSecrets.fromPrincipalSecrets(newSecrets); + var key = model.getPrincipalClientId(); + + var fromModel = ModelPrincipalSecrets.toPrincipalSecrets(model); + + assertThat(fromModel) + .extracting( + PolarisPrincipalSecrets::getMainSecret, PolarisPrincipalSecrets::getSecondarySecret) + .containsOnlyNulls(); + + assertThat(model) + .extracting( + ModelPrincipalSecrets::getPrincipalId, + ModelPrincipalSecrets::getPrincipalClientId, + ModelPrincipalSecrets::getSecretSalt, + ModelPrincipalSecrets::getMainSecretHash, + ModelPrincipalSecrets::getSecondarySecretHash) + .containsExactly( + newSecrets.getPrincipalId(), + newSecrets.getPrincipalClientId(), + newSecrets.getSecretSalt(), + newSecrets.getMainSecretHash(), + newSecrets.getSecondarySecretHash()); try (var emf = createEntityManagerFactory("polaris")) { var entityManager = emf.createEntityManager(); @@ -115,31 +139,54 @@ void testRotateLegacyPrincipalSecret() { 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()); + assertThat(retrievedModel) + .extracting( + ModelPrincipalSecrets::getPrincipalId, + ModelPrincipalSecrets::getPrincipalClientId, + ModelPrincipalSecrets::getSecretSalt, + ModelPrincipalSecrets::getMainSecretHash, + ModelPrincipalSecrets::getSecondarySecretHash) + .containsExactly( + model.getPrincipalId(), + model.getPrincipalClientId(), + model.getSecretSalt(), + model.getMainSecretHash(), + model.getSecondarySecretHash()); // Now read using PolarisEclipseLinkStore - PolarisEclipseLinkStore store = new PolarisEclipseLinkStore(diagServices); + var store = new PolarisEclipseLinkStore(new PolarisDefaultDiagServiceImpl()); 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()); + assertThat(principalSecrets) + .extracting( + PolarisPrincipalSecrets::getPrincipalId, + PolarisPrincipalSecrets::getPrincipalClientId, + PolarisPrincipalSecrets::getSecretSalt, + PolarisPrincipalSecrets::getMainSecret, + PolarisPrincipalSecrets::getMainSecretHash, + PolarisPrincipalSecrets::getSecondarySecret, + PolarisPrincipalSecrets::getSecondarySecretHash) + .containsExactly( + fromModel.getPrincipalId(), + fromModel.getPrincipalClientId(), + fromModel.getSecretSalt(), + null, + fromModel.getMainSecretHash(), + null, + fromModel.getSecondarySecretHash()); // 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()); + assertThat(principalSecrets.getMainSecret()).isNotEqualTo(newSecrets.getMainSecret()); + assertThat(principalSecrets.getMainSecretHash()).isNotEqualTo(newSecrets.getMainSecretHash()); + assertThat(principalSecrets) + .extracting( + PolarisPrincipalSecrets::getSecondarySecret, + PolarisPrincipalSecrets::getSecondarySecretHash) + .containsExactly(null, newSecrets.getMainSecretHash()); // Persist the rotated credential: store.deletePrincipalSecrets(entityManager, key); @@ -148,13 +195,10 @@ void testRotateLegacyPrincipalSecret() { // 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)); + Assertions.assertTrue(reloadedSecrets.matchesSecret(newSecrets.getMainSecret())); + Assertions.assertFalse(reloadedSecrets.matchesSecret(newSecrets.getSecondarySecret())); } } diff --git a/extension/persistence/jpa-model/src/main/java/org/apache/polaris/jpa/models/ModelPrincipalSecrets.java b/extension/persistence/jpa-model/src/main/java/org/apache/polaris/jpa/models/ModelPrincipalSecrets.java index 11428ad8f..19559642f 100644 --- a/extension/persistence/jpa-model/src/main/java/org/apache/polaris/jpa/models/ModelPrincipalSecrets.java +++ b/extension/persistence/jpa-model/src/main/java/org/apache/polaris/jpa/models/ModelPrincipalSecrets.java @@ -37,12 +37,6 @@ public class ModelPrincipalSecrets { // the client id for that principal @Id private String principalClientId; - // the main secret for that principal - private String mainSecret; - - // the secondary secret for that principal - private String secondarySecret; - // Hash of mainSecret private String mainSecretHash; @@ -62,14 +56,6 @@ public String getPrincipalClientId() { return principalClientId; } - public String getMainSecret() { - return mainSecret; - } - - public String getSecondarySecret() { - return secondarySecret; - } - public String getSecretSalt() { return secretSalt; } @@ -103,16 +89,6 @@ public Builder principalClientId(String principalClientId) { return this; } - public Builder mainSecret(String mainSecret) { - principalSecrets.mainSecret = mainSecret; - return this; - } - - public Builder secondarySecret(String secondarySecret) { - principalSecrets.secondarySecret = secondarySecret; - return this; - } - public Builder secretSalt(String secretSalt) { principalSecrets.secretSalt = secretSalt; return this; @@ -151,8 +127,8 @@ public static PolarisPrincipalSecrets toPrincipalSecrets(ModelPrincipalSecrets m return new PolarisPrincipalSecrets( model.getPrincipalId(), model.getPrincipalClientId(), - model.getMainSecret(), - model.getSecondarySecret(), + null, + null, model.getSecretSalt(), model.getMainSecretHash(), model.getSecondarySecretHash()); @@ -164,17 +140,7 @@ 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; - } } }