From de3f50f965dafdd884aace0c5e9d4fba15dc1b72 Mon Sep 17 00:00:00 2001 From: Hauke Hund Date: Wed, 14 Oct 2026 23:53:51 +0200 Subject: [PATCH] Improved OpenId token verification - Consolidated duplicated token validation code in new BaseJwtVerifier. - Added test for BaseJwtVerifier. - JWKS parsing code now supports key_ops property. - Added new properties to configure validation leeway for authentication via bearer tokens and back-channel logout tokens. - JWKS keys are now excepted for token validation if they declare use property with value "sig", declare a key_ops property array that include "verify" value or no use and key_ops properties. - The alg property in JWKS is now optional. Tokens are validated if the JWKS key specifies an algorithm and the token declared the same algorithm. Or if the JWKS key does not specify the algorithm but the token declares RS256, RS384 or RS512 and the key is an RSA key or the token declares ES256, ES384 or ES512 and the key is an EC key. Otherwise the token is rejected without trying a signature validation. fixes #586 --- .../v2/client/oidc/OidcClientDelegate.java | 13 + .../dev/dsf/bpe/api/client/oidc/Jwks.java | 3 + .../dev/dsf/bpe/client/oidc/JwksImpl.java | 12 +- .../dsf/bpe/client/oidc/OidcClientJersey.java | 102 ++-- .../auth/BackChannelLogoutAuthenticator.java | 6 +- .../common/config/AbstractJettyConfig.java | 24 +- dsf-common/dsf-common-oidc/pom.xml | 6 + .../dev/dsf/common/oidc/BaseJwtVerifier.java | 23 + .../dsf/common/oidc/BaseJwtVerifierImpl.java | 78 ++++ .../main/java/dev/dsf/common/oidc/Jwks.java | 115 +++-- .../java/dev/dsf/common/oidc/JwtVerifier.java | 2 +- .../dev/dsf/common/oidc/JwtVerifierImpl.java | 62 +-- .../dsf/common/oidc/BaseJwtVerifierTest.java | 439 ++++++++++++++++++ .../java/dev/dsf/common/oidc/JwksTest.java | 152 +++++- .../src/test/resources/log4j2.xml | 32 ++ 15 files changed, 918 insertions(+), 151 deletions(-) create mode 100644 dsf-common/dsf-common-oidc/src/main/java/dev/dsf/common/oidc/BaseJwtVerifier.java create mode 100644 dsf-common/dsf-common-oidc/src/main/java/dev/dsf/common/oidc/BaseJwtVerifierImpl.java create mode 100644 dsf-common/dsf-common-oidc/src/test/java/dev/dsf/common/oidc/BaseJwtVerifierTest.java create mode 100644 dsf-common/dsf-common-oidc/src/test/resources/log4j2.xml diff --git a/dsf-bpe/dsf-bpe-process-api-v2-impl/src/main/java/dev/dsf/bpe/v2/client/oidc/OidcClientDelegate.java b/dsf-bpe/dsf-bpe-process-api-v2-impl/src/main/java/dev/dsf/bpe/v2/client/oidc/OidcClientDelegate.java index ad9621d29..5f8a01e2c 100644 --- a/dsf-bpe/dsf-bpe-process-api-v2-impl/src/main/java/dev/dsf/bpe/v2/client/oidc/OidcClientDelegate.java +++ b/dsf-bpe/dsf-bpe-process-api-v2-impl/src/main/java/dev/dsf/bpe/v2/client/oidc/OidcClientDelegate.java @@ -15,6 +15,7 @@ */ package dev.dsf.bpe.v2.client.oidc; +import java.util.List; import java.util.Optional; import java.util.Set; import java.util.stream.Collectors; @@ -174,6 +175,11 @@ public String getUse() return delegate.getUse(); } + public List getKeyOps() + { + return delegate.getKeyOps(); + } + @Override public String getN() { @@ -238,6 +244,13 @@ public String getUse() return delegate.getUse(); } + @Override + public List getKeyOps() + { + // TODO add getKeyOps method to JwksKey in API v3 + return delegate instanceof JwksKeyApiDelegate d ? d.getKeyOps() : List.of(); + } + @Override public String getN() { diff --git a/dsf-bpe/dsf-bpe-process-api/src/main/java/dev/dsf/bpe/api/client/oidc/Jwks.java b/dsf-bpe/dsf-bpe-process-api/src/main/java/dev/dsf/bpe/api/client/oidc/Jwks.java index 06e9a611e..83931ab1f 100644 --- a/dsf-bpe/dsf-bpe-process-api/src/main/java/dev/dsf/bpe/api/client/oidc/Jwks.java +++ b/dsf-bpe/dsf-bpe-process-api/src/main/java/dev/dsf/bpe/api/client/oidc/Jwks.java @@ -15,6 +15,7 @@ */ package dev.dsf.bpe.api.client.oidc; +import java.util.List; import java.util.Optional; import java.util.Set; @@ -32,6 +33,8 @@ public static interface JwksKey String getUse(); + List getKeyOps(); + String getN(); String getE(); diff --git a/dsf-bpe/dsf-bpe-server/src/main/java/dev/dsf/bpe/client/oidc/JwksImpl.java b/dsf-bpe/dsf-bpe-server/src/main/java/dev/dsf/bpe/client/oidc/JwksImpl.java index 219af7cac..4d61f7433 100644 --- a/dsf-bpe/dsf-bpe-server/src/main/java/dev/dsf/bpe/client/oidc/JwksImpl.java +++ b/dsf-bpe/dsf-bpe-server/src/main/java/dev/dsf/bpe/client/oidc/JwksImpl.java @@ -40,6 +40,7 @@ public static class JwksKeyImpl implements JwksKey private final String alg; private final String crv; private final String use; + private final List keyOps; private final String n; private final String e; private final String x; @@ -48,14 +49,15 @@ public static class JwksKeyImpl implements JwksKey @JsonCreator public JwksKeyImpl(@JsonProperty("kid") String kid, @JsonProperty("kty") String kty, @JsonProperty("alg") String alg, @JsonProperty("crv") String crv, @JsonProperty("use") String use, - @JsonProperty("n") String n, @JsonProperty("e") String e, @JsonProperty("x") String x, - @JsonProperty("y") String y) + @JsonProperty("key_ops") List keyOps, @JsonProperty("n") String n, @JsonProperty("e") String e, + @JsonProperty("x") String x, @JsonProperty("y") String y) { this.kid = kid; this.kty = kty; this.alg = alg; this.crv = crv; this.use = use; + this.keyOps = keyOps == null ? List.of() : List.copyOf(keyOps); this.n = n; this.e = e; this.x = x; @@ -92,6 +94,12 @@ public String getUse() return use; } + @Override + public List getKeyOps() + { + return keyOps; + } + @Override public String getN() { diff --git a/dsf-bpe/dsf-bpe-server/src/main/java/dev/dsf/bpe/client/oidc/OidcClientJersey.java b/dsf-bpe/dsf-bpe-server/src/main/java/dev/dsf/bpe/client/oidc/OidcClientJersey.java index 7f96d9c16..6817a596f 100644 --- a/dsf-bpe/dsf-bpe-server/src/main/java/dev/dsf/bpe/client/oidc/OidcClientJersey.java +++ b/dsf-bpe/dsf-bpe-server/src/main/java/dev/dsf/bpe/client/oidc/OidcClientJersey.java @@ -23,14 +23,11 @@ import java.util.Base64; import java.util.List; import java.util.Objects; -import java.util.Optional; import java.util.logging.Level; import org.slf4j.Logger; import org.slf4j.LoggerFactory; -import com.auth0.jwt.JWT; -import com.auth0.jwt.algorithms.Algorithm; import com.auth0.jwt.exceptions.AlgorithmMismatchException; import com.auth0.jwt.exceptions.IncorrectClaimException; import com.auth0.jwt.exceptions.JWTDecodeException; @@ -43,6 +40,8 @@ import dev.dsf.bpe.api.client.oidc.Configuration; import dev.dsf.bpe.api.client.oidc.OidcClientException; import dev.dsf.bpe.client.oidc.JwksImpl.JwksKeyImpl; +import dev.dsf.common.oidc.BaseJwtVerifier; +import dev.dsf.common.oidc.BaseJwtVerifierImpl; import dev.dsf.common.oidc.BaseOidcClientJersey; import dev.dsf.common.oidc.Jwks; import dev.dsf.common.oidc.Jwks.JwksKey; @@ -67,10 +66,11 @@ public class OidcClientJersey extends BaseOidcClientJersey private final String clientId; private final char[] clientSecret; - private final Duration notBeforeIssuedAtExpiresAtLeewaySeconds; private final List requiredAudiences = new ArrayList<>(); private final boolean verifyAuthorizedParty; + private final BaseJwtVerifier accessTokenVerifier; + /** * @param baseUrl * not null @@ -99,7 +99,7 @@ public class OidcClientJersey extends BaseOidcClientJersey * @param readTimeout * not null * @param logRequestsAndResponses - * @param notBeforeIssuedAtExpiresAtLeewaySeconds + * @param notBeforeIssuedAtExpiresAtLeeway * not null * @param requiredAudiences * may be null @@ -108,7 +108,7 @@ public class OidcClientJersey extends BaseOidcClientJersey public OidcClientJersey(String baseUrl, String discoveryPath, String clientId, char[] clientSecret, KeyStore trustStore, KeyStore keyStore, char[] keyStorePassword, String proxySchemeHostPort, String proxyUserName, char[] proxyPassword, String userAgentValue, Duration connectTimeout, - Duration readTimeout, boolean logRequestsAndResponses, Duration notBeforeIssuedAtExpiresAtLeewaySeconds, + Duration readTimeout, boolean logRequestsAndResponses, Duration notBeforeIssuedAtExpiresAtLeeway, List requiredAudiences, boolean verifyAuthorizedParty) { super(baseUrl, discoveryPath, trustStore, keyStore, keyStorePassword, proxySchemeHostPort, proxyUserName, @@ -116,11 +116,12 @@ public OidcClientJersey(String baseUrl, String discoveryPath, String clientId, c this.clientId = clientId; this.clientSecret = clientSecret; - this.notBeforeIssuedAtExpiresAtLeewaySeconds = Objects.requireNonNull(notBeforeIssuedAtExpiresAtLeewaySeconds, - "notBeforeIssuedAtExpiresAtLeewaySeconds"); if (requiredAudiences != null) this.requiredAudiences.addAll(requiredAudiences); this.verifyAuthorizedParty = verifyAuthorizedParty; + + accessTokenVerifier = new BaseJwtVerifierImpl("Access", baseUrl, this::getJwks, + notBeforeIssuedAtExpiresAtLeeway); } private void logUnexpectedResponseAndClose(Response response) @@ -176,64 +177,42 @@ private DecodedJWT verifyAndDecodeAccessToken(String accessToken, Jwks jwks) thr { try { - DecodedJWT decoded = JWT.decode(accessToken); - String keyId = decoded.getKeyId(); - - if (keyId == null) - throw new OidcClientException("Access token has no kid property"); - - Optional key = jwks.getKey(keyId); - if (key.isEmpty() || !key.get().use().equals("sig")) - throw new OidcClientException("Access token key with kid '" + keyId + "' and use 'sig' not in JWKS"); + Verification v = accessTokenVerifier.createVerification(accessToken); - Optional algorithm = key.flatMap(JwksKey::toAlgorithm); - if (algorithm.isEmpty()) - { - throw new OidcClientException("Access token key with kid '" + keyId - + "' has unsupported type (kty) / algorithm (alg) / key-size in JWKS"); - } - - try - { - Verification v = JWT.require(algorithm.get()) - .acceptLeeway(notBeforeIssuedAtExpiresAtLeewaySeconds.getSeconds()); + if (requiredAudiences.size() == 1) + v.withAudience(requiredAudiences.get(0)); + else if (requiredAudiences.size() > 1) + v.withAudience(requiredAudiences.toArray(String[]::new)); - if (requiredAudiences.size() == 1) - v.withAudience(requiredAudiences.get(0)); - else if (requiredAudiences.size() > 1) - v.withAudience(requiredAudiences.toArray(String[]::new)); + if (verifyAuthorizedParty) + v.withClaim("azp", clientId); - if (verifyAuthorizedParty) - v.withClaim("azp", clientId); - - return v.build().verify(decoded); - } - catch (AlgorithmMismatchException e) - { - throw new OidcClientException( - "JWT verification failed: algorithm not '" + algorithm.get().getName() + "'", e); - } - catch (SignatureVerificationException e) - { - throw new OidcClientException("JWT verification failed: signature invalid", e); - } - catch (TokenExpiredException e) - { - throw new OidcClientException("JWT verification failed: token expired", e); - } - catch (IncorrectClaimException e) - { - throw new OidcClientException("JWT verification failed: claim contained unexpected value", e); - } - catch (JWTVerificationException e) - { - throw new OidcClientException("JWT verification failed", e); - } + return v.build().verify(accessToken); + } + catch (AlgorithmMismatchException e) + { + throw new OidcClientException("JWT verification failed: JWKS/token algorithm mismatch", e); + } + catch (SignatureVerificationException e) + { + throw new OidcClientException("JWT verification failed: signature invalid", e); + } + catch (TokenExpiredException e) + { + throw new OidcClientException("JWT verification failed: token expired", e); + } + catch (IncorrectClaimException e) + { + throw new OidcClientException("JWT verification failed: claim contained unexpected value", e); } catch (JWTDecodeException e) { throw new OidcClientException("Unable to parse access token", e); } + catch (JWTVerificationException e) + { + throw new OidcClientException("JWT verification failed", e); + } } public OidcClientWithDecodedJwt asOidcClientWithDecodedJwt() @@ -273,9 +252,8 @@ private OidcConfiguration toOidcConfiguration(Configuration configuration) private dev.dsf.bpe.api.client.oidc.Jwks toJwksImpl(Jwks jwks) { - return new JwksImpl(jwks.getKeys().stream().map( - k -> new JwksKeyImpl(k.kid(), k.kty(), k.alg(), k.crv(), k.use(), k.n(), k.e(), k.x(), k.y())) - .toList()); + return new JwksImpl(jwks.getKeys().stream().map(k -> new JwksKeyImpl(k.kid(), k.kty(), k.alg(), k.crv(), + k.use(), k.keyOps(), k.n(), k.e(), k.x(), k.y())).toList()); } private Configuration toConfigurationImpl(OidcConfiguration configuration) @@ -287,7 +265,7 @@ private Configuration toConfigurationImpl(OidcConfiguration configuration) private Jwks toJwks(dev.dsf.bpe.api.client.oidc.Jwks jwks) { return new Jwks(jwks.getKeys().stream().map(k -> new JwksKey(k.getKid(), k.getKty(), k.getAlg(), - k.getCrv(), k.getUse(), k.getN(), k.getE(), k.getX(), k.getY())).toList()); + k.getCrv(), k.getUse(), k.getKeyOps(), k.getN(), k.getE(), k.getX(), k.getY())).toList()); } }; } diff --git a/dsf-common/dsf-common-jetty/src/main/java/dev/dsf/common/auth/BackChannelLogoutAuthenticator.java b/dsf-common/dsf-common-jetty/src/main/java/dev/dsf/common/auth/BackChannelLogoutAuthenticator.java index 5e85c011f..43ad25310 100644 --- a/dsf-common/dsf-common-jetty/src/main/java/dev/dsf/common/auth/BackChannelLogoutAuthenticator.java +++ b/dsf-common/dsf-common-jetty/src/main/java/dev/dsf/common/auth/BackChannelLogoutAuthenticator.java @@ -114,7 +114,7 @@ public AuthenticationState validateRequest(Request request, Response response, C try { - DecodedJWT jwt = jwtVerifier.verifyBackchannelLogout(logoutTokenField.getValue()); + DecodedJWT jwt = jwtVerifier.verifyBackchannelLogoutToken(logoutTokenField.getValue()); if (!jwt.getClaims().containsKey("sub") && !jwt.getClaims().containsKey("sid")) { @@ -130,12 +130,12 @@ public AuthenticationState validateRequest(Request request, Response response, C logger.debug("Invalidating session for sub/sid {}/{}", sub, sid); - HttpSession sessionBySub = sessionsBySub.get(sub); + HttpSession sessionBySub = sub == null ? null : sessionsBySub.get(sub); if (sessionBySub != null) sessionBySub.invalidate(); // session will have been removed if found by sub and invalidated - HttpSession sessionBySid = sessionsBySid.get(sid); + HttpSession sessionBySid = sid == null ? null : sessionsBySid.get(sid); if (sessionBySid != null) sessionBySid.invalidate(); diff --git a/dsf-common/dsf-common-jetty/src/main/java/dev/dsf/common/config/AbstractJettyConfig.java b/dsf-common/dsf-common-jetty/src/main/java/dev/dsf/common/config/AbstractJettyConfig.java index b7fd7f6ce..0f82cebf7 100644 --- a/dsf-common/dsf-common-jetty/src/main/java/dev/dsf/common/config/AbstractJettyConfig.java +++ b/dsf-common/dsf-common-jetty/src/main/java/dev/dsf/common/config/AbstractJettyConfig.java @@ -156,6 +156,10 @@ public abstract class AbstractJettyConfig extends AbstractCertificateConfig @Value("${dev.dsf.server.auth.oidc.bearer.token.audience:#{null}}") private String oidcBearerTokenAudience; + @Documentation(description = "OIDC bearer tokens time validation leeway for 'Not Before', 'Issued At' and 'Expires At' values") + @Value("${dev.dsf.server.auth.oidc.bearer.token.time.validation.leeway:PT10S}") + private String oidcBearerTokenNotBeforeIssuedAtExpiresAtLeeway; + @Documentation(description = "OIDC provider realm base url", example = "https://keycloak.test.com:8443/realms/example-realm-name") @Value("${dev.dsf.server.auth.oidc.provider.realm.base.url:#{null}}") private String oidcProviderRealmBaseUrl; @@ -212,6 +216,10 @@ public abstract class AbstractJettyConfig extends AbstractCertificateConfig @Value("${dev.dsf.server.auth.oidc.back.channel.logout.path:/back-channel-logout}") private String oidcBackChannelPath; + @Documentation(description = "OIDC back-channel logout tokens time validation leeway for 'Not Before', 'Issued At' and 'Expires At' values") + @Value("${dev.dsf.server.auth.oidc.back.channel.logout.token.time.validation.leeway:PT10S}") + private String oidcBackChannelNotBeforeIssuedAtExpiresAtLeeway; + @Documentation(description = "Maximum inactivity period after which the server session for OIDC logins is invalidated; the access token may expire earlier, resulting in earlier session invalidation") @Value("${dev.dsf.server.auth.oidc.session.timeout:PT30M}") private String oidcSessionTimeout; @@ -405,11 +413,23 @@ else if (oidcBackChannelPath == null) sessionHandler.addEventListener(backChannelLogoutAuthenticator); } + private Duration getOidcBearerTokenNotBeforeIssuedAtExpiresAtLeeway() + { + return assertPositive(Duration.parse(oidcBearerTokenNotBeforeIssuedAtExpiresAtLeeway)); + } + + private Duration getOidcBackChannelNotBeforeIssuedAtExpiresAtLeeway() + { + return assertPositive(Duration.parse(oidcBackChannelNotBeforeIssuedAtExpiresAtLeeway)); + } + @Bean @Lazy public JwtVerifier jwtVerifier() { - return new JwtVerifierImpl(oidcProviderRealmBaseUrl, oidcClientId, oidcBearerTokenAudience, baseOidcClient()); + return new JwtVerifierImpl(oidcProviderRealmBaseUrl, oidcClientId, oidcBearerTokenAudience, + getOidcBearerTokenNotBeforeIssuedAtExpiresAtLeeway(), + getOidcBackChannelNotBeforeIssuedAtExpiresAtLeeway(), baseOidcClient()); } @Bean @@ -461,7 +481,7 @@ private Duration oidcProviderClientTimeoutConnect() private int oidcSessionTimeout() { - long seconds = assertPositive(Duration.parse(oidcSessionTimeout)).getSeconds(); + long seconds = assertPositive(Duration.parse(oidcSessionTimeout)).toSeconds(); if (seconds >= Integer.MAX_VALUE) seconds = Integer.MAX_VALUE; diff --git a/dsf-common/dsf-common-oidc/pom.xml b/dsf-common/dsf-common-oidc/pom.xml index 7e744c147..3f28ead64 100644 --- a/dsf-common/dsf-common-oidc/pom.xml +++ b/dsf-common/dsf-common-oidc/pom.xml @@ -61,5 +61,11 @@ jersey-media-json-jackson ${jersey.version} + + + org.apache.logging.log4j + log4j-slf4j2-impl + test + \ No newline at end of file diff --git a/dsf-common/dsf-common-oidc/src/main/java/dev/dsf/common/oidc/BaseJwtVerifier.java b/dsf-common/dsf-common-oidc/src/main/java/dev/dsf/common/oidc/BaseJwtVerifier.java new file mode 100644 index 000000000..f51ee7326 --- /dev/null +++ b/dsf-common/dsf-common-oidc/src/main/java/dev/dsf/common/oidc/BaseJwtVerifier.java @@ -0,0 +1,23 @@ +/* + * Copyright 2018-2025 Heilbronn University of Applied Sciences + * + * Licensed 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 dev.dsf.common.oidc; + +import com.auth0.jwt.interfaces.Verification; + +public interface BaseJwtVerifier +{ + Verification createVerification(String token) throws OidcClientException; +} diff --git a/dsf-common/dsf-common-oidc/src/main/java/dev/dsf/common/oidc/BaseJwtVerifierImpl.java b/dsf-common/dsf-common-oidc/src/main/java/dev/dsf/common/oidc/BaseJwtVerifierImpl.java new file mode 100644 index 000000000..4d2ab2fea --- /dev/null +++ b/dsf-common/dsf-common-oidc/src/main/java/dev/dsf/common/oidc/BaseJwtVerifierImpl.java @@ -0,0 +1,78 @@ +/* + * Copyright 2018-2025 Heilbronn University of Applied Sciences + * + * Licensed 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 dev.dsf.common.oidc; + +import java.time.Duration; +import java.util.Objects; +import java.util.Optional; +import java.util.function.Supplier; + +import com.auth0.jwt.JWT; +import com.auth0.jwt.algorithms.Algorithm; +import com.auth0.jwt.interfaces.DecodedJWT; +import com.auth0.jwt.interfaces.Verification; + +import dev.dsf.common.oidc.Jwks.JwksKey; + +public class BaseJwtVerifierImpl implements BaseJwtVerifier +{ + private final String tokenType; + private final String issuer; + private final Supplier jwksSupplier; + private final Duration notBeforeIssuedAtExpiresAtLeeway; + + /** + * @param tokenType + * not null + * @param issuer + * not null + * @param jwksSupplier + * not null + * @param notBeforeIssuedAtExpiresAtLeeway + * not null + */ + public BaseJwtVerifierImpl(String tokenType, String issuer, Supplier jwksSupplier, + Duration notBeforeIssuedAtExpiresAtLeeway) + { + this.tokenType = Objects.requireNonNull(tokenType, "tokenType"); + this.issuer = Objects.requireNonNull(issuer, "issuer"); + this.jwksSupplier = Objects.requireNonNull(jwksSupplier, "jwksSupplier"); + this.notBeforeIssuedAtExpiresAtLeeway = Objects.requireNonNull(notBeforeIssuedAtExpiresAtLeeway, + "notBeforeIssuedAtExpiresAtLeeway"); + } + + @Override + public Verification createVerification(String token) throws OidcClientException + { + DecodedJWT decoded = JWT.decode(token); + String keyId = decoded.getKeyId(); + String alg = decoded.getAlgorithm(); + + Optional key = jwksSupplier.get().getKey(keyId); + if (key.isEmpty()) + throw new OidcClientException(tokenType + " token key not in JWKS"); + else if (!key.get().isSignatureVerifyKey()) + throw new OidcClientException(tokenType + " token key with use 'sig' or key_ops 'verify' not in JWKS"); + + Optional algorithm = key.flatMap(k -> k.toAlgorithm(alg)); + if (algorithm.isEmpty()) + throw new OidcClientException( + tokenType + " token key has unsupported type (kty) / algorithm (alg) / key-size"); + + return JWT.require(algorithm.get()).withIssuer(issuer) + .acceptLeeway(notBeforeIssuedAtExpiresAtLeeway.toSeconds()); + } +} diff --git a/dsf-common/dsf-common-oidc/src/main/java/dev/dsf/common/oidc/Jwks.java b/dsf-common/dsf-common-oidc/src/main/java/dev/dsf/common/oidc/Jwks.java index 34a1200b4..27111b909 100644 --- a/dsf-common/dsf-common-oidc/src/main/java/dev/dsf/common/oidc/Jwks.java +++ b/dsf-common/dsf-common-oidc/src/main/java/dev/dsf/common/oidc/Jwks.java @@ -59,46 +59,56 @@ public class Jwks @JsonIgnoreProperties(ignoreUnknown = true) public static record JwksKey(@JsonProperty("kid") String kid, @JsonProperty("kty") String kty, @JsonProperty("alg") String alg, @JsonProperty("crv") String crv, @JsonProperty("use") String use, - @JsonProperty("n") String n, @JsonProperty("e") String e, @JsonProperty("x") String x, - @JsonProperty("y") String y) + @JsonProperty("key_ops") List keyOps, @JsonProperty("n") String n, @JsonProperty("e") String e, + @JsonProperty("x") String x, @JsonProperty("y") String y) { @JsonCreator public JwksKey(@JsonProperty("kid") String kid, @JsonProperty("kty") String kty, @JsonProperty("alg") String alg, @JsonProperty("crv") String crv, @JsonProperty("use") String use, - @JsonProperty("n") String n, @JsonProperty("e") String e, @JsonProperty("x") String x, - @JsonProperty("y") String y) + @JsonProperty("key_ops") List keyOps, @JsonProperty("n") String n, @JsonProperty("e") String e, + @JsonProperty("x") String x, @JsonProperty("y") String y) { this.kid = kid; this.kty = kty; this.alg = alg; this.crv = crv; this.use = use; + this.keyOps = keyOps == null ? List.of() : List.copyOf(keyOps); this.n = n; this.e = e; this.x = x; this.y = y; } + public boolean isSignatureVerifyKey() + { + boolean useOk = use == null || "sig".equals(use); + boolean keyOpsOk = keyOps.isEmpty() || keyOps.contains("verify"); + + return useOk && keyOpsOk; + } + /** * @return algorithm for the enclosed key material + * @param jwtAlg * @throws JwksException * if {@link Algorithm} can't be created or is not supported for the enclosed key material */ - public Optional toAlgorithm() throws JwksException + public Optional toAlgorithm(String jwtAlg) throws JwksException { return Optional.ofNullable(switch (kty) { - case "RSA" -> toRsaAlgorithm(); - case "EC" -> toEcdsaAlgorithm(); + case "RSA" -> toRsaAlgorithm(jwtAlg); + case "EC" -> toEcdsaAlgorithm(jwtAlg); default -> { - logger.warn("JWKS kty property value '{}' not one of 'RSA' or 'EC'", kty); + logger.warn("JWKS kty property value not one of 'RSA' or 'EC'"); yield null; } }); } - private Algorithm toRsaAlgorithm() + private Algorithm toRsaAlgorithm(String jwtAlg) { RSAPublicKey key = toRsaPublicKey(n, e); @@ -107,20 +117,42 @@ private Algorithm toRsaAlgorithm() RSAKeyProvider keyProvider = toRsaKeyProvider(key, kid); - return switch (alg) + if (alg != null && alg.equals(jwtAlg)) { - case "RS256" -> Algorithm.RSA256(keyProvider); - case "RS384" -> Algorithm.RSA384(keyProvider); - case "RS512" -> Algorithm.RSA512(keyProvider); - - default -> { - logger.warn("JWKS alg property value '{}' not one of 'RS256', 'RS384' or 'RS512'", alg); - yield null; - } - }; + return switch (alg) + { + case "RS256" -> Algorithm.RSA256(keyProvider); + case "RS384" -> Algorithm.RSA384(keyProvider); + case "RS512" -> Algorithm.RSA512(keyProvider); + + default -> { + logger.warn("JWKS alg property value not one of 'RS256', 'RS384' or 'RS512'"); + yield null; + } + }; + } + else if (alg == null && jwtAlg != null) + { + return switch (jwtAlg) + { + case "RS256" -> Algorithm.RSA256(keyProvider); + case "RS384" -> Algorithm.RSA384(keyProvider); + case "RS512" -> Algorithm.RSA512(keyProvider); + + default -> { + logger.warn("JWT alg property value not one of 'RS256', 'RS384' or 'RS512'"); + yield null; + } + }; + } + else + { + logger.warn("JWT and JWKS alg property value not the same"); + return null; + } } - private Algorithm toEcdsaAlgorithm() + private Algorithm toEcdsaAlgorithm(String jwtAlg) { ECPublicKey key = toEcPublicKey(x, y, crv); @@ -129,17 +161,40 @@ private Algorithm toEcdsaAlgorithm() ECDSAKeyProvider keyProvider = toEcKeyProvider(key, kid); - return switch (alg) + if (alg != null && alg.equals(jwtAlg)) + { + return switch (alg) + { + case "ES256" -> Algorithm.ECDSA256(keyProvider); + case "ES384" -> Algorithm.ECDSA384(keyProvider); + case "ES512" -> Algorithm.ECDSA512(keyProvider); + + default -> { + logger.warn("JWKS alg property value not one of 'ES256', 'ES384' or 'ES512'"); + yield null; + } + }; + } + else if (alg == null && jwtAlg != null) { - case "ES256" -> Algorithm.ECDSA256(keyProvider); - case "ES384" -> Algorithm.ECDSA384(keyProvider); - case "ES512" -> Algorithm.ECDSA512(keyProvider); - default -> { - logger.warn("JWKS crv property value '{}' not one of 'ES256', 'ES384' or 'ES512'", alg); - yield null; - } - }; + return switch (jwtAlg) + { + case "ES256" -> Algorithm.ECDSA256(keyProvider); + case "ES384" -> Algorithm.ECDSA384(keyProvider); + case "ES512" -> Algorithm.ECDSA512(keyProvider); + + default -> { + logger.warn("JWT alg property value not one of 'ES256', 'ES384' or 'ES512'"); + yield null; + } + }; + } + else + { + logger.warn("JWT and JWKS alg property value not the same"); + return null; + } } private RSAKeyProvider toRsaKeyProvider(RSAPublicKey key, String kid) @@ -235,7 +290,7 @@ private ECPublicKey toEcPublicKey(String x, String y, String crv) case "P-521" -> toEcPublicKey(xCoordinate, yCoordinate, new ECGenParameterSpec("secp521r1")); default -> { - logger.warn("JWKS crv property value '{}' not one of 'P-256', 'P-384' or 'P-521'", crv); + logger.warn("JWKS crv property value not one of 'P-256', 'P-384' or 'P-521'"); yield null; } }; diff --git a/dsf-common/dsf-common-oidc/src/main/java/dev/dsf/common/oidc/JwtVerifier.java b/dsf-common/dsf-common-oidc/src/main/java/dev/dsf/common/oidc/JwtVerifier.java index 60ec1d76b..0d42f90a1 100644 --- a/dsf-common/dsf-common-oidc/src/main/java/dev/dsf/common/oidc/JwtVerifier.java +++ b/dsf-common/dsf-common-oidc/src/main/java/dev/dsf/common/oidc/JwtVerifier.java @@ -20,7 +20,7 @@ public interface JwtVerifier { - DecodedJWT verifyBackchannelLogout(String token) throws JWTVerificationException, OidcClientException; + DecodedJWT verifyBackchannelLogoutToken(String token) throws JWTVerificationException, OidcClientException; DecodedJWT verifyBearerToken(String token) throws JWTVerificationException, OidcClientException; } diff --git a/dsf-common/dsf-common-oidc/src/main/java/dev/dsf/common/oidc/JwtVerifierImpl.java b/dsf-common/dsf-common-oidc/src/main/java/dev/dsf/common/oidc/JwtVerifierImpl.java index 5afb0b7e7..b753d9924 100644 --- a/dsf-common/dsf-common-oidc/src/main/java/dev/dsf/common/oidc/JwtVerifierImpl.java +++ b/dsf-common/dsf-common-oidc/src/main/java/dev/dsf/common/oidc/JwtVerifierImpl.java @@ -15,25 +15,21 @@ */ package dev.dsf.common.oidc; +import java.time.Duration; import java.util.Objects; -import java.util.Optional; -import com.auth0.jwt.JWT; -import com.auth0.jwt.algorithms.Algorithm; import com.auth0.jwt.exceptions.JWTVerificationException; import com.auth0.jwt.interfaces.DecodedJWT; import com.auth0.jwt.interfaces.JWTVerifier; import com.auth0.jwt.interfaces.Verification; -import dev.dsf.common.oidc.Jwks.JwksKey; - public class JwtVerifierImpl implements JwtVerifier { - private final String issuer; private final String clientId; private final String bearerTokenAudience; - private final BaseOidcClient oidcClient; + private final BaseJwtVerifier backchannelLogoutTokenVerifier; + private final BaseJwtVerifier bearerTokenVerifier; /** * @param issuer @@ -42,34 +38,30 @@ public class JwtVerifierImpl implements JwtVerifier * not null * @param bearerTokenAudience * if null, uses value from clientId, will not check audience if value is blank + * @param bearerTokenNotBeforeIssuedAtExpiresAtLeeway + * not null + * @param backchannelLogoutNotBeforeIssuedAtExpiresAtLeeway + * not null * @param oidcClient * not null */ - public JwtVerifierImpl(String issuer, String clientId, String bearerTokenAudience, BaseOidcClient oidcClient) + public JwtVerifierImpl(String issuer, String clientId, String bearerTokenAudience, + Duration bearerTokenNotBeforeIssuedAtExpiresAtLeeway, + Duration backchannelLogoutNotBeforeIssuedAtExpiresAtLeeway, BaseOidcClient oidcClient) { - this.issuer = Objects.requireNonNull(issuer, "issuer"); this.clientId = Objects.requireNonNull(clientId, "clientId"); this.bearerTokenAudience = bearerTokenAudience == null ? clientId : bearerTokenAudience; - this.oidcClient = Objects.requireNonNull(oidcClient, "oidcClient"); + + this.backchannelLogoutTokenVerifier = new BaseJwtVerifierImpl("Logout", issuer, oidcClient::getJwks, + backchannelLogoutNotBeforeIssuedAtExpiresAtLeeway); + this.bearerTokenVerifier = new BaseJwtVerifierImpl("Bearer", issuer, oidcClient::getJwks, + bearerTokenNotBeforeIssuedAtExpiresAtLeeway); } @Override - public DecodedJWT verifyBackchannelLogout(String token) throws JWTVerificationException, OidcClientException + public DecodedJWT verifyBackchannelLogoutToken(String token) throws JWTVerificationException, OidcClientException { - final String keyId = JWT.decode(token).getKeyId(); - - Optional key = oidcClient.getJwks().getKey(keyId); - if (key.isEmpty() || !key.get().use().equals("sig")) - throw new OidcClientException("Logout token key with kid '" + keyId + "' and use 'sig' not in JWKS"); - - Optional algorithm = key.flatMap(JwksKey::toAlgorithm); - if (key.isEmpty()) - { - throw new OidcClientException("Logout token key with kid '" + keyId - + "' has unsupported type (kty) / algorithm (alg) / key-size in JWKS"); - } - - JWTVerifier verifier = createVerification(algorithm.get()).withAudience(clientId) + JWTVerifier verifier = backchannelLogoutTokenVerifier.createVerification(token).withAudience(clientId) .withClaim("events", (claim, _) -> claim.asMap().containsKey("http://schemas.openid.net/event/backchannel-logout")) .build(); @@ -77,28 +69,10 @@ public DecodedJWT verifyBackchannelLogout(String token) throws JWTVerificationEx return verifier.verify(token); } - private Verification createVerification(Algorithm algorithm) - { - return JWT.require(algorithm).withIssuer(issuer).acceptLeeway(1); - } - @Override public DecodedJWT verifyBearerToken(String token) throws JWTVerificationException, OidcClientException { - final String keyId = JWT.decode(token).getKeyId(); - - Optional key = oidcClient.getJwks().getKey(keyId); - if (key.isEmpty() || !key.get().use().equals("sig")) - throw new OidcClientException("Bearer token key with kid '" + keyId + "' and use 'sig' not in JWKS"); - - Optional algorithm = key.flatMap(JwksKey::toAlgorithm); - if (key.isEmpty()) - { - throw new OidcClientException("Bearer token key with kid '" + keyId - + "' has unsupported type (kty) / algorithm (alg) / key-size in JWKS"); - } - - Verification verification = createVerification(algorithm.get()); + Verification verification = bearerTokenVerifier.createVerification(token); if (!bearerTokenAudience.isBlank()) verification.withAnyOfAudience(bearerTokenAudience); diff --git a/dsf-common/dsf-common-oidc/src/test/java/dev/dsf/common/oidc/BaseJwtVerifierTest.java b/dsf-common/dsf-common-oidc/src/test/java/dev/dsf/common/oidc/BaseJwtVerifierTest.java new file mode 100644 index 000000000..8bfa332ad --- /dev/null +++ b/dsf-common/dsf-common-oidc/src/test/java/dev/dsf/common/oidc/BaseJwtVerifierTest.java @@ -0,0 +1,439 @@ +/* + * Copyright 2018-2025 Heilbronn University of Applied Sciences + * + * Licensed 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 dev.dsf.common.oidc; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertThrows; + +import java.math.BigInteger; +import java.nio.charset.StandardCharsets; +import java.security.KeyPair; +import java.security.KeyPairGenerator; +import java.security.interfaces.ECPrivateKey; +import java.security.interfaces.ECPublicKey; +import java.security.interfaces.RSAPrivateKey; +import java.security.interfaces.RSAPublicKey; +import java.security.spec.ECGenParameterSpec; +import java.time.Duration; +import java.time.Instant; +import java.util.Arrays; +import java.util.Base64; +import java.util.Date; +import java.util.List; +import java.util.stream.Collectors; + +import org.junit.Before; +import org.junit.Test; + +import com.auth0.jwt.JWT; +import com.auth0.jwt.JWTVerifier; +import com.auth0.jwt.algorithms.Algorithm; +import com.auth0.jwt.exceptions.JWTVerificationException; +import com.auth0.jwt.interfaces.DecodedJWT; +import com.fasterxml.jackson.core.JsonProcessingException; +import com.fasterxml.jackson.databind.JsonMappingException; +import com.fasterxml.jackson.databind.ObjectMapper; + +public class BaseJwtVerifierTest +{ + private static final String ISSUER = "https://issuer.example.com"; + private static final String TOKEN_TYPE = "Bearer"; + private static final String KEY_ID = "p384-test-key"; + private static final String SUBJECT = "alice"; + + private static final Duration LEEWAY = Duration.ofSeconds(30); + + private static final ObjectMapper objectMapper = new ObjectMapper(); + + private KeyPair keyPair; + private ECPublicKey publicKey; + private ECPrivateKey privateKey; + + private BaseJwtVerifier verifier; + + @Before + public void setUp() throws Exception + { + KeyPairGenerator generator = KeyPairGenerator.getInstance("EC"); + generator.initialize(new ECGenParameterSpec("secp384r1")); + + keyPair = generator.generateKeyPair(); + + publicKey = (ECPublicKey) keyPair.getPublic(); + privateKey = (ECPrivateKey) keyPair.getPrivate(); + + Jwks jwks = loadJwks(createJwksJson(publicKey)); + + verifier = new BaseJwtVerifierImpl(TOKEN_TYPE, ISSUER, () -> jwks, LEEWAY); + } + + private static Jwks loadJwks(String jwksJson) throws JsonMappingException, JsonProcessingException + { + return objectMapper.readValue(jwksJson, Jwks.class); + } + + private static String createJwksJson(ECPublicKey publicKey) + { + return """ + { + "keys": [ + { + "kty": "EC", + "use": "sig", + "key_ops": ["verify"], + "alg": "ES384", + "crv": "P-384", + "kid": "%s", + "x": "%s", + "y": "%s" + } + ] + } + """.formatted(KEY_ID, encodeCoordinate(publicKey.getW().getAffineX()), + encodeCoordinate(publicKey.getW().getAffineY())); + } + + private static String encodeCoordinate(BigInteger coordinate) + { + byte[] bytes = coordinate.toByteArray(); + byte[] fixed = new byte[48]; + + int sourceOffset = bytes.length > 48 ? bytes.length - 48 : 0; + int length = bytes.length - sourceOffset; + + System.arraycopy(bytes, sourceOffset, fixed, 48 - length, length); + + return Base64.getUrlEncoder().withoutPadding().encodeToString(fixed); + } + + @Test + public void shouldVerifyJwtSignedWithP384Key() throws Exception + { + Instant now = Instant.now(); + + String token = JWT.create().withKeyId(KEY_ID).withIssuer(ISSUER).withSubject(SUBJECT) + .withIssuedAt(Date.from(now.minusSeconds(10))).withNotBefore(Date.from(now.minusSeconds(5))) + .withExpiresAt(Date.from(now.plusSeconds(300))).sign(Algorithm.ECDSA384(null, privateKey)); + + JWTVerifier jwtVerifier = verifier.createVerification(token).build(); + + DecodedJWT decoded = jwtVerifier.verify(token); + + assertEquals(ISSUER, decoded.getIssuer()); + assertEquals(SUBJECT, decoded.getSubject()); + assertEquals(KEY_ID, decoded.getKeyId()); + assertEquals("ES384", decoded.getAlgorithm()); + } + + @Test + public void shouldRejectJwtSignedWithDifferentP384Key() throws Exception + { + KeyPairGenerator generator = KeyPairGenerator.getInstance("EC"); + generator.initialize(new ECGenParameterSpec("secp384r1")); + KeyPair differentKeyPair = generator.generateKeyPair(); + + Instant now = Instant.now(); + + String token = JWT.create().withKeyId(KEY_ID).withIssuer(ISSUER).withSubject(SUBJECT) + .withIssuedAt(Date.from(now.minusSeconds(10))).withNotBefore(Date.from(now.minusSeconds(5))) + .withExpiresAt(Date.from(now.plusSeconds(300))) + .sign(Algorithm.ECDSA384(null, (ECPrivateKey) differentKeyPair.getPrivate())); + + JWTVerifier jwtVerifier = verifier.createVerification(token).build(); + + assertThrows(JWTVerificationException.class, () -> jwtVerifier.verify(token)); + } + + @Test + public void shouldRejectWrongIssuer() throws Exception + { + Instant now = Instant.now(); + + String token = JWT.create().withKeyId(KEY_ID).withIssuer("https://wrong-issuer.example.com") + .withSubject(SUBJECT).withIssuedAt(Date.from(now.minusSeconds(10))) + .withNotBefore(Date.from(now.minusSeconds(5))).withExpiresAt(Date.from(now.plusSeconds(300))) + .sign(Algorithm.ECDSA384(null, privateKey)); + + JWTVerifier jwtVerifier = verifier.createVerification(token).build(); + + assertThrows(JWTVerificationException.class, () -> jwtVerifier.verify(token)); + } + + @Test + public void shouldRejectExpiredTokenBeyondLeeway() throws Exception + { + Instant now = Instant.now(); + + String token = JWT.create().withKeyId(KEY_ID).withIssuer(ISSUER).withSubject(SUBJECT) + .withIssuedAt(Date.from(now.minusSeconds(120))).withNotBefore(Date.from(now.minusSeconds(120))) + .withExpiresAt(Date.from(now.minusSeconds(60))).sign(Algorithm.ECDSA384(null, privateKey)); + + JWTVerifier jwtVerifier = verifier.createVerification(token).build(); + + assertThrows(JWTVerificationException.class, () -> jwtVerifier.verify(token)); + } + + private static KeyPair generateRsaKeyPair() throws Exception + { + KeyPairGenerator generator = KeyPairGenerator.getInstance("RSA"); + generator.initialize(2048); + return generator.generateKeyPair(); + } + + private static String encodeUnsigned(BigInteger value) + { + byte[] bytes = value.toByteArray(); + + if (bytes.length > 1 && bytes[0] == 0) + bytes = Arrays.copyOfRange(bytes, 1, bytes.length); + + return Base64.getUrlEncoder().withoutPadding().encodeToString(bytes); + } + + private static String rsaJwk(KeyPair pair, String alg, String use, List keyOps) + { + RSAPublicKey publicKey = (RSAPublicKey) pair.getPublic(); + + String operations = keyOps.stream().map(op -> "\"" + op + "\"").collect(Collectors.joining(",")); + + return """ + { + "kty": "RSA", + "kid": "test-key", + "alg": "%s", + "use": "%s", + "key_ops": [%s], + "n": "%s", + "e": "%s" + } + """.formatted(alg, use, operations, encodeUnsigned(publicKey.getModulus()), + encodeUnsigned(publicKey.getPublicExponent())); + } + + private static String jwksDocument(String jwk) + { + return """ + {"keys": [%s]} + """.formatted(jwk); + } + + private static String createTokenWithHeaderAlgorithm(String alg) + { + String header = """ + {"alg":"%s","typ":"JWT","kid":"test-key"} + """.formatted(alg).trim(); + + String payload = """ + {"iss":"https://issuer.example.com","sub":"alice"} + """.trim(); + + Base64.Encoder encoder = Base64.getUrlEncoder().withoutPadding(); + + return encoder.encodeToString(header.getBytes(StandardCharsets.UTF_8)) + "." + + encoder.encodeToString(payload.getBytes(StandardCharsets.UTF_8)) + "." + + Base64.getUrlEncoder().withoutPadding().encodeToString(new byte[] { 1, 2, 3 }); + } + + private BaseJwtVerifierImpl verifierFor(String jwksJson) throws JsonMappingException, JsonProcessingException + { + Jwks jwks = loadJwks(jwksJson); + + return new BaseJwtVerifierImpl(TOKEN_TYPE, "https://issuer.example.com", () -> jwks, Duration.ofSeconds(30)); + } + + @Test + public void shouldRejectHmacAlgorithm() throws Exception + { + KeyPair rsa = generateRsaKeyPair(); + + String jwks = jwksDocument(rsaJwk(rsa, "RS256", "sig", List.of("verify"))); + + BaseJwtVerifierImpl verifier = verifierFor(jwks); + + assertThrows(OidcClientException.class, + () -> verifier.createVerification(createTokenWithHeaderAlgorithm("HS256"))); + } + + @Test + public void shouldRejectNoneAlgorithm() throws Exception + { + KeyPair rsa = generateRsaKeyPair(); + + BaseJwtVerifierImpl verifier = verifierFor(jwksDocument(rsaJwk(rsa, "RS256", "sig", List.of("verify")))); + + assertThrows(OidcClientException.class, + () -> verifier.createVerification(createTokenWithHeaderAlgorithm("none"))); + } + + @Test + public void shouldRejectUnsupportedPssAlgorithm() throws Exception + { + KeyPair rsa = generateRsaKeyPair(); + + BaseJwtVerifierImpl verifier = verifierFor(jwksDocument(rsaJwk(rsa, "RS256", "sig", List.of("verify")))); + + assertThrows(OidcClientException.class, + () -> verifier.createVerification(createTokenWithHeaderAlgorithm("PS256"))); + } + + @Test + public void shouldRejectSigningOnlyKey() throws Exception + { + KeyPair rsa = generateRsaKeyPair(); + + String jwks = jwksDocument(rsaJwk(rsa, "RS256", "sig", List.of("sign"))); + + BaseJwtVerifierImpl verifier = verifierFor(jwks); + + OidcClientException exception = assertThrows(OidcClientException.class, + () -> verifier.createVerification(createTokenWithHeaderAlgorithm("RS256"))); + + assertEquals(TOKEN_TYPE + " token key with use 'sig' or key_ops 'verify' not in JWKS", exception.getMessage()); + } + + @Test + public void shouldRejectEncryptionOnlyKey() throws Exception + { + KeyPair rsa = generateRsaKeyPair(); + + String jwks = jwksDocument(rsaJwk(rsa, "RS256", "enc", List.of("verify"))); + + BaseJwtVerifierImpl verifier = verifierFor(jwks); + + OidcClientException exception = assertThrows(OidcClientException.class, + () -> verifier.createVerification(createTokenWithHeaderAlgorithm("RS256"))); + + assertEquals(TOKEN_TYPE + " token key with use 'sig' or key_ops 'verify' not in JWKS", exception.getMessage()); + } + + @Test + public void shouldRejectContradictoryKeyMetadata() throws Exception + { + KeyPair rsa = generateRsaKeyPair(); + + // "use" allows signatures, but key_ops permits signing only. + String jwks = jwksDocument(rsaJwk(rsa, "RS256", "sig", List.of("sign"))); + + BaseJwtVerifierImpl verifier = verifierFor(jwks); + + assertThrows(OidcClientException.class, + () -> verifier.createVerification(createTokenWithHeaderAlgorithm("RS256"))); + } + + @Test + public void shouldAcceptRsaKeyWithoutAlgWhenTokenUsesRs256() throws Exception + { + KeyPair pair = generateRsaKeyPair(); + + String jwk = """ + { + "kty": "RSA", + "kid": "test-key", + "use": "sig", + "key_ops": ["verify"], + "n": "%s", + "e": "%s" + } + """.formatted(encodeUnsigned(((RSAPublicKey) pair.getPublic()).getModulus()), + encodeUnsigned(((RSAPublicKey) pair.getPublic()).getPublicExponent())); + + BaseJwtVerifierImpl verifier = verifierFor(jwksDocument(jwk)); + + Instant now = Instant.now(); + + String token = JWT.create().withKeyId("test-key").withIssuer("https://issuer.example.com").withSubject("alice") + .withIssuedAt(Date.from(now.minusSeconds(10))).withNotBefore(Date.from(now.minusSeconds(5))) + .withExpiresAt(Date.from(now.plusSeconds(300))) + .sign(Algorithm.RSA256(null, (RSAPrivateKey) pair.getPrivate())); + + DecodedJWT decoded = verifier.createVerification(token).build().verify(token); + + assertEquals("RS256", decoded.getAlgorithm()); + assertEquals("alice", decoded.getSubject()); + } + + @Test + public void shouldRejectUnsupportedAlgorithmWhenJwkHasNoAlg() throws Exception + { + KeyPair pair = generateRsaKeyPair(); + + String jwk = """ + { + "kty": "RSA", + "kid": "test-key", + "use": "sig", + "key_ops": ["verify"], + "n": "%s", + "e": "%s" + } + """.formatted(encodeUnsigned(((RSAPublicKey) pair.getPublic()).getModulus()), + encodeUnsigned(((RSAPublicKey) pair.getPublic()).getPublicExponent())); + + BaseJwtVerifierImpl verifier = verifierFor(jwksDocument(jwk)); + + String token = createTokenWithHeaderAlgorithm("HS256"); + + OidcClientException exception = assertThrows(OidcClientException.class, + () -> verifier.createVerification(token)); + + assertEquals(TOKEN_TYPE + " token key has unsupported type (kty) / algorithm (alg) / key-size", + exception.getMessage()); + } + + @Test + public void shouldRejectEcAlgorithmForRsaKeyWithoutAlg() throws Exception + { + KeyPair pair = generateRsaKeyPair(); + + String jwk = """ + { + "kty": "RSA", + "kid": "test-key", + "use": "sig", + "key_ops": ["verify"], + "n": "%s", + "e": "%s" + } + """.formatted(encodeUnsigned(((RSAPublicKey) pair.getPublic()).getModulus()), + encodeUnsigned(((RSAPublicKey) pair.getPublic()).getPublicExponent())); + + BaseJwtVerifierImpl verifier = verifierFor(jwksDocument(jwk)); + + String token = createTokenWithHeaderAlgorithm("ES384"); + + OidcClientException exception = assertThrows(OidcClientException.class, + () -> verifier.createVerification(token)); + + assertEquals(TOKEN_TYPE + " token key has unsupported type (kty) / algorithm (alg) / key-size", + exception.getMessage()); + } + + @Test + public void shouldRejectTokenWhenJwksContainsNoKeys() throws JsonMappingException, JsonProcessingException + { + Jwks jwks = loadJwks("{\"keys\": []}"); + + BaseJwtVerifierImpl verifier = new BaseJwtVerifierImpl(TOKEN_TYPE, "https://issuer.example.com", () -> jwks, + Duration.ofSeconds(30)); + + String token = createTokenWithHeaderAlgorithm("RS256"); + + OidcClientException exception = assertThrows(OidcClientException.class, + () -> verifier.createVerification(token)); + + assertEquals(TOKEN_TYPE + " token key not in JWKS", exception.getMessage()); + } +} \ No newline at end of file diff --git a/dsf-common/dsf-common-oidc/src/test/java/dev/dsf/common/oidc/JwksTest.java b/dsf-common/dsf-common-oidc/src/test/java/dev/dsf/common/oidc/JwksTest.java index 54b734b85..9aa28cc3c 100644 --- a/dsf-common/dsf-common-oidc/src/test/java/dev/dsf/common/oidc/JwksTest.java +++ b/dsf-common/dsf-common-oidc/src/test/java/dev/dsf/common/oidc/JwksTest.java @@ -18,6 +18,7 @@ import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertNull; import static org.junit.Assert.assertTrue; import java.util.Optional; @@ -73,7 +74,47 @@ public class JwksTest "crv": "P-384", "x": "3Nb3kbLLkQ6y_E8yDKrB3LZVo1aXQV12OR3yWFl9D_Xc4RZ874NngV9FVccqVS41", "y": "WZp5HbiH_OhIgvKvyRE97BRZu_i-UhZz59l74fogV4sNGLtbnbzA62eQbIAu_c_k" - } + }, + { + "kid": "kncc6492FTtclCO8qJvhS2PvYap_VabfAPOLhK3mkfB", + "kty": "RSA", + "use": "sig", + "n": "5XJkcdAy5gNu9KUrPDAnVK3hsiIT9EMDM81TN_qCcg4eDkn_MWUwwfj8gqaYj5qhJe6rmwhAedUTOvQtZcbKeIrNLPyDCGaQ4R7uIe37sxq6tPiezMBHSLpsLeb2LI2NHuY58HISAjL1cM3pEr6VatbXjomUdLWm6EcNc5ZJg9QuOXkymVdzVVDNy19EyyIzPyrBnl3_6zqwD6r031BNR7pHRstmPxqyiyc_F8559NF34Pnm0R777ItV4M1HRY580V9E84Vv1baBaeC-t_EldVCZo7hkvt5tFoiGm0xecXTKdsFgiKzp1MX8U1P-ci0BPGjYVSjJlQOc9VXGOGEk4Q", + "e": "AQAB", + "x5c": [ + "MIIClTCCAX0CBgGG0YH3jjANBgkqhkiG9w0BAQsFADAOMQwwCgYDVQQDDANkc2YwHhcNMjMwMzExMTYyODIxWhcNMzMwMzExMTYzMDAxWjAOMQwwCgYDVQQDDANkc2YwggEiMA0GCSqGSIb3DQEBAQUAA4IBDwAwggEKAoIBAQDlcmRx0DLmA270pSs8MCdUreGyIhP0QwMzzVM3+oJyDh4OSf8xZTDB+PyCppiPmqEl7qubCEB51RM69C1lxsp4is0s/IMIZpDhHu4h7fuzGrq0+J7MwEdIumwt5vYsjY0e5jnwchICMvVwzekSvpVq1teOiZR0taboRw1zlkmD1C45eTKZV3NVUM3LX0TLIjM/KsGeXf/rOrAPqvTfUE1HukdGy2Y/GrKLJz8Xznn00Xfg+ebRHvvsi1XgzUdFjnzRX0TzhW/VtoFp4L638SV1UJmjuGS+3m0WiIabTF5xdMp2wWCIrOnUxfxTU/5yLQE8aNhVKMmVA5z1VcY4YSThAgMBAAEwDQYJKoZIhvcNAQELBQADggEBAA74Cd8sgFwzcvaqYp8DRVehsl+ObzkQ/RTSQ4I8FEA5eu0526NwGP4AaYeMETkl22N7oG04Bl50oLW5kQTaPU91J3IknXPUME3EadVaXWLgheC1AEDvP2Oe7xwRwl7rJRUxfJGDs0q3onRLHNTzP8eQAHHJoJDU35EkI14TNebdDDfSGNWWB5Ucics0Gb4+WoIVWLFlndkYmPnm1a4+C3p/SXO/9ZGM35tpeHfpk8Ov+0gfYvaG9NutpfZpH1XfzgAmJGDUjufYvm2qCFVv0pqNmVrBawj/zx6g1zFCOuDcdxYG7EYxKeuCYfIMVWdIqPkCr8FISNnwhNhlqRXWvMg=" + ], + "x5t": "FXkv4BHgjJ2Qix-TESdDXlcwTOw", + "x5t#S256": "OISxUBXQFT3gnnZFlmYeBozuqcygK7_Cebq0QYh-E7g" + }, + { + "kid": "BMvf48wBJERBDMGInNfOsSiTWAnNiWGinVPnjSCeWch", + "kty": "EC", + "use": "sig", + "x5c": [ + "MIIBTTCB1AIGAZ0bZiPeMAoGCCqGSM49BAMCMBIxEDAOBgNVBAMMB2VjX3Rlc3QwHhcNMjYwMzIzMTU1MTExWhcNMzYwMzIzMTU1MjUxWjASMRAwDgYDVQQDDAdlY190ZXN0MHYwEAYHKoZIzj0CAQYFK4EEACIDYgAE3Nb3kbLLkQ6y/E8yDKrB3LZVo1aXQV12OR3yWFl9D/Xc4RZ874NngV9FVccqVS41WZp5HbiH/OhIgvKvyRE97BRZu/i+UhZz59l74fogV4sNGLtbnbzA62eQbIAu/c/kMAoGCCqGSM49BAMCA2gAMGUCMFI+m0Z21NfMGjlPpr4v64DOEzjoP4Y9fS4SJK3YIEfHvkVidDgm2A1lZD0ZRKXb7AIxAPi1ScvwtrIl0oadty4Qjg1+lWFAp943fHdFEuNE6GeagkB/dm9Nuo1wqy6hKm5O9Q==" + ], + "x5t": "3luSUTuX5v_MVEGVP_WDwy1e9GI", + "x5t#S256": "ra4lrnu_zhxYpamDEvtcxEvAFenH9CAuS5OvEiDtTho", + "crv": "P-384", + "x": "3Nb3kbLLkQ6y_E8yDKrB3LZVo1aXQV12OR3yWFl9D_Xc4RZ874NngV9FVccqVS41", + "y": "WZp5HbiH_OhIgvKvyRE97BRZu_i-UhZz59l74fogV4sNGLtbnbzA62eQbIAu_c_k" + }, + { + "kid": "kncc6492FTtclCO8qJvhS2PvYap_VabfAPOLhK3mkfC", + "kty": "RSA", + "alg": "RS256", + "key_ops": [ + "verify" + ], + "n": "5XJkcdAy5gNu9KUrPDAnVK3hsiIT9EMDM81TN_qCcg4eDkn_MWUwwfj8gqaYj5qhJe6rmwhAedUTOvQtZcbKeIrNLPyDCGaQ4R7uIe37sxq6tPiezMBHSLpsLeb2LI2NHuY58HISAjL1cM3pEr6VatbXjomUdLWm6EcNc5ZJg9QuOXkymVdzVVDNy19EyyIzPyrBnl3_6zqwD6r031BNR7pHRstmPxqyiyc_F8559NF34Pnm0R777ItV4M1HRY580V9E84Vv1baBaeC-t_EldVCZo7hkvt5tFoiGm0xecXTKdsFgiKzp1MX8U1P-ci0BPGjYVSjJlQOc9VXGOGEk4Q", + "e": "AQAB", + "x5c": [ + "MIIClTCCAX0CBgGG0YH3jjANBgkqhkiG9w0BAQsFADAOMQwwCgYDVQQDDANkc2YwHhcNMjMwMzExMTYyODIxWhcNMzMwMzExMTYzMDAxWjAOMQwwCgYDVQQDDANkc2YwggEiMA0GCSqGSIb3DQEBAQUAA4IBDwAwggEKAoIBAQDlcmRx0DLmA270pSs8MCdUreGyIhP0QwMzzVM3+oJyDh4OSf8xZTDB+PyCppiPmqEl7qubCEB51RM69C1lxsp4is0s/IMIZpDhHu4h7fuzGrq0+J7MwEdIumwt5vYsjY0e5jnwchICMvVwzekSvpVq1teOiZR0taboRw1zlkmD1C45eTKZV3NVUM3LX0TLIjM/KsGeXf/rOrAPqvTfUE1HukdGy2Y/GrKLJz8Xznn00Xfg+ebRHvvsi1XgzUdFjnzRX0TzhW/VtoFp4L638SV1UJmjuGS+3m0WiIabTF5xdMp2wWCIrOnUxfxTU/5yLQE8aNhVKMmVA5z1VcY4YSThAgMBAAEwDQYJKoZIhvcNAQELBQADggEBAA74Cd8sgFwzcvaqYp8DRVehsl+ObzkQ/RTSQ4I8FEA5eu0526NwGP4AaYeMETkl22N7oG04Bl50oLW5kQTaPU91J3IknXPUME3EadVaXWLgheC1AEDvP2Oe7xwRwl7rJRUxfJGDs0q3onRLHNTzP8eQAHHJoJDU35EkI14TNebdDDfSGNWWB5Ucics0Gb4+WoIVWLFlndkYmPnm1a4+C3p/SXO/9ZGM35tpeHfpk8Ov+0gfYvaG9NutpfZpH1XfzgAmJGDUjufYvm2qCFVv0pqNmVrBawj/zx6g1zFCOuDcdxYG7EYxKeuCYfIMVWdIqPkCr8FISNnwhNhlqRXWvMg=" + ], + "x5t": "FXkv4BHgjJ2Qix-TESdDXlcwTOw", + "x5t#S256": "OISxUBXQFT3gnnZFlmYeBozuqcygK7_Cebq0QYh-E7g" + } ] }"""; @@ -85,41 +126,138 @@ public void testDecodeJwks() throws Exception assertNotNull(jwks); assertNotNull(jwks.getKeys()); - assertEquals(3, jwks.getKeys().size()); + assertEquals(6, jwks.getKeys().size()); Optional jwk0o = jwks.getKey("kncc6492FTtclCO8qJvhS2PvYap_VabfAPOLhK3mkfA"); Optional jwk1o = jwks.getKey("Zp7ockRwsxqM6FrZlDJUOVwAxPICO2jBW0Rbk25oYGk"); Optional jwk2o = jwks.getKey("BMvf48wBJERBDMGInNfOsSiTWAnNiWGinVPnjSCeWcg"); + Optional jwk3o = jwks.getKey("kncc6492FTtclCO8qJvhS2PvYap_VabfAPOLhK3mkfB"); + Optional jwk4o = jwks.getKey("BMvf48wBJERBDMGInNfOsSiTWAnNiWGinVPnjSCeWch"); + Optional jwk5o = jwks.getKey("kncc6492FTtclCO8qJvhS2PvYap_VabfAPOLhK3mkfC"); assertTrue(jwk0o.isPresent()); assertTrue(jwk1o.isPresent()); assertTrue(jwk2o.isPresent()); + assertTrue(jwk3o.isPresent()); + assertTrue(jwk4o.isPresent()); + assertTrue(jwk5o.isPresent()); assertTrue(jwks.getKey(null).isEmpty()); assertTrue(jwks.getKey("not existing").isEmpty()); JwksKey jwk0 = jwk0o.get(); JwksKey jwk1 = jwk1o.get(); JwksKey jwk2 = jwk2o.get(); + JwksKey jwk3 = jwk3o.get(); + JwksKey jwk4 = jwk4o.get(); + JwksKey jwk5 = jwk5o.get(); assertNotNull(jwk0.kid()); assertEquals("kncc6492FTtclCO8qJvhS2PvYap_VabfAPOLhK3mkfA", jwk0.kid()); + assertNotNull(jwk0.alg()); + assertEquals("RS256", jwk0.alg()); + assertNotNull(jwk0.use()); + assertEquals("sig", jwk0.use()); + assertNotNull(jwk0.keyOps()); + assertEquals(0, jwk0.keyOps().size()); + assertNotNull(jwk1.kid()); assertEquals("Zp7ockRwsxqM6FrZlDJUOVwAxPICO2jBW0Rbk25oYGk", jwk1.kid()); + assertNotNull(jwk1.alg()); + assertEquals("RSA-OAEP", jwk1.alg()); + assertNotNull(jwk1.use()); + assertEquals("enc", jwk1.use()); + assertNotNull(jwk1.keyOps()); + assertEquals(0, jwk1.keyOps().size()); + assertNotNull(jwk2.kid()); assertEquals("BMvf48wBJERBDMGInNfOsSiTWAnNiWGinVPnjSCeWcg", jwk2.kid()); + assertNotNull(jwk2.alg()); + assertEquals("ES384", jwk2.alg()); + assertNotNull(jwk2.use()); + assertEquals("sig", jwk2.use()); + assertNotNull(jwk2.keyOps()); + assertEquals(0, jwk2.keyOps().size()); - Optional jwk0a = jwk0.toAlgorithm(); + assertNotNull(jwk3.kid()); + assertEquals("kncc6492FTtclCO8qJvhS2PvYap_VabfAPOLhK3mkfB", jwk3.kid()); + assertNull(jwk3.alg()); + assertNotNull(jwk3.use()); + assertEquals("sig", jwk3.use()); + assertNotNull(jwk3.keyOps()); + assertEquals(0, jwk3.keyOps().size()); + + assertNotNull(jwk4.kid()); + assertEquals("BMvf48wBJERBDMGInNfOsSiTWAnNiWGinVPnjSCeWch", jwk4.kid()); + assertNull(jwk4.alg()); + assertNotNull(jwk4.use()); + assertEquals("sig", jwk4.use()); + assertNotNull(jwk4.keyOps()); + assertEquals(0, jwk4.keyOps().size()); + + assertNotNull(jwk5.kid()); + assertEquals("kncc6492FTtclCO8qJvhS2PvYap_VabfAPOLhK3mkfC", jwk5.kid()); + assertEquals("RS256", jwk5.alg()); + assertNull(jwk5.use()); + assertEquals(1, jwk5.keyOps().size()); + assertEquals("verify", jwk5.keyOps().get(0)); + + Optional jwk0a = jwk0.toAlgorithm("RS256"); assertNotNull(jwk0a); assertTrue(jwk0a.isPresent()); - assertEquals("RS256", jwk0.toAlgorithm().get().getName()); + assertEquals("RS256", jwk0.toAlgorithm("RS256").get().getName()); - Optional jwk1a = jwk1.toAlgorithm(); + assertFalse(jwk0.toAlgorithm(null).isPresent()); + assertFalse(jwk0.toAlgorithm("none").isPresent()); + assertFalse(jwk0.toAlgorithm("HS256").isPresent()); + assertFalse(jwk0.toAlgorithm("ES384").isPresent()); + assertFalse(jwk0.toAlgorithm("RS384").isPresent()); + assertFalse(jwk0.toAlgorithm("RS512").isPresent()); + + Optional jwk1a = jwk1.toAlgorithm("RS256"); assertNotNull(jwk1a); assertFalse(jwk1a.isPresent()); - Optional jwk2a = jwk2.toAlgorithm(); + Optional jwk2a = jwk2.toAlgorithm("ES384"); assertNotNull(jwk2a); assertTrue(jwk2a.isPresent()); - assertEquals("ES384", jwk2.toAlgorithm().get().getName()); + assertEquals("ES384", jwk2.toAlgorithm("ES384").get().getName()); + + Optional jwk3aRs256 = jwk3.toAlgorithm("RS256"); + assertNotNull(jwk3aRs256); + assertTrue(jwk3aRs256.isPresent()); + assertEquals("RS256", jwk3.toAlgorithm("RS256").get().getName()); + + Optional jwk3aRs384 = jwk3.toAlgorithm("RS384"); + assertNotNull(jwk3aRs384); + assertTrue(jwk3aRs384.isPresent()); + assertEquals("RS384", jwk3.toAlgorithm("RS384").get().getName()); + + Optional jwk3aRs512 = jwk3.toAlgorithm("RS512"); + assertNotNull(jwk3aRs512); + assertTrue(jwk3aRs512.isPresent()); + assertEquals("RS512", jwk3.toAlgorithm("RS512").get().getName()); + + assertFalse(jwk3.toAlgorithm(null).isPresent()); + assertFalse(jwk3.toAlgorithm("none").isPresent()); + assertFalse(jwk3.toAlgorithm("HS256").isPresent()); + + Optional jwk4aEs256 = jwk4.toAlgorithm("ES256"); + assertNotNull(jwk4aEs256); + assertTrue(jwk4aEs256.isPresent()); + assertEquals("ES256", jwk4.toAlgorithm("ES256").get().getName()); + + Optional jwk4aEs384 = jwk4.toAlgorithm("ES384"); + assertNotNull(jwk4aEs384); + assertTrue(jwk4aEs384.isPresent()); + assertEquals("ES384", jwk4.toAlgorithm("ES384").get().getName()); + + Optional jwk4aEs512 = jwk4.toAlgorithm("ES512"); + assertNotNull(jwk4aEs512); + assertTrue(jwk4aEs512.isPresent()); + assertEquals("ES512", jwk4.toAlgorithm("ES512").get().getName()); + + assertFalse(jwk4.toAlgorithm(null).isPresent()); + assertFalse(jwk4.toAlgorithm("none").isPresent()); + assertFalse(jwk4.toAlgorithm("HS256").isPresent()); } } diff --git a/dsf-common/dsf-common-oidc/src/test/resources/log4j2.xml b/dsf-common/dsf-common-oidc/src/test/resources/log4j2.xml new file mode 100644 index 000000000..93e3a9dcb --- /dev/null +++ b/dsf-common/dsf-common-oidc/src/test/resources/log4j2.xml @@ -0,0 +1,32 @@ + + + + + + + + + + + + + + \ No newline at end of file