From 3d4e257323d504bdbc9eae28d5f3463b43271862 Mon Sep 17 00:00:00 2001 From: Davis Wolfermann Date: Wed, 2 Sep 2026 13:57:56 +0200 Subject: [PATCH 01/35] feat(security): end the session when the app leaves the foreground --- core/security/build.gradle.kts | 1 + .../core/security/data/SessionLockObserver.kt | 27 +++++++++++++++++++ gradle/libs.versions.toml | 1 + 3 files changed, 29 insertions(+) create mode 100644 core/security/src/main/kotlin/de/davis/keygo/core/security/data/SessionLockObserver.kt diff --git a/core/security/build.gradle.kts b/core/security/build.gradle.kts index 8f2ecd7a9..aa808cb96 100644 --- a/core/security/build.gradle.kts +++ b/core/security/build.gradle.kts @@ -12,6 +12,7 @@ android { dependencies { implementation(libs.androidx.biometric) + implementation(libs.androidx.lifecycle.process) implementation(projects.core.item) api(projects.core.util) diff --git a/core/security/src/main/kotlin/de/davis/keygo/core/security/data/SessionLockObserver.kt b/core/security/src/main/kotlin/de/davis/keygo/core/security/data/SessionLockObserver.kt new file mode 100644 index 000000000..53a1292f8 --- /dev/null +++ b/core/security/src/main/kotlin/de/davis/keygo/core/security/data/SessionLockObserver.kt @@ -0,0 +1,27 @@ +package de.davis.keygo.core.security.data + +import androidx.lifecycle.DefaultLifecycleObserver +import androidx.lifecycle.LifecycleOwner +import androidx.lifecycle.ProcessLifecycleOwner +import de.davis.keygo.core.security.domain.Session +import org.koin.core.annotation.Single + +/** + * Ends the session the instant the app leaves the foreground, so returning to it requires + * re-authentication rather than staying unlocked indefinitely. Registered once per process, the + * same pattern [de.davis.keygo.feature.backup.domain.BackupEscrowReconciler] already uses for its + * own process-start hook. + */ +@Single(createdAtStart = true) +internal class SessionLockObserver( + private val session: Session, +) : DefaultLifecycleObserver { + + init { + ProcessLifecycleOwner.get().lifecycle.addObserver(this) + } + + override fun onStop(owner: LifecycleOwner) { + session.endSession() + } +} diff --git a/gradle/libs.versions.toml b/gradle/libs.versions.toml index 053038475..280e2c0cc 100644 --- a/gradle/libs.versions.toml +++ b/gradle/libs.versions.toml @@ -59,6 +59,7 @@ androidx-core-splashscreen = { group = "androidx.core", name = "core-splashscree androidx-junit = { group = "androidx.test.ext", name = "junit", version.ref = "junitVersion" } androidx-espresso-core = { group = "androidx.test.espresso", name = "espresso-core", version.ref = "espressoCore" } androidx-lifecycle-runtime-ktx = { group = "androidx.lifecycle", name = "lifecycle-runtime-ktx", version.ref = "lifecycleRuntimeKtx" } +androidx-lifecycle-process = { group = "androidx.lifecycle", name = "lifecycle-process", version.ref = "lifecycleRuntimeKtx" } androidx-activity-compose = { group = "androidx.activity", name = "activity-compose", version.ref = "activityCompose" } androidx-compose-bom = { group = "androidx.compose", name = "compose-bom", version.ref = "composeBom" } androidx-compose-runtime = { group = "androidx.compose.runtime", name = "runtime" } From bbbdcd175d7893f43a982cb6f8af8d6487d1c915 Mon Sep 17 00:00:00 2001 From: Davis Wolfermann Date: Wed, 2 Sep 2026 14:07:12 +0200 Subject: [PATCH 02/35] feat(app): expose a transition-only isLocked signal from AppViewModel --- app/build.gradle.kts | 4 + .../keygo/app/presentation/AppViewModel.kt | 24 ++++-- .../app/presentation/AppViewModelTest.kt | 84 +++++++++++++++++++ 3 files changed, 106 insertions(+), 6 deletions(-) create mode 100644 app/src/test/kotlin/de/davis/keygo/app/presentation/AppViewModelTest.kt diff --git a/app/build.gradle.kts b/app/build.gradle.kts index c5d49e7fc..6ea894324 100644 --- a/app/build.gradle.kts +++ b/app/build.gradle.kts @@ -161,6 +161,10 @@ dependencies { testImplementation(libs.kotlin.test) testImplementation(libs.robolectric) testImplementation(libs.androidx.junit) + testImplementation(libs.kotlinx.coroutines.test) + testImplementation(testFixtures(projects.core.security)) + testImplementation(testFixtures(projects.core.identity)) + testImplementation(testFixtures(projects.legacyMigration)) androidTestImplementation(libs.androidx.junit) androidTestImplementation(libs.androidx.espresso.core) diff --git a/app/src/main/kotlin/de/davis/keygo/app/presentation/AppViewModel.kt b/app/src/main/kotlin/de/davis/keygo/app/presentation/AppViewModel.kt index 82f4a4199..00130cebf 100644 --- a/app/src/main/kotlin/de/davis/keygo/app/presentation/AppViewModel.kt +++ b/app/src/main/kotlin/de/davis/keygo/app/presentation/AppViewModel.kt @@ -6,8 +6,12 @@ import de.davis.keygo.core.identity.domain.repository.AccountRepository import de.davis.keygo.core.security.domain.Session import de.davis.keygo.legacy_migration.domain.usecase.HasMainPasswordUseCase import kotlinx.coroutines.flow.MutableStateFlow +import kotlinx.coroutines.flow.SharingStarted import kotlinx.coroutines.flow.StateFlow import kotlinx.coroutines.flow.asStateFlow +import kotlinx.coroutines.flow.map +import kotlinx.coroutines.flow.scan +import kotlinx.coroutines.flow.stateIn import kotlinx.coroutines.flow.update import kotlinx.coroutines.launch import org.koin.core.annotation.KoinViewModel @@ -23,16 +27,24 @@ internal class AppViewModel( val isReturningUser = _isReturningUser.asStateFlow() /** - * A restored back stack can hand the app proper the window straight after process death, - * skipping the launch flow; the fresh process's [Session] is never unlocked in that case, and - * nothing routes back to the unlock on its own once the launch stack has been emptied. - * [MainActivity] observes this and redirects whenever it goes false. + * True exactly when a session that was active has just ended - never at first launch, before + * the session has ever been active. A level read of "not active" would also be true before the + * very first login, before [MainActivity.launchRoute]'s onboarding or deep-link auth screen has + * had a chance to show, and would clobber it. [MainActivity] observes this to put the re-auth + * gate up. */ - val isSessionActive: StateFlow = session.isActive + val isLocked: StateFlow = session.isActive + .scan(false to false) { (wasActive, _), isActive -> isActive to (wasActive && !isActive) } + .map { it.second } + .stateIn( + scope = viewModelScope, + started = SharingStarted.WhileSubscribed(5_000), + initialValue = false, + ) init { viewModelScope.launch { _isReturningUser.update { accountRepository.getOrNull() != null || hasV1Password() } } } -} \ No newline at end of file +} diff --git a/app/src/test/kotlin/de/davis/keygo/app/presentation/AppViewModelTest.kt b/app/src/test/kotlin/de/davis/keygo/app/presentation/AppViewModelTest.kt new file mode 100644 index 000000000..3f33e7cef --- /dev/null +++ b/app/src/test/kotlin/de/davis/keygo/app/presentation/AppViewModelTest.kt @@ -0,0 +1,84 @@ +package de.davis.keygo.app.presentation + +import de.davis.keygo.core.identity.FakeAccountRepository +import de.davis.keygo.core.security.crypto.FakeSession +import de.davis.keygo.legacy_migration.FakeMainPasswordRepository +import de.davis.keygo.legacy_migration.hasMainPasswordUseCase +import kotlinx.coroutines.Dispatchers +import kotlinx.coroutines.ExperimentalCoroutinesApi +import kotlinx.coroutines.flow.launchIn +import kotlinx.coroutines.test.StandardTestDispatcher +import kotlinx.coroutines.test.TestScope +import kotlinx.coroutines.test.advanceUntilIdle +import kotlinx.coroutines.test.resetMain +import kotlinx.coroutines.test.runTest +import kotlinx.coroutines.test.setMain +import kotlin.test.AfterTest +import kotlin.test.BeforeTest +import kotlin.test.Test +import kotlin.test.assertFalse +import kotlin.test.assertTrue + +@OptIn(ExperimentalCoroutinesApi::class) +class AppViewModelTest { + + private val dispatcher = StandardTestDispatcher() + + @BeforeTest + fun setUp() = Dispatchers.setMain(dispatcher) + + @AfterTest + fun tearDown() = Dispatchers.resetMain() + + private fun TestScope.viewModel(session: FakeSession): AppViewModel = AppViewModel( + accountRepository = FakeAccountRepository(), + hasV1Password = hasMainPasswordUseCase(FakeMainPasswordRepository()), + session = session, + ).also { it.isLocked.launchIn(backgroundScope) } + + @Test + fun `isLocked stays false through a cold start that never logs in`() = runTest(dispatcher) { + val session = FakeSession() + val vm = viewModel(session) + advanceUntilIdle() + + assertFalse(vm.isLocked.value) + } + + @Test + fun `isLocked turns true only after an active session ends`() = runTest(dispatcher) { + val session = FakeSession() + val vm = viewModel(session) + // isLocked's collector must be subscribed (and so already see isActive = false) before + // the first mutation - StandardTestDispatcher defers launchIn's collection until this + // point, so starting the session any earlier would be missed rather than seen as a + // false -> true transition. + advanceUntilIdle() + + session.startSession(ByteArray(32)) + advanceUntilIdle() + assertFalse(vm.isLocked.value) + + session.endSession() + advanceUntilIdle() + assertTrue(vm.isLocked.value) + } + + @Test + fun `isLocked returns to false once the session is reinitialized`() = runTest(dispatcher) { + val session = FakeSession() + val vm = viewModel(session) + advanceUntilIdle() + + session.startSession(ByteArray(32)) + advanceUntilIdle() + session.endSession() + advanceUntilIdle() + assertTrue(vm.isLocked.value) + + session.startSession(ByteArray(32)) + advanceUntilIdle() + + assertFalse(vm.isLocked.value) + } +} From d99891fe1df08ee2303743931815252ccf87c234 Mon Sep 17 00:00:00 2001 From: Davis Wolfermann Date: Wed, 2 Sep 2026 14:20:56 +0200 Subject: [PATCH 03/35] fix(app): keep isLocked's upstream collection alive across UI subscriber churn --- .../davis/keygo/app/presentation/AppViewModel.kt | 2 +- .../davis/keygo/app/presentation/MainActivity.kt | 14 +++++++------- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/app/src/main/kotlin/de/davis/keygo/app/presentation/AppViewModel.kt b/app/src/main/kotlin/de/davis/keygo/app/presentation/AppViewModel.kt index 00130cebf..bcb2648b1 100644 --- a/app/src/main/kotlin/de/davis/keygo/app/presentation/AppViewModel.kt +++ b/app/src/main/kotlin/de/davis/keygo/app/presentation/AppViewModel.kt @@ -38,7 +38,7 @@ internal class AppViewModel( .map { it.second } .stateIn( scope = viewModelScope, - started = SharingStarted.WhileSubscribed(5_000), + started = SharingStarted.Eagerly, initialValue = false, ) diff --git a/app/src/main/kotlin/de/davis/keygo/app/presentation/MainActivity.kt b/app/src/main/kotlin/de/davis/keygo/app/presentation/MainActivity.kt index e818f2ec4..145cf4502 100644 --- a/app/src/main/kotlin/de/davis/keygo/app/presentation/MainActivity.kt +++ b/app/src/main/kotlin/de/davis/keygo/app/presentation/MainActivity.kt @@ -60,7 +60,7 @@ class MainActivity : FragmentActivity() { setContent { // Null until the account has been looked up, which the splash screen waits out. val hasAccess = viewModel.isReturningUser.collectAsState().value ?: return@setContent - val isSessionActive by viewModel.isSessionActive.collectAsState() + val isLocked by viewModel.isLocked.collectAsState() KeyGoTheme { val snackbarManager = koinInject() @@ -70,7 +70,7 @@ class MainActivity : FragmentActivity() { App( hasAccess = hasAccess, launchRoute = launchRoute(hasAccess), - isSessionActive = isSessionActive, + isLocked = isLocked, ) } } @@ -89,7 +89,7 @@ private fun Intent.totpImportRedirect(): TotpImportRedirect? { @OptIn(ExperimentalMaterial3AdaptiveApi::class) @Composable -private fun App(hasAccess: Boolean, launchRoute: NavKey, isSessionActive: Boolean) { +private fun App(hasAccess: Boolean, launchRoute: NavKey, isLocked: Boolean) { val navigationState = rememberAppNavigationState( launchRoute = launchRoute, startRoute = RouteDestination.Home, @@ -97,7 +97,7 @@ private fun App(hasAccess: Boolean, launchRoute: NavKey, isSessionActive: Boolea ) val navigator = remember(navigationState) { AppNavigator(navigationState) } - RedirectToAuthWhenSessionEnds(isSessionActive, navigator) + RedirectToAuthWhenSessionEnds(isLocked, navigator) val windowAdaptiveInfo = currentWindowAdaptiveInfoV2() val directive = remember(windowAdaptiveInfo) { @@ -153,12 +153,12 @@ private fun App(hasAccess: Boolean, launchRoute: NavKey, isSessionActive: Boolea */ @Composable private fun RedirectToAuthWhenSessionEnds( - isSessionActive: Boolean, + isLocked: Boolean, navigator: AppNavigator, ) { val isLaunching = navigator.state.isLaunching - LaunchedEffect(isSessionActive, isLaunching) { - if (!isSessionActive && !isLaunching) navigator.replaceLaunchFlow(AuthRoute()) + LaunchedEffect(isLocked, isLaunching) { + if (isLocked && !isLaunching) navigator.replaceLaunchFlow(AuthRoute()) } } From 65d09fa61ee19cb446eafa70994563b52945fce5 Mon Sep 17 00:00:00 2001 From: Davis Wolfermann Date: Wed, 2 Sep 2026 14:29:05 +0200 Subject: [PATCH 04/35] feat(app): add AppNavigator.lock/unlock and make goBack respect the gate --- .../navigation/AppNavigationState.kt | 3 + .../presentation/navigation/AppNavigator.kt | 31 +++++- .../navigation/AppNavigatorTest.kt | 98 +++++++++++++++++++ 3 files changed, 130 insertions(+), 2 deletions(-) diff --git a/app/src/main/kotlin/de/davis/keygo/app/presentation/navigation/AppNavigationState.kt b/app/src/main/kotlin/de/davis/keygo/app/presentation/navigation/AppNavigationState.kt index 5bea0a802..234aa05dd 100644 --- a/app/src/main/kotlin/de/davis/keygo/app/presentation/navigation/AppNavigationState.kt +++ b/app/src/main/kotlin/de/davis/keygo/app/presentation/navigation/AppNavigationState.kt @@ -67,6 +67,9 @@ class AppNavigationState( /** The selected navigation bar destination. */ var topLevelRoute: NavKey by topLevelRoute + /** True while [AppNavigator.lock] has a gate up. Makes [AppNavigator.goBack] a no-op. */ + var isGated: Boolean = false + /** Whether the launch flow still owns the window. */ val isLaunching: Boolean get() = launchStack.isNotEmpty() diff --git a/app/src/main/kotlin/de/davis/keygo/app/presentation/navigation/AppNavigator.kt b/app/src/main/kotlin/de/davis/keygo/app/presentation/navigation/AppNavigator.kt index e9186609b..68db8128d 100644 --- a/app/src/main/kotlin/de/davis/keygo/app/presentation/navigation/AppNavigator.kt +++ b/app/src/main/kotlin/de/davis/keygo/app/presentation/navigation/AppNavigator.kt @@ -37,6 +37,31 @@ class AppNavigator(val state: AppNavigationState) { state.launchStack.clear() } + /** Adds [route] to the launch flow without disturbing whatever is already on it. */ + fun pushOntoLaunchFlow(route: NavKey) { + state.launchStack.add(route) + } + + /** + * Hides every tab behind [gate] and blocks all back navigation until [unlock] is called. Every + * tab other than the one currently selected is truncated to its base, tearing down whatever + * ViewModels it held; the selected tab, and anything already on the launch stack (an + * in-progress TOTP import, say), are left exactly as they were, restored for free once the gate + * lifts. + */ + fun lock(gate: NavKey) { + val activeRoute = state.topLevelRoute + state.backStacks.forEach { (route, stack) -> if (route != activeRoute) stack.popToBase() } + pushOntoLaunchFlow(gate) + state.isGated = true + } + + /** Lifts the gate [lock] put up, revealing whatever was underneath it. */ + fun unlock() { + state.launchStack.removeLastOrNull() + state.isGated = false + } + /** * Shows [detail] in the dashboard's detail pane, replacing any detail already open, so back * from a detail always lands on the list. @@ -67,10 +92,12 @@ class AppNavigator(val state: AppNavigationState) { } /** - * Goes back one destination, but never down to nothing. The display stops handling back once a - * stack is one deep, so the app is what closes. + * Goes back one destination, but never down to nothing, and never while a lock's gate is up. + * The launch stack can hold more than one entry while gated (a picker preserved under the + * gate, say), so a plain depth check would let back press pop the gate itself away. */ fun goBack() { + if (state.isGated) return val stack = state.currentStack if (stack.size > 1) stack.removeLastOrNull() } diff --git a/app/src/test/kotlin/de/davis/keygo/app/presentation/navigation/AppNavigatorTest.kt b/app/src/test/kotlin/de/davis/keygo/app/presentation/navigation/AppNavigatorTest.kt index 9c704d80e..223107fef 100644 --- a/app/src/test/kotlin/de/davis/keygo/app/presentation/navigation/AppNavigatorTest.kt +++ b/app/src/test/kotlin/de/davis/keygo/app/presentation/navigation/AppNavigatorTest.kt @@ -286,6 +286,104 @@ class AppNavigatorTest { assertEquals(listOf(RouteDestination.Home), navigator.shown) } + // ---- locking ---- + + @Test + fun `locking truncates every tab but the one shown`() { + val navigator = unlocked() + navigator.navigate(SettingsRoute) + navigator.navigate(ChangePasswordRoute) + navigator.navigate(RouteDestination.Home) + val itemId = newItemId() + navigator.showDetail(RouteDestination.ViewItem(itemId)) + + navigator.lock(AuthRoute()) + + assertEquals( + listOf(RouteDestination.Home, RouteDestination.ViewItem(itemId)), + navigator.state.backStacks.getValue(RouteDestination.Home).toList(), + ) + assertEquals( + listOf(SettingsRoute), + navigator.state.backStacks.getValue(SettingsRoute).toList(), + ) + } + + @Test + fun `locking pushes the gate without clearing what was already on the launch stack`() { + val navigator = navigator() + navigator.replaceLaunchFlow(SelectItemForTotpRoute(DEEP_LINK_URI)) + + navigator.lock(AuthRoute()) + + assertEquals( + listOf(SelectItemForTotpRoute(DEEP_LINK_URI), AuthRoute()), + navigator.shown, + ) + } + + @Test + fun `locking from a tab pushes the gate onto an otherwise empty launch stack`() { + val navigator = unlocked() + navigator.navigate(SettingsRoute) + + navigator.lock(AuthRoute()) + + assertTrue(navigator.state.isLaunching) + assertEquals(listOf(AuthRoute()), navigator.shown) + } + + @Test + fun `unlocking reveals the active tab exactly as it was`() { + val navigator = unlocked() + navigator.navigate(SettingsRoute) + navigator.navigate(ChangePasswordRoute) + navigator.lock(AuthRoute()) + + navigator.unlock() + + assertFalse(navigator.state.isLaunching) + assertEquals(listOf(SettingsRoute, ChangePasswordRoute), navigator.shown) + } + + @Test + fun `unlocking reveals a picker that was preserved under the gate`() { + val navigator = navigator() + navigator.replaceLaunchFlow(SelectItemForTotpRoute(DEEP_LINK_URI)) + navigator.lock(AuthRoute()) + + navigator.unlock() + + assertEquals(listOf(SelectItemForTotpRoute(DEEP_LINK_URI)), navigator.shown) + } + + @Test + fun `back cannot pop the gate away, even over a picker underneath it`() { + val navigator = navigator() + navigator.replaceLaunchFlow(SelectItemForTotpRoute(DEEP_LINK_URI)) + navigator.lock(AuthRoute()) + + navigator.goBack() + + assertEquals( + listOf(SelectItemForTotpRoute(DEEP_LINK_URI), AuthRoute()), + navigator.shown, + ) + } + + @Test + fun `back works normally again once unlocked`() { + val navigator = unlocked() + navigator.navigate(SettingsRoute) + navigator.navigate(ChangePasswordRoute) + navigator.lock(AuthRoute()) + navigator.unlock() + + navigator.goBack() + + assertEquals(listOf(SettingsRoute), navigator.shown) + } + private fun unlocked(): AppNavigator = navigator().apply { finishLaunchFlow() } private companion object { From f966b9a25a756ba4f149db59b2ec5ca1a7ab418a Mon Sep 17 00:00:00 2001 From: Davis Wolfermann Date: Wed, 2 Sep 2026 14:37:18 +0200 Subject: [PATCH 05/35] feat(app): lock on background and unlock through the navigator's gate --- .../keygo/app/presentation/MainActivity.kt | 25 ++++++------------- .../presentation/navigation/EntryProvider.kt | 9 +++++-- 2 files changed, 14 insertions(+), 20 deletions(-) diff --git a/app/src/main/kotlin/de/davis/keygo/app/presentation/MainActivity.kt b/app/src/main/kotlin/de/davis/keygo/app/presentation/MainActivity.kt index 145cf4502..eb4da7cde 100644 --- a/app/src/main/kotlin/de/davis/keygo/app/presentation/MainActivity.kt +++ b/app/src/main/kotlin/de/davis/keygo/app/presentation/MainActivity.kt @@ -97,7 +97,7 @@ private fun App(hasAccess: Boolean, launchRoute: NavKey, isLocked: Boolean) { ) val navigator = remember(navigationState) { AppNavigator(navigationState) } - RedirectToAuthWhenSessionEnds(isLocked, navigator) + LockAppWhenSessionEnds(isLocked, navigator) val windowAdaptiveInfo = currentWindowAdaptiveInfoV2() val directive = remember(windowAdaptiveInfo) { @@ -140,25 +140,14 @@ private fun App(hasAccess: Boolean, launchRoute: NavKey, isLocked: Boolean) { } /** - * The navigation state outlives the process, so a restored back stack can hand the app proper the - * window again without the launch flow ever running. The fresh process has no unlocked session in - * that case, and nothing routes back to the unlock on its own once - * [AppNavigator.finishLaunchFlow] has emptied the launch stack, so the unlock is put back on top - * here. - * - * Keyed on the launch state as well, so this only acts while the app proper owns the window, and - * so a session that dies later (an auto lock, say) redirects at once rather than waiting for the - * next navigation. Onboarding and the unlock itself both run with no session by design, and - * redirecting there would take a first run user straight back out of setup. + * [AppViewModel.isLocked] only turns true for a session that was active and then ended, so this + * cannot fire before the very first login and cannot clobber [MainActivity.launchRoute]'s + * onboarding or deep-link destination the way a level read of "not active" would. */ @Composable -private fun RedirectToAuthWhenSessionEnds( - isLocked: Boolean, - navigator: AppNavigator, -) { - val isLaunching = navigator.state.isLaunching - LaunchedEffect(isLocked, isLaunching) { - if (isLocked && !isLaunching) navigator.replaceLaunchFlow(AuthRoute()) +private fun LockAppWhenSessionEnds(isLocked: Boolean, navigator: AppNavigator) { + LaunchedEffect(isLocked) { + if (isLocked) navigator.lock(AuthRoute()) } } diff --git a/app/src/main/kotlin/de/davis/keygo/app/presentation/navigation/EntryProvider.kt b/app/src/main/kotlin/de/davis/keygo/app/presentation/navigation/EntryProvider.kt index 64fa44276..d57114051 100644 --- a/app/src/main/kotlin/de/davis/keygo/app/presentation/navigation/EntryProvider.kt +++ b/app/src/main/kotlin/de/davis/keygo/app/presentation/navigation/EntryProvider.kt @@ -137,7 +137,12 @@ internal fun AppNavigator.openGateFor(hasAccess: Boolean, uri: String) { replaceLaunchFlow(if (hasAccess) AuthRoute(uri = uri) else OnboardingRoute(uri = uri)) } +/** + * Pops whatever gate or cold-start screen just finished authenticating. That reveals a picker + * preserved underneath a lock's gate on its own; a fresh totpUri from this run instead replaces + * that reveal with the picker for it, the same as it always did at cold start. + */ private fun AppNavigator.finishUnlock(totpUri: String?) { - if (totpUri == null) finishLaunchFlow() - else replaceLaunchFlow(SelectItemForTotpRoute(totpUri)) + unlock() + if (totpUri != null) pushOntoLaunchFlow(SelectItemForTotpRoute(totpUri)) } From 7f2028419e98d0f417919e9c353cc18b2c60d064 Mon Sep 17 00:00:00 2001 From: Davis Wolfermann Date: Wed, 2 Sep 2026 14:44:16 +0200 Subject: [PATCH 06/35] feat(settings): clear change-password fields when the session ends --- .../changepassword/ChangePasswordViewModel.kt | 23 +++++++++++++++ .../ChangePasswordViewModelTest.kt | 28 +++++++++++++++++++ 2 files changed, 51 insertions(+) diff --git a/feature/settings/src/main/kotlin/de/davis/keygo/feature/settings/presentation/changepassword/ChangePasswordViewModel.kt b/feature/settings/src/main/kotlin/de/davis/keygo/feature/settings/presentation/changepassword/ChangePasswordViewModel.kt index b5f7ce2ce..458918cd1 100644 --- a/feature/settings/src/main/kotlin/de/davis/keygo/feature/settings/presentation/changepassword/ChangePasswordViewModel.kt +++ b/feature/settings/src/main/kotlin/de/davis/keygo/feature/settings/presentation/changepassword/ChangePasswordViewModel.kt @@ -1,5 +1,6 @@ package de.davis.keygo.feature.settings.presentation.changepassword +import androidx.compose.foundation.text.input.TextFieldState import androidx.compose.runtime.snapshotFlow import androidx.lifecycle.ViewModel import androidx.lifecycle.viewModelScope @@ -9,6 +10,7 @@ import de.davis.keygo.core.identity.domain.repository.AccountRepository import de.davis.keygo.core.identity.domain.usecase.ChangePasswordUseCase import de.davis.keygo.core.item.domain.estimator.PasswordStrengthEstimator import de.davis.keygo.core.item.domain.model.PasswordScore +import de.davis.keygo.core.security.domain.Session import de.davis.keygo.core.security.domain.model.BiometricAuthError import de.davis.keygo.core.security.domain.model.CiphertextData import de.davis.keygo.core.security.domain.repository.BiometricAvailabilityRepository @@ -21,9 +23,11 @@ import kotlinx.coroutines.FlowPreview import kotlinx.coroutines.channels.Channel import kotlinx.coroutines.flow.MutableStateFlow import kotlinx.coroutines.flow.SharingStarted +import kotlinx.coroutines.flow.collect import kotlinx.coroutines.flow.combine import kotlinx.coroutines.flow.debounce import kotlinx.coroutines.flow.distinctUntilChanged +import kotlinx.coroutines.flow.filter import kotlinx.coroutines.flow.mapLatest import kotlinx.coroutines.flow.onStart import kotlinx.coroutines.flow.receiveAsFlow @@ -40,6 +44,7 @@ internal class ChangePasswordViewModel( private val biometricAvailabilityRepository: BiometricAvailabilityRepository, private val passwordStrengthEstimator: PasswordStrengthEstimator, private val changePassword: ChangePasswordUseCase, + private val session: Session, ) : ViewModel() { private val _state = MutableStateFlow(ChangePasswordState()) @@ -70,6 +75,9 @@ internal class ChangePasswordViewModel( init { resolveBiometricAvailability() + viewModelScope.launch { + session.isActive.filter { !it }.collect { clearSensitiveFields() } + } } private fun resolveBiometricAvailability() { @@ -195,4 +203,19 @@ internal class ChangePasswordViewModel( else -> _event.trySend(ChangePasswordEvent.GenericError) } } + + /** + * The current password is the RootKek derivation input - the one secret this codebase never + * retains anywhere, including across a lock. Everything else about the screen is left alone, + * the same as any other screen the app leaves in place while locked. + */ + private fun clearSensitiveFields() { + _state.update { + it.copy( + currentPassword = TextFieldState(), + newPassword = TextFieldState(), + confirmPassword = TextFieldState(), + ) + } + } } diff --git a/feature/settings/src/test/kotlin/de/davis/keygo/feature/settings/presentation/changepassword/ChangePasswordViewModelTest.kt b/feature/settings/src/test/kotlin/de/davis/keygo/feature/settings/presentation/changepassword/ChangePasswordViewModelTest.kt index 8b122172f..32df8361f 100644 --- a/feature/settings/src/test/kotlin/de/davis/keygo/feature/settings/presentation/changepassword/ChangePasswordViewModelTest.kt +++ b/feature/settings/src/test/kotlin/de/davis/keygo/feature/settings/presentation/changepassword/ChangePasswordViewModelTest.kt @@ -8,6 +8,7 @@ import de.davis.keygo.core.identity.domain.usecase.ChangePasswordUseCase import de.davis.keygo.core.item.domain.estimator.PasswordStrengthEstimator import de.davis.keygo.core.item.domain.model.PasswordScore import de.davis.keygo.core.security.crypto.FakeBiometricAvailabilityRepository +import de.davis.keygo.core.security.crypto.FakeSession import de.davis.keygo.core.security.domain.model.BiometricAuthError import de.davis.keygo.core.ui.model.UiFieldError import de.davis.keygo.core.util.Result @@ -38,6 +39,7 @@ class ChangePasswordViewModelTest { private val accountRepository = FakeAccountRepository() private val biometricAvailability = FakeBiometricAvailabilityRepository() + private val session = FakeSession(startOnConstruct = true) private val keyDeriver = FakeKeyDeriver() private val keyWrapper = FakeKeyWrapper() private val estimator = object : PasswordStrengthEstimator { @@ -90,6 +92,7 @@ class ChangePasswordViewModelTest { biometricAvailabilityRepository = biometricAvailability, passwordStrengthEstimator = estimator, changePassword = changePassword, + session = session, ).also { it.state.launchIn(backgroundScope) } @Test @@ -313,4 +316,29 @@ class ChangePasswordViewModelTest { assertEquals(false, vm.state.value.showReauthDialog) } + + @Test + fun `the session ending clears all three password fields`() = runTest(dispatcher) { + val vm = viewModel() + vm.state.value.currentPassword.edit { append("old-pw") } + vm.state.value.newPassword.edit { append("new-pw") } + vm.state.value.confirmPassword.edit { append("new-pw") } + advanceUntilIdle() + + session.endSession() + advanceUntilIdle() + + assertEquals("", vm.state.value.currentPassword.text.toString()) + assertEquals("", vm.state.value.newPassword.text.toString()) + assertEquals("", vm.state.value.confirmPassword.text.toString()) + } + + @Test + fun `ordinary use does not clear the fields while the session stays active`() = runTest(dispatcher) { + val vm = viewModel() + vm.state.value.currentPassword.edit { append("old-pw") } + advanceUntilIdle() + + assertEquals("old-pw", vm.state.value.currentPassword.text.toString()) + } } From 2d154767bfd0c3d1f347927c16e1b86bd9299743 Mon Sep 17 00:00:00 2001 From: Davis Wolfermann Date: Wed, 2 Sep 2026 17:02:28 +0200 Subject: [PATCH 07/35] fix: make the app-lock gate survive configuration change and process death --- .../keygo/app/presentation/AppViewModel.kt | 9 ++++ .../keygo/app/presentation/MainActivity.kt | 37 ++++++++++---- .../navigation/AppNavigationState.kt | 16 ++++++- .../presentation/navigation/AppNavigator.kt | 16 ++++++- .../app/presentation/AppViewModelTest.kt | 25 ++++++++++ .../navigation/AppNavigatorTest.kt | 48 +++++++++++++++++++ .../changepassword/ChangePasswordViewModel.kt | 18 ++++--- .../ChangePasswordViewModelTest.kt | 39 +++++++++++++++ 8 files changed, 190 insertions(+), 18 deletions(-) diff --git a/app/src/main/kotlin/de/davis/keygo/app/presentation/AppViewModel.kt b/app/src/main/kotlin/de/davis/keygo/app/presentation/AppViewModel.kt index bcb2648b1..fc3722d7e 100644 --- a/app/src/main/kotlin/de/davis/keygo/app/presentation/AppViewModel.kt +++ b/app/src/main/kotlin/de/davis/keygo/app/presentation/AppViewModel.kt @@ -42,6 +42,15 @@ internal class AppViewModel( initialValue = false, ) + /** + * The session's raw current state, for [MainActivity] to self-heal a back stack a + * configuration change or process death restored straight into the app proper with a session + * that never got re-established. [isLocked] alone cannot catch this: it only fires on a + * transition, and a freshly restored [AppViewModel] has no memory of the session ever having + * been active to transition from. + */ + val isSessionActive: StateFlow = session.isActive + init { viewModelScope.launch { _isReturningUser.update { accountRepository.getOrNull() != null || hasV1Password() } diff --git a/app/src/main/kotlin/de/davis/keygo/app/presentation/MainActivity.kt b/app/src/main/kotlin/de/davis/keygo/app/presentation/MainActivity.kt index eb4da7cde..c3780c993 100644 --- a/app/src/main/kotlin/de/davis/keygo/app/presentation/MainActivity.kt +++ b/app/src/main/kotlin/de/davis/keygo/app/presentation/MainActivity.kt @@ -61,6 +61,7 @@ class MainActivity : FragmentActivity() { // Null until the account has been looked up, which the splash screen waits out. val hasAccess = viewModel.isReturningUser.collectAsState().value ?: return@setContent val isLocked by viewModel.isLocked.collectAsState() + val isSessionActive by viewModel.isSessionActive.collectAsState() KeyGoTheme { val snackbarManager = koinInject() @@ -71,6 +72,7 @@ class MainActivity : FragmentActivity() { hasAccess = hasAccess, launchRoute = launchRoute(hasAccess), isLocked = isLocked, + isSessionActive = isSessionActive, ) } } @@ -89,7 +91,12 @@ private fun Intent.totpImportRedirect(): TotpImportRedirect? { @OptIn(ExperimentalMaterial3AdaptiveApi::class) @Composable -private fun App(hasAccess: Boolean, launchRoute: NavKey, isLocked: Boolean) { +private fun App( + hasAccess: Boolean, + launchRoute: NavKey, + isLocked: Boolean, + isSessionActive: Boolean, +) { val navigationState = rememberAppNavigationState( launchRoute = launchRoute, startRoute = RouteDestination.Home, @@ -97,7 +104,7 @@ private fun App(hasAccess: Boolean, launchRoute: NavKey, isLocked: Boolean) { ) val navigator = remember(navigationState) { AppNavigator(navigationState) } - LockAppWhenSessionEnds(isLocked, navigator) + LockAppWhenSessionEnds(isLocked, isSessionActive, navigator) val windowAdaptiveInfo = currentWindowAdaptiveInfoV2() val directive = remember(windowAdaptiveInfo) { @@ -140,14 +147,28 @@ private fun App(hasAccess: Boolean, launchRoute: NavKey, isLocked: Boolean) { } /** - * [AppViewModel.isLocked] only turns true for a session that was active and then ended, so this - * cannot fire before the very first login and cannot clobber [MainActivity.launchRoute]'s - * onboarding or deep-link destination the way a level read of "not active" would. + * Locks the app in two cases: + * - [isLocked] catches the instant a session that was active ends, whether the app proper + * currently owns the window or a launch-flow screen does (an in-progress TOTP-import picker, + * say) - [AppNavigator.lock] pushes over either without disturbing what's underneath. + * - The level check (`!isSessionActive && !isLaunching`) catches a back stack a configuration + * change or process death restored straight into the app proper with a session that never got + * re-established: [isLocked] alone can't see this, since it only fires on a transition a freshly + * restored [AppViewModel] has no memory of. Gated by `!isLaunching` so it never fires during + * onboarding or the very first login (both show with the launch flow already owning the window + * and no session yet, which looks the same as this case unless launch state is checked too), and + * so it never fights [AppNavigator.lock]'s own idempotency for a gate or picker Nav3 already + * restored correctly. */ @Composable -private fun LockAppWhenSessionEnds(isLocked: Boolean, navigator: AppNavigator) { - LaunchedEffect(isLocked) { - if (isLocked) navigator.lock(AuthRoute()) +private fun LockAppWhenSessionEnds( + isLocked: Boolean, + isSessionActive: Boolean, + navigator: AppNavigator, +) { + val isLaunching = navigator.state.isLaunching + LaunchedEffect(isLocked, isSessionActive, isLaunching) { + if (isLocked || (!isSessionActive && !isLaunching)) navigator.lock(AuthRoute()) } } diff --git a/app/src/main/kotlin/de/davis/keygo/app/presentation/navigation/AppNavigationState.kt b/app/src/main/kotlin/de/davis/keygo/app/presentation/navigation/AppNavigationState.kt index 234aa05dd..0174c3dc1 100644 --- a/app/src/main/kotlin/de/davis/keygo/app/presentation/navigation/AppNavigationState.kt +++ b/app/src/main/kotlin/de/davis/keygo/app/presentation/navigation/AppNavigationState.kt @@ -5,6 +5,7 @@ import androidx.compose.runtime.MutableState import androidx.compose.runtime.getValue import androidx.compose.runtime.mutableStateOf import androidx.compose.runtime.remember +import androidx.compose.runtime.saveable.rememberSaveable import androidx.compose.runtime.saveable.rememberSerializable import androidx.compose.runtime.setValue import androidx.navigation3.runtime.NavBackStack @@ -37,6 +38,7 @@ fun rememberAppNavigationState( ) { mutableStateOf(startRoute) } + val isGated = rememberSaveable { mutableStateOf(false) } val launchStack = rememberNavBackStack(launchRoute) val backStacks = topLevelRoutes.associateWith { key -> rememberNavBackStack(key) } @@ -46,6 +48,7 @@ fun rememberAppNavigationState( launchStack = launchStack, topLevelRoute = topLevelRoute, backStacks = backStacks, + isGated = isGated, ) } } @@ -62,13 +65,22 @@ class AppNavigationState( val launchStack: NavBackStack, topLevelRoute: MutableState, val backStacks: Map>, + isGated: MutableState = mutableStateOf(false), ) { /** The selected navigation bar destination. */ var topLevelRoute: NavKey by topLevelRoute - /** True while [AppNavigator.lock] has a gate up. Makes [AppNavigator.goBack] a no-op. */ - var isGated: Boolean = false + /** + * True while [AppNavigator.lock] has a gate up. Makes [AppNavigator.goBack] a no-op. + * + * Backed by [rememberSaveable] in [rememberAppNavigationState], the same as [topLevelRoute] is + * backed by [rememberSerializable] - not just [remember]. [launchStack] survives a + * configuration change and a process death by construction; if a gate was on it when either + * happened, this flag has to come back `true` too, or a restored gate would show with nothing + * stopping [AppNavigator.goBack] from popping it away. + */ + var isGated: Boolean by isGated /** Whether the launch flow still owns the window. */ val isLaunching: Boolean get() = launchStack.isNotEmpty() diff --git a/app/src/main/kotlin/de/davis/keygo/app/presentation/navigation/AppNavigator.kt b/app/src/main/kotlin/de/davis/keygo/app/presentation/navigation/AppNavigator.kt index 68db8128d..ae6745c3c 100644 --- a/app/src/main/kotlin/de/davis/keygo/app/presentation/navigation/AppNavigator.kt +++ b/app/src/main/kotlin/de/davis/keygo/app/presentation/navigation/AppNavigator.kt @@ -48,15 +48,29 @@ class AppNavigator(val state: AppNavigationState) { * ViewModels it held; the selected tab, and anything already on the launch stack (an * in-progress TOTP import, say), are left exactly as they were, restored for free once the gate * lifts. + * + * A no-op if already gated. This matters because the caller's trigger is collected by a + * `LaunchedEffect` that re-fires on every fresh composition - including one rebuilt by a + * configuration change while the app is still locked - with no memory of having already run; + * [AppNavigationState.isGated] is durable across that rebuild, so it is what keeps a second + * call from pushing a second gate. */ fun lock(gate: NavKey) { + if (state.isGated) return val activeRoute = state.topLevelRoute state.backStacks.forEach { (route, stack) -> if (route != activeRoute) stack.popToBase() } pushOntoLaunchFlow(gate) state.isGated = true } - /** Lifts the gate [lock] put up, revealing whatever was underneath it. */ + /** + * Lifts the gate [lock] put up, revealing whatever was underneath it. + * + * Pops unconditionally rather than only while gated. This is also what dismisses the cold-start + * auth screen and the onboarding screen, neither of which [lock] ever gated - they are on the + * launch stack because they were the launch route. Returning early on `!isGated` would leave + * both up for good after a successful first login. + */ fun unlock() { state.launchStack.removeLastOrNull() state.isGated = false diff --git a/app/src/test/kotlin/de/davis/keygo/app/presentation/AppViewModelTest.kt b/app/src/test/kotlin/de/davis/keygo/app/presentation/AppViewModelTest.kt index 3f33e7cef..dee52c0c0 100644 --- a/app/src/test/kotlin/de/davis/keygo/app/presentation/AppViewModelTest.kt +++ b/app/src/test/kotlin/de/davis/keygo/app/presentation/AppViewModelTest.kt @@ -81,4 +81,29 @@ class AppViewModelTest { assertFalse(vm.isLocked.value) } + + @Test + fun `isLocked keeps tracking transitions with no collector ever attached`() = + runTest(dispatcher) { + val session = FakeSession() + // Deliberately not using the shared viewModel() helper here - it holds a permanent + // collector via launchIn(backgroundScope), which would pass under either SharingStarted + // strategy and wouldn't actually distinguish Eagerly from the WhileSubscribed(5_000) + // this replaced. Eagerly's whole point is that isLocked keeps tracking transitions even + // with zero collectors ever subscribed; WhileSubscribed would never even start + // collecting. + val vm = AppViewModel( + accountRepository = FakeAccountRepository(), + hasV1Password = hasMainPasswordUseCase(FakeMainPasswordRepository()), + session = session, + ) + advanceUntilIdle() + + session.startSession(ByteArray(32)) + advanceUntilIdle() + session.endSession() + advanceUntilIdle() + + assertTrue(vm.isLocked.value) + } } diff --git a/app/src/test/kotlin/de/davis/keygo/app/presentation/navigation/AppNavigatorTest.kt b/app/src/test/kotlin/de/davis/keygo/app/presentation/navigation/AppNavigatorTest.kt index 223107fef..d14c384ec 100644 --- a/app/src/test/kotlin/de/davis/keygo/app/presentation/navigation/AppNavigatorTest.kt +++ b/app/src/test/kotlin/de/davis/keygo/app/presentation/navigation/AppNavigatorTest.kt @@ -23,6 +23,7 @@ class AppNavigatorTest { launchStack = NavBackStack(launchRoute), topLevelRoute = mutableStateOf(RouteDestination.Home), backStacks = TOP_LEVEL_ROUTES.associateWith { NavBackStack(it) }, + isGated = mutableStateOf(false), ) return AppNavigator(state) } @@ -384,6 +385,53 @@ class AppNavigatorTest { assertEquals(listOf(SettingsRoute), navigator.shown) } + @Test + fun `unlocking a cold start gate hands the window to the app proper`() { + // AppNavigator.finishUnlock calls unlock() for the cold-start auth and onboarding screens + // too, which are on the launch stack without lock() ever having gated anything. Popping + // has to happen there as well, or authenticating at cold start would leave the auth screen + // up forever. + val navigator = navigator() + + navigator.unlock() + + assertFalse(navigator.state.isLaunching) + assertEquals(listOf(RouteDestination.Home), navigator.shown) + } + + @Test + fun `a gate restored from saved state is not pushed a second time`() { + val state = AppNavigationState( + launchStack = NavBackStack(AuthRoute()), + topLevelRoute = mutableStateOf(RouteDestination.Home), + backStacks = TOP_LEVEL_ROUTES.associateWith { NavBackStack(it) }, + isGated = mutableStateOf(true), + ) + val navigator = AppNavigator(state) + + navigator.lock(AuthRoute()) + + assertEquals(listOf(AuthRoute()), navigator.shown) + } + + @Test + fun `back still cannot pop a gate restored from saved state, even over a preserved picker`() { + val state = AppNavigationState( + launchStack = NavBackStack(SelectItemForTotpRoute(DEEP_LINK_URI), AuthRoute()), + topLevelRoute = mutableStateOf(RouteDestination.Home), + backStacks = TOP_LEVEL_ROUTES.associateWith { NavBackStack(it) }, + isGated = mutableStateOf(true), + ) + val navigator = AppNavigator(state) + + navigator.goBack() + + assertEquals( + listOf(SelectItemForTotpRoute(DEEP_LINK_URI), AuthRoute()), + navigator.shown, + ) + } + private fun unlocked(): AppNavigator = navigator().apply { finishLaunchFlow() } private companion object { diff --git a/feature/settings/src/main/kotlin/de/davis/keygo/feature/settings/presentation/changepassword/ChangePasswordViewModel.kt b/feature/settings/src/main/kotlin/de/davis/keygo/feature/settings/presentation/changepassword/ChangePasswordViewModel.kt index 458918cd1..a392f907c 100644 --- a/feature/settings/src/main/kotlin/de/davis/keygo/feature/settings/presentation/changepassword/ChangePasswordViewModel.kt +++ b/feature/settings/src/main/kotlin/de/davis/keygo/feature/settings/presentation/changepassword/ChangePasswordViewModel.kt @@ -1,6 +1,7 @@ package de.davis.keygo.feature.settings.presentation.changepassword import androidx.compose.foundation.text.input.TextFieldState +import androidx.compose.foundation.text.input.delete import androidx.compose.runtime.snapshotFlow import androidx.lifecycle.ViewModel import androidx.lifecycle.viewModelScope @@ -208,14 +209,17 @@ internal class ChangePasswordViewModel( * The current password is the RootKek derivation input - the one secret this codebase never * retains anywhere, including across a lock. Everything else about the screen is left alone, * the same as any other screen the app leaves in place while locked. + * + * Mutates the existing [TextFieldState] instances rather than replacing them: [passwordStrength] + * tracks a snapshot-state read on whichever instance `_state.value.newPassword` pointed to the + * last time it ran, and a fresh replacement instance's changes would go unobserved - the old, + * abandoned instance is simply never mutated, so nothing ever re-triggers the flow, and + * [ChangePasswordState.passwordScore] would freeze at whatever it was the moment before the + * clear for the rest of the ViewModel's life. */ private fun clearSensitiveFields() { - _state.update { - it.copy( - currentPassword = TextFieldState(), - newPassword = TextFieldState(), - confirmPassword = TextFieldState(), - ) - } + _state.value.currentPassword.edit { delete(0, length) } + _state.value.newPassword.edit { delete(0, length) } + _state.value.confirmPassword.edit { delete(0, length) } } } diff --git a/feature/settings/src/test/kotlin/de/davis/keygo/feature/settings/presentation/changepassword/ChangePasswordViewModelTest.kt b/feature/settings/src/test/kotlin/de/davis/keygo/feature/settings/presentation/changepassword/ChangePasswordViewModelTest.kt index 32df8361f..6bee0f889 100644 --- a/feature/settings/src/test/kotlin/de/davis/keygo/feature/settings/presentation/changepassword/ChangePasswordViewModelTest.kt +++ b/feature/settings/src/test/kotlin/de/davis/keygo/feature/settings/presentation/changepassword/ChangePasswordViewModelTest.kt @@ -1,5 +1,6 @@ package de.davis.keygo.feature.settings.presentation.changepassword +import androidx.compose.runtime.snapshots.Snapshot import de.davis.keygo.core.identity.FakeAccountRepository import de.davis.keygo.core.identity.domain.model.Account import de.davis.keygo.core.identity.domain.model.BiometricWrappedArk @@ -341,4 +342,42 @@ class ChangePasswordViewModelTest { assertEquals("old-pw", vm.state.value.currentPassword.text.toString()) } + + @Test + fun `the strength meter still tracks the new password after a clear`() = runTest(dispatcher) { + // The meter's snapshotFlow tracks the TextFieldState instance it last read, so clearing by + // swapping in fresh instances would leave it watching an abandoned one that is never + // mutated again - freezing the score for the rest of the ViewModel's life. + val vm = ChangePasswordViewModel( + accountRepository = accountRepository, + biometricAvailabilityRepository = biometricAvailability, + passwordStrengthEstimator = object : PasswordStrengthEstimator { + override suspend fun estimate(password: String) = + PasswordScore(password.length.coerceAtMost(5)) + }, + changePassword = changePassword, + session = session, + ).also { it.state.launchIn(backgroundScope) } + // No Recomposer drives the frame clock here, so snapshotFlow is told about writes by hand. + // The first advance is what lets the session-ended collector do its write in the first + // place; the notification has to come after it, and the debounce after that. + suspend fun settle() { + advanceUntilIdle() + Snapshot.sendApplyNotifications() + advanceUntilIdle() + } + + vm.state.value.newPassword.edit { append("aaaa") } + settle() + assertEquals(PasswordScore.Strong, vm.state.value.passwordScore) + + session.endSession() + settle() + assertEquals(PasswordScore.None, vm.state.value.passwordScore) + + vm.state.value.newPassword.edit { append("aa") } + settle() + + assertEquals(PasswordScore.Weak, vm.state.value.passwordScore) + } } From b3158b2dd95d237eaca60d1964c9af861a39e219 Mon Sep 17 00:00:00 2001 From: Davis Wolfermann Date: Wed, 2 Sep 2026 17:37:51 +0200 Subject: [PATCH 08/35] fix: clear TextFieldState undo history when clearing sensitive password fields --- .../changepassword/ChangePasswordViewModel.kt | 10 ++++++++++ .../changepassword/ChangePasswordViewModelTest.kt | 6 ++++++ 2 files changed, 16 insertions(+) diff --git a/feature/settings/src/main/kotlin/de/davis/keygo/feature/settings/presentation/changepassword/ChangePasswordViewModel.kt b/feature/settings/src/main/kotlin/de/davis/keygo/feature/settings/presentation/changepassword/ChangePasswordViewModel.kt index a392f907c..db117c310 100644 --- a/feature/settings/src/main/kotlin/de/davis/keygo/feature/settings/presentation/changepassword/ChangePasswordViewModel.kt +++ b/feature/settings/src/main/kotlin/de/davis/keygo/feature/settings/presentation/changepassword/ChangePasswordViewModel.kt @@ -1,5 +1,6 @@ package de.davis.keygo.feature.settings.presentation.changepassword +import androidx.compose.foundation.ExperimentalFoundationApi import androidx.compose.foundation.text.input.TextFieldState import androidx.compose.foundation.text.input.delete import androidx.compose.runtime.snapshotFlow @@ -216,10 +217,19 @@ internal class ChangePasswordViewModel( * abandoned instance is simply never mutated, so nothing ever re-triggers the flow, and * [ChangePasswordState.passwordScore] would freeze at whatever it was the moment before the * clear for the rest of the ViewModel's life. + * + * `undoState.clearHistory()` matters for the same reason: `edit {}` records the pre-clear text + * into the field's own undo stack, and this screen is deliberately kept alive (not torn down) + * while the app is locked, so without clearing it a re-authenticated user could Ctrl+Z the + * "cleared" password straight back. */ + @OptIn(ExperimentalFoundationApi::class) private fun clearSensitiveFields() { _state.value.currentPassword.edit { delete(0, length) } + _state.value.currentPassword.undoState.clearHistory() _state.value.newPassword.edit { delete(0, length) } + _state.value.newPassword.undoState.clearHistory() _state.value.confirmPassword.edit { delete(0, length) } + _state.value.confirmPassword.undoState.clearHistory() } } diff --git a/feature/settings/src/test/kotlin/de/davis/keygo/feature/settings/presentation/changepassword/ChangePasswordViewModelTest.kt b/feature/settings/src/test/kotlin/de/davis/keygo/feature/settings/presentation/changepassword/ChangePasswordViewModelTest.kt index 6bee0f889..0fce4a6b7 100644 --- a/feature/settings/src/test/kotlin/de/davis/keygo/feature/settings/presentation/changepassword/ChangePasswordViewModelTest.kt +++ b/feature/settings/src/test/kotlin/de/davis/keygo/feature/settings/presentation/changepassword/ChangePasswordViewModelTest.kt @@ -1,5 +1,6 @@ package de.davis.keygo.feature.settings.presentation.changepassword +import androidx.compose.foundation.ExperimentalFoundationApi import androidx.compose.runtime.snapshots.Snapshot import de.davis.keygo.core.identity.FakeAccountRepository import de.davis.keygo.core.identity.domain.model.Account @@ -32,6 +33,7 @@ import kotlin.test.AfterTest import kotlin.test.BeforeTest import kotlin.test.Test import kotlin.test.assertEquals +import kotlin.test.assertFalse @OptIn(ExperimentalCoroutinesApi::class) class ChangePasswordViewModelTest { @@ -318,6 +320,7 @@ class ChangePasswordViewModelTest { assertEquals(false, vm.state.value.showReauthDialog) } + @OptIn(ExperimentalFoundationApi::class) @Test fun `the session ending clears all three password fields`() = runTest(dispatcher) { val vm = viewModel() @@ -332,6 +335,9 @@ class ChangePasswordViewModelTest { assertEquals("", vm.state.value.currentPassword.text.toString()) assertEquals("", vm.state.value.newPassword.text.toString()) assertEquals("", vm.state.value.confirmPassword.text.toString()) + assertFalse(vm.state.value.currentPassword.undoState.canUndo) + assertFalse(vm.state.value.newPassword.undoState.canUndo) + assertFalse(vm.state.value.confirmPassword.undoState.canUndo) } @Test From 9e65e40b7c3d34e700e02ba52c21c232d9c70a4c Mon Sep 17 00:00:00 2001 From: Davis Wolfermann Date: Thu, 3 Sep 2026 01:44:14 +0200 Subject: [PATCH 09/35] refactor(app): derive the lock gate from the back stack instead of saved state --- .../keygo/app/presentation/MainActivity.kt | 2 +- .../navigation/AppNavigationState.kt | 16 --------- .../presentation/navigation/AppNavigator.kt | 31 +++++++++------- .../navigation/AppNavigatorTest.kt | 35 +++++++++++++------ 4 files changed, 43 insertions(+), 41 deletions(-) diff --git a/app/src/main/kotlin/de/davis/keygo/app/presentation/MainActivity.kt b/app/src/main/kotlin/de/davis/keygo/app/presentation/MainActivity.kt index c3780c993..c54852c12 100644 --- a/app/src/main/kotlin/de/davis/keygo/app/presentation/MainActivity.kt +++ b/app/src/main/kotlin/de/davis/keygo/app/presentation/MainActivity.kt @@ -168,7 +168,7 @@ private fun LockAppWhenSessionEnds( ) { val isLaunching = navigator.state.isLaunching LaunchedEffect(isLocked, isSessionActive, isLaunching) { - if (isLocked || (!isSessionActive && !isLaunching)) navigator.lock(AuthRoute()) + if (isLocked || (!isSessionActive && !isLaunching)) navigator.lock() } } diff --git a/app/src/main/kotlin/de/davis/keygo/app/presentation/navigation/AppNavigationState.kt b/app/src/main/kotlin/de/davis/keygo/app/presentation/navigation/AppNavigationState.kt index 0174c3dc1..91e9e882b 100644 --- a/app/src/main/kotlin/de/davis/keygo/app/presentation/navigation/AppNavigationState.kt +++ b/app/src/main/kotlin/de/davis/keygo/app/presentation/navigation/AppNavigationState.kt @@ -5,7 +5,6 @@ import androidx.compose.runtime.MutableState import androidx.compose.runtime.getValue import androidx.compose.runtime.mutableStateOf import androidx.compose.runtime.remember -import androidx.compose.runtime.saveable.rememberSaveable import androidx.compose.runtime.saveable.rememberSerializable import androidx.compose.runtime.setValue import androidx.navigation3.runtime.NavBackStack @@ -38,8 +37,6 @@ fun rememberAppNavigationState( ) { mutableStateOf(startRoute) } - val isGated = rememberSaveable { mutableStateOf(false) } - val launchStack = rememberNavBackStack(launchRoute) val backStacks = topLevelRoutes.associateWith { key -> rememberNavBackStack(key) } @@ -48,7 +45,6 @@ fun rememberAppNavigationState( launchStack = launchStack, topLevelRoute = topLevelRoute, backStacks = backStacks, - isGated = isGated, ) } } @@ -65,23 +61,11 @@ class AppNavigationState( val launchStack: NavBackStack, topLevelRoute: MutableState, val backStacks: Map>, - isGated: MutableState = mutableStateOf(false), ) { /** The selected navigation bar destination. */ var topLevelRoute: NavKey by topLevelRoute - /** - * True while [AppNavigator.lock] has a gate up. Makes [AppNavigator.goBack] a no-op. - * - * Backed by [rememberSaveable] in [rememberAppNavigationState], the same as [topLevelRoute] is - * backed by [rememberSerializable] - not just [remember]. [launchStack] survives a - * configuration change and a process death by construction; if a gate was on it when either - * happened, this flag has to come back `true` too, or a restored gate would show with nothing - * stopping [AppNavigator.goBack] from popping it away. - */ - var isGated: Boolean by isGated - /** Whether the launch flow still owns the window. */ val isLaunching: Boolean get() = launchStack.isNotEmpty() diff --git a/app/src/main/kotlin/de/davis/keygo/app/presentation/navigation/AppNavigator.kt b/app/src/main/kotlin/de/davis/keygo/app/presentation/navigation/AppNavigator.kt index ae6745c3c..662695850 100644 --- a/app/src/main/kotlin/de/davis/keygo/app/presentation/navigation/AppNavigator.kt +++ b/app/src/main/kotlin/de/davis/keygo/app/presentation/navigation/AppNavigator.kt @@ -3,6 +3,7 @@ package de.davis.keygo.app.presentation.navigation import androidx.navigation3.runtime.NavBackStack import androidx.navigation3.runtime.NavKey import de.davis.keygo.core.presentation.model.RouteDestination +import de.davis.keygo.feature.auth.presentation.AuthRoute /** * Handles navigation events by updating [AppNavigationState]. Everything the UI can do to the back @@ -10,6 +11,13 @@ import de.davis.keygo.core.presentation.model.RouteDestination */ class AppNavigator(val state: AppNavigationState) { + /** + * True while [lock]'s gate is the launch flow's top entry. Derived, so a restored gate reports + * itself. The type is what decides it: the `otpauth://` import shares this stack, so neither + * emptiness nor depth tells a gate from an import screen back may legitimately pop. + */ + private val isGated: Boolean get() = state.launchStack.lastOrNull() is AuthRoute + fun navigate(route: NavKey) { val isTopLevel = !state.isLaunching && route in state.backStacks if (isTopLevel) selectTopLevel(route) @@ -43,24 +51,22 @@ class AppNavigator(val state: AppNavigationState) { } /** - * Hides every tab behind [gate] and blocks all back navigation until [unlock] is called. Every - * tab other than the one currently selected is truncated to its base, tearing down whatever - * ViewModels it held; the selected tab, and anything already on the launch stack (an - * in-progress TOTP import, say), are left exactly as they were, restored for free once the gate + * Hides every tab behind an unlock gate and blocks all back navigation until [unlock] is + * called. Every tab other than the one currently selected is truncated to its base, tearing + * down whatever ViewModels it held; the selected tab, and anything already on the launch stack + * (an in-progress TOTP import, say), are left exactly as they were, restored once the gate * lifts. * * A no-op if already gated. This matters because the caller's trigger is collected by a * `LaunchedEffect` that re-fires on every fresh composition - including one rebuilt by a - * configuration change while the app is still locked - with no memory of having already run; - * [AppNavigationState.isGated] is durable across that rebuild, so it is what keeps a second - * call from pushing a second gate. + * configuration change while the app is still locked - with no memory of having already run. + * The restored stack is what remembers, so a second call pushes nothing. */ - fun lock(gate: NavKey) { - if (state.isGated) return + fun lock() { + if (isGated) return val activeRoute = state.topLevelRoute state.backStacks.forEach { (route, stack) -> if (route != activeRoute) stack.popToBase() } - pushOntoLaunchFlow(gate) - state.isGated = true + pushOntoLaunchFlow(AuthRoute()) } /** @@ -73,7 +79,6 @@ class AppNavigator(val state: AppNavigationState) { */ fun unlock() { state.launchStack.removeLastOrNull() - state.isGated = false } /** @@ -111,7 +116,7 @@ class AppNavigator(val state: AppNavigationState) { * gate, say), so a plain depth check would let back press pop the gate itself away. */ fun goBack() { - if (state.isGated) return + if (isGated) return val stack = state.currentStack if (stack.size > 1) stack.removeLastOrNull() } diff --git a/app/src/test/kotlin/de/davis/keygo/app/presentation/navigation/AppNavigatorTest.kt b/app/src/test/kotlin/de/davis/keygo/app/presentation/navigation/AppNavigatorTest.kt index d14c384ec..7354b7b5f 100644 --- a/app/src/test/kotlin/de/davis/keygo/app/presentation/navigation/AppNavigatorTest.kt +++ b/app/src/test/kotlin/de/davis/keygo/app/presentation/navigation/AppNavigatorTest.kt @@ -23,7 +23,6 @@ class AppNavigatorTest { launchStack = NavBackStack(launchRoute), topLevelRoute = mutableStateOf(RouteDestination.Home), backStacks = TOP_LEVEL_ROUTES.associateWith { NavBackStack(it) }, - isGated = mutableStateOf(false), ) return AppNavigator(state) } @@ -298,7 +297,7 @@ class AppNavigatorTest { val itemId = newItemId() navigator.showDetail(RouteDestination.ViewItem(itemId)) - navigator.lock(AuthRoute()) + navigator.lock() assertEquals( listOf(RouteDestination.Home, RouteDestination.ViewItem(itemId)), @@ -315,7 +314,7 @@ class AppNavigatorTest { val navigator = navigator() navigator.replaceLaunchFlow(SelectItemForTotpRoute(DEEP_LINK_URI)) - navigator.lock(AuthRoute()) + navigator.lock() assertEquals( listOf(SelectItemForTotpRoute(DEEP_LINK_URI), AuthRoute()), @@ -328,7 +327,7 @@ class AppNavigatorTest { val navigator = unlocked() navigator.navigate(SettingsRoute) - navigator.lock(AuthRoute()) + navigator.lock() assertTrue(navigator.state.isLaunching) assertEquals(listOf(AuthRoute()), navigator.shown) @@ -339,7 +338,7 @@ class AppNavigatorTest { val navigator = unlocked() navigator.navigate(SettingsRoute) navigator.navigate(ChangePasswordRoute) - navigator.lock(AuthRoute()) + navigator.lock() navigator.unlock() @@ -351,7 +350,7 @@ class AppNavigatorTest { fun `unlocking reveals a picker that was preserved under the gate`() { val navigator = navigator() navigator.replaceLaunchFlow(SelectItemForTotpRoute(DEEP_LINK_URI)) - navigator.lock(AuthRoute()) + navigator.lock() navigator.unlock() @@ -362,7 +361,7 @@ class AppNavigatorTest { fun `back cannot pop the gate away, even over a picker underneath it`() { val navigator = navigator() navigator.replaceLaunchFlow(SelectItemForTotpRoute(DEEP_LINK_URI)) - navigator.lock(AuthRoute()) + navigator.lock() navigator.goBack() @@ -377,7 +376,7 @@ class AppNavigatorTest { val navigator = unlocked() navigator.navigate(SettingsRoute) navigator.navigate(ChangePasswordRoute) - navigator.lock(AuthRoute()) + navigator.lock() navigator.unlock() navigator.goBack() @@ -405,11 +404,10 @@ class AppNavigatorTest { launchStack = NavBackStack(AuthRoute()), topLevelRoute = mutableStateOf(RouteDestination.Home), backStacks = TOP_LEVEL_ROUTES.associateWith { NavBackStack(it) }, - isGated = mutableStateOf(true), ) val navigator = AppNavigator(state) - navigator.lock(AuthRoute()) + navigator.lock() assertEquals(listOf(AuthRoute()), navigator.shown) } @@ -420,7 +418,6 @@ class AppNavigatorTest { launchStack = NavBackStack(SelectItemForTotpRoute(DEEP_LINK_URI), AuthRoute()), topLevelRoute = mutableStateOf(RouteDestination.Home), backStacks = TOP_LEVEL_ROUTES.associateWith { NavBackStack(it) }, - isGated = mutableStateOf(true), ) val navigator = AppNavigator(state) @@ -432,6 +429,22 @@ class AppNavigatorTest { ) } + @Test + fun `an import restored on the launch stack does not block back the way a gate does`() { + // Out of reach is not the same as back being forbidden: stepping from the import's + // assign screen back to its picker is ordinary navigation. + val state = AppNavigationState( + launchStack = NavBackStack(SelectItemForTotpRoute(DEEP_LINK_URI), TestAssignRoute), + topLevelRoute = mutableStateOf(RouteDestination.Home), + backStacks = TOP_LEVEL_ROUTES.associateWith { NavBackStack(it) }, + ) + val navigator = AppNavigator(state) + + navigator.goBack() + + assertEquals(listOf(SelectItemForTotpRoute(DEEP_LINK_URI)), navigator.shown) + } + private fun unlocked(): AppNavigator = navigator().apply { finishLaunchFlow() } private companion object { From 97ca92dc316df4e734af4e5fd5052ea1a2edea38 Mon Sep 17 00:00:00 2001 From: Davis Wolfermann Date: Thu, 3 Sep 2026 01:44:36 +0200 Subject: [PATCH 10/35] refactor(app): rename the launch stack to the overlay --- .../keygo/app/presentation/MainActivity.kt | 18 +++--- .../navigation/AppNavigationState.kt | 24 ++++---- .../presentation/navigation/AppNavigator.kt | 39 +++++++------ .../presentation/navigation/EntryProvider.kt | 8 +-- .../navigation/AppNavigatorTest.kt | 56 +++++++++---------- 5 files changed, 72 insertions(+), 73 deletions(-) diff --git a/app/src/main/kotlin/de/davis/keygo/app/presentation/MainActivity.kt b/app/src/main/kotlin/de/davis/keygo/app/presentation/MainActivity.kt index c54852c12..10b2c0046 100644 --- a/app/src/main/kotlin/de/davis/keygo/app/presentation/MainActivity.kt +++ b/app/src/main/kotlin/de/davis/keygo/app/presentation/MainActivity.kt @@ -149,14 +149,14 @@ private fun App( /** * Locks the app in two cases: * - [isLocked] catches the instant a session that was active ends, whether the app proper - * currently owns the window or a launch-flow screen does (an in-progress TOTP-import picker, - * say) - [AppNavigator.lock] pushes over either without disturbing what's underneath. - * - The level check (`!isSessionActive && !isLaunching`) catches a back stack a configuration + * currently owns the window or an overlay screen does (an in-progress TOTP-import picker, say) + * - [AppNavigator.lock] pushes over either without disturbing what's underneath. + * - The level check (`!isSessionActive && !isOverlaid`) catches a back stack a configuration * change or process death restored straight into the app proper with a session that never got * re-established: [isLocked] alone can't see this, since it only fires on a transition a freshly - * restored [AppViewModel] has no memory of. Gated by `!isLaunching` so it never fires during - * onboarding or the very first login (both show with the launch flow already owning the window - * and no session yet, which looks the same as this case unless launch state is checked too), and + * restored [AppViewModel] has no memory of. Gated by `!isOverlaid` so it never fires during + * onboarding or the very first login (both show with the overlay already owning the window + * and no session yet, which looks the same as this case unless the overlay is checked too), and * so it never fights [AppNavigator.lock]'s own idempotency for a gate or picker Nav3 already * restored correctly. */ @@ -166,9 +166,9 @@ private fun LockAppWhenSessionEnds( isSessionActive: Boolean, navigator: AppNavigator, ) { - val isLaunching = navigator.state.isLaunching - LaunchedEffect(isLocked, isSessionActive, isLaunching) { - if (isLocked || (!isSessionActive && !isLaunching)) navigator.lock() + val isOverlaid = navigator.state.isOverlaid + LaunchedEffect(isLocked, isSessionActive, isOverlaid) { + if (isLocked || (!isSessionActive && !isOverlaid)) navigator.lock() } } diff --git a/app/src/main/kotlin/de/davis/keygo/app/presentation/navigation/AppNavigationState.kt b/app/src/main/kotlin/de/davis/keygo/app/presentation/navigation/AppNavigationState.kt index 91e9e882b..5ab6e1e8a 100644 --- a/app/src/main/kotlin/de/davis/keygo/app/presentation/navigation/AppNavigationState.kt +++ b/app/src/main/kotlin/de/davis/keygo/app/presentation/navigation/AppNavigationState.kt @@ -20,8 +20,8 @@ import de.davis.keygo.core.ui.navigation.rememberNavEntryDecorators /** * Creates the app's navigation state. It survives configuration changes and process death. * - * @param launchRoute what the launch flow starts on. Only used the first time the state is - * created; after that the saved stack wins. + * @param launchRoute what the overlay starts on. Only used the first time the state is created; + * after that the saved stack wins. * @param startRoute the top level route the app opens on. Must be one of [topLevelRoutes]. * @param topLevelRoutes the navigation bar's destinations, one back stack each. */ @@ -37,12 +37,12 @@ fun rememberAppNavigationState( ) { mutableStateOf(startRoute) } - val launchStack = rememberNavBackStack(launchRoute) + val overlayStack = rememberNavBackStack(launchRoute) val backStacks = topLevelRoutes.associateWith { key -> rememberNavBackStack(key) } return remember(startRoute, topLevelRoutes) { AppNavigationState( - launchStack = launchStack, + overlayStack = overlayStack, topLevelRoute = topLevelRoute, backStacks = backStacks, ) @@ -52,13 +52,13 @@ fun rememberAppNavigationState( /** * The app's navigation state, modified through [AppNavigator]. It holds two things: * - * - The **launch stack**, carrying whatever has to happen before the app proper: unlocking, first - * run, or importing an incoming `otpauth://` link. While it holds anything it is all that shows. + * - The **overlay stack**: the unlock gate, first run, or an incoming `otpauth://` link. While it + * holds anything it is all that shows. * - One **back stack per top level route**, each keeping its own history. Only the selected one is * shown, with nothing underneath it, so back out of its base leaves the app. */ class AppNavigationState( - val launchStack: NavBackStack, + val overlayStack: NavBackStack, topLevelRoute: MutableState, val backStacks: Map>, ) { @@ -66,12 +66,12 @@ class AppNavigationState( /** The selected navigation bar destination. */ var topLevelRoute: NavKey by topLevelRoute - /** Whether the launch flow still owns the window. */ - val isLaunching: Boolean get() = launchStack.isNotEmpty() + /** Whether the overlay owns the window, hiding the app proper underneath. */ + val isOverlaid: Boolean get() = overlayStack.isNotEmpty() /** The stack destinations are currently pushed onto and popped from. */ val currentStack: NavBackStack - get() = if (isLaunching) launchStack else backStacks.getValue(topLevelRoute) + get() = if (isOverlaid) overlayStack else backStacks.getValue(topLevelRoute) /** * What the detail pane is showing, or null while the list has the window to itself. @@ -92,12 +92,12 @@ class AppNavigationState( fun toDecoratedEntries( entryProvider: (NavKey) -> NavEntry, ): List> { - val launchEntries = rememberDecoratedEntries(launchStack, entryProvider) + val overlayEntries = rememberDecoratedEntries(overlayStack, entryProvider) val topLevelEntries = backStacks.mapValues { (_, stack) -> rememberDecoratedEntries(stack, entryProvider) } - return if (isLaunching) launchEntries + return if (isOverlaid) overlayEntries else topLevelEntries.getValue(topLevelRoute) } } diff --git a/app/src/main/kotlin/de/davis/keygo/app/presentation/navigation/AppNavigator.kt b/app/src/main/kotlin/de/davis/keygo/app/presentation/navigation/AppNavigator.kt index 662695850..786209d01 100644 --- a/app/src/main/kotlin/de/davis/keygo/app/presentation/navigation/AppNavigator.kt +++ b/app/src/main/kotlin/de/davis/keygo/app/presentation/navigation/AppNavigator.kt @@ -12,14 +12,14 @@ import de.davis.keygo.feature.auth.presentation.AuthRoute class AppNavigator(val state: AppNavigationState) { /** - * True while [lock]'s gate is the launch flow's top entry. Derived, so a restored gate reports + * True while [lock]'s gate is the overlay's top entry. Derived, so a restored gate reports * itself. The type is what decides it: the `otpauth://` import shares this stack, so neither * emptiness nor depth tells a gate from an import screen back may legitimately pop. */ - private val isGated: Boolean get() = state.launchStack.lastOrNull() is AuthRoute + private val isGated: Boolean get() = state.overlayStack.lastOrNull() is AuthRoute fun navigate(route: NavKey) { - val isTopLevel = !state.isLaunching && route in state.backStacks + val isTopLevel = !state.isOverlaid && route in state.backStacks if (isTopLevel) selectTopLevel(route) else state.currentStack.add(route) } @@ -34,28 +34,27 @@ class AppNavigator(val state: AppNavigationState) { else state.topLevelRoute = route } - /** Replaces the launch flow with [route], so back from it leaves the app. */ - fun replaceLaunchFlow(route: NavKey) { - state.launchStack.clear() - state.launchStack.add(route) + /** Replaces the overlay with [route], so back from it leaves the app. */ + fun replaceOverlay(route: NavKey) { + state.overlayStack.clear() + state.overlayStack.add(route) } - /** Ends the launch flow and hands the window to the app proper. */ - fun finishLaunchFlow() { - state.launchStack.clear() + /** Clears the overlay and hands the window to the app proper. */ + fun clearOverlay() { + state.overlayStack.clear() } - /** Adds [route] to the launch flow without disturbing whatever is already on it. */ - fun pushOntoLaunchFlow(route: NavKey) { - state.launchStack.add(route) + /** Adds [route] to the overlay without disturbing whatever is already on it. */ + fun pushOntoOverlay(route: NavKey) { + state.overlayStack.add(route) } /** * Hides every tab behind an unlock gate and blocks all back navigation until [unlock] is * called. Every tab other than the one currently selected is truncated to its base, tearing - * down whatever ViewModels it held; the selected tab, and anything already on the launch stack - * (an in-progress TOTP import, say), are left exactly as they were, restored once the gate - * lifts. + * down whatever ViewModels it held; the selected tab, and anything already on the overlay (an + * in-progress TOTP import, say), are left exactly as they were, restored once the gate lifts. * * A no-op if already gated. This matters because the caller's trigger is collected by a * `LaunchedEffect` that re-fires on every fresh composition - including one rebuilt by a @@ -66,7 +65,7 @@ class AppNavigator(val state: AppNavigationState) { if (isGated) return val activeRoute = state.topLevelRoute state.backStacks.forEach { (route, stack) -> if (route != activeRoute) stack.popToBase() } - pushOntoLaunchFlow(AuthRoute()) + pushOntoOverlay(AuthRoute()) } /** @@ -74,11 +73,11 @@ class AppNavigator(val state: AppNavigationState) { * * Pops unconditionally rather than only while gated. This is also what dismisses the cold-start * auth screen and the onboarding screen, neither of which [lock] ever gated - they are on the - * launch stack because they were the launch route. Returning early on `!isGated` would leave + * overlay because they were the launch route. Returning early on `!isGated` would leave * both up for good after a successful first login. */ fun unlock() { - state.launchStack.removeLastOrNull() + state.overlayStack.removeLastOrNull() } /** @@ -112,7 +111,7 @@ class AppNavigator(val state: AppNavigationState) { /** * Goes back one destination, but never down to nothing, and never while a lock's gate is up. - * The launch stack can hold more than one entry while gated (a picker preserved under the + * The overlay can hold more than one entry while gated (a picker preserved under the * gate, say), so a plain depth check would let back press pop the gate itself away. */ fun goBack() { diff --git a/app/src/main/kotlin/de/davis/keygo/app/presentation/navigation/EntryProvider.kt b/app/src/main/kotlin/de/davis/keygo/app/presentation/navigation/EntryProvider.kt index d57114051..d67012a84 100644 --- a/app/src/main/kotlin/de/davis/keygo/app/presentation/navigation/EntryProvider.kt +++ b/app/src/main/kotlin/de/davis/keygo/app/presentation/navigation/EntryProvider.kt @@ -63,7 +63,7 @@ fun keyGoEntryProvider(navigator: AppNavigator, hasAccess: Boolean): (NavKey) -> assignTotpEntries( metadata = WindowOwning, - onImportFinished = { navigator.finishLaunchFlow() }, + onImportFinished = { navigator.clearOverlay() }, navigateUp = { navigator.goBack() }, ) @@ -132,9 +132,9 @@ fun keyGoEntryProvider(navigator: AppNavigator, hasAccess: Boolean): (NavKey) -> } } -/** Replaces the launch flow, so back from the gate leaves the app rather than a consumed link. */ +/** Replaces the overlay, so back from the gate leaves the app rather than a consumed link. */ internal fun AppNavigator.openGateFor(hasAccess: Boolean, uri: String) { - replaceLaunchFlow(if (hasAccess) AuthRoute(uri = uri) else OnboardingRoute(uri = uri)) + replaceOverlay(if (hasAccess) AuthRoute(uri = uri) else OnboardingRoute(uri = uri)) } /** @@ -144,5 +144,5 @@ internal fun AppNavigator.openGateFor(hasAccess: Boolean, uri: String) { */ private fun AppNavigator.finishUnlock(totpUri: String?) { unlock() - if (totpUri != null) pushOntoLaunchFlow(SelectItemForTotpRoute(totpUri)) + if (totpUri != null) pushOntoOverlay(SelectItemForTotpRoute(totpUri)) } diff --git a/app/src/test/kotlin/de/davis/keygo/app/presentation/navigation/AppNavigatorTest.kt b/app/src/test/kotlin/de/davis/keygo/app/presentation/navigation/AppNavigatorTest.kt index 7354b7b5f..e4ec2b04a 100644 --- a/app/src/test/kotlin/de/davis/keygo/app/presentation/navigation/AppNavigatorTest.kt +++ b/app/src/test/kotlin/de/davis/keygo/app/presentation/navigation/AppNavigatorTest.kt @@ -20,7 +20,7 @@ class AppNavigatorTest { private fun navigator(launchRoute: NavKey = AuthRoute()): AppNavigator { val state = AppNavigationState( - launchStack = NavBackStack(launchRoute), + overlayStack = NavBackStack(launchRoute), topLevelRoute = mutableStateOf(RouteDestination.Home), backStacks = TOP_LEVEL_ROUTES.associateWith { NavBackStack(it) }, ) @@ -28,21 +28,21 @@ class AppNavigatorTest { } private val AppNavigator.shown: List - get() = if (state.isLaunching) state.launchStack.toList() + get() = if (state.isOverlaid) state.overlayStack.toList() else state.backStacks.getValue(state.topLevelRoute).toList() - // ---- the launch flow ---- + // ---- the overlay ---- @Test - fun `the launch flow owns the window until it finishes`() { + fun `the overlay owns the window until it is cleared`() { val navigator = navigator() - assertTrue(navigator.state.isLaunching) + assertTrue(navigator.state.isOverlaid) assertEquals(listOf(AuthRoute()), navigator.shown) - navigator.finishLaunchFlow() + navigator.clearOverlay() - assertFalse(navigator.state.isLaunching) + assertFalse(navigator.state.isOverlaid) assertEquals(listOf(RouteDestination.Home), navigator.shown) } @@ -68,16 +68,16 @@ class AppNavigatorTest { fun `the picker replaces the gate, so back leaves the app`() { val navigator = navigator() - navigator.replaceLaunchFlow(SelectItemForTotpRoute(DEEP_LINK_URI)) + navigator.replaceOverlay(SelectItemForTotpRoute(DEEP_LINK_URI)) assertEquals(listOf(SelectItemForTotpRoute(DEEP_LINK_URI)), navigator.shown) assertEquals(1, navigator.shown.size) } @Test - fun `assigning a code pushes onto the launch flow and back returns to the picker`() { + fun `assigning a code pushes onto the overlay and back returns to the picker`() { val navigator = navigator() - navigator.replaceLaunchFlow(SelectItemForTotpRoute(DEEP_LINK_URI)) + navigator.replaceOverlay(SelectItemForTotpRoute(DEEP_LINK_URI)) navigator.navigate(TestAssignRoute) assertEquals(listOf(SelectItemForTotpRoute(DEEP_LINK_URI), TestAssignRoute), navigator.shown) @@ -87,22 +87,22 @@ class AppNavigatorTest { } @Test - fun `back never empties the launch flow, so the app is what exits`() { + fun `back never empties the overlay, so the app is what exits`() { val navigator = navigator() navigator.goBack() - assertTrue(navigator.state.isLaunching) + assertTrue(navigator.state.isOverlaid) assertEquals(listOf(AuthRoute()), navigator.shown) } @Test - fun `a top level route is not switched to while the launch flow is running`() { + fun `a top level route is not switched to while the overlay owns the window`() { val navigator = navigator() navigator.navigate(SettingsRoute) - assertTrue(navigator.state.isLaunching) + assertTrue(navigator.state.isOverlaid) assertEquals(listOf(AuthRoute(), SettingsRoute), navigator.shown) } @@ -310,9 +310,9 @@ class AppNavigatorTest { } @Test - fun `locking pushes the gate without clearing what was already on the launch stack`() { + fun `locking pushes the gate without clearing what was already on the overlay`() { val navigator = navigator() - navigator.replaceLaunchFlow(SelectItemForTotpRoute(DEEP_LINK_URI)) + navigator.replaceOverlay(SelectItemForTotpRoute(DEEP_LINK_URI)) navigator.lock() @@ -323,13 +323,13 @@ class AppNavigatorTest { } @Test - fun `locking from a tab pushes the gate onto an otherwise empty launch stack`() { + fun `locking from a tab pushes the gate onto an otherwise empty overlay`() { val navigator = unlocked() navigator.navigate(SettingsRoute) navigator.lock() - assertTrue(navigator.state.isLaunching) + assertTrue(navigator.state.isOverlaid) assertEquals(listOf(AuthRoute()), navigator.shown) } @@ -342,14 +342,14 @@ class AppNavigatorTest { navigator.unlock() - assertFalse(navigator.state.isLaunching) + assertFalse(navigator.state.isOverlaid) assertEquals(listOf(SettingsRoute, ChangePasswordRoute), navigator.shown) } @Test fun `unlocking reveals a picker that was preserved under the gate`() { val navigator = navigator() - navigator.replaceLaunchFlow(SelectItemForTotpRoute(DEEP_LINK_URI)) + navigator.replaceOverlay(SelectItemForTotpRoute(DEEP_LINK_URI)) navigator.lock() navigator.unlock() @@ -360,7 +360,7 @@ class AppNavigatorTest { @Test fun `back cannot pop the gate away, even over a picker underneath it`() { val navigator = navigator() - navigator.replaceLaunchFlow(SelectItemForTotpRoute(DEEP_LINK_URI)) + navigator.replaceOverlay(SelectItemForTotpRoute(DEEP_LINK_URI)) navigator.lock() navigator.goBack() @@ -387,21 +387,21 @@ class AppNavigatorTest { @Test fun `unlocking a cold start gate hands the window to the app proper`() { // AppNavigator.finishUnlock calls unlock() for the cold-start auth and onboarding screens - // too, which are on the launch stack without lock() ever having gated anything. Popping + // too, which are on the overlay without lock() ever having gated anything. Popping // has to happen there as well, or authenticating at cold start would leave the auth screen // up forever. val navigator = navigator() navigator.unlock() - assertFalse(navigator.state.isLaunching) + assertFalse(navigator.state.isOverlaid) assertEquals(listOf(RouteDestination.Home), navigator.shown) } @Test fun `a gate restored from saved state is not pushed a second time`() { val state = AppNavigationState( - launchStack = NavBackStack(AuthRoute()), + overlayStack = NavBackStack(AuthRoute()), topLevelRoute = mutableStateOf(RouteDestination.Home), backStacks = TOP_LEVEL_ROUTES.associateWith { NavBackStack(it) }, ) @@ -415,7 +415,7 @@ class AppNavigatorTest { @Test fun `back still cannot pop a gate restored from saved state, even over a preserved picker`() { val state = AppNavigationState( - launchStack = NavBackStack(SelectItemForTotpRoute(DEEP_LINK_URI), AuthRoute()), + overlayStack = NavBackStack(SelectItemForTotpRoute(DEEP_LINK_URI), AuthRoute()), topLevelRoute = mutableStateOf(RouteDestination.Home), backStacks = TOP_LEVEL_ROUTES.associateWith { NavBackStack(it) }, ) @@ -430,11 +430,11 @@ class AppNavigatorTest { } @Test - fun `an import restored on the launch stack does not block back the way a gate does`() { + fun `an import restored on the overlay does not block back the way a gate does`() { // Out of reach is not the same as back being forbidden: stepping from the import's // assign screen back to its picker is ordinary navigation. val state = AppNavigationState( - launchStack = NavBackStack(SelectItemForTotpRoute(DEEP_LINK_URI), TestAssignRoute), + overlayStack = NavBackStack(SelectItemForTotpRoute(DEEP_LINK_URI), TestAssignRoute), topLevelRoute = mutableStateOf(RouteDestination.Home), backStacks = TOP_LEVEL_ROUTES.associateWith { NavBackStack(it) }, ) @@ -445,7 +445,7 @@ class AppNavigatorTest { assertEquals(listOf(SelectItemForTotpRoute(DEEP_LINK_URI)), navigator.shown) } - private fun unlocked(): AppNavigator = navigator().apply { finishLaunchFlow() } + private fun unlocked(): AppNavigator = navigator().apply { clearOverlay() } private companion object { val TOP_LEVEL_ROUTES: Set = linkedSetOf( From 67ea13927be68179c1c83f981f86e0e4daf5e940 Mon Sep 17 00:00:00 2001 From: Davis Wolfermann Date: Thu, 3 Sep 2026 01:44:48 +0200 Subject: [PATCH 11/35] refactor(app): stop truncating the other tabs when locking --- .../keygo/app/presentation/navigation/AppNavigator.kt | 9 +++------ .../app/presentation/navigation/AppNavigatorTest.kt | 4 ++-- 2 files changed, 5 insertions(+), 8 deletions(-) diff --git a/app/src/main/kotlin/de/davis/keygo/app/presentation/navigation/AppNavigator.kt b/app/src/main/kotlin/de/davis/keygo/app/presentation/navigation/AppNavigator.kt index 786209d01..7e422e616 100644 --- a/app/src/main/kotlin/de/davis/keygo/app/presentation/navigation/AppNavigator.kt +++ b/app/src/main/kotlin/de/davis/keygo/app/presentation/navigation/AppNavigator.kt @@ -51,10 +51,9 @@ class AppNavigator(val state: AppNavigationState) { } /** - * Hides every tab behind an unlock gate and blocks all back navigation until [unlock] is - * called. Every tab other than the one currently selected is truncated to its base, tearing - * down whatever ViewModels it held; the selected tab, and anything already on the overlay (an - * in-progress TOTP import, say), are left exactly as they were, restored once the gate lifts. + * Hides what is showing behind an unlock gate and blocks back until [unlock]. Nothing + * underneath is disturbed or torn down: a screen holding a secret clears it by observing the + * session, the way ChangePasswordViewModel does. * * A no-op if already gated. This matters because the caller's trigger is collected by a * `LaunchedEffect` that re-fires on every fresh composition - including one rebuilt by a @@ -63,8 +62,6 @@ class AppNavigator(val state: AppNavigationState) { */ fun lock() { if (isGated) return - val activeRoute = state.topLevelRoute - state.backStacks.forEach { (route, stack) -> if (route != activeRoute) stack.popToBase() } pushOntoOverlay(AuthRoute()) } diff --git a/app/src/test/kotlin/de/davis/keygo/app/presentation/navigation/AppNavigatorTest.kt b/app/src/test/kotlin/de/davis/keygo/app/presentation/navigation/AppNavigatorTest.kt index e4ec2b04a..55201141a 100644 --- a/app/src/test/kotlin/de/davis/keygo/app/presentation/navigation/AppNavigatorTest.kt +++ b/app/src/test/kotlin/de/davis/keygo/app/presentation/navigation/AppNavigatorTest.kt @@ -289,7 +289,7 @@ class AppNavigatorTest { // ---- locking ---- @Test - fun `locking truncates every tab but the one shown`() { + fun `locking leaves every tab exactly as it was`() { val navigator = unlocked() navigator.navigate(SettingsRoute) navigator.navigate(ChangePasswordRoute) @@ -304,7 +304,7 @@ class AppNavigatorTest { navigator.state.backStacks.getValue(RouteDestination.Home).toList(), ) assertEquals( - listOf(SettingsRoute), + listOf(SettingsRoute, ChangePasswordRoute), navigator.state.backStacks.getValue(SettingsRoute).toList(), ) } From de4e7a0ff9b30d617a73a307d1f7592507e91329 Mon Sep 17 00:00:00 2001 From: Davis Wolfermann Date: Thu, 3 Sep 2026 01:45:42 +0200 Subject: [PATCH 12/35] fix(app): take first run down without unlock so the gate guard holds --- .../presentation/navigation/AppNavigator.kt | 10 ++---- .../presentation/navigation/EntryProvider.kt | 21 +++++++---- .../navigation/AppNavigatorTest.kt | 35 ++++++++++++++++--- 3 files changed, 47 insertions(+), 19 deletions(-) diff --git a/app/src/main/kotlin/de/davis/keygo/app/presentation/navigation/AppNavigator.kt b/app/src/main/kotlin/de/davis/keygo/app/presentation/navigation/AppNavigator.kt index 7e422e616..a9a4e516e 100644 --- a/app/src/main/kotlin/de/davis/keygo/app/presentation/navigation/AppNavigator.kt +++ b/app/src/main/kotlin/de/davis/keygo/app/presentation/navigation/AppNavigator.kt @@ -65,15 +65,9 @@ class AppNavigator(val state: AppNavigationState) { pushOntoOverlay(AuthRoute()) } - /** - * Lifts the gate [lock] put up, revealing whatever was underneath it. - * - * Pops unconditionally rather than only while gated. This is also what dismisses the cold-start - * auth screen and the onboarding screen, neither of which [lock] ever gated - they are on the - * overlay because they were the launch route. Returning early on `!isGated` would leave - * both up for good after a successful first login. - */ + /** Lifts the gate. Only gates reach here; first run is taken down with [clearOverlay]. */ fun unlock() { + if (!isGated) return state.overlayStack.removeLastOrNull() } diff --git a/app/src/main/kotlin/de/davis/keygo/app/presentation/navigation/EntryProvider.kt b/app/src/main/kotlin/de/davis/keygo/app/presentation/navigation/EntryProvider.kt index d67012a84..5bcb6e0bc 100644 --- a/app/src/main/kotlin/de/davis/keygo/app/presentation/navigation/EntryProvider.kt +++ b/app/src/main/kotlin/de/davis/keygo/app/presentation/navigation/EntryProvider.kt @@ -74,7 +74,7 @@ fun keyGoEntryProvider(navigator: AppNavigator, hasAccess: Boolean): (NavKey) -> onboardingEntries( metadata = WindowOwning, - onSuccess = { totpUri -> navigator.finishUnlock(totpUri) }, + onSuccess = { totpUri -> navigator.finishFirstRun(totpUri) }, ) dashboardEntries(navigator = navigator) @@ -137,12 +137,19 @@ internal fun AppNavigator.openGateFor(hasAccess: Boolean, uri: String) { replaceOverlay(if (hasAccess) AuthRoute(uri = uri) else OnboardingRoute(uri = uri)) } -/** - * Pops whatever gate or cold-start screen just finished authenticating. That reveals a picker - * preserved underneath a lock's gate on its own; a fresh totpUri from this run instead replaces - * that reveal with the picker for it, the same as it always did at cold start. - */ -private fun AppNavigator.finishUnlock(totpUri: String?) { +/** Lifts the gate that just authenticated, revealing a picker preserved under it. */ +internal fun AppNavigator.finishUnlock(totpUri: String?) { unlock() + startImport(totpUri) +} + +/** Takes first run down. It is on the overlay as the launch route, not as a gate. */ +internal fun AppNavigator.finishFirstRun(totpUri: String?) { + clearOverlay() + startImport(totpUri) +} + +/** A code this run carried replaces whatever the dismissal revealed. */ +private fun AppNavigator.startImport(totpUri: String?) { if (totpUri != null) pushOntoOverlay(SelectItemForTotpRoute(totpUri)) } diff --git a/app/src/test/kotlin/de/davis/keygo/app/presentation/navigation/AppNavigatorTest.kt b/app/src/test/kotlin/de/davis/keygo/app/presentation/navigation/AppNavigatorTest.kt index 55201141a..64d9b4793 100644 --- a/app/src/test/kotlin/de/davis/keygo/app/presentation/navigation/AppNavigatorTest.kt +++ b/app/src/test/kotlin/de/davis/keygo/app/presentation/navigation/AppNavigatorTest.kt @@ -386,10 +386,8 @@ class AppNavigatorTest { @Test fun `unlocking a cold start gate hands the window to the app proper`() { - // AppNavigator.finishUnlock calls unlock() for the cold-start auth and onboarding screens - // too, which are on the overlay without lock() ever having gated anything. Popping - // has to happen there as well, or authenticating at cold start would leave the auth screen - // up forever. + // The cold-start screen is on the overlay as the launch route, not because lock() gated + // it - but it is an AuthRoute all the same, so unlock() pops it. val navigator = navigator() navigator.unlock() @@ -398,6 +396,35 @@ class AppNavigatorTest { assertEquals(listOf(RouteDestination.Home), navigator.shown) } + @Test + fun `finishing first run hands the window to the app proper`() { + // First run is not a gate, so unlock() would refuse it. It is cleared instead. + val navigator = navigator(launchRoute = OnboardingRoute()) + + navigator.finishFirstRun(totpUri = null) + + assertFalse(navigator.state.isOverlaid) + assertEquals(listOf(RouteDestination.Home), navigator.shown) + } + + @Test + fun `a code carried through first run opens its picker`() { + val navigator = navigator(launchRoute = OnboardingRoute(uri = DEEP_LINK_URI)) + + navigator.finishFirstRun(DEEP_LINK_URI) + + assertEquals(listOf(SelectItemForTotpRoute(DEEP_LINK_URI)), navigator.shown) + } + + @Test + fun `a code carried through the unlock opens its picker`() { + val navigator = navigator(launchRoute = AuthRoute(uri = DEEP_LINK_URI)) + + navigator.finishUnlock(DEEP_LINK_URI) + + assertEquals(listOf(SelectItemForTotpRoute(DEEP_LINK_URI)), navigator.shown) + } + @Test fun `a gate restored from saved state is not pushed a second time`() { val state = AppNavigationState( From f8f4011224232017c8f0cabeb71a7ac41de18a04 Mon Sep 17 00:00:00 2001 From: Davis Wolfermann Date: Thu, 3 Sep 2026 01:45:57 +0200 Subject: [PATCH 13/35] docs(app): shorten the navigation comments --- .../davis/keygo/app/presentation/MainActivity.kt | 16 ++++------------ .../navigation/AppNavigationState.kt | 8 ++++---- .../app/presentation/navigation/AppNavigator.kt | 13 ++++--------- 3 files changed, 12 insertions(+), 25 deletions(-) diff --git a/app/src/main/kotlin/de/davis/keygo/app/presentation/MainActivity.kt b/app/src/main/kotlin/de/davis/keygo/app/presentation/MainActivity.kt index 10b2c0046..cdb969dc5 100644 --- a/app/src/main/kotlin/de/davis/keygo/app/presentation/MainActivity.kt +++ b/app/src/main/kotlin/de/davis/keygo/app/presentation/MainActivity.kt @@ -147,18 +147,10 @@ private fun App( } /** - * Locks the app in two cases: - * - [isLocked] catches the instant a session that was active ends, whether the app proper - * currently owns the window or an overlay screen does (an in-progress TOTP-import picker, say) - * - [AppNavigator.lock] pushes over either without disturbing what's underneath. - * - The level check (`!isSessionActive && !isOverlaid`) catches a back stack a configuration - * change or process death restored straight into the app proper with a session that never got - * re-established: [isLocked] alone can't see this, since it only fires on a transition a freshly - * restored [AppViewModel] has no memory of. Gated by `!isOverlaid` so it never fires during - * onboarding or the very first login (both show with the overlay already owning the window - * and no session yet, which looks the same as this case unless the overlay is checked too), and - * so it never fights [AppNavigator.lock]'s own idempotency for a gate or picker Nav3 already - * restored correctly. + * [isLocked] catches a live session ending. `!isSessionActive && !isOverlaid` catches a back stack + * restored into the app proper with no session, which a freshly restored [AppViewModel] has no + * transition to report; `!isOverlaid` keeps that off onboarding and the first login, which run + * without a session by design. */ @Composable private fun LockAppWhenSessionEnds( diff --git a/app/src/main/kotlin/de/davis/keygo/app/presentation/navigation/AppNavigationState.kt b/app/src/main/kotlin/de/davis/keygo/app/presentation/navigation/AppNavigationState.kt index 5ab6e1e8a..ccb7828fa 100644 --- a/app/src/main/kotlin/de/davis/keygo/app/presentation/navigation/AppNavigationState.kt +++ b/app/src/main/kotlin/de/davis/keygo/app/presentation/navigation/AppNavigationState.kt @@ -37,6 +37,7 @@ fun rememberAppNavigationState( ) { mutableStateOf(startRoute) } + val overlayStack = rememberNavBackStack(launchRoute) val backStacks = topLevelRoutes.associateWith { key -> rememberNavBackStack(key) } @@ -76,10 +77,9 @@ class AppNavigationState( /** * What the detail pane is showing, or null while the list has the window to itself. * - * A dialog is pushed onto the same stack but is drawn over the pane rather than taking it, so - * it is looked past. Reporting nothing while one is open makes the list pick a row on its own - * and push it above the dialog, which closes the dialog and leaves the pane the only thing the - * scene knows about. + * A dialog is pushed onto the same stack but drawn over the pane, so it is looked past. + * Reporting nothing while one is open makes the list pick a row and push it above the dialog, + * closing it. */ val openDetail: RouteDestination.Detail? get() = currentStack.filterIsInstance().lastOrNull() diff --git a/app/src/main/kotlin/de/davis/keygo/app/presentation/navigation/AppNavigator.kt b/app/src/main/kotlin/de/davis/keygo/app/presentation/navigation/AppNavigator.kt index a9a4e516e..397076aac 100644 --- a/app/src/main/kotlin/de/davis/keygo/app/presentation/navigation/AppNavigator.kt +++ b/app/src/main/kotlin/de/davis/keygo/app/presentation/navigation/AppNavigator.kt @@ -53,12 +53,8 @@ class AppNavigator(val state: AppNavigationState) { /** * Hides what is showing behind an unlock gate and blocks back until [unlock]. Nothing * underneath is disturbed or torn down: a screen holding a secret clears it by observing the - * session, the way ChangePasswordViewModel does. - * - * A no-op if already gated. This matters because the caller's trigger is collected by a - * `LaunchedEffect` that re-fires on every fresh composition - including one rebuilt by a - * configuration change while the app is still locked - with no memory of having already run. - * The restored stack is what remembers, so a second call pushes nothing. + * session, the way ChangePasswordViewModel does. A no-op if already gated, since the caller's + * `LaunchedEffect` re-fires on every fresh composition. */ fun lock() { if (isGated) return @@ -101,9 +97,8 @@ class AppNavigator(val state: AppNavigationState) { } /** - * Goes back one destination, but never down to nothing, and never while a lock's gate is up. - * The overlay can hold more than one entry while gated (a picker preserved under the - * gate, say), so a plain depth check would let back press pop the gate itself away. + * Goes back one destination, never down to nothing, and never while the gate is up: the + * overlay can be deeper than one entry then, so a depth check alone would pop the gate. */ fun goBack() { if (isGated) return From e0561cddd059a3af9274f39f8b85c6dabc181678 Mon Sep 17 00:00:00 2001 From: Davis Wolfermann Date: Thu, 3 Sep 2026 01:58:50 +0200 Subject: [PATCH 14/35] refactor(app): gate on the session state alone and drop the transition signal --- app/build.gradle.kts | 4 - .../keygo/app/presentation/AppViewModel.kt | 27 +---- .../keygo/app/presentation/MainActivity.kt | 26 ++--- .../presentation/navigation/AppNavigator.kt | 19 ++- .../app/presentation/AppViewModelTest.kt | 109 ------------------ .../navigation/AppNavigatorTest.kt | 19 +++ 6 files changed, 47 insertions(+), 157 deletions(-) delete mode 100644 app/src/test/kotlin/de/davis/keygo/app/presentation/AppViewModelTest.kt diff --git a/app/build.gradle.kts b/app/build.gradle.kts index 6ea894324..c5d49e7fc 100644 --- a/app/build.gradle.kts +++ b/app/build.gradle.kts @@ -161,10 +161,6 @@ dependencies { testImplementation(libs.kotlin.test) testImplementation(libs.robolectric) testImplementation(libs.androidx.junit) - testImplementation(libs.kotlinx.coroutines.test) - testImplementation(testFixtures(projects.core.security)) - testImplementation(testFixtures(projects.core.identity)) - testImplementation(testFixtures(projects.legacyMigration)) androidTestImplementation(libs.androidx.junit) androidTestImplementation(libs.androidx.espresso.core) diff --git a/app/src/main/kotlin/de/davis/keygo/app/presentation/AppViewModel.kt b/app/src/main/kotlin/de/davis/keygo/app/presentation/AppViewModel.kt index fc3722d7e..f7ef1770f 100644 --- a/app/src/main/kotlin/de/davis/keygo/app/presentation/AppViewModel.kt +++ b/app/src/main/kotlin/de/davis/keygo/app/presentation/AppViewModel.kt @@ -6,12 +6,8 @@ import de.davis.keygo.core.identity.domain.repository.AccountRepository import de.davis.keygo.core.security.domain.Session import de.davis.keygo.legacy_migration.domain.usecase.HasMainPasswordUseCase import kotlinx.coroutines.flow.MutableStateFlow -import kotlinx.coroutines.flow.SharingStarted import kotlinx.coroutines.flow.StateFlow import kotlinx.coroutines.flow.asStateFlow -import kotlinx.coroutines.flow.map -import kotlinx.coroutines.flow.scan -import kotlinx.coroutines.flow.stateIn import kotlinx.coroutines.flow.update import kotlinx.coroutines.launch import org.koin.core.annotation.KoinViewModel @@ -27,27 +23,8 @@ internal class AppViewModel( val isReturningUser = _isReturningUser.asStateFlow() /** - * True exactly when a session that was active has just ended - never at first launch, before - * the session has ever been active. A level read of "not active" would also be true before the - * very first login, before [MainActivity.launchRoute]'s onboarding or deep-link auth screen has - * had a chance to show, and would clobber it. [MainActivity] observes this to put the re-auth - * gate up. - */ - val isLocked: StateFlow = session.isActive - .scan(false to false) { (wasActive, _), isActive -> isActive to (wasActive && !isActive) } - .map { it.second } - .stateIn( - scope = viewModelScope, - started = SharingStarted.Eagerly, - initialValue = false, - ) - - /** - * The session's raw current state, for [MainActivity] to self-heal a back stack a - * configuration change or process death restored straight into the app proper with a session - * that never got re-established. [isLocked] alone cannot catch this: it only fires on a - * transition, and a freshly restored [AppViewModel] has no memory of the session ever having - * been active to transition from. + * The session's raw state, which [MainActivity] gates on: no ARK means the user has to + * authenticate again, whether the session just ended or a restored process never had one. */ val isSessionActive: StateFlow = session.isActive diff --git a/app/src/main/kotlin/de/davis/keygo/app/presentation/MainActivity.kt b/app/src/main/kotlin/de/davis/keygo/app/presentation/MainActivity.kt index cdb969dc5..9005b6209 100644 --- a/app/src/main/kotlin/de/davis/keygo/app/presentation/MainActivity.kt +++ b/app/src/main/kotlin/de/davis/keygo/app/presentation/MainActivity.kt @@ -60,7 +60,6 @@ class MainActivity : FragmentActivity() { setContent { // Null until the account has been looked up, which the splash screen waits out. val hasAccess = viewModel.isReturningUser.collectAsState().value ?: return@setContent - val isLocked by viewModel.isLocked.collectAsState() val isSessionActive by viewModel.isSessionActive.collectAsState() KeyGoTheme { @@ -71,7 +70,6 @@ class MainActivity : FragmentActivity() { App( hasAccess = hasAccess, launchRoute = launchRoute(hasAccess), - isLocked = isLocked, isSessionActive = isSessionActive, ) } @@ -94,7 +92,6 @@ private fun Intent.totpImportRedirect(): TotpImportRedirect? { private fun App( hasAccess: Boolean, launchRoute: NavKey, - isLocked: Boolean, isSessionActive: Boolean, ) { val navigationState = rememberAppNavigationState( @@ -104,7 +101,7 @@ private fun App( ) val navigator = remember(navigationState) { AppNavigator(navigationState) } - LockAppWhenSessionEnds(isLocked, isSessionActive, navigator) + LockAppWhenSessionEnds(isSessionActive, navigator) val windowAdaptiveInfo = currentWindowAdaptiveInfoV2() val directive = remember(windowAdaptiveInfo) { @@ -147,20 +144,17 @@ private fun App( } /** - * [isLocked] catches a live session ending. `!isSessionActive && !isOverlaid` catches a back stack - * restored into the app proper with no session, which a freshly restored [AppViewModel] has no - * transition to report; `!isOverlaid` keeps that off onboarding and the first login, which run - * without a session by design. + * No ARK means the user has to authenticate again, so the session's own state is the whole rule - + * a session that just ended and a restored process that never had one are the same thing here. + * [AppNavigator.lock] is what knows when a gate would be wrong, so onboarding and the deep link + * redirect need no special case. Keyed on what the overlay is showing as well, so it re-decides + * when that changes under a session that is still ended. */ @Composable -private fun LockAppWhenSessionEnds( - isLocked: Boolean, - isSessionActive: Boolean, - navigator: AppNavigator, -) { - val isOverlaid = navigator.state.isOverlaid - LaunchedEffect(isLocked, isSessionActive, isOverlaid) { - if (isLocked || (!isSessionActive && !isOverlaid)) navigator.lock() +private fun LockAppWhenSessionEnds(isSessionActive: Boolean, navigator: AppNavigator) { + val topOverlayRoute = navigator.state.overlayStack.lastOrNull() + LaunchedEffect(isSessionActive, topOverlayRoute) { + if (!isSessionActive) navigator.lock() } } diff --git a/app/src/main/kotlin/de/davis/keygo/app/presentation/navigation/AppNavigator.kt b/app/src/main/kotlin/de/davis/keygo/app/presentation/navigation/AppNavigator.kt index 397076aac..76b89e733 100644 --- a/app/src/main/kotlin/de/davis/keygo/app/presentation/navigation/AppNavigator.kt +++ b/app/src/main/kotlin/de/davis/keygo/app/presentation/navigation/AppNavigator.kt @@ -4,6 +4,8 @@ import androidx.navigation3.runtime.NavBackStack import androidx.navigation3.runtime.NavKey import de.davis.keygo.core.presentation.model.RouteDestination import de.davis.keygo.feature.auth.presentation.AuthRoute +import de.davis.keygo.feature.onboarding.presentation.OnboardingRoute +import de.davis.keygo.feature.totp.presentation.TotpImportRedirect /** * Handles navigation events by updating [AppNavigationState]. Everything the UI can do to the back @@ -18,6 +20,17 @@ class AppNavigator(val state: AppNavigationState) { */ private val isGated: Boolean get() = state.overlayStack.lastOrNull() is AuthRoute + /** + * True while the overlay is showing a screen that runs before there is a session, so a locked + * session is what it is there for rather than a reason to gate it. The import flow is not one + * of these: it only ever runs after an unlock, so a session that ends under it does gate it. + */ + private val runsWithoutSession: Boolean + get() = when (state.overlayStack.lastOrNull()) { + is AuthRoute, is OnboardingRoute, is TotpImportRedirect -> true + else -> false + } + fun navigate(route: NavKey) { val isTopLevel = !state.isOverlaid && route in state.backStacks if (isTopLevel) selectTopLevel(route) @@ -53,11 +66,11 @@ class AppNavigator(val state: AppNavigationState) { /** * Hides what is showing behind an unlock gate and blocks back until [unlock]. Nothing * underneath is disturbed or torn down: a screen holding a secret clears it by observing the - * session, the way ChangePasswordViewModel does. A no-op if already gated, since the caller's - * `LaunchedEffect` re-fires on every fresh composition. + * session, the way ChangePasswordViewModel does. A no-op while a screen that runs without a + * session is already up, which covers both a gate already in place and the caller re-firing. */ fun lock() { - if (isGated) return + if (runsWithoutSession) return pushOntoOverlay(AuthRoute()) } diff --git a/app/src/test/kotlin/de/davis/keygo/app/presentation/AppViewModelTest.kt b/app/src/test/kotlin/de/davis/keygo/app/presentation/AppViewModelTest.kt deleted file mode 100644 index dee52c0c0..000000000 --- a/app/src/test/kotlin/de/davis/keygo/app/presentation/AppViewModelTest.kt +++ /dev/null @@ -1,109 +0,0 @@ -package de.davis.keygo.app.presentation - -import de.davis.keygo.core.identity.FakeAccountRepository -import de.davis.keygo.core.security.crypto.FakeSession -import de.davis.keygo.legacy_migration.FakeMainPasswordRepository -import de.davis.keygo.legacy_migration.hasMainPasswordUseCase -import kotlinx.coroutines.Dispatchers -import kotlinx.coroutines.ExperimentalCoroutinesApi -import kotlinx.coroutines.flow.launchIn -import kotlinx.coroutines.test.StandardTestDispatcher -import kotlinx.coroutines.test.TestScope -import kotlinx.coroutines.test.advanceUntilIdle -import kotlinx.coroutines.test.resetMain -import kotlinx.coroutines.test.runTest -import kotlinx.coroutines.test.setMain -import kotlin.test.AfterTest -import kotlin.test.BeforeTest -import kotlin.test.Test -import kotlin.test.assertFalse -import kotlin.test.assertTrue - -@OptIn(ExperimentalCoroutinesApi::class) -class AppViewModelTest { - - private val dispatcher = StandardTestDispatcher() - - @BeforeTest - fun setUp() = Dispatchers.setMain(dispatcher) - - @AfterTest - fun tearDown() = Dispatchers.resetMain() - - private fun TestScope.viewModel(session: FakeSession): AppViewModel = AppViewModel( - accountRepository = FakeAccountRepository(), - hasV1Password = hasMainPasswordUseCase(FakeMainPasswordRepository()), - session = session, - ).also { it.isLocked.launchIn(backgroundScope) } - - @Test - fun `isLocked stays false through a cold start that never logs in`() = runTest(dispatcher) { - val session = FakeSession() - val vm = viewModel(session) - advanceUntilIdle() - - assertFalse(vm.isLocked.value) - } - - @Test - fun `isLocked turns true only after an active session ends`() = runTest(dispatcher) { - val session = FakeSession() - val vm = viewModel(session) - // isLocked's collector must be subscribed (and so already see isActive = false) before - // the first mutation - StandardTestDispatcher defers launchIn's collection until this - // point, so starting the session any earlier would be missed rather than seen as a - // false -> true transition. - advanceUntilIdle() - - session.startSession(ByteArray(32)) - advanceUntilIdle() - assertFalse(vm.isLocked.value) - - session.endSession() - advanceUntilIdle() - assertTrue(vm.isLocked.value) - } - - @Test - fun `isLocked returns to false once the session is reinitialized`() = runTest(dispatcher) { - val session = FakeSession() - val vm = viewModel(session) - advanceUntilIdle() - - session.startSession(ByteArray(32)) - advanceUntilIdle() - session.endSession() - advanceUntilIdle() - assertTrue(vm.isLocked.value) - - session.startSession(ByteArray(32)) - advanceUntilIdle() - - assertFalse(vm.isLocked.value) - } - - @Test - fun `isLocked keeps tracking transitions with no collector ever attached`() = - runTest(dispatcher) { - val session = FakeSession() - // Deliberately not using the shared viewModel() helper here - it holds a permanent - // collector via launchIn(backgroundScope), which would pass under either SharingStarted - // strategy and wouldn't actually distinguish Eagerly from the WhileSubscribed(5_000) - // this replaced. Eagerly's whole point is that isLocked keeps tracking transitions even - // with zero collectors ever subscribed; WhileSubscribed would never even start - // collecting. - val vm = AppViewModel( - accountRepository = FakeAccountRepository(), - hasV1Password = hasMainPasswordUseCase(FakeMainPasswordRepository()), - session = session, - ) - advanceUntilIdle() - - session.startSession(ByteArray(32)) - advanceUntilIdle() - session.endSession() - advanceUntilIdle() - - assertTrue(vm.isLocked.value) - } -} diff --git a/app/src/test/kotlin/de/davis/keygo/app/presentation/navigation/AppNavigatorTest.kt b/app/src/test/kotlin/de/davis/keygo/app/presentation/navigation/AppNavigatorTest.kt index 64d9b4793..110f8ac9f 100644 --- a/app/src/test/kotlin/de/davis/keygo/app/presentation/navigation/AppNavigatorTest.kt +++ b/app/src/test/kotlin/de/davis/keygo/app/presentation/navigation/AppNavigatorTest.kt @@ -11,6 +11,7 @@ import de.davis.keygo.feature.onboarding.presentation.OnboardingRoute import de.davis.keygo.feature.settings.presentation.ChangePasswordRoute import de.davis.keygo.feature.settings.presentation.SettingsRoute import de.davis.keygo.feature.totp.presentation.SelectItemForTotpRoute +import de.davis.keygo.feature.totp.presentation.TotpImportRedirect import kotlin.test.Test import kotlin.test.assertEquals import kotlin.test.assertFalse @@ -333,6 +334,24 @@ class AppNavigatorTest { assertEquals(listOf(AuthRoute()), navigator.shown) } + @Test + fun `locking is refused while first run is showing`() { + val navigator = navigator(launchRoute = OnboardingRoute()) + + navigator.lock() + + assertEquals(listOf(OnboardingRoute()), navigator.shown) + } + + @Test + fun `locking is refused while the deep link redirect is showing`() { + val navigator = navigator(launchRoute = TotpImportRedirect(DEEP_LINK_URI)) + + navigator.lock() + + assertEquals(listOf(TotpImportRedirect(DEEP_LINK_URI)), navigator.shown) + } + @Test fun `unlocking reveals the active tab exactly as it was`() { val navigator = unlocked() From 7988ed298f6d217321930ed5d5cd932977124e67 Mon Sep 17 00:00:00 2001 From: Davis Wolfermann Date: Thu, 3 Sep 2026 12:50:50 +0200 Subject: [PATCH 15/35] refactor(app): implement gate checks to prevent navigation while locked --- .../presentation/navigation/AppNavigator.kt | 23 ++++- .../navigation/AppNavigatorTest.kt | 85 +++++++++++++++++-- .../core/security/data/SessionLockObserver.kt | 6 -- .../changepassword/ChangePasswordViewModel.kt | 28 ++---- .../ChangePasswordViewModelTest.kt | 23 +++++ 5 files changed, 133 insertions(+), 32 deletions(-) diff --git a/app/src/main/kotlin/de/davis/keygo/app/presentation/navigation/AppNavigator.kt b/app/src/main/kotlin/de/davis/keygo/app/presentation/navigation/AppNavigator.kt index 76b89e733..905c99c34 100644 --- a/app/src/main/kotlin/de/davis/keygo/app/presentation/navigation/AppNavigator.kt +++ b/app/src/main/kotlin/de/davis/keygo/app/presentation/navigation/AppNavigator.kt @@ -10,6 +10,11 @@ import de.davis.keygo.feature.totp.presentation.TotpImportRedirect /** * Handles navigation events by updating [AppNavigationState]. Everything the UI can do to the back * stacks goes through here, so the rules for what replaces what live in one place. + * + * Every entry point the UI can reach refuses to run while the gate is up. The chrome does not + * vanish the instant [lock] fires, it animates out, so the navigation bar and the create button + * stay composed and clickable for a moment behind the gate. Without the guard a tap in that window + * would push its destination onto the overlay, above the gate, and show it unauthenticated. */ class AppNavigator(val state: AppNavigationState) { @@ -32,6 +37,7 @@ class AppNavigator(val state: AppNavigationState) { } fun navigate(route: NavKey) { + if (isGated) return val isTopLevel = !state.isOverlaid && route in state.backStacks if (isTopLevel) selectTopLevel(route) else state.currentStack.add(route) @@ -53,8 +59,13 @@ class AppNavigator(val state: AppNavigationState) { state.overlayStack.add(route) } - /** Clears the overlay and hands the window to the app proper. */ + /** + * Clears the overlay and hands the window to the app proper. Refused while the gate is up: + * this is the one path that would drop a gate without anything having authenticated, and the + * screens that call it sit under the gate rather than over it. + */ fun clearOverlay() { + if (isGated) return state.overlayStack.clear() } @@ -85,12 +96,14 @@ class AppNavigator(val state: AppNavigationState) { * from a detail always lands on the list. */ fun showDetail(detail: RouteDestination.Detail) { + if (isGated) return closeDetail() state.currentStack.add(detail) } /** Opens [detail] on top of the detail already showing, so back returns to it. */ fun openOnTopOfDetail(detail: RouteDestination.Detail) { + if (isGated) return state.currentStack.add(detail) } @@ -103,9 +116,15 @@ class AppNavigator(val state: AppNavigationState) { /** * Drops a detail the list picked on the user's behalf. A form is left alone: it may hold typing * that is not saved yet. + * + * Reaches past the overlay to the tab that owns the detail, rather than going through + * [AppNavigationState.currentStack]. The window can narrow while the gate is up - rotating at + * the lock screen is an ordinary thing to do - and the overlay's top entry is never a detail, + * so this would find nothing to drop and the tab would keep a selection the user never made, + * waiting full screen behind the unlock. */ fun dropAutoSelectedDetail() { - val stack = state.currentStack + val stack = state.backStacks.getValue(state.topLevelRoute) if (stack.lastOrNull() is RouteDestination.ViewItem) stack.removeLastOrNull() } diff --git a/app/src/test/kotlin/de/davis/keygo/app/presentation/navigation/AppNavigatorTest.kt b/app/src/test/kotlin/de/davis/keygo/app/presentation/navigation/AppNavigatorTest.kt index 110f8ac9f..e38e2eb3d 100644 --- a/app/src/test/kotlin/de/davis/keygo/app/presentation/navigation/AppNavigatorTest.kt +++ b/app/src/test/kotlin/de/davis/keygo/app/presentation/navigation/AppNavigatorTest.kt @@ -36,10 +36,11 @@ class AppNavigatorTest { @Test fun `the overlay owns the window until it is cleared`() { - val navigator = navigator() + // Cleared with a first run overlay: a gate refuses clearOverlay and is popped by unlock. + val navigator = navigator(launchRoute = OnboardingRoute()) assertTrue(navigator.state.isOverlaid) - assertEquals(listOf(AuthRoute()), navigator.shown) + assertEquals(listOf(OnboardingRoute()), navigator.shown) navigator.clearOverlay() @@ -99,12 +100,15 @@ class AppNavigatorTest { @Test fun `a top level route is not switched to while the overlay owns the window`() { - val navigator = navigator() + val navigator = navigator(launchRoute = TotpImportRedirect(DEEP_LINK_URI)) navigator.navigate(SettingsRoute) assertTrue(navigator.state.isOverlaid) - assertEquals(listOf(AuthRoute(), SettingsRoute), navigator.shown) + assertEquals( + listOf(TotpImportRedirect(DEEP_LINK_URI), SettingsRoute), + navigator.shown, + ) } // ---- top level routes ---- @@ -390,6 +394,77 @@ class AppNavigatorTest { ) } + @Test + fun `the chrome still standing behind the gate cannot navigate anywhere`() { + // The navigation bar animates out rather than disappearing, so it stays clickable for a + // moment after the gate goes up. A tap landing then must not push over the gate. + val navigator = unlocked() + navigator.lock() + + navigator.navigate(SettingsRoute) + navigator.navigate(RouteDestination.Home) + + assertEquals(listOf(AuthRoute()), navigator.shown) + assertEquals(RouteDestination.Home, navigator.state.topLevelRoute) + } + + @Test + fun `the create button still standing behind the gate cannot open a detail`() { + val navigator = unlocked() + navigator.lock() + + navigator.showDetail(RouteDestination.CreateItem(VaultItemType.Login)) + navigator.openOnTopOfDetail(RouteDestination.ViewItem(newItemId())) + + assertEquals(listOf(AuthRoute()), navigator.shown) + } + + @Test + fun `a narrowing window drops the tab's auto-selected detail even behind the gate`() { + // Rotating at the lock screen is ordinary now that the gate shows on every resume. The + // overlay's top is the gate, never a detail, so reading it would drop nothing and leave the + // tab holding a selection the user never made. + val navigator = unlocked() + navigator.showDetail(RouteDestination.ViewItem(newItemId())) + navigator.lock() + + navigator.dropAutoSelectedDetail() + + assertEquals( + listOf(RouteDestination.Home), + navigator.state.backStacks.getValue(RouteDestination.Home).toList(), + ) + assertEquals(listOf(AuthRoute()), navigator.shown) + } + + @Test + fun `a form is still left alone when the window narrows behind the gate`() { + val navigator = unlocked() + navigator.showDetail(RouteDestination.CreateItem(VaultItemType.Login)) + navigator.lock() + + navigator.dropAutoSelectedDetail() + + assertEquals( + listOf(RouteDestination.Home, RouteDestination.CreateItem(VaultItemType.Login)), + navigator.state.backStacks.getValue(RouteDestination.Home).toList(), + ) + } + + @Test + fun `the gate is not cleared away by a flow finishing underneath it`() { + val navigator = navigator() + navigator.replaceOverlay(SelectItemForTotpRoute(DEEP_LINK_URI)) + navigator.lock() + + navigator.clearOverlay() + + assertEquals( + listOf(SelectItemForTotpRoute(DEEP_LINK_URI), AuthRoute()), + navigator.shown, + ) + } + @Test fun `back works normally again once unlocked`() { val navigator = unlocked() @@ -491,7 +566,7 @@ class AppNavigatorTest { assertEquals(listOf(SelectItemForTotpRoute(DEEP_LINK_URI)), navigator.shown) } - private fun unlocked(): AppNavigator = navigator().apply { clearOverlay() } + private fun unlocked(): AppNavigator = navigator().apply { unlock() } private companion object { val TOP_LEVEL_ROUTES: Set = linkedSetOf( diff --git a/core/security/src/main/kotlin/de/davis/keygo/core/security/data/SessionLockObserver.kt b/core/security/src/main/kotlin/de/davis/keygo/core/security/data/SessionLockObserver.kt index 53a1292f8..eeb94b625 100644 --- a/core/security/src/main/kotlin/de/davis/keygo/core/security/data/SessionLockObserver.kt +++ b/core/security/src/main/kotlin/de/davis/keygo/core/security/data/SessionLockObserver.kt @@ -6,12 +6,6 @@ import androidx.lifecycle.ProcessLifecycleOwner import de.davis.keygo.core.security.domain.Session import org.koin.core.annotation.Single -/** - * Ends the session the instant the app leaves the foreground, so returning to it requires - * re-authentication rather than staying unlocked indefinitely. Registered once per process, the - * same pattern [de.davis.keygo.feature.backup.domain.BackupEscrowReconciler] already uses for its - * own process-start hook. - */ @Single(createdAtStart = true) internal class SessionLockObserver( private val session: Session, diff --git a/feature/settings/src/main/kotlin/de/davis/keygo/feature/settings/presentation/changepassword/ChangePasswordViewModel.kt b/feature/settings/src/main/kotlin/de/davis/keygo/feature/settings/presentation/changepassword/ChangePasswordViewModel.kt index db117c310..c5ccd0fb9 100644 --- a/feature/settings/src/main/kotlin/de/davis/keygo/feature/settings/presentation/changepassword/ChangePasswordViewModel.kt +++ b/feature/settings/src/main/kotlin/de/davis/keygo/feature/settings/presentation/changepassword/ChangePasswordViewModel.kt @@ -1,7 +1,6 @@ package de.davis.keygo.feature.settings.presentation.changepassword import androidx.compose.foundation.ExperimentalFoundationApi -import androidx.compose.foundation.text.input.TextFieldState import androidx.compose.foundation.text.input.delete import androidx.compose.runtime.snapshotFlow import androidx.lifecycle.ViewModel @@ -25,7 +24,6 @@ import kotlinx.coroutines.FlowPreview import kotlinx.coroutines.channels.Channel import kotlinx.coroutines.flow.MutableStateFlow import kotlinx.coroutines.flow.SharingStarted -import kotlinx.coroutines.flow.collect import kotlinx.coroutines.flow.combine import kotlinx.coroutines.flow.debounce import kotlinx.coroutines.flow.distinctUntilChanged @@ -206,23 +204,6 @@ internal class ChangePasswordViewModel( } } - /** - * The current password is the RootKek derivation input - the one secret this codebase never - * retains anywhere, including across a lock. Everything else about the screen is left alone, - * the same as any other screen the app leaves in place while locked. - * - * Mutates the existing [TextFieldState] instances rather than replacing them: [passwordStrength] - * tracks a snapshot-state read on whichever instance `_state.value.newPassword` pointed to the - * last time it ran, and a fresh replacement instance's changes would go unobserved - the old, - * abandoned instance is simply never mutated, so nothing ever re-triggers the flow, and - * [ChangePasswordState.passwordScore] would freeze at whatever it was the moment before the - * clear for the rest of the ViewModel's life. - * - * `undoState.clearHistory()` matters for the same reason: `edit {}` records the pre-clear text - * into the field's own undo stack, and this screen is deliberately kept alive (not torn down) - * while the app is locked, so without clearing it a re-authenticated user could Ctrl+Z the - * "cleared" password straight back. - */ @OptIn(ExperimentalFoundationApi::class) private fun clearSensitiveFields() { _state.value.currentPassword.edit { delete(0, length) } @@ -231,5 +212,14 @@ internal class ChangePasswordViewModel( _state.value.newPassword.undoState.clearHistory() _state.value.confirmPassword.edit { delete(0, length) } _state.value.confirmPassword.undoState.clearHistory() + + _state.update { + it.copy( + currentPasswordError = null, + newPasswordError = null, + confirmPasswordError = null, + showReauthDialog = false, + ) + } } } diff --git a/feature/settings/src/test/kotlin/de/davis/keygo/feature/settings/presentation/changepassword/ChangePasswordViewModelTest.kt b/feature/settings/src/test/kotlin/de/davis/keygo/feature/settings/presentation/changepassword/ChangePasswordViewModelTest.kt index 0fce4a6b7..67eda9830 100644 --- a/feature/settings/src/test/kotlin/de/davis/keygo/feature/settings/presentation/changepassword/ChangePasswordViewModelTest.kt +++ b/feature/settings/src/test/kotlin/de/davis/keygo/feature/settings/presentation/changepassword/ChangePasswordViewModelTest.kt @@ -34,6 +34,7 @@ import kotlin.test.BeforeTest import kotlin.test.Test import kotlin.test.assertEquals import kotlin.test.assertFalse +import kotlin.test.assertNull @OptIn(ExperimentalCoroutinesApi::class) class ChangePasswordViewModelTest { @@ -340,6 +341,28 @@ class ChangePasswordViewModelTest { assertFalse(vm.state.value.confirmPassword.undoState.canUndo) } + @Test + fun `the session ending puts the rest of the screen back to rest too`() = runTest(dispatcher) { + // The errors and the dialog all describe input the clear just removed. Left standing, the + // user comes back from the unlock to a re-auth dialog over three emptied fields, or to + // "this field is empty" on a form they did fill in. + val vm = viewModel() + vm.onBiometricResult(Result.Failure(BiometricAuthError.Declined)) + vm.submitWithPassword() + advanceUntilIdle() + + assertEquals(true, vm.state.value.showReauthDialog) + assertEquals(UiFieldError.Empty, vm.state.value.newPasswordError) + + session.endSession() + advanceUntilIdle() + + assertEquals(false, vm.state.value.showReauthDialog) + assertNull(vm.state.value.newPasswordError) + assertNull(vm.state.value.currentPasswordError) + assertNull(vm.state.value.confirmPasswordError) + } + @Test fun `ordinary use does not clear the fields while the session stays active`() = runTest(dispatcher) { val vm = viewModel() From e852cd54e450b56273434e4f8dec69a16ccd2738 Mon Sep 17 00:00:00 2001 From: Davis Wolfermann Date: Thu, 3 Sep 2026 23:36:11 +0200 Subject: [PATCH 16/35] refactor(app): replace direct ark access with withArkOr for session handling --- .../BiometricEnrollmentAdapterImpl.kt | 9 +- .../domain/usecase/CreateAccessUseCaseTest.kt | 3 +- .../usecase/UnlockWithPasswordUseCaseTest.kt | 2 +- .../keygo/core/security/data/SessionImpl.kt | 20 ++- .../crypto/CryptographicScopeProviderImpl.kt | 19 +-- .../keygo/core/security/domain/ArkHolder.kt | 58 +++++++++ .../keygo/core/security/domain/Session.kt | 23 ++-- .../crypto/CryptographicScopeImplTest.kt | 5 +- .../security/data/SessionArkLifetimeTest.kt | 117 ++++++++++++++++++ .../core/security/data/SessionImplTest.kt | 30 +++-- .../keygo/core/security/crypto/FakeSession.kt | 29 +++-- .../feature/backup/data/BackupSession.kt | 4 +- .../backup/domain/BackupArkUnlocker.kt | 10 +- .../usecase/FinishExportWizardUseCase.kt | 9 +- .../domain/usecase/ImportBackupUseCase.kt | 46 ++++--- .../backup/domain/BackupArkUnlockerTest.kt | 10 +- .../domain/usecase/ExportBackupUseCaseTest.kt | 7 +- .../usecase/FinishExportWizardUseCaseTest.kt | 4 +- .../domain/usecase/ImportBackupUseCaseTest.kt | 2 +- .../domain/usecase/CreateVaultUseCase.kt | 9 +- .../usecase/MoveItemsToVaultUseCaseTest.kt | 2 +- 21 files changed, 313 insertions(+), 105 deletions(-) create mode 100644 core/security/src/main/kotlin/de/davis/keygo/core/security/domain/ArkHolder.kt create mode 100644 core/security/src/test/kotlin/de/davis/keygo/core/security/data/SessionArkLifetimeTest.kt diff --git a/core/identity/src/main/kotlin/de/davis/keygo/core/identity/presentation/BiometricEnrollmentAdapterImpl.kt b/core/identity/src/main/kotlin/de/davis/keygo/core/identity/presentation/BiometricEnrollmentAdapterImpl.kt index 70543cf06..54b3e447d 100644 --- a/core/identity/src/main/kotlin/de/davis/keygo/core/identity/presentation/BiometricEnrollmentAdapterImpl.kt +++ b/core/identity/src/main/kotlin/de/davis/keygo/core/identity/presentation/BiometricEnrollmentAdapterImpl.kt @@ -8,6 +8,7 @@ import de.davis.keygo.core.security.domain.Session import de.davis.keygo.core.security.domain.model.BiometricPolicy import de.davis.keygo.core.security.domain.model.CryptographicMode import de.davis.keygo.core.security.domain.model.KeyId +import de.davis.keygo.core.security.domain.withArkOr import de.davis.keygo.core.security.presentation.BiometricCryptoController import de.davis.keygo.core.util.Result import de.davis.keygo.core.util.asResult @@ -32,11 +33,9 @@ internal class BiometricEnrollmentAdapterImpl( val cipher = requestCipher(KeyId.BiometricVaultKek, CryptographicMode.Wrap, policy) .bind { BiometricEnrollmentError.BiometricFailed(it) } - - val ark = session.ark.asResult(BiometricEnrollmentError.NoActiveSession).bind() - - val wrapped = wrapArk(ark, cipher) - .asResult(BiometricEnrollmentError.WrappingFailed).bind() + val wrapped = session.withArkOr(BiometricEnrollmentError.NoActiveSession) { ark -> + wrapArk(ark, cipher).asResult(BiometricEnrollmentError.WrappingFailed) + }.bind() accountRepository.set(account.copy(biometricWrappedArk = wrapped)).bind { BiometricEnrollmentError.PersistenceFailed diff --git a/core/identity/src/test/kotlin/de/davis/keygo/core/identity/domain/usecase/CreateAccessUseCaseTest.kt b/core/identity/src/test/kotlin/de/davis/keygo/core/identity/domain/usecase/CreateAccessUseCaseTest.kt index 1added90e..307307244 100644 --- a/core/identity/src/test/kotlin/de/davis/keygo/core/identity/domain/usecase/CreateAccessUseCaseTest.kt +++ b/core/identity/src/test/kotlin/de/davis/keygo/core/identity/domain/usecase/CreateAccessUseCaseTest.kt @@ -15,6 +15,7 @@ import kotlinx.coroutines.test.runTest import javax.crypto.Cipher import javax.crypto.KeyGenerator import kotlin.test.Test +import kotlin.test.assertContentEquals import kotlin.test.assertEquals import kotlin.test.assertNotNull import kotlin.test.assertTrue @@ -90,7 +91,7 @@ class CreateAccessUseCaseTest { assertTrue(result.isSuccess()) assertTrue(session.startSessionCalled) - assertEquals(accountManager.createAccount.account.ark, session.ark) + assertContentEquals(accountManager.createAccount.account.ark, session.currentArk) } @Test diff --git a/core/identity/src/test/kotlin/de/davis/keygo/core/identity/domain/usecase/UnlockWithPasswordUseCaseTest.kt b/core/identity/src/test/kotlin/de/davis/keygo/core/identity/domain/usecase/UnlockWithPasswordUseCaseTest.kt index b4be406da..0d1c5c644 100644 --- a/core/identity/src/test/kotlin/de/davis/keygo/core/identity/domain/usecase/UnlockWithPasswordUseCaseTest.kt +++ b/core/identity/src/test/kotlin/de/davis/keygo/core/identity/domain/usecase/UnlockWithPasswordUseCaseTest.kt @@ -91,6 +91,6 @@ class UnlockWithPasswordUseCaseTest { assertTrue(result.isSuccess()) assertTrue(session.startSessionCalled) - assertContentEquals(ark, session.ark) + assertContentEquals(ark, session.currentArk) } } diff --git a/core/security/src/main/kotlin/de/davis/keygo/core/security/data/SessionImpl.kt b/core/security/src/main/kotlin/de/davis/keygo/core/security/data/SessionImpl.kt index 2c4169f1b..9c91ec390 100644 --- a/core/security/src/main/kotlin/de/davis/keygo/core/security/data/SessionImpl.kt +++ b/core/security/src/main/kotlin/de/davis/keygo/core/security/data/SessionImpl.kt @@ -1,36 +1,30 @@ package de.davis.keygo.core.security.data +import de.davis.keygo.core.security.domain.ArkHolder import de.davis.keygo.core.security.domain.Session import kotlinx.coroutines.flow.MutableStateFlow import kotlinx.coroutines.flow.StateFlow import kotlinx.coroutines.flow.asStateFlow import org.koin.core.annotation.Single -import javax.security.auth.DestroyFailedException @Single internal class SessionImpl : Session { - private var _ark: ByteArray? = null + private val holder = ArkHolder() private val _isActive = MutableStateFlow(false) - override val ark: ByteArray? - get() = _ark - override val isActive: StateFlow = _isActive.asStateFlow() + override suspend fun withArk(block: suspend (ByteArray) -> R): R? = holder.withArk(block) + override fun startSession(ark: ByteArray) { - endSession() - _ark = ark + holder.set(ark) _isActive.value = true } + /** [isActive] goes false at once even with a block in flight: the gate never waits on it. */ override fun endSession() { - try { - _ark?.fill(0) - } catch (_: DestroyFailedException) { - // Not all SecretKey implementations support destroy - } - _ark = null + holder.clear() _isActive.value = false } } diff --git a/core/security/src/main/kotlin/de/davis/keygo/core/security/data/crypto/CryptographicScopeProviderImpl.kt b/core/security/src/main/kotlin/de/davis/keygo/core/security/data/crypto/CryptographicScopeProviderImpl.kt index 5ef7b20f3..318f688f0 100644 --- a/core/security/src/main/kotlin/de/davis/keygo/core/security/data/crypto/CryptographicScopeProviderImpl.kt +++ b/core/security/src/main/kotlin/de/davis/keygo/core/security/data/crypto/CryptographicScopeProviderImpl.kt @@ -9,8 +9,9 @@ import de.davis.keygo.core.security.domain.crypto.CryptographicScopeProvider import de.davis.keygo.core.security.domain.crypto.model.WrappedItemKeyInformation import de.davis.keygo.core.security.domain.crypto.model.WrappedVaultKeyInformation import de.davis.keygo.core.security.domain.model.CryptoScopeError +import de.davis.keygo.core.security.domain.withArkOr import de.davis.keygo.core.util.Result -import de.davis.keygo.core.util.asResult +import de.davis.keygo.core.util.mapFailure import de.davis.keygo.core.util.mapSuccess import de.davis.keygo.core.util.resultBinding import de.davis.keygo.rust.item.ItemManager @@ -111,14 +112,14 @@ internal class CryptographicScopeProviderImpl( ).mapSuccess { it.toKeyInformation() }.bind(CryptoScopeError::KeyWrapError) } - private fun unwrapVaultKeyWithResult(info: WrappedVaultKeyInformation) = resultBinding { - val ark = session.ark.asResult(CryptoScopeError.NoActiveSession).bind() - keyWrapper.unwrapVaultKeyWithResult( - ark = ark, - wrapped = info.wrappedVaultKey.toWrappedKeyBlob(), - vaultId = info.vaultId, - ).bind(CryptoScopeError::KeyWrapError) - } + private suspend fun unwrapVaultKeyWithResult(info: WrappedVaultKeyInformation) = + session.withArkOr(CryptoScopeError.NoActiveSession) { ark -> + keyWrapper.unwrapVaultKeyWithResult( + ark = ark, + wrapped = info.wrappedVaultKey.toWrappedKeyBlob(), + vaultId = info.vaultId, + ).mapFailure(CryptoScopeError::KeyWrapError) + } } private fun KeyInformation.toWrappedKeyBlob() = WrappedKeyBlob( diff --git a/core/security/src/main/kotlin/de/davis/keygo/core/security/domain/ArkHolder.kt b/core/security/src/main/kotlin/de/davis/keygo/core/security/domain/ArkHolder.kt new file mode 100644 index 000000000..50fb022f0 --- /dev/null +++ b/core/security/src/main/kotlin/de/davis/keygo/core/security/domain/ArkHolder.kt @@ -0,0 +1,58 @@ +package de.davis.keygo.core.security.domain + +/** + * Holds a session's ARK and owns its wipe. A [withArk] block keeps intact bytes for its whole + * duration: a session ending underneath defers the wipe to the last block out. Shared by the real + * session and its fake so the rule cannot drift between them. + */ +class ArkHolder { + + /** Never held across [withArk]'s block - a monitor cannot span a suspension point. */ + private val lock = Any() + + private var ark: ByteArray? = null + + /** How many [withArk] blocks are running. A wipe waits for this to reach zero. */ + private var readers = 0 + + /** ARKs dropped while a reader held them. Several pile up across repeated end and start. */ + private val awaitingWipe = mutableListOf() + + /** Runs [block] with the held ARK, or returns `null` without running it when there is none. */ + suspend fun withArk(block: suspend (ByteArray) -> R): R? { + val live = synchronized(lock) { + val current = ark ?: return null + readers++ + current + } + + try { + return block(live) + } finally { + synchronized(lock) { + readers-- + if (readers == 0) wipePending() + } + } + } + + /** Takes [ark] as the held one, dropping whatever it replaces. */ + fun set(ark: ByteArray) = replace(ark) + + /** Puts the ARK out of reach at once, wiping it unless a [withArk] block still holds it. */ + fun clear() = replace(null) + + private fun replace(next: ByteArray?) { + synchronized(lock) { + ark?.let { awaitingWipe += it } + ark = next + if (readers == 0) wipePending() + } + } + + /** Callers hold [lock]. */ + private fun wipePending() { + awaitingWipe.forEach { it.fill(0) } + awaitingWipe.clear() + } +} diff --git a/core/security/src/main/kotlin/de/davis/keygo/core/security/domain/Session.kt b/core/security/src/main/kotlin/de/davis/keygo/core/security/domain/Session.kt index e6f5e148d..3c1e6b51b 100644 --- a/core/security/src/main/kotlin/de/davis/keygo/core/security/domain/Session.kt +++ b/core/security/src/main/kotlin/de/davis/keygo/core/security/domain/Session.kt @@ -1,20 +1,29 @@ package de.davis.keygo.core.security.domain +import de.davis.keygo.core.util.Result import kotlinx.coroutines.flow.StateFlow interface Session { + /** Observable lock state, for callers that have to react to a session ending rather than read it. */ + val isActive: StateFlow + /** - * The ARK of the live session, or `null` when there is no active session. + * Runs [block] with the live ARK, or returns `null` without running it when locked. Null is the + * ordinary locked branch every caller handles. * - * Nullable on purpose: a locked session is an ordinary branch every caller has to handle, not an - * exceptional one. Reading it is the liveness check, so there is no separate guard to forget. + * The ARK is wiped in place when a session ends, so the array is only valid inside [block] - + * copy what has to outlive it. The bytes stay intact for the whole of [block] however long it + * suspends, even if the session ends underneath. */ - val ark: ByteArray? - - /** Observable lock state, for callers that have to react to a session ending rather than read it. */ - val isActive: StateFlow + suspend fun withArk(block: suspend (ByteArray) -> R): R? fun startSession(ark: ByteArray) fun endSession() } + +/** [Session.withArk] for callers in [Result]: a locked session becomes [locked], not a null. */ +suspend fun Session.withArkOr( + locked: E, + block: suspend (ByteArray) -> Result, +): Result = withArk(block) ?: Result.Failure(locked) diff --git a/core/security/src/test/kotlin/de/davis/keygo/core/security/crypto/CryptographicScopeImplTest.kt b/core/security/src/test/kotlin/de/davis/keygo/core/security/crypto/CryptographicScopeImplTest.kt index f6884dcde..a83da6d26 100644 --- a/core/security/src/test/kotlin/de/davis/keygo/core/security/crypto/CryptographicScopeImplTest.kt +++ b/core/security/src/test/kotlin/de/davis/keygo/core/security/crypto/CryptographicScopeImplTest.kt @@ -3,7 +3,6 @@ package de.davis.keygo.core.security.crypto import de.davis.keygo.core.item.FakeItemRepository import de.davis.keygo.core.item.domain.model.KeyInformation import de.davis.keygo.core.security.data.crypto.CryptographicScopeProviderImpl -import de.davis.keygo.core.security.domain.Session import de.davis.keygo.core.security.domain.crypto.model.CryptographicData import de.davis.keygo.core.security.domain.crypto.model.WrappedItemKeyInformation import de.davis.keygo.core.security.domain.crypto.model.WrappedVaultKeyInformation @@ -29,7 +28,7 @@ class CryptographicScopeImplTest { private val random = Random(42) - private val session: Session = FakeSession(startOnConstruct = true) + private val session = FakeSession(startOnConstruct = true) private val itemRepository = FakeItemRepository() private val itemManager = FakeItemManager() private val keyWrapper = FakeKeyWrapper() @@ -43,7 +42,7 @@ class CryptographicScopeImplTest { vaultId: UUID = UUID.randomUUID(), ): WrappedVaultKeyInformation { val blob = keyWrapper.wrapVaultKey( - ark = assertNotNull(session.ark), + ark = assertNotNull(session.currentArk), vaultKey = ByteArray(32) { random.nextBytes(1)[0] }, vaultId = vaultId, ) diff --git a/core/security/src/test/kotlin/de/davis/keygo/core/security/data/SessionArkLifetimeTest.kt b/core/security/src/test/kotlin/de/davis/keygo/core/security/data/SessionArkLifetimeTest.kt new file mode 100644 index 000000000..5a550cf4f --- /dev/null +++ b/core/security/src/test/kotlin/de/davis/keygo/core/security/data/SessionArkLifetimeTest.kt @@ -0,0 +1,117 @@ +package de.davis.keygo.core.security.data + +import kotlinx.coroutines.CompletableDeferred +import kotlinx.coroutines.ExperimentalCoroutinesApi +import kotlinx.coroutines.Job +import kotlinx.coroutines.launch +import kotlinx.coroutines.test.TestScope +import kotlinx.coroutines.test.advanceUntilIdle +import kotlinx.coroutines.test.runTest +import kotlin.test.Test +import kotlin.test.assertContentEquals +import kotlin.test.assertEquals +import kotlin.test.assertTrue + +@OptIn(ExperimentalCoroutinesApi::class) +class SessionArkLifetimeTest { + + private val session = SessionImpl() + + private fun generateArk(): ByteArray = ByteArray(32) { (it + 1).toByte() } + + private class HeldArk( + val ark: ByteArray, + private val job: Job, + private val resume: CompletableDeferred, + ) { + + suspend fun finish() { + resume.complete(Unit) + job.join() + } + } + + private fun TestScope.holdArk(): HeldArk { + val resume = CompletableDeferred() + val handedOut = CompletableDeferred() + + val job = launch { + session.withArk { ark -> + handedOut.complete(ark) + resume.await() + } + } + advanceUntilIdle() + + return HeldArk(handedOut.getCompleted(), job, resume) + } + + @Test + fun `a session ending does not zero an ark a suspended block still holds`() = runTest { + // The defect this file exists for: CreateVaultUseCase and FinishExportWizardUseCase read + // the ark, suspend, then use it. Zeroing under them persists a key wrapped with zeros. + val expected = generateArk() + session.startSession(expected.copyOf()) + + val held = holdArk() + session.endSession() + + assertContentEquals(expected, held.ark, "wiped while the block was still holding it") + held.finish() + } + + @Test + fun `the ark is zeroed once the last in-flight block finishes`() = runTest { + // Deferring the wipe must not cancel it: the ark still has to leave memory. + session.startSession(generateArk()) + + val held = holdArk() + session.endSession() + held.finish() + + assertTrue(held.ark.all { it == 0.toByte() }, "never wiped after the block finished") + } + + @Test + fun `the session reports itself ended at once even with a block in flight`() = runTest { + // The UI gate keys on isActive, so it must not wait for crypto to drain. + session.startSession(generateArk()) + + val held = holdArk() + session.endSession() + + assertEquals(false, session.isActive.value) + held.finish() + } + + @Test + fun `an ark left over from a replaced session is still zeroed`() = runTest { + // The old ark must not be forgotten in favour of the new one. + val first = generateArk() + session.startSession(first) + + val held = holdArk() + session.endSession() + session.startSession(generateArk()) + session.endSession() + held.finish() + + assertTrue(first.all { it == 0.toByte() }, "the replaced ark was never wiped") + } + + @Test + fun `a block that throws still releases the ark for wiping`() = runTest { + session.startSession(generateArk()) + val handedOut = CompletableDeferred() + + runCatching { + session.withArk { ark -> + handedOut.complete(ark) + error("boom") + } + } + session.endSession() + + assertTrue(handedOut.await().all { it == 0.toByte() }) + } +} diff --git a/core/security/src/test/kotlin/de/davis/keygo/core/security/data/SessionImplTest.kt b/core/security/src/test/kotlin/de/davis/keygo/core/security/data/SessionImplTest.kt index 3c93ac157..5d83d3d82 100644 --- a/core/security/src/test/kotlin/de/davis/keygo/core/security/data/SessionImplTest.kt +++ b/core/security/src/test/kotlin/de/davis/keygo/core/security/data/SessionImplTest.kt @@ -1,9 +1,11 @@ package de.davis.keygo.core.security.data +import kotlinx.coroutines.test.runTest import kotlin.test.Test +import kotlin.test.assertContentEquals import kotlin.test.assertEquals -import kotlin.test.assertNotEquals import kotlin.test.assertNull +import kotlin.test.assertSame class SessionImplTest { @@ -12,8 +14,8 @@ class SessionImplTest { private fun generateArk(): ByteArray = ByteArray(32) { it.toByte() } @Test - fun `dek is null when no active session`() { - assertNull(session.ark) + fun `no ark is handed out when there is no active session`() = runTest { + assertNull(session.withArk { it }) } @Test @@ -35,30 +37,38 @@ class SessionImplTest { } @Test - fun `startSession makes dek available`() { + fun `startSession makes the ark available`() = runTest { val key = generateArk() session.startSession(key) - assertEquals(key, session.ark) + assertSame(key, session.withArk { it }) } @Test - fun `endSession clears dek`() { + fun `endSession clears the ark`() = runTest { session.startSession(generateArk()) session.endSession() - assertNull(session.ark) + assertNull(session.withArk { it }) } @Test - fun `startSession replaces previous session`() { + fun `startSession replaces previous session`() = runTest { val key1 = generateArk() val key2 = generateArk() session.startSession(key1) session.startSession(key2) - assertNotEquals(key1, session.ark) - assertEquals(key2, session.ark) + assertSame(key2, session.withArk { it }) + } + + @Test + fun `startSession wipes the ark it replaces`() { + val replaced = generateArk() + session.startSession(replaced) + session.startSession(generateArk()) + + assertContentEquals(ByteArray(32), replaced) } @Test diff --git a/core/security/src/testFixtures/kotlin/de/davis/keygo/core/security/crypto/FakeSession.kt b/core/security/src/testFixtures/kotlin/de/davis/keygo/core/security/crypto/FakeSession.kt index 0d13c4627..0ba67fd5b 100644 --- a/core/security/src/testFixtures/kotlin/de/davis/keygo/core/security/crypto/FakeSession.kt +++ b/core/security/src/testFixtures/kotlin/de/davis/keygo/core/security/crypto/FakeSession.kt @@ -1,42 +1,51 @@ package de.davis.keygo.core.security.crypto +import de.davis.keygo.core.security.domain.ArkHolder import de.davis.keygo.core.security.domain.Session import kotlinx.coroutines.flow.MutableStateFlow import kotlinx.coroutines.flow.StateFlow import kotlinx.coroutines.flow.asStateFlow /** - * A fake implementation of [Session] that provides a fixed DEK for testing purposes. + * A fake [Session] with a fixed ARK. Shares [ArkHolder] with the real one, so it wipes the same + * way - a fake that skipped the wipe would hide use-after-wipe bugs from every test. */ class FakeSession( - private val startOnConstruct: Boolean = false + startOnConstruct: Boolean = false ) : Session { var startSessionCalled = false - private var _ark: ByteArray? = null + private val holder = ArkHolder() + private var live: ByteArray? = null private val _isActive = MutableStateFlow(false) - override val ark: ByteArray? - get() = _ark + /** The live ARK as a copy, for assertions. Null once the session has ended. */ + val currentArk: ByteArray? + get() = live?.copyOf() override val isActive: StateFlow = _isActive.asStateFlow() init { + // Constructing pre-unlocked is not a startSession call. if (startOnConstruct) { - _ark = ByteArray(32) { it.toByte() } - _isActive.value = true + startSession(ByteArray(32) { it.toByte() }) + startSessionCalled = false } } + override suspend fun withArk(block: suspend (ByteArray) -> R): R? = holder.withArk(block) + override fun startSession(ark: ByteArray) { - _ark = ark + live = ark + holder.set(ark) _isActive.value = true startSessionCalled = true } override fun endSession() { - _ark = null + live = null + holder.clear() _isActive.value = false } -} \ No newline at end of file +} diff --git a/feature/backup/src/main/kotlin/de/davis/keygo/feature/backup/data/BackupSession.kt b/feature/backup/src/main/kotlin/de/davis/keygo/feature/backup/data/BackupSession.kt index 75a29fda2..a7d9a4f71 100644 --- a/feature/backup/src/main/kotlin/de/davis/keygo/feature/backup/data/BackupSession.kt +++ b/feature/backup/src/main/kotlin/de/davis/keygo/feature/backup/data/BackupSession.kt @@ -10,9 +10,11 @@ import kotlinx.coroutines.flow.StateFlow */ internal class BackupSession(private val backupArk: ByteArray) : Session { - override val ark: ByteArray get() = backupArk override val isActive: StateFlow = MutableStateFlow(true) + /** Always runs [block]: the ARK was already recovered, and whoever recovered it wipes it. */ + override suspend fun withArk(block: suspend (ByteArray) -> R): R? = block(backupArk) + override fun startSession(ark: ByteArray) = error("BackupSession is read-only") diff --git a/feature/backup/src/main/kotlin/de/davis/keygo/feature/backup/domain/BackupArkUnlocker.kt b/feature/backup/src/main/kotlin/de/davis/keygo/feature/backup/domain/BackupArkUnlocker.kt index 935b4ffb9..e9791112f 100644 --- a/feature/backup/src/main/kotlin/de/davis/keygo/feature/backup/domain/BackupArkUnlocker.kt +++ b/feature/backup/src/main/kotlin/de/davis/keygo/feature/backup/domain/BackupArkUnlocker.kt @@ -32,11 +32,10 @@ internal class BackupArkUnlocker( /** * Runs [block] with the ARK for this backup. A recovered ARK is zeroed afterwards; a live - * [Session.ark] is the app's own session key and is left alone. + * session's ARK is the app's own key, left for the session to wipe. */ suspend fun withArk(block: suspend (ByteArray) -> R): Result { - val live = session.ark - if (live != null) return Result.Success(block(live)) + session.withArk { ark -> Result.Success(block(ark)) }?.let { return it } return resultBinding { val ark = recoverArk().bind() @@ -53,8 +52,9 @@ internal class BackupArkUnlocker( suspend fun withScope( block: suspend (ItemWithCryptoScopeUseCase) -> R, ): Result { - val live = session.ark - if (live != null) return Result.Success(block(scopeFor(session))) + // The ark itself goes unused: this is the liveness check that prefers the live session. + session.withArk { Result.Success(block(scopeFor(session))) } + ?.let { return it } return resultBinding { val ark = recoverArk().bind() diff --git a/feature/backup/src/main/kotlin/de/davis/keygo/feature/backup/domain/usecase/FinishExportWizardUseCase.kt b/feature/backup/src/main/kotlin/de/davis/keygo/feature/backup/domain/usecase/FinishExportWizardUseCase.kt index 7f329ec4e..5ac38c9d8 100644 --- a/feature/backup/src/main/kotlin/de/davis/keygo/feature/backup/domain/usecase/FinishExportWizardUseCase.kt +++ b/feature/backup/src/main/kotlin/de/davis/keygo/feature/backup/domain/usecase/FinishExportWizardUseCase.kt @@ -6,8 +6,10 @@ import de.davis.keygo.core.security.domain.crypto.model.CryptographicData import de.davis.keygo.core.security.domain.crypto.suspendDoFinal import de.davis.keygo.core.security.domain.model.CryptographicMode import de.davis.keygo.core.security.domain.model.KeyId +import de.davis.keygo.core.security.domain.withArkOr import de.davis.keygo.core.util.Result import de.davis.keygo.core.util.asResult +import de.davis.keygo.core.util.mapFailure import de.davis.keygo.core.util.onFailure import de.davis.keygo.core.util.resultBinding import de.davis.keygo.feature.backup.domain.BackupDestinationResolver @@ -108,15 +110,14 @@ class FinishExportWizardUseCase( } private suspend fun provisionBackupArk() = resultBinding { - val ark = session.ark - .asResult(FinishExportWizardError.CryptoFailed).bind() - val cipher = keyStoreManager.getOrCreateCipherFor( keyId = KeyId.BackupArkKey, cryptographicMode = CryptographicMode.Encrypt, ) - val data = cipher.suspendDoFinal(ark).bind { FinishExportWizardError.CryptoFailed } + val data = session.withArkOr(FinishExportWizardError.CryptoFailed) { ark -> + cipher.suspendDoFinal(ark).mapFailure { FinishExportWizardError.CryptoFailed } + }.bind() arkKeyStore.save(CryptographicData(data, cipher.iv)) } } diff --git a/feature/backup/src/main/kotlin/de/davis/keygo/feature/backup/domain/usecase/ImportBackupUseCase.kt b/feature/backup/src/main/kotlin/de/davis/keygo/feature/backup/domain/usecase/ImportBackupUseCase.kt index 31da26f10..49b69b03e 100644 --- a/feature/backup/src/main/kotlin/de/davis/keygo/feature/backup/domain/usecase/ImportBackupUseCase.kt +++ b/feature/backup/src/main/kotlin/de/davis/keygo/feature/backup/domain/usecase/ImportBackupUseCase.kt @@ -1,8 +1,10 @@ package de.davis.keygo.feature.backup.domain.usecase import de.davis.keygo.core.security.domain.Session +import de.davis.keygo.core.security.domain.withArkOr import de.davis.keygo.core.util.Result import de.davis.keygo.core.util.fold +import de.davis.keygo.core.util.mapFailure import de.davis.keygo.core.util.resultBinding import de.davis.keygo.feature.backup.domain.BackupFileStore import de.davis.keygo.feature.backup.domain.BackupRestorer @@ -39,7 +41,7 @@ internal class ImportBackupUseCase( */ operator fun invoke(request: ImportRequest): Flow = channelFlow { val outcome = resultBinding { - if (session.ark == null) + if (!session.isActive.value) Result.Failure(ImportError.SessionLocked).bind() send(ImportProgress.Reading) @@ -66,28 +68,26 @@ internal class ImportBackupUseCase( private suspend fun parse(request: ImportRequest, text: String): Result = resultBinding { when (request.format) { - FileFormat.JSON -> { - val credential = when ( - jsonBackupManager.inspectWithResult(text).bind { it.toImportError() } - ) { - JsonEncryption.PASSPHRASE -> request.passphrase + FileFormat.JSON -> when ( + jsonBackupManager.inspectWithResult(text).bind { it.toImportError() } + ) { + JsonEncryption.PASSPHRASE -> { + val passphrase = request.passphrase ?.takeIf(String::isNotBlank) - ?.let { BackupCredential.Passphrase(it.encodeToByteArray()) } ?: return Result.Failure(ImportError.PassphraseRequired) - - JsonEncryption.ARK -> BackupCredential.Ark( - session.ark - ?: return Result.Failure(ImportError.SessionLocked), - ) - } - // Zero the derived passphrase bytes once Rust is done with them, mirroring the - // export path. The live ARK belongs to the session and is left alone. - try { - jsonBackupManager.importWithResult(text, credential) - .bind { it.toImportError() } - } finally { - (credential as? BackupCredential.Passphrase)?.bytes?.fill(0) + val credential = + BackupCredential.Passphrase(passphrase.encodeToByteArray()) + // Zero the derived bytes once Rust is done, mirroring the export path. + try { + importJson(text, credential).bind() + } finally { + credential.bytes.fill(0) + } } + + JsonEncryption.ARK -> session.withArkOr(ImportError.SessionLocked) { ark -> + importJson(text, BackupCredential.Ark(ark)) + }.bind() } FileFormat.CSV -> { @@ -100,4 +100,10 @@ internal class ImportBackupUseCase( } } } + + private suspend fun importJson( + text: String, + credential: BackupCredential, + ): Result = jsonBackupManager.importWithResult(text, credential) + .mapFailure { it.toImportError() } } diff --git a/feature/backup/src/test/kotlin/de/davis/keygo/feature/backup/domain/BackupArkUnlockerTest.kt b/feature/backup/src/test/kotlin/de/davis/keygo/feature/backup/domain/BackupArkUnlockerTest.kt index 64984eab9..85c44b99f 100644 --- a/feature/backup/src/test/kotlin/de/davis/keygo/feature/backup/domain/BackupArkUnlockerTest.kt +++ b/feature/backup/src/test/kotlin/de/davis/keygo/feature/backup/domain/BackupArkUnlockerTest.kt @@ -66,7 +66,7 @@ class BackupArkUnlockerTest { val result = unlocker(FakeSession(startOnConstruct = false)).withScope { val used = factory.lastSession assertIs(used) - assertContentEquals(ark, used.ark) + used.withArk { assertContentEquals(ark, it) } } assertIs>(result) @@ -84,7 +84,7 @@ class BackupArkUnlockerTest { @Test fun `withArk hands over the live session ark`() = runTest { val session = FakeSession(startOnConstruct = true) - val expected = assertNotNull(session.ark).copyOf() + val expected = assertNotNull(session.currentArk) val result = unlocker(session).withArk { assertContentEquals(expected, it) } @@ -132,14 +132,14 @@ class BackupArkUnlockerTest { val result = unlocker(FakeSession(startOnConstruct = false)).withScope { val used = factory.lastSession assertIs(used) - assertContentEquals(ark, used.ark) + used.withArk { assertContentEquals(ark, it) } } assertIs>(result) val used = factory.lastSession assertIs(used) - assertTrue(assertNotNull(used.ark).all { it == 0.toByte() }) + used.withArk { recovered -> assertTrue(recovered.all { it == 0.toByte() }) } } @Test @@ -150,6 +150,6 @@ class BackupArkUnlockerTest { unlocker(session).withArk { } - assertTrue(assertNotNull(session.ark).any { it != 0.toByte() }) + assertTrue(assertNotNull(session.currentArk).any { it != 0.toByte() }) } } diff --git a/feature/backup/src/test/kotlin/de/davis/keygo/feature/backup/domain/usecase/ExportBackupUseCaseTest.kt b/feature/backup/src/test/kotlin/de/davis/keygo/feature/backup/domain/usecase/ExportBackupUseCaseTest.kt index 5584961b2..52c397eb9 100644 --- a/feature/backup/src/test/kotlin/de/davis/keygo/feature/backup/domain/usecase/ExportBackupUseCaseTest.kt +++ b/feature/backup/src/test/kotlin/de/davis/keygo/feature/backup/domain/usecase/ExportBackupUseCaseTest.kt @@ -76,7 +76,8 @@ class ExportBackupUseCaseTest { private suspend fun provision(session: FakeSession) { val cipher = keyStore.getOrCreateCipherFor(KeyId.BackupArkKey, CryptographicMode.Encrypt) - arkStore.save(CryptographicData(cipher.doFinal(assertNotNull(session.ark)), cipher.iv)) + val ark = assertNotNull(session.currentArk) + arkStore.save(CryptographicData(cipher.doFinal(ark), cipher.iv)) } private val csvJob = BackupJob( @@ -197,7 +198,7 @@ class ExportBackupUseCaseTest { assertIs(emissions.last()) val credential = assertIs(json.exportCalls.single().credential) - assertContentEquals(session.ark, credential.key) + assertContentEquals(session.currentArk, credential.key) } @Test @@ -217,7 +218,7 @@ class ExportBackupUseCaseTest { assertIs(emissions.last()) val credential = assertIs(json.exportCalls.single().credential) - assertContentEquals(unlockedSession.ark, credential.key) + assertContentEquals(unlockedSession.currentArk, credential.key) } @Test diff --git a/feature/backup/src/test/kotlin/de/davis/keygo/feature/backup/domain/usecase/FinishExportWizardUseCaseTest.kt b/feature/backup/src/test/kotlin/de/davis/keygo/feature/backup/domain/usecase/FinishExportWizardUseCaseTest.kt index bfeebab9d..c874bea79 100644 --- a/feature/backup/src/test/kotlin/de/davis/keygo/feature/backup/domain/usecase/FinishExportWizardUseCaseTest.kt +++ b/feature/backup/src/test/kotlin/de/davis/keygo/feature/backup/domain/usecase/FinishExportWizardUseCaseTest.kt @@ -21,8 +21,8 @@ import de.davis.keygo.feature.backup.domain.model.FinishExportWizardError import de.davis.keygo.feature.backup.domain.model.IntervalUnit import kotlinx.coroutines.test.runTest import kotlin.test.Test -import kotlin.test.assertEquals import kotlin.test.assertContentEquals +import kotlin.test.assertEquals import kotlin.test.assertIs import kotlin.test.assertNotNull import kotlin.test.assertNull @@ -135,7 +135,7 @@ class FinishExportWizardUseCaseTest { val recovered = keyStoreManager .getOrCreateCipherFor(KeyId.BackupArkKey, CryptographicMode.Decrypt, wrapped.iv) .doFinal(wrapped.data) - assertContentEquals(session.ark, recovered) + assertContentEquals(session.currentArk, recovered) } @Test diff --git a/feature/backup/src/test/kotlin/de/davis/keygo/feature/backup/domain/usecase/ImportBackupUseCaseTest.kt b/feature/backup/src/test/kotlin/de/davis/keygo/feature/backup/domain/usecase/ImportBackupUseCaseTest.kt index c31d6b238..3f8e91b59 100644 --- a/feature/backup/src/test/kotlin/de/davis/keygo/feature/backup/domain/usecase/ImportBackupUseCaseTest.kt +++ b/feature/backup/src/test/kotlin/de/davis/keygo/feature/backup/domain/usecase/ImportBackupUseCaseTest.kt @@ -230,7 +230,7 @@ class ImportBackupUseCaseTest { assertIs(emissions.last()) val credential = assertIs(json.importCalls.single().credential) - assertContentEquals(session.ark, credential.key) + assertContentEquals(session.currentArk, credential.key) } @Test diff --git a/feature/vault/src/main/kotlin/de/davis/keygo/feature/vault/domain/usecase/CreateVaultUseCase.kt b/feature/vault/src/main/kotlin/de/davis/keygo/feature/vault/domain/usecase/CreateVaultUseCase.kt index 3ca66263a..6df4ce96d 100644 --- a/feature/vault/src/main/kotlin/de/davis/keygo/feature/vault/domain/usecase/CreateVaultUseCase.kt +++ b/feature/vault/src/main/kotlin/de/davis/keygo/feature/vault/domain/usecase/CreateVaultUseCase.kt @@ -6,8 +6,9 @@ import de.davis.keygo.core.item.domain.model.Vault import de.davis.keygo.core.item.domain.repository.VaultContextRepository import de.davis.keygo.core.item.domain.repository.VaultRepository import de.davis.keygo.core.security.domain.Session +import de.davis.keygo.core.security.domain.withArkOr import de.davis.keygo.core.util.Result -import de.davis.keygo.core.util.asResult +import de.davis.keygo.core.util.mapFailure import de.davis.keygo.core.util.resultBinding import de.davis.keygo.feature.vault.domain.model.VaultCreationError import de.davis.keygo.rust.vault.VaultManager @@ -36,11 +37,11 @@ class CreateVaultUseCase( val vaultId = newVaultId() - val ark = session.ark.asResult(VaultCreationError.NoActiveSession).bind() val vaultKey = vaultManager.createNewVaultKey() - val wrappedVaultKey = + val wrappedVaultKey = session.withArkOr(VaultCreationError.NoActiveSession) { ark -> keyWrapper.wrapVaultKeyWithResult(ark, vaultKey, vaultId) - .bind { VaultCreationError.WrapFailed } + .mapFailure { VaultCreationError.WrapFailed } + }.bind() val vault = Vault( id = vaultId, diff --git a/feature/vault/src/test/kotlin/de/davis/keygo/feature/vault/domain/usecase/MoveItemsToVaultUseCaseTest.kt b/feature/vault/src/test/kotlin/de/davis/keygo/feature/vault/domain/usecase/MoveItemsToVaultUseCaseTest.kt index b26790aa5..51a7ad054 100644 --- a/feature/vault/src/test/kotlin/de/davis/keygo/feature/vault/domain/usecase/MoveItemsToVaultUseCaseTest.kt +++ b/feature/vault/src/test/kotlin/de/davis/keygo/feature/vault/domain/usecase/MoveItemsToVaultUseCaseTest.kt @@ -316,7 +316,7 @@ class MoveItemsToVaultUseCaseTest { private fun makeVault(name: String, id: VaultId = newVaultId()): Vault { val vaultKey = ByteArray(32) { (id.hashCode() + it).toByte() } val wrapped = keyWrapper.wrapVaultKey( - ark = assertNotNull(session.ark), + ark = assertNotNull(session.currentArk), vaultKey = vaultKey, vaultId = id, ) From 086c72cf36d87ae7e595a3f067fcc449725dd3bb Mon Sep 17 00:00:00 2001 From: Davis Wolfermann Date: Fri, 4 Sep 2026 17:06:40 +0200 Subject: [PATCH 17/35] feat(app): implement SystemHandoff for session management and activity launching --- core/security/build.gradle.kts | 1 + .../core/security/data/SessionLockObserver.kt | 43 ++++++++++ .../core/security/data/SystemHandoffImpl.kt | 24 ++++++ .../keygo/core/security/domain/ArkHolder.kt | 14 ---- .../core/security/domain/SystemHandoff.kt | 20 +++++ .../security/presentation/HandoffLauncher.kt | 39 +++++++++ .../security/data/SessionLockObserverTest.kt | 82 +++++++++++++++++++ .../security/data/SystemHandoffImplTest.kt | 59 +++++++++++++ .../core/security/domain/SystemHandoffTest.kt | 38 +++++++++ .../presentation/HandoffLauncherTest.kt | 50 +++++++++++ .../ChromeAutofillRepositoryImpl.kt | 5 +- .../presentation/activity/AutofillActivity.kt | 4 +- .../presentation/export/ExportWizardScreen.kt | 4 +- .../presentation/import/ImportFilePicker.kt | 4 +- feature/credit-card/build.gradle.kts | 1 + .../credit_card/presentation/NfcInfoCard.kt | 6 +- .../item/view/data/WebsiteHandlerImpl.kt | 26 +++--- .../presentation/OnboardingScreen.kt | 4 +- .../settings/presentation/SettingsScreen.kt | 4 +- .../totp/presentation/component/QRScanner.kt | 4 +- 20 files changed, 393 insertions(+), 39 deletions(-) create mode 100644 core/security/src/main/kotlin/de/davis/keygo/core/security/data/SystemHandoffImpl.kt create mode 100644 core/security/src/main/kotlin/de/davis/keygo/core/security/domain/SystemHandoff.kt create mode 100644 core/security/src/main/kotlin/de/davis/keygo/core/security/presentation/HandoffLauncher.kt create mode 100644 core/security/src/test/kotlin/de/davis/keygo/core/security/data/SessionLockObserverTest.kt create mode 100644 core/security/src/test/kotlin/de/davis/keygo/core/security/data/SystemHandoffImplTest.kt create mode 100644 core/security/src/test/kotlin/de/davis/keygo/core/security/domain/SystemHandoffTest.kt create mode 100644 core/security/src/test/kotlin/de/davis/keygo/core/security/presentation/HandoffLauncherTest.kt diff --git a/core/security/build.gradle.kts b/core/security/build.gradle.kts index aa808cb96..6c0db9e1b 100644 --- a/core/security/build.gradle.kts +++ b/core/security/build.gradle.kts @@ -18,6 +18,7 @@ dependencies { api(projects.core.util) api(projects.rust) + testImplementation(libs.robolectric) testImplementation(testFixtures(projects.rust)) testImplementation(testFixtures(projects.core.item)) testImplementation(testFixtures(projects.core.util)) diff --git a/core/security/src/main/kotlin/de/davis/keygo/core/security/data/SessionLockObserver.kt b/core/security/src/main/kotlin/de/davis/keygo/core/security/data/SessionLockObserver.kt index eeb94b625..ed8ed7cc5 100644 --- a/core/security/src/main/kotlin/de/davis/keygo/core/security/data/SessionLockObserver.kt +++ b/core/security/src/main/kotlin/de/davis/keygo/core/security/data/SessionLockObserver.kt @@ -1,21 +1,64 @@ package de.davis.keygo.core.security.data +import android.content.BroadcastReceiver +import android.content.Context +import android.content.Intent +import android.content.IntentFilter +import androidx.core.content.ContextCompat import androidx.lifecycle.DefaultLifecycleObserver import androidx.lifecycle.LifecycleOwner import androidx.lifecycle.ProcessLifecycleOwner import de.davis.keygo.core.security.domain.Session +import de.davis.keygo.core.security.domain.SystemHandoff import org.koin.core.annotation.Single @Single(createdAtStart = true) internal class SessionLockObserver( + private val context: Context, private val session: Session, + private val handoff: SystemHandoff, ) : DefaultLifecycleObserver { + private val screenOffReceiver = object : BroadcastReceiver() { + override fun onReceive(context: Context, intent: Intent) = endSession() + } + + private var watchingScreenOff = false + init { ProcessLifecycleOwner.get().lifecycle.addObserver(this) } + override fun onStart(owner: LifecycleOwner) { + stopWatchingScreenOff() + handoff.clear() + } + override fun onStop(owner: LifecycleOwner) { + if (handoff.isPending) watchScreenOff() + else session.endSession() + } + + private fun endSession() { + stopWatchingScreenOff() + handoff.clear() session.endSession() } + + private fun watchScreenOff() { + if (watchingScreenOff) return + ContextCompat.registerReceiver( + context, + screenOffReceiver, + IntentFilter(Intent.ACTION_SCREEN_OFF), + ContextCompat.RECEIVER_NOT_EXPORTED, + ) + watchingScreenOff = true + } + + private fun stopWatchingScreenOff() { + if (!watchingScreenOff) return + context.unregisterReceiver(screenOffReceiver) + watchingScreenOff = false + } } diff --git a/core/security/src/main/kotlin/de/davis/keygo/core/security/data/SystemHandoffImpl.kt b/core/security/src/main/kotlin/de/davis/keygo/core/security/data/SystemHandoffImpl.kt new file mode 100644 index 000000000..54a7bda63 --- /dev/null +++ b/core/security/src/main/kotlin/de/davis/keygo/core/security/data/SystemHandoffImpl.kt @@ -0,0 +1,24 @@ +package de.davis.keygo.core.security.data + +import de.davis.keygo.core.security.domain.SystemHandoff +import org.koin.core.annotation.Single +import java.util.concurrent.atomic.AtomicInteger + +@Single +internal class SystemHandoffImpl : SystemHandoff { + + private val pending = AtomicInteger() + + override val isPending: Boolean + get() = pending.get() > 0 + + override fun expectReturn() { + pending.incrementAndGet() + } + + override fun returned() { + pending.updateAndGet { (it - 1).coerceAtLeast(0) } + } + + override fun clear() = pending.set(0) +} diff --git a/core/security/src/main/kotlin/de/davis/keygo/core/security/domain/ArkHolder.kt b/core/security/src/main/kotlin/de/davis/keygo/core/security/domain/ArkHolder.kt index 50fb022f0..067dc7400 100644 --- a/core/security/src/main/kotlin/de/davis/keygo/core/security/domain/ArkHolder.kt +++ b/core/security/src/main/kotlin/de/davis/keygo/core/security/domain/ArkHolder.kt @@ -1,24 +1,13 @@ package de.davis.keygo.core.security.domain -/** - * Holds a session's ARK and owns its wipe. A [withArk] block keeps intact bytes for its whole - * duration: a session ending underneath defers the wipe to the last block out. Shared by the real - * session and its fake so the rule cannot drift between them. - */ class ArkHolder { - /** Never held across [withArk]'s block - a monitor cannot span a suspension point. */ private val lock = Any() private var ark: ByteArray? = null - - /** How many [withArk] blocks are running. A wipe waits for this to reach zero. */ private var readers = 0 - - /** ARKs dropped while a reader held them. Several pile up across repeated end and start. */ private val awaitingWipe = mutableListOf() - /** Runs [block] with the held ARK, or returns `null` without running it when there is none. */ suspend fun withArk(block: suspend (ByteArray) -> R): R? { val live = synchronized(lock) { val current = ark ?: return null @@ -36,10 +25,8 @@ class ArkHolder { } } - /** Takes [ark] as the held one, dropping whatever it replaces. */ fun set(ark: ByteArray) = replace(ark) - /** Puts the ARK out of reach at once, wiping it unless a [withArk] block still holds it. */ fun clear() = replace(null) private fun replace(next: ByteArray?) { @@ -50,7 +37,6 @@ class ArkHolder { } } - /** Callers hold [lock]. */ private fun wipePending() { awaitingWipe.forEach { it.fill(0) } awaitingWipe.clear() diff --git a/core/security/src/main/kotlin/de/davis/keygo/core/security/domain/SystemHandoff.kt b/core/security/src/main/kotlin/de/davis/keygo/core/security/domain/SystemHandoff.kt new file mode 100644 index 000000000..abb75256d --- /dev/null +++ b/core/security/src/main/kotlin/de/davis/keygo/core/security/domain/SystemHandoff.kt @@ -0,0 +1,20 @@ +package de.davis.keygo.core.security.domain + +interface SystemHandoff { + + val isPending: Boolean + + fun expectReturn() + fun returned() + fun clear() +} + +inline fun SystemHandoff.forRoundTrip(open: () -> Unit) { + expectReturn() + try { + open() + } catch (e: Throwable) { + returned() + throw e + } +} diff --git a/core/security/src/main/kotlin/de/davis/keygo/core/security/presentation/HandoffLauncher.kt b/core/security/src/main/kotlin/de/davis/keygo/core/security/presentation/HandoffLauncher.kt new file mode 100644 index 000000000..571894269 --- /dev/null +++ b/core/security/src/main/kotlin/de/davis/keygo/core/security/presentation/HandoffLauncher.kt @@ -0,0 +1,39 @@ +package de.davis.keygo.core.security.presentation + +import android.content.Intent +import androidx.activity.compose.rememberLauncherForActivityResult +import androidx.activity.result.contract.ActivityResultContract +import androidx.compose.runtime.Composable +import androidx.compose.runtime.remember +import androidx.compose.ui.platform.LocalContext +import de.davis.keygo.core.security.domain.SystemHandoff +import de.davis.keygo.core.security.domain.forRoundTrip +import org.koin.compose.koinInject + +class HandoffLauncher( + private val handoff: SystemHandoff, + private val onLaunch: (I) -> Unit, +) { + + fun launch(input: I) = handoff.forRoundTrip { onLaunch(input) } +} + +@Composable +fun rememberHandoffLauncher( + contract: ActivityResultContract, + onResult: (O) -> Unit, +): HandoffLauncher { + val handoff = koinInject() + val launcher = rememberLauncherForActivityResult(contract) { + handoff.returned() + onResult(it) + } + return remember(handoff, launcher) { HandoffLauncher(handoff) { launcher.launch(it) } } +} + +@Composable +fun rememberHandoffStarter(): HandoffLauncher { + val handoff = koinInject() + val context = LocalContext.current + return remember(handoff, context) { HandoffLauncher(handoff) { context.startActivity(it) } } +} diff --git a/core/security/src/test/kotlin/de/davis/keygo/core/security/data/SessionLockObserverTest.kt b/core/security/src/test/kotlin/de/davis/keygo/core/security/data/SessionLockObserverTest.kt new file mode 100644 index 000000000..c99d05158 --- /dev/null +++ b/core/security/src/test/kotlin/de/davis/keygo/core/security/data/SessionLockObserverTest.kt @@ -0,0 +1,82 @@ +package de.davis.keygo.core.security.data + +import android.content.Intent +import android.os.Looper +import androidx.lifecycle.Lifecycle +import androidx.lifecycle.LifecycleOwner +import androidx.lifecycle.LifecycleRegistry +import org.junit.runner.RunWith +import org.robolectric.RobolectricTestRunner +import org.robolectric.RuntimeEnvironment +import org.robolectric.Shadows.shadowOf +import org.robolectric.annotation.Config +import kotlin.test.Test +import kotlin.test.assertEquals + +@RunWith(RobolectricTestRunner::class) +@Config(sdk = [34]) +internal class SessionLockObserverTest { + + private val context = RuntimeEnvironment.getApplication() + private val session = SessionImpl().apply { startSession(ByteArray(32) { it.toByte() }) } + private val handoff = SystemHandoffImpl() + private val observer = SessionLockObserver(context, session, handoff) + private val owner = StubLifecycleOwner() + + private fun screenOff() { + context.sendBroadcast(Intent(Intent.ACTION_SCREEN_OFF)) + shadowOf(Looper.getMainLooper()).idle() + } + + @Test + fun `backgrounding ends the session`() { + observer.onStop(owner) + + assertEquals(false, session.isActive.value) + } + + @Test + fun `backgrounding for a system screen we launched keeps the session`() { + handoff.expectReturn() + + observer.onStop(owner) + + assertEquals(true, session.isActive.value) + } + + @Test + fun `a handoff covers one round trip, not the background after it`() { + handoff.expectReturn() + observer.onStop(owner) + observer.onStart(owner) + + observer.onStop(owner) + + assertEquals(false, session.isActive.value) + } + + @Test + fun `the screen going off during a handoff ends the session`() { + handoff.expectReturn() + observer.onStop(owner) + + screenOff() + + assertEquals(false, session.isActive.value) + } + + @Test + fun `the screen stops being watched once the app is back in the foreground`() { + handoff.expectReturn() + observer.onStop(owner) + observer.onStart(owner) + + screenOff() + + assertEquals(true, session.isActive.value) + } +} + +private class StubLifecycleOwner : LifecycleOwner { + override val lifecycle: Lifecycle = LifecycleRegistry.createUnsafe(this) +} diff --git a/core/security/src/test/kotlin/de/davis/keygo/core/security/data/SystemHandoffImplTest.kt b/core/security/src/test/kotlin/de/davis/keygo/core/security/data/SystemHandoffImplTest.kt new file mode 100644 index 000000000..e4f65451a --- /dev/null +++ b/core/security/src/test/kotlin/de/davis/keygo/core/security/data/SystemHandoffImplTest.kt @@ -0,0 +1,59 @@ +package de.davis.keygo.core.security.data + +import kotlin.test.Test +import kotlin.test.assertFalse +import kotlin.test.assertTrue + +internal class SystemHandoffImplTest { + + private val handoff = SystemHandoffImpl() + + @Test + fun `nothing is pending before a handoff is armed`() { + assertFalse(handoff.isPending) + } + + @Test + fun `expectReturn arms a handoff`() { + handoff.expectReturn() + + assertTrue(handoff.isPending) + } + + @Test + fun `returned disarms the handoff`() { + handoff.expectReturn() + handoff.returned() + + assertFalse(handoff.isPending) + } + + @Test + fun `two armed handoffs need two returns`() { + handoff.expectReturn() + handoff.expectReturn() + handoff.returned() + + assertTrue(handoff.isPending) + } + + @Test + fun `clear drops every armed handoff at once`() { + handoff.expectReturn() + handoff.expectReturn() + handoff.clear() + + assertFalse(handoff.isPending) + } + + @Test + fun `a return that arrives after a clear does not eat the next handoff`() { + handoff.expectReturn() + handoff.clear() + handoff.returned() + + handoff.expectReturn() + + assertTrue(handoff.isPending) + } +} diff --git a/core/security/src/test/kotlin/de/davis/keygo/core/security/domain/SystemHandoffTest.kt b/core/security/src/test/kotlin/de/davis/keygo/core/security/domain/SystemHandoffTest.kt new file mode 100644 index 000000000..a44962ca8 --- /dev/null +++ b/core/security/src/test/kotlin/de/davis/keygo/core/security/domain/SystemHandoffTest.kt @@ -0,0 +1,38 @@ +package de.davis.keygo.core.security.domain + +import de.davis.keygo.core.security.data.SystemHandoffImpl +import kotlin.test.Test +import kotlin.test.assertEquals +import kotlin.test.assertFailsWith +import kotlin.test.assertFalse +import kotlin.test.assertTrue + +internal class SystemHandoffTest { + + private val handoff = SystemHandoffImpl() + + @Test + fun `a round trip stays armed after the system screen has been opened`() { + handoff.forRoundTrip { } + + assertTrue(handoff.isPending) + } + + @Test + fun `a system screen that will not open leaves no handoff armed behind it`() { + assertFailsWith { + handoff.forRoundTrip { error("nothing resolves this intent") } + } + + assertFalse(handoff.isPending) + } + + @Test + fun `a system screen that will not open still reports the failure to the caller`() { + val thrown = assertFailsWith { + handoff.forRoundTrip { error("nothing resolves this intent") } + } + + assertEquals("nothing resolves this intent", thrown.message) + } +} diff --git a/core/security/src/test/kotlin/de/davis/keygo/core/security/presentation/HandoffLauncherTest.kt b/core/security/src/test/kotlin/de/davis/keygo/core/security/presentation/HandoffLauncherTest.kt new file mode 100644 index 000000000..b8fad9087 --- /dev/null +++ b/core/security/src/test/kotlin/de/davis/keygo/core/security/presentation/HandoffLauncherTest.kt @@ -0,0 +1,50 @@ +package de.davis.keygo.core.security.presentation + +import de.davis.keygo.core.security.data.SystemHandoffImpl +import kotlin.test.Test +import kotlin.test.assertEquals +import kotlin.test.assertFailsWith +import kotlin.test.assertFalse +import kotlin.test.assertTrue + +internal class HandoffLauncherTest { + + private val handoff = SystemHandoffImpl() + + @Test + fun `launching arms a handoff, so the background it causes keeps the session`() { + val launcher = HandoffLauncher(handoff) {} + + launcher.launch(Unit) + + assertTrue(handoff.isPending) + } + + @Test + fun `the input reaches the launcher being wrapped`() { + var launched: String? = null + val launcher = HandoffLauncher(handoff) { launched = it } + + launcher.launch("text/csv") + + assertEquals("text/csv", launched) + } + + @Test + fun `a system screen that will not open leaves no handoff armed behind it`() { + val launcher = HandoffLauncher(handoff) { error("nothing resolves this intent") } + + assertFailsWith { launcher.launch(Unit) } + + assertFalse(handoff.isPending) + } + + @Test + fun `a system screen that will not open still reports the failure to the caller`() { + val launcher = HandoffLauncher(handoff) { error("nothing resolves this intent") } + + val thrown = assertFailsWith { launcher.launch(Unit) } + + assertEquals("nothing resolves this intent", thrown.message) + } +} diff --git a/feature/autofill/src/main/kotlin/de/davis/keygo/feature/autofill/data/repository/ChromeAutofillRepositoryImpl.kt b/feature/autofill/src/main/kotlin/de/davis/keygo/feature/autofill/data/repository/ChromeAutofillRepositoryImpl.kt index a656c23d2..baa252249 100644 --- a/feature/autofill/src/main/kotlin/de/davis/keygo/feature/autofill/data/repository/ChromeAutofillRepositoryImpl.kt +++ b/feature/autofill/src/main/kotlin/de/davis/keygo/feature/autofill/data/repository/ChromeAutofillRepositoryImpl.kt @@ -6,6 +6,8 @@ import android.content.Intent import android.database.Cursor import android.net.Uri import android.util.Log +import de.davis.keygo.core.security.domain.SystemHandoff +import de.davis.keygo.core.security.domain.forRoundTrip import de.davis.keygo.feature.autofill.domain.repository.ChromeAutofillRepository import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.withContext @@ -14,6 +16,7 @@ import org.koin.core.annotation.Single @Single internal class ChromeAutofillRepositoryImpl( private val context: Context, + private val handoff: SystemHandoff, ) : ChromeAutofillRepository { private val thirdPartyModeUri: Uri @@ -60,7 +63,7 @@ internal class ChromeAutofillRepositoryImpl( val chooser = Intent.createChooser(intent, "Pick Chrome Channel") .addFlags(Intent.FLAG_ACTIVITY_NEW_TASK) - context.startActivity(chooser) + handoff.forRoundTrip { context.startActivity(chooser) } } private companion object { diff --git a/feature/autofill/src/main/kotlin/de/davis/keygo/feature/autofill/presentation/activity/AutofillActivity.kt b/feature/autofill/src/main/kotlin/de/davis/keygo/feature/autofill/presentation/activity/AutofillActivity.kt index 1a5d776d0..a1d9667eb 100644 --- a/feature/autofill/src/main/kotlin/de/davis/keygo/feature/autofill/presentation/activity/AutofillActivity.kt +++ b/feature/autofill/src/main/kotlin/de/davis/keygo/feature/autofill/presentation/activity/AutofillActivity.kt @@ -5,7 +5,6 @@ import android.content.Intent import android.os.Bundle import android.service.autofill.Dataset import android.view.autofill.AutofillManager -import androidx.activity.compose.rememberLauncherForActivityResult import androidx.activity.compose.setContent import androidx.activity.result.IntentSenderRequest import androidx.activity.result.contract.ActivityResultContracts @@ -20,6 +19,7 @@ import de.davis.keygo.core.identity.presentation.rememberBiometricUnlockAdapter import de.davis.keygo.core.identity.presentation.useAdapter import de.davis.keygo.core.security.domain.model.BiometricPolicy import de.davis.keygo.core.security.presentation.rememberBiometricCryptoController +import de.davis.keygo.core.security.presentation.rememberHandoffLauncher import de.davis.keygo.core.ui.clipboard.setText import de.davis.keygo.core.ui.theme.KeyGoTheme import de.davis.keygo.core.util.onFailure @@ -69,7 +69,7 @@ internal class AutofillActivity : FragmentActivity() { val clipboard = LocalClipboard.current val passwordLabel = stringResource(CoreItemR.string.password) - val smsConsentLauncher = rememberLauncherForActivityResult( + val smsConsentLauncher = rememberHandoffLauncher( ActivityResultContracts.StartIntentSenderForResult(), ) { result -> viewModel.onEvent( diff --git a/feature/backup/src/main/kotlin/de/davis/keygo/feature/backup/presentation/export/ExportWizardScreen.kt b/feature/backup/src/main/kotlin/de/davis/keygo/feature/backup/presentation/export/ExportWizardScreen.kt index e1b86ab3a..08e84cbc4 100644 --- a/feature/backup/src/main/kotlin/de/davis/keygo/feature/backup/presentation/export/ExportWizardScreen.kt +++ b/feature/backup/src/main/kotlin/de/davis/keygo/feature/backup/presentation/export/ExportWizardScreen.kt @@ -1,10 +1,10 @@ package de.davis.keygo.feature.backup.presentation.export -import androidx.activity.compose.rememberLauncherForActivityResult import androidx.activity.result.contract.ActivityResultContracts import androidx.compose.runtime.Composable import androidx.compose.runtime.getValue import androidx.lifecycle.compose.collectAsStateWithLifecycle +import de.davis.keygo.core.security.presentation.rememberHandoffLauncher import de.davis.keygo.core.util.presentation.ObserveAsEvents import de.davis.keygo.feature.backup.domain.model.BackupDestinationUri import de.davis.keygo.feature.backup.presentation.export.model.ExportWizardEvent @@ -15,7 +15,7 @@ fun ExportWizardScreen(navigateUp: () -> Unit) { val viewModel = koinViewModel() val state by viewModel.state.collectAsStateWithLifecycle() - val folderPicker = rememberLauncherForActivityResult( + val folderPicker = rememberHandoffLauncher( ActivityResultContracts.OpenDocumentTree(), ) { uri -> viewModel.onDestinationPicked(uri?.let { BackupDestinationUri(it.toString()) }) diff --git a/feature/backup/src/main/kotlin/de/davis/keygo/feature/backup/presentation/import/ImportFilePicker.kt b/feature/backup/src/main/kotlin/de/davis/keygo/feature/backup/presentation/import/ImportFilePicker.kt index 792c2a0f2..6961f418f 100644 --- a/feature/backup/src/main/kotlin/de/davis/keygo/feature/backup/presentation/import/ImportFilePicker.kt +++ b/feature/backup/src/main/kotlin/de/davis/keygo/feature/backup/presentation/import/ImportFilePicker.kt @@ -1,9 +1,9 @@ package de.davis.keygo.feature.backup.presentation.import -import androidx.activity.compose.rememberLauncherForActivityResult import androidx.activity.result.contract.ActivityResultContracts import androidx.compose.runtime.Composable import androidx.compose.runtime.remember +import de.davis.keygo.core.security.presentation.rememberHandoffLauncher import de.davis.keygo.feature.backup.domain.model.BackupDestinationUri import de.davis.keygo.feature.backup.domain.model.FileFormat @@ -15,7 +15,7 @@ fun interface FilePickerAction { @Composable fun rememberImportFilePicker(onPicked: (BackupDestinationUri) -> Unit): FilePickerAction { - val launcher = rememberLauncherForActivityResult( + val launcher = rememberHandoffLauncher( ActivityResultContracts.OpenDocument(), ) { uri -> uri?.let { onPicked(BackupDestinationUri(it.toString())) } diff --git a/feature/credit-card/build.gradle.kts b/feature/credit-card/build.gradle.kts index 18b67b8b5..147e591b2 100644 --- a/feature/credit-card/build.gradle.kts +++ b/feature/credit-card/build.gradle.kts @@ -12,6 +12,7 @@ android { dependencies { api(projects.core.util) + implementation(projects.core.security) implementation(projects.core.ui) implementation(libs.devnied.emvnfccard) diff --git a/feature/credit-card/src/main/kotlin/de/davis/keygo/feature/credit_card/presentation/NfcInfoCard.kt b/feature/credit-card/src/main/kotlin/de/davis/keygo/feature/credit_card/presentation/NfcInfoCard.kt index 0215fc765..b6be94858 100644 --- a/feature/credit-card/src/main/kotlin/de/davis/keygo/feature/credit_card/presentation/NfcInfoCard.kt +++ b/feature/credit-card/src/main/kotlin/de/davis/keygo/feature/credit_card/presentation/NfcInfoCard.kt @@ -35,7 +35,6 @@ import androidx.compose.ui.Modifier import androidx.compose.ui.draw.clipToBounds import androidx.compose.ui.graphics.vector.ImageVector import androidx.compose.ui.layout.layout -import androidx.compose.ui.platform.LocalContext import androidx.compose.ui.res.stringResource import androidx.compose.ui.text.style.TextAlign import androidx.compose.ui.tooling.preview.Preview @@ -43,6 +42,7 @@ import androidx.compose.ui.tooling.preview.PreviewParameter import androidx.compose.ui.tooling.preview.PreviewParameterProvider import androidx.compose.ui.unit.Dp import androidx.compose.ui.unit.dp +import de.davis.keygo.core.security.presentation.rememberHandoffStarter import de.davis.keygo.feature.credit_card.R import de.davis.keygo.feature.credit_card.domain.model.Card import de.davis.keygo.feature.credit_card.domain.model.CardReadFailure @@ -81,8 +81,8 @@ internal fun NfcInfoCard( onRetry: () -> Unit, modifier: Modifier = Modifier, ) { - val context = LocalContext.current - val onEnableNfc = { context.startActivity(Intent(Settings.ACTION_NFC_SETTINGS)) } + val openSystemScreen = rememberHandoffStarter() + val onEnableNfc = { openSystemScreen.launch(Intent(Settings.ACTION_NFC_SETTINGS)) } // 0 = no action (text sits lower under the indicator), 1 = action shown // (text slid up, action revealed below). Hoisted out of AnimatedContent so a diff --git a/feature/item/view/src/main/kotlin/de/davis/keygo/feature/item/view/data/WebsiteHandlerImpl.kt b/feature/item/view/src/main/kotlin/de/davis/keygo/feature/item/view/data/WebsiteHandlerImpl.kt index 17e6bd026..2c2db3280 100644 --- a/feature/item/view/src/main/kotlin/de/davis/keygo/feature/item/view/data/WebsiteHandlerImpl.kt +++ b/feature/item/view/src/main/kotlin/de/davis/keygo/feature/item/view/data/WebsiteHandlerImpl.kt @@ -2,27 +2,33 @@ package de.davis.keygo.feature.item.view.data import android.content.Context import android.content.Intent +import android.util.Log import androidx.core.net.toUri +import de.davis.keygo.core.security.domain.SystemHandoff +import de.davis.keygo.core.security.domain.forRoundTrip +import de.davis.keygo.core.util.onFailure import de.davis.keygo.feature.item.view.domain.WebsiteHandler import org.koin.core.annotation.Single @Single internal class WebsiteHandlerImpl( - private val context: Context + private val context: Context, + private val handoff: SystemHandoff, ) : WebsiteHandler { override fun openWebsite(url: String) { - Intent( - Intent.ACTION_VIEW, - url.ensureProtocol().toUri() - ).addFlags(Intent.FLAG_ACTIVITY_NEW_TASK).let { - runCatching { - context.startActivity(it) - } - } + val intent = Intent(Intent.ACTION_VIEW, url.ensureProtocol().toUri()) + .addFlags(Intent.FLAG_ACTIVITY_NEW_TASK) + + handoff.forRoundTrip { context.startActivity(intent) } + .onFailure { Log.w(TAG, "Failed to open a website", it) } } private fun String.ensureProtocol(): String = if (startsWith("http://") || startsWith("https://")) this else "https://$this" -} \ No newline at end of file + + private companion object { + private const val TAG = "WebsiteHandlerImpl" + } +} diff --git a/feature/onboarding/src/main/kotlin/de/davis/keygo/feature/onboarding/presentation/OnboardingScreen.kt b/feature/onboarding/src/main/kotlin/de/davis/keygo/feature/onboarding/presentation/OnboardingScreen.kt index 9e5c19c06..9903af7a9 100644 --- a/feature/onboarding/src/main/kotlin/de/davis/keygo/feature/onboarding/presentation/OnboardingScreen.kt +++ b/feature/onboarding/src/main/kotlin/de/davis/keygo/feature/onboarding/presentation/OnboardingScreen.kt @@ -5,7 +5,6 @@ import android.content.Intent import android.provider.Settings import android.util.Log import androidx.activity.compose.BackHandler -import androidx.activity.compose.rememberLauncherForActivityResult import androidx.activity.result.contract.ActivityResultContracts import androidx.compose.animation.AnimatedContent import androidx.compose.animation.AnimatedVisibility @@ -59,6 +58,7 @@ import androidx.lifecycle.compose.collectAsStateWithLifecycle import de.davis.keygo.core.security.domain.model.CryptographicMode import de.davis.keygo.core.security.domain.model.KeyId import de.davis.keygo.core.security.presentation.rememberBiometricCryptoController +import de.davis.keygo.core.security.presentation.rememberHandoffLauncher import de.davis.keygo.core.util.onFailure import de.davis.keygo.core.util.onSuccess import de.davis.keygo.core.util.presentation.ObserveAsEvents @@ -106,7 +106,7 @@ fun OnboardingScreen(route: OnboardingRoute, onSuccess: () -> Unit) { val context = LocalContext.current val autofillPickerLauncher = - rememberLauncherForActivityResult(ActivityResultContracts.StartActivityForResult()) {} + rememberHandoffLauncher(ActivityResultContracts.StartActivityForResult()) {} ObserveAsEvents(viewModel.autofillPickerFlow) { try { diff --git a/feature/settings/src/main/kotlin/de/davis/keygo/feature/settings/presentation/SettingsScreen.kt b/feature/settings/src/main/kotlin/de/davis/keygo/feature/settings/presentation/SettingsScreen.kt index a11c13481..4d895b4f0 100644 --- a/feature/settings/src/main/kotlin/de/davis/keygo/feature/settings/presentation/SettingsScreen.kt +++ b/feature/settings/src/main/kotlin/de/davis/keygo/feature/settings/presentation/SettingsScreen.kt @@ -2,7 +2,6 @@ package de.davis.keygo.feature.settings.presentation import android.content.Intent import android.provider.Settings -import androidx.activity.compose.rememberLauncherForActivityResult import androidx.activity.result.contract.ActivityResultContracts import androidx.compose.runtime.Composable import androidx.compose.runtime.getValue @@ -16,6 +15,7 @@ import de.davis.keygo.core.identity.presentation.rememberBiometricEnrollmentAdap import de.davis.keygo.core.identity.presentation.useEnrollmentAdapter import de.davis.keygo.core.security.domain.model.BiometricAuthError import de.davis.keygo.core.security.presentation.rememberBiometricCryptoController +import de.davis.keygo.core.security.presentation.rememberHandoffLauncher import de.davis.keygo.core.util.domain.model.snackbar.SnackbarMessage import de.davis.keygo.core.util.onFailure import de.davis.keygo.core.util.presentation.ObserveAsEvents @@ -37,7 +37,7 @@ fun SettingsScreen( val enrollmentAdapter = rememberBiometricEnrollmentAdapter() val enableAutofillLauncher = - rememberLauncherForActivityResult(ActivityResultContracts.StartActivityForResult()) {} + rememberHandoffLauncher(ActivityResultContracts.StartActivityForResult()) {} // OS-owned state (autofill / biometric availability) can change while the user is // in a system screen; re-read it whenever we come back to the foreground. diff --git a/feature/totp/src/main/kotlin/de/davis/keygo/feature/totp/presentation/component/QRScanner.kt b/feature/totp/src/main/kotlin/de/davis/keygo/feature/totp/presentation/component/QRScanner.kt index f9f869d43..26e8f23b6 100644 --- a/feature/totp/src/main/kotlin/de/davis/keygo/feature/totp/presentation/component/QRScanner.kt +++ b/feature/totp/src/main/kotlin/de/davis/keygo/feature/totp/presentation/component/QRScanner.kt @@ -45,6 +45,7 @@ import com.google.accompanist.permissions.ExperimentalPermissionsApi import com.google.accompanist.permissions.isGranted import com.google.accompanist.permissions.rememberPermissionState import com.google.accompanist.permissions.shouldShowRationale +import de.davis.keygo.core.security.presentation.rememberHandoffStarter import de.davis.keygo.feature.totp.R import de.davis.keygo.feature.totp.domain.model.camera.Frame import de.davis.keygo.feature.totp.domain.qr.QRScanner @@ -90,9 +91,10 @@ fun QRScanner( permissionRequested -> { // Permission was denied permanently (no rationale, not granted, already requested) val context = LocalContext.current + val openSystemScreen = rememberHandoffStarter() PermissionDeniedDialog( onOpenSettings = { - context.startActivity( + openSystemScreen.launch( Intent( Settings.ACTION_APPLICATION_DETAILS_SETTINGS, Uri.fromParts("package", context.packageName, null) From fd860efa6eead9fd97d74a4ff3e19374568789da Mon Sep 17 00:00:00 2001 From: Davis Wolfermann Date: Fri, 4 Sep 2026 17:55:45 +0200 Subject: [PATCH 18/35] feat(security): introduce lock info storage --- core/security/build.gradle.kts | 2 +- .../security/data/mapper/LockInfoMapper.kt | 17 +++++++++ .../data/repository/LockInfoRepositoryImpl.kt | 36 +++++++++++++++++++ .../core/security/di/CoreSecurityModule.kt | 22 +++++++++++- .../di/annotation/LockInfoQualifier.kt | 6 ++++ .../core/security/domain/model/LockInfo.kt | 13 +++++++ .../domain/repository/LockInfoRepository.kt | 11 ++++++ core/security/src/main/proto/lock_info.proto | 21 +++++++++++ 8 files changed, 126 insertions(+), 2 deletions(-) create mode 100644 core/security/src/main/kotlin/de/davis/keygo/core/security/data/mapper/LockInfoMapper.kt create mode 100644 core/security/src/main/kotlin/de/davis/keygo/core/security/data/repository/LockInfoRepositoryImpl.kt create mode 100644 core/security/src/main/kotlin/de/davis/keygo/core/security/di/annotation/LockInfoQualifier.kt create mode 100644 core/security/src/main/kotlin/de/davis/keygo/core/security/domain/model/LockInfo.kt create mode 100644 core/security/src/main/kotlin/de/davis/keygo/core/security/domain/repository/LockInfoRepository.kt create mode 100644 core/security/src/main/proto/lock_info.proto diff --git a/core/security/build.gradle.kts b/core/security/build.gradle.kts index 6c0db9e1b..56c6f1190 100644 --- a/core/security/build.gradle.kts +++ b/core/security/build.gradle.kts @@ -1,5 +1,6 @@ plugins { alias(libs.plugins.keygo.android.compose) + alias(libs.plugins.keygo.android.protobuf) } android { @@ -31,5 +32,4 @@ dependencies { testFixturesImplementation(libs.androidx.compose.runtime) { because("https://issuetracker.google.com/issues/259523353#comment32") } - } diff --git a/core/security/src/main/kotlin/de/davis/keygo/core/security/data/mapper/LockInfoMapper.kt b/core/security/src/main/kotlin/de/davis/keygo/core/security/data/mapper/LockInfoMapper.kt new file mode 100644 index 000000000..4fae8ed75 --- /dev/null +++ b/core/security/src/main/kotlin/de/davis/keygo/core/security/data/mapper/LockInfoMapper.kt @@ -0,0 +1,17 @@ +package de.davis.keygo.core.security.data.mapper + +import de.davis.keygo.core.security.data.local.model.ProtoLockInfo +import de.davis.keygo.core.security.domain.model.LockInfo + +internal fun ProtoLockInfo.toDomain() = LockInfo( + autoLockTimeout = autoLockTimeout.toDomain(), + backgroundedAt = backgroundedAt +) + +internal fun LockInfo.Timeout.toProto() = ProtoLockInfo.LockTimeout.entries[ordinal] + +/** + * Maps the [ProtoLockInfo.LockTimeout] to the corresponding [LockInfo.Timeout]. The entries + * must be exactly in the same order. + */ +private fun ProtoLockInfo.LockTimeout.toDomain() = LockInfo.Timeout.entries[ordinal] \ No newline at end of file diff --git a/core/security/src/main/kotlin/de/davis/keygo/core/security/data/repository/LockInfoRepositoryImpl.kt b/core/security/src/main/kotlin/de/davis/keygo/core/security/data/repository/LockInfoRepositoryImpl.kt new file mode 100644 index 000000000..82e092d9d --- /dev/null +++ b/core/security/src/main/kotlin/de/davis/keygo/core/security/data/repository/LockInfoRepositoryImpl.kt @@ -0,0 +1,36 @@ +package de.davis.keygo.core.security.data.repository + +import androidx.datastore.core.DataStore +import de.davis.keygo.core.security.data.local.model.ProtoLockInfo +import de.davis.keygo.core.security.data.mapper.toDomain +import de.davis.keygo.core.security.data.mapper.toProto +import de.davis.keygo.core.security.di.annotation.LockInfoQualifier +import de.davis.keygo.core.security.domain.model.LockInfo +import de.davis.keygo.core.security.domain.repository.LockInfoRepository +import kotlinx.coroutines.flow.first +import org.koin.core.annotation.Single + +@Single +internal class LockInfoRepositoryImpl( + @param:LockInfoQualifier + private val dataStore: DataStore +) : LockInfoRepository { + + override suspend fun setAutoLockTimeout(timeout: LockInfo.Timeout) { + dataStore.updateData { + it.toBuilder() + .setAutoLockTimeout(timeout.toProto()) + .build() + } + } + + override suspend fun setBackgroundedAt(backgroundedAt: Long) { + dataStore.updateData { + it.toBuilder() + .setBackgroundedAt(backgroundedAt) + .build() + } + } + + override suspend fun getLockInfo(): LockInfo = dataStore.data.first().toDomain() +} \ No newline at end of file diff --git a/core/security/src/main/kotlin/de/davis/keygo/core/security/di/CoreSecurityModule.kt b/core/security/src/main/kotlin/de/davis/keygo/core/security/di/CoreSecurityModule.kt index 7d45f9d76..cc432cb9d 100644 --- a/core/security/src/main/kotlin/de/davis/keygo/core/security/di/CoreSecurityModule.kt +++ b/core/security/src/main/kotlin/de/davis/keygo/core/security/di/CoreSecurityModule.kt @@ -1,10 +1,30 @@ package de.davis.keygo.core.security.di +import android.content.Context +import androidx.datastore.dataStore +import de.davis.keygo.core.security.data.local.model.ProtoLockInfo +import de.davis.keygo.core.security.di.annotation.LockInfoQualifier +import de.davis.keygo.core.util.data.serializer.DefaultProtoSerializer import org.koin.core.annotation.ComponentScan import org.koin.core.annotation.Configuration import org.koin.core.annotation.Module +import org.koin.core.annotation.Single @Module @Configuration @ComponentScan("de.davis.keygo.core.security") -object CoreSecurityModule \ No newline at end of file +object CoreSecurityModule { + + private val Context.protoLockInfoDataStore by dataStore( + "lock_info.pb", + DefaultProtoSerializer( + defaultInstance = ProtoLockInfo.getDefaultInstance(), + parser = ProtoLockInfo.parser() + ) + ) + + @Single + @LockInfoQualifier + internal fun provideLockInfoDataStore(context: Context) = + context.protoLockInfoDataStore +} \ No newline at end of file diff --git a/core/security/src/main/kotlin/de/davis/keygo/core/security/di/annotation/LockInfoQualifier.kt b/core/security/src/main/kotlin/de/davis/keygo/core/security/di/annotation/LockInfoQualifier.kt new file mode 100644 index 000000000..dccbad3c2 --- /dev/null +++ b/core/security/src/main/kotlin/de/davis/keygo/core/security/di/annotation/LockInfoQualifier.kt @@ -0,0 +1,6 @@ +package de.davis.keygo.core.security.di.annotation + +import org.koin.core.annotation.Named + +@Named +internal annotation class LockInfoQualifier diff --git a/core/security/src/main/kotlin/de/davis/keygo/core/security/domain/model/LockInfo.kt b/core/security/src/main/kotlin/de/davis/keygo/core/security/domain/model/LockInfo.kt new file mode 100644 index 000000000..c832c94a1 --- /dev/null +++ b/core/security/src/main/kotlin/de/davis/keygo/core/security/domain/model/LockInfo.kt @@ -0,0 +1,13 @@ +package de.davis.keygo.core.security.domain.model + +data class LockInfo( + val autoLockTimeout: Timeout, + val backgroundedAt: Long, +) { + enum class Timeout { + IMMEDIATELY, + ONE_MINUTE, + TWO_MINUTES, + FIVE_MINUTES, + } +} diff --git a/core/security/src/main/kotlin/de/davis/keygo/core/security/domain/repository/LockInfoRepository.kt b/core/security/src/main/kotlin/de/davis/keygo/core/security/domain/repository/LockInfoRepository.kt new file mode 100644 index 000000000..1421a7e58 --- /dev/null +++ b/core/security/src/main/kotlin/de/davis/keygo/core/security/domain/repository/LockInfoRepository.kt @@ -0,0 +1,11 @@ +package de.davis.keygo.core.security.domain.repository + +import de.davis.keygo.core.security.domain.model.LockInfo + +interface LockInfoRepository { + + suspend fun setAutoLockTimeout(timeout: LockInfo.Timeout) + suspend fun setBackgroundedAt(backgroundedAt: Long) + + suspend fun getLockInfo(): LockInfo +} \ No newline at end of file diff --git a/core/security/src/main/proto/lock_info.proto b/core/security/src/main/proto/lock_info.proto new file mode 100644 index 000000000..11927c170 --- /dev/null +++ b/core/security/src/main/proto/lock_info.proto @@ -0,0 +1,21 @@ +syntax = "proto3"; + +package de.davis.keygo.core.security.data.local.model; +option java_multiple_files = true; + +message ProtoLockInfo { + + /* + * The timeout for the auto-lock feature. The entries must be exactly the same as the values in + * the LockInfo.Timeout enum. The default value is LOCK_TIMEOUT_ONE_MINUTE. + */ + enum LockTimeout { + LOCK_TIMEOUT_IMMEDIATELY = 0; + LOCK_TIMEOUT_ONE_MINUTE = 1; + LOCK_TIMEOUT_TWO_MINUTES = 2; + LOCK_TIMEOUT_FIVE_MINUTES = 3; + } + + LockTimeout auto_lock_timeout = 1; + int64 backgrounded_at = 2; +} From c1f18d281db7ca1399c8f79000de2960df221814 Mon Sep 17 00:00:00 2001 From: Davis Wolfermann Date: Fri, 4 Sep 2026 18:02:01 +0200 Subject: [PATCH 19/35] feat(security): introduce elapsed time provider --- .../security/data/time/ElapsedTimeProviderImpl.kt | 11 +++++++++++ .../core/security/domain/time/ElapsedTimeProvider.kt | 6 ++++++ 2 files changed, 17 insertions(+) create mode 100644 core/security/src/main/kotlin/de/davis/keygo/core/security/data/time/ElapsedTimeProviderImpl.kt create mode 100644 core/security/src/main/kotlin/de/davis/keygo/core/security/domain/time/ElapsedTimeProvider.kt diff --git a/core/security/src/main/kotlin/de/davis/keygo/core/security/data/time/ElapsedTimeProviderImpl.kt b/core/security/src/main/kotlin/de/davis/keygo/core/security/data/time/ElapsedTimeProviderImpl.kt new file mode 100644 index 000000000..4d0a041be --- /dev/null +++ b/core/security/src/main/kotlin/de/davis/keygo/core/security/data/time/ElapsedTimeProviderImpl.kt @@ -0,0 +1,11 @@ +package de.davis.keygo.core.security.data.time + +import android.os.SystemClock +import de.davis.keygo.core.security.domain.time.ElapsedTimeProvider +import org.koin.core.annotation.Single + +@Single +internal class ElapsedTimeProviderImpl : ElapsedTimeProvider { + + override fun elapsedTime(): Long = SystemClock.elapsedRealtime() +} \ No newline at end of file diff --git a/core/security/src/main/kotlin/de/davis/keygo/core/security/domain/time/ElapsedTimeProvider.kt b/core/security/src/main/kotlin/de/davis/keygo/core/security/domain/time/ElapsedTimeProvider.kt new file mode 100644 index 000000000..84f470c9e --- /dev/null +++ b/core/security/src/main/kotlin/de/davis/keygo/core/security/domain/time/ElapsedTimeProvider.kt @@ -0,0 +1,6 @@ +package de.davis.keygo.core.security.domain.time + +interface ElapsedTimeProvider { + + fun elapsedTime(): Long +} \ No newline at end of file From 24aa8f85880c80497cc6a411684b49b9a787a933 Mon Sep 17 00:00:00 2001 From: Davis Wolfermann Date: Fri, 4 Sep 2026 18:06:30 +0200 Subject: [PATCH 20/35] feat(security): introduce RecordBackgroundedAtUseCase.kt --- .../usecase/RecordBackgroundedAtUseCase.kt | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) create mode 100644 core/security/src/main/kotlin/de/davis/keygo/core/security/domain/usecase/RecordBackgroundedAtUseCase.kt diff --git a/core/security/src/main/kotlin/de/davis/keygo/core/security/domain/usecase/RecordBackgroundedAtUseCase.kt b/core/security/src/main/kotlin/de/davis/keygo/core/security/domain/usecase/RecordBackgroundedAtUseCase.kt new file mode 100644 index 000000000..8d38dbe96 --- /dev/null +++ b/core/security/src/main/kotlin/de/davis/keygo/core/security/domain/usecase/RecordBackgroundedAtUseCase.kt @@ -0,0 +1,17 @@ +package de.davis.keygo.core.security.domain.usecase + +import de.davis.keygo.core.security.domain.repository.LockInfoRepository +import de.davis.keygo.core.security.domain.time.ElapsedTimeProvider +import org.koin.core.annotation.Single + +@Single +class RecordBackgroundedAtUseCase( + private val elapsedTimeProvider: ElapsedTimeProvider, + private val lockInfoRepository: LockInfoRepository, +) { + + suspend operator fun invoke() { + val currentTime = elapsedTimeProvider.elapsedTime() + lockInfoRepository.setBackgroundedAt(currentTime) + } +} \ No newline at end of file From 3b9427ed8a04cdada33ffc7da61c28ab7dca34ab Mon Sep 17 00:00:00 2001 From: Davis Wolfermann Date: Sat, 5 Sep 2026 17:50:00 +0200 Subject: [PATCH 21/35] feat: add auto lock picker --- .../data/repository/LockInfoRepositoryImpl.kt | 5 +- .../core/security/domain/model/LockInfo.kt | 14 +-- .../domain/repository/LockInfoRepository.kt | 3 +- .../core/security/FakeLockInfoRepository.kt | 28 ++++++ .../settings/presentation/SettingsContent.kt | 28 +++++- .../settings/presentation/SettingsUiEvent.kt | 3 + .../settings/presentation/SettingsUiState.kt | 3 + .../presentation/SettingsViewModel.kt | 13 ++- .../presentation/component/SettingsDsl.kt | 20 +++++ .../presentation/component/SettingsEntry.kt | 13 +++ .../presentation/component/SettingsList.kt | 90 +++++++++++++++++++ .../settings/src/main/res/values/strings.xml | 8 ++ .../presentation/SettingsViewModelTest.kt | 29 ++++++ .../presentation/component/SettingsDslTest.kt | 77 ++++++++++++++++ 14 files changed, 323 insertions(+), 11 deletions(-) create mode 100644 core/security/src/testFixtures/kotlin/de/davis/keygo/core/security/FakeLockInfoRepository.kt diff --git a/core/security/src/main/kotlin/de/davis/keygo/core/security/data/repository/LockInfoRepositoryImpl.kt b/core/security/src/main/kotlin/de/davis/keygo/core/security/data/repository/LockInfoRepositoryImpl.kt index 82e092d9d..2ce2b67b1 100644 --- a/core/security/src/main/kotlin/de/davis/keygo/core/security/data/repository/LockInfoRepositoryImpl.kt +++ b/core/security/src/main/kotlin/de/davis/keygo/core/security/data/repository/LockInfoRepositoryImpl.kt @@ -7,7 +7,8 @@ import de.davis.keygo.core.security.data.mapper.toProto import de.davis.keygo.core.security.di.annotation.LockInfoQualifier import de.davis.keygo.core.security.domain.model.LockInfo import de.davis.keygo.core.security.domain.repository.LockInfoRepository -import kotlinx.coroutines.flow.first +import kotlinx.coroutines.flow.Flow +import kotlinx.coroutines.flow.map import org.koin.core.annotation.Single @Single @@ -32,5 +33,5 @@ internal class LockInfoRepositoryImpl( } } - override suspend fun getLockInfo(): LockInfo = dataStore.data.first().toDomain() + override fun observeLockInfo(): Flow = dataStore.data.map(toDomain) } \ No newline at end of file diff --git a/core/security/src/main/kotlin/de/davis/keygo/core/security/domain/model/LockInfo.kt b/core/security/src/main/kotlin/de/davis/keygo/core/security/domain/model/LockInfo.kt index c832c94a1..74c88cece 100644 --- a/core/security/src/main/kotlin/de/davis/keygo/core/security/domain/model/LockInfo.kt +++ b/core/security/src/main/kotlin/de/davis/keygo/core/security/domain/model/LockInfo.kt @@ -1,13 +1,17 @@ package de.davis.keygo.core.security.domain.model +import kotlin.time.Duration +import kotlin.time.Duration.Companion.milliseconds +import kotlin.time.Duration.Companion.minutes + data class LockInfo( val autoLockTimeout: Timeout, val backgroundedAt: Long, ) { - enum class Timeout { - IMMEDIATELY, - ONE_MINUTE, - TWO_MINUTES, - FIVE_MINUTES, + enum class Timeout(val duration: Duration) { + IMMEDIATELY(0.milliseconds), + ONE_MINUTE(1.minutes), + TWO_MINUTES(2.minutes), + FIVE_MINUTES(5.minutes), } } diff --git a/core/security/src/main/kotlin/de/davis/keygo/core/security/domain/repository/LockInfoRepository.kt b/core/security/src/main/kotlin/de/davis/keygo/core/security/domain/repository/LockInfoRepository.kt index 1421a7e58..87ad2cdc8 100644 --- a/core/security/src/main/kotlin/de/davis/keygo/core/security/domain/repository/LockInfoRepository.kt +++ b/core/security/src/main/kotlin/de/davis/keygo/core/security/domain/repository/LockInfoRepository.kt @@ -1,11 +1,12 @@ package de.davis.keygo.core.security.domain.repository import de.davis.keygo.core.security.domain.model.LockInfo +import kotlinx.coroutines.flow.Flow interface LockInfoRepository { suspend fun setAutoLockTimeout(timeout: LockInfo.Timeout) suspend fun setBackgroundedAt(backgroundedAt: Long) - suspend fun getLockInfo(): LockInfo + fun observeLockInfo(): Flow } \ No newline at end of file diff --git a/core/security/src/testFixtures/kotlin/de/davis/keygo/core/security/FakeLockInfoRepository.kt b/core/security/src/testFixtures/kotlin/de/davis/keygo/core/security/FakeLockInfoRepository.kt new file mode 100644 index 000000000..0687dbf82 --- /dev/null +++ b/core/security/src/testFixtures/kotlin/de/davis/keygo/core/security/FakeLockInfoRepository.kt @@ -0,0 +1,28 @@ +package de.davis.keygo.core.security + +import de.davis.keygo.core.security.domain.model.LockInfo +import de.davis.keygo.core.security.domain.repository.LockInfoRepository +import kotlinx.coroutines.flow.Flow +import kotlinx.coroutines.flow.MutableStateFlow +import kotlinx.coroutines.flow.asStateFlow +import kotlinx.coroutines.flow.update + +class FakeLockInfoRepository( + initLockInfo: LockInfo = LockInfo( + autoLockTimeout = LockInfo.Timeout.IMMEDIATELY, + backgroundedAt = 0L, + ), +) : LockInfoRepository { + + private val lockInfo = MutableStateFlow(initLockInfo) + + override suspend fun setAutoLockTimeout(timeout: LockInfo.Timeout) { + lockInfo.update { it.copy(autoLockTimeout = timeout) } + } + + override suspend fun setBackgroundedAt(backgroundedAt: Long) { + lockInfo.update { it.copy(backgroundedAt = backgroundedAt) } + } + + override fun observeLockInfo(): Flow = lockInfo.asStateFlow() +} diff --git a/feature/settings/src/main/kotlin/de/davis/keygo/feature/settings/presentation/SettingsContent.kt b/feature/settings/src/main/kotlin/de/davis/keygo/feature/settings/presentation/SettingsContent.kt index 472f265a9..3925f2311 100644 --- a/feature/settings/src/main/kotlin/de/davis/keygo/feature/settings/presentation/SettingsContent.kt +++ b/feature/settings/src/main/kotlin/de/davis/keygo/feature/settings/presentation/SettingsContent.kt @@ -9,6 +9,7 @@ import androidx.compose.material.icons.filled.Backup import androidx.compose.material.icons.filled.BugReport import androidx.compose.material.icons.filled.Code import androidx.compose.material.icons.filled.Fingerprint +import androidx.compose.material.icons.filled.Lock import androidx.compose.material.icons.filled.LockReset import androidx.compose.material.icons.filled.Password import androidx.compose.material.icons.filled.Public @@ -22,7 +23,9 @@ import androidx.compose.runtime.Composable import androidx.compose.ui.Modifier import androidx.compose.ui.tooling.preview.Preview import androidx.compose.ui.unit.dp +import de.davis.keygo.core.security.domain.model.LockInfo import de.davis.keygo.core.util.presentation.UIText +import de.davis.keygo.core.util.presentation.UIText.Companion.PluralsString import de.davis.keygo.core.util.presentation.UIText.Companion.ResourceString import de.davis.keygo.feature.settings.R import de.davis.keygo.feature.settings.presentation.component.SectionScope @@ -82,6 +85,16 @@ internal fun SettingsContent( ) autofillEntries(state, warningColors, onEvent) + + picker( + title = R.string.settings_auto_lock, + icon = Icons.Default.Lock, + colors = defaultColors, + selected = state.lockTimeout, + options = LockInfo.Timeout.entries, + label = { it.label }, + onSelect = { onEvent(SettingsUiEvent.SetAutoLockTimeout(it)) }, + ) } section(title = R.string.settings_backup) { @@ -154,6 +167,16 @@ private fun lastBackupText(lastBackupAt: Long?): UIText = when (lastBackupAt) { ) } +private val LockInfo.Timeout.label: UIText + get() = when (this) { + LockInfo.Timeout.IMMEDIATELY -> ResourceString(R.string.settings_auto_lock_immediately) + else -> PluralsString( + R.plurals.settings_auto_lock_n_minute, + duration.inWholeMinutes.toInt(), + duration.inWholeMinutes, + ) + } + @Preview @Composable private fun SettingsContentPreview() { @@ -162,7 +185,10 @@ private fun SettingsContentPreview() { modifier = Modifier.fillMaxSize(), ) { SettingsContent( - state = SettingsUiState(autofillEnabled = true), + state = SettingsUiState( + autofillEnabled = true, + lockTimeout = LockInfo.Timeout.TWO_MINUTES + ), onEvent = {}, ) } diff --git a/feature/settings/src/main/kotlin/de/davis/keygo/feature/settings/presentation/SettingsUiEvent.kt b/feature/settings/src/main/kotlin/de/davis/keygo/feature/settings/presentation/SettingsUiEvent.kt index 21eb2a21c..4e0e23e12 100644 --- a/feature/settings/src/main/kotlin/de/davis/keygo/feature/settings/presentation/SettingsUiEvent.kt +++ b/feature/settings/src/main/kotlin/de/davis/keygo/feature/settings/presentation/SettingsUiEvent.kt @@ -1,7 +1,10 @@ package de.davis.keygo.feature.settings.presentation +import de.davis.keygo.core.security.domain.model.LockInfo + internal sealed interface SettingsUiEvent { data class SetBiometrics(val enabled: Boolean) : SettingsUiEvent + data class SetAutoLockTimeout(val timeout: LockInfo.Timeout) : SettingsUiEvent data class SetAutofill(val enabledRequest: Boolean) : SettingsUiEvent data object OpenChromeAutofillSettings : SettingsUiEvent data object ResetPassword : SettingsUiEvent diff --git a/feature/settings/src/main/kotlin/de/davis/keygo/feature/settings/presentation/SettingsUiState.kt b/feature/settings/src/main/kotlin/de/davis/keygo/feature/settings/presentation/SettingsUiState.kt index 628517600..ec73c5965 100644 --- a/feature/settings/src/main/kotlin/de/davis/keygo/feature/settings/presentation/SettingsUiState.kt +++ b/feature/settings/src/main/kotlin/de/davis/keygo/feature/settings/presentation/SettingsUiState.kt @@ -1,5 +1,7 @@ package de.davis.keygo.feature.settings.presentation +import de.davis.keygo.core.security.domain.model.LockInfo + internal data class SettingsUiState( val autofillEnabled: Boolean = false, val chromeAutofillEnabled: Boolean = false, @@ -8,4 +10,5 @@ internal data class SettingsUiState( val version: String = "2.0.0", /** When the newest successful backup finished, or `null` while none has. */ val lastBackupAt: Long? = null, + val lockTimeout: LockInfo.Timeout = LockInfo.Timeout.IMMEDIATELY, ) diff --git a/feature/settings/src/main/kotlin/de/davis/keygo/feature/settings/presentation/SettingsViewModel.kt b/feature/settings/src/main/kotlin/de/davis/keygo/feature/settings/presentation/SettingsViewModel.kt index 42f128d0e..d2bff1d7d 100644 --- a/feature/settings/src/main/kotlin/de/davis/keygo/feature/settings/presentation/SettingsViewModel.kt +++ b/feature/settings/src/main/kotlin/de/davis/keygo/feature/settings/presentation/SettingsViewModel.kt @@ -4,6 +4,8 @@ import androidx.lifecycle.ViewModel import androidx.lifecycle.viewModelScope import de.davis.keygo.core.identity.domain.repository.AccountRepository import de.davis.keygo.core.security.domain.repository.BiometricAvailabilityRepository +import de.davis.keygo.core.security.domain.repository.LockInfoRepository +import de.davis.keygo.core.util.combine import de.davis.keygo.feature.autofill.domain.repository.AutofillServiceRepository import de.davis.keygo.feature.autofill.domain.repository.ChromeAutofillRepository import de.davis.keygo.feature.backup.domain.usecase.ObserveLastBackupUseCase @@ -11,7 +13,6 @@ import de.davis.keygo.feature.settings.domain.repository.AppVersionRepository import kotlinx.coroutines.channels.Channel import kotlinx.coroutines.flow.MutableStateFlow import kotlinx.coroutines.flow.SharingStarted -import kotlinx.coroutines.flow.combine import kotlinx.coroutines.flow.receiveAsFlow import kotlinx.coroutines.flow.stateIn import kotlinx.coroutines.flow.update @@ -23,6 +24,7 @@ internal class SettingsViewModel( private val biometricAvailabilityRepository: BiometricAvailabilityRepository, private val autofillServiceRepository: AutofillServiceRepository, private val chromeAutofillRepository: ChromeAutofillRepository, + private val lockInfoRepository: LockInfoRepository, accountRepository: AccountRepository, appVersionRepository: AppVersionRepository, observeLastBackup: ObserveLastBackupUseCase, @@ -45,11 +47,12 @@ internal class SettingsViewModel( val state = combine( accountRepository.observe(), + lockInfoRepository.observeLockInfo(), autofillEnabled, chromeAutofillEnabled, biometricsAvailable, observeLastBackup(), - ) { account, autofill, chromeAutofill, biometrics, lastBackup -> + ) { account, lockInfo, autofill, chromeAutofill, biometrics, lastBackup -> SettingsUiState( autofillEnabled = autofill, chromeAutofillEnabled = chromeAutofill, @@ -57,6 +60,7 @@ internal class SettingsViewModel( biometricsEnabled = biometrics && account?.biometricWrappedArk != null, version = versionName, lastBackupAt = lastBackup?.finishedAt, + lockTimeout = lockInfo.autoLockTimeout, ) }.stateIn( scope = viewModelScope, @@ -77,6 +81,11 @@ internal class SettingsViewModel( fun onEvent(event: SettingsUiEvent) { when (event) { is SettingsUiEvent.SetBiometrics -> _event.trySend(SettingsEvent.EnableBiometric(event.enabled)) + + is SettingsUiEvent.SetAutoLockTimeout -> viewModelScope.launch { + lockInfoRepository.setAutoLockTimeout(event.timeout) + } + is SettingsUiEvent.SetAutofill -> when { event.enabledRequest -> _event.trySend(SettingsEvent.OpenAutofillSelection) else -> { diff --git a/feature/settings/src/main/kotlin/de/davis/keygo/feature/settings/presentation/component/SettingsDsl.kt b/feature/settings/src/main/kotlin/de/davis/keygo/feature/settings/presentation/component/SettingsDsl.kt index 6029084b2..3a889ce50 100644 --- a/feature/settings/src/main/kotlin/de/davis/keygo/feature/settings/presentation/component/SettingsDsl.kt +++ b/feature/settings/src/main/kotlin/de/davis/keygo/feature/settings/presentation/component/SettingsDsl.kt @@ -77,5 +77,25 @@ internal class SectionScope { ) } + fun picker( + @StringRes title: Int, + selected: T, + options: List, + label: (T) -> UIText, + onSelect: (T) -> Unit, + colors: ListItemColors, + icon: ImageVector? = null, + ) { + entries += SettingsEntry.Picker( + title = title, + icon = icon, + colors = colors, + selectedIndex = options.indexOf(selected), + options = options.map { option -> + SettingsEntry.Picker.Option(label(option)) { onSelect(option) } + }, + ) + } + fun build(): List = entries.toList() } diff --git a/feature/settings/src/main/kotlin/de/davis/keygo/feature/settings/presentation/component/SettingsEntry.kt b/feature/settings/src/main/kotlin/de/davis/keygo/feature/settings/presentation/component/SettingsEntry.kt index f6828c026..f4f5e733f 100644 --- a/feature/settings/src/main/kotlin/de/davis/keygo/feature/settings/presentation/component/SettingsEntry.kt +++ b/feature/settings/src/main/kotlin/de/davis/keygo/feature/settings/presentation/component/SettingsEntry.kt @@ -41,6 +41,19 @@ internal sealed interface SettingsEntry { val value: String, val onClick: (() -> Unit)? = null, ) : SettingsEntry + + data class Picker( + @param:StringRes override val title: Int, + override val icon: ImageVector? = null, + override val colors: ListItemColors, + val selectedIndex: Int, + val options: List