From 96415873bf645ee752a507e41a0a91a9b3de3b59 Mon Sep 17 00:00:00 2001 From: Colm O hEigeartaigh Date: Fri, 21 Aug 2026 16:30:33 +0100 Subject: [PATCH] Validate token hashes if they are available --- .../rs/security/oidc/rp/IdTokenReader.java | 16 +++- .../security/oidc/rp/IdTokenReaderTest.java | 78 ++++++++++++++++++- 2 files changed, 92 insertions(+), 2 deletions(-) diff --git a/rt/rs/security/sso/oidc/src/main/java/org/apache/cxf/rs/security/oidc/rp/IdTokenReader.java b/rt/rs/security/sso/oidc/src/main/java/org/apache/cxf/rs/security/oidc/rp/IdTokenReader.java index 38e0488e838..9ed1d18cf4e 100644 --- a/rt/rs/security/sso/oidc/src/main/java/org/apache/cxf/rs/security/oidc/rp/IdTokenReader.java +++ b/rt/rs/security/sso/oidc/src/main/java/org/apache/cxf/rs/security/oidc/rp/IdTokenReader.java @@ -21,6 +21,7 @@ import org.apache.cxf.rs.security.jose.jwt.JwtToken; import org.apache.cxf.rs.security.oauth2.client.Consumer; import org.apache.cxf.rs.security.oauth2.common.ClientAccessToken; +import org.apache.cxf.rs.security.oauth2.provider.OAuthServiceException; import org.apache.cxf.rs.security.oauth2.utils.OAuthConstants; import org.apache.cxf.rs.security.oidc.common.IdToken; import org.apache.cxf.rs.security.oidc.utils.OidcUtils; @@ -43,7 +44,7 @@ public IdToken getIdToken(String idJwtToken, Consumer client) { } public JwtToken getIdJwtToken(ClientAccessToken at, String code, Consumer client) { String idJwtToken = at.getParameters().get(OidcUtils.ID_TOKEN); - JwtToken jwt = getIdJwtToken(idJwtToken, client); + JwtToken jwt = parseAndValidateClaims(idJwtToken, client); OidcUtils.validateAccessTokenHash(at, jwt, requireAtHash); if (code != null) { // The spec requires c_hash to be present in the id_token for hybrid flows, @@ -55,7 +56,20 @@ public JwtToken getIdJwtToken(ClientAccessToken at, String code, Consumer client public JwtToken getIdJwtToken(ClientAccessToken at, Consumer client) { return getIdJwtToken(at, null, client); } + // No access_token/code is available on this path, so at_hash/c_hash binding cannot be + // verified here; reject tokens that assert such a binding rather than silently accepting it. public JwtToken getIdJwtToken(String idJwtToken, Consumer client) { + JwtToken jwt = parseAndValidateClaims(idJwtToken, client); + if (requireAtHash && jwt.getClaims().getClaim(IdToken.ACCESS_TOKEN_HASH_CLAIM) != null) { + throw new OAuthServiceException("at_hash claim cannot be validated without an access token"); + } + if (requireCodeHash && jwt.getClaims().getClaim(IdToken.AUTH_CODE_HASH_CLAIM) != null) { + throw new OAuthServiceException("c_hash claim cannot be validated without an authorization code"); + } + return jwt; + } + + private JwtToken parseAndValidateClaims(String idJwtToken, Consumer client) { JwtToken jwt = getJwtToken(idJwtToken, client.getClientSecret()); validateJwtClaims(jwt.getClaims(), client.getClientId(), true); return jwt; diff --git a/rt/rs/security/sso/oidc/src/test/java/org/apache/cxf/rs/security/oidc/rp/IdTokenReaderTest.java b/rt/rs/security/sso/oidc/src/test/java/org/apache/cxf/rs/security/oidc/rp/IdTokenReaderTest.java index 7f8bb97f2e8..d997697ed2b 100644 --- a/rt/rs/security/sso/oidc/src/test/java/org/apache/cxf/rs/security/oidc/rp/IdTokenReaderTest.java +++ b/rt/rs/security/sso/oidc/src/test/java/org/apache/cxf/rs/security/oidc/rp/IdTokenReaderTest.java @@ -24,6 +24,7 @@ import org.apache.cxf.rs.security.oauth2.common.ClientAccessToken; import org.apache.cxf.rs.security.oauth2.provider.OAuthServiceException; import org.apache.cxf.rs.security.oauth2.utils.OAuthConstants; +import org.apache.cxf.rs.security.oidc.common.IdToken; import org.apache.cxf.rs.security.oidc.utils.OidcUtils; import org.junit.Test; @@ -55,6 +56,61 @@ public void testCodeHashIsRequiredByDefaultForHybridTokenEndpointIdToken() { idTokenReader.getIdJwtToken(accessToken, "auth-code", new Consumer("client-id")); } + // The String overload has no access_token/code to check at_hash/c_hash against: if the + // id_token asserts such a claim while hash validation is required, it must be rejected. + @Test(expected = OAuthServiceException.class) + public void testStringOverloadRejectsUnverifiableAtHashByDefault() { + JwtClaims claims = validClaims(); + claims.setClaim(IdToken.ACCESS_TOKEN_HASH_CLAIM, "some-hash"); + IdTokenReader idTokenReader = new StubJwtParsingIdTokenReader(new JwtToken(claims)); + idTokenReader.setIssuerId("https://idp.example.com"); + + idTokenReader.getIdJwtToken("id-token", new Consumer("client-id")); + } + + @Test(expected = OAuthServiceException.class) + public void testStringOverloadRejectsUnverifiableCodeHashWhenRequired() { + JwtClaims claims = validClaims(); + claims.setClaim(IdToken.AUTH_CODE_HASH_CLAIM, "some-hash"); + IdTokenReader idTokenReader = new StubJwtParsingIdTokenReader(new JwtToken(claims)); + idTokenReader.setIssuerId("https://idp.example.com"); + idTokenReader.setRequireAccessTokenHash(false); + idTokenReader.setRequireCodeHash(true); + + idTokenReader.getIdJwtToken("id-token", new Consumer("client-id")); + } + + @Test + public void testStringOverloadAcceptsTokenWithoutHashClaimsByDefault() { + JwtClaims claims = validClaims(); + IdTokenReader idTokenReader = new StubJwtParsingIdTokenReader(new JwtToken(claims)); + idTokenReader.setIssuerId("https://idp.example.com"); + + assertNotNull(idTokenReader.getIdJwtToken("id-token", new Consumer("client-id"))); + } + + @Test + public void testStringOverloadAcceptsAtHashWhenNotRequired() { + JwtClaims claims = validClaims(); + claims.setClaim(IdToken.ACCESS_TOKEN_HASH_CLAIM, "some-hash"); + IdTokenReader idTokenReader = new StubJwtParsingIdTokenReader(new JwtToken(claims)); + idTokenReader.setIssuerId("https://idp.example.com"); + idTokenReader.setRequireAccessTokenHash(false); + + assertNotNull(idTokenReader.getIdJwtToken("id-token", new Consumer("client-id"))); + } + + private static JwtClaims validClaims() { + JwtClaims claims = new JwtClaims(); + claims.setIssuer("https://idp.example.com"); + claims.setSubject("subject"); + claims.setAudience("client-id"); + long now = System.currentTimeMillis() / 1000L; + claims.setIssuedAt(now); + claims.setExpiryTime(now + 300L); + return claims; + } + private static final class StubIdTokenReader extends IdTokenReader { private final JwtToken jwt; @@ -63,7 +119,27 @@ private StubIdTokenReader(JwtToken jwt) { } @Override - public JwtToken getIdJwtToken(String idJwtToken, Consumer client) { + public JwtToken getJwtToken(String wrappedJwtToken, String clientSecret) { + return jwt; + } + + @Override + public void validateJwtClaims(JwtClaims claims, String clientId, boolean validateClaimsAlways) { + // Claims validation is exercised separately in OidcClaimsValidatorTest. + } + } + + // Bypasses actual JWS parsing/signature verification so the real getIdJwtToken(String, Consumer) + // logic (claims validation + hash-claim guard) under test still executes. + private static final class StubJwtParsingIdTokenReader extends IdTokenReader { + private final JwtToken jwt; + + private StubJwtParsingIdTokenReader(JwtToken jwt) { + this.jwt = jwt; + } + + @Override + public JwtToken getJwtToken(String wrappedJwtToken, String clientSecret) { return jwt; } }