mirror of
https://github.com/signalapp/Signal-Android.git
synced 2026-09-20 00:35:47 +01:00
Add text size limits to login boxes.
This commit is contained in:
+8
-14
@@ -10,10 +10,9 @@ import org.signal.core.util.censor
|
||||
|
||||
/**
|
||||
* Recovery key text as the user has typed it so far, alongside the normalized form and whatever is currently wrong with
|
||||
* it. Every screen that collects a recovery key shares this so they all agree on when a key is too long, malformed, or
|
||||
* finished.
|
||||
* it. Every screen that collects a recovery key shares this so they all agree on when a key is malformed or finished.
|
||||
*
|
||||
* @param enteredText The typed text, preserved verbatim (illegal characters stripped) so #/= stay visible as they are typed.
|
||||
* @param enteredText The typed text, preserved verbatim (illegal characters stripped, cut off at a complete key) so #/= stay visible as they are typed.
|
||||
* @param normalized Storage-normalized lowercase form of [enteredText], used for validation and submit.
|
||||
*/
|
||||
data class AepInput(
|
||||
@@ -27,29 +26,24 @@ data class AepInput(
|
||||
|
||||
companion object {
|
||||
/**
|
||||
* Normalizes [input] and works out what, if anything, is wrong with it. An error the user has already been shown
|
||||
* sticks around until it is actually resolved, so [previousError] gets a say in the outcome.
|
||||
* Normalizes [input], cutting off anything past a complete key, and works out what, if anything, is wrong with what
|
||||
* is left. An error the user has already been shown sticks around until it is actually resolved, so [previousError]
|
||||
* gets a say in the outcome.
|
||||
*/
|
||||
fun from(input: String, previousError: AepValidationError? = null): AepInput {
|
||||
val enteredText = AccountEntropyPool.removeIllegalCharacters(input).take(AccountEntropyPool.LENGTH + 16)
|
||||
val enteredText = AccountEntropyPool.removeIllegalCharacters(input).take(AccountEntropyPool.LENGTH)
|
||||
val normalized = AccountEntropyPool.formatForStorage(enteredText).lowercase()
|
||||
|
||||
val isValid = AccountEntropyPool.isFullyValid(normalized)
|
||||
val isShort = normalized.length < AccountEntropyPool.LENGTH
|
||||
val isExact = normalized.length == AccountEntropyPool.LENGTH
|
||||
val isComplete = normalized.length == AccountEntropyPool.LENGTH
|
||||
|
||||
val carriedError = when (previousError) {
|
||||
is AepValidationError.TooLong -> if (isShort || isExact) null else previousError.copy(count = normalized.length)
|
||||
AepValidationError.Invalid -> if (isValid) null else previousError
|
||||
AepValidationError.Incorrect -> null
|
||||
null -> null
|
||||
}
|
||||
|
||||
val error = carriedError ?: when {
|
||||
!isShort && !isExact -> AepValidationError.TooLong(normalized.length, AccountEntropyPool.LENGTH)
|
||||
!isValid && isExact -> AepValidationError.Invalid
|
||||
else -> null
|
||||
}
|
||||
val error = carriedError ?: AepValidationError.Invalid.takeIf { isComplete && !isValid }
|
||||
|
||||
return AepInput(enteredText = enteredText, normalized = normalized, isValid = isValid, error = error)
|
||||
}
|
||||
|
||||
+1
-2
@@ -311,8 +311,7 @@ private fun RecoveryKeyTextField(state: EnterAepState, onEvent: (EnterAepEvents)
|
||||
}
|
||||
),
|
||||
supportingText = {
|
||||
when (val error = state.recoveryKey.error) {
|
||||
is AepValidationError.TooLong -> Text(stringResource(R.string.EnterAepScreen__too_long, error.count, error.max))
|
||||
when (state.recoveryKey.error) {
|
||||
is AepValidationError.Invalid -> Text(stringResource(R.string.EnterAepScreen__invalid_recovery_key))
|
||||
is AepValidationError.Incorrect -> Text(stringResource(R.string.EnterAepScreen__incorrect_recovery_key))
|
||||
null -> {}
|
||||
|
||||
-1
@@ -19,7 +19,6 @@ data class EnterAepState(
|
||||
}
|
||||
|
||||
sealed interface AepValidationError {
|
||||
data class TooLong(val count: Int, val max: Int) : AepValidationError
|
||||
data object Invalid : AepValidationError
|
||||
data object Incorrect : AepValidationError
|
||||
}
|
||||
|
||||
+6
-2
@@ -91,6 +91,7 @@ import org.signal.registration.screens.RegistrationScaffold
|
||||
import org.signal.registration.screens.TwoPaneRegistrationScaffold
|
||||
import org.signal.registration.screens.attachDebugLogHelper
|
||||
import org.signal.registration.screens.shared.AccountIdErrorText
|
||||
import org.signal.registration.screens.shared.AccountIdFormat
|
||||
import org.signal.registration.screens.shared.AccountIdVisualTransformation
|
||||
import org.signal.registration.screens.shared.accountIdTextStyle
|
||||
import org.signal.registration.test.TestTags
|
||||
@@ -590,8 +591,11 @@ private fun PhoneNumberInputFields(
|
||||
TextField(
|
||||
value = phoneNumberTextFieldValue,
|
||||
onValueChange = { newValue ->
|
||||
onEvent(PhoneNumberEntryScreenEvents.NationalNumberChanged(oldValue = phoneNumberTextFieldValue.text, newValue = newValue.text))
|
||||
phoneNumberTextFieldValue = newValue
|
||||
// An account ID that is already complete leaves the state untouched, so there is no re-sync to lean on: the
|
||||
// field has to turn away the extra characters itself.
|
||||
val accepted = if (isAccountId) newValue.copy(text = AccountIdFormat.normalizeAndTruncate(newValue.text)) else newValue
|
||||
onEvent(PhoneNumberEntryScreenEvents.NationalNumberChanged(oldValue = phoneNumberTextFieldValue.text, newValue = accepted.text))
|
||||
phoneNumberTextFieldValue = accepted
|
||||
},
|
||||
modifier = Modifier
|
||||
.weight(1f)
|
||||
|
||||
-3
@@ -7,9 +7,6 @@ package org.signal.registration.screens.shared
|
||||
|
||||
/** Why the entered account ID can't be submitted. Shown beneath the text field rather than in a dialog. */
|
||||
sealed interface AccountIdError {
|
||||
/** More than [AccountIdFormat.ACCOUNT_ID_LENGTH] characters were entered. */
|
||||
data class TooLong(val count: Int) : AccountIdError
|
||||
|
||||
/** The entered text contains characters that can't appear in an account ID. */
|
||||
data object Invalid : AccountIdError
|
||||
}
|
||||
|
||||
-1
@@ -32,7 +32,6 @@ internal fun accountIdTextStyle(): TextStyle {
|
||||
@Composable
|
||||
internal fun AccountIdErrorText(error: AccountIdError) {
|
||||
when (error) {
|
||||
is AccountIdError.TooLong -> Text(stringResource(R.string.AccountIdField__too_long, error.count, AccountIdFormat.ACCOUNT_ID_LENGTH))
|
||||
is AccountIdError.Invalid -> Text(stringResource(R.string.AccountIdField__invalid_account_id))
|
||||
}
|
||||
}
|
||||
|
||||
+11
-7
@@ -28,9 +28,13 @@ internal object AccountIdFormat {
|
||||
/** Strips the formatting a user may have typed or pasted, leaving the raw form the account ID is stored in. */
|
||||
fun normalize(input: String): String = input.replace(FORMATTING_CHARACTERS, "").lowercase()
|
||||
|
||||
/** [normalize]s [input] and cuts it down to [ACCOUNT_ID_LENGTH], since nothing longer than a complete ID can be entered. */
|
||||
fun normalizeAndTruncate(input: String): String = normalize(input).take(ACCOUNT_ID_LENGTH)
|
||||
|
||||
/**
|
||||
* Reads [input] as a raw account ID, or null if it doesn't read as one. Only text that couldn't plausibly be a phone
|
||||
* number qualifies: it has to be entirely hex, and either contain a letter or be longer than any E164 number.
|
||||
* Reads [input] as a raw account ID, truncated to [ACCOUNT_ID_LENGTH], or null if it doesn't read as one. Only text
|
||||
* that couldn't plausibly be a phone number qualifies: it has to be entirely hex, and either contain a letter or be
|
||||
* longer than any E164 number.
|
||||
*/
|
||||
fun asAccountIdOrNull(input: String): String? {
|
||||
val raw = normalize(input)
|
||||
@@ -40,7 +44,7 @@ internal object AccountIdFormat {
|
||||
}
|
||||
|
||||
return if (raw.any { !it.isDigit() } || raw.length > MAX_PHONE_NUMBER_DIGITS) {
|
||||
raw
|
||||
raw.take(ACCOUNT_ID_LENGTH)
|
||||
} else {
|
||||
null
|
||||
}
|
||||
@@ -51,10 +55,10 @@ internal object AccountIdFormat {
|
||||
|
||||
/** Why [accountId] can't be submitted, or null if there's nothing wrong with it. A too-short ID is not an error, since the user may be mid-entry. */
|
||||
fun validate(accountId: String): AccountIdError? {
|
||||
return when {
|
||||
accountId.length > ACCOUNT_ID_LENGTH -> AccountIdError.TooLong(accountId.length)
|
||||
!containsOnlyAccountIdCharacters(accountId) -> AccountIdError.Invalid
|
||||
else -> null
|
||||
return if (containsOnlyAccountIdCharacters(accountId)) {
|
||||
null
|
||||
} else {
|
||||
AccountIdError.Invalid
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
+1
-4
@@ -76,7 +76,6 @@ import org.signal.registration.screens.OnePaneRegistrationScaffold
|
||||
import org.signal.registration.screens.RegistrationScaffold
|
||||
import org.signal.registration.screens.TwoPaneRegistrationScaffold
|
||||
import org.signal.registration.screens.aepentry.AepInput
|
||||
import org.signal.registration.screens.aepentry.AepValidationError
|
||||
import org.signal.registration.screens.aepentry.AepVisualTransformation
|
||||
import org.signal.registration.screens.attachDebugLogHelper
|
||||
import org.signal.registration.screens.shared.AccountIdErrorText
|
||||
@@ -397,12 +396,10 @@ private fun RecoveryKeyTextField(
|
||||
}
|
||||
},
|
||||
supportingText = {
|
||||
val error = state.recoveryKey.error
|
||||
when {
|
||||
state.areCredentialsIncorrect && state.mode == SignalLoginCredentialEntryState.Mode.ConfirmSaved -> Text(stringResource(R.string.SignalLoginCredentialEntryScreen__that_doesnt_match_the_signal_login_you_were_shown))
|
||||
state.areCredentialsIncorrect -> Text(stringResource(R.string.SignalLoginCredentialEntryScreen__incorrect_account_id_or_recovery_key))
|
||||
error is AepValidationError.TooLong -> Text(stringResource(R.string.EnterAepScreen__too_long, error.count, error.max))
|
||||
error != null -> Text(stringResource(R.string.EnterAepScreen__invalid_recovery_key))
|
||||
state.recoveryKey.error != null -> Text(stringResource(R.string.EnterAepScreen__invalid_recovery_key))
|
||||
}
|
||||
},
|
||||
isError = isError,
|
||||
|
||||
+1
-1
@@ -17,7 +17,7 @@ object SignalLoginCredentialEntryScreenEventHandler {
|
||||
fun applyEvent(state: SignalLoginCredentialEntryState, event: SignalLoginCredentialEntryScreenEvents): SignalLoginCredentialEntryState {
|
||||
return when (event) {
|
||||
is SignalLoginCredentialEntryScreenEvents.AccountIdChanged -> {
|
||||
val accountId = AccountIdFormat.normalize(event.value)
|
||||
val accountId = AccountIdFormat.normalizeAndTruncate(event.value)
|
||||
state.copy(accountId = accountId, accountIdError = AccountIdFormat.validate(accountId), isAccountIdPrefilled = false, areCredentialsIncorrect = false)
|
||||
}
|
||||
|
||||
|
||||
+1
-1
@@ -124,7 +124,7 @@ class SignalLoginCredentialEntryViewModel(
|
||||
parentEventEmitter: (RegistrationFlowEvent) -> Unit,
|
||||
stateEmitter: (SignalLoginCredentialEntryState) -> Unit
|
||||
) {
|
||||
val accountId = AccountIdFormat.normalize(event.accountId).ifEmpty { state.accountId }
|
||||
val accountId = AccountIdFormat.normalizeAndTruncate(event.accountId).ifEmpty { state.accountId }
|
||||
val filledState = state.copy(
|
||||
accountId = accountId,
|
||||
accountIdError = AccountIdFormat.validate(accountId),
|
||||
|
||||
+1
-1
@@ -100,7 +100,7 @@ class SignalLoginManualSaveConfirmationViewModel(
|
||||
parentEventEmitter: (RegistrationFlowEvent) -> Unit,
|
||||
stateEmitter: (SignalLoginCredentialEntryState) -> Unit
|
||||
) {
|
||||
val accountId = AccountIdFormat.normalize(event.accountId).ifEmpty { state.accountId }
|
||||
val accountId = AccountIdFormat.normalizeAndTruncate(event.accountId).ifEmpty { state.accountId }
|
||||
val filledState = state.copy(
|
||||
accountId = accountId,
|
||||
accountIdError = AccountIdFormat.validate(accountId),
|
||||
|
||||
@@ -224,8 +224,6 @@
|
||||
<string name="EnterAepScreen__recovery_key">Recovery key</string>
|
||||
<!-- Link for users who don\'t have their recovery key -->
|
||||
<string name="EnterAepScreen__no_recovery_key">No recovery key?</string>
|
||||
<!-- Error text when key is too long -->
|
||||
<string name="EnterAepScreen__too_long">Too long (%1$d/%2$d)</string>
|
||||
<!-- Error text when key is invalid -->
|
||||
<string name="EnterAepScreen__invalid_recovery_key">Invalid recovery key</string>
|
||||
<!-- Error text when key is correct format but does not match account -->
|
||||
@@ -725,8 +723,6 @@
|
||||
<string name="ConfirmLoginSavedBottomSheet__show_login_info_again">Show login info again</string>
|
||||
|
||||
<!-- Errors shown beneath an account ID text field, wherever one is offered -->
|
||||
<!-- Error shown when too many characters were entered in an account ID field. First placeholder is how many were entered, second is the maximum. -->
|
||||
<string name="AccountIdField__too_long">Too long. %1$d/%2$d characters.</string>
|
||||
<!-- Error shown when the entered account ID contains characters that can\'t appear in one. -->
|
||||
<string name="AccountIdField__invalid_account_id">Invalid account ID</string>
|
||||
|
||||
|
||||
+4
-3
@@ -2250,7 +2250,7 @@ class PhoneNumberEntryViewModelTest {
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `NationalNumberChanged with an over-long account ID reports the error rather than silently refusing to submit`() = runTest {
|
||||
fun `NationalNumberChanged with an over-long account ID keeps only a complete ID`() = runTest {
|
||||
val initialState = PhoneNumberEntryState(isPhoneNumberlessRegistrationAvailable = true)
|
||||
|
||||
viewModel.applyEvent(
|
||||
@@ -2260,8 +2260,9 @@ class PhoneNumberEntryViewModelTest {
|
||||
stateEmitter
|
||||
)
|
||||
|
||||
assertThat(emittedStates.last().accountIdError).isEqualTo(AccountIdError.TooLong(34))
|
||||
assertThat(emittedStates.last().isNextEnabled).isFalse()
|
||||
assertThat(emittedStates.last().enteredAccountId).isEqualTo("a6b284822e3283d07f2391360a4c2b91")
|
||||
assertThat(emittedStates.last().accountIdError).isNull()
|
||||
assertThat(emittedStates.last().isNextEnabled).isTrue()
|
||||
}
|
||||
|
||||
@Test
|
||||
|
||||
+14
-5
@@ -17,7 +17,10 @@ import androidx.compose.ui.test.onNodeWithTag
|
||||
import androidx.compose.ui.test.onNodeWithText
|
||||
import androidx.compose.ui.test.performClick
|
||||
import androidx.compose.ui.test.performImeAction
|
||||
import androidx.compose.ui.test.performTextInput
|
||||
import androidx.test.core.app.ApplicationProvider
|
||||
import assertk.assertThat
|
||||
import assertk.assertions.hasLength
|
||||
import org.junit.Rule
|
||||
import org.junit.Test
|
||||
import org.junit.runner.RunWith
|
||||
@@ -26,7 +29,7 @@ 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.screens.shared.AccountIdError
|
||||
import org.signal.registration.screens.shared.AccountIdFormat
|
||||
import org.signal.registration.test.TestTags
|
||||
|
||||
/**
|
||||
@@ -253,19 +256,25 @@ class PhoneNumberScreenTest {
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `an over-long account ID says why it can't be submitted`() {
|
||||
fun `account ID entry turns away anything typed past a complete ID`() {
|
||||
// Given
|
||||
var emittedEvent: PhoneNumberEntryScreenEvents? = null
|
||||
|
||||
composeTestRule.setContent {
|
||||
SignalTheme {
|
||||
PhoneNumberScreen(
|
||||
state = accountIdState().copy(accountIdError = AccountIdError.TooLong(34)),
|
||||
onEvent = {}
|
||||
state = accountIdState(),
|
||||
onEvent = { emittedEvent = it }
|
||||
)
|
||||
}
|
||||
}
|
||||
|
||||
// When
|
||||
composeTestRule.onNodeWithTag(TestTags.PHONE_NUMBER_PHONE_FIELD).performTextInput("ff")
|
||||
|
||||
// Then
|
||||
composeTestRule.onNodeWithText(context.getString(R.string.AccountIdField__too_long, 34, 32)).assertExists()
|
||||
val newValue = (emittedEvent as PhoneNumberEntryScreenEvents.NationalNumberChanged).newValue
|
||||
assertThat(newValue).hasLength(AccountIdFormat.ACCOUNT_ID_LENGTH)
|
||||
}
|
||||
|
||||
@Test
|
||||
|
||||
+13
-2
@@ -22,6 +22,13 @@ class AccountIdFormatTest {
|
||||
assertThat(AccountIdFormat.normalize(" a6b28482 2e32 ")).isEqualTo("a6b284822e32")
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `normalizeAndTruncate cuts off anything past a complete ID`() {
|
||||
assertThat(AccountIdFormat.normalizeAndTruncate(FULL_ID + "ff")).isEqualTo(FULL_ID)
|
||||
assertThat(AccountIdFormat.normalizeAndTruncate("A6B28482-2E32-83D0-7F23-91360A4C2B91-FF")).isEqualTo(FULL_ID)
|
||||
assertThat(AccountIdFormat.normalizeAndTruncate("a6b28482")).isEqualTo("a6b28482")
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `text containing a letter reads as an account ID at any length`() {
|
||||
assertThat(AccountIdFormat.asAccountIdOrNull("a")).isEqualTo("a")
|
||||
@@ -34,6 +41,11 @@ class AccountIdFormatTest {
|
||||
assertThat(AccountIdFormat.asAccountIdOrNull("1".repeat(16))).isEqualTo("1".repeat(16))
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `text longer than a complete account ID reads as one, cut off at the maximum length`() {
|
||||
assertThat(AccountIdFormat.asAccountIdOrNull(FULL_ID + "ff")).isEqualTo(FULL_ID)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `a phone number never reads as an account ID`() {
|
||||
assertThat(AccountIdFormat.asAccountIdOrNull("+1 555 123 4567")).isNull()
|
||||
@@ -54,10 +66,9 @@ class AccountIdFormatTest {
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `validate only complains about length and alphabet, never about being mid-entry`() {
|
||||
fun `validate only complains about the alphabet, never about being mid-entry`() {
|
||||
assertThat(AccountIdFormat.validate("a6b28482")).isNull()
|
||||
assertThat(AccountIdFormat.validate(FULL_ID)).isNull()
|
||||
assertThat(AccountIdFormat.validate(FULL_ID + "ff")).isEqualTo(AccountIdError.TooLong(34))
|
||||
assertThat(AccountIdFormat.validate("a6b28482g")).isEqualTo(AccountIdError.Invalid)
|
||||
}
|
||||
|
||||
|
||||
+12
-3
@@ -162,11 +162,11 @@ class SignalLoginCredentialEntryViewModelTest {
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `AccountIdChanged reports an over-long ID as too long`() = runTest(testDispatcher) {
|
||||
fun `AccountIdChanged ignores anything typed past a complete ID`() = runTest(testDispatcher) {
|
||||
val state = applyAccountId(VALID_ACCOUNT_ID + "ab")
|
||||
|
||||
assertThat(state.accountIdError).isEqualTo(AccountIdError.TooLong(34))
|
||||
assertThat(state.isNextEnabled).isFalse()
|
||||
assertThat(state.accountId).isEqualTo(VALID_ACCOUNT_ID)
|
||||
assertThat(state.accountIdError).isNull()
|
||||
}
|
||||
|
||||
@Test
|
||||
@@ -177,6 +177,15 @@ class SignalLoginCredentialEntryViewModelTest {
|
||||
assertThat(emittedStates.last().recoveryKey.isValid).isTrue()
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `RecoveryKeyChanged ignores anything typed past a complete key`() = runTest(testDispatcher) {
|
||||
applyEvent(SignalLoginCredentialEntryState(), SignalLoginCredentialEntryScreenEvents.RecoveryKeyChanged(VALID_AEP + "abc"))
|
||||
|
||||
assertThat(emittedStates.last().recoveryKey.normalized).isEqualTo(VALID_AEP)
|
||||
assertThat(emittedStates.last().recoveryKey.error).isNull()
|
||||
assertThat(emittedStates.last().recoveryKey.isValid).isTrue()
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `editing either half clears a rejected login`() = runTest(testDispatcher) {
|
||||
val rejected = completeState().copy(areCredentialsIncorrect = true)
|
||||
|
||||
Reference in New Issue
Block a user