From f3125d52bc25c7aaba380e70c2f7a799d70354f0 Mon Sep 17 00:00:00 2001 From: vignesh_A Date: Fri, 17 Jul 2026 13:25:09 +0530 Subject: [PATCH 1/5] fix(auth): bind internal JWTs to principal credentials version Embed a credentials-generation fingerprint (polaris-cv claim) in minted access tokens. The version is derived from the principal's current main secret hash (hash of the hash), so the secret-verification artifact itself is never exposed in the unencrypted token and no schema change is needed. On verify (and thus token exchange), tokens whose version no longer matches are rejected, so credential rotate/reset invalidates outstanding sessions without waiting for token expiry. Tokens minted before this change carry no claim and remain valid until expiry for upgrade compatibility; they become bound on the next rotate/reset. --- CHANGELOG.md | 1 + .../core/entity/PolarisPrincipalSecrets.java | 16 ++ .../auth/internal/broker/JWTBroker.java | 136 +++++++++-- .../JWTBrokerCredentialsBindingTest.java | 224 ++++++++++++++++++ .../broker/JWTSymmetricKeyGeneratorTest.java | 3 + 5 files changed, 357 insertions(+), 23 deletions(-) create mode 100644 runtime/service/src/test/java/org/apache/polaris/service/auth/internal/broker/JWTBrokerCredentialsBindingTest.java diff --git a/CHANGELOG.md b/CHANGELOG.md index 3ddadba6df..026adabdca 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 the principal's current credentials version (`polaris-cv` claim, derived from the main secret hash so the hash itself is never exposed in the token). After credential rotate or reset, previously issued access tokens are rejected on verify, so sessions cannot outlive a secret compromise response. Tokens minted before this change carry no claim and remain valid until expiry for upgrade compatibility; they become bound on the next rotate/reset. ## [1.6.0] 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..94c8e2cb24 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,22 @@ public String getMainSecretHash() { return mainSecretHash; } + /** + * Credentials-generation fingerprint bound to the current main secret. Changes whenever the main + * secret is rotated or reset, so tokens carrying a prior value can be rejected on verify. + * + *

