This is an automated email from the ASF dual-hosted git repository. rcordier pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/james-project.git
commit 885fafa80670f77d0fc8b494a227b4ea65cc6271 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 | 20 +++++++++++ .../apache/james/jwt/OidcSASLConfiguration.java | 38 +++++++++++++++----- .../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, 95 insertions(+), 11 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 637f57de5f..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,17 +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 reactor.core.publisher.Mono; public class OidcJwtTokenVerifier { public static final CheckTokenClient CHECK_TOKEN_CLIENT = new DefaultCheckTokenClient(); + private static final Logger LOGGER = LoggerFactory.getLogger(OidcJwtTokenVerifier.class); private final OidcSASLConfiguration oidcSASLConfiguration; @@ -77,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 c7db3d1e99..cb59ef1811 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 @@ -38,6 +38,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() { @@ -51,6 +52,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() { @@ -106,6 +108,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"); @@ -113,7 +120,7 @@ public class OidcSASLConfiguration { Preconditions.checkNotNull(scope, "scope is mandatory"); return new OidcSASLConfiguration(jwksURL, claim, oidcConfigurationURL, scope, - introspectionEndpoint, introspectionEndpointAuthorization, userInfoEndpoint); + introspectionEndpoint, introspectionEndpointAuthorization, userInfoEndpoint, aud); } } @@ -130,6 +137,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) { @@ -139,9 +147,17 @@ public class OidcSASLConfiguration { } } + 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 URI(jwksURL).toURL(), claim, new URI(oidcConfigurationURL).toURL(), scope, Optional.ofNullable(introspectionUrl) .map(Throwing.function(value -> new URI(value).toURL())), Optional.ofNullable(configuration.getString("introspection.auth", null)), - Optional.ofNullable(userInfoUrl).map(Throwing.function(value -> new URI(value).toURL()))); + Optional.ofNullable(userInfoUrl).map(Throwing.function(value -> new URI(value).toURL())), Optional.ofNullable(aud)); } private final URL jwksURL; @@ -149,21 +165,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; } @@ -200,6 +219,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 8fe910dfba..3755d2fb2d 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 @@ -412,6 +412,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 45e27242cc..236dbb519f 100644 --- a/server/protocols/protocols-imap4/pom.xml +++ b/server/protocols/protocols-imap4/pom.xml @@ -188,7 +188,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 - -Xms1024m -Xmx2048m -Djames.sasl.oidc.force.introspect=false</argLine> + -Xms1024m -Xmx2048m -Djames.sasl.oidc.force.introspect=false -Djames.sasl.oidc.validate.aud=false</argLine> <reuseForks>true</reuseForks> <!-- Fail tests longer than 30 minutes, prevent form random locking tests --> <forkedProcessTimeoutInSeconds>1800</forkedProcessTimeoutInSeconds> diff --git a/server/protocols/protocols-lmtp/pom.xml b/server/protocols/protocols-lmtp/pom.xml index e780309b15..cf4984acf2 100644 --- a/server/protocols/protocols-lmtp/pom.xml +++ b/server/protocols/protocols-lmtp/pom.xml @@ -195,7 +195,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 30 minutes, prevent form random locking tests --> <forkedProcessTimeoutInSeconds>1800</forkedProcessTimeoutInSeconds> diff --git a/server/protocols/protocols-smtp/pom.xml b/server/protocols/protocols-smtp/pom.xml index eae01359bd..e28a2c45f9 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 30 minutes, prevent form random locking tests --> <forkedProcessTimeoutInSeconds>1800</forkedProcessTimeoutInSeconds> --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
