This is an automated email from the ASF dual-hosted git repository. btellier pushed a commit to branch 3.8.x in repository https://gitbox.apache.org/repos/asf/james-project.git
commit 44a21f88591548ad169dac2e6bddc1091a8e62c9 Author: Benoit TELLIER <[email protected]> AuthorDate: Mon Dec 8 15:24:50 2025 +0100 [ENHANCEMENT] OIDC SASL should validat AUD --- .../org/apache/james/jwt/OidcJwtTokenVerifier.java | 50 ++++++++-------------- .../apache/james/jwt/OidcSASLConfiguration.java | 37 ++++++++++++---- .../apache/james/jwt/OidcJwtTokenVerifierTest.java | 42 ++++++++++++++++++ server/protocols/protocols-imap4/pom.xml | 2 +- server/protocols/protocols-lmtp/pom.xml | 2 +- server/protocols/protocols-smtp/pom.xml | 2 +- 6 files changed, 93 insertions(+), 42 deletions(-) diff --git a/server/protocols/jwt/src/main/java/org/apache/james/jwt/OidcJwtTokenVerifier.java b/server/protocols/jwt/src/main/java/org/apache/james/jwt/OidcJwtTokenVerifier.java index 83bd172846..7f87132bb7 100644 --- a/server/protocols/jwt/src/main/java/org/apache/james/jwt/OidcJwtTokenVerifier.java +++ b/server/protocols/jwt/src/main/java/org/apache/james/jwt/OidcJwtTokenVerifier.java @@ -21,49 +21,22 @@ package org.apache.james.jwt; import java.net.URL; import java.util.Optional; +import java.util.function.Predicate; import org.apache.james.core.Username; import org.apache.james.jwt.introspection.IntrospectionEndpoint; import org.apache.james.jwt.introspection.TokenIntrospectionResponse; import org.reactivestreams.Publisher; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; import com.google.common.annotations.VisibleForTesting; -import io.jsonwebtoken.Claims; -import io.jsonwebtoken.Header; -import io.jsonwebtoken.Jwt; -import io.jsonwebtoken.JwtException; -import io.jsonwebtoken.Jwts; import reactor.core.publisher.Mono; public class OidcJwtTokenVerifier { public static final CheckTokenClient CHECK_TOKEN_CLIENT = new DefaultCheckTokenClient(); - - public static Optional<String> verifySignatureAndExtractClaim(String jwtToken, URL jwksURL, String claimName) { - Optional<String> unverifiedClaim = getClaimWithoutSignatureVerification(jwtToken, "kid"); - PublicKeyProvider jwksPublicKeyProvider = unverifiedClaim - .map(kidValue -> JwksPublicKeyProvider.of(jwksURL, kidValue)) - .orElse(JwksPublicKeyProvider.of(jwksURL)); - return new JwtTokenVerifier(jwksPublicKeyProvider).verifyAndExtractClaim(jwtToken, claimName, String.class); - } - - public static <T> Optional<T> getClaimWithoutSignatureVerification(String token, String claimName) { - int signatureIndex = token.lastIndexOf('.'); - if (signatureIndex <= 0) { - return Optional.empty(); - } - String nonSignedToken = token.substring(0, signatureIndex + 1); - try { - Jwt<Header, Claims> headerClaims = Jwts.parserBuilder().build().parseClaimsJwt(nonSignedToken); - T claim = (T) headerClaims.getHeader().get(claimName); - if (claim == null) { - return Optional.empty(); - } - return Optional.of(claim); - } catch (JwtException e) { - return Optional.empty(); - } - } + private static final Logger LOGGER = LoggerFactory.getLogger(OidcJwtTokenVerifier.class); private final OidcSASLConfiguration oidcSASLConfiguration; @@ -109,12 +82,27 @@ public class OidcJwtTokenVerifier { .flatMap(optional -> optional.map(Mono::just).orElseGet(Mono::empty)) .flatMap(claimResult -> Mono.from(CHECK_TOKEN_CLIENT.introspect(introspectionEndpoint, jwtToken)) .filter(TokenIntrospectionResponse::active) + .filter(validateAud()) .filter(tokenIntrospectionResponse -> tokenIntrospectionResponse.claimByPropertyName(oidcSASLConfiguration.getClaim()) .map(claim -> claim.equals(claimResult)) .orElse(false)) .map(activeResponse -> claimResult)); } + private Predicate<TokenIntrospectionResponse> validateAud() { + return oidcSASLConfiguration.getAud() + .map(this::validateAud) + .orElse(any -> true); + } + + private Predicate<TokenIntrospectionResponse> validateAud(String expectedAud) { + return token -> { + boolean result = token.aud().map(expectedAud::equals).orElse(false); + LOGGER.warn("Wrong aud. Expected {} got {}", expectedAud, token.aud()); + return result; + }; + } + @VisibleForTesting Publisher<String> verifyWithUserinfo(String jwtToken, URL userinfoEndpoint) { return Mono.fromCallable(() -> verifySignatureAndExtractClaim(jwtToken)) diff --git a/server/protocols/jwt/src/main/java/org/apache/james/jwt/OidcSASLConfiguration.java b/server/protocols/jwt/src/main/java/org/apache/james/jwt/OidcSASLConfiguration.java index 403a33f966..4a0db876cd 100644 --- a/server/protocols/jwt/src/main/java/org/apache/james/jwt/OidcSASLConfiguration.java +++ b/server/protocols/jwt/src/main/java/org/apache/james/jwt/OidcSASLConfiguration.java @@ -36,6 +36,7 @@ public class OidcSASLConfiguration { private static final Logger LOGGER = LoggerFactory.getLogger(OidcSASLConfiguration.class); private static final boolean FORCE_INTROSPECT = Boolean.parseBoolean(System.getProperty("james.sasl.oidc.force.introspect", "true")); + private static final boolean VALIDATE_AUD = Boolean.parseBoolean(System.getProperty("james.sasl.oidc.validate.aud", "true")); @VisibleForTesting static Builder builder() { @@ -49,6 +50,7 @@ public class OidcSASLConfiguration { private String scope; private Optional<URL> introspectionEndpoint = Optional.empty(); private Optional<String> introspectionEndpointAuthorization = Optional.empty(); + private Optional<String> aud = Optional.empty(); private Optional<URL> userInfoEndpoint = Optional.empty(); private Builder() { @@ -104,6 +106,11 @@ public class OidcSASLConfiguration { return this; } + public Builder aud(String aud) { + this.aud = Optional.ofNullable(aud); + return this; + } + public OidcSASLConfiguration build() { Preconditions.checkNotNull(jwksURL, "jwksURL is mandatory"); Preconditions.checkNotNull(claim, "claim is mandatory"); @@ -111,7 +118,7 @@ public class OidcSASLConfiguration { Preconditions.checkNotNull(scope, "scope is mandatory"); return new OidcSASLConfiguration(jwksURL, claim, oidcConfigurationURL, scope, - introspectionEndpoint, introspectionEndpointAuthorization, userInfoEndpoint); + introspectionEndpoint, introspectionEndpointAuthorization, userInfoEndpoint, aud); } } @@ -128,6 +135,7 @@ public class OidcSASLConfiguration { String introspectionUrl = configuration.getString("introspection.url", null); String userInfoUrl = configuration.getString("userinfo.url", null); + String aud = configuration.getString("aud", null); if (introspectionUrl == null) { if (FORCE_INTROSPECT) { @@ -136,10 +144,17 @@ public class OidcSASLConfiguration { LOGGER.warn("'introspection.url' is mandatory for secure set up. This check was disabled with -Djames.sasl.oidc.force.introspect=false."); } } + if (aud == null) { + if (VALIDATE_AUD) { + throw new IllegalArgumentException("'aud' is mandatory for secure set up. Disable this check with -Djames.sasl.oidc.validate.aud=false."); + } else { + LOGGER.warn("'aud' is mandatory for secure set up. This check was disabled with -Djames.sasl.oidc.validate.aud=false."); + } + } return new OidcSASLConfiguration(new URL(jwksURL), claim, new URL(oidcConfigurationURL), scope, Optional.ofNullable(introspectionUrl) .map(Throwing.function(URL::new)), Optional.ofNullable(configuration.getString("introspection.auth", null)), - Optional.ofNullable(userInfoUrl).map(Throwing.function(URL::new))); + Optional.ofNullable(userInfoUrl).map(Throwing.function(URL::new)), Optional.ofNullable(aud)); } private final URL jwksURL; @@ -147,21 +162,24 @@ public class OidcSASLConfiguration { private final URL oidcConfigurationURL; private final String scope; private final Optional<URL> introspectionEndpoint; + private final Optional<String> aud; private final Optional<String> introspectionEndpointAuthorization; private final Optional<URL> userInfoEndpoint; private OidcSASLConfiguration(URL jwksURL, - String claim, - URL oidcConfigurationURL, - String scope, - Optional<URL> introspectionEndpoint, - Optional<String> introspectionEndpointAuthorization, - Optional<URL> userInfoEndpoint) { + String claim, + URL oidcConfigurationURL, + String scope, + Optional<URL> introspectionEndpoint, + Optional<String> introspectionEndpointAuthorization, + Optional<URL> userInfoEndpoint, + Optional<String> aud) { this.jwksURL = jwksURL; this.claim = claim; this.oidcConfigurationURL = oidcConfigurationURL; this.scope = scope; this.introspectionEndpoint = introspectionEndpoint; + this.aud = aud; this.introspectionEndpointAuthorization = introspectionEndpointAuthorization; this.userInfoEndpoint = userInfoEndpoint; } @@ -198,6 +216,9 @@ public class OidcSASLConfiguration { return getIntrospectionEndpoint().isPresent(); } + public Optional<String> getAud() { + return aud; + } public boolean isCheckTokenByUserinfoEndpoint() { return getUserInfoEndpoint().isPresent(); diff --git a/server/protocols/jwt/src/test/java/org/apache/james/jwt/OidcJwtTokenVerifierTest.java b/server/protocols/jwt/src/test/java/org/apache/james/jwt/OidcJwtTokenVerifierTest.java index 03674929b8..0aee8af43c 100644 --- a/server/protocols/jwt/src/test/java/org/apache/james/jwt/OidcJwtTokenVerifierTest.java +++ b/server/protocols/jwt/src/test/java/org/apache/james/jwt/OidcJwtTokenVerifierTest.java @@ -368,6 +368,48 @@ class OidcJwtTokenVerifierTest { .isNull(); } + @Test + void verifyWithIntrospectionShouldReturnWhenValidAud() throws Exception { + mockServer + .when(HttpRequest.request().withPath(INTROSPECTION_PATH)) + .respond(HttpResponse.response().withStatusCode(200) + .withHeader("Content-Type", "application/json") + .withBody(INTROSPECTION_RESPONSE, StandardCharsets.UTF_8)); + + assertThat(Mono.from(new OidcJwtTokenVerifier( + OidcSASLConfiguration.builder() + .jwksURL(getJwksURL()) + .scope("email") + .oidcConfigurationURL(new URL("https://whatever.nte";)) + .claim("email_address") + .aud("account") + .build()) + .verifyWithIntrospection(OidcTokenFixture.VALID_TOKEN, new IntrospectionEndpoint(getIntrospectionEndpoint(), Optional.empty()))) + .block()) + .isNotNull(); + } + + @Test + void verifyWithIntrospectionShouldReturnEmptyWhenWrongAud() throws Exception { + mockServer + .when(HttpRequest.request().withPath(INTROSPECTION_PATH)) + .respond(HttpResponse.response().withStatusCode(200) + .withHeader("Content-Type", "application/json") + .withBody(INTROSPECTION_RESPONSE, StandardCharsets.UTF_8)); + + assertThat(Mono.from(new OidcJwtTokenVerifier( + OidcSASLConfiguration.builder() + .jwksURL(getJwksURL()) + .scope("email") + .oidcConfigurationURL(new URL("https://whatever.nte";)) + .claim("email_address") + .aud("other") + .build()) + .verifyWithIntrospection(OidcTokenFixture.VALID_TOKEN, new IntrospectionEndpoint(getIntrospectionEndpoint(), Optional.empty()))) + .block()) + .isNull(); + } + @Test void verifyWithIntrospectionShouldReturnEmptyWhenINVALIDToken() { mockServer diff --git a/server/protocols/protocols-imap4/pom.xml b/server/protocols/protocols-imap4/pom.xml index cdb72157bf..69ea06b187 100644 --- a/server/protocols/protocols-imap4/pom.xml +++ b/server/protocols/protocols-imap4/pom.xml @@ -192,7 +192,7 @@ </systemPropertyVariables> <argLine>-Djava.library.path= -javaagent:"${settings.localRepository}"/org/jacoco/org.jacoco.agent/${jacoco-maven-plugin.version}/org.jacoco.agent-${jacoco-maven-plugin.version}-runtime.jar=destfile=${basedir}/target/jacoco.exec - -Xms512m -Xmx1024m -Djames.sasl.oidc.force.introspect=false</argLine> + -Xms512m -Xmx1024m -Djames.sasl.oidc.force.introspect=false -Djames.sasl.oidc.validate.aud=false</argLine> <reuseForks>true</reuseForks> <!-- Fail tests longer than 20 minutes, prevent form random locking tests --> <forkedProcessTimeoutInSeconds>1200</forkedProcessTimeoutInSeconds> diff --git a/server/protocols/protocols-lmtp/pom.xml b/server/protocols/protocols-lmtp/pom.xml index 368ef652c4..0368fa6302 100644 --- a/server/protocols/protocols-lmtp/pom.xml +++ b/server/protocols/protocols-lmtp/pom.xml @@ -190,7 +190,7 @@ </systemPropertyVariables> <argLine>-Djava.library.path= -javaagent:"${settings.localRepository}"/org/jacoco/org.jacoco.agent/${jacoco-maven-plugin.version}/org.jacoco.agent-${jacoco-maven-plugin.version}-runtime.jar=destfile=${basedir}/target/jacoco.exec - -Xms512m -Xmx1024m</argLine> + -Xms512m -Xmx1024m -Djames.sasl.oidc.force.introspect=false -Djames.sasl.oidc.validate.aud=false</argLine> <reuseForks>true</reuseForks> <!-- Fail tests longer than 20 minutes, prevent form random locking tests --> <forkedProcessTimeoutInSeconds>1200</forkedProcessTimeoutInSeconds> diff --git a/server/protocols/protocols-smtp/pom.xml b/server/protocols/protocols-smtp/pom.xml index 49a4e2d46d..2c6f13f3e7 100644 --- a/server/protocols/protocols-smtp/pom.xml +++ b/server/protocols/protocols-smtp/pom.xml @@ -226,7 +226,7 @@ </systemPropertyVariables> <argLine>-Djava.library.path= -javaagent:"${settings.localRepository}"/org/jacoco/org.jacoco.agent/${jacoco-maven-plugin.version}/org.jacoco.agent-${jacoco-maven-plugin.version}-runtime.jar=destfile=${basedir}/target/jacoco.exec - -Xms512m -Xmx1024m -Djames.sasl.oidc.force.introspect=false</argLine> + -Xms512m -Xmx1024m -Djames.sasl.oidc.force.introspect=false -Djames.sasl.oidc.validate.aud=false</argLine> <reuseForks>true</reuseForks> <!-- Fail tests longer than 20 minutes, prevent form random locking tests --> <forkedProcessTimeoutInSeconds>1200</forkedProcessTimeoutInSeconds> --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
