fix(security): app-lock lifecycle consistency (grace across Back, passphrase eviction limits) #119

Merged
JMR-dev merged 2 commits from fix-applock-lifecycle into main 2026-07-02 08:50:15 +00:00
6 changed files with 189 additions and 9 deletions
@@ -15,6 +15,14 @@ enum class LockState { LOCKED, UNLOCKED }
* - Returning to the foreground within [graceMillis] of backgrounding stays unlocked; after the
* grace period it re-locks. Foregrounding without a preceding background (e.g. a configuration
* change / rotation) does not re-lock an already-unlocked session.
*
* Lifetime: provided as an application-scoped singleton (see `SecurityModule`) so the grace window
* survives Activity recreation. If this state lived in the Activity-scoped [org.libremail.ui.lock
* .AppLockViewModel]'s own field, Back on the task root — which finishes the Activity and clears its
* ViewModelStore on API 29/30 — would drop the [backgroundedAt] marker and re-arm a fresh LOCKED gate,
* so returning within the grace period would wrongly demand re-auth (unlike leaving via Home). Being
* process-scoped, the same instance is reused across recreation, while a genuine cold start (process
* death) constructs a fresh one that correctly starts [LockState.LOCKED].
*/
class AppLockGate(private val graceMillis: Long = DEFAULT_GRACE_MILLIS) {
@@ -12,12 +12,25 @@ import javax.inject.Singleton
*
* When app-lock is enabled the database passphrase is sealed by an auth-bound Keystore key
* ([DatabaseKeyCipher]) and is only recoverable after the user passes a `BiometricPrompt`. The
* unwrapped passphrase is kept here for the lifetime of the unlocked session — never persisted — and
* cleared on lock, timeout, or when app-lock is disabled. `DatabaseModule` reads it through [await]
* so the encrypted database is opened only after authentication.
* unwrapped passphrase is kept here for the unlocked session — never persisted. `DatabaseModule`
* reads it through [await] so the encrypted database is opened only after authentication.
*
* The value is an immutable [String] to stay consistent with the rest of the passphrase plumbing; it
* cannot be zeroed in place, which is an accepted limitation of the existing design.
* Eviction — current limitation: the passphrase is NOT guaranteed to leave the process when the app
* re-locks. [lock] is called on app-lock disable and cache reset ([DatabaseKeyStore]), but it is
* deliberately NOT called on the app-lock UI gate's grace-expiry/timeout re-lock, and even if it were
* it would only drop THIS holder's reference:
* - the value is an immutable [String] and cannot be zeroed in place — [lock] merely releases it
* for GC;
* - once the encrypted cache has been opened, SQLCipher keeps the key inside the already-open
* Room/native handle. `DatabaseModule.provideDatabase` runs once per process, so nothing here
* closes that handle; the key stays resident for the process lifetime regardless of [lock];
* - this holder's unlocked-ness also drives [EncryptedCacheGuard], so clearing it while the app is
* merely locked (not exited) would stall background sync/push even though the DB is still open.
*
* Truly evicting the passphrase on lock/timeout therefore requires a database close/reopen mechanism,
* which is owned by the DB-lifecycle work in issues #93 (provideDatabase) and #111 (DB
* re-architecture) and is intentionally out of scope here. Until those land, do not rely on [lock]
* for cryptographic erasure of the passphrase from the process.
*/
@Singleton
class PassphraseSession @Inject constructor() {
@@ -29,7 +42,11 @@ class PassphraseSession @Inject constructor() {
passphrase.value = value
}
/** Clear the passphrase from memory (on lock, timeout, or app-lock disable). */
/**
* Drop this holder's reference to the passphrase (on app-lock disable / cache reset). See the
* class KDoc: this is a partial eviction only — it does not close the open database, so SQLCipher
* keeps the key resident for the process lifetime (full eviction is deferred to #93 / #111).
*/
fun lock() {
passphrase.value = null
}
@@ -3,9 +3,11 @@ package org.libremail.di
import dagger.Binds
import dagger.Module
import dagger.Provides
import dagger.hilt.InstallIn
import dagger.hilt.components.SingletonComponent
import org.libremail.data.security.AndroidAppLockManager
import org.libremail.data.security.AppLockGate
import org.libremail.data.security.AppLockManager
import javax.inject.Singleton
@@ -16,4 +18,16 @@ abstract class SecurityModule {
@Binds
@Singleton
abstract fun bindAppLockManager(impl: AndroidAppLockManager): AppLockManager
companion object {
/**
* The app-lock UI gate is application-scoped, NOT Activity/ViewModel-scoped, so its grace
* window survives Activity recreation (e.g. Back finishing the task root on API 29/30, which
* clears the Activity's ViewModelStore). See [AppLockGate] for why this must outlive the
* ViewModel. A fresh process constructs a new instance that correctly starts LOCKED.
*/
@Provides
@Singleton
fun provideAppLockGate(): AppLockGate = AppLockGate()
}
}
@@ -73,10 +73,13 @@ class AppLockViewModel @Inject constructor(
private val databaseKeyCipher: DatabaseKeyCipher,
private val session: PassphraseSession,
private val syncScheduler: SyncScheduler,
// Application-scoped (see SecurityModule): the gate is injected rather than owned by this
// Activity-scoped ViewModel so the inactivity grace window survives Activity recreation — Back on
// the task root finishes the Activity and clears its ViewModelStore on API 29/30, which would
// otherwise drop the grace marker and force a full re-auth on return within the grace period.
private val gate: AppLockGate,
) : ViewModel() {
private val gate = AppLockGate()
private val _uiState = MutableStateFlow<AppLockUiState>(AppLockUiState.Checking)
val uiState: StateFlow<AppLockUiState> = _uiState.asStateFlow()
@@ -134,7 +137,17 @@ class AppLockViewModel @Inject constructor(
LockAction.REQUIRE_AUTH -> {
val state = gate.onForeground(foregroundAt, appLockEnabled = true)
if (state == LockState.UNLOCKED) _uiState.value = AppLockUiState.Unlocked else emitLocked()
if (state == LockState.UNLOCKED) {
_uiState.value = AppLockUiState.Unlocked
} else {
// Re-lock on grace expiry (timeout). The SQLCipher passphrase is intentionally
// NOT evicted here: PassphraseSession is the only separately-held copy, but
// clearing it flips EncryptedCacheGuard to "locked" and would stall background
// sync/push while locked, and the already-open Room handle keeps the key
// resident regardless. Full eviction needs the DB close/reopen owned by #93 /
// #111 — see PassphraseSession's KDoc.
emitLocked()
}
}
}
}
@@ -100,4 +100,45 @@ class AppLockGateTest {
gate.onForeground(now = 1_000L, appLockEnabled = true) // stale -> LOCKED, marker retained
assertEquals(LockState.LOCKED, gate.onForeground(now = 3_000L, appLockEnabled = true))
}
@Test
fun `grace survives activity recreation when the gate instance is reused (app-scoped singleton)`() {
// #101: Back on the task root (API 29/30) finishes the Activity and clears its ViewModelStore,
// so AppLockViewModel is destroyed and re-created. Because the gate is application-scoped the
// SAME instance is reused across that recreation: the background marker recorded before the
// store was cleared is still present, so returning within grace stays unlocked — identical to
// leaving via Home and returning.
val survivingGate = AppLockGate(grace)
survivingGate.onForeground(now = 0, appLockEnabled = true)
survivingGate.onAuthenticated()
survivingGate.onBackground(now = 1_000) // ON_STOP as Back finishes the Activity
// ...Activity destroyed + re-created; the same singleton gate is handed to the new ViewModel...
assertEquals(
LockState.UNLOCKED,
survivingGate.onForeground(now = 1_000 + grace, appLockEnabled = true),
)
}
@Test
fun `grace still expires across activity recreation when the reused gate is out of the window`() {
val survivingGate = AppLockGate(grace)
survivingGate.onForeground(now = 0, appLockEnabled = true)
survivingGate.onAuthenticated()
survivingGate.onBackground(now = 1_000)
assertEquals(
LockState.LOCKED,
survivingGate.onForeground(now = 1_000 + grace + 1, appLockEnabled = true),
)
}
@Test
fun `a fresh gate (process death, or the pre-fix Activity-scoped bug) starts locked`() {
// Contrast with the reused instance above: if the gate were Activity/ViewModel-scoped (the
// pre-fix bug) or after a genuine cold start (process death), recreation builds a FRESH gate.
// It must start LOCKED and stay locked on the first foreground even within the grace window —
// the re-auth-on-Back inconsistency #101 fixes, and the correct cold-start behavior.
val freshGate = AppLockGate(grace)
assertEquals(LockState.LOCKED, freshGate.state)
assertEquals(LockState.LOCKED, freshGate.onForeground(now = 1_000, appLockEnabled = true))
}
}
@@ -0,0 +1,87 @@
// SPDX-License-Identifier: GPL-3.0-or-later
package org.libremail.ui.lock
import android.os.SystemClock
import io.mockk.every
import io.mockk.mockk
import io.mockk.mockkStatic
import io.mockk.unmockkAll
import io.mockk.verify
import kotlinx.coroutines.Dispatchers
import kotlinx.coroutines.ExperimentalCoroutinesApi
import kotlinx.coroutines.flow.flowOf
import kotlinx.coroutines.test.UnconfinedTestDispatcher
import kotlinx.coroutines.test.resetMain
import kotlinx.coroutines.test.runTest
import kotlinx.coroutines.test.setMain
import org.junit.After
import org.junit.Before
import org.junit.Test
import org.libremail.data.security.AppLockGate
import org.libremail.data.settings.AppSettings
import org.libremail.data.settings.SettingsRepository
import kotlin.test.assertEquals
import kotlin.test.assertIs
/**
* Wiring guard for #101: the app-lock gate must be an INJECTED, application-scoped dependency so its
* grace window survives Activity recreation — NOT a field the Activity-scoped ViewModel constructs
* itself (which Back on the task root would drop on API 29/30). These tests exercise the synchronous
* paths that delegate to the injected gate; the grace math itself is covered exhaustively — and
* deterministically — by AppLockGateTest. Broader ViewModel coverage is issue #100.
*/
@OptIn(ExperimentalCoroutinesApi::class)
class AppLockViewModelTest {
private val dispatcher = UnconfinedTestDispatcher()
@Before
fun setUp() = Dispatchers.setMain(dispatcher)
@After
fun tearDown() {
Dispatchers.resetMain()
unmockkAll()
}
private fun viewModel(gate: AppLockGate, appLock: Boolean = true): AppLockViewModel {
val settings = mockk<SettingsRepository>()
every { settings.settings } returns flowOf(AppSettings(appLock = appLock))
return AppLockViewModel(
context = mockk(relaxed = true),
settingsRepository = settings,
appLockManager = mockk(relaxed = true),
databaseKeyStore = mockk(relaxed = true),
databaseKeyCipher = mockk(relaxed = true),
session = mockk(relaxed = true),
syncScheduler = mockk(relaxed = true),
gate = gate,
)
}
@Test
fun `onBackground records the background on the injected gate`() = runTest(dispatcher) {
mockkStatic(SystemClock::class)
every { SystemClock.elapsedRealtime() } returns 5_000L
val gate = mockk<AppLockGate>(relaxed = true)
viewModel(gate).onBackground()
// Delegating to the gate passed into the constructor (an injected, shareable singleton) rather
// than an internally-constructed one is the whole point of the #101 hoist: the same instance
// must survive Activity recreation so the grace marker persists.
verify { gate.onBackground(5_000L) }
}
@Test
fun `onAuthError re-locks through the injected gate and surfaces the error`() = runTest(dispatcher) {
val gate = mockk<AppLockGate>(relaxed = true)
val vm = viewModel(gate)
vm.onAuthError("boom")
verify { gate.lock() }
val state = assertIs<AppLockUiState.Locked>(vm.uiState.value)
assertEquals("boom", state.error)
}
}