From 53a38214c20fe5af969e5a455ddb2c49977e588c Mon Sep 17 00:00:00 2001 From: Davis Wolfermann Date: Sun, 23 Aug 2026 00:48:15 +0200 Subject: [PATCH] feat: validate totp secret for manual input --- .../usecase/ValidateTotpInputUseCase.kt | 17 ++++ .../usecase/ValidateTotpInputUseCaseTest.kt | 78 +++++++++++++++++++ .../create/presentation/login/LoginContent.kt | 1 + .../presentation/login/LoginViewModel.kt | 18 ++++- .../presentation/login/model/LoginUiState.kt | 1 + .../item/view/login/ViewLoginContent.kt | 1 + .../item/view/login/ViewLoginViewModel.kt | 19 ++++- .../kotlin/de/davis/keygo/rust/totp/Totp.kt | 14 ++++ .../de/davis/keygo/rust/FakeTotpService.kt | 8 +- 9 files changed, 150 insertions(+), 7 deletions(-) create mode 100644 feature/item/core/src/main/kotlin/de/davis/keygo/feature/item/core/domain/usecase/ValidateTotpInputUseCase.kt create mode 100644 feature/item/core/src/test/kotlin/de/davis/keygo/feature/item/core/domain/usecase/ValidateTotpInputUseCaseTest.kt diff --git a/feature/item/core/src/main/kotlin/de/davis/keygo/feature/item/core/domain/usecase/ValidateTotpInputUseCase.kt b/feature/item/core/src/main/kotlin/de/davis/keygo/feature/item/core/domain/usecase/ValidateTotpInputUseCase.kt new file mode 100644 index 000000000..8b5e54f99 --- /dev/null +++ b/feature/item/core/src/main/kotlin/de/davis/keygo/feature/item/core/domain/usecase/ValidateTotpInputUseCase.kt @@ -0,0 +1,17 @@ +package de.davis.keygo.feature.item.core.domain.usecase + +import de.davis.keygo.core.util.isSuccess +import de.davis.keygo.rust.totp.TotpService +import de.davis.keygo.rust.totp.getInfoFromUriWithResult +import de.davis.keygo.rust.totp.isValidSecret +import org.koin.core.annotation.Single + +@Single +class ValidateTotpInputUseCase( + private val totpService: TotpService, +) { + + operator fun invoke(uriOrSecret: String): Boolean = + totpService.getInfoFromUriWithResult(uriOrSecret).isSuccess() + || totpService.isValidSecret(uriOrSecret) +} diff --git a/feature/item/core/src/test/kotlin/de/davis/keygo/feature/item/core/domain/usecase/ValidateTotpInputUseCaseTest.kt b/feature/item/core/src/test/kotlin/de/davis/keygo/feature/item/core/domain/usecase/ValidateTotpInputUseCaseTest.kt new file mode 100644 index 000000000..3b3bf4dbb --- /dev/null +++ b/feature/item/core/src/test/kotlin/de/davis/keygo/feature/item/core/domain/usecase/ValidateTotpInputUseCaseTest.kt @@ -0,0 +1,78 @@ +package de.davis.keygo.feature.item.core.domain.usecase + +import de.davis.keygo.rust.FakeTotpService +import de.davisalessandro.keygo.rust.Algorithm +import de.davisalessandro.keygo.rust.TotpInfo +import kotlin.test.Test +import kotlin.test.assertFalse +import kotlin.test.assertTrue + +class ValidateTotpInputUseCaseTest { + + private val parsedUri = TotpInfo( + secret = SECRET, + issuer = "GitHub", + accountName = "alice@github.com", + algorithm = Algorithm.SHA1, + digits = 6, + period = 30, + ) + + @Test + fun `accepts an otpauth uri the parser understands`() { + val validate = makeUseCase(FakeTotpService().apply { infoFromUriResult = parsedUri }) + + assertTrue(validate(URI)) + } + + @Test + fun `accepts a bare secret codes can be generated from`() { + // No infoFromUriResult, so the fake parser rejects the input and only the secret probe + // can carry it. + val validate = makeUseCase(FakeTotpService()) + + assertTrue(validate(SECRET)) + } + + @Test + fun `rejects a bare secret codes cannot be generated from`() { + val validate = makeUseCase( + FakeTotpService().apply { invalidSecrets = setOf(NOT_BASE32) }, + ) + + assertFalse(validate(NOT_BASE32)) + } + + @Test + fun `rejects input that is neither a parsable uri nor a usable secret`() { + val malformedUri = "otpauth://totp/GitHub:alice@github.com?secret=$NOT_BASE32" + val validate = makeUseCase( + FakeTotpService().apply { invalidSecrets = setOf(malformedUri) }, + ) + + assertFalse(validate(malformedUri)) + } + + @Test + fun `falls back to the secret probe when the uri parser rejects the input`() { + // A URI that parses is never handed to the probe, so a service that rejects every secret + // still validates it. + val validate = makeUseCase( + FakeTotpService().apply { + infoFromUriResult = parsedUri + invalidSecrets = setOf(URI) + }, + ) + + assertTrue(validate(URI)) + } + + private fun makeUseCase(totpService: FakeTotpService) = + ValidateTotpInputUseCase(totpService = totpService) + + companion object { + private const val SECRET = "JBSWY3DPEHPK3PXP" + private const val NOT_BASE32 = "not base32!" + private const val URI = "otpauth://totp/GitHub:alice@github.com?secret=$SECRET" + } +} 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..ba3c5cecf 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 @@ -228,6 +228,7 @@ private fun LoginReadyContent( state = state.totpTextFieldState, label = { Text(text = stringResource(R.string.totp_secret)) }, placeholder = { Text(text = stringResource(R.string.totp_secret)) }, + error = state.totpError, outsideTrailingContent = { IconButton( onClick = { onEvent(LoginUiEvent.OnScanCodeRequest) }, 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..738fb0d20 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 @@ -30,6 +30,7 @@ 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.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.core.presentation.model.InputFieldError @@ -69,6 +70,7 @@ internal class LoginViewModel( private val loginRepository: LoginRepository, private val passwordStrengthEstimator: PasswordStrengthEstimator, private val createNewOrUpdateLogin: CreateNewOrUpdateLoginUseCase, + private val validateTotpInput: ValidateTotpInputUseCase, private val getTdlMatchedLogins: GetTdlMatchedLoginsUseCase, private val snackbarManager: SnackbarManager, private val totpService: TotpService, @@ -254,6 +256,17 @@ internal class LoginViewModel( val selectedVaultId = ready.shared.vaultsState.selectedVaultId // Independent: a save can both register a passkey and drop another one. val pendingPasskey = base.passkeys.any { it.pending } + + // A scan is rejected while it is still a scan, so only what was typed or pasted can be + // unusable by the time it reaches here. + val totpInput = base.totpTextFieldState.text.toString() + val totpError = + if (totpInput.isNotBlank() && !validateTotpInput(totpInput)) InputFieldError.Invalid + else null + + _base.update { it.copy(totpError = totpError) } + if (totpError != null) return + viewModelScope.launch { val upsert = itemId?.let { itemId -> UpsertLogin.update( @@ -264,7 +277,7 @@ internal class LoginViewModel( domains = set(base.domains), tags = set(assignedTags), password = fieldUpdate(base.passwordTextFieldState.text.toString()), - totpUriOrSecret = fieldUpdate(base.totpTextFieldState.text.toString()), + totpUriOrSecret = fieldUpdate(totpInput), note = fieldUpdate(notesTextFieldState.text.toString()), removedPasskeys = base.deletedPasskeys, pendingPasskey = pendingPasskey, @@ -276,7 +289,7 @@ internal class LoginViewModel( domains = base.domains, tags = assignedTags, password = base.passwordTextFieldState.text.toString(), - totpUriOrSecret = base.totpTextFieldState.text.toString(), + totpUriOrSecret = totpInput, note = notesTextFieldState.text.toString(), pendingPasskey = pendingPasskey, ) @@ -562,6 +575,7 @@ internal class LoginViewModel( val currentState = _base.value secret?.let { currentState.totpTextFieldState.setTextAndPlaceCursorAtEnd(it) + _base.update { state -> state.copy(totpError = null) } } issuer?.let { 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..561103de3 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 @@ -37,6 +37,7 @@ internal data class LoginBaseState( val generatePasswordBottomSheetVisible: Boolean = false, val dialogState: DialogState = DialogState.None, val nameError: InputFieldError? = null, + val totpError: InputFieldError? = null, val scanning: Boolean = false, val updating: Boolean = false, ) { diff --git a/feature/item/view/src/main/kotlin/de/davis/keygo/feature/item/view/login/ViewLoginContent.kt b/feature/item/view/src/main/kotlin/de/davis/keygo/feature/item/view/login/ViewLoginContent.kt index a13bc2ed6..21a554d04 100644 --- a/feature/item/view/src/main/kotlin/de/davis/keygo/feature/item/view/login/ViewLoginContent.kt +++ b/feature/item/view/src/main/kotlin/de/davis/keygo/feature/item/view/login/ViewLoginContent.kt @@ -451,6 +451,7 @@ fun ViewLoginContent(state: ViewLoginState, onEvent: (ViewLoginUiEvent) -> Unit) } } } else null, + error = dialog.error, isSecure = dialog.fieldType.isSensitive, inputTransformation = transformation, ) diff --git a/feature/item/view/src/main/kotlin/de/davis/keygo/feature/item/view/login/ViewLoginViewModel.kt b/feature/item/view/src/main/kotlin/de/davis/keygo/feature/item/view/login/ViewLoginViewModel.kt index b080275af..c338565dd 100644 --- a/feature/item/view/src/main/kotlin/de/davis/keygo/feature/item/view/login/ViewLoginViewModel.kt +++ b/feature/item/view/src/main/kotlin/de/davis/keygo/feature/item/view/login/ViewLoginViewModel.kt @@ -22,9 +22,11 @@ import de.davis.keygo.core.util.onSuccess 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.getValue import de.davis.keygo.feature.item.core.domain.model.onSet 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.domain.usecase.ValidateTotpInputUseCase import de.davis.keygo.feature.item.core.presentation.login.model.FieldType import de.davis.keygo.feature.item.core.presentation.model.InputFieldError import de.davis.keygo.feature.item.core.presentation.model.NavigationEvent @@ -64,6 +66,7 @@ internal class ViewLoginViewModel( private val itemRepository: ItemRepository, private val vaultRepository: VaultRepository, private val updateLogin: CreateNewOrUpdateLoginUseCase, + private val validateTotpInput: ValidateTotpInputUseCase, private val isValidUrl: IsValidUrlUseCase, private val sort: SortUseCase, private val websiteHandler: WebsiteHandler, @@ -277,10 +280,18 @@ internal class ViewLoginViewModel( password = newText, ) - FieldType.Totp -> UpsertLogin.update( - itemId = id, - totpUriOrSecret = newText, - ) + FieldType.Totp -> { + val uriOrSecret = newText.getValue() + if (uriOrSecret != null && !validateTotpInput(uriOrSecret)) { + _modificationDialogState.update { dialog.copy(error = InputFieldError.Invalid) } + return@launch + } + + UpsertLogin.update( + itemId = id, + totpUriOrSecret = newText, + ) + } FieldType.Username -> UpsertLogin.update( itemId = id, diff --git a/rust/src/main/kotlin/de/davis/keygo/rust/totp/Totp.kt b/rust/src/main/kotlin/de/davis/keygo/rust/totp/Totp.kt index 9f8bf1172..3a913b095 100644 --- a/rust/src/main/kotlin/de/davis/keygo/rust/totp/Totp.kt +++ b/rust/src/main/kotlin/de/davis/keygo/rust/totp/Totp.kt @@ -1,6 +1,7 @@ package de.davis.keygo.rust.totp import de.davis.keygo.core.util.Result +import de.davis.keygo.core.util.isSuccess import de.davis.keygo.core.util.resultBinding import de.davisalessandro.keygo.rust.Algorithm import de.davisalessandro.keygo.rust.TotpException @@ -51,3 +52,16 @@ fun TotpServiceInterface.getInfoFromUriWithResult( onSuccess = { Result.Success(it) }, onFailure = { Result.Failure(it as TotpException) } ) + +/** + * Whether a code can actually be generated from [secret]. + */ +fun TotpServiceInterface.isValidSecret(secret: String): Boolean = getTotpWithResult( + algorithm = Algorithm.SHA1, + digits = PROBE_DIGITS, + step = PROBE_STEP, + secret = secret, +).isSuccess() + +private const val PROBE_DIGITS = 6 +private const val PROBE_STEP = 30 diff --git a/rust/src/testFixtures/kotlin/de/davis/keygo/rust/FakeTotpService.kt b/rust/src/testFixtures/kotlin/de/davis/keygo/rust/FakeTotpService.kt index 354147040..72e457726 100644 --- a/rust/src/testFixtures/kotlin/de/davis/keygo/rust/FakeTotpService.kt +++ b/rust/src/testFixtures/kotlin/de/davis/keygo/rust/FakeTotpService.kt @@ -11,12 +11,18 @@ class FakeTotpService : TotpServiceInterface { var urlResult: String = "" var infoFromUriResult: TotpInfo? = null + var invalidSecrets: Set = emptySet() + override fun getTotp( algorithm: Algorithm, digits: Int, step: Int, secret: String - ): String = totpResult + ): String { + if (secret in invalidSecrets) throw TotpException.InvalidInput() + + return totpResult + } override fun getUrl( algorithm: Algorithm,