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 fa91376c8b..6f43f614c5 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 @@ -107,7 +107,11 @@ class SignalLoginCredentialEntryViewModel( } is SignalLoginCredentialEntryScreenEvents.TwoFactorCodeEntered -> { - applyNextClicked(state, totp = event.code.toIntOrNull(), parentEventEmitter, stateEmitter) + if (state.isNextEnabled) { + applyNextClicked(state, totp = event.code.toIntOrNull(), parentEventEmitter, stateEmitter) + } else { + Log.w(TAG, "[TwoFactorCodeEntered] Got a two-factor code, but the login on screen is no longer submittable. Leaving the user on the credential screen to re-enter it.") + } } } } @@ -140,6 +144,12 @@ class SignalLoginCredentialEntryViewModel( } } + /** + * Submits the login currently on screen. Callers are expected to have checked [SignalLoginCredentialEntryState.isNextEnabled] + * first, but the incomplete cases are handled rather than asserted: a two-factor code can arrive from the TOTP screen + * long after this view model was recreated with empty fields (e.g. after process death), so an incomplete login here is + * something to log and drop, not a crash. + */ private suspend fun applyNextClicked( state: SignalLoginCredentialEntryState, totp: Int?, @@ -153,7 +163,10 @@ class SignalLoginCredentialEntryViewModel( return } - check(state.recoveryKey.isValid) { "Recovery key is not valid, should not have gotten here." } + if (!state.recoveryKey.isValid) { + Log.w(TAG, "[Next] The recovery key on screen isn't complete, so there is nothing to submit.") + return + } val aep = AccountEntropyPool(state.recoveryKey.normalized) 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 c22aac1380..f248168797 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 @@ -451,6 +451,24 @@ class SignalLoginCredentialEntryViewModelTest { } } + @Test + fun `TwoFactorCodeEntered for a screen whose recovery key is gone does not attempt a login`() = runTest(testDispatcher) { + val state = SignalLoginCredentialEntryState(accountId = VALID_ACCOUNT_ID) + + applyEvent(state, SignalLoginCredentialEntryScreenEvents.TwoFactorCodeEntered("123456")) + + assertThat(emittedStates).isEmpty() + assertThat(emittedParentEvents).isEmpty() + coVerify(exactly = 0) { mockRepository.reRegisterAccountWithoutPhoneNumber(any(), any(), any(), any(), any(), any()) } + } + + @Test + fun `TwoFactorCodeEntered while the login is already in flight does not attempt a second login`() = runTest(testDispatcher) { + applyEvent(completeState().copy(isLoggingIn = true), SignalLoginCredentialEntryScreenEvents.TwoFactorCodeEntered("123456")) + + coVerify(exactly = 0) { mockRepository.reRegisterAccountWithoutPhoneNumber(any(), any(), any(), any(), any(), any()) } + } + @Test(expected = IllegalStateException::class) fun `NextClicked with SessionNotFoundOrNotVerified throws`() = runTest(testDispatcher) { coEvery { mockRepository.reRegisterAccountWithoutPhoneNumber(any(), any(), any(), any(), any()) } returns