From f6f28f6a58b2385cdd5552dd1327da0ff854329f Mon Sep 17 00:00:00 2001 From: David Perez Date: Thu, 11 Jul 2024 12:16:35 -0500 Subject: [PATCH] Add logic to find missing username fields (#3440) --- .../data/autofill/model/AutofillView.kt | 7 ++ .../autofill/parser/AutofillParserImpl.kt | 52 ++++++++- .../data/autofill/util/ViewNodeExtensions.kt | 8 +- .../autofill/parser/AutofillParserTests.kt | 100 ++++++++++++++++++ .../autofill/util/ViewNodeExtensionsTest.kt | 7 +- 5 files changed, 169 insertions(+), 5 deletions(-) diff --git a/app/src/main/java/com/x8bit/bitwarden/data/autofill/model/AutofillView.kt b/app/src/main/java/com/x8bit/bitwarden/data/autofill/model/AutofillView.kt index d6be85315..4e0838a47 100644 --- a/app/src/main/java/com/x8bit/bitwarden/data/autofill/model/AutofillView.kt +++ b/app/src/main/java/com/x8bit/bitwarden/data/autofill/model/AutofillView.kt @@ -86,4 +86,11 @@ sealed class AutofillView { override val data: Data, ) : Login() } + + /** + * A view that is an input field but does not correspond to any known autofill field. + */ + data class Unused( + override val data: Data, + ) : AutofillView() } diff --git a/app/src/main/java/com/x8bit/bitwarden/data/autofill/parser/AutofillParserImpl.kt b/app/src/main/java/com/x8bit/bitwarden/data/autofill/parser/AutofillParserImpl.kt index 02f547ed6..164bf74df 100644 --- a/app/src/main/java/com/x8bit/bitwarden/data/autofill/parser/AutofillParserImpl.kt +++ b/app/src/main/java/com/x8bit/bitwarden/data/autofill/parser/AutofillParserImpl.kt @@ -105,6 +105,11 @@ class AutofillParserImpl( views = autofillViews.filterIsInstance(), ) } + + is AutofillView.Unused -> { + // The view is unfillable since the field is not meant to be used for autofill. + return AutofillRequest.Unfillable + } } // Flatten the ignorable autofill ids. val ignoreAutofillIds = traversalDataList @@ -139,7 +144,52 @@ class AutofillParserImpl( private fun AssistStructure.traverse(): List = (0 until windowNodeCount) .map { getWindowNodeAt(it) } - .mapNotNull { windowNode -> windowNode.rootViewNode?.traverse() } + .mapNotNull { windowNode -> + windowNode + .rootViewNode + ?.traverse() + ?.updateForMissingUsernameFields() + } + +/** + * This helper function updates the [ViewNodeTraversalData] if necessary for missing username + * fields that could have been missed. If the current `ViewNodeTraversalData` contains password + * fields but no username fields, we check to see if there are any unused fields directly above + * the password fields and we assume that those are the missing username fields. + */ +private fun ViewNodeTraversalData.updateForMissingUsernameFields(): ViewNodeTraversalData { + val passwordPositions = this.autofillViews.mapIndexedNotNull { index, autofillView -> + (autofillView as? AutofillView.Login.Password)?.let { index } + } + return if (passwordPositions.any() && + this.autofillViews.none { it is AutofillView.Login.Username } + ) { + val updatedAutofillViews = autofillViews.mapIndexed { index, autofillView -> + if (autofillView is AutofillView.Unused && passwordPositions.contains(index + 1)) { + AutofillView.Login.Username(data = autofillView.data) + } else { + autofillView + } + } + val previousUnusedIds = autofillViews + .filterIsInstance() + .map { it.data.autofillId } + .toSet() + val currentUnusedIds = updatedAutofillViews + .filterIsInstance() + .map { it.data.autofillId } + .toSet() + val unignoredAutofillIds = previousUnusedIds - currentUnusedIds + this.copy( + autofillViews = updatedAutofillViews, + ignoreAutofillIds = this.ignoreAutofillIds - unignoredAutofillIds, + ) + } else { + // We already have username fields available or there are no password fields, so no need + // to search for them. + this + } +} /** * Recursively traverse this [AssistStructure.ViewNode] and all of its descendants. Convert the diff --git a/app/src/main/java/com/x8bit/bitwarden/data/autofill/util/ViewNodeExtensions.kt b/app/src/main/java/com/x8bit/bitwarden/data/autofill/util/ViewNodeExtensions.kt index 2d39d623f..166c71a6f 100644 --- a/app/src/main/java/com/x8bit/bitwarden/data/autofill/util/ViewNodeExtensions.kt +++ b/app/src/main/java/com/x8bit/bitwarden/data/autofill/util/ViewNodeExtensions.kt @@ -112,7 +112,7 @@ private fun AssistStructure.ViewNode.buildAutofillView( autofillOptions: List, autofillViewData: AutofillView.Data, supportedHint: String?, -): AutofillView? = when { +): AutofillView = when { supportedHint == View.AUTOFILL_HINT_CREDIT_CARD_EXPIRATION_MONTH -> { val monthValue = this .autofillValue @@ -156,7 +156,11 @@ private fun AssistStructure.ViewNode.buildAutofillView( ) } - else -> null + else -> { + AutofillView.Unused( + data = autofillViewData, + ) + } } /** diff --git a/app/src/test/java/com/x8bit/bitwarden/data/autofill/parser/AutofillParserTests.kt b/app/src/test/java/com/x8bit/bitwarden/data/autofill/parser/AutofillParserTests.kt index 3adf68910..71372de71 100644 --- a/app/src/test/java/com/x8bit/bitwarden/data/autofill/parser/AutofillParserTests.kt +++ b/app/src/test/java/com/x8bit/bitwarden/data/autofill/parser/AutofillParserTests.kt @@ -28,6 +28,7 @@ import org.junit.jupiter.api.Assertions.assertEquals import org.junit.jupiter.api.BeforeEach import org.junit.jupiter.api.Test +@Suppress("LargeClass") class AutofillParserTests { private lateinit var parser: AutofillParser @@ -377,6 +378,105 @@ class AutofillParserTests { } } + @Suppress("MaxLineLength") + @Test + fun `parse should have Username and Password AutofillView when the Username field is not identifiable but directly above the Password field in the hierarchy`() { + // Setup + val hiddenUserNameViewNode: AssistStructure.ViewNode = mockk { + every { this@mockk.autofillHints } returns emptyArray() + every { this@mockk.autofillId } returns loginAutofillId + every { this@mockk.childCount } returns 0 + every { this@mockk.idPackage } returns ID_PACKAGE + every { this@mockk.website } returns WEBSITE + } + val passwordAutofillId = mockk() + val passwordViewNode: AssistStructure.ViewNode = mockk { + every { this@mockk.autofillHints } returns emptyArray() + every { this@mockk.autofillId } returns passwordAutofillId + every { this@mockk.childCount } returns 0 + every { this@mockk.idPackage } returns ID_PACKAGE + every { this@mockk.website } returns WEBSITE + } + val rootAutofillId = mockk() + val rootViewNode: AssistStructure.ViewNode = mockk { + every { this@mockk.autofillHints } returns emptyArray() + every { this@mockk.autofillId } returns rootAutofillId + every { this@mockk.childCount } returns 0 + every { this@mockk.idPackage } returns ID_PACKAGE + every { this@mockk.website } returns WEBSITE + every { this@mockk.childCount } returns 2 + every { this@mockk.getChildAt(0) } returns hiddenUserNameViewNode + every { this@mockk.getChildAt(1) } returns passwordViewNode + } + val windowNode: AssistStructure.WindowNode = mockk { + every { this@mockk.rootViewNode } returns rootViewNode + } + every { assistStructure.windowNodeCount } returns 1 + every { assistStructure.getWindowNodeAt(0) } returns windowNode + val unusedAutofillView: AutofillView.Unused = AutofillView.Unused( + data = AutofillView.Data( + autofillId = loginAutofillId, + autofillOptions = emptyList(), + autofillType = AUTOFILL_TYPE, + isFocused = true, + textValue = null, + ), + ) + val loginUsernameAutofillView: AutofillView.Login = AutofillView.Login.Username( + data = AutofillView.Data( + autofillId = loginAutofillId, + autofillOptions = emptyList(), + autofillType = AUTOFILL_TYPE, + isFocused = true, + textValue = null, + ), + ) + val loginPasswordAutofillView: AutofillView.Login = AutofillView.Login.Password( + data = AutofillView.Data( + autofillId = passwordAutofillId, + autofillOptions = emptyList(), + autofillType = AUTOFILL_TYPE, + isFocused = false, + textValue = null, + ), + ) + val autofillPartition = AutofillPartition.Login( + views = listOf(loginUsernameAutofillView, loginPasswordAutofillView), + ) + val expected = AutofillRequest.Fillable( + ignoreAutofillIds = listOf(rootAutofillId), + inlinePresentationSpecs = inlinePresentationSpecs, + maxInlineSuggestionsCount = MAX_INLINE_SUGGESTION_COUNT, + packageName = PACKAGE_NAME, + partition = autofillPartition, + uri = URI, + ) + every { rootViewNode.toAutofillView() } returns null + every { hiddenUserNameViewNode.toAutofillView() } returns unusedAutofillView + every { passwordViewNode.toAutofillView() } returns loginPasswordAutofillView + + // Test + val actual = parser.parse( + autofillAppInfo = autofillAppInfo, + fillRequest = fillRequest, + ) + + // Verify + assertEquals(expected, actual) + verify(exactly = 1) { + fillRequest.getInlinePresentationSpecs( + autofillAppInfo = autofillAppInfo, + isInlineAutofillEnabled = true, + ) + fillRequest.getMaxInlineSuggestionsCount( + autofillAppInfo = autofillAppInfo, + isInlineAutofillEnabled = true, + ) + any>().buildPackageNameOrNull(assistStructure) + any>().buildUriOrNull(PACKAGE_NAME) + } + } + @Test fun `parse should choose first focused AutofillView for partition when there are multiple`() { // Setup diff --git a/app/src/test/java/com/x8bit/bitwarden/data/autofill/util/ViewNodeExtensionsTest.kt b/app/src/test/java/com/x8bit/bitwarden/data/autofill/util/ViewNodeExtensionsTest.kt index 09299d0dd..4867d632f 100644 --- a/app/src/test/java/com/x8bit/bitwarden/data/autofill/util/ViewNodeExtensionsTest.kt +++ b/app/src/test/java/com/x8bit/bitwarden/data/autofill/util/ViewNodeExtensionsTest.kt @@ -230,15 +230,18 @@ class ViewNodeExtensionsTest { @Suppress("MaxLineLength") @Test - fun `toAutofillView should return null when hint is not supported, is an inputField, and isn't a username or password`() { + fun `toAutofillView should return only unused field when hint is not supported, is an inputField, and isn't a username or password`() { // Setup setupUnsupportedInputFieldViewNode() + val expected = AutofillView.Unused( + data = autofillViewData, + ) // Test val actual = viewNode.toAutofillView() // Verify - assertNull(actual) + assertEquals(expected, actual) } @Test