From c54b95b2c60c4ac98c6420abc9b566aba1bac980 Mon Sep 17 00:00:00 2001 From: cketti Date: Tue, 2 May 2017 03:57:25 +0200 Subject: [PATCH] Don't try fallback authentication for non-authentication error responses --- .../k9/mail/store/imap/ImapConnection.java | 28 ++++++++++---- .../imap/NegativeImapResponseException.java | 10 +++-- .../store/imap/ResponseCodeExtractor.java | 19 ++++++++++ .../mail/store/imap/ImapConnectionTest.java | 24 ++++++++++++ .../store/imap/ResponseCodeExtractorTest.java | 38 +++++++++++++++++++ 5 files changed, 109 insertions(+), 10 deletions(-) create mode 100644 k9mail-library/src/main/java/com/fsck/k9/mail/store/imap/ResponseCodeExtractor.java create mode 100644 k9mail-library/src/test/java/com/fsck/k9/mail/store/imap/ResponseCodeExtractorTest.java diff --git a/k9mail-library/src/main/java/com/fsck/k9/mail/store/imap/ImapConnection.java b/k9mail-library/src/main/java/com/fsck/k9/mail/store/imap/ImapConnection.java index b95e27a59..4b549fe70 100644 --- a/k9mail-library/src/main/java/com/fsck/k9/mail/store/imap/ImapConnection.java +++ b/k9mail-library/src/main/java/com/fsck/k9/mail/store/imap/ImapConnection.java @@ -376,6 +376,7 @@ class ImapConnection { try { attemptXOAuth2(); } catch (NegativeImapResponseException e) { + //TODO: Check response code so we don't needlessly invalidate the token. oauthTokenProvider.invalidateToken(settings.getUsername()); if (!retryXoauth2WithNewToken) { @@ -456,7 +457,7 @@ class ImapConnection { try { extractCapabilities(responseParser.readStatusResponse(tag, command, getLogId(), null)); } catch (NegativeImapResponseException e) { - throw new AuthenticationFailedException(e.getMessage()); + handleAuthenticationFailure(e); } } @@ -489,11 +490,7 @@ class ImapConnection { try { extractCapabilities(responseParser.readStatusResponse(tag, command, getLogId(), null)); } catch (NegativeImapResponseException e) { - if (e.wasByeResponseReceived()) { - close(); - } - - throw new AuthenticationFailedException(e.getMessage()); + handleAuthenticationFailure(e); } } @@ -514,7 +511,7 @@ class ImapConnection { String command = String.format(Commands.LOGIN + " \"%s\" \"%s\"", username, password); extractCapabilities(executeSimpleCommand(command, true)); } catch (NegativeImapResponseException e) { - throw new AuthenticationFailedException(e.getMessage()); + handleAuthenticationFailure(e); } } @@ -534,6 +531,23 @@ class ImapConnection { } } + private void handleAuthenticationFailure(NegativeImapResponseException e) throws MessagingException { + ImapResponse lastResponse = e.getLastResponse(); + String responseCode = ResponseCodeExtractor.getResponseCode(lastResponse); + + // If there's no response code we simply assume it was an authentication failure. + if (responseCode == null || responseCode.equals(ResponseCodeExtractor.AUTHENTICATION_FAILED)) { + if (e.wasByeResponseReceived()) { + close(); + } + + throw new AuthenticationFailedException(e.getMessage()); + } else { + close(); + throw e; + } + } + private void enableCompressionIfRequested() throws IOException, MessagingException { if (hasCapability(Capabilities.COMPRESS_DEFLATE) && shouldEnableCompression()) { enableCompression(); diff --git a/k9mail-library/src/main/java/com/fsck/k9/mail/store/imap/NegativeImapResponseException.java b/k9mail-library/src/main/java/com/fsck/k9/mail/store/imap/NegativeImapResponseException.java index 783cb8ea8..6a1c44f0c 100644 --- a/k9mail-library/src/main/java/com/fsck/k9/mail/store/imap/NegativeImapResponseException.java +++ b/k9mail-library/src/main/java/com/fsck/k9/mail/store/imap/NegativeImapResponseException.java @@ -20,20 +20,24 @@ class NegativeImapResponseException extends MessagingException { public String getAlertText() { if (alertText == null) { - ImapResponse lastResponse = responses.get(responses.size() - 1); + ImapResponse lastResponse = getLastResponse(); alertText = AlertResponse.getAlertText(lastResponse); } return alertText; } - + public boolean wasByeResponseReceived() { for (ImapResponse response : responses) { if (response.getTag() == null && response.size() >= 1 && equalsIgnoreCase(response.get(0), Responses.BYE)) { return true; } } - + return false; } + + public ImapResponse getLastResponse() { + return responses.get(responses.size() - 1); + } } diff --git a/k9mail-library/src/main/java/com/fsck/k9/mail/store/imap/ResponseCodeExtractor.java b/k9mail-library/src/main/java/com/fsck/k9/mail/store/imap/ResponseCodeExtractor.java new file mode 100644 index 000000000..2f622c4d4 --- /dev/null +++ b/k9mail-library/src/main/java/com/fsck/k9/mail/store/imap/ResponseCodeExtractor.java @@ -0,0 +1,19 @@ +package com.fsck.k9.mail.store.imap; + + +class ResponseCodeExtractor { + public static final String AUTHENTICATION_FAILED = "AUTHENTICATIONFAILED"; + + + private ResponseCodeExtractor() { + } + + public static String getResponseCode(ImapResponse response) { + if (response.size() < 2 || !response.isList(1)) { + return null; + } + + ImapList responseTextCode = response.getList(1); + return responseTextCode.size() != 1 ? null : responseTextCode.getString(0); + } +} diff --git a/k9mail-library/src/test/java/com/fsck/k9/mail/store/imap/ImapConnectionTest.java b/k9mail-library/src/test/java/com/fsck/k9/mail/store/imap/ImapConnectionTest.java index 60bf3e77b..f80ecdda5 100644 --- a/k9mail-library/src/test/java/com/fsck/k9/mail/store/imap/ImapConnectionTest.java +++ b/k9mail-library/src/test/java/com/fsck/k9/mail/store/imap/ImapConnectionTest.java @@ -197,6 +197,30 @@ public class ImapConnectionTest { server.verifyInteractionCompleted(); } + @Test + public void open_authPlainFailureAndDisconnect_shouldThrow() throws Exception { + settings.setAuthType(AuthType.PLAIN); + MockImapServer server = new MockImapServer(); + preAuthenticationDialog(server, "AUTH=PLAIN"); + server.expect("2 AUTHENTICATE PLAIN"); + server.output("+"); + server.expect(ByteString.encodeUtf8("\000" + USERNAME + "\000" + PASSWORD).base64()); + server.output("2 NO [UNAVAILABLE] Maximum number of connections from user+IP exceeded"); + server.closeConnection(); + ImapConnection imapConnection = startServerAndCreateImapConnection(server); + + try { + imapConnection.open(); + fail("Expected exception"); + } catch (NegativeImapResponseException e) { + assertThat(e.getMessage(), containsString("Maximum number of connections from user+IP exceeded")); + } + + assertFalse(imapConnection.isConnected()); + server.verifyConnectionClosed(); + server.verifyInteractionCompleted(); + } + @Test public void open_authPlainWithByeResponseAndConnectionClose_shouldThrowAuthenticationFailedException() throws Exception { diff --git a/k9mail-library/src/test/java/com/fsck/k9/mail/store/imap/ResponseCodeExtractorTest.java b/k9mail-library/src/test/java/com/fsck/k9/mail/store/imap/ResponseCodeExtractorTest.java new file mode 100644 index 000000000..df5b92dc3 --- /dev/null +++ b/k9mail-library/src/test/java/com/fsck/k9/mail/store/imap/ResponseCodeExtractorTest.java @@ -0,0 +1,38 @@ +package com.fsck.k9.mail.store.imap; + + +import org.junit.Test; + +import static com.fsck.k9.mail.store.imap.ImapResponseHelper.createImapResponse; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNull; + + +public class ResponseCodeExtractorTest { + @Test + public void getResponseCode_withResponseCode() throws Exception { + ImapResponse imapResponse = createImapResponse("x NO [AUTHENTICATIONFAILED] No sir"); + + String result = ResponseCodeExtractor.getResponseCode(imapResponse); + + assertEquals("AUTHENTICATIONFAILED", result); + } + + @Test + public void getResponseCode_withoutResponseCode() throws Exception { + ImapResponse imapResponse = createImapResponse("x NO Authentication failed"); + + String result = ResponseCodeExtractor.getResponseCode(imapResponse); + + assertNull(result); + } + + @Test + public void getResponseCode_withoutSingleItemResponse() throws Exception { + ImapResponse imapResponse = createImapResponse("x NO"); + + String result = ResponseCodeExtractor.getResponseCode(imapResponse); + + assertNull(result); + } +}