diff --git a/CHANGELOG.md b/CHANGELOG.md index 3ddadba6df..67727c75b3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -79,6 +79,7 @@ request adding CHANGELOG notes for breaking (!) changes and possibly other secti - `TableCleanupTaskHandler` now clamps `TABLE_METADATA_CLEANUP_BATCH_SIZE` to at least 1. Previously a non-positive realm override caused an infinite loop (0) or `IllegalArgumentException` (<0) when splitting metadata files for cleanup. - `ManifestFileCleanupTaskHandler` now handles Iceberg v2 delete manifests in addition to data manifests. Previously, `DROP TABLE PURGE` on a v2 table that had been updated via merge-on-read DML left position-delete files and their manifests as orphans in object storage; the cleanup task failed silently because `ManifestFiles.read()` rejects delete manifests. - OPA authorizer now includes the realm identifier in the authorization context sent to OPA (`input.context.realm`). This ensures OPA policies can enforce tenant isolation across realms, preventing potential collisions if identical principal or resource names exist in different realms. +- Internal JWTs are bound to a salted credentials-version fingerprint (`polaris-cv` claim, derived from secret hashes plus the principal salt so raw hashes are never exposed in the token). Verify accepts the current main or secondary generation so dual-secret rotate grace matches client-secret matching; a further rotate/reset invalidates older bound tokens. Tokens minted before this change carry no claim and remain valid until expiry for upgrade compatibility. ## [1.6.0] diff --git a/polaris-core/src/main/java/org/apache/polaris/core/DigestUtils.java b/polaris-core/src/main/java/org/apache/polaris/core/DigestUtils.java index 7b95628002..219b3e7033 100644 --- a/polaris-core/src/main/java/org/apache/polaris/core/DigestUtils.java +++ b/polaris-core/src/main/java/org/apache/polaris/core/DigestUtils.java @@ -20,6 +20,8 @@ import com.google.common.hash.Hashing; import java.nio.charset.StandardCharsets; +import java.security.MessageDigest; +import java.util.Objects; public final class DigestUtils { private DigestUtils() { @@ -29,4 +31,14 @@ private DigestUtils() { public static String sha256Hex(String input) { return Hashing.sha256().hashString(input, StandardCharsets.UTF_8).toString(); } + + /** + * Constant-time equality check for secret material. Both arguments must be non-null; callers are + * expected to guard nullable inputs before comparing. + */ + public static boolean constantTimeEquals(String a, String b) { + return MessageDigest.isEqual( + Objects.requireNonNull(a).getBytes(StandardCharsets.UTF_8), + Objects.requireNonNull(b).getBytes(StandardCharsets.UTF_8)); + } } 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 f0d8128e5a..60cf479acd 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 @@ -185,6 +185,55 @@ public String getMainSecretHash() { return mainSecretHash; } + /** + * Credentials-generation fingerprint for tokens minted against the current main secret. + * Derived from existing fields with the principal's secret salt, so no schema change is required. + * The value is not itself a credential and is safe to embed in signed (but not encrypted) JWTs. + * + * @return the main credentials version, or {@code null} when no main secret hash is set + */ + @Nullable + public String getCredentialsVersion() { + return credentialsVersionForHash(mainSecretHash); + } + + /** + * Returns true when {@code credentialsVersion} matches the salted fingerprint of the current main + * secret hash or the secondary secret hash. After a single rotate, the previous main + * hash is kept as secondary so client secrets and bound JWTs both remain valid for that + * generation; a further rotate/reset advances secondary and invalidates the older fingerprint. + * + *

