Create better AuthenticationFailedException instances

Provide a value for `messageFromServer` whenever possible. We're showing this to the user when checking server settings has failed, so they get a better idea of what went wrong.

mail/protocols/pop3/src/main/java/com/fsck/k9/mail/store/pop3/Pop3Connection.java

@@ -255,8 +255,7 @@ class Pop3Connection {
         try {
             executeSimpleCommand(PASS_COMMAND + " " + settings.getPassword(), true);
         } catch (Pop3ErrorResponse e) {
-            throw new AuthenticationFailedException(
-                    "POP3 login authentication failed: " + e.getMessage(), e);
+            throw new AuthenticationFailedException("USER/PASS failed", e, e.getResponseText());
         }
     }
 
@@ -267,9 +266,7 @@ class Pop3Connection {
                     + "\000" + settings.getPassword()).getBytes());
             executeSimpleCommand(new String(encodedBytes), true);
         } catch (Pop3ErrorResponse e) {
-            throw new AuthenticationFailedException(
-                    "POP3 SASL auth PLAIN authentication failed: "
-                            + e.getMessage(), e);
+            throw new AuthenticationFailedException("AUTH PLAIN failed", e, e.getResponseText());
         }
     }
 
@@ -293,8 +290,7 @@ class Pop3Connection {
         try {
             executeSimpleCommand("APOP " + settings.getUsername() + " " + hexDigest, true);
         } catch (Pop3ErrorResponse e) {
-            throw new AuthenticationFailedException(
-                    "POP3 APOP authentication failed: " + e.getMessage(), e);
+            throw new AuthenticationFailedException("APOP failed", e, e.getResponseText());
         }
     }
 
@@ -305,9 +301,7 @@ class Pop3Connection {
         try {
             executeSimpleCommand(b64CRAM, true);
         } catch (Pop3ErrorResponse e) {
-            throw new AuthenticationFailedException(
-                    "POP3 CRAM-MD5 authentication failed: "
-                            + e.getMessage(), e);
+            throw new AuthenticationFailedException("AUTH CRAM-MD5 failed", e, e.getResponseText());
         }
     }
 

mail/protocols/pop3/src/main/java/com/fsck/k9/mail/store/pop3/Pop3ErrorResponse.java

@@ -13,4 +13,9 @@ class Pop3ErrorResponse extends MessagingException {
     public Pop3ErrorResponse(String message) {
         super(message, true);
     }
+
+    public String getResponseText() {
+        // TODO: Extract response text from response line
+        return getMessage();
+    }
 }

mail/protocols/smtp/src/main/java/com/fsck/k9/mail/transport/smtp/SmtpTransport.kt

@@ -552,11 +552,7 @@ class SmtpTransport(
             executeSensitiveCommand(Base64.encode(username))
             executeSensitiveCommand(Base64.encode(password))
         } catch (exception: NegativeSmtpReplyException) {
-            if (exception.replyCode == SMTP_AUTHENTICATION_FAILURE_ERROR_CODE) {
-                throw AuthenticationFailedException("AUTH LOGIN failed (${exception.message})")
-            } else {
-                throw exception
-            }
+            handlePossibleAuthenticationFailure("AUTH LOGIN", exception)
         }
     }
 
@@ -565,11 +561,7 @@ class SmtpTransport(
         try {
             executeSensitiveCommand("AUTH PLAIN %s", data)
         } catch (exception: NegativeSmtpReplyException) {
-            if (exception.replyCode == SMTP_AUTHENTICATION_FAILURE_ERROR_CODE) {
-                throw AuthenticationFailedException("AUTH PLAIN failed (${exception.message})")
-            } else {
-                throw exception
-            }
+            handlePossibleAuthenticationFailure("AUTH PLAIN", exception)
         }
     }
 
@@ -584,11 +576,7 @@ class SmtpTransport(
         try {
             executeSensitiveCommand(b64CRAMString)
         } catch (exception: NegativeSmtpReplyException) {
-            if (exception.replyCode == SMTP_AUTHENTICATION_FAILURE_ERROR_CODE) {
-                throw AuthenticationFailedException(exception.message!!, exception)
-            } else {
-                throw exception
-            }
+            handlePossibleAuthenticationFailure("AUTH CRAM-MD5", exception)
         }
     }
 
@@ -604,18 +592,25 @@ class SmtpTransport(
             oauthTokenProvider!!.invalidateToken()
 
             if (!retryOAuthWithNewToken) {
-                handlePermanentFailure(negativeResponse)
+                handlePermanentOAuthFailure(method, negativeResponse)
             } else {
-                handleTemporaryFailure(method, username, negativeResponse)
+                handleTemporaryOAuthFailure(method, username, negativeResponse)
             }
         }
     }
 
