diff --git a/.mergify.yml b/.mergify.yml index fb38ab9..6e95b77 100644 --- a/.mergify.yml +++ b/.mergify.yml @@ -6,9 +6,17 @@ # Spec: docs/ci/mergify-integration-spec.md + docs/ci/mergify.yml.proposed # (issues #407 / #408). # Schema: https://docs.mergify.com/configuration/file-format/ -# Verified against the LIVE Mergify docs on 2026-07-06 (queue rules, -# the queue action, priority rules, parallel checks, batches, setup and -# lifecycle pages) — the config format evolves, so this is not from memory. +# Verified against the LIVE Mergify docs on 2026-07-07 (file-format, queue +# rules, priority, merge-queue lifecycle/setup/batches, and the +# merge-protections auto-merge pages) — the config format evolves, so this is +# not from memory. +# 2026-07-07 CHANGE: auto-queueing migrated OFF the `pull_request_rules` +# queue-action path (which no longer auto-queues — a green matching PR just +# reported "Merge queue is ready — use `@Mergifyio queue`" and sat there) ONTO +# `merge_protections_settings.auto_merge_conditions` (see that block below). +# The old `autoqueue`/queue-action auto path is DEPRECATED and "will stop +# working on 2026-07-16" (docs.mergify.com/merge-queue/rules). This changes only +# the TRIGGER; the queue's merge semantics (below) are untouched. # ============================================================================ # # WHAT THIS DOES @@ -31,6 +39,23 @@ # * The single required status check stays "CI passed" — the exact `name:` of the # `ci-passed` job in .github/workflows/ci.yml. NOT "ci-passed". A wrong name means # PRs queue but never merge. +# * IN-PLACE CHECKS, not speculative draft-PR checks. GitHub's strict +# `required_status_checks` ruleset (require-branches-up-to-date) rejects +# speculative checks outright — Mergify surfaced this as a "Configuration not +# compatible with `required_status_checks` ruleset rule" check on #422. The fix +# (per Mergify: docs.mergify.com/merge-queue/rules) is to make Mergify validate +# each PR IN PLACE, on the real PR branch, which requires ALL THREE of: +# (a) `merge_queue.max_parallel_checks: 1` (below), +# (b) every `queue_rules[].batch_size: 1` (below), and +# (c) `queue_rules.default.queue_conditions` IDENTICAL (same conditions, same +# order) to `queue_rules.default.merge_conditions` — i.e. no "two-step CI" +# where the conditions to ENTER the queue differ from the conditions to +# MERGE. Mergify runs three condition sets, sequentially: +# `merge_protections_settings.auto_merge_conditions` (TRIGGERS auto-queueing) +# → `queue_conditions` (validates a PR's queue ENTRY) → `merge_conditions` +# (validates the MERGE). Omitting `queue_conditions` — as this config first +# did — reads as a two-step-CI mismatch and re-trips the incompatibility +# check, so we keep all three lists identical. Do not let them drift apart. # --------------------------------------------------------------------------- # queue_rules — how a queued PR is validated and merged. @@ -38,16 +63,35 @@ queue_rules: - name: default # Final merge gate. Merge ONLY when the single required context is green (the exact - # same check branch protection requires), the PR is not a draft, has no merge - # conflicts, and is not flagged `broken`. NOTE: branch protection requires 0 + # same check branch protection requires), the PR targets `main`, is not a draft, has + # no merge conflicts, and is not flagged `broken`. NOTE: branch protection requires 0 # approvals here (the active repository ruleset sets required_approving_review_count # = 0), so there is deliberately NO `#approved-reviews-by` condition — adding one # would wedge the solo-maintainer flow, where nobody can approve their own PR. - merge_conditions: - - check-success = CI passed + # + # IN-PLACE CHECKS: `queue_conditions` (what a PR must satisfy to ENTER/stay in the + # queue) MUST be IDENTICAL (same conditions, same order) to `merge_conditions` (what + # it must satisfy to MERGE) below. When those two lists match — plus batch_size 1 and + # max_parallel_checks 1 — Mergify validates each PR IN PLACE on the real PR branch + # instead of running speculative draft-PR checks, which is what GitHub's strict + # `required_status_checks` ruleset (require-branches-up-to-date) demands. Omitting + # `queue_conditions` (as this config originally did) is treated as a "two-step CI" + # mismatch and Mergify flags the ruleset as incompatible. Keep the three lists here — + # `queue_conditions`, `merge_conditions`, and + # `merge_protections_settings.auto_merge_conditions` — all identical; if any diverge, + # Mergify's ruleset-compatibility check fails again. + queue_conditions: + - base = main - -draft - -conflict - label != broken + - check-success = CI passed + merge_conditions: + - base = main + - -draft + - -conflict + - label != broken + - check-success = CI passed # SERIAL: exactly one PR per merge. No batching (Phase 2 / #410). One merge commit # per PR, which is what lets require-up-to-date stay literally ON. batch_size: 1 @@ -125,22 +169,43 @@ priority_rules: priority: 1000 # --------------------------------------------------------------------------- -# pull_request_rules — WHICH PRs enter the queue. -# The `queue` action is what actually ADDS a PR to the merge queue: per -# docs.mergify.com/merge-queue/lifecycle, queue_conditions alone do NOT auto-queue a PR — -# a queue action (or an `@mergifyio queue` command / auto_merge) is required, otherwise the -# "Mergify Merge Queue" check sits permanently pending. A PR is queued as soon as it is green -# on "CI passed", targets `main`, is not a draft, has no conflicts, and is not flagged -# `broken`. `broken` / `draft` PRs are never queued. +# merge_protections_settings — WHICH PRs are AUTOMATICALLY added to the queue. +# +# This REPLACES the old `pull_request_rules` `queue` action. That action no longer +# auto-queues in current Mergify: a green, matching PR just reported "Merge queue is +# ready — use `@Mergifyio queue`" and sat there forever (never merged). Automatic +# queueing now lives in `auto_merge_conditions` under `merge_protections_settings`. The +# old `queue_rules[].autoqueue` field (and the queue-action auto path) is DEPRECATED and +# "will stop working on 2026-07-16. Use `auto_merge_conditions` in +# `merge_protections_settings` instead" (docs.mergify.com/merge-queue/rules). +# +# `auto_merge_conditions` accepts `true` (auto-queue every mergeable PR) or, as here, +# "a list of conditions to restrict the audience" (docs.mergify.com/configuration/ +# file-format). We give the SAME set the old queue action used, so EXACTLY the same PRs +# auto-queue: green on "CI passed", targeting `main`, not a draft, no conflicts, not +# `broken`. +# +# WHY THIS PRESERVES require-up-to-date: this changes only the TRIGGER (manual → +# automatic). It does NOT touch how the queue validates or merges — batch_size 1, +# merge_method merge, and max_parallel_checks 1 above are unchanged — and those are the +# settings that interact with require-up-to-date (only BATCHING, batch_size > 1, forces +# that checkbox OFF; see the invariants header + docs.mergify.com/merge-queue/batches). +# When a merge queue is configured, a matched PR is auto-QUEUED, not merged directly: +# "Every PR is auto-queued. The merge queue then handles routing and merging" +# (docs.mergify.com/merge-protections/auto-merge) — so it still goes through the serial +# queue, gets updated onto the latest `main`, re-runs CI, and merges on the real green +# "CI passed". Mergify also auto-reads GitHub branch protection (the required "CI passed" +# check + require-up-to-date) and injects it as a merge condition, so the GitHub gate is +# enforced on top of queue_rules.merge_conditions. +# +# This list MUST stay IDENTICAL (same conditions, same order) to +# `queue_rules.default.queue_conditions` and `.merge_conditions` above — see the note +# there and the IN-PLACE CHECKS hard invariant at the top of this file. # --------------------------------------------------------------------------- -pull_request_rules: - - name: Queue green, non-draft, non-conflicting PRs targeting main - conditions: - - base = main - - -draft - - -conflict - - label != broken - - check-success = CI passed - actions: - queue: - name: default +merge_protections_settings: + auto_merge_conditions: + - base = main + - -draft + - -conflict + - label != broken + - check-success = CI passed diff --git a/app/src/androidTest/kotlin/org/libremail/data/local/DatabaseEncryptionProbeInstrumentedTest.kt b/app/src/androidTest/kotlin/org/libremail/data/local/DatabaseEncryptionProbeInstrumentedTest.kt new file mode 100644 index 0000000..955049a --- /dev/null +++ b/app/src/androidTest/kotlin/org/libremail/data/local/DatabaseEncryptionProbeInstrumentedTest.kt @@ -0,0 +1,64 @@ +// SPDX-License-Identifier: GPL-3.0-or-later +package org.libremail.data.local + +import android.content.Context +import androidx.test.core.app.ApplicationProvider +import androidx.test.ext.junit.runners.AndroidJUnit4 +import org.junit.After +import org.junit.Assert.assertFalse +import org.junit.Before +import org.junit.Test +import org.junit.runner.RunWith +import java.io.File + +/** + * On-device cover for the issue-#359 keyed-open probe ([DatabaseEncryption.probeKeyedOpen]). The probe + * is the fix for gap 2: it reaches the REAL `SQLiteConnection.nativeOpen` — the exact #359 crash site — + * from inside `DatabaseProvisioner`'s fail-closed handler, so an open-time `UnsatisfiedLinkError` on an + * incompatible device is caught and converted to `CacheEncryptionUnavailableException` before Room's + * later deferred open can crash on it uncaught. + * + * This runs the real SQLCipher native path (mocked out of the JVM unit tests), asserting two things a + * device is needed for: on a compatible device the probe opens+closes the keyed database WITHOUT + * throwing, and — on every device — it leaves no file behind (it opens a throwaway sibling, never the + * real cache, and cleans up its sidecars). All data is synthetic; nothing here is PII. + */ +@RunWith(AndroidJUnit4::class) +class DatabaseEncryptionProbeInstrumentedTest { + + private val context = ApplicationProvider.getApplicationContext() + private val cacheName = "probe_test_cache.db" + private val cacheFile: File get() = context.getDatabasePath(cacheName) + + // 64 hex chars == a 32-byte SQLCipher passphrase, matching DatabaseKeyStore's format. + private val passphrase = "0123456789abcdef".repeat(4) + + @Before + @After + fun clean() { + cacheFile.parentFile + ?.listFiles { f -> f.name.startsWith(cacheName) } + ?.forEach { it.delete() } + } + + @Test + fun probeKeyedOpen_reachesNativeOpen_thenLeavesNoFileBehind() { + // The provisioner loads the native library immediately before probing (probeKeyedOpen's + // precondition, kept out of the probe so the load happens exactly once per open); mirror that here. + DatabaseEncryption.ensureNativeLibraryLoaded() + // On a compatible device this returns normally (keyed nativeOpen succeeds); on an incompatible one + // it throws UnsatisfiedLinkError here — the exact #359 signature the provisioner now fails closed + // on. Either way, no probe file may survive. + DatabaseEncryption.probeKeyedOpen(cacheFile, passphrase) + + val dir = cacheFile.parentFile!! + listOf("", "-wal", "-shm", "-journal").forEach { suffix -> + assertFalse( + "probe left a '$cacheName.openprobe$suffix' file behind", + File(dir, "$cacheName.openprobe$suffix").exists(), + ) + } + // The probe uses a throwaway sibling, so it must never create the real cache file. + assertFalse("probe must not create the real cache file", cacheFile.exists()) + } +} diff --git a/app/src/androidTest/kotlin/org/libremail/data/repository/AccountAddCredentialOrderingInstrumentedTest.kt b/app/src/androidTest/kotlin/org/libremail/data/repository/AccountAddCredentialOrderingInstrumentedTest.kt new file mode 100644 index 0000000..418d6c7 --- /dev/null +++ b/app/src/androidTest/kotlin/org/libremail/data/repository/AccountAddCredentialOrderingInstrumentedTest.kt @@ -0,0 +1,144 @@ +// SPDX-License-Identifier: GPL-3.0-or-later +package org.libremail.data.repository + +import android.content.Context +import androidx.room.Room +import androidx.test.core.app.ApplicationProvider +import androidx.test.ext.junit.runners.AndroidJUnit4 +import io.mockk.coEvery +import io.mockk.mockk +import io.mockk.unmockkAll +import kotlinx.coroutines.CompletableDeferred +import kotlinx.coroutines.Dispatchers +import kotlinx.coroutines.launch +import kotlinx.coroutines.runBlocking +import kotlinx.coroutines.withTimeout +import org.junit.After +import org.junit.Assert.assertEquals +import org.junit.Before +import org.junit.Test +import org.junit.runner.RunWith +import org.libremail.data.attachment.AttachmentUriGrants +import org.libremail.data.local.AccountDatabase +import org.libremail.data.local.dao.BackfillProgressDao +import org.libremail.data.local.dao.DraftDao +import org.libremail.data.local.dao.FolderDao +import org.libremail.data.local.dao.MessageDao +import org.libremail.data.security.CredentialStore +import org.libremail.data.security.KeystoreCrypto +import org.libremail.data.settings.AccountSettingsRepository +import org.libremail.data.sync.SyncScheduler +import org.libremail.domain.model.Account +import org.libremail.domain.model.AuthType +import org.libremail.domain.model.MailSecurity +import org.libremail.domain.model.ServerConfig +import org.libremail.mail.FetchedFolder +import org.libremail.mail.ImapClient +import org.libremail.notifications.MailNotifier + +/** + * On-device proof of the #403 fix against real SQLite and the real Keystore-backed [CredentialStore]: + * when [AccountRepositoryImpl] adds an account, its credential must be resolvable the instant the new + * account row becomes observable — the exact moment the push watchers (LibreMailApplication's collector + * and [org.libremail.push.IdleService.reconcileWatchers], both keyed on the *accounts* table) react. + * + * The JVM `AccountRepositoryImplTest` pins the call ORDER with `coVerifyOrder`; this drives the same + * production code against a real in-memory [AccountDatabase] (real `accountDao` + real `credentialDao` + * via a real [CredentialStore]/[KeystoreCrypto]) so the transaction-commit ordering — not just the call + * ordering — is exercised. A background collector reads the credential the moment the account first + * appears, reproducing the reactive watcher; with the secret committed before the row it always + * resolves. Non-DB collaborators (IMAP, scheduler, notifier, settings) are mocked the same way + * `WorkerCacheLockDeferralInstrumentedTest` fakes its non-framework collaborators — never a framework + * `Context`, which is the real application context. + */ +@RunWith(AndroidJUnit4::class) +class AccountAddCredentialOrderingInstrumentedTest { + + private val context: Context = ApplicationProvider.getApplicationContext() + + private lateinit var db: AccountDatabase + private lateinit var credentialStore: CredentialStore + private lateinit var imapClient: ImapClient + private lateinit var repository: AccountRepositoryImpl + + @Before + fun setUp() { + db = Room.inMemoryDatabaseBuilder(context, AccountDatabase::class.java).build() + credentialStore = CredentialStore(KeystoreCrypto(), db.credentialDao()) + imapClient = mockk() + coEvery { imapClient.listFolders(any()) } returns listOf( + FetchedFolder("INBOX", "INBOX", emptyList(), selectable = true), + ) + repository = AccountRepositoryImpl( + context = context, + accountDao = db.accountDao(), + messageDao = mockk(relaxed = true), + folderDao = mockk(relaxed = true), + backfillProgressDao = mockk(relaxed = true), + draftDao = mockk(relaxed = true), + credentialStore = credentialStore, + imapClient = imapClient, + syncScheduler = mockk(relaxed = true), + accountSettingsRepository = mockk(relaxed = true), + mailNotifier = mockk(relaxed = true), + attachmentUriGrants = mockk(relaxed = true), + ) + } + + @After + fun tearDown() { + db.close() + unmockkAll() + } + + @Test + fun newAccountRowIsObservableOnlyAfterItsCredentialIsResolvable() = runBlocking { + val account = imapAccount() + + // A reactive watcher: read the stored secret the moment the new account row first appears in the + // observed accounts list — exactly what IdleService.reconcileWatchers does before opening IDLE. + val credentialAtFirstSight = CompletableDeferred() + val observer = launch(Dispatchers.IO) { + repository.observeAccounts().collect { accounts -> + if (accounts.any { it.id == account.id } && !credentialAtFirstSight.isCompleted) { + credentialAtFirstSight.complete(credentialStore.loadSecret(account.id)) + } + } + } + + repository.addImapAccount(account, PASSWORD).getOrThrow() + + assertEquals( + "the credential must be resolvable the instant the account row becomes observable (#403)", + PASSWORD, + withTimeout(TIMEOUT_MS) { credentialAtFirstSight.await() }, + ) + observer.cancel() + } + + @Test + fun addImapAccountLeavesTheCredentialResolvableForTheStoredAccount() = runBlocking { + val account = imapAccount() + + repository.addImapAccount(account, PASSWORD).getOrThrow() + + // Durability backstop: the account is persisted AND its secret round-trips through the real + // Keystore-sealed store, so any later push (re)start resolves it rather than hitting a hard miss. + assertEquals(PASSWORD, credentialStore.loadSecret(account.id)) + assertEquals(account.id, db.accountDao().getById(account.id)?.id) + } + + private fun imapAccount(id: String = "imap:ada@example.org") = Account( + id = id, + email = "ada@example.org", + displayName = "Ada", + authType = AuthType.PASSWORD_IMAP, + imap = ServerConfig("imap.example.org", 993, MailSecurity.SSL_TLS), + smtp = ServerConfig("smtp.example.org", 587, MailSecurity.STARTTLS), + ) + + private companion object { + const val PASSWORD = "app-password" + const val TIMEOUT_MS = 5_000L + } +} diff --git a/app/src/androidTest/kotlin/org/libremail/ui/onboarding/OnboardingFlowTest.kt b/app/src/androidTest/kotlin/org/libremail/ui/onboarding/OnboardingFlowTest.kt index 4cf6d67..52eb3a8 100644 --- a/app/src/androidTest/kotlin/org/libremail/ui/onboarding/OnboardingFlowTest.kt +++ b/app/src/androidTest/kotlin/org/libremail/ui/onboarding/OnboardingFlowTest.kt @@ -134,6 +134,20 @@ class OnboardingFlowTest { navController.navigate(Routes.onboardingAppPassword(provider.key)) }, onManualSetup = {}, + onPickOutlook = { navController.navigate(Routes.ONBOARDING_OUTLOOK_IMAP) }, + viewModel = viewModel, + ) + } + composable(Routes.ONBOARDING_OUTLOOK_IMAP) { + val viewModel = remember { AccountSetupViewModel(outlookAuthManager, accountRepo) } + OutlookImapNoticeScreen( + onBack = { navController.popBackStack() }, + onAccountAdded = { id -> + onboarding.onAccountAdded(id) + navController.navigate(Routes.ONBOARDING_ADD_ANOTHER) { + popUpTo(Routes.ONBOARDING_PICKER) + } + }, viewModel = viewModel, ) } @@ -252,6 +266,23 @@ class OnboardingFlowTest { composeTestRule.onNodeWithText("E2E first message").assertIsDisplayed() } + @Test + fun outlookPick_showsImapNoticeBeforeAuth() { + setOnboardingContent(FakeAccountRepository(), FakeMailRepository()) + + // Welcome → picker → tap Outlook. + composeTestRule.onNodeWithText(string(R.string.onboarding_add_account)).performClick() + waitForText(string(R.string.account_setup_outlook)) + composeTestRule.onNodeWithText(string(R.string.account_setup_outlook)).performClick() + + // Picking Outlook lands on the IMAP-enablement notice BEFORE any OAuth browser opens (#411): + // the interstitial's question is shown and its bottom "Sign in" button (which would continue + // the existing Outlook auth flow) is present. + waitForText(string(R.string.outlook_imap_question)) + composeTestRule.onNodeWithText(string(R.string.outlook_imap_question)).assertIsDisplayed() + composeTestRule.onNodeWithText(string(R.string.outlook_imap_sign_in)).performScrollTo().assertIsDisplayed() + } + @Test fun yahooSetup_hasNoTwoFactorHelpLink() { setOnboardingContent(FakeAccountRepository(), FakeMailRepository()) diff --git a/app/src/androidTest/kotlin/org/libremail/ui/onboarding/OutlookImapNoticeScreenTest.kt b/app/src/androidTest/kotlin/org/libremail/ui/onboarding/OutlookImapNoticeScreenTest.kt new file mode 100644 index 0000000..6b2417e --- /dev/null +++ b/app/src/androidTest/kotlin/org/libremail/ui/onboarding/OutlookImapNoticeScreenTest.kt @@ -0,0 +1,121 @@ +// SPDX-License-Identifier: GPL-3.0-or-later +package org.libremail.ui.onboarding + +import android.app.Activity +import android.app.Instrumentation +import android.content.Context +import android.content.Intent +import androidx.activity.ComponentActivity +import androidx.compose.ui.test.assertIsDisplayed +import androidx.compose.ui.test.junit4.createAndroidComposeRule +import androidx.compose.ui.test.onNodeWithText +import androidx.compose.ui.test.performClick +import androidx.compose.ui.test.performScrollTo +import androidx.test.espresso.intent.Intents +import androidx.test.espresso.intent.matcher.IntentMatchers.hasAction +import androidx.test.espresso.intent.matcher.IntentMatchers.hasComponent +import androidx.test.espresso.intent.matcher.IntentMatchers.hasData +import androidx.test.espresso.intent.matcher.UriMatchers.hasHost +import androidx.test.ext.junit.runners.AndroidJUnit4 +import androidx.test.platform.app.InstrumentationRegistry +import net.openid.appauth.AuthorizationManagementActivity +import org.hamcrest.CoreMatchers.allOf +import org.hamcrest.CoreMatchers.equalTo +import org.junit.Rule +import org.junit.Test +import org.junit.runner.RunWith +import org.libremail.R +import org.libremail.auth.OutlookAuthManager +import org.libremail.ui.FakeAccountRepository +import org.libremail.ui.accountsetup.AccountSetupViewModel +import org.libremail.ui.theme.LibreMailTheme + +/** + * End-to-end UI test for the pre-auth Outlook IMAP-enablement notice (#411). Drives the real + * [OutlookImapNoticeScreen] + [AccountSetupViewModel] over a [FakeAccountRepository]: the IMAP + * question and both outbound links render, tapping a help link fires the browser `ACTION_VIEW` + * intent, and tapping the bottom "Sign in" button starts the existing Microsoft OAuth (AppAuth) + * flow. Both launches are asserted with Espresso-Intents (mirroring `AccountPickerScreenTest`), so no + * real browser ever opens; the interstitial → OAuth navigation in the full onboarding graph is + * covered by `OnboardingFlowTest`. + */ +@RunWith(AndroidJUnit4::class) +class OutlookImapNoticeScreenTest { + + @get:Rule + val composeTestRule = createAndroidComposeRule() + + private val context: Context = + InstrumentationRegistry.getInstrumentation().targetContext.applicationContext + + private fun string(resId: Int) = composeTestRule.activity.getString(resId) + + private fun setContent(onAccountAdded: (String) -> Unit = {}) { + val viewModel = AccountSetupViewModel(OutlookAuthManager(context), FakeAccountRepository()) + composeTestRule.setContent { + LibreMailTheme(darkTheme = false, dynamicColor = false) { + OutlookImapNoticeScreen(onBack = {}, onAccountAdded = onAccountAdded, viewModel = viewModel) + } + } + } + + @Test + fun showsImapQuestion_bothLinks_andSignIn() { + setContent() + + composeTestRule.onNodeWithText(string(R.string.outlook_imap_question)).performScrollTo().assertIsDisplayed() + composeTestRule.onNodeWithText(string(R.string.outlook_imap_help)).performScrollTo().assertIsDisplayed() + composeTestRule.onNodeWithText(string(R.string.outlook_imap_settings)).performScrollTo().assertIsDisplayed() + composeTestRule.onNodeWithText(string(R.string.outlook_imap_sign_in)).performScrollTo().assertIsDisplayed() + } + + /** + * Tapping the "How to enable IMAP" link opens Microsoft's help article via + * [androidx.compose.ui.platform.UriHandler], which starts an `ACTION_VIEW` intent. Stubbing that + * intent both proves the tap launched it and stops a real browser from opening on the device. + */ + @Test + fun tappingImapHelpLink_opensTheMicrosoftArticle() { + setContent() + + Intents.init() + try { + Intents.intending(hasAction(Intent.ACTION_VIEW)) + .respondWith(Instrumentation.ActivityResult(Activity.RESULT_CANCELED, null)) + + composeTestRule.onNodeWithText(string(R.string.outlook_imap_help)) + .performScrollTo() + .performClick() + + Intents.intended(allOf(hasAction(Intent.ACTION_VIEW), hasData(hasHost(equalTo("support.microsoft.com"))))) + } finally { + Intents.release() + } + } + + /** + * Tapping "Sign in" must continue the existing Outlook OAuth flow — i.e. fire the AppAuth + * authorization intent (mirroring `AccountPickerScreenTest.tappingOutlook_...`). AppAuth wraps its + * browser launch in an intent targeting [AuthorizationManagementActivity]; that component name is + * the guaranteed, browser-independent signature of the launch. Stubbing a canceled result stops + * that activity from ever resuming and opening a real browser. + */ + @Test + fun tappingSignIn_launchesTheAppAuthBrowserIntent() { + setContent() + + Intents.init() + try { + Intents.intending(hasComponent(AuthorizationManagementActivity::class.java.name)) + .respondWith(Instrumentation.ActivityResult(Activity.RESULT_CANCELED, null)) + + composeTestRule.onNodeWithText(string(R.string.outlook_imap_sign_in)) + .performScrollTo() + .performClick() + + Intents.intended(hasComponent(AuthorizationManagementActivity::class.java.name)) + } finally { + Intents.release() + } + } +} diff --git a/app/src/main/kotlin/org/libremail/data/local/CacheEncryptionUnavailableException.kt b/app/src/main/kotlin/org/libremail/data/local/CacheEncryptionUnavailableException.kt index a73a30a..8bc4c8b 100644 --- a/app/src/main/kotlin/org/libremail/data/local/CacheEncryptionUnavailableException.kt +++ b/app/src/main/kotlin/org/libremail/data/local/CacheEncryptionUnavailableException.kt @@ -22,3 +22,19 @@ class CacheEncryptionUnavailableException(cause: Throwable) : "Encrypted cache unavailable: the SQLCipher native library failed to load on this device", cause, ) + +/** + * True when this throwable (or anything in its cause chain) signals that SQLCipher's native library is + * unavailable on this device (issue #359): a [CacheEncryptionUnavailableException] the provisioner raised + * when it failed closed, or — defensively — a bare [LinkageError] (an open-time `UnsatisfiedLinkError` at + * `SQLiteConnection.nativeOpen` that reached a headless entry point before the provisioner wrapped it). + * + * Headless entry points that inject the Room cache directly — the WorkManager workers and `IdleService` — + * have no UI gate (that is `CacheEncryptionGate`'s job), so they consult this to treat such a failure as a + * soft defer/skip (a worker retries; the push service stops) instead of crashing. A later launch may load + * the library and recover, so deferring rather than failing hard is correct. The whole cause chain is + * walked because coroutine stack-trace recovery can re-wrap the throwable as it crosses the database-open + * boundary (the same reason [DatabaseProvisioner]'s own tests assert on the cause chain, not the instance). + */ +fun Throwable.isCacheEncryptionUnavailable(): Boolean = generateSequence(this) { it.cause } + .any { it is CacheEncryptionUnavailableException || it is LinkageError } diff --git a/app/src/main/kotlin/org/libremail/data/local/DatabaseEncryption.kt b/app/src/main/kotlin/org/libremail/data/local/DatabaseEncryption.kt index 7122f26..3ede05f 100644 --- a/app/src/main/kotlin/org/libremail/data/local/DatabaseEncryption.kt +++ b/app/src/main/kotlin/org/libremail/data/local/DatabaseEncryption.kt @@ -115,8 +115,45 @@ object DatabaseEncryption { } } + /** + * Opens a throwaway keyed SQLCipher database next to [cacheFile] and immediately closes it, purely to + * reach `SQLiteConnection.nativeOpen` — the exact call site of the #359 crash — from inside + * [DatabaseProvisioner]'s fail-closed handler. [ensureNativeLibraryLoaded] + * (`System.loadLibrary("sqlcipher")`) can succeed on a device whose bundled `.so` is otherwise + * incompatible, yet the JNI-bound `nativeOpen` still be unresolved; that only surfaces when a keyed + * database is actually opened, which Room does LATER via its deferred open helper + * ([org.libremail.di.DatabaseModule]) — outside any handler. Probing the real open here lets the + * provisioner catch that [LinkageError] and fail closed BEFORE Room reaches it. + * + * Deliberately opens a sibling throwaway file (never the real cache) so it can never create, mutate, + * or leave `-wal`/`-shm` sidecars on the cache, then deletes the probe and its sidecars in a `finally`. + * Mirrors `SqlCipherOpenSpikeTest`'s stage-B keyed-open probe (issue #359). + * + * PRECONDITION: the caller must have already loaded the native library (via [ensureNativeLibraryLoaded]). + * The sole production caller — [DatabaseProvisioner]'s encrypted branch — does so on the line above its + * probe call. Kept out of here on purpose so the provisioner loads the library exactly ONCE per open + * (the `ensureNativeLibraryLoaded()`-exactly-once invariant its instrumented tests pin), not twice. + */ + fun probeKeyedOpen(cacheFile: File, passphrase: String) { + val dir = cacheFile.parentFile ?: error("cache database file has no parent directory") + val probe = File(dir, cacheFile.name + PROBE_SUFFIX) + try { + SQLiteDatabase.openOrCreateDatabase( + probe.absolutePath, + passphrase.toByteArray(Charsets.US_ASCII), + null, // no CursorFactory + null, // no DatabaseErrorHandler + ).close() + } finally { + listOf("", "-wal", "-shm", "-journal").forEach { File(dir, probe.name + it).delete() } + } + } + private const val TAG = "LibreMailDbCrypto" + // Suffix of the throwaway file [probeKeyedOpen] opens to reach nativeOpen without touching the cache. + private const val PROBE_SUFFIX = ".openprobe" + // The 16-byte magic that opens every plaintext SQLite file: "SQLite format 3" + a NUL terminator. // Spelled out as bytes to keep the trailing NUL unambiguous. private val SQLITE_HEADER = byteArrayOf( diff --git a/app/src/main/kotlin/org/libremail/data/local/DatabaseProvisioner.kt b/app/src/main/kotlin/org/libremail/data/local/DatabaseProvisioner.kt index e301c02..409e3eb 100644 --- a/app/src/main/kotlin/org/libremail/data/local/DatabaseProvisioner.kt +++ b/app/src/main/kotlin/org/libremail/data/local/DatabaseProvisioner.kt @@ -104,35 +104,45 @@ class DatabaseProvisioner internal constructor( keyStore.clearClearPending() } - // One-time move of accounts/credentials/settings/signatures into the non-auth AccountDatabase - // (issue #111). MUST run before the cache opens: opening it applies MIGRATION_15_16, which drops - // the moved tables. Runs AFTER the wipe above so an unrecoverable-key cache is gone first - // (nothing left to move) and we never block waiting on a passphrase we can't get. - accountDataMigrator.migrateIfNeeded() - - // Opt-in at-rest encryption of the local cache (off by default). The conversion runs here — - // before the database is opened — so it never races an open connection; toggling the setting - // therefore takes effect on the next app start. The passphrase source is resolved from which - // seal actually exists (DatabaseKeyStore.resolvePassphrase), NOT from the app-lock setting (a - // separate DataStore that can disagree). When app-lock is ON the sealing key is auth-bound, so - // resolvePassphrase waits on PassphraseSession until the user authenticates — which is why this - // must never run on the main thread while the cache is locked (issue #93). - val settings = settingsRepository.settings.first() + // FAIL CLOSED (issue #359, security rework of #367). SQLCipher's native library can fail to LOAD + // (UnsatisfiedLinkError from ensureNativeLibraryLoaded) OR to LINK (the library loads, but the + // JNI-bound SQLiteConnection.nativeOpen is unresolved and throws only when a keyed database is + // actually opened — the exact #359 signature). EVERY startup step that touches that library must + // therefore run inside this ONE handler, so any such LinkageError becomes a single fail-closed + // signal rather than escaping as a raw crash. On that signal we deliberately do NOT silently + // degrade to an unencrypted cache (that would defeat the user's opt-in encryption): we never open + // plaintext, wipe the on-disk ciphertext, or write the encryptCache setting — we raise a distinct + // exception the startup UI (CacheEncryptionGate) catches to show the encryption error gate. It is + // NOT memoized (a throw skips prepareCache's `.also { prepared = it }`), so the next launch + // re-attempts and recovers automatically if the library later loads. + // + // The catch stays typed LinkageError ONLY. It must NOT be broadened to Exception/Throwable: the + // migrator below deliberately throws a NON-linkage error ("crash-loop rather than lose data") on + // an unexpected copy failure, and that — like any other non-linkage error — must still propagate + // uncaught so we never drop the not-yet-copied source tables. return try { + // One-time move of accounts/credentials/settings/signatures into the non-auth AccountDatabase + // (issue #111). MUST run before the cache opens: opening it applies MIGRATION_15_16, which drops + // the moved tables. Runs AFTER the wipe above so an unrecoverable-key cache is gone first + // (nothing left to move) and we never block waiting on a passphrase we can't get. Runs INSIDE + // this handler (issue #359 gap 1): its copyAccountTables loads SQLCipher and does a keyed + // openOrCreateDatabase + ATTACH … KEY (a real nativeOpen) even when encryption is OFF, so a + // LinkageError there used to escape this handler entirely and crash-loop. + accountDataMigrator.migrateIfNeeded() + + // Opt-in at-rest encryption of the local cache (off by default). The conversion runs here — + // before the database is opened — so it never races an open connection; toggling the setting + // therefore takes effect on the next app start. The passphrase source is resolved from which + // seal actually exists (DatabaseKeyStore.resolvePassphrase), NOT from the app-lock setting (a + // separate DataStore that can disagree). When app-lock is ON the sealing key is auth-bound, so + // resolvePassphrase waits on PassphraseSession until the user authenticates — which is why this + // must never run on the main thread while the cache is locked (issue #93). + val settings = settingsRepository.settings.first() resolveOpenMode(settings, dbFile) } catch (nativeLoadFailure: LinkageError) { - // FAIL CLOSED (issue #359, security rework of #367). SQLCipher's native library could not be - // loaded/linked (e.g. UnsatisfiedLinkError at SQLiteConnection.nativeOpen or from - // ensureNativeLibraryLoaded), so the encrypted cache cannot be opened OR converted. We must - // NOT silently degrade to an unencrypted cache (that would defeat the user's opt-in - // encryption), so we deliberately do NOT: open plaintext, wipe the on-disk ciphertext, or - // write the encryptCache setting. Instead raise a distinct signal the startup UI catches to - // show the encryption error gate. This throw is NOT memoized (it skips prepareCache's - // `.also { prepared = it }`), so the next launch re-attempts and recovers automatically if - // the library later loads. AppLog.w( TAG, - "SQLCipher native library failed to load; failing closed (encrypted cache unavailable)", + "SQLCipher native library failed to load or link; failing closed (encrypted cache unavailable)", nativeLoadFailure, ) throw CacheEncryptionUnavailableException(nativeLoadFailure) @@ -159,6 +169,13 @@ class DatabaseProvisioner internal constructor( // load the keyed open reaches SQLiteConnection.nativeOpen with no library loaded and // crashes with UnsatisfiedLinkError on every cold start once encryption is enabled. DatabaseEncryption.ensureNativeLibraryLoaded() + // Probe the REAL keyed open HERE (issue #359 gap 2), inside the fail-closed handler. + // ensureNativeLibraryLoaded() above only loads the .so; the keyed nativeOpen that can still + // throw on an incompatible device fires LATER, in DatabaseModule's DeferredOpenHelperFactory + // AFTER prepareCache() returns — outside any handler — so an open-time UnsatisfiedLinkError + // there would escape uncaught. Reaching a keyed nativeOpen now converts that LinkageError to + // CacheEncryptionUnavailableException before Room's deferred open can crash on it. + DatabaseEncryption.probeKeyedOpen(dbFile, passphrase) CacheOpenMode.Encrypted(passphrase) } diff --git a/app/src/main/kotlin/org/libremail/data/repository/AccountRepositoryImpl.kt b/app/src/main/kotlin/org/libremail/data/repository/AccountRepositoryImpl.kt index 61f16be..07b8f84 100644 --- a/app/src/main/kotlin/org/libremail/data/repository/AccountRepositoryImpl.kt +++ b/app/src/main/kotlin/org/libremail/data/repository/AccountRepositoryImpl.kt @@ -24,6 +24,8 @@ import org.libremail.domain.model.ImapConnectionParams import org.libremail.domain.repository.AccountRepository import org.libremail.mail.ImapClient import org.libremail.notifications.MailNotifier +import org.libremail.reporting.AppLog +import org.libremail.reporting.accountLogRef import javax.inject.Inject import javax.inject.Singleton @@ -53,10 +55,17 @@ class AccountRepositoryImpl @Inject constructor( override suspend fun addImapAccount(account: Account, password: String): Result> = runCatching { val folders = imapClient.listFolders(account.toImapParams(secret = password, useXoauth2 = false)) + // Persist the credential BEFORE inserting the account row (#403). Both LibreMailApplication's + // push collector and IdleService.reconcileWatchers react to the *accounts* table; committing the + // secret first guarantees any watcher that observes the new row can already resolve it, instead + // of firing a transient "No stored credentials" IDLE miss on every account add. The credentials + // table has no foreign key to accounts, so it can be written first; account_settings does (FK), + // so ensureDefaults must still follow the account row. + credentialStore.saveSecret(account.id, password) accountDao.insertAtEnd(account.toEntity()) accountSettingsRepository.ensureDefaults(account.id) - credentialStore.saveSecret(account.id, password) mailNotifier.ensureAccountChannel(account) + AppLog.i(TAG, "IMAP account added ${accountLogRef(account.id)}; credential persisted before account row") syncScheduler.syncNow() syncScheduler.backfillNow() // start caching this account's full history in the background (#12) folders.map { it.fullName } @@ -69,10 +78,15 @@ class AccountRepositoryImpl @Inject constructor( ): Result> = runCatching { val account = Account.outlook(email) val folders = imapClient.listFolders(account.toImapParams(secret = accessToken, useXoauth2 = true)) + // Persist the durable AuthState BEFORE the account row (#403) — same ordering rationale as + // addImapAccount: the push watchers observe the accounts table, so the secret must be committed + // first for the newly-observed account to resolve. account_settings' FK still needs the row, so + // ensureDefaults follows the insert. + credentialStore.saveSecret(account.id, authStateJson) accountDao.insertAtEnd(account.toEntity()) accountSettingsRepository.ensureDefaults(account.id) - credentialStore.saveSecret(account.id, authStateJson) mailNotifier.ensureAccountChannel(account) + AppLog.i(TAG, "Outlook account added ${accountLogRef(account.id)}; credential persisted before account row") syncScheduler.syncNow() syncScheduler.backfillNow() // start caching this account's full history in the background (#12) folders.map { it.fullName } @@ -113,4 +127,8 @@ class AccountRepositoryImpl @Inject constructor( if (accountId != null) backfillProgressDao.deleteForAccount(accountId) else backfillProgressDao.deleteAll() syncScheduler.backfillNow() } + + private companion object { + const val TAG = "AccountRepository" + } } diff --git a/app/src/main/kotlin/org/libremail/data/sync/BackfillWorker.kt b/app/src/main/kotlin/org/libremail/data/sync/BackfillWorker.kt index d35402a..8a5ea04 100644 --- a/app/src/main/kotlin/org/libremail/data/sync/BackfillWorker.kt +++ b/app/src/main/kotlin/org/libremail/data/sync/BackfillWorker.kt @@ -10,6 +10,7 @@ import dagger.assisted.Assisted import dagger.assisted.AssistedInject import kotlinx.coroutines.CancellationException import org.libremail.BuildConfig +import org.libremail.data.local.isCacheEncryptionUnavailable import org.libremail.data.security.EncryptedCacheGuard import org.libremail.reporting.AppLog @@ -60,7 +61,14 @@ class BackfillWorker @AssistedInject constructor( }, onFailure = { error -> if (error is CancellationException) throw error - AppLog.w(TAG, "backfill worker: retry", error) + // A DB open that fails because SQLCipher's native library is unavailable (issue #359) lands + // here (thrown inside the runCatching above); log it distinctly but still defer softly — a + // later launch may load the library and recover — instead of a generic retry. + if (error.isCacheEncryptionUnavailable()) { + AppLog.w(TAG, "backfill deferred: encrypted cache unavailable (SQLCipher native library)", error) + } else { + AppLog.w(TAG, "backfill worker: retry", error) + } Result.retry() }, ) diff --git a/app/src/main/kotlin/org/libremail/data/sync/EncryptedCacheWorkerGuard.kt b/app/src/main/kotlin/org/libremail/data/sync/EncryptedCacheWorkerGuard.kt new file mode 100644 index 0000000..5d8164f --- /dev/null +++ b/app/src/main/kotlin/org/libremail/data/sync/EncryptedCacheWorkerGuard.kt @@ -0,0 +1,24 @@ +// SPDX-License-Identifier: GPL-3.0-or-later +package org.libremail.data.sync + +import androidx.work.ListenableWorker.Result +import org.libremail.data.local.isCacheEncryptionUnavailable +import org.libremail.reporting.AppLog + +/** + * Runs [block] and, if opening the Room cache fails because SQLCipher's native library is unavailable on + * this device (issue #359 — a `CacheEncryptionUnavailableException` the provisioner raised, or defensively + * a bare `LinkageError` at `nativeOpen`), logs a PII-free breadcrumb under [tag] and returns + * [Result.retry] rather than letting the failure crash the worker. WorkManager workers inject the cache + * directly, with no UI gate; a later launch may load the library and recover, so a soft retry — not a hard + * failure — is correct. Every other throwable (including `CancellationException`) propagates unchanged. + * + * `inline` so the (suspending) [block] runs in the worker's own coroutine context, mirroring `runCatching`. + */ +internal inline fun retryIfEncryptedCacheUnavailable(tag: String, block: () -> Result): Result = try { + block() +} catch (failure: Throwable) { + if (!failure.isCacheEncryptionUnavailable()) throw failure + AppLog.w(tag, "deferred: encrypted cache unavailable (SQLCipher native library)", failure) + Result.retry() +} diff --git a/app/src/main/kotlin/org/libremail/data/sync/MailConnectionFactory.kt b/app/src/main/kotlin/org/libremail/data/sync/MailConnectionFactory.kt index 26bdf62..ec29fe7 100644 --- a/app/src/main/kotlin/org/libremail/data/sync/MailConnectionFactory.kt +++ b/app/src/main/kotlin/org/libremail/data/sync/MailConnectionFactory.kt @@ -51,7 +51,7 @@ class MailConnectionFactory @Inject constructor( private suspend fun resolveSecret(account: Account): String = when (account.authType) { AuthType.PASSWORD_IMAP -> - credentialStore.loadSecret(account.id) ?: error("No stored credentials for ${account.email}") + credentialStore.loadSecret(account.id) ?: throw MissingCredentialsException() AuthType.OAUTH_OUTLOOK -> cachedAccessToken(account.id, SCOPE_OUTLOOK, outlookAuthManager::freshOutlookToken) } @@ -70,7 +70,7 @@ class MailConnectionFactory @Inject constructor( // computeIfAbsent (not getOrPut) so concurrent first-callers share one mutex per account. return refreshMutexes.computeIfAbsent(accountId) { Mutex() }.withLock { validCachedToken(accountId, scope)?.let { return@withLock it } - val stored = credentialStore.loadSecret(accountId) ?: error("No stored credentials for $accountId") + val stored = credentialStore.loadSecret(accountId) ?: throw MissingCredentialsException() val fresh = refresh(stored) if (fresh.authStateJson != stored) credentialStore.saveSecret(accountId, fresh.authStateJson) tokenCache["$accountId|$scope"] = CachedToken(fresh.accessToken, fresh.accessTokenExpiry) diff --git a/app/src/main/kotlin/org/libremail/data/sync/MissingCredentialsException.kt b/app/src/main/kotlin/org/libremail/data/sync/MissingCredentialsException.kt new file mode 100644 index 0000000..55667cf --- /dev/null +++ b/app/src/main/kotlin/org/libremail/data/sync/MissingCredentialsException.kt @@ -0,0 +1,20 @@ +// SPDX-License-Identifier: GPL-3.0-or-later +package org.libremail.data.sync + +/** + * Thrown by [MailConnectionFactory] when an account has no stored credential to resolve. + * + * It extends [IllegalStateException] — the type [kotlin.error] previously raised here — so existing + * callers that treat a missing credential as a hard failure are unaffected. What it adds is a type a + * caller can catch *specifically*: the IMAP IDLE watcher ([org.libremail.push.IdleService]) tolerates + * the brief account-add write race (#403), where a reactive observer of the accounts table can see a + * newly-added account a beat before its secret has finished persisting, by catching this and deferring + * instead of logging a connection failure. Genuinely-absent credentials still surface as an error to + * every other caller. + * + * The message is deliberately PII-free (no email, host, or account id): an account id embeds the raw + * email address, so it must never appear in an exception message that could reach Logcat. A caller that + * needs to name the account in a log line uses [org.libremail.reporting.accountLogRef] on the id it + * already holds. + */ +class MissingCredentialsException : IllegalStateException("No stored credentials for account") diff --git a/app/src/main/kotlin/org/libremail/data/sync/PruneWorker.kt b/app/src/main/kotlin/org/libremail/data/sync/PruneWorker.kt index 003c9c4..93b76c6 100644 --- a/app/src/main/kotlin/org/libremail/data/sync/PruneWorker.kt +++ b/app/src/main/kotlin/org/libremail/data/sync/PruneWorker.kt @@ -8,6 +8,7 @@ import androidx.work.WorkerParameters import dagger.Lazy import dagger.assisted.Assisted import dagger.assisted.AssistedInject +import org.libremail.data.local.isCacheEncryptionUnavailable import org.libremail.data.security.EncryptedCacheGuard import org.libremail.reporting.AppLog @@ -39,7 +40,14 @@ class PruneWorker @AssistedInject constructor( Result.success() }, onFailure = { error -> - AppLog.w(TAG, "prune worker: retry", error) + // A DB open that fails because SQLCipher's native library is unavailable (issue #359) lands + // here (it is thrown inside the runCatching above); log it distinctly but still defer softly + // — a later launch may load the library and recover — instead of a generic retry. + if (error.isCacheEncryptionUnavailable()) { + AppLog.w(TAG, "prune deferred: encrypted cache unavailable (SQLCipher native library)", error) + } else { + AppLog.w(TAG, "prune worker: retry", error) + } Result.retry() }, ) diff --git a/app/src/main/kotlin/org/libremail/data/sync/SendWorker.kt b/app/src/main/kotlin/org/libremail/data/sync/SendWorker.kt index 4bade40..7e18f0a 100644 --- a/app/src/main/kotlin/org/libremail/data/sync/SendWorker.kt +++ b/app/src/main/kotlin/org/libremail/data/sync/SendWorker.kt @@ -52,6 +52,13 @@ class SendWorker @AssistedInject constructor( override suspend fun doWork(): Result { if (cacheGuard.isCacheLocked()) return Result.retry() + // Resolving the Lazy DB deps below opens the Room cache; if SQLCipher's native library is + // unavailable on this device (issue #359) that open throws — with no UI gate here, treat it as a + // soft retry rather than letting it crash the worker. + return retryIfEncryptedCacheUnavailable(TAG) { drainOutbox() } + } + + private suspend fun drainOutbox(): Result { val outboxDao = this.outboxDao.get() val accountDao = this.accountDao.get() val connectionFactory = this.connectionFactory.get() diff --git a/app/src/main/kotlin/org/libremail/data/sync/SyncWorker.kt b/app/src/main/kotlin/org/libremail/data/sync/SyncWorker.kt index 987162c..0f5e2f0 100644 --- a/app/src/main/kotlin/org/libremail/data/sync/SyncWorker.kt +++ b/app/src/main/kotlin/org/libremail/data/sync/SyncWorker.kt @@ -28,16 +28,21 @@ class SyncWorker @AssistedInject constructor( AppLog.i(TAG, "sync deferred: cache locked") return Result.retry() } - return mailSyncer.get().syncAll().fold( - onSuccess = { - AppLog.i(TAG, "sync worker: success") - Result.success() - }, - onFailure = { error -> - AppLog.w(TAG, "sync worker: retry", error) - Result.retry() - }, - ) + // syncAll()'s first DB access (and thus Room's deferred cache open) can throw if SQLCipher's + // native library is unavailable on this device (issue #359) — that open fires OUTSIDE syncAll's own + // runCatching, so without this guard it would escape doWork. Treat it as a soft retry, not a crash. + return retryIfEncryptedCacheUnavailable(TAG) { + mailSyncer.get().syncAll().fold( + onSuccess = { + AppLog.i(TAG, "sync worker: success") + Result.success() + }, + onFailure = { error -> + AppLog.w(TAG, "sync worker: retry", error) + Result.retry() + }, + ) + } } private companion object { diff --git a/app/src/main/kotlin/org/libremail/push/IdleService.kt b/app/src/main/kotlin/org/libremail/push/IdleService.kt index 6120467..249b501 100644 --- a/app/src/main/kotlin/org/libremail/push/IdleService.kt +++ b/app/src/main/kotlin/org/libremail/push/IdleService.kt @@ -27,10 +27,12 @@ import kotlinx.coroutines.isActive import kotlinx.coroutines.launch import kotlinx.coroutines.withTimeoutOrNull import org.libremail.data.local.dao.AccountDao +import org.libremail.data.local.isCacheEncryptionUnavailable import org.libremail.data.local.toDomain import org.libremail.data.security.EncryptedCacheGuard import org.libremail.data.sync.MailConnectionFactory import org.libremail.data.sync.MailSyncer +import org.libremail.data.sync.MissingCredentialsException import org.libremail.data.sync.PushMode import org.libremail.data.sync.SyncResourcePolicy import org.libremail.data.sync.SyncScheduler @@ -110,7 +112,20 @@ class IdleService : Service() { stopSelf() return@launch } - reconcileWatchers() + // Headless tolerance for issue #359: this service injects the Room cache with NO UI gate + // (CacheEncryptionGate wraps only MainActivity), so if SQLCipher's native library is unavailable + // on this device reconcileWatchers()'s first DB access throws — a CacheEncryptionUnavailableException + // from the provisioner, or defensively a bare LinkageError at nativeOpen. Left uncaught, a child of + // this SupervisorJob would route it to the app's default handler and crash the process. Treat it + // like the cache-locked case: log PII-free and stop. The app's CacheEncryptionGate surfaces the + // error to the user, and a later restart re-probes and recovers if the library loads. + try { + reconcileWatchers() + } catch (unavailable: Throwable) { + if (!unavailable.isCacheEncryptionUnavailable()) throw unavailable + AppLog.w(TAG, "encrypted cache unavailable (SQLCipher native lib); deferring IDLE push", unavailable) + stopSelf() + } } // The reuse cache (issue #357 Part 2) keeps interactive/sync IMAP connections warm; sweep // them so a socket that has gone idle past the reuse timeout is closed rather than left @@ -301,6 +316,16 @@ class IdleService : Service() { backoffMs = INITIAL_BACKOFF_MS } catch (e: CancellationException) { throw e + } catch (ignored: MissingCredentialsException) { + // #403: a just-added account can be observed here a beat before its secret finishes + // persisting. AccountRepository now commits the secret before the account row, so this is + // rare — but tolerate any residual race as a transient miss: defer quietly and re-check + // soon, WITHOUT the warn + exponential backoff a real connection drop gets. A genuinely + // absent credential simply keeps deferring (no mail, but no error noise) until it appears + // or the account is removed. The sentinel exception carries no diagnostic value beyond the + // message below, so it is intentionally not re-logged with its (empty) trace. + AppLog.i(TAG, "IDLE deferred ${accountLogRef(account.id)}: credentials not yet persisted") + delay(CREDENTIALS_DEFER_RETRY_MS) } catch (e: Exception) { AppLog.w(TAG, "IDLE for ${accountLogRef(account.id)} dropped; retrying in ${backoffMs}ms", e) delay(backoffMs) @@ -332,6 +357,12 @@ class IdleService : Service() { const val INITIAL_BACKOFF_MS = 5_000L const val MAX_BACKOFF_MS = 5 * 60_000L + // #403: how long to wait before re-checking after a transient missing-credential miss on a + // just-added account. Short — the account-add write race resolves in milliseconds once the + // secret commit lands — and deliberately flat (no exponential escalation) because this is an + // expected persist-ordering blip, not a connection failure. + const val CREDENTIALS_DEFER_RETRY_MS = 1_000L + // Cadence of the reuse-cache idle-eviction sweep (issue #357 Part 2). Tighter than the reuse // idle timeout so an idle socket is closed shortly after it crosses it. const val REUSE_EVICTION_SWEEP_MS = 2 * 60_000L diff --git a/app/src/main/kotlin/org/libremail/ui/LibreMailApp.kt b/app/src/main/kotlin/org/libremail/ui/LibreMailApp.kt index 7363c07..8c6baa1 100644 --- a/app/src/main/kotlin/org/libremail/ui/LibreMailApp.kt +++ b/app/src/main/kotlin/org/libremail/ui/LibreMailApp.kt @@ -42,6 +42,7 @@ import org.libremail.ui.onboarding.ContactsAccessScreen import org.libremail.ui.onboarding.LicenseScreen import org.libremail.ui.onboarding.OnboardingViewModel import org.libremail.ui.onboarding.OnboardingWelcomeScreen +import org.libremail.ui.onboarding.OutlookImapNoticeScreen import org.libremail.ui.outbox.OutboxScreen import org.libremail.ui.reader.ReaderScreen import org.libremail.ui.reporting.ProblemReportsScreen @@ -344,29 +345,50 @@ private fun NavGraphBuilder.onboardingGraph(navController: NavHostController, li navController.navigate(Routes.onboardingAppPassword(provider.key)) }, onManualSetup = { navController.navigate(Routes.ONBOARDING_MANUAL) }, + // Onboarding interposes the IMAP-enablement notice before Microsoft sign-in (#411); + // the notice screen then runs the same Outlook OAuth flow the picker would inline. + onPickOutlook = { navController.navigate(Routes.ONBOARDING_OUTLOOK_IMAP) }, ) } - composable( - route = Routes.ONBOARDING_APP_PASSWORD_PATTERN, - arguments = listOf(navArgument(Routes.APP_PASSWORD_ARG_PROVIDER) { type = NavType.StringType }), - ) { entry -> - val onboarding = onboardingViewModel(navController, entry) - AppPasswordSetupScreen( - onBack = navController::popBackStack, - onAccountAdded = { id -> onboarding.completeAdd(navController, id) }, - ) - } - composable(Routes.ONBOARDING_MANUAL) { entry -> - val onboarding = onboardingViewModel(navController, entry) - ManualSetupScreen( - onBack = navController::popBackStack, - onAccountAdded = { id -> onboarding.completeAdd(navController, id) }, - ) - } + onboardingSetupDestinations(navController) onboardingFinishDestinations(navController) } } +/** + * The account-setup destinations reached from the vendor picker: the pre-auth Outlook IMAP-enablement + * notice (#411), the guided app-password form, and manual IMAP/SMTP setup. Split out of + * [onboardingGraph] so each stays a readable length; all share the graph-scoped [OnboardingViewModel] + * and report a completed add through [completeAdd], which drops the setup screen and advances to the + * "add another?" prompt. + */ +private fun NavGraphBuilder.onboardingSetupDestinations(navController: NavHostController) { + composable(Routes.ONBOARDING_OUTLOOK_IMAP) { entry -> + val onboarding = onboardingViewModel(navController, entry) + OutlookImapNoticeScreen( + onBack = navController::popBackStack, + onAccountAdded = { id -> onboarding.completeAdd(navController, id) }, + ) + } + composable( + route = Routes.ONBOARDING_APP_PASSWORD_PATTERN, + arguments = listOf(navArgument(Routes.APP_PASSWORD_ARG_PROVIDER) { type = NavType.StringType }), + ) { entry -> + val onboarding = onboardingViewModel(navController, entry) + AppPasswordSetupScreen( + onBack = navController::popBackStack, + onAccountAdded = { id -> onboarding.completeAdd(navController, id) }, + ) + } + composable(Routes.ONBOARDING_MANUAL) { entry -> + val onboarding = onboardingViewModel(navController, entry) + ManualSetupScreen( + onBack = navController::popBackStack, + onAccountAdded = { id -> onboarding.completeAdd(navController, id) }, + ) + } +} + /** * The tail of onboarding: the "add another?" prompt and the optional contacts + battery opt-in steps. * Split out of [onboardingGraph] so each stays a readable length; all share the graph-scoped diff --git a/app/src/main/kotlin/org/libremail/ui/accountsetup/AccountPickerScreen.kt b/app/src/main/kotlin/org/libremail/ui/accountsetup/AccountPickerScreen.kt index ce3dd1c..a96ff39 100644 --- a/app/src/main/kotlin/org/libremail/ui/accountsetup/AccountPickerScreen.kt +++ b/app/src/main/kotlin/org/libremail/ui/accountsetup/AccountPickerScreen.kt @@ -57,6 +57,9 @@ import org.libremail.domain.model.MailProvider * * @param onAccountAdded invoked with the new account id when the *inline* Outlook flow completes. * The app-password and manual paths report their own completion from their own screens. + * @param onPickOutlook when non-null, tapping Outlook delegates here instead of launching auth + * inline — onboarding uses it to first show the pre-auth IMAP-enablement notice (#411). Left null + * for the standalone "Add account" entry, which keeps launching Microsoft sign-in directly. */ @OptIn(ExperimentalMaterial3Api::class) @Composable @@ -65,6 +68,7 @@ fun AccountPickerScreen( onAccountAdded: (String) -> Unit, onPickProvider: (MailProvider) -> Unit, onManualSetup: () -> Unit, + onPickOutlook: (() -> Unit)? = null, viewModel: AccountSetupViewModel = hiltViewModel(), ) { val state by viewModel.state.collectAsStateWithLifecycle() @@ -74,6 +78,19 @@ fun AccountPickerScreen( ActivityResultContracts.StartActivityForResult(), ) { result -> viewModel.onOutlookResult(result.data) } + // Launches Microsoft sign-in in place (the standalone "Add account" behaviour). Onboarding + // overrides the Outlook tap with [onPickOutlook] to interpose the IMAP-enablement notice (#411), + // which then runs this exact same flow from its "Sign in" button. + val launchOutlookInline: () -> Unit = { + viewModel.outlookAuthIntent().fold( + onSuccess = { intent -> + runCatching { outlookLauncher.launch(intent) } + .onFailure { viewModel.onOutlookLaunchFailed(it) } + }, + onFailure = { viewModel.onOutlookLaunchFailed(it) }, + ) + } + LaunchedEffect(state.status, state.addedAccountId) { if (state.status == SetupStatus.DONE) { state.addedAccountId?.let(onAccountAdded) @@ -124,15 +141,7 @@ fun AccountPickerScreen( icon = Icons.Filled.Email, label = stringResource(R.string.account_setup_outlook), enabled = !busy, - onClick = { - viewModel.outlookAuthIntent().fold( - onSuccess = { intent -> - runCatching { outlookLauncher.launch(intent) } - .onFailure { viewModel.onOutlookLaunchFailed(it) } - }, - onFailure = { viewModel.onOutlookLaunchFailed(it) }, - ) - }, + onClick = onPickOutlook ?: launchOutlookInline, ) MailProvider.entries.forEach { provider -> ProviderRow( diff --git a/app/src/main/kotlin/org/libremail/ui/navigation/Routes.kt b/app/src/main/kotlin/org/libremail/ui/navigation/Routes.kt index 3c9b1da..3c4f8bd 100644 --- a/app/src/main/kotlin/org/libremail/ui/navigation/Routes.kt +++ b/app/src/main/kotlin/org/libremail/ui/navigation/Routes.kt @@ -39,6 +39,14 @@ object Routes { const val ONBOARDING_LICENSE = "onboarding/license" const val ONBOARDING_WELCOME = "onboarding/welcome" const val ONBOARDING_PICKER = "onboarding/picker" + + // Pre-auth interstitial shown when the user picks Outlook during onboarding (#411): new + // personal outlook.com accounts ship with IMAP OFF by default, so OAuth can succeed while the + // IMAP AUTHENTICATE step later fails. This screen asks the user to confirm IMAP is on and links + // Microsoft's help/settings pages before its bottom "Sign in" button continues the existing + // Outlook OAuth flow. Onboarding-only; the standalone "Add account" picker still launches auth + // inline (the reactive complement for that path is #390). + const val ONBOARDING_OUTLOOK_IMAP = "onboarding/outlook_imap" const val ONBOARDING_MANUAL = "onboarding/manual" const val ONBOARDING_ADD_ANOTHER = "onboarding/add_another" diff --git a/app/src/main/kotlin/org/libremail/ui/onboarding/OutlookImapNoticeScreen.kt b/app/src/main/kotlin/org/libremail/ui/onboarding/OutlookImapNoticeScreen.kt new file mode 100644 index 0000000..a287e7f --- /dev/null +++ b/app/src/main/kotlin/org/libremail/ui/onboarding/OutlookImapNoticeScreen.kt @@ -0,0 +1,213 @@ +// SPDX-License-Identifier: GPL-3.0-or-later +package org.libremail.ui.onboarding + +import androidx.activity.compose.rememberLauncherForActivityResult +import androidx.activity.result.contract.ActivityResultContracts +import androidx.compose.foundation.background +import androidx.compose.foundation.layout.Box +import androidx.compose.foundation.layout.Column +import androidx.compose.foundation.layout.Spacer +import androidx.compose.foundation.layout.fillMaxSize +import androidx.compose.foundation.layout.fillMaxWidth +import androidx.compose.foundation.layout.height +import androidx.compose.foundation.layout.padding +import androidx.compose.foundation.layout.size +import androidx.compose.foundation.rememberScrollState +import androidx.compose.foundation.verticalScroll +import androidx.compose.material.icons.Icons +import androidx.compose.material.icons.automirrored.filled.ArrowBack +import androidx.compose.material.icons.filled.Email +import androidx.compose.material3.Button +import androidx.compose.material3.CircularProgressIndicator +import androidx.compose.material3.ExperimentalMaterial3Api +import androidx.compose.material3.Icon +import androidx.compose.material3.IconButton +import androidx.compose.material3.MaterialTheme +import androidx.compose.material3.OutlinedButton +import androidx.compose.material3.Scaffold +import androidx.compose.material3.SnackbarHost +import androidx.compose.material3.SnackbarHostState +import androidx.compose.material3.Text +import androidx.compose.material3.TopAppBar +import androidx.compose.runtime.Composable +import androidx.compose.runtime.LaunchedEffect +import androidx.compose.runtime.getValue +import androidx.compose.runtime.remember +import androidx.compose.runtime.rememberCoroutineScope +import androidx.compose.ui.Alignment +import androidx.compose.ui.Modifier +import androidx.compose.ui.platform.LocalUriHandler +import androidx.compose.ui.res.stringResource +import androidx.compose.ui.unit.dp +import androidx.hilt.lifecycle.viewmodel.compose.hiltViewModel +import androidx.lifecycle.compose.collectAsStateWithLifecycle +import kotlinx.coroutines.launch +import org.libremail.R +import org.libremail.reporting.AppLog +import org.libremail.ui.accountsetup.AccountSetupViewModel +import org.libremail.ui.accountsetup.SetupStatus + +/** + * Pre-auth interstitial shown when the user picks Outlook during onboarding (#411), before the + * Microsoft OAuth browser opens. New personal outlook.com accounts ship with IMAP **OFF** by + * default, so OAuth can succeed while the later IMAP `AUTHENTICATE` step fails — a confusing + * dead-end (the *reactive* complement is #390). This screen asks the user to confirm IMAP is on + * first, links Microsoft's help article and the Outlook IMAP settings page, and only then continues + * the **existing** Outlook OAuth flow from its bottom "Sign in" button. + * + * The sign-in wiring is identical to the picker's Outlook row: build the AppAuth intent via + * [AccountSetupViewModel.outlookAuthIntent], launch it, and hand the redirect back to + * [AccountSetupViewModel.onOutlookResult]; a completed add is reported through [onAccountAdded]. No + * PII is logged — the account address is embedded in the id and never touched here (see + * [AccountSetupViewModel.onOutlookResult] for the add breadcrumb). + * + * @param onBack returns to the vendor picker (e.g. to choose a different provider). + * @param onAccountAdded invoked with the new account id once the Outlook flow completes. + */ +@OptIn(ExperimentalMaterial3Api::class) +@Composable +fun OutlookImapNoticeScreen( + onBack: () -> Unit, + onAccountAdded: (String) -> Unit, + viewModel: AccountSetupViewModel = hiltViewModel(), +) { + val state by viewModel.state.collectAsStateWithLifecycle() + val snackbarHostState = remember { SnackbarHostState() } + val uriHandler = LocalUriHandler.current + val scope = rememberCoroutineScope() + // Resolved up front so the non-composable failure handler can use it. + val openFailedMessage = stringResource(R.string.app_password_open_failed) + + val outlookLauncher = rememberLauncherForActivityResult( + ActivityResultContracts.StartActivityForResult(), + ) { result -> viewModel.onOutlookResult(result.data) } + + // One-shot breadcrumb so a debug report shows the user reached the pre-auth IMAP notice (#411). + LaunchedEffect(Unit) { AppLog.i(TAG, "Outlook IMAP notice shown") } + + LaunchedEffect(state.status, state.addedAccountId) { + if (state.status == SetupStatus.DONE) { + state.addedAccountId?.let(onAccountAdded) + } + } + LaunchedEffect(state.error) { + state.error?.let { + snackbarHostState.showSnackbar(it) + viewModel.consumeError() + } + } + + // openUri throws when no browser/handler is installed; surface that as a snackbar, not a crash. + val openUrl: (String) -> Unit = { url -> + runCatching { uriHandler.openUri(url) } + .onFailure { scope.launch { snackbarHostState.showSnackbar(openFailedMessage) } } + } + + val busy = state.status == SetupStatus.CONNECTING + + Scaffold( + topBar = { + TopAppBar( + title = { Text(stringResource(R.string.outlook_imap_title)) }, + navigationIcon = { + IconButton(onClick = onBack) { + Icon( + Icons.AutoMirrored.Filled.ArrowBack, + contentDescription = stringResource(R.string.action_back), + ) + } + }, + ) + }, + snackbarHost = { SnackbarHost(snackbarHostState) }, + ) { padding -> + Box(Modifier.fillMaxSize().padding(padding)) { + Column( + modifier = Modifier + .fillMaxSize() + .verticalScroll(rememberScrollState()) + .padding(24.dp), + ) { + Icon( + Icons.Filled.Email, + contentDescription = null, + modifier = Modifier.size(64.dp), + tint = MaterialTheme.colorScheme.primary, + ) + Spacer(Modifier.height(20.dp)) + Text( + text = stringResource(R.string.outlook_imap_question), + style = MaterialTheme.typography.headlineSmall, + ) + Spacer(Modifier.height(12.dp)) + Text( + text = stringResource(R.string.outlook_imap_body), + style = MaterialTheme.typography.bodyLarge, + color = MaterialTheme.colorScheme.onSurfaceVariant, + ) + Spacer(Modifier.height(24.dp)) + OutlinedButton( + onClick = { + AppLog.i(TAG, "Outlook IMAP help article opened") + openUrl(IMAP_HELP_URL) + }, + modifier = Modifier.fillMaxWidth(), + ) { + Text(stringResource(R.string.outlook_imap_help)) + } + Spacer(Modifier.height(8.dp)) + OutlinedButton( + onClick = { + AppLog.i(TAG, "Outlook IMAP settings page opened") + openUrl(IMAP_SETTINGS_URL) + }, + modifier = Modifier.fillMaxWidth(), + ) { + Text(stringResource(R.string.outlook_imap_settings)) + } + Spacer(Modifier.height(32.dp)) + // Bottom of the visual hierarchy (#411): continues the existing Outlook OAuth flow, + // unchanged. Append any new controls AFTER this so onboarding E2E clicks stay stable. + Button( + onClick = { + AppLog.i(TAG, "Outlook sign-in continued from IMAP notice") + viewModel.outlookAuthIntent().fold( + onSuccess = { intent -> + runCatching { outlookLauncher.launch(intent) } + .onFailure { viewModel.onOutlookLaunchFailed(it) } + }, + onFailure = { viewModel.onOutlookLaunchFailed(it) }, + ) + }, + enabled = !busy, + modifier = Modifier.fillMaxWidth(), + ) { + Text(stringResource(R.string.outlook_imap_sign_in)) + } + } + if (busy) { + Box( + modifier = Modifier + .fillMaxSize() + .background(MaterialTheme.colorScheme.scrim.copy(alpha = 0.32f)), + contentAlignment = Alignment.Center, + ) { + CircularProgressIndicator() + } + } + } + } +} + +private const val TAG = "OutlookImapNotice" + +// Microsoft's canonical "POP, IMAP, and SMTP settings for Outlook.com" support article — the +// authoritative walkthrough for switching IMAP on (verified 2026-07, issue #411). +private const val IMAP_HELP_URL = + "https://support.microsoft.com/en-us/office/pop-imap-and-smtp-settings-for-outlook-com-" + + "d088b986-291d-42b8-9564-9c414e2aa040" + +// Deep link to the Outlook.com POP/IMAP settings page ("Let devices and apps use IMAP"). If +// Microsoft changes the options path this still lands the user in Outlook.com mail settings; the +// help article above is the durable fallback. +private const val IMAP_SETTINGS_URL = "https://outlook.live.com/mail/0/options/mail/accounts/popImap" diff --git a/app/src/main/res/values/strings.xml b/app/src/main/res/values/strings.xml index f02be49..9212a5b 100644 --- a/app/src/main/res/values/strings.xml +++ b/app/src/main/res/values/strings.xml @@ -205,6 +205,14 @@ Other (IMAP/SMTP) Choose your email provider to get started. + + Turn on IMAP for Outlook + Have you enabled IMAP for your Outlook account? + New Outlook.com accounts often have IMAP switched off. LibreMail needs IMAP turned on to receive your mail — please enable it before signing in. + How to enable IMAP for Outlook + Open Outlook IMAP settings + Sign in + Connect %1$s Unknown email provider. diff --git a/app/src/test/kotlin/org/libremail/data/local/CacheEncryptionUnavailableExceptionTest.kt b/app/src/test/kotlin/org/libremail/data/local/CacheEncryptionUnavailableExceptionTest.kt new file mode 100644 index 0000000..9307a8c --- /dev/null +++ b/app/src/test/kotlin/org/libremail/data/local/CacheEncryptionUnavailableExceptionTest.kt @@ -0,0 +1,54 @@ +// SPDX-License-Identifier: GPL-3.0-or-later +package org.libremail.data.local + +import kotlinx.coroutines.CancellationException +import org.junit.Test +import kotlin.test.assertFalse +import kotlin.test.assertTrue + +/** + * [isCacheEncryptionUnavailable] is the shared classifier the headless entry points (WorkManager workers + * and `IdleService`) use to decide whether a database-open failure is the issue-#359 "SQLCipher native + * library unavailable" condition — which they defer/skip softly — versus any other failure, which they + * handle normally. It must recognise the provisioner's [CacheEncryptionUnavailableException] AND a bare + * [LinkageError] (defensive), even when wrapped several layers deep (coroutine stack-trace recovery + * re-wraps the throwable across the open boundary), and reject everything else — including + * [CancellationException], which must always propagate. + */ +class CacheEncryptionUnavailableExceptionTest { + + @Test + fun `recognises a direct CacheEncryptionUnavailableException`() { + val failure = CacheEncryptionUnavailableException(UnsatisfiedLinkError("nativeOpen")) + assertTrue(failure.isCacheEncryptionUnavailable()) + } + + @Test + fun `recognises a bare LinkageError`() { + assertTrue(UnsatisfiedLinkError("SQLiteConnection.nativeOpen").isCacheEncryptionUnavailable()) + } + + @Test + fun `recognises a CacheEncryptionUnavailableException wrapped deep in the cause chain`() { + val wrapped = RuntimeException( + "room open failed", + IllegalStateException("delegate", CacheEncryptionUnavailableException(UnsatisfiedLinkError())), + ) + assertTrue(wrapped.isCacheEncryptionUnavailable()) + } + + @Test + fun `recognises a LinkageError nested as a cause`() { + assertTrue(RuntimeException("open", UnsatisfiedLinkError("nativeOpen")).isCacheEncryptionUnavailable()) + } + + @Test + fun `rejects an unrelated exception`() { + assertFalse(IllegalStateException("database is locked").isCacheEncryptionUnavailable()) + } + + @Test + fun `rejects a CancellationException so cancellation still propagates`() { + assertFalse(CancellationException("job cancelled").isCacheEncryptionUnavailable()) + } +} diff --git a/app/src/test/kotlin/org/libremail/data/local/DatabaseProvisionerTest.kt b/app/src/test/kotlin/org/libremail/data/local/DatabaseProvisionerTest.kt index 7695bb4..3844381 100644 --- a/app/src/test/kotlin/org/libremail/data/local/DatabaseProvisionerTest.kt +++ b/app/src/test/kotlin/org/libremail/data/local/DatabaseProvisionerTest.kt @@ -71,6 +71,7 @@ class DatabaseProvisionerTest { every { DatabaseEncryption.ensureEncrypted(any(), any()) } just Runs every { DatabaseEncryption.ensurePlaintext(any(), any()) } just Runs every { DatabaseEncryption.ensureNativeLibraryLoaded() } just Runs + every { DatabaseEncryption.probeKeyedOpen(any(), any()) } just Runs every { settingsRepository.settings } returns flowOf(AppSettings()) coEvery { keyStore.isClearPending() } returns false @@ -104,6 +105,10 @@ class DatabaseProvisionerTest { // load only rode on that conversion, Room's keyed open would hit nativeOpen with no .so loaded // and throw UnsatisfiedLinkError on every cold start. verify(exactly = 1) { DatabaseEncryption.ensureNativeLibraryLoaded() } + // Issue #359 gap 2: the encrypted branch also probes a REAL keyed open (nativeOpen) inside the + // fail-closed handler, so an open-time UnsatisfiedLinkError is caught here rather than escaping + // Room's later deferred open. + verify(exactly = 1) { DatabaseEncryption.probeKeyedOpen(any(), PASSPHRASE) } } @Test @@ -147,6 +152,53 @@ class DatabaseProvisionerTest { coVerify(exactly = 0) { settingsRepository.setEncryptCache(any()) } } + @Test + fun `a native-library load failure in the account migrator fails closed`() = runTest { + // Issue #359 gap 1 (PRIMARY): AccountDataMigrator.copyAccountTables loads SQLCipher and does a + // keyed openOrCreateDatabase + ATTACH … KEY (a real nativeOpen) even when encryption is OFF, so a + // LinkageError there must fail closed too. It used to ESCAPE the handler because migrateIfNeeded() + // ran OUTSIDE the try/catch (crash-loop). Un-mock the `just Runs` default (which hid the gap) and + // make the migrator throw the exact #359 error — encryption stays at its default OFF here to prove + // the migrator drags EVERY upgrader through the native library regardless of the setting. + coEvery { accountDataMigrator.migrateIfNeeded() } throws + UnsatisfiedLinkError("dlopen failed: libsqlcipher.so is not 16 KB aligned") + + val error = assertFailsWith { provisioner().prepareCache() } + + assertTrue( + generateSequence(error.cause) { it.cause }.any { it is LinkageError }, + "the native-load LinkageError must be preserved as the cause, not surface as a raw LinkageError", + ) + // Fail CLOSED, not open: nothing wiped, no seal reset, the setting untouched. + verify(exactly = 0) { DatabaseFiles.clear(any()) } + coVerify(exactly = 0) { keyStore.resetSealedPassphrase() } + coVerify(exactly = 0) { settingsRepository.setEncryptCache(any()) } + } + + @Test + fun `a nativeOpen failure while probing the encrypted open fails closed`() = runTest { + every { settingsRepository.settings } returns flowOf(AppSettings(encryptCache = true, appLock = false)) + // Issue #359 gap 2 (SECONDARY): loadLibrary succeeds, but the REAL keyed open throws an + // UnsatisfiedLinkError at SQLiteConnection.nativeOpen — the exact #359 signature. The provisioner + // probes that open INSIDE the fail-closed handler, so it converts here rather than letting Room's + // later deferred open (DatabaseModule) crash uncaught. + every { DatabaseEncryption.probeKeyedOpen(any(), any()) } throws + UnsatisfiedLinkError("SQLiteConnection.nativeOpen") + + val error = assertFailsWith { provisioner().prepareCache() } + + assertTrue( + generateSequence(error.cause) { it.cause }.any { it is LinkageError }, + "the nativeOpen LinkageError must be preserved as the cause", + ) + // The library loaded fine (loadLibrary was not the failure); it was the keyed nativeOpen probe. + verify(exactly = 1) { DatabaseEncryption.ensureNativeLibraryLoaded() } + verify(exactly = 1) { DatabaseEncryption.probeKeyedOpen(any(), PASSPHRASE) } + // No fail-open side effects. + verify(exactly = 0) { DatabaseFiles.clear(any()) } + coVerify(exactly = 0) { keyStore.resetSealedPassphrase() } + } + @Test fun `a native-library load failure is not memoized and retries on the next open`() = runTest { every { settingsRepository.settings } returns flowOf(AppSettings(encryptCache = true, appLock = false)) @@ -237,8 +289,10 @@ class DatabaseProvisionerTest { verify(exactly = 0) { DatabaseEncryption.ensureEncrypted(any(), any()) } verify(exactly = 0) { DatabaseEncryption.ensurePlaintext(any(), any()) } // A plaintext cache opens with the framework helper, never SQLCipher, so it must not touch the - // native library — the counterpart to the encrypted path's mandatory load above. + // native library — the counterpart to the encrypted path's mandatory load above — nor probe a + // keyed open (issue #359: the probe belongs to the encrypted branch only). verify(exactly = 0) { DatabaseEncryption.ensureNativeLibraryLoaded() } + verify(exactly = 0) { DatabaseEncryption.probeKeyedOpen(any(), any()) } } @Test diff --git a/app/src/test/kotlin/org/libremail/data/repository/AccountRepositoryImplTest.kt b/app/src/test/kotlin/org/libremail/data/repository/AccountRepositoryImplTest.kt index 3e30517..c348fa5 100644 --- a/app/src/test/kotlin/org/libremail/data/repository/AccountRepositoryImplTest.kt +++ b/app/src/test/kotlin/org/libremail/data/repository/AccountRepositoryImplTest.kt @@ -2,17 +2,23 @@ package org.libremail.data.repository import android.content.Context +import android.util.Log import app.cash.turbine.test import io.mockk.Runs import io.mockk.coEvery import io.mockk.coVerify +import io.mockk.coVerifyOrder import io.mockk.every import io.mockk.just import io.mockk.mockk +import io.mockk.mockkStatic import io.mockk.slot +import io.mockk.unmockkAll import io.mockk.verify import kotlinx.coroutines.flow.flowOf import kotlinx.coroutines.test.runTest +import org.junit.After +import org.junit.Before import org.junit.Test import org.libremail.data.attachment.AttachmentUriGrants import org.libremail.data.attachmentCacheDir @@ -76,6 +82,17 @@ class AccountRepositoryImplTest { attachmentUriGrants = attachmentUriGrants, ) + // addImapAccount/addOutlookAccount now breadcrumb via AppLog (#403); android.util.Log is a no-op + // stub under plain JVM tests, so mock it class-wide so no test crashes on the unmocked method. + @Before + fun setUp() { + mockkStatic(Log::class) + every { Log.i(any(), any()) } returns 0 + } + + @After + fun tearDown() = unmockkAll() + @Test fun `observeAccounts maps the stored account rows to domain models`() = runTest { every { accountDao.observeAll() } returns flowOf(listOf(accountEntity())) @@ -134,6 +151,40 @@ class AccountRepositoryImplTest { verify { syncScheduler.backfillNow() } } + @Test + fun `addImapAccount persists the credential before the account row (issue 403)`() = runTest { + val account = account() + coEvery { imapClient.listFolders(any()) } returns listOf( + FetchedFolder("INBOX", "INBOX", emptyList(), selectable = true), + ) + coEvery { accountDao.insertAtEnd(any()) } just Runs + + repository.addImapAccount(account, "app-password").getOrThrow() + + // The push collector (LibreMailApplication) and IdleService.reconcileWatchers both react to the + // accounts table; the secret must be committed FIRST so a watcher observing the new row can + // resolve it, rather than logging a transient "No stored credentials" IDLE miss on every add. + coVerifyOrder { + credentialStore.saveSecret(account.id, "app-password") + accountDao.insertAtEnd(any()) + } + } + + @Test + fun `addOutlookAccount persists the credential before the account row (issue 403)`() = runTest { + coEvery { imapClient.listFolders(any()) } returns listOf( + FetchedFolder("INBOX", "INBOX", emptyList(), selectable = true), + ) + coEvery { accountDao.insertAtEnd(any()) } just Runs + + repository.addOutlookAccount("me@outlook.com", "access-token", "{authstate}").getOrThrow() + + coVerifyOrder { + credentialStore.saveSecret("outlook:me@outlook.com", "{authstate}") + accountDao.insertAtEnd(any()) + } + } + @Test fun `addImapAccount persists nothing when the initial folder list fails`() = runTest { coEvery { imapClient.listFolders(any()) } throws RuntimeException("bad credentials") diff --git a/app/src/test/kotlin/org/libremail/data/sync/BackfillWorkerTest.kt b/app/src/test/kotlin/org/libremail/data/sync/BackfillWorkerTest.kt index 0550f6c..8652c05 100644 --- a/app/src/test/kotlin/org/libremail/data/sync/BackfillWorkerTest.kt +++ b/app/src/test/kotlin/org/libremail/data/sync/BackfillWorkerTest.kt @@ -15,6 +15,7 @@ import kotlinx.coroutines.test.runTest import org.junit.After import org.junit.Before import org.junit.Test +import org.libremail.data.local.CacheEncryptionUnavailableException import org.libremail.data.security.EncryptedCacheGuard import org.libremail.reporting.AppLog import org.libremail.reporting.RingLogBuffer @@ -86,6 +87,22 @@ class BackfillWorkerTest { assertEquals(Result.retry(), worker().doWork()) } + @Test + fun `defers with a retry and a distinct breadcrumb when the encrypted cache is unavailable`() = runTest { + // Issue #359: the DB open fails because SQLCipher's native library is unavailable. It lands in the + // worker's runCatching (thrown inside runBackfill()), so it retries — but it must now log a DISTINCT + // breadcrumb (not the generic retry, and not mistaken for a cancellation) and still never crash. + coEvery { cacheGuard.isCacheLocked() } returns false + coEvery { backfiller.runBackfill(any()) } throws + CacheEncryptionUnavailableException(UnsatisfiedLinkError("SQLiteConnection.nativeOpen")) + + assertEquals(Result.retry(), worker().doWork()) + + val entry = logBuffer.snapshot().single() + assertEquals('W', entry.level) + assertTrue(entry.message.startsWith("backfill deferred: encrypted cache unavailable"), entry.message) + } + // --- issue #329: AppLog breadcrumbs --------------------------------------------------------- @Test diff --git a/app/src/test/kotlin/org/libremail/data/sync/MailConnectionFactoryTest.kt b/app/src/test/kotlin/org/libremail/data/sync/MailConnectionFactoryTest.kt index 5256161..5d846f7 100644 --- a/app/src/test/kotlin/org/libremail/data/sync/MailConnectionFactoryTest.kt +++ b/app/src/test/kotlin/org/libremail/data/sync/MailConnectionFactoryTest.kt @@ -80,7 +80,9 @@ class MailConnectionFactoryTest { fun `a missing password credential is a hard error`() = runTest { coEvery { credentialStore.loadSecret("acct") } returns null - assertFailsWith { factory().imapParamsFor(passwordAccount) } + // A typed MissingCredentialsException (#403), not a bare IllegalStateException, so the IMAP IDLE + // watcher can catch it specifically and defer on the account-add write race. + assertFailsWith { factory().imapParamsFor(passwordAccount) } } @Test @@ -170,6 +172,6 @@ class MailConnectionFactoryTest { fun `a missing OAuth credential is a hard error`() = runTest { coEvery { credentialStore.loadSecret(outlookAccount.id) } returns null - assertFailsWith { factory().graphTokenFor(outlookAccount) } + assertFailsWith { factory().graphTokenFor(outlookAccount) } } } diff --git a/app/src/test/kotlin/org/libremail/data/sync/PruneWorkerTest.kt b/app/src/test/kotlin/org/libremail/data/sync/PruneWorkerTest.kt index 45fc95d..e91c9db 100644 --- a/app/src/test/kotlin/org/libremail/data/sync/PruneWorkerTest.kt +++ b/app/src/test/kotlin/org/libremail/data/sync/PruneWorkerTest.kt @@ -15,6 +15,7 @@ import kotlinx.coroutines.test.runTest import org.junit.After import org.junit.Before import org.junit.Test +import org.libremail.data.local.CacheEncryptionUnavailableException import org.libremail.data.security.EncryptedCacheGuard import org.libremail.reporting.AppLog import org.libremail.reporting.RingLogBuffer @@ -84,6 +85,23 @@ class PruneWorkerTest { assertEquals(Result.retry(), worker().doWork()) } + @Test + fun `defers with a retry and a distinct breadcrumb when the encrypted cache is unavailable`() = runTest { + // Issue #359: the DB open fails because SQLCipher's native library is unavailable. It lands in the + // worker's runCatching (thrown inside prune()), so it already retried — but it must now log a + // DISTINCT breadcrumb so a debug report shows "encrypted cache unavailable" rather than a generic + // retry, and still never crash. + coEvery { cacheGuard.isCacheLocked() } returns false + coEvery { pruner.prune(any()) } throws + CacheEncryptionUnavailableException(UnsatisfiedLinkError("SQLiteConnection.nativeOpen")) + + assertEquals(Result.retry(), worker().doWork()) + + val entry = logBuffer.snapshot().single() + assertEquals('W', entry.level) + assertTrue(entry.message.startsWith("prune deferred: encrypted cache unavailable"), entry.message) + } + // --- issue #329: AppLog breadcrumbs --------------------------------------------------------- @Test diff --git a/app/src/test/kotlin/org/libremail/data/sync/SendWorkerTest.kt b/app/src/test/kotlin/org/libremail/data/sync/SendWorkerTest.kt index 1326337..a272323 100644 --- a/app/src/test/kotlin/org/libremail/data/sync/SendWorkerTest.kt +++ b/app/src/test/kotlin/org/libremail/data/sync/SendWorkerTest.kt @@ -19,6 +19,7 @@ import org.junit.After import org.junit.Before import org.junit.Test import org.libremail.data.attachment.AttachmentUriGrants +import org.libremail.data.local.CacheEncryptionUnavailableException import org.libremail.data.local.dao.AccountDao import org.libremail.data.local.dao.OutboxDao import org.libremail.data.local.entity.AccountEntity @@ -139,6 +140,21 @@ class SendWorkerTest { verify(exactly = 0) { lazyGrants.get() } } + @Test + fun `defers with a retry when the encrypted cache is unavailable, without crashing`() = runTest { + // Cache unlocked (setUp), so the worker resolves its Lazy DB deps and reads the outbox; that first + // DB access (Room's deferred cache open) throws because SQLCipher's native library is unavailable + // on this device (issue #359). It is OUTSIDE any per-message runCatching, so without the guard it + // would escape doWork — the guard turns it into a soft retry with a PII-free breadcrumb. + coEvery { outboxDao.getAll() } throws + CacheEncryptionUnavailableException(UnsatisfiedLinkError("SQLiteConnection.nativeOpen")) + + assertEquals(Result.retry(), worker().doWork()) + + val messages = logBuffer.snapshot().map { it.message } + assertTrue(messages.any { it.startsWith("deferred: encrypted cache unavailable") }, "messages=$messages") + } + @Test fun `an empty outbox succeeds without sending`() = runTest { coEvery { outboxDao.getAll() } returns emptyList() diff --git a/app/src/test/kotlin/org/libremail/data/sync/SyncWorkerTest.kt b/app/src/test/kotlin/org/libremail/data/sync/SyncWorkerTest.kt index 17faa97..f7126d4 100644 --- a/app/src/test/kotlin/org/libremail/data/sync/SyncWorkerTest.kt +++ b/app/src/test/kotlin/org/libremail/data/sync/SyncWorkerTest.kt @@ -15,6 +15,7 @@ import kotlinx.coroutines.test.runTest import org.junit.After import org.junit.Before import org.junit.Test +import org.libremail.data.local.CacheEncryptionUnavailableException import org.libremail.data.security.EncryptedCacheGuard import org.libremail.reporting.AppLog import org.libremail.reporting.RingLogBuffer @@ -82,6 +83,24 @@ class SyncWorkerTest { assertEquals(ListenableWorker.Result.retry(), worker().doWork()) } + // --- issue #359: headless tolerance when SQLCipher's native library is unavailable ------------ + + @Test + fun `defers with a retry when the encrypted cache is unavailable, without crashing`() = runTest { + coEvery { cacheGuard.isCacheLocked() } returns false + // The cache is unlocked, so the worker resolves MailSyncer; its first DB access (Room's deferred + // cache open) throws because SQLCipher's native library is unavailable on this device (issue #359). + // That open is OUTSIDE syncAll's own runCatching, so without the guard it would escape doWork. + coEvery { mailSyncer.syncAll() } throws + CacheEncryptionUnavailableException(UnsatisfiedLinkError("SQLiteConnection.nativeOpen")) + + assertEquals(ListenableWorker.Result.retry(), worker().doWork()) + + val entry = logBuffer.snapshot().single() + assertEquals('W', entry.level) + assertTrue(entry.message.startsWith("deferred: encrypted cache unavailable"), entry.message) + } + // --- issue #329: AppLog breadcrumbs --------------------------------------------------------- @Test diff --git a/app/src/test/kotlin/org/libremail/ui/accountsetup/AccountPickerScreenJvmTest.kt b/app/src/test/kotlin/org/libremail/ui/accountsetup/AccountPickerScreenJvmTest.kt index 33b1557..6572617 100644 --- a/app/src/test/kotlin/org/libremail/ui/accountsetup/AccountPickerScreenJvmTest.kt +++ b/app/src/test/kotlin/org/libremail/ui/accountsetup/AccountPickerScreenJvmTest.kt @@ -101,6 +101,7 @@ class AccountPickerScreenJvmTest { onAccountAdded: (String) -> Unit = {}, onPickProvider: (MailProvider) -> Unit = {}, onManualSetup: () -> Unit = {}, + onPickOutlook: (() -> Unit)? = null, ) { composeTestRule.setContent { CompositionLocalProvider( @@ -113,6 +114,7 @@ class AccountPickerScreenJvmTest { onAccountAdded = onAccountAdded, onPickProvider = onPickProvider, onManualSetup = onManualSetup, + onPickOutlook = onPickOutlook, viewModel = viewModel, ) } @@ -185,6 +187,20 @@ class AccountPickerScreenJvmTest { verify { vm.onOutlookLaunchFailed(any()) } } + @Test + fun tappingOutlook_whenOnPickOutlookProvided_delegatesInsteadOfLaunching() { + // Onboarding passes onPickOutlook to interpose the IMAP-enablement notice (#411): the tap + // must route there, NOT build/launch the auth intent inline. + val vm = viewModel() + var outlookPicked = false + setContent(vm, onPickOutlook = { outlookPicked = true }) + + composeTestRule.onNodeWithText(string(R.string.account_setup_outlook)).performClick() + + assertTrue("Outlook tap must delegate to onPickOutlook", outlookPicked) + verify(exactly = 0) { vm.outlookAuthIntent() } + } + @Test fun doneStatus_reportsTheNewAccountIdToOnAccountAdded() { var addedId: String? = null diff --git a/app/src/test/kotlin/org/libremail/ui/onboarding/OutlookImapNoticeScreenJvmTest.kt b/app/src/test/kotlin/org/libremail/ui/onboarding/OutlookImapNoticeScreenJvmTest.kt new file mode 100644 index 0000000..c0f286c --- /dev/null +++ b/app/src/test/kotlin/org/libremail/ui/onboarding/OutlookImapNoticeScreenJvmTest.kt @@ -0,0 +1,246 @@ +// SPDX-License-Identifier: GPL-3.0-or-later +package org.libremail.ui.onboarding + +import android.content.ActivityNotFoundException +import android.content.Context +import android.content.Intent +import androidx.activity.compose.LocalActivityResultRegistryOwner +import androidx.activity.result.ActivityResultRegistry +import androidx.activity.result.ActivityResultRegistryOwner +import androidx.activity.result.contract.ActivityResultContract +import androidx.compose.runtime.CompositionLocalProvider +import androidx.compose.ui.platform.LocalUriHandler +import androidx.compose.ui.platform.UriHandler +import androidx.compose.ui.semantics.ProgressBarRangeInfo +import androidx.compose.ui.test.assertIsDisplayed +import androidx.compose.ui.test.assertIsNotEnabled +import androidx.compose.ui.test.hasProgressBarRangeInfo +import androidx.compose.ui.test.junit4.v2.createComposeRule +import androidx.compose.ui.test.onAllNodesWithText +import androidx.compose.ui.test.onNodeWithContentDescription +import androidx.compose.ui.test.onNodeWithText +import androidx.compose.ui.test.performClick +import androidx.core.app.ActivityOptionsCompat +import androidx.lifecycle.Lifecycle +import androidx.lifecycle.LifecycleOwner +import androidx.lifecycle.LifecycleRegistry +import androidx.lifecycle.compose.LocalLifecycleOwner +import io.mockk.every +import io.mockk.mockk +import io.mockk.verify +import kotlinx.coroutines.flow.MutableStateFlow +import org.junit.Assert.assertEquals +import org.junit.Assert.assertTrue +import org.junit.Rule +import org.junit.Test +import org.junit.runner.RunWith +import org.libremail.R +import org.libremail.ui.accountsetup.AccountSetupUiState +import org.libremail.ui.accountsetup.AccountSetupViewModel +import org.libremail.ui.accountsetup.SetupStatus +import org.libremail.ui.theme.LibreMailTheme +import org.robolectric.RobolectricTestRunner +import org.robolectric.RuntimeEnvironment +import org.robolectric.annotation.Config +import org.robolectric.annotation.GraphicsMode + +/** + * Robolectric JVM Compose test for the pre-auth Outlook IMAP-enablement notice (#411). Drives the + * real [OutlookImapNoticeScreen] on the JVM via the v2 `createComposeRule()` under + * [RobolectricTestRunner] — no emulator — so the screen counts toward JaCoCo's JVM-testable surface. + * The instrumented [OutlookImapNoticeScreenTest] stays as the on-device E2E (with Espresso-Intents it + * owns the real browser/AppAuth launch this JVM rule cannot safely surface). + * + * [AccountSetupViewModel] is mocked (its own logic is covered by `AccountSetupViewModelTest`); a + * recording [UriHandler] captures the help/settings link launches and a no-op + * [ActivityResultRegistry] lets the "Sign in" launcher register/launch without a real activity. + */ +@RunWith(RobolectricTestRunner::class) +@GraphicsMode(GraphicsMode.Mode.NATIVE) +// A tall display so the whole scrolling column fits Robolectric's small default viewport, keeping +// every control on-screen for assertIsDisplayed / performClick without scrolling. +@Config(sdk = [36], qualifiers = "+w411dp-h2000dp") +class OutlookImapNoticeScreenJvmTest { + + @get:Rule + val composeTestRule = createComposeRule() + + private val context: Context get() = RuntimeEnvironment.getApplication() + + private fun string(resId: Int): String = context.getString(resId) + + /** RESUMED owner so `collectAsStateWithLifecycle` collects the view-model state. */ + private val resumedOwner = object : LifecycleOwner { + private val registry = + LifecycleRegistry.createUnsafe(this).apply { currentState = Lifecycle.State.RESUMED } + override val lifecycle: Lifecycle get() = registry + } + + /** A no-op registry so the "Sign in" launcher can register/launch without a real activity. */ + private val noopRegistryOwner = object : ActivityResultRegistryOwner { + override val activityResultRegistry = object : ActivityResultRegistry() { + override fun onLaunch( + requestCode: Int, + contract: ActivityResultContract, + input: I, + options: ActivityOptionsCompat?, + ) { + // Intentionally never dispatch a result: the launch is a no-op in this JVM test. + } + } + } + + /** Records outbound link launches so the screen's help/settings links are exercised. */ + private val openedUrls = mutableListOf() + private val recordingUriHandler = object : UriHandler { + override fun openUri(uri: String) { + openedUrls.add(uri) + } + } + + /** A handler that always fails, to drive the "couldn't open your browser" snackbar path. */ + private val throwingUriHandler = object : UriHandler { + override fun openUri(uri: String): Unit = throw ActivityNotFoundException("no browser") + } + + private fun viewModel(state: AccountSetupUiState = AccountSetupUiState()): AccountSetupViewModel { + val vm = mockk(relaxed = true) + every { vm.state } returns MutableStateFlow(state) + return vm + } + + private fun setContent( + viewModel: AccountSetupViewModel, + uriHandler: UriHandler = recordingUriHandler, + onBack: () -> Unit = {}, + onAccountAdded: (String) -> Unit = {}, + ) { + composeTestRule.setContent { + CompositionLocalProvider( + LocalLifecycleOwner provides resumedOwner, + LocalActivityResultRegistryOwner provides noopRegistryOwner, + LocalUriHandler provides uriHandler, + ) { + LibreMailTheme(darkTheme = false, dynamicColor = false) { + OutlookImapNoticeScreen( + onBack = onBack, + onAccountAdded = onAccountAdded, + viewModel = viewModel, + ) + } + } + } + } + + @Test + fun rendersImapQuestion_bothLinks_andSignIn() { + setContent(viewModel()) + + composeTestRule.onNodeWithText(string(R.string.outlook_imap_title)).assertIsDisplayed() + composeTestRule.onNodeWithText(string(R.string.outlook_imap_question)).assertIsDisplayed() + composeTestRule.onNodeWithText(string(R.string.outlook_imap_body)).assertIsDisplayed() + composeTestRule.onNodeWithText(string(R.string.outlook_imap_help)).assertIsDisplayed() + composeTestRule.onNodeWithText(string(R.string.outlook_imap_settings)).assertIsDisplayed() + composeTestRule.onNodeWithText(string(R.string.outlook_imap_sign_in)).assertIsDisplayed() + } + + @Test + fun tappingHelpLink_opensTheMicrosoftArticle() { + setContent(viewModel()) + + composeTestRule.onNodeWithText(string(R.string.outlook_imap_help)).performClick() + + assertTrue( + "Help link must open a support.microsoft.com article", + openedUrls.any { it.startsWith("https://support.microsoft.com/") }, + ) + } + + @Test + fun tappingSettingsLink_opensOutlookSettings() { + setContent(viewModel()) + + composeTestRule.onNodeWithText(string(R.string.outlook_imap_settings)).performClick() + + assertTrue( + "Settings link must open an outlook.live.com settings page", + openedUrls.any { it.startsWith("https://outlook.live.com/") }, + ) + } + + @Test + fun tappingSignIn_buildsTheAuthIntentAndLaunchesIt() { + val vm = viewModel() + // A real (empty) Intent is safe here: the no-op registry never actually starts it. + every { vm.outlookAuthIntent() } returns Result.success(Intent()) + setContent(vm) + + composeTestRule.onNodeWithText(string(R.string.outlook_imap_sign_in)).performClick() + + verify { vm.outlookAuthIntent() } + } + + @Test + fun tappingSignIn_whenTheIntentCannotBeBuilt_reportsLaunchFailure() { + val vm = viewModel() + every { vm.outlookAuthIntent() } returns Result.failure(ActivityNotFoundException("no browser")) + setContent(vm) + + composeTestRule.onNodeWithText(string(R.string.outlook_imap_sign_in)).performClick() + + verify { vm.onOutlookLaunchFailed(any()) } + } + + @Test + fun doneStatus_reportsTheNewAccountIdToOnAccountAdded() { + var addedId: String? = null + setContent( + viewModel(AccountSetupUiState(status = SetupStatus.DONE, addedAccountId = "outlook:me@outlook.com")), + onAccountAdded = { addedId = it }, + ) + + composeTestRule.waitUntil(5_000) { addedId != null } + assertEquals("outlook:me@outlook.com", addedId) + } + + @Test + fun anError_isSurfacedAsASnackbar() { + setContent(viewModel(AccountSetupUiState(error = "Microsoft sign-in failed"))) + + composeTestRule.waitUntil(5_000) { + composeTestRule.onAllNodesWithText("Microsoft sign-in failed").fetchSemanticsNodes().isNotEmpty() + } + composeTestRule.onNodeWithText("Microsoft sign-in failed").assertIsDisplayed() + } + + @Test + fun connectingStatus_showsBusySpinner_andDisablesSignIn() { + setContent(viewModel(AccountSetupUiState(status = SetupStatus.CONNECTING))) + + composeTestRule.onNode(hasProgressBarRangeInfo(ProgressBarRangeInfo.Indeterminate)).assertIsDisplayed() + composeTestRule.onNodeWithText(string(R.string.outlook_imap_sign_in)).assertIsNotEnabled() + } + + @Test + fun tappingBack_invokesOnBack() { + var backed = false + setContent(viewModel(), onBack = { backed = true }) + + composeTestRule.onNodeWithContentDescription(string(R.string.action_back)).performClick() + + assertTrue(backed) + } + + @Test + fun openingALink_whenNoBrowser_surfacesTheOpenFailedSnackbar() { + setContent(viewModel(), uriHandler = throwingUriHandler) + + composeTestRule.onNodeWithText(string(R.string.outlook_imap_help)).performClick() + + val openFailed = string(R.string.app_password_open_failed) + composeTestRule.waitUntil(5_000) { + composeTestRule.onAllNodesWithText(openFailed).fetchSemanticsNodes().isNotEmpty() + } + composeTestRule.onNodeWithText(openFailed).assertIsDisplayed() + } +} diff --git a/config/detekt/detekt.yml b/config/detekt/detekt.yml index 0b34fe2..f190f3c 100644 --- a/config/detekt/detekt.yml +++ b/config/detekt/detekt.yml @@ -74,6 +74,9 @@ style: # log via AppLog, so their unit tests mockkStatic(Log) too. - '**/data/repository/MailRepositoryImplTest.kt' - '**/data/repository/MailRepositoryImplCoverageTest.kt' + # Account-add breadcrumb (issue #403): addImapAccount/addOutlookAccount log via AppLog, so this + # suite mockkStatic(Log) so the calls don't crash on the throwing JVM stub. + - '**/data/repository/AccountRepositoryImplTest.kt' - '**/ui/reader/ReaderViewModelTest.kt' - '**/ui/reader/ReaderViewModelActionsTest.kt' MagicNumber: