Merge pull request #6911 from thundernest/new_autodiscovery_api

Change AutoDiscovery API

core/common/src/main/kotlin/app/k9mail/core/common/net/Hostname.kt

@@ -0,0 +1,13 @@
+package app.k9mail.core.common.net
+
+/**
+ * Represents a hostname, IPv4, or IPv6 address.
+ */
+@JvmInline
+value class Hostname(val value: String) {
+    init {
+        requireNotNull(HostNameUtils.isLegalHostNameOrIP(value)) { "Not a valid domain or IP: '$value'" }
+    }
+}
+
+fun String.toHostname() = Hostname(this)

core/common/src/main/kotlin/app/k9mail/core/common/net/Port.kt

@@ -0,0 +1,11 @@
+package app.k9mail.core.common.net
+
+@Suppress("MagicNumber")
+@JvmInline
+value class Port(val value: Int) {
+    init {
+        require(value in 0..65535) { "Not a valid port number: $value" }
+    }
+}
+
+fun Int.toPort() = Port(this)

core/common/src/test/kotlin/app/k9mail/core/common/net/HostnameTest.kt

@@ -0,0 +1,47 @@
+package app.k9mail.core.common.net
+
+import assertk.assertFailure
+import assertk.assertThat
+import assertk.assertions.hasMessage
+import assertk.assertions.isEqualTo
+import assertk.assertions.isInstanceOf
+import kotlin.test.Test
+
+class HostnameTest {
+    @Test
+    fun `valid domain`() {
+        val hostname = Hostname("domain.example")
+
+        assertThat(hostname.value).isEqualTo("domain.example")
+    }
+
+    @Test
+    fun `valid IPv4`() {
+        val hostname = Hostname("127.0.0.1")
+
+        assertThat(hostname.value).isEqualTo("127.0.0.1")
+    }
+
+    @Test
+    fun `valid IPv6`() {
+        val hostname = Hostname("fc00::1")
+
+        assertThat(hostname.value).isEqualTo("fc00::1")
+    }
+
+    @Test
+    fun `invalid domain should throw`() {
+        assertFailure {
+            Hostname("invalid domain")
+        }.isInstanceOf<IllegalArgumentException>()
+            .hasMessage("Not a valid domain or IP: 'invalid domain'")
+    }
+
+    @Test
+    fun `invalid IPv6 should throw`() {
+        assertFailure {
+            Hostname("fc00:1")
+        }.isInstanceOf<IllegalArgumentException>()
+            .hasMessage("Not a valid domain or IP: 'fc00:1'")
+    }
+}

core/common/src/test/kotlin/app/k9mail/core/common/net/PortTest.kt

@@ -0,0 +1,33 @@
+package app.k9mail.core.common.net
+
+import assertk.assertFailure
+import assertk.assertThat
+import assertk.assertions.hasMessage
+import assertk.assertions.isEqualTo
+import assertk.assertions.isInstanceOf
+import kotlin.test.Test
+
+class PortTest {
+    @Test
+    fun `valid port number`() {
+        val port = Port(993)
+
+        assertThat(port.value).isEqualTo(993)
+    }
+
+    @Test
+    fun `negative port number should throw`() {
+        assertFailure {
+            Port(-1)
+        }.isInstanceOf<IllegalArgumentException>()
+            .hasMessage("Not a valid port number: -1")
+    }
+
+    @Test
+    fun `port number exceeding valid range should throw`() {
+        assertFailure {
+            Port(65536)
+        }.isInstanceOf<IllegalArgumentException>()
+            .hasMessage("Not a valid port number: 65536")
+    }
+}

feature/autodiscovery/api/build.gradle.kts

