mirrors/bitwarden-android
1
0
Fork 0
mirror of https://github.com/bitwarden/android.git synced 2025-02-16 11:59:57 +03:00
Code Issues Projects Releases Packages Wiki Activity

PM-10559: Add logic to re-evaluate invalid password fields for Autofill (#3668)

Browse source
This commit is contained in:
David Perez 2024-08-02 16:52:39 -05:00 • committed by GitHub
parent aae7a6e895
commit bbe50ae0ff
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
10 changed files with 181 additions and 33 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

2
app/src/main/java/com/x8bit/bitwarden/data/autofill/model/AutofillView.kt
View file

@ -15,6 +15,7 @@ sealed class AutofillView {
* @param autofillType The autofill field type. (ex: View.AUTOFILL_TYPE_TEXT)
* @param isFocused Whether the view is currently focused.
* @param textValue A text value that represents the input present in the field.
* @param hasPasswordTerms Indicates that the field includes password terms.
*/
data class Data(
val autofillId: AutofillId,
@ -22,6 +23,7 @@ sealed class AutofillView {
val autofillType: Int,
val isFocused: Boolean,
val textValue: String?,
val hasPasswordTerms: Boolean,
)
/**

58
app/src/main/java/com/x8bit/bitwarden/data/autofill/parser/AutofillParserImpl.kt
View file

@ -161,9 +161,29 @@ private fun AssistStructure.traverse(): List<ViewNodeTraversalData> =
windowNode
.rootViewNode
?.traverse()
?.updateForMissingPasswordFields()
?.updateForMissingUsernameFields()
}
/**
* This helper function updates the [ViewNodeTraversalData] if necessary for missing password
* fields that were marked invalid because they contained a specific `hint` or `idEntry`. If the
* current `ViewNodeTraversalData` contains at least one password fields, we do not add any fields.
*/
private fun ViewNodeTraversalData.updateForMissingPasswordFields(): ViewNodeTraversalData =
if (this.autofillViews.none { it is AutofillView.Login.Password }) {
this.copyAndMapAutofillViews { _, autofillView ->
if (autofillView is AutofillView.Unused && autofillView.data.hasPasswordTerms) {
AutofillView.Login.Password(data = autofillView.data)
} else {
autofillView
}
}
} else {
// We already have password fields available, so no need to add more.
this
}
/**
* This helper function updates the [ViewNodeTraversalData] if necessary for missing username
* fields that could have been missed. If the current `ViewNodeTraversalData` contains password
@ -177,26 +197,13 @@ private fun ViewNodeTraversalData.updateForMissingUsernameFields(): ViewNodeTrav
return if (passwordPositions.any() &&
this.autofillViews.none { it is AutofillView.Login.Username }
) {
val updatedAutofillViews = autofillViews.mapIndexed { index, autofillView ->
this.copyAndMapAutofillViews { index, autofillView ->
if (autofillView is AutofillView.Unused && passwordPositions.contains(index + 1)) {
AutofillView.Login.Username(data = autofillView.data)
} else {
autofillView
}
}
val previousUnusedIds = autofillViews
.filterIsInstance<AutofillView.Unused>()
.map { it.data.autofillId }
.toSet()
val currentUnusedIds = updatedAutofillViews
.filterIsInstance<AutofillView.Unused>()
.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.
@ -204,6 +211,29 @@ private fun ViewNodeTraversalData.updateForMissingUsernameFields(): ViewNodeTrav
}
}
/**
* This helper function loops through all the [ViewNodeTraversalData.autofillViews] and returns the
* fully updated `ViewNodeTraversalData`.
*/
private fun ViewNodeTraversalData.copyAndMapAutofillViews(
mapper: (index: Int, autofillView: AutofillView) -> AutofillView,
): ViewNodeTraversalData {
val updatedAutofillViews = autofillViews.mapIndexed(mapper)
val previousUnusedIds = autofillViews
.filterIsInstance<AutofillView.Unused>()
.map { it.data.autofillId }
.toSet()
val currentUnusedIds = updatedAutofillViews
.filterIsInstance<AutofillView.Unused>()
.map { it.data.autofillId }
.toSet()
val unignoredAutofillIds = previousUnusedIds - currentUnusedIds
return this.copy(
autofillViews = updatedAutofillViews,
ignoreAutofillIds = this.ignoreAutofillIds - unignoredAutofillIds,
)
}
/**
* Recursively traverse this [AssistStructure.ViewNode] and all of its descendants. Convert the
* data into [ViewNodeTraversalData].

10
app/src/main/java/com/x8bit/bitwarden/data/autofill/util/ViewNodeExtensions.kt
View file

@ -94,6 +94,7 @@ fun AssistStructure.ViewNode.toAutofillView(): AutofillView? =
autofillType = this.autofillType,
isFocused = this.isFocused,
textValue = this.autofillValue?.extractTextValue(),
hasPasswordTerms = this.hasPasswordTerms(),
)
buildAutofillView(
autofillOptions = autofillOptions,
@ -171,8 +172,6 @@ fun AssistStructure.ViewNode.isPasswordField(
): Boolean {
if (supportedHint == View.AUTOFILL_HINT_PASSWORD) return true
if (this.hint?.containsAnyTerms(SUPPORTED_RAW_PASSWORD_HINTS) == true) return true
val isInvalidField = this.idEntry?.containsAnyTerms(IGNORED_RAW_HINTS) == true ||
this.hint?.containsAnyTerms(IGNORED_RAW_HINTS) == true
val isUsernameField = this.isUsernameField(supportedHint)
@ -183,6 +182,13 @@ fun AssistStructure.ViewNode.isPasswordField(
.isPasswordField()
}
/**
* Check whether this [AssistStructure.ViewNode] includes any password specific terms.
*/
fun AssistStructure.ViewNode.hasPasswordTerms(): Boolean =
this.idEntry?.containsAnyTerms(SUPPORTED_RAW_PASSWORD_HINTS) == true ||
this.hint?.containsAnyTerms(SUPPORTED_RAW_PASSWORD_HINTS) == true
/**
* Check whether this [AssistStructure.ViewNode] represents a username field.
*/

2
app/src/test/java/com/x8bit/bitwarden/data/autofill/builder/FillResponseBuilderTest.kt
View file

@ -144,6 +144,7 @@ class FillResponseBuilderTest {
autofillType = AUTOFILL_TYPE,
isFocused = true,
textValue = null,
hasPasswordTerms = false,
),
),
),
@ -244,6 +245,7 @@ class FillResponseBuilderTest {
autofillType = AUTOFILL_TYPE,
isFocused = true,
textValue = null,
hasPasswordTerms = false,
),
),
),

