This is an automated email from the ASF dual-hosted git repository. chibenwa pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/james-project.git
commit 25971be7726ffe95505decf9921117537a1e729a Author: Quan Tran <[email protected]> AuthorDate: Wed Jun 24 10:56:46 2026 +0700 JAMES-4210 Extend shared SASL mechanisms for SMTP adoption Add shared SASL behavior needed by SMTP: transport availability on factories and mechanisms, OAuth invalid-token challenge flow, server-error convenience failure, and legacy PLAIN trailing-NUL compatibility. --- .../james/protocols/api/sasl/SaslFailure.java | 4 +++ .../sasl/OauthBearerSaslMechanismFactory.java | 6 +++- .../protocols/sasl/OidcSaslMechanismFactory.java | 18 +++++++++-- .../protocols/sasl/PlainSaslMechanismFactory.java | 12 +++++++- .../sasl/XOauth2SaslMechanismFactory.java | 6 +++- .../protocols/sasl/oidc/OAuthSaslMechanism.java | 31 +++++++++++++++++-- .../protocols/sasl/plain/PlainSaslMechanism.java | 9 ++++++ .../protocols/sasl/oidc/OidcSaslMechanismTest.java | 36 ++++++++++++++++------ 8 files changed, 104 insertions(+), 18 deletions(-) diff --git a/protocols/api/src/main/java/org/apache/james/protocols/api/sasl/SaslFailure.java b/protocols/api/src/main/java/org/apache/james/protocols/api/sasl/SaslFailure.java index 385ef2140b..798214d074 100644 --- a/protocols/api/src/main/java/org/apache/james/protocols/api/sasl/SaslFailure.java +++ b/protocols/api/src/main/java/org/apache/james/protocols/api/sasl/SaslFailure.java @@ -63,4 +63,8 @@ public record SaslFailure(Type type, public static SaslFailure serverError(Optional<Username> authenticationId, Optional<Username> authorizationId, String reason, Throwable cause) { return new SaslFailure(Type.SERVER_ERROR, authenticationId, authorizationId, reason, Optional.ofNullable(cause)); } + + public static SaslFailure serverError(Optional<Username> authenticationId, Optional<Username> authorizationId, String reason) { + return new SaslFailure(Type.SERVER_ERROR, authenticationId, authorizationId, reason, Optional.empty()); + } } diff --git a/protocols/sasl/src/main/java/org/apache/james/protocols/sasl/OauthBearerSaslMechanismFactory.java b/protocols/sasl/src/main/java/org/apache/james/protocols/sasl/OauthBearerSaslMechanismFactory.java index 49de4f2e40..9af3d36a91 100644 --- a/protocols/sasl/src/main/java/org/apache/james/protocols/sasl/OauthBearerSaslMechanismFactory.java +++ b/protocols/sasl/src/main/java/org/apache/james/protocols/sasl/OauthBearerSaslMechanismFactory.java @@ -22,6 +22,8 @@ package org.apache.james.protocols.sasl; import org.apache.commons.configuration2.HierarchicalConfiguration; import org.apache.commons.configuration2.ex.ConfigurationException; import org.apache.commons.configuration2.tree.ImmutableNode; +import org.apache.james.jwt.OidcJwtTokenVerifier; +import org.apache.james.jwt.OidcSASLConfiguration; import org.apache.james.protocols.api.sasl.SaslMechanism; import org.apache.james.protocols.api.sasl.SaslMechanismNames; import org.apache.james.protocols.sasl.oidc.OAuthSaslMechanism; @@ -29,6 +31,8 @@ import org.apache.james.protocols.sasl.oidc.OAuthSaslMechanism; public class OauthBearerSaslMechanismFactory extends OidcSaslMechanismFactory { @Override public SaslMechanism create(HierarchicalConfiguration<ImmutableNode> serverConfiguration) throws ConfigurationException { - return new OAuthSaslMechanism(SaslMechanismNames.OAUTHBEARER, parseVerifier(serverConfiguration)); + OidcSASLConfiguration oidcConfiguration = parseConfiguration(serverConfiguration); + return new OAuthSaslMechanism(SaslMechanismNames.OAUTHBEARER, new OidcJwtTokenVerifier(oidcConfiguration), + requiresSsl(serverConfiguration), invalidTokenResponse(oidcConfiguration)); } } diff --git a/protocols/sasl/src/main/java/org/apache/james/protocols/sasl/OidcSaslMechanismFactory.java b/protocols/sasl/src/main/java/org/apache/james/protocols/sasl/OidcSaslMechanismFactory.java index 3cf55ed170..6038580424 100644 --- a/protocols/sasl/src/main/java/org/apache/james/protocols/sasl/OidcSaslMechanismFactory.java +++ b/protocols/sasl/src/main/java/org/apache/james/protocols/sasl/OidcSaslMechanismFactory.java @@ -19,23 +19,35 @@ package org.apache.james.protocols.sasl; +import static java.nio.charset.StandardCharsets.UTF_8; + import java.net.MalformedURLException; import java.net.URISyntaxException; import org.apache.commons.configuration2.HierarchicalConfiguration; import org.apache.commons.configuration2.ex.ConfigurationException; import org.apache.commons.configuration2.tree.ImmutableNode; -import org.apache.james.jwt.OidcJwtTokenVerifier; import org.apache.james.jwt.OidcSASLConfiguration; import org.apache.james.protocols.api.sasl.SaslMechanismFactory; abstract class OidcSaslMechanismFactory implements SaslMechanismFactory { - protected OidcJwtTokenVerifier parseVerifier(HierarchicalConfiguration<ImmutableNode> serverConfiguration) throws ConfigurationException { + protected boolean requiresSsl(HierarchicalConfiguration<ImmutableNode> serverConfiguration) { + return serverConfiguration.getBoolean("auth.requireSSL", false); + } + + protected byte[] invalidTokenResponse(OidcSASLConfiguration configuration) { + return String.format("{\"status\":\"invalid_token\",\"scope\":\"%s\",\"schemes\":\"%s\"}", + configuration.getScope(), + configuration.getOidcConfigurationURL().toString()) + .getBytes(UTF_8); + } + + protected OidcSASLConfiguration parseConfiguration(HierarchicalConfiguration<ImmutableNode> serverConfiguration) throws ConfigurationException { if (serverConfiguration.immutableConfigurationsAt("auth.oidc").isEmpty()) { throw new ConfigurationException("OAuth SASL mechanisms require an auth.oidc configuration"); } try { - return new OidcJwtTokenVerifier(OidcSASLConfiguration.parse(serverConfiguration.configurationAt("auth.oidc"))); + return OidcSASLConfiguration.parse(serverConfiguration.configurationAt("auth.oidc")); } catch (MalformedURLException | URISyntaxException | NullPointerException e) { throw new ConfigurationException("Failed to retrieve oauth component", e); } diff --git a/protocols/sasl/src/main/java/org/apache/james/protocols/sasl/PlainSaslMechanismFactory.java b/protocols/sasl/src/main/java/org/apache/james/protocols/sasl/PlainSaslMechanismFactory.java index 9a3537a772..7964454d43 100644 --- a/protocols/sasl/src/main/java/org/apache/james/protocols/sasl/PlainSaslMechanismFactory.java +++ b/protocols/sasl/src/main/java/org/apache/james/protocols/sasl/PlainSaslMechanismFactory.java @@ -29,6 +29,16 @@ public class PlainSaslMechanismFactory implements SaslMechanismFactory { private static final boolean PLAIN_AUTH_DISALLOWED_DEFAULT = true; private static final boolean PLAIN_AUTH_ENABLED_DEFAULT = true; + private final boolean requireSslDefault; + + public PlainSaslMechanismFactory() { + this(PLAIN_AUTH_DISALLOWED_DEFAULT); + } + + public PlainSaslMechanismFactory(boolean requireSslDefault) { + this.requireSslDefault = requireSslDefault; + } + @Override public SaslMechanism create(HierarchicalConfiguration<ImmutableNode> serverConfiguration) { return new PlainSaslMechanism(plainAuthEnabled(serverConfiguration), requiresSsl(serverConfiguration)); @@ -40,6 +50,6 @@ public class PlainSaslMechanismFactory implements SaslMechanismFactory { protected boolean requiresSsl(HierarchicalConfiguration<ImmutableNode> serverConfiguration) { return serverConfiguration.getBoolean("auth.requireSSL", - serverConfiguration.getBoolean("plainAuthDisallowed", PLAIN_AUTH_DISALLOWED_DEFAULT)); + serverConfiguration.getBoolean("plainAuthDisallowed", requireSslDefault)); } } diff --git a/protocols/sasl/src/main/java/org/apache/james/protocols/sasl/XOauth2SaslMechanismFactory.java b/protocols/sasl/src/main/java/org/apache/james/protocols/sasl/XOauth2SaslMechanismFactory.java index 9f9c895b7f..f852f3fe23 100644 --- a/protocols/sasl/src/main/java/org/apache/james/protocols/sasl/XOauth2SaslMechanismFactory.java +++ b/protocols/sasl/src/main/java/org/apache/james/protocols/sasl/XOauth2SaslMechanismFactory.java @@ -22,6 +22,8 @@ package org.apache.james.protocols.sasl; import org.apache.commons.configuration2.HierarchicalConfiguration; import org.apache.commons.configuration2.ex.ConfigurationException; import org.apache.commons.configuration2.tree.ImmutableNode; +import org.apache.james.jwt.OidcJwtTokenVerifier; +import org.apache.james.jwt.OidcSASLConfiguration; import org.apache.james.protocols.api.sasl.SaslMechanism; import org.apache.james.protocols.api.sasl.SaslMechanismNames; import org.apache.james.protocols.sasl.oidc.OAuthSaslMechanism; @@ -29,6 +31,8 @@ import org.apache.james.protocols.sasl.oidc.OAuthSaslMechanism; public class XOauth2SaslMechanismFactory extends OidcSaslMechanismFactory { @Override public SaslMechanism create(HierarchicalConfiguration<ImmutableNode> serverConfiguration) throws ConfigurationException { - return new OAuthSaslMechanism(SaslMechanismNames.XOAUTH2, parseVerifier(serverConfiguration)); + OidcSASLConfiguration oidcConfiguration = parseConfiguration(serverConfiguration); + return new OAuthSaslMechanism(SaslMechanismNames.XOAUTH2, new OidcJwtTokenVerifier(oidcConfiguration), + requiresSsl(serverConfiguration), invalidTokenResponse(oidcConfiguration)); } } diff --git a/protocols/sasl/src/main/java/org/apache/james/protocols/sasl/oidc/OAuthSaslMechanism.java b/protocols/sasl/src/main/java/org/apache/james/protocols/sasl/oidc/OAuthSaslMechanism.java index cd2106d1f0..d9bc1366da 100644 --- a/protocols/sasl/src/main/java/org/apache/james/protocols/sasl/oidc/OAuthSaslMechanism.java +++ b/protocols/sasl/src/main/java/org/apache/james/protocols/sasl/oidc/OAuthSaslMechanism.java @@ -41,10 +41,18 @@ import org.apache.james.protocols.api.sasl.SaslStep; public class OAuthSaslMechanism implements SaslMechanism { private final String name; private final OidcJwtTokenVerifier verifier; + private final boolean requiresSsl; + private final byte[] invalidTokenResponse; - public OAuthSaslMechanism(String name, OidcJwtTokenVerifier verifier) { + public OAuthSaslMechanism(String name, OidcJwtTokenVerifier verifier, boolean requiresSsl) { + this(name, verifier, requiresSsl, new byte[0]); + } + + public OAuthSaslMechanism(String name, OidcJwtTokenVerifier verifier, boolean requiresSsl, byte[] invalidTokenResponse) { this.name = name; this.verifier = verifier; + this.requiresSsl = requiresSsl; + this.invalidTokenResponse = invalidTokenResponse.clone(); } @Override @@ -52,6 +60,11 @@ public class OAuthSaslMechanism implements SaslMechanism { return name; } + @Override + public boolean isAvailableOnTransport(boolean channelEncrypted) { + return !requiresSsl || channelEncrypted; + } + @Override public SaslExchange start(SaslInitialRequest request, SaslAuthenticator authenticator) { return new OAuthSaslExchange(request.initialResponse(), authenticator); @@ -60,10 +73,12 @@ public class OAuthSaslMechanism implements SaslMechanism { private class OAuthSaslExchange implements SaslExchange { private final Optional<byte[]> initialResponse; private final SaslAuthenticator authenticator; + private Optional<SaslFailure> pendingFailure; private OAuthSaslExchange(Optional<byte[]> initialResponse, SaslAuthenticator authenticator) { this.initialResponse = initialResponse; this.authenticator = authenticator; + this.pendingFailure = Optional.empty(); } @Override @@ -75,6 +90,11 @@ public class OAuthSaslMechanism implements SaslMechanism { @Override public SaslStep onResponse(byte[] clientResponse) { + if (pendingFailure.isPresent()) { + SaslFailure failure = pendingFailure.orElseThrow(); + pendingFailure = Optional.empty(); + return new SaslStep.Failure(failure); + } return authenticate(clientResponse); } @@ -88,12 +108,17 @@ public class OAuthSaslMechanism implements SaslMechanism { Username authorizationId = Username.of(response.getAssociatedUser()); return verifier.validateToken(response.getToken()) .map(authenticationId -> authorize(authenticationId, authorizationId)) - .orElseGet(() -> new SaslStep.Failure(SaslFailure.authenticationFailed( - Optional.empty(), Optional.of(authorizationId), "OAuth authentication failed."))); + .orElseGet(() -> invalidToken(authorizationId)); }) .orElseGet(() -> new SaslStep.Failure(SaslFailure.malformed("Malformed authentication command."))); } + private SaslStep invalidToken(Username authorizationId) { + pendingFailure = Optional.of(SaslFailure.authenticationFailed( + Optional.empty(), Optional.of(authorizationId), "OAuth authentication failed.")); + return new SaslStep.Challenge(Optional.of(invalidTokenResponse.clone())); + } + private SaslStep authorize(Username authenticationId, Username authorizationId) { SaslAuthenticationResult result = authenticator.authorize(new SaslIdentity(authenticationId, authorizationId)); return switch (result) { diff --git a/protocols/sasl/src/main/java/org/apache/james/protocols/sasl/plain/PlainSaslMechanism.java b/protocols/sasl/src/main/java/org/apache/james/protocols/sasl/plain/PlainSaslMechanism.java index 96aa8212c1..641171f9e5 100644 --- a/protocols/sasl/src/main/java/org/apache/james/protocols/sasl/plain/PlainSaslMechanism.java +++ b/protocols/sasl/src/main/java/org/apache/james/protocols/sasl/plain/PlainSaslMechanism.java @@ -21,6 +21,7 @@ package org.apache.james.protocols.sasl.plain; import java.nio.charset.StandardCharsets; import java.util.Arrays; +import java.util.List; import java.util.Optional; import java.util.function.Function; @@ -130,6 +131,14 @@ public class PlainSaslMechanism implements SaslMechanism { ImmutableList<String> tokens = Arrays.stream(new String(clientResponse, StandardCharsets.UTF_8).split("\0", -1)) .collect(ImmutableList.toImmutableList()); + // Preserve legacy SMTP compatibility: some clients send a trailing NUL after the password. + if (tokens.size() == 4 && tokens.get(3).isEmpty()) { + return credentials(tokens.subList(0, 3)); + } + return credentials(tokens); + } + + private Optional<PlainCredentials> credentials(List<String> tokens) { if (tokens.size() == 2) { return Optional.of(credentials(Optional.empty(), Username.of(tokens.get(0)), tokens.get(1))); } diff --git a/protocols/sasl/src/test/java/org/apache/james/protocols/sasl/oidc/OidcSaslMechanismTest.java b/protocols/sasl/src/test/java/org/apache/james/protocols/sasl/oidc/OidcSaslMechanismTest.java index ddc1eeae5f..97c89b143c 100644 --- a/protocols/sasl/src/test/java/org/apache/james/protocols/sasl/oidc/OidcSaslMechanismTest.java +++ b/protocols/sasl/src/test/java/org/apache/james/protocols/sasl/oidc/OidcSaslMechanismTest.java @@ -28,6 +28,7 @@ import org.apache.james.core.Username; import org.apache.james.jwt.OidcJwtTokenVerifier; import org.apache.james.protocols.api.sasl.SaslAuthenticationResult; import org.apache.james.protocols.api.sasl.SaslAuthenticator; +import org.apache.james.protocols.api.sasl.SaslExchange; import org.apache.james.protocols.api.sasl.SaslFailure; import org.apache.james.protocols.api.sasl.SaslIdentity; import org.apache.james.protocols.api.sasl.SaslInitialRequest; @@ -39,6 +40,7 @@ class OidcSaslMechanismTest { private static final Username USER = Username.of("[email protected]"); private static final Username TOKEN_SUBJECT = Username.of("[email protected]"); private static final String TOKEN = "token"; + private static final byte[] INVALID_TOKEN_RESPONSE = bytes("{\"status\":\"invalid_token\"}"); @Test void oauthBearerShouldValidateTokenAndAuthorizeDecodedInitialResponse() { @@ -47,7 +49,7 @@ class OidcSaslMechanismTest { Optional.of(bytes("n,a=" + USER.asString() + ",\u0001auth=Bearer " + TOKEN + "\u0001\u0001"))); // WHEN the mechanism consumes and validates the response - SaslStep step = new OAuthSaslMechanism(SaslMechanismNames.OAUTHBEARER, verifyingToken()).start(request, authorizing()).firstStep(); + SaslStep step = mechanism(SaslMechanismNames.OAUTHBEARER, verifyingToken()).start(request, authorizing()).firstStep(); // THEN it returns the authorized identity directly to the protocol driver assertThat(step).isEqualTo(new SaslStep.Success(new SaslIdentity(TOKEN_SUBJECT, USER), Optional.empty())); @@ -60,7 +62,7 @@ class OidcSaslMechanismTest { Optional.of(bytes("user=" + USER.asString() + "\u0001auth=Bearer " + TOKEN + "\u0001\u0001"))); // WHEN the mechanism consumes and validates the response - SaslStep step = new OAuthSaslMechanism(SaslMechanismNames.XOAUTH2, verifyingToken()).start(request, authorizing()).firstStep(); + SaslStep step = mechanism(SaslMechanismNames.XOAUTH2, verifyingToken()).start(request, authorizing()).firstStep(); // THEN it exposes the same authorized identity shape assertThat(step).isEqualTo(new SaslStep.Success(new SaslIdentity(TOKEN_SUBJECT, USER), Optional.empty())); @@ -72,7 +74,7 @@ class OidcSaslMechanismTest { SaslInitialRequest request = new SaslInitialRequest(SaslMechanismNames.OAUTHBEARER, Optional.empty()); // WHEN the mechanism starts - SaslStep firstStep = new OAuthSaslMechanism(SaslMechanismNames.OAUTHBEARER, verifyingToken()).start(request, authorizing()).firstStep(); + SaslStep firstStep = mechanism(SaslMechanismNames.OAUTHBEARER, verifyingToken()).start(request, authorizing()).firstStep(); // THEN the server asks for one client response assertThat(firstStep).isEqualTo(new SaslStep.Challenge(Optional.empty())); @@ -85,23 +87,27 @@ class OidcSaslMechanismTest { Optional.of(bytes("invalid"))); // WHEN the mechanism consumes the response - SaslStep step = new OAuthSaslMechanism(SaslMechanismNames.OAUTHBEARER, verifyingToken()).start(request, authorizing()).firstStep(); + SaslStep step = mechanism(SaslMechanismNames.OAUTHBEARER, verifyingToken()).start(request, authorizing()).firstStep(); // THEN it fails before any token validation side effect assertThat(step).isEqualTo(new SaslStep.Failure(SaslFailure.malformed("Malformed authentication command."))); } @Test - void shouldFailWhenTokenIsRejected() { + void shouldReturnInvalidTokenChallengeThenFailWhenTokenIsRejected() { // GIVEN an OIDC SASL response with an invalid bearer token SaslInitialRequest request = new SaslInitialRequest(SaslMechanismNames.OAUTHBEARER, Optional.of(bytes("n,a=" + USER.asString() + ",\u0001auth=Bearer " + TOKEN + "\u0001\u0001"))); + SaslExchange exchange = mechanism(SaslMechanismNames.OAUTHBEARER, rejectingToken()).start(request, authorizing()); // WHEN token validation rejects the token - SaslStep step = new OAuthSaslMechanism(SaslMechanismNames.OAUTHBEARER, rejectingToken()).start(request, authorizing()).firstStep(); + SaslStep firstStep = exchange.firstStep(); + SaslStep secondStep = exchange.onResponse(new byte[0]); - // THEN the mechanism returns a typed authentication failure - assertThat(step).isEqualTo(new SaslStep.Failure(SaslFailure.authenticationFailed( + // THEN the mechanism owns the OIDC error challenge before returning the typed authentication failure + assertThat(firstStep).isInstanceOfSatisfying(SaslStep.Challenge.class, + challenge -> assertThat(challenge.payload()).hasValueSatisfying(payload -> assertThat(payload).containsExactly(INVALID_TOKEN_RESPONSE))); + assertThat(secondStep).isEqualTo(new SaslStep.Failure(SaslFailure.authenticationFailed( Optional.empty(), Optional.of(USER), "OAuth authentication failed."))); } @@ -113,12 +119,24 @@ class OidcSaslMechanismTest { SaslFailure failure = SaslFailure.delegationForbidden(TOKEN_SUBJECT, USER, "forbidden"); // WHEN authorization rejects the identity - SaslStep step = new OAuthSaslMechanism(SaslMechanismNames.OAUTHBEARER, verifyingToken()).start(request, rejectingAuthorization(failure)).firstStep(); + SaslStep step = mechanism(SaslMechanismNames.OAUTHBEARER, verifyingToken()).start(request, rejectingAuthorization(failure)).firstStep(); // THEN the failure is returned to the protocol driver assertThat(step).isEqualTo(new SaslStep.Failure(failure)); } + @Test + void shouldBeUnavailableOnClearTransportWhenSslIsRequired() { + OAuthSaslMechanism mechanism = new OAuthSaslMechanism(SaslMechanismNames.OAUTHBEARER, verifyingToken(), true); + + assertThat(mechanism.isAvailableOnTransport(false)).isFalse(); + assertThat(mechanism.isAvailableOnTransport(true)).isTrue(); + } + + private static OAuthSaslMechanism mechanism(String mechanismName, OidcJwtTokenVerifier verifier) { + return new OAuthSaslMechanism(mechanismName, verifier, false, INVALID_TOKEN_RESPONSE); + } + private static OidcJwtTokenVerifier verifyingToken() { return new TestOidcJwtTokenVerifier(Optional.of(TOKEN_SUBJECT)); } --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