Comparisons are constant-time against each candidate fingerprint. + */ + public boolean matchesCredentialsVersion(@Nullable String credentialsVersion) { + if (credentialsVersion == null || credentialsVersion.isEmpty()) { + return false; + } + boolean matches = false; + String mainVersion = credentialsVersionForHash(mainSecretHash); + if (mainVersion != null) { + matches |= DigestUtils.constantTimeEquals(credentialsVersion, mainVersion); + } + String secondaryVersion = credentialsVersionForHash(secondarySecretHash); + if (secondaryVersion != null) { + matches |= DigestUtils.constantTimeEquals(credentialsVersion, secondaryVersion); + } + return matches; + } + + /** + * Salted, non-reversible fingerprint of a secret-verification hash for embedding in JWTs. Uses + * the same per-principal {@link #secretSalt} already stored for secret hashing, which is always + * set by the constructors. + */ + @Nullable + private String credentialsVersionForHash(@Nullable String secretHash) { + if (secretHash == null || secretHash.isEmpty()) { + return null; + } + return DigestUtils.sha256Hex(secretHash + ":" + secretSalt); + } + public String getSecondarySecretHash() { return secondarySecretHash; } diff --git a/polaris-core/src/test/java/org/apache/polaris/core/DigestUtilsTest.java b/polaris-core/src/test/java/org/apache/polaris/core/DigestUtilsTest.java new file mode 100644 index 0000000000..9b930363a7 --- /dev/null +++ b/polaris-core/src/test/java/org/apache/polaris/core/DigestUtilsTest.java @@ -0,0 +1,45 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.polaris.core; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatThrownBy; + +import org.junit.jupiter.api.Test; + +public class DigestUtilsTest { + + @Test + void constantTimeEquals() { + assertThat(DigestUtils.constantTimeEquals("abc", "abc")).isTrue(); + assertThat(DigestUtils.constantTimeEquals("abc", "abd")).isFalse(); + assertThat(DigestUtils.constantTimeEquals("abc", "abcd")).isFalse(); + assertThat(DigestUtils.constantTimeEquals("", "")).isTrue(); + } + + @Test + void constantTimeEqualsRejectsNull() { + assertThatThrownBy(() -> DigestUtils.constantTimeEquals(null, "a")) + .isInstanceOf(NullPointerException.class); + assertThatThrownBy(() -> DigestUtils.constantTimeEquals("a", null)) + .isInstanceOf(NullPointerException.class); + assertThatThrownBy(() -> DigestUtils.constantTimeEquals(null, null)) + .isInstanceOf(NullPointerException.class); + } +} diff --git a/runtime/service/src/main/java/org/apache/polaris/service/auth/internal/broker/JWTBroker.java b/runtime/service/src/main/java/org/apache/polaris/service/auth/internal/broker/JWTBroker.java index f828894952..63c8c82d47 100644 --- a/runtime/service/src/main/java/org/apache/polaris/service/auth/internal/broker/JWTBroker.java +++ b/runtime/service/src/main/java/org/apache/polaris/service/auth/internal/broker/JWTBroker.java @@ -22,6 +22,7 @@ import com.auth0.jwt.algorithms.Algorithm; import com.auth0.jwt.interfaces.DecodedJWT; import com.auth0.jwt.interfaces.JWTVerifier; +import com.google.common.annotations.VisibleForTesting; import java.time.Instant; import java.time.temporal.ChronoUnit; import java.util.Optional; @@ -29,6 +30,7 @@ import java.util.UUID; import org.apache.iceberg.exceptions.NotAuthorizedException; import org.apache.polaris.core.PolarisCallContext; +import org.apache.polaris.core.entity.PolarisPrincipalSecrets; import org.apache.polaris.core.entity.PrincipalEntity; import org.apache.polaris.core.persistence.PolarisMetaStoreManager; import org.apache.polaris.core.persistence.dao.entity.PrincipalSecretsResult; @@ -36,6 +38,7 @@ import org.apache.polaris.service.auth.PolarisCredential; import org.apache.polaris.service.auth.internal.service.OAuthError; import org.apache.polaris.service.types.TokenType; +import org.jspecify.annotations.Nullable; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -49,6 +52,14 @@ public class JWTBroker implements TokenBroker { private static final String CLAIM_KEY_PRINCIPAL_ID = "principalId"; private static final String CLAIM_KEY_SCOPE = "scope"; + /** + * Credentials-generation fingerprint bound to the principal's current credentials version (see + * {@link PolarisPrincipalSecrets#getCredentialsVersion()}). When secrets are rotated or reset, + * the version changes and tokens carrying a prior value are rejected on verify. The claim name is + * collision-resistant per RFC 7519. + */ + @VisibleForTesting static final String CLAIM_KEY_CREDENTIALS_VERSION = "polaris-cv"; + private final PolarisMetaStoreManager metaStoreManager; private final PolarisCallContext polarisCallContext; private final int maxTokenGenerationInSeconds; @@ -78,24 +89,62 @@ static JWTVerifier buildVerifier(Algorithm algorithm) { @Override public PolarisCredential verify(String token) { - return verifyInternal(token); + return verifyInternal(token).token(); } - private InternalPolarisToken verifyInternal(String token) { + private VerifiedToken verifyInternal(String token) { try { DecodedJWT decodedJWT = verifier.verify(token); - return InternalPolarisToken.of( - decodedJWT.getSubject(), - decodedJWT.getClaim(CLAIM_KEY_PRINCIPAL_ID).asLong(), - decodedJWT.getClaim(CLAIM_KEY_CLIENT_ID).asString(), - decodedJWT.getClaim(CLAIM_KEY_SCOPE).asString()); + String clientId = decodedJWT.getClaim(CLAIM_KEY_CLIENT_ID).asString(); + String credentialsVersion = decodedJWT.getClaim(CLAIM_KEY_CREDENTIALS_VERSION).asString(); + PolarisPrincipalSecrets secrets = + assertCredentialsVersionCurrent(clientId, credentialsVersion); + return new VerifiedToken( + InternalPolarisToken.of( + decodedJWT.getSubject(), + decodedJWT.getClaim(CLAIM_KEY_PRINCIPAL_ID).asLong(), + clientId, + decodedJWT.getClaim(CLAIM_KEY_SCOPE).asString()), + secrets); + } catch (NotAuthorizedException e) { + throw e; } catch (Exception e) { throw (NotAuthorizedException) new NotAuthorizedException("Failed to verify the token").initCause(e); } } + /** + * Rejects tokens that were minted against a credentials generation that is no longer current. + * Accepts the main or secondary generation fingerprint so dual-secret rotate grace matches + * client-secret matching. Tokens minted before credentials binding (no claim) are accepted so + * existing clients are not broken on upgrade; they remain unbound until expiry. + * + * @return the loaded principal secrets when the claim was checked, or {@code null} for legacy + * tokens without the claim (callers can reuse the secrets to avoid a second load) + */ + @Nullable + private PolarisPrincipalSecrets assertCredentialsVersionCurrent( + String clientId, String credentialsVersion) { + if (credentialsVersion == null) { + // Legacy token minted before credentials binding existed. + return null; + } + if (clientId == null || clientId.isBlank() || credentialsVersion.isBlank()) { + throw new NotAuthorizedException("Failed to verify the token"); + } + PrincipalSecretsResult secretsResult = + metaStoreManager.loadPrincipalSecrets(polarisCallContext, clientId); + if (!secretsResult.isSuccess() || secretsResult.getPrincipalSecrets() == null) { + throw new NotAuthorizedException("Failed to verify the token"); + } + if (!secretsResult.getPrincipalSecrets().matchesCredentialsVersion(credentialsVersion)) { + throw new NotAuthorizedException("Failed to verify the token"); + } + return secretsResult.getPrincipalSecrets(); + } + @Override public TokenResponse generateFromToken( TokenType subjectTokenType, @@ -112,13 +161,15 @@ public TokenResponse generateFromToken( if (subjectToken == null || subjectToken.isBlank()) { return TokenResponse.of(OAuthError.invalid_request); } - InternalPolarisToken decodedToken; + VerifiedToken verified; try { - decodedToken = verifyInternal(subjectToken); + // verifyInternal enforces credentials-version binding (rejects after rotate/reset). + verified = verifyInternal(subjectToken); } catch (NotAuthorizedException e) { LOGGER.error("Failed to verify the token", e.getCause()); return TokenResponse.of(OAuthError.invalid_client); } + InternalPolarisToken decodedToken = verified.token(); Optional principalLookup = metaStoreManager.findPrincipalById(polarisCallContext, decodedToken.getPrincipalId()); if (principalLookup.isEmpty()) { @@ -138,12 +189,28 @@ public TokenResponse generateFromToken( } tokenScope = scope; } + String credentialsVersion; + if (verified.principalSecrets() != null) { + // Secrets were already loaded during verify; reuse them instead of a second read. + credentialsVersion = verified.principalSecrets().getCredentialsVersion(); + if (credentialsVersion == null) { + return TokenResponse.of(OAuthError.unauthorized_client); + } + } else { + // Legacy token without the claim: load once to bind the re-minted token. + Optional loaded = loadCurrentCredentialsVersion(decodedToken.getClientId()); + if (loaded.isEmpty()) { + return TokenResponse.of(OAuthError.unauthorized_client); + } + credentialsVersion = loaded.get(); + } String tokenString = generateTokenString( decodedToken.getPrincipalName(), decodedToken.getPrincipalId(), decodedToken.getClientId(), - tokenScope); + tokenScope, + credentialsVersion); return TokenResponse.of( tokenString, TokenType.ACCESS_TOKEN.getValue(), maxTokenGenerationInSeconds); } @@ -163,30 +230,45 @@ public TokenResponse generateFromClientSecrets( return TokenResponse.of(initialValidationResponse.get()); } - Optional principal = findPrincipalEntity(clientId, clientSecret); - if (principal.isEmpty()) { + Optional authenticated = + authenticateWithClientSecrets(clientId, clientSecret); + if (authenticated.isEmpty()) { return TokenResponse.of(OAuthError.unauthorized_client); } + AuthenticatedPrincipal principal = authenticated.get(); String tokenString = - generateTokenString(principal.get().getName(), principal.get().getId(), clientId, scope); + generateTokenString( + principal.entity().getName(), + principal.entity().getId(), + clientId, + scope, + principal.credentialsVersion()); return TokenResponse.of( tokenString, TokenType.ACCESS_TOKEN.getValue(), maxTokenGenerationInSeconds); } private String generateTokenString( - String principalName, long principalId, String clientId, String scope) { + String principalName, + long principalId, + String clientId, + String scope, + String credentialsVersion) { Instant now = Instant.now(); - return JWT.create() - .withIssuer(ISSUER_KEY) - .withSubject(principalName) - .withIssuedAt(now) - .withExpiresAt(now.plus(maxTokenGenerationInSeconds, ChronoUnit.SECONDS)) - .withJWTId(UUID.randomUUID().toString()) - .withClaim(CLAIM_KEY_ACTIVE, true) - .withClaim(CLAIM_KEY_CLIENT_ID, clientId) - .withClaim(CLAIM_KEY_PRINCIPAL_ID, principalId) - .withClaim(CLAIM_KEY_SCOPE, scopes(scope)) - .sign(algorithm); + var builder = + JWT.create() + .withIssuer(ISSUER_KEY) + .withSubject(principalName) + .withIssuedAt(now) + .withExpiresAt(now.plus(maxTokenGenerationInSeconds, ChronoUnit.SECONDS)) + .withJWTId(UUID.randomUUID().toString()) + .withClaim(CLAIM_KEY_ACTIVE, true) + .withClaim(CLAIM_KEY_CLIENT_ID, clientId) + .withClaim(CLAIM_KEY_PRINCIPAL_ID, principalId) + .withClaim(CLAIM_KEY_SCOPE, scopes(scope)); + if (credentialsVersion != null) { + builder.withClaim(CLAIM_KEY_CREDENTIALS_VERSION, credentialsVersion); + } + return builder.sign(algorithm); } @Override @@ -203,17 +285,41 @@ private String scopes(String scope) { return scope == null || scope.isBlank() ? DefaultAuthenticator.PRINCIPAL_ROLE_ALL : scope; } - private Optional findPrincipalEntity(String clientId, String clientSecret) { - // Validate the principal is present and secrets match + private Optional loadCurrentCredentialsVersion(String clientId) { PrincipalSecretsResult principalSecrets = metaStoreManager.loadPrincipalSecrets(polarisCallContext, clientId); - if (!principalSecrets.isSuccess()) { + if (!principalSecrets.isSuccess() || principalSecrets.getPrincipalSecrets() == null) { return Optional.empty(); } - if (!principalSecrets.getPrincipalSecrets().matchesSecret(clientSecret)) { + return Optional.ofNullable(principalSecrets.getPrincipalSecrets().getCredentialsVersion()); + } + + private Optional authenticateWithClientSecrets( + String clientId, String clientSecret) { + PrincipalSecretsResult principalSecrets = + metaStoreManager.loadPrincipalSecrets(polarisCallContext, clientId); + if (!principalSecrets.isSuccess() || principalSecrets.getPrincipalSecrets() == null) { return Optional.empty(); } - return metaStoreManager.findPrincipalById( - polarisCallContext, principalSecrets.getPrincipalSecrets().getPrincipalId()); + PolarisPrincipalSecrets secrets = principalSecrets.getPrincipalSecrets(); + if (!secrets.matchesSecret(clientSecret)) { + return Optional.empty(); + } + Optional principal = + metaStoreManager.findPrincipalById(polarisCallContext, secrets.getPrincipalId()); + if (principal.isEmpty()) { + return Optional.empty(); + } + return Optional.of( + new AuthenticatedPrincipal(principal.get(), secrets.getCredentialsVersion())); } + + private record AuthenticatedPrincipal(PrincipalEntity entity, String credentialsVersion) {} + + /** + * Result of token verification. {@code principalSecrets} carries the secrets loaded while + * checking the credentials-version claim, or {@code null} for legacy tokens without the claim. + */ + private record VerifiedToken( + InternalPolarisToken token, @Nullable PolarisPrincipalSecrets principalSecrets) {} } diff --git a/runtime/service/src/test/java/org/apache/polaris/service/auth/internal/broker/JWTBrokerCredentialsBindingTest.java b/runtime/service/src/test/java/org/apache/polaris/service/auth/internal/broker/JWTBrokerCredentialsBindingTest.java new file mode 100644 index 0000000000..5aec0b2a63 --- /dev/null +++ b/runtime/service/src/test/java/org/apache/polaris/service/auth/internal/broker/JWTBrokerCredentialsBindingTest.java @@ -0,0 +1,258 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.polaris.service.auth.internal.broker; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatThrownBy; +import static org.mockito.Mockito.when; + +import com.auth0.jwt.JWT; +import com.auth0.jwt.algorithms.Algorithm; +import com.auth0.jwt.interfaces.DecodedJWT; +import java.util.Optional; +import org.apache.iceberg.exceptions.NotAuthorizedException; +import org.apache.polaris.core.DigestUtils; +import org.apache.polaris.core.PolarisCallContext; +import org.apache.polaris.core.entity.PolarisPrincipalSecrets; +import org.apache.polaris.core.entity.PrincipalEntity; +import org.apache.polaris.core.persistence.PolarisMetaStoreManager; +import org.apache.polaris.core.persistence.dao.entity.PrincipalSecretsResult; +import org.apache.polaris.service.auth.internal.service.OAuthError; +import org.apache.polaris.service.types.TokenType; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.mockito.Mockito; + +/** + * Internal JWTs are bound to a salted credentials-version fingerprint derived from secret hashes. + * One rotate keeps the previous main hash as secondary so bound tokens remain valid for that grace + * generation; a further rotate/reset invalidates them. Tokens without the claim stay valid until + * expiry for upgrade compatibility. + */ +public class JWTBrokerCredentialsBindingTest { + + private static final long PRINCIPAL_ID = 99L; + private static final String CLIENT_ID = "client-abc"; + private static final String MAIN_SECRET = "main-secret-value"; + private static final String SECONDARY_SECRET = "secondary-secret-value"; + private static final String SCOPE = "PRINCIPAL_ROLE:ALL"; + + private PolarisCallContext callContext; + private PolarisMetaStoreManager metaStore; + private Algorithm algorithm; + private JWTBroker broker; + private PolarisPrincipalSecrets secrets; + + @BeforeEach + void setUp() { + callContext = Mockito.mock(PolarisCallContext.class); + metaStore = Mockito.mock(PolarisMetaStoreManager.class); + algorithm = Algorithm.HMAC256("test-hmac-key"); + secrets = new PolarisPrincipalSecrets(PRINCIPAL_ID, CLIENT_ID, MAIN_SECRET, SECONDARY_SECRET); + broker = + new JWTBroker(metaStore, callContext, 3600, algorithm, JWTBroker.buildVerifier(algorithm)); + + when(metaStore.loadPrincipalSecrets(callContext, CLIENT_ID)) + .thenReturn(new PrincipalSecretsResult(secrets)); + when(metaStore.findPrincipalById(callContext, PRINCIPAL_ID)) + .thenReturn( + Optional.of( + new PrincipalEntity.Builder().setId(PRINCIPAL_ID).setName("alice").build())); + } + + @Test + void mintedTokenCarriesSaltedCredentialsVersion() { + TokenResponse response = + broker.generateFromClientSecrets( + CLIENT_ID, + MAIN_SECRET, + TokenRequestValidator.CLIENT_CREDENTIALS, + SCOPE, + TokenType.ACCESS_TOKEN); + assertThat(response.getError()).isNull(); + DecodedJWT jwt = JWT.decode(response.getAccessToken()); + String claim = jwt.getClaim(JWTBroker.CLAIM_KEY_CREDENTIALS_VERSION).asString(); + assertThat(claim).isEqualTo(secrets.getCredentialsVersion()); + // Must not expose the raw secret-verification hash, nor an unsalted hash of it. + assertThat(claim).isNotEqualTo(secrets.getMainSecretHash()); + assertThat(claim).isNotEqualTo(DigestUtils.sha256Hex(secrets.getMainSecretHash())); + assertThat(secrets.matchesCredentialsVersion(claim)).isTrue(); + } + + @Test + void verifySucceedsForTokenBoundToCurrentSecrets() { + TokenResponse response = + broker.generateFromClientSecrets( + CLIENT_ID, + MAIN_SECRET, + TokenRequestValidator.CLIENT_CREDENTIALS, + SCOPE, + TokenType.ACCESS_TOKEN); + assertThat(broker.verify(response.getAccessToken()).getPrincipalId()).isEqualTo(PRINCIPAL_ID); + } + + @Test + void verifyStillSucceedsAfterOneRotateViaSecondaryGrace() { + TokenResponse response = + broker.generateFromClientSecrets( + CLIENT_ID, + MAIN_SECRET, + TokenRequestValidator.CLIENT_CREDENTIALS, + SCOPE, + TokenType.ACCESS_TOKEN); + String accessToken = response.getAccessToken(); + + // After one rotate, previous main becomes secondary; dual-secret grace keeps the JWT valid. + secrets.rotateSecrets(secrets.getMainSecretHash()); + when(metaStore.loadPrincipalSecrets(callContext, CLIENT_ID)) + .thenReturn(new PrincipalSecretsResult(secrets)); + + assertThat(broker.verify(accessToken).getPrincipalId()).isEqualTo(PRINCIPAL_ID); + } + + @Test + void verifyFailsAfterSecondRotate() { + TokenResponse response = + broker.generateFromClientSecrets( + CLIENT_ID, + MAIN_SECRET, + TokenRequestValidator.CLIENT_CREDENTIALS, + SCOPE, + TokenType.ACCESS_TOKEN); + String accessToken = response.getAccessToken(); + + secrets.rotateSecrets(secrets.getMainSecretHash()); + secrets.rotateSecrets(secrets.getMainSecretHash()); + when(metaStore.loadPrincipalSecrets(callContext, CLIENT_ID)) + .thenReturn(new PrincipalSecretsResult(secrets)); + + assertThatThrownBy(() -> broker.verify(accessToken)) + .isInstanceOf(NotAuthorizedException.class) + .hasMessageContaining("Failed to verify the token"); + } + + @Test + void tokenExchangeFailsAfterSecondRotate() { + TokenResponse original = + broker.generateFromClientSecrets( + CLIENT_ID, + MAIN_SECRET, + TokenRequestValidator.CLIENT_CREDENTIALS, + SCOPE, + TokenType.ACCESS_TOKEN); + + secrets.rotateSecrets(secrets.getMainSecretHash()); + secrets.rotateSecrets(secrets.getMainSecretHash()); + when(metaStore.loadPrincipalSecrets(callContext, CLIENT_ID)) + .thenReturn(new PrincipalSecretsResult(secrets)); + + TokenResponse exchanged = + broker.generateFromToken( + TokenType.ACCESS_TOKEN, + original.getAccessToken(), + TokenRequestValidator.TOKEN_EXCHANGE, + SCOPE, + TokenType.ACCESS_TOKEN); + assertThat(exchanged.getError()).isEqualTo(OAuthError.invalid_client); + } + + @Test + void tokenExchangeSucceedsAfterOneRotate() { + TokenResponse original = + broker.generateFromClientSecrets( + CLIENT_ID, + MAIN_SECRET, + TokenRequestValidator.CLIENT_CREDENTIALS, + SCOPE, + TokenType.ACCESS_TOKEN); + + secrets.rotateSecrets(secrets.getMainSecretHash()); + when(metaStore.loadPrincipalSecrets(callContext, CLIENT_ID)) + .thenReturn(new PrincipalSecretsResult(secrets)); + Mockito.clearInvocations(metaStore); + + TokenResponse exchanged = + broker.generateFromToken( + TokenType.ACCESS_TOKEN, + original.getAccessToken(), + TokenRequestValidator.TOKEN_EXCHANGE, + SCOPE, + TokenType.ACCESS_TOKEN); + assertThat(exchanged.getError()).isNull(); + // Re-minted token is bound to the *current* main generation. + assertThat( + JWT.decode(exchanged.getAccessToken()) + .getClaim(JWTBroker.CLAIM_KEY_CREDENTIALS_VERSION) + .asString()) + .isEqualTo(secrets.getCredentialsVersion()); + // The secrets loaded during verify are reused for re-minting: no second metastore read. + Mockito.verify(metaStore, Mockito.times(1)).loadPrincipalSecrets(callContext, CLIENT_ID); + } + + @Test + void verifyAcceptsLegacyTokenWithoutCredentialsVersionClaim() { + String legacy = legacyToken(); + assertThat(broker.verify(legacy).getPrincipalId()).isEqualTo(PRINCIPAL_ID); + } + + @Test + void legacyTokenWithoutClaimIsNotBoundByRotate() { + String legacy = legacyToken(); + + secrets.rotateSecrets(secrets.getMainSecretHash()); + when(metaStore.loadPrincipalSecrets(callContext, CLIENT_ID)) + .thenReturn(new PrincipalSecretsResult(secrets)); + + assertThat(broker.verify(legacy).getPrincipalId()).isEqualTo(PRINCIPAL_ID); + } + + @Test + void newTokenAfterRotateUsesNewMainVersion() { + secrets.rotateSecrets(secrets.getMainSecretHash()); + String newMain = secrets.getMainSecret(); + when(metaStore.loadPrincipalSecrets(callContext, CLIENT_ID)) + .thenReturn(new PrincipalSecretsResult(secrets)); + + TokenResponse after = + broker.generateFromClientSecrets( + CLIENT_ID, + newMain, + TokenRequestValidator.CLIENT_CREDENTIALS, + SCOPE, + TokenType.ACCESS_TOKEN); + assertThat(after.getError()).isNull(); + assertThat( + JWT.decode(after.getAccessToken()) + .getClaim(JWTBroker.CLAIM_KEY_CREDENTIALS_VERSION) + .asString()) + .isEqualTo(secrets.getCredentialsVersion()); + assertThat(broker.verify(after.getAccessToken()).getPrincipalId()).isEqualTo(PRINCIPAL_ID); + } + + private String legacyToken() { + return JWT.create() + .withIssuer("polaris") + .withSubject("alice") + .withClaim("active", true) + .withClaim("client_id", CLIENT_ID) + .withClaim("principalId", PRINCIPAL_ID) + .withClaim("scope", SCOPE) + .sign(algorithm); + } +} diff --git a/runtime/service/src/test/java/org/apache/polaris/service/auth/internal/broker/JWTSymmetricKeyGeneratorTest.java b/runtime/service/src/test/java/org/apache/polaris/service/auth/internal/broker/JWTSymmetricKeyGeneratorTest.java index 5ec26b77bb..41083bda66 100644 --- a/runtime/service/src/test/java/org/apache/polaris/service/auth/internal/broker/JWTSymmetricKeyGeneratorTest.java +++ b/runtime/service/src/test/java/org/apache/polaris/service/auth/internal/broker/JWTSymmetricKeyGeneratorTest.java @@ -78,6 +78,9 @@ public void testJWTSymmetricKeyGenerator() { assertThat(token.getExpiresIn()).isEqualTo(666); assertThat(decodedJWT.getClaim("scope").asString()).isEqualTo("PRINCIPAL_ROLE:TEST"); assertThat(decodedJWT.getClaim("client_id").asString()).isEqualTo(clientId); + assertThat(decodedJWT.getClaim(JWTBroker.CLAIM_KEY_CREDENTIALS_VERSION).asString()) + .isEqualTo(principalSecrets.getCredentialsVersion()); + assertThat(generator.verify(token.getAccessToken()).getPrincipalId()).isEqualTo(principalId); } @Test diff --git a/runtime/service/src/test/java/org/apache/polaris/service/ratelimiter/RateLimiterFilterTest.java b/runtime/service/src/test/java/org/apache/polaris/service/ratelimiter/RateLimiterFilterTest.java index a7e7d766ec..5aee782379 100644 --- a/runtime/service/src/test/java/org/apache/polaris/service/ratelimiter/RateLimiterFilterTest.java +++ b/runtime/service/src/test/java/org/apache/polaris/service/ratelimiter/RateLimiterFilterTest.java @@ -30,7 +30,9 @@ import jakarta.inject.Inject; import jakarta.ws.rs.client.Client; import jakarta.ws.rs.client.ClientBuilder; +import jakarta.ws.rs.client.Entity; import jakarta.ws.rs.core.MediaType; +import jakarta.ws.rs.core.MultivaluedHashMap; import jakarta.ws.rs.core.Response; import jakarta.ws.rs.core.Response.Status; import java.util.Map; @@ -38,6 +40,7 @@ import java.util.function.Consumer; import org.apache.iceberg.rest.responses.ErrorResponse; import org.apache.iceberg.rest.responses.ErrorResponseParser; +import org.apache.iceberg.rest.responses.OAuthTokenResponse; import org.apache.polaris.service.events.EventAttributes; import org.apache.polaris.service.events.PolarisEvent; import org.apache.polaris.service.events.PolarisEventType; @@ -80,6 +83,11 @@ public Map getConfigOverrides() { .put("polaris.rate-limiter.token-bucket.type", "default") .put("polaris.metrics.tags.environment", "prod") .put("polaris.realm-context.realms", "POLARIS,POLARIS2") + // Bootstrapping both realms with known credentials so each realm can mint its own + // access token. Tokens are bound to per-realm secret hashes and cannot be shared. + .put( + "polaris.bootstrap.credentials", + "POLARIS,test-admin,test-secret;POLARIS2,test-admin2,test-secret2") .put("polaris.metrics.realm-id-tag.enable-in-api-metrics", "true") .put("polaris.metrics.realm-id-tag.enable-in-http-metrics", "true") .put("polaris.authentication.token-broker.type", "symmetric-key") @@ -138,10 +146,14 @@ public void testRateLimiter() { requestAsserter.accept(Status.TOO_MANY_REQUESTS); } - // Ensure that a different realm identifier gets a separate limit + // Ensure that a different realm identifier gets a separate limit. Access tokens are + // bound to per-realm principal secrets, so mint a token for POLARIS2 rather than reusing + // the POLARIS admin token (which would correctly return 401 after credentials binding). MockRateLimiter.allowProceed = true; + String polaris2Token = + obtainAccessTokenForRealm(polarisEndpoints, "POLARIS2", "test-admin2", "test-secret2"); Consumer requestAsserter2 = - constructRequestAsserter(polarisEndpoints, adminToken, "POLARIS2"); + constructRequestAsserter(polarisEndpoints, polaris2Token, "POLARIS2"); requestAsserter2.accept(Status.OK); } @@ -243,4 +255,27 @@ private static Consumer constructRequestAsserter( } }; } + + /** + * Mints an access token in the given realm using client credentials. Realm must be bootstrapped + * with those credentials (see {@link Profile}). + */ + private static String obtainAccessTokenForRealm( + PolarisApiEndpoints endpoints, String realm, String clientId, String clientSecret) { + MultivaluedHashMap form = new MultivaluedHashMap<>(); + form.add("grant_type", "client_credentials"); + form.add("client_id", clientId); + form.add("client_secret", clientSecret); + form.add("scope", "PRINCIPAL_ROLE:ALL"); + try (Client httpClient = ClientBuilder.newBuilder().build(); + Response response = + httpClient + .target(String.format("%s/v1/oauth/tokens", endpoints.catalogApiEndpoint())) + .request(MediaType.APPLICATION_JSON_TYPE) + .header("Polaris-Realm", realm) + .post(Entity.form(form))) { + assertThat(response.getStatus()).isEqualTo(Status.OK.getStatusCode()); + return response.readEntity(OAuthTokenResponse.class).token(); + } + } }