From f79db8e28ded4094cfde4f35038b9161f5aee663 Mon Sep 17 00:00:00 2001 From: cketti Date: Mon, 27 Nov 2023 18:02:01 +0100 Subject: [PATCH] Add `MissingCapabilityException` --- .../k9/mail/MissingCapabilityException.kt | 5 ++ .../k9/mail/store/imap/RealImapConnection.kt | 23 +++----- .../mail/store/imap/RealImapConnectionTest.kt | 52 +++++++++++++++---- .../k9/mail/store/pop3/Pop3Connection.java | 20 ++++--- .../k9/mail/store/pop3/Pop3ConnectionTest.kt | 15 +++--- .../k9/mail/transport/smtp/SmtpTransport.kt | 22 +++----- .../mail/transport/smtp/SmtpTransportTest.kt | 46 ++++++++++++---- 7 files changed, 118 insertions(+), 65 deletions(-) create mode 100644 mail/common/src/main/java/com/fsck/k9/mail/MissingCapabilityException.kt diff --git a/mail/common/src/main/java/com/fsck/k9/mail/MissingCapabilityException.kt b/mail/common/src/main/java/com/fsck/k9/mail/MissingCapabilityException.kt new file mode 100644 index 00000000000..cc7f1952038 --- /dev/null +++ b/mail/common/src/main/java/com/fsck/k9/mail/MissingCapabilityException.kt @@ -0,0 +1,5 @@ +package com.fsck.k9.mail + +class MissingCapabilityException( + val capabilityName: String, +) : MessagingException("Missing capability: $capabilityName", true) diff --git a/mail/protocols/imap/src/main/java/com/fsck/k9/mail/store/imap/RealImapConnection.kt b/mail/protocols/imap/src/main/java/com/fsck/k9/mail/store/imap/RealImapConnection.kt index daba1f44762..2e5099bd34b 100644 --- a/mail/protocols/imap/src/main/java/com/fsck/k9/mail/store/imap/RealImapConnection.kt +++ b/mail/protocols/imap/src/main/java/com/fsck/k9/mail/store/imap/RealImapConnection.kt @@ -8,6 +8,7 @@ import com.fsck.k9.mail.CertificateValidationException import com.fsck.k9.mail.ConnectionSecurity import com.fsck.k9.mail.K9MailLib import com.fsck.k9.mail.MessagingException +import com.fsck.k9.mail.MissingCapabilityException import com.fsck.k9.mail.NetworkTimeouts.SOCKET_CONNECT_TIMEOUT import com.fsck.k9.mail.NetworkTimeouts.SOCKET_READ_TIMEOUT import com.fsck.k9.mail.filter.Base64 @@ -260,14 +261,7 @@ internal class RealImapConnection( private fun upgradeToTls() { if (!hasCapability(Capabilities.STARTTLS)) { - /* - * This exception triggers a "Certificate error" - * notification that takes the user to the incoming - * server settings for review. This might be needed if - * the account was configured with an obsolete - * "STARTTLS (if available)" setting. - */ - throw CertificateValidationException("STARTTLS connection security not available") + throw MissingCapabilityException(Capabilities.STARTTLS) } startTls() @@ -298,20 +292,20 @@ internal class RealImapConnection( if (oauthTokenProvider == null) { throw MessagingException("No OAuthToken Provider available.") } else if (!hasCapability(Capabilities.SASL_IR)) { - throw MessagingException("SASL-IR capability is missing.") + throw MissingCapabilityException(Capabilities.SASL_IR) } else if (hasCapability(Capabilities.AUTH_OAUTHBEARER)) { authWithOAuthToken(OAuthMethod.OAUTHBEARER) } else if (hasCapability(Capabilities.AUTH_XOAUTH2)) { authWithOAuthToken(OAuthMethod.XOAUTH2) } else { - throw MessagingException("Server doesn't support SASL OAUTHBEARER or XOAUTH2.") + throw MissingCapabilityException(Capabilities.AUTH_OAUTHBEARER) } } AuthType.CRAM_MD5 -> { if (hasCapability(Capabilities.AUTH_CRAM_MD5)) { authCramMD5() } else { - throw MessagingException("Server doesn't support encrypted passwords using CRAM-MD5.") + throw MissingCapabilityException(Capabilities.AUTH_CRAM_MD5) } } AuthType.PLAIN -> { @@ -320,17 +314,14 @@ internal class RealImapConnection( } else if (!hasCapability(Capabilities.LOGINDISABLED)) { login() } else { - throw MessagingException( - "Server doesn't support unencrypted passwords using AUTH=PLAIN and LOGIN is disabled.", - ) + throw MissingCapabilityException(Capabilities.AUTH_PLAIN) } } AuthType.EXTERNAL -> { if (hasCapability(Capabilities.AUTH_EXTERNAL)) { saslAuthExternal() } else { - // Provide notification to user of a problem authenticating using client certificates - throw CertificateValidationException(CertificateValidationException.Reason.MissingCapability) + throw MissingCapabilityException(Capabilities.AUTH_EXTERNAL) } } else -> { diff --git a/mail/protocols/imap/src/test/java/com/fsck/k9/mail/store/imap/RealImapConnectionTest.kt b/mail/protocols/imap/src/test/java/com/fsck/k9/mail/store/imap/RealImapConnectionTest.kt index 7ce7782c1f4..d3814537ee0 100644 --- a/mail/protocols/imap/src/test/java/com/fsck/k9/mail/store/imap/RealImapConnectionTest.kt +++ b/mail/protocols/imap/src/test/java/com/fsck/k9/mail/store/imap/RealImapConnectionTest.kt @@ -19,7 +19,7 @@ import com.fsck.k9.mail.AuthenticationFailedException import com.fsck.k9.mail.CertificateValidationException import com.fsck.k9.mail.ConnectionSecurity import com.fsck.k9.mail.K9MailLib -import com.fsck.k9.mail.MessagingException +import com.fsck.k9.mail.MissingCapabilityException import com.fsck.k9.mail.SystemOutLogger import com.fsck.k9.mail.XOAuth2ChallengeParserTest import com.fsck.k9.mail.helpers.TestTrustedSocketFactory @@ -143,8 +143,8 @@ class RealImapConnectionTest { assertFailure { imapConnection.open() - }.isInstanceOf() - .hasMessage("Server doesn't support unencrypted passwords using AUTH=PLAIN and LOGIN is disabled.") + }.isInstanceOf() + .prop(MissingCapabilityException::capabilityName).isEqualTo("AUTH=PLAIN") server.verifyConnectionClosed() server.verifyInteractionCompleted() @@ -302,8 +302,8 @@ class RealImapConnectionTest { assertFailure { imapConnection.open() - }.isInstanceOf() - .hasMessage("Server doesn't support encrypted passwords using CRAM-MD5.") + }.isInstanceOf() + .prop(MissingCapabilityException::capabilityName).isEqualTo("AUTH=CRAM-MD5") server.verifyConnectionClosed() server.verifyInteractionCompleted() @@ -375,6 +375,38 @@ class RealImapConnectionTest { server.verifyInteractionCompleted() } + @Test + fun `open() AUTH OAUTHBEARER with SASL-IR capability missing`() { + val server = MockImapServer().apply { + preAuthenticationDialog(capabilities = "AUTH=OAUTHBEARER") + } + val imapConnection = startServerAndCreateImapConnection(server, authType = AuthType.XOAUTH2) + + assertFailure { + imapConnection.open() + }.isInstanceOf() + .prop(MissingCapabilityException::capabilityName).isEqualTo("SASL-IR") + + server.verifyConnectionClosed() + server.verifyInteractionCompleted() + } + + @Test + fun `open() AUTH OAUTHBEARER with AUTH=OAUTHBEARER capability missing`() { + val server = MockImapServer().apply { + preAuthenticationDialog(capabilities = "SASL-IR") + } + val imapConnection = startServerAndCreateImapConnection(server, authType = AuthType.XOAUTH2) + + assertFailure { + imapConnection.open() + }.isInstanceOf() + .prop(MissingCapabilityException::capabilityName).isEqualTo("AUTH=OAUTHBEARER") + + server.verifyConnectionClosed() + server.verifyInteractionCompleted() + } + @Test fun `open() AUTH XOAUTH2 throws exception on 401 response`() { val server = MockImapServer().apply { @@ -539,9 +571,8 @@ class RealImapConnectionTest { assertFailure { imapConnection.open() - }.isInstanceOf() - .prop(CertificateValidationException::getReason) - .isEqualTo(CertificateValidationException.Reason.MissingCapability) + }.isInstanceOf() + .prop(MissingCapabilityException::capabilityName).isEqualTo("AUTH=EXTERNAL") server.verifyConnectionClosed() server.verifyInteractionCompleted() @@ -686,11 +717,10 @@ class RealImapConnectionTest { connectionSecurity = ConnectionSecurity.STARTTLS_REQUIRED, ) - // FIXME: CertificateValidationException seems wrong assertFailure { imapConnection.open() - }.isInstanceOf() - .hasMessage("STARTTLS connection security not available") + }.isInstanceOf() + .prop(MissingCapabilityException::capabilityName).isEqualTo("STARTTLS") server.verifyConnectionClosed() server.verifyInteractionCompleted() diff --git a/mail/protocols/pop3/src/main/java/com/fsck/k9/mail/store/pop3/Pop3Connection.java b/mail/protocols/pop3/src/main/java/com/fsck/k9/mail/store/pop3/Pop3Connection.java index bbcca763081..895649aecc6 100644 --- a/mail/protocols/pop3/src/main/java/com/fsck/k9/mail/store/pop3/Pop3Connection.java +++ b/mail/protocols/pop3/src/main/java/com/fsck/k9/mail/store/pop3/Pop3Connection.java @@ -26,16 +26,26 @@ import com.fsck.k9.mail.ConnectionSecurity; import com.fsck.k9.mail.K9MailLib; import com.fsck.k9.mail.MessagingException; +import com.fsck.k9.mail.MissingCapabilityException; import com.fsck.k9.mail.filter.Base64; import com.fsck.k9.mail.filter.Hex; import com.fsck.k9.mail.ssl.TrustedSocketFactory; import javax.net.ssl.SSLException; -import static com.fsck.k9.mail.CertificateValidationException.Reason.MissingCapability; import static com.fsck.k9.mail.K9MailLib.DEBUG_PROTOCOL_POP3; import static com.fsck.k9.mail.NetworkTimeouts.SOCKET_CONNECT_TIMEOUT; import static com.fsck.k9.mail.NetworkTimeouts.SOCKET_READ_TIMEOUT; -import static com.fsck.k9.mail.store.pop3.Pop3Commands.*; +import static com.fsck.k9.mail.store.pop3.Pop3Commands.AUTH_CRAM_MD5_CAPABILITY; +import static com.fsck.k9.mail.store.pop3.Pop3Commands.AUTH_EXTERNAL_CAPABILITY; +import static com.fsck.k9.mail.store.pop3.Pop3Commands.AUTH_PLAIN_CAPABILITY; +import static com.fsck.k9.mail.store.pop3.Pop3Commands.CAPA_COMMAND; +import static com.fsck.k9.mail.store.pop3.Pop3Commands.PASS_COMMAND; +import static com.fsck.k9.mail.store.pop3.Pop3Commands.SASL_CAPABILITY; +import static com.fsck.k9.mail.store.pop3.Pop3Commands.STLS_CAPABILITY; +import static com.fsck.k9.mail.store.pop3.Pop3Commands.STLS_COMMAND; +import static com.fsck.k9.mail.store.pop3.Pop3Commands.TOP_CAPABILITY; +import static com.fsck.k9.mail.store.pop3.Pop3Commands.UIDL_CAPABILITY; +import static com.fsck.k9.mail.store.pop3.Pop3Commands.USER_COMMAND; class Pop3Connection { @@ -159,8 +169,7 @@ private void performStartTlsUpgrade(TrustedSocketFactory trustedSocketFactory, } capabilities = getCapabilities(); } else { - throw new CertificateValidationException( - "STARTTLS connection security not available"); + throw new MissingCapabilityException(STLS_CAPABILITY); } } @@ -188,8 +197,7 @@ private void performAuthentication(AuthType authType, String serverGreeting) if (capabilities.external) { authExternal(); } else { - // Provide notification to user of a problem authenticating using client certificates - throw new CertificateValidationException(MissingCapability); + throw new MissingCapabilityException(SASL_CAPABILITY + " " + AUTH_EXTERNAL_CAPABILITY); } break; diff --git a/mail/protocols/pop3/src/test/java/com/fsck/k9/mail/store/pop3/Pop3ConnectionTest.kt b/mail/protocols/pop3/src/test/java/com/fsck/k9/mail/store/pop3/Pop3ConnectionTest.kt index 3a1b150197e..608484ca2a4 100644 --- a/mail/protocols/pop3/src/test/java/com/fsck/k9/mail/store/pop3/Pop3ConnectionTest.kt +++ b/mail/protocols/pop3/src/test/java/com/fsck/k9/mail/store/pop3/Pop3ConnectionTest.kt @@ -17,6 +17,7 @@ import com.fsck.k9.mail.ConnectionSecurity.NONE import com.fsck.k9.mail.ConnectionSecurity.SSL_TLS_REQUIRED import com.fsck.k9.mail.ConnectionSecurity.STARTTLS_REQUIRED import com.fsck.k9.mail.MessagingException +import com.fsck.k9.mail.MissingCapabilityException import com.fsck.k9.mail.helpers.TestTrustedSocketFactory import com.fsck.k9.mail.ssl.TrustedSocketFactory import java.io.IOException @@ -78,14 +79,17 @@ class Pop3ConnectionTest { createAndOpenPop3Connection(settings, mockSocketFactory) } - @Test(expected = CertificateValidationException::class) - fun `open() with STLS capability unavailable should throw CertificateValidationException`() { + @Test + fun `open() with STLS capability unavailable should throw`() { val server = startServer { setupServerWithAuthenticationMethods("PLAIN") } val settings = server.createSettings(connectionSecurity = STARTTLS_REQUIRED) - createAndOpenPop3Connection(settings) + assertFailure { + createAndOpenPop3Connection(settings) + }.isInstanceOf() + .prop(MissingCapabilityException::capabilityName).isEqualTo("STLS") } @Test(expected = Pop3ErrorResponse::class) @@ -300,9 +304,8 @@ class Pop3ConnectionTest { assertFailure { createAndOpenPop3Connection(settings) - }.isInstanceOf() - .prop(CertificateValidationException::getReason) - .isEqualTo(CertificateValidationException.Reason.MissingCapability) + }.isInstanceOf() + .prop(MissingCapabilityException::capabilityName).isEqualTo("SASL EXTERNAL") server.verifyConnectionStillOpen() server.verifyInteractionCompleted() diff --git a/mail/protocols/smtp/src/main/java/com/fsck/k9/mail/transport/smtp/SmtpTransport.kt b/mail/protocols/smtp/src/main/java/com/fsck/k9/mail/transport/smtp/SmtpTransport.kt index 4d905f8c5c1..a9499ffe55c 100644 --- a/mail/protocols/smtp/src/main/java/com/fsck/k9/mail/transport/smtp/SmtpTransport.kt +++ b/mail/protocols/smtp/src/main/java/com/fsck/k9/mail/transport/smtp/SmtpTransport.kt @@ -11,6 +11,7 @@ import com.fsck.k9.mail.K9MailLib import com.fsck.k9.mail.Message import com.fsck.k9.mail.Message.RecipientType import com.fsck.k9.mail.MessagingException +import com.fsck.k9.mail.MissingCapabilityException import com.fsck.k9.mail.NetworkTimeouts.SOCKET_CONNECT_TIMEOUT import com.fsck.k9.mail.NetworkTimeouts.SOCKET_READ_TIMEOUT import com.fsck.k9.mail.ServerSettings @@ -125,10 +126,7 @@ class SmtpTransport( extensions = sendHello(helloName) secureConnection = true } else { - // This exception triggers a "Certificate error" notification that takes the user to the incoming - // server settings for review. This might be needed if the account was configured with an obsolete - // "STARTTLS (if available)" setting. - throw CertificateValidationException("STARTTLS connection security not available") + throw MissingCapabilityException("STARTTLS") } } @@ -161,14 +159,14 @@ class SmtpTransport( } else if (authLoginSupported) { saslAuthLogin() } else { - throw MessagingException("Authentication methods SASL PLAIN and LOGIN are unavailable.") + throw MissingCapabilityException("AUTH PLAIN") } } AuthType.CRAM_MD5 -> { if (authCramMD5Supported) { saslAuthCramMD5() } else { - throw MessagingException("Authentication method CRAM-MD5 is unavailable.") + throw MissingCapabilityException("AUTH CRAM-MD5") } } AuthType.XOAUTH2 -> { @@ -179,20 +177,14 @@ class SmtpTransport( } else if (authXoauth2Supported) { saslOAuth(OAuthMethod.XOAUTH2) } else { - throw MessagingException("Server doesn't support SASL OAUTHBEARER or XOAUTH2.") + throw MissingCapabilityException("AUTH OAUTHBEARER") } } AuthType.EXTERNAL -> { if (authExternalSupported) { saslAuthExternal() } else { - // Some SMTP servers are known to provide no error indication when a client certificate - // fails to validate, other than to not offer the AUTH EXTERNAL capability. - // So, we treat it is an error to not offer AUTH EXTERNAL when using client certificates. - // That way, the user can be notified of a problem during account setup. - throw CertificateValidationException( - CertificateValidationException.Reason.MissingCapability, - ) + throw MissingCapabilityException("AUTH EXTERNAL") } } AuthType.AUTOMATIC -> { @@ -205,7 +197,7 @@ class SmtpTransport( } else if (authCramMD5Supported) { saslAuthCramMD5() } else { - throw MessagingException("No supported authentication methods available.") + throw MissingCapabilityException("AUTH PLAIN") } } else { if (authCramMD5Supported) { diff --git a/mail/protocols/smtp/src/test/java/com/fsck/k9/mail/transport/smtp/SmtpTransportTest.kt b/mail/protocols/smtp/src/test/java/com/fsck/k9/mail/transport/smtp/SmtpTransportTest.kt index 927262d91af..1f25d08ecb3 100644 --- a/mail/protocols/smtp/src/test/java/com/fsck/k9/mail/transport/smtp/SmtpTransportTest.kt +++ b/mail/protocols/smtp/src/test/java/com/fsck/k9/mail/transport/smtp/SmtpTransportTest.kt @@ -10,10 +10,10 @@ import assertk.assertions.isTrue import assertk.assertions.prop import com.fsck.k9.mail.AuthType import com.fsck.k9.mail.AuthenticationFailedException -import com.fsck.k9.mail.CertificateValidationException import com.fsck.k9.mail.ConnectionSecurity import com.fsck.k9.mail.Message import com.fsck.k9.mail.MessagingException +import com.fsck.k9.mail.MissingCapabilityException import com.fsck.k9.mail.ServerSettings import com.fsck.k9.mail.XOAuth2ChallengeParserTest import com.fsck.k9.mail.filter.Base64 @@ -124,8 +124,8 @@ class SmtpTransportTest { assertFailure { transport.open() - }.isInstanceOf() - .hasMessage("Authentication methods SASL PLAIN and LOGIN are unavailable.") + }.isInstanceOf() + .prop(MissingCapabilityException::capabilityName).isEqualTo("AUTH PLAIN") server.verifyConnectionClosed() server.verifyInteractionCompleted() @@ -165,8 +165,8 @@ class SmtpTransportTest { assertFailure { transport.open() - }.isInstanceOf() - .hasMessage("Authentication method CRAM-MD5 is unavailable.") + }.isInstanceOf() + .prop(MissingCapabilityException::capabilityName).isEqualTo("AUTH CRAM-MD5") server.verifyConnectionClosed() server.verifyInteractionCompleted() @@ -405,7 +405,7 @@ class SmtpTransportTest { } @Test - fun `open() without XOAUTH2 extension should throw`() { + fun `open() without OAUTHBEARER extension should throw`() { val server = MockSmtpServer().apply { output("220 localhost Simple Mail Transfer Service Ready") expect("EHLO [127.0.0.1]") @@ -418,8 +418,8 @@ class SmtpTransportTest { assertFailure { transport.open() - }.isInstanceOf() - .hasMessage("Server doesn't support SASL OAUTHBEARER or XOAUTH2.") + }.isInstanceOf() + .prop(MissingCapabilityException::capabilityName).isEqualTo("AUTH OAUTHBEARER") server.verifyConnectionClosed() server.verifyInteractionCompleted() @@ -457,9 +457,8 @@ class SmtpTransportTest { assertFailure { transport.open() - }.isInstanceOf() - .prop(CertificateValidationException::getReason) - .isEqualTo(CertificateValidationException.Reason.MissingCapability) + }.isInstanceOf() + .prop(MissingCapabilityException::capabilityName).isEqualTo("AUTH EXTERNAL") server.verifyConnectionClosed() server.verifyInteractionCompleted() @@ -616,6 +615,31 @@ class SmtpTransportTest { server.verifyInteractionCompleted() } + @Test + fun `open() with STARTTLS but without STARTTLS capability should throw`() { + val server = MockSmtpServer().apply { + output("220 localhost Simple Mail Transfer Service Ready") + expect("EHLO [127.0.0.1]") + output("250-localhost Hello 127.0.0.1") + output("250 HELP") + expect("QUIT") + closeConnection() + } + val transport = startServerAndCreateSmtpTransport( + server, + authenticationType = AuthType.PLAIN, + connectionSecurity = ConnectionSecurity.STARTTLS_REQUIRED, + ) + + assertFailure { + transport.open() + }.isInstanceOf() + .prop(MissingCapabilityException::capabilityName).isEqualTo("STARTTLS") + + server.verifyConnectionClosed() + server.verifyInteractionCompleted() + } + @Test fun `sendMessage() without address to send to should not open connection`() { val message = MimeMessage()