Closed Bug 1147563 Opened 9 years ago Closed 5 years ago

Provide autocomplete experience when formSubmitURL does not match

Categories

(Toolkit :: Password Manager, defect, P2)

Product:
Toolkit
Component:
Password Manager
Type:
defect

Tracking

()

Status:
VERIFIED FIXED
Milestone:
mozilla68
Tracking Flags:
Tracking Status
firefox68 --- verified

People

(Reporter: tanvi, Assigned: jaws)

References

Details

(Whiteboard: [passwords:cross-origin] [passwords:fill-ui])

Attachments

(2 files)

Bug 1147563 - Provide autocomplete experience when formSubmitURL does not match. r?MattN
47 bytes, text/x-phabricator-request
Details | Review
Bug 1147563 - Deprecate the 'testuser' login that is created during initialization of the password manager tests. r?MattN
47 bytes, text/x-phabricator-request
Details | Review 
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.
Will the formSubmitURL be updated, so users don't have to go through the slow path repeatedly?
You can achieve this by marking the username field for fields where formSubmitURL doesn't match. Something like this._formFillService.markAsLoginManagerField(usernameField);
Depends on: 1166113
Priority: -- → P2
Whiteboard: [passwords:cross-origin]
Whiteboard: [passwords:cross-origin] → [passwords:cross-origin] [passwords:fill-ui]
See Also: → 1168654
Depends on: 1294194
Assignee: nobody → jaws
Status: NEW → ASSIGNED

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

(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

Pushed by jwein@mozilla.com:
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

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

Status: ASSIGNED → RESOLVED
Closed: 5 years ago
status-firefox68: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla68

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

Status: RESOLVED → REOPENED
status-firefox68: fixed → ?
Flags: needinfo?(jaws)
Resolution: FIXED → ---
Flags: needinfo?(jaws)
Pushed by jwein@mozilla.com:
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

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

Status: REOPENED → RESOLVED
Closed: 5 years ago → 5 years ago
status-firefox68: ? → fixed
Resolution: --- → FIXED

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

Flags: needinfo?(jaws)
Status: RESOLVED → REOPENED
status-firefox68: fixed → ---
Resolution: FIXED → ---
Flags: needinfo?(jaws)
Pushed by jwein@mozilla.com:
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

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

Status: REOPENED → RESOLVED
Closed: 5 years ago → 5 years ago
status-firefox68: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla68

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

Flags: needinfo?(MattN+bmo)
Flags: needinfo?(MattN+bmo) → needinfo?(jaws)

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)

Flags: needinfo?(jaws)

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
Status: RESOLVED → VERIFIED
status-firefox68: fixed → verified

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

You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: