From b0e1c7ac2a39a4a1c22fa029cc63e934ff51c995 Mon Sep 17 00:00:00 2001 From: Felix Auringer Date: Fri, 15 May 2026 11:20:09 +0200 Subject: [PATCH 1/6] JAMES-4205 Create default mailboxes after OIDC login Currently, mailboxes (even INBOX) are only created when logging in using PLAIN or LOGIN. --- .../imap/processor/AbstractAuthProcessor.java | 15 +++++++++++---- .../imap/processor/AuthenticateProcessor.java | 2 +- 2 files changed, 12 insertions(+), 5 deletions(-) diff --git a/protocols/imap/src/main/java/org/apache/james/imap/processor/AbstractAuthProcessor.java b/protocols/imap/src/main/java/org/apache/james/imap/processor/AbstractAuthProcessor.java index 34cc3eb3bef..abbeadb1950 100644 --- a/protocols/imap/src/main/java/org/apache/james/imap/processor/AbstractAuthProcessor.java +++ b/protocols/imap/src/main/java/org/apache/james/imap/processor/AbstractAuthProcessor.java @@ -253,10 +253,17 @@ protected static AuthenticationAttempt noDelegation(Username authenticationId, S return new AuthenticationAttempt(Optional.empty(), authenticationId, password); } - protected void authSuccess(Username username, ImapSession session, ImapRequest request, Responder responder) { - session.authenticated(); - session.setMailboxSession(getMailboxManager().createSystemSession(username)); - okComplete(request, responder); + protected void oauthSuccess(Username username, ImapSession session, ImapRequest request, Responder responder) { + try { + session.authenticated(); + final MailboxSession mailboxSession = getMailboxManager().createSystemSession(username); + session.setMailboxSession(mailboxSession); + provisionInbox(session, getMailboxManager(), mailboxSession); + okComplete(request, responder); + } catch (MailboxException e) { + LOGGER.error("Error encountered while login", e); + no(request, responder, HumanReadableText.GENERIC_FAILURE_DURING_PROCESSING); + } } protected static class AuthenticationAttempt { diff --git a/protocols/imap/src/main/java/org/apache/james/imap/processor/AuthenticateProcessor.java b/protocols/imap/src/main/java/org/apache/james/imap/processor/AuthenticateProcessor.java index cd22c4ca68f..981f07f1603 100644 --- a/protocols/imap/src/main/java/org/apache/james/imap/processor/AuthenticateProcessor.java +++ b/protocols/imap/src/main/java/org/apache/james/imap/processor/AuthenticateProcessor.java @@ -220,7 +220,7 @@ private void doOAuth(OIDCSASLParser.OIDCInitialResponse oidcInitialResponse, Oid .as(associatedUser), session, request, responder, authenticatedUser, associatedUser); } else { - authSuccess(authenticatedUser, session, request, responder); + oauthSuccess(authenticatedUser, session, request, responder); } }, () -> manageFailureCount(session, request, responder)); } From 5340accebde7915b54acd3a7b2d6de8eb72a25df Mon Sep 17 00:00:00 2001 From: Felix Auringer Date: Mon, 18 May 2026 15:47:30 +0200 Subject: [PATCH 2/6] refactor(imap): fail early if OIDC is disabled --- .../imap/processor/AuthenticateProcessor.java | 39 ++++++++++--------- 1 file changed, 20 insertions(+), 19 deletions(-) diff --git a/protocols/imap/src/main/java/org/apache/james/imap/processor/AuthenticateProcessor.java b/protocols/imap/src/main/java/org/apache/james/imap/processor/AuthenticateProcessor.java index 981f07f1603..9ca4adaa995 100644 --- a/protocols/imap/src/main/java/org/apache/james/imap/processor/AuthenticateProcessor.java +++ b/protocols/imap/src/main/java/org/apache/james/imap/processor/AuthenticateProcessor.java @@ -105,19 +105,24 @@ protected void processRequest(AuthenticateRequest request, ImapSession session, } } } else if (authType.equalsIgnoreCase(AUTH_TYPE_OAUTHBEARER) || authType.equalsIgnoreCase(AUTH_TYPE_XOAUTH2)) { - if (request instanceof IRAuthenticateRequest) { - IRAuthenticateRequest irRequest = (IRAuthenticateRequest) request; - doOAuth(irRequest.getInitialClientResponse(), session, request, responder); + if (!session.supportsOAuth()) { + LOGGER.warn("OAuth authentication rejected because it is disabled"); + no(request, responder, HumanReadableText.UNSUPPORTED_AUTHENTICATION_MECHANISM); } else { - session.executeSafely(() -> { - responder.respond(new AuthenticateResponse()); - responder.flush(); - session.pushLineHandler((requestSession, data) -> Mono.fromRunnable(() -> { - doOAuth(extractInitialClientResponse(data), requestSession, request, responder); - requestSession.popLineHandler(); + if (request instanceof IRAuthenticateRequest) { + IRAuthenticateRequest irRequest = (IRAuthenticateRequest) request; + doOAuth(irRequest.getInitialClientResponse(), session, request, responder); + } else { + session.executeSafely(() -> { + responder.respond(new AuthenticateResponse()); responder.flush(); - }).subscribeOn(ReactorUtils.BLOCKING_CALL_WRAPPER).then()); - }); + session.pushLineHandler((requestSession, data) -> Mono.fromRunnable(() -> { + doOAuth(extractInitialClientResponse(data), requestSession, request, responder); + requestSession.popLineHandler(); + responder.flush(); + }).subscribeOn(ReactorUtils.BLOCKING_CALL_WRAPPER).then()); + }); + } } } else { LOGGER.debug("Unsupported authentication mechanism '{}'", authType); @@ -197,14 +202,10 @@ protected MDCBuilder mdc(AuthenticateRequest request) { } protected void doOAuth(String initialResponse, ImapSession session, ImapRequest request, Responder responder) { - if (!session.supportsOAuth()) { - no(request, responder, HumanReadableText.UNSUPPORTED_AUTHENTICATION_MECHANISM); - } else { - OIDCSASLParser.parse(initialResponse) - .flatMap(oidcInitialResponseValue -> session.oidcSaslConfiguration().map(configure -> Pair.of(oidcInitialResponseValue, configure))) - .ifPresentOrElse(pair -> doOAuth(pair.getLeft(), pair.getRight(), session, request, responder), - () -> manageFailureCount(session, request, responder)); - } + OIDCSASLParser.parse(initialResponse) + .flatMap(oidcInitialResponseValue -> session.oidcSaslConfiguration().map(configure -> Pair.of(oidcInitialResponseValue, configure))) + .ifPresentOrElse(pair -> doOAuth(pair.getLeft(), pair.getRight(), session, request, responder), + () -> manageFailureCount(session, request, responder)); session.stopDetectingCommandInjection(); } From 6639ca98de727cb2f900943b000f41283b60653d Mon Sep 17 00:00:00 2001 From: Felix Auringer Date: Mon, 18 May 2026 15:52:05 +0200 Subject: [PATCH 3/6] refactor(imap): better names and comments for authentication processors --- .../imap/processor/AbstractAuthProcessor.java | 4 +-- .../imap/processor/AuthenticateProcessor.java | 25 +++++++++++-------- .../james/imap/processor/LoginProcessor.java | 7 ++++-- 3 files changed, 21 insertions(+), 15 deletions(-) diff --git a/protocols/imap/src/main/java/org/apache/james/imap/processor/AbstractAuthProcessor.java b/protocols/imap/src/main/java/org/apache/james/imap/processor/AbstractAuthProcessor.java index abbeadb1950..4bd2cbb6332 100644 --- a/protocols/imap/src/main/java/org/apache/james/imap/processor/AbstractAuthProcessor.java +++ b/protocols/imap/src/main/java/org/apache/james/imap/processor/AbstractAuthProcessor.java @@ -77,7 +77,7 @@ public void configure(ImapConfiguration imapConfiguration) { this.imapConfiguration = imapConfiguration; } - protected void doAuth(AuthenticationAttempt authenticationAttempt, ImapSession session, ImapRequest request, Responder responder, HumanReadableText failed) { + protected void doPasswordAuth(AuthenticationAttempt authenticationAttempt, ImapSession session, ImapRequest request, Responder responder, HumanReadableText failed) { Preconditions.checkArgument(!authenticationAttempt.isDelegation()); try { boolean authFailure = false; @@ -118,7 +118,7 @@ protected void doAuth(AuthenticationAttempt authenticationAttempt, ImapSession s } } - protected void doAuthWithDelegation(AuthenticationAttempt authenticationAttempt, ImapSession session, ImapRequest request, Responder responder) { + protected void doPasswordAuthWithDelegation(AuthenticationAttempt authenticationAttempt, ImapSession session, ImapRequest request, Responder responder) { Preconditions.checkArgument(authenticationAttempt.isDelegation()); Username givenUser = authenticationAttempt.getAuthenticationId(); if (givenUser == null) { diff --git a/protocols/imap/src/main/java/org/apache/james/imap/processor/AuthenticateProcessor.java b/protocols/imap/src/main/java/org/apache/james/imap/processor/AuthenticateProcessor.java index 9ca4adaa995..1f66be3daa0 100644 --- a/protocols/imap/src/main/java/org/apache/james/imap/processor/AuthenticateProcessor.java +++ b/protocols/imap/src/main/java/org/apache/james/imap/processor/AuthenticateProcessor.java @@ -55,7 +55,7 @@ import reactor.core.publisher.Mono; /** - * Processor which handles the AUTHENTICATE command. Only authtype of PLAIN is supported ATM. + * Processor which handles the AUTHENTICATE command. Supported authentication mechanisms are PLAIN, XOAUTH2, and OAUTHBEARER. */ public class AuthenticateProcessor extends AbstractAuthProcessor implements CapabilityImplementingProcessor { public static final String AUTH_PLAIN = "AUTH=PLAIN"; @@ -85,18 +85,18 @@ protected void processRequest(AuthenticateRequest request, ImapSession session, if (authType.equalsIgnoreCase(AUTH_TYPE_PLAIN)) { // See if AUTH=PLAIN is allowed. See IMAP-304 if (session.isPlainAuthDisallowed()) { - LOGGER.warn("Login attempt over clear channel rejected"); + LOGGER.warn("Plain authentication rejected because it is disabled or not allowed over insecure channel"); no(request, responder, HumanReadableText.DISABLED_LOGIN); } else { if (request instanceof IRAuthenticateRequest) { IRAuthenticateRequest irRequest = (IRAuthenticateRequest) request; - doPlainAuth(irRequest.getInitialClientResponse(), session, request, responder); + parseAndDoPlainAuth(irRequest.getInitialClientResponse(), session, request, responder); } else { session.executeSafely(() -> { responder.respond(new AuthenticateResponse()); responder.flush(); session.pushLineHandler((requestSession, data) -> Mono.fromRunnable(() -> { - doPlainAuth(extractInitialClientResponse(data), requestSession, request, responder); + parseAndDoPlainAuth(extractInitialClientResponse(data), requestSession, request, responder); // remove the handler now requestSession.popLineHandler(); responder.flush(); @@ -111,13 +111,13 @@ protected void processRequest(AuthenticateRequest request, ImapSession session, } else { if (request instanceof IRAuthenticateRequest) { IRAuthenticateRequest irRequest = (IRAuthenticateRequest) request; - doOAuth(irRequest.getInitialClientResponse(), session, request, responder); + parseAndDoOAuth(irRequest.getInitialClientResponse(), session, request, responder); } else { session.executeSafely(() -> { responder.respond(new AuthenticateResponse()); responder.flush(); session.pushLineHandler((requestSession, data) -> Mono.fromRunnable(() -> { - doOAuth(extractInitialClientResponse(data), requestSession, request, responder); + parseAndDoOAuth(extractInitialClientResponse(data), requestSession, request, responder); requestSession.popLineHandler(); responder.flush(); }).subscribeOn(ReactorUtils.BLOCKING_CALL_WRAPPER).then()); @@ -131,14 +131,14 @@ protected void processRequest(AuthenticateRequest request, ImapSession session, } /** - * Parse the initialClientResponse and do a PLAIN AUTH with it + * Parse the initial client response and start plain authentication. */ - protected void doPlainAuth(String initialClientResponse, ImapSession session, ImapRequest request, Responder responder) { + protected void parseAndDoPlainAuth(String initialClientResponse, ImapSession session, ImapRequest request, Responder responder) { AuthenticationAttempt authenticationAttempt = parseDelegationAttempt(initialClientResponse); if (authenticationAttempt.isDelegation()) { - doAuthWithDelegation(authenticationAttempt, session, request, responder); + doPasswordAuthWithDelegation(authenticationAttempt, session, request, responder); } else { - doAuth(authenticationAttempt, session, request, responder, HumanReadableText.AUTHENTICATION_FAILED); + doPasswordAuth(authenticationAttempt, session, request, responder, HumanReadableText.AUTHENTICATION_FAILED); } session.stopDetectingCommandInjection(); } @@ -201,7 +201,10 @@ protected MDCBuilder mdc(AuthenticateRequest request) { .addToContext("authType", request.getAuthType()); } - protected void doOAuth(String initialResponse, ImapSession session, ImapRequest request, Responder responder) { + /** + * Parse the initial client response and start oauth authentication. + */ + protected void parseAndDoOAuth(String initialResponse, ImapSession session, ImapRequest request, Responder responder) { OIDCSASLParser.parse(initialResponse) .flatMap(oidcInitialResponseValue -> session.oidcSaslConfiguration().map(configure -> Pair.of(oidcInitialResponseValue, configure))) .ifPresentOrElse(pair -> doOAuth(pair.getLeft(), pair.getRight(), session, request, responder), diff --git a/protocols/imap/src/main/java/org/apache/james/imap/processor/LoginProcessor.java b/protocols/imap/src/main/java/org/apache/james/imap/processor/LoginProcessor.java index b4d20915f92..989a4879655 100644 --- a/protocols/imap/src/main/java/org/apache/james/imap/processor/LoginProcessor.java +++ b/protocols/imap/src/main/java/org/apache/james/imap/processor/LoginProcessor.java @@ -50,14 +50,17 @@ public LoginProcessor(MailboxManager mailboxManager, StatusResponseFactory facto super(LoginRequest.class, mailboxManager, factory, metricFactory, pathConverterFactory); } + /** + * Start password authentication if enabled. + */ @Override protected void processRequest(LoginRequest request, ImapSession session, Responder responder) { // check if the login is allowed with LOGIN command. See IMAP-304 if (session.isPlainAuthDisallowed()) { - LOGGER.warn("Login attempt over clear channel rejected"); + LOGGER.warn("Login rejected because it is disabled or not allowed over insecure channel"); no(request, responder, HumanReadableText.DISABLED_LOGIN); } else { - doAuth(noDelegation(request.getUserid(), request.getPassword()), + doPasswordAuth(noDelegation(request.getUserid(), request.getPassword()), session, request, responder, HumanReadableText.INVALID_LOGIN); } } From 0ddbe41640162367759b3810ad54d867e6f96df3 Mon Sep 17 00:00:00 2001 From: Felix Auringer Date: Mon, 18 May 2026 12:38:52 +0200 Subject: [PATCH 4/6] refactor(imap): move oauth methods to its plain auth counterparts --- .../imap/processor/AbstractAuthProcessor.java | 20 +++++++++ .../imap/processor/AuthenticateProcessor.java | 41 +++++-------------- 2 files changed, 31 insertions(+), 30 deletions(-) diff --git a/protocols/imap/src/main/java/org/apache/james/imap/processor/AbstractAuthProcessor.java b/protocols/imap/src/main/java/org/apache/james/imap/processor/AbstractAuthProcessor.java index 4bd2cbb6332..69cb152ea5a 100644 --- a/protocols/imap/src/main/java/org/apache/james/imap/processor/AbstractAuthProcessor.java +++ b/protocols/imap/src/main/java/org/apache/james/imap/processor/AbstractAuthProcessor.java @@ -27,6 +27,8 @@ import org.apache.james.imap.api.message.response.StatusResponseFactory; import org.apache.james.imap.api.process.ImapSession; import org.apache.james.imap.main.PathConverter; +import org.apache.james.jwt.OidcJwtTokenVerifier; +import org.apache.james.jwt.OidcSASLConfiguration; import org.apache.james.mailbox.Authorizator; import org.apache.james.mailbox.DefaultMailboxes; import org.apache.james.mailbox.MailboxManager; @@ -39,6 +41,7 @@ import org.apache.james.mailbox.model.MailboxConstants; import org.apache.james.mailbox.model.MailboxPath; import org.apache.james.metrics.api.MetricFactory; +import org.apache.james.protocols.api.OIDCSASLParser; import org.apache.james.util.AuditTrail; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -191,6 +194,23 @@ protected void doAuthWithDelegation(MailboxSessionAuthWithDelegationSupplier mai } } + protected void doOAuth(OIDCSASLParser.OIDCInitialResponse oidcInitialResponse, OidcSASLConfiguration oidcSASLConfiguration, + ImapSession session, ImapRequest request, Responder responder) { + new OidcJwtTokenVerifier(oidcSASLConfiguration).validateToken(oidcInitialResponse.getToken()) + .ifPresentOrElse(authenticatedUser -> { + Username associatedUser = Username.of(oidcInitialResponse.getAssociatedUser()); + if (!associatedUser.equals(authenticatedUser)) { + doAuthWithDelegation(() -> getMailboxManager() + .withExtraAuthorizator(withAdminUsers()) + .authenticate(authenticatedUser) + .as(associatedUser), + session, request, responder, authenticatedUser, associatedUser); + } else { + authSuccess(authenticatedUser, session, request, responder); + } + }, () -> manageFailureCount(session, request, responder)); + } + protected void provisionInbox(ImapSession session, MailboxManager mailboxManager, MailboxSession mailboxSession) throws MailboxException { MailboxPath inboxPath = pathConverterFactory.forSession(session).buildFullPath(MailboxConstants.INBOX); if (Mono.from(mailboxManager.mailboxExists(inboxPath, mailboxSession)).block()) { diff --git a/protocols/imap/src/main/java/org/apache/james/imap/processor/AuthenticateProcessor.java b/protocols/imap/src/main/java/org/apache/james/imap/processor/AuthenticateProcessor.java index 1f66be3daa0..2988064e82d 100644 --- a/protocols/imap/src/main/java/org/apache/james/imap/processor/AuthenticateProcessor.java +++ b/protocols/imap/src/main/java/org/apache/james/imap/processor/AuthenticateProcessor.java @@ -39,8 +39,6 @@ import org.apache.james.imap.message.request.AuthenticateRequest; import org.apache.james.imap.message.request.IRAuthenticateRequest; import org.apache.james.imap.message.response.AuthenticateResponse; -import org.apache.james.jwt.OidcJwtTokenVerifier; -import org.apache.james.jwt.OidcSASLConfiguration; import org.apache.james.mailbox.MailboxManager; import org.apache.james.metrics.api.MetricFactory; import org.apache.james.protocols.api.OIDCSASLParser; @@ -143,6 +141,17 @@ protected void parseAndDoPlainAuth(String initialClientResponse, ImapSession ses session.stopDetectingCommandInjection(); } + /** + * Parse the initial client response and start oauth authentication. + */ + protected void parseAndDoOAuth(String initialResponse, ImapSession session, ImapRequest request, Responder responder) { + OIDCSASLParser.parse(initialResponse) + .flatMap(oidcInitialResponseValue -> session.oidcSaslConfiguration().map(configure -> Pair.of(oidcInitialResponseValue, configure))) + .ifPresentOrElse(pair -> doOAuth(pair.getLeft(), pair.getRight(), session, request, responder), + () -> manageFailureCount(session, request, responder)); + session.stopDetectingCommandInjection(); + } + private AuthenticationAttempt parseDelegationAttempt(String initialClientResponse) { try { String userpass = new String(Base64.getDecoder().decode(initialClientResponse)); @@ -201,34 +210,6 @@ protected MDCBuilder mdc(AuthenticateRequest request) { .addToContext("authType", request.getAuthType()); } - /** - * Parse the initial client response and start oauth authentication. - */ - protected void parseAndDoOAuth(String initialResponse, ImapSession session, ImapRequest request, Responder responder) { - OIDCSASLParser.parse(initialResponse) - .flatMap(oidcInitialResponseValue -> session.oidcSaslConfiguration().map(configure -> Pair.of(oidcInitialResponseValue, configure))) - .ifPresentOrElse(pair -> doOAuth(pair.getLeft(), pair.getRight(), session, request, responder), - () -> manageFailureCount(session, request, responder)); - session.stopDetectingCommandInjection(); - } - - private void doOAuth(OIDCSASLParser.OIDCInitialResponse oidcInitialResponse, OidcSASLConfiguration oidcSASLConfiguration, - ImapSession session, ImapRequest request, Responder responder) { - new OidcJwtTokenVerifier(oidcSASLConfiguration).validateToken(oidcInitialResponse.getToken()) - .ifPresentOrElse(authenticatedUser -> { - Username associatedUser = Username.of(oidcInitialResponse.getAssociatedUser()); - if (!associatedUser.equals(authenticatedUser)) { - doAuthWithDelegation(() -> getMailboxManager() - .withExtraAuthorizator(withAdminUsers()) - .authenticate(authenticatedUser) - .as(associatedUser), - session, request, responder, authenticatedUser, associatedUser); - } else { - oauthSuccess(authenticatedUser, session, request, responder); - } - }, () -> manageFailureCount(session, request, responder)); - } - private static String extractInitialClientResponse(byte[] data) { // cut of the CRLF return new String(data, 0, data.length - 2, StandardCharsets.US_ASCII); From 4e3baabfedf8067aff5f7d55eb7185c7d4101c39 Mon Sep 17 00:00:00 2001 From: Felix Auringer Date: Tue, 19 May 2026 12:22:45 +0200 Subject: [PATCH 5/6] refactor(imap): consistent handling of auth failure and success There are now two methods (authFailure and authSuccess) and all paths that result in successful authentication or an error caused by the user are now handled in those two places. There are still some errors that are caused by James itself which are not handled by those two methods. --- .../imap/processor/AbstractAuthProcessor.java | 182 +++++++++--------- .../imap/processor/AuthenticateProcessor.java | 8 +- .../james/imap/processor/LoginProcessor.java | 3 +- 3 files changed, 93 insertions(+), 100 deletions(-) diff --git a/protocols/imap/src/main/java/org/apache/james/imap/processor/AbstractAuthProcessor.java b/protocols/imap/src/main/java/org/apache/james/imap/processor/AbstractAuthProcessor.java index 69cb152ea5a..7b43ad24b95 100644 --- a/protocols/imap/src/main/java/org/apache/james/imap/processor/AbstractAuthProcessor.java +++ b/protocols/imap/src/main/java/org/apache/james/imap/processor/AbstractAuthProcessor.java @@ -80,62 +80,52 @@ public void configure(ImapConfiguration imapConfiguration) { this.imapConfiguration = imapConfiguration; } - protected void doPasswordAuth(AuthenticationAttempt authenticationAttempt, ImapSession session, ImapRequest request, Responder responder, HumanReadableText failed) { + protected void doPasswordAuth(AuthenticationAttempt authenticationAttempt, ImapSession session, ImapRequest request, Responder responder) { Preconditions.checkArgument(!authenticationAttempt.isDelegation()); - try { - boolean authFailure = false; - if (authenticationAttempt.getAuthenticationId() == null) { - authFailure = true; - } - if (!authFailure) { - try { - final MailboxSession mailboxSession = getMailboxManager().authenticate(authenticationAttempt.getAuthenticationId(), - authenticationAttempt.getPassword()) - .withoutDelegation(); - session.authenticated(); - session.setMailboxSession(mailboxSession); - provisionInbox(session, getMailboxManager(), mailboxSession); - AuditTrail.entry() - .username(() -> mailboxSession.getUser().asString()) - .sessionId(() -> session.sessionId().asString()) - .protocol("IMAP") - .action("AUTH") - .log("IMAP Authentication succeeded."); - okComplete(request, responder); - session.stopDetectingCommandInjection(); - } catch (BadCredentialsException e) { - authFailure = true; - AuditTrail.entry() - .username(() -> authenticationAttempt.getAuthenticationId().asString()) - .protocol("IMAP") - .action("AUTH") - .log("IMAP Authentication failed because of bad credentials."); - } - } - if (authFailure) { - manageFailureCount(session, request, responder, failed); + + if (authenticationAttempt.getAuthenticationId() == null || authenticationAttempt.getPassword() == null) { + authFailure(session, request, responder, HumanReadableText.AUTHENTICATION_FAILED, Optional.empty(), Optional.empty(), + "Malformed authentication command." + ); + } else { + try { + final MailboxSession mailboxSession = getMailboxManager().authenticate( + authenticationAttempt.getAuthenticationId(), + authenticationAttempt.getPassword() + ).withoutDelegation(); + authSuccess(session, mailboxSession, request, responder, "Password authentication succeeded."); + } catch (BadCredentialsException e) { + authFailure(session, request, responder, HumanReadableText.INVALID_LOGIN, + Optional.of(authenticationAttempt.getAuthenticationId()), + Optional.empty(), + "Password authentication failed because of bad credentials." + ); + } catch (MailboxException e) { + // This is probably not a user error, so we do not increase the failure count or add the + // event to the audit log. + LOGGER.error("Authentication failed", e); + no(request, responder, HumanReadableText.GENERIC_FAILURE_DURING_PROCESSING); } - } catch (MailboxException e) { - LOGGER.error("Error encountered while login", e); - no(request, responder, HumanReadableText.GENERIC_FAILURE_DURING_PROCESSING); } } protected void doPasswordAuthWithDelegation(AuthenticationAttempt authenticationAttempt, ImapSession session, ImapRequest request, Responder responder) { Preconditions.checkArgument(authenticationAttempt.isDelegation()); + Username otherUser = authenticationAttempt.getDelegateUserName().orElseThrow(); + Username givenUser = authenticationAttempt.getAuthenticationId(); if (givenUser == null) { - manageFailureCount(session, request, responder); - return; + authFailure(session, request, responder, HumanReadableText.AUTHENTICATION_FAILED, + Optional.empty(), Optional.of(otherUser), "Malformed authentication command."); + } else { + doAuthWithDelegation(() -> getMailboxManager() + .withExtraAuthorizator(withAdminUsers()) + .authenticate(givenUser, authenticationAttempt.getPassword()) + .as(otherUser), + session, + request, responder, + givenUser, otherUser); } - Username otherUser = authenticationAttempt.getDelegateUserName().orElseThrow(); - doAuthWithDelegation(() -> getMailboxManager() - .withExtraAuthorizator(withAdminUsers()) - .authenticate(givenUser, authenticationAttempt.getPassword()) - .as(otherUser), - session, - request, responder, - givenUser, otherUser); } protected Authorizator withAdminUsers() { @@ -151,45 +141,20 @@ protected void doAuthWithDelegation(MailboxSessionAuthWithDelegationSupplier mai ImapSession session, ImapRequest request, Responder responder, Username authenticateUser, Username delegatorUser) { try { - MailboxManager mailboxManager = getMailboxManager(); - MailboxSession mailboxSession = mailboxSessionSupplier.get(); - session.authenticated(); - session.setMailboxSession(mailboxSession); - AuditTrail.entry() - .username(() -> mailboxSession.getLoggedInUser() - .map(Username::asString) - .orElse("")) - .sessionId(() -> session.sessionId().asString()) - .protocol("IMAP") - .action("AUTH") - .remoteIP(() -> Optional.ofNullable(session.getRemoteAddress())) - .parameters(() -> ImmutableMap.of("delegatorUser", mailboxSession.getUser().asString())) - .log("IMAP Authentication with delegation succeeded."); - okComplete(request, responder); - provisionInbox(session, mailboxManager, mailboxSession); + authSuccess(session, mailboxSessionSupplier.get(), request, responder, "Authentication with delegation succeeded."); } catch (BadCredentialsException e) { - AuditTrail.entry() - .username(authenticateUser::asString) - .protocol("IMAP") - .action("AUTH") - .remoteIP(() -> Optional.ofNullable(session.getRemoteAddress())) - .parameters(() -> ImmutableMap.of("delegatorUser", delegatorUser.asString())) - .log("IMAP Authentication with delegation failed because of bad credentials."); - manageFailureCount(session, request, responder); + authFailure(session, request, responder, HumanReadableText.INVALID_LOGIN, Optional.of(authenticateUser), + Optional.of(delegatorUser), "Password authentication with delegation failed because of bad credentials."); } catch (UserDoesNotExistException e) { - LOGGER.info("User does not exist", e); - no(request, responder, HumanReadableText.USER_DOES_NOT_EXIST); + authFailure(session, request, responder, HumanReadableText.USER_DOES_NOT_EXIST, Optional.of(authenticateUser), + Optional.of(delegatorUser), "Delegation target user does not exist."); } catch (ForbiddenDelegationException e) { - LOGGER.info("Delegate forbidden", e); - AuditTrail.entry() - .username(authenticateUser::asString) - .protocol("IMAP") - .action("AUTH") - .parameters(() -> ImmutableMap.of("delegatorUser", delegatorUser.asString())) - .log("IMAP Authentication with delegation failed because of non existing delegation."); - no(request, responder, HumanReadableText.DELEGATION_FORBIDDEN); + authFailure(session, request, responder, HumanReadableText.DELEGATION_FORBIDDEN, Optional.of(authenticateUser), + Optional.of(delegatorUser), "Requested delegation is forbidden."); } catch (MailboxException e) { - LOGGER.info("Login failed", e); + // This is probably not a user error, so we do not increase the failure count or add the + // event to the audit log. + LOGGER.info("Authentication failed", e); no(request, responder, HumanReadableText.GENERIC_FAILURE_DURING_PROCESSING); } } @@ -206,9 +171,16 @@ protected void doOAuth(OIDCSASLParser.OIDCInitialResponse oidcInitialResponse, O .as(associatedUser), session, request, responder, authenticatedUser, associatedUser); } else { - authSuccess(authenticatedUser, session, request, responder); + authSuccess(session, getMailboxManager().createSystemSession(authenticatedUser), request, responder, + "OAuth authentication succeeded." + ); } - }, () -> manageFailureCount(session, request, responder)); + }, () -> { + authFailure(session, request, responder, HumanReadableText.AUTHENTICATION_FAILED, Optional.empty(), + Optional.of(Username.of(oidcInitialResponse.getAssociatedUser())), + "OAuth authentication failed." + ); + }); } protected void provisionInbox(ImapSession session, MailboxManager mailboxManager, MailboxSession mailboxSession) throws MailboxException { @@ -243,10 +215,6 @@ private void provisionMailbox(String mailbox, MailboxManager mailboxManager, } } - protected void manageFailureCount(ImapSession session, ImapRequest request, Responder responder) { - manageFailureCount(session, request, responder, HumanReadableText.AUTHENTICATION_FAILED); - } - protected void manageFailureCount(ImapSession session, ImapRequest request, Responder responder, HumanReadableText failed) { Integer currentNumberOfFailures = (Integer) session.getAttribute(ATTRIBUTE_NUMBER_OF_FAILURES); int failures; @@ -273,17 +241,43 @@ protected static AuthenticationAttempt noDelegation(Username authenticationId, S return new AuthenticationAttempt(Optional.empty(), authenticationId, password); } - protected void oauthSuccess(Username username, ImapSession session, ImapRequest request, Responder responder) { + protected void authSuccess(ImapSession session, MailboxSession mailboxSession, ImapRequest request, Responder responder, String log) { + session.authenticated(); + session.setMailboxSession(mailboxSession); try { - session.authenticated(); - final MailboxSession mailboxSession = getMailboxManager().createSystemSession(username); - session.setMailboxSession(mailboxSession); provisionInbox(session, getMailboxManager(), mailboxSession); - okComplete(request, responder); } catch (MailboxException e) { - LOGGER.error("Error encountered while login", e); - no(request, responder, HumanReadableText.GENERIC_FAILURE_DURING_PROCESSING); + // TODO: Should this abort and cancel the authentication? + LOGGER.error("Provisioning mailboxes failed", e); + } + + AuditTrail.Entry entry = AuditTrail.entry() + .username(() -> mailboxSession.getUser().asString()) + .sessionId(() -> session.sessionId().asString()) + .protocol("IMAP") + .action("AUTH") + .remoteIP(() -> Optional.ofNullable(session.getRemoteAddress())); + Optional assumedUser = mailboxSession.getLoggedInUser(); + if (assumedUser.isPresent()) { + entry = entry.parameters(() -> ImmutableMap.of("delegatorUser", assumedUser.get().asString())); + } + entry.log(log); + okComplete(request, responder); + session.stopDetectingCommandInjection(); + } + + protected void authFailure(ImapSession session, ImapRequest request, Responder responder, HumanReadableText failed, Optional username, Optional assumedUser, String log) { + AuditTrail.Entry entry = AuditTrail.entry() + .username(() -> username.map(name -> name.asString()).orElse(null)) + .sessionId(() -> session.sessionId().asString()) + .protocol("IMAP") + .action("AUTH") + .remoteIP(() -> Optional.ofNullable(session.getRemoteAddress())); + if (assumedUser.isPresent()) { + entry = entry.parameters(() -> ImmutableMap.of("delegatorUser", assumedUser.get().asString())); } + entry.log(log); + manageFailureCount(session, request, responder, failed); } protected static class AuthenticationAttempt { diff --git a/protocols/imap/src/main/java/org/apache/james/imap/processor/AuthenticateProcessor.java b/protocols/imap/src/main/java/org/apache/james/imap/processor/AuthenticateProcessor.java index 2988064e82d..cfae43c6f37 100644 --- a/protocols/imap/src/main/java/org/apache/james/imap/processor/AuthenticateProcessor.java +++ b/protocols/imap/src/main/java/org/apache/james/imap/processor/AuthenticateProcessor.java @@ -24,6 +24,7 @@ import java.util.Arrays; import java.util.Base64; import java.util.List; +import java.util.Optional; import java.util.stream.Collectors; import jakarta.inject.Inject; @@ -136,9 +137,8 @@ protected void parseAndDoPlainAuth(String initialClientResponse, ImapSession ses if (authenticationAttempt.isDelegation()) { doPasswordAuthWithDelegation(authenticationAttempt, session, request, responder); } else { - doPasswordAuth(authenticationAttempt, session, request, responder, HumanReadableText.AUTHENTICATION_FAILED); + doPasswordAuth(authenticationAttempt, session, request, responder); } - session.stopDetectingCommandInjection(); } /** @@ -148,8 +148,8 @@ protected void parseAndDoOAuth(String initialResponse, ImapSession session, Imap OIDCSASLParser.parse(initialResponse) .flatMap(oidcInitialResponseValue -> session.oidcSaslConfiguration().map(configure -> Pair.of(oidcInitialResponseValue, configure))) .ifPresentOrElse(pair -> doOAuth(pair.getLeft(), pair.getRight(), session, request, responder), - () -> manageFailureCount(session, request, responder)); - session.stopDetectingCommandInjection(); + () -> authFailure(session, request, responder, HumanReadableText.AUTHENTICATION_FAILED, Optional.empty(), + Optional.empty(), "Malformed authentication command.")); } private AuthenticationAttempt parseDelegationAttempt(String initialClientResponse) { diff --git a/protocols/imap/src/main/java/org/apache/james/imap/processor/LoginProcessor.java b/protocols/imap/src/main/java/org/apache/james/imap/processor/LoginProcessor.java index 989a4879655..c7c2a845b33 100644 --- a/protocols/imap/src/main/java/org/apache/james/imap/processor/LoginProcessor.java +++ b/protocols/imap/src/main/java/org/apache/james/imap/processor/LoginProcessor.java @@ -60,8 +60,7 @@ protected void processRequest(LoginRequest request, ImapSession session, Respond LOGGER.warn("Login rejected because it is disabled or not allowed over insecure channel"); no(request, responder, HumanReadableText.DISABLED_LOGIN); } else { - doPasswordAuth(noDelegation(request.getUserid(), request.getPassword()), - session, request, responder, HumanReadableText.INVALID_LOGIN); + doPasswordAuth(noDelegation(request.getUserid(), request.getPassword()), session, request, responder); } } From f2ecd05e2361a4b9fe820efe87329b247178ac2b Mon Sep 17 00:00:00 2001 From: Felix Auringer Date: Wed, 20 May 2026 09:57:10 +0200 Subject: [PATCH 6/6] fix(imap): use consistens error message in code and test for bad credentials --- examples/custom-imap/README.md | 2 +- .../org/apache/james/imap/scripts/AuthenticatePlain.test | 4 ++-- .../main/resources/org/apache/james/imap/scripts/Login.test | 4 ++-- .../org/apache/james/imap/scripts/LoginThreeStrikes.test | 4 ++-- .../org/apache/james/imap/api/display/HumanReadableText.java | 2 +- .../apache/james/imap/processor/AbstractAuthProcessor.java | 4 ++-- 6 files changed, 10 insertions(+), 10 deletions(-) diff --git a/examples/custom-imap/README.md b/examples/custom-imap/README.md index 1ca468c25c5..7d0eee03256 100644 --- a/examples/custom-imap/README.md +++ b/examples/custom-imap/README.md @@ -50,7 +50,7 @@ Connected to localhost. Escape character is '^]'. * OK JAMES IMAP4rev1 Server james.local is ready. a01 LOGIN bob secret -a01 NO LOGIN failed. Invalid login/password. +a01 NO LOGIN failed. Invalid credentials. a02 LOGIN bob@localhost secret a02 OK LOGIN completed. A03 PING diff --git a/mpt/impl/imap-mailbox/core/src/main/resources/org/apache/james/imap/scripts/AuthenticatePlain.test b/mpt/impl/imap-mailbox/core/src/main/resources/org/apache/james/imap/scripts/AuthenticatePlain.test index b5ca18a5c6d..ffe42f2f18d 100644 --- a/mpt/impl/imap-mailbox/core/src/main/resources/org/apache/james/imap/scripts/AuthenticatePlain.test +++ b/mpt/impl/imap-mailbox/core/src/main/resources/org/apache/james/imap/scripts/AuthenticatePlain.test @@ -85,7 +85,7 @@ REINIT C: 0005 AUTHENTICATE "PLAIN" {28+} # \0imapuser\0badpassword C: AGltYXB1c2VyAGJhZHBhc3N3b3Jk -S: 0005 NO AUTHENTICATE failed. Authentication failed. +S: 0005 NO AUTHENTICATE failed. Invalid credentials. REINIT @@ -93,7 +93,7 @@ REINIT C: 0006 AUTHENTICATE "PLAIN" {24+} # \0baduser\0password C: AGJhZHVzZXIAcGFzc3dvcmQ= -S: 0006 NO AUTHENTICATE failed. Authentication failed. +S: 0006 NO AUTHENTICATE failed. Invalid credentials. REINIT diff --git a/mpt/impl/imap-mailbox/core/src/main/resources/org/apache/james/imap/scripts/Login.test b/mpt/impl/imap-mailbox/core/src/main/resources/org/apache/james/imap/scripts/Login.test index 84c8d7b7403..500d2a310e8 100644 --- a/mpt/impl/imap-mailbox/core/src/main/resources/org/apache/james/imap/scripts/Login.test +++ b/mpt/impl/imap-mailbox/core/src/main/resources/org/apache/james/imap/scripts/Login.test @@ -27,10 +27,10 @@ C: a002a LOGIN imapuser password extra S: a002a BAD LOGIN failed. Illegal arguments. C: a003 LOGIN invaliduser password -S: a003 NO LOGIN failed. Invalid login/password. +S: a003 NO LOGIN failed. Invalid credentials. C: a004 LOGIN imapuser invalid -S: a004 NO LOGIN failed. Invalid login/password. +S: a004 NO LOGIN failed. Invalid credentials. C: a005 LOGIN imapuser password S: a005 OK LOGIN completed. \ No newline at end of file diff --git a/mpt/impl/imap-mailbox/core/src/main/resources/org/apache/james/imap/scripts/LoginThreeStrikes.test b/mpt/impl/imap-mailbox/core/src/main/resources/org/apache/james/imap/scripts/LoginThreeStrikes.test index 231f4a20791..c73fb9321c9 100644 --- a/mpt/impl/imap-mailbox/core/src/main/resources/org/apache/james/imap/scripts/LoginThreeStrikes.test +++ b/mpt/impl/imap-mailbox/core/src/main/resources/org/apache/james/imap/scripts/LoginThreeStrikes.test @@ -21,10 +21,10 @@ # S: \* OK IMAP4rev1 Server ready C: a003 LOGIN invaliduser password -S: a003 NO LOGIN failed. Invalid login/password. +S: a003 NO LOGIN failed. Invalid credentials. C: a004 LOGIN imapuser invalid -S: a004 NO LOGIN failed. Invalid login/password. +S: a004 NO LOGIN failed. Invalid credentials. C: a005 LOGIN imapuser bogus S: \* BYE Login failed too many times. \ No newline at end of file diff --git a/protocols/imap/src/main/java/org/apache/james/imap/api/display/HumanReadableText.java b/protocols/imap/src/main/java/org/apache/james/imap/api/display/HumanReadableText.java index 9746c5ca85e..7b170d05269 100644 --- a/protocols/imap/src/main/java/org/apache/james/imap/api/display/HumanReadableText.java +++ b/protocols/imap/src/main/java/org/apache/james/imap/api/display/HumanReadableText.java @@ -141,7 +141,7 @@ public static HumanReadableText permanentFlags(Flags flags) { public static final HumanReadableText COMPLETED = new HumanReadableText("org.apache.james.imap.COMPLETED", "completed."); public static final HumanReadableText REPLACE_READY = new HumanReadableText("org.apache.james.imap.REPLACE", "Replacement Message ready"); - public static final HumanReadableText INVALID_LOGIN = new HumanReadableText("org.apache.james.imap.INVALID_LOGIN", "failed. Invalid login/password."); + public static final HumanReadableText INVALID_CREDENTIALS = new HumanReadableText("org.apache.james.imap.INVALID_CREDENTIALS", "failed. Invalid credentials."); public static final HumanReadableText DISABLED_LOGIN = new HumanReadableText("org.apache.james.imap.DISABLED_LOGIN", "failed. Plain login / authentication are disabled."); diff --git a/protocols/imap/src/main/java/org/apache/james/imap/processor/AbstractAuthProcessor.java b/protocols/imap/src/main/java/org/apache/james/imap/processor/AbstractAuthProcessor.java index 7b43ad24b95..7b9cdbdb08f 100644 --- a/protocols/imap/src/main/java/org/apache/james/imap/processor/AbstractAuthProcessor.java +++ b/protocols/imap/src/main/java/org/apache/james/imap/processor/AbstractAuthProcessor.java @@ -95,7 +95,7 @@ protected void doPasswordAuth(AuthenticationAttempt authenticationAttempt, ImapS ).withoutDelegation(); authSuccess(session, mailboxSession, request, responder, "Password authentication succeeded."); } catch (BadCredentialsException e) { - authFailure(session, request, responder, HumanReadableText.INVALID_LOGIN, + authFailure(session, request, responder, HumanReadableText.INVALID_CREDENTIALS, Optional.of(authenticationAttempt.getAuthenticationId()), Optional.empty(), "Password authentication failed because of bad credentials." @@ -143,7 +143,7 @@ protected void doAuthWithDelegation(MailboxSessionAuthWithDelegationSupplier mai try { authSuccess(session, mailboxSessionSupplier.get(), request, responder, "Authentication with delegation succeeded."); } catch (BadCredentialsException e) { - authFailure(session, request, responder, HumanReadableText.INVALID_LOGIN, Optional.of(authenticateUser), + authFailure(session, request, responder, HumanReadableText.INVALID_CREDENTIALS, Optional.of(authenticateUser), Optional.of(delegatorUser), "Password authentication with delegation failed because of bad credentials."); } catch (UserDoesNotExistException e) { authFailure(session, request, responder, HumanReadableText.USER_DOES_NOT_EXIST, Optional.of(authenticateUser),