Skip to content

[PM-41319] fix: Autofill phone-number login credentials via fill-assist #7250

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
aj-rosado merged 1 commit into from
Aug 11, 2026
+130 −11
Merged

[PM-41319] fix: Autofill phone-number login credentials via fill-assist #7250

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion app/src/main/kotlin/com/x8bit/bitwarden/data/autofill/builder/FilledDataBuilderImpl.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -154,7 +154,7 @@ class FilledDataBuilderImpl(
) {
val value = when (autofillView) {
is AutofillView.Login.Email -> {
if (!autofillCipher.username.isValidEmail()) {
if (!autofillCipher.username.trim().isValidEmail()) {

@andrebispo5 andrebispo5 Aug 10, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

having the trim here will create the valid email but will pass along the spaces it removes to the autofill value.

@aj-rosado aj-rosado Aug 10, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was something extra ticket that I noticed while testing (my email for a field had an empty space so it was not being identified as an email by our function)

Did not want to modify the value autofilled to keep it as it is saved. If it is not intentional the user might notice and fix it.

return@mapNotNull null
}
autofillCipher.username
Expand Down
16 changes: 11 additions & 5 deletions app/src/main/kotlin/com/x8bit/bitwarden/data/autofill/util/FillAssistViewNodeExtensions.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import com.x8bit.bitwarden.data.autofill.model.FillAssistRules

private const val FIELD_KEY_USERNAME = "username"
private const val FIELD_KEY_EMAIL = "email"
private const val FIELD_KEY_PHONE = "phone"
private const val FIELD_KEY_PASSWORD = "password"
private const val FIELD_KEY_NEW_PASSWORD = "newPassword"
private const val FIELD_KEY_CARD_NUMBER = "cardNumber"
Expand Down Expand Up @@ -46,11 +47,16 @@ private fun AssistStructure.ViewNode.traverseForFillAssist(
.takeIf { it.isNotEmpty() }
?.let { matchingEntries ->
val data = toAutofillViewData(autofillId = id, website = website)
matchingEntries.firstNotNullOfOrNull { (key, _) ->
key.toAutofillViewForFieldKey(
data = data,
)
val candidateViews = matchingEntries.mapNotNull { (key, _) ->
key.toAutofillViewForFieldKey(data = data)
}
// A single field can legitimately match both the "email" and "phone"/"username"
// keys (e.g. a combined phone-or-email login field). Login.Username has no format
// gate and fills any stored value, while Login.Email rejects non-email values via
// isValidEmail(). Preferring Username when both match avoids rejecting a phone
// number credential on a field that would have accepted it.
candidateViews.firstOrNull { it is AutofillView.Login.Username }
?: candidateViews.firstOrNull()
}
}
val childViews = (0 until childCount)
Expand All @@ -64,7 +70,7 @@ private fun AssistStructure.ViewNode.traverseForFillAssist(
}

private fun String.toAutofillViewForFieldKey(data: AutofillView.Data): AutofillView? = when (this) {
FIELD_KEY_USERNAME -> AutofillView.Login.Username(data = data)
FIELD_KEY_USERNAME, FIELD_KEY_PHONE -> AutofillView.Login.Username(data = data)
FIELD_KEY_EMAIL -> AutofillView.Login.Email(data = data)
FIELD_KEY_PASSWORD, FIELD_KEY_NEW_PASSWORD -> AutofillView.Login.Password(data = data)
FIELD_KEY_CARD_NUMBER -> AutofillView.Card.Number(data = data)
Expand Down
57 changes: 57 additions & 0 deletions app/src/test/kotlin/com/x8bit/bitwarden/data/autofill/builder/FilledDataBuilderTest.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ import org.junit.jupiter.api.Assertions.assertEquals
import org.junit.jupiter.api.BeforeEach
import org.junit.jupiter.api.Test

@Suppress("LargeClass")
class FilledDataBuilderTest {
private lateinit var filledDataBuilder: FilledDataBuilder

Expand Down Expand Up @@ -238,6 +239,62 @@ class FilledDataBuilderTest {
verify(exactly = 0) { autofillViewEmail.buildFilledItemOrNull(any()) }
}

@Suppress("MaxLineLength")
@Test
fun `build should fill email field when cipher username email has leading or trailing whitespace`() =
runTest {
// Setup
val emailUsername = " user@example.com "
val autofillCipher = AutofillCipher.Login(
cipherId = null,
name = "Cipher One",
isTotpEnabled = false,
password = "Password",
username = emailUsername,
subtitle = "Subtitle",
website = URI,
)
val filledItemEmail: FilledItem = mockk()
val autofillViewEmail: AutofillView.Login.Email = mockk {
every { data } returns mockk { every { website } returns URI }
every { buildFilledItemOrNull(emailUsername) } returns filledItemEmail
}
val autofillPartition = AutofillPartition.Login(views = listOf(autofillViewEmail))
val ignoreAutofillIds: List<AutofillId> = mockk()
val autofillRequest = AutofillRequest.Fillable(
ignoreAutofillIds = ignoreAutofillIds,
inlinePresentationSpecs = emptyList(),
maxInlineSuggestionsCount = 0,
packageName = null,
partition = autofillPartition,
uri = URI,
)
val expected = FilledData(
filledPartitions = listOf(
FilledPartition(
autofillCipher = autofillCipher,
filledItems = listOf(filledItemEmail),
inlinePresentationSpec = null,
),
),
ignoreAutofillIds = ignoreAutofillIds,
originalPartition = autofillPartition,
uri = URI,
vaultItemInlinePresentationSpec = null,
isVaultLocked = false,
)
coEvery {
autofillCipherProvider.getLoginAutofillCiphers(uri = URI)
} returns listOf(autofillCipher)

// Test
val actual = filledDataBuilder.build(autofillRequest = autofillRequest)

// Verify
assertEquals(expected, actual)
verify(exactly = 1) { autofillViewEmail.buildFilledItemOrNull(emailUsername) }
}

@Test
fun `build should return no partitions and ignored AutofillIds when Login and no URI`() =
runTest {
Expand Down
66 changes: 61 additions & 5 deletions ...rc/test/kotlin/com/x8bit/bitwarden/data/autofill/util/FillAssistViewNodeExtensionsTest.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -105,6 +105,34 @@ class FillAssistViewNodeExtensionsTest {
assertEquals(listOf(AutofillView.Login.Email(data = data)), actual)
}

@Test
fun `buildFillAssistViews should return Login Username when htmlInfo matches phone clause`() {
val htmlInfo = createHtmlInfo()
val viewNode = createViewNode(htmlInfo = htmlInfo)
val assistStructure = createAssistStructure(viewNode)
val data = autofillData()
every {
viewNode.toAutofillViewData(
autofillId = autofillId,
website = null,
)
} returns data

val hostRule = FillAssistRules.HostRule(
category = "account-login",
fields = mapOf(
"phone" to listOf(selectorClause(tag = "input", id = "phone")),
),
)

val actual = assistStructure.buildFillAssistViews(
hostRules = listOf(hostRule),
urlBarWebsite = null,
)

assertEquals(listOf(AutofillView.Login.Username(data = data)), actual)
}

@Suppress("MaxLineLength")
@Test
fun `buildFillAssistViews should return Login Password when htmlInfo matches password clause`() {
Expand Down Expand Up @@ -226,10 +254,11 @@ class FillAssistViewNodeExtensionsTest {

@Suppress("MaxLineLength")
@Test
fun `buildFillAssistViews should pick first mapped key when multiple keys match the same node`() {
// Two field keys ("email" and "username") both match the same node. The implementation
// iterates in insertion order and selects the first key whose mapping is non-null.
// Since "email" is listed first, it wins and produces a Login.Email view.
fun `buildFillAssistViews should prefer Login Username over Login Email when both keys match the same node`() {
// Two field keys ("email" and "username") both match the same node, e.g. a combined
// phone-or-email login field. Login.Username has no format gate and fills any stored
// value, while Login.Email rejects non-email values, so Username is preferred even
// though "email" is listed first.
val htmlInfo = createHtmlInfo()
val viewNode = createViewNode(htmlInfo = htmlInfo)
val assistStructure = createAssistStructure(viewNode)
Expand All @@ -249,7 +278,34 @@ class FillAssistViewNodeExtensionsTest {
urlBarWebsite = null,
)

assertEquals(listOf(AutofillView.Login.Email(data = data)), actual)
assertEquals(listOf(AutofillView.Login.Username(data = data)), actual)
}

@Suppress("MaxLineLength")
@Test
fun `buildFillAssistViews should prefer Login Username over Login Email when phone and email keys match the same node`() {
// Mirrors a real fill-assist rule where a single combined phone-or-email field is
// declared under both the "email" and "phone" keys.
val htmlInfo = createHtmlInfo()
val viewNode = createViewNode(htmlInfo = htmlInfo)
val assistStructure = createAssistStructure(viewNode)
val data = autofillData()
every { viewNode.toAutofillViewData(autofillId = autofillId, website = null) } returns data

val hostRule = FillAssistRules.HostRule(
category = "account-login",
fields = linkedMapOf(
"email" to listOf(selectorClause(tag = "input", id = "shared")),
"phone" to listOf(selectorClause(tag = "input", id = "shared")),
),
)

val actual = assistStructure.buildFillAssistViews(
hostRules = listOf(hostRule),
urlBarWebsite = null,
)

assertEquals(listOf(AutofillView.Login.Username(data = data)), actual)
}

@Test
Expand Down
Loading