@@ -5,4 +5,5 @@ plugins {
 
 dependencies {
     api(projects.mail.common)
+    api(projects.core.common)
 }

feature/autodiscovery/api/src/main/kotlin/app/k9mail/autodiscovery/api/AuthenticationType.kt

@@ -0,0 +1,13 @@
+package app.k9mail.autodiscovery.api
+
+/**
+ * The authentication types supported when using the [AutoDiscovery] mechanism.
+ *
+ * Note: Currently we support the same set of values in [ImapServerSettings] and [SmtpServerSettings]. As soon as this
+ * changes, this type should be replaced with `ImapAuthenticationType` and `SmtpAuthenticationType`.
+ */
+enum class AuthenticationType {
+    PasswordCleartext,
+    PasswordEncrypted,
+    OAuth2,
+}

feature/autodiscovery/api/src/main/kotlin/app/k9mail/autodiscovery/api/AutoDiscovery.kt

@@ -0,0 +1,13 @@
+package app.k9mail.autodiscovery.api
+
+import app.k9mail.core.common.mail.EmailAddress
+
+/**
+ * Provides a mechanism to find mail server settings for a given email address.
+ */
+interface AutoDiscovery {
+    /**
+     * Returns a list of [AutoDiscoveryRunnable]s that perform the actual mail server settings discovery.
+     */
+    fun initDiscovery(email: EmailAddress): List<AutoDiscoveryRunnable>
+}

feature/autodiscovery/api/src/main/kotlin/app/k9mail/autodiscovery/api/AutoDiscoveryResult.kt

@@ -0,0 +1,23 @@
+package app.k9mail.autodiscovery.api
+
+/**
+ * Results of a mail server settings lookup.
+ */
+data class AutoDiscoveryResult(
+    val incomingServerSettings: IncomingServerSettings,
+    val outgoingServerSettings: OutgoingServerSettings,
+)
+
+/**
+ * Incoming mail server settings.
+ *
+ * Implementations contain protocol-specific properties.
+ */
+interface IncomingServerSettings
+
+/**
+ * Outgoing mail server settings.
+ *
+ * Implementations contain protocol-specific properties.
+ */
+interface OutgoingServerSettings

feature/autodiscovery/api/src/main/kotlin/app/k9mail/autodiscovery/api/AutoDiscoveryRunnable.kt

@@ -0,0 +1,10 @@
+package app.k9mail.autodiscovery.api
+
+/**
+ * Performs a mail server settings lookup.
+ *
+ * This is an abstraction that allows us to run multiple lookups in parallel.
+ */
+fun interface AutoDiscoveryRunnable {
+    suspend fun run(): AutoDiscoveryResult?
+}

feature/autodiscovery/api/src/main/kotlin/app/k9mail/autodiscovery/api/ConnectionSecurity.kt

@@ -0,0 +1,9 @@
+package app.k9mail.autodiscovery.api
+
+/**
+ * The connection security methods supported when using the [AutoDiscovery] mechanism.
+ */
+enum class ConnectionSecurity {
+    StartTLS,
+    TLS,
+}

feature/autodiscovery/api/src/main/kotlin/app/k9mail/autodiscovery/api/ConnectionSettingsDiscovery.kt

@@ -3,6 +3,7 @@ package app.k9mail.autodiscovery.api
 import com.fsck.k9.mail.AuthType
 import com.fsck.k9.mail.ConnectionSecurity
 
+@Deprecated("New code should use app.k9mail.autodiscovery.api.AutoDiscovery")
 interface ConnectionSettingsDiscovery {
     fun discover(email: String): DiscoveryResults?
 }

feature/autodiscovery/api/src/main/kotlin/app/k9mail/autodiscovery/api/ImapServerSettings.kt

@@ -0,0 +1,12 @@
+package app.k9mail.autodiscovery.api
+
+import app.k9mail.core.common.net.Hostname
+import app.k9mail.core.common.net.Port
+
+data class ImapServerSettings(
+    val hostname: Hostname,
+    val port: Port,
+    val connectionSecurity: ConnectionSecurity,
+    val authenticationType: AuthenticationType,
+    val username: String,
+) : IncomingServerSettings

feature/autodiscovery/api/src/main/kotlin/app/k9mail/autodiscovery/api/SmtpServerSettings.kt

@@ -0,0 +1,12 @@
+package app.k9mail.autodiscovery.api
+
+import app.k9mail.core.common.net.Hostname
+import app.k9mail.core.common.net.Port
+
+data class SmtpServerSettings(
+    val hostname: Hostname,
+    val port: Port,
+    val connectionSecurity: ConnectionSecurity,
+    val authenticationType: AuthenticationType,
+    val username: String,
+) : OutgoingServerSettings

feature/autodiscovery/autoconfig/build.gradle.kts

@@ -6,10 +6,10 @@ plugins {
 dependencies {
     api(projects.feature.autodiscovery.api)
 
-    compileOnly(libs.xmlpull)
-    implementation(projects.core.common)
+    implementation(libs.kotlinx.coroutines.core)
     implementation(libs.okhttp)
     implementation(libs.minidns.hla)
+    compileOnly(libs.xmlpull)
 
     testImplementation(libs.kxml2)
     testImplementation(libs.jsoup)

feature/autodiscovery/autoconfig/src/main/kotlin/app/k9mail/autodiscovery/autoconfig/AutoconfigDiscovery.kt

@@ -1,34 +1,54 @@
 package app.k9mail.autodiscovery.autoconfig
 
-import app.k9mail.autodiscovery.api.DiscoveryResults
+import app.k9mail.autodiscovery.api.AutoDiscovery
+import app.k9mail.autodiscovery.api.AutoDiscoveryResult
+import app.k9mail.autodiscovery.api.AutoDiscoveryRunnable
 import app.k9mail.core.common.mail.EmailAddress
 import app.k9mail.core.common.net.toDomain
 import com.fsck.k9.helper.EmailHelper
+import com.fsck.k9.logging.Timber
+import java.io.IOException
+import kotlinx.coroutines.Dispatchers
+import kotlinx.coroutines.withContext
+import okhttp3.HttpUrl
 
 class AutoconfigDiscovery(
     private val urlProvider: AutoconfigUrlProvider,
     private val fetcher: AutoconfigFetcher,
     private val parser: AutoconfigParser,
-) {
+) : AutoDiscovery {
 
-    fun discover(email: EmailAddress): DiscoveryResults? {
+    override fun initDiscovery(email: EmailAddress): List<AutoDiscoveryRunnable> {
         val domain = requireNotNull(EmailHelper.getDomainFromEmailAddress(email.address)?.toDomain()) {
             "Couldn't extract domain from email address: $email"
         }
 
         val autoconfigUrls = urlProvider.getAutoconfigUrls(domain, email)
 
-        return autoconfigUrls
-            .asSequence()
-            .mapNotNull { autoconfigUrl ->
-                fetcher.fetchAutoconfigFile(autoconfigUrl)?.use { inputStream ->
-                    parser.parseSettings(inputStream, email)
-                }
-            }
-            .firstOrNull { result ->
-                result.incoming.isNotEmpty() || result.outgoing.isNotEmpty()
+        return autoconfigUrls.map { autoconfigUrl ->
+            AutoDiscoveryRunnable {
+                getConfigInBackground(email, autoconfigUrl)
             }
+        }
     }
 
-    override fun toString(): String = "Thunderbird autoconfig"
+    private suspend fun getConfigInBackground(email: EmailAddress, autoconfigUrl: HttpUrl): AutoDiscoveryResult? {
+        return withContext(Dispatchers.IO) {
+            getAutoconfig(email, autoconfigUrl)
+        }
+    }
+
+    private fun getAutoconfig(email: EmailAddress, autoconfigUrl: HttpUrl): AutoDiscoveryResult? {
+        return try {
+            fetcher.fetchAutoconfigFile(autoconfigUrl)?.use { inputStream ->
+                parser.parseSettings(inputStream, email)
+            }
+        } catch (e: AutoconfigParserException) {
+            Timber.d(e, "Failed to parse config from URL: %s", autoconfigUrl)
+            null
+        } catch (e: IOException) {
+            Timber.d(e, "Error fetching Autoconfig from URL: %s", autoconfigUrl)
+            null
+        }
+    }
 }

feature/autodiscovery/autoconfig/src/main/kotlin/app/k9mail/autodiscovery/autoconfig/AutoconfigParser.kt

@@ -1,26 +1,46 @@
 package app.k9mail.autodiscovery.autoconfig
 
-import app.k9mail.autodiscovery.api.DiscoveredServerSettings
-import app.k9mail.autodiscovery.api.DiscoveryResults
+import app.k9mail.autodiscovery.api.AuthenticationType
+import app.k9mail.autodiscovery.api.AuthenticationType.OAuth2
+import app.k9mail.autodiscovery.api.AuthenticationType.PasswordCleartext
+import app.k9mail.autodiscovery.api.AuthenticationType.PasswordEncrypted
+import app.k9mail.autodiscovery.api.AutoDiscoveryResult
+import app.k9mail.autodiscovery.api.ConnectionSecurity
+import app.k9mail.autodiscovery.api.ConnectionSecurity.StartTLS
+import app.k9mail.autodiscovery.api.ConnectionSecurity.TLS
+import app.k9mail.autodiscovery.api.ImapServerSettings
+import app.k9mail.autodiscovery.api.IncomingServerSettings
+import app.k9mail.autodiscovery.api.OutgoingServerSettings
+import app.k9mail.autodiscovery.api.SmtpServerSettings
 import app.k9mail.core.common.mail.EmailAddress
 import app.k9mail.core.common.net.HostNameUtils
+import app.k9mail.core.common.net.Hostname
+import app.k9mail.core.common.net.Port
+import app.k9mail.core.common.net.toHostname
+import app.k9mail.core.common.net.toPort
 import com.fsck.k9.helper.EmailHelper
 import com.fsck.k9.logging.Timber
-import com.fsck.k9.mail.AuthType
-import com.fsck.k9.mail.ConnectionSecurity
 import java.io.InputStream
 import java.io.InputStreamReader
 import org.xmlpull.v1.XmlPullParser
 import org.xmlpull.v1.XmlPullParserException
 import org.xmlpull.v1.XmlPullParserFactory
 
+private typealias ServerSettingsFactory<T> = (
+    hostname: Hostname,
+    port: Port,
+    connectionSecurity: ConnectionSecurity,
+    authenticationType: AuthenticationType,
+    username: String,
+) -> T
+
 /**
  * Parser for Thunderbird's Autoconfig file format.
  *
  * See [https://github.com/thundernest/autoconfig](https://github.com/thundernest/autoconfig)
  */
 class AutoconfigParser {
-    fun parseSettings(stream: InputStream, email: EmailAddress): DiscoveryResults {
+    fun parseSettings(stream: InputStream, email: EmailAddress): AutoDiscoveryResult? {
         return try {
             ClientConfigParser(stream, email.address).parse()
         } catch (e: XmlPullParserException) {
@@ -41,54 +61,49 @@ private class ClientConfigParser(
         setInput(InputStreamReader(inputStream))
     }
 
-    private val incomingServers = mutableListOf<DiscoveredServerSettings>()
-    private val outgoingServers = mutableListOf<DiscoveredServerSettings>()
-
-    fun parse(): DiscoveryResults {
-        var clientConfigFound = false
+    fun parse(): AutoDiscoveryResult {
+        var autoDiscoveryResult: AutoDiscoveryResult? = null
         do {
             val eventType = pullParser.next()
 
             if (eventType == XmlPullParser.START_TAG) {
                 when (pullParser.name) {
                     "clientConfig" -> {
-                        clientConfigFound = true
-                        parseClientConfig()
+                        autoDiscoveryResult = parseClientConfig()
                     }
                     else -> skipElement()
                 }
             }
         } while (eventType != XmlPullParser.END_DOCUMENT)
 
-        if (!clientConfigFound) {
+        if (autoDiscoveryResult == null) {
             parserError("Missing 'clientConfig' element")
         }
 
-        return DiscoveryResults(incomingServers, outgoingServers)
+        return autoDiscoveryResult
     }
 
-    private fun parseClientConfig() {
-        var emailProviderFound = false
+    private fun parseClientConfig(): AutoDiscoveryResult {
+        var autoDiscoveryResult: AutoDiscoveryResult? = null
 
         readElement { eventType ->
             if (eventType == XmlPullParser.START_TAG) {
                 when (pullParser.name) {
                     "emailProvider" -> {
-                        emailProviderFound = true
-                        parseEmailProvider()
+                        autoDiscoveryResult = parseEmailProvider()
                     }
                     else -> skipElement()
                 }
             }
         }
 
-        if (!emailProviderFound) {
-            parserError("Missing 'emailProvider' element")
-        }
+        return autoDiscoveryResult ?: parserError("Missing 'emailProvider' element")
     }
 
-    private fun parseEmailProvider() {
+    private fun parseEmailProvider(): AutoDiscoveryResult {
         var domainFound = false
+        var incomingServerSettings: IncomingServerSettings? = null
+        var outgoingServerSettings: OutgoingServerSettings? = null
 
         // The 'id' attribute is required (but not really used) by Thunderbird desktop.
         val emailProviderId = pullParser.getAttributeValue(null, "id")
@@ -108,13 +123,15 @@ private class ClientConfigParser(
                         }
                     }
                     "incomingServer" -> {
-                        parseServer("imap")?.let { serverSettings ->
-                            incomingServers.add(serverSettings)
+                        val serverSettings = parseServer("imap", ::createImapServerSettings)
+                        if (incomingServerSettings == null) {
+                            incomingServerSettings = serverSettings
                         }
                     }
                     "outgoingServer" -> {
-                        parseServer("smtp")?.let { serverSettings ->
-                            outgoingServers.add(serverSettings)
+                        val serverSettings = parseServer("smtp", ::createSmtpServerSettings)
+                        if (outgoingServerSettings == null) {
+                            outgoingServerSettings = serverSettings
                         }
                     }
                     else -> {
@@ -129,54 +146,54 @@ private class ClientConfigParser(
             parserError("Valid 'domain' element required")
         }
 
-        if (incomingServers.isEmpty()) {
-            parserError("Missing 'incomingServer' element")
-        }
-
-        if (outgoingServers.isEmpty()) {
-            parserError("Missing 'outgoingServer' element")
-        }
+        return AutoDiscoveryResult(
+            incomingServerSettings = incomingServerSettings ?: parserError("Missing 'incomingServer' element"),
+            outgoingServerSettings = outgoingServerSettings ?: parserError("Missing 'outgoingServer' element"),
+        )
     }
 
-    private fun parseServer(vararg supportedTypes: String): DiscoveredServerSettings? {
+    private fun <T> parseServer(
+        protocolType: String,
+        createServerSettings: ServerSettingsFactory<T>,
+    ): T? {
         val type = pullParser.getAttributeValue(null, "type")
-        if (type !in supportedTypes) {
+        if (type != protocolType) {
             Timber.d("Unsupported '%s[type]' value: '%s'", pullParser.name, type)
             skipElement()
             return null
         }
 
-        var hostName: String? = null
+        var hostname: String? = null
         var port: Int? = null
         var userName: String? = null
-        var authType: AuthType? = null
+        var authenticationType: AuthenticationType? = null
         var connectionSecurity: ConnectionSecurity? = null
 
         readElement { eventType ->
             if (eventType == XmlPullParser.START_TAG) {
                 when (pullParser.name) {
-                    "hostname" -> hostName = readHostname()
+                    "hostname" -> hostname = readHostname()
                     "port" -> port = readPort()
                     "username" -> userName = readUsername()
-                    "authentication" -> authType = readAuthentication(authType)
+                    "authentication" -> authenticationType = readAuthentication(authenticationType)
                     "socketType" -> connectionSecurity = readSocketType()
                 }
             }
         }
 
-        if (hostName == null) {
-            parserError("Missing 'hostname' element")
-        } else if (port == null) {
-            parserError("Missing 'port' element")
-        } else if (userName == null) {
-            parserError("Missing 'username' element")
-        } else if (authType == null) {
-            parserError("No usable 'authentication' element found")
-        } else if (connectionSecurity == null) {
-            parserError("Missing 'socketType' element")
-        }
+        val finalHostname = hostname ?: parserError("Missing 'hostname' element")
+        val finalPort = port ?: parserError("Missing 'port' element")
+        val finalUserName = userName ?: parserError("Missing 'username' element")
+        val finalAuthenticationType = authenticationType ?: parserError("No usable 'authentication' element found")
+        val finalConnectionSecurity = connectionSecurity ?: parserError("Missing 'socketType' element")
 
-        return DiscoveredServerSettings(type, hostName!!, port!!, connectionSecurity!!, authType, userName)
+        return createServerSettings(
+            finalHostname.toHostname(),
+            finalPort.toPort(),
+            finalConnectionSecurity,
+            finalAuthenticationType,
+            finalUserName,
+        )
     }
 
     private fun readHostname(): String {
@@ -194,17 +211,17 @@ private class ClientConfigParser(
 
     private fun readUsername(): String = readText().replaceVariables()
 
-    private fun readAuthentication(authType: AuthType?): AuthType? {
-        return authType ?: readText().toAuthType()
+    private fun readAuthentication(authenticationType: AuthenticationType?): AuthenticationType? {
+        return authenticationType ?: readText().toAuthenticationType()
     }
 
     private fun readSocketType() = readText().toConnectionSecurity()
 
-    private fun String.toAuthType(): AuthType? {
+    private fun String.toAuthenticationType(): AuthenticationType? {
         return when (this) {
-            "OAuth2" -> AuthType.XOAUTH2
-            "password-cleartext" -> AuthType.PLAIN
-            "password-encrypted" -> AuthType.CRAM_MD5
+            "OAuth2" -> OAuth2
+            "password-cleartext" -> PasswordCleartext
+            "password-encrypted" -> PasswordEncrypted
             else -> {
                 Timber.d("Ignoring unknown 'authentication' value '$this'")
                 null
@@ -214,8 +231,8 @@ private class ClientConfigParser(
 
     private fun String.toConnectionSecurity(): ConnectionSecurity {
         return when (this) {
-            "SSL" -> ConnectionSecurity.SSL_TLS_REQUIRED
-            "STARTTLS" -> ConnectionSecurity.STARTTLS_REQUIRED
+            "SSL" -> TLS
+            "STARTTLS" -> StartTLS
             else -> parserError("Unknown 'socketType' value: '$this'")
         }
     }
@@ -278,6 +295,26 @@ private class ClientConfigParser(
             .replace("%EMAILLOCALPART%", localPart)
             .replace("%EMAILADDRESS%", email)
     }
+
+    private fun createImapServerSettings(
+        hostname: Hostname,
+        port: Port,
+        connectionSecurity: ConnectionSecurity,
+        authenticationType: AuthenticationType,
+        username: String,
+    ): ImapServerSettings {
+        return ImapServerSettings(hostname, port, connectionSecurity, authenticationType, username)
+    }
+
+    private fun createSmtpServerSettings(
+        hostname: Hostname,
+        port: Port,
+        connectionSecurity: ConnectionSecurity,
+        authenticationType: AuthenticationType,
+        username: String,
+    ): SmtpServerSettings {
+        return SmtpServerSettings(hostname, port, connectionSecurity, authenticationType, username)
+    }
 }
 
 class AutoconfigParserException(message: String, cause: Throwable? = null) : RuntimeException(message, cause)

feature/autodiscovery/autoconfig/src/main/kotlin/app/k9mail/autodiscovery/autoconfig/MxLookupAutoconfigDiscovery.kt

@@ -1,10 +1,17 @@
 package app.k9mail.autodiscovery.autoconfig
 
-import app.k9mail.autodiscovery.api.DiscoveryResults
+import app.k9mail.autodiscovery.api.AutoDiscovery
+import app.k9mail.autodiscovery.api.AutoDiscoveryResult
+import app.k9mail.autodiscovery.api.AutoDiscoveryRunnable
 import app.k9mail.core.common.mail.EmailAddress
 import app.k9mail.core.common.net.Domain
 import app.k9mail.core.common.net.toDomain
 import com.fsck.k9.helper.EmailHelper
+import com.fsck.k9.logging.Timber
+import java.io.IOException
+import kotlinx.coroutines.Dispatchers
+import kotlinx.coroutines.withContext
+import okhttp3.HttpUrl
 
 class MxLookupAutoconfigDiscovery(
     private val mxResolver: MxResolver,
@@ -13,10 +20,24 @@ class MxLookupAutoconfigDiscovery(
     private val urlProvider: AutoconfigUrlProvider,
     private val fetcher: AutoconfigFetcher,
     private val parser: AutoconfigParser,
-) {
+) : AutoDiscovery {
+
+    override fun initDiscovery(email: EmailAddress): List<AutoDiscoveryRunnable> {
+        return listOf(
+            AutoDiscoveryRunnable {
+                mxLookupInBackground(email)
+            },
+        )
+    }
+
+    private suspend fun mxLookupInBackground(email: EmailAddress): AutoDiscoveryResult? {
+        return withContext(Dispatchers.IO) {
+            mxLookupAutoconfig(email)
+        }
+    }
 
     @Suppress("ReturnCount")
-    fun discover(email: EmailAddress): DiscoveryResults? {
+    private fun mxLookupAutoconfig(email: EmailAddress): AutoDiscoveryResult? {
         val domain = requireNotNull(EmailHelper.getDomainFromEmailAddress(email.address)?.toDomain()) {
             "Couldn't extract domain from email address: ${email.address}"
         }
@@ -36,17 +57,18 @@ class MxLookupAutoconfigDiscovery(
         return listOfNotNull(mxSubDomain, mxBaseDomain)
             .asSequence()
             .flatMap { domainToCheck -> urlProvider.getAutoconfigUrls(domainToCheck) }
-            .mapNotNull { autoconfigUrl ->
-                fetcher.fetchAutoconfigFile(autoconfigUrl)?.use { inputStream ->
-                    parser.parseSettings(inputStream, email)
-                }
-            }
+            .mapNotNull { autoconfigUrl -> getAutoconfig(email, autoconfigUrl) }
             .firstOrNull()
     }
 
     private fun mxLookup(domain: Domain): Domain? {
         // Only return the most preferred entry to match Thunderbird's behavior.
-        return mxResolver.lookup(domain).firstOrNull()
+        return try {
+            mxResolver.lookup(domain).firstOrNull()
+        } catch (e: IOException) {
+            Timber.d(e, "Failed to get MX record for domain: %s", domain.value)
+            null
+        }
     }
 
     private fun getMxBaseDomain(mxHostName: Domain): Domain {
@@ -56,4 +78,18 @@ class MxLookupAutoconfigDiscovery(
     private fun getNextSubDomain(domain: Domain): Domain? {
         return subDomainExtractor.extractSubDomain(domain)
     }
+
+    private fun getAutoconfig(email: EmailAddress, autoconfigUrl: HttpUrl): AutoDiscoveryResult? {
+        return try {
+            fetcher.fetchAutoconfigFile(autoconfigUrl)?.use { inputStream ->
+                parser.parseSettings(inputStream, email)
+            }
+        } catch (e: AutoconfigParserException) {
+            Timber.d(e, "Failed to parse config from URL: %s", autoconfigUrl)
+            null
+        } catch (e: IOException) {
+            Timber.d(e, "Error fetching Autoconfig from URL: %s", autoconfigUrl)
+            null
+        }
+    }
 }

feature/autodiscovery/autoconfig/src/test/kotlin/app/k9mail/autodiscovery/autoconfig/AutoconfigParserTest.kt

@@ -1,18 +1,22 @@
 package app.k9mail.autodiscovery.autoconfig
 
-import app.k9mail.autodiscovery.api.DiscoveredServerSettings
-import app.k9mail.autodiscovery.api.DiscoveryResults
+import app.k9mail.autodiscovery.api.AuthenticationType.OAuth2
+import app.k9mail.autodiscovery.api.AuthenticationType.PasswordCleartext
+import app.k9mail.autodiscovery.api.AutoDiscoveryResult
+import app.k9mail.autodiscovery.api.ConnectionSecurity.StartTLS
+import app.k9mail.autodiscovery.api.ConnectionSecurity.TLS
+import app.k9mail.autodiscovery.api.ImapServerSettings
+import app.k9mail.autodiscovery.api.SmtpServerSettings
 import app.k9mail.core.common.mail.toEmailAddress
-import assertk.all
+import app.k9mail.core.common.net.toHostname
+import app.k9mail.core.common.net.toPort
 import assertk.assertFailure
 import assertk.assertThat
-import assertk.assertions.containsExactly
 import assertk.assertions.hasMessage
+import assertk.assertions.isEqualTo
 import assertk.assertions.isInstanceOf
 import assertk.assertions.isNotNull
 import assertk.assertions.prop
-import com.fsck.k9.mail.AuthType
-import com.fsck.k9.mail.ConnectionSecurity
 import java.io.InputStream
 import org.intellij.lang.annotations.Language
 import org.jsoup.Jsoup
@@ -59,28 +63,24 @@ class AutoconfigParserTest {
 
         val result = parser.parseSettings(inputStream, email = "user@domain.example".toEmailAddress())
 
-        assertThat(result).isNotNull().all {
-            prop(DiscoveryResults::incoming).containsExactly(
-                DiscoveredServerSettings(
-                    protocol = "imap",
-                    host = "imap.domain.example",
-                    port = 993,
-                    security = ConnectionSecurity.SSL_TLS_REQUIRED,
-                    authType = AuthType.PLAIN,
+        assertThat(result).isNotNull().isEqualTo(
+            AutoDiscoveryResult(
+                ImapServerSettings(
+                    hostname = "imap.domain.example".toHostname(),
+                    port = 993.toPort(),
+                    connectionSecurity = TLS,
+                    authenticationType = PasswordCleartext,
                     username = "user@domain.example",
                 ),
-            )
-            prop(DiscoveryResults::outgoing).containsExactly(
-                DiscoveredServerSettings(
-                    protocol = "smtp",
-                    host = "smtp.domain.example",
-                    port = 587,
-                    security = ConnectionSecurity.STARTTLS_REQUIRED,
-                    authType = AuthType.PLAIN,
+                SmtpServerSettings(
+                    hostname = "smtp.domain.example".toHostname(),
+                    port = 587.toPort(),
+                    connectionSecurity = StartTLS,
+                    authenticationType = PasswordCleartext,
                     username = "user@domain.example",
                 ),
-            )
-        }
+            ),
+        )
     }
 
     @Test
@@ -89,28 +89,24 @@ class AutoconfigParserTest {
 
         val result = parser.parseSettings(inputStream, email = "test@gmail.com".toEmailAddress())
 
-        assertThat(result).isNotNull().all {
-            prop(DiscoveryResults::incoming).containsExactly(
-                DiscoveredServerSettings(
-                    protocol = "imap",
-                    host = "imap.gmail.com",
-                    port = 993,
-                    security = ConnectionSecurity.SSL_TLS_REQUIRED,
-                    authType = AuthType.XOAUTH2,
+        assertThat(result).isNotNull().isEqualTo(
+            AutoDiscoveryResult(
+                ImapServerSettings(
+                    hostname = "imap.gmail.com".toHostname(),
+                    port = 993.toPort(),
+                    connectionSecurity = TLS,
+                    authenticationType = OAuth2,
                     username = "test@gmail.com",
                 ),
-            )
-            prop(DiscoveryResults::outgoing).containsExactly(
-                DiscoveredServerSettings(
-                    protocol = "smtp",
-                    host = "smtp.gmail.com",
-                    port = 465,
-                    security = ConnectionSecurity.SSL_TLS_REQUIRED,
-                    authType = AuthType.XOAUTH2,
+                SmtpServerSettings(
+                    hostname = "smtp.gmail.com".toHostname(),
+                    port = 465.toPort(),
+                    connectionSecurity = TLS,
+                    authenticationType = OAuth2,
                     username = "test@gmail.com",
                 ),
-            )
-        }
+            ),
+        )
     }
 
     @Test
@@ -123,28 +119,24 @@ class AutoconfigParserTest {
 
         val result = parser.parseSettings(inputStream, email = "user@domain.example".toEmailAddress())
 
-        assertThat(result).isNotNull().all {
-            prop(DiscoveryResults::incoming).containsExactly(
-                DiscoveredServerSettings(
-                    protocol = "imap",
-                    host = "user.domain.example",
-                    port = 993,
-                    security = ConnectionSecurity.SSL_TLS_REQUIRED,
-                    authType = AuthType.PLAIN,
+        assertThat(result).isNotNull().isEqualTo(
+            AutoDiscoveryResult(
+                ImapServerSettings(
+                    hostname = "user.domain.example".toHostname(),
+                    port = 993.toPort(),
+                    connectionSecurity = TLS,
+                    authenticationType = PasswordCleartext,
                     username = "user@domain.example",
                 ),
-            )
-            prop(DiscoveryResults::outgoing).containsExactly(
-                DiscoveredServerSettings(
-                    protocol = "smtp",
-                    host = "user.outgoing.domain.example",
-                    port = 587,
-                    security = ConnectionSecurity.STARTTLS_REQUIRED,
-                    authType = AuthType.PLAIN,
+                SmtpServerSettings(
+                    hostname = "user.outgoing.domain.example".toHostname(),
+                    port = 587.toPort(),
+                    connectionSecurity = StartTLS,
+                    authenticationType = PasswordCleartext,
                     username = "domain.example",
                 ),
-            )
-        }
+            ),
+        )
     }
 
     @Test
@@ -159,16 +151,16 @@ class AutoconfigParserTest {
 
         val result = parser.parseSettings(inputStream, email = "user@domain.example".toEmailAddress())
 
-        assertThat(result.incoming).containsExactly(
-            DiscoveredServerSettings(
-                protocol = "imap",
-                host = "imap.domain.example",
-                port = 993,
-                security = ConnectionSecurity.SSL_TLS_REQUIRED,
-                authType = AuthType.PLAIN,
-                username = "user@domain.example",
-            ),
-        )
+        assertThat(result).isNotNull()
+            .prop(AutoDiscoveryResult::incomingServerSettings).isEqualTo(
+                ImapServerSettings(
+                    hostname = "imap.domain.example".toHostname(),
+                    port = 993.toPort(),
+                    connectionSecurity = TLS,
+                    authenticationType = PasswordCleartext,
+                    username = "user@domain.example",
+                ),
+            )
     }
 
     @Test
@@ -179,16 +171,16 @@ class AutoconfigParserTest {
 
         val result = parser.parseSettings(inputStream, email = "user@domain.example".toEmailAddress())
 
-        assertThat(result.incoming).containsExactly(
-            DiscoveredServerSettings(
-                protocol = "imap",
-                host = "imap.domain.example",
-                port = 993,
-                security = ConnectionSecurity.SSL_TLS_REQUIRED,
-                authType = AuthType.PLAIN,
-                username = "user@domain.example",
-            ),
-        )
+        assertThat(result).isNotNull()
+            .prop(AutoDiscoveryResult::incomingServerSettings).isEqualTo(
+                ImapServerSettings(
+                    hostname = "imap.domain.example".toHostname(),
+                    port = 993.toPort(),
+                    connectionSecurity = TLS,
+                    authenticationType = PasswordCleartext,
+                    username = "user@domain.example",
+                ),
+            )
     }
 
     @Test
@@ -199,16 +191,16 @@ class AutoconfigParserTest {
 
         val result = parser.parseSettings(inputStream, email = "user@domain.example".toEmailAddress())
 
-        assertThat(result.outgoing).containsExactly(
-            DiscoveredServerSettings(
-                protocol = "smtp",
-                host = "smtp.domain.example",
-                port = 587,
-                security = ConnectionSecurity.STARTTLS_REQUIRED,
-                authType = AuthType.PLAIN,
-                username = "user@domain.example",
-            ),
-        )
+        assertThat(result).isNotNull()
+            .prop(AutoDiscoveryResult::outgoingServerSettings).isEqualTo(
+                SmtpServerSettings(
+                    hostname = "smtp.domain.example".toHostname(),
+                    port = 587.toPort(),
+                    connectionSecurity = StartTLS,
+                    authenticationType = PasswordCleartext,
+                    username = "user@domain.example",
+                ),
+            )
     }
 
     @Test
@@ -219,16 +211,16 @@ class AutoconfigParserTest {
 
         val result = parser.parseSettings(inputStream, email = "user@domain.example".toEmailAddress())
 
-        assertThat(result.incoming).containsExactly(
-            DiscoveredServerSettings(
-                protocol = "imap",
-                host = "imap.domain.example",
-                port = 993,
-                security = ConnectionSecurity.SSL_TLS_REQUIRED,
-                authType = AuthType.PLAIN,
-                username = "user@domain.example",
-            ),
-        )
+        assertThat(result).isNotNull()
+            .prop(AutoDiscoveryResult::incomingServerSettings).isEqualTo(
+                ImapServerSettings(
+                    hostname = "imap.domain.example".toHostname(),
+                    port = 993.toPort(),
+                    connectionSecurity = TLS,
+                    authenticationType = PasswordCleartext,
+                    username = "user@domain.example",
+                ),
+            )
     }
 
     @Test