1147563 - Provide autocomplete experience when formSubmitURL does not match

Status: VERIFIED FIXED

(Toolkit :: Password Manager, defect, P2)

Description

[Tanvi Vyas[:tanvi]]

When finding all the login candidates for a form in the password manager database, we use the formSubmitURL as one of the keys.  If the formSubmitURL doesn't match the login form's action, we do not autofill or autocomplete the password on the login form.  Instead, we should provide the autocomplete experience so the user can find the username/password and login if they choose to.

This improves user experience without causing the security issues noted in bugs 360493 and 1121119, because the user is taking a specific action to fill the password.

Comment 1

[Jesse Ruderman]

Will the formSubmitURL be updated, so users don't have to go through the slow path repeatedly?

Comment 2

[Tanvi Vyas[:tanvi]]

You can achieve this by marking the username field for fields where formSubmitURL doesn't match. Something like this._formFillService.markAsLoginManagerField(usernameField);

Comment 5

[Jared Wein [:jaws] (please needinfo? me)]

Attachment: Bug 1147563 - Provide autocomplete experience when formSubmitURL does not match. r?MattN

Comment 6

[Jared Wein [:jaws] (please needinfo? me)]

Attachment: Bug 1147563 - Deprecate the 'testuser' login that is created during initialization of the password manager tests. r?MattN

This login should not show up in tests that don't expect it once we allow non-matching formSubmitURLs.

Comment 7

[Matthew N. [:MattN]]

Looks like there are still some xpcshell and mochitest failures: https://treeherder.mozilla.org/testview.html?repo=try&revision=b4374a50fd7470b580b791812e9dd5e454ddfa43&filter=pass

Comment 8

[Jared Wein [:jaws] (please needinfo? me)]

