Password update flows #453

GioSensation · 2023-12-20T15:57:07Z

Reviewer: @shakyShane
Asana: https://app.asana.com/0/0/1206019054894764/f

Description

Distinguish between current password and new password with a new variant in the scoring attribute.
Different icons for the two cases
Whole lotta test updates.
A few minor fixes.

Steps to test

Test page here: https://privacy-test-pages.site/autofill/password-update.html
Build on Asana.
Added a bunch of test forms.
Updated one of the integration tests to validate that the correct icons are showing.

Signed-off-by: Emanuele Feliziani <[email protected]>

… ema/password-update-flows

Signed-off-by: Emanuele Feliziani <[email protected]>

GioSensation · 2023-12-20T15:59:05Z

src/DeviceInterface/InterfacePrototype.js

-            topContextData.inputType === 'credentials.password',
-            this.settings.featureToggles.password_generation,
-            form.isSignup
+            topContextData.inputType === 'credentials.password.new',


No need to rely on form.isSignup because we read the password.new attribute instead.

GioSensation · 2023-12-20T16:00:21Z

src/DeviceInterface/InterfacePrototype.js

-    static default () {
-        const globalConfig = createGlobalConfig()
+    static default (globalConfigOverrides) {
+        const globalConfig = createGlobalConfig(globalConfigOverrides)


This isn't really needed. But I had to add it temporarily to test something in the test suite and thought we can leave it here in case it's useful in the future. It just enables passing specific configs during tests.

GioSensation · 2023-12-20T16:06:25Z

src/Form/Form.js

-            dataType === 'identities' && // only for identities
+            dataType !== 'creditCards' && // creditCards always override, the others only when we're focusing the input
            input.nodeName !== 'SELECT' && input.value !== '' && // if the input is not empty
-            this.activeInput !== input && // and this is not the active input
-            !isEmailAutofill // and we're not auto-filling email
+            this.activeInput !== input // and this is not the active input


Avoid overriding any inserted data unless it's a credit card form or we're trying to override the current field. It's useful to avoid overriding the old password when generating a new one in 3-field update-password forms.

GioSensation · 2023-12-20T16:07:17Z

src/Form/Form.js

+                if (variant === 'new' && AUTOGENERATED_KEY in data) {
+                    return this.autofillInput(input, autofillData, dataType)
+                }
+                if (variant === 'current' && !(AUTOGENERATED_KEY in data)) {
+                    return this.autofillInput(input, autofillData, dataType)
+                }


Fill all related fields, so if there's a repeat new password field we fill it in one go, but won't override current when filling new.

GioSensation · 2023-12-20T16:10:52Z

src/Form/FormAnalyzer.js

-            const strength = likelyASubmit ? 20 : 2
-            this.updateSignal({string, strength, signalType: `submit: ${string}`})
+            const strength = likelyASubmit ? 20 : 4
+            this.updateSignal({string, strength, signalType: `button: ${string}`, shouldFlip})


This just takes into account forms with multiple buttons where only one is obviously a submit. Fixes a long-standing bug for Twitch, already in the test suite 💪.

GioSensation · 2023-12-20T16:11:34Z

src/Form/FormAnalyzer.js


        // We check for more than one to minimise false positives
-        this._isCCForm = Boolean(textMatches && textMatches.length > 1)
+        this._isCCForm = Boolean(textMatches && deDupedMatches.size > 1)


Again, fixes a bug added to the test suite. Strings could be counted multiple times and result in false matching credit card fields.

GioSensation · 2023-12-20T16:12:44Z

src/Form/input-classifiers.test.js


+const getMismatchedValue = (score) => {


Had to tweak how we compare in the test suite so we can account for the new variant without breaking all existing tests. I still back-filled about 100 of them 💪 .

GioSensation · 2023-12-20T16:18:37Z

src/Form/inputTypeConfig.js

@@ -100,12 +100,26 @@ const inputTypeConfig = {
            if (!canBeInteractedWith(input)) return ''

            if (device.settings.featureToggles.inlineIcon_credentials) {
+                const subtype = getInputSubtype(input)


Just tweaks to account for the different icons.

src/Form/inputTypeConfig.js

GioSensation · 2023-12-20T16:20:04Z

src/Form/matching.js

+                if (!this.isInputLargeEnough('emailAddress', input)) {
+                    if (shouldLog()) {
+                        console.log('Field matched for Email Address, but discarded because too small when scanned')
+                    }
+                    return 'unknown'
+                }


Same functionality, but adds logging to simplify debugging.

src/Form/matching.js

GioSensation · 2023-12-20T16:23:23Z

src/Form/matching.js

 /**
 * Remove whitespace of more than 2 in a row and trim the string
 * @param {string | null} string
 * @return {string}
 */
 const removeExcessWhitespace = (string = '') => {
+    string = string?.trim() || ''


Just moved the trimming here so the length cutoff is more accurate. Form test added.

Signed-off-by: Emanuele Feliziani <[email protected]>

integration-test/helpers/mocks.js

shakyShane

Just a single none-blocking question - everything looks good to me from the code-changes POV :)

shakyShane · 2023-12-21T11:48:18Z

src/Form/matching.js

+        // Check attributes first
+        // This is done mainly to ensure coverage for all languages, since attributes are usually in English
+        const attrsToCheck = [input.autocomplete, input.name, input.id]
+        if (
+            opts.isSignup &&
+            attrsToCheck.some(str => safeRegexTest(/new.?password|password.?new/i, str))
+        ) {
+            return 'credentials.password.new'
+        }
+        if (
+            (opts.isLogin || opts.isHybrid) &&
+            attrsToCheck.some(str => safeRegexTest(/(current|old|previous).?password|password.?(current|old|previous)/i, str))
+        ) {
+            return 'credentials.password.current'
+        }


none-blocking question: in your opinion, does this workaround indicate a flaw in our implementation of matchers?

I only ask because I'm curious if we can expand the design later to include things like this, so that we can keep these regexes together.

I don't feel strongly, I think your solution here is great, more of a talking point for later :)

If I understand your question correctly, unfortunately this has nothing to do with our implementation. It's that some pages use autocomplete=current-password for signup pages and autocomplete=new-password for login pages, exactly backwards 🤦‍♂️. We found this even on very high-profile pages like Pinterest and Twitch. This workaround is needed to ensure that when that happens we still try to show the best option to users.

@GioSensation

Task/Issue URL: https://app.asana.com/0/1206327382038426/1206327382038426 Autofill Release: https://github.com/duckduckgo/duckduckgo-autofill/releases/tag/10.0.3 ## Description Updates Autofill to version [10.0.3](https://github.com/duckduckgo/duckduckgo-autofill/releases/tag/10.0.3). ### Autofill 10.0.3 release notes ## What's Changed * Use the default branch when checking out repos by @GioSensation in duckduckgo/duckduckgo-autofill#444 * Password update flows by @GioSensation in duckduckgo/duckduckgo-autofill#453 * Bump @types/jest from 29.5.5 to 29.5.11 by @dependabot in duckduckgo/duckduckgo-autofill#439 * Update password-related json files (2023-12-21) by @daxmobile in duckduckgo/duckduckgo-autofill#454 * Move test forms out of src by @GioSensation in duckduckgo/duckduckgo-autofill#457 * Move integration tests around by @GioSensation in duckduckgo/duckduckgo-autofill#455 * Fixes by @GioSensation in duckduckgo/duckduckgo-autofill#467 * Update password-related json files (2024-01-11) by @daxmobile in duckduckgo/duckduckgo-autofill#466 **Full Changelog**: duckduckgo/duckduckgo-autofill@10.0.2...10.0.3 ## Steps to test This release has been tested during autofill development. For smoke test steps see [this task](https://app.asana.com/0/1198964220583541/1200583647142330/f). Co-authored-by: GioSensation <[email protected]>

GioSensation added 21 commits November 14, 2023 15:52

Less filled fields overwriting

40a4cb1

Signed-off-by: Emanuele Feliziani <[email protected]>

Add basic support for variants and password.new

31bf3b9

Signed-off-by: Emanuele Feliziani <[email protected]>

Add the new icon for password generation

ad265e1

Signed-off-by: Emanuele Feliziani <[email protected]>

Add test forms

7dd2047

Signed-off-by: Emanuele Feliziani <[email protected]>

Add new generate password icon to config

3ac2780

Signed-off-by: Emanuele Feliziani <[email protected]>

Merge branch 'main' of github.com:duckduckgo/duckduckgo-autofill into…

5f8aa82

… ema/password-update-flows

Initial password variant implementation

0ff30f0

Signed-off-by: Emanuele Feliziani <[email protected]>

Add testing capabilities for password variants

c4aba00

Signed-off-by: Emanuele Feliziani <[email protected]>

Update icons

568b34e

Signed-off-by: Emanuele Feliziani <[email protected]>

Allow overriding the default device

4536dba

Signed-off-by: Emanuele Feliziani <[email protected]>

Fix

20c5005

Signed-off-by: Emanuele Feliziani <[email protected]>

Improve username search on password generation

d66917c

Signed-off-by: Emanuele Feliziani <[email protected]>

Log when skipping email field because of size

ff5ce53

Signed-off-by: Emanuele Feliziani <[email protected]>

Trim before checking for length

ff006db

Signed-off-by: Emanuele Feliziani <[email protected]>

Improve accuracy when web authors lie

20ee395

Signed-off-by: Emanuele Feliziani <[email protected]>

Minor regex improvements

b2a2c71

Signed-off-by: Emanuele Feliziani <[email protected]>

Improve button reading in FormAnalyzer

6451b9b

Signed-off-by: Emanuele Feliziani <[email protected]>

Dedupe cc text matches

ac78267

Signed-off-by: Emanuele Feliziani <[email protected]>

Test galore

624bdbe

Signed-off-by: Emanuele Feliziani <[email protected]>

Improve integration tests

22450a2

Signed-off-by: Emanuele Feliziani <[email protected]>

Commit compiled files

12be76c

Signed-off-by: Emanuele Feliziani <[email protected]>

GioSensation self-assigned this Dec 20, 2023

GioSensation commented Dec 20, 2023

View reviewed changes

src/Form/inputTypeConfig.js Outdated Show resolved Hide resolved

GioSensation commented Dec 20, 2023

View reviewed changes

src/Form/matching.js Outdated Show resolved Hide resolved

GioSensation commented Dec 20, 2023

View reviewed changes

GioSensation added 4 commits December 21, 2023 11:30

Remove unnecessary complexity

0cf0495

Signed-off-by: Emanuele Feliziani <[email protected]>

Simplify password matching and add comments

e067087

Signed-off-by: Emanuele Feliziani <[email protected]>

Remove unused flag

18439c3

Signed-off-by: Emanuele Feliziani <[email protected]>

Update Dax icon in tests

518227a

Signed-off-by: Emanuele Feliziani <[email protected]>

GioSensation commented Dec 21, 2023

View reviewed changes

integration-test/helpers/mocks.js Show resolved Hide resolved

GioSensation requested a review from shakyShane December 21, 2023 11:16

GioSensation marked this pull request as ready for review December 21, 2023 11:17

shakyShane approved these changes Dec 21, 2023

View reviewed changes

GioSensation merged commit 8ef4a88 into main Dec 21, 2023
1 check passed

GioSensation deleted the ema/password-update-flows branch December 21, 2023 14:12

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Password update flows #453

Password update flows #453

GioSensation commented Dec 20, 2023 •

edited

Loading

GioSensation Dec 20, 2023 •

edited

Loading

GioSensation Dec 20, 2023

GioSensation Dec 20, 2023

GioSensation Dec 20, 2023 •

edited

Loading

GioSensation Dec 20, 2023

GioSensation Dec 20, 2023

GioSensation Dec 20, 2023

GioSensation Dec 20, 2023

GioSensation Dec 20, 2023

GioSensation Dec 20, 2023

shakyShane left a comment

shakyShane Dec 21, 2023

GioSensation Dec 21, 2023

Password update flows #453

Password update flows #453

Conversation

GioSensation commented Dec 20, 2023 • edited Loading

Description

Steps to test

GioSensation Dec 20, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

GioSensation Dec 20, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

shakyShane left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

GioSensation commented Dec 20, 2023 •

edited

Loading

GioSensation Dec 20, 2023 •

edited

Loading

GioSensation Dec 20, 2023 •

edited

Loading