The value is derived from existing fields (a hash of {@link #mainSecretHash}) and therefore + * requires no schema change. It is not itself usable as a credential, and unlike {@code + * mainSecretHash} it is not the artifact secrets are verified against, so it is safe to embed in + * signed (but not encrypted) tokens. + * + * @return the credentials version, or {@code null} when no main secret hash is set + */ + @Nullable + public String getCredentialsVersion() { + return mainSecretHash == null ? null : DigestUtils.sha256Hex(mainSecretHash); + } + public String getSecondarySecretHash() { return secondarySecretHash; } 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..288c9d75d0 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,9 @@ 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.nio.charset.StandardCharsets; +import java.security.MessageDigest; import java.time.Instant; import java.time.temporal.ChronoUnit; import java.util.Optional; @@ -29,6 +32,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; @@ -49,6 +53,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; @@ -84,18 +96,58 @@ public PolarisCredential verify(String token) { private InternalPolarisToken verifyInternal(String token) { try { DecodedJWT decodedJWT = verifier.verify(token); + String clientId = decodedJWT.getClaim(CLAIM_KEY_CLIENT_ID).asString(); + String credentialsVersion = decodedJWT.getClaim(CLAIM_KEY_CREDENTIALS_VERSION).asString(); + assertCredentialsVersionCurrent(clientId, credentialsVersion); return InternalPolarisToken.of( decodedJWT.getSubject(), decodedJWT.getClaim(CLAIM_KEY_PRINCIPAL_ID).asLong(), - decodedJWT.getClaim(CLAIM_KEY_CLIENT_ID).asString(), + clientId, decodedJWT.getClaim(CLAIM_KEY_SCOPE).asString()); + } 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 previous credentials generation (i.e. before the + * principal's secrets were last rotated or reset). Tokens minted before credentials binding was + * introduced carry no claim and are accepted so existing clients are not broken on upgrade; + * binding activates for them on the next rotate/reset, when their principal's version changes and + * all bound tokens are rejected. + */ + private void assertCredentialsVersionCurrent(String clientId, String credentialsVersion) { + if (credentialsVersion == null) { + // Legacy token minted before credentials binding existed. + return; + } + 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"); + } + String currentVersion = secretsResult.getPrincipalSecrets().getCredentialsVersion(); + if (!constantTimeEquals(credentialsVersion, currentVersion)) { + throw new NotAuthorizedException("Failed to verify the token"); + } + } + + @VisibleForTesting + static boolean constantTimeEquals(String a, String b) { + if (a == null || b == null) { + return false; + } + return MessageDigest.isEqual( + a.getBytes(StandardCharsets.UTF_8), b.getBytes(StandardCharsets.UTF_8)); + } + @Override public TokenResponse generateFromToken( TokenType subjectTokenType, @@ -114,6 +166,7 @@ public TokenResponse generateFromToken( } InternalPolarisToken decodedToken; try { + // verifyInternal enforces credentials-version binding (rejects after rotate/reset). decodedToken = verifyInternal(subjectToken); } catch (NotAuthorizedException e) { LOGGER.error("Failed to verify the token", e.getCause()); @@ -138,12 +191,17 @@ public TokenResponse generateFromToken( } tokenScope = scope; } + Optional credentialsVersion = loadCurrentCredentialsVersion(decodedToken.getClientId()); + if (credentialsVersion.isEmpty()) { + return TokenResponse.of(OAuthError.unauthorized_client); + } String tokenString = generateTokenString( decodedToken.getPrincipalName(), decodedToken.getPrincipalId(), decodedToken.getClientId(), - tokenScope); + tokenScope, + credentialsVersion.get()); return TokenResponse.of( tokenString, TokenType.ACCESS_TOKEN.getValue(), maxTokenGenerationInSeconds); } @@ -163,30 +221,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 +276,34 @@ 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() || principalSecrets.getPrincipalSecrets() == null) { + return Optional.empty(); + } + return Optional.ofNullable(principalSecrets.getPrincipalSecrets().getCredentialsVersion()); + } + + private Optional authenticateWithClientSecrets( + String clientId, String clientSecret) { PrincipalSecretsResult principalSecrets = metaStoreManager.loadPrincipalSecrets(polarisCallContext, clientId); - if (!principalSecrets.isSuccess()) { + if (!principalSecrets.isSuccess() || principalSecrets.getPrincipalSecrets() == null) { return Optional.empty(); } - if (!principalSecrets.getPrincipalSecrets().matchesSecret(clientSecret)) { + PolarisPrincipalSecrets secrets = principalSecrets.getPrincipalSecrets(); + if (!secrets.matchesSecret(clientSecret)) { return Optional.empty(); } - return metaStoreManager.findPrincipalById( - polarisCallContext, principalSecrets.getPrincipalSecrets().getPrincipalId()); + 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) {} } 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..a8c360f174 --- /dev/null +++ b/runtime/service/src/test/java/org/apache/polaris/service/auth/internal/broker/JWTBrokerCredentialsBindingTest.java @@ -0,0 +1,224 @@ +/* + * 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.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 the principal's current credentials version (derived from the main + * secret hash) so rotate/reset invalidates outstanding access tokens. Tokens minted before + * credentials binding existed (no claim) remain 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 mintedTokenCarriesDerivedCredentialsVersion() { + 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()); + // The claim must not expose the raw secret-verification artifact. + assertThat(claim).isNotEqualTo(secrets.getMainSecretHash()); + } + + @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 verifyFailsAfterSecretRotate() { + TokenResponse response = + broker.generateFromClientSecrets( + CLIENT_ID, + MAIN_SECRET, + TokenRequestValidator.CLIENT_CREDENTIALS, + SCOPE, + TokenType.ACCESS_TOKEN); + String accessToken = response.getAccessToken(); + + // Simulate rotate: main secret hash changes, and so does the derived credentials version. + 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 tokenExchangeFailsAfterSecretRotate() { + 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)); + + 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 verifyAcceptsLegacyTokenWithoutCredentialsVersionClaim() { + // Craft a token with signature/issuer/active but no cv claim (pre-binding tokens). These + // must keep working so existing clients are not broken on upgrade. + String legacy = legacyToken(); + + assertThat(broker.verify(legacy).getPrincipalId()).isEqualTo(PRINCIPAL_ID); + } + + @Test + void legacyTokenWithoutClaimIsNotBoundByRotate() { + // Documents the accepted trade-off: tokens minted before credentials binding existed carry + // no version to check, so they remain valid until expiry even after rotate/reset. All + // tokens minted after the upgrade are bound. + 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 newTokenAfterRotateUsesNewVersionAndVerifies() { + TokenResponse before = + broker.generateFromClientSecrets( + CLIENT_ID, + MAIN_SECRET, + TokenRequestValidator.CLIENT_CREDENTIALS, + SCOPE, + TokenType.ACCESS_TOKEN); + String oldToken = before.getAccessToken(); + + 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); + assertThatThrownBy(() -> broker.verify(oldToken)).isInstanceOf(NotAuthorizedException.class); + } + + @Test + void constantTimeEquals() { + assertThat(JWTBroker.constantTimeEquals("abc", "abc")).isTrue(); + assertThat(JWTBroker.constantTimeEquals("abc", "abd")).isFalse(); + assertThat(JWTBroker.constantTimeEquals(null, "a")).isFalse(); + } + + 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 From 70f3089c39c120954e73bc453f6b51b02a1852fe Mon Sep 17 00:00:00 2001 From: vignesh_A Date: Fri, 17 Jul 2026 13:25:09 +0530 Subject: [PATCH 2/5] test(auth): mint per-realm token in RateLimiterFilterTest Access tokens are bound to per-realm principal secret hashes. Reusing a POLARIS admin token against POLARIS2 correctly returns 401. Bootstrap both realms with known credentials and obtain a POLARIS2 token for the cross-realm rate-limit isolation assertion. --- .../ratelimiter/RateLimiterFilterTest.java | 39 ++++++++++++++++++- 1 file changed, 37 insertions(+), 2 deletions(-) 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(); + } + } } From 984cfa42dd3025692c474856be0a5128331cd03f Mon Sep 17 00:00:00 2001 From: vignesh_A Date: Mon, 20 Jul 2026 18:08:49 +0530 Subject: [PATCH 3/5] fix(auth): salt credentials version and honor secondary generation Address review feedback on JWT credentials binding: derive polaris-cv with the principal secret salt, and accept tokens matching the main or secondary secret-hash fingerprint so dual-secret rotate grace aligns with client-secret matching. A second rotate/reset invalidates older bound tokens. --- CHANGELOG.md | 2 +- .../core/entity/PolarisPrincipalSecrets.java | 58 ++++++++++-- .../auth/internal/broker/JWTBroker.java | 12 ++- .../JWTBrokerCredentialsBindingTest.java | 88 +++++++++++++------ 4 files changed, 118 insertions(+), 42 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 026adabdca..67727c75b3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -79,7 +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 the principal's current credentials version (`polaris-cv` claim, derived from the main secret hash so the hash itself is never exposed in the token). After credential rotate or reset, previously issued access tokens are rejected on verify, so sessions cannot outlive a secret compromise response. Tokens minted before this change carry no claim and remain valid until expiry for upgrade compatibility; they become bound on the next rotate/reset. +- 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/entity/PolarisPrincipalSecrets.java b/polaris-core/src/main/java/org/apache/polaris/core/entity/PolarisPrincipalSecrets.java index 94c8e2cb24..5a57a983a8 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,8 @@ import com.fasterxml.jackson.annotation.JsonCreator; import com.fasterxml.jackson.annotation.JsonProperty; +import java.nio.charset.StandardCharsets; +import java.security.MessageDigest; import java.security.SecureRandom; import org.apache.polaris.core.DigestUtils; import org.jspecify.annotations.Nullable; @@ -186,19 +188,57 @@ public String getMainSecretHash() { } /** - * Credentials-generation fingerprint bound to the current main secret. Changes whenever the main - * secret is rotated or reset, so tokens carrying a prior value can be rejected on verify. + * 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. * - *

The value is derived from existing fields (a hash of {@link #mainSecretHash}) and therefore - * requires no schema change. It is not itself usable as a credential, and unlike {@code - * mainSecretHash} it is not the artifact secrets are verified against, so it is safe to embed in - * signed (but not encrypted) tokens. - * - * @return the credentials version, or {@code null} when no main secret hash is set + * @return the main credentials version, or {@code null} when no main secret hash is set */ @Nullable public String getCredentialsVersion() { - return mainSecretHash == null ? null : DigestUtils.sha256Hex(mainSecretHash); + 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 |= constantTimeEquals(credentialsVersion, mainVersion); + } + String secondaryVersion = credentialsVersionForHash(secondarySecretHash); + if (secondaryVersion != null) { + matches |= 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. + */ + @Nullable + private String credentialsVersionForHash(@Nullable String secretHash) { + if (secretHash == null || secretHash.isEmpty()) { + return null; + } + String salt = secretSalt == null ? "" : secretSalt; + return DigestUtils.sha256Hex(secretHash + ":" + salt); + } + + private static boolean constantTimeEquals(String a, String b) { + return MessageDigest.isEqual( + a.getBytes(StandardCharsets.UTF_8), b.getBytes(StandardCharsets.UTF_8)); } public String getSecondarySecretHash() { 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 288c9d75d0..eab5a1417d 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 @@ -114,11 +114,10 @@ private InternalPolarisToken verifyInternal(String token) { } /** - * Rejects tokens that were minted against a previous credentials generation (i.e. before the - * principal's secrets were last rotated or reset). Tokens minted before credentials binding was - * introduced carry no claim and are accepted so existing clients are not broken on upgrade; - * binding activates for them on the next rotate/reset, when their principal's version changes and - * all bound tokens are rejected. + * 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. */ private void assertCredentialsVersionCurrent(String clientId, String credentialsVersion) { if (credentialsVersion == null) { @@ -133,8 +132,7 @@ private void assertCredentialsVersionCurrent(String clientId, String credentials if (!secretsResult.isSuccess() || secretsResult.getPrincipalSecrets() == null) { throw new NotAuthorizedException("Failed to verify the token"); } - String currentVersion = secretsResult.getPrincipalSecrets().getCredentialsVersion(); - if (!constantTimeEquals(credentialsVersion, currentVersion)) { + if (!secretsResult.getPrincipalSecrets().matchesCredentialsVersion(credentialsVersion)) { throw new NotAuthorizedException("Failed to verify the token"); } } 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 index a8c360f174..7dacd413c6 100644 --- 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 @@ -27,6 +27,7 @@ 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; @@ -39,9 +40,10 @@ import org.mockito.Mockito; /** - * Internal JWTs are bound to the principal's current credentials version (derived from the main - * secret hash) so rotate/reset invalidates outstanding access tokens. Tokens minted before - * credentials binding existed (no claim) remain valid until expiry for upgrade compatibility. + * 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 { @@ -75,7 +77,7 @@ void setUp() { } @Test - void mintedTokenCarriesDerivedCredentialsVersion() { + void mintedTokenCarriesSaltedCredentialsVersion() { TokenResponse response = broker.generateFromClientSecrets( CLIENT_ID, @@ -87,8 +89,10 @@ void mintedTokenCarriesDerivedCredentialsVersion() { DecodedJWT jwt = JWT.decode(response.getAccessToken()); String claim = jwt.getClaim(JWTBroker.CLAIM_KEY_CREDENTIALS_VERSION).asString(); assertThat(claim).isEqualTo(secrets.getCredentialsVersion()); - // The claim must not expose the raw secret-verification artifact. + // 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 @@ -104,7 +108,7 @@ void verifySucceedsForTokenBoundToCurrentSecrets() { } @Test - void verifyFailsAfterSecretRotate() { + void verifyStillSucceedsAfterOneRotateViaSecondaryGrace() { TokenResponse response = broker.generateFromClientSecrets( CLIENT_ID, @@ -114,7 +118,26 @@ void verifyFailsAfterSecretRotate() { TokenType.ACCESS_TOKEN); String accessToken = response.getAccessToken(); - // Simulate rotate: main secret hash changes, and so does the derived credentials version. + // 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)); @@ -125,7 +148,7 @@ void verifyFailsAfterSecretRotate() { } @Test - void tokenExchangeFailsAfterSecretRotate() { + void tokenExchangeFailsAfterSecondRotate() { TokenResponse original = broker.generateFromClientSecrets( CLIENT_ID, @@ -134,6 +157,7 @@ void tokenExchangeFailsAfterSecretRotate() { SCOPE, TokenType.ACCESS_TOKEN); + secrets.rotateSecrets(secrets.getMainSecretHash()); secrets.rotateSecrets(secrets.getMainSecretHash()); when(metaStore.loadPrincipalSecrets(callContext, CLIENT_ID)) .thenReturn(new PrincipalSecretsResult(secrets)); @@ -148,20 +172,44 @@ void tokenExchangeFailsAfterSecretRotate() { 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)); + + 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()); + } + @Test void verifyAcceptsLegacyTokenWithoutCredentialsVersionClaim() { - // Craft a token with signature/issuer/active but no cv claim (pre-binding tokens). These - // must keep working so existing clients are not broken on upgrade. String legacy = legacyToken(); - assertThat(broker.verify(legacy).getPrincipalId()).isEqualTo(PRINCIPAL_ID); } @Test void legacyTokenWithoutClaimIsNotBoundByRotate() { - // Documents the accepted trade-off: tokens minted before credentials binding existed carry - // no version to check, so they remain valid until expiry even after rotate/reset. All - // tokens minted after the upgrade are bound. String legacy = legacyToken(); secrets.rotateSecrets(secrets.getMainSecretHash()); @@ -172,16 +220,7 @@ void legacyTokenWithoutClaimIsNotBoundByRotate() { } @Test - void newTokenAfterRotateUsesNewVersionAndVerifies() { - TokenResponse before = - broker.generateFromClientSecrets( - CLIENT_ID, - MAIN_SECRET, - TokenRequestValidator.CLIENT_CREDENTIALS, - SCOPE, - TokenType.ACCESS_TOKEN); - String oldToken = before.getAccessToken(); - + void newTokenAfterRotateUsesNewMainVersion() { secrets.rotateSecrets(secrets.getMainSecretHash()); String newMain = secrets.getMainSecret(); when(metaStore.loadPrincipalSecrets(callContext, CLIENT_ID)) @@ -201,7 +240,6 @@ void newTokenAfterRotateUsesNewVersionAndVerifies() { .asString()) .isEqualTo(secrets.getCredentialsVersion()); assertThat(broker.verify(after.getAccessToken()).getPrincipalId()).isEqualTo(PRINCIPAL_ID); - assertThatThrownBy(() -> broker.verify(oldToken)).isInstanceOf(NotAuthorizedException.class); } @Test From aa5b85cb843790f1403a1d4f6122ec3d719bcd0a Mon Sep 17 00:00:00 2001 From: vignesh_A Date: Tue, 21 Jul 2026 02:07:59 +0530 Subject: [PATCH 4/5] refactor(auth): move constant-time compare to DigestUtils, reject nulls --- .../org/apache/polaris/core/DigestUtils.java | 12 +++++ .../core/entity/PolarisPrincipalSecrets.java | 11 +---- .../apache/polaris/core/DigestUtilsTest.java | 45 +++++++++++++++++++ .../auth/internal/broker/JWTBroker.java | 11 ----- .../JWTBrokerCredentialsBindingTest.java | 7 --- 5 files changed, 59 insertions(+), 27 deletions(-) create mode 100644 polaris-core/src/test/java/org/apache/polaris/core/DigestUtilsTest.java 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 5a57a983a8..aef0e7d06d 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,8 +20,6 @@ import com.fasterxml.jackson.annotation.JsonCreator; import com.fasterxml.jackson.annotation.JsonProperty; -import java.nio.charset.StandardCharsets; -import java.security.MessageDigest; import java.security.SecureRandom; import org.apache.polaris.core.DigestUtils; import org.jspecify.annotations.Nullable; @@ -214,11 +212,11 @@ public boolean matchesCredentialsVersion(@Nullable String credentialsVersion) { boolean matches = false; String mainVersion = credentialsVersionForHash(mainSecretHash); if (mainVersion != null) { - matches |= constantTimeEquals(credentialsVersion, mainVersion); + matches |= DigestUtils.constantTimeEquals(credentialsVersion, mainVersion); } String secondaryVersion = credentialsVersionForHash(secondarySecretHash); if (secondaryVersion != null) { - matches |= constantTimeEquals(credentialsVersion, secondaryVersion); + matches |= DigestUtils.constantTimeEquals(credentialsVersion, secondaryVersion); } return matches; } @@ -236,11 +234,6 @@ private String credentialsVersionForHash(@Nullable String secretHash) { return DigestUtils.sha256Hex(secretHash + ":" + salt); } - private static boolean constantTimeEquals(String a, String b) { - return MessageDigest.isEqual( - a.getBytes(StandardCharsets.UTF_8), b.getBytes(StandardCharsets.UTF_8)); - } - 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 eab5a1417d..e5b727b67e 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 @@ -23,8 +23,6 @@ import com.auth0.jwt.interfaces.DecodedJWT; import com.auth0.jwt.interfaces.JWTVerifier; import com.google.common.annotations.VisibleForTesting; -import java.nio.charset.StandardCharsets; -import java.security.MessageDigest; import java.time.Instant; import java.time.temporal.ChronoUnit; import java.util.Optional; @@ -137,15 +135,6 @@ private void assertCredentialsVersionCurrent(String clientId, String credentials } } - @VisibleForTesting - static boolean constantTimeEquals(String a, String b) { - if (a == null || b == null) { - return false; - } - return MessageDigest.isEqual( - a.getBytes(StandardCharsets.UTF_8), b.getBytes(StandardCharsets.UTF_8)); - } - @Override public TokenResponse generateFromToken( TokenType subjectTokenType, 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 index 7dacd413c6..2a09cdc5a9 100644 --- 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 @@ -242,13 +242,6 @@ void newTokenAfterRotateUsesNewMainVersion() { assertThat(broker.verify(after.getAccessToken()).getPrincipalId()).isEqualTo(PRINCIPAL_ID); } - @Test - void constantTimeEquals() { - assertThat(JWTBroker.constantTimeEquals("abc", "abc")).isTrue(); - assertThat(JWTBroker.constantTimeEquals("abc", "abd")).isFalse(); - assertThat(JWTBroker.constantTimeEquals(null, "a")).isFalse(); - } - private String legacyToken() { return JWT.create() .withIssuer("polaris") From a66743e91f05cd722b4f63896aa02f9012aa3737 Mon Sep 17 00:00:00 2001 From: vignesh_A Date: Tue, 21 Jul 2026 02:23:50 +0530 Subject: [PATCH 5/5] refactor(auth): reuse secrets loaded during token verify on re-mint --- .../core/entity/PolarisPrincipalSecrets.java | 6 +- .../auth/internal/broker/JWTBroker.java | 61 ++++++++++++++----- .../JWTBrokerCredentialsBindingTest.java | 3 + 3 files changed, 51 insertions(+), 19 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 aef0e7d06d..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 @@ -223,15 +223,15 @@ public boolean matchesCredentialsVersion(@Nullable String credentialsVersion) { /** * 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. + * 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; } - String salt = secretSalt == null ? "" : secretSalt; - return DigestUtils.sha256Hex(secretHash + ":" + salt); + return DigestUtils.sha256Hex(secretHash + ":" + secretSalt); } public String getSecondarySecretHash() { 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 e5b727b67e..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 @@ -38,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; @@ -88,20 +89,23 @@ 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); String clientId = decodedJWT.getClaim(CLAIM_KEY_CLIENT_ID).asString(); String credentialsVersion = decodedJWT.getClaim(CLAIM_KEY_CREDENTIALS_VERSION).asString(); - assertCredentialsVersionCurrent(clientId, credentialsVersion); - return InternalPolarisToken.of( - decodedJWT.getSubject(), - decodedJWT.getClaim(CLAIM_KEY_PRINCIPAL_ID).asLong(), - clientId, - decodedJWT.getClaim(CLAIM_KEY_SCOPE).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; @@ -116,11 +120,16 @@ private InternalPolarisToken verifyInternal(String token) { * 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) */ - private void assertCredentialsVersionCurrent(String clientId, String credentialsVersion) { + @Nullable + private PolarisPrincipalSecrets assertCredentialsVersionCurrent( + String clientId, String credentialsVersion) { if (credentialsVersion == null) { // Legacy token minted before credentials binding existed. - return; + return null; } if (clientId == null || clientId.isBlank() || credentialsVersion.isBlank()) { throw new NotAuthorizedException("Failed to verify the token"); @@ -133,6 +142,7 @@ private void assertCredentialsVersionCurrent(String clientId, String credentials if (!secretsResult.getPrincipalSecrets().matchesCredentialsVersion(credentialsVersion)) { throw new NotAuthorizedException("Failed to verify the token"); } + return secretsResult.getPrincipalSecrets(); } @Override @@ -151,14 +161,15 @@ public TokenResponse generateFromToken( if (subjectToken == null || subjectToken.isBlank()) { return TokenResponse.of(OAuthError.invalid_request); } - InternalPolarisToken decodedToken; + VerifiedToken verified; try { // verifyInternal enforces credentials-version binding (rejects after rotate/reset). - decodedToken = verifyInternal(subjectToken); + 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()) { @@ -178,9 +189,20 @@ public TokenResponse generateFromToken( } tokenScope = scope; } - Optional credentialsVersion = loadCurrentCredentialsVersion(decodedToken.getClientId()); - if (credentialsVersion.isEmpty()) { - return TokenResponse.of(OAuthError.unauthorized_client); + 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( @@ -188,7 +210,7 @@ public TokenResponse generateFromToken( decodedToken.getPrincipalId(), decodedToken.getClientId(), tokenScope, - credentialsVersion.get()); + credentialsVersion); return TokenResponse.of( tokenString, TokenType.ACCESS_TOKEN.getValue(), maxTokenGenerationInSeconds); } @@ -293,4 +315,11 @@ private Optional authenticateWithClientSecrets( } 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 index 2a09cdc5a9..5aec0b2a63 100644 --- 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 @@ -185,6 +185,7 @@ void tokenExchangeSucceedsAfterOneRotate() { secrets.rotateSecrets(secrets.getMainSecretHash()); when(metaStore.loadPrincipalSecrets(callContext, CLIENT_ID)) .thenReturn(new PrincipalSecretsResult(secrets)); + Mockito.clearInvocations(metaStore); TokenResponse exchanged = broker.generateFromToken( @@ -200,6 +201,8 @@ void tokenExchangeSucceedsAfterOneRotate() { .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