2
app/src/test/java/com/x8bit/bitwarden/data/autofill/builder/SaveInfoBuilderTest.kt
View file

@ -35,6 +35,7 @@ class SaveInfoBuilderTest {
autofillType = View.AUTOFILL_TYPE_TEXT,
isFocused = true,
textValue = null,
hasPasswordTerms = false,
)
private val autofillIdValid: AutofillId = mockk()
private val autofillViewDataValid = AutofillView.Data(
@ -43,6 +44,7 @@ class SaveInfoBuilderTest {
autofillType = View.AUTOFILL_TYPE_TEXT,
isFocused = true,
textValue = null,
hasPasswordTerms = false,
)
private val autofillPartitionCard: AutofillPartition.Card = AutofillPartition.Card(
views = listOf(

72
app/src/test/java/com/x8bit/bitwarden/data/autofill/parser/AutofillParserTests.kt
View file

@ -205,6 +205,7 @@ class AutofillParserTests {
autofillType = AUTOFILL_TYPE,
isFocused = true,
textValue = null,
hasPasswordTerms = false,
),
monthValue = null,
)
@ -271,6 +272,7 @@ class AutofillParserTests {
autofillType = AUTOFILL_TYPE,
isFocused = true,
textValue = null,
hasPasswordTerms = false,
),
monthValue = null,
)
@ -281,6 +283,7 @@ class AutofillParserTests {
autofillType = AUTOFILL_TYPE,
isFocused = false,
textValue = null,
hasPasswordTerms = false,
),
)
val autofillPartition = AutofillPartition.Card(
@ -330,6 +333,7 @@ class AutofillParserTests {
autofillType = AUTOFILL_TYPE,
isFocused = false,
textValue = null,
hasPasswordTerms = false,
),
monthValue = null,
)
@ -340,6 +344,7 @@ class AutofillParserTests {
autofillType = AUTOFILL_TYPE,
isFocused = true,
textValue = null,
hasPasswordTerms = false,
),
)
val autofillPartition = AutofillPartition.Login(
@ -378,6 +383,60 @@ class AutofillParserTests {
}
}
@Suppress("MaxLineLength")
@Test
fun `parse should have Password AutofillView when the Password field is invalid, contains no other Password fields, and contains a password term`() {
// Setup
every { assistStructure.windowNodeCount } returns 1
every { assistStructure.getWindowNodeAt(0) } returns loginWindowNode
val unusedAutofillView: AutofillView.Unused = AutofillView.Unused(
data = AutofillView.Data(
autofillId = loginAutofillId,
autofillOptions = emptyList(),
autofillType = AUTOFILL_TYPE,
isFocused = true,
textValue = null,
hasPasswordTerms = true,
),
)
val loginAutofillView: AutofillView.Login = AutofillView.Login.Password(
data = unusedAutofillView.data,
)
val autofillPartition = AutofillPartition.Login(
views = listOf(loginAutofillView),
)
val expected = AutofillRequest.Fillable(
ignoreAutofillIds = emptyList(),
inlinePresentationSpecs = inlinePresentationSpecs,
maxInlineSuggestionsCount = MAX_INLINE_SUGGESTION_COUNT,
packageName = PACKAGE_NAME,
partition = autofillPartition,
uri = URI,
)
every { loginViewNode.toAutofillView() } returns unusedAutofillView
// 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<List<ViewNodeTraversalData>>().buildPackageNameOrNull(assistStructure)
any<List<ViewNodeTraversalData>>().buildUriOrNull(PACKAGE_NAME)
}
}
@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`() {
@ -420,6 +479,7 @@ class AutofillParserTests {
autofillType = AUTOFILL_TYPE,
isFocused = true,
textValue = null,
hasPasswordTerms = false,
),
)
val loginUsernameAutofillView: AutofillView.Login = AutofillView.Login.Username(
@ -429,6 +489,7 @@ class AutofillParserTests {
autofillType = AUTOFILL_TYPE,
isFocused = true,
textValue = null,
hasPasswordTerms = false,
),
)
val loginPasswordAutofillView: AutofillView.Login = AutofillView.Login.Password(
@ -438,6 +499,7 @@ class AutofillParserTests {
autofillType = AUTOFILL_TYPE,
isFocused = false,
textValue = null,
hasPasswordTerms = false,
),
)
val autofillPartition = AutofillPartition.Login(
@ -488,6 +550,7 @@ class AutofillParserTests {
autofillType = AUTOFILL_TYPE,
isFocused = true,
textValue = null,
hasPasswordTerms = false,
),
monthValue = null,
)
@ -498,6 +561,7 @@ class AutofillParserTests {
autofillType = AUTOFILL_TYPE,
isFocused = true,
textValue = null,
hasPasswordTerms = false,
),
)
val autofillPartition = AutofillPartition.Card(
@ -548,6 +612,7 @@ class AutofillParserTests {
autofillType = AUTOFILL_TYPE,
isFocused = false,
textValue = null,
hasPasswordTerms = false,
),
monthValue = null,
)
@ -558,6 +623,7 @@ class AutofillParserTests {
autofillType = AUTOFILL_TYPE,
isFocused = false,
textValue = null,
hasPasswordTerms = false,
),
)
val autofillPartition = AutofillPartition.Card(
@ -608,6 +674,7 @@ class AutofillParserTests {
autofillType = AUTOFILL_TYPE,
isFocused = true,
textValue = null,
hasPasswordTerms = false,
),
monthValue = null,
)
@ -618,6 +685,7 @@ class AutofillParserTests {
autofillType = AUTOFILL_TYPE,
isFocused = true,
textValue = null,
hasPasswordTerms = false,
),
)
val autofillPartition = AutofillPartition.Card(
@ -668,6 +736,7 @@ class AutofillParserTests {
autofillType = AUTOFILL_TYPE,
isFocused = true,
textValue = null,
hasPasswordTerms = false,
),
monthValue = null,
)
@ -678,6 +747,7 @@ class AutofillParserTests {
autofillType = AUTOFILL_TYPE,
isFocused = true,
textValue = null,
hasPasswordTerms = false,
),
)
val autofillPartition = AutofillPartition.Card(
@ -727,6 +797,7 @@ class AutofillParserTests {
autofillType = AUTOFILL_TYPE,
isFocused = true,
textValue = null,
hasPasswordTerms = false,
),
monthValue = null,
)
@ -737,6 +808,7 @@ class AutofillParserTests {
autofillType = AUTOFILL_TYPE,
isFocused = true,
textValue = null,
hasPasswordTerms = false,
),
)
val remoteBlockList = listOf(

2
app/src/test/java/com/x8bit/bitwarden/data/autofill/util/AutofillPartitionExtensionsTest.kt
View file

@ -15,6 +15,7 @@ class AutofillPartitionExtensionsTest {
autofillType = View.AUTOFILL_TYPE_TEXT,
isFocused = false,
textValue = null,
hasPasswordTerms = false,
)
private val autofillDataValidText: AutofillView.Data = AutofillView.Data(
autofillId = mockk(),
@ -22,6 +23,7 @@ class AutofillPartitionExtensionsTest {
autofillType = View.AUTOFILL_TYPE_TEXT,
isFocused = false,
textValue = TEXT_VALUE,
hasPasswordTerms = false,
)
@Test

1
app/src/test/java/com/x8bit/bitwarden/data/autofill/util/AutofillViewExtensionsTest.kt
View file

@ -25,6 +25,7 @@ class AutofillViewExtensionsTest {
autofillType = View.AUTOFILL_TYPE_TEXT,
isFocused = false,
textValue = null,
hasPasswordTerms = false,
)
@BeforeEach

1
app/src/test/java/com/x8bit/bitwarden/data/autofill/util/FilledDataExtensionsTest.kt
View file

@ -74,6 +74,7 @@ class FilledDataExtensionsTest {
autofillType = View.AUTOFILL_TYPE_TEXT,
isFocused = true,
textValue = null,
hasPasswordTerms = false,
),
),
),

64
app/src/test/java/com/x8bit/bitwarden/data/autofill/util/ViewNodeExtensionsTest.kt
View file

@ -27,11 +27,14 @@ class ViewNodeExtensionsTest {
autofillType = AUTOFILL_TYPE,
isFocused = expectedIsFocused,
textValue = TEXT_VALUE,
hasPasswordTerms = false,
)
private val testAutofillValue: AutofillValue = mockk()
private val viewNode: AssistStructure.ViewNode = mockk {
every { this@mockk.autofillId } returns expectedAutofillId
every { this@mockk.idEntry } returns null
every { this@mockk.hint } returns null
every { this@mockk.autofillOptions } returns AUTOFILL_OPTIONS_ARRAY
every { this@mockk.autofillType } returns AUTOFILL_TYPE
every { this@mockk.autofillValue } returns testAutofillValue
@ -275,23 +278,6 @@ class ViewNodeExtensionsTest {
assertTrue(actual)
}
@Test
fun `isPasswordField returns true when supportedHint is null and hint is supported`() {
SUPPORTED_RAW_PASSWORD_HINTS
.forEach { hint ->
// Setup
every { viewNode.hint } returns hint
// Test
val actual = viewNode.isPasswordField(
supportedHint = null,
)
// Verify
assertTrue(actual)
}
}
@Suppress("MaxLineLength")
@Test
fun `isPasswordField returns true when hints aren't supported, isPasswordInputType, isValidField, and isn't username`() {
@ -453,6 +439,50 @@ class ViewNodeExtensionsTest {
assertTrue(actual)
}
@Test
fun `hasPasswordTerms returns true when idEntry contains a raw password term`() {
every { viewNode.idEntry } returns null
every { viewNode.hint } returns null
// Test
val actual = viewNode.hasPasswordTerms()
// Verify
assertFalse(actual)
SUPPORTED_RAW_PASSWORD_HINTS.map {
every { viewNode.idEntry } returns it
// Test
val actual = viewNode.hasPasswordTerms()
// Verify
assertTrue(actual)
}
}
@Test
fun `hasPasswordTerms returns true when hint contains a raw password term`() {
every { viewNode.idEntry } returns null
every { viewNode.hint } returns null
// Test
val actual = viewNode.hasPasswordTerms()
// Verify
assertFalse(actual)
SUPPORTED_RAW_PASSWORD_HINTS.map {
every { viewNode.hint } returns it
// Test
val actual = viewNode.hasPasswordTerms()
// Verify
assertTrue(actual)
}
}
@Test
fun `website should return URI if domain and scheme are valid`() {
// Setup