diff --git a/feature/registration/src/main/java/org/signal/registration/PersistedFlowState.kt b/feature/registration/src/main/java/org/signal/registration/PersistedFlowState.kt index 9f3011214d..7d7e6793cd 100644 --- a/feature/registration/src/main/java/org/signal/registration/PersistedFlowState.kt +++ b/feature/registration/src/main/java/org/signal/registration/PersistedFlowState.kt @@ -21,6 +21,7 @@ data class PersistedFlowState( val backStack: List, val sessionMetadata: NetworkController.SessionMetadata?, val sessionE164: String?, + val submittedVerificationCode: String? = null, val doNotAttemptRecoveryPassword: Boolean, val pendingRestoreOption: PendingRestoreOption? = null, val restoredAepValue: String? = null, @@ -38,6 +39,7 @@ fun RegistrationFlowState.toPersistedFlowState(): PersistedFlowState { backStack = backStack, sessionMetadata = sessionMetadata, sessionE164 = sessionE164, + submittedVerificationCode = submittedVerificationCode, doNotAttemptRecoveryPassword = doNotAttemptRecoveryPassword, pendingRestoreOption = pendingRestoreOption, restoredAepValue = unverifiedRestoredAep?.value, @@ -64,6 +66,7 @@ fun PersistedFlowState.toRegistrationFlowState( backStack = backStack, sessionMetadata = sessionMetadata, sessionE164 = sessionE164, + submittedVerificationCode = submittedVerificationCode, accountEntropyPool = accountEntropyPool, temporaryMasterKey = temporaryMasterKey, preExistingRegistrationData = preExistingRegistrationData, diff --git a/feature/registration/src/main/java/org/signal/registration/RegistrationFlowEvent.kt b/feature/registration/src/main/java/org/signal/registration/RegistrationFlowEvent.kt index cb9b176835..60c72959ce 100644 --- a/feature/registration/src/main/java/org/signal/registration/RegistrationFlowEvent.kt +++ b/feature/registration/src/main/java/org/signal/registration/RegistrationFlowEvent.kt @@ -33,6 +33,11 @@ sealed interface RegistrationFlowEvent { /** The e164 associated with this registration attempt has been updated. */ data class E164Chosen(val e164: String) : RegistrationFlowEvent + /** The user's phone number was successfully verified with the given code. Retained so later PIN screens can warn if the user re-enters it as their PIN. */ + data class VerificationCodeAccepted(val code: String) : RegistrationFlowEvent { + override fun toString(): String = "VerificationCodeAccepted(code=${code.censor()})" + } + /** * A verification code was requested for [e164] — either fulfilled or rejected as rate-limited. Records the epoch-millis * times at which the server will allow the next SMS and call requests, since the response reports both regardless of diff --git a/feature/registration/src/main/java/org/signal/registration/RegistrationFlowState.kt b/feature/registration/src/main/java/org/signal/registration/RegistrationFlowState.kt index b45439da1c..ac566653b9 100644 --- a/feature/registration/src/main/java/org/signal/registration/RegistrationFlowState.kt +++ b/feature/registration/src/main/java/org/signal/registration/RegistrationFlowState.kt @@ -28,6 +28,9 @@ data class RegistrationFlowState( /** The e164 associated with the [sessionMetadata]. */ val sessionE164: String? = null, + /** The verification code the user successfully used to verify their phone number, if they went through SMS/call verification. */ + val submittedVerificationCode: String? = null, + /** The AEP we generated as part of this registration. */ val accountEntropyPool: AccountEntropyPool? = null, @@ -65,7 +68,7 @@ data class RegistrationFlowState( val isRestoringNavigationState: Boolean = true ) : Parcelable { override fun toString(): String { - return "RegistrationFlowState(backStack=${backStack.joinToString()}, sessionMetadata=$sessionMetadata, sessionE164=$sessionE164, accountEntropyPool=${accountEntropyPool?.displayValue?.censor()}, storageCapable=$storageCapable, temporaryMasterKey=${temporaryMasterKey?.toString()?.censor()}, preExistingRegistrationData=$preExistingRegistrationData, doNotAttemptRecoveryPassword=$doNotAttemptRecoveryPassword, pendingRestoreOption=$pendingRestoreOption, unverifiedRestoredAep=${unverifiedRestoredAep?.displayValue?.censor()}, restoreMethodToken=${restoreMethodToken?.censor()}, lastSmsVerificationCodeRequest=$lastSmsVerificationCodeRequest, lastCallVerificationCodeRequest=$lastCallVerificationCodeRequest, isRestoringNavigation=$isRestoringNavigationState)" + return "RegistrationFlowState(backStack=${backStack.joinToString()}, sessionMetadata=$sessionMetadata, sessionE164=$sessionE164, submittedVerificationCode=${submittedVerificationCode?.censor()}, accountEntropyPool=${accountEntropyPool?.displayValue?.censor()}, storageCapable=$storageCapable, temporaryMasterKey=${temporaryMasterKey?.toString()?.censor()}, preExistingRegistrationData=$preExistingRegistrationData, doNotAttemptRecoveryPassword=$doNotAttemptRecoveryPassword, pendingRestoreOption=$pendingRestoreOption, unverifiedRestoredAep=${unverifiedRestoredAep?.displayValue?.censor()}, restoreMethodToken=${restoreMethodToken?.censor()}, lastSmsVerificationCodeRequest=$lastSmsVerificationCodeRequest, lastCallVerificationCodeRequest=$lastCallVerificationCodeRequest, isRestoringNavigation=$isRestoringNavigationState)" } } diff --git a/feature/registration/src/main/java/org/signal/registration/RegistrationViewModel.kt b/feature/registration/src/main/java/org/signal/registration/RegistrationViewModel.kt index 90be65facd..1c58a8e960 100644 --- a/feature/registration/src/main/java/org/signal/registration/RegistrationViewModel.kt +++ b/feature/registration/src/main/java/org/signal/registration/RegistrationViewModel.kt @@ -101,6 +101,7 @@ class RegistrationViewModel( is RegistrationFlowEvent.ResetState -> RegistrationFlowState(isRestoringNavigationState = false) is RegistrationFlowEvent.SessionUpdated -> state.copy(sessionMetadata = event.session) is RegistrationFlowEvent.E164Chosen -> state.copy(sessionE164 = event.e164) + is RegistrationFlowEvent.VerificationCodeAccepted -> state.copy(submittedVerificationCode = event.code) is RegistrationFlowEvent.VerificationCodeRequested -> state.copy( lastSmsVerificationCodeRequest = event.nextSmsAllowedTimestamp?.let { VerificationCodeRequest(event.e164, it) } ?: state.lastSmsVerificationCodeRequest, lastCallVerificationCodeRequest = event.nextCallAllowedTimestamp?.let { VerificationCodeRequest(event.e164, it) } ?: state.lastCallVerificationCodeRequest @@ -225,6 +226,7 @@ class RegistrationViewModel( is RegistrationFlowEvent.NavigateBackToScreen, is RegistrationFlowEvent.SessionUpdated, is RegistrationFlowEvent.E164Chosen, + is RegistrationFlowEvent.VerificationCodeAccepted, is RegistrationFlowEvent.VerificationCodeRequested, is RegistrationFlowEvent.RecoveryPasswordInvalid, is RegistrationFlowEvent.PendingRestoreOptionSelected, diff --git a/feature/registration/src/main/java/org/signal/registration/screens/pincreation/PinCreationScreen.kt b/feature/registration/src/main/java/org/signal/registration/screens/pincreation/PinCreationScreen.kt index e4a38c1bad..65582bf61d 100644 --- a/feature/registration/src/main/java/org/signal/registration/screens/pincreation/PinCreationScreen.kt +++ b/feature/registration/src/main/java/org/signal/registration/screens/pincreation/PinCreationScreen.kt @@ -400,7 +400,8 @@ private fun PinInputSection( PinInputLabel( isConfirm = isConfirm, isAlphanumericKeyboard = state.isAlphanumericKeyboard, - isMismatch = state.pinMismatch + isMismatch = state.pinMismatch, + matchesVerificationCode = state.pinMatchesVerificationCode ) Spacer(modifier = Modifier.height(16.dp)) KeyboardToggleButton( @@ -443,18 +444,22 @@ private fun PinInputLabel( isConfirm: Boolean, isAlphanumericKeyboard: Boolean, isMismatch: Boolean, + matchesVerificationCode: Boolean, modifier: Modifier = Modifier ) { + val isError = !isConfirm && (isMismatch || matchesVerificationCode) + Text( text = when { isConfirm -> stringResource(R.string.PinCreationScreen__reenter_pin) + matchesVerificationCode -> stringResource(R.string.PinCreationScreen__reentered_verification_code) isMismatch -> stringResource(R.string.PinCreationScreen__pins_dont_match) isAlphanumericKeyboard -> stringResource(R.string.PinCreationScreen__pin_at_least_4_characters) else -> stringResource(R.string.PinCreationScreen__pin_at_least_4_digits) }, style = MaterialTheme.typography.bodyMedium, textAlign = TextAlign.Center, - color = if (!isConfirm && isMismatch) MaterialTheme.colorScheme.error else MaterialTheme.colorScheme.onSurfaceVariant, + color = if (isError) MaterialTheme.colorScheme.error else MaterialTheme.colorScheme.onSurfaceVariant, modifier = modifier.fillMaxWidth() ) } diff --git a/feature/registration/src/main/java/org/signal/registration/screens/pincreation/PinCreationState.kt b/feature/registration/src/main/java/org/signal/registration/screens/pincreation/PinCreationState.kt index 93e8e00e05..ba0e8152a4 100644 --- a/feature/registration/src/main/java/org/signal/registration/screens/pincreation/PinCreationState.kt +++ b/feature/registration/src/main/java/org/signal/registration/screens/pincreation/PinCreationState.kt @@ -13,13 +13,15 @@ data class PinCreationState( val isAlphanumericKeyboard: Boolean = false, val isConfirmEnabled: Boolean = false, val pinMismatch: Boolean = false, + val pinMatchesVerificationCode: Boolean = false, val loading: Boolean = false, val firstPin: String? = null, + val submittedVerificationCode: String? = null, val accountEntropyPool: AccountEntropyPool? = null, val dialogs: Dialogs = Dialogs() ) { override fun toString(): String { - return "PinCreationState(isAlphanumericKeyboard=$isAlphanumericKeyboard, isConfirmEnabled=$isConfirmEnabled, pinMismatch=$pinMismatch, loading=$loading, firstPin=${firstPin?.let { "${it.length} chars" }}, accountEntropyPool=${accountEntropyPool?.displayValue?.censor()}, dialogs=$dialogs)" + return "PinCreationState(isAlphanumericKeyboard=$isAlphanumericKeyboard, isConfirmEnabled=$isConfirmEnabled, pinMismatch=$pinMismatch, pinMatchesVerificationCode=$pinMatchesVerificationCode, loading=$loading, firstPin=${firstPin?.let { "${it.length} chars" }}, submittedVerificationCode=${submittedVerificationCode?.censor()}, accountEntropyPool=${accountEntropyPool?.displayValue?.censor()}, dialogs=$dialogs)" } data class Dialogs( diff --git a/feature/registration/src/main/java/org/signal/registration/screens/pincreation/PinCreationViewModel.kt b/feature/registration/src/main/java/org/signal/registration/screens/pincreation/PinCreationViewModel.kt index aad3ae72ab..58e05087b4 100644 --- a/feature/registration/src/main/java/org/signal/registration/screens/pincreation/PinCreationViewModel.kt +++ b/feature/registration/src/main/java/org/signal/registration/screens/pincreation/PinCreationViewModel.kt @@ -64,9 +64,14 @@ class PinCreationViewModel( } is PinCreationScreenEvents.PinSubmitted -> { when { + !state.isConfirmEnabled && event.pin == state.submittedVerificationCode -> { + Log.w(TAG, "[PinSubmitted] User entered their verification code as their PIN. Prompting them to choose a different PIN.") + _state.value = state.copy(pinMatchesVerificationCode = true, pinMismatch = false) + } + !state.isConfirmEnabled -> { Log.d(TAG, "[PinSubmitted] First PIN entered. Asking the user to confirm it.") - _state.value = state.copy(firstPin = event.pin, isConfirmEnabled = true, pinMismatch = false) + _state.value = state.copy(firstPin = event.pin, isConfirmEnabled = true, pinMismatch = false, pinMatchesVerificationCode = false) } event.pin != state.firstPin -> { @@ -117,7 +122,7 @@ class PinCreationViewModel( } private fun applyParentState(state: PinCreationState, parentState: RegistrationFlowState): PinCreationState { - return state.copy(accountEntropyPool = parentState.accountEntropyPool) + return state.copy(accountEntropyPool = parentState.accountEntropyPool, submittedVerificationCode = parentState.submittedVerificationCode) } private suspend fun applyPinSubmitted(state: PinCreationState, pin: String): PinCreationState { diff --git a/feature/registration/src/main/java/org/signal/registration/screens/pinentry/PinEntryForRegistrationLockViewModel.kt b/feature/registration/src/main/java/org/signal/registration/screens/pinentry/PinEntryForRegistrationLockViewModel.kt index 5e93d8d902..862a5ec063 100644 --- a/feature/registration/src/main/java/org/signal/registration/screens/pinentry/PinEntryForRegistrationLockViewModel.kt +++ b/feature/registration/src/main/java/org/signal/registration/screens/pinentry/PinEntryForRegistrationLockViewModel.kt @@ -58,6 +58,10 @@ class PinEntryForRegistrationLockViewModel( _state .onEach { Log.d(TAG, "[State] $it") } .launchIn(viewModelScope) + + parentState + .onEach { onEvent(PinEntryScreenEvents.ParentStateChanged(it)) } + .launchIn(viewModelScope) } override suspend fun processEvent(event: PinEntryScreenEvents) { @@ -76,8 +80,10 @@ class PinEntryForRegistrationLockViewModel( throw NotImplementedError("Skip is not a valid action during registration lock PIN entry") } is PinEntryScreenEvents.CreateNewPin, - is PinEntryScreenEvents.ContactSupport, - is PinEntryScreenEvents.ParentStateChanged -> Unit + is PinEntryScreenEvents.ContactSupport -> Unit + is PinEntryScreenEvents.ParentStateChanged -> { + stateEmitter(applyParentState(state, event.parentState)) + } is PinEntryScreenEvents.ToggleKeyboard, is PinEntryScreenEvents.NetworkErrorDialogDismissed, is PinEntryScreenEvents.RateLimitedDialogDismissed, @@ -87,6 +93,10 @@ class PinEntryForRegistrationLockViewModel( } } + private fun applyParentState(state: PinEntryState, parentState: RegistrationFlowState): PinEntryState { + return state.copy(submittedVerificationCode = parentState.submittedVerificationCode) + } + private suspend fun applyPinEntered(state: PinEntryState, event: PinEntryScreenEvents.PinEntered, parentEventEmitter: (RegistrationFlowEvent) -> Unit): PinEntryState { Log.d(TAG, "[PinEntered] Attempting to restore master key from SVR...") @@ -106,7 +116,7 @@ class PinEntryForRegistrationLockViewModel( parentEventEmitter.navigateTo(RegistrationRoute.AccountLocked(timeRemainingMs = timeRemaining)) state } else { - state.copy(loading = false, triesRemaining = error.triesRemaining) + state.copy(loading = false, triesRemaining = error.triesRemaining, enteredVerificationCode = event.pin == state.submittedVerificationCode) } } is NetworkController.RestoreMasterKeyError.NoDataFound -> { diff --git a/feature/registration/src/main/java/org/signal/registration/screens/pinentry/PinEntryForSmsBypassViewModel.kt b/feature/registration/src/main/java/org/signal/registration/screens/pinentry/PinEntryForSmsBypassViewModel.kt index 7dc7d86f91..33878ca675 100644 --- a/feature/registration/src/main/java/org/signal/registration/screens/pinentry/PinEntryForSmsBypassViewModel.kt +++ b/feature/registration/src/main/java/org/signal/registration/screens/pinentry/PinEntryForSmsBypassViewModel.kt @@ -96,7 +96,7 @@ class PinEntryForSmsBypassViewModel( } private fun applyParentState(state: PinEntryState, parentState: RegistrationFlowState): PinEntryState { - return state.copy(e164 = parentState.sessionE164) + return state.copy(e164 = parentState.sessionE164, submittedVerificationCode = parentState.submittedVerificationCode) } private suspend fun applyPinEntered( @@ -122,7 +122,7 @@ class PinEntryForSmsBypassViewModel( when (val error = result.error) { is NetworkController.RestoreMasterKeyError.WrongPin -> { Log.w(TAG, "[PinEntered] Wrong PIN. Tries remaining: ${error.triesRemaining}") - state.copy(loading = false, triesRemaining = error.triesRemaining) + state.copy(loading = false, triesRemaining = error.triesRemaining, enteredVerificationCode = event.pin == state.submittedVerificationCode) } is NetworkController.RestoreMasterKeyError.NoDataFound -> { Log.w(TAG, "[PinEntered] No SVR data found for sms-bypass credential. Marking RRP as invalid and navigating back.") diff --git a/feature/registration/src/main/java/org/signal/registration/screens/pinentry/PinEntryForSvrRestoreViewModel.kt b/feature/registration/src/main/java/org/signal/registration/screens/pinentry/PinEntryForSvrRestoreViewModel.kt index 0392845a31..7ec0b748cd 100644 --- a/feature/registration/src/main/java/org/signal/registration/screens/pinentry/PinEntryForSvrRestoreViewModel.kt +++ b/feature/registration/src/main/java/org/signal/registration/screens/pinentry/PinEntryForSvrRestoreViewModel.kt @@ -53,6 +53,10 @@ class PinEntryForSvrRestoreViewModel( _state .onEach { Log.d(TAG, "[State] $it") } .launchIn(viewModelScope) + + parentState + .onEach { onEvent(PinEntryScreenEvents.ParentStateChanged(it)) } + .launchIn(viewModelScope) } override suspend fun processEvent(event: PinEntryScreenEvents) { @@ -90,10 +94,16 @@ class PinEntryForSvrRestoreViewModel( is PinEntryScreenEvents.UnknownErrorDialogDismissed -> { stateEmitter(PinEntryScreenEventHandler.applyEvent(state, event)) } - is PinEntryScreenEvents.ParentStateChanged -> Unit + is PinEntryScreenEvents.ParentStateChanged -> { + stateEmitter(applyParentState(state, event.parentState)) + } } } + private fun applyParentState(state: PinEntryState, parentState: RegistrationFlowState): PinEntryState { + return state.copy(submittedVerificationCode = parentState.submittedVerificationCode) + } + private suspend fun applyPinEntered( state: PinEntryState, event: PinEntryScreenEvents.PinEntered, @@ -141,7 +151,7 @@ class PinEntryForSvrRestoreViewModel( when (val error = result.error) { is NetworkController.RestoreMasterKeyError.WrongPin -> { Log.w(TAG, "[PinEntered] Wrong PIN. Tries remaining: ${error.triesRemaining}") - state.copy(loading = false, triesRemaining = error.triesRemaining) + state.copy(loading = false, triesRemaining = error.triesRemaining, enteredVerificationCode = event.pin == state.submittedVerificationCode) } is NetworkController.RestoreMasterKeyError.NoDataFound -> { Log.w(TAG, "[PinEntered] No SVR data found. Prompting user to create a new PIN.") diff --git a/feature/registration/src/main/java/org/signal/registration/screens/pinentry/PinEntryScreen.kt b/feature/registration/src/main/java/org/signal/registration/screens/pinentry/PinEntryScreen.kt index b5f10617ca..4616814394 100644 --- a/feature/registration/src/main/java/org/signal/registration/screens/pinentry/PinEntryScreen.kt +++ b/feature/registration/src/main/java/org/signal/registration/screens/pinentry/PinEntryScreen.kt @@ -386,18 +386,21 @@ private fun PinInputField( imeAction = ImeAction.Done ), keyboardActions = KeyboardActions(onDone = { if (canSubmitPin) onSubmit() }), - isError = state.triesRemaining != null, + isError = state.triesRemaining != null || state.enteredVerificationCode, visualTransformation = PinVisualTransformation ) - if (state.triesRemaining != null) { - Spacer(modifier = Modifier.height(8.dp)) + Spacer(modifier = Modifier.height(8.dp)) + if (state.enteredVerificationCode) { + PinInputLabel( + text = stringResource(R.string.PinEntryScreen__reentered_verification_code), + isError = true + ) + } else if (state.triesRemaining != null) { PinInputLabel( text = pluralStringResource(R.plurals.PinEntryScreen__incorrect_pin, state.triesRemaining, state.triesRemaining), isError = true ) - } else { - Spacer(modifier = Modifier.height(8.dp)) } Spacer(modifier = Modifier.height(16.dp)) diff --git a/feature/registration/src/main/java/org/signal/registration/screens/pinentry/PinEntryState.kt b/feature/registration/src/main/java/org/signal/registration/screens/pinentry/PinEntryState.kt index 5acac619d4..90ae92fd2f 100644 --- a/feature/registration/src/main/java/org/signal/registration/screens/pinentry/PinEntryState.kt +++ b/feature/registration/src/main/java/org/signal/registration/screens/pinentry/PinEntryState.kt @@ -5,6 +5,7 @@ package org.signal.registration.screens.pinentry +import org.signal.core.util.censor import kotlin.time.Duration data class PinEntryState( @@ -13,10 +14,18 @@ data class PinEntryState( val loading: Boolean = false, val showNoDataToRestoreDialog: Boolean = false, val triesRemaining: Int? = null, + /** True when the last wrong PIN the user entered matched the code they used to verify their phone number. */ + val enteredVerificationCode: Boolean = false, val mode: Mode = Mode.SvrRestore, val dialogs: Dialogs = Dialogs(), - val e164: String? = null + val e164: String? = null, + /** The code the user used to verify their phone number, copied from the parent flow state. Used to detect when they re-enter it as their PIN. */ + val submittedVerificationCode: String? = null ) { + override fun toString(): String { + return "PinEntryState(showNeedHelp=$showNeedHelp, isAlphanumericKeyboard=$isAlphanumericKeyboard, loading=$loading, showNoDataToRestoreDialog=$showNoDataToRestoreDialog, triesRemaining=$triesRemaining, enteredVerificationCode=$enteredVerificationCode, mode=$mode, dialogs=$dialogs, e164=$e164, submittedVerificationCode=${submittedVerificationCode?.censor()})" + } + enum class Mode { RegistrationLock, SmsBypass, diff --git a/feature/registration/src/main/java/org/signal/registration/screens/verificationcode/VerificationCodeViewModel.kt b/feature/registration/src/main/java/org/signal/registration/screens/verificationcode/VerificationCodeViewModel.kt index 1e72217e32..18178542f3 100644 --- a/feature/registration/src/main/java/org/signal/registration/screens/verificationcode/VerificationCodeViewModel.kt +++ b/feature/registration/src/main/java/org/signal/registration/screens/verificationcode/VerificationCodeViewModel.kt @@ -356,6 +356,8 @@ class VerificationCodeViewModel( return state.copy(snackbars = state.snackbars.copy(incorrectVerificationCode = true), incorrectCodeAttempts = newAttempts, digits = VerificationCodeState.emptyDigits(), focusedDigitIndex = 0) } + parentEventEmitter(RegistrationFlowEvent.VerificationCodeAccepted(code)) + // Attempt to register val registerResult = repository.registerAccountWithSession(e164 = state.e164, sessionId = sessionMetadata.id, skipDeviceTransfer = true) diff --git a/feature/registration/src/main/res/values/strings.xml b/feature/registration/src/main/res/values/strings.xml index ebcde549d5..650ac71916 100644 --- a/feature/registration/src/main/res/values/strings.xml +++ b/feature/registration/src/main/res/values/strings.xml @@ -373,6 +373,8 @@ Re-enter PIN PINs don\'t match. Try again. + + You re-entered the code that was already used to verify your phone number. Choose a new and unique Signal PIN. Learn more about PINs @@ -396,6 +398,8 @@ Enter your PIN Enter the PIN you created when you first installed Signal + + You re-entered the code that was already used to verify your phone number. Enter the unique Signal PIN that you previously chose for your account. Incorrect PIN. %1$d attempt remaining. diff --git a/feature/registration/src/test/java/org/signal/registration/PersistedFlowStateTest.kt b/feature/registration/src/test/java/org/signal/registration/PersistedFlowStateTest.kt index 1ba97b9782..20f9bef330 100644 --- a/feature/registration/src/test/java/org/signal/registration/PersistedFlowStateTest.kt +++ b/feature/registration/src/test/java/org/signal/registration/PersistedFlowStateTest.kt @@ -175,6 +175,7 @@ class PersistedFlowStateTest { backStack = listOf(RegistrationRoute.Welcome, RegistrationRoute.PinCreate), sessionMetadata = session, sessionE164 = "+15551234567", + submittedVerificationCode = "123456", accountEntropyPool = AccountEntropyPool.generate(), storageCapable = true, temporaryMasterKey = MasterKey(ByteArray(32)), @@ -188,6 +189,7 @@ class PersistedFlowStateTest { assertThat(persisted.backStack).isEqualTo(flowState.backStack) assertThat(persisted.sessionMetadata).isEqualTo(session) assertThat(persisted.sessionE164).isEqualTo("+15551234567") + assertThat(persisted.submittedVerificationCode).isEqualTo("123456") assertThat(persisted.doNotAttemptRecoveryPassword).isEqualTo(true) assertThat(persisted.storageCapable).isEqualTo(true) assertThat(persisted.smsVerificationCodeRequest).isEqualTo(VerificationCodeRequest("+15551234567", 12_345L)) @@ -210,6 +212,7 @@ class PersistedFlowStateTest { backStack = listOf(RegistrationRoute.Welcome, RegistrationRoute.PinCreate), sessionMetadata = session, sessionE164 = "+15551234567", + submittedVerificationCode = "123456", doNotAttemptRecoveryPassword = true, storageCapable = true, smsVerificationCodeRequest = VerificationCodeRequest("+15551234567", 12_345L), @@ -228,6 +231,7 @@ class PersistedFlowStateTest { assertThat(flowState.backStack).isEqualTo(persisted.backStack) assertThat(flowState.sessionMetadata).isEqualTo(session) assertThat(flowState.sessionE164).isEqualTo("+15551234567") + assertThat(flowState.submittedVerificationCode).isEqualTo("123456") assertThat(flowState.accountEntropyPool).isEqualTo(aep) assertThat(flowState.temporaryMasterKey).isEqualTo(masterKey) assertThat(flowState.preExistingRegistrationData).isNull() 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 c6a891bc7f..1ea871f5d5 100644 --- a/feature/registration/src/test/java/org/signal/registration/RegistrationEndToEndTest.kt +++ b/feature/registration/src/test/java/org/signal/registration/RegistrationEndToEndTest.kt @@ -24,6 +24,7 @@ import androidx.compose.ui.semantics.SemanticsProperties import androidx.compose.ui.semantics.getOrNull import androidx.compose.ui.test.junit4.createComposeRule import androidx.compose.ui.test.onAllNodesWithTag +import androidx.compose.ui.test.onAllNodesWithText import androidx.compose.ui.test.onNodeWithTag import androidx.compose.ui.test.performClick import androidx.compose.ui.test.performTextClearance @@ -144,6 +145,88 @@ class RegistrationEndToEndTest { assert(storageController.restoreDecision == RestoreDecision.NEW_ACCOUNT) { "Expected NEW_ACCOUNT restore decision but was ${storageController.restoreDecision}" } } + @Test + fun `entering the verification code as a new pin warns the user and blocks it until a different pin is chosen`() { + val warning = ApplicationProvider.getApplicationContext().getString(R.string.PinCreationScreen__reentered_verification_code) + + var registrationComplete = false + launchRegistrationFlow(onRegistrationComplete = { registrationComplete = true }) + + submitPhoneNumber() + submitVerificationCode(VERIFICATION_CODE) + + // On the PIN creation screen, re-entering the verification code as the new PIN is rejected with a warning + waitForTag(TestTags.PIN_CREATION_SCREEN) + composeTestRule.onNodeWithTag(TestTags.PIN_CREATION_INPUT).performTextInput(VERIFICATION_CODE) + composeTestRule.onNodeWithTag(TestTags.PIN_CREATION_NEXT_BUTTON).performClick() + + waitForText(warning) + assert(composeTestRule.onAllNodesWithTag(TestTags.PIN_CREATION_CONFIRM_INPUT).fetchSemanticsNodes().isEmpty()) { + "Expected to stay on the PIN creation step rather than advancing to confirmation" + } + assert(networkController.lastSetPinRequest == null) { "Should not have backed up the verification code as a PIN" } + + // Choosing a different PIN is accepted and completes registration + composeTestRule.onNodeWithTag(TestTags.PIN_CREATION_INPUT).performTextClearance() + createPin(PIN) + + waitFor("registration to complete") { registrationComplete } + + val committed = storageController.committedData + assert(committed != null) { "Expected registration data to be committed" } + assert(committed!!.pin == PIN) { "Expected committed pin $PIN but was ${committed.pin}" } + assert(networkController.lastSetPinRequest?.pin == PIN) { "Expected pin $PIN on SVR but was ${networkController.lastSetPinRequest?.pin}" } + } + + @Test + fun `entering the verification code as an existing pin warns the user before the correct pin restores the account`() { + val warning = ApplicationProvider.getApplicationContext().getString(R.string.PinEntryScreen__reentered_verification_code) + val masterKey = MasterKey(ByteArray(32) { it.toByte() }) + + // The account already has SVR data, so after verification the user is asked to enter their existing PIN + networkController.onRegisterAccount = { request -> + RequestResult.Success(networkController.registerAccountResponse(request.e164, storageCapable = true)) + } + networkController.onGetSvrCredentials = { + RequestResult.Success(SvrCredentials(username = "svr-user", password = "svr-pass")) + } + networkController.onRestoreMasterKeyFromSvr = { request -> + if (request.pin == PIN) { + RequestResult.Success(MasterKeyResponse(masterKey)) + } else { + RequestResult.NonSuccess(RestoreMasterKeyError.WrongPin(triesRemaining = 3)) + } + } + + var registrationComplete = false + launchRegistrationFlow(onRegistrationComplete = { registrationComplete = true }) + + submitPhoneNumber() + submitVerificationCode(VERIFICATION_CODE) + + // On the PIN entry screen, entering the verification code as the PIN is a wrong PIN and warns the user + waitForTag(TestTags.PIN_ENTRY_SCREEN) + composeTestRule.onNodeWithTag(TestTags.PIN_ENTRY_INPUT).performTextInput(VERIFICATION_CODE) + composeTestRule.onNodeWithTag(TestTags.PIN_ENTRY_CONTINUE_BUTTON).performClick() + + waitForText(warning) + assert(!registrationComplete) { "Registration should not complete with the verification code as the PIN" } + + // Entering the correct PIN restores the account and completes registration + composeTestRule.onNodeWithTag(TestTags.PIN_ENTRY_INPUT).performTextClearance() + composeTestRule.onNodeWithTag(TestTags.PIN_ENTRY_INPUT).performTextInput(PIN) + composeTestRule.onNodeWithTag(TestTags.PIN_ENTRY_CONTINUE_BUTTON).performClick() + + waitFor("registration to complete") { registrationComplete } + + assert(networkController.lastRestoreMasterKeyRequest?.pin == PIN) { "Expected master key restore with pin $PIN but was ${networkController.lastRestoreMasterKeyRequest}" } + + val committed = storageController.committedData + assert(committed != null) { "Expected registration data to be committed" } + assert(committed!!.accountData?.e164 == E164) { "Expected committed e164 $E164 but was ${committed.accountData?.e164}" } + assert(storageController.restoreDecision == RestoreDecision.COMPLETED) { "Expected COMPLETED restore decision but was ${storageController.restoreDecision}" } + } + @Test fun `a captcha submission for a session that no longer exists resets the flow, which can then be restarted to completion`() { // The session demands a captcha, but expires server-side before the solved captcha is submitted @@ -1298,6 +1381,12 @@ class RegistrationEndToEndTest { ) } + private fun waitForText(text: String) { + waitFor("node with text $text") { + composeTestRule.onAllNodesWithText(text).fetchSemanticsNodes().isNotEmpty() + } + } + private fun createMockPermissionsState(): MockMultiplePermissionsState { return MockMultiplePermissionsState( allPermissionsGranted = true, diff --git a/feature/registration/src/test/java/org/signal/registration/screens/pincreation/PinCreationViewModelTest.kt b/feature/registration/src/test/java/org/signal/registration/screens/pincreation/PinCreationViewModelTest.kt index f90a047029..6737127fdb 100644 --- a/feature/registration/src/test/java/org/signal/registration/screens/pincreation/PinCreationViewModelTest.kt +++ b/feature/registration/src/test/java/org/signal/registration/screens/pincreation/PinCreationViewModelTest.kt @@ -125,6 +125,29 @@ class PinCreationViewModelTest { assertThat(states.last().pinMismatch).isFalse() } + @Test + fun `first PinSubmitted matching the verification code warns and does not advance`() = runTest(testDispatcher) { + val states = collectStates() + val initialState = PinCreationState(accountEntropyPool = AccountEntropyPool.generate(), submittedVerificationCode = "123456") + + viewModel.applyEvent(initialState, PinCreationScreenEvents.PinSubmitted("123456")) + + assertThat(emittedParentEvents).hasSize(0) + assertThat(states.last().isConfirmEnabled).isFalse() + assertThat(states.last().pinMatchesVerificationCode).isTrue() + } + + @Test + fun `first PinSubmitted not matching the verification code advances to confirm`() = runTest(testDispatcher) { + val states = collectStates() + val initialState = PinCreationState(accountEntropyPool = AccountEntropyPool.generate(), submittedVerificationCode = "123456") + + viewModel.applyEvent(initialState, PinCreationScreenEvents.PinSubmitted("987654")) + + assertThat(states.last().isConfirmEnabled).isTrue() + assertThat(states.last().pinMatchesVerificationCode).isFalse() + } + // ==================== PinSubmitted Success Tests ==================== @Test diff --git a/feature/registration/src/test/java/org/signal/registration/screens/pinentry/PinEntryForSvrRestoreViewModelTest.kt b/feature/registration/src/test/java/org/signal/registration/screens/pinentry/PinEntryForSvrRestoreViewModelTest.kt index 56e773bd85..06d79c53b1 100644 --- a/feature/registration/src/test/java/org/signal/registration/screens/pinentry/PinEntryForSvrRestoreViewModelTest.kt +++ b/feature/registration/src/test/java/org/signal/registration/screens/pinentry/PinEntryForSvrRestoreViewModelTest.kt @@ -8,6 +8,7 @@ package org.signal.registration.screens.pinentry import assertk.assertThat import assertk.assertions.hasSize import assertk.assertions.isEqualTo +import assertk.assertions.isFalse import assertk.assertions.isInstanceOf import assertk.assertions.isTrue import assertk.assertions.prop @@ -170,6 +171,55 @@ class PinEntryForSvrRestoreViewModelTest { assertThat(emittedStates.last().loading).isEqualTo(false) } + @Test + fun `ParentStateChanged copies submittedVerificationCode from parent`() = runTest { + val parentFlowState = RegistrationFlowState(submittedVerificationCode = "123456") + + viewModel.applyEvent(PinEntryState(), PinEntryScreenEvents.ParentStateChanged(parentFlowState), parentEventEmitter, stateEmitter) + + assertThat(emittedStates.last().submittedVerificationCode).isEqualTo("123456") + } + + @Test + fun `PinEntered with wrong PIN matching the verification code flags enteredVerificationCode`() = runTest { + val svrCredentials = NetworkController.SvrCredentials( + username = "test-username", + password = "test-password" + ) + val initialState = PinEntryState(mode = PinEntryState.Mode.SvrRestore, submittedVerificationCode = "123456") + + coEvery { mockRepository.getSvrCredentials() } returns + RequestResult.Success(svrCredentials) + coEvery { mockRepository.restoreMasterKeyFromSvr(any(), any(), forRegistrationLock = false) } returns + RequestResult.NonSuccess( + NetworkController.RestoreMasterKeyError.WrongPin(3) + ) + + viewModel.applyEvent(initialState, PinEntryScreenEvents.PinEntered("123456"), parentEventEmitter, stateEmitter) + + assertThat(emittedStates.last().enteredVerificationCode).isTrue() + } + + @Test + fun `PinEntered with wrong PIN not matching the verification code does not flag enteredVerificationCode`() = runTest { + val svrCredentials = NetworkController.SvrCredentials( + username = "test-username", + password = "test-password" + ) + val initialState = PinEntryState(mode = PinEntryState.Mode.SvrRestore, submittedVerificationCode = "123456") + + coEvery { mockRepository.getSvrCredentials() } returns + RequestResult.Success(svrCredentials) + coEvery { mockRepository.restoreMasterKeyFromSvr(any(), any(), forRegistrationLock = false) } returns + RequestResult.NonSuccess( + NetworkController.RestoreMasterKeyError.WrongPin(3) + ) + + viewModel.applyEvent(initialState, PinEntryScreenEvents.PinEntered("987654"), parentEventEmitter, stateEmitter) + + assertThat(emittedStates.last().enteredVerificationCode).isFalse() + } + @Test fun `PinEntered with no SVR data shows the no-data-to-restore dialog without navigating`() = runTest { val svrCredentials = NetworkController.SvrCredentials( diff --git a/feature/registration/src/test/java/org/signal/registration/screens/verificationcode/VerificationCodeViewModelTest.kt b/feature/registration/src/test/java/org/signal/registration/screens/verificationcode/VerificationCodeViewModelTest.kt index d5eacd6b22..0644fa7710 100644 --- a/feature/registration/src/test/java/org/signal/registration/screens/verificationcode/VerificationCodeViewModelTest.kt +++ b/feature/registration/src/test/java/org/signal/registration/screens/verificationcode/VerificationCodeViewModelTest.kt @@ -550,9 +550,13 @@ class VerificationCodeViewModelTest { viewModel.applyEvent(initialState, VerificationCodeScreenEvents.CodeEntered("123456"), stateEmitter) - assertThat(emittedEvents).hasSize(2) - assertThat(emittedEvents[0]).isInstanceOf() - assertThat(emittedEvents[1]) + assertThat(emittedEvents).hasSize(3) + assertThat(emittedEvents[0]) + .isInstanceOf() + .prop(RegistrationFlowEvent.VerificationCodeAccepted::code) + .isEqualTo("123456") + assertThat(emittedEvents[1]).isInstanceOf() + assertThat(emittedEvents[2]) .isInstanceOf() .prop(RegistrationFlowEvent.NavigateToScreen::route) .isInstanceOf() @@ -576,7 +580,7 @@ class VerificationCodeViewModelTest { viewModel.applyEvent(initialState, VerificationCodeScreenEvents.CodeEntered("123456"), stateEmitter) - assertThat(emittedEvents[1]) + assertThat(emittedEvents[2]) .isInstanceOf() .prop(RegistrationFlowEvent.NavigateToScreen::route) .isInstanceOf() @@ -602,7 +606,7 @@ class VerificationCodeViewModelTest { viewModel.applyEvent(initialState, VerificationCodeScreenEvents.CodeEntered("123456"), stateEmitter) - assertThat(emittedEvents[1]) + assertThat(emittedEvents[2]) .isInstanceOf() .prop(RegistrationFlowEvent.NavigateToScreen::route) .isInstanceOf() @@ -695,9 +699,10 @@ class VerificationCodeViewModelTest { viewModel.applyEvent(initialState, VerificationCodeScreenEvents.CodeEntered("123456"), stateEmitter) - assertThat(emittedEvents).hasSize(2) - assertThat(emittedEvents[0]).isInstanceOf() - assertThat(emittedEvents[1]) + assertThat(emittedEvents).hasSize(3) + assertThat(emittedEvents[0]).isInstanceOf() + assertThat(emittedEvents[1]).isInstanceOf() + assertThat(emittedEvents[2]) .isInstanceOf() .prop(RegistrationFlowEvent.NavigateToScreen::route) .isInstanceOf()