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 65582bf61d..d0596211bb 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
@@ -401,7 +401,8 @@ private fun PinInputSection(
isConfirm = isConfirm,
isAlphanumericKeyboard = state.isAlphanumericKeyboard,
isMismatch = state.pinMismatch,
- matchesVerificationCode = state.pinMatchesVerificationCode
+ matchesVerificationCode = state.pinMatchesVerificationCode,
+ isTooWeak = state.pinTooWeak
)
Spacer(modifier = Modifier.height(16.dp))
KeyboardToggleButton(
@@ -445,14 +446,16 @@ private fun PinInputLabel(
isAlphanumericKeyboard: Boolean,
isMismatch: Boolean,
matchesVerificationCode: Boolean,
+ isTooWeak: Boolean,
modifier: Modifier = Modifier
) {
- val isError = !isConfirm && (isMismatch || matchesVerificationCode)
+ val isError = !isConfirm && (isMismatch || matchesVerificationCode || isTooWeak)
Text(
text = when {
isConfirm -> stringResource(R.string.PinCreationScreen__reenter_pin)
matchesVerificationCode -> stringResource(R.string.PinCreationScreen__reentered_verification_code)
+ isTooWeak -> stringResource(R.string.PinCreationScreen__choose_a_stronger_pin)
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)
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 ba0e8152a4..6279e6dcc3 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
@@ -14,6 +14,7 @@ data class PinCreationState(
val isConfirmEnabled: Boolean = false,
val pinMismatch: Boolean = false,
val pinMatchesVerificationCode: Boolean = false,
+ val pinTooWeak: Boolean = false,
val loading: Boolean = false,
val firstPin: String? = null,
val submittedVerificationCode: String? = null,
@@ -21,7 +22,7 @@ data class PinCreationState(
val dialogs: Dialogs = Dialogs()
) {
override fun toString(): String {
- 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)"
+ return "PinCreationState(isAlphanumericKeyboard=$isAlphanumericKeyboard, isConfirmEnabled=$isConfirmEnabled, pinMismatch=$pinMismatch, pinMatchesVerificationCode=$pinMatchesVerificationCode, pinTooWeak=$pinTooWeak, 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 58e05087b4..01781a4124 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
@@ -22,6 +22,7 @@ import org.signal.registration.RegistrationFlowEvent
import org.signal.registration.RegistrationFlowState
import org.signal.registration.RegistrationRepository
import org.signal.registration.RestoreDecision
+import org.whispersystems.signalservice.api.kbs.PinValidityChecker
import kotlin.time.toKotlinDuration
/**
@@ -66,12 +67,17 @@ class PinCreationViewModel(
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.value = state.copy(pinMatchesVerificationCode = true, pinMismatch = false, pinTooWeak = false)
+ }
+
+ !state.isConfirmEnabled && !PinValidityChecker.valid(event.pin) -> {
+ Log.w(TAG, "[PinSubmitted] User entered a PIN that is too common. Prompting them to choose a stronger PIN.")
+ _state.value = state.copy(pinTooWeak = true, pinMismatch = false, pinMatchesVerificationCode = 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, pinMatchesVerificationCode = false)
+ _state.value = state.copy(firstPin = event.pin, isConfirmEnabled = true, pinMismatch = false, pinMatchesVerificationCode = false, pinTooWeak = false)
}
event.pin != state.firstPin -> {
diff --git a/feature/registration/src/main/res/values/strings.xml b/feature/registration/src/main/res/values/strings.xml
index 4219c48937..9cea5c7ba6 100644
--- a/feature/registration/src/main/res/values/strings.xml
+++ b/feature/registration/src/main/res/values/strings.xml
@@ -377,6 +377,8 @@
Re-enter PIN
PINs don\'t match. Try again.
+
+ Choose a stronger PIN
You re-entered the code that was already used to verify your phone number. Choose a new and unique Signal PIN.
diff --git a/feature/registration/src/test/java/org/signal/registration/screens/pincreation/PinCreationScreenTest.kt b/feature/registration/src/test/java/org/signal/registration/screens/pincreation/PinCreationScreenTest.kt
index 9a629c0530..234781d01a 100644
--- a/feature/registration/src/test/java/org/signal/registration/screens/pincreation/PinCreationScreenTest.kt
+++ b/feature/registration/src/test/java/org/signal/registration/screens/pincreation/PinCreationScreenTest.kt
@@ -10,6 +10,7 @@ import androidx.compose.ui.test.assertIsDisplayed
import androidx.compose.ui.test.assertIsNotEnabled
import androidx.compose.ui.test.junit4.createComposeRule
import androidx.compose.ui.test.onNodeWithTag
+import androidx.compose.ui.test.onNodeWithText
import androidx.compose.ui.test.performClick
import androidx.compose.ui.test.performTextInput
import androidx.test.core.app.ApplicationProvider
@@ -20,6 +21,7 @@ import org.robolectric.RobolectricTestRunner
import org.robolectric.annotation.Config
import org.signal.core.ui.CoreUiDependenciesRule
import org.signal.core.ui.compose.theme.SignalTheme
+import org.signal.registration.R
import org.signal.registration.test.TestTags
/**
@@ -101,4 +103,19 @@ class PinCreationScreenTest {
assert(emittedEvent is PinCreationScreenEvents.PinSubmitted)
}
+
+ @Test
+ fun `when the pin is too weak, the stronger pin error is displayed`() {
+ composeTestRule.setContent {
+ SignalTheme {
+ PinCreationScreen(
+ state = PinCreationState(pinTooWeak = true),
+ onEvent = {}
+ )
+ }
+ }
+
+ val context = ApplicationProvider.getApplicationContext()
+ composeTestRule.onNodeWithText(context.getString(R.string.PinCreationScreen__choose_a_stronger_pin)).assertIsDisplayed()
+ }
}
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 6737127fdb..fdbbaf6240 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
@@ -82,7 +82,7 @@ class PinCreationViewModelTest {
val states = collectStates()
val initialState = PinCreationState(accountEntropyPool = AccountEntropyPool.generate())
- viewModel.applyEvent(initialState, PinCreationScreenEvents.PinSubmitted("123456"))
+ viewModel.applyEvent(initialState, PinCreationScreenEvents.PinSubmitted("148502"))
coVerify(exactly = 0) { mockRepository.setNewlyCreatedPin(any(), any(), any()) }
assertThat(emittedParentEvents).hasSize(0)
@@ -142,12 +142,107 @@ class PinCreationViewModelTest {
val states = collectStates()
val initialState = PinCreationState(accountEntropyPool = AccountEntropyPool.generate(), submittedVerificationCode = "123456")
- viewModel.applyEvent(initialState, PinCreationScreenEvents.PinSubmitted("987654"))
+ viewModel.applyEvent(initialState, PinCreationScreenEvents.PinSubmitted("148502"))
assertThat(states.last().isConfirmEnabled).isTrue()
assertThat(states.last().pinMatchesVerificationCode).isFalse()
}
+ // ==================== Weak PIN Tests ====================
+
+ @Test
+ fun `first PinSubmitted with an ascending sequential PIN is rejected as too weak`() = runTest(testDispatcher) {
+ val states = collectStates()
+ val initialState = PinCreationState(accountEntropyPool = AccountEntropyPool.generate())
+
+ viewModel.applyEvent(initialState, PinCreationScreenEvents.PinSubmitted("1234"))
+
+ coVerify(exactly = 0) { mockRepository.setNewlyCreatedPin(any(), any(), any()) }
+ assertThat(emittedParentEvents).hasSize(0)
+ assertThat(states.last().pinTooWeak).isTrue()
+ assertThat(states.last().isConfirmEnabled).isFalse()
+ assertThat(states.last().firstPin).isNull()
+ }
+
+ @Test
+ fun `first PinSubmitted with a descending sequential PIN is rejected as too weak`() = runTest(testDispatcher) {
+ val states = collectStates()
+ val initialState = PinCreationState(accountEntropyPool = AccountEntropyPool.generate())
+
+ viewModel.applyEvent(initialState, PinCreationScreenEvents.PinSubmitted("43210"))
+
+ assertThat(states.last().pinTooWeak).isTrue()
+ assertThat(states.last().isConfirmEnabled).isFalse()
+ }
+
+ @Test
+ fun `first PinSubmitted with a repeated-digit PIN is rejected as too weak`() = runTest(testDispatcher) {
+ val states = collectStates()
+ val initialState = PinCreationState(accountEntropyPool = AccountEntropyPool.generate())
+
+ viewModel.applyEvent(initialState, PinCreationScreenEvents.PinSubmitted("9999"))
+
+ assertThat(states.last().pinTooWeak).isTrue()
+ assertThat(states.last().isConfirmEnabled).isFalse()
+ }
+
+ @Test
+ fun `first PinSubmitted with a sequential non-arabic-numeral PIN is rejected as too weak`() = runTest(testDispatcher) {
+ val states = collectStates()
+ val initialState = PinCreationState(accountEntropyPool = AccountEntropyPool.generate())
+
+ viewModel.applyEvent(initialState, PinCreationScreenEvents.PinSubmitted("١٢٣٤٥"))
+
+ assertThat(states.last().pinTooWeak).isTrue()
+ assertThat(states.last().isConfirmEnabled).isFalse()
+ }
+
+ @Test
+ fun `first PinSubmitted with a sequential alphanumeric PIN is allowed`() = runTest(testDispatcher) {
+ val states = collectStates()
+ val initialState = PinCreationState(accountEntropyPool = AccountEntropyPool.generate(), isAlphanumericKeyboard = true)
+
+ viewModel.applyEvent(initialState, PinCreationScreenEvents.PinSubmitted("abcd"))
+
+ assertThat(states.last().pinTooWeak).isFalse()
+ assertThat(states.last().isConfirmEnabled).isTrue()
+ }
+
+ @Test
+ fun `first PinSubmitted matching the verification code is reported as such rather than as too weak`() = runTest(testDispatcher) {
+ val states = collectStates()
+ val initialState = PinCreationState(accountEntropyPool = AccountEntropyPool.generate(), submittedVerificationCode = "123456")
+
+ viewModel.applyEvent(initialState, PinCreationScreenEvents.PinSubmitted("123456"))
+
+ assertThat(states.last().pinMatchesVerificationCode).isTrue()
+ assertThat(states.last().pinTooWeak).isFalse()
+ }
+
+ @Test
+ fun `first PinSubmitted with a strong PIN clears a previous too-weak error`() = runTest(testDispatcher) {
+ val states = collectStates()
+ val weakState = PinCreationState(accountEntropyPool = AccountEntropyPool.generate(), pinTooWeak = true)
+
+ viewModel.applyEvent(weakState, PinCreationScreenEvents.PinSubmitted("148502"))
+
+ assertThat(states.last().pinTooWeak).isFalse()
+ assertThat(states.last().isConfirmEnabled).isTrue()
+ }
+
+ @Test
+ fun `weak PIN check is skipped on the confirmation step`() = runTest(testDispatcher) {
+ val aep = AccountEntropyPool.generate()
+ val confirmState = PinCreationState(accountEntropyPool = aep, isConfirmEnabled = true, firstPin = "148502")
+
+ coEvery { mockRepository.setNewlyCreatedPin(any(), any(), any()) } returns RequestResult.Success(null)
+
+ viewModel.applyEvent(confirmState, PinCreationScreenEvents.PinSubmitted("148502"))
+
+ coVerify { mockRepository.setNewlyCreatedPin("148502", any(), any()) }
+ assertThat(emittedParentEvents).contains(RegistrationFlowEvent.RegistrationComplete)
+ }
+
// ==================== PinSubmitted Success Tests ====================
@Test