Block creation of weak pins.

This commit is contained in:
Greyson Parrelli
2026-07-29 15:45:14 -04:00
parent 9b9f59f1b0
commit 74d391d95e
6 changed files with 131 additions and 7 deletions
@@ -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)
@@ -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(
@@ -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 -> {
@@ -377,6 +377,8 @@
<string name="PinCreationScreen__reenter_pin">Re-enter PIN</string>
<!-- Error shown below the PIN field when the confirmation PIN does not match the one the user first entered. -->
<string name="PinCreationScreen__pins_dont_match">PINs don\'t match. Try again.</string>
<!-- Error shown below the PIN field when the user tries to create a PIN that is too easy to guess, such as one made of sequential or repeated digits. -->
<string name="PinCreationScreen__choose_a_stronger_pin">Choose a stronger PIN</string>
<!-- Error shown below the PIN field when the user tries to create a PIN that matches the code used to verify their phone number. -->
<string name="PinCreationScreen__reentered_verification_code">You re-entered the code that was already used to verify your phone number. Choose a new and unique Signal PIN.</string>
<!-- Overflow menu item that opens a help article about Signal PINs. -->
@@ -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<Application>()
composeTestRule.onNodeWithText(context.getString(R.string.PinCreationScreen__choose_a_stronger_pin)).assertIsDisplayed()
}
}
@@ -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<MasterKey>()) }
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<MasterKey>()) }
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<MasterKey>()) } returns RequestResult.Success(null)
viewModel.applyEvent(confirmState, PinCreationScreenEvents.PinSubmitted("148502"))
coVerify { mockRepository.setNewlyCreatedPin("148502", any(), any<MasterKey>()) }
assertThat(emittedParentEvents).contains(RegistrationFlowEvent.RegistrationComplete)
}
// ==================== PinSubmitted Success Tests ====================
@Test