Don't let a prefilled account ID suppress the password manager prompt.

This commit is contained in:
Greyson Parrelli
2026-09-09 16:37:40 -04:00
committed by Cody Henthorne
parent d079b46aa0
commit 116bf6cd09
5 changed files with 89 additions and 8 deletions
@@ -252,8 +252,8 @@ private fun CredentialTextFields(
/**
* Builds a modifier that prompts the password manager the first time either credential field is tapped, so a saved
* login can fill both halves at once. Only fires while the fields are still empty, and only once per screen so a
* dismissed prompt doesn't keep coming back.
* login can fill both halves at once. Only fires while the user hasn't filled anything in themselves, and only once
* per screen so a dismissed prompt doesn't keep coming back.
*/
@Composable
private fun passwordManagerPromptOnFocus(
@@ -265,8 +265,7 @@ private fun passwordManagerPromptOnFocus(
var hasPrompted by rememberSaveable { mutableStateOf(false) }
return Modifier.onFocusChanged { focusState ->
val fieldsAreEmpty = state.accountId.isEmpty() && state.recoveryKey.enteredText.isEmpty()
if (focusState.isFocused && !hasPrompted && fieldsAreEmpty && SignalCredentialManager.isSupported(context)) {
if (focusState.isFocused && !hasPrompted && state.canPromptPasswordManager && SignalCredentialManager.isSupported(context)) {
hasPrompted = true
coroutineScope.launch {
val credential = SignalCredentialManager.getCredential(context)
@@ -20,6 +20,8 @@ import org.signal.registration.screens.shared.AccountIdFormat
data class SignalLoginCredentialEntryState(
val accountId: String = "",
val accountIdError: AccountIdError? = null,
/** Whether [accountId] was handed to the screen by the flow rather than typed by the user, and so isn't user input. */
val isAccountIdPrefilled: Boolean = false,
val recoveryKey: AepInput = AepInput(),
/** Whether the recovery key is spelled out rather than masked like a password. */
val isRecoveryKeyRevealed: Boolean = false,
@@ -38,7 +40,11 @@ data class SignalLoginCredentialEntryState(
!areCredentialsIncorrect &&
!isLoggingIn
override fun toString(): String = "SignalLoginCredentialEntryState(accountId=${accountId.censor()}, accountIdError=$accountIdError, recoveryKey=$recoveryKey, isRecoveryKeyRevealed=$isRecoveryKeyRevealed, areCredentialsIncorrect=$areCredentialsIncorrect, isLoggingIn=$isLoggingIn, loginError=$loginError)"
/** Allow prompting if it's empty or came pre-filled */
val canPromptPasswordManager: Boolean
get() = recoveryKey.enteredText.isEmpty() && (accountId.isEmpty() || isAccountIdPrefilled)
override fun toString(): String = "SignalLoginCredentialEntryState(accountId=${accountId.censor()}, accountIdError=$accountIdError, isAccountIdPrefilled=$isAccountIdPrefilled, recoveryKey=$recoveryKey, isRecoveryKeyRevealed=$isRecoveryKeyRevealed, areCredentialsIncorrect=$areCredentialsIncorrect, isLoggingIn=$isLoggingIn, loginError=$loginError)"
}
/** A login failure that the text fields can't express, so it gets a dialog instead. */
@@ -49,7 +49,12 @@ class SignalLoginCredentialEntryViewModel(
private val TAG = Log.tag(SignalLoginCredentialEntryViewModel::class)
}
private val _state = MutableStateFlow(SignalLoginCredentialEntryState(accountId = prefilledAccountId ?: ""))
private val _state = MutableStateFlow(
SignalLoginCredentialEntryState(
accountId = prefilledAccountId ?: "",
isAccountIdPrefilled = !prefilledAccountId.isNullOrEmpty()
)
)
val state: StateFlow<SignalLoginCredentialEntryState> = _state.asStateFlow()
private val _actions = Channel<SignalLoginCredentialEntryScreenActions>(Channel.BUFFERED)
@@ -79,7 +84,7 @@ class SignalLoginCredentialEntryViewModel(
is SignalLoginCredentialEntryScreenEvents.AccountIdChanged -> {
val accountId = AccountIdFormat.normalize(event.value)
stateEmitter(state.copy(accountId = accountId, accountIdError = AccountIdFormat.validate(accountId), areCredentialsIncorrect = false))
stateEmitter(state.copy(accountId = accountId, accountIdError = AccountIdFormat.validate(accountId), isAccountIdPrefilled = false, areCredentialsIncorrect = false))
}
is SignalLoginCredentialEntryScreenEvents.RecoveryKeyChanged -> {
@@ -126,10 +131,11 @@ class SignalLoginCredentialEntryViewModel(
parentEventEmitter: (RegistrationFlowEvent) -> Unit,
stateEmitter: (SignalLoginCredentialEntryState) -> Unit
) {
val accountId = AccountIdFormat.normalize(event.accountId)
val accountId = AccountIdFormat.normalize(event.accountId).ifEmpty { state.accountId }
val filledState = state.copy(
accountId = accountId,
accountIdError = AccountIdFormat.validate(accountId),
isAccountIdPrefilled = false,
recoveryKey = AepInput.from(event.recoveryKey),
areCredentialsIncorrect = false
)
@@ -83,6 +83,28 @@ class SignalLoginCredentialEntryScreenTest {
coVerify(exactly = 0) { SignalCredentialManager.getCredential(any()) }
}
@Test
fun `when a field is tapped with only a prefilled account ID, the password manager is still prompted`() {
stubPasswordManager()
setContent(SignalLoginCredentialEntryState(accountId = VALID_ACCOUNT_ID, isAccountIdPrefilled = true))
composeTestRule.onNodeWithTag(TestTags.SIGNAL_LOGIN_CREDENTIAL_ACCOUNT_ID_FIELD).performClick()
composeTestRule.waitForIdle()
assertThat(events).contains(SignalLoginCredentialEntryScreenEvents.PasswordManagerCredentialSelected(accountId = VALID_ACCOUNT_ID, recoveryKey = VALID_RECOVERY_KEY))
}
@Test
fun `when a field is tapped with a user-typed account ID, the password manager is not prompted`() {
stubPasswordManager()
setContent(SignalLoginCredentialEntryState(accountId = VALID_ACCOUNT_ID))
composeTestRule.onNodeWithTag(TestTags.SIGNAL_LOGIN_CREDENTIAL_ACCOUNT_ID_FIELD).performClick()
composeTestRule.waitForIdle()
coVerify(exactly = 0) { SignalCredentialManager.getCredential(any()) }
}
@Test
fun `the account ID field is tagged for autofill as the username`() {
setContent(SignalLoginCredentialEntryState())
@@ -95,6 +95,41 @@ class SignalLoginCredentialEntryViewModelTest {
assertThat(prefilled.state.value.accountId).isEqualTo(VALID_ACCOUNT_ID)
}
@Test
fun `a prefilled account ID does not count as user input, so the password manager can still be offered`() = runTest(testDispatcher) {
val prefilled = SignalLoginCredentialEntryViewModel(repository = mockRepository, parentEventEmitter = parentEventEmitter, prefilledAccountId = VALID_ACCOUNT_ID)
assertThat(prefilled.state.value.isAccountIdPrefilled).isTrue()
assertThat(prefilled.state.value.canPromptPasswordManager).isTrue()
}
@Test
fun `without a prefilled account ID the screen starts empty and the password manager can be offered`() = runTest(testDispatcher) {
assertThat(viewModel.state.value.isAccountIdPrefilled).isFalse()
assertThat(viewModel.state.value.canPromptPasswordManager).isTrue()
}
@Test
fun `an account ID the user typed themselves stops the password manager from being offered`() = runTest(testDispatcher) {
applyEvent(
SignalLoginCredentialEntryState(accountId = VALID_ACCOUNT_ID, isAccountIdPrefilled = true),
SignalLoginCredentialEntryScreenEvents.AccountIdChanged("a6b28482")
)
assertThat(emittedStates.last().isAccountIdPrefilled).isFalse()
assertThat(emittedStates.last().canPromptPasswordManager).isFalse()
}
@Test
fun `an entered recovery key stops the password manager from being offered`() = runTest(testDispatcher) {
applyEvent(
SignalLoginCredentialEntryState(accountId = VALID_ACCOUNT_ID, isAccountIdPrefilled = true),
SignalLoginCredentialEntryScreenEvents.RecoveryKeyChanged("uy38")
)
assertThat(emittedStates.last().canPromptPasswordManager).isFalse()
}
@Test
fun `BackClicked navigates back`() = runTest(testDispatcher) {
applyEvent(SignalLoginCredentialEntryState(), SignalLoginCredentialEntryScreenEvents.BackClicked)
@@ -191,6 +226,19 @@ class SignalLoginCredentialEntryViewModelTest {
coVerify(exactly = 0) { mockRepository.reRegisterAccountWithoutPhoneNumber(any(), any(), any(), any(), any()) }
}
@Test
fun `PasswordManagerCredentialSelected keeps a prefilled account ID when the credential has no username`() = runTest(testDispatcher) {
stubSuccessfulLogin(AccountEntropyPool(VALID_AEP))
applyEvent(
SignalLoginCredentialEntryState(accountId = VALID_ACCOUNT_ID, isAccountIdPrefilled = true),
SignalLoginCredentialEntryScreenEvents.PasswordManagerCredentialSelected(accountId = "", recoveryKey = VALID_AEP)
)
assertThat(emittedStates.first().accountId).isEqualTo(VALID_ACCOUNT_ID)
coVerify { mockRepository.reRegisterAccountWithoutPhoneNumber(any(), any(), any(), any(), any()) }
}
@Test
fun `PasswordManagerCredentialSelected clears a previously rejected login`() = runTest(testDispatcher) {
stubSuccessfulLogin(AccountEntropyPool(VALID_AEP))