Merge release/v1.3.0 into feat/68-self-signed-http

CHANGELOG.md

@@ -7,24 +7,6 @@ All notable changes to this project are documented here. The format follows
 
 ## [Unreleased]
 
-### Added
-- Self-hosted servers with a self-signed certificate can be added: Agendula
-  shows the certificate's fingerprint and asks whether to trust it. A server
-  reached over plain http can be added too, after a warning. If a trusted
-  certificate is later renewed, "Sign in again" on the account asks about the
-  new one (#68).
-- An account can be given its own name (#105).
-- Pull down on the task list to sync (#73).
-- Task list sections can be collapsed (#91).
-- A pure black theme for AMOLED screens (#99).
-- Agendula can show up as "Tasks" in your launcher (#104).
-- Reminder notifications show the task's description (#94).
-
-### Changed
-- Adding or opening a task from a widget returns you to the home screen
-  afterwards (#93).
-- A swipe that starts at the screen edge no longer swipes the task (#90).
-
 ## [1.2.0] - 2026-10-26
 
 ### Added

app/build.gradle.kts

@@ -33,8 +33,8 @@ android {
         // (1.0.x: 1.0.0 -> 10000; from 1.1.0: 1.1.0-beta.1 -> 1010001,
         // 1.1.0 -> 1010099), and CI fails if the committed one doesn't match.
         // See docs/RELEASING.md.
-        versionCode = 1030001
-        versionName = "1.3.0-beta.1"
+        versionCode = 1020099
+        versionName = "1.2.0"
 
         testInstrumentationRunner = "androidx.test.runner.AndroidJUnitRunner"
 

app/src/full/java/de/jeanlucmakiola/agendula/data/sync/AccountRepository.kt

@@ -17,7 +17,6 @@ import kotlinx.coroutines.flow.first
 import kotlinx.coroutines.withContext
 import okhttp3.HttpUrl
 import okhttp3.HttpUrl.Companion.toHttpUrlOrNull
-import java.io.IOException
 import javax.inject.Inject
 import javax.inject.Singleton
 
@@ -194,15 +193,7 @@ class AccountRepository @Inject constructor(
                 inserted += attach(id, selected)
                 id
             }
-            // Inside the rollback's reach: a DataStore failure here would otherwise
-            // leave the row with no credential, the hazard the comment above names.
-            if (!saveTrust(accountId, trust)) {
-                rollback(accountId, inserted)
-                return@withContext Outcome.CredentialFailed(
-                    Outcome.Cause.NOT_SAVED,
-                    "the server trust could not be stored",
-                )
-            }
+            serverTrust.put(accountId, trust)
 
             if (!credentials.put(accountId, appPassword)) {
                 // Never leave a half-made account behind: without a credential it
@@ -332,8 +323,7 @@ class AccountRepository @Inject constructor(
             -> Collections.NeedsSignIn
             is CalDavDiscovery.Outcome.NotCalDav -> Collections.Failed(outcome.cause)
             is CalDavDiscovery.Outcome.Failed -> Collections.Failed(outcome.cause)
-            // A certificate that changed since it was trusted. The account offers
-            // "Sign in again" for a certificate failure, and that flow re-asks.
+            // A certificate that changed since it was trusted; signing in again re-asks.
             is CalDavDiscovery.Outcome.UntrustedCertificate ->
                 Collections.Failed(CalDavDiscovery.Outcome.Cause.TLS)
         }
@@ -425,11 +415,6 @@ class AccountRepository @Inject constructor(
                 "the device keystore would not store the password",
             )
         }
-        // Before anything else changes, so a failure leaves the account stopped
-        // and the sign-in retryable rather than signed in with the old trust.
-        if (!withContext(NonCancellable) { saveTrust(accountId, trust) }) {
-            return Outcome.CredentialFailed(Outcome.Cause.NOT_SAVED, "the server trust could not be stored")
-        }
         withContext(NonCancellable) {
             database.runInTransaction {
                 database.accounts().update(
@@ -444,6 +429,7 @@ class AccountRepository @Inject constructor(
                 )
                 attach(accountId, selected)
             }
+            serverTrust.put(accountId, trust)
             // ⚠️ Ticked lists are attached; unticked ones are left alone. The
             // picker pre-ticks everything *writable*, not everything already
             // attached, so detaching what is unticked would silently stop
@@ -478,18 +464,9 @@ class AccountRepository @Inject constructor(
             inserted.forEach { database.taskLists().delete(it) }
             database.accounts().delete(accountId)
         }
-        // Ids are never reused, and a later create overwrites the keys anyway.
-        runCatching { serverTrust.forget(accountId) }
+        serverTrust.forget(accountId)
     }
 
-    private suspend fun saveTrust(accountId: Long, trust: ServerTrust): Boolean =
-        try {
-            serverTrust.put(accountId, trust)
-            true
-        } catch (_: IOException) {
-            false
-        }
-
     /**
      * Puts every existing account back on the periodic schedule.
      *

app/src/full/java/de/jeanlucmakiola/agendula/data/sync/PendingLoginFlowStore.kt

@@ -2,9 +2,11 @@ package de.jeanlucmakiola.agendula.data.sync
 
 import androidx.datastore.core.DataStore
 import androidx.datastore.preferences.core.Preferences
+import androidx.datastore.preferences.core.booleanPreferencesKey
 import androidx.datastore.preferences.core.edit
 import androidx.datastore.preferences.core.longPreferencesKey
 import androidx.datastore.preferences.core.stringPreferencesKey
+import androidx.datastore.preferences.core.stringSetPreferencesKey
 import de.jeanlucmakiola.agendula.data.di.SyncStateDataStore
 import de.jeanlucmakiola.caldav.NextcloudLoginFlow
 import de.jeanlucmakiola.caldav.ServerTrust
@@ -65,7 +67,8 @@ class PendingLoginFlowStore @Inject constructor(
             prefs[POLL_ENDPOINT] = flow.pollEndpoint.toString()
             prefs[POLL_TOKEN] = flow.pollToken
             prefs[DEADLINE] = flow.deadlineEpochSeconds
-            TRUST.write(prefs, trust)
+            prefs[TRUSTED_CERTIFICATES] = trust.pinnedCertificates
+            prefs[CLEARTEXT] = trust.allowCleartext
         }
     }
 
@@ -76,7 +79,8 @@ class PendingLoginFlowStore @Inject constructor(
             prefs.remove(POLL_ENDPOINT)
             prefs.remove(POLL_TOKEN)
             prefs.remove(DEADLINE)
-            TRUST.remove(prefs)
+            prefs.remove(TRUSTED_CERTIFICATES)
+            prefs.remove(CLEARTEXT)
         }
     }
 
@@ -141,13 +145,20 @@ class PendingLoginFlowStore @Inject constructor(
         )
     }
 
-    private suspend fun trust(): ServerTrust = TRUST.read(dataStore.data.first())
+    private suspend fun trust(): ServerTrust {
+        val prefs = dataStore.data.first()
+        return ServerTrust(
+            pinnedCertificates = prefs[TRUSTED_CERTIFICATES].orEmpty(),
+            allowCleartext = prefs[CLEARTEXT] ?: false,
+        )
+    }
 
     private companion object {
         val LOGIN_URL = stringPreferencesKey("login_flow_url")
         val POLL_ENDPOINT = stringPreferencesKey("login_flow_poll_endpoint")
         val POLL_TOKEN = stringPreferencesKey("login_flow_poll_token")
         val DEADLINE = longPreferencesKey("login_flow_deadline")
-        val TRUST = ServerTrustKeys("login_flow_trusted_certificates", "login_flow_cleartext")
+        val TRUSTED_CERTIFICATES = stringSetPreferencesKey("login_flow_trusted_certificates")
+        val CLEARTEXT = booleanPreferencesKey("login_flow_cleartext")
     }
 }

app/src/full/java/de/jeanlucmakiola/agendula/data/sync/ServerTrustKeys.kt

@@ -1,44 +0,0 @@
-package de.jeanlucmakiola.agendula.data.sync
-
-import androidx.datastore.preferences.core.MutablePreferences
-import androidx.datastore.preferences.core.Preferences
-import androidx.datastore.preferences.core.booleanPreferencesKey
-import androidx.datastore.preferences.core.stringSetPreferencesKey
-import androidx.lifecycle.SavedStateHandle
-import de.jeanlucmakiola.caldav.ServerTrust
-
-/**
- * The one mapping between a [ServerTrust] and the key-value stores it is kept
- * in, so a field added to it cannot be dropped by one of them.
- */
-class ServerTrustKeys(private val certificatesName: String, private val cleartextName: String) {
-
-    private val certificates = stringSetPreferencesKey(certificatesName)
-    private val cleartext = booleanPreferencesKey(cleartextName)
-
-    fun read(prefs: Preferences) = ServerTrust(
-        pinnedCertificates = prefs[certificates].orEmpty(),
-        allowCleartext = prefs[cleartext] ?: false,
-    )
-
-    fun write(prefs: MutablePreferences, trust: ServerTrust) {
-        if (trust.pinnedCertificates.isEmpty()) {
-            prefs.remove(certificates)
-        } else {
-            prefs[certificates] = trust.pinnedCertificates
-        }
-        if (trust.allowCleartext) prefs[cleartext] = true else prefs.remove(cleartext)
-    }
-
-    fun remove(prefs: MutablePreferences) = write(prefs, ServerTrust.DEFAULT)
-
-    fun read(handle: SavedStateHandle) = ServerTrust(
-        pinnedCertificates = handle.get<ArrayList<String>>(certificatesName).orEmpty().toSet(),
-        allowCleartext = handle[cleartextName] ?: false,
-    )
-
-    fun write(handle: SavedStateHandle, trust: ServerTrust) {
-        handle[certificatesName] = ArrayList(trust.pinnedCertificates)
-        handle[cleartextName] = trust.allowCleartext
-    }
-}

app/src/full/java/de/jeanlucmakiola/agendula/data/sync/ServerTrustStore.kt

@@ -2,7 +2,9 @@ package de.jeanlucmakiola.agendula.data.sync
 
 import androidx.datastore.core.DataStore
 import androidx.datastore.preferences.core.Preferences
+import androidx.datastore.preferences.core.booleanPreferencesKey
 import androidx.datastore.preferences.core.edit
+import androidx.datastore.preferences.core.stringSetPreferencesKey
 import de.jeanlucmakiola.agendula.data.di.SyncStateDataStore
 import de.jeanlucmakiola.caldav.ServerTrust
 import kotlinx.coroutines.flow.Flow
@@ -24,15 +26,29 @@ class ServerTrustStore @Inject constructor(
         dataStore.data.map { read(it, accountId) }.distinctUntilChanged()
 
     suspend fun put(accountId: Long, trust: ServerTrust) {
-        dataStore.edit { keys(accountId).write(it, trust) }
+        dataStore.edit { prefs ->
+            if (trust.pinnedCertificates.isEmpty()) {
+                prefs.remove(certificates(accountId))
+            } else {
+                prefs[certificates(accountId)] = trust.pinnedCertificates
+            }
+            if (trust.allowCleartext) prefs[cleartext(accountId)] = true else prefs.remove(cleartext(accountId))
+        }
     }
 
     suspend fun forget(accountId: Long) {
-        dataStore.edit { keys(accountId).remove(it) }
+        dataStore.edit { prefs ->
+            prefs.remove(certificates(accountId))
+            prefs.remove(cleartext(accountId))
+        }
     }
 
-    private fun read(prefs: Preferences, accountId: Long) = keys(accountId).read(prefs)
+    private fun read(prefs: Preferences, accountId: Long) = ServerTrust(
+        pinnedCertificates = prefs[certificates(accountId)].orEmpty(),
+        allowCleartext = prefs[cleartext(accountId)] ?: false,
+    )
 
-    private fun keys(accountId: Long) =
-        ServerTrustKeys("server_trust_certs_$accountId", "server_trust_cleartext_$accountId")
+    private fun certificates(accountId: Long) = stringSetPreferencesKey("server_trust_certs_$accountId")
+
+    private fun cleartext(accountId: Long) = booleanPreferencesKey("server_trust_cleartext_$accountId")
 }

app/src/full/java/de/jeanlucmakiola/agendula/ui/accounts/AccountDetailScreen.kt

@@ -169,7 +169,7 @@ internal fun AccountDetailScreen(
                     )
                 }
             }
-            if (shown.offersSignIn) {
+            if (shown.needsSignIn) {
                 add { position ->
                     GroupedRow(
                         title = stringResource(R.string.accounts_sign_in_again),
@@ -178,8 +178,7 @@ internal fun AccountDetailScreen(
                         onClick = { onSignInAgain(account) },
                     )
                 }
-            }
-            if (!shown.needsSignIn) {
+            } else {
                 add { position ->
                     GroupedRow(
                         title = stringResource(R.string.accounts_sync_now),

app/src/full/java/de/jeanlucmakiola/agendula/ui/accounts/AccountsScreen.kt

@@ -119,7 +119,7 @@ internal fun AccountsScreen(
                         when {
                             row.removing -> CircularProgressIndicator(Modifier.size(20.dp))
                             !usable -> Unit
-                            row.offersSignIn -> IconButton(onClick = { onSignInAgain(account) }) {
+                            row.needsSignIn -> IconButton(onClick = { onSignInAgain(account) }) {
                                 Icon(
                                     Icons.AutoMirrored.Rounded.Login,
                                     contentDescription = stringResource(R.string.accounts_sign_in_again),

app/src/full/java/de/jeanlucmakiola/agendula/ui/accounts/AccountsViewModel.kt

@@ -10,7 +10,6 @@ import de.jeanlucmakiola.agendula.data.sync.AccountRepository
 import de.jeanlucmakiola.agendula.data.sync.AccountStateStore
 import de.jeanlucmakiola.agendula.data.sync.ServerTrustStore
 import de.jeanlucmakiola.agendula.data.sync.SyncAvailability
-import de.jeanlucmakiola.agendula.data.sync.SyncFailure
 import de.jeanlucmakiola.agendula.data.sync.SyncNotice
 import de.jeanlucmakiola.agendula.data.sync.SyncNoticeStore
 import de.jeanlucmakiola.agendula.data.sync.SyncTrigger
@@ -192,18 +191,7 @@ class AccountsViewModel @Inject constructor(
          */
         val notices: List<SyncNotice> = emptyList(),
         val lists: ListPush = ListPush.NONE,
-    ) {
-        /**
-         * The last sync was refused over the server's certificate — typically a
-         * pinned one that was renewed. Nothing sets [needsSignIn] for that, yet
-         * signing in again is the only route that can trust the new one.
-         */
-        val certificateRefused: Boolean
-            get() = account.lastSyncError?.let { SyncFailure.of(it).kind == SyncFailure.Kind.CERTIFICATE } == true
-
-        /** Whether "Sign in again" is the way forward for this account. */
-        val offersSignIn: Boolean get() = needsSignIn || certificateRefused
-    }
+    )
 
     /**
      * Accounts whose removal has started and not finished.

app/src/full/java/de/jeanlucmakiola/agendula/ui/accounts/add/AddAccountScreen.kt

@@ -440,19 +440,11 @@ private fun ColumnScope.StepActions(state: AddAccountUiState, viewModel: AddAcco
             ) { Text(stringResource(R.string.add_account_change_address)) }
         }
 
-        // The safe answer stays visible, as it does for plain http: back is
-        // not something every user knows to look for on a security prompt.
-        is AddAccountStep.TrustCertificate -> {
-            PrimaryAction(
-                label = stringResource(R.string.add_account_certificate_trust),
-                enabled = true,
-                onClick = viewModel::onCertificateTrusted,
-            )
-            TextButton(
-                onClick = { viewModel.onBackWithin() },
-                modifier = Modifier.fillMaxWidth(),
-            ) { Text(stringResource(R.string.add_account_certificate_decline)) }
-        }
+        is AddAccountStep.TrustCertificate -> PrimaryAction(
+            label = stringResource(R.string.add_account_certificate_trust),
+            enabled = true,
+            onClick = viewModel::onCertificateTrusted,
+        )
 
         is AddAccountStep.WaitingForBrowser -> {
             // A failed flow is retried by default. With no browser a retry

app/src/full/java/de/jeanlucmakiola/agendula/ui/accounts/add/AddAccountViewModel.kt

@@ -8,7 +8,6 @@ import de.jeanlucmakiola.agendula.data.sync.AccountCreator
 import de.jeanlucmakiola.agendula.data.sync.AccountRepository
 import de.jeanlucmakiola.agendula.data.sync.CalDavGateway
 import de.jeanlucmakiola.agendula.data.sync.LoginFlowRecord
-import de.jeanlucmakiola.agendula.data.sync.ServerTrustKeys
 import de.jeanlucmakiola.caldav.CalDavDiscovery
 import de.jeanlucmakiola.caldav.CalDavProvider
 import de.jeanlucmakiola.caldav.NextcloudLoginFlow
@@ -18,8 +17,6 @@ import de.jeanlucmakiola.caldav.ServerTrust
 import de.jeanlucmakiola.caldav.ServiceDiscovery
 import de.jeanlucmakiola.caldav.TaskCollection
 import de.jeanlucmakiola.agendula.data.di.ApplicationScope
-import kotlinx.coroutines.CancellationException
-import kotlinx.coroutines.CompletableDeferred
 import kotlinx.coroutines.CoroutineScope
 import kotlinx.coroutines.NonCancellable
 import kotlinx.coroutines.withContext
@@ -340,33 +337,26 @@ class AddAccountViewModel @Inject constructor(
 
     /** What the user agreed to trust in this flow, handed to every gateway call. */
     private var trust: ServerTrust
-        get() = TRUST_KEYS.read(handle)
-        set(value) = TRUST_KEYS.write(handle, value)
+        get() = ServerTrust(
+            pinnedCertificates = handle.get<ArrayList<String>>(KEY_TRUSTED_CERTIFICATES).orEmpty().toSet(),
+            allowCleartext = handle[KEY_CLEARTEXT] ?: false,
+        )
+        set(value) {
+            handle[KEY_TRUSTED_CERTIFICATES] = ArrayList(value.pinnedCertificates)
+            handle[KEY_CLEARTEXT] = value.allowCleartext
+        }
 
-    /**
-     * The address [trust] was given for. Pins and plain http both belong to it,
-     * so submitting any other address starts from nothing.
-     */
-    private var trustAddress: String?
-        get() = handle[KEY_TRUST_ADDRESS]
-        set(value) { handle[KEY_TRUST_ADDRESS] = value }
-
-    /**
-     * The pins in [trust] came from the account being signed in to, not from
-     * this flow. The first certificate trusted now replaces them, so a renewed
-     * certificate retires the one it renews instead of joining it.
-     */
-    private var inheritedPins: Boolean
-        get() = handle[KEY_INHERITED_PINS] ?: false
-        set(value) { handle[KEY_INHERITED_PINS] = value }
+    /** The address plain http was accepted for; another address asks again. */
+    private var cleartextAddress: String?
+        get() = handle[KEY_CLEARTEXT_ADDRESS]
+        set(value) { handle[KEY_CLEARTEXT_ADDRESS] = value }
 
     /** Loading a re-authenticated account's stored trust, which discovery waits for. */
     private var trustLoad: Job? = null
 
-    /** The open [AddAccountStep.TrustCertificate] question, and where declining it goes. */
-    private class TrustPrompt(val answer: CompletableDeferred<Unit>, val back: AddAccountStep)
-
-    private var trustPrompt: TrustPrompt? = null
+    /** What [AddAccountStep.TrustCertificate] re-runs once trusted, and where Back goes. */
+    private var trustRetry: (() -> Unit)? = null
+    private var trustDeclined: AddAccountStep? = null
 
     /** Which hosts asked for credentials, for the cross-domain diagnostic. */
     private var hostsNeedingAuth: List<String>
@@ -471,8 +461,7 @@ class AddAccountViewModel @Inject constructor(
         trustLoad = viewModelScope.launch {
             val stored = runCatching { repository.trustFor(accountId) }.getOrDefault(ServerTrust.DEFAULT)
             trust = stored
-            trustAddress = address
-            inheritedPins = stored.pinnedCertificates.isNotEmpty()
+            if (stored.allowCleartext) cleartextAddress = address
         }
     }
 
@@ -532,6 +521,9 @@ class AddAccountViewModel @Inject constructor(
         updateState { it.copy(originMismatch = null) }
         typedInput = input
         addressInput = input
+        trustRetry = null
+        trustDeclined = null
+        if (input != cleartextAddress) trust = trust.copy(allowCleartext = false)
 
         val quirk = ServerQuirk.forInput(input)
         if (quirk?.isFatal == true) {
@@ -555,18 +547,8 @@ class AddAccountViewModel @Inject constructor(
     private fun discoverAddress() {
         working(AddAccountMessage.Progress.Discovering)
         viewModelScope.launch {
-            // ⚠️ After the stored trust has landed, not before: reset first and
-            // the load would put the old address's trust back on top.
             trustLoad?.join()
-            if (typedInput != trustAddress) {
-                trust = ServerTrust.DEFAULT
-                inheritedPins = false
-                trustAddress = typedInput
-            }
-            val outcome = discoverTrusting(addressStep(typedInput), AddAccountMessage.Progress.Discovering) {
-                gateway.discover(typedInput, trust = trust)
-            } ?: return@launch
-            when (outcome) {
+            when (val outcome = gateway.discover(typedInput, trust = trust)) {
                 is CalDavDiscovery.Outcome.Found -> onDiscovered(outcome)
 
                 is CalDavDiscovery.Outcome.NeedsAuthentication -> {
@@ -588,8 +570,8 @@ class AddAccountViewModel @Inject constructor(
                     backToAddressOrServer(outcome.cause)
                 }
 
-                // Asked about inside discoverTrusting, which only returns it declined.
-                is CalDavDiscovery.Outcome.UntrustedCertificate -> Unit
+                is CalDavDiscovery.Outcome.UntrustedCertificate ->
+                    askToTrust(outcome.certificate, back = addressStep(typedInput)) { discoverAddress() }
             }
         }
     }
@@ -602,49 +584,29 @@ class AddAccountViewModel @Inject constructor(
     /** The warning is read and plain http is wanted anyway. */
     fun onCleartextConfirmed() {
         if (current.step !is AddAccountStep.ConfirmCleartext) return
+        cleartextAddress = typedInput
         trust = trust.copy(allowCleartext = true)
-        trustAddress = typedInput
         discoverAddress()
     }
 
     /** The fingerprint is checked; pin it and re-run what it stopped. */
     fun onCertificateTrusted() {
         val step = current.step as? AddAccountStep.TrustCertificate ?: return
-        val prompt = trustPrompt ?: return
-        trustPrompt = null
-        val fingerprint = step.certificate.fingerprint
-        trust = if (inheritedPins) {
-            trust.copy(pinnedCertificates = setOf(fingerprint.lowercase()))
-        } else {
-            trust.withCertificate(fingerprint)
-        }
-        inheritedPins = false
-        prompt.answer.complete(Unit)
+        val retry = trustRetry ?: return
+        trust = trust.withCertificate(step.certificate.fingerprint)
+        trustRetry = null
+        trustDeclined = null
+        retry()
     }
 
-    /**
-     * Runs [discover], and on a certificate the user could trust asks about it
-     * and runs it again once they have. Null when they declined; the step is
-     * then already [back].
-     */
-    private suspend fun discoverTrusting(
-        back: AddAccountStep,
-        progress: AddAccountMessage.Progress,
-        discover: suspend () -> CalDavDiscovery.Outcome,
-    ): CalDavDiscovery.Outcome? {
-        while (true) {
-            val outcome = discover()
-            if (outcome !is CalDavDiscovery.Outcome.UntrustedCertificate) return outcome
-            val prompt = TrustPrompt(CompletableDeferred(), back)
-            trustPrompt = prompt
-            updateState { it.copy(step = AddAccountStep.TrustCertificate(outcome.certificate), openInBrowser = null) }
-            try {
-                prompt.answer.await()
-            } catch (_: CancellationException) {
-                return null
-            }
-            working(progress)
+    private fun askToTrust(certificate: ServerCertificate, back: AddAccountStep, retry: () -> Unit) {
+        // Already pinned and still refused: asking again could only loop.
+        if (certificate.fingerprint in trust.pinnedCertificates) {
+            return backToAddress(CalDavDiscovery.Outcome.Cause.TLS)
         }
+        trustRetry = retry
+        trustDeclined = back
+        updateState { it.copy(step = AddAccountStep.TrustCertificate(certificate), openInBrowser = null) }
     }
 
     /**
@@ -701,11 +663,7 @@ class AddAccountViewModel @Inject constructor(
             val root = serverRoot
                 ?: return@launch backToAddress(CalDavDiscovery.Outcome.Cause.NOT_AN_ADDRESS)
             val credentials = CalDavGateway.Credentials(username, appPassword, root)
-            val outcome = discoverTrusting(
-                AddAccountStep.EnterCredentials(username = username),
-                AddAccountMessage.Progress.SigningIn,
-            ) { gateway.discover(typedInput, credentials, trust) } ?: return@launch
-            when (outcome) {
+            when (val outcome = gateway.discover(typedInput, credentials, trust)) {
                 is CalDavDiscovery.Outcome.Found -> onDiscovered(outcome)
 
                 // A second rejection is the point at which naming the provider's
@@ -727,7 +685,10 @@ class AddAccountViewModel @Inject constructor(
 
                 is CalDavDiscovery.Outcome.NotCalDav -> backToAddressOrServer(outcome.cause)
                 is CalDavDiscovery.Outcome.Failed -> backToAddressOrServer(outcome.cause)
-                is CalDavDiscovery.Outcome.UntrustedCertificate -> Unit
+                is CalDavDiscovery.Outcome.UntrustedCertificate -> askToTrust(
+                    outcome.certificate,
+                    back = AddAccountStep.EnterCredentials(username = username),
+                ) { discoverWithCredentials() }
             }
         }
     }
@@ -911,17 +872,15 @@ class AddAccountViewModel @Inject constructor(
     /** Discovery with a password the login flow just minted, at the server that issued it. */
     private suspend fun discoverApproved(server: HttpUrl) {
         working(AddAccountMessage.Progress.ReadingLists)
-        val outcome = discoverTrusting(addressStep(typedInput), AddAccountMessage.Progress.ReadingLists) {
-            gateway.discover(
-                // The server the credentials were *issued by*, not the
-                // host the user typed — the two differ in exactly the
-                // host-mismatch case this flow already models, and
-                // getting it wrong burns the one-shot app password.
-                target = server.toString(),
-                credentials = CalDavGateway.Credentials(username, appPassword, server),
-                trust = trust,
-            )
-        } ?: return
+        val outcome = gateway.discover(
+            // The server the credentials were *issued by*, not the
+            // host the user typed — the two differ in exactly the
+            // host-mismatch case this flow already models, and
+            // getting it wrong burns the one-shot app password.
+            target = server.toString(),
+            credentials = CalDavGateway.Credentials(username, appPassword, server),
+            trust = trust,
+        )
         when (outcome) {
             is CalDavDiscovery.Outcome.Found -> onDiscovered(outcome)
             // ⚠️ Forward what actually happened. Reporting every
@@ -950,7 +909,10 @@ class AddAccountViewModel @Inject constructor(
             CalDavDiscovery.Outcome.Unauthenticated ->
                 backToAddress(CalDavDiscovery.Outcome.Cause.SERVER_ERROR)
 
-            is CalDavDiscovery.Outcome.UntrustedCertificate -> Unit
+            is CalDavDiscovery.Outcome.UntrustedCertificate ->
+                askToTrust(outcome.certificate, back = addressStep(typedInput)) {
+                    viewModelScope.launch { discoverApproved(server) }
+                }
         }
     }
 
@@ -1150,10 +1112,10 @@ class AddAccountViewModel @Inject constructor(
         }
 
         is AddAccountStep.TrustCertificate -> {
-            val prompt = trustPrompt
-            trustPrompt = null
-            prompt?.answer?.cancel()
-            updateState { it.copy(step = prompt?.back ?: addressStep(typedInput), fatal = null) }
+            val back = trustDeclined ?: addressStep(typedInput)
+            trustRetry = null
+            trustDeclined = null
+            updateState { it.copy(step = back, fatal = null) }
             true
         }
 
@@ -1201,10 +1163,9 @@ class AddAccountViewModel @Inject constructor(
         trustLoad?.cancel()
         trustLoad = null
         trust = ServerTrust.DEFAULT
-        trustAddress = null
-        inheritedPins = false
-        trustPrompt?.answer?.cancel()
-        trustPrompt = null
+        cleartextAddress = null
+        trustRetry = null
+        trustDeclined = null
         current = AddAccountUiState()
     }
 
@@ -1363,10 +1324,7 @@ class AddAccountViewModel @Inject constructor(
         )
         viewModelScope.launch {
             val credentials = CalDavGateway.Credentials(username, appPassword, root)
-            val outcome = discoverTrusting(addressStep(addressInput), AddAccountMessage.Progress.ReadingLists) {
-                gateway.discover(target, credentials, trust)
-            } ?: return@launch
-            when (outcome) {
+            when (val outcome = gateway.discover(target, credentials, trust)) {
                 is CalDavDiscovery.Outcome.Found -> onDiscovered(outcome, selection)
                 is CalDavDiscovery.Outcome.NotCalDav -> backToAddressOrServer(outcome.cause)
                 is CalDavDiscovery.Outcome.Failed -> backToAddressOrServer(outcome.cause)
@@ -1377,7 +1335,8 @@ class AddAccountViewModel @Inject constructor(
                     credentialsRejected()
                 }
                 CalDavDiscovery.Outcome.Unauthenticated -> credentialsRejected()
-                is CalDavDiscovery.Outcome.UntrustedCertificate -> Unit
+                is CalDavDiscovery.Outcome.UntrustedCertificate ->
+                    askToTrust(outcome.certificate, back = addressStep(addressInput)) { resumeDiscovery(hasSetup) }
             }
         }
     }
@@ -1607,19 +1566,13 @@ class AddAccountViewModel @Inject constructor(
         const val KEY_FALLBACK_EMAIL = "add_account.fallback_email"
         const val KEY_TRUSTED_CERTIFICATES = "add_account.trusted_certificates"
         const val KEY_CLEARTEXT = "add_account.cleartext"
-        const val KEY_TRUST_ADDRESS = "add_account.trust_address"
-        const val KEY_INHERITED_PINS = "add_account.inherited_pins"
-
-        val TRUST_KEYS = ServerTrustKeys(KEY_TRUSTED_CERTIFICATES, KEY_CLEARTEXT)
+        const val KEY_CLEARTEXT_ADDRESS = "add_account.cleartext_address"
 
         /** What sends an email to the server field: no CalDAV on its domain, or nothing answering for it. */
         val NO_SERVER_FOUND = setOf(
             CalDavDiscovery.Outcome.Cause.NOT_A_DAV_SERVER,
             CalDavDiscovery.Outcome.Cause.NO_CALENDAR_SUPPORT,
             CalDavDiscovery.Outcome.Cause.UNREACHABLE,
-            // The domain's own host would not set up TLS: usually its website,
-            // not a CalDAV server, so the server field is the way on.
-            CalDavDiscovery.Outcome.Cause.TLS,
         )
 
         /**

app/src/full/res/xml/network_security_config.xml

@@ -11,9 +11,6 @@
     itself refuses an http:// URL for everyone else. This file cannot express a
     per-account choice, only a per-host one, and the hosts are the user's.
 
-    ⚠️ That makes CalDavHttp the only gate. A client built any other way would
-    speak http silently, so HttpClientConstructionTest fails the build on one.
-
     ⚠️ KNOWN TRADE-OFF, and it is the widest form of this. base-config applies to
     *all* traffic, so any CA in the user store — a corporate MDM profile, a "free
     VPN" app's certificate, one installed during some earlier debugging — can

app/src/main/res/values/strings.xml

@@ -832,7 +832,6 @@
     <string name="add_account_certificate_issuer">Issued by</string>
     <string name="add_account_certificate_expires">Valid until</string>
     <string name="add_account_certificate_trust">Trust this certificate</string>
-    <string name="add_account_certificate_decline">Don\u2019t trust it</string>
 
     <string name="add_account_lists_title">Which lists should sync?</string>
     <string name="add_account_lists_read_only">Read-only</string>

app/src/testFull/java/de/jeanlucmakiola/agendula/data/sync/HttpClientConstructionTest.kt

@@ -1,41 +0,0 @@
-package de.jeanlucmakiola.agendula.data.sync
-
-import com.google.common.truth.Truth.assertThat
-import org.junit.jupiter.api.Test
-import java.io.File
-
-/**
- * `network_security_config.xml` permits cleartext app-wide, and the per-account
- * opt-in is enforced only by the clients `CalDavHttp` builds. Any HTTP client
- * built elsewhere would speak plain http without asking.
- */
-class HttpClientConstructionTest {
-
-    private val forbidden = listOf(
-        "OkHttpClient()",
-        "OkHttpClient.Builder()",
-        "HttpURLConnection",
-        "HttpsURLConnection",
-        ".openConnection(",
-        ".openStream(",
-    )
-
-    @Test
-    fun `every HTTP client is built through CalDavHttp`() {
-        val roots = listOf(File("src"), File("../caldav/src/main"))
-        assertThat(roots.filter { it.isDirectory }).isNotEmpty()
-
-        val offenders = roots
-            .filter { it.isDirectory }
-            .flatMap { root -> root.walk().filter { it.isFile && it.extension in setOf("kt", "java") }.toList() }
-            .filterNot { "/test" in it.invariantSeparatorsPath || "/androidTest" in it.invariantSeparatorsPath }
-            .filterNot { it.name == "CalDavHttp.kt" }
-            .flatMap { file ->
-                file.readLines().mapIndexedNotNull { index, line ->
-                    forbidden.firstOrNull { it in line }?.let { "${file.path}:${index + 1}: $it" }
-                }
-            }
-
-        assertThat(offenders).isEmpty()
-    }
-}

app/src/testFull/java/de/jeanlucmakiola/agendula/ui/accounts/add/AddAccountViewModelTest.kt

@@ -1525,52 +1525,18 @@ class AddAccountViewModelTest {
         }
 
         @Test
-        fun `a certificate trusted for one address is not carried to another`() = runTest(dispatcher) {
+        fun `a certificate already pinned is not asked about again`() = runTest(dispatcher) {
             gateway.discoveryOutcomes += CalDavDiscovery.Outcome.UntrustedCertificate(certificate)
-            gateway.discoveryOutcomes += CalDavDiscovery.Outcome.NeedsAuthentication(emptyList())
-            gateway.loginFlow = null
-            val vm = viewModel()
-            vm.enterAddress("https://a.lan")
-            vm.onAddressSubmitted()
-            advanceUntilIdle()
-            vm.onCertificateTrusted()
-            advanceUntilIdle()
-            assertThat(vm.onBackWithin()).isTrue()
-
-            gateway.discoveryOutcomes += CalDavDiscovery.Outcome.NeedsAuthentication(emptyList())
-            vm.enterAddress("https://b.example.com")
-            vm.onAddressSubmitted()
-            advanceUntilIdle()
-
-            assertThat(gateway.discoveries.last().trust).isEqualTo(ServerTrust.DEFAULT)
-        }
-
-        @Test
-        fun `declining a certificate is offered without the back gesture`() = runTest(dispatcher) {
             gateway.discoveryOutcomes += CalDavDiscovery.Outcome.UntrustedCertificate(certificate)
             val vm = viewModel()
             vm.enterAddress("https://nas.home.arpa")
             vm.onAddressSubmitted()
             advanceUntilIdle()
-
-            assertThat(vm.onBackWithin()).isTrue()
-            advanceUntilIdle()
-            assertThat((vm.state.value.step as AddAccountStep.EnterAddress).input).isEqualTo("https://nas.home.arpa")
-            assertThat(gateway.discoveries).hasSize(1)
-        }
-
-        @Test
-        fun `an email whose domain will not set up TLS asks for the server`() = runTest(dispatcher) {
-            gateway.discoveryOutcomes += CalDavDiscovery.Outcome.Failed(CalDavDiscovery.Outcome.Cause.TLS, "tls")
-            val vm = viewModel()
-            vm.onProviderChosen(ProviderChoice.Service(CalDavProvider.MAILBOX_ORG))
-            vm.onAddressChanged("me@mailbox.org")
-            vm.onAddressSubmitted()
+            vm.onCertificateTrusted()
             advanceUntilIdle()
 
-            val step = vm.state.value.step as AddAccountStep.EnterAddress
-            assertThat(step.manualServer).isTrue()
-            assertThat(step.error).isNull()
+            assertThat((vm.state.value.step as AddAccountStep.EnterAddress).error)
+                .isEqualTo(CalDavDiscovery.Outcome.Cause.TLS)
         }
 
         @Test
@@ -1587,42 +1553,6 @@ class AddAccountViewModelTest {
             assertThat(gateway.discoveries.single().trust).isEqualTo(stored)
         }
 
-        @Test
-        fun `a stored trust that loads late is not applied to an edited address`() = runTest(dispatcher) {
-            creator.storedTrust[9] = ServerTrust(allowCleartext = true)
-            gateway.discoveryOutcomes += CalDavDiscovery.Outcome.Failed(
-                CalDavDiscovery.Outcome.Cause.INSECURE,
-                "http",
-            )
-            val vm = viewModel()
-            vm.startReauthentication(9, "http://nas.local/dav/principals/me/", "me")
-            // Submitted before the stored trust has been read.
-            vm.onAddressChanged("http://other.local")
-            vm.onAddressSubmitted()
-            advanceUntilIdle()
-
-            assertThat(gateway.discoveries.single().trust).isEqualTo(ServerTrust.DEFAULT)
-            assertThat(vm.state.value.step).isEqualTo(AddAccountStep.ConfirmCleartext("http://other.local"))
-        }
-
-        @Test
-        fun `a renewed certificate replaces the one the account trusted`() = runTest(dispatcher) {
-            creator.storedTrust[9] = ServerTrust(pinnedCertificates = setOf("cd".repeat(32)))
-            gateway.discoveryOutcomes += CalDavDiscovery.Outcome.UntrustedCertificate(certificate)
-            gateway.discoveryOutcomes += CalDavDiscovery.Outcome.NeedsAuthentication(emptyList())
-            gateway.loginFlow = null
-            val vm = viewModel()
-            vm.startReauthentication(9, "https://nas.home.arpa/dav/principals/me/", "me")
-            advanceUntilIdle()
-            vm.onAddressSubmitted()
-            advanceUntilIdle()
-            vm.onCertificateTrusted()
-            advanceUntilIdle()
-
-            assertThat(gateway.discoveries.last().trust.pinnedCertificates)
-                .containsExactly(certificate.fingerprint)
-        }
-
         @Test
         fun `starting over forgets what was trusted`() = runTest(dispatcher) {
             gateway.discoveryOutcomes += CalDavDiscovery.Outcome.UntrustedCertificate(certificate)

caldav/src/main/kotlin/de/jeanlucmakiola/caldav/CalDavDiscovery.kt

@@ -157,11 +157,6 @@ class CalDavDiscovery(
      * A 401 stops the walk immediately: it means we found a DAV server and simply
      * have no credentials for it, and continuing down the ladder would replace a
      * precise "sign in" with a vague "nothing found".
-     *
-     * An untrusted certificate does not: a lower-priority SRV target may answer
-     * with one the phone trusts, so it is only offered once the ladder is spent.
-     * On a guessed host — the email's own domain, with no SRV record — it is not
-     * offered at all, since that host is most likely the domain's website.
      */
     fun discover(input: String): Outcome {
         ServiceDiscovery.asBaseUrl(input)?.let { typed ->
@@ -179,31 +174,17 @@ class CalDavDiscovery(
         }
 
         var lastFailure: Outcome = Outcome.Failed(Outcome.Cause.UNREACHABLE, "no candidate answered")
-        var untrusted: Outcome.UntrustedCertificate? = null
-        // Same host, same certificate: the remaining rungs on it would only fail again.
-        val refusedOrigins = mutableSetOf<String>()
         for (candidate in candidates) {
-            if (candidate.url.originKey() in refusedOrigins) continue
             when (val outcome = probe(candidate.url)) {
                 is Outcome.Found,
                 is Outcome.NeedsAuthentication,
                 Outcome.Unauthenticated,
+                is Outcome.UntrustedCertificate,
                 -> return outcome
-                is Outcome.UntrustedCertificate -> {
-                    refusedOrigins += candidate.url.originKey()
-                    if (candidate.guessed) {
-                        lastFailure = Outcome.Failed(
-                            Outcome.Cause.TLS,
-                            "untrusted certificate on guessed host ${candidate.url.host}",
-                        )
-                    } else if (untrusted == null) {
-                        untrusted = outcome
-                    }
-                }
                 else -> lastFailure = outcome
             }
         }
-        return untrusted ?: lastFailure
+        return lastFailure
     }
 
     internal fun probe(base: HttpUrl): Outcome {
@@ -373,8 +354,6 @@ class CalDavDiscovery(
         untrustedCertificateIn(error)?.let { Outcome.UntrustedCertificate(ServerCertificate.of(it)) }
             ?: Outcome.Failed(causeOf(error), error.message ?: error.toString())
 
-    private fun HttpUrl.originKey() = "$scheme://$host:$port"
-
     private fun isUnauthorized(error: Throwable): Boolean =
         error is at.bitfire.dav4jvm.exception.UnauthorizedException
 

caldav/src/main/kotlin/de/jeanlucmakiola/caldav/CalDavHttp.kt

@@ -9,6 +9,7 @@ import okhttp3.Response
 import okhttp3.ResponseBody.Companion.asResponseBody
 import okio.GzipSource
 import okio.buffer
+import java.util.concurrent.ConcurrentHashMap
 import java.util.concurrent.TimeUnit
 import javax.net.ssl.SSLContext
 import javax.net.ssl.SSLSocketFactory
@@ -137,8 +138,7 @@ object CalDavHttp {
             )
         }
 
-    private class Tls(pins: Set<String>) {
-        private val trust = ServerTrust(pinnedCertificates = pins)
+    private class Tls(trust: ServerTrust) {
         val trustManager = PinningTrustManager(trust)
         val socketFactory: SSLSocketFactory = SSLContext.getInstance("TLS")
             .apply { init(null, arrayOf(trustManager), null) }
@@ -147,21 +147,13 @@ object CalDavHttp {
     }
 
     /**
-     * One socket factory per distinct pin set, because OkHttp only pools a
+     * One socket factory per distinct trust, because OkHttp only pools a
      * connection between clients whose factory and verifier are the same
      * instances — a fresh one per call would reopen TLS for every client.
-     * Keyed on the pins alone (cleartext has no bearing on TLS), and bounded,
-     * since every abandoned setup attempt leaves a pin set behind.
      */
-    private val tlsCache = object : LinkedHashMap<Set<String>, Tls>(TLS_CACHE_SIZE, 0.75f, true) {
-        override fun removeEldestEntry(eldest: MutableMap.MutableEntry<Set<String>, Tls>) = size > TLS_CACHE_SIZE
-    }
+    private val tlsCache = ConcurrentHashMap<ServerTrust, Tls>()
 
-    private fun tlsFor(trust: ServerTrust): Tls = synchronized(tlsCache) {
-        tlsCache.getOrPut(trust.pinnedCertificates) { Tls(trust.pinnedCertificates) }
-    }
-
-    private const val TLS_CACHE_SIZE = 16
+    private fun tlsFor(trust: ServerTrust): Tls = tlsCache.getOrPut(trust) { Tls(trust) }
 
     /**
      * The two headers that decide whether ETags are usable and whether our bytes

caldav/src/main/kotlin/de/jeanlucmakiola/caldav/ServiceDiscovery.kt

@@ -51,14 +51,8 @@ object ServiceDiscovery {
     private const val SRV_SERVICE = "_caldavs._tcp"
     private const val WELL_KNOWN = "/.well-known/caldav"
 
-    /**
-     * A URL to probe, and where it came from — the "why" a failure report needs.
-     *
-     * @property guessed the host is the email's domain taken on spec, with no
-     *   SRV record naming it. Usually that is the domain's website, so its
-     *   certificate is not one worth asking the user to trust.
-     */
-    data class Candidate(val url: HttpUrl, val origin: String, val guessed: Boolean = false)
+    /** A URL to probe, and where it came from — the "why" a failure report needs. */
+    data class Candidate(val url: HttpUrl, val origin: String)
 
     /**
      * Hosts whose SRV record leads somewhere that is not a DAV server.
@@ -155,15 +149,16 @@ object ServiceDiscovery {
             // explicit, which is the whole point of Posteo's 8443.
             val port = if (record.port == 443) "" else ":${record.port}"
             "https://$host$port" to "SRV $host$port"
+        }.ifEmpty {
+            listOf("https://$domain" to "domain as typed")
         }
-        val guessed = origins.isEmpty()
 
-        for ((origin, label) in origins.ifEmpty { listOf("https://$domain" to "domain as typed") }) {
+        for ((origin, label) in origins) {
             if (txtPath != null) {
-                add(candidates, origin, txtPath, "$label + TXT path=$txtPath", guessed)
+                add(candidates, origin, txtPath, "$label + TXT path=$txtPath")
             }
-            add(candidates, origin, WELL_KNOWN, "$label + .well-known", guessed)
-            add(candidates, origin, "/", "$label + root", guessed)
+            add(candidates, origin, WELL_KNOWN, "$label + .well-known")
+            add(candidates, origin, "/", "$label + root")
         }
         return candidates
     }
@@ -186,15 +181,9 @@ object ServiceDiscovery {
         return url.topPrivateDomain() ?: url.host
     }
 
-    private fun add(
-        into: MutableList<Candidate>,
-        origin: String,
-        path: String,
-        label: String,
-        guessed: Boolean = false,
-    ) {
+    private fun add(into: MutableList<Candidate>, origin: String, path: String, label: String) {
         val url = (origin.trimEnd('/') + "/" + path.trimStart('/')).toHttpUrlOrNull() ?: return
-        if (into.none { it.url == url }) into += Candidate(url, label, guessed)
+        if (into.none { it.url == url }) into += Candidate(url, label)
     }
 
     /**

caldav/src/test/kotlin/de/jeanlucmakiola/caldav/ServerTrustTest.kt

@@ -12,7 +12,6 @@ import org.junit.Before
 import org.junit.Test
 import java.io.IOException
 import java.net.ConnectException
-import java.net.InetAddress
 import java.net.UnknownServiceException
 import java.security.cert.CertificateException
 import javax.net.ssl.SSLContext
@@ -161,51 +160,4 @@ class ServerTrustTest {
         assertThat(discovery.probe(server.url("/dav/")))
             .isInstanceOf(CalDavDiscovery.Outcome.NeedsAuthentication::class.java)
     }
-
-    @Test
-    fun `an untrusted SRV target does not stop the ladder before one that answers`() {
-        val loopback = InetAddress.getByName("127.0.0.1")
-        fun tlsServer(certificate: HeldCertificate) = MockWebServer().apply {
-            useHttps(HandshakeCertificates.Builder().heldCertificate(certificate).build().sslSocketFactory(), false)
-            start(loopback, 0)
-        }
-        val untrusted = tlsServer(selfSigned("127.0.0.1"))
-        val pinned = selfSigned("127.0.0.1")
-        val answering = tlsServer(pinned).apply {
-            enqueue(options())
-            enqueue(MockResponse().setResponseCode(401))
-        }
-        val dns = object : DnsResolver {
-            override fun lookup(name: String) = DnsRecords(
-                srv = listOf(
-                    SrvRecord(priority = 0, weight = 0, port = untrusted.port, target = "127.0.0.1."),
-                    SrvRecord(priority = 10, weight = 0, port = answering.port, target = "127.0.0.1."),
-                ),
-                txt = emptyList(),
-            )
-        }
-        val trust = ServerTrust.DEFAULT.withCertificate(fingerprintOf(pinned.certificate))
-
-        try {
-            val outcome = CalDavDiscovery(CalDavHttp.anonymous("test", trust), dns).discover("me@127.0.0.1")
-            assertThat(outcome).isInstanceOf(CalDavDiscovery.Outcome.NeedsAuthentication::class.java)
-        } finally {
-            untrusted.shutdown()
-            answering.shutdown()
-        }
-    }
-
-    @Test
-    fun `the email's own domain is a guess, and an SRV target is not`() {
-        assertThat(ServiceDiscovery.candidatesFor("me@example.com").all { it.guessed }).isTrue()
-
-        val dns = object : DnsResolver {
-            override fun lookup(name: String) = DnsRecords(
-                srv = listOf(SrvRecord(priority = 0, weight = 0, port = 443, target = "dav.example.com.")),
-                txt = emptyList(),
-            )
-        }
-        assertThat(ServiceDiscovery.candidatesFor("me@example.com", dns).none { it.guessed }).isTrue()
-        assertThat(ServiceDiscovery.candidatesFor("https://example.com").none { it.guessed }).isTrue()
-    }
 }