(In reply to Matthew N. [:MattN] (PM me if requests are blocking you) from comment #7)

Looks like there are still some xpcshell and mochitest failures: https://treeherder.mozilla.org/testview.html?repo=try&revision=b4374a50fd7470b580b791812e9dd5e454ddfa43&filter=pass

These should now be fixed in the patch I just updated on phabricator. New try push, https://treeherder.mozilla.org/#/jobs?repo=try&revision=d58e9ad67216836c48ce550d0937345306bed990

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/b2f2db06d094
Provide autocomplete experience when formSubmitURL does not match. r=MattN
https://hg.mozilla.org/integration/autoland/rev/5e0ac9f08356
Deprecate the 'testuser' login that is created during initialization of the password manager tests. r=MattN

Comment 10

[Stefan Hindli [:stefan_hindli]]

https://hg.mozilla.org/mozilla-central/rev/b2f2db06d094
https://hg.mozilla.org/mozilla-central/rev/5e0ac9f08356

Comment 11

[Natalia Csoregi [:nataliaCs]]

Backed out for /test_basic_form_autocomplete.html permafails

backout: https://hg.mozilla.org/integration/autoland/rev/5721f39a987d5d17dd0acc230a2908177e22cfc8

push: https://treeherder.mozilla.org/#/jobs?repo=autoland&job_type_name=test-linux64-qr%2Fdebug-mochitest-e10s-15&selectedJob=235616092&revision=5e0ac9f08356c1f0c58cf49b2c79ef8f1f2c677c

failure log: https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=235616092&repo=autoland&lineNumber=20103

[task 2019-03-23T02:36:39.656Z] 02:36:39 INFO - TEST-PASS | toolkit/components/passwordmgr/test/mochitest/test_basic_form_autocomplete.html | Checking form8 password is:
[task 2019-03-23T02:36:39.657Z] 02:36:39 INFO - AddTask.js | Leaving test test_form8_3
[task 2019-03-23T02:36:39.657Z] 02:36:39 INFO - AddTask.js | Entering test test_form9_filtering
[task 2019-03-23T02:36:39.658Z] 02:36:39 INFO - Buffered messages finished
[task 2019-03-23T02:36:39.662Z] 02:36:39 INFO - TEST-UNEXPECTED-FAIL | toolkit/components/passwordmgr/test/mochitest/test_basic_form_autocomplete.html | Test timed out.
[task 2019-03-23T02:36:39.663Z] 02:36:39 INFO - SimpleTest.ok@https://example.com/tests/SimpleTest/SimpleTest.js:275:18
[task 2019-03-23T02:36:39.663Z] 02:36:39 INFO - reportError@https://example.com/tests/SimpleTest/TestRunner.js:121:22
[task 2019-03-23T02:36:39.664Z] 02:36:39 INFO - TestRunner._checkForHangs@https://example.com/tests/SimpleTest/TestRunner.js:142:7
[task 2019-03-23T02:36:39.664Z] 02:36:39 INFO - setTimeout handlerTestRunner._checkForHangs@https://example.com/tests/SimpleTest/TestRunner.js:163:5
[task 2019-03-23T02:36:39.666Z] 02:36:39 INFO - setTimeout handlerTestRunner._checkForHangs@https://example.com/tests/SimpleTest/TestRunner.js:163:5
[task 2019-03-23T02:36:39.667Z] 02:36:39 INFO - setTimeout handlerTestRunner._checkForHangs@https://example.com/tests/SimpleTest/TestRunner.js:163:5
[task 2019-03-23T02:36:39.667Z] 02:36:39 INFO - setTimeout handlerTestRunner._checkForHangs@https://example.com/tests/SimpleTest/TestRunner.js:163:5
[task 2019-03-23T02:36:39.667Z] 02:36:39 INFO - setTimeout handlerTestRunner._checkForHangs@https://example.com/tests/SimpleTest/TestRunner.js:163:5
[task 2019-03-23T02:36:39.667Z] 02:36:39 INFO - setTimeout handlerTestRunner._checkForHangs@https://example.com/tests/SimpleTest/TestRunner.js:163:5
[task 2019-03-23T02:36:39.667Z] 02:36:39 INFO - setTimeout handlerTestRunner._checkForHangs@https://example.com/tests/SimpleTest/TestRunner.js:163:5
[task 2019-03-23T02:36:39.667Z] 02:36:39 INFO - setTimeout handlerTestRunner._checkForHangs@https://example.com/tests/SimpleTest/TestRunner.js:163:5
[task 2019-03-23T02:36:39.667Z] 02:36:39 INFO - setTimeout handlerTestRunner._checkForHangs@https://example.com/tests/SimpleTest/TestRunner.js:163:5
[task 2019-03-23T02:36:39.667Z] 02:36:39 INFO - setTimeout handlerTestRunner._checkForHangs@https://example.com/tests/SimpleTest/TestRunner.js:163:5
[task 2019-03-23T02:36:39.667Z] 02:36:39 INFO - setTimeout handlerTestRunner._checkForHangs@https://example.com/tests/SimpleTest/TestRunner.js:163:5
[task 2019-03-23T02:36:39.667Z] 02:36:39 INFO - TestRunner.runTests/<@https://example.com/tests/SimpleTest/TestRunner.js:381:20
[task 2019-03-23T02:36:39.667Z] 02:36:39 INFO - promise callbackTestRunner.runTests@https://example.com/tests/SimpleTest/TestRunner.js:368:50
[task 2019-03-23T02:36:39.667Z] 02:36:39 INFO - RunSet.runtests@https://example.com/tests/SimpleTest/setup.js:201:14
[task 2019-03-23T02:36:39.667Z] 02:36:39 INFO - RunSet.runall@https://example.com/tests/SimpleTest/setup.js:180:12
[task 2019-03-23T02:36:39.667Z] 02:36:39 INFO - hookupTests@https://example.com/tests/SimpleTest/setup.js:273:12
[task 2019-03-23T02:36:39.667Z] 02:36:39 INFO - parseTestManifest@https://example.com/manifestLibrary.js:36:5
[task 2019-03-23T02:36:39.668Z] 02:36:39 INFO - getTestManifest/req.onload@https://example.com/manifestLibrary.js:49:11
[task 2019-03-23T02:36:39.668Z] 02:36:39 INFO - EventHandlerNonNullgetTestManifest@https://example.com/manifestLibrary.js:45:3
[task 2019-03-23T02:36:39.668Z] 02:36:39 INFO - hookup@https://example.com/tests/SimpleTest/setup.js:253:5
[task 2019-03-23T02:36:39.668Z] 02:36:39 INFO - EventHandlerNonNull@https://example.com/tests?autorun=1&closeWhenDone=1&consoleLevel=INFO&manifestFile=tests.json&dumpOutputDirectory=%2Ftmp&cleanupCrashes=true:11:1
[task 2019-03-23T02:36:40.328Z] 02:36:40 INFO - GECKO(4498) | MEMORY STAT | vsize 1499MB | residentFast 137MB | heapAllocated 13MB
[task 2019-03-23T02:36:40.344Z] 02:36:40 INFO - TEST-OK | toolkit/components/passwordmgr/test/mochitest/test_basic_form_autocomplete.html | took 310399ms

Comment 12

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/fef0ddcd6bba
Provide autocomplete experience when formSubmitURL does not match. r=MattN
https://hg.mozilla.org/integration/autoland/rev/53c71a1d9183
Deprecate the 'testuser' login that is created during initialization of the password manager tests. r=MattN

Comment 13

[Raul Gurzau (:RaulG)]

https://hg.mozilla.org/mozilla-central/rev/fef0ddcd6bba
https://hg.mozilla.org/mozilla-central/rev/53c71a1d9183

Comment 14

[Noemi Erli[:noemi_erli]]

Since this bug was landed, we have a perma-fail on test_insecure_form_field_autocomplete.html: http://tinyurl.com/y2obfbe4
and since then, we have this failure on almost each push

Log: https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=236526804&repo=autoland&lineNumber=30866
https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=235998253&repo=autoland&lineNumber=30797

Backed out 2 changesets (Bug 1147563) for perma-failures in test_insecure_form_field_autocomplete.html a=backout

Backout: https://hg.mozilla.org/mozilla-central/rev/0b5c2f979c83ffaba7bfec7229ea3588118313d4

Comment 15

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/b09ce6eebb49
Provide autocomplete experience when formSubmitURL does not match. r=MattN
https://hg.mozilla.org/integration/autoland/rev/49a9bb5d764c
Deprecate the 'testuser' login that is created during initialization of the password manager tests. r=MattN

Comment 16

[Raul Gurzau (:RaulG)]

https://hg.mozilla.org/mozilla-central/rev/b09ce6eebb49
https://hg.mozilla.org/mozilla-central/rev/49a9bb5d764c

Comment 17

[Daniel Bodea [:danibodea]]

Can you provide me with a valid test case to verify this bug? Thanks.

Comment 18

[Matthew N. [:MattN]]

I think what you can do for this bug is:

1. Find a password field inside a <form> element on a public website (this might not work on sites which use CSP) and add/change the action attribute to point to a different origin e.g. action="https://localhost" (will give an error if you aren't running a localhost HTTPS server but that shouldn't affect the doorhanger behaviour which is relevant here)
2. Submit the form and save the login. If you look in logins.json of your profile folder you should see the formSubmitURL is https://localhost.
3. Reload that same login form without the action changes.
4. Only after this bug, you should get autocomplete with that new login.
5. Fill that new login via autocomplete
6. Submit the form

Expected result:
No doorhanger to save the login again (with the new formSubmitURL)

Comment 19

[Adrian Florinescu [:aflorinescu]]

Using STR from commment 18, verified as fixed on: 68.0a1 2019-05-05 on Windows 10 Pro, Ubuntu 16.04, Mac 10.13.6 :

• credentials saved with localhost formSubmitURL
• formSubmitURL confirmed in logins.json
• reloading the page with the original formSubmitURL does not autofill the credentials
• password manager credential are available on password manager trigger (click / down arrow key / autocomplete)
• auto complete filled user/password combination is not resaved with the original/different formSubmitURL

Comment 20

[John]

Since this bug was landed, a perma-fail on test_insecure_form_field_autocomplete https://www.aftya.com/pages/who-we-are
and since then, we have this failure on almost each push