From 0a57b9bcb5a2f80eaa55d076b7f0c42eed09f700 Mon Sep 17 00:00:00 2001 From: aj-rosado <109146700+aj-rosado@users.noreply.github.com> Date: Mon, 20 Jul 2026 18:30:36 +0100 Subject: [PATCH] [PM-39564] feat: Fill Assist support email field key for email-based login forms (#7107) Co-authored-by: claude[bot] <41898282+claude[bot]@users.noreply.github.com> Co-authored-by: aj-rosado --- .../autofill/builder/FilledDataBuilderImpl.kt | 8 ++ .../data/autofill/model/AutofillView.kt | 8 ++ .../autofill/parser/AutofillParserImpl.kt | 1 + .../util/AutofillPartitionExtensions.kt | 4 +- .../autofill/util/AutofillViewExtensions.kt | 1 + .../util/FillAssistViewNodeExtensions.kt | 2 + .../autofill/builder/FilledDataBuilderTest.kt | 102 ++++++++++++++++++ .../util/AutofillPartitionExtensionsTest.kt | 36 +++++++ .../util/FillAssistViewNodeExtensionsTest.kt | 37 ++++++- 9 files changed, 193 insertions(+), 6 deletions(-) diff --git a/app/src/main/kotlin/com/x8bit/bitwarden/data/autofill/builder/FilledDataBuilderImpl.kt b/app/src/main/kotlin/com/x8bit/bitwarden/data/autofill/builder/FilledDataBuilderImpl.kt index 710288c925..551f2b9063 100644 --- a/app/src/main/kotlin/com/x8bit/bitwarden/data/autofill/builder/FilledDataBuilderImpl.kt +++ b/app/src/main/kotlin/com/x8bit/bitwarden/data/autofill/builder/FilledDataBuilderImpl.kt @@ -1,6 +1,7 @@ package com.x8bit.bitwarden.data.autofill.builder import android.widget.inline.InlinePresentationSpec +import com.bitwarden.ui.platform.base.util.isValidEmail import com.x8bit.bitwarden.data.autofill.model.AutofillCipher import com.x8bit.bitwarden.data.autofill.model.AutofillPartition import com.x8bit.bitwarden.data.autofill.model.AutofillRequest @@ -152,6 +153,13 @@ class FilledDataBuilderImpl( buildUri(packageName.orEmpty(), "androidapp") == autofillCipher.website ) { val value = when (autofillView) { + is AutofillView.Login.Email -> { + if (!autofillCipher.username.isValidEmail()) { + return@mapNotNull null + } + autofillCipher.username + } + is AutofillView.Login.Username -> autofillCipher.username is AutofillView.Login.Password -> autofillCipher.password } diff --git a/app/src/main/kotlin/com/x8bit/bitwarden/data/autofill/model/AutofillView.kt b/app/src/main/kotlin/com/x8bit/bitwarden/data/autofill/model/AutofillView.kt index fe7d287215..701b84d9a7 100644 --- a/app/src/main/kotlin/com/x8bit/bitwarden/data/autofill/model/AutofillView.kt +++ b/app/src/main/kotlin/com/x8bit/bitwarden/data/autofill/model/AutofillView.kt @@ -118,6 +118,14 @@ sealed class AutofillView { data class Username( override val data: Data, ) : Login() + + /** + * The email [AutofillView] for the [Login] data partition. Filled only when the cipher's + * username is a valid email address. + */ + data class Email( + override val data: Data, + ) : Login() } /** diff --git a/app/src/main/kotlin/com/x8bit/bitwarden/data/autofill/parser/AutofillParserImpl.kt b/app/src/main/kotlin/com/x8bit/bitwarden/data/autofill/parser/AutofillParserImpl.kt index 4540960242..5605f3b0d3 100644 --- a/app/src/main/kotlin/com/x8bit/bitwarden/data/autofill/parser/AutofillParserImpl.kt +++ b/app/src/main/kotlin/com/x8bit/bitwarden/data/autofill/parser/AutofillParserImpl.kt @@ -445,6 +445,7 @@ private fun AutofillView.updateWebsiteIfNecessary(website: String?): AutofillVie is AutofillView.Card.ExpirationYear -> this.copy(data = this.data.copy(website = site)) is AutofillView.Card.Number -> this.copy(data = this.data.copy(website = site)) is AutofillView.Card.SecurityCode -> this.copy(data = this.data.copy(website = site)) + is AutofillView.Login.Email -> this.copy(data = this.data.copy(website = site)) is AutofillView.Login.Password -> this.copy(data = this.data.copy(website = site)) is AutofillView.Login.Username -> this.copy(data = this.data.copy(website = site)) is AutofillView.Unused -> this.copy(data = this.data.copy(website = site)) diff --git a/app/src/main/kotlin/com/x8bit/bitwarden/data/autofill/util/AutofillPartitionExtensions.kt b/app/src/main/kotlin/com/x8bit/bitwarden/data/autofill/util/AutofillPartitionExtensions.kt index 749d670259..4effcbf33f 100644 --- a/app/src/main/kotlin/com/x8bit/bitwarden/data/autofill/util/AutofillPartitionExtensions.kt +++ b/app/src/main/kotlin/com/x8bit/bitwarden/data/autofill/util/AutofillPartitionExtensions.kt @@ -67,7 +67,9 @@ val AutofillPartition.Login.passwordSaveValue: String? */ val AutofillPartition.Login.usernameSaveValue: String? get() = this - .extractNonNullTextValueOrNull { it is AutofillView.Login.Username } + .extractNonNullTextValueOrNull { + it is AutofillView.Login.Username || it is AutofillView.Login.Email + } /** * Search [AutofillPartition.views] for an [AutofillView] that matches [condition] and has a diff --git a/app/src/main/kotlin/com/x8bit/bitwarden/data/autofill/util/AutofillViewExtensions.kt b/app/src/main/kotlin/com/x8bit/bitwarden/data/autofill/util/AutofillViewExtensions.kt index 8271643d9a..d4751d3a6e 100644 --- a/app/src/main/kotlin/com/x8bit/bitwarden/data/autofill/util/AutofillViewExtensions.kt +++ b/app/src/main/kotlin/com/x8bit/bitwarden/data/autofill/util/AutofillViewExtensions.kt @@ -90,6 +90,7 @@ private fun AutofillView.buildListAutofillValueOrNull( is AutofillView.Card.ExpirationDate, is AutofillView.Card.Number, is AutofillView.Card.SecurityCode, + is AutofillView.Login.Email, is AutofillView.Login.Password, is AutofillView.Login.Username, is AutofillView.Unused, diff --git a/app/src/main/kotlin/com/x8bit/bitwarden/data/autofill/util/FillAssistViewNodeExtensions.kt b/app/src/main/kotlin/com/x8bit/bitwarden/data/autofill/util/FillAssistViewNodeExtensions.kt index d35d058a0f..fee821bbb8 100644 --- a/app/src/main/kotlin/com/x8bit/bitwarden/data/autofill/util/FillAssistViewNodeExtensions.kt +++ b/app/src/main/kotlin/com/x8bit/bitwarden/data/autofill/util/FillAssistViewNodeExtensions.kt @@ -5,6 +5,7 @@ import com.x8bit.bitwarden.data.autofill.model.AutofillView 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_PASSWORD = "password" private const val FIELD_KEY_NEW_PASSWORD = "newPassword" private const val FIELD_KEY_CARD_NUMBER = "cardNumber" @@ -64,6 +65,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_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) FIELD_KEY_CARDHOLDER_NAME -> AutofillView.Card.CardholderName(data = data) diff --git a/app/src/test/kotlin/com/x8bit/bitwarden/data/autofill/builder/FilledDataBuilderTest.kt b/app/src/test/kotlin/com/x8bit/bitwarden/data/autofill/builder/FilledDataBuilderTest.kt index 4b5813ea9a..386a250cf1 100644 --- a/app/src/test/kotlin/com/x8bit/bitwarden/data/autofill/builder/FilledDataBuilderTest.kt +++ b/app/src/test/kotlin/com/x8bit/bitwarden/data/autofill/builder/FilledDataBuilderTest.kt @@ -136,6 +136,108 @@ class FilledDataBuilderTest { } } + @Test + fun `build should fill email field when cipher username is a valid email address`() = + 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 = 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 not fill email field when cipher username is not a valid email address`() = + runTest { + // Setup + val nonEmailUsername = "johndoe" + val autofillCipher = AutofillCipher.Login( + cipherId = null, + name = "Cipher One", + isTotpEnabled = false, + password = "Password", + username = nonEmailUsername, + subtitle = "Subtitle", + website = URI, + ) + val autofillViewEmail: AutofillView.Login.Email = mockk { + every { data } returns mockk { every { website } returns URI } + } + val autofillPartition = AutofillPartition.Login(views = listOf(autofillViewEmail)) + val ignoreAutofillIds: List = mockk() + val autofillRequest = AutofillRequest.Fillable( + ignoreAutofillIds = ignoreAutofillIds, + inlinePresentationSpecs = emptyList(), + maxInlineSuggestionsCount = 0, + packageName = null, + partition = autofillPartition, + uri = URI, + ) + + val expected = FilledData( + filledPartitions = emptyList(), + 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 = 0) { autofillViewEmail.buildFilledItemOrNull(any()) } + } + @Test fun `build should return no partitions and ignored AutofillIds when Login and no URI`() = runTest { diff --git a/app/src/test/kotlin/com/x8bit/bitwarden/data/autofill/util/AutofillPartitionExtensionsTest.kt b/app/src/test/kotlin/com/x8bit/bitwarden/data/autofill/util/AutofillPartitionExtensionsTest.kt index 18ed088c37..b0734d692e 100644 --- a/app/src/test/kotlin/com/x8bit/bitwarden/data/autofill/util/AutofillPartitionExtensionsTest.kt +++ b/app/src/test/kotlin/com/x8bit/bitwarden/data/autofill/util/AutofillPartitionExtensionsTest.kt @@ -482,6 +482,42 @@ class AutofillPartitionExtensionsTest { // Verify assertEquals(TEXT_VALUE, actual) } + + @Test + fun `usernameSaveValue should return text value when has email view with textValue`() { + // Setup + val autofillPartition = AutofillPartition.Login( + views = listOf( + AutofillView.Login.Email( + data = autofillDataValidText, + ), + ), + ) + + // Test + val actual = autofillPartition.usernameSaveValue + + // Verify + assertEquals(TEXT_VALUE, actual) + } + + @Test + fun `usernameSaveValue should return null when has email view but no textValue`() { + // Setup + val autofillPartition = AutofillPartition.Login( + views = listOf( + AutofillView.Login.Email( + data = autofillDataEmptyText, + ), + ), + ) + + // Test + val actual = autofillPartition.usernameSaveValue + + // Verify + assertNull(actual) + } //endregion Login tests } diff --git a/app/src/test/kotlin/com/x8bit/bitwarden/data/autofill/util/FillAssistViewNodeExtensionsTest.kt b/app/src/test/kotlin/com/x8bit/bitwarden/data/autofill/util/FillAssistViewNodeExtensionsTest.kt index ed4a08c74c..88e747d931 100644 --- a/app/src/test/kotlin/com/x8bit/bitwarden/data/autofill/util/FillAssistViewNodeExtensionsTest.kt +++ b/app/src/test/kotlin/com/x8bit/bitwarden/data/autofill/util/FillAssistViewNodeExtensionsTest.kt @@ -77,6 +77,34 @@ class FillAssistViewNodeExtensionsTest { assertEquals(listOf(AutofillView.Login.Username(data = data)), actual) } + @Test + fun `buildFillAssistViews should return Login Email when htmlInfo matches email 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( + "email" to listOf(selectorClause(tag = "input", id = "email")), + ), + ) + + val actual = assistStructure.buildFillAssistViews( + hostRules = listOf(hostRule), + urlBarWebsite = null, + ) + + assertEquals(listOf(AutofillView.Login.Email(data = data)), actual) + } + @Suppress("MaxLineLength") @Test fun `buildFillAssistViews should return Login Password when htmlInfo matches password clause`() { @@ -199,10 +227,9 @@ class FillAssistViewNodeExtensionsTest { @Suppress("MaxLineLength") @Test fun `buildFillAssistViews should pick first mapped key when multiple keys match the same node`() { - // Two field keys ("email" - unknown - 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 unknown, "username" wins; this also demonstrates that an - // earlier known key wins over a later one. + // 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. val htmlInfo = createHtmlInfo() val viewNode = createViewNode(htmlInfo = htmlInfo) val assistStructure = createAssistStructure(viewNode) @@ -222,7 +249,7 @@ class FillAssistViewNodeExtensionsTest { urlBarWebsite = null, ) - assertEquals(listOf(AutofillView.Login.Username(data = data)), actual) + assertEquals(listOf(AutofillView.Login.Email(data = data)), actual) } @Test