-    private fun handlePermanentFailure(negativeResponse: NegativeSmtpReplyException): Nothing {
-        throw AuthenticationFailedException(negativeResponse.message!!, negativeResponse)
+    private fun handlePermanentOAuthFailure(
+        method: OAuthMethod,
+        negativeResponse: NegativeSmtpReplyException,
+    ): Nothing {
+        throw AuthenticationFailedException(
+            message = "${method.command} failed",
+            throwable = negativeResponse,
+            messageFromServer = negativeResponse.replyText,
+        )
     }
 
-    private fun handleTemporaryFailure(
+    private fun handleTemporaryOAuthFailure(
         method: OAuthMethod,
         username: String,
         negativeResponseFromOldToken: NegativeSmtpReplyException,
@@ -635,7 +630,7 @@ class SmtpTransport(
             Timber.v(negativeResponseFromNewToken, "Authentication exception for new token, permanent error assumed")
 
             oauthTokenProvider!!.invalidateToken()
-            handlePermanentFailure(negativeResponseFromNewToken)
+            handlePermanentOAuthFailure(method, negativeResponseFromNewToken)
         }
     }
 
@@ -657,6 +652,21 @@ class SmtpTransport(
         executeCommand("AUTH EXTERNAL %s", Base64.encode(username))
     }
 
+    private fun handlePossibleAuthenticationFailure(
+        authenticationMethod: String,
+        negativeResponse: NegativeSmtpReplyException,
+    ): Nothing {
+        if (negativeResponse.replyCode == SMTP_AUTHENTICATION_FAILURE_ERROR_CODE) {
+            throw AuthenticationFailedException(
+                message = "$authenticationMethod failed",
+                throwable = negativeResponse,
+                messageFromServer = negativeResponse.replyText,
+            )
+        } else {
+            throw negativeResponse
+        }
+    }
+
     @Throws(MessagingException::class)
     fun checkSettings() {
         ensureClosed()

mail/protocols/smtp/src/test/java/com/fsck/k9/mail/transport/smtp/SmtpTransportTest.kt

@@ -1,5 +1,6 @@
 package com.fsck.k9.mail.transport.smtp
 
+import assertk.Assert
 import assertk.all
 import assertk.assertFailure
 import assertk.assertions.hasMessage
@@ -231,6 +232,7 @@ class SmtpTransportTest {
             output("220 localhost Simple Mail Transfer Service Ready")
             expect("EHLO [127.0.0.1]")
             output("250-localhost Hello client.localhost")
+            output("250-ENHANCEDSTATUSCODES")
             output("250 AUTH XOAUTH2")
             expect("AUTH XOAUTH2 dXNlcj11c2VyAWF1dGg9QmVhcmVyIG9sZFRva2VuAQE=")
             output("334 " + XOAuth2ChallengeParserTest.STATUS_401_RESPONSE)
@@ -245,9 +247,9 @@ class SmtpTransportTest {
         assertFailure {
             transport.open()
         }.isInstanceOf<AuthenticationFailedException>()
-            .hasMessage(
-                "5.7.1 Username and Password not accepted. Learn more at " +
-                    "5.7.1 http://support.google.com/mail/bin/answer.py?answer=14257 hx9sm5317360pbc.68",
+            .hasServerMessage(
+                "Username and Password not accepted. Learn more at " +
+                    "http://support.google.com/mail/bin/answer.py?answer=14257 hx9sm5317360pbc.68",
             )
 
         inOrder(oAuth2TokenProvider) {
@@ -348,6 +350,7 @@ class SmtpTransportTest {
             output("220 localhost Simple Mail Transfer Service Ready")
             expect("EHLO [127.0.0.1]")
             output("250-localhost Hello client.localhost")
+            output("250-ENHANCEDSTATUSCODES")
             output("250 AUTH XOAUTH2")
             expect("AUTH XOAUTH2 dXNlcj11c2VyAWF1dGg9QmVhcmVyIG9sZFRva2VuAQE=")
             output("334 " + XOAuth2ChallengeParserTest.STATUS_400_RESPONSE)
@@ -368,9 +371,9 @@ class SmtpTransportTest {
         assertFailure {
             transport.open()
         }.isInstanceOf<AuthenticationFailedException>()
-            .hasMessage(
-                "5.7.1 Username and Password not accepted. Learn more at " +
-                    "5.7.1 http://support.google.com/mail/bin/answer.py?answer=14257 hx9sm5317360pbc.68",
+            .hasServerMessage(
+                "Username and Password not accepted. Learn more at " +
+                    "http://support.google.com/mail/bin/answer.py?answer=14257 hx9sm5317360pbc.68",
             )
 
         server.verifyConnectionClosed()
@@ -547,7 +550,7 @@ class SmtpTransportTest {
         assertFailure {
             transport.open()
         }.isInstanceOf<AuthenticationFailedException>()
-            .hasMessage(
+            .hasServerMessage(
                 "Username and Password not accepted. " +
                     "Learn more at http://support.google.com/mail/bin/answer.py?answer=14257 hx9sm5317360pbc.68",
             )
@@ -972,3 +975,7 @@ class SmtpTransportTest {
         }
     }
 }
+
+private fun Assert<AuthenticationFailedException>.hasServerMessage(expected: String) = given { actual ->
+    assertThat(actual.messageFromServer).isEqualTo(expected)
+}