From 935599c60186ad9fa7d7475d06adcfd8c22e8171 Mon Sep 17 00:00:00 2001 From: Davis Wolfermann Date: Sun, 23 Aug 2026 01:42:37 +0200 Subject: [PATCH 01/20] feat(totp): let a scanned code choose its item A deep-linked otpauth:// URI offered an item picker only when the code's issuer matched a stored login by registrable domain, and dropped into a blank form otherwise. The picker is now unconditional and mirrors the passkey flow: the whole item list with its own search, plus a FAB out to a new item. Domain matches become a suggested group at the top of that list rather than a gate. HeaderContent gains a Suggested variant so they group the way pinned items already do, and ItemListScreen takes the ids through a new suggestedItemIds parameter that defaults to empty, leaving the dashboard list unchanged. Suggestions stand down while a search query is active. Matching now runs through resolveTotpDomain instead of the issuer alone, so a code carrying no issuer matches on the domain in its account name. The removed dialog's copy promised that; the code never did it. It also keeps the suggestions consistent with the domain a new item is prefilled with. Ids the list does not hold are ignored rather than fetched: a blank-query list is scoped to the selected vault while getLoginsByTLD searches all of them, and splicing in another vault's item would misrepresent what the list is showing. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_019NwwBkR3ks2QMJSRmP1iDH --- .../core/ui/components/KeyGoLazyColumn.kt | 9 + feature/item/create/build.gradle.kts | 7 + .../SelectItemForTotpModificationDialog.kt | 162 ---------- .../component/SelectItemForTotpScreen.kt | 77 +++++ .../create/presentation/login/LoginContent.kt | 25 +- .../presentation/login/LoginViewModel.kt | 44 ++- .../presentation/login/model/DialogState.kt | 2 - .../presentation/login/model/LoginUiState.kt | 3 + .../create/src/main/res/values/strings.xml | 7 +- .../presentation/login/LoginViewModelTest.kt | 289 ++++++++++++++++++ .../presentation/ItemListScreen.kt | 12 +- .../presentation/SuggestedItems.kt | 11 + .../components/ItemListContent.kt | 11 +- .../presentation/SuggestedItemsTest.kt | 80 +++++ 14 files changed, 536 insertions(+), 203 deletions(-) delete mode 100644 feature/item/create/src/main/kotlin/de/davis/keygo/feature/item/create/presentation/component/SelectItemForTotpModificationDialog.kt create mode 100644 feature/item/create/src/main/kotlin/de/davis/keygo/feature/item/create/presentation/component/SelectItemForTotpScreen.kt create mode 100644 feature/item/create/src/test/java/de/davis/keygo/feature/item/create/presentation/login/LoginViewModelTest.kt create mode 100644 feature/list_screen/src/main/kotlin/de/davis/keygo/feature/list_screen/presentation/SuggestedItems.kt create mode 100644 feature/list_screen/src/test/kotlin/de/davis/keygo/feature/list_screen/presentation/SuggestedItemsTest.kt diff --git a/core/ui/src/main/kotlin/de/davis/keygo/core/ui/components/KeyGoLazyColumn.kt b/core/ui/src/main/kotlin/de/davis/keygo/core/ui/components/KeyGoLazyColumn.kt index 5cc9f5a53..1c3299df1 100644 --- a/core/ui/src/main/kotlin/de/davis/keygo/core/ui/components/KeyGoLazyColumn.kt +++ b/core/ui/src/main/kotlin/de/davis/keygo/core/ui/components/KeyGoLazyColumn.kt @@ -17,6 +17,7 @@ import androidx.compose.foundation.lazy.LazyItemScope import androidx.compose.foundation.lazy.itemsIndexed import androidx.compose.foundation.lazy.rememberLazyListState import androidx.compose.material.icons.Icons +import androidx.compose.material.icons.filled.AutoAwesome import androidx.compose.material.icons.filled.PushPin import androidx.compose.material3.ExperimentalMaterial3ExpressiveApi import androidx.compose.material3.Icon @@ -55,6 +56,7 @@ import kotlin.math.roundToInt sealed interface HeaderContent { data class Letter(val char: Char) : HeaderContent data object Pin : HeaderContent + data object Suggested : HeaderContent } data class KeyGoColumnItem( @@ -294,6 +296,13 @@ private fun KeyGoInlineHeader( tint = color, modifier = Modifier.size(20.dp), ) + + is HeaderContent.Suggested -> Icon( + imageVector = Icons.Default.AutoAwesome, + contentDescription = null, + tint = color, + modifier = Modifier.size(20.dp), + ) } } } diff --git a/feature/item/create/build.gradle.kts b/feature/item/create/build.gradle.kts index 4f128a744..14bb9d17c 100644 --- a/feature/item/create/build.gradle.kts +++ b/feature/item/create/build.gradle.kts @@ -15,9 +15,16 @@ dependencies { implementation(projects.core.item) implementation(projects.core.security) implementation(projects.feature.item.core) + implementation(projects.feature.listScreen) implementation(projects.feature.totp) implementation(projects.feature.creditCard) implementation(libs.offrange.passgen) + testImplementation(testFixtures(projects.core.item)) + testImplementation(testFixtures(projects.core.security)) + testImplementation(testFixtures(projects.rust)) + testImplementation(libs.robolectric) + testImplementation(libs.androidx.junit) + } diff --git a/feature/item/create/src/main/kotlin/de/davis/keygo/feature/item/create/presentation/component/SelectItemForTotpModificationDialog.kt b/feature/item/create/src/main/kotlin/de/davis/keygo/feature/item/create/presentation/component/SelectItemForTotpModificationDialog.kt deleted file mode 100644 index bd580a661..000000000 --- a/feature/item/create/src/main/kotlin/de/davis/keygo/feature/item/create/presentation/component/SelectItemForTotpModificationDialog.kt +++ /dev/null @@ -1,162 +0,0 @@ -package de.davis.keygo.feature.item.create.presentation.component - -import android.content.res.Configuration -import androidx.compose.foundation.layout.Arrangement -import androidx.compose.foundation.layout.Column -import androidx.compose.foundation.layout.fillMaxSize -import androidx.compose.foundation.lazy.LazyColumn -import androidx.compose.foundation.lazy.items -import androidx.compose.material.icons.Icons -import androidx.compose.material.icons.filled.Edit -import androidx.compose.material3.AlertDialog -import androidx.compose.material3.CardDefaults -import androidx.compose.material3.HorizontalDivider -import androidx.compose.material3.Icon -import androidx.compose.material3.OutlinedButton -import androidx.compose.material3.OutlinedCard -import androidx.compose.material3.Surface -import androidx.compose.material3.Text -import androidx.compose.runtime.Composable -import androidx.compose.ui.Modifier -import androidx.compose.ui.graphics.Color -import androidx.compose.ui.res.stringResource -import androidx.compose.ui.text.SpanStyle -import androidx.compose.ui.text.buildAnnotatedString -import androidx.compose.ui.text.font.FontStyle -import androidx.compose.ui.text.withStyle -import androidx.compose.ui.tooling.preview.Preview -import androidx.compose.ui.unit.dp -import de.davis.keygo.core.item.domain.alias.newItemId -import de.davis.keygo.core.item.domain.model.DomainInfo -import de.davis.keygo.core.item.domain.model.lite.LiteLogin -import de.davis.keygo.core.ui.theme.KeyGoTheme -import de.davis.keygo.feature.item.create.R - -@Composable -fun SelectItemForTotpModificationDialog( - onDismissRequest: () -> Unit, - items: List, - onItemClicked: (LiteLogin) -> Unit, - onCreateNew: () -> Unit, - modifier: Modifier = Modifier -) { - AlertDialog( - onDismissRequest = onDismissRequest, - modifier = modifier, - confirmButton = { - OutlinedButton( - onClick = onCreateNew - ) { - Text(text = stringResource(R.string.create_new)) - } - }, - icon = { - Icon( - imageVector = Icons.Default.Edit, - contentDescription = null - ) - }, - title = { - Text(text = stringResource(R.string.existing_entries_found)) - }, - text = { - Column( - verticalArrangement = Arrangement.spacedBy(8.dp), - ) { - Text(text = stringResource(R.string.existing_entries_found_description)) - HorizontalDivider() - LazyColumn( - verticalArrangement = Arrangement.spacedBy(8.dp), - ) { - items(items = items, key = { it.id }) { item -> - OutlinedCard( - onClick = { - onItemClicked(item) - }, - colors = CardDefaults.outlinedCardColors( - containerColor = Color.Transparent - ) - ) { - MultiSupportingLineItem( - headlineContent = { - Text(text = item.name) - }, - supportingContent = { - item.username?.let { - Text(text = stringResource(R.string.list_entry, it)) - } - - Text( - text = buildAnnotatedString { - item.domains.take(3).joinToString { it.value }.also { - append(stringResource(R.string.list_entry, it)) - } - - if (item.domains.size <= 3) - return@buildAnnotatedString - - append(", ") - withStyle(SpanStyle(fontStyle = FontStyle.Italic)) { - append( - stringResource( - R.string.n_more, - item.domains.size - 3 - ) - ) - } - } - ) - } - ) - } - } - } - } - } - ) -} - -@Preview -@Preview(uiMode = Configuration.UI_MODE_NIGHT_YES) -@Composable -private fun SelectItemForTotpModificationDialogPreview() { - KeyGoTheme { - Surface(modifier = Modifier.fillMaxSize()) { - SelectItemForTotpModificationDialog( - onDismissRequest = {}, - onItemClicked = {}, - onCreateNew = {}, - items = listOf( - LiteLogin( - id = newItemId(), - name = "${if (1 >= 5) 'A' else 'B'} Item 1", - username = "User 1", - domains = listOf( - DomainInfo( - loginId = newItemId(), - value = "Website", - eTLD1 = "website.com" - ) - ), - pinned = false, - hasPassword = true, - ), - LiteLogin( - id = newItemId(), - name = "${if (2 >= 5) 'A' else 'B'} Item 2", - username = "User 2", - domains = listOf( - DomainInfo( - loginId = newItemId(), - value = "Website", - eTLD1 = "website.com" - ) - ), - pinned = false, - hasPassword = true, - ) - ) - ) - } - } -} \ No newline at end of file diff --git a/feature/item/create/src/main/kotlin/de/davis/keygo/feature/item/create/presentation/component/SelectItemForTotpScreen.kt b/feature/item/create/src/main/kotlin/de/davis/keygo/feature/item/create/presentation/component/SelectItemForTotpScreen.kt new file mode 100644 index 000000000..89059775f --- /dev/null +++ b/feature/item/create/src/main/kotlin/de/davis/keygo/feature/item/create/presentation/component/SelectItemForTotpScreen.kt @@ -0,0 +1,77 @@ +package de.davis.keygo.feature.item.create.presentation.component + +import androidx.activity.compose.BackHandler +import androidx.compose.foundation.layout.consumeWindowInsets +import androidx.compose.foundation.layout.fillMaxSize +import androidx.compose.foundation.layout.padding +import androidx.compose.material.icons.Icons +import androidx.compose.material.icons.automirrored.filled.ArrowBack +import androidx.compose.material.icons.filled.Add +import androidx.compose.material3.ExperimentalMaterial3Api +import androidx.compose.material3.FloatingActionButton +import androidx.compose.material3.Icon +import androidx.compose.material3.IconButton +import androidx.compose.material3.Scaffold +import androidx.compose.material3.Text +import androidx.compose.material3.TopAppBar +import androidx.compose.runtime.Composable +import androidx.compose.ui.Modifier +import androidx.compose.ui.res.stringResource +import de.davis.keygo.core.item.domain.alias.ItemId +import de.davis.keygo.core.item.generated.domain.model.VaultItemType +import de.davis.keygo.feature.item.create.R +import de.davis.keygo.feature.list_screen.presentation.ItemListScreen +import de.davis.keygo.feature.list_screen.presentation.NoItemStrategy +import de.davis.keygo.feature.item.core.R as ItemCoreR + +@OptIn(ExperimentalMaterial3Api::class) +@Composable +internal fun SelectItemForTotpScreen( + suggestedItemIds: Set, + onItemClick: (ItemId) -> Unit, + onCreateNew: () -> Unit, + onClose: () -> Unit, + modifier: Modifier = Modifier, +) { + BackHandler { onClose() } + + Scaffold( + modifier = modifier.fillMaxSize(), + topBar = { + TopAppBar( + title = { Text(text = stringResource(R.string.select_item_for_totp)) }, + navigationIcon = { + IconButton(onClick = onClose) { + Icon( + imageVector = Icons.AutoMirrored.Default.ArrowBack, + contentDescription = stringResource(ItemCoreR.string.back_content_description), + ) + } + }, + ) + }, + floatingActionButton = { + FloatingActionButton(onClick = onCreateNew) { + Icon( + imageVector = Icons.Default.Add, + contentDescription = stringResource(R.string.create_new), + ) + } + }, + ) { innerPadding -> + ItemListScreen( + onItemClick = onItemClick, + onItemLongClick = { }, + onCreateItemRequest = { onCreateNew() }, + restrictedItemType = VaultItemType.Login, + suggestedItemIds = suggestedItemIds, + notFoundStrategy = NoItemStrategy.ShowMessage, + enableDeletion = false, + enableSelection = false, + dockedSearchResults = false, + modifier = Modifier + .consumeWindowInsets(innerPadding) + .padding(innerPadding), + ) + } +} diff --git a/feature/item/create/src/main/kotlin/de/davis/keygo/feature/item/create/presentation/login/LoginContent.kt b/feature/item/create/src/main/kotlin/de/davis/keygo/feature/item/create/presentation/login/LoginContent.kt index 699ed810f..23a7f0994 100644 --- a/feature/item/create/src/main/kotlin/de/davis/keygo/feature/item/create/presentation/login/LoginContent.kt +++ b/feature/item/create/src/main/kotlin/de/davis/keygo/feature/item/create/presentation/login/LoginContent.kt @@ -59,7 +59,7 @@ import de.davis.keygo.feature.item.create.presentation.component.FormGroup import de.davis.keygo.feature.item.create.presentation.component.ItemContentWrapper import de.davis.keygo.feature.item.create.presentation.component.KeyGoItemForm import de.davis.keygo.feature.item.create.presentation.component.OverrideTotpDialog -import de.davis.keygo.feature.item.create.presentation.component.SelectItemForTotpModificationDialog +import de.davis.keygo.feature.item.create.presentation.component.SelectItemForTotpScreen import de.davis.keygo.feature.item.create.presentation.component.TAG_DELIMITERS import de.davis.keygo.feature.item.create.presentation.login.model.DialogState import de.davis.keygo.feature.item.create.presentation.login.model.LoginBaseState @@ -295,20 +295,6 @@ private fun LoginReadyContent( ) } - is DialogState.SelectItemForModification -> { - SelectItemForTotpModificationDialog( - onDismissRequest = { - // Don't allow dismissal - }, - items = state.dialogState.items, - onItemClicked = { item -> - onEvent(LoginUiEvent.OnTotpModificationItemSelected(item.id)) - }, - onCreateNew = { onEvent(LoginUiEvent.OnCreateNewItemForTotp) }, - modifier = Modifier.fillMaxWidth(), - ) - } - is DialogState.OverrideTotp -> { OverrideTotpDialog( onDismissRequest = { @@ -358,6 +344,15 @@ private fun LoginReadyContent( }, ) } + + if (state.selectingItemForTotp) { + SelectItemForTotpScreen( + suggestedItemIds = state.totpSuggestedItemIds, + onItemClick = { onEvent(LoginUiEvent.OnTotpModificationItemSelected(it)) }, + onCreateNew = { onEvent(LoginUiEvent.OnCreateNewItemForTotp) }, + onClose = { onEvent(LoginUiEvent.ItemUi(ItemUiEvent.OnBackClick)) }, + ) + } } private val DELIMITERS = setOf(',', ' ') diff --git a/feature/item/create/src/main/kotlin/de/davis/keygo/feature/item/create/presentation/login/LoginViewModel.kt b/feature/item/create/src/main/kotlin/de/davis/keygo/feature/item/create/presentation/login/LoginViewModel.kt index 1b63f3400..f8f96118c 100644 --- a/feature/item/create/src/main/kotlin/de/davis/keygo/feature/item/create/presentation/login/LoginViewModel.kt +++ b/feature/item/create/src/main/kotlin/de/davis/keygo/feature/item/create/presentation/login/LoginViewModel.kt @@ -226,27 +226,35 @@ internal class LoginViewModel( }.onSuccess { secret -> totpSecretInformation = secret totpOriginalUri = totpUri - viewModelScope.launch { - val matchedItems = secret.issuer?.let { - getTdlMatchedLogins(it) - } - - if (matchedItems.isNullOrEmpty()) { - updateUiWithTotpSecretInfo(secret, totpUri) - return@launch - } + _base.update { it.copy(selectingItemForTotp = true) } + viewModelScope.launch { + val suggestedIds = suggestedItemIdsFor(secret) + // A choice made before the query returned closed the picker for good; the late + // result has nothing left to reorder. _base.update { - it.copy( - dialogState = DialogState.SelectItemForModification( - items = matchedItems, - ) - ) + if (it.selectingItemForTotp) it.copy(totpSuggestedItemIds = suggestedIds) + else it } } } } + /** + * The logins whose registrable domain matches the code's own. + * + * [resolveTotpDomain] is what fills the domain field for a new item, so the suggestions agree + * with it: a code that carries no issuer still matches on the domain in `user@example.com`. + */ + private suspend fun suggestedItemIdsFor(secretInformation: TotpInfo): Set { + val domain = resolveTotpDomain( + issuer = secretInformation.issuer, + accountName = secretInformation.accountName, + ) ?: return emptySet() + + return getTdlMatchedLogins(domain).mapTo(mutableSetOf()) { it.id } + } + override fun onSubmit() { val ready = state.value as? ItemUiState.Ready ?: return val base = ready.base @@ -358,10 +366,12 @@ internal class LoginViewModel( } is LoginUiEvent.OnTotpModificationItemSelected -> { + closeTotpItemPicker() viewModelScope.launch { initWithId(event.itemId) } } is LoginUiEvent.OnCreateNewItemForTotp -> { + closeTotpItemPicker() totpSecretInformation?.let { updateUiWithTotpSecretInfo(it, totpOriginalUri) } @@ -580,6 +590,12 @@ internal class LoginViewModel( } } + private fun closeTotpItemPicker() { + _base.update { + it.copy(selectingItemForTotp = false, totpSuggestedItemIds = emptySet()) + } + } + private fun showTotpParseError() { _base.update { it.copy( diff --git a/feature/item/create/src/main/kotlin/de/davis/keygo/feature/item/create/presentation/login/model/DialogState.kt b/feature/item/create/src/main/kotlin/de/davis/keygo/feature/item/create/presentation/login/model/DialogState.kt index 2c008e25e..2819dffa7 100644 --- a/feature/item/create/src/main/kotlin/de/davis/keygo/feature/item/create/presentation/login/model/DialogState.kt +++ b/feature/item/create/src/main/kotlin/de/davis/keygo/feature/item/create/presentation/login/model/DialogState.kt @@ -1,12 +1,10 @@ package de.davis.keygo.feature.item.create.presentation.login.model import de.davis.keygo.core.item.domain.model.PasskeyRef -import de.davis.keygo.core.item.domain.model.lite.LiteLogin sealed interface DialogState { data object None : DialogState data object TotpParseError : DialogState - data class SelectItemForModification(val items: List) : DialogState data class OverrideTotp(val fields: Set) : DialogState data class DeletePasskey(val passkey: PasskeyRef) : DialogState diff --git a/feature/item/create/src/main/kotlin/de/davis/keygo/feature/item/create/presentation/login/model/LoginUiState.kt b/feature/item/create/src/main/kotlin/de/davis/keygo/feature/item/create/presentation/login/model/LoginUiState.kt index 9c75a64ce..90794588e 100644 --- a/feature/item/create/src/main/kotlin/de/davis/keygo/feature/item/create/presentation/login/model/LoginUiState.kt +++ b/feature/item/create/src/main/kotlin/de/davis/keygo/feature/item/create/presentation/login/model/LoginUiState.kt @@ -2,6 +2,7 @@ package de.davis.keygo.feature.item.create.presentation.login.model import androidx.compose.foundation.text.input.TextFieldState import androidx.compose.runtime.Stable +import de.davis.keygo.core.item.domain.alias.ItemId import de.davis.keygo.core.item.domain.model.DomainInfo import de.davis.keygo.core.item.domain.model.PasskeyRef import de.davis.keygo.core.item.domain.model.PasswordScore @@ -38,6 +39,8 @@ internal data class LoginBaseState( val dialogState: DialogState = DialogState.None, val nameError: InputFieldError? = null, val scanning: Boolean = false, + val selectingItemForTotp: Boolean = false, + val totpSuggestedItemIds: Set = emptySet(), val updating: Boolean = false, ) { /** diff --git a/feature/item/create/src/main/res/values/strings.xml b/feature/item/create/src/main/res/values/strings.xml index 819cf1e82..4e04876a8 100644 --- a/feature/item/create/src/main/res/values/strings.xml +++ b/feature/item/create/src/main/res/values/strings.xml @@ -18,13 +18,11 @@ Select an Item Create New + Add code to which item? Length [%d] Character Sets - Existing Entries found - One or more entries match the issuer or account name from the totp code. Would you like to update an existing entry or create a new one? - Warning Cancel @@ -48,9 +46,6 @@ Enter Name Write your Note - %d more - - \u2022 %s \u2022 Before: %s \u2022 After: %s diff --git a/feature/item/create/src/test/java/de/davis/keygo/feature/item/create/presentation/login/LoginViewModelTest.kt b/feature/item/create/src/test/java/de/davis/keygo/feature/item/create/presentation/login/LoginViewModelTest.kt new file mode 100644 index 000000000..48e627252 --- /dev/null +++ b/feature/item/create/src/test/java/de/davis/keygo/feature/item/create/presentation/login/LoginViewModelTest.kt @@ -0,0 +1,289 @@ +package de.davis.keygo.feature.item.create.presentation.login + +import de.davis.keygo.core.item.FakeCreditCardRepository +import de.davis.keygo.core.item.FakeItemRepository +import de.davis.keygo.core.item.FakeLoginRepository +import de.davis.keygo.core.item.FakePasswordStrengthEstimator +import de.davis.keygo.core.item.FakeVaultContextRepository +import de.davis.keygo.core.item.FakeVaultRepository +import de.davis.keygo.core.item.domain.alias.ItemId +import de.davis.keygo.core.item.domain.alias.newItemId +import de.davis.keygo.core.item.domain.alias.newVaultId +import de.davis.keygo.core.item.domain.model.DomainInfo +import de.davis.keygo.core.item.domain.model.KeyInformation +import de.davis.keygo.core.item.domain.model.Login +import de.davis.keygo.core.item.domain.model.Timestamp +import de.davis.keygo.core.item.domain.model.Vault +import de.davis.keygo.core.item.domain.usecase.ObserveAllTagsSortedUseCase +import de.davis.keygo.core.item.domain.usecase.UpsertVaultItemUseCase +import de.davis.keygo.core.item.generated.domain.model.VaultItemType +import de.davis.keygo.core.security.crypto.FakeCryptographicScopeProvider +import de.davis.keygo.core.security.domain.usecase.GetTdlMatchedLoginsUseCase +import de.davis.keygo.core.security.domain.usecase.ItemWithCryptoScopeUseCase +import de.davis.keygo.core.util.domain.model.snackbar.SnackbarMessage +import de.davis.keygo.core.util.domain.resolver.RegistrableDomainResolver +import de.davis.keygo.core.util.domain.snackbar.SnackbarManager +import de.davis.keygo.core.util.domain.usecase.SortUseCase +import de.davis.keygo.feature.item.core.domain.usecase.CreateNewOrUpdateLoginUseCase +import de.davis.keygo.feature.item.core.presentation.model.DetailPaneInformation +import de.davis.keygo.feature.item.create.presentation.login.model.LoginBaseState +import de.davis.keygo.feature.item.create.presentation.login.model.LoginUiEvent +import de.davis.keygo.feature.item.create.presentation.model.ItemUiState +import de.davis.keygo.rust.FakeTotpService +import de.davisalessandro.keygo.rust.Algorithm +import de.davisalessandro.keygo.rust.TotpInfo +import kotlinx.coroutines.Dispatchers +import kotlinx.coroutines.ExperimentalCoroutinesApi +import kotlinx.coroutines.flow.Flow +import kotlinx.coroutines.flow.emptyFlow +import kotlinx.coroutines.launch +import kotlinx.coroutines.test.TestScope +import kotlinx.coroutines.test.UnconfinedTestDispatcher +import kotlinx.coroutines.test.advanceUntilIdle +import kotlinx.coroutines.test.resetMain +import kotlinx.coroutines.test.runTest +import kotlinx.coroutines.test.setMain +import org.junit.runner.RunWith +import org.robolectric.RobolectricTestRunner +import org.robolectric.annotation.Config +import kotlin.test.AfterTest +import kotlin.test.BeforeTest +import kotlin.test.Test +import kotlin.test.assertEquals +import kotlin.test.assertFalse +import kotlin.test.assertIs +import kotlin.test.assertTrue + +/** + * Covers what a scanned `otpauth://` deep link does before the form is filled in: it hands the + * user the item picker rather than guessing a target, and offers the logins on the code's own + * domain as suggestions. + */ +@OptIn(ExperimentalCoroutinesApi::class) +@RunWith(RobolectricTestRunner::class) +@Config(sdk = [34]) +class LoginViewModelTest { + + private val mainDispatcher = UnconfinedTestDispatcher() + + private val defaultVault = Vault( + id = newVaultId(), + name = "Default vault", + keyInformation = KeyInformation(byteArrayOf(), byteArrayOf()), + icon = Vault.Icon.Default, + ) + + private val loginRepository = FakeLoginRepository() + private val itemRepository = FakeItemRepository(loginRepository) + private val vaultRepository = FakeVaultRepository() + private val vaultContextRepository = FakeVaultContextRepository() + private val cryptoProvider = FakeCryptographicScopeProvider(itemRepository) + private val totpService = FakeTotpService() + private val domainResolver = TestRegistrableDomainResolver() + + @BeforeTest + fun setUp() { + Dispatchers.setMain(mainDispatcher) + vaultRepository.seed(defaultVault) + vaultContextRepository.seedLastInteracted(defaultVault.id) + } + + @AfterTest + fun tearDown() { + Dispatchers.resetMain() + } + + @Test + fun `a deep link opens the picker instead of filling the form`() = runVmTest { + totpService.infoFromUriResult = totpInfo(issuer = "github.com") + + val viewModel = initWithDeepLink() + + val base = viewModel.readyBase() + assertTrue(base.selectingItemForTotp) + assertEquals("", base.totpTextFieldState.text.toString()) + assertEquals("", base.usernameTextFieldState.text.toString()) + assertEquals(emptySet(), base.domains) + } + + @Test + fun `the picker opens even when nothing matches the domain`() = runVmTest { + totpService.infoFromUriResult = totpInfo(issuer = "github.com") + + val viewModel = initWithDeepLink() + + val base = viewModel.readyBase() + assertTrue(base.selectingItemForTotp) + assertEquals(emptySet(), base.totpSuggestedItemIds) + } + + @Test + fun `logins on the code's domain are suggested`() = runVmTest { + val onDomain = seedLogin(name = "GitHub", domain = "github.com") + seedLogin(name = "Google", domain = "google.com") + totpService.infoFromUriResult = totpInfo(issuer = "github.com") + + val viewModel = initWithDeepLink() + + assertEquals(setOf(onDomain), viewModel.readyBase().totpSuggestedItemIds) + } + + @Test + fun `a code without an issuer suggests on the account name's domain`() = runVmTest { + val onDomain = seedLogin(name = "GitHub", domain = "github.com") + totpService.infoFromUriResult = totpInfo(issuer = null, accountName = "me@github.com") + + val viewModel = initWithDeepLink() + + assertEquals(setOf(onDomain), viewModel.readyBase().totpSuggestedItemIds) + } + + @Test + fun `creating a new item closes the picker and fills the form from the code`() = runVmTest { + totpService.infoFromUriResult = totpInfo(issuer = "github.com") + val viewModel = initWithDeepLink() + + viewModel.onEvent(LoginUiEvent.OnCreateNewItemForTotp) + advanceUntilIdle() + + val base = viewModel.readyBase() + assertFalse(base.selectingItemForTotp) + assertEquals(DEEP_LINK_URI, base.totpTextFieldState.text.toString()) + assertEquals("me@github.com", base.usernameTextFieldState.text.toString()) + assertEquals(setOf("github.com"), base.domains.mapTo(mutableSetOf()) { it.value }) + } + + @Test + fun `choosing an existing item closes the picker and loads that item`() = runVmTest { + val existing = seedLogin(name = "GitHub", domain = "github.com") + totpService.infoFromUriResult = totpInfo(issuer = "github.com") + val viewModel = initWithDeepLink() + + viewModel.onEvent(LoginUiEvent.OnTotpModificationItemSelected(existing)) + advanceUntilIdle() + + val state = viewModel.readyState() + assertFalse(state.base.selectingItemForTotp) + assertTrue(state.base.updating) + assertEquals("GitHub", state.shared.nameTextFieldState.text.toString()) + assertEquals(DEEP_LINK_URI, state.base.totpTextFieldState.text.toString()) + assertEquals("me@github.com", state.base.usernameTextFieldState.text.toString()) + } + + @Test + fun `an unparsable code shows the error instead of the picker`() = runVmTest { + totpService.infoFromUriResult = null + + val viewModel = initWithDeepLink() + + assertFalse(viewModel.readyBase().selectingItemForTotp) + } + + // Helpers + + private fun runVmTest(body: suspend TestScope.() -> Unit) = + runTest(mainDispatcher.scheduler) { body() } + + private fun TestScope.initWithDeepLink(uri: String = DEEP_LINK_URI): LoginViewModel { + val viewModel = buildViewModel() + backgroundScope.launch(mainDispatcher) { viewModel.state.collect { } } + viewModel.init(DetailPaneInformation.Init.TOTP(VaultItemType.Login, uri)) + advanceUntilIdle() + return viewModel + } + + private fun LoginViewModel.readyState(): ItemUiState.Ready { + val state = state.value + assertIs>(state) + return state + } + + private fun LoginViewModel.readyBase(): LoginBaseState = readyState().base + + private fun seedLogin(name: String, domain: String): ItemId { + val id = newItemId() + loginRepository.seed( + Login( + id = id, + name = name, + username = null, + domainInfos = setOf( + DomainInfo( + loginId = id, + value = domain, + eTLD1 = domainResolver.resolve(domain), + ), + ), + passwordCredential = null, + totp = null, + passkeys = emptySet(), + note = null, + pinned = false, + vaultId = defaultVault.id, + keyInformation = KeyInformation(byteArrayOf(), byteArrayOf()), + timestamp = Timestamp(), + ), + ) + return id + } + + private fun totpInfo( + issuer: String?, + accountName: String = "me@github.com", + secret: String = "JBSWY3DPEHPK3PXP", + ) = TotpInfo( + secret = secret, + issuer = issuer, + accountName = accountName, + algorithm = Algorithm.SHA1, + digits = 6, + period = 30, + ) + + private fun buildViewModel() = LoginViewModel( + itemWithCryptoScope = ItemWithCryptoScopeUseCase(vaultRepository, cryptoProvider), + loginRepository = loginRepository, + passwordStrengthEstimator = FakePasswordStrengthEstimator(), + createNewOrUpdateLogin = CreateNewOrUpdateLoginUseCase( + cryptographicScopeProvider = cryptoProvider, + loginRepository = loginRepository, + vaultRepository = vaultRepository, + upsertVaultItem = UpsertVaultItemUseCase( + loginRepository, + FakeCreditCardRepository(), + ), + passwordStrengthEstimator = FakePasswordStrengthEstimator(), + totpService = totpService, + ), + getTdlMatchedLogins = GetTdlMatchedLoginsUseCase(domainResolver, loginRepository), + snackbarManager = TestSnackbarManager(), + totpService = totpService, + registrableDomainResolver = domainResolver, + vaultContextRepository = vaultContextRepository, + itemRepository = itemRepository, + observeAllTags = ObserveAllTagsSortedUseCase(itemRepository, SortUseCase()), + vaultRepository = vaultRepository, + ) + + /** Resolves an eTLD+1 by keeping the last two labels, which is enough for the test domains. */ + private class TestRegistrableDomainResolver : RegistrableDomainResolver { + override fun resolve(domain: String): String? { + val labels = domain.substringAfter("://") + .substringBefore('/') + .split('.') + .filter { it.isNotBlank() } + + return if (labels.size >= 2) labels.takeLast(2).joinToString(".") else null + } + } + + private class TestSnackbarManager : SnackbarManager { + override val oneShotEvents: Flow = emptyFlow() + override fun sendMessage(message: SnackbarMessage) = Unit + } + + companion object { + private const val DEEP_LINK_URI = + "otpauth://totp/GitHub:me@github.com?secret=JBSWY3DPEHPK3PXP&issuer=github.com" + } +} diff --git a/feature/list_screen/src/main/kotlin/de/davis/keygo/feature/list_screen/presentation/ItemListScreen.kt b/feature/list_screen/src/main/kotlin/de/davis/keygo/feature/list_screen/presentation/ItemListScreen.kt index 36486d222..5e5a77f43 100644 --- a/feature/list_screen/src/main/kotlin/de/davis/keygo/feature/list_screen/presentation/ItemListScreen.kt +++ b/feature/list_screen/src/main/kotlin/de/davis/keygo/feature/list_screen/presentation/ItemListScreen.kt @@ -9,6 +9,7 @@ import androidx.compose.runtime.Composable import androidx.compose.runtime.LaunchedEffect import androidx.compose.runtime.Stable import androidx.compose.runtime.getValue +import androidx.compose.runtime.remember import androidx.compose.runtime.rememberUpdatedState import androidx.compose.runtime.snapshotFlow import androidx.compose.ui.Modifier @@ -38,6 +39,7 @@ fun ItemListScreen( onItemsDelete: (deleted: Set, firstItemId: ItemId?) -> Unit = { _, _ -> }, restrictedItemType: VaultItemType? = null, notFoundStrategy: NoItemStrategy = NoItemStrategy.ShowCreateNewItemCard, + suggestedItemIds: Set = emptySet(), autoSelectFirst: Boolean = false, enableDeletion: Boolean = true, enableSelection: Boolean = true, @@ -47,9 +49,16 @@ fun ItemListScreen( val viewModel = koinViewModel { parametersOf(enableSelection, restrictedItemType) } - val uiState by viewModel.listItemState.collectAsStateWithLifecycle() + val collectedState by viewModel.listItemState.collectAsStateWithLifecycle() val filterSheetState by viewModel.filterBottomSheetState.collectAsStateWithLifecycle() + // Suggestions rank the default list only. Once the user searches, the results carry their own + // relevance and a group pinned above them would fight it. + val suggested = if (collectedState.hasSearchQuery) emptySet() else suggestedItemIds + val uiState = remember(collectedState, suggested) { + collectedState.copy(items = collectedState.items.withSuggestedFirst(suggested)) + } + LaunchedEffect(autoSelectFirst) { if (!autoSelectFirst) viewModel.resetHighlight() } @@ -101,6 +110,7 @@ fun ItemListScreen( autoSelectFirst = autoSelectFirst, notFoundStrategy = notFoundStrategy, restrictedItemType = restrictedItemType, + suggestedItemIds = suggested, onCreateItemRequest = onCreateItemRequest, onSubmitQuery = viewModel::onSubmitQuery, onClearQuery = viewModel::onClearQuery, diff --git a/feature/list_screen/src/main/kotlin/de/davis/keygo/feature/list_screen/presentation/SuggestedItems.kt b/feature/list_screen/src/main/kotlin/de/davis/keygo/feature/list_screen/presentation/SuggestedItems.kt new file mode 100644 index 000000000..b9adbffe2 --- /dev/null +++ b/feature/list_screen/src/main/kotlin/de/davis/keygo/feature/list_screen/presentation/SuggestedItems.kt @@ -0,0 +1,11 @@ +package de.davis.keygo.feature.list_screen.presentation + +import de.davis.keygo.core.item.domain.alias.ItemId +import de.davis.keygo.core.item.domain.model.lite.LiteItem + +internal fun List.withSuggestedFirst(suggestedItemIds: Set): List { + if (suggestedItemIds.isEmpty()) return this + + val (suggested, rest) = partition { it.id in suggestedItemIds } + return if (suggested.isEmpty()) this else suggested + rest +} diff --git a/feature/list_screen/src/main/kotlin/de/davis/keygo/feature/list_screen/presentation/components/ItemListContent.kt b/feature/list_screen/src/main/kotlin/de/davis/keygo/feature/list_screen/presentation/components/ItemListContent.kt index 2252e9721..3b5b41b25 100644 --- a/feature/list_screen/src/main/kotlin/de/davis/keygo/feature/list_screen/presentation/components/ItemListContent.kt +++ b/feature/list_screen/src/main/kotlin/de/davis/keygo/feature/list_screen/presentation/components/ItemListContent.kt @@ -68,6 +68,7 @@ internal fun ItemListContent( autoSelectFirst: Boolean, notFoundStrategy: NoItemStrategy, restrictedItemType: VaultItemType?, + suggestedItemIds: Set, onCreateItemRequest: (VaultItemType) -> Unit, onSubmitQuery: () -> Unit, onClearQuery: () -> Unit, @@ -217,11 +218,14 @@ internal fun ItemListContent( } false -> { - val items = remember(uiState.items) { + val items = remember(uiState.items, suggestedItemIds) { uiState.items.map { KeyGoColumnItem( - header = if (it.pinned) HeaderContent.Pin - else HeaderContent.Letter(it.name.first().uppercaseChar()), + header = when { + it.id in suggestedItemIds -> HeaderContent.Suggested + it.pinned -> HeaderContent.Pin + else -> HeaderContent.Letter(it.name.first().uppercaseChar()) + }, title = it.name, id = it.id, itemType = it.itemType, @@ -300,6 +304,7 @@ private fun ItemListContentPreview() { autoSelectFirst = false, notFoundStrategy = NoItemStrategy.ShowCreateNewItemCard, restrictedItemType = null, + suggestedItemIds = emptySet(), onCreateItemRequest = {}, onSubmitQuery = {}, onClearQuery = {}, diff --git a/feature/list_screen/src/test/kotlin/de/davis/keygo/feature/list_screen/presentation/SuggestedItemsTest.kt b/feature/list_screen/src/test/kotlin/de/davis/keygo/feature/list_screen/presentation/SuggestedItemsTest.kt new file mode 100644 index 000000000..fa5be35a5 --- /dev/null +++ b/feature/list_screen/src/test/kotlin/de/davis/keygo/feature/list_screen/presentation/SuggestedItemsTest.kt @@ -0,0 +1,80 @@ +package de.davis.keygo.feature.list_screen.presentation + +import de.davis.keygo.core.item.domain.alias.ItemId +import de.davis.keygo.core.item.domain.model.lite.LiteItem +import de.davis.keygo.core.item.generated.domain.model.VaultItemType +import java.util.UUID +import kotlin.test.Test +import kotlin.test.assertEquals +import kotlin.test.assertSame + +class SuggestedItemsTest { + + private data class TestLiteItem( + override val name: String, + override val id: ItemId = UUID.nameUUIDFromBytes(name.toByteArray()), + override val itemType: VaultItemType = VaultItemType.Login, + override val pinned: Boolean = false, + ) : LiteItem + + private fun items(vararg names: String) = names.map { TestLiteItem(name = it) } + + private fun idsOf(vararg names: String) = names.mapTo(mutableSetOf()) { TestLiteItem(it).id } + + private fun List.names() = map { it.name } + + @Test + fun `no suggestions leaves the list untouched`() { + val provided = items("Amazon", "GitHub", "Google") + + val result = provided.withSuggestedFirst(emptySet()) + + assertSame(provided, result) + } + + @Test + fun `suggested items move to the front`() { + val provided = items("Amazon", "GitHub", "Google") + + val result = provided.withSuggestedFirst(idsOf("GitHub")) + + assertEquals(listOf("GitHub", "Amazon", "Google"), result.names()) + } + + @Test + fun `order within each group is preserved`() { + val provided = items("Amazon", "GitHub", "Google", "GitHub (work)") + + val result = provided.withSuggestedFirst(idsOf("GitHub", "GitHub (work)")) + + assertEquals( + listOf("GitHub", "GitHub (work)", "Amazon", "Google"), + result.names(), + ) + } + + @Test + fun `ids that are not in the list are ignored`() { + val provided = items("Amazon", "Google") + + val result = provided.withSuggestedFirst(idsOf("GitHub")) + + assertSame(provided, result) + } + + @Test + fun `a partially present suggestion set hoists only what the list holds`() { + val provided = items("Amazon", "Google") + + val result = provided.withSuggestedFirst(idsOf("GitHub", "Google")) + + assertEquals(listOf("Google", "Amazon"), result.names()) + } + + @Test + fun `an empty list stays empty`() { + val result = emptyList().withSuggestedFirst(idsOf("GitHub")) + + assertEquals(emptyList(), result) + } +} From e38fc6a14996285f888d7606c05fb4388b03c023 Mon Sep 17 00:00:00 2001 From: Davis Wolfermann Date: Sun, 23 Aug 2026 15:53:31 +0200 Subject: [PATCH 02/20] feat(totp): keep the import on the picker until it is saved Back from the form returns to the picker with a clean slate instead of leaving the flow, and the wait before the vault is readable now wears the picker's chrome rather than the form's. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_019NwwBkR3ks2QMJSRmP1iDH --- .../component/SelectItemForTotpScreen.kt | 90 +++++++++++-------- .../create/presentation/login/LoginContent.kt | 27 +++++- .../create/presentation/login/LoginScreen.kt | 1 + .../presentation/login/LoginViewModel.kt | 58 +++++++++++- .../presentation/login/model/LoginUiState.kt | 7 ++ .../presentation/login/LoginViewModelTest.kt | 87 ++++++++++++++++++ 6 files changed, 228 insertions(+), 42 deletions(-) diff --git a/feature/item/create/src/main/kotlin/de/davis/keygo/feature/item/create/presentation/component/SelectItemForTotpScreen.kt b/feature/item/create/src/main/kotlin/de/davis/keygo/feature/item/create/presentation/component/SelectItemForTotpScreen.kt index 89059775f..af87fc6b0 100644 --- a/feature/item/create/src/main/kotlin/de/davis/keygo/feature/item/create/presentation/component/SelectItemForTotpScreen.kt +++ b/feature/item/create/src/main/kotlin/de/davis/keygo/feature/item/create/presentation/component/SelectItemForTotpScreen.kt @@ -1,20 +1,23 @@ package de.davis.keygo.feature.item.create.presentation.component import androidx.activity.compose.BackHandler +import androidx.activity.compose.LocalActivity +import androidx.compose.foundation.layout.Box import androidx.compose.foundation.layout.consumeWindowInsets import androidx.compose.foundation.layout.fillMaxSize import androidx.compose.foundation.layout.padding import androidx.compose.material.icons.Icons -import androidx.compose.material.icons.automirrored.filled.ArrowBack import androidx.compose.material.icons.filled.Add +import androidx.compose.material3.ContainedLoadingIndicator import androidx.compose.material3.ExperimentalMaterial3Api +import androidx.compose.material3.ExperimentalMaterial3ExpressiveApi import androidx.compose.material3.FloatingActionButton import androidx.compose.material3.Icon -import androidx.compose.material3.IconButton import androidx.compose.material3.Scaffold import androidx.compose.material3.Text import androidx.compose.material3.TopAppBar import androidx.compose.runtime.Composable +import androidx.compose.ui.Alignment import androidx.compose.ui.Modifier import androidx.compose.ui.res.stringResource import de.davis.keygo.core.item.domain.alias.ItemId @@ -22,56 +25,69 @@ import de.davis.keygo.core.item.generated.domain.model.VaultItemType import de.davis.keygo.feature.item.create.R import de.davis.keygo.feature.list_screen.presentation.ItemListScreen import de.davis.keygo.feature.list_screen.presentation.NoItemStrategy -import de.davis.keygo.feature.item.core.R as ItemCoreR -@OptIn(ExperimentalMaterial3Api::class) +/** + * Asks which item a scanned code belongs to, listing every login with the ones on the code's own + * domain grouped first. + * + * The screen carries no back affordance. It is the first step of a deep-linked import, which + * replaced the whole back stack on its way here, so there is nothing behind it: back leaves the app + * rather than revealing a dashboard the user never opened. + * + * @param loading shows the chrome without the list, for the moment before the vault is readable. + * Choosing anything then would have nothing to choose from, so both actions are withheld too. + */ +@OptIn(ExperimentalMaterial3Api::class, ExperimentalMaterial3ExpressiveApi::class) @Composable internal fun SelectItemForTotpScreen( suggestedItemIds: Set, onItemClick: (ItemId) -> Unit, onCreateNew: () -> Unit, - onClose: () -> Unit, modifier: Modifier = Modifier, + loading: Boolean = false, ) { - BackHandler { onClose() } + val activity = LocalActivity.current + BackHandler { activity?.finish() } Scaffold( modifier = modifier.fillMaxSize(), topBar = { - TopAppBar( - title = { Text(text = stringResource(R.string.select_item_for_totp)) }, - navigationIcon = { - IconButton(onClick = onClose) { - Icon( - imageVector = Icons.AutoMirrored.Default.ArrowBack, - contentDescription = stringResource(ItemCoreR.string.back_content_description), - ) - } - }, - ) + TopAppBar(title = { Text(text = stringResource(R.string.select_item_for_totp)) }) }, floatingActionButton = { - FloatingActionButton(onClick = onCreateNew) { - Icon( - imageVector = Icons.Default.Add, - contentDescription = stringResource(R.string.create_new), - ) - } + if (!loading) + FloatingActionButton(onClick = onCreateNew) { + Icon( + imageVector = Icons.Default.Add, + contentDescription = stringResource(R.string.create_new), + ) + } }, ) { innerPadding -> - ItemListScreen( - onItemClick = onItemClick, - onItemLongClick = { }, - onCreateItemRequest = { onCreateNew() }, - restrictedItemType = VaultItemType.Login, - suggestedItemIds = suggestedItemIds, - notFoundStrategy = NoItemStrategy.ShowMessage, - enableDeletion = false, - enableSelection = false, - dockedSearchResults = false, - modifier = Modifier - .consumeWindowInsets(innerPadding) - .padding(innerPadding), - ) + val content = Modifier + .consumeWindowInsets(innerPadding) + .padding(innerPadding) + + when (loading) { + true -> Box( + modifier = content.fillMaxSize(), + contentAlignment = Alignment.Center, + ) { + ContainedLoadingIndicator() + } + + false -> ItemListScreen( + onItemClick = onItemClick, + onItemLongClick = { }, + onCreateItemRequest = { onCreateNew() }, + restrictedItemType = VaultItemType.Login, + suggestedItemIds = suggestedItemIds, + notFoundStrategy = NoItemStrategy.ShowMessage, + enableDeletion = false, + enableSelection = false, + dockedSearchResults = false, + modifier = content, + ) + } } } diff --git a/feature/item/create/src/main/kotlin/de/davis/keygo/feature/item/create/presentation/login/LoginContent.kt b/feature/item/create/src/main/kotlin/de/davis/keygo/feature/item/create/presentation/login/LoginContent.kt index 23a7f0994..467018468 100644 --- a/feature/item/create/src/main/kotlin/de/davis/keygo/feature/item/create/presentation/login/LoginContent.kt +++ b/feature/item/create/src/main/kotlin/de/davis/keygo/feature/item/create/presentation/login/LoginContent.kt @@ -1,6 +1,7 @@ package de.davis.keygo.feature.item.create.presentation.login import android.content.res.Configuration +import androidx.activity.compose.BackHandler import androidx.compose.foundation.layout.Arrangement import androidx.compose.foundation.layout.Column import androidx.compose.foundation.layout.FlowRow @@ -77,7 +78,24 @@ import de.davis.keygo.core.item.R as CoreItemR import de.davis.keygo.feature.item.core.R as ItemCoreR @Composable -internal fun LoginContent(state: LoginUiState, onEvent: (LoginUiEvent) -> Unit) { +internal fun LoginContent( + state: LoginUiState, + onEvent: (LoginUiEvent) -> Unit, + totpImportPending: Boolean = false, +) { + // A deep-linked import opens on the picker, so the picker's chrome is what the wait belongs + // to. The shared loading scaffold would flash a title and a back arrow from the form, a screen + // the user has not asked for yet and may never reach. + if (totpImportPending && state is ItemUiState.Loading) { + SelectItemForTotpScreen( + suggestedItemIds = emptySet(), + onItemClick = {}, + onCreateNew = {}, + loading = true, + ) + return + } + ItemContentWrapper( itemType = VaultItemType.Login, state = state, @@ -98,6 +116,12 @@ private fun LoginReadyContent( shared: SharedItemState, onEvent: (LoginUiEvent) -> Unit, ) { + // An import opened this form from the picker, so back has to reach the ViewModel and return + // there. Without this the pane navigator takes it first and pops the whole screen instead. + BackHandler(enabled = state.totpImportActive) { + onEvent(LoginUiEvent.ItemUi(ItemUiEvent.OnBackClick)) + } + val scrollBehavior = TopAppBarDefaults.enterAlwaysScrollBehavior() val domainTextFieldState = rememberTextFieldState() val tagsTextFieldState = rememberTextFieldState() @@ -350,7 +374,6 @@ private fun LoginReadyContent( suggestedItemIds = state.totpSuggestedItemIds, onItemClick = { onEvent(LoginUiEvent.OnTotpModificationItemSelected(it)) }, onCreateNew = { onEvent(LoginUiEvent.OnCreateNewItemForTotp) }, - onClose = { onEvent(LoginUiEvent.ItemUi(ItemUiEvent.OnBackClick)) }, ) } } diff --git a/feature/item/create/src/main/kotlin/de/davis/keygo/feature/item/create/presentation/login/LoginScreen.kt b/feature/item/create/src/main/kotlin/de/davis/keygo/feature/item/create/presentation/login/LoginScreen.kt index e894c1966..b599e5cf9 100644 --- a/feature/item/create/src/main/kotlin/de/davis/keygo/feature/item/create/presentation/login/LoginScreen.kt +++ b/feature/item/create/src/main/kotlin/de/davis/keygo/feature/item/create/presentation/login/LoginScreen.kt @@ -40,5 +40,6 @@ fun LoginScreen( LoginContent( state = state, onEvent = viewmodel::onEvent, + totpImportPending = detailPaneInformation is DetailPaneInformation.Init.TOTP, ) } diff --git a/feature/item/create/src/main/kotlin/de/davis/keygo/feature/item/create/presentation/login/LoginViewModel.kt b/feature/item/create/src/main/kotlin/de/davis/keygo/feature/item/create/presentation/login/LoginViewModel.kt index f8f96118c..8b1327e3c 100644 --- a/feature/item/create/src/main/kotlin/de/davis/keygo/feature/item/create/presentation/login/LoginViewModel.kt +++ b/feature/item/create/src/main/kotlin/de/davis/keygo/feature/item/create/presentation/login/LoginViewModel.kt @@ -6,6 +6,7 @@ import androidx.compose.foundation.text.input.setTextAndPlaceCursorAtEnd import androidx.compose.runtime.snapshotFlow import androidx.lifecycle.viewModelScope import de.davis.keygo.core.item.domain.alias.ItemId +import de.davis.keygo.core.item.domain.alias.VaultId import de.davis.keygo.core.item.domain.estimator.PasswordStrengthEstimator import de.davis.keygo.core.item.domain.model.DomainInfo import de.davis.keygo.core.item.domain.model.PasswordScore @@ -111,6 +112,21 @@ internal class LoginViewModel( private var totpSecretInformation: TotpInfo? = null private var totpOriginalUri: String? = null + /** + * Whether this screen was opened by a scanned code rather than by the user. + * + * Only a deep link owns the whole screen, so only it gets the picker and the back behaviour + * that belongs to it. A code scanned from within the form arrives at an item the user already + * chose and must keep leaving the screen as it always has. + */ + private var totpItemPickerFlow = false + + /** Kept so returning to the picker shows the same suggestions without querying again. */ + private var totpSuggestedItemIds: Set = emptySet() + + /** The vault the picker was showing, restored when the form is abandoned back to it. */ + private var totpPickerVaultId: VaultId? = null + /** * Shows a passkey for [rp] as pending until the item is saved. * @@ -226,12 +242,14 @@ internal class LoginViewModel( }.onSuccess { secret -> totpSecretInformation = secret totpOriginalUri = totpUri - _base.update { it.copy(selectingItemForTotp = true) } + totpItemPickerFlow = true + _base.update { it.copy(totpImportActive = true, selectingItemForTotp = true) } viewModelScope.launch { val suggestedIds = suggestedItemIdsFor(secret) - // A choice made before the query returned closed the picker for good; the late - // result has nothing left to reorder. + totpSuggestedItemIds = suggestedIds + // A choice made before the query returned leaves the picker; the late result has + // nothing left to reorder until the user comes back to it. _base.update { if (it.selectingItemForTotp) it.copy(totpSuggestedItemIds = suggestedIds) else it @@ -330,6 +348,15 @@ internal class LoginViewModel( return } + // Back belongs to the import while one is running: from the form it returns to the picker + // the form was opened from. The picker itself is the flow's first step and has nothing + // behind it, so leaving it is the screen's own business (it closes the app) and never a + // navigation back to a dashboard the user did not open. + if (totpItemPickerFlow) { + if (!_base.value.selectingItemForTotp) reopenTotpItemPicker() + return + } + navigateUp() } @@ -591,11 +618,36 @@ internal class LoginViewModel( } private fun closeTotpItemPicker() { + totpPickerVaultId = selectedVaultId.value _base.update { it.copy(selectingItemForTotp = false, totpSuggestedItemIds = emptySet()) } } + /** + * Returns to the picker, throwing away whatever the choice made of the form. + * + * The form is rebuilt from scratch rather than hidden: the choice may have loaded an existing + * item into it, and carrying that item's name, password or vault into the next choice would + * write it onto the wrong login. Only the scanned code and its suggestions survive, because + * those belong to the import rather than to the item. + */ + private fun reopenTotpItemPicker() { + itemId = null + nameTextFieldState.setTextAndPlaceCursorAtEnd("") + notesTextFieldState.setTextAndPlaceCursorAtEnd("") + passwordTextFieldState.setTextAndPlaceCursorAtEnd("") + setAssignedTags(emptySet()) + totpPickerVaultId?.let { setSelectedVaultId(it) } + + _base.value = LoginBaseState( + passwordTextFieldState = passwordTextFieldState, + totpImportActive = true, + selectingItemForTotp = true, + totpSuggestedItemIds = totpSuggestedItemIds, + ) + } + private fun showTotpParseError() { _base.update { it.copy( diff --git a/feature/item/create/src/main/kotlin/de/davis/keygo/feature/item/create/presentation/login/model/LoginUiState.kt b/feature/item/create/src/main/kotlin/de/davis/keygo/feature/item/create/presentation/login/model/LoginUiState.kt index 90794588e..d0d8552ef 100644 --- a/feature/item/create/src/main/kotlin/de/davis/keygo/feature/item/create/presentation/login/model/LoginUiState.kt +++ b/feature/item/create/src/main/kotlin/de/davis/keygo/feature/item/create/presentation/login/model/LoginUiState.kt @@ -39,6 +39,13 @@ internal data class LoginBaseState( val dialogState: DialogState = DialogState.None, val nameError: InputFieldError? = null, val scanning: Boolean = false, + /** + * Whether a deep-linked code is being imported, from the picker until the item is saved. + * + * The screen has to claim back for as long as this runs: the form was opened by the picker, so + * back belongs to that flow rather than to the pane the screen sits in. + */ + val totpImportActive: Boolean = false, val selectingItemForTotp: Boolean = false, val totpSuggestedItemIds: Set = emptySet(), val updating: Boolean = false, diff --git a/feature/item/create/src/test/java/de/davis/keygo/feature/item/create/presentation/login/LoginViewModelTest.kt b/feature/item/create/src/test/java/de/davis/keygo/feature/item/create/presentation/login/LoginViewModelTest.kt index 48e627252..b2d29ef92 100644 --- a/feature/item/create/src/test/java/de/davis/keygo/feature/item/create/presentation/login/LoginViewModelTest.kt +++ b/feature/item/create/src/test/java/de/davis/keygo/feature/item/create/presentation/login/LoginViewModelTest.kt @@ -28,6 +28,7 @@ import de.davis.keygo.feature.item.core.domain.usecase.CreateNewOrUpdateLoginUse import de.davis.keygo.feature.item.core.presentation.model.DetailPaneInformation import de.davis.keygo.feature.item.create.presentation.login.model.LoginBaseState import de.davis.keygo.feature.item.create.presentation.login.model.LoginUiEvent +import de.davis.keygo.feature.item.create.presentation.model.ItemUiEvent import de.davis.keygo.feature.item.create.presentation.model.ItemUiState import de.davis.keygo.rust.FakeTotpService import de.davisalessandro.keygo.rust.Algorithm @@ -101,6 +102,7 @@ class LoginViewModelTest { val base = viewModel.readyBase() assertTrue(base.selectingItemForTotp) + assertTrue(base.totpImportActive) assertEquals("", base.totpTextFieldState.text.toString()) assertEquals("", base.usernameTextFieldState.text.toString()) assertEquals(emptySet(), base.domains) @@ -148,6 +150,8 @@ class LoginViewModelTest { val base = viewModel.readyBase() assertFalse(base.selectingItemForTotp) + // The form still belongs to the import, so it keeps claiming back. + assertTrue(base.totpImportActive) assertEquals(DEEP_LINK_URI, base.totpTextFieldState.text.toString()) assertEquals("me@github.com", base.usernameTextFieldState.text.toString()) assertEquals(setOf("github.com"), base.domains.mapTo(mutableSetOf()) { it.value }) @@ -170,6 +174,79 @@ class LoginViewModelTest { assertEquals("me@github.com", state.base.usernameTextFieldState.text.toString()) } + @Test + fun `back stays on the picker instead of leaving the import`() = runVmTest { + totpService.infoFromUriResult = totpInfo(issuer = "github.com") + val viewModel = initWithDeepLink() + val navigation = collectNavigation(viewModel) + + viewModel.onEvent(LoginUiEvent.ItemUi(ItemUiEvent.OnBackClick)) + advanceUntilIdle() + + assertTrue(viewModel.readyBase().selectingItemForTotp) + assertEquals(emptyList(), navigation) + } + + @Test + fun `back from a chosen item returns to the picker with a clean form`() = runVmTest { + val existing = seedLogin(name = "GitHub", domain = "github.com") + totpService.infoFromUriResult = totpInfo(issuer = "github.com") + val viewModel = initWithDeepLink() + val navigation = collectNavigation(viewModel) + viewModel.onEvent(LoginUiEvent.OnTotpModificationItemSelected(existing)) + advanceUntilIdle() + + viewModel.onEvent(LoginUiEvent.ItemUi(ItemUiEvent.OnBackClick)) + advanceUntilIdle() + + val state = viewModel.readyState() + assertTrue(state.base.selectingItemForTotp) + assertFalse(state.base.updating) + assertEquals(setOf(existing), state.base.totpSuggestedItemIds) + assertEquals("", state.shared.nameTextFieldState.text.toString()) + assertEquals("", state.base.totpTextFieldState.text.toString()) + assertEquals("", state.base.usernameTextFieldState.text.toString()) + assertEquals(emptySet(), state.base.domains) + assertEquals(emptyList(), navigation) + } + + @Test + fun `back from a new item returns to the picker`() = runVmTest { + totpService.infoFromUriResult = totpInfo(issuer = "github.com") + val viewModel = initWithDeepLink() + val navigation = collectNavigation(viewModel) + viewModel.onEvent(LoginUiEvent.OnCreateNewItemForTotp) + advanceUntilIdle() + + viewModel.onEvent(LoginUiEvent.ItemUi(ItemUiEvent.OnBackClick)) + advanceUntilIdle() + + val base = viewModel.readyBase() + assertTrue(base.selectingItemForTotp) + assertEquals("", base.totpTextFieldState.text.toString()) + assertEquals(emptyList(), navigation) + } + + @Test + fun `back leaves the screen when the code was scanned into an open form`() = runVmTest { + totpService.infoFromUriResult = totpInfo(issuer = "github.com") + val viewModel = buildViewModel() + backgroundScope.launch(mainDispatcher) { viewModel.state.collect { } } + viewModel.init(DetailPaneInformation.Init.New(VaultItemType.Login)) + advanceUntilIdle() + val navigation = collectNavigation(viewModel) + viewModel.onEvent(LoginUiEvent.OnCodesScanned(listOf(DEEP_LINK_URI))) + advanceUntilIdle() + + viewModel.onEvent(LoginUiEvent.ItemUi(ItemUiEvent.OnBackClick)) + advanceUntilIdle() + + val base = viewModel.readyBase() + assertFalse(base.selectingItemForTotp) + assertFalse(base.totpImportActive) + assertEquals(listOf(null), navigation) + } + @Test fun `an unparsable code shows the error instead of the picker`() = runVmTest { totpService.infoFromUriResult = null @@ -192,6 +269,16 @@ class LoginViewModelTest { return viewModel } + /** + * Records what the screen asks navigation to do. Leaving raises `null`, a saved item raises its + * id, and an empty list is the assertion that the screen stayed where it was. + */ + private fun TestScope.collectNavigation(viewModel: LoginViewModel): List { + val events = mutableListOf() + backgroundScope.launch(mainDispatcher) { viewModel.itemCreatedEvent.collect { events += it } } + return events + } + private fun LoginViewModel.readyState(): ItemUiState.Ready { val state = state.value assertIs>(state) From a81f5da116b50275989d51ce367808962773225a Mon Sep 17 00:00:00 2001 From: Davis Wolfermann Date: Sun, 23 Aug 2026 16:02:13 +0200 Subject: [PATCH 03/20] feat(totp): add a ViewModel for the item picker Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_019NwwBkR3ks2QMJSRmP1iDH --- .../totp/SelectItemForTotpUiState.kt | 15 ++ .../totp/SelectItemForTotpViewModel.kt | 73 ++++++++ .../TestRegistrableDomainResolver.kt | 15 ++ .../presentation/login/LoginViewModelTest.kt | 14 +- .../totp/SelectItemForTotpViewModelTest.kt | 177 ++++++++++++++++++ 5 files changed, 281 insertions(+), 13 deletions(-) create mode 100644 feature/item/create/src/main/kotlin/de/davis/keygo/feature/item/create/presentation/totp/SelectItemForTotpUiState.kt create mode 100644 feature/item/create/src/main/kotlin/de/davis/keygo/feature/item/create/presentation/totp/SelectItemForTotpViewModel.kt create mode 100644 feature/item/create/src/test/java/de/davis/keygo/feature/item/create/presentation/TestRegistrableDomainResolver.kt create mode 100644 feature/item/create/src/test/java/de/davis/keygo/feature/item/create/presentation/totp/SelectItemForTotpViewModelTest.kt diff --git a/feature/item/create/src/main/kotlin/de/davis/keygo/feature/item/create/presentation/totp/SelectItemForTotpUiState.kt b/feature/item/create/src/main/kotlin/de/davis/keygo/feature/item/create/presentation/totp/SelectItemForTotpUiState.kt new file mode 100644 index 000000000..45b0776e6 --- /dev/null +++ b/feature/item/create/src/main/kotlin/de/davis/keygo/feature/item/create/presentation/totp/SelectItemForTotpUiState.kt @@ -0,0 +1,15 @@ +package de.davis.keygo.feature.item.create.presentation.totp + +import de.davis.keygo.core.item.domain.alias.ItemId + +/** + * What the picker knows about the scanned code. + * + * @param suggestedItemIds the logins on the code's own registrable domain, shown first. An empty + * set is the ordinary case for a code whose domain matches nothing, not an error. + * @param parseError the code could not be read at all, so there is nothing to attach anywhere. + */ +internal data class SelectItemForTotpUiState( + val suggestedItemIds: Set = emptySet(), + val parseError: Boolean = false, +) diff --git a/feature/item/create/src/main/kotlin/de/davis/keygo/feature/item/create/presentation/totp/SelectItemForTotpViewModel.kt b/feature/item/create/src/main/kotlin/de/davis/keygo/feature/item/create/presentation/totp/SelectItemForTotpViewModel.kt new file mode 100644 index 000000000..2ebe08aae --- /dev/null +++ b/feature/item/create/src/main/kotlin/de/davis/keygo/feature/item/create/presentation/totp/SelectItemForTotpViewModel.kt @@ -0,0 +1,73 @@ +package de.davis.keygo.feature.item.create.presentation.totp + +import android.util.Log +import androidx.lifecycle.ViewModel +import androidx.lifecycle.viewModelScope +import de.davis.keygo.core.item.domain.alias.ItemId +import de.davis.keygo.core.security.domain.usecase.GetTdlMatchedLoginsUseCase +import de.davis.keygo.core.util.onFailure +import de.davis.keygo.core.util.onSuccess +import de.davis.keygo.feature.item.core.domain.model.resolveTotpDomain +import de.davis.keygo.rust.totp.TotpService +import de.davis.keygo.rust.totp.getInfoFromUriWithResult +import de.davisalessandro.keygo.rust.TotpInfo +import kotlinx.coroutines.flow.MutableStateFlow +import kotlinx.coroutines.flow.StateFlow +import kotlinx.coroutines.flow.asStateFlow +import kotlinx.coroutines.flow.update +import kotlinx.coroutines.launch +import org.koin.core.annotation.InjectedParam +import org.koin.core.annotation.KoinViewModel + +/** + * Backs the screen that asks which login a scanned code belongs to. + * + * The code arrives as a uri rather than as parsed info because it travels through a navigation + * argument, and navigation carries primitives. Parsing it here is also what lets the picker be the + * screen that reports an unreadable code, since it is the first screen the code reaches. + */ +@KoinViewModel +internal class SelectItemForTotpViewModel( + @InjectedParam private val totpUri: String, + private val totpService: TotpService, + private val getTdlMatchedLogins: GetTdlMatchedLoginsUseCase, +) : ViewModel() { + + private val _state = MutableStateFlow(SelectItemForTotpUiState()) + val state: StateFlow = _state.asStateFlow() + + init { + totpService.getInfoFromUriWithResult(totpUri).onFailure { failure -> + Log.e(TAG, "Error parsing TOTP URI: $failure") + _state.update { it.copy(parseError = true) } + }.onSuccess { info -> + viewModelScope.launch { + val suggested = suggestedItemIdsFor(info) + _state.update { it.copy(suggestedItemIds = suggested) } + } + } + } + + fun onParseErrorDismissed() { + _state.update { it.copy(parseError = false) } + } + + /** + * The logins whose registrable domain matches the code's own. + * + * [resolveTotpDomain] is what fills the domain field for a new item, so the suggestions agree + * with it: a code that carries no issuer still matches on the domain in `user@example.com`. + */ + private suspend fun suggestedItemIdsFor(info: TotpInfo): Set { + val domain = resolveTotpDomain( + issuer = info.issuer, + accountName = info.accountName, + ) ?: return emptySet() + + return getTdlMatchedLogins(domain).mapTo(mutableSetOf()) { it.id } + } + + companion object { + private const val TAG = "SelectItemForTotpVM" + } +} diff --git a/feature/item/create/src/test/java/de/davis/keygo/feature/item/create/presentation/TestRegistrableDomainResolver.kt b/feature/item/create/src/test/java/de/davis/keygo/feature/item/create/presentation/TestRegistrableDomainResolver.kt new file mode 100644 index 000000000..af5c7fc21 --- /dev/null +++ b/feature/item/create/src/test/java/de/davis/keygo/feature/item/create/presentation/TestRegistrableDomainResolver.kt @@ -0,0 +1,15 @@ +package de.davis.keygo.feature.item.create.presentation + +import de.davis.keygo.core.util.domain.resolver.RegistrableDomainResolver + +/** Resolves an eTLD+1 by keeping the last two labels, which is enough for the test domains. */ +internal class TestRegistrableDomainResolver : RegistrableDomainResolver { + override fun resolve(domain: String): String? { + val labels = domain.substringAfter("://") + .substringBefore('/') + .split('.') + .filter { it.isNotBlank() } + + return if (labels.size >= 2) labels.takeLast(2).joinToString(".") else null + } +} diff --git a/feature/item/create/src/test/java/de/davis/keygo/feature/item/create/presentation/login/LoginViewModelTest.kt b/feature/item/create/src/test/java/de/davis/keygo/feature/item/create/presentation/login/LoginViewModelTest.kt index b2d29ef92..e8ae73a22 100644 --- a/feature/item/create/src/test/java/de/davis/keygo/feature/item/create/presentation/login/LoginViewModelTest.kt +++ b/feature/item/create/src/test/java/de/davis/keygo/feature/item/create/presentation/login/LoginViewModelTest.kt @@ -21,11 +21,11 @@ import de.davis.keygo.core.security.crypto.FakeCryptographicScopeProvider import de.davis.keygo.core.security.domain.usecase.GetTdlMatchedLoginsUseCase import de.davis.keygo.core.security.domain.usecase.ItemWithCryptoScopeUseCase import de.davis.keygo.core.util.domain.model.snackbar.SnackbarMessage -import de.davis.keygo.core.util.domain.resolver.RegistrableDomainResolver import de.davis.keygo.core.util.domain.snackbar.SnackbarManager import de.davis.keygo.core.util.domain.usecase.SortUseCase import de.davis.keygo.feature.item.core.domain.usecase.CreateNewOrUpdateLoginUseCase import de.davis.keygo.feature.item.core.presentation.model.DetailPaneInformation +import de.davis.keygo.feature.item.create.presentation.TestRegistrableDomainResolver import de.davis.keygo.feature.item.create.presentation.login.model.LoginBaseState import de.davis.keygo.feature.item.create.presentation.login.model.LoginUiEvent import de.davis.keygo.feature.item.create.presentation.model.ItemUiEvent @@ -352,18 +352,6 @@ class LoginViewModelTest { vaultRepository = vaultRepository, ) - /** Resolves an eTLD+1 by keeping the last two labels, which is enough for the test domains. */ - private class TestRegistrableDomainResolver : RegistrableDomainResolver { - override fun resolve(domain: String): String? { - val labels = domain.substringAfter("://") - .substringBefore('/') - .split('.') - .filter { it.isNotBlank() } - - return if (labels.size >= 2) labels.takeLast(2).joinToString(".") else null - } - } - private class TestSnackbarManager : SnackbarManager { override val oneShotEvents: Flow = emptyFlow() override fun sendMessage(message: SnackbarMessage) = Unit diff --git a/feature/item/create/src/test/java/de/davis/keygo/feature/item/create/presentation/totp/SelectItemForTotpViewModelTest.kt b/feature/item/create/src/test/java/de/davis/keygo/feature/item/create/presentation/totp/SelectItemForTotpViewModelTest.kt new file mode 100644 index 000000000..e84de6adc --- /dev/null +++ b/feature/item/create/src/test/java/de/davis/keygo/feature/item/create/presentation/totp/SelectItemForTotpViewModelTest.kt @@ -0,0 +1,177 @@ +package de.davis.keygo.feature.item.create.presentation.totp + +import de.davis.keygo.core.item.FakeItemRepository +import de.davis.keygo.core.item.FakeLoginRepository +import de.davis.keygo.core.item.domain.alias.ItemId +import de.davis.keygo.core.item.domain.alias.newItemId +import de.davis.keygo.core.item.domain.alias.newVaultId +import de.davis.keygo.core.item.domain.model.DomainInfo +import de.davis.keygo.core.item.domain.model.KeyInformation +import de.davis.keygo.core.item.domain.model.Login +import de.davis.keygo.core.item.domain.model.Timestamp +import de.davis.keygo.core.security.domain.usecase.GetTdlMatchedLoginsUseCase +import de.davis.keygo.feature.item.create.presentation.TestRegistrableDomainResolver +import de.davis.keygo.rust.FakeTotpService +import de.davisalessandro.keygo.rust.Algorithm +import de.davisalessandro.keygo.rust.TotpInfo +import kotlinx.coroutines.Dispatchers +import kotlinx.coroutines.ExperimentalCoroutinesApi +import kotlinx.coroutines.test.TestScope +import kotlinx.coroutines.test.UnconfinedTestDispatcher +import kotlinx.coroutines.test.advanceUntilIdle +import kotlinx.coroutines.test.resetMain +import kotlinx.coroutines.test.runTest +import kotlinx.coroutines.test.setMain +import org.junit.runner.RunWith +import org.robolectric.RobolectricTestRunner +import org.robolectric.annotation.Config +import kotlin.test.AfterTest +import kotlin.test.BeforeTest +import kotlin.test.Test +import kotlin.test.assertEquals +import kotlin.test.assertFalse +import kotlin.test.assertTrue + +/** + * Covers what the picker knows before the user has chosen anything: which logins the scanned code + * points at, and what happens when the code cannot be read at all. + */ +@OptIn(ExperimentalCoroutinesApi::class) +@RunWith(RobolectricTestRunner::class) +@Config(sdk = [34]) +class SelectItemForTotpViewModelTest { + + private val mainDispatcher = UnconfinedTestDispatcher() + + private val vaultId = newVaultId() + private val loginRepository = FakeLoginRepository() + private val itemRepository = FakeItemRepository(loginRepository) + private val totpService = FakeTotpService() + private val domainResolver = TestRegistrableDomainResolver() + + @BeforeTest + fun setUp() { + Dispatchers.setMain(mainDispatcher) + } + + @AfterTest + fun tearDown() { + Dispatchers.resetMain() + } + + @Test + fun `logins on the code's domain are suggested`() = runVmTest { + val onDomain = seedLogin(name = "GitHub", domain = "github.com") + seedLogin(name = "Google", domain = "google.com") + totpService.infoFromUriResult = totpInfo(issuer = "github.com") + + val viewModel = buildViewModel() + advanceUntilIdle() + + assertEquals(setOf(onDomain), viewModel.state.value.suggestedItemIds) + } + + @Test + fun `a code without an issuer suggests on the account name's domain`() = runVmTest { + val onDomain = seedLogin(name = "GitHub", domain = "github.com") + totpService.infoFromUriResult = totpInfo(issuer = null, accountName = "me@github.com") + + val viewModel = buildViewModel() + advanceUntilIdle() + + assertEquals(setOf(onDomain), viewModel.state.value.suggestedItemIds) + } + + @Test + fun `a code that matches no domain suggests nothing`() = runVmTest { + seedLogin(name = "Google", domain = "google.com") + totpService.infoFromUriResult = totpInfo(issuer = null, accountName = "no-domain-here") + + val viewModel = buildViewModel() + advanceUntilIdle() + + val state = viewModel.state.value + assertEquals(emptySet(), state.suggestedItemIds) + assertFalse(state.parseError) + } + + @Test + fun `an unreadable code surfaces the parse error`() = runVmTest { + totpService.infoFromUriResult = null + + val viewModel = buildViewModel() + advanceUntilIdle() + + val state = viewModel.state.value + assertTrue(state.parseError) + assertEquals(emptySet(), state.suggestedItemIds) + } + + @Test + fun `dismissing the parse error clears it`() = runVmTest { + totpService.infoFromUriResult = null + val viewModel = buildViewModel() + advanceUntilIdle() + + viewModel.onParseErrorDismissed() + advanceUntilIdle() + + assertFalse(viewModel.state.value.parseError) + } + + // Helpers + + private fun runVmTest(body: suspend TestScope.() -> Unit) = + runTest(mainDispatcher.scheduler) { body() } + + private fun buildViewModel(uri: String = DEEP_LINK_URI) = SelectItemForTotpViewModel( + totpUri = uri, + totpService = totpService, + getTdlMatchedLogins = GetTdlMatchedLoginsUseCase(domainResolver, loginRepository), + ) + + private fun seedLogin(name: String, domain: String): ItemId { + val id = newItemId() + loginRepository.seed( + Login( + id = id, + name = name, + username = null, + domainInfos = setOf( + DomainInfo( + loginId = id, + value = domain, + eTLD1 = domainResolver.resolve(domain), + ), + ), + passwordCredential = null, + totp = null, + passkeys = emptySet(), + note = null, + pinned = false, + vaultId = vaultId, + keyInformation = KeyInformation(byteArrayOf(), byteArrayOf()), + timestamp = Timestamp(), + ), + ) + return id + } + + private fun totpInfo( + issuer: String?, + accountName: String = "me@github.com", + secret: String = "JBSWY3DPEHPK3PXP", + ) = TotpInfo( + secret = secret, + issuer = issuer, + accountName = accountName, + algorithm = Algorithm.SHA1, + digits = 6, + period = 30, + ) + + companion object { + private const val DEEP_LINK_URI = + "otpauth://totp/GitHub:me@github.com?secret=JBSWY3DPEHPK3PXP&issuer=github.com" + } +} From 076ab3704f801dc970734822a33af7b2540951e7 Mon Sep 17 00:00:00 2001 From: Davis Wolfermann Date: Sun, 23 Aug 2026 16:45:13 +0200 Subject: [PATCH 04/20] feat(totp): let the login form take a pending code Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_019NwwBkR3ks2QMJSRmP1iDH --- .../model/DetailPaneInformation.kt | 17 +++- .../presentation/login/LoginViewModel.kt | 25 +++++- .../presentation/login/LoginViewModelTest.kt | 80 ++++++++++++++++++- 3 files changed, 116 insertions(+), 6 deletions(-) diff --git a/feature/item/core/src/main/kotlin/de/davis/keygo/feature/item/core/presentation/model/DetailPaneInformation.kt b/feature/item/core/src/main/kotlin/de/davis/keygo/feature/item/core/presentation/model/DetailPaneInformation.kt index 710f823ed..dd380953e 100644 --- a/feature/item/core/src/main/kotlin/de/davis/keygo/feature/item/core/presentation/model/DetailPaneInformation.kt +++ b/feature/item/core/src/main/kotlin/de/davis/keygo/feature/item/core/presentation/model/DetailPaneInformation.kt @@ -8,8 +8,21 @@ sealed interface DetailPaneInformation { sealed interface Init : DetailPaneInformation { val itemType: VaultItemType - data class New(override val itemType: VaultItemType) : Init - data class Existing(override val itemType: VaultItemType, val id: ItemId) : Init + /** + * @param pendingTotpUri a scanned code the form should fold in once it is built. Null for + * an ordinary create or edit. + */ + data class New( + override val itemType: VaultItemType, + val pendingTotpUri: String? = null, + ) : Init + + data class Existing( + override val itemType: VaultItemType, + val id: ItemId, + val pendingTotpUri: String? = null, + ) : Init + data class TOTP(override val itemType: VaultItemType, val uri: String) : Init } diff --git a/feature/item/create/src/main/kotlin/de/davis/keygo/feature/item/create/presentation/login/LoginViewModel.kt b/feature/item/create/src/main/kotlin/de/davis/keygo/feature/item/create/presentation/login/LoginViewModel.kt index 8b1327e3c..15e41d12d 100644 --- a/feature/item/create/src/main/kotlin/de/davis/keygo/feature/item/create/presentation/login/LoginViewModel.kt +++ b/feature/item/create/src/main/kotlin/de/davis/keygo/feature/item/create/presentation/login/LoginViewModel.kt @@ -144,9 +144,16 @@ internal class LoginViewModel( fun init(information: DetailPaneInformation) { when (information) { - is DetailPaneInformation.Init.Existing -> viewModelScope.launch { initWithId(information.id) } + is DetailPaneInformation.Init.Existing -> viewModelScope.launch { + information.pendingTotpUri?.let { parsePendingTotp(it) } + initWithId(information.id) + } + is DetailPaneInformation.Init.TOTP -> initWithTotpUri(information.uri) - is DetailPaneInformation.Init.New -> {} // Don't init anything + + is DetailPaneInformation.Init.New -> information.pendingTotpUri?.let { uri -> + parsePendingTotp(uri)?.let { updateUiWithTotpSecretInfo(it, uri) } + } is DetailPaneInformation.CreateRaw -> initWithRawItem(information) } @@ -235,6 +242,20 @@ internal class LoginViewModel( } } + /** + * Reads a code the picker handed over and remembers it, so [initWithId] can fold it into + * whichever login was chosen. Returns null when the code cannot be read, having already put the + * parse error on screen. + */ + private fun parsePendingTotp(uri: String): TotpInfo? = + totpService.getInfoFromUriWithResult(uri).onFailure { failure -> + Log.e(TAG, "Error parsing TOTP URI: $failure") + showTotpParseError() + }.getOrNull()?.also { + totpSecretInformation = it + totpOriginalUri = uri + } + private fun initWithTotpUri(totpUri: String) { totpService.getInfoFromUriWithResult(totpUri).onFailure { Log.e(TAG, "Error parsing TOTP URI: $it") diff --git a/feature/item/create/src/test/java/de/davis/keygo/feature/item/create/presentation/login/LoginViewModelTest.kt b/feature/item/create/src/test/java/de/davis/keygo/feature/item/create/presentation/login/LoginViewModelTest.kt index e8ae73a22..17cf41e2a 100644 --- a/feature/item/create/src/test/java/de/davis/keygo/feature/item/create/presentation/login/LoginViewModelTest.kt +++ b/feature/item/create/src/test/java/de/davis/keygo/feature/item/create/presentation/login/LoginViewModelTest.kt @@ -24,8 +24,10 @@ import de.davis.keygo.core.util.domain.model.snackbar.SnackbarMessage import de.davis.keygo.core.util.domain.snackbar.SnackbarManager import de.davis.keygo.core.util.domain.usecase.SortUseCase import de.davis.keygo.feature.item.core.domain.usecase.CreateNewOrUpdateLoginUseCase +import de.davis.keygo.feature.item.core.presentation.login.model.FieldType import de.davis.keygo.feature.item.core.presentation.model.DetailPaneInformation import de.davis.keygo.feature.item.create.presentation.TestRegistrableDomainResolver +import de.davis.keygo.feature.item.create.presentation.login.model.DialogState import de.davis.keygo.feature.item.create.presentation.login.model.LoginBaseState import de.davis.keygo.feature.item.create.presentation.login.model.LoginUiEvent import de.davis.keygo.feature.item.create.presentation.model.ItemUiEvent @@ -256,6 +258,76 @@ class LoginViewModelTest { assertFalse(viewModel.readyBase().selectingItemForTotp) } + @Test + fun `a new item is prefilled from the picker's code`() = runVmTest { + totpService.infoFromUriResult = totpInfo(issuer = "github.com") + + val viewModel = buildViewModel() + backgroundScope.launch(mainDispatcher) { viewModel.state.collect { } } + viewModel.init( + DetailPaneInformation.Init.New( + itemType = VaultItemType.Login, + pendingTotpUri = DEEP_LINK_URI, + ), + ) + advanceUntilIdle() + + val base = viewModel.readyBase() + assertEquals(DEEP_LINK_URI, base.totpTextFieldState.text.toString()) + assertEquals("me@github.com", base.usernameTextFieldState.text.toString()) + assertEquals(setOf("github.com"), base.domains.mapTo(mutableSetOf()) { it.value }) + } + + @Test + fun `an existing item is loaded and the picker's code folded in`() = runVmTest { + val existing = seedLogin(name = "GitHub", domain = "github.com") + totpService.infoFromUriResult = totpInfo(issuer = "github.com") + + val viewModel = buildViewModel() + backgroundScope.launch(mainDispatcher) { viewModel.state.collect { } } + viewModel.init( + DetailPaneInformation.Init.Existing( + itemType = VaultItemType.Login, + id = existing, + pendingTotpUri = DEEP_LINK_URI, + ), + ) + advanceUntilIdle() + + val state = viewModel.readyState() + assertTrue(state.base.updating) + assertEquals("GitHub", state.shared.nameTextFieldState.text.toString()) + assertEquals(DEEP_LINK_URI, state.base.totpTextFieldState.text.toString()) + assertEquals("me@github.com", state.base.usernameTextFieldState.text.toString()) + } + + @Test + fun `a code that collides with the chosen item raises the override dialog`() = runVmTest { + val existing = seedLogin( + name = "GitHub", + domain = "github.com", + username = "old@github.com", + ) + totpService.infoFromUriResult = totpInfo(issuer = "github.com") + + val viewModel = buildViewModel() + backgroundScope.launch(mainDispatcher) { viewModel.state.collect { } } + viewModel.init( + DetailPaneInformation.Init.Existing( + itemType = VaultItemType.Login, + id = existing, + pendingTotpUri = DEEP_LINK_URI, + ), + ) + advanceUntilIdle() + + val dialog = viewModel.readyBase().dialogState + assertIs(dialog) + val usernameField = dialog.fields.single { it.fieldType == FieldType.Username } + assertEquals("old@github.com", usernameField.before) + assertEquals("me@github.com", usernameField.after) + } + // Helpers private fun runVmTest(body: suspend TestScope.() -> Unit) = @@ -287,13 +359,17 @@ class LoginViewModelTest { private fun LoginViewModel.readyBase(): LoginBaseState = readyState().base - private fun seedLogin(name: String, domain: String): ItemId { + private fun seedLogin( + name: String, + domain: String, + username: String? = null, + ): ItemId { val id = newItemId() loginRepository.seed( Login( id = id, name = name, - username = null, + username = username, domainInfos = setOf( DomainInfo( loginId = id, From e1dddd3d94cb04631d1bce8dc2cd50a98bf84872 Mon Sep 17 00:00:00 2001 From: Davis Wolfermann Date: Sun, 23 Aug 2026 16:59:01 +0200 Subject: [PATCH 05/20] feat(totp): give the item picker its own destination Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_019NwwBkR3ks2QMJSRmP1iDH --- .../keygo/app/presentation/MainActivity.kt | 14 ++- .../keygo/app/presentation/TotpImportGraph.kt | 77 +++++++++++++ .../presentation/TotpImportNavGraphTest.kt | 60 ++++++++++ .../totp/SelectItemForTotpScreen.kt | 108 ++++++++++++++++++ 4 files changed, 257 insertions(+), 2 deletions(-) create mode 100644 app/src/main/kotlin/de/davis/keygo/app/presentation/TotpImportGraph.kt create mode 100644 feature/item/create/src/main/kotlin/de/davis/keygo/feature/item/create/presentation/totp/SelectItemForTotpScreen.kt 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 119e8637d..c8a8d60c6 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 @@ -153,10 +153,20 @@ private fun App(hasAccess: Boolean) { } ) + totpImportGraph( + navigateToDestination = navController::navigate, + onImportFinished = { + navController.navigate(RouteDestination.TopLevelAppGraph) { + popUpTo { inclusive = true } + } + }, + navigateUp = { navController.navigateUp() }, + ) + authGraph( onSuccess = { totpUri -> val dest = totpUri?.let { - RouteDestination.Home.Root(it) + SelectItemForTotpRoute(it) } ?: RouteDestination.TopLevelAppGraph navController.navigate(dest) { @@ -168,7 +178,7 @@ private fun App(hasAccess: Boolean) { onboardingGraph( onSuccess = { totpUri -> val dest = totpUri?.let { - RouteDestination.Home.Root(it) + SelectItemForTotpRoute(it) } ?: RouteDestination.TopLevelAppGraph navController.navigate(dest) { diff --git a/app/src/main/kotlin/de/davis/keygo/app/presentation/TotpImportGraph.kt b/app/src/main/kotlin/de/davis/keygo/app/presentation/TotpImportGraph.kt new file mode 100644 index 000000000..0945bf875 --- /dev/null +++ b/app/src/main/kotlin/de/davis/keygo/app/presentation/TotpImportGraph.kt @@ -0,0 +1,77 @@ +package de.davis.keygo.app.presentation + +import androidx.navigation.NavGraphBuilder +import androidx.navigation.compose.composable +import androidx.navigation.toRoute +import de.davis.keygo.core.item.domain.alias.ItemId +import de.davis.keygo.core.item.generated.domain.model.VaultItemType +import de.davis.keygo.core.ui.RouteDestination +import de.davis.keygo.feature.item.core.presentation.model.DetailPaneInformation +import de.davis.keygo.feature.item.create.presentation.login.LoginScreen +import de.davis.keygo.feature.item.create.presentation.totp.SelectItemForTotpScreen +import kotlinx.serialization.Serializable +import java.util.UUID + +/** + * Where a deep-linked code lands once the user is through the door. The uri travels whole, unlike + * on [de.davis.keygo.feature.auth.presentation.AuthRoute], because only the deep link's own + * `otpauth://totp/{totpInfo}?{queries}` pattern forces that split. + */ +@Serializable +data class SelectItemForTotpRoute(val totpUri: String) : RouteDestination + +/** + * The login form for a chosen item, or for a new one when [itemId] is null. + * + * The id travels as a String because [ItemId] is a [UUID] and type-safe navigation has no + * [androidx.navigation.NavType] for it. Supplying one through a typeMap for a single nullable id + * costs more than the conversion does. + */ +@Serializable +data class AssignTotpRoute( + val totpUri: String, + val itemId: String? = null, +) : RouteDestination { + val selectedItemId: ItemId? + get() = itemId?.let(UUID::fromString) +} + +/** + * @param onImportFinished the import is over, either saved or abandoned. Leads back into the app + * with the import routes popped, so back does not return the user to a code they already handled. + */ +fun NavGraphBuilder.totpImportGraph( + navigateToDestination: (Any) -> Unit, + onImportFinished: () -> Unit, + navigateUp: () -> Unit, +) { + composable { entry -> + val route = entry.toRoute() + SelectItemForTotpScreen( + totpUri = route.totpUri, + onItemSelected = { itemId -> + navigateToDestination(AssignTotpRoute(route.totpUri, itemId.toString())) + }, + onCreateNew = { navigateToDestination(AssignTotpRoute(route.totpUri)) }, + onImportAbandoned = onImportFinished, + ) + } + + composable { entry -> + val route = entry.toRoute() + LoginScreen( + detailPaneInformation = route.selectedItemId?.let { itemId -> + DetailPaneInformation.Init.Existing( + itemType = VaultItemType.Login, + id = itemId, + pendingTotpUri = route.totpUri, + ) + } ?: DetailPaneInformation.Init.New( + itemType = VaultItemType.Login, + pendingTotpUri = route.totpUri, + ), + loginCreated = { onImportFinished() }, + navigateBack = navigateUp, + ) + } +} diff --git a/app/src/test/kotlin/de/davis/keygo/app/presentation/TotpImportNavGraphTest.kt b/app/src/test/kotlin/de/davis/keygo/app/presentation/TotpImportNavGraphTest.kt index b6c381712..b1d1003bd 100644 --- a/app/src/test/kotlin/de/davis/keygo/app/presentation/TotpImportNavGraphTest.kt +++ b/app/src/test/kotlin/de/davis/keygo/app/presentation/TotpImportNavGraphTest.kt @@ -8,6 +8,7 @@ import androidx.navigation.createGraph import androidx.navigation.testing.TestNavHostController import androidx.navigation.toRoute import androidx.test.core.app.ApplicationProvider +import de.davis.keygo.core.item.domain.alias.newItemId import de.davis.keygo.core.ui.model.PendingTotpImport import de.davis.keygo.feature.auth.presentation.AuthRoute import de.davis.keygo.feature.auth.presentation.authGraph @@ -18,6 +19,7 @@ import org.robolectric.RobolectricTestRunner import org.robolectric.annotation.Config import kotlin.test.Test import kotlin.test.assertEquals +import kotlin.test.assertFalse import kotlin.test.assertNotNull import kotlin.test.assertNull import kotlin.test.assertTrue @@ -36,6 +38,11 @@ class TotpImportNavGraphTest { startDestination = if (hasAccess) AuthRoute() else OnboardingRoute(), ) { totpImportRedirectGraph(hasAccess = hasAccess, navigateAndReplace = {}) + totpImportGraph( + navigateToDestination = {}, + onImportFinished = {}, + navigateUp = {}, + ) authGraph(onSuccess = {}) onboardingGraph(onSuccess = {}) } @@ -117,4 +124,57 @@ class TotpImportNavGraphTest { assertEquals(PendingTotpImport(), route.pendingTotpImport) assertNull(route.uri) } + + @Test + fun `the picker route carries the whole uri`() { + val controller = navController(hasAccess = true) + + controller.navigate(SelectItemForTotpRoute(DEEP_LINK_URI)) + + val entry = assertNotNull(controller.currentBackStackEntry) + assertTrue(entry.destination.hasRoute()) + assertEquals(DEEP_LINK_URI, entry.toRoute().totpUri) + } + + @Test + fun `choosing an item carries its id to the form`() { + val controller = navController(hasAccess = true) + val itemId = newItemId() + + controller.navigate(AssignTotpRoute(DEEP_LINK_URI, itemId.toString())) + + val route = assertNotNull(controller.currentBackStackEntry).toRoute() + assertEquals(DEEP_LINK_URI, route.totpUri) + assertEquals(itemId, route.selectedItemId) + } + + @Test + fun `creating a new item carries no id`() { + val controller = navController(hasAccess = true) + + controller.navigate(AssignTotpRoute(DEEP_LINK_URI)) + + val route = assertNotNull(controller.currentBackStackEntry).toRoute() + assertEquals(DEEP_LINK_URI, route.totpUri) + assertNull(route.selectedItemId) + } + + @Test + fun `the picker replaces the auth entry so back leaves the app`() { + val controller = navController(hasAccess = true) + + controller.navigate(SelectItemForTotpRoute(DEEP_LINK_URI)) { + popUpTo { inclusive = true } + } + + assertTrue(controller.currentDestination?.hasRoute() == true) + assertFalse( + controller.currentBackStack.value.any { it.destination.hasRoute() }, + ) + } + + private companion object { + const val DEEP_LINK_URI = + "otpauth://totp/GitHub:me@github.com?secret=JBSWY3DPEHPK3PXP&issuer=github.com" + } } diff --git a/feature/item/create/src/main/kotlin/de/davis/keygo/feature/item/create/presentation/totp/SelectItemForTotpScreen.kt b/feature/item/create/src/main/kotlin/de/davis/keygo/feature/item/create/presentation/totp/SelectItemForTotpScreen.kt new file mode 100644 index 000000000..4602113af --- /dev/null +++ b/feature/item/create/src/main/kotlin/de/davis/keygo/feature/item/create/presentation/totp/SelectItemForTotpScreen.kt @@ -0,0 +1,108 @@ +package de.davis.keygo.feature.item.create.presentation.totp + +import androidx.compose.foundation.layout.consumeWindowInsets +import androidx.compose.foundation.layout.fillMaxSize +import androidx.compose.foundation.layout.fillMaxWidth +import androidx.compose.foundation.layout.padding +import androidx.compose.material.icons.Icons +import androidx.compose.material.icons.filled.Add +import androidx.compose.material3.ExperimentalMaterial3Api +import androidx.compose.material3.FloatingActionButton +import androidx.compose.material3.Icon +import androidx.compose.material3.Scaffold +import androidx.compose.material3.Text +import androidx.compose.material3.TopAppBar +import androidx.compose.runtime.Composable +import androidx.compose.runtime.getValue +import androidx.compose.ui.Modifier +import androidx.compose.ui.res.stringResource +import androidx.lifecycle.compose.collectAsStateWithLifecycle +import de.davis.keygo.core.item.domain.alias.ItemId +import de.davis.keygo.core.item.generated.domain.model.VaultItemType +import de.davis.keygo.feature.item.create.R +import de.davis.keygo.feature.list_screen.presentation.ItemListScreen +import de.davis.keygo.feature.list_screen.presentation.NoItemStrategy +import de.davis.keygo.feature.totp.presentation.component.TotpParseErrorDialog +import org.koin.androidx.compose.koinViewModel +import org.koin.core.parameter.parametersOf + +/** + * Asks which item a scanned code belongs to, listing every login with the ones on the code's own + * domain grouped first. Only logins are listed, because a TOTP secret can only be attached to one. + * + * The screen carries no back affordance and no back handler. It is the first step of a deep-linked + * import, which replaced the whole back stack on its way here, so the NavHost does not consume a + * back press and the activity finishes on its own. + * + * @param onImportAbandoned the code could not be read, so there is nothing to attach and the flow + * ends. The user stays in the app they just unlocked rather than being thrown out over a malformed + * code from somewhere else. + */ +@Composable +fun SelectItemForTotpScreen( + totpUri: String, + onItemSelected: (ItemId) -> Unit, + onCreateNew: () -> Unit, + onImportAbandoned: () -> Unit, + modifier: Modifier = Modifier, +) { + val viewModel: SelectItemForTotpViewModel = koinViewModel { parametersOf(totpUri) } + val state by viewModel.state.collectAsStateWithLifecycle() + + SelectItemForTotpContent( + state = state, + onItemSelected = onItemSelected, + onCreateNew = onCreateNew, + onParseErrorDismiss = { + viewModel.onParseErrorDismissed() + onImportAbandoned() + }, + modifier = modifier, + ) +} + +@OptIn(ExperimentalMaterial3Api::class) +@Composable +private fun SelectItemForTotpContent( + state: SelectItemForTotpUiState, + onItemSelected: (ItemId) -> Unit, + onCreateNew: () -> Unit, + onParseErrorDismiss: () -> Unit, + modifier: Modifier = Modifier, +) { + Scaffold( + modifier = modifier.fillMaxSize(), + topBar = { + TopAppBar(title = { Text(text = stringResource(R.string.select_item_for_totp)) }) + }, + floatingActionButton = { + FloatingActionButton(onClick = onCreateNew) { + Icon( + imageVector = Icons.Default.Add, + contentDescription = stringResource(R.string.create_new), + ) + } + }, + ) { innerPadding -> + ItemListScreen( + onItemClick = onItemSelected, + onItemLongClick = { }, + onCreateItemRequest = { onCreateNew() }, + restrictedItemType = VaultItemType.Login, + suggestedItemIds = state.suggestedItemIds, + notFoundStrategy = NoItemStrategy.ShowMessage, + enableDeletion = false, + enableSelection = false, + dockedSearchResults = false, + modifier = Modifier + .consumeWindowInsets(innerPadding) + .padding(innerPadding), + ) + } + + if (state.parseError) + TotpParseErrorDialog( + onDismiss = onParseErrorDismiss, + modifier = Modifier.fillMaxWidth(), + ) +} From 2fd8d1ddca2f30782b92dac8dc2f3e7913254158 Mon Sep 17 00:00:00 2001 From: Davis Wolfermann Date: Sun, 23 Aug 2026 17:16:10 +0200 Subject: [PATCH 06/20] refactor(totp): drop the overlay item picker The picker is a destination now, so the login form no longer has to fake one: returning to it pops an entry instead of hand-resetting every field. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_019NwwBkR3ks2QMJSRmP1iDH --- .../keygo/app/presentation/MainActivity.kt | 2 +- .../presentation/model/RouteDestination.kt | 2 +- .../dashboard/presentation/DashboardGraph.kt | 13 +- .../dashboard/presentation/DetailType.kt | 7 - .../model/DetailPaneInformation.kt | 12 +- .../component/SelectItemForTotpScreen.kt | 93 ---------- .../creditcard/CreditCardViewModel.kt | 1 - .../create/presentation/login/LoginContent.kt | 30 ---- .../create/presentation/login/LoginScreen.kt | 1 - .../presentation/login/LoginViewModel.kt | 110 ------------ .../presentation/login/model/LoginUiEvent.kt | 3 - .../presentation/login/model/LoginUiState.kt | 10 -- .../presentation/login/LoginViewModelTest.kt | 161 +----------------- 13 files changed, 11 insertions(+), 434 deletions(-) delete mode 100644 feature/item/create/src/main/kotlin/de/davis/keygo/feature/item/create/presentation/component/SelectItemForTotpScreen.kt 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 c8a8d60c6..8ca0bf45b 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 @@ -191,7 +191,7 @@ private fun App(hasAccess: Boolean) { startDestination = RouteDestination.Home.NavGraph ) { navigation( - startDestination = RouteDestination.Home.Root() + startDestination = RouteDestination.Home.Root ) { dialog { SelectItemContent( diff --git a/app/src/main/kotlin/de/davis/keygo/core/presentation/model/RouteDestination.kt b/app/src/main/kotlin/de/davis/keygo/core/presentation/model/RouteDestination.kt index fb3e4bb33..21399bbc3 100644 --- a/app/src/main/kotlin/de/davis/keygo/core/presentation/model/RouteDestination.kt +++ b/app/src/main/kotlin/de/davis/keygo/core/presentation/model/RouteDestination.kt @@ -20,7 +20,7 @@ sealed interface RouteDestination : UiRouteDestination { data object NavGraph : Home @Serializable - data class Root(val totpUri: String? = null) : Home + data object Root : Home @Serializable data object SelectItem : Home diff --git a/app/src/main/kotlin/de/davis/keygo/dashboard/presentation/DashboardGraph.kt b/app/src/main/kotlin/de/davis/keygo/dashboard/presentation/DashboardGraph.kt index a6b867980..6f040f114 100644 --- a/app/src/main/kotlin/de/davis/keygo/dashboard/presentation/DashboardGraph.kt +++ b/app/src/main/kotlin/de/davis/keygo/dashboard/presentation/DashboardGraph.kt @@ -19,7 +19,6 @@ import androidx.lifecycle.ViewModelStoreOwner import androidx.lifecycle.viewmodel.compose.LocalViewModelStoreOwner import androidx.navigation.NavGraphBuilder import androidx.navigation.compose.composable -import androidx.navigation.toRoute import de.davis.keygo.core.presentation.model.RouteDestination import de.davis.keygo.core.ui.composition.LocalIsInSinglePaneMode import de.davis.keygo.feature.item.core.presentation.model.NavigationEvent @@ -32,7 +31,7 @@ import kotlinx.coroutines.launch fun NavGraphBuilder.dashboardGraph( listNavigator: ThreePaneScaffoldNavigator, ) { - composable { + composable { _ -> val isSinglePaneMode by remember(listNavigator.scaffoldDirective) { derivedStateOf { listNavigator.scaffoldDirective.maxHorizontalPartitions == 1 @@ -58,16 +57,6 @@ fun NavGraphBuilder.dashboardGraph( } } - val route = it.toRoute() - LaunchedEffect(route) { - route.totpUri?.let { totpUri -> - listNavigator.navigateTo( - ListDetailPaneScaffoldRole.Detail, - DetailType.Modify.Totp(totpUri) - ) - } - } - CompositionLocalProvider( LocalIsInSinglePaneMode provides isSinglePaneMode, ) { diff --git a/app/src/main/kotlin/de/davis/keygo/dashboard/presentation/DetailType.kt b/app/src/main/kotlin/de/davis/keygo/dashboard/presentation/DetailType.kt index 9f9bf47e8..09a058634 100644 --- a/app/src/main/kotlin/de/davis/keygo/dashboard/presentation/DetailType.kt +++ b/app/src/main/kotlin/de/davis/keygo/dashboard/presentation/DetailType.kt @@ -4,7 +4,6 @@ import android.os.Parcelable import de.davis.keygo.core.item.domain.alias.ItemId import de.davis.keygo.core.item.generated.domain.model.VaultItemType import de.davis.keygo.feature.item.core.presentation.model.DetailPaneInformation -import kotlinx.parcelize.IgnoredOnParcel import kotlinx.parcelize.Parcelize @Parcelize @@ -16,11 +15,6 @@ sealed interface DetailType : Parcelable { data class CreateNew(override val vaultItemType: VaultItemType) : Modify data class Edit(override val vaultItemType: VaultItemType, val itemId: ItemId) : Modify - data class Totp(val uri: String) : Modify { - - @IgnoredOnParcel - override val vaultItemType: VaultItemType = VaultItemType.Login - } } data class View(val itemId: ItemId) : DetailType @@ -29,5 +23,4 @@ sealed interface DetailType : Parcelable { fun DetailType.Modify.asDetailPaneInformation() = when (this) { is DetailType.Modify.CreateNew -> DetailPaneInformation.Init.New(vaultItemType) is DetailType.Modify.Edit -> DetailPaneInformation.Init.Existing(vaultItemType, itemId) - is DetailType.Modify.Totp -> DetailPaneInformation.Init.TOTP(vaultItemType, uri) } \ No newline at end of file diff --git a/feature/item/core/src/main/kotlin/de/davis/keygo/feature/item/core/presentation/model/DetailPaneInformation.kt b/feature/item/core/src/main/kotlin/de/davis/keygo/feature/item/core/presentation/model/DetailPaneInformation.kt index dd380953e..8e2332529 100644 --- a/feature/item/core/src/main/kotlin/de/davis/keygo/feature/item/core/presentation/model/DetailPaneInformation.kt +++ b/feature/item/core/src/main/kotlin/de/davis/keygo/feature/item/core/presentation/model/DetailPaneInformation.kt @@ -9,21 +9,21 @@ sealed interface DetailPaneInformation { val itemType: VaultItemType /** - * @param pendingTotpUri a scanned code the form should fold in once it is built. Null for - * an ordinary create or edit. + * A scanned code the form should fold in once it is built. Null for an ordinary create or + * edit. */ + val pendingTotpUri: String? + data class New( override val itemType: VaultItemType, - val pendingTotpUri: String? = null, + override val pendingTotpUri: String? = null, ) : Init data class Existing( override val itemType: VaultItemType, val id: ItemId, - val pendingTotpUri: String? = null, + override val pendingTotpUri: String? = null, ) : Init - - data class TOTP(override val itemType: VaultItemType, val uri: String) : Init } @Serializable diff --git a/feature/item/create/src/main/kotlin/de/davis/keygo/feature/item/create/presentation/component/SelectItemForTotpScreen.kt b/feature/item/create/src/main/kotlin/de/davis/keygo/feature/item/create/presentation/component/SelectItemForTotpScreen.kt deleted file mode 100644 index af87fc6b0..000000000 --- a/feature/item/create/src/main/kotlin/de/davis/keygo/feature/item/create/presentation/component/SelectItemForTotpScreen.kt +++ /dev/null @@ -1,93 +0,0 @@ -package de.davis.keygo.feature.item.create.presentation.component - -import androidx.activity.compose.BackHandler -import androidx.activity.compose.LocalActivity -import androidx.compose.foundation.layout.Box -import androidx.compose.foundation.layout.consumeWindowInsets -import androidx.compose.foundation.layout.fillMaxSize -import androidx.compose.foundation.layout.padding -import androidx.compose.material.icons.Icons -import androidx.compose.material.icons.filled.Add -import androidx.compose.material3.ContainedLoadingIndicator -import androidx.compose.material3.ExperimentalMaterial3Api -import androidx.compose.material3.ExperimentalMaterial3ExpressiveApi -import androidx.compose.material3.FloatingActionButton -import androidx.compose.material3.Icon -import androidx.compose.material3.Scaffold -import androidx.compose.material3.Text -import androidx.compose.material3.TopAppBar -import androidx.compose.runtime.Composable -import androidx.compose.ui.Alignment -import androidx.compose.ui.Modifier -import androidx.compose.ui.res.stringResource -import de.davis.keygo.core.item.domain.alias.ItemId -import de.davis.keygo.core.item.generated.domain.model.VaultItemType -import de.davis.keygo.feature.item.create.R -import de.davis.keygo.feature.list_screen.presentation.ItemListScreen -import de.davis.keygo.feature.list_screen.presentation.NoItemStrategy - -/** - * Asks which item a scanned code belongs to, listing every login with the ones on the code's own - * domain grouped first. - * - * The screen carries no back affordance. It is the first step of a deep-linked import, which - * replaced the whole back stack on its way here, so there is nothing behind it: back leaves the app - * rather than revealing a dashboard the user never opened. - * - * @param loading shows the chrome without the list, for the moment before the vault is readable. - * Choosing anything then would have nothing to choose from, so both actions are withheld too. - */ -@OptIn(ExperimentalMaterial3Api::class, ExperimentalMaterial3ExpressiveApi::class) -@Composable -internal fun SelectItemForTotpScreen( - suggestedItemIds: Set, - onItemClick: (ItemId) -> Unit, - onCreateNew: () -> Unit, - modifier: Modifier = Modifier, - loading: Boolean = false, -) { - val activity = LocalActivity.current - BackHandler { activity?.finish() } - - Scaffold( - modifier = modifier.fillMaxSize(), - topBar = { - TopAppBar(title = { Text(text = stringResource(R.string.select_item_for_totp)) }) - }, - floatingActionButton = { - if (!loading) - FloatingActionButton(onClick = onCreateNew) { - Icon( - imageVector = Icons.Default.Add, - contentDescription = stringResource(R.string.create_new), - ) - } - }, - ) { innerPadding -> - val content = Modifier - .consumeWindowInsets(innerPadding) - .padding(innerPadding) - - when (loading) { - true -> Box( - modifier = content.fillMaxSize(), - contentAlignment = Alignment.Center, - ) { - ContainedLoadingIndicator() - } - - false -> ItemListScreen( - onItemClick = onItemClick, - onItemLongClick = { }, - onCreateItemRequest = { onCreateNew() }, - restrictedItemType = VaultItemType.Login, - suggestedItemIds = suggestedItemIds, - notFoundStrategy = NoItemStrategy.ShowMessage, - enableDeletion = false, - enableSelection = false, - dockedSearchResults = false, - modifier = content, - ) - } - } -} diff --git a/feature/item/create/src/main/kotlin/de/davis/keygo/feature/item/create/presentation/creditcard/CreditCardViewModel.kt b/feature/item/create/src/main/kotlin/de/davis/keygo/feature/item/create/presentation/creditcard/CreditCardViewModel.kt index acb4c970f..1b84f2202 100644 --- a/feature/item/create/src/main/kotlin/de/davis/keygo/feature/item/create/presentation/creditcard/CreditCardViewModel.kt +++ b/feature/item/create/src/main/kotlin/de/davis/keygo/feature/item/create/presentation/creditcard/CreditCardViewModel.kt @@ -77,7 +77,6 @@ internal class CreditCardViewModel( viewModelScope.launch { initWithId(information.id) } is DetailPaneInformation.Init.New, - is DetailPaneInformation.Init.TOTP, is DetailPaneInformation.CreateRaw -> Unit // nothing to prefill } } diff --git a/feature/item/create/src/main/kotlin/de/davis/keygo/feature/item/create/presentation/login/LoginContent.kt b/feature/item/create/src/main/kotlin/de/davis/keygo/feature/item/create/presentation/login/LoginContent.kt index 467018468..5c89f0bb4 100644 --- a/feature/item/create/src/main/kotlin/de/davis/keygo/feature/item/create/presentation/login/LoginContent.kt +++ b/feature/item/create/src/main/kotlin/de/davis/keygo/feature/item/create/presentation/login/LoginContent.kt @@ -1,7 +1,6 @@ package de.davis.keygo.feature.item.create.presentation.login import android.content.res.Configuration -import androidx.activity.compose.BackHandler import androidx.compose.foundation.layout.Arrangement import androidx.compose.foundation.layout.Column import androidx.compose.foundation.layout.FlowRow @@ -60,7 +59,6 @@ import de.davis.keygo.feature.item.create.presentation.component.FormGroup import de.davis.keygo.feature.item.create.presentation.component.ItemContentWrapper import de.davis.keygo.feature.item.create.presentation.component.KeyGoItemForm import de.davis.keygo.feature.item.create.presentation.component.OverrideTotpDialog -import de.davis.keygo.feature.item.create.presentation.component.SelectItemForTotpScreen import de.davis.keygo.feature.item.create.presentation.component.TAG_DELIMITERS import de.davis.keygo.feature.item.create.presentation.login.model.DialogState import de.davis.keygo.feature.item.create.presentation.login.model.LoginBaseState @@ -81,21 +79,7 @@ import de.davis.keygo.feature.item.core.R as ItemCoreR internal fun LoginContent( state: LoginUiState, onEvent: (LoginUiEvent) -> Unit, - totpImportPending: Boolean = false, ) { - // A deep-linked import opens on the picker, so the picker's chrome is what the wait belongs - // to. The shared loading scaffold would flash a title and a back arrow from the form, a screen - // the user has not asked for yet and may never reach. - if (totpImportPending && state is ItemUiState.Loading) { - SelectItemForTotpScreen( - suggestedItemIds = emptySet(), - onItemClick = {}, - onCreateNew = {}, - loading = true, - ) - return - } - ItemContentWrapper( itemType = VaultItemType.Login, state = state, @@ -116,12 +100,6 @@ private fun LoginReadyContent( shared: SharedItemState, onEvent: (LoginUiEvent) -> Unit, ) { - // An import opened this form from the picker, so back has to reach the ViewModel and return - // there. Without this the pane navigator takes it first and pops the whole screen instead. - BackHandler(enabled = state.totpImportActive) { - onEvent(LoginUiEvent.ItemUi(ItemUiEvent.OnBackClick)) - } - val scrollBehavior = TopAppBarDefaults.enterAlwaysScrollBehavior() val domainTextFieldState = rememberTextFieldState() val tagsTextFieldState = rememberTextFieldState() @@ -368,14 +346,6 @@ private fun LoginReadyContent( }, ) } - - if (state.selectingItemForTotp) { - SelectItemForTotpScreen( - suggestedItemIds = state.totpSuggestedItemIds, - onItemClick = { onEvent(LoginUiEvent.OnTotpModificationItemSelected(it)) }, - onCreateNew = { onEvent(LoginUiEvent.OnCreateNewItemForTotp) }, - ) - } } private val DELIMITERS = setOf(',', ' ') diff --git a/feature/item/create/src/main/kotlin/de/davis/keygo/feature/item/create/presentation/login/LoginScreen.kt b/feature/item/create/src/main/kotlin/de/davis/keygo/feature/item/create/presentation/login/LoginScreen.kt index b599e5cf9..e894c1966 100644 --- a/feature/item/create/src/main/kotlin/de/davis/keygo/feature/item/create/presentation/login/LoginScreen.kt +++ b/feature/item/create/src/main/kotlin/de/davis/keygo/feature/item/create/presentation/login/LoginScreen.kt @@ -40,6 +40,5 @@ fun LoginScreen( LoginContent( state = state, onEvent = viewmodel::onEvent, - totpImportPending = detailPaneInformation is DetailPaneInformation.Init.TOTP, ) } diff --git a/feature/item/create/src/main/kotlin/de/davis/keygo/feature/item/create/presentation/login/LoginViewModel.kt b/feature/item/create/src/main/kotlin/de/davis/keygo/feature/item/create/presentation/login/LoginViewModel.kt index 15e41d12d..45f5c0d42 100644 --- a/feature/item/create/src/main/kotlin/de/davis/keygo/feature/item/create/presentation/login/LoginViewModel.kt +++ b/feature/item/create/src/main/kotlin/de/davis/keygo/feature/item/create/presentation/login/LoginViewModel.kt @@ -6,7 +6,6 @@ import androidx.compose.foundation.text.input.setTextAndPlaceCursorAtEnd import androidx.compose.runtime.snapshotFlow import androidx.lifecycle.viewModelScope import de.davis.keygo.core.item.domain.alias.ItemId -import de.davis.keygo.core.item.domain.alias.VaultId import de.davis.keygo.core.item.domain.estimator.PasswordStrengthEstimator import de.davis.keygo.core.item.domain.model.DomainInfo import de.davis.keygo.core.item.domain.model.PasswordScore @@ -16,7 +15,6 @@ import de.davis.keygo.core.item.domain.repository.VaultContextRepository import de.davis.keygo.core.item.domain.repository.VaultRepository import de.davis.keygo.core.item.domain.usecase.ObserveAllTagsSortedUseCase import de.davis.keygo.core.security.domain.crypto.decrypt -import de.davis.keygo.core.security.domain.usecase.GetTdlMatchedLoginsUseCase import de.davis.keygo.core.security.domain.usecase.ItemWithCryptoScopeUseCase import de.davis.keygo.core.util.domain.model.snackbar.SnackbarMessage import de.davis.keygo.core.util.domain.resolver.RegistrableDomainResolver @@ -70,7 +68,6 @@ internal class LoginViewModel( private val loginRepository: LoginRepository, private val passwordStrengthEstimator: PasswordStrengthEstimator, private val createNewOrUpdateLogin: CreateNewOrUpdateLoginUseCase, - private val getTdlMatchedLogins: GetTdlMatchedLoginsUseCase, private val snackbarManager: SnackbarManager, private val totpService: TotpService, private val registrableDomainResolver: RegistrableDomainResolver, @@ -112,21 +109,6 @@ internal class LoginViewModel( private var totpSecretInformation: TotpInfo? = null private var totpOriginalUri: String? = null - /** - * Whether this screen was opened by a scanned code rather than by the user. - * - * Only a deep link owns the whole screen, so only it gets the picker and the back behaviour - * that belongs to it. A code scanned from within the form arrives at an item the user already - * chose and must keep leaving the screen as it always has. - */ - private var totpItemPickerFlow = false - - /** Kept so returning to the picker shows the same suggestions without querying again. */ - private var totpSuggestedItemIds: Set = emptySet() - - /** The vault the picker was showing, restored when the form is abandoned back to it. */ - private var totpPickerVaultId: VaultId? = null - /** * Shows a passkey for [rp] as pending until the item is saved. * @@ -149,8 +131,6 @@ internal class LoginViewModel( initWithId(information.id) } - is DetailPaneInformation.Init.TOTP -> initWithTotpUri(information.uri) - is DetailPaneInformation.Init.New -> information.pendingTotpUri?.let { uri -> parsePendingTotp(uri)?.let { updateUiWithTotpSecretInfo(it, uri) } } @@ -256,44 +236,6 @@ internal class LoginViewModel( totpOriginalUri = uri } - private fun initWithTotpUri(totpUri: String) { - totpService.getInfoFromUriWithResult(totpUri).onFailure { - Log.e(TAG, "Error parsing TOTP URI: $it") - showTotpParseError() - }.onSuccess { secret -> - totpSecretInformation = secret - totpOriginalUri = totpUri - totpItemPickerFlow = true - _base.update { it.copy(totpImportActive = true, selectingItemForTotp = true) } - - viewModelScope.launch { - val suggestedIds = suggestedItemIdsFor(secret) - totpSuggestedItemIds = suggestedIds - // A choice made before the query returned leaves the picker; the late result has - // nothing left to reorder until the user comes back to it. - _base.update { - if (it.selectingItemForTotp) it.copy(totpSuggestedItemIds = suggestedIds) - else it - } - } - } - } - - /** - * The logins whose registrable domain matches the code's own. - * - * [resolveTotpDomain] is what fills the domain field for a new item, so the suggestions agree - * with it: a code that carries no issuer still matches on the domain in `user@example.com`. - */ - private suspend fun suggestedItemIdsFor(secretInformation: TotpInfo): Set { - val domain = resolveTotpDomain( - issuer = secretInformation.issuer, - accountName = secretInformation.accountName, - ) ?: return emptySet() - - return getTdlMatchedLogins(domain).mapTo(mutableSetOf()) { it.id } - } - override fun onSubmit() { val ready = state.value as? ItemUiState.Ready ?: return val base = ready.base @@ -369,15 +311,6 @@ internal class LoginViewModel( return } - // Back belongs to the import while one is running: from the form it returns to the picker - // the form was opened from. The picker itself is the flow's first step and has nothing - // behind it, so leaving it is the screen's own business (it closes the app) and never a - // navigation back to a dashboard the user did not open. - if (totpItemPickerFlow) { - if (!_base.value.selectingItemForTotp) reopenTotpItemPicker() - return - } - navigateUp() } @@ -413,18 +346,6 @@ internal class LoginViewModel( } ?: showTotpParseError() } - is LoginUiEvent.OnTotpModificationItemSelected -> { - closeTotpItemPicker() - viewModelScope.launch { initWithId(event.itemId) } - } - - is LoginUiEvent.OnCreateNewItemForTotp -> { - closeTotpItemPicker() - totpSecretInformation?.let { - updateUiWithTotpSecretInfo(it, totpOriginalUri) - } - } - is LoginUiEvent.OnOverrideFieldClicked -> { _base.update { it.copy( @@ -638,37 +559,6 @@ internal class LoginViewModel( } } - private fun closeTotpItemPicker() { - totpPickerVaultId = selectedVaultId.value - _base.update { - it.copy(selectingItemForTotp = false, totpSuggestedItemIds = emptySet()) - } - } - - /** - * Returns to the picker, throwing away whatever the choice made of the form. - * - * The form is rebuilt from scratch rather than hidden: the choice may have loaded an existing - * item into it, and carrying that item's name, password or vault into the next choice would - * write it onto the wrong login. Only the scanned code and its suggestions survive, because - * those belong to the import rather than to the item. - */ - private fun reopenTotpItemPicker() { - itemId = null - nameTextFieldState.setTextAndPlaceCursorAtEnd("") - notesTextFieldState.setTextAndPlaceCursorAtEnd("") - passwordTextFieldState.setTextAndPlaceCursorAtEnd("") - setAssignedTags(emptySet()) - totpPickerVaultId?.let { setSelectedVaultId(it) } - - _base.value = LoginBaseState( - passwordTextFieldState = passwordTextFieldState, - totpImportActive = true, - selectingItemForTotp = true, - totpSuggestedItemIds = totpSuggestedItemIds, - ) - } - private fun showTotpParseError() { _base.update { it.copy( diff --git a/feature/item/create/src/main/kotlin/de/davis/keygo/feature/item/create/presentation/login/model/LoginUiEvent.kt b/feature/item/create/src/main/kotlin/de/davis/keygo/feature/item/create/presentation/login/model/LoginUiEvent.kt index 9e868b8ed..bae406b35 100644 --- a/feature/item/create/src/main/kotlin/de/davis/keygo/feature/item/create/presentation/login/model/LoginUiEvent.kt +++ b/feature/item/create/src/main/kotlin/de/davis/keygo/feature/item/create/presentation/login/model/LoginUiEvent.kt @@ -1,6 +1,5 @@ package de.davis.keygo.feature.item.create.presentation.login.model -import de.davis.keygo.core.item.domain.alias.ItemId import de.davis.keygo.core.item.domain.model.PasskeyRef import de.davis.keygo.feature.item.core.presentation.login.model.FieldType import de.davis.keygo.feature.item.create.presentation.model.ItemUiEvent @@ -22,8 +21,6 @@ internal sealed interface LoginUiEvent { data object OnTotpParseErrorDismiss : LoginUiEvent data class OnCodesScanned(val codes: List) : LoginUiEvent - data class OnTotpModificationItemSelected(val itemId: ItemId) : LoginUiEvent - data object OnCreateNewItemForTotp : LoginUiEvent data class OnOverrideFieldClicked(val fieldType: FieldType) : LoginUiEvent data object OnOverrideTotpFieldsConfirmed : LoginUiEvent diff --git a/feature/item/create/src/main/kotlin/de/davis/keygo/feature/item/create/presentation/login/model/LoginUiState.kt b/feature/item/create/src/main/kotlin/de/davis/keygo/feature/item/create/presentation/login/model/LoginUiState.kt index d0d8552ef..9c75a64ce 100644 --- a/feature/item/create/src/main/kotlin/de/davis/keygo/feature/item/create/presentation/login/model/LoginUiState.kt +++ b/feature/item/create/src/main/kotlin/de/davis/keygo/feature/item/create/presentation/login/model/LoginUiState.kt @@ -2,7 +2,6 @@ package de.davis.keygo.feature.item.create.presentation.login.model import androidx.compose.foundation.text.input.TextFieldState import androidx.compose.runtime.Stable -import de.davis.keygo.core.item.domain.alias.ItemId import de.davis.keygo.core.item.domain.model.DomainInfo import de.davis.keygo.core.item.domain.model.PasskeyRef import de.davis.keygo.core.item.domain.model.PasswordScore @@ -39,15 +38,6 @@ internal data class LoginBaseState( val dialogState: DialogState = DialogState.None, val nameError: InputFieldError? = null, val scanning: Boolean = false, - /** - * Whether a deep-linked code is being imported, from the picker until the item is saved. - * - * The screen has to claim back for as long as this runs: the form was opened by the picker, so - * back belongs to that flow rather than to the pane the screen sits in. - */ - val totpImportActive: Boolean = false, - val selectingItemForTotp: Boolean = false, - val totpSuggestedItemIds: Set = emptySet(), val updating: Boolean = false, ) { /** diff --git a/feature/item/create/src/test/java/de/davis/keygo/feature/item/create/presentation/login/LoginViewModelTest.kt b/feature/item/create/src/test/java/de/davis/keygo/feature/item/create/presentation/login/LoginViewModelTest.kt index 17cf41e2a..9c4ffc967 100644 --- a/feature/item/create/src/test/java/de/davis/keygo/feature/item/create/presentation/login/LoginViewModelTest.kt +++ b/feature/item/create/src/test/java/de/davis/keygo/feature/item/create/presentation/login/LoginViewModelTest.kt @@ -18,7 +18,6 @@ import de.davis.keygo.core.item.domain.usecase.ObserveAllTagsSortedUseCase import de.davis.keygo.core.item.domain.usecase.UpsertVaultItemUseCase import de.davis.keygo.core.item.generated.domain.model.VaultItemType import de.davis.keygo.core.security.crypto.FakeCryptographicScopeProvider -import de.davis.keygo.core.security.domain.usecase.GetTdlMatchedLoginsUseCase import de.davis.keygo.core.security.domain.usecase.ItemWithCryptoScopeUseCase import de.davis.keygo.core.util.domain.model.snackbar.SnackbarMessage import de.davis.keygo.core.util.domain.snackbar.SnackbarManager @@ -53,14 +52,12 @@ import kotlin.test.AfterTest import kotlin.test.BeforeTest import kotlin.test.Test import kotlin.test.assertEquals -import kotlin.test.assertFalse import kotlin.test.assertIs import kotlin.test.assertTrue /** - * Covers what a scanned `otpauth://` deep link does before the form is filled in: it hands the - * user the item picker rather than guessing a target, and offers the logins on the code's own - * domain as suggestions. + * Covers what the login form does with a code the picker handed it: a new item is prefilled from + * the code, and a chosen item has it folded in, raising the override dialog when the two collide. */ @OptIn(ExperimentalCoroutinesApi::class) @RunWith(RobolectricTestRunner::class) @@ -96,139 +93,6 @@ class LoginViewModelTest { Dispatchers.resetMain() } - @Test - fun `a deep link opens the picker instead of filling the form`() = runVmTest { - totpService.infoFromUriResult = totpInfo(issuer = "github.com") - - val viewModel = initWithDeepLink() - - val base = viewModel.readyBase() - assertTrue(base.selectingItemForTotp) - assertTrue(base.totpImportActive) - assertEquals("", base.totpTextFieldState.text.toString()) - assertEquals("", base.usernameTextFieldState.text.toString()) - assertEquals(emptySet(), base.domains) - } - - @Test - fun `the picker opens even when nothing matches the domain`() = runVmTest { - totpService.infoFromUriResult = totpInfo(issuer = "github.com") - - val viewModel = initWithDeepLink() - - val base = viewModel.readyBase() - assertTrue(base.selectingItemForTotp) - assertEquals(emptySet(), base.totpSuggestedItemIds) - } - - @Test - fun `logins on the code's domain are suggested`() = runVmTest { - val onDomain = seedLogin(name = "GitHub", domain = "github.com") - seedLogin(name = "Google", domain = "google.com") - totpService.infoFromUriResult = totpInfo(issuer = "github.com") - - val viewModel = initWithDeepLink() - - assertEquals(setOf(onDomain), viewModel.readyBase().totpSuggestedItemIds) - } - - @Test - fun `a code without an issuer suggests on the account name's domain`() = runVmTest { - val onDomain = seedLogin(name = "GitHub", domain = "github.com") - totpService.infoFromUriResult = totpInfo(issuer = null, accountName = "me@github.com") - - val viewModel = initWithDeepLink() - - assertEquals(setOf(onDomain), viewModel.readyBase().totpSuggestedItemIds) - } - - @Test - fun `creating a new item closes the picker and fills the form from the code`() = runVmTest { - totpService.infoFromUriResult = totpInfo(issuer = "github.com") - val viewModel = initWithDeepLink() - - viewModel.onEvent(LoginUiEvent.OnCreateNewItemForTotp) - advanceUntilIdle() - - val base = viewModel.readyBase() - assertFalse(base.selectingItemForTotp) - // The form still belongs to the import, so it keeps claiming back. - assertTrue(base.totpImportActive) - assertEquals(DEEP_LINK_URI, base.totpTextFieldState.text.toString()) - assertEquals("me@github.com", base.usernameTextFieldState.text.toString()) - assertEquals(setOf("github.com"), base.domains.mapTo(mutableSetOf()) { it.value }) - } - - @Test - fun `choosing an existing item closes the picker and loads that item`() = runVmTest { - val existing = seedLogin(name = "GitHub", domain = "github.com") - totpService.infoFromUriResult = totpInfo(issuer = "github.com") - val viewModel = initWithDeepLink() - - viewModel.onEvent(LoginUiEvent.OnTotpModificationItemSelected(existing)) - advanceUntilIdle() - - val state = viewModel.readyState() - assertFalse(state.base.selectingItemForTotp) - assertTrue(state.base.updating) - assertEquals("GitHub", state.shared.nameTextFieldState.text.toString()) - assertEquals(DEEP_LINK_URI, state.base.totpTextFieldState.text.toString()) - assertEquals("me@github.com", state.base.usernameTextFieldState.text.toString()) - } - - @Test - fun `back stays on the picker instead of leaving the import`() = runVmTest { - totpService.infoFromUriResult = totpInfo(issuer = "github.com") - val viewModel = initWithDeepLink() - val navigation = collectNavigation(viewModel) - - viewModel.onEvent(LoginUiEvent.ItemUi(ItemUiEvent.OnBackClick)) - advanceUntilIdle() - - assertTrue(viewModel.readyBase().selectingItemForTotp) - assertEquals(emptyList(), navigation) - } - - @Test - fun `back from a chosen item returns to the picker with a clean form`() = runVmTest { - val existing = seedLogin(name = "GitHub", domain = "github.com") - totpService.infoFromUriResult = totpInfo(issuer = "github.com") - val viewModel = initWithDeepLink() - val navigation = collectNavigation(viewModel) - viewModel.onEvent(LoginUiEvent.OnTotpModificationItemSelected(existing)) - advanceUntilIdle() - - viewModel.onEvent(LoginUiEvent.ItemUi(ItemUiEvent.OnBackClick)) - advanceUntilIdle() - - val state = viewModel.readyState() - assertTrue(state.base.selectingItemForTotp) - assertFalse(state.base.updating) - assertEquals(setOf(existing), state.base.totpSuggestedItemIds) - assertEquals("", state.shared.nameTextFieldState.text.toString()) - assertEquals("", state.base.totpTextFieldState.text.toString()) - assertEquals("", state.base.usernameTextFieldState.text.toString()) - assertEquals(emptySet(), state.base.domains) - assertEquals(emptyList(), navigation) - } - - @Test - fun `back from a new item returns to the picker`() = runVmTest { - totpService.infoFromUriResult = totpInfo(issuer = "github.com") - val viewModel = initWithDeepLink() - val navigation = collectNavigation(viewModel) - viewModel.onEvent(LoginUiEvent.OnCreateNewItemForTotp) - advanceUntilIdle() - - viewModel.onEvent(LoginUiEvent.ItemUi(ItemUiEvent.OnBackClick)) - advanceUntilIdle() - - val base = viewModel.readyBase() - assertTrue(base.selectingItemForTotp) - assertEquals("", base.totpTextFieldState.text.toString()) - assertEquals(emptyList(), navigation) - } - @Test fun `back leaves the screen when the code was scanned into an open form`() = runVmTest { totpService.infoFromUriResult = totpInfo(issuer = "github.com") @@ -243,21 +107,9 @@ class LoginViewModelTest { viewModel.onEvent(LoginUiEvent.ItemUi(ItemUiEvent.OnBackClick)) advanceUntilIdle() - val base = viewModel.readyBase() - assertFalse(base.selectingItemForTotp) - assertFalse(base.totpImportActive) assertEquals(listOf(null), navigation) } - @Test - fun `an unparsable code shows the error instead of the picker`() = runVmTest { - totpService.infoFromUriResult = null - - val viewModel = initWithDeepLink() - - assertFalse(viewModel.readyBase().selectingItemForTotp) - } - @Test fun `a new item is prefilled from the picker's code`() = runVmTest { totpService.infoFromUriResult = totpInfo(issuer = "github.com") @@ -333,14 +185,6 @@ class LoginViewModelTest { private fun runVmTest(body: suspend TestScope.() -> Unit) = runTest(mainDispatcher.scheduler) { body() } - private fun TestScope.initWithDeepLink(uri: String = DEEP_LINK_URI): LoginViewModel { - val viewModel = buildViewModel() - backgroundScope.launch(mainDispatcher) { viewModel.state.collect { } } - viewModel.init(DetailPaneInformation.Init.TOTP(VaultItemType.Login, uri)) - advanceUntilIdle() - return viewModel - } - /** * Records what the screen asks navigation to do. Leaving raises `null`, a saved item raises its * id, and an empty list is the assertion that the screen stayed where it was. @@ -418,7 +262,6 @@ class LoginViewModelTest { passwordStrengthEstimator = FakePasswordStrengthEstimator(), totpService = totpService, ), - getTdlMatchedLogins = GetTdlMatchedLoginsUseCase(domainResolver, loginRepository), snackbarManager = TestSnackbarManager(), totpService = totpService, registrableDomainResolver = domainResolver, From bd6e085d42be73d8361a68d0e58c3c2874a8b4cf Mon Sep 17 00:00:00 2001 From: Davis Wolfermann Date: Sun, 23 Aug 2026 17:41:35 +0200 Subject: [PATCH 07/20] fix(totp): apply review fix wave for item picker navigation Addresses the important and minor findings from the branch review of the TOTP item picker's move to its own navigation destination: - dedupe a double-tap on a picker row with launchSingleTop, since the picker keeps collecting through its exit transition and a second tap inside that window pushed AssignTotpRoute twice - record the invariant that lets Init.Existing safely run parsePendingTotp before initWithId overwrites dialogState, so a future caller does not reintroduce a silently dropped parse error - complete the KDoc on the now-public SelectItemForTotpScreen - drop a dead FakeItemRepository field from its ViewModel test - collapse the duplicated post-unlock destination mapping in MainActivity into destinationAfterUnlock - drop an unused lambda parameter in DashboardGraph - rename a preview function left over from the deleted overlay dialog - remove a stray blank line in feature/item/create's build.gradle.kts Also corrects the design spec's Files section, which claimed no dependency changes were needed even though this branch added implementation(projects.feature.listScreen) to feature/item/create. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_019NwwBkR3ks2QMJSRmP1iDH --- .../keygo/app/presentation/MainActivity.kt | 29 ++++++++++++------- .../dashboard/presentation/DashboardGraph.kt | 2 +- feature/item/create/build.gradle.kts | 1 - .../component/OverrideTotpDialog.kt | 2 +- .../presentation/login/LoginViewModel.kt | 6 ++++ .../totp/SelectItemForTotpScreen.kt | 4 +++ .../totp/SelectItemForTotpViewModelTest.kt | 2 -- 7 files changed, 30 insertions(+), 16 deletions(-) diff --git a/app/src/main/kotlin/de/davis/keygo/app/presentation/MainActivity.kt b/app/src/main/kotlin/de/davis/keygo/app/presentation/MainActivity.kt index 8ca0bf45b..7aac067c3 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 @@ -86,6 +86,13 @@ class MainActivity : FragmentActivity() { } } +/** + * Where a pending code sends the user once they are through the door, whether that door was auth + * or onboarding. No code means there is nothing to pick an item for. + */ +private fun destinationAfterUnlock(totpUri: String?): Any = + totpUri?.let { SelectItemForTotpRoute(it) } ?: RouteDestination.TopLevelAppGraph + @OptIn(ExperimentalMaterial3AdaptiveApi::class) @Composable private fun App(hasAccess: Boolean) { @@ -154,7 +161,15 @@ private fun App(hasAccess: Boolean) { ) totpImportGraph( - navigateToDestination = navController::navigate, + // The picker stays composed and collecting through its exit transition, so a + // double tap on a row can fire twice before the first navigation leaves it. + // AssignTotpRoute is a data class, so launchSingleTop dedupes the repeat instead + // of pushing it twice onto the back stack. + navigateToDestination = { dest -> + navController.navigate(dest) { + launchSingleTop = true + } + }, onImportFinished = { navController.navigate(RouteDestination.TopLevelAppGraph) { popUpTo { inclusive = true } @@ -165,11 +180,7 @@ private fun App(hasAccess: Boolean) { authGraph( onSuccess = { totpUri -> - val dest = totpUri?.let { - SelectItemForTotpRoute(it) - } ?: RouteDestination.TopLevelAppGraph - - navController.navigate(dest) { + navController.navigate(destinationAfterUnlock(totpUri)) { popUpTo { inclusive = true } } } @@ -177,11 +188,7 @@ private fun App(hasAccess: Boolean) { onboardingGraph( onSuccess = { totpUri -> - val dest = totpUri?.let { - SelectItemForTotpRoute(it) - } ?: RouteDestination.TopLevelAppGraph - - navController.navigate(dest) { + navController.navigate(destinationAfterUnlock(totpUri)) { popUpTo { inclusive = true } } } diff --git a/app/src/main/kotlin/de/davis/keygo/dashboard/presentation/DashboardGraph.kt b/app/src/main/kotlin/de/davis/keygo/dashboard/presentation/DashboardGraph.kt index 6f040f114..5e0f4ca57 100644 --- a/app/src/main/kotlin/de/davis/keygo/dashboard/presentation/DashboardGraph.kt +++ b/app/src/main/kotlin/de/davis/keygo/dashboard/presentation/DashboardGraph.kt @@ -31,7 +31,7 @@ import kotlinx.coroutines.launch fun NavGraphBuilder.dashboardGraph( listNavigator: ThreePaneScaffoldNavigator, ) { - composable { _ -> + composable { val isSinglePaneMode by remember(listNavigator.scaffoldDirective) { derivedStateOf { listNavigator.scaffoldDirective.maxHorizontalPartitions == 1 diff --git a/feature/item/create/build.gradle.kts b/feature/item/create/build.gradle.kts index 14bb9d17c..f1f0ccd68 100644 --- a/feature/item/create/build.gradle.kts +++ b/feature/item/create/build.gradle.kts @@ -26,5 +26,4 @@ dependencies { testImplementation(testFixtures(projects.rust)) testImplementation(libs.robolectric) testImplementation(libs.androidx.junit) - } diff --git a/feature/item/create/src/main/kotlin/de/davis/keygo/feature/item/create/presentation/component/OverrideTotpDialog.kt b/feature/item/create/src/main/kotlin/de/davis/keygo/feature/item/create/presentation/component/OverrideTotpDialog.kt index 76f4d0e14..f1a642c17 100644 --- a/feature/item/create/src/main/kotlin/de/davis/keygo/feature/item/create/presentation/component/OverrideTotpDialog.kt +++ b/feature/item/create/src/main/kotlin/de/davis/keygo/feature/item/create/presentation/component/OverrideTotpDialog.kt @@ -123,7 +123,7 @@ fun OverrideTotpDialog( @Preview @Preview(uiMode = Configuration.UI_MODE_NIGHT_YES) @Composable -private fun SelectItemForTotpModificationDialogPreview() { +private fun OverrideTotpDialogPreview() { KeyGoTheme { Surface(modifier = Modifier.fillMaxSize()) { OverrideTotpDialog( diff --git a/feature/item/create/src/main/kotlin/de/davis/keygo/feature/item/create/presentation/login/LoginViewModel.kt b/feature/item/create/src/main/kotlin/de/davis/keygo/feature/item/create/presentation/login/LoginViewModel.kt index 45f5c0d42..0bfecd4c8 100644 --- a/feature/item/create/src/main/kotlin/de/davis/keygo/feature/item/create/presentation/login/LoginViewModel.kt +++ b/feature/item/create/src/main/kotlin/de/davis/keygo/feature/item/create/presentation/login/LoginViewModel.kt @@ -127,6 +127,12 @@ internal class LoginViewModel( fun init(information: DetailPaneInformation) { when (information) { is DetailPaneInformation.Init.Existing -> viewModelScope.launch { + // initWithId always resets dialogState to DialogState.None, which would silently + // wipe a parse error shown here. That is safe only because the one caller that + // reaches this branch with a pendingTotpUri, the item picker, already parsed the + // same uri with the same service and abandons the import on failure before + // navigating here. A new caller constructing Init.Existing(pendingTotpUri = ...) + // needs its own parse gate upstream, since this branch will not provide one. information.pendingTotpUri?.let { parsePendingTotp(it) } initWithId(information.id) } diff --git a/feature/item/create/src/main/kotlin/de/davis/keygo/feature/item/create/presentation/totp/SelectItemForTotpScreen.kt b/feature/item/create/src/main/kotlin/de/davis/keygo/feature/item/create/presentation/totp/SelectItemForTotpScreen.kt index 4602113af..3244aede9 100644 --- a/feature/item/create/src/main/kotlin/de/davis/keygo/feature/item/create/presentation/totp/SelectItemForTotpScreen.kt +++ b/feature/item/create/src/main/kotlin/de/davis/keygo/feature/item/create/presentation/totp/SelectItemForTotpScreen.kt @@ -34,6 +34,10 @@ import org.koin.core.parameter.parametersOf * import, which replaced the whole back stack on its way here, so the NavHost does not consume a * back press and the activity finishes on its own. * + * @param totpUri the scanned deep link, carried whole so the screen and its ViewModel can parse it + * independently. + * @param onItemSelected the user picked an existing login to attach the code to. + * @param onCreateNew the user chose to attach the code to a login that does not exist yet. * @param onImportAbandoned the code could not be read, so there is nothing to attach and the flow * ends. The user stays in the app they just unlocked rather than being thrown out over a malformed * code from somewhere else. diff --git a/feature/item/create/src/test/java/de/davis/keygo/feature/item/create/presentation/totp/SelectItemForTotpViewModelTest.kt b/feature/item/create/src/test/java/de/davis/keygo/feature/item/create/presentation/totp/SelectItemForTotpViewModelTest.kt index e84de6adc..10148b0c8 100644 --- a/feature/item/create/src/test/java/de/davis/keygo/feature/item/create/presentation/totp/SelectItemForTotpViewModelTest.kt +++ b/feature/item/create/src/test/java/de/davis/keygo/feature/item/create/presentation/totp/SelectItemForTotpViewModelTest.kt @@ -1,6 +1,5 @@ package de.davis.keygo.feature.item.create.presentation.totp -import de.davis.keygo.core.item.FakeItemRepository import de.davis.keygo.core.item.FakeLoginRepository import de.davis.keygo.core.item.domain.alias.ItemId import de.davis.keygo.core.item.domain.alias.newItemId @@ -45,7 +44,6 @@ class SelectItemForTotpViewModelTest { private val vaultId = newVaultId() private val loginRepository = FakeLoginRepository() - private val itemRepository = FakeItemRepository(loginRepository) private val totpService = FakeTotpService() private val domainResolver = TestRegistrableDomainResolver() From 3d7461dcb3618769cecdfc202be5de3aefad446c Mon Sep 17 00:00:00 2001 From: Davis Wolfermann Date: Sun, 23 Aug 2026 23:26:55 +0200 Subject: [PATCH 08/20] feat(totp): validate a deep-linked code before the unlock --- feature/totp/build.gradle.kts | 6 ++ .../totp/presentation/TotpImportRedirect.kt | 75 ++++++++++++++++++ .../presentation/TotpImportRedirectState.kt | 17 ++++ .../TotpImportRedirectViewModel.kt | 63 +++++++++++++++ .../TotpImportRedirectViewModelTest.kt | 79 +++++++++++++++++++ 5 files changed, 240 insertions(+) create mode 100644 feature/totp/src/main/kotlin/de/davis/keygo/feature/totp/presentation/TotpImportRedirect.kt create mode 100644 feature/totp/src/main/kotlin/de/davis/keygo/feature/totp/presentation/TotpImportRedirectState.kt create mode 100644 feature/totp/src/main/kotlin/de/davis/keygo/feature/totp/presentation/TotpImportRedirectViewModel.kt create mode 100644 feature/totp/src/test/kotlin/de/davis/keygo/feature/totp/presentation/TotpImportRedirectViewModelTest.kt diff --git a/feature/totp/build.gradle.kts b/feature/totp/build.gradle.kts index afb6146ee..41fe347d1 100644 --- a/feature/totp/build.gradle.kts +++ b/feature/totp/build.gradle.kts @@ -1,5 +1,6 @@ plugins { alias(libs.plugins.keygo.android.compose) + alias(libs.plugins.kotlin.serialization) } android { @@ -21,10 +22,13 @@ dependencies { implementation(libs.com.google.accompanist.permissions) implementation(projects.rust) + implementation(projects.core.ui) implementation(projects.core.security) implementation(projects.core.item) implementation(projects.core.util) + implementation(libs.androidx.navigation.compose) + implementation(libs.androidx.camera.camera2) implementation(libs.androidx.camera.compose) implementation(libs.androidx.camera.lifecycle) @@ -35,4 +39,6 @@ dependencies { testImplementation(testFixtures(projects.core.security)) testImplementation(testFixtures(projects.core.item)) testImplementation(testFixtures(projects.rust)) + testImplementation(libs.robolectric) + testImplementation(libs.androidx.junit) } diff --git a/feature/totp/src/main/kotlin/de/davis/keygo/feature/totp/presentation/TotpImportRedirect.kt b/feature/totp/src/main/kotlin/de/davis/keygo/feature/totp/presentation/TotpImportRedirect.kt new file mode 100644 index 000000000..bac91a139 --- /dev/null +++ b/feature/totp/src/main/kotlin/de/davis/keygo/feature/totp/presentation/TotpImportRedirect.kt @@ -0,0 +1,75 @@ +package de.davis.keygo.feature.totp.presentation + +import androidx.compose.foundation.layout.fillMaxWidth +import androidx.compose.runtime.LaunchedEffect +import androidx.compose.runtime.getValue +import androidx.compose.ui.Modifier +import androidx.lifecycle.compose.collectAsStateWithLifecycle +import androidx.navigation.NavGraphBuilder +import androidx.navigation.compose.composable +import androidx.navigation.navDeepLink +import androidx.navigation.toRoute +import de.davis.keygo.core.ui.RouteDestination +import de.davis.keygo.core.ui.model.PendingTotpImport +import de.davis.keygo.feature.totp.presentation.component.TotpParseErrorDialog +import kotlinx.serialization.Serializable +import org.koin.androidx.compose.koinViewModel +import org.koin.core.parameter.parametersOf + +/** + * Where a deep-linked code lands first, before anything else in the app sees it. + * + * The code travels as the two halves the deep link splits it into rather than as an assembled uri, + * because `otpauth://totp/{totpInfo}?{queries}` is the shape the navigation pattern matches. + */ +@Serializable +data class TotpImportRedirect( + val totpInfo: String? = null, + val queries: String? = null, +) : RouteDestination { + val pendingImport: PendingTotpImport + get() = PendingTotpImport(totpInfo, queries) +} + +/** + * Validates the code a deep link carried and lets the caller decide where a good one leads. + * + * The destination is the caller's to pick: auth and onboarding are the two ways into the app, and + * this module knows about neither. + * + * @param onValidated the code parses, so the import is worth an authentication. It carries the code + * onward so the caller can hand it to whichever screen it sends the user to. + * @param onRejected the code is unusable and the user has acknowledged it. The app was launched only + * to import that code, so nothing is left to do. Closing belongs to whoever owns the Activity, which + * is not this module. + */ +fun NavGraphBuilder.totpImportRedirectGraph( + onValidated: (PendingTotpImport) -> Unit, + onRejected: () -> Unit, +) { + composable( + deepLinks = listOf( + navDeepLink(basePath = PendingTotpImport.BASE_PATH) { + uriPattern = PendingTotpImport.URI_PATTERN + }, + ), + ) { entry -> + val route = entry.toRoute() + val viewModel: TotpImportRedirectViewModel = + koinViewModel { parametersOf(route.pendingImport) } + val state by viewModel.state.collectAsStateWithLifecycle() + + when (state) { + TotpImportRedirectState.Validating -> Unit + + TotpImportRedirectState.Valid -> LaunchedEffect(route) { + onValidated(route.pendingImport) + } + + TotpImportRedirectState.Invalid -> TotpParseErrorDialog( + onDismiss = onRejected, + modifier = Modifier.fillMaxWidth(), + ) + } + } +} diff --git a/feature/totp/src/main/kotlin/de/davis/keygo/feature/totp/presentation/TotpImportRedirectState.kt b/feature/totp/src/main/kotlin/de/davis/keygo/feature/totp/presentation/TotpImportRedirectState.kt new file mode 100644 index 000000000..b4fb14ca0 --- /dev/null +++ b/feature/totp/src/main/kotlin/de/davis/keygo/feature/totp/presentation/TotpImportRedirectState.kt @@ -0,0 +1,17 @@ +package de.davis.keygo.feature.totp.presentation + +/** + * What the redirect knows about the code its deep link carried. + * + * The parse is a single synchronous call, so [Validating] is what the screen holds for at most one + * frame. It exists so the screen never has to read a missing verdict as either "not yet" or + * "unreadable". + */ +internal sealed interface TotpImportRedirectState { + + data object Validating : TotpImportRedirectState + + data object Valid : TotpImportRedirectState + + data object Invalid : TotpImportRedirectState +} diff --git a/feature/totp/src/main/kotlin/de/davis/keygo/feature/totp/presentation/TotpImportRedirectViewModel.kt b/feature/totp/src/main/kotlin/de/davis/keygo/feature/totp/presentation/TotpImportRedirectViewModel.kt new file mode 100644 index 000000000..6e21460cf --- /dev/null +++ b/feature/totp/src/main/kotlin/de/davis/keygo/feature/totp/presentation/TotpImportRedirectViewModel.kt @@ -0,0 +1,63 @@ +package de.davis.keygo.feature.totp.presentation + +import android.util.Log +import androidx.lifecycle.ViewModel +import de.davis.keygo.core.ui.model.PendingTotpImport +import de.davis.keygo.core.util.fold +import de.davis.keygo.rust.totp.TotpService +import de.davis.keygo.rust.totp.getInfoFromUriWithResult +import kotlinx.coroutines.flow.MutableStateFlow +import kotlinx.coroutines.flow.StateFlow +import kotlinx.coroutines.flow.asStateFlow +import org.koin.core.annotation.InjectedParam +import org.koin.core.annotation.KoinViewModel + +/** + * Decides whether a deep-linked code is worth asking the user to authenticate for. + * + * The import used to reach the item picker before anything read the code, so a malformed one cost + * the user a full unlock before it told them anything. This is the single gate that check now + * passes through, which is why neither the picker nor the login form reports a parse failure of its + * own any more. + * + * The pending import arrives whole rather than as its assembled uri because that uri is null for a + * structurally incomplete link, and Koin resolves an injected parameter by type, which a null value + * does not have. + */ +@KoinViewModel +internal class TotpImportRedirectViewModel( + @InjectedParam private val pendingImport: PendingTotpImport, + private val totpService: TotpService, +) : ViewModel() { + + private val _state = + MutableStateFlow(TotpImportRedirectState.Validating) + val state: StateFlow = _state.asStateFlow() + + init { + _state.value = validate() + } + + /** + * A null uri means the link carried no path or no query string, which leaves as little to + * import as a code the parser rejects. Both end the import here. + */ + private fun validate(): TotpImportRedirectState { + val uri = pendingImport.uri ?: run { + Log.e(TAG, "Deep link carried no complete otpauth uri") + return TotpImportRedirectState.Invalid + } + + return totpService.getInfoFromUriWithResult(uri).fold( + onSuccess = { TotpImportRedirectState.Valid }, + onFailure = { failure -> + Log.e(TAG, "Error parsing TOTP URI: $failure") + TotpImportRedirectState.Invalid + }, + ) + } + + companion object { + private const val TAG = "TotpImportRedirectVM" + } +} diff --git a/feature/totp/src/test/kotlin/de/davis/keygo/feature/totp/presentation/TotpImportRedirectViewModelTest.kt b/feature/totp/src/test/kotlin/de/davis/keygo/feature/totp/presentation/TotpImportRedirectViewModelTest.kt new file mode 100644 index 000000000..6d024ee93 --- /dev/null +++ b/feature/totp/src/test/kotlin/de/davis/keygo/feature/totp/presentation/TotpImportRedirectViewModelTest.kt @@ -0,0 +1,79 @@ +package de.davis.keygo.feature.totp.presentation + +import de.davis.keygo.core.ui.model.PendingTotpImport +import de.davis.keygo.rust.FakeTotpService +import de.davisalessandro.keygo.rust.Algorithm +import de.davisalessandro.keygo.rust.TotpInfo +import org.junit.runner.RunWith +import org.robolectric.RobolectricTestRunner +import org.robolectric.annotation.Config +import kotlin.test.Test +import kotlin.test.assertEquals + +/** + * Covers the gate a deep-linked code passes before the user is asked to authenticate. A code that + * cannot be read has to be rejected here, because everything downstream now assumes it was. + */ +@RunWith(RobolectricTestRunner::class) +@Config(sdk = [34]) +class TotpImportRedirectViewModelTest { + + private val totpService = FakeTotpService() + + @Test + fun `a readable code is valid`() { + totpService.infoFromUriResult = totpInfo() + + val viewModel = buildViewModel(PendingTotpImport(TOTP_INFO, QUERIES)) + + assertEquals(TotpImportRedirectState.Valid, viewModel.state.value) + } + + @Test + fun `an unreadable code is invalid`() { + totpService.infoFromUriResult = null + + val viewModel = buildViewModel(PendingTotpImport(TOTP_INFO, QUERIES)) + + assertEquals(TotpImportRedirectState.Invalid, viewModel.state.value) + } + + @Test + fun `a link with no query string is invalid`() { + totpService.infoFromUriResult = totpInfo() + + val viewModel = buildViewModel(PendingTotpImport(totpInfo = TOTP_INFO, queries = null)) + + assertEquals(TotpImportRedirectState.Invalid, viewModel.state.value) + } + + @Test + fun `a link with no path is invalid`() { + totpService.infoFromUriResult = totpInfo() + + val viewModel = buildViewModel(PendingTotpImport(totpInfo = null, queries = QUERIES)) + + assertEquals(TotpImportRedirectState.Invalid, viewModel.state.value) + } + + // Helpers + + private fun buildViewModel(pendingImport: PendingTotpImport) = TotpImportRedirectViewModel( + pendingImport = pendingImport, + totpService = totpService, + ) + + private fun totpInfo() = TotpInfo( + secret = "JBSWY3DPEHPK3PXP", + issuer = "github.com", + accountName = "me@github.com", + algorithm = Algorithm.SHA1, + digits = 6, + period = 30, + ) + + companion object { + private const val TOTP_INFO = "GitHub:me@github.com" + private const val QUERIES = "secret=JBSWY3DPEHPK3PXP&issuer=github.com" + } +} From aea2ac0234e375be70ae70a804af6e5d63f175d4 Mon Sep 17 00:00:00 2001 From: Davis Wolfermann Date: Sun, 23 Aug 2026 23:58:22 +0200 Subject: [PATCH 09/20] refactor(totp): move the deep link redirect into the totp feature --- .../keygo/app/presentation/MainActivity.kt | 23 ++++++++-- .../app/presentation/TotpImportRedirect.kt | 42 ------------------- .../presentation/TotpImportNavGraphTest.kt | 21 +++++++++- 3 files changed, 39 insertions(+), 47 deletions(-) delete mode 100644 app/src/main/kotlin/de/davis/keygo/app/presentation/TotpImportRedirect.kt 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 7aac067c3..08025d9c4 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 @@ -1,6 +1,7 @@ package de.davis.keygo.app.presentation import android.os.Bundle +import androidx.activity.compose.LocalActivity import androidx.activity.compose.setContent import androidx.activity.enableEdgeToEdge import androidx.compose.foundation.layout.Box @@ -51,6 +52,8 @@ import de.davis.keygo.feature.onboarding.presentation.OnboardingRoute import de.davis.keygo.feature.onboarding.presentation.onboardingGraph import de.davis.keygo.feature.settings.presentation.ChangePasswordRoute import de.davis.keygo.feature.settings.presentation.settingsGraph +import de.davis.keygo.feature.totp.presentation.TotpImportRedirect +import de.davis.keygo.feature.totp.presentation.totpImportRedirectGraph import de.davis.keygo.item.dialog.SelectItemContent import kotlinx.coroutines.launch import org.koin.androidx.viewmodel.ext.android.viewModel @@ -98,6 +101,7 @@ private fun destinationAfterUnlock(totpUri: String?): Any = private fun App(hasAccess: Boolean) { val listNavigator = rememberListDetailPaneScaffoldNavigator() val navController = rememberNavController() + val activity = LocalActivity.current val navBackStackEntry by navController.currentBackStackEntryAsState() val currentDestination = navBackStackEntry?.destination @@ -152,12 +156,23 @@ private fun App(hasAccess: Boolean) { startDestination = if (hasAccess) AuthRoute() else OnboardingRoute(), ) { totpImportRedirectGraph( - hasAccess = hasAccess, - navigateAndReplace = { dest -> - navController.navigate(dest) { + onValidated = { pending -> + navController.navigate( + if (hasAccess) AuthRoute( + totpInfo = pending.totpInfo, + queries = pending.queries, + ) + else OnboardingRoute( + totpInfo = pending.totpInfo, + queries = pending.queries, + ), + ) { popUpTo { inclusive = true } } - } + }, + // The app was launched only to import this code. With nothing left to import, the + // Activity is what closes, and :app is the only module that owns one. + onRejected = { activity?.finish() }, ) totpImportGraph( diff --git a/app/src/main/kotlin/de/davis/keygo/app/presentation/TotpImportRedirect.kt b/app/src/main/kotlin/de/davis/keygo/app/presentation/TotpImportRedirect.kt deleted file mode 100644 index 60a420298..000000000 --- a/app/src/main/kotlin/de/davis/keygo/app/presentation/TotpImportRedirect.kt +++ /dev/null @@ -1,42 +0,0 @@ -package de.davis.keygo.app.presentation - -import androidx.compose.runtime.LaunchedEffect -import androidx.navigation.NavGraphBuilder -import androidx.navigation.compose.composable -import androidx.navigation.navDeepLink -import androidx.navigation.toRoute -import de.davis.keygo.core.ui.RouteDestination -import de.davis.keygo.core.ui.model.PendingTotpImport -import de.davis.keygo.feature.auth.presentation.AuthRoute -import de.davis.keygo.feature.onboarding.presentation.OnboardingRoute -import kotlinx.serialization.Serializable - -@Serializable -data class TotpImportRedirect( - val totpInfo: String? = null, - val queries: String? = null, -) : RouteDestination { - val pendingImport: PendingTotpImport - get() = PendingTotpImport(totpInfo, queries) -} - -fun NavGraphBuilder.totpImportRedirectGraph( - hasAccess: Boolean, - navigateAndReplace: (Any) -> Unit, -) { - composable( - deepLinks = listOf( - navDeepLink(basePath = PendingTotpImport.BASE_PATH) { - uriPattern = PendingTotpImport.URI_PATTERN - } - ) - ) { entry -> - val route = entry.toRoute() - LaunchedEffect(route) { - navigateAndReplace( - if (hasAccess) AuthRoute(totpInfo = route.totpInfo, queries = route.queries) - else OnboardingRoute(totpInfo = route.totpInfo, queries = route.queries) - ) - } - } -} diff --git a/app/src/test/kotlin/de/davis/keygo/app/presentation/TotpImportNavGraphTest.kt b/app/src/test/kotlin/de/davis/keygo/app/presentation/TotpImportNavGraphTest.kt index b1d1003bd..8d1429b99 100644 --- a/app/src/test/kotlin/de/davis/keygo/app/presentation/TotpImportNavGraphTest.kt +++ b/app/src/test/kotlin/de/davis/keygo/app/presentation/TotpImportNavGraphTest.kt @@ -14,6 +14,8 @@ import de.davis.keygo.feature.auth.presentation.AuthRoute import de.davis.keygo.feature.auth.presentation.authGraph import de.davis.keygo.feature.onboarding.presentation.OnboardingRoute import de.davis.keygo.feature.onboarding.presentation.onboardingGraph +import de.davis.keygo.feature.totp.presentation.TotpImportRedirect +import de.davis.keygo.feature.totp.presentation.totpImportRedirectGraph import org.junit.runner.RunWith import org.robolectric.RobolectricTestRunner import org.robolectric.annotation.Config @@ -37,7 +39,7 @@ class TotpImportNavGraphTest { controller.graph = controller.createGraph( startDestination = if (hasAccess) AuthRoute() else OnboardingRoute(), ) { - totpImportRedirectGraph(hasAccess = hasAccess, navigateAndReplace = {}) + totpImportRedirectGraph(onValidated = {}, onRejected = {}) totpImportGraph( navigateToDestination = {}, onImportFinished = {}, @@ -173,6 +175,23 @@ class TotpImportNavGraphTest { ) } + @Test + fun `a validated code replaces the redirect entry`() { + val controller = navController(hasAccess = true) + controller.navigate( + TotpImportRedirect(totpInfo = "Example:me@example.com", queries = "secret=ABC"), + ) + + controller.navigate(AuthRoute(totpInfo = "Example:me@example.com", queries = "secret=ABC")) { + popUpTo { inclusive = true } + } + + assertTrue(controller.currentDestination?.hasRoute() == true) + assertFalse( + controller.currentBackStack.value.any { it.destination.hasRoute() }, + ) + } + private companion object { const val DEEP_LINK_URI = "otpauth://totp/GitHub:me@github.com?secret=JBSWY3DPEHPK3PXP&issuer=github.com" From f8d68ce576f65eace777bee284e9e905a074995b Mon Sep 17 00:00:00 2001 From: Davis Wolfermann Date: Mon, 24 Aug 2026 00:14:50 +0200 Subject: [PATCH 10/20] test(totp): cover the validated import routing the app performs --- .../keygo/app/presentation/MainActivity.kt | 38 ++++++++++++------- .../presentation/TotpImportNavGraphTest.kt | 35 +++++++++++++++-- 2 files changed, 55 insertions(+), 18 deletions(-) diff --git a/app/src/main/kotlin/de/davis/keygo/app/presentation/MainActivity.kt b/app/src/main/kotlin/de/davis/keygo/app/presentation/MainActivity.kt index 08025d9c4..dc1a085fa 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 @@ -25,6 +25,7 @@ import androidx.compose.ui.Modifier import androidx.compose.ui.res.stringResource import androidx.core.splashscreen.SplashScreen.Companion.installSplashScreen import androidx.fragment.app.FragmentActivity +import androidx.navigation.NavController import androidx.navigation.NavDestination.Companion.hasRoute import androidx.navigation.NavDestination.Companion.hierarchy import androidx.navigation.compose.NavHost @@ -38,6 +39,7 @@ import com.mikepenz.aboutlibraries.ui.compose.m3.LibrariesContainer import de.davis.keygo.R import de.davis.keygo.app.presentation.component.KeyGoNavigationWrapper import de.davis.keygo.core.presentation.model.RouteDestination +import de.davis.keygo.core.ui.model.PendingTotpImport import de.davis.keygo.core.ui.theme.KeyGoTheme import de.davis.keygo.core.util.domain.snackbar.SnackbarManager import de.davis.keygo.core.util.presentation.snackbar.LocalSnackbarManager @@ -96,6 +98,27 @@ class MainActivity : FragmentActivity() { private fun destinationAfterUnlock(totpUri: String?): Any = totpUri?.let { SelectItemForTotpRoute(it) } ?: RouteDestination.TopLevelAppGraph +/** + * Where a validated code goes once the gate has cleared it. Auth and onboarding are the two ways + * into the app, and the code rides along to whichever one the user needs. The redirect is popped on + * the way out: it exists only to hold the code while it was checked, so there is nothing to come + * back to. + */ +internal fun NavController.navigateToValidatedImport(hasAccess: Boolean, pending: PendingTotpImport) { + navigate( + if (hasAccess) AuthRoute( + totpInfo = pending.totpInfo, + queries = pending.queries, + ) + else OnboardingRoute( + totpInfo = pending.totpInfo, + queries = pending.queries, + ), + ) { + popUpTo { inclusive = true } + } +} + @OptIn(ExperimentalMaterial3AdaptiveApi::class) @Composable private fun App(hasAccess: Boolean) { @@ -156,20 +179,7 @@ private fun App(hasAccess: Boolean) { startDestination = if (hasAccess) AuthRoute() else OnboardingRoute(), ) { totpImportRedirectGraph( - onValidated = { pending -> - navController.navigate( - if (hasAccess) AuthRoute( - totpInfo = pending.totpInfo, - queries = pending.queries, - ) - else OnboardingRoute( - totpInfo = pending.totpInfo, - queries = pending.queries, - ), - ) { - popUpTo { inclusive = true } - } - }, + onValidated = { pending -> navController.navigateToValidatedImport(hasAccess, pending) }, // The app was launched only to import this code. With nothing left to import, the // Activity is what closes, and :app is the only module that owns one. onRejected = { activity?.finish() }, diff --git a/app/src/test/kotlin/de/davis/keygo/app/presentation/TotpImportNavGraphTest.kt b/app/src/test/kotlin/de/davis/keygo/app/presentation/TotpImportNavGraphTest.kt index 8d1429b99..bcacadf55 100644 --- a/app/src/test/kotlin/de/davis/keygo/app/presentation/TotpImportNavGraphTest.kt +++ b/app/src/test/kotlin/de/davis/keygo/app/presentation/TotpImportNavGraphTest.kt @@ -176,17 +176,44 @@ class TotpImportNavGraphTest { } @Test - fun `a validated code replaces the redirect entry`() { + fun `a validated code sends an account with access to AuthRoute`() { val controller = navController(hasAccess = true) controller.navigate( TotpImportRedirect(totpInfo = "Example:me@example.com", queries = "secret=ABC"), ) - controller.navigate(AuthRoute(totpInfo = "Example:me@example.com", queries = "secret=ABC")) { - popUpTo { inclusive = true } - } + controller.navigateToValidatedImport( + hasAccess = true, + pending = PendingTotpImport(totpInfo = "Example:me@example.com", queries = "secret=ABC"), + ) assertTrue(controller.currentDestination?.hasRoute() == true) + + val route = assertNotNull(controller.currentBackStackEntry).toRoute() + assertEquals("Example:me@example.com", route.totpInfo) + assertEquals("secret=ABC", route.queries) + assertFalse( + controller.currentBackStack.value.any { it.destination.hasRoute() }, + ) + } + + @Test + fun `a validated code sends an account without access to OnboardingRoute`() { + val controller = navController(hasAccess = false) + controller.navigate( + TotpImportRedirect(totpInfo = "Example:me@example.com", queries = "secret=ABC"), + ) + + controller.navigateToValidatedImport( + hasAccess = false, + pending = PendingTotpImport(totpInfo = "Example:me@example.com", queries = "secret=ABC"), + ) + + assertTrue(controller.currentDestination?.hasRoute() == true) + + val route = assertNotNull(controller.currentBackStackEntry).toRoute() + assertEquals("Example:me@example.com", route.totpInfo) + assertEquals("secret=ABC", route.queries) assertFalse( controller.currentBackStack.value.any { it.destination.hasRoute() }, ) From 1e38a4a1cd93ba7a7a86adbe3489ff27cc7501c8 Mon Sep 17 00:00:00 2001 From: Davis Wolfermann Date: Mon, 24 Aug 2026 00:34:37 +0200 Subject: [PATCH 11/20] refactor(totp): drop the parse error surfaces the gate made dead --- .../keygo/app/presentation/TotpImportGraph.kt | 3 +- .../presentation/login/LoginViewModel.kt | 11 ++------ .../totp/SelectItemForTotpScreen.kt | 20 ++----------- .../totp/SelectItemForTotpUiState.kt | 7 ++--- .../totp/SelectItemForTotpViewModel.kt | 9 ++---- .../totp/SelectItemForTotpViewModelTest.kt | 28 ++++--------------- 6 files changed, 17 insertions(+), 61 deletions(-) diff --git a/app/src/main/kotlin/de/davis/keygo/app/presentation/TotpImportGraph.kt b/app/src/main/kotlin/de/davis/keygo/app/presentation/TotpImportGraph.kt index 0945bf875..28cd31d93 100644 --- a/app/src/main/kotlin/de/davis/keygo/app/presentation/TotpImportGraph.kt +++ b/app/src/main/kotlin/de/davis/keygo/app/presentation/TotpImportGraph.kt @@ -37,7 +37,7 @@ data class AssignTotpRoute( } /** - * @param onImportFinished the import is over, either saved or abandoned. Leads back into the app + * @param onImportFinished the import is over and the code has been saved. Leads back into the app * with the import routes popped, so back does not return the user to a code they already handled. */ fun NavGraphBuilder.totpImportGraph( @@ -53,7 +53,6 @@ fun NavGraphBuilder.totpImportGraph( navigateToDestination(AssignTotpRoute(route.totpUri, itemId.toString())) }, onCreateNew = { navigateToDestination(AssignTotpRoute(route.totpUri)) }, - onImportAbandoned = onImportFinished, ) } diff --git a/feature/item/create/src/main/kotlin/de/davis/keygo/feature/item/create/presentation/login/LoginViewModel.kt b/feature/item/create/src/main/kotlin/de/davis/keygo/feature/item/create/presentation/login/LoginViewModel.kt index 0bfecd4c8..07f451b48 100644 --- a/feature/item/create/src/main/kotlin/de/davis/keygo/feature/item/create/presentation/login/LoginViewModel.kt +++ b/feature/item/create/src/main/kotlin/de/davis/keygo/feature/item/create/presentation/login/LoginViewModel.kt @@ -127,12 +127,6 @@ internal class LoginViewModel( fun init(information: DetailPaneInformation) { when (information) { is DetailPaneInformation.Init.Existing -> viewModelScope.launch { - // initWithId always resets dialogState to DialogState.None, which would silently - // wipe a parse error shown here. That is safe only because the one caller that - // reaches this branch with a pendingTotpUri, the item picker, already parsed the - // same uri with the same service and abandons the import on failure before - // navigating here. A new caller constructing Init.Existing(pendingTotpUri = ...) - // needs its own parse gate upstream, since this branch will not provide one. information.pendingTotpUri?.let { parsePendingTotp(it) } initWithId(information.id) } @@ -230,13 +224,12 @@ internal class LoginViewModel( /** * Reads a code the picker handed over and remembers it, so [initWithId] can fold it into - * whichever login was chosen. Returns null when the code cannot be read, having already put the - * parse error on screen. + * whichever login was chosen. Returns null when the code cannot be read, which the redirect that + * starts the import already ruled out, so there is nothing to tell the user about here. */ private fun parsePendingTotp(uri: String): TotpInfo? = totpService.getInfoFromUriWithResult(uri).onFailure { failure -> Log.e(TAG, "Error parsing TOTP URI: $failure") - showTotpParseError() }.getOrNull()?.also { totpSecretInformation = it totpOriginalUri = uri diff --git a/feature/item/create/src/main/kotlin/de/davis/keygo/feature/item/create/presentation/totp/SelectItemForTotpScreen.kt b/feature/item/create/src/main/kotlin/de/davis/keygo/feature/item/create/presentation/totp/SelectItemForTotpScreen.kt index 3244aede9..84d6e19fe 100644 --- a/feature/item/create/src/main/kotlin/de/davis/keygo/feature/item/create/presentation/totp/SelectItemForTotpScreen.kt +++ b/feature/item/create/src/main/kotlin/de/davis/keygo/feature/item/create/presentation/totp/SelectItemForTotpScreen.kt @@ -2,7 +2,6 @@ package de.davis.keygo.feature.item.create.presentation.totp import androidx.compose.foundation.layout.consumeWindowInsets import androidx.compose.foundation.layout.fillMaxSize -import androidx.compose.foundation.layout.fillMaxWidth import androidx.compose.foundation.layout.padding import androidx.compose.material.icons.Icons import androidx.compose.material.icons.filled.Add @@ -22,7 +21,6 @@ import de.davis.keygo.core.item.generated.domain.model.VaultItemType import de.davis.keygo.feature.item.create.R import de.davis.keygo.feature.list_screen.presentation.ItemListScreen import de.davis.keygo.feature.list_screen.presentation.NoItemStrategy -import de.davis.keygo.feature.totp.presentation.component.TotpParseErrorDialog import org.koin.androidx.compose.koinViewModel import org.koin.core.parameter.parametersOf @@ -34,20 +32,19 @@ import org.koin.core.parameter.parametersOf * import, which replaced the whole back stack on its way here, so the NavHost does not consume a * back press and the activity finishes on its own. * + * It also carries no error surface. A code that cannot be read never reaches this screen, because + * the redirect that starts the import ends the flow there. + * * @param totpUri the scanned deep link, carried whole so the screen and its ViewModel can parse it * independently. * @param onItemSelected the user picked an existing login to attach the code to. * @param onCreateNew the user chose to attach the code to a login that does not exist yet. - * @param onImportAbandoned the code could not be read, so there is nothing to attach and the flow - * ends. The user stays in the app they just unlocked rather than being thrown out over a malformed - * code from somewhere else. */ @Composable fun SelectItemForTotpScreen( totpUri: String, onItemSelected: (ItemId) -> Unit, onCreateNew: () -> Unit, - onImportAbandoned: () -> Unit, modifier: Modifier = Modifier, ) { val viewModel: SelectItemForTotpViewModel = koinViewModel { parametersOf(totpUri) } @@ -57,10 +54,6 @@ fun SelectItemForTotpScreen( state = state, onItemSelected = onItemSelected, onCreateNew = onCreateNew, - onParseErrorDismiss = { - viewModel.onParseErrorDismissed() - onImportAbandoned() - }, modifier = modifier, ) } @@ -71,7 +64,6 @@ private fun SelectItemForTotpContent( state: SelectItemForTotpUiState, onItemSelected: (ItemId) -> Unit, onCreateNew: () -> Unit, - onParseErrorDismiss: () -> Unit, modifier: Modifier = Modifier, ) { Scaffold( @@ -103,10 +95,4 @@ private fun SelectItemForTotpContent( .padding(innerPadding), ) } - - if (state.parseError) - TotpParseErrorDialog( - onDismiss = onParseErrorDismiss, - modifier = Modifier.fillMaxWidth(), - ) } diff --git a/feature/item/create/src/main/kotlin/de/davis/keygo/feature/item/create/presentation/totp/SelectItemForTotpUiState.kt b/feature/item/create/src/main/kotlin/de/davis/keygo/feature/item/create/presentation/totp/SelectItemForTotpUiState.kt index 45b0776e6..a9edeb31c 100644 --- a/feature/item/create/src/main/kotlin/de/davis/keygo/feature/item/create/presentation/totp/SelectItemForTotpUiState.kt +++ b/feature/item/create/src/main/kotlin/de/davis/keygo/feature/item/create/presentation/totp/SelectItemForTotpUiState.kt @@ -5,11 +5,10 @@ import de.davis.keygo.core.item.domain.alias.ItemId /** * What the picker knows about the scanned code. * - * @param suggestedItemIds the logins on the code's own registrable domain, shown first. An empty - * set is the ordinary case for a code whose domain matches nothing, not an error. - * @param parseError the code could not be read at all, so there is nothing to attach anywhere. + * @param suggestedItemIds the logins on the code's own registrable domain, shown first. An empty set + * is the ordinary case for a code whose domain matches nothing, and is also what an unreadable code + * produces, since the redirect that starts the import already turned those away. */ internal data class SelectItemForTotpUiState( val suggestedItemIds: Set = emptySet(), - val parseError: Boolean = false, ) diff --git a/feature/item/create/src/main/kotlin/de/davis/keygo/feature/item/create/presentation/totp/SelectItemForTotpViewModel.kt b/feature/item/create/src/main/kotlin/de/davis/keygo/feature/item/create/presentation/totp/SelectItemForTotpViewModel.kt index 2ebe08aae..7eaf3827f 100644 --- a/feature/item/create/src/main/kotlin/de/davis/keygo/feature/item/create/presentation/totp/SelectItemForTotpViewModel.kt +++ b/feature/item/create/src/main/kotlin/de/davis/keygo/feature/item/create/presentation/totp/SelectItemForTotpViewModel.kt @@ -23,8 +23,8 @@ import org.koin.core.annotation.KoinViewModel * Backs the screen that asks which login a scanned code belongs to. * * The code arrives as a uri rather than as parsed info because it travels through a navigation - * argument, and navigation carries primitives. Parsing it here is also what lets the picker be the - * screen that reports an unreadable code, since it is the first screen the code reaches. + * argument, and navigation carries primitives. A code that cannot be read gets no suggestions and + * no error, because the redirect that starts the import already refused those. */ @KoinViewModel internal class SelectItemForTotpViewModel( @@ -39,7 +39,6 @@ internal class SelectItemForTotpViewModel( init { totpService.getInfoFromUriWithResult(totpUri).onFailure { failure -> Log.e(TAG, "Error parsing TOTP URI: $failure") - _state.update { it.copy(parseError = true) } }.onSuccess { info -> viewModelScope.launch { val suggested = suggestedItemIdsFor(info) @@ -48,10 +47,6 @@ internal class SelectItemForTotpViewModel( } } - fun onParseErrorDismissed() { - _state.update { it.copy(parseError = false) } - } - /** * The logins whose registrable domain matches the code's own. * diff --git a/feature/item/create/src/test/java/de/davis/keygo/feature/item/create/presentation/totp/SelectItemForTotpViewModelTest.kt b/feature/item/create/src/test/java/de/davis/keygo/feature/item/create/presentation/totp/SelectItemForTotpViewModelTest.kt index 10148b0c8..b69bd20f6 100644 --- a/feature/item/create/src/test/java/de/davis/keygo/feature/item/create/presentation/totp/SelectItemForTotpViewModelTest.kt +++ b/feature/item/create/src/test/java/de/davis/keygo/feature/item/create/presentation/totp/SelectItemForTotpViewModelTest.kt @@ -28,12 +28,11 @@ import kotlin.test.AfterTest import kotlin.test.BeforeTest import kotlin.test.Test import kotlin.test.assertEquals -import kotlin.test.assertFalse -import kotlin.test.assertTrue /** * Covers what the picker knows before the user has chosen anything: which logins the scanned code - * points at, and what happens when the code cannot be read at all. + * points at. An unreadable code is not covered here, because the redirect that starts the import + * rejects those before the picker is ever reached. */ @OptIn(ExperimentalCoroutinesApi::class) @RunWith(RobolectricTestRunner::class) @@ -88,33 +87,18 @@ class SelectItemForTotpViewModelTest { val viewModel = buildViewModel() advanceUntilIdle() - val state = viewModel.state.value - assertEquals(emptySet(), state.suggestedItemIds) - assertFalse(state.parseError) + assertEquals(emptySet(), viewModel.state.value.suggestedItemIds) } @Test - fun `an unreadable code surfaces the parse error`() = runVmTest { + fun `an unreadable code suggests nothing instead of raising an error`() = runVmTest { + seedLogin(name = "GitHub", domain = "github.com") totpService.infoFromUriResult = null val viewModel = buildViewModel() advanceUntilIdle() - val state = viewModel.state.value - assertTrue(state.parseError) - assertEquals(emptySet(), state.suggestedItemIds) - } - - @Test - fun `dismissing the parse error clears it`() = runVmTest { - totpService.infoFromUriResult = null - val viewModel = buildViewModel() - advanceUntilIdle() - - viewModel.onParseErrorDismissed() - advanceUntilIdle() - - assertFalse(viewModel.state.value.parseError) + assertEquals(emptySet(), viewModel.state.value.suggestedItemIds) } // Helpers From c42e14b126a68825385de830e67e6c62aff36c11 Mon Sep 17 00:00:00 2001 From: Davis Wolfermann Date: Mon, 24 Aug 2026 00:44:12 +0200 Subject: [PATCH 12/20] test(totp): name the picker's unreadable code test for what it checks --- .../create/presentation/totp/SelectItemForTotpViewModelTest.kt | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/feature/item/create/src/test/java/de/davis/keygo/feature/item/create/presentation/totp/SelectItemForTotpViewModelTest.kt b/feature/item/create/src/test/java/de/davis/keygo/feature/item/create/presentation/totp/SelectItemForTotpViewModelTest.kt index b69bd20f6..be5797dbb 100644 --- a/feature/item/create/src/test/java/de/davis/keygo/feature/item/create/presentation/totp/SelectItemForTotpViewModelTest.kt +++ b/feature/item/create/src/test/java/de/davis/keygo/feature/item/create/presentation/totp/SelectItemForTotpViewModelTest.kt @@ -91,8 +91,7 @@ class SelectItemForTotpViewModelTest { } @Test - fun `an unreadable code suggests nothing instead of raising an error`() = runVmTest { - seedLogin(name = "GitHub", domain = "github.com") + fun `an unreadable code leaves the picker with no suggestions`() = runVmTest { totpService.infoFromUriResult = null val viewModel = buildViewModel() From 467901e5490aec1e39b0cd7a8e2735621c150ce4 Mon Sep 17 00:00:00 2001 From: Davis Wolfermann Date: Mon, 24 Aug 2026 10:51:33 +0200 Subject: [PATCH 13/20] fix(totp): let back close the deep link gate and restore the picker test's teeth Finishes a fix wave on the TOTP deep link validation gate: - TotpParseErrorDialog now takes an onDismissRequest, wired to the deep link gate's Invalid branch so a back press (which AlertDialog routes through onDismissRequest, not onDismiss) actually closes the gate instead of doing nothing. The two in-app camera scanner call sites keep their old no-op default. - SelectItemForTotpViewModelTest's "an unreadable code leaves the picker with no suggestions" test seeds a login again before asserting on suggestions, so an empty repository cannot make the test pass vacuously. - MainActivity's onRejected callback now logs a warning instead of silently doing nothing when there is no Activity to finish. Not a live bug (LocalActivity.current is never null in production), but a null branch that swallowed the failure would otherwise leave a dead OK button with no trace. - parsePendingTotp's KDoc calls out that it trusts its caller to have validated the uri already, so a new caller passing pendingTotpUri onto DetailPaneInformation.Init needs its own parse gate upstream. Verified :feature:totp and :feature:item:create unit tests plus the full ./gradlew test suite pass. Confirmed the restored seedLogin line matters: breaking SelectItemForTotpViewModel's init to fall through to the suggestion lookup on a failed parse turns the "unreadable code" test red, and reverting brings it back to green. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_019NwwBkR3ks2QMJSRmP1iDH --- .../davis/keygo/app/presentation/MainActivity.kt | 7 ++++++- .../create/presentation/login/LoginViewModel.kt | 4 +++- .../totp/SelectItemForTotpViewModelTest.kt | 1 + .../totp/presentation/TotpImportRedirect.kt | 1 + .../presentation/component/TotpParseErrorDialog.kt | 14 ++++++++++++-- 5 files changed, 23 insertions(+), 4 deletions(-) diff --git a/app/src/main/kotlin/de/davis/keygo/app/presentation/MainActivity.kt b/app/src/main/kotlin/de/davis/keygo/app/presentation/MainActivity.kt index dc1a085fa..0dbdd3332 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 @@ -1,6 +1,7 @@ package de.davis.keygo.app.presentation import android.os.Bundle +import android.util.Log import androidx.activity.compose.LocalActivity import androidx.activity.compose.setContent import androidx.activity.enableEdgeToEdge @@ -61,6 +62,8 @@ import kotlinx.coroutines.launch import org.koin.androidx.viewmodel.ext.android.viewModel import org.koin.compose.koinInject +private const val TAG = "MainActivity" + class MainActivity : FragmentActivity() { private val viewModel by viewModel() @@ -182,7 +185,9 @@ private fun App(hasAccess: Boolean) { onValidated = { pending -> navController.navigateToValidatedImport(hasAccess, pending) }, // The app was launched only to import this code. With nothing left to import, the // Activity is what closes, and :app is the only module that owns one. - onRejected = { activity?.finish() }, + onRejected = { + activity?.finish() ?: Log.w(TAG, "No activity to finish after rejecting an invalid TOTP deep link") + }, ) totpImportGraph( diff --git a/feature/item/create/src/main/kotlin/de/davis/keygo/feature/item/create/presentation/login/LoginViewModel.kt b/feature/item/create/src/main/kotlin/de/davis/keygo/feature/item/create/presentation/login/LoginViewModel.kt index 07f451b48..fd9ba6c62 100644 --- a/feature/item/create/src/main/kotlin/de/davis/keygo/feature/item/create/presentation/login/LoginViewModel.kt +++ b/feature/item/create/src/main/kotlin/de/davis/keygo/feature/item/create/presentation/login/LoginViewModel.kt @@ -225,7 +225,9 @@ internal class LoginViewModel( /** * Reads a code the picker handed over and remembers it, so [initWithId] can fold it into * whichever login was chosen. Returns null when the code cannot be read, which the redirect that - * starts the import already ruled out, so there is nothing to tell the user about here. + * starts the import already ruled out, so there is nothing to tell the user about here. This + * function assumes its caller already validated the uri, so a new caller passing + * `pendingTotpUri` needs its own parse gate upstream. */ private fun parsePendingTotp(uri: String): TotpInfo? = totpService.getInfoFromUriWithResult(uri).onFailure { failure -> diff --git a/feature/item/create/src/test/java/de/davis/keygo/feature/item/create/presentation/totp/SelectItemForTotpViewModelTest.kt b/feature/item/create/src/test/java/de/davis/keygo/feature/item/create/presentation/totp/SelectItemForTotpViewModelTest.kt index be5797dbb..39c7cebfd 100644 --- a/feature/item/create/src/test/java/de/davis/keygo/feature/item/create/presentation/totp/SelectItemForTotpViewModelTest.kt +++ b/feature/item/create/src/test/java/de/davis/keygo/feature/item/create/presentation/totp/SelectItemForTotpViewModelTest.kt @@ -92,6 +92,7 @@ class SelectItemForTotpViewModelTest { @Test fun `an unreadable code leaves the picker with no suggestions`() = runVmTest { + seedLogin(name = "GitHub", domain = "github.com") totpService.infoFromUriResult = null val viewModel = buildViewModel() diff --git a/feature/totp/src/main/kotlin/de/davis/keygo/feature/totp/presentation/TotpImportRedirect.kt b/feature/totp/src/main/kotlin/de/davis/keygo/feature/totp/presentation/TotpImportRedirect.kt index bac91a139..d1a5ef336 100644 --- a/feature/totp/src/main/kotlin/de/davis/keygo/feature/totp/presentation/TotpImportRedirect.kt +++ b/feature/totp/src/main/kotlin/de/davis/keygo/feature/totp/presentation/TotpImportRedirect.kt @@ -69,6 +69,7 @@ fun NavGraphBuilder.totpImportRedirectGraph( TotpImportRedirectState.Invalid -> TotpParseErrorDialog( onDismiss = onRejected, modifier = Modifier.fillMaxWidth(), + onDismissRequest = onRejected, ) } } diff --git a/feature/totp/src/main/kotlin/de/davis/keygo/feature/totp/presentation/component/TotpParseErrorDialog.kt b/feature/totp/src/main/kotlin/de/davis/keygo/feature/totp/presentation/component/TotpParseErrorDialog.kt index 5a6d7b104..8ba0d065f 100644 --- a/feature/totp/src/main/kotlin/de/davis/keygo/feature/totp/presentation/component/TotpParseErrorDialog.kt +++ b/feature/totp/src/main/kotlin/de/davis/keygo/feature/totp/presentation/component/TotpParseErrorDialog.kt @@ -8,10 +8,20 @@ import androidx.compose.ui.Modifier import androidx.compose.ui.res.stringResource import de.davis.keygo.feature.totp.R +/** + * @param onDismissRequest what a back press (or outside tap) does. Defaults to doing nothing, + * because the scanner's dialog sits inside a screen the user can still use, so a stray back press + * should not dismiss it. The deep link gate's dialog is the whole screen, so back has to be a real + * exit and passes [onDismiss] here too. + */ @Composable -fun TotpParseErrorDialog(onDismiss: () -> Unit, modifier: Modifier = Modifier) { +fun TotpParseErrorDialog( + onDismiss: () -> Unit, + modifier: Modifier = Modifier, + onDismissRequest: () -> Unit = {}, +) { AlertDialog( - onDismissRequest = {}, + onDismissRequest = onDismissRequest, confirmButton = { TextButton( onClick = onDismiss From 0e09c50de0473e2b89b468012b972f3d80152283 Mon Sep 17 00:00:00 2001 From: Davis Wolfermann Date: Mon, 24 Aug 2026 12:16:27 +0200 Subject: [PATCH 14/20] refactor(totp): move the item picker into the totp feature The import flow was split three ways: :feature:totp validated the deep link, :feature:item:create drew the picker, and :app owned a totpImportGraph holding both halves of the post-gate flow. The picker is TOTP work, so it belongs with the rest of it. totpImportGraph could not move wholesale, because its AssignTotpRoute renders LoginScreen and :feature:item:create already depends on :feature:totp. But the picker never touches that form -- it only emits an ItemId -- so the graph splits cleanly instead: - :feature:totp owns SelectItemForTotpRoute and selectItemForTotpGraph. Both of its callbacks hand the uri back out, so the module never names a login form. - :feature:item:create owns AssignTotpRoute and assignTotpGraph. - :app keeps only the wiring between the two, in navigateToAssignTotp next to navigateToValidatedImport. resolveTotpDomain moves out of :feature:item:core, which is what let the picker follow it: item:core already depends on totp, so importing it back would have cycled. It was pure TOTP logic in the wrong module regardless. The one new edge is :feature:totp -> :feature:list_screen, since the picker's body is ItemListScreen. Every totp consumer inherits it. TestRegistrableDomainResolver backed both the picker test and LoginViewModelTest, which now live in different modules, so it moves to core:util testFixtures as FakeRegistrableDomainResolver rather than being duplicated. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_019NwwBkR3ks2QMJSRmP1iDH --- .../keygo/app/presentation/MainActivity.kt | 34 +++++++++++----- .../presentation/TotpImportNavGraphTest.kt | 11 ++--- .../util/FakeRegistrableDomainResolver.kt | 4 +- feature/item/create/build.gradle.kts | 4 ++ .../presentation/login/LoginViewModel.kt | 2 +- .../presentation/totp/AssignTotpGraph.kt | 32 +++++---------- .../create/src/main/res/values/strings.xml | 2 - .../presentation/login/LoginViewModelTest.kt | 4 +- feature/totp/build.gradle.kts | 2 + .../feature/totp}/domain/model/TotpDomain.kt | 2 +- .../presentation/SelectItemForTotpGraph.kt | 40 +++++++++++++++++++ .../presentation}/SelectItemForTotpScreen.kt | 4 +- .../presentation}/SelectItemForTotpUiState.kt | 2 +- .../SelectItemForTotpViewModel.kt | 4 +- feature/totp/src/main/res/values/strings.xml | 3 ++ .../totp}/domain/model/TotpDomainTest.kt | 2 +- .../SelectItemForTotpViewModelTest.kt | 6 +-- 17 files changed, 104 insertions(+), 54 deletions(-) rename feature/item/create/src/test/java/de/davis/keygo/feature/item/create/presentation/TestRegistrableDomainResolver.kt => core/util/src/testFixtures/kotlin/de/davis/keygo/core/util/FakeRegistrableDomainResolver.kt (78%) rename app/src/main/kotlin/de/davis/keygo/app/presentation/TotpImportGraph.kt => feature/item/create/src/main/kotlin/de/davis/keygo/feature/item/create/presentation/totp/AssignTotpGraph.kt (66%) rename feature/{item/core/src/main/kotlin/de/davis/keygo/feature/item/core => totp/src/main/kotlin/de/davis/keygo/feature/totp}/domain/model/TotpDomain.kt (83%) create mode 100644 feature/totp/src/main/kotlin/de/davis/keygo/feature/totp/presentation/SelectItemForTotpGraph.kt rename feature/{item/create/src/main/kotlin/de/davis/keygo/feature/item/create/presentation/totp => totp/src/main/kotlin/de/davis/keygo/feature/totp/presentation}/SelectItemForTotpScreen.kt (97%) rename feature/{item/create/src/main/kotlin/de/davis/keygo/feature/item/create/presentation/totp => totp/src/main/kotlin/de/davis/keygo/feature/totp/presentation}/SelectItemForTotpUiState.kt (89%) rename feature/{item/create/src/main/kotlin/de/davis/keygo/feature/item/create/presentation/totp => totp/src/main/kotlin/de/davis/keygo/feature/totp/presentation}/SelectItemForTotpViewModel.kt (95%) rename feature/{item/core/src/test/kotlin/de/davis/keygo/feature/item/core => totp/src/test/kotlin/de/davis/keygo/feature/totp}/domain/model/TotpDomainTest.kt (96%) rename feature/{item/create/src/test/java/de/davis/keygo/feature/item/create/presentation/totp => totp/src/test/kotlin/de/davis/keygo/feature/totp/presentation}/SelectItemForTotpViewModelTest.kt (96%) 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 0dbdd3332..488d3c6a6 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 @@ -51,11 +51,15 @@ import de.davis.keygo.feature.auth.presentation.AuthRoute import de.davis.keygo.feature.auth.presentation.authGraph import de.davis.keygo.feature.backup.presentation.BackupHubRoute import de.davis.keygo.feature.backup.presentation.backupGraph +import de.davis.keygo.feature.item.create.presentation.totp.AssignTotpRoute +import de.davis.keygo.feature.item.create.presentation.totp.assignTotpGraph import de.davis.keygo.feature.onboarding.presentation.OnboardingRoute import de.davis.keygo.feature.onboarding.presentation.onboardingGraph import de.davis.keygo.feature.settings.presentation.ChangePasswordRoute import de.davis.keygo.feature.settings.presentation.settingsGraph +import de.davis.keygo.feature.totp.presentation.SelectItemForTotpRoute import de.davis.keygo.feature.totp.presentation.TotpImportRedirect +import de.davis.keygo.feature.totp.presentation.selectItemForTotpGraph import de.davis.keygo.feature.totp.presentation.totpImportRedirectGraph import de.davis.keygo.item.dialog.SelectItemContent import kotlinx.coroutines.launch @@ -122,6 +126,18 @@ internal fun NavController.navigateToValidatedImport(hasAccess: Boolean, pending } } +/** + * Where the picker's answer goes. The picker stays composed and collecting through its exit + * transition, so a double tap on a row can fire twice before the first navigation leaves it. + * [AssignTotpRoute] is a data class, so launchSingleTop dedupes the repeat instead of pushing it + * twice onto the back stack. + */ +internal fun NavController.navigateToAssignTotp(route: AssignTotpRoute) { + navigate(route) { + launchSingleTop = true + } +} + @OptIn(ExperimentalMaterial3AdaptiveApi::class) @Composable private fun App(hasAccess: Boolean) { @@ -190,16 +206,16 @@ private fun App(hasAccess: Boolean) { }, ) - totpImportGraph( - // The picker stays composed and collecting through its exit transition, so a - // double tap on a row can fire twice before the first navigation leaves it. - // AssignTotpRoute is a data class, so launchSingleTop dedupes the repeat instead - // of pushing it twice onto the back stack. - navigateToDestination = { dest -> - navController.navigate(dest) { - launchSingleTop = true - } + selectItemForTotpGraph( + onItemSelected = { totpUri, itemId -> + navController.navigateToAssignTotp(AssignTotpRoute(totpUri, itemId.toString())) + }, + onCreateNew = { totpUri -> + navController.navigateToAssignTotp(AssignTotpRoute(totpUri)) }, + ) + + assignTotpGraph( onImportFinished = { navController.navigate(RouteDestination.TopLevelAppGraph) { popUpTo { inclusive = true } diff --git a/app/src/test/kotlin/de/davis/keygo/app/presentation/TotpImportNavGraphTest.kt b/app/src/test/kotlin/de/davis/keygo/app/presentation/TotpImportNavGraphTest.kt index bcacadf55..1e009fef6 100644 --- a/app/src/test/kotlin/de/davis/keygo/app/presentation/TotpImportNavGraphTest.kt +++ b/app/src/test/kotlin/de/davis/keygo/app/presentation/TotpImportNavGraphTest.kt @@ -12,9 +12,13 @@ import de.davis.keygo.core.item.domain.alias.newItemId import de.davis.keygo.core.ui.model.PendingTotpImport import de.davis.keygo.feature.auth.presentation.AuthRoute import de.davis.keygo.feature.auth.presentation.authGraph +import de.davis.keygo.feature.item.create.presentation.totp.AssignTotpRoute +import de.davis.keygo.feature.item.create.presentation.totp.assignTotpGraph import de.davis.keygo.feature.onboarding.presentation.OnboardingRoute import de.davis.keygo.feature.onboarding.presentation.onboardingGraph +import de.davis.keygo.feature.totp.presentation.SelectItemForTotpRoute import de.davis.keygo.feature.totp.presentation.TotpImportRedirect +import de.davis.keygo.feature.totp.presentation.selectItemForTotpGraph import de.davis.keygo.feature.totp.presentation.totpImportRedirectGraph import org.junit.runner.RunWith import org.robolectric.RobolectricTestRunner @@ -40,11 +44,8 @@ class TotpImportNavGraphTest { startDestination = if (hasAccess) AuthRoute() else OnboardingRoute(), ) { totpImportRedirectGraph(onValidated = {}, onRejected = {}) - totpImportGraph( - navigateToDestination = {}, - onImportFinished = {}, - navigateUp = {}, - ) + selectItemForTotpGraph(onItemSelected = { _, _ -> }, onCreateNew = {}) + assignTotpGraph(onImportFinished = {}, navigateUp = {}) authGraph(onSuccess = {}) onboardingGraph(onSuccess = {}) } diff --git a/feature/item/create/src/test/java/de/davis/keygo/feature/item/create/presentation/TestRegistrableDomainResolver.kt b/core/util/src/testFixtures/kotlin/de/davis/keygo/core/util/FakeRegistrableDomainResolver.kt similarity index 78% rename from feature/item/create/src/test/java/de/davis/keygo/feature/item/create/presentation/TestRegistrableDomainResolver.kt rename to core/util/src/testFixtures/kotlin/de/davis/keygo/core/util/FakeRegistrableDomainResolver.kt index af5c7fc21..0b00de1e7 100644 --- a/feature/item/create/src/test/java/de/davis/keygo/feature/item/create/presentation/TestRegistrableDomainResolver.kt +++ b/core/util/src/testFixtures/kotlin/de/davis/keygo/core/util/FakeRegistrableDomainResolver.kt @@ -1,9 +1,9 @@ -package de.davis.keygo.feature.item.create.presentation +package de.davis.keygo.core.util import de.davis.keygo.core.util.domain.resolver.RegistrableDomainResolver /** Resolves an eTLD+1 by keeping the last two labels, which is enough for the test domains. */ -internal class TestRegistrableDomainResolver : RegistrableDomainResolver { +class FakeRegistrableDomainResolver : RegistrableDomainResolver { override fun resolve(domain: String): String? { val labels = domain.substringAfter("://") .substringBefore('/') diff --git a/feature/item/create/build.gradle.kts b/feature/item/create/build.gradle.kts index f1f0ccd68..57a45e494 100644 --- a/feature/item/create/build.gradle.kts +++ b/feature/item/create/build.gradle.kts @@ -1,5 +1,6 @@ plugins { alias(libs.plugins.keygo.android.compose) + alias(libs.plugins.kotlin.serialization) } android { @@ -11,6 +12,8 @@ android { } dependencies { + implementation(libs.androidx.navigation.compose) + implementation(projects.core.ui) implementation(projects.core.item) implementation(projects.core.security) @@ -23,6 +26,7 @@ dependencies { testImplementation(testFixtures(projects.core.item)) testImplementation(testFixtures(projects.core.security)) + testImplementation(testFixtures(projects.core.util)) testImplementation(testFixtures(projects.rust)) testImplementation(libs.robolectric) testImplementation(libs.androidx.junit) diff --git a/feature/item/create/src/main/kotlin/de/davis/keygo/feature/item/create/presentation/login/LoginViewModel.kt b/feature/item/create/src/main/kotlin/de/davis/keygo/feature/item/create/presentation/login/LoginViewModel.kt index fd9ba6c62..e019c6441 100644 --- a/feature/item/create/src/main/kotlin/de/davis/keygo/feature/item/create/presentation/login/LoginViewModel.kt +++ b/feature/item/create/src/main/kotlin/de/davis/keygo/feature/item/create/presentation/login/LoginViewModel.kt @@ -26,7 +26,6 @@ import de.davis.keygo.core.util.presentation.UIText.Companion.ResourceString import de.davis.keygo.feature.item.core.domain.model.ItemUpsertError import de.davis.keygo.feature.item.core.domain.model.UpsertLogin import de.davis.keygo.feature.item.core.domain.model.fieldUpdate -import de.davis.keygo.feature.item.core.domain.model.resolveTotpDomain import de.davis.keygo.feature.item.core.domain.model.set import de.davis.keygo.feature.item.core.domain.usecase.CreateNewOrUpdateLoginUseCase import de.davis.keygo.feature.item.core.presentation.login.model.FieldType @@ -40,6 +39,7 @@ import de.davis.keygo.feature.item.create.presentation.login.model.LoginPasskeyI import de.davis.keygo.feature.item.create.presentation.login.model.LoginUiEvent import de.davis.keygo.feature.item.create.presentation.login.model.OverrideTotpField import de.davis.keygo.feature.item.create.presentation.model.ItemUiState +import de.davis.keygo.feature.totp.domain.model.resolveTotpDomain import de.davis.keygo.rust.totp.TotpService import de.davis.keygo.rust.totp.getInfoFromUriWithResult import de.davis.keygo.rust.totp.getUrlWithResult diff --git a/app/src/main/kotlin/de/davis/keygo/app/presentation/TotpImportGraph.kt b/feature/item/create/src/main/kotlin/de/davis/keygo/feature/item/create/presentation/totp/AssignTotpGraph.kt similarity index 66% rename from app/src/main/kotlin/de/davis/keygo/app/presentation/TotpImportGraph.kt rename to feature/item/create/src/main/kotlin/de/davis/keygo/feature/item/create/presentation/totp/AssignTotpGraph.kt index 28cd31d93..cb271da6b 100644 --- a/app/src/main/kotlin/de/davis/keygo/app/presentation/TotpImportGraph.kt +++ b/feature/item/create/src/main/kotlin/de/davis/keygo/feature/item/create/presentation/totp/AssignTotpGraph.kt @@ -1,4 +1,4 @@ -package de.davis.keygo.app.presentation +package de.davis.keygo.feature.item.create.presentation.totp import androidx.navigation.NavGraphBuilder import androidx.navigation.compose.composable @@ -8,18 +8,9 @@ import de.davis.keygo.core.item.generated.domain.model.VaultItemType import de.davis.keygo.core.ui.RouteDestination import de.davis.keygo.feature.item.core.presentation.model.DetailPaneInformation import de.davis.keygo.feature.item.create.presentation.login.LoginScreen -import de.davis.keygo.feature.item.create.presentation.totp.SelectItemForTotpScreen import kotlinx.serialization.Serializable import java.util.UUID -/** - * Where a deep-linked code lands once the user is through the door. The uri travels whole, unlike - * on [de.davis.keygo.feature.auth.presentation.AuthRoute], because only the deep link's own - * `otpauth://totp/{totpInfo}?{queries}` pattern forces that split. - */ -@Serializable -data class SelectItemForTotpRoute(val totpUri: String) : RouteDestination - /** * The login form for a chosen item, or for a new one when [itemId] is null. * @@ -37,25 +28,20 @@ data class AssignTotpRoute( } /** + * The last step of a deep-linked import: the login form, opened on the item the picker chose, with + * the scanned code already pending on it. + * + * The picker itself lives in the totp feature, which knows nothing about a login form. It hands the + * uri back to whoever wired the two graphs together, and that caller navigates to + * [AssignTotpRoute]. + * * @param onImportFinished the import is over and the code has been saved. Leads back into the app * with the import routes popped, so back does not return the user to a code they already handled. */ -fun NavGraphBuilder.totpImportGraph( - navigateToDestination: (Any) -> Unit, +fun NavGraphBuilder.assignTotpGraph( onImportFinished: () -> Unit, navigateUp: () -> Unit, ) { - composable { entry -> - val route = entry.toRoute() - SelectItemForTotpScreen( - totpUri = route.totpUri, - onItemSelected = { itemId -> - navigateToDestination(AssignTotpRoute(route.totpUri, itemId.toString())) - }, - onCreateNew = { navigateToDestination(AssignTotpRoute(route.totpUri)) }, - ) - } - composable { entry -> val route = entry.toRoute() LoginScreen( diff --git a/feature/item/create/src/main/res/values/strings.xml b/feature/item/create/src/main/res/values/strings.xml index 4e04876a8..0ede2902d 100644 --- a/feature/item/create/src/main/res/values/strings.xml +++ b/feature/item/create/src/main/res/values/strings.xml @@ -17,8 +17,6 @@ Keep Select an Item - Create New - Add code to which item? Length [%d] Character Sets diff --git a/feature/item/create/src/test/java/de/davis/keygo/feature/item/create/presentation/login/LoginViewModelTest.kt b/feature/item/create/src/test/java/de/davis/keygo/feature/item/create/presentation/login/LoginViewModelTest.kt index 9c4ffc967..2e2d96106 100644 --- a/feature/item/create/src/test/java/de/davis/keygo/feature/item/create/presentation/login/LoginViewModelTest.kt +++ b/feature/item/create/src/test/java/de/davis/keygo/feature/item/create/presentation/login/LoginViewModelTest.kt @@ -19,13 +19,13 @@ import de.davis.keygo.core.item.domain.usecase.UpsertVaultItemUseCase import de.davis.keygo.core.item.generated.domain.model.VaultItemType import de.davis.keygo.core.security.crypto.FakeCryptographicScopeProvider import de.davis.keygo.core.security.domain.usecase.ItemWithCryptoScopeUseCase +import de.davis.keygo.core.util.FakeRegistrableDomainResolver import de.davis.keygo.core.util.domain.model.snackbar.SnackbarMessage import de.davis.keygo.core.util.domain.snackbar.SnackbarManager import de.davis.keygo.core.util.domain.usecase.SortUseCase import de.davis.keygo.feature.item.core.domain.usecase.CreateNewOrUpdateLoginUseCase import de.davis.keygo.feature.item.core.presentation.login.model.FieldType import de.davis.keygo.feature.item.core.presentation.model.DetailPaneInformation -import de.davis.keygo.feature.item.create.presentation.TestRegistrableDomainResolver import de.davis.keygo.feature.item.create.presentation.login.model.DialogState import de.davis.keygo.feature.item.create.presentation.login.model.LoginBaseState import de.davis.keygo.feature.item.create.presentation.login.model.LoginUiEvent @@ -79,7 +79,7 @@ class LoginViewModelTest { private val vaultContextRepository = FakeVaultContextRepository() private val cryptoProvider = FakeCryptographicScopeProvider(itemRepository) private val totpService = FakeTotpService() - private val domainResolver = TestRegistrableDomainResolver() + private val domainResolver = FakeRegistrableDomainResolver() @BeforeTest fun setUp() { diff --git a/feature/totp/build.gradle.kts b/feature/totp/build.gradle.kts index 41fe347d1..a03af0446 100644 --- a/feature/totp/build.gradle.kts +++ b/feature/totp/build.gradle.kts @@ -26,6 +26,7 @@ dependencies { implementation(projects.core.security) implementation(projects.core.item) implementation(projects.core.util) + implementation(projects.feature.listScreen) implementation(libs.androidx.navigation.compose) @@ -38,6 +39,7 @@ dependencies { testImplementation(testFixtures(projects.core.security)) testImplementation(testFixtures(projects.core.item)) + testImplementation(testFixtures(projects.core.util)) testImplementation(testFixtures(projects.rust)) testImplementation(libs.robolectric) testImplementation(libs.androidx.junit) diff --git a/feature/item/core/src/main/kotlin/de/davis/keygo/feature/item/core/domain/model/TotpDomain.kt b/feature/totp/src/main/kotlin/de/davis/keygo/feature/totp/domain/model/TotpDomain.kt similarity index 83% rename from feature/item/core/src/main/kotlin/de/davis/keygo/feature/item/core/domain/model/TotpDomain.kt rename to feature/totp/src/main/kotlin/de/davis/keygo/feature/totp/domain/model/TotpDomain.kt index f6d6a97c0..024c831f9 100644 --- a/feature/item/core/src/main/kotlin/de/davis/keygo/feature/item/core/domain/model/TotpDomain.kt +++ b/feature/totp/src/main/kotlin/de/davis/keygo/feature/totp/domain/model/TotpDomain.kt @@ -1,4 +1,4 @@ -package de.davis.keygo.feature.item.core.domain.model +package de.davis.keygo.feature.totp.domain.model fun resolveTotpDomain(issuer: String?, accountName: String): String? { if (!issuer.isNullOrEmpty()) return issuer diff --git a/feature/totp/src/main/kotlin/de/davis/keygo/feature/totp/presentation/SelectItemForTotpGraph.kt b/feature/totp/src/main/kotlin/de/davis/keygo/feature/totp/presentation/SelectItemForTotpGraph.kt new file mode 100644 index 000000000..3e31d30b2 --- /dev/null +++ b/feature/totp/src/main/kotlin/de/davis/keygo/feature/totp/presentation/SelectItemForTotpGraph.kt @@ -0,0 +1,40 @@ +package de.davis.keygo.feature.totp.presentation + +import androidx.navigation.NavGraphBuilder +import androidx.navigation.compose.composable +import androidx.navigation.toRoute +import de.davis.keygo.core.item.domain.alias.ItemId +import de.davis.keygo.core.ui.RouteDestination +import kotlinx.serialization.Serializable + +/** + * Where a deep-linked code lands once the user is through the door. The uri travels whole here, + * unlike on the route that carried it through the gate, because only the deep link's own + * `otpauth://totp/{totpInfo}?{queries}` pattern forces that split. + */ +@Serializable +data class SelectItemForTotpRoute(val totpUri: String) : RouteDestination + +/** + * Asks which login a validated code belongs to, and lets the caller decide where the answer leads. + * + * The destination is the caller's to pick: the code ends up on a login form, and this module knows + * nothing about one. Both callbacks hand the uri back out, because the form needs it and only this + * graph's route is holding it. + * + * @param onItemSelected the user picked an existing login to attach the code to. + * @param onCreateNew the user chose to attach the code to a login that does not exist yet. + */ +fun NavGraphBuilder.selectItemForTotpGraph( + onItemSelected: (totpUri: String, itemId: ItemId) -> Unit, + onCreateNew: (totpUri: String) -> Unit, +) { + composable { entry -> + val route = entry.toRoute() + SelectItemForTotpScreen( + totpUri = route.totpUri, + onItemSelected = { itemId -> onItemSelected(route.totpUri, itemId) }, + onCreateNew = { onCreateNew(route.totpUri) }, + ) + } +} diff --git a/feature/item/create/src/main/kotlin/de/davis/keygo/feature/item/create/presentation/totp/SelectItemForTotpScreen.kt b/feature/totp/src/main/kotlin/de/davis/keygo/feature/totp/presentation/SelectItemForTotpScreen.kt similarity index 97% rename from feature/item/create/src/main/kotlin/de/davis/keygo/feature/item/create/presentation/totp/SelectItemForTotpScreen.kt rename to feature/totp/src/main/kotlin/de/davis/keygo/feature/totp/presentation/SelectItemForTotpScreen.kt index 84d6e19fe..0cd315071 100644 --- a/feature/item/create/src/main/kotlin/de/davis/keygo/feature/item/create/presentation/totp/SelectItemForTotpScreen.kt +++ b/feature/totp/src/main/kotlin/de/davis/keygo/feature/totp/presentation/SelectItemForTotpScreen.kt @@ -1,4 +1,4 @@ -package de.davis.keygo.feature.item.create.presentation.totp +package de.davis.keygo.feature.totp.presentation import androidx.compose.foundation.layout.consumeWindowInsets import androidx.compose.foundation.layout.fillMaxSize @@ -18,9 +18,9 @@ import androidx.compose.ui.res.stringResource import androidx.lifecycle.compose.collectAsStateWithLifecycle import de.davis.keygo.core.item.domain.alias.ItemId import de.davis.keygo.core.item.generated.domain.model.VaultItemType -import de.davis.keygo.feature.item.create.R import de.davis.keygo.feature.list_screen.presentation.ItemListScreen import de.davis.keygo.feature.list_screen.presentation.NoItemStrategy +import de.davis.keygo.feature.totp.R import org.koin.androidx.compose.koinViewModel import org.koin.core.parameter.parametersOf diff --git a/feature/item/create/src/main/kotlin/de/davis/keygo/feature/item/create/presentation/totp/SelectItemForTotpUiState.kt b/feature/totp/src/main/kotlin/de/davis/keygo/feature/totp/presentation/SelectItemForTotpUiState.kt similarity index 89% rename from feature/item/create/src/main/kotlin/de/davis/keygo/feature/item/create/presentation/totp/SelectItemForTotpUiState.kt rename to feature/totp/src/main/kotlin/de/davis/keygo/feature/totp/presentation/SelectItemForTotpUiState.kt index a9edeb31c..ea4610e86 100644 --- a/feature/item/create/src/main/kotlin/de/davis/keygo/feature/item/create/presentation/totp/SelectItemForTotpUiState.kt +++ b/feature/totp/src/main/kotlin/de/davis/keygo/feature/totp/presentation/SelectItemForTotpUiState.kt @@ -1,4 +1,4 @@ -package de.davis.keygo.feature.item.create.presentation.totp +package de.davis.keygo.feature.totp.presentation import de.davis.keygo.core.item.domain.alias.ItemId diff --git a/feature/item/create/src/main/kotlin/de/davis/keygo/feature/item/create/presentation/totp/SelectItemForTotpViewModel.kt b/feature/totp/src/main/kotlin/de/davis/keygo/feature/totp/presentation/SelectItemForTotpViewModel.kt similarity index 95% rename from feature/item/create/src/main/kotlin/de/davis/keygo/feature/item/create/presentation/totp/SelectItemForTotpViewModel.kt rename to feature/totp/src/main/kotlin/de/davis/keygo/feature/totp/presentation/SelectItemForTotpViewModel.kt index 7eaf3827f..a3628c5aa 100644 --- a/feature/item/create/src/main/kotlin/de/davis/keygo/feature/item/create/presentation/totp/SelectItemForTotpViewModel.kt +++ b/feature/totp/src/main/kotlin/de/davis/keygo/feature/totp/presentation/SelectItemForTotpViewModel.kt @@ -1,4 +1,4 @@ -package de.davis.keygo.feature.item.create.presentation.totp +package de.davis.keygo.feature.totp.presentation import android.util.Log import androidx.lifecycle.ViewModel @@ -7,7 +7,7 @@ import de.davis.keygo.core.item.domain.alias.ItemId import de.davis.keygo.core.security.domain.usecase.GetTdlMatchedLoginsUseCase import de.davis.keygo.core.util.onFailure import de.davis.keygo.core.util.onSuccess -import de.davis.keygo.feature.item.core.domain.model.resolveTotpDomain +import de.davis.keygo.feature.totp.domain.model.resolveTotpDomain import de.davis.keygo.rust.totp.TotpService import de.davis.keygo.rust.totp.getInfoFromUriWithResult import de.davisalessandro.keygo.rust.TotpInfo diff --git a/feature/totp/src/main/res/values/strings.xml b/feature/totp/src/main/res/values/strings.xml index d8e13a17d..57cb6710d 100644 --- a/feature/totp/src/main/res/values/strings.xml +++ b/feature/totp/src/main/res/values/strings.xml @@ -13,4 +13,7 @@ TOTP Parse Error The TOTP code could not be parsed. + + Add code to which item? + Create New \ No newline at end of file diff --git a/feature/item/core/src/test/kotlin/de/davis/keygo/feature/item/core/domain/model/TotpDomainTest.kt b/feature/totp/src/test/kotlin/de/davis/keygo/feature/totp/domain/model/TotpDomainTest.kt similarity index 96% rename from feature/item/core/src/test/kotlin/de/davis/keygo/feature/item/core/domain/model/TotpDomainTest.kt rename to feature/totp/src/test/kotlin/de/davis/keygo/feature/totp/domain/model/TotpDomainTest.kt index 5b491ff8a..c6dbbcbe7 100644 --- a/feature/item/core/src/test/kotlin/de/davis/keygo/feature/item/core/domain/model/TotpDomainTest.kt +++ b/feature/totp/src/test/kotlin/de/davis/keygo/feature/totp/domain/model/TotpDomainTest.kt @@ -1,4 +1,4 @@ -package de.davis.keygo.feature.item.core.domain.model +package de.davis.keygo.feature.totp.domain.model import kotlin.test.Test import kotlin.test.assertEquals diff --git a/feature/item/create/src/test/java/de/davis/keygo/feature/item/create/presentation/totp/SelectItemForTotpViewModelTest.kt b/feature/totp/src/test/kotlin/de/davis/keygo/feature/totp/presentation/SelectItemForTotpViewModelTest.kt similarity index 96% rename from feature/item/create/src/test/java/de/davis/keygo/feature/item/create/presentation/totp/SelectItemForTotpViewModelTest.kt rename to feature/totp/src/test/kotlin/de/davis/keygo/feature/totp/presentation/SelectItemForTotpViewModelTest.kt index 39c7cebfd..fd7928f45 100644 --- a/feature/item/create/src/test/java/de/davis/keygo/feature/item/create/presentation/totp/SelectItemForTotpViewModelTest.kt +++ b/feature/totp/src/test/kotlin/de/davis/keygo/feature/totp/presentation/SelectItemForTotpViewModelTest.kt @@ -1,4 +1,4 @@ -package de.davis.keygo.feature.item.create.presentation.totp +package de.davis.keygo.feature.totp.presentation import de.davis.keygo.core.item.FakeLoginRepository import de.davis.keygo.core.item.domain.alias.ItemId @@ -9,7 +9,7 @@ import de.davis.keygo.core.item.domain.model.KeyInformation import de.davis.keygo.core.item.domain.model.Login import de.davis.keygo.core.item.domain.model.Timestamp import de.davis.keygo.core.security.domain.usecase.GetTdlMatchedLoginsUseCase -import de.davis.keygo.feature.item.create.presentation.TestRegistrableDomainResolver +import de.davis.keygo.core.util.FakeRegistrableDomainResolver import de.davis.keygo.rust.FakeTotpService import de.davisalessandro.keygo.rust.Algorithm import de.davisalessandro.keygo.rust.TotpInfo @@ -44,7 +44,7 @@ class SelectItemForTotpViewModelTest { private val vaultId = newVaultId() private val loginRepository = FakeLoginRepository() private val totpService = FakeTotpService() - private val domainResolver = TestRegistrableDomainResolver() + private val domainResolver = FakeRegistrableDomainResolver() @BeforeTest fun setUp() { From 37e6ca0570995aca511ba12d5f7cbf825b17e63d Mon Sep 17 00:00:00 2001 From: Davis Wolfermann Date: Mon, 24 Aug 2026 12:39:01 +0200 Subject: [PATCH 15/20] refactor(totp): pass the scanned code instead of parking it on the view model totpSecretInformation and totpOriginalUri held a code across calls, but only one of their seven uses actually needed that. OnCodesScanned wrote both and then passed the same two values as arguments on the next line. The override handlers wrapped their work in totpSecretInformation?.let { } without ever touching it: applyToUi reads before and after off DialogState.OverrideTotp.fields, so the field was a null guard, and a redundant one. OverrideTotp is only set at the end of requestTotpSecretUpdate, whose callers had both just assigned the field non-null, so the dialog state check above it was already the whole guard. The one real carry was the deep-linked import onto an existing item, where init parsed the uri before the suspending load and initWithId folded it in after. That is now a parameter, and the parse happens where the result is used. With no caller left that lacks a uri, requestTotpSecretUpdate and updateUiWithTotpSecretInfo take a non-null originalUri, which retires the `?: secretInformation.secret` fallback neither could reach. Confirm and keep on the override dialog had no coverage, so they get tests before the guard comes off. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_019NwwBkR3ks2QMJSRmP1iDH --- .../presentation/login/LoginViewModel.kt | 56 ++++++++----------- .../presentation/login/LoginViewModelTest.kt | 49 +++++++++++++--- 2 files changed, 64 insertions(+), 41 deletions(-) diff --git a/feature/item/create/src/main/kotlin/de/davis/keygo/feature/item/create/presentation/login/LoginViewModel.kt b/feature/item/create/src/main/kotlin/de/davis/keygo/feature/item/create/presentation/login/LoginViewModel.kt index e019c6441..0e7e0580a 100644 --- a/feature/item/create/src/main/kotlin/de/davis/keygo/feature/item/create/presentation/login/LoginViewModel.kt +++ b/feature/item/create/src/main/kotlin/de/davis/keygo/feature/item/create/presentation/login/LoginViewModel.kt @@ -106,9 +106,6 @@ internal class LoginViewModel( base.copy(strengthScore = score) } - private var totpSecretInformation: TotpInfo? = null - private var totpOriginalUri: String? = null - /** * Shows a passkey for [rp] as pending until the item is saved. * @@ -127,8 +124,7 @@ internal class LoginViewModel( fun init(information: DetailPaneInformation) { when (information) { is DetailPaneInformation.Init.Existing -> viewModelScope.launch { - information.pendingTotpUri?.let { parsePendingTotp(it) } - initWithId(information.id) + initWithId(information.id, information.pendingTotpUri) } is DetailPaneInformation.Init.New -> information.pendingTotpUri?.let { uri -> @@ -164,7 +160,12 @@ internal class LoginViewModel( } } - private suspend fun initWithId(itemId: ItemId) { + /** + * @param pendingTotpUri a code the picker handed over, folded in once the item it belongs to is + * on screen. It is parsed here rather than before the load, so nothing has to be carried across + * the two. + */ + private suspend fun initWithId(itemId: ItemId, pendingTotpUri: String?) { this.itemId = itemId itemWithCryptoScope.oneShot( @@ -216,26 +217,22 @@ internal class LoginViewModel( ) } - totpSecretInformation?.let { - requestTotpSecretUpdate(it, totpOriginalUri) + pendingTotpUri?.let { uri -> + parsePendingTotp(uri)?.let { requestTotpSecretUpdate(it, uri) } } } } /** - * Reads a code the picker handed over and remembers it, so [initWithId] can fold it into - * whichever login was chosen. Returns null when the code cannot be read, which the redirect that - * starts the import already ruled out, so there is nothing to tell the user about here. This - * function assumes its caller already validated the uri, so a new caller passing + * Reads a code the picker handed over. Returns null when the code cannot be read, which the + * redirect that starts the import already ruled out, so there is nothing to tell the user about + * here. This function assumes its caller already validated the uri, so a new caller passing * `pendingTotpUri` needs its own parse gate upstream. */ private fun parsePendingTotp(uri: String): TotpInfo? = totpService.getInfoFromUriWithResult(uri).onFailure { failure -> Log.e(TAG, "Error parsing TOTP URI: $failure") - }.getOrNull()?.also { - totpSecretInformation = it - totpOriginalUri = uri - } + }.getOrNull() override fun onSubmit() { val ready = state.value as? ItemUiState.Ready ?: return @@ -333,16 +330,12 @@ internal class LoginViewModel( is LoginUiEvent.OnCodesScanned -> { event.codes.firstNotNullOfOrNull { code -> - totpService.getInfoFromUriWithResult(code).onFailure { failure -> - Log.e(TAG, "Error parsing TOTP URI: $failure") - }.getOrNull()?.let { code to it } + parsePendingTotp(code)?.let { code to it } }?.let { (scannedUri, secretInfo) -> _base.update { state -> state.copy(scanning = false) } - totpOriginalUri = scannedUri - totpSecretInformation = secretInfo requestTotpSecretUpdate(secretInfo, scannedUri) } ?: showTotpParseError() } @@ -370,21 +363,16 @@ internal class LoginViewModel( val currentDialogState = _base.value.dialogState if (currentDialogState !is DialogState.OverrideTotp) return - totpSecretInformation?.let { - val selectedFields = - currentDialogState.fields.filter { field -> field.selected } - - selectedFields.applyToUi { after } - } + currentDialogState.fields + .filter { field -> field.selected } + .applyToUi { after } } is LoginUiEvent.OnOverrideTotpFieldsKept -> { val currentDialogState = _base.value.dialogState if (currentDialogState !is DialogState.OverrideTotp) return - totpSecretInformation?.let { - currentDialogState.fields.applyToUi { before } - } + currentDialogState.fields.applyToUi { before } } is LoginUiEvent.OnTotpParseErrorDismiss -> { @@ -467,14 +455,14 @@ internal class LoginViewModel( private fun requestTotpSecretUpdate( secretInformation: TotpInfo, - originalUri: String? = null, + originalUri: String, ) { val currentState = _base.value val currentTotpSecret = currentState.totpTextFieldState.text.toString() val currentIssuers = currentState.domains val currentAccountName = currentState.usernameTextFieldState.text.toString() - val newTotpSecret = originalUri ?: secretInformation.secret + val newTotpSecret = originalUri val newDomain = resolveTotpDomain(secretInformation.issuer, secretInformation.accountName) val newAccountName = secretInformation.accountName @@ -526,9 +514,9 @@ internal class LoginViewModel( private fun updateUiWithTotpSecretInfo( secretInformation: TotpInfo, - originalUri: String? = null, + originalUri: String, ) = updateUiWithSpecificTotpSecretInfo( - secret = originalUri ?: secretInformation.secret, + secret = originalUri, issuer = resolveTotpDomain(secretInformation.issuer, secretInformation.accountName), accountName = secretInformation.accountName, ) diff --git a/feature/item/create/src/test/java/de/davis/keygo/feature/item/create/presentation/login/LoginViewModelTest.kt b/feature/item/create/src/test/java/de/davis/keygo/feature/item/create/presentation/login/LoginViewModelTest.kt index 2e2d96106..4bfc7ecd5 100644 --- a/feature/item/create/src/test/java/de/davis/keygo/feature/item/create/presentation/login/LoginViewModelTest.kt +++ b/feature/item/create/src/test/java/de/davis/keygo/feature/item/create/presentation/login/LoginViewModelTest.kt @@ -155,6 +155,47 @@ class LoginViewModelTest { @Test fun `a code that collides with the chosen item raises the override dialog`() = runVmTest { + val viewModel = viewModelOnOverrideDialog() + + val dialog = viewModel.readyBase().dialogState + assertIs(dialog) + val usernameField = dialog.fields.single { it.fieldType == FieldType.Username } + assertEquals("old@github.com", usernameField.before) + assertEquals("me@github.com", usernameField.after) + } + + @Test + fun `confirming the override writes the code's fields into the form`() = runVmTest { + val viewModel = viewModelOnOverrideDialog() + + viewModel.onEvent(LoginUiEvent.OnOverrideTotpFieldsConfirmed) + advanceUntilIdle() + + val base = viewModel.readyBase() + assertEquals("me@github.com", base.usernameTextFieldState.text.toString()) + assertEquals(DialogState.None, base.dialogState) + } + + @Test + fun `keeping the current fields leaves the form as the item had it`() = runVmTest { + val viewModel = viewModelOnOverrideDialog() + + viewModel.onEvent(LoginUiEvent.OnOverrideTotpFieldsKept) + advanceUntilIdle() + + val base = viewModel.readyBase() + assertEquals("old@github.com", base.usernameTextFieldState.text.toString()) + assertEquals(DialogState.None, base.dialogState) + } + + // Helpers + + /** + * A form on an existing login whose username the scanned code disagrees with, left sitting on + * the override dialog that disagreement raises. Every field arrives selected, so a confirm + * straight after this takes the code's side of all of them. + */ + private fun TestScope.viewModelOnOverrideDialog(): LoginViewModel { val existing = seedLogin( name = "GitHub", domain = "github.com", @@ -173,15 +214,9 @@ class LoginViewModelTest { ) advanceUntilIdle() - val dialog = viewModel.readyBase().dialogState - assertIs(dialog) - val usernameField = dialog.fields.single { it.fieldType == FieldType.Username } - assertEquals("old@github.com", usernameField.before) - assertEquals("me@github.com", usernameField.after) + return viewModel } - // Helpers - private fun runVmTest(body: suspend TestScope.() -> Unit) = runTest(mainDispatcher.scheduler) { body() } From 5e3cfbd06762ebee1ff16c9852ea2d3ed59647c7 Mon Sep 17 00:00:00 2001 From: Davis Wolfermann Date: Mon, 24 Aug 2026 12:55:51 +0200 Subject: [PATCH 16/20] doc: remove comments --- .../keygo/app/presentation/MainActivity.kt | 27 ++++++++++--------- .../presentation/totp/AssignTotpGraph.kt | 18 ------------- .../presentation/SelectItemForTotpGraph.kt | 15 ----------- .../presentation/SelectItemForTotpScreen.kt | 16 ----------- .../presentation/SelectItemForTotpUiState.kt | 7 ----- .../SelectItemForTotpViewModel.kt | 13 --------- .../totp/presentation/TotpImportRedirect.kt | 18 ------------- .../presentation/TotpImportRedirectState.kt | 7 ----- .../TotpImportRedirectViewModel.kt | 16 ----------- .../component/TotpParseErrorDialog.kt | 6 ----- .../SelectItemForTotpViewModelTest.kt | 5 ---- .../TotpImportRedirectViewModelTest.kt | 4 --- 12 files changed, 14 insertions(+), 138 deletions(-) diff --git a/app/src/main/kotlin/de/davis/keygo/app/presentation/MainActivity.kt b/app/src/main/kotlin/de/davis/keygo/app/presentation/MainActivity.kt index 488d3c6a6..f6539f946 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 @@ -98,20 +98,13 @@ class MainActivity : FragmentActivity() { } } -/** - * Where a pending code sends the user once they are through the door, whether that door was auth - * or onboarding. No code means there is nothing to pick an item for. - */ private fun destinationAfterUnlock(totpUri: String?): Any = totpUri?.let { SelectItemForTotpRoute(it) } ?: RouteDestination.TopLevelAppGraph -/** - * Where a validated code goes once the gate has cleared it. Auth and onboarding are the two ways - * into the app, and the code rides along to whichever one the user needs. The redirect is popped on - * the way out: it exists only to hold the code while it was checked, so there is nothing to come - * back to. - */ -internal fun NavController.navigateToValidatedImport(hasAccess: Boolean, pending: PendingTotpImport) { +internal fun NavController.navigateToValidatedImport( + hasAccess: Boolean, + pending: PendingTotpImport +) { navigate( if (hasAccess) AuthRoute( totpInfo = pending.totpInfo, @@ -198,11 +191,19 @@ private fun App(hasAccess: Boolean) { startDestination = if (hasAccess) AuthRoute() else OnboardingRoute(), ) { totpImportRedirectGraph( - onValidated = { pending -> navController.navigateToValidatedImport(hasAccess, pending) }, + onValidated = { pending -> + navController.navigateToValidatedImport( + hasAccess, + pending + ) + }, // The app was launched only to import this code. With nothing left to import, the // Activity is what closes, and :app is the only module that owns one. onRejected = { - activity?.finish() ?: Log.w(TAG, "No activity to finish after rejecting an invalid TOTP deep link") + activity?.finish() ?: Log.w( + TAG, + "No activity to finish after rejecting an invalid TOTP deep link" + ) }, ) diff --git a/feature/item/create/src/main/kotlin/de/davis/keygo/feature/item/create/presentation/totp/AssignTotpGraph.kt b/feature/item/create/src/main/kotlin/de/davis/keygo/feature/item/create/presentation/totp/AssignTotpGraph.kt index cb271da6b..a58270bc9 100644 --- a/feature/item/create/src/main/kotlin/de/davis/keygo/feature/item/create/presentation/totp/AssignTotpGraph.kt +++ b/feature/item/create/src/main/kotlin/de/davis/keygo/feature/item/create/presentation/totp/AssignTotpGraph.kt @@ -11,13 +11,6 @@ import de.davis.keygo.feature.item.create.presentation.login.LoginScreen import kotlinx.serialization.Serializable import java.util.UUID -/** - * The login form for a chosen item, or for a new one when [itemId] is null. - * - * The id travels as a String because [ItemId] is a [UUID] and type-safe navigation has no - * [androidx.navigation.NavType] for it. Supplying one through a typeMap for a single nullable id - * costs more than the conversion does. - */ @Serializable data class AssignTotpRoute( val totpUri: String, @@ -27,17 +20,6 @@ data class AssignTotpRoute( get() = itemId?.let(UUID::fromString) } -/** - * The last step of a deep-linked import: the login form, opened on the item the picker chose, with - * the scanned code already pending on it. - * - * The picker itself lives in the totp feature, which knows nothing about a login form. It hands the - * uri back to whoever wired the two graphs together, and that caller navigates to - * [AssignTotpRoute]. - * - * @param onImportFinished the import is over and the code has been saved. Leads back into the app - * with the import routes popped, so back does not return the user to a code they already handled. - */ fun NavGraphBuilder.assignTotpGraph( onImportFinished: () -> Unit, navigateUp: () -> Unit, diff --git a/feature/totp/src/main/kotlin/de/davis/keygo/feature/totp/presentation/SelectItemForTotpGraph.kt b/feature/totp/src/main/kotlin/de/davis/keygo/feature/totp/presentation/SelectItemForTotpGraph.kt index 3e31d30b2..51babbd14 100644 --- a/feature/totp/src/main/kotlin/de/davis/keygo/feature/totp/presentation/SelectItemForTotpGraph.kt +++ b/feature/totp/src/main/kotlin/de/davis/keygo/feature/totp/presentation/SelectItemForTotpGraph.kt @@ -7,24 +7,9 @@ import de.davis.keygo.core.item.domain.alias.ItemId import de.davis.keygo.core.ui.RouteDestination import kotlinx.serialization.Serializable -/** - * Where a deep-linked code lands once the user is through the door. The uri travels whole here, - * unlike on the route that carried it through the gate, because only the deep link's own - * `otpauth://totp/{totpInfo}?{queries}` pattern forces that split. - */ @Serializable data class SelectItemForTotpRoute(val totpUri: String) : RouteDestination -/** - * Asks which login a validated code belongs to, and lets the caller decide where the answer leads. - * - * The destination is the caller's to pick: the code ends up on a login form, and this module knows - * nothing about one. Both callbacks hand the uri back out, because the form needs it and only this - * graph's route is holding it. - * - * @param onItemSelected the user picked an existing login to attach the code to. - * @param onCreateNew the user chose to attach the code to a login that does not exist yet. - */ fun NavGraphBuilder.selectItemForTotpGraph( onItemSelected: (totpUri: String, itemId: ItemId) -> Unit, onCreateNew: (totpUri: String) -> Unit, diff --git a/feature/totp/src/main/kotlin/de/davis/keygo/feature/totp/presentation/SelectItemForTotpScreen.kt b/feature/totp/src/main/kotlin/de/davis/keygo/feature/totp/presentation/SelectItemForTotpScreen.kt index 0cd315071..07b48eb28 100644 --- a/feature/totp/src/main/kotlin/de/davis/keygo/feature/totp/presentation/SelectItemForTotpScreen.kt +++ b/feature/totp/src/main/kotlin/de/davis/keygo/feature/totp/presentation/SelectItemForTotpScreen.kt @@ -24,22 +24,6 @@ import de.davis.keygo.feature.totp.R import org.koin.androidx.compose.koinViewModel import org.koin.core.parameter.parametersOf -/** - * Asks which item a scanned code belongs to, listing every login with the ones on the code's own - * domain grouped first. Only logins are listed, because a TOTP secret can only be attached to one. - * - * The screen carries no back affordance and no back handler. It is the first step of a deep-linked - * import, which replaced the whole back stack on its way here, so the NavHost does not consume a - * back press and the activity finishes on its own. - * - * It also carries no error surface. A code that cannot be read never reaches this screen, because - * the redirect that starts the import ends the flow there. - * - * @param totpUri the scanned deep link, carried whole so the screen and its ViewModel can parse it - * independently. - * @param onItemSelected the user picked an existing login to attach the code to. - * @param onCreateNew the user chose to attach the code to a login that does not exist yet. - */ @Composable fun SelectItemForTotpScreen( totpUri: String, diff --git a/feature/totp/src/main/kotlin/de/davis/keygo/feature/totp/presentation/SelectItemForTotpUiState.kt b/feature/totp/src/main/kotlin/de/davis/keygo/feature/totp/presentation/SelectItemForTotpUiState.kt index ea4610e86..660c22f06 100644 --- a/feature/totp/src/main/kotlin/de/davis/keygo/feature/totp/presentation/SelectItemForTotpUiState.kt +++ b/feature/totp/src/main/kotlin/de/davis/keygo/feature/totp/presentation/SelectItemForTotpUiState.kt @@ -2,13 +2,6 @@ package de.davis.keygo.feature.totp.presentation import de.davis.keygo.core.item.domain.alias.ItemId -/** - * What the picker knows about the scanned code. - * - * @param suggestedItemIds the logins on the code's own registrable domain, shown first. An empty set - * is the ordinary case for a code whose domain matches nothing, and is also what an unreadable code - * produces, since the redirect that starts the import already turned those away. - */ internal data class SelectItemForTotpUiState( val suggestedItemIds: Set = emptySet(), ) diff --git a/feature/totp/src/main/kotlin/de/davis/keygo/feature/totp/presentation/SelectItemForTotpViewModel.kt b/feature/totp/src/main/kotlin/de/davis/keygo/feature/totp/presentation/SelectItemForTotpViewModel.kt index a3628c5aa..5d0f5905a 100644 --- a/feature/totp/src/main/kotlin/de/davis/keygo/feature/totp/presentation/SelectItemForTotpViewModel.kt +++ b/feature/totp/src/main/kotlin/de/davis/keygo/feature/totp/presentation/SelectItemForTotpViewModel.kt @@ -19,13 +19,6 @@ import kotlinx.coroutines.launch import org.koin.core.annotation.InjectedParam import org.koin.core.annotation.KoinViewModel -/** - * Backs the screen that asks which login a scanned code belongs to. - * - * The code arrives as a uri rather than as parsed info because it travels through a navigation - * argument, and navigation carries primitives. A code that cannot be read gets no suggestions and - * no error, because the redirect that starts the import already refused those. - */ @KoinViewModel internal class SelectItemForTotpViewModel( @InjectedParam private val totpUri: String, @@ -47,12 +40,6 @@ internal class SelectItemForTotpViewModel( } } - /** - * The logins whose registrable domain matches the code's own. - * - * [resolveTotpDomain] is what fills the domain field for a new item, so the suggestions agree - * with it: a code that carries no issuer still matches on the domain in `user@example.com`. - */ private suspend fun suggestedItemIdsFor(info: TotpInfo): Set { val domain = resolveTotpDomain( issuer = info.issuer, diff --git a/feature/totp/src/main/kotlin/de/davis/keygo/feature/totp/presentation/TotpImportRedirect.kt b/feature/totp/src/main/kotlin/de/davis/keygo/feature/totp/presentation/TotpImportRedirect.kt index d1a5ef336..655d66782 100644 --- a/feature/totp/src/main/kotlin/de/davis/keygo/feature/totp/presentation/TotpImportRedirect.kt +++ b/feature/totp/src/main/kotlin/de/davis/keygo/feature/totp/presentation/TotpImportRedirect.kt @@ -16,12 +16,6 @@ import kotlinx.serialization.Serializable import org.koin.androidx.compose.koinViewModel import org.koin.core.parameter.parametersOf -/** - * Where a deep-linked code lands first, before anything else in the app sees it. - * - * The code travels as the two halves the deep link splits it into rather than as an assembled uri, - * because `otpauth://totp/{totpInfo}?{queries}` is the shape the navigation pattern matches. - */ @Serializable data class TotpImportRedirect( val totpInfo: String? = null, @@ -31,18 +25,6 @@ data class TotpImportRedirect( get() = PendingTotpImport(totpInfo, queries) } -/** - * Validates the code a deep link carried and lets the caller decide where a good one leads. - * - * The destination is the caller's to pick: auth and onboarding are the two ways into the app, and - * this module knows about neither. - * - * @param onValidated the code parses, so the import is worth an authentication. It carries the code - * onward so the caller can hand it to whichever screen it sends the user to. - * @param onRejected the code is unusable and the user has acknowledged it. The app was launched only - * to import that code, so nothing is left to do. Closing belongs to whoever owns the Activity, which - * is not this module. - */ fun NavGraphBuilder.totpImportRedirectGraph( onValidated: (PendingTotpImport) -> Unit, onRejected: () -> Unit, diff --git a/feature/totp/src/main/kotlin/de/davis/keygo/feature/totp/presentation/TotpImportRedirectState.kt b/feature/totp/src/main/kotlin/de/davis/keygo/feature/totp/presentation/TotpImportRedirectState.kt index b4fb14ca0..5570aa4f1 100644 --- a/feature/totp/src/main/kotlin/de/davis/keygo/feature/totp/presentation/TotpImportRedirectState.kt +++ b/feature/totp/src/main/kotlin/de/davis/keygo/feature/totp/presentation/TotpImportRedirectState.kt @@ -1,12 +1,5 @@ package de.davis.keygo.feature.totp.presentation -/** - * What the redirect knows about the code its deep link carried. - * - * The parse is a single synchronous call, so [Validating] is what the screen holds for at most one - * frame. It exists so the screen never has to read a missing verdict as either "not yet" or - * "unreadable". - */ internal sealed interface TotpImportRedirectState { data object Validating : TotpImportRedirectState diff --git a/feature/totp/src/main/kotlin/de/davis/keygo/feature/totp/presentation/TotpImportRedirectViewModel.kt b/feature/totp/src/main/kotlin/de/davis/keygo/feature/totp/presentation/TotpImportRedirectViewModel.kt index 6e21460cf..2d0c74b0d 100644 --- a/feature/totp/src/main/kotlin/de/davis/keygo/feature/totp/presentation/TotpImportRedirectViewModel.kt +++ b/feature/totp/src/main/kotlin/de/davis/keygo/feature/totp/presentation/TotpImportRedirectViewModel.kt @@ -12,18 +12,6 @@ import kotlinx.coroutines.flow.asStateFlow import org.koin.core.annotation.InjectedParam import org.koin.core.annotation.KoinViewModel -/** - * Decides whether a deep-linked code is worth asking the user to authenticate for. - * - * The import used to reach the item picker before anything read the code, so a malformed one cost - * the user a full unlock before it told them anything. This is the single gate that check now - * passes through, which is why neither the picker nor the login form reports a parse failure of its - * own any more. - * - * The pending import arrives whole rather than as its assembled uri because that uri is null for a - * structurally incomplete link, and Koin resolves an injected parameter by type, which a null value - * does not have. - */ @KoinViewModel internal class TotpImportRedirectViewModel( @InjectedParam private val pendingImport: PendingTotpImport, @@ -38,10 +26,6 @@ internal class TotpImportRedirectViewModel( _state.value = validate() } - /** - * A null uri means the link carried no path or no query string, which leaves as little to - * import as a code the parser rejects. Both end the import here. - */ private fun validate(): TotpImportRedirectState { val uri = pendingImport.uri ?: run { Log.e(TAG, "Deep link carried no complete otpauth uri") diff --git a/feature/totp/src/main/kotlin/de/davis/keygo/feature/totp/presentation/component/TotpParseErrorDialog.kt b/feature/totp/src/main/kotlin/de/davis/keygo/feature/totp/presentation/component/TotpParseErrorDialog.kt index 8ba0d065f..458579086 100644 --- a/feature/totp/src/main/kotlin/de/davis/keygo/feature/totp/presentation/component/TotpParseErrorDialog.kt +++ b/feature/totp/src/main/kotlin/de/davis/keygo/feature/totp/presentation/component/TotpParseErrorDialog.kt @@ -8,12 +8,6 @@ import androidx.compose.ui.Modifier import androidx.compose.ui.res.stringResource import de.davis.keygo.feature.totp.R -/** - * @param onDismissRequest what a back press (or outside tap) does. Defaults to doing nothing, - * because the scanner's dialog sits inside a screen the user can still use, so a stray back press - * should not dismiss it. The deep link gate's dialog is the whole screen, so back has to be a real - * exit and passes [onDismiss] here too. - */ @Composable fun TotpParseErrorDialog( onDismiss: () -> Unit, diff --git a/feature/totp/src/test/kotlin/de/davis/keygo/feature/totp/presentation/SelectItemForTotpViewModelTest.kt b/feature/totp/src/test/kotlin/de/davis/keygo/feature/totp/presentation/SelectItemForTotpViewModelTest.kt index fd7928f45..e8facfd5d 100644 --- a/feature/totp/src/test/kotlin/de/davis/keygo/feature/totp/presentation/SelectItemForTotpViewModelTest.kt +++ b/feature/totp/src/test/kotlin/de/davis/keygo/feature/totp/presentation/SelectItemForTotpViewModelTest.kt @@ -29,11 +29,6 @@ import kotlin.test.BeforeTest import kotlin.test.Test import kotlin.test.assertEquals -/** - * Covers what the picker knows before the user has chosen anything: which logins the scanned code - * points at. An unreadable code is not covered here, because the redirect that starts the import - * rejects those before the picker is ever reached. - */ @OptIn(ExperimentalCoroutinesApi::class) @RunWith(RobolectricTestRunner::class) @Config(sdk = [34]) diff --git a/feature/totp/src/test/kotlin/de/davis/keygo/feature/totp/presentation/TotpImportRedirectViewModelTest.kt b/feature/totp/src/test/kotlin/de/davis/keygo/feature/totp/presentation/TotpImportRedirectViewModelTest.kt index 6d024ee93..0eedbef65 100644 --- a/feature/totp/src/test/kotlin/de/davis/keygo/feature/totp/presentation/TotpImportRedirectViewModelTest.kt +++ b/feature/totp/src/test/kotlin/de/davis/keygo/feature/totp/presentation/TotpImportRedirectViewModelTest.kt @@ -10,10 +10,6 @@ import org.robolectric.annotation.Config import kotlin.test.Test import kotlin.test.assertEquals -/** - * Covers the gate a deep-linked code passes before the user is asked to authenticate. A code that - * cannot be read has to be rejected here, because everything downstream now assumes it was. - */ @RunWith(RobolectricTestRunner::class) @Config(sdk = [34]) class TotpImportRedirectViewModelTest { From a9f7defe8c9506dd763d68f03fcabf5163fc6ab3 Mon Sep 17 00:00:00 2001 From: Davis Wolfermann Date: Mon, 24 Aug 2026 13:01:35 +0200 Subject: [PATCH 17/20] refactor(test): fold the three RegistrableDomainResolver fakes into one Three modules each grew their own fake, one per testing need: core:util computed an eTLD+1 from the last two labels, autofill looked answers up in a seeded map and recorded what it was asked, and legacy-migration answered example.com to anything containing "example" and null to everything else. The core:util one absorbs all three. A seeded resolutions entry wins, an explicit null included, and anything unseeded falls back to the label heuristic, so a test names only the domains it cares about. resolvedDomains keeps autofill's call log for the tests that assert nothing was resolved. Nothing relied on the semantics that went away. Autofill seeds every domain it resolves, and the two tests that want a null seed one. Every origin the migration tests carry is https://example.com, which the heuristic resolves to the same example.com the fixed answer gave, and no migration test asserts the null branch. The fake now lives in the module that owns the interface, so a fourth need does not grow a fourth fake. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_019NwwBkR3ks2QMJSRmP1iDH --- .../util/FakeRegistrableDomainResolver.kt | 22 ++++++++++++++++++- ...AddRegistrableDomainsToLoginUseCaseTest.kt | 2 +- ...DoesItemHaveDomainReferencesUseCaseTest.kt | 2 +- .../activity/AutofillViewModelTest.kt | 2 +- .../dataset/SuggestionFinderTest.kt | 2 +- .../autofill/FakeRegistrableDomainResolver.kt | 22 ------------------- .../LegacyMigrationEndToEndTest.kt | 2 +- .../LegacyMigrationRealDatabaseTest.kt | 2 +- .../domain/mapper/LegacyItemConverterTest.kt | 2 +- .../usecase/MigrateLegacyDataUseCaseTest.kt | 2 +- .../data/FakeRegistrableDomainResolver.kt | 17 -------------- 11 files changed, 29 insertions(+), 48 deletions(-) delete mode 100644 feature/autofill/src/testFixtures/kotlin/de/davis/keygo/core/feature/autofill/FakeRegistrableDomainResolver.kt delete mode 100644 legacy-migration/src/testFixtures/kotlin/de/davis/keygo/legacy_migration/data/FakeRegistrableDomainResolver.kt diff --git a/core/util/src/testFixtures/kotlin/de/davis/keygo/core/util/FakeRegistrableDomainResolver.kt b/core/util/src/testFixtures/kotlin/de/davis/keygo/core/util/FakeRegistrableDomainResolver.kt index 0b00de1e7..f5983f8fd 100644 --- a/core/util/src/testFixtures/kotlin/de/davis/keygo/core/util/FakeRegistrableDomainResolver.kt +++ b/core/util/src/testFixtures/kotlin/de/davis/keygo/core/util/FakeRegistrableDomainResolver.kt @@ -2,9 +2,29 @@ package de.davis.keygo.core.util import de.davis.keygo.core.util.domain.resolver.RegistrableDomainResolver -/** Resolves an eTLD+1 by keeping the last two labels, which is enough for the test domains. */ +/** + * Answers as a resolver that can see only the last two labels of a host, which is enough for the + * domains tests use. + * + * Seed [resolutions] to pin an exact answer for one domain, a null one included. A host the real + * resolver cannot place is a case callers have to carry through, and an explicit null is the only + * way to stage it for a host the label heuristic would otherwise resolve. Anything unseeded falls + * back to the heuristic, so a test names only the domains it cares about. + * + * [resolvedDomains] records every domain asked about, in order, for tests where resolving nothing + * at all is the behaviour under test. + */ class FakeRegistrableDomainResolver : RegistrableDomainResolver { + + var resolutions: Map = emptyMap() + + val resolvedDomains: MutableList = mutableListOf() + override fun resolve(domain: String): String? { + resolvedDomains += domain + + if (resolutions.containsKey(domain)) return resolutions[domain] + val labels = domain.substringAfter("://") .substringBefore('/') .split('.') diff --git a/feature/autofill/src/test/kotlin/de/davis/keygo/feature/autofill/domain/usecase/AddRegistrableDomainsToLoginUseCaseTest.kt b/feature/autofill/src/test/kotlin/de/davis/keygo/feature/autofill/domain/usecase/AddRegistrableDomainsToLoginUseCaseTest.kt index f211964ce..fee11e09f 100644 --- a/feature/autofill/src/test/kotlin/de/davis/keygo/feature/autofill/domain/usecase/AddRegistrableDomainsToLoginUseCaseTest.kt +++ b/feature/autofill/src/test/kotlin/de/davis/keygo/feature/autofill/domain/usecase/AddRegistrableDomainsToLoginUseCaseTest.kt @@ -1,6 +1,5 @@ package de.davis.keygo.feature.autofill.domain.usecase -import de.davis.keygo.core.feature.autofill.FakeRegistrableDomainResolver import de.davis.keygo.core.item.FakeLoginRepository import de.davis.keygo.core.item.domain.alias.ItemId import de.davis.keygo.core.item.domain.alias.newItemId @@ -9,6 +8,7 @@ import de.davis.keygo.core.item.domain.model.DomainInfo import de.davis.keygo.core.item.domain.model.KeyInformation import de.davis.keygo.core.item.domain.model.Login import de.davis.keygo.core.item.domain.model.Timestamp +import de.davis.keygo.core.util.FakeRegistrableDomainResolver import kotlinx.coroutines.test.runTest import kotlin.test.BeforeTest import kotlin.test.Test diff --git a/feature/autofill/src/test/kotlin/de/davis/keygo/feature/autofill/domain/usecase/DoesItemHaveDomainReferencesUseCaseTest.kt b/feature/autofill/src/test/kotlin/de/davis/keygo/feature/autofill/domain/usecase/DoesItemHaveDomainReferencesUseCaseTest.kt index 3f58d2f27..b5bebd81a 100644 --- a/feature/autofill/src/test/kotlin/de/davis/keygo/feature/autofill/domain/usecase/DoesItemHaveDomainReferencesUseCaseTest.kt +++ b/feature/autofill/src/test/kotlin/de/davis/keygo/feature/autofill/domain/usecase/DoesItemHaveDomainReferencesUseCaseTest.kt @@ -1,6 +1,5 @@ package de.davis.keygo.feature.autofill.domain.usecase -import de.davis.keygo.core.feature.autofill.FakeRegistrableDomainResolver import de.davis.keygo.core.item.FakeLoginRepository import de.davis.keygo.core.item.domain.alias.ItemId import de.davis.keygo.core.item.domain.alias.newItemId @@ -9,6 +8,7 @@ import de.davis.keygo.core.item.domain.model.DomainInfo import de.davis.keygo.core.item.domain.model.KeyInformation import de.davis.keygo.core.item.domain.model.Login import de.davis.keygo.core.item.domain.model.Timestamp +import de.davis.keygo.core.util.FakeRegistrableDomainResolver import kotlinx.coroutines.test.runTest import kotlin.test.BeforeTest import kotlin.test.Test diff --git a/feature/autofill/src/test/kotlin/de/davis/keygo/feature/autofill/presentation/activity/AutofillViewModelTest.kt b/feature/autofill/src/test/kotlin/de/davis/keygo/feature/autofill/presentation/activity/AutofillViewModelTest.kt index fb8038195..a99c042be 100644 --- a/feature/autofill/src/test/kotlin/de/davis/keygo/feature/autofill/presentation/activity/AutofillViewModelTest.kt +++ b/feature/autofill/src/test/kotlin/de/davis/keygo/feature/autofill/presentation/activity/AutofillViewModelTest.kt @@ -3,7 +3,6 @@ package de.davis.keygo.feature.autofill.presentation.activity import androidx.lifecycle.SavedStateHandle import de.davis.keygo.core.feature.autofill.FakeAutofillDatasetProvider import de.davis.keygo.core.feature.autofill.FakeDigitalAssetLinkRepository -import de.davis.keygo.core.feature.autofill.FakeRegistrableDomainResolver import de.davis.keygo.core.feature.autofill.FakeSignatureInfoProvider import de.davis.keygo.core.feature.autofill.FakeTotpGenerator import de.davis.keygo.core.feature.autofill.FakeTotpRepository @@ -21,6 +20,7 @@ import de.davis.keygo.core.item.domain.model.Login import de.davis.keygo.core.item.domain.model.Timestamp import de.davis.keygo.core.item.domain.model.Totp import de.davis.keygo.core.security.crypto.FakeCryptographicScopeProvider +import de.davis.keygo.core.util.FakeRegistrableDomainResolver import de.davis.keygo.core.util.Result import de.davis.keygo.feature.autofill.domain.usecase.AddRegistrableDomainsToLoginUseCase import de.davis.keygo.feature.autofill.domain.usecase.DoesItemHaveDomainReferencesUseCase diff --git a/feature/autofill/src/test/kotlin/de/davis/keygo/feature/autofill/presentation/dataset/SuggestionFinderTest.kt b/feature/autofill/src/test/kotlin/de/davis/keygo/feature/autofill/presentation/dataset/SuggestionFinderTest.kt index d0247ccd8..ab377c7fc 100644 --- a/feature/autofill/src/test/kotlin/de/davis/keygo/feature/autofill/presentation/dataset/SuggestionFinderTest.kt +++ b/feature/autofill/src/test/kotlin/de/davis/keygo/feature/autofill/presentation/dataset/SuggestionFinderTest.kt @@ -1,6 +1,5 @@ package de.davis.keygo.feature.autofill.presentation.dataset -import de.davis.keygo.core.feature.autofill.FakeRegistrableDomainResolver import de.davis.keygo.core.feature.autofill.autofillId import de.davis.keygo.core.item.FakeLoginRepository import de.davis.keygo.core.item.domain.alias.newItemId @@ -15,6 +14,7 @@ import de.davis.keygo.core.item.domain.model.PasswordSecret import de.davis.keygo.core.item.domain.model.Timestamp import de.davis.keygo.core.item.domain.model.Totp import de.davis.keygo.core.security.domain.usecase.GetTdlMatchedLoginsUseCase +import de.davis.keygo.core.util.FakeRegistrableDomainResolver import de.davis.keygo.feature.autofill.presentation.model.FieldType import de.davis.keygo.feature.autofill.presentation.model.Form import de.davis.keygo.feature.autofill.presentation.model.FormField diff --git a/feature/autofill/src/testFixtures/kotlin/de/davis/keygo/core/feature/autofill/FakeRegistrableDomainResolver.kt b/feature/autofill/src/testFixtures/kotlin/de/davis/keygo/core/feature/autofill/FakeRegistrableDomainResolver.kt deleted file mode 100644 index 2ce212a66..000000000 --- a/feature/autofill/src/testFixtures/kotlin/de/davis/keygo/core/feature/autofill/FakeRegistrableDomainResolver.kt +++ /dev/null @@ -1,22 +0,0 @@ -package de.davis.keygo.core.feature.autofill - -import de.davis.keygo.core.util.domain.resolver.RegistrableDomainResolver - -/** - * In-memory [RegistrableDomainResolver] for tests. - * - * Set [resolutions] to configure which domains resolve to which eTLD1 value. - * Inspect [resolvedDomains] to verify which domains were resolved. - */ -class FakeRegistrableDomainResolver : RegistrableDomainResolver { - // Configurable: map from domain to resolved eTLD1 (or null) - var resolutions: Map = emptyMap() - - // Track calls for assertion - val resolvedDomains: MutableList = mutableListOf() - - override fun resolve(domain: String): String? { - resolvedDomains += domain - return resolutions[domain] - } -} diff --git a/legacy-migration/src/test/kotlin/de/davis/keygo/legacy_migration/LegacyMigrationEndToEndTest.kt b/legacy-migration/src/test/kotlin/de/davis/keygo/legacy_migration/LegacyMigrationEndToEndTest.kt index 733928797..0eba3384b 100644 --- a/legacy-migration/src/test/kotlin/de/davis/keygo/legacy_migration/LegacyMigrationEndToEndTest.kt +++ b/legacy-migration/src/test/kotlin/de/davis/keygo/legacy_migration/LegacyMigrationEndToEndTest.kt @@ -20,11 +20,11 @@ import de.davis.keygo.core.security.crypto.FakeCryptographicScopeProvider import de.davis.keygo.core.security.domain.crypto.decrypt 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.util.FakeRegistrableDomainResolver import de.davis.keygo.core.util.Result import de.davis.keygo.core.util.assertSuccess import de.davis.keygo.legacy_migration.data.FakeLegacyDatabaseProvider import de.davis.keygo.legacy_migration.data.FakeLegacyKeyRepository -import de.davis.keygo.legacy_migration.data.FakeRegistrableDomainResolver import de.davis.keygo.legacy_migration.data.crypto.LegacyAesGcmCipher import de.davis.keygo.legacy_migration.data.encryptLikeV1 import de.davis.keygo.legacy_migration.data.json.LegacyDetailParser diff --git a/legacy-migration/src/test/kotlin/de/davis/keygo/legacy_migration/LegacyMigrationRealDatabaseTest.kt b/legacy-migration/src/test/kotlin/de/davis/keygo/legacy_migration/LegacyMigrationRealDatabaseTest.kt index 059fb7c73..d2721327a 100644 --- a/legacy-migration/src/test/kotlin/de/davis/keygo/legacy_migration/LegacyMigrationRealDatabaseTest.kt +++ b/legacy-migration/src/test/kotlin/de/davis/keygo/legacy_migration/LegacyMigrationRealDatabaseTest.kt @@ -17,10 +17,10 @@ import de.davis.keygo.core.security.crypto.FakeCryptographicScopeProvider import de.davis.keygo.core.security.domain.crypto.decrypt 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.util.FakeRegistrableDomainResolver import de.davis.keygo.core.util.assertSuccess import de.davis.keygo.legacy_migration.data.FakeLegacyDatabaseProvider import de.davis.keygo.legacy_migration.data.FakeLegacyKeyRepository -import de.davis.keygo.legacy_migration.data.FakeRegistrableDomainResolver import de.davis.keygo.legacy_migration.data.crypto.LegacyAesGcmCipher import de.davis.keygo.legacy_migration.data.json.LegacyDetailParser import de.davis.keygo.legacy_migration.data.local.datasource.LegacyDatabase diff --git a/legacy-migration/src/test/kotlin/de/davis/keygo/legacy_migration/domain/mapper/LegacyItemConverterTest.kt b/legacy-migration/src/test/kotlin/de/davis/keygo/legacy_migration/domain/mapper/LegacyItemConverterTest.kt index 54ea8c916..b53da349c 100644 --- a/legacy-migration/src/test/kotlin/de/davis/keygo/legacy_migration/domain/mapper/LegacyItemConverterTest.kt +++ b/legacy-migration/src/test/kotlin/de/davis/keygo/legacy_migration/domain/mapper/LegacyItemConverterTest.kt @@ -11,10 +11,10 @@ import de.davis.keygo.core.security.crypto.FakeCryptographicScopeProvider import de.davis.keygo.core.security.domain.crypto.decrypt 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.util.FakeRegistrableDomainResolver import de.davis.keygo.core.util.assertSuccess import de.davis.keygo.legacy_migration.data.FAKE_LEGACY_KEY import de.davis.keygo.legacy_migration.data.FakeLegacyCipher -import de.davis.keygo.legacy_migration.data.FakeRegistrableDomainResolver import de.davis.keygo.legacy_migration.domain.crypto.LegacyCipher import de.davis.keygo.legacy_migration.domain.model.LegacyDetail import de.davis.keygo.legacy_migration.domain.model.LegacyItem diff --git a/legacy-migration/src/test/kotlin/de/davis/keygo/legacy_migration/domain/usecase/MigrateLegacyDataUseCaseTest.kt b/legacy-migration/src/test/kotlin/de/davis/keygo/legacy_migration/domain/usecase/MigrateLegacyDataUseCaseTest.kt index 6dbc73b27..dc3b265cc 100644 --- a/legacy-migration/src/test/kotlin/de/davis/keygo/legacy_migration/domain/usecase/MigrateLegacyDataUseCaseTest.kt +++ b/legacy-migration/src/test/kotlin/de/davis/keygo/legacy_migration/domain/usecase/MigrateLegacyDataUseCaseTest.kt @@ -14,12 +14,12 @@ import de.davis.keygo.core.item.domain.usecase.UpsertVaultItemUseCase import de.davis.keygo.core.security.crypto.FakeCryptographicScopeProvider import de.davis.keygo.core.security.domain.crypto.CryptographicScopeProvider import de.davis.keygo.core.security.domain.model.CryptoScopeError +import de.davis.keygo.core.util.FakeRegistrableDomainResolver import de.davis.keygo.core.util.Result import de.davis.keygo.legacy_migration.data.FAKE_LEGACY_KEY import de.davis.keygo.legacy_migration.data.FakeLegacyCipher import de.davis.keygo.legacy_migration.data.FakeLegacyItemRepository import de.davis.keygo.legacy_migration.data.FakeLegacyKeyRepository -import de.davis.keygo.legacy_migration.data.FakeRegistrableDomainResolver import de.davis.keygo.legacy_migration.domain.crypto.LegacyCipher import de.davis.keygo.legacy_migration.domain.mapper.LegacyItemConverter import de.davis.keygo.legacy_migration.domain.model.LegacyDetail diff --git a/legacy-migration/src/testFixtures/kotlin/de/davis/keygo/legacy_migration/data/FakeRegistrableDomainResolver.kt b/legacy-migration/src/testFixtures/kotlin/de/davis/keygo/legacy_migration/data/FakeRegistrableDomainResolver.kt deleted file mode 100644 index 7bf837b87..000000000 --- a/legacy-migration/src/testFixtures/kotlin/de/davis/keygo/legacy_migration/data/FakeRegistrableDomainResolver.kt +++ /dev/null @@ -1,17 +0,0 @@ -package de.davis.keygo.legacy_migration.data - -import de.davis.keygo.core.util.domain.resolver.RegistrableDomainResolver - -/** - * Resolves anything under `example` to `example.com` and everything else to null. - * - * Every origin the migration tests seed is an example.com URL, so a fixed answer keeps eTLD+1 - * resolution from becoming a second thing that can fail in a test about the import. The null branch - * is still real: it is what a host the resolver cannot place answers with, and the converter has to - * carry that through as a domain info without an eTLD+1 rather than dropping the origin. - */ -internal class FakeRegistrableDomainResolver : RegistrableDomainResolver { - - override fun resolve(domain: String): String? = - if (domain.contains("example")) "example.com" else null -} From 0eeb3281e0e776dbf040adf50065c654d8269fc7 Mon Sep 17 00:00:00 2001 From: Davis Wolfermann Date: Mon, 24 Aug 2026 13:35:41 +0200 Subject: [PATCH 18/20] fix: leave app when back pressed during totp scanned flow --- .../keygo/app/presentation/MainActivity.kt | 4 +- .../presentation/TotpImportNavGraphTest.kt | 40 +++++++++++++++++++ 2 files changed, 42 insertions(+), 2 deletions(-) diff --git a/app/src/main/kotlin/de/davis/keygo/app/presentation/MainActivity.kt b/app/src/main/kotlin/de/davis/keygo/app/presentation/MainActivity.kt index f6539f946..017eec32a 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 @@ -29,6 +29,7 @@ import androidx.fragment.app.FragmentActivity import androidx.navigation.NavController import androidx.navigation.NavDestination.Companion.hasRoute import androidx.navigation.NavDestination.Companion.hierarchy +import androidx.navigation.NavGraph.Companion.findStartDestination import androidx.navigation.compose.NavHost import androidx.navigation.compose.composable import androidx.navigation.compose.currentBackStackEntryAsState @@ -58,7 +59,6 @@ import de.davis.keygo.feature.onboarding.presentation.onboardingGraph import de.davis.keygo.feature.settings.presentation.ChangePasswordRoute import de.davis.keygo.feature.settings.presentation.settingsGraph import de.davis.keygo.feature.totp.presentation.SelectItemForTotpRoute -import de.davis.keygo.feature.totp.presentation.TotpImportRedirect import de.davis.keygo.feature.totp.presentation.selectItemForTotpGraph import de.davis.keygo.feature.totp.presentation.totpImportRedirectGraph import de.davis.keygo.item.dialog.SelectItemContent @@ -115,7 +115,7 @@ internal fun NavController.navigateToValidatedImport( queries = pending.queries, ), ) { - popUpTo { inclusive = true } + popUpTo(graph.findStartDestination().id) { inclusive = true } } } diff --git a/app/src/test/kotlin/de/davis/keygo/app/presentation/TotpImportNavGraphTest.kt b/app/src/test/kotlin/de/davis/keygo/app/presentation/TotpImportNavGraphTest.kt index 1e009fef6..ec7188cb1 100644 --- a/app/src/test/kotlin/de/davis/keygo/app/presentation/TotpImportNavGraphTest.kt +++ b/app/src/test/kotlin/de/davis/keygo/app/presentation/TotpImportNavGraphTest.kt @@ -176,6 +176,46 @@ class TotpImportNavGraphTest { ) } + /** + * The same claim as above, but reached the way a deep link reaches it. The gate the deep link + * opens is a second entry on a destination the launch already put on the stack, so a pop that + * only reaches the nearest one leaves the first behind for back to land on. + */ + @Test + fun `back leaves the app after a deep link opened the gate`() { + val controller = navController(hasAccess = true) + controller.navigate(DEEP_LINK_URI.toUri()) + val redirect = assertNotNull(controller.currentBackStackEntry).toRoute() + + controller.navigateToValidatedImport(hasAccess = true, pending = redirect.pendingImport) + controller.navigate(SelectItemForTotpRoute(DEEP_LINK_URI)) { + popUpTo { inclusive = true } + } + + assertTrue(controller.currentDestination?.hasRoute() == true) + assertFalse( + controller.currentBackStack.value.any { it.destination.hasRoute() }, + ) + } + + /** The onboarding half of the same claim, for an account that has no access yet. */ + @Test + fun `back leaves the app after a deep link opened onboarding`() { + val controller = navController(hasAccess = false) + controller.navigate(DEEP_LINK_URI.toUri()) + val redirect = assertNotNull(controller.currentBackStackEntry).toRoute() + + controller.navigateToValidatedImport(hasAccess = false, pending = redirect.pendingImport) + controller.navigate(SelectItemForTotpRoute(DEEP_LINK_URI)) { + popUpTo { inclusive = true } + } + + assertTrue(controller.currentDestination?.hasRoute() == true) + assertFalse( + controller.currentBackStack.value.any { it.destination.hasRoute() }, + ) + } + @Test fun `a validated code sends an account with access to AuthRoute`() { val controller = navController(hasAccess = true) From 5e5b2bae29bc69f93f31f2a7ed26d922b0511ee3 Mon Sep 17 00:00:00 2001 From: Davis Wolfermann Date: Mon, 24 Aug 2026 19:19:59 +0200 Subject: [PATCH 19/20] fix: create item fab --- .../keygo/app/presentation/component/NavigationWrapper.kt | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/app/src/main/kotlin/de/davis/keygo/app/presentation/component/NavigationWrapper.kt b/app/src/main/kotlin/de/davis/keygo/app/presentation/component/NavigationWrapper.kt index deba4ca0f..d525ed786 100644 --- a/app/src/main/kotlin/de/davis/keygo/app/presentation/component/NavigationWrapper.kt +++ b/app/src/main/kotlin/de/davis/keygo/app/presentation/component/NavigationWrapper.kt @@ -206,6 +206,10 @@ fun KeyGoNavigationWrapper( FloatingActionButtonMenu( expanded = fabMenuExpanded, + modifier = Modifier.animateFloatingActionButton( + visible = (showChrome && showPrimaryActionButton) || fabMenuExpanded, + alignment = Alignment.BottomEnd, + ), button = { TooltipBox( positionProvider = @@ -226,10 +230,6 @@ fun KeyGoNavigationWrapper( .semantics { traversalIndex = -1f } - .animateFloatingActionButton( - visible = (showChrome && showPrimaryActionButton) || fabMenuExpanded, - alignment = Alignment.BottomEnd, - ) .focusRequester(focusRequester), ) { val imageVector by remember { From b80ce478d887a5cb0a75faaf37373f1a76e0acf5 Mon Sep 17 00:00:00 2001 From: Davis Wolfermann Date: Tue, 25 Aug 2026 10:05:38 +0200 Subject: [PATCH 20/20] fix: add missing dependencies --- .../item/create/presentation/login/LoginViewModelTest.kt | 2 ++ 1 file changed, 2 insertions(+) diff --git a/feature/item/create/src/test/java/de/davis/keygo/feature/item/create/presentation/login/LoginViewModelTest.kt b/feature/item/create/src/test/java/de/davis/keygo/feature/item/create/presentation/login/LoginViewModelTest.kt index 4bfc7ecd5..b51d9775e 100644 --- a/feature/item/create/src/test/java/de/davis/keygo/feature/item/create/presentation/login/LoginViewModelTest.kt +++ b/feature/item/create/src/test/java/de/davis/keygo/feature/item/create/presentation/login/LoginViewModelTest.kt @@ -24,6 +24,7 @@ import de.davis.keygo.core.util.domain.model.snackbar.SnackbarMessage import de.davis.keygo.core.util.domain.snackbar.SnackbarManager import de.davis.keygo.core.util.domain.usecase.SortUseCase import de.davis.keygo.feature.item.core.domain.usecase.CreateNewOrUpdateLoginUseCase +import de.davis.keygo.feature.item.core.domain.usecase.ValidateTotpInputUseCase import de.davis.keygo.feature.item.core.presentation.login.model.FieldType import de.davis.keygo.feature.item.core.presentation.model.DetailPaneInformation import de.davis.keygo.feature.item.create.presentation.login.model.DialogState @@ -304,6 +305,7 @@ class LoginViewModelTest { itemRepository = itemRepository, observeAllTags = ObserveAllTagsSortedUseCase(itemRepository, SortUseCase()), vaultRepository = vaultRepository, + validateTotpInput = ValidateTotpInputUseCase(totpService) ) private class TestSnackbarManager : SnackbarManager {