mirror of
https://github.com/bitwarden/android.git
synced 2026-10-22 02:53:19 -05:00
[PM-41319] fix: Autofill phone-number login credentials via fill-assist (#7250)
This commit is contained in:
1 parent
eda4fa42ba
commit
11077ba0e9
4 files changed
+130
-11
No files matched your search
+1
-1
@@ -154,7 +154,7 @@ class FilledDataBuilderImpl(
|
||||
) {
|
||||
val value = when (autofillView) {
|
||||
is AutofillView.Login.Email -> {
|
||||
if (!autofillCipher.username.isValidEmail()) {
|
||||
if (!autofillCipher.username.trim().isValidEmail()) {
|
||||
return@mapNotNull null
|
||||
}
|
||||
autofillCipher.username
|
||||
|
||||
+11
-5
@@ -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"
|
||||
@@ -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)
|
||||
@@ -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)
|
||||
|
||||
+57
@@ -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
|
||||
|
||||
@@ -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 {
|
||||
|
||||
+61
-5
@@ -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`() {
|
||||
@@ -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)
|
||||
@@ -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
|
||||
|
||||
Reference in new issue
Block a user