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..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 @@ -23,10 +23,8 @@ 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. + * 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 @@ -35,4 +33,4 @@ internal class AppViewModel( _isReturningUser.update { accountRepository.getOrNull() != null || hasV1Password() } } } -} \ No newline at end of file +} 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..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 @@ -89,7 +89,11 @@ 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, + isSessionActive: Boolean, +) { val navigationState = rememberAppNavigationState( launchRoute = launchRoute, startRoute = RouteDestination.Home, @@ -97,7 +101,7 @@ private fun App(hasAccess: Boolean, launchRoute: NavKey, isSessionActive: Boolea ) val navigator = remember(navigationState) { AppNavigator(navigationState) } - RedirectToAuthWhenSessionEnds(isSessionActive, navigator) + LockAppWhenSessionEnds(isSessionActive, navigator) val windowAdaptiveInfo = currentWindowAdaptiveInfoV2() val directive = remember(windowAdaptiveInfo) { @@ -140,25 +144,17 @@ private fun App(hasAccess: Boolean, launchRoute: NavKey, isSessionActive: Boolea } /** - * 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. + * 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 RedirectToAuthWhenSessionEnds( - isSessionActive: Boolean, - navigator: AppNavigator, -) { - val isLaunching = navigator.state.isLaunching - LaunchedEffect(isSessionActive, isLaunching) { - if (!isSessionActive && !isLaunching) navigator.replaceLaunchFlow(AuthRoute()) +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/AppNavigationState.kt b/app/src/main/kotlin/de/davis/keygo/app/presentation/navigation/AppNavigationState.kt index 5bea0a802..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 @@ -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. */ @@ -38,12 +38,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, ) @@ -53,13 +53,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>, ) { @@ -67,20 +67,19 @@ 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. * - * 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() @@ -93,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 e9186609b..67d1210d8 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,15 +3,46 @@ 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 +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 * 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) { - fun navigate(route: NavKey) { - val isTopLevel = !state.isLaunching && route in state.backStacks + /** + * 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.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 + } + + /** The one choke point every gated mutator below shares, so none can forget the check. */ + private inline fun whenUnlocked(action: () -> Unit) { + if (!isGated) action() + } + + fun navigate(route: NavKey) = whenUnlocked { + val isTopLevel = !state.isOverlaid && route in state.backStacks if (isTopLevel) selectTopLevel(route) else state.currentStack.add(route) } @@ -26,28 +57,59 @@ 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. Deliberately not gated, + * unlike [pushOntoOverlay]: it clears whatever is on top first - including a gate - rather + * than stacking above it, so it also doubles as how a gate is legitimately swapped for another + * (see `AppNavigator.openGateFor`). + */ + 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. 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() = whenUnlocked { state.overlayStack.clear() } + + /** + * Adds [route] to the overlay without disturbing whatever is already on it. Gated, unlike + * [replaceOverlay]: stacking on top of an existing gate would show [route] unauthenticated + * above it. + */ + fun pushOntoOverlay(route: NavKey) = whenUnlocked { state.overlayStack.add(route) } + + /** + * 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 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 (runsWithoutSession) return + pushOntoOverlay(AuthRoute()) + } + + /** Lifts the gate. Only gates reach here; first run is taken down with [clearOverlay]. */ + fun unlock() { + if (!isGated) return + state.overlayStack.removeLastOrNull() } /** * Shows [detail] in the dashboard's detail pane, replacing any detail already open, so back * from a detail always lands on the list. */ - fun showDetail(detail: RouteDestination.Detail) { + fun showDetail(detail: RouteDestination.Detail) = whenUnlocked { closeDetail() state.currentStack.add(detail) } /** Opens [detail] on top of the detail already showing, so back returns to it. */ - fun openOnTopOfDetail(detail: RouteDestination.Detail) { + fun openOnTopOfDetail(detail: RouteDestination.Detail) = whenUnlocked { state.currentStack.add(detail) } @@ -60,17 +122,23 @@ 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() } /** - * 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, 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() { + fun goBack() = whenUnlocked { val stack = state.currentStack if (stack.size > 1) stack.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 64fa44276..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 @@ -63,7 +63,7 @@ fun keyGoEntryProvider(navigator: AppNavigator, hasAccess: Boolean): (NavKey) -> assignTotpEntries( metadata = WindowOwning, - onImportFinished = { navigator.finishLaunchFlow() }, + onImportFinished = { navigator.clearOverlay() }, navigateUp = { navigator.goBack() }, ) @@ -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) @@ -132,12 +132,24 @@ 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)) +} + +/** 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) } -private fun AppNavigator.finishUnlock(totpUri: String?) { - if (totpUri == null) finishLaunchFlow() - else replaceLaunchFlow(SelectItemForTotpRoute(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 9c704d80e..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 @@ -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 @@ -20,7 +21,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 +29,22 @@ 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`() { - val navigator = navigator() + fun `the overlay owns the window until it is cleared`() { + // Cleared with a first run overlay: a gate refuses clearOverlay and is popped by unlock. + val navigator = navigator(launchRoute = OnboardingRoute()) - assertTrue(navigator.state.isLaunching) - assertEquals(listOf(AuthRoute()), navigator.shown) + assertTrue(navigator.state.isOverlaid) + assertEquals(listOf(OnboardingRoute()), navigator.shown) - navigator.finishLaunchFlow() + navigator.clearOverlay() - assertFalse(navigator.state.isLaunching) + assertFalse(navigator.state.isOverlaid) assertEquals(listOf(RouteDestination.Home), navigator.shown) } @@ -68,16 +70,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,23 +89,26 @@ 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`() { - val navigator = navigator() + fun `a top level route is not switched to while the overlay owns the window`() { + val navigator = navigator(launchRoute = TotpImportRedirect(DEEP_LINK_URI)) navigator.navigate(SettingsRoute) - assertTrue(navigator.state.isLaunching) - assertEquals(listOf(AuthRoute(), SettingsRoute), navigator.shown) + assertTrue(navigator.state.isOverlaid) + assertEquals( + listOf(TotpImportRedirect(DEEP_LINK_URI), SettingsRoute), + navigator.shown, + ) } // ---- top level routes ---- @@ -286,7 +291,282 @@ class AppNavigatorTest { assertEquals(listOf(RouteDestination.Home), navigator.shown) } - private fun unlocked(): AppNavigator = navigator().apply { finishLaunchFlow() } + // ---- locking ---- + + @Test + fun `locking leaves every tab exactly as it was`() { + val navigator = unlocked() + navigator.navigate(SettingsRoute) + navigator.navigate(ChangePasswordRoute) + navigator.navigate(RouteDestination.Home) + val itemId = newItemId() + navigator.showDetail(RouteDestination.ViewItem(itemId)) + + navigator.lock() + + assertEquals( + listOf(RouteDestination.Home, RouteDestination.ViewItem(itemId)), + navigator.state.backStacks.getValue(RouteDestination.Home).toList(), + ) + assertEquals( + listOf(SettingsRoute, ChangePasswordRoute), + navigator.state.backStacks.getValue(SettingsRoute).toList(), + ) + } + + @Test + fun `locking pushes the gate without clearing what was already on the overlay`() { + val navigator = navigator() + navigator.replaceOverlay(SelectItemForTotpRoute(DEEP_LINK_URI)) + + navigator.lock() + + assertEquals( + listOf(SelectItemForTotpRoute(DEEP_LINK_URI), AuthRoute()), + navigator.shown, + ) + } + + @Test + fun `locking from a tab pushes the gate onto an otherwise empty overlay`() { + val navigator = unlocked() + navigator.navigate(SettingsRoute) + + navigator.lock() + + assertTrue(navigator.state.isOverlaid) + 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() + navigator.navigate(SettingsRoute) + navigator.navigate(ChangePasswordRoute) + navigator.lock() + + navigator.unlock() + + 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.replaceOverlay(SelectItemForTotpRoute(DEEP_LINK_URI)) + navigator.lock() + + 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.replaceOverlay(SelectItemForTotpRoute(DEEP_LINK_URI)) + navigator.lock() + + navigator.goBack() + + assertEquals( + listOf(SelectItemForTotpRoute(DEEP_LINK_URI), AuthRoute()), + navigator.shown, + ) + } + + @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() + navigator.navigate(SettingsRoute) + navigator.navigate(ChangePasswordRoute) + navigator.lock() + navigator.unlock() + + navigator.goBack() + + assertEquals(listOf(SettingsRoute), navigator.shown) + } + + @Test + fun `unlocking a cold start gate hands the window to the app proper`() { + // 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() + + assertFalse(navigator.state.isOverlaid) + 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( + overlayStack = NavBackStack(AuthRoute()), + topLevelRoute = mutableStateOf(RouteDestination.Home), + backStacks = TOP_LEVEL_ROUTES.associateWith { NavBackStack(it) }, + ) + val navigator = AppNavigator(state) + + navigator.lock() + + 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( + overlayStack = NavBackStack(SelectItemForTotpRoute(DEEP_LINK_URI), AuthRoute()), + topLevelRoute = mutableStateOf(RouteDestination.Home), + backStacks = TOP_LEVEL_ROUTES.associateWith { NavBackStack(it) }, + ) + val navigator = AppNavigator(state) + + navigator.goBack() + + assertEquals( + listOf(SelectItemForTotpRoute(DEEP_LINK_URI), AuthRoute()), + navigator.shown, + ) + } + + @Test + 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( + overlayStack = 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 { unlock() } private companion object { val TOP_LEVEL_ROUTES: Set = linkedSetOf( 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/build.gradle.kts b/core/security/build.gradle.kts index 8f2ecd7a9..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 { @@ -12,11 +13,13 @@ android { dependencies { implementation(libs.androidx.biometric) + implementation(libs.androidx.lifecycle.process) implementation(projects.core.item) api(projects.core.util) api(projects.rust) + testImplementation(libs.robolectric) testImplementation(testFixtures(projects.rust)) testImplementation(testFixtures(projects.core.item)) testImplementation(testFixtures(projects.core.util)) @@ -29,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/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/SessionLockObserver.kt b/core/security/src/main/kotlin/de/davis/keygo/core/security/data/SessionLockObserver.kt new file mode 100644 index 000000000..84a1ee63f --- /dev/null +++ b/core/security/src/main/kotlin/de/davis/keygo/core/security/data/SessionLockObserver.kt @@ -0,0 +1,134 @@ +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 de.davis.keygo.core.security.domain.model.LockInfo +import de.davis.keygo.core.security.domain.repository.LockInfoRepository +import de.davis.keygo.core.security.domain.time.SessionClock +import de.davis.keygo.core.util.di.annotation.AppScopeQualifier +import kotlinx.coroutines.CoroutineScope +import kotlinx.coroutines.Job +import kotlinx.coroutines.delay +import kotlinx.coroutines.flow.SharingStarted +import kotlinx.coroutines.flow.map +import kotlinx.coroutines.flow.stateIn +import kotlinx.coroutines.launch +import org.koin.core.annotation.Single + +@Single(createdAtStart = true) +internal class SessionLockObserver( + private val context: Context, + private val session: Session, + private val handoff: SystemHandoff, + private val sessionClock: SessionClock, + @param:AppScopeQualifier private val scope: CoroutineScope, + lockInfoRepository: LockInfoRepository, +) : DefaultLifecycleObserver { + + private val screenOffReceiver = object : BroadcastReceiver() { + override fun onReceive(context: Context, intent: Intent) = endSession() + } + + private var watchingScreenOff = false + private var lockJob: Job? = null + + private var backgroundWindow = LockInfo.Timeout.IMMEDIATELY + + private val lockGuard = Any() + + private val autoLockTimeout = lockInfoRepository.observeLockInfo() + .map { it.autoLockTimeout } + .stateIn(scope, SharingStarted.Eagerly, LockInfo.Timeout.IMMEDIATELY) + + init { + ProcessLifecycleOwner.get().lifecycle.addObserver(this) + } + + override fun onStart(owner: LifecycleOwner) { + stopWatchingScreenOff() + handoff.clear() + lockJob?.cancel() + + synchronized(lockGuard) { + // Read before markActive drops the stamp this depends on. This is the authority, not + // the scheduled wipe: a frozen or dozing process can hold that timer past its delay, + // and only the stamp survives being frozen. + if (sessionClock.expired(backgroundWindow)) session.endSession() + sessionClock.markActive() + } + } + + override fun onStop(owner: LifecycleOwner) { + val window = if (handoff.isPending) HANDOFF_GRACE else autoLockTimeout.value + + // Spend it here, not only on the way back. A launch the user backed straight out of never + // stopped us - ProcessLifecycleOwner cancels the debounced ON_STOP without dispatching + // ON_START either - so the arming is left behind with no return to clear it. Consuming it + // on the background it is read for keeps that residue to one, rather than arming every + // later background until the app is next brought forward. + handoff.clear() + + synchronized(lockGuard) { + backgroundWindow = window + sessionClock.markInactive() + } + + if (window == LockInfo.Timeout.IMMEDIATELY) session.endSession() + else { + // A locked screen is the user leaving, whatever window they were granted. + watchScreenOff() + scheduleWipe(window) + } + } + + private fun scheduleWipe(window: LockInfo.Timeout) { + lockJob?.cancel() + lockJob = scope.launch { + // A frozen or dozing process can hold this past its delay, which is why the check in + // [onStart] stays the authority. This only ever locks earlier, never later. + delay(window.duration) + + synchronized(lockGuard) { + // Re-read rather than trusting the cancel alone: a foreground that raced the delay + // has already dropped the stamp, and must not be locked out by a timer it just beat. + if (sessionClock.expired(window)) session.endSession() + } + } + } + + private fun endSession() { + lockJob?.cancel() + 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 + } + + private companion object { + private val HANDOFF_GRACE = LockInfo.Timeout.FIVE_MINUTES + } +} 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/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/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..9f7d9604a --- /dev/null +++ b/core/security/src/main/kotlin/de/davis/keygo/core/security/data/mapper/LockInfoMapper.kt @@ -0,0 +1,23 @@ +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(), +) + +internal fun LockInfo.Timeout.toProto() = when (this) { + LockInfo.Timeout.IMMEDIATELY -> ProtoLockInfo.LockTimeout.LOCK_TIMEOUT_IMMEDIATELY + LockInfo.Timeout.ONE_MINUTE -> ProtoLockInfo.LockTimeout.LOCK_TIMEOUT_ONE_MINUTE + LockInfo.Timeout.TWO_MINUTES -> ProtoLockInfo.LockTimeout.LOCK_TIMEOUT_TWO_MINUTES + LockInfo.Timeout.FIVE_MINUTES -> ProtoLockInfo.LockTimeout.LOCK_TIMEOUT_FIVE_MINUTES +} + +private fun ProtoLockInfo.LockTimeout.toDomain() = when (this) { + ProtoLockInfo.LockTimeout.LOCK_TIMEOUT_IMMEDIATELY -> LockInfo.Timeout.IMMEDIATELY + ProtoLockInfo.LockTimeout.LOCK_TIMEOUT_ONE_MINUTE -> LockInfo.Timeout.ONE_MINUTE + ProtoLockInfo.LockTimeout.LOCK_TIMEOUT_TWO_MINUTES -> LockInfo.Timeout.TWO_MINUTES + ProtoLockInfo.LockTimeout.LOCK_TIMEOUT_FIVE_MINUTES -> LockInfo.Timeout.FIVE_MINUTES + ProtoLockInfo.LockTimeout.UNRECOGNIZED -> LockInfo.Timeout.IMMEDIATELY +} 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..3f03f2e2e --- /dev/null +++ b/core/security/src/main/kotlin/de/davis/keygo/core/security/data/repository/LockInfoRepositoryImpl.kt @@ -0,0 +1,29 @@ +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.Flow +import kotlinx.coroutines.flow.map +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 fun observeLockInfo(): Flow = dataStore.data.map(ProtoLockInfo::toDomain) +} 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..e2f56b6f6 --- /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() +} diff --git a/core/security/src/main/kotlin/de/davis/keygo/core/security/data/time/SessionClockImpl.kt b/core/security/src/main/kotlin/de/davis/keygo/core/security/data/time/SessionClockImpl.kt new file mode 100644 index 000000000..74be364d3 --- /dev/null +++ b/core/security/src/main/kotlin/de/davis/keygo/core/security/data/time/SessionClockImpl.kt @@ -0,0 +1,30 @@ +package de.davis.keygo.core.security.data.time + +import de.davis.keygo.core.security.domain.model.LockInfo +import de.davis.keygo.core.security.domain.time.ElapsedTimeProvider +import de.davis.keygo.core.security.domain.time.SessionClock +import org.koin.core.annotation.Single + +@Single +internal class SessionClockImpl( + private val timeProvider: ElapsedTimeProvider, +) : SessionClock { + + // Written on the main thread from the process lifecycle callbacks, read from there and from the + // scheduled wipe on the app scope's dispatcher. + @Volatile + private var backgroundedAt: Long? = null + + override fun markActive() { + backgroundedAt = null + } + + override fun markInactive() { + backgroundedAt = timeProvider.elapsedTime() + } + + override fun expired(timeout: LockInfo.Timeout): Boolean { + val since = backgroundedAt ?: return false + return timeProvider.elapsedTime() - since >= timeout.duration.inWholeMilliseconds + } +} 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..449e6058d 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 +} 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/ArkHolder.kt b/core/security/src/main/kotlin/de/davis/keygo/core/security/domain/ArkHolder.kt new file mode 100644 index 000000000..783ba4135 --- /dev/null +++ b/core/security/src/main/kotlin/de/davis/keygo/core/security/domain/ArkHolder.kt @@ -0,0 +1,44 @@ +package de.davis.keygo.core.security.domain + +class ArkHolder { + + private val lock = Any() + + private class Generation(val ark: ByteArray) { + var readers = 0 + var wipe = false + } + + private var current: Generation? = null + + suspend fun withArk(block: suspend (ByteArray) -> R): R? { + val generation = synchronized(lock) { + val gen = current ?: return null + gen.readers++ + gen + } + + try { + return block(generation.ark) + } finally { + synchronized(lock) { + generation.readers-- + if (generation.readers == 0 && generation.wipe) generation.ark.fill(0) + } + } + } + + fun set(ark: ByteArray) = replace(ark) + + fun clear() = replace(null) + + private fun replace(next: ByteArray?) { + synchronized(lock) { + current?.let { retiring -> + retiring.wipe = true + if (retiring.readers == 0) retiring.ark.fill(0) + } + current = next?.let { Generation(it) } + } + } +} 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/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..17c4bbfe3 --- /dev/null +++ b/core/security/src/main/kotlin/de/davis/keygo/core/security/domain/SystemHandoff.kt @@ -0,0 +1,27 @@ +package de.davis.keygo.core.security.domain + +import de.davis.keygo.core.util.Result + +interface SystemHandoff { + + val isPending: Boolean + + fun expectReturn() + fun returned() + fun clear() +} + +/** + * [open] failing to launch the system screen (no activity resolves the intent, say) is expected + * and foreseeable, not exceptional, so it is reported as a [Result.Failure] rather than thrown. + */ +inline fun SystemHandoff.forRoundTrip(open: () -> Unit): Result { + expectReturn() + return try { + open() + Result.Success(Unit) + } catch (e: Throwable) { + returned() + Result.Failure(e) + } +} 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..53628d887 --- /dev/null +++ b/core/security/src/main/kotlin/de/davis/keygo/core/security/domain/model/LockInfo.kt @@ -0,0 +1,16 @@ +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, +) { + 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 new file mode 100644 index 000000000..899cc6cda --- /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 +import kotlinx.coroutines.flow.Flow + +interface LockInfoRepository { + + suspend fun setAutoLockTimeout(timeout: LockInfo.Timeout) + + fun observeLockInfo(): Flow +} 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..547ede7ca --- /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 +} diff --git a/core/security/src/main/kotlin/de/davis/keygo/core/security/domain/time/SessionClock.kt b/core/security/src/main/kotlin/de/davis/keygo/core/security/domain/time/SessionClock.kt new file mode 100644 index 000000000..3dc8dc39f --- /dev/null +++ b/core/security/src/main/kotlin/de/davis/keygo/core/security/domain/time/SessionClock.kt @@ -0,0 +1,11 @@ +package de.davis.keygo.core.security.domain.time + +import de.davis.keygo.core.security.domain.model.LockInfo + +interface SessionClock { + + fun markActive() + fun markInactive() + + fun expired(timeout: LockInfo.Timeout): Boolean +} 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..66ee31f5a --- /dev/null +++ b/core/security/src/main/kotlin/de/davis/keygo/core/security/presentation/HandoffLauncher.kt @@ -0,0 +1,40 @@ +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 de.davis.keygo.core.util.Result +import org.koin.compose.koinInject + +class HandoffLauncher( + private val handoff: SystemHandoff, + private val onLaunch: (I) -> Unit, +) { + + fun launch(input: I): Result = 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/main/proto/lock_info.proto b/core/security/src/main/proto/lock_info.proto new file mode 100644 index 000000000..f4c159f23 --- /dev/null +++ b/core/security/src/main/proto/lock_info.proto @@ -0,0 +1,20 @@ +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_IMMEDIATELY. + */ + 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; +} 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..587d92c88 --- /dev/null +++ b/core/security/src/test/kotlin/de/davis/keygo/core/security/data/SessionArkLifetimeTest.kt @@ -0,0 +1,140 @@ +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 still-held ark is wiped once its own last reader finishes, not blocked by a newer generation's readers`() = + runTest { + // The defect this test guards: a reader count shared across generations meant an + // overlapping reader on a newer ark could keep an older, already-replaced one resident + // well past when its own last reader was done with it. + val first = generateArk() + session.startSession(first) + + val heldFirst = holdArk() + session.endSession() + session.startSession(generateArk()) + + val heldSecond = holdArk() + heldFirst.finish() + + assertTrue( + first.all { it == 0.toByte() }, + "old generation not wiped once its own last reader finished" + ) + heldSecond.finish() + } + + @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..2d7977c26 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,14 @@ package de.davis.keygo.core.security.data +import kotlinx.coroutines.ExperimentalCoroutinesApi +import kotlinx.coroutines.launch +import kotlinx.coroutines.test.UnconfinedTestDispatcher +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 +17,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,32 +40,58 @@ 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) + } + + @OptIn(ExperimentalCoroutinesApi::class) + @Test + fun `startSession does not pulse isActive when replacing an already-active session`() = + runTest(UnconfinedTestDispatcher()) { + // A swap is not a lock. The app gate locks on any false it observes and only a + // successful unlock takes it back down, so a pulse here would make replacing a live + // session cost a re-auth. Unconfined so a collector that could see the edge does. + session.startSession(generateArk()) + + val collected = mutableListOf() + val job = launch { session.isActive.collect { collected.add(it) } } + + session.startSession(generateArk()) + + job.cancel() + assertEquals(listOf(true), collected) + } + @Test fun `endSession is safe to call without active session`() { session.endSession() // should not throw 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..1ad7a1f11 --- /dev/null +++ b/core/security/src/test/kotlin/de/davis/keygo/core/security/data/SessionLockObserverTest.kt @@ -0,0 +1,309 @@ +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 de.davis.keygo.core.security.FakeLockInfoRepository +import de.davis.keygo.core.security.data.time.SessionClockImpl +import de.davis.keygo.core.security.domain.model.LockInfo +import de.davis.keygo.core.security.domain.repository.LockInfoRepository +import de.davis.keygo.core.security.time.FakeElapsedTimeProvider +import kotlinx.coroutines.ExperimentalCoroutinesApi +import kotlinx.coroutines.flow.Flow +import kotlinx.coroutines.flow.MutableSharedFlow +import kotlinx.coroutines.test.TestScope +import kotlinx.coroutines.test.UnconfinedTestDispatcher +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 time = FakeElapsedTimeProvider() + private val handoff = SystemHandoffImpl() + private val clock = SessionClockImpl(time) + private val lockInfoRepository = FakeLockInfoRepository() + + @OptIn(ExperimentalCoroutinesApi::class) + private val scope = TestScope(UnconfinedTestDispatcher()) + private val owner = StubLifecycleOwner() + + private val fiveMinutes = LockInfo.Timeout.FIVE_MINUTES.duration.inWholeMilliseconds + + private fun observer( + timeout: LockInfo.Timeout = LockInfo.Timeout.IMMEDIATELY, + ): SessionLockObserver { + lockInfoRepository.lockInfo = LockInfo(autoLockTimeout = timeout) + return SessionLockObserver(context, session, handoff, clock, scope, lockInfoRepository) + } + + /** + * Moves the session clock and virtual time together, so a scheduled wipe both fires and sees + * the time it was waiting for. Tests that advance `time` alone are checking the onStart path + * in isolation, as it behaves when a frozen process held the timer past its delay. + */ + @OptIn(ExperimentalCoroutinesApi::class) + private fun elapse(millis: Long) { + time.advanceBy(millis) + scope.testScheduler.advanceTimeBy(millis) + scope.testScheduler.runCurrent() + } + + private fun screenOff() { + context.sendBroadcast(Intent(Intent.ACTION_SCREEN_OFF)) + shadowOf(Looper.getMainLooper()).idle() + } + + @Test + fun `backgrounding ends the session`() { + val observer = observer() + + observer.onStop(owner) + + assertEquals(false, session.isActive.value) + } + + @Test + fun `backgrounding for a system screen we launched keeps the session`() { + val observer = observer() + handoff.expectReturn() + + observer.onStop(owner) + + assertEquals(true, session.isActive.value) + } + + @Test + fun `a handoff covers one round trip, not the background after it`() { + val observer = observer() + 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`() { + val observer = observer() + handoff.expectReturn() + observer.onStop(owner) + + screenOff() + + assertEquals(false, session.isActive.value) + } + + @Test + fun `the screen going off during an ordinary background ends the session`() { + val observer = observer(LockInfo.Timeout.FIVE_MINUTES) + observer.onStop(owner) + + screenOff() + + assertEquals(false, session.isActive.value) + } + + @Test + fun `the screen stops being watched once the app is back in the foreground`() { + val observer = observer() + handoff.expectReturn() + observer.onStop(owner) + observer.onStart(owner) + + screenOff() + + assertEquals(true, session.isActive.value) + } + + @Test + fun `backgrounding under a timeout keeps the session`() { + val observer = observer(LockInfo.Timeout.FIVE_MINUTES) + + observer.onStop(owner) + + assertEquals(true, session.isActive.value) + } + + @Test + fun `returning within the timeout keeps the session`() { + val observer = observer(LockInfo.Timeout.FIVE_MINUTES) + observer.onStop(owner) + + time.advanceBy(fiveMinutes - 1) + observer.onStart(owner) + + assertEquals(true, session.isActive.value) + } + + @Test + fun `returning after the timeout ends the session`() { + val observer = observer(LockInfo.Timeout.FIVE_MINUTES) + observer.onStop(owner) + + time.advanceBy(fiveMinutes) + observer.onStart(owner) + + assertEquals(false, session.isActive.value) + } + + @Test + fun `a handoff is timed against its grace period, not the auto lock timeout`() { + val observer = observer(LockInfo.Timeout.ONE_MINUTE) + handoff.expectReturn() + observer.onStop(owner) + + time.advanceBy(LockInfo.Timeout.ONE_MINUTE.duration.inWholeMilliseconds) + observer.onStart(owner) + + assertEquals(true, session.isActive.value) + } + + @Test + fun `a handoff that outlasts its grace period ends the session on return`() { + // The frozen process case: the scheduled wipe never got to run, so the stamp taken on the + // way out is all that is left to judge the return by. Without it the ARK stays resident for + // however long the user was gone. + val observer = observer(LockInfo.Timeout.FIVE_MINUTES) + handoff.expectReturn() + observer.onStop(owner) + + time.advanceBy(fiveMinutes * 2) + observer.onStart(owner) + + assertEquals(false, session.isActive.value) + } + + @Test + fun `a handoff arming outlives at most one background`() { + // Backing straight out of the screen we opened happens inside the ON_STOP debounce, so no + // lifecycle callback runs to spend the arming and it is still there at the next background. + // That one gets the grace; every background after it must not. + val observer = observer() + handoff.expectReturn() + + observer.onStop(owner) + observer.onStop(owner) + + assertEquals(false, session.isActive.value) + } + + @Test + fun `a backgrounded session is wiped once the timeout passes`() { + val observer = observer(LockInfo.Timeout.FIVE_MINUTES) + + observer.onStop(owner) + elapse(fiveMinutes) + + assertEquals(false, session.isActive.value) + } + + @Test + fun `the wipe does not fire before the timeout`() { + val observer = observer(LockInfo.Timeout.FIVE_MINUTES) + + observer.onStop(owner) + elapse(fiveMinutes - 1) + + assertEquals(true, session.isActive.value) + } + + @Test + fun `returning to the foreground cancels the pending wipe`() { + val observer = observer(LockInfo.Timeout.FIVE_MINUTES) + observer.onStop(owner) + elapse(fiveMinutes - 1) + + observer.onStart(owner) + elapse(fiveMinutes) + + assertEquals(true, session.isActive.value) + } + + @Test + fun `a handoff schedules no wipe against the configured timeout`() { + val observer = observer(LockInfo.Timeout.FIVE_MINUTES) + handoff.expectReturn() + + observer.onStop(owner) + elapse(fiveMinutes - 1) + + assertEquals(true, session.isActive.value) + } + + @Test + fun `an abandoned handoff still ends the session once the grace period passes`() { + // The defect this guards: a handoff that is never returned from (the user never comes + // back, the screen never turns off) must not hold auto-lock open forever. + val observer = observer() + handoff.expectReturn() + + observer.onStop(owner) + elapse(LockInfo.Timeout.FIVE_MINUTES.duration.inWholeMilliseconds) + + assertEquals(false, session.isActive.value) + } + + @Test + fun `a handoff still within its grace period keeps the session`() { + val observer = observer() + handoff.expectReturn() + + observer.onStop(owner) + elapse(LockInfo.Timeout.FIVE_MINUTES.duration.inWholeMilliseconds - 1) + + assertEquals(true, session.isActive.value) + } + + @Test + fun `a new session is not judged by the stamp of the one before it`() { + val observer = observer(LockInfo.Timeout.FIVE_MINUTES) + observer.onStop(owner) + time.advanceBy(fiveMinutes * 2) + observer.onStart(owner) + + session.startSession(ByteArray(32) { it.toByte() }) + observer.onStart(owner) + + assertEquals(true, session.isActive.value) + } + + @Test + fun `a setting that has not been read yet locks rather than lingers`() { + // The seed stands in until the first read lands. It has to be the locking one: a timeout + // we do not know yet must not be read as permission to leave the ARK in memory. + val observer = SessionLockObserver( + context, + session, + handoff, + clock, + scope, + NeverEmittingLockInfoRepository, + ) + + observer.onStop(owner) + + assertEquals(false, session.isActive.value) + } +} + +private object NeverEmittingLockInfoRepository : LockInfoRepository { + override suspend fun setAutoLockTimeout(timeout: LockInfo.Timeout) = Unit + override fun observeLockInfo(): Flow = MutableSharedFlow() +} + +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/data/mapper/LockInfoMapperTest.kt b/core/security/src/test/kotlin/de/davis/keygo/core/security/data/mapper/LockInfoMapperTest.kt new file mode 100644 index 000000000..fd0392b98 --- /dev/null +++ b/core/security/src/test/kotlin/de/davis/keygo/core/security/data/mapper/LockInfoMapperTest.kt @@ -0,0 +1,37 @@ +package de.davis.keygo.core.security.data.mapper + +import de.davis.keygo.core.security.data.local.model.ProtoLockInfo +import de.davis.keygo.core.security.data.local.model.copy +import de.davis.keygo.core.security.domain.model.LockInfo +import kotlin.test.Test +import kotlin.test.assertEquals + +internal class LockInfoMapperTest { + + @Test + fun `every domain timeout round trips through proto and back`() { + LockInfo.Timeout.entries.forEach { timeout -> + val proto = + ProtoLockInfo.getDefaultInstance().copy { autoLockTimeout = timeout.toProto() } + + assertEquals(timeout, proto.toDomain().autoLockTimeout) + } + } + + @Test + fun `an unrecognized proto value falls back to IMMEDIATELY instead of crashing`() { + // A raw ordinal no build-time LockTimeout entry claims - stands in for a value a newer + // app version wrote, read back after a downgrade. setAutoLockTimeout(UNRECOGNIZED) itself + // throws, so the raw *Value setter is the only way to construct this on purpose. + val proto = ProtoLockInfo.getDefaultInstance().copy { autoLockTimeoutValue = 99 } + + assertEquals(LockInfo.Timeout.IMMEDIATELY, proto.toDomain().autoLockTimeout) + } + + @Test + fun `an unset proto field - the proto3 wire default - maps to IMMEDIATELY`() { + val proto = ProtoLockInfo.getDefaultInstance() + + assertEquals(LockInfo.Timeout.IMMEDIATELY, proto.toDomain().autoLockTimeout) + } +} diff --git a/core/security/src/test/kotlin/de/davis/keygo/core/security/data/time/SessionClockImplTest.kt b/core/security/src/test/kotlin/de/davis/keygo/core/security/data/time/SessionClockImplTest.kt new file mode 100644 index 000000000..c25af5687 --- /dev/null +++ b/core/security/src/test/kotlin/de/davis/keygo/core/security/data/time/SessionClockImplTest.kt @@ -0,0 +1,86 @@ +package de.davis.keygo.core.security.data.time + +import de.davis.keygo.core.security.domain.model.LockInfo +import de.davis.keygo.core.security.time.FakeElapsedTimeProvider +import kotlin.test.Test +import kotlin.test.assertFalse +import kotlin.test.assertTrue + +internal class SessionClockImplTest { + + private val time = FakeElapsedTimeProvider() + private val clock = SessionClockImpl(time) + + private val oneMinute = LockInfo.Timeout.ONE_MINUTE.duration.inWholeMilliseconds + + @Test + fun `a session that was never backgrounded does not expire`() { + time.advanceBy(oneMinute * 10) + + assertFalse(clock.expired(LockInfo.Timeout.ONE_MINUTE)) + } + + @Test + fun `a session backgrounded for less than the timeout has not expired`() { + clock.markInactive() + + time.advanceBy(oneMinute - 1) + + assertFalse(clock.expired(LockInfo.Timeout.ONE_MINUTE)) + } + + @Test + fun `a session backgrounded for exactly the timeout has expired`() { + clock.markInactive() + + time.advanceBy(oneMinute) + + assertTrue(clock.expired(LockInfo.Timeout.ONE_MINUTE)) + } + + @Test + fun `a session backgrounded for longer than the timeout has expired`() { + clock.markInactive() + + time.advanceBy(oneMinute * 5) + + assertTrue(clock.expired(LockInfo.Timeout.ONE_MINUTE)) + } + + @Test + fun `IMMEDIATELY expires the moment the session goes inactive`() { + clock.markInactive() + + assertTrue(clock.expired(LockInfo.Timeout.IMMEDIATELY)) + } + + @Test + fun `the stamp is taken when the session goes inactive, not when it is read`() { + time.advanceBy(oneMinute * 10) + clock.markInactive() + + assertFalse(clock.expired(LockInfo.Timeout.ONE_MINUTE)) + } + + @Test + fun `returning to the foreground drops a stamp that had already expired`() { + clock.markInactive() + time.advanceBy(oneMinute * 5) + + clock.markActive() + + assertFalse(clock.expired(LockInfo.Timeout.ONE_MINUTE)) + } + + @Test + fun `each background is timed from its own stamp`() { + clock.markInactive() + time.advanceBy(oneMinute * 5) + clock.markActive() + + clock.markInactive() + time.advanceBy(oneMinute - 1) + + assertFalse(clock.expired(LockInfo.Timeout.ONE_MINUTE)) + } +} 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..bd58d25af --- /dev/null +++ b/core/security/src/test/kotlin/de/davis/keygo/core/security/domain/SystemHandoffTest.kt @@ -0,0 +1,48 @@ +package de.davis.keygo.core.security.domain + +import de.davis.keygo.core.security.data.SystemHandoffImpl +import de.davis.keygo.core.util.Result +import de.davis.keygo.core.util.getOrNull +import de.davis.keygo.core.util.isFailure +import kotlin.test.Test +import kotlin.test.assertEquals +import kotlin.test.assertFalse +import kotlin.test.assertNotNull +import kotlin.test.assertNull +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`() { + val result = handoff.forRoundTrip { error("nothing resolves this intent") } + + assertTrue(result.isFailure()) + assertFalse(handoff.isPending) + } + + @Test + fun `a system screen that will not open still reports the failure to the caller`() { + val result = handoff.forRoundTrip { error("nothing resolves this intent") } + + assertTrue(result.isFailure()) + assertEquals("nothing resolves this intent", (result as Result.Failure).error.message) + assertNull(result.getOrNull()) + } + + @Test + fun `a round trip that opens successfully reports success`() { + val result = handoff.forRoundTrip { } + + assertNotNull(result.getOrNull()) + } +} 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..dc542e540 --- /dev/null +++ b/core/security/src/test/kotlin/de/davis/keygo/core/security/presentation/HandoffLauncherTest.kt @@ -0,0 +1,52 @@ +package de.davis.keygo.core.security.presentation + +import de.davis.keygo.core.security.data.SystemHandoffImpl +import de.davis.keygo.core.util.Result +import de.davis.keygo.core.util.isFailure +import kotlin.test.Test +import kotlin.test.assertEquals +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") } + + 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 result = launcher.launch(Unit) + + assertTrue(result.isFailure()) + assertEquals("nothing resolves this intent", (result as Result.Failure).error.message) + } +} 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..c0f9df508 --- /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), +) : LockInfoRepository { + + private val _lockInfo = MutableStateFlow(initLockInfo) + + /** The stored record, settable so a test can arrange one without going through the setters. */ + var lockInfo: LockInfo + get() = _lockInfo.value + set(value) { + _lockInfo.update { value } + } + + override suspend fun setAutoLockTimeout(timeout: LockInfo.Timeout) { + _lockInfo.update { it.copy(autoLockTimeout = timeout) } + } + + override fun observeLockInfo(): Flow = _lockInfo.asStateFlow() +} 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..aa5671cca 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,54 @@ 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 +import kotlinx.coroutines.runBlocking /** - * 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 val _isActive = MutableStateFlow(false) - override val ark: ByteArray? - get() = _ark + /** + * The live ARK as a copy, for assertions. Null once the session has ended. Goes through + * [ArkHolder.withArk] like any other reader - `runBlocking` only bridges the suspend call for + * a synchronous test property, it does not bypass the reader accounting the way a raw peek + * would. + */ + val currentArk: ByteArray? + get() = runBlocking { holder.withArk { it.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 + holder.set(ark) _isActive.value = true startSessionCalled = true } override fun endSession() { - _ark = null + holder.clear() _isActive.value = false } -} \ No newline at end of file +} diff --git a/core/security/src/testFixtures/kotlin/de/davis/keygo/core/security/time/FakeElapsedTimeProvider.kt b/core/security/src/testFixtures/kotlin/de/davis/keygo/core/security/time/FakeElapsedTimeProvider.kt new file mode 100644 index 000000000..8d50c29c4 --- /dev/null +++ b/core/security/src/testFixtures/kotlin/de/davis/keygo/core/security/time/FakeElapsedTimeProvider.kt @@ -0,0 +1,12 @@ +package de.davis.keygo.core.security.time + +import de.davis.keygo.core.security.domain.time.ElapsedTimeProvider + +class FakeElapsedTimeProvider(var now: Long = 0L) : ElapsedTimeProvider { + + override fun elapsedTime(): Long = now + + fun advanceBy(millis: Long) { + now += millis + } +} diff --git a/core/util/src/main/kotlin/de/davis/keygo/core/util/di/CoreUtilModule.kt b/core/util/src/main/kotlin/de/davis/keygo/core/util/di/CoreUtilModule.kt index fda8daade..3c71cd167 100644 --- a/core/util/src/main/kotlin/de/davis/keygo/core/util/di/CoreUtilModule.kt +++ b/core/util/src/main/kotlin/de/davis/keygo/core/util/di/CoreUtilModule.kt @@ -1,10 +1,27 @@ package de.davis.keygo.core.util.di +import de.davis.keygo.core.util.di.annotation.AppScopeQualifier +import kotlinx.coroutines.CoroutineScope +import kotlinx.coroutines.Dispatchers +import kotlinx.coroutines.SupervisorJob 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.util") -object CoreUtilModule \ No newline at end of file +object CoreUtilModule { + + /** + * Lives as long as the process, for work that has to outlive whatever started it. A + * [SupervisorJob] so one failed child cannot take the rest down with it. + * + * Callers that need a different dispatcher pass one to their own launch. This is not a home for + * work scoped to a screen or a framework callback: those belong to a scope that dies with them. + */ + @Single + @AppScopeQualifier + fun provideAppScope(): CoroutineScope = CoroutineScope(SupervisorJob() + Dispatchers.Default) +} diff --git a/core/util/src/main/kotlin/de/davis/keygo/core/util/di/annotation/AppScopeQualifier.kt b/core/util/src/main/kotlin/de/davis/keygo/core/util/di/annotation/AppScopeQualifier.kt new file mode 100644 index 000000000..7695bb448 --- /dev/null +++ b/core/util/src/main/kotlin/de/davis/keygo/core/util/di/annotation/AppScopeQualifier.kt @@ -0,0 +1,6 @@ +package de.davis.keygo.core.util.di.annotation + +import org.koin.core.annotation.Named + +@Named +annotation class AppScopeQualifier 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..5d0dd2904 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,9 @@ 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.core.util.onFailure import de.davis.keygo.feature.autofill.domain.repository.ChromeAutofillRepository import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.withContext @@ -14,6 +17,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 +64,8 @@ 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) } + .onFailure { Log.w(TAG, "Failed to open Chrome's autofill settings", it) } } 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..da5b535ce 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 @@ -4,8 +4,8 @@ import android.content.Context import android.content.Intent import android.os.Bundle import android.service.autofill.Dataset +import android.util.Log 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 +20,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 @@ -39,6 +40,7 @@ import de.davis.keygo.feature.item.create.presentation.password.GeneratePassword import org.koin.androidx.compose.koinViewModel import de.davis.keygo.core.item.R as CoreItemR +private const val TAG = "AutofillActivity" /** * This activity is transparent and does not show up in the recent apps list. It is used to gather @@ -69,7 +71,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( @@ -95,7 +97,9 @@ internal class AutofillActivity : FragmentActivity() { is AutofillEvent.RequestSmsConsent -> smsConsentLauncher.launch( IntentSenderRequest.Builder(event.intentSender).build(), - ) + ).onFailure { + Log.w(TAG, "Failed to launch the SMS consent prompt", it) + } } } 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/BackupEscrowReconciler.kt b/feature/backup/src/main/kotlin/de/davis/keygo/feature/backup/domain/BackupEscrowReconciler.kt index c3cfc2d47..34e2a3439 100644 --- a/feature/backup/src/main/kotlin/de/davis/keygo/feature/backup/domain/BackupEscrowReconciler.kt +++ b/feature/backup/src/main/kotlin/de/davis/keygo/feature/backup/domain/BackupEscrowReconciler.kt @@ -1,9 +1,9 @@ package de.davis.keygo.feature.backup.domain +import de.davis.keygo.core.util.di.annotation.AppScopeQualifier import de.davis.keygo.feature.backup.domain.usecase.CleanupBackupResourcesUseCase import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.Dispatchers -import kotlinx.coroutines.SupervisorJob import kotlinx.coroutines.launch import org.koin.core.annotation.Single @@ -24,10 +24,13 @@ import org.koin.core.annotation.Single @Single(createdAtStart = true) internal class BackupEscrowReconciler( cleanupBackupResources: CleanupBackupResourcesUseCase, + @AppScopeQualifier appScope: CoroutineScope, ) { init { - CoroutineScope(SupervisorJob() + Dispatchers.IO).launch { + // Dispatchers.IO rather than the app scope's default: reconcile reads DataStore and the + // WorkManager schedule. + appScope.launch(Dispatchers.IO) { cleanupBackupResources.reconcile() } } 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..e9cb6b424 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,11 @@ 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.mapSuccess import de.davis.keygo.core.util.onFailure import de.davis.keygo.core.util.resultBinding import de.davis.keygo.feature.backup.domain.BackupDestinationResolver @@ -108,15 +111,17 @@ class FinishExportWizardUseCase( } private suspend fun provisionBackupArk() = resultBinding { - val ark = session.ark - .asResult(FinishExportWizardError.CryptoFailed).bind() + val escrowed = session.withArkOr(FinishExportWizardError.CryptoFailed) { ark -> + val cipher = keyStoreManager.getOrCreateCipherFor( + keyId = KeyId.BackupArkKey, + cryptographicMode = CryptographicMode.Encrypt, + ) - val cipher = keyStoreManager.getOrCreateCipherFor( - keyId = KeyId.BackupArkKey, - cryptographicMode = CryptographicMode.Encrypt, - ) + cipher.suspendDoFinal(ark) + .mapSuccess { CryptographicData(it, cipher.iv) } + .mapFailure { FinishExportWizardError.CryptoFailed } + }.bind() - val data = cipher.suspendDoFinal(ark).bind { FinishExportWizardError.CryptoFailed } - arkKeyStore.save(CryptographicData(data, cipher.iv)) + arkKeyStore.save(escrowed) } } 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/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..a0b204bf2 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,21 +1,25 @@ package de.davis.keygo.feature.backup.presentation.export -import androidx.activity.compose.rememberLauncherForActivityResult +import android.util.Log 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.onFailure 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 import org.koin.androidx.compose.koinViewModel +private const val TAG = "ExportWizardScreen" + @Composable 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()) }) @@ -24,7 +28,9 @@ fun ExportWizardScreen(navigateUp: () -> Unit) { ObserveAsEvents(flow = viewModel.event) { when (it) { ExportWizardEvent.Finished -> navigateUp() - ExportWizardEvent.PickFolder -> folderPicker.launch(null) + ExportWizardEvent.PickFolder -> folderPicker.launch(null).onFailure { + Log.w(TAG, "No activity found to handle the folder picker", it) + } } } 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..48a4d274e 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,12 +1,16 @@ package de.davis.keygo.feature.backup.presentation.import -import androidx.activity.compose.rememberLauncherForActivityResult +import android.util.Log 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.core.util.onFailure import de.davis.keygo.feature.backup.domain.model.BackupDestinationUri import de.davis.keygo.feature.backup.domain.model.FileFormat +private const val TAG = "ImportFilePicker" + private val ImportFileMimeTypes = (FileFormat.entries.map { it.mimeType } + "*/*").toTypedArray() fun interface FilePickerAction { @@ -15,10 +19,16 @@ 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())) } } - return remember(launcher) { FilePickerAction { launcher.launch(ImportFileMimeTypes) } } + return remember(launcher) { + FilePickerAction { + launcher.launch(ImportFileMimeTypes).onFailure { + Log.w(TAG, "No activity found to handle the import file picker", it) + } + } + } } 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/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..14fb2270f 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 @@ -2,6 +2,7 @@ package de.davis.keygo.feature.credit_card.presentation import android.content.Intent import android.provider.Settings +import android.util.Log import androidx.compose.animation.AnimatedContent import androidx.compose.animation.core.animateFloatAsState import androidx.compose.animation.fadeIn @@ -35,7 +36,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,11 +43,14 @@ 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.core.util.onFailure 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 import java.time.YearMonth +private const val TAG = "NfcInfoCard" private const val DescriptionLines = 2 private val IndicatorSize = 56.dp @@ -81,8 +84,13 @@ 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: () -> Unit = { + openSystemScreen.launch(Intent(Settings.ACTION_NFC_SETTINGS)).onFailure { + // A few OEM builds have nothing that resolves NFC settings. + Log.w(TAG, "No activity found to handle NFC settings", it) + } + } // 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/item/view/src/main/kotlin/de/davis/keygo/feature/item/view/domain/WebsiteHandler.kt b/feature/item/view/src/main/kotlin/de/davis/keygo/feature/item/view/domain/WebsiteHandler.kt index 396e86b17..c01b29c62 100644 --- a/feature/item/view/src/main/kotlin/de/davis/keygo/feature/item/view/domain/WebsiteHandler.kt +++ b/feature/item/view/src/main/kotlin/de/davis/keygo/feature/item/view/domain/WebsiteHandler.kt @@ -3,4 +3,4 @@ package de.davis.keygo.feature.item.view.domain interface WebsiteHandler { fun openWebsite(url: String) -} \ No newline at end of file +} 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..c02605748 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 @@ -1,11 +1,9 @@ package de.davis.keygo.feature.onboarding.presentation -import android.content.ActivityNotFoundException 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 +57,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,20 +105,18 @@ fun OnboardingScreen(route: OnboardingRoute, onSuccess: () -> Unit) { val context = LocalContext.current val autofillPickerLauncher = - rememberLauncherForActivityResult(ActivityResultContracts.StartActivityForResult()) {} + rememberHandoffLauncher(ActivityResultContracts.StartActivityForResult()) {} ObserveAsEvents(viewModel.autofillPickerFlow) { - try { - autofillPickerLauncher.launch( - Intent(Settings.ACTION_REQUEST_SET_AUTOFILL_SERVICE).apply { - data = "package:${context.packageName}".toUri() - } - ) - } catch (e: ActivityNotFoundException) { + autofillPickerLauncher.launch( + Intent(Settings.ACTION_REQUEST_SET_AUTOFILL_SERVICE).apply { + data = "package:${context.packageName}".toUri() + } + ).onFailure { // Some AOSP builds, Android TV, and a few OEM ROMs have nothing that resolves this // intent. The user still has the "Finish setup" button to move past the step, so // failing quietly here is acceptable as long as it stays diagnosable. - Log.w(TAG, "No activity found to handle the system autofill picker", e) + Log.w(TAG, "No activity found to handle the system autofill picker", it) } } 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..a941e8d4c 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/SettingsScreen.kt b/feature/settings/src/main/kotlin/de/davis/keygo/feature/settings/presentation/SettingsScreen.kt index a11c13481..d754035b6 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,7 @@ package de.davis.keygo.feature.settings.presentation import android.content.Intent import android.provider.Settings -import androidx.activity.compose.rememberLauncherForActivityResult +import android.util.Log import androidx.activity.result.contract.ActivityResultContracts import androidx.compose.runtime.Composable import androidx.compose.runtime.getValue @@ -16,6 +16,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 @@ -24,6 +25,8 @@ import de.davis.keygo.core.util.presentation.snackbar.LocalSnackbarManager import de.davis.keygo.feature.settings.R import org.koin.androidx.compose.koinViewModel +private const val TAG = "SettingsScreen" + @Composable fun SettingsScreen( showLibraries: () -> Unit, @@ -37,7 +40,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. @@ -60,7 +63,9 @@ fun SettingsScreen( Intent(Settings.ACTION_REQUEST_SET_AUTOFILL_SERVICE).apply { data = "package:${context.packageName}".toUri() } - ) + ).onFailure { + Log.w(TAG, "No activity found to handle the autofill selection", it) + } } is SettingsEvent.EnableBiometric -> { 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/changepassword/ChangePasswordViewModel.kt b/feature/settings/src/main/kotlin/de/davis/keygo/feature/settings/presentation/changepassword/ChangePasswordViewModel.kt index b5f7ce2ce..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,5 +1,7 @@ package de.davis.keygo.feature.settings.presentation.changepassword +import androidx.compose.foundation.ExperimentalFoundationApi +import androidx.compose.foundation.text.input.delete import androidx.compose.runtime.snapshotFlow import androidx.lifecycle.ViewModel import androidx.lifecycle.viewModelScope @@ -9,6 +11,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 @@ -24,6 +27,7 @@ import kotlinx.coroutines.flow.SharingStarted 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,23 @@ internal class ChangePasswordViewModel( else -> _event.trySend(ChangePasswordEvent.GenericError) } } + + @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() + + _state.update { + it.copy( + currentPasswordError = null, + newPasswordError = null, + confirmPasswordError = null, + showReauthDialog = false, + ) + } + } } 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