Merge pull request #6903 from thundernest/mx_lookup_autoconfig_discovery

Add support for ISPDB query after DNS MX lookup

feature/autodiscovery/autoconfig/build.gradle.kts

@@ -8,6 +8,7 @@ dependencies {
 
     compileOnly(libs.xmlpull)
     implementation(libs.okhttp)
+    implementation(libs.minidns.hla)
 
     testImplementation(libs.kxml2)
     testImplementation(libs.jsoup)

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

@@ -2,6 +2,7 @@ package app.k9mail.autodiscovery.autoconfig
 
 import app.k9mail.autodiscovery.api.ConnectionSettingsDiscovery
 import app.k9mail.autodiscovery.api.DiscoveryResults
+import com.fsck.k9.helper.EmailHelper
 
 class AutoconfigDiscovery(
     private val urlProvider: AutoconfigUrlProvider,
@@ -10,7 +11,11 @@ class AutoconfigDiscovery(
 ) : ConnectionSettingsDiscovery {
 
     override fun discover(email: String): DiscoveryResults? {
-        val autoconfigUrls = urlProvider.getAutoconfigUrls(email)
+        val domain = requireNotNull(EmailHelper.getDomainFromEmailAddress(email)) {
+            "Couldn't extract domain from email address: $email"
+        }
+
+        val autoconfigUrls = urlProvider.getAutoconfigUrls(domain, email)
 
         return autoconfigUrls
             .asSequence()

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

@@ -3,5 +3,5 @@ package app.k9mail.autodiscovery.autoconfig
 import okhttp3.HttpUrl
 
 interface AutoconfigUrlProvider {
-    fun getAutoconfigUrls(email: String): List<HttpUrl>
+    fun getAutoconfigUrls(domain: String, email: String? = null): List<HttpUrl>
 }

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

@@ -0,0 +1,10 @@
+package app.k9mail.autodiscovery.autoconfig
+
+/**
+ * Extract the base domain from a host name.
+ *
+ * An implementation needs to respect the [Public Suffix List](https://publicsuffix.org/).
+ */
+interface BaseDomainExtractor {
+    fun extractBaseDomain(domain: String): String
+}

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

@@ -1,14 +1,10 @@
 package app.k9mail.autodiscovery.autoconfig
 
-import com.fsck.k9.helper.EmailHelper
 import okhttp3.HttpUrl
 import okhttp3.HttpUrl.Companion.toHttpUrl
 
 class IspDbAutoconfigUrlProvider : AutoconfigUrlProvider {
-    override fun getAutoconfigUrls(email: String): List<HttpUrl> {
-        val domain = EmailHelper.getDomainFromEmailAddress(email)
-        requireNotNull(domain) { "Couldn't extract domain from email address: $email" }
-
+    override fun getAutoconfigUrls(domain: String, email: String?): List<HttpUrl> {
         return listOf(createIspDbUrl(domain))
     }
 

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

@@ -0,0 +1,13 @@
+package app.k9mail.autodiscovery.autoconfig
+
+import org.minidns.hla.ResolverApi
+import org.minidns.record.MX
+
+class MiniDnsMxResolver : MxResolver {
+    override fun lookup(domain: String): List<String> {
+        val result = ResolverApi.INSTANCE.resolve(domain, MX::class.java)
+        return result.answersOrEmptySet
+            .sortedBy { it.priority }
+            .map { it.target.toString() }
+    }
+}

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

@@ -0,0 +1,57 @@
+package app.k9mail.autodiscovery.autoconfig
+
+import app.k9mail.autodiscovery.api.ConnectionSettingsDiscovery
+import app.k9mail.autodiscovery.api.DiscoveryResults
+import com.fsck.k9.helper.EmailHelper
+
+class MxLookupAutoconfigDiscovery(
+    private val mxResolver: MxResolver,
+    private val baseDomainExtractor: BaseDomainExtractor,
+    private val subDomainExtractor: SubDomainExtractor,
+    private val urlProvider: AutoconfigUrlProvider,
+    private val fetcher: AutoconfigFetcher,
+    private val parser: AutoconfigParser,
+) : ConnectionSettingsDiscovery {
+
+    @Suppress("ReturnCount")
+    override fun discover(email: String): DiscoveryResults? {
+        val domain = requireNotNull(EmailHelper.getDomainFromEmailAddress(email)) {
+            "Couldn't extract domain from email address: $email"
+        }
+
+        val mxHostName = mxLookup(domain) ?: return null
+
+        val mxBaseDomain = getMxBaseDomain(mxHostName)
+        if (mxBaseDomain == domain) {
+            // Exit early to match Thunderbird's behavior.
+            return null
+        }
+
+        // In addition to just the base domain, also check the MX hostname without the first label to differentiate
+        // between Outlook.com/Hotmail and Office365 business domains.
+        val mxSubDomain = getNextSubDomain(mxHostName)?.takeIf { it != mxBaseDomain }
+
+        return listOfNotNull(mxSubDomain, mxBaseDomain)
+            .asSequence()
+            .flatMap { domainToCheck -> urlProvider.getAutoconfigUrls(domainToCheck) }
+            .mapNotNull { autoconfigUrl ->
+                fetcher.fetchAutoconfigFile(autoconfigUrl)?.use { inputStream ->
+                    parser.parseSettings(inputStream, email)
+                }
+            }
+            .firstOrNull()
+    }
+
+    private fun mxLookup(domain: String): String? {
+        // Only return the most preferred entry to match Thunderbird's behavior.
+        return mxResolver.lookup(domain).firstOrNull()
+    }
+
+    private fun getMxBaseDomain(mxHostName: String): String {
+        return baseDomainExtractor.extractBaseDomain(mxHostName)
+    }
+
+    private fun getNextSubDomain(domain: String): String? {
+        return subDomainExtractor.extractSubDomain(domain)
+    }
+}

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

@@ -0,0 +1,8 @@
+package app.k9mail.autodiscovery.autoconfig
+
+/**
+ * Look up MX records for a domain.
+ */
+interface MxResolver {
+    fun lookup(domain: String): List<String>
+}

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

@@ -0,0 +1,16 @@
+package app.k9mail.autodiscovery.autoconfig
+
+import okhttp3.HttpUrl
+
+class OkHttpBaseDomainExtractor : BaseDomainExtractor {
+    override fun extractBaseDomain(domain: String): String {
+        return domain.toHttpUrlOrNull().topPrivateDomain() ?: domain
+    }
+
+    private fun String.toHttpUrlOrNull(): HttpUrl {
+        return HttpUrl.Builder()
+            .scheme("https")
+            .host(this)
+            .build()
+    }
+}

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

@@ -1,13 +1,9 @@
 package app.k9mail.autodiscovery.autoconfig
 
-import com.fsck.k9.helper.EmailHelper
 import okhttp3.HttpUrl
 
 class ProviderAutoconfigUrlProvider(private val config: AutoconfigUrlConfig) : AutoconfigUrlProvider {
-    override fun getAutoconfigUrls(email: String): List<HttpUrl> {
-        val domain = EmailHelper.getDomainFromEmailAddress(email)
-        requireNotNull(domain) { "Couldn't extract domain from email address: $email" }
-
+    override fun getAutoconfigUrls(domain: String, email: String?): List<HttpUrl> {
         return buildList {
             add(createProviderUrl(domain, email, useHttps = true))
             add(createDomainUrl(domain, email, useHttps = true))
@@ -19,7 +15,7 @@ class ProviderAutoconfigUrlProvider(private val config: AutoconfigUrlConfig) : A
         }
     }
 
-    private fun createProviderUrl(domain: String, email: String, useHttps: Boolean): HttpUrl {
+    private fun createProviderUrl(domain: String, email: String?, useHttps: Boolean): HttpUrl {
         // https://autoconfig.{domain}/mail/config-v1.1.xml?emailaddress={email}
         // http://autoconfig.{domain}/mail/config-v1.1.xml?emailaddress={email}
         return HttpUrl.Builder()
@@ -27,14 +23,14 @@ class ProviderAutoconfigUrlProvider(private val config: AutoconfigUrlConfig) : A
             .host("autoconfig.$domain")
             .addEncodedPathSegments("mail/config-v1.1.xml")
             .apply {
-                if (config.includeEmailAddress) {
+                if (email != null && config.includeEmailAddress) {
                     addQueryParameter("emailaddress", email)
                 }
             }
             .build()
     }
 
-    private fun createDomainUrl(domain: String, email: String, useHttps: Boolean): HttpUrl {
+    private fun createDomainUrl(domain: String, email: String?, useHttps: Boolean): HttpUrl {
         // https://{domain}/.well-known/autoconfig/mail/config-v1.1.xml?emailaddress={email}
         // http://{domain}/.well-known/autoconfig/mail/config-v1.1.xml?emailaddress={email}
         return HttpUrl.Builder()
@@ -42,7 +38,7 @@ class ProviderAutoconfigUrlProvider(private val config: AutoconfigUrlConfig) : A
             .host(domain)
             .addEncodedPathSegments(".well-known/autoconfig/mail/config-v1.1.xml")
             .apply {
-                if (config.includeEmailAddress) {
+                if (email != null && config.includeEmailAddress) {
                     addQueryParameter("emailaddress", email)
                 }
             }

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

@@ -0,0 +1,22 @@
+package app.k9mail.autodiscovery.autoconfig
+
+class RealSubDomainExtractor(private val baseDomainExtractor: BaseDomainExtractor) : SubDomainExtractor {
+    @Suppress("ReturnCount")
+    override fun extractSubDomain(domain: String): String? {
+        val baseDomain = baseDomainExtractor.extractBaseDomain(domain)
+        if (baseDomain == domain) {
+            // The domain doesn't have a sub domain.
+            return null
+        }
+
+        val domainPrefix = domain.removeSuffix(".$baseDomain")
+        val index = domainPrefix.indexOf('.')
+        if (index == -1) {
+            // The prefix is the sub domain. When we remove it only the base domain remains.
+            return baseDomain
+        }
+
+        val prefixWithoutFirstLabel = domainPrefix.substring(index + 1)
+        return "$prefixWithoutFirstLabel.$baseDomain"
+    }
+}

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

@@ -0,0 +1,10 @@
+package app.k9mail.autodiscovery.autoconfig
+
+/**
+ * Extract the sub domain from a host name.
+ *
+ * An implementation needs to respect the [Public Suffix List](https://publicsuffix.org/).
+ */
+interface SubDomainExtractor {
+    fun extractSubDomain(domain: String): String?
+}

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

@@ -9,7 +9,7 @@ class IspDbAutoconfigUrlProviderTest {
 
     @Test
     fun `getAutoconfigUrls with ASCII email address`() {
-        val autoconfigUrls = urlProvider.getAutoconfigUrls("test@domain.example")
+        val autoconfigUrls = urlProvider.getAutoconfigUrls(domain = "domain.example")
 
         assertThat(autoconfigUrls.map { it.toString() }).containsExactly(
             "https://autoconfig.thunderbird.net/v1.1/domain.example",

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

@@ -0,0 +1,30 @@
+package app.k9mail.autodiscovery.autoconfig
+
+import assertk.all
+import assertk.assertThat
+import assertk.assertions.containsExactlyInAnyOrder
+import assertk.assertions.index
+import assertk.assertions.isEqualTo
+import kotlin.test.Ignore
+import kotlin.test.Test
+
+class MiniDnsMxResolverTest {
+    private val resolver = MiniDnsMxResolver()
+
+    @Test
+    @Ignore("Requires internet")
+    fun `MX lookup for known domain`() {
+        val result = resolver.lookup("thunderbird.net")
+
+        assertThat(result).all {
+            index(0).isEqualTo("aspmx.l.google.com")
+            containsExactlyInAnyOrder(
+                "aspmx.l.google.com",
+                "alt1.aspmx.l.google.com",
+                "alt2.aspmx.l.google.com",
+                "alt4.aspmx.l.google.com",
+                "alt3.aspmx.l.google.com",
+            )
+        }
+    }
+}

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

@@ -0,0 +1,29 @@
+package app.k9mail.autodiscovery.autoconfig
+
+import assertk.assertThat
+import assertk.assertions.isEqualTo
+import org.junit.Test
+
+class OkHttpBaseDomainExtractorTest {
+    private val baseDomainExtractor = OkHttpBaseDomainExtractor()
+
+    @Test
+    fun `basic domain`() {
+        assertThat(baseDomainExtractor.extractBaseDomain("domain.example")).isEqualTo("domain.example")
+    }
+
+    @Test
+    fun `basic subdomain`() {
+        assertThat(baseDomainExtractor.extractBaseDomain("subdomain.domain.example")).isEqualTo("domain.example")
+    }
+
+    @Test
+    fun `domain with public suffix`() {
+        assertThat(baseDomainExtractor.extractBaseDomain("example.co.uk")).isEqualTo("example.co.uk")
+    }
+
+    @Test
+    fun `subdomain with public suffix`() {
+        assertThat(baseDomainExtractor.extractBaseDomain("subdomain.example.co.uk")).isEqualTo("example.co.uk")
+    }
+}

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

@@ -10,7 +10,8 @@ class ProviderAutoconfigUrlProviderTest {
         val urlProvider = ProviderAutoconfigUrlProvider(
             AutoconfigUrlConfig(httpsOnly = false, includeEmailAddress = true),
         )
-        val autoconfigUrls = urlProvider.getAutoconfigUrls("test@domain.example")
+
+        val autoconfigUrls = urlProvider.getAutoconfigUrls(domain = "domain.example", email = "test@domain.example")
 
         assertThat(autoconfigUrls.map { it.toString() }).containsExactly(
             "https://autoconfig.domain.example/mail/config-v1.1.xml?emailaddress=test%40domain.example",
@@ -25,7 +26,8 @@ class ProviderAutoconfigUrlProviderTest {
         val urlProvider = ProviderAutoconfigUrlProvider(
             AutoconfigUrlConfig(httpsOnly = true, includeEmailAddress = true),
         )
-        val autoconfigUrls = urlProvider.getAutoconfigUrls("test@domain.example")
+
+        val autoconfigUrls = urlProvider.getAutoconfigUrls(domain = "domain.example", email = "test@domain.example")
 
         assertThat(autoconfigUrls.map { it.toString() }).containsExactly(
             "https://autoconfig.domain.example/mail/config-v1.1.xml?emailaddress=test%40domain.example",
@@ -38,7 +40,8 @@ class ProviderAutoconfigUrlProviderTest {
         val urlProvider = ProviderAutoconfigUrlProvider(
             AutoconfigUrlConfig(httpsOnly = true, includeEmailAddress = false),
         )
-        val autoconfigUrls = urlProvider.getAutoconfigUrls("test@domain.example")
+
+        val autoconfigUrls = urlProvider.getAutoconfigUrls(domain = "domain.example", email = "test@domain.example")
 
         assertThat(autoconfigUrls.map { it.toString() }).containsExactly(
             "https://autoconfig.domain.example/mail/config-v1.1.xml",
@@ -51,7 +54,24 @@ class ProviderAutoconfigUrlProviderTest {
         val urlProvider = ProviderAutoconfigUrlProvider(
             AutoconfigUrlConfig(httpsOnly = false, includeEmailAddress = false),
         )
-        val autoconfigUrls = urlProvider.getAutoconfigUrls("test@domain.example")
+
+        val autoconfigUrls = urlProvider.getAutoconfigUrls(domain = "domain.example", email = "test@domain.example")
+
+        assertThat(autoconfigUrls.map { it.toString() }).containsExactly(
+            "https://autoconfig.domain.example/mail/config-v1.1.xml",
+            "https://domain.example/.well-known/autoconfig/mail/config-v1.1.xml",
+            "http://autoconfig.domain.example/mail/config-v1.1.xml",
+            "http://domain.example/.well-known/autoconfig/mail/config-v1.1.xml",
+        )
+    }
+
+    @Test
+    fun `getAutoconfigUrls with http allowed and email address included, but none provided`() {
+        val urlProvider = ProviderAutoconfigUrlProvider(
+            AutoconfigUrlConfig(httpsOnly = false, includeEmailAddress = true),
+        )
+
+        val autoconfigUrls = urlProvider.getAutoconfigUrls(domain = "domain.example")
 
         assertThat(autoconfigUrls.map { it.toString() }).containsExactly(
             "https://autoconfig.domain.example/mail/config-v1.1.xml",

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

@@ -0,0 +1,81 @@
+package app.k9mail.autodiscovery.autoconfig
+
+import assertk.assertThat
+import assertk.assertions.isEqualTo
+import assertk.assertions.isNull
+import kotlin.test.Test
+
+class RealSubDomainExtractorTest {
+    private val testBaseDomainExtractor = TestBaseDomainExtractor(baseDomain = "domain.example")
+    private val baseSubDomainExtractor = RealSubDomainExtractor(testBaseDomainExtractor)
+
+    @Test
+    fun `input has one more label than the base domain`() {
+        val result = baseSubDomainExtractor.extractSubDomain("subdomain.domain.example")
+
+        assertThat(result).isEqualTo("domain.example")
+    }
+
+    @Test
+    fun `input has two more labels than the base domain`() {
+        val result = baseSubDomainExtractor.extractSubDomain("more.subdomain.domain.example")
+
+        assertThat(result).isEqualTo("subdomain.domain.example")
+    }
+
+    @Test
+    fun `input has three more labels than the base domain`() {
+        val result = baseSubDomainExtractor.extractSubDomain("three.two.one.domain.example")
+
+        assertThat(result).isEqualTo("two.one.domain.example")
+    }
+
+    @Test
+    fun `no sub domain available`() {
+        val result = baseSubDomainExtractor.extractSubDomain("domain.example")
+
+        assertThat(result).isNull()
+    }
+
+    @Test
+    fun `input has one more label than the base domain with public suffix`() {
+        testBaseDomainExtractor.baseDomain = "example.co.uk"
+
+        val result = baseSubDomainExtractor.extractSubDomain("subdomain.example.co.uk")
+
+        assertThat(result).isEqualTo("example.co.uk")
+    }
+
+    @Test
+    fun `input has two more labels than the base domain with public suffix`() {
+        testBaseDomainExtractor.baseDomain = "example.co.uk"
+
+        val result = baseSubDomainExtractor.extractSubDomain("more.subdomain.example.co.uk")
+
+        assertThat(result).isEqualTo("subdomain.example.co.uk")
+    }
+
+    @Test
+    fun `input has three more labels than the base domain with public suffix`() {
+        testBaseDomainExtractor.baseDomain = "example.co.uk"
+
+        val result = baseSubDomainExtractor.extractSubDomain("three.two.one.example.co.uk")
+
+        assertThat(result).isEqualTo("two.one.example.co.uk")
+    }
+
+    @Test
+    fun `no sub domain available with public suffix`() {
+        testBaseDomainExtractor.baseDomain = "example.co.uk"
+
+        val result = baseSubDomainExtractor.extractSubDomain("example.co.uk")
+
+        assertThat(result).isNull()
+    }
+}
+
+private class TestBaseDomainExtractor(var baseDomain: String) : BaseDomainExtractor {
+    override fun extractBaseDomain(domain: String): String {
+        return baseDomain
+    }
+}