From 05973d9f021ff212c5c91cbe9c1eb1c23f0d8c3d Mon Sep 17 00:00:00 2001 From: Greyson Parrelli Date: Mon, 14 Sep 2026 23:48:05 -0400 Subject: [PATCH] Skip two-factor selection if there's only one option. --- .../SignalLoginCredentialEntryViewModel.kt | 10 +++++++--- .../screens/twofactorselection/TwoFactorMethod.kt | 14 ++++++++++++++ .../registration/RegistrationEndToEndTest.kt | 8 +++++--- .../SignalLoginCredentialEntryViewModelTest.kt | 7 ++----- 4 files changed, 28 insertions(+), 11 deletions(-) diff --git a/feature/registration/src/main/java/org/signal/registration/screens/signallogincredentials/SignalLoginCredentialEntryViewModel.kt b/feature/registration/src/main/java/org/signal/registration/screens/signallogincredentials/SignalLoginCredentialEntryViewModel.kt index 9ed976042d..9a678f9417 100644 --- a/feature/registration/src/main/java/org/signal/registration/screens/signallogincredentials/SignalLoginCredentialEntryViewModel.kt +++ b/feature/registration/src/main/java/org/signal/registration/screens/signallogincredentials/SignalLoginCredentialEntryViewModel.kt @@ -30,6 +30,7 @@ import org.signal.registration.screens.aepentry.AepInput import org.signal.registration.screens.shared.AccountIdError import org.signal.registration.screens.shared.AccountIdFormat import org.signal.registration.screens.twofactorselection.TwoFactorMethod +import org.signal.registration.screens.twofactorselection.toAuthenticationRoute import org.signal.registration.screens.util.navigateBack import org.signal.registration.screens.util.navigateTo @@ -244,10 +245,13 @@ class SignalLoginCredentialEntryViewModel( } RegisterAccountError.TotpMissingOrIncorrect -> { // For now this error only means TOTP, but in the future it will indicate that some two-factor method is - // required, so we treat it generically and route through the method selection screen. - Log.w(TAG, "[Next] A two-factor code is required. Sending the user to two-factor method selection.") + // required, so we treat it generically and let the method list decide where to go. + val methods = listOf(TwoFactorMethod.AuthenticatorApp) + val route = methods.toAuthenticationRoute() + + Log.w(TAG, "[Next] A two-factor code is required. Sending the user to $route.") stateEmitter(inputState.copy(isLoggingIn = false)) - parentEventEmitter.navigateTo(RegistrationRoute.TwoFactorSelection(methods = listOf(TwoFactorMethod.AuthenticatorApp))) + parentEventEmitter.navigateTo(route) } is RegisterAccountError.InvalidRequest, is RegisterAccountError.InvalidReceiptCredentialPresentation, diff --git a/feature/registration/src/main/java/org/signal/registration/screens/twofactorselection/TwoFactorMethod.kt b/feature/registration/src/main/java/org/signal/registration/screens/twofactorselection/TwoFactorMethod.kt index fb70688212..2b45a87cf9 100644 --- a/feature/registration/src/main/java/org/signal/registration/screens/twofactorselection/TwoFactorMethod.kt +++ b/feature/registration/src/main/java/org/signal/registration/screens/twofactorselection/TwoFactorMethod.kt @@ -5,6 +5,8 @@ package org.signal.registration.screens.twofactorselection +import org.signal.registration.RegistrationRoute + /** * Two-factor authentication methods that may be presented on the two-factor selection screen. Which ones are actually * offered depends on what the account has registered. @@ -16,3 +18,15 @@ enum class TwoFactorMethod { /** A one-time code generated by the user's authenticator app. */ AuthenticatorApp } + +/** + * The route to send the user to in order to authenticate with one of these [TwoFactorMethod]s. When they're all of the + * same kind, the selection screen has nothing to choose between, so we skip straight to that method. Multiple entries + * of a kind are fine: the service checks the credential against every one the account has registered. + */ +fun List.toAuthenticationRoute(): RegistrationRoute { + return when (toSet().singleOrNull()) { + TwoFactorMethod.AuthenticatorApp -> RegistrationRoute.TotpEntry + else -> RegistrationRoute.TwoFactorSelection(methods = this) + } +} diff --git a/feature/registration/src/test/java/org/signal/registration/RegistrationEndToEndTest.kt b/feature/registration/src/test/java/org/signal/registration/RegistrationEndToEndTest.kt index c9c9fcb982..0bb1a667f6 100644 --- a/feature/registration/src/test/java/org/signal/registration/RegistrationEndToEndTest.kt +++ b/feature/registration/src/test/java/org/signal/registration/RegistrationEndToEndTest.kt @@ -1938,14 +1938,16 @@ class RegistrationEndToEndTest { useExistingSignalLogin() enterSignalLogin(login) - // The service wants a second factor, so the user picks one and enters a code from it - waitForTag(TestTags.TWO_FACTOR_SELECTION_AUTHENTICATOR_APP_OPTION) - composeTestRule.onNodeWithTag(TestTags.TWO_FACTOR_SELECTION_AUTHENTICATOR_APP_OPTION).performClick() + // The authenticator app is the only second factor available, so the user lands straight on code entry waitForTag(CodeEntryFieldTestTags.digit(0)) composeTestRule.onNodeWithTag(CodeEntryFieldTestTags.digit(0)).performTextInput(totp) waitFor("registration to complete") { registrationComplete } + assert(composeTestRule.onAllNodesWithTag(TestTags.TWO_FACTOR_SELECTION_AUTHENTICATOR_APP_OPTION).fetchSemanticsNodes().isEmpty()) { + "There was only one two-factor method, so the selection screen should have been skipped" + } + assert(networkController.lastRegisterAccountRequest?.totp == totp.toInt()) { "Expected the entered code to be sent with the login but was ${networkController.lastRegisterAccountRequest}" } assert(storageController.committedData?.accountData?.e164 == null) { "Expected an account with no phone number" } } diff --git a/feature/registration/src/test/java/org/signal/registration/screens/signallogincredentials/SignalLoginCredentialEntryViewModelTest.kt b/feature/registration/src/test/java/org/signal/registration/screens/signallogincredentials/SignalLoginCredentialEntryViewModelTest.kt index 61bd364b58..806845a7fc 100644 --- a/feature/registration/src/test/java/org/signal/registration/screens/signallogincredentials/SignalLoginCredentialEntryViewModelTest.kt +++ b/feature/registration/src/test/java/org/signal/registration/screens/signallogincredentials/SignalLoginCredentialEntryViewModelTest.kt @@ -50,7 +50,6 @@ import org.signal.registration.screens.aepentry.AepInput import org.signal.registration.screens.restoreselection.ArchiveRestoreOption import org.signal.registration.screens.restoreselection.RegisteredState import org.signal.registration.screens.shared.AccountIdError -import org.signal.registration.screens.twofactorselection.TwoFactorMethod import java.io.IOException import java.util.UUID import kotlin.time.Duration @@ -462,7 +461,7 @@ class SignalLoginCredentialEntryViewModelTest { } @Test - fun `NextClicked requiring a two-factor code navigates to two-factor selection offering only the authenticator app`() = runTest(testDispatcher) { + fun `NextClicked requiring a two-factor code navigates directly to TOTP entry`() = runTest(testDispatcher) { coEvery { mockRepository.reRegisterAccountWithoutPhoneNumber(any(), any(), any(), any(), any()) } returns RequestResult.NonSuccess(RegisterAccountError.TotpMissingOrIncorrect) @@ -472,9 +471,7 @@ class SignalLoginCredentialEntryViewModelTest { assertThat(emittedParentEvents.last()) .isInstanceOf() .prop(RegistrationFlowEvent.NavigateToScreen::route) - .isInstanceOf() - .prop(RegistrationRoute.TwoFactorSelection::methods) - .containsExactly(TwoFactorMethod.AuthenticatorApp) + .isEqualTo(RegistrationRoute.TotpEntry) } @Test