This is an automated email from the ASF dual-hosted git repository. btellier pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/james-project.git
commit 1cd3f9547e6e9ae913fc0c5615b28a891a409b66 Author: Felix Auringer <[email protected]> AuthorDate: Wed Oct 8 17:20:38 2025 +0200 refactor: add more errors and reset authentication state on authentication error While trying to get the MPT tests for managesieve running, I realized that a client is trapped in the state "AuthenticationInProgress" when using continuation. This made longer test scripts impossible. To consistently reset the session, I introduced more errors that are handled in one place instead of returning a "NO" answer directly during authentication. Those changes also required some small changes to the MPT and the normal tests. Additionally, there were a parsing bug when sending space-separated username and password in the authentication continuation which is fixed now. --- .../james/managesieve/scripts/authenticate.test | 23 +++++---- .../managesieve/scripts/authenticateBase64.test | 2 +- .../james/managesieve/scripts/capability.test | 7 +-- .../james/managesieve/scripts/checkscript.test | 6 +-- .../james/managesieve/scripts/deletescript.test | 7 +-- .../james/managesieve/scripts/getscript.test | 7 ++- .../james/managesieve/scripts/havespace.test | 7 +-- .../james/managesieve/scripts/listscripts.test | 5 +- .../apache/james/managesieve/scripts/logout.test | 2 +- .../org/apache/james/managesieve/scripts/noop.test | 2 +- .../james/managesieve/scripts/putscript.test | 6 +-- .../james/managesieve/scripts/renamescript.test | 7 ++- .../james/managesieve/scripts/setactive.test | 5 +- .../apache/james/managesieve/scripts/starttls.test | 7 +-- .../james/managesieve/scripts/unauthenticate.test | 7 +-- .../james/managesieve/core/CoreProcessor.java | 50 ++++++++++-------- .../core/PlainAuthenticationProcessor.java | 6 +-- .../transcode/ManageSieveProcessor.java | 59 +++++++++++++--------- .../apache/james/managesieve/util/ParserUtils.java | 4 +- .../james/managesieveserver/AuthenticateTest.java | 22 ++++---- .../apache/james/managesieveserver/OIDCTest.java | 4 +- 21 files changed, 137 insertions(+), 108 deletions(-) diff --git a/mpt/impl/managesieve/core/src/main/resources/org/apache/james/managesieve/scripts/authenticate.test b/mpt/impl/managesieve/core/src/main/resources/org/apache/james/managesieve/scripts/authenticate.test index 334699cf14..9914f24f28 100644 --- a/mpt/impl/managesieve/core/src/main/resources/org/apache/james/managesieve/scripts/authenticate.test +++ b/mpt/impl/managesieve/core/src/main/resources/org/apache/james/managesieve/scripts/authenticate.test @@ -18,20 +18,25 @@ ################################################################ C: AUTHENTICATE -S: NO ManageSieve syntax is incorrect : You must specify a SASL mechanism as an argument of AUTHENTICATE command +S: NO "ManageSieve syntax is incorrect: quoted SASL mechanism must be supplied" C: AUTHENTICATE "UNKNOWN" -S: NO Unknown SASL mechanism UNKNOWN +S: NO "Unknown SASL mechanism UNKNOWN" C: AUTHENTICATE "PLAIN" -S: \+ "" +S: "" +S: OK C: GETSCRIPT toto.sieve -S: NO ManageSieve syntax is incorrect : You must supply a password for the authentication mechanism. Formal syntax : <NULL>username<NULL>password +S: NO "ManageSieve syntax is incorrect: quoted authentication data must be supplied" -C: tin password -S: NO authentication failed +C: AUTHENTICATE "PLAIN" +S: "" +S: OK +C: "tin password" +S: NO "Authentication failed with: Verification of credentials failed" C: AUTHENTICATE "PLAIN" -S: \+ "" -C: user password -S: OK \ No newline at end of file +S: "" +S: OK +C: "user password" +S: OK diff --git a/mpt/impl/managesieve/core/src/main/resources/org/apache/james/managesieve/scripts/authenticateBase64.test b/mpt/impl/managesieve/core/src/main/resources/org/apache/james/managesieve/scripts/authenticateBase64.test index cee22fb254..720257bbb2 100644 --- a/mpt/impl/managesieve/core/src/main/resources/org/apache/james/managesieve/scripts/authenticateBase64.test +++ b/mpt/impl/managesieve/core/src/main/resources/org/apache/james/managesieve/scripts/authenticateBase64.test @@ -18,4 +18,4 @@ ################################################################ C: AUTHENTICATE "PLAIN" "AHVzZXIAcGFzc3dvcmQ=" -S: OK \ No newline at end of file +S: OK diff --git a/mpt/impl/managesieve/core/src/main/resources/org/apache/james/managesieve/scripts/capability.test b/mpt/impl/managesieve/core/src/main/resources/org/apache/james/managesieve/scripts/capability.test index 1b2141b124..16e6a75e02 100644 --- a/mpt/impl/managesieve/core/src/main/resources/org/apache/james/managesieve/scripts/capability.test +++ b/mpt/impl/managesieve/core/src/main/resources/org/apache/james/managesieve/scripts/capability.test @@ -38,8 +38,9 @@ S: "VERSION" "1.0" S: OK C: AUTHENTICATE "PLAIN" -S: \+ "" -C: user password +S: "" +S: OK +C: "user password" S: OK C: CAPABILITY @@ -51,4 +52,4 @@ S: "STARTTLS" S: "IMPLEMENTATION" "Apache ManageSieve v1.0" S: "VERSION" "1.0" } -S: OK \ No newline at end of file +S: OK diff --git a/mpt/impl/managesieve/core/src/main/resources/org/apache/james/managesieve/scripts/checkscript.test b/mpt/impl/managesieve/core/src/main/resources/org/apache/james/managesieve/scripts/checkscript.test index 8fc76dd500..33cc62e821 100644 --- a/mpt/impl/managesieve/core/src/main/resources/org/apache/james/managesieve/scripts/checkscript.test +++ b/mpt/impl/managesieve/core/src/main/resources/org/apache/james/managesieve/scripts/checkscript.test @@ -34,8 +34,9 @@ C: S: NO C: AUTHENTICATE "PLAIN" -S: \+ "" -C: user password +S: "" +S: OK +C: "user password" S: OK C: CHECKSCRIPT {99+} @@ -51,4 +52,3 @@ C: #comment C: InvalidSieveCommand C: S: NO "Syntax Error: org.apache.jsieve.parser.generated.ParseException: Encountered "<EOF>" at line 2, column 21. - diff --git a/mpt/impl/managesieve/core/src/main/resources/org/apache/james/managesieve/scripts/deletescript.test b/mpt/impl/managesieve/core/src/main/resources/org/apache/james/managesieve/scripts/deletescript.test index 5352d2aaf6..91658e2b55 100644 --- a/mpt/impl/managesieve/core/src/main/resources/org/apache/james/managesieve/scripts/deletescript.test +++ b/mpt/impl/managesieve/core/src/main/resources/org/apache/james/managesieve/scripts/deletescript.test @@ -24,8 +24,9 @@ C: DELETESCRIPT "foo" S: NO C: AUTHENTICATE "PLAIN" -S: \+ "" -C: user password +S: "" +S: OK +C: "user password" S: OK C: DELETESCRIPT "foo" @@ -77,4 +78,4 @@ C: SETACTIVE "mysievescript" S: OK C: DELETESCRIPT "mysievescript" -S: NO \(ACTIVE\) "You may not delete an active script" \ No newline at end of file +S: NO \(ACTIVE\) "You may not delete an active script" diff --git a/mpt/impl/managesieve/core/src/main/resources/org/apache/james/managesieve/scripts/getscript.test b/mpt/impl/managesieve/core/src/main/resources/org/apache/james/managesieve/scripts/getscript.test index aa5ca1d9bb..8d1359100e 100644 --- a/mpt/impl/managesieve/core/src/main/resources/org/apache/james/managesieve/scripts/getscript.test +++ b/mpt/impl/managesieve/core/src/main/resources/org/apache/james/managesieve/scripts/getscript.test @@ -24,8 +24,9 @@ C: GETSCRIPT "foo" S: NO C: AUTHENTICATE "PLAIN" -S: \+ "" -C: user password +S: "" +S: OK +C: "user password" S: OK C: GETSCRIPT "foo" @@ -48,5 +49,3 @@ S: fileinto "INBOX.sent"; S: \} S: S: OK - - diff --git a/mpt/impl/managesieve/core/src/main/resources/org/apache/james/managesieve/scripts/havespace.test b/mpt/impl/managesieve/core/src/main/resources/org/apache/james/managesieve/scripts/havespace.test index 2b0958276e..653f60fe43 100644 --- a/mpt/impl/managesieve/core/src/main/resources/org/apache/james/managesieve/scripts/havespace.test +++ b/mpt/impl/managesieve/core/src/main/resources/org/apache/james/managesieve/scripts/havespace.test @@ -27,12 +27,13 @@ C: HAVESPACE "scriptname" 49 S: NO C: AUTHENTICATE "PLAIN" -S: \+ "" -C: user password +S: "" +S: OK +C: "user password" S: OK C: HAVESPACE "scriptname" 49 S: OK C: HAVESPACE "scriptname" 51 -S: NO \(QUOTA/MAXSIZE\) "Quota exceeded" \ No newline at end of file +S: NO \(QUOTA/MAXSIZE\) "Quota exceeded" diff --git a/mpt/impl/managesieve/core/src/main/resources/org/apache/james/managesieve/scripts/listscripts.test b/mpt/impl/managesieve/core/src/main/resources/org/apache/james/managesieve/scripts/listscripts.test index 1539277af0..a59dd4a700 100644 --- a/mpt/impl/managesieve/core/src/main/resources/org/apache/james/managesieve/scripts/listscripts.test +++ b/mpt/impl/managesieve/core/src/main/resources/org/apache/james/managesieve/scripts/listscripts.test @@ -21,8 +21,9 @@ C: LISTSCRIPTS S: NO C: AUTHENTICATE "PLAIN" -S: \+ "" -C: user password +S: "" +S: OK +C: "user password" S: OK C: LISTSCRIPTS diff --git a/mpt/impl/managesieve/core/src/main/resources/org/apache/james/managesieve/scripts/logout.test b/mpt/impl/managesieve/core/src/main/resources/org/apache/james/managesieve/scripts/logout.test index 125c4210e4..65bb0a2cc5 100644 --- a/mpt/impl/managesieve/core/src/main/resources/org/apache/james/managesieve/scripts/logout.test +++ b/mpt/impl/managesieve/core/src/main/resources/org/apache/james/managesieve/scripts/logout.test @@ -18,4 +18,4 @@ ################################################################ C: LOGOUT -S: OK channel is closing \ No newline at end of file +S: OK channel is closing diff --git a/mpt/impl/managesieve/core/src/main/resources/org/apache/james/managesieve/scripts/noop.test b/mpt/impl/managesieve/core/src/main/resources/org/apache/james/managesieve/scripts/noop.test index 837fb93001..bebba0bd4a 100644 --- a/mpt/impl/managesieve/core/src/main/resources/org/apache/james/managesieve/scripts/noop.test +++ b/mpt/impl/managesieve/core/src/main/resources/org/apache/james/managesieve/scripts/noop.test @@ -25,4 +25,4 @@ S: OK \(TAG \{16\} S: STARTTLS-SYNC-42\) "DONE" C: NooP -S: OK "NOOP completed" \ No newline at end of file +S: OK "NOOP completed" diff --git a/mpt/impl/managesieve/core/src/main/resources/org/apache/james/managesieve/scripts/putscript.test b/mpt/impl/managesieve/core/src/main/resources/org/apache/james/managesieve/scripts/putscript.test index e8e300f64d..2b9c72e33f 100644 --- a/mpt/impl/managesieve/core/src/main/resources/org/apache/james/managesieve/scripts/putscript.test +++ b/mpt/impl/managesieve/core/src/main/resources/org/apache/james/managesieve/scripts/putscript.test @@ -39,8 +39,9 @@ C: S: NO C: AUTHENTICATE "PLAIN" -S: \+ "" -C: user password +S: "" +S: OK +C: "user password" S: OK C: PUTSCRIPT "mysievescript" {97+} @@ -56,4 +57,3 @@ C: #comment C: InvalidSieveCommand C: S: NO "Syntax Error: org.apache.jsieve.parser.generated.ParseException: Encountered "<EOF>" at line 2, column 21. - diff --git a/mpt/impl/managesieve/core/src/main/resources/org/apache/james/managesieve/scripts/renamescript.test b/mpt/impl/managesieve/core/src/main/resources/org/apache/james/managesieve/scripts/renamescript.test index 355cd84461..1de65f25d0 100644 --- a/mpt/impl/managesieve/core/src/main/resources/org/apache/james/managesieve/scripts/renamescript.test +++ b/mpt/impl/managesieve/core/src/main/resources/org/apache/james/managesieve/scripts/renamescript.test @@ -27,8 +27,9 @@ C: RENAMESCRIPT "foo" "bar" S: NO C: AUTHENTICATE "PLAIN" -S: \+ "" -C: user password +S: "" +S: OK +C: "user password" S: OK C: PUTSCRIPT "mysievescript" {99+} @@ -75,5 +76,3 @@ S: OK C: RENAMESCRIPT "mysievescript" "mysievescriptbis" S: NO \(ALREADYEXISTS\) "A script with that name already exists" - - diff --git a/mpt/impl/managesieve/core/src/main/resources/org/apache/james/managesieve/scripts/setactive.test b/mpt/impl/managesieve/core/src/main/resources/org/apache/james/managesieve/scripts/setactive.test index c05ff82e95..5a836693f6 100644 --- a/mpt/impl/managesieve/core/src/main/resources/org/apache/james/managesieve/scripts/setactive.test +++ b/mpt/impl/managesieve/core/src/main/resources/org/apache/james/managesieve/scripts/setactive.test @@ -24,8 +24,9 @@ C: SETACTIVE "foo" S: NO C: AUTHENTICATE "PLAIN" -S: \+ "" -C: user password +S: "" +S: OK +C: "user password" S: OK C: SETACTIVE "foo" diff --git a/mpt/impl/managesieve/core/src/main/resources/org/apache/james/managesieve/scripts/starttls.test b/mpt/impl/managesieve/core/src/main/resources/org/apache/james/managesieve/scripts/starttls.test index 7e973540b4..8220140f7e 100644 --- a/mpt/impl/managesieve/core/src/main/resources/org/apache/james/managesieve/scripts/starttls.test +++ b/mpt/impl/managesieve/core/src/main/resources/org/apache/james/managesieve/scripts/starttls.test @@ -24,11 +24,12 @@ C: STARTTLS S: NO You can't enable two time SSL encryption C: AUTHENTICATE "PLAIN" -S: \+ "" -C: user password +S: "" +S: OK +C: "user password" S: OK C: STARTTLS S: NO command STARTTLS is issued in the wrong state. It must be issued as you are unauthenticated -C: LOGOUT \ No newline at end of file +C: LOGOUT diff --git a/mpt/impl/managesieve/core/src/main/resources/org/apache/james/managesieve/scripts/unauthenticate.test b/mpt/impl/managesieve/core/src/main/resources/org/apache/james/managesieve/scripts/unauthenticate.test index 4acc436f4a..21ef39e49f 100644 --- a/mpt/impl/managesieve/core/src/main/resources/org/apache/james/managesieve/scripts/unauthenticate.test +++ b/mpt/impl/managesieve/core/src/main/resources/org/apache/james/managesieve/scripts/unauthenticate.test @@ -27,8 +27,9 @@ C: UNAUTHENTICATE S: NO UNAUTHENTICATE command must be issued in authenticated state C: AUTHENTICATE "PLAIN" -S: \+ "" -C: user password +S: "" +S: OK +C: "user password" S: OK C: GETSCRIPT any @@ -38,4 +39,4 @@ C: UNAUTHENTICATE S: OK C: GETSCRIPT any -S: NO \ No newline at end of file +S: NO diff --git a/protocols/managesieve/src/main/java/org/apache/james/managesieve/core/CoreProcessor.java b/protocols/managesieve/src/main/java/org/apache/james/managesieve/core/CoreProcessor.java index ebb8b7e36d..33cc055fb1 100644 --- a/protocols/managesieve/src/main/java/org/apache/james/managesieve/core/CoreProcessor.java +++ b/protocols/managesieve/src/main/java/org/apache/james/managesieve/core/CoreProcessor.java @@ -210,25 +210,26 @@ public class CoreProcessor implements CoreCommands { @Override public String chooseMechanism(Session session, String mechanism) { - if (session.isAuthenticated()) { - return "NO \"already authenticated\""; - } try { if (Strings.isNullOrEmpty(mechanism)) { - return "NO ManageSieve syntax is incorrect : You must specify a SASL mechanism as an argument of AUTHENTICATE command"; + throw new SyntaxException("quoted SASL mechanism must be supplied"); } - String unquotedMechanism = ParserUtils.unquoteFirst(mechanism); - SupportedMechanism supportedMechanism = SupportedMechanism.retrieveMechanism(unquotedMechanism); + + SupportedMechanism supportedMechanism = SupportedMechanism.retrieveMechanism(mechanism); if (!this.authenticationProcessorMap.containsKey(supportedMechanism)) { - throw new UnknownSaslMechanism("SASL mechanism disabled: " + unquotedMechanism); + throw new UnknownSaslMechanism("SASL mechanism disabled: " + mechanism); } session.setChoosedAuthenticationMechanism(supportedMechanism); session.setState(Session.State.AUTHENTICATION_IN_PROGRESS); AuthenticationProcessor authenticationProcessor = authenticationProcessorMap.get(supportedMechanism); return authenticationProcessor.initialServerResponse(session); - } catch (UnknownSaslMechanism unknownSaslMechanism) { - return "NO " + unknownSaslMechanism.getMessage(); + } catch (UnknownSaslMechanism e) { + resetSession(session); + return "NO \"" + e.getMessage() + "\""; + } catch (SyntaxException e) { + resetSession(session); + return "NO \"ManageSieve syntax is incorrect: " + e.getMessage() + "\""; } } @@ -237,41 +238,46 @@ public class CoreProcessor implements CoreCommands { try { SupportedMechanism currentAuthenticationMechanism = session.getChoosedAuthenticationMechanism(); AuthenticationProcessor authenticationProcessor = authenticationProcessorMap.get(currentAuthenticationMechanism); - String unquotedSuppliedData = ParserUtils.unquoteFirst(suppliedData); - if (unquotedSuppliedData == null) { - return "NO \"authentication failed\""; + if (Strings.isNullOrEmpty(suppliedData)) { + throw new SyntaxException("quoted authentication data must be supplied"); } - if (unquotedSuppliedData.equals("*")) { - return "NO \"authentication aborted\""; + if (suppliedData.equals("*")) { + throw new AuthenticationException("authentication aborted by client"); } - Username authenticatedUsername = authenticationProcessor.isAuthenticationSuccesfull(session, unquotedSuppliedData); + Username authenticatedUsername = authenticationProcessor.isAuthenticationSuccesfull(session, suppliedData); if (authenticatedUsername != null) { session.setUser(authenticatedUsername); session.setState(Session.State.AUTHENTICATED); return "OK"; } else { - session.setState(Session.State.UNAUTHENTICATED); - session.setUser(null); - return "NO authentication failed"; + resetSession(session); + return "NO \"authentication failed\""; } } catch (AuthenticationException e) { - return "NO Authentication failed with: " + e.getMessage(); + resetSession(session); + return "NO \"Authentication failed with: " + e.getMessage() + "\""; } catch (SyntaxException e) { - return "NO ManageSieve syntax is incorrect : " + e.getMessage(); + resetSession(session); + return "NO \"ManageSieve syntax is incorrect: " + e.getMessage() + "\""; } } @Override public String unauthenticate(Session session) { if (session.isAuthenticated()) { - session.setState(Session.State.UNAUTHENTICATED); - session.setUser(null); + resetSession(session); return "OK"; } else { return "NO UNAUTHENTICATE command must be issued in authenticated state"; } } + private static void resetSession(Session session) { + session.setState(Session.State.UNAUTHENTICATED); + session.setUser(null); + session.setChoosedAuthenticationMechanism(null); + } + @Override public void logout() throws SessionTerminatedException { throw new SessionTerminatedException(); diff --git a/protocols/managesieve/src/main/java/org/apache/james/managesieve/core/PlainAuthenticationProcessor.java b/protocols/managesieve/src/main/java/org/apache/james/managesieve/core/PlainAuthenticationProcessor.java index 72837b37e6..1a6164b5d8 100644 --- a/protocols/managesieve/src/main/java/org/apache/james/managesieve/core/PlainAuthenticationProcessor.java +++ b/protocols/managesieve/src/main/java/org/apache/james/managesieve/core/PlainAuthenticationProcessor.java @@ -72,11 +72,11 @@ public class PlainAuthenticationProcessor implements AuthenticationProcessor { private Username authenticateWithSeparator(Session session, String suppliedClientData, char c) throws SyntaxException, AuthenticationException { Iterator<String> it = Splitter.on(c).omitEmptyStrings().split(suppliedClientData).iterator(); if (!it.hasNext()) { - throw new SyntaxException("You must supply a username for the authentication mechanism. Formal syntax : <NULL>username<NULL>password"); + throw new SyntaxException("You must supply a username for the authentication mechanism. Formal syntax: <NULL>username<NULL>password"); } Username userName = Username.of(it.next()); if (!it.hasNext()) { - throw new SyntaxException("You must supply a password for the authentication mechanism. Formal syntax : <NULL>username<NULL>password"); + throw new SyntaxException("You must supply a password for the authentication mechanism. Formal syntax: <NULL>username<NULL>password"); } String password = it.next(); session.setUser(userName); @@ -85,7 +85,7 @@ public class PlainAuthenticationProcessor implements AuthenticationProcessor { if (user != null && user.verifyPassword(password)) { return user.getUserName(); } else { - return null; + throw new AuthenticationException("Verification of credentials failed"); } } catch (UsersRepositoryException e) { throw new AuthenticationException(e.getMessage()); diff --git a/protocols/managesieve/src/main/java/org/apache/james/managesieve/transcode/ManageSieveProcessor.java b/protocols/managesieve/src/main/java/org/apache/james/managesieve/transcode/ManageSieveProcessor.java index d07b323c4d..83cca6f8a0 100644 --- a/protocols/managesieve/src/main/java/org/apache/james/managesieve/transcode/ManageSieveProcessor.java +++ b/protocols/managesieve/src/main/java/org/apache/james/managesieve/transcode/ManageSieveProcessor.java @@ -22,10 +22,10 @@ package org.apache.james.managesieve.transcode; import jakarta.inject.Inject; -import org.apache.commons.lang3.StringUtils; import org.apache.james.managesieve.api.ManageSieveException; import org.apache.james.managesieve.api.Session; import org.apache.james.managesieve.api.SessionTerminatedException; +import org.apache.james.managesieve.util.ParserUtils; import org.apache.james.sieverepository.api.exception.SieveRepositoryException; public class ManageSieveProcessor { @@ -54,6 +54,17 @@ public class ManageSieveProcessor { } public String handleRequest(Session session, String request) throws ManageSieveException, SieveRepositoryException { + if (request.endsWith("\n")) { + request = request.substring(0, request.length() - 1); + } + if (request.endsWith("\r")) { + request = request.substring(0, request.length() - 1); + } + + if (session.getState() == Session.State.AUTHENTICATION_IN_PROGRESS) { + return matchCommandWithImplementation(session, request.trim(), AUTHENTICATE) + "\r\n"; + } + int firstWordEndIndex = request.indexOf(' '); String arguments = parseArguments(request, firstWordEndIndex); String command = parseCommand(request, firstWordEndIndex); @@ -67,12 +78,6 @@ public class ManageSieveProcessor { } else { command = request; } - if (command.endsWith("\n")) { - command = command.substring(0, command.length() - 1); - } - if (command.endsWith("\r")) { - command = command.substring(0, command.length() - 1); - } return command; } @@ -85,27 +90,35 @@ public class ManageSieveProcessor { } private String matchCommandWithImplementation(Session session, String arguments, String command) throws SessionTerminatedException { - if (session.getState() == Session.State.AUTHENTICATION_IN_PROGRESS) { - return argumentParser.authenticate(session, command); - } if (command.equalsIgnoreCase(AUTHENTICATE)) { - if (StringUtils.countMatches(arguments, "\"") == 4) { - String result = argumentParser.chooseMechanism(session, arguments); + // The RFC forbids the AUTHENTICATE command if the session is already authenticated. + if (session.isAuthenticated()) { + return "NO \"already authenticated\""; + } + + // If no authentication is in progress, the authentication mechanism needs to be chosen. + if (session.getState() != Session.State.AUTHENTICATION_IN_PROGRESS) { + String mechanism = ParserUtils.unquoteFirst(arguments); + String result = argumentParser.chooseMechanism(session, mechanism); + // If the authentication is not in progress, return the result (error) because choosing the mechanism has failed. if (session.getState() != Session.State.AUTHENTICATION_IN_PROGRESS) { return result; } - int bracket1 = arguments.indexOf('\"'); - int bracket2 = arguments.indexOf('\"', bracket1 + 1); - int bracket3 = arguments.indexOf('\"', bracket2 + 1); - int bracket4 = arguments.indexOf('\"', bracket3 + 1); - - return argumentParser.authenticate(session, arguments.substring(bracket3, bracket4 + 1)); - } else if (arguments.split(" ").length != 1) { - // The client send additional arguments but didn't quote them. It probably thinks that it does not need - // to send more, but the server expects more. Reject this authentication now to solve this conflict. - return "NO \"unquoted argument found\""; + + // Skips the whole mechanism, the closing quote, and the space if present. + // If the request is well-formatted, the arguments are now empty or contain the client's initial response. + arguments = arguments.substring(arguments.indexOf(mechanism) + mechanism.length() + 1); + if (arguments.startsWith(" ")) { + arguments = arguments.substring(1); + } + // If there are is no initial client response left, return the result (initial server response). + if (arguments.isEmpty()) { + return result; + } } - return argumentParser.chooseMechanism(session, arguments); + + // The authentication is in progress, the mechanism has been chosen, and the arguments contain an initial client response. + return argumentParser.authenticate(session, ParserUtils.unquoteFirst(arguments)); } else if (command.equalsIgnoreCase(CAPABILITY)) { return argumentParser.capability(session, arguments); } else if (command.equalsIgnoreCase(CHECKSCRIPT)) { diff --git a/protocols/managesieve/src/main/java/org/apache/james/managesieve/util/ParserUtils.java b/protocols/managesieve/src/main/java/org/apache/james/managesieve/util/ParserUtils.java index f07198843f..86fbd7c02d 100644 --- a/protocols/managesieve/src/main/java/org/apache/james/managesieve/util/ParserUtils.java +++ b/protocols/managesieve/src/main/java/org/apache/james/managesieve/util/ParserUtils.java @@ -56,8 +56,8 @@ public class ParserUtils { } if (quoted.length() > 2 && quoted.startsWith("\"") && quoted.indexOf('\"', 1) >= 0) { return quoted.substring(1, quoted.indexOf('\"', 1)); - } else if (quoted.startsWith("'") && quoted.endsWith("'")) { - return quoted.substring(1, quoted.length() - 1); + } else if (quoted.length() > 2 && quoted.startsWith("'") && quoted.indexOf('\'', 1) >= 0) { + return quoted.substring(1, quoted.indexOf('\'', 1)); } return null; } diff --git a/server/protocols/protocols-managesieve/src/test/java/org/apache/james/managesieveserver/AuthenticateTest.java b/server/protocols/protocols-managesieve/src/test/java/org/apache/james/managesieveserver/AuthenticateTest.java index b9d0829aa3..91d99139c2 100644 --- a/server/protocols/protocols-managesieve/src/test/java/org/apache/james/managesieveserver/AuthenticateTest.java +++ b/server/protocols/protocols-managesieve/src/test/java/org/apache/james/managesieveserver/AuthenticateTest.java @@ -57,7 +57,7 @@ public class AuthenticateTest { @Test void plainLoginWithWrongPasswordShouldNotSucceed() throws IOException { - String initialClientResponse = ("\0" + ManageSieveServerTestSystem.USERNAME.asString() + "\0" + ManageSieveServerTestSystem.PASSWORD + "wrong"); + String initialClientResponse = "\0" + ManageSieveServerTestSystem.USERNAME.asString() + "\0" + ManageSieveServerTestSystem.PASSWORD + "wrong"; this.client.sendCommand("AUTHENTICATE \"PLAIN\" \"" + Base64.getEncoder().encodeToString(initialClientResponse.getBytes(StandardCharsets.UTF_8)) + "\""); ManageSieveClient.ServerResponse authenticationResponse = this.client.readResponse(); Assertions.assertThat(authenticationResponse.responseType()).isEqualTo(ManageSieveClient.ResponseType.NO); @@ -65,7 +65,7 @@ public class AuthenticateTest { @Test void plainLoginWithNotExistingUserShouldNotSucceed() throws IOException { - String initialClientResponse = ("\0" + ManageSieveServerTestSystem.USERNAME.asString() + "not-existing" + "\0" + "pwd"); + String initialClientResponse = "\0" + ManageSieveServerTestSystem.USERNAME.asString() + "not-existing" + "\0" + "pwd"; this.client.sendCommand("AUTHENTICATE \"PLAIN\" \"" + Base64.getEncoder().encodeToString(initialClientResponse.getBytes(StandardCharsets.UTF_8)) + "\""); ManageSieveClient.ServerResponse authenticationResponse = this.client.readResponse(); Assertions.assertThat(authenticationResponse.responseType()).isEqualTo(ManageSieveClient.ResponseType.NO); @@ -73,7 +73,7 @@ public class AuthenticateTest { @Test void plainLoginWithoutPasswordShouldNotSucceed() throws IOException { - String initialClientResponse = ("\0" + ManageSieveServerTestSystem.USERNAME.asString() + "\0"); + String initialClientResponse = "\0" + ManageSieveServerTestSystem.USERNAME.asString() + "\0"; this.client.sendCommand("AUTHENTICATE \"PLAIN\" \"" + Base64.getEncoder().encodeToString(initialClientResponse.getBytes(StandardCharsets.UTF_8)) + "\""); ManageSieveClient.ServerResponse authenticationResponse = this.client.readResponse(); Assertions.assertThat(authenticationResponse.responseType()).isEqualTo(ManageSieveClient.ResponseType.NO); @@ -81,11 +81,11 @@ public class AuthenticateTest { // The SASL PLAIN standard (https://datatracker.ietf.org/doc/html/rfc4616) defines the following message: // message = [authzid] UTF8NUL authcid UTF8NUL passwd - // The current code is more lenient. + // The current code is more lenient and accepts the message without the first null byte. @Disabled @Test void plainLoginWithMalformedMessageShouldNotSucceed() throws IOException { - String initialClientResponse = (ManageSieveServerTestSystem.USERNAME.asString() + "\0" + ManageSieveServerTestSystem.PASSWORD); + String initialClientResponse = ManageSieveServerTestSystem.USERNAME.asString() + "\0" + ManageSieveServerTestSystem.PASSWORD; this.client.sendCommand("AUTHENTICATE \"PLAIN\" \"" + Base64.getEncoder().encodeToString(initialClientResponse.getBytes(StandardCharsets.UTF_8)) + "\""); ManageSieveClient.ServerResponse authenticationResponse = this.client.readResponse(); Assertions.assertThat(authenticationResponse.responseType()).isEqualTo(ManageSieveClient.ResponseType.NO); @@ -93,7 +93,7 @@ public class AuthenticateTest { @Test void plainLoginWithoutMechanismQuotesShouldNotSucceed() throws IOException { - String initialClientResponse = ("\0" + ManageSieveServerTestSystem.USERNAME.asString() + "\0" + ManageSieveServerTestSystem.PASSWORD); + String initialClientResponse = "\0" + ManageSieveServerTestSystem.USERNAME.asString() + "\0" + ManageSieveServerTestSystem.PASSWORD; this.client.sendCommand("AUTHENTICATE PLAIN \"" + Base64.getEncoder().encodeToString(initialClientResponse.getBytes(StandardCharsets.UTF_8)) + "\""); ManageSieveClient.ServerResponse authenticationResponse = this.client.readResponse(); Assertions.assertThat(authenticationResponse.responseType()).isEqualTo(ManageSieveClient.ResponseType.NO); @@ -101,7 +101,7 @@ public class AuthenticateTest { @Test void plainLoginWithoutInitialResponseQuotesShouldNotSucceed() throws IOException { - String initialClientResponse = ("\0" + ManageSieveServerTestSystem.USERNAME.asString() + "\0" + ManageSieveServerTestSystem.PASSWORD); + String initialClientResponse = "\0" + ManageSieveServerTestSystem.USERNAME.asString() + "\0" + ManageSieveServerTestSystem.PASSWORD; this.client.sendCommand("AUTHENTICATE \"PLAIN\" " + Base64.getEncoder().encodeToString(initialClientResponse.getBytes(StandardCharsets.UTF_8))); ManageSieveClient.ServerResponse authenticationResponse = this.client.readResponse(); Assertions.assertThat(authenticationResponse.responseType()).isEqualTo(ManageSieveClient.ResponseType.NO); @@ -114,7 +114,7 @@ public class AuthenticateTest { Assertions.assertThat(continuationResponse.responseType()).isEqualTo(ManageSieveClient.ResponseType.OK); Assertions.assertThat(continuationResponse.responseLines()).containsExactly("\"\""); - String initialClientResponse = ("\0" + ManageSieveServerTestSystem.USERNAME.asString() + "\0" + ManageSieveServerTestSystem.PASSWORD); + String initialClientResponse = "\0" + ManageSieveServerTestSystem.USERNAME.asString() + "\0" + ManageSieveServerTestSystem.PASSWORD; this.client.sendCommand("\"" + Base64.getEncoder().encodeToString(initialClientResponse.getBytes(StandardCharsets.UTF_8)) + "\""); ManageSieveClient.ServerResponse authenticationResponse = this.client.readResponse(); Assertions.assertThat(authenticationResponse.responseType()).isEqualTo(ManageSieveClient.ResponseType.OK); @@ -130,12 +130,12 @@ public class AuthenticateTest { this.client.sendCommand("\"*\""); ManageSieveClient.ServerResponse authenticationResponse = this.client.readResponse(); Assertions.assertThat(authenticationResponse.responseType()).isEqualTo(ManageSieveClient.ResponseType.NO); - Assertions.assertThat(authenticationResponse.explanation()).get().isEqualTo("authentication aborted"); + Assertions.assertThat(authenticationResponse.explanation()).get().isEqualTo("Authentication failed with: authentication aborted by client"); } @Test void doubleAuthenticationShouldFail() throws IOException { - String initialClientResponse = ("\0" + ManageSieveServerTestSystem.USERNAME.asString() + "\0" + ManageSieveServerTestSystem.PASSWORD); + String initialClientResponse = "\0" + ManageSieveServerTestSystem.USERNAME.asString() + "\0" + ManageSieveServerTestSystem.PASSWORD; String command = "AUTHENTICATE \"PLAIN\" \"" + Base64.getEncoder().encodeToString(initialClientResponse.getBytes(StandardCharsets.UTF_8)) + "\""; this.client.sendCommand(command); @@ -212,7 +212,7 @@ public class AuthenticateTest { } void authenticatePlain() throws IOException { - String initialClientResponse = ("\0" + ManageSieveServerTestSystem.USERNAME.asString() + "\0" + ManageSieveServerTestSystem.PASSWORD); + String initialClientResponse = "\0" + ManageSieveServerTestSystem.USERNAME.asString() + "\0" + ManageSieveServerTestSystem.PASSWORD; this.client.sendCommand("AUTHENTICATE \"PLAIN\" \"" + Base64.getEncoder().encodeToString(initialClientResponse.getBytes(StandardCharsets.UTF_8)) + "\""); ManageSieveClient.ServerResponse authenticationResponse = this.client.readResponse(); Assertions.assertThat(authenticationResponse.responseType()).isEqualTo(ManageSieveClient.ResponseType.OK); diff --git a/server/protocols/protocols-managesieve/src/test/java/org/apache/james/managesieveserver/OIDCTest.java b/server/protocols/protocols-managesieve/src/test/java/org/apache/james/managesieveserver/OIDCTest.java index d331c24f71..b122d8ddf6 100644 --- a/server/protocols/protocols-managesieve/src/test/java/org/apache/james/managesieveserver/OIDCTest.java +++ b/server/protocols/protocols-managesieve/src/test/java/org/apache/james/managesieveserver/OIDCTest.java @@ -132,7 +132,7 @@ public class OIDCTest { this.client.sendCommand("\"*\""); ManageSieveClient.ServerResponse authenticationResponse = this.client.readResponse(); Assertions.assertThat(authenticationResponse.responseType()).isEqualTo(ManageSieveClient.ResponseType.NO); - Assertions.assertThat(authenticationResponse.explanation()).get().isEqualTo("authentication aborted"); + Assertions.assertThat(authenticationResponse.explanation()).get().isEqualTo("Authentication failed with: authentication aborted by client"); } @Test @@ -171,7 +171,7 @@ public class OIDCTest { this.client.sendCommand("\"*\""); ManageSieveClient.ServerResponse authenticationResponse = this.client.readResponse(); Assertions.assertThat(authenticationResponse.responseType()).isEqualTo(ManageSieveClient.ResponseType.NO); - Assertions.assertThat(authenticationResponse.explanation()).get().isEqualTo("authentication aborted"); + Assertions.assertThat(authenticationResponse.explanation()).get().isEqualTo("Authentication failed with: authentication aborted by client"); } @Test --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
