Closed
Bug 1322673
Opened 7 years ago
Closed 5 years ago
Login/Password not updated if "Saved Logins" has stored http and https for this site
Categories
(Toolkit :: Password Manager, defect, P1)
Toolkit
Password Manager
Tracking
()
People
(Reporter: nachtigall, Assigned: MattN)
References
Details
(Keywords: regression)
Attachments
(2 files)
Reproduced with Firefox 52 in failsafe mode with all add-ons disabled. I think this might have been introduced by Bug 667233 but might be wrong. I could not update my login to the new password on a particular site, let's call it example.com. It turned out that I had the credentials stored twice: once for http://example.com and once for https://example.com. I guess this is something that might occur more and more with many sites moving from http to https. I could reproduce with the following steps: 1. Duplicate any of your https login creditials of any site. I did so with https://addons.mozilla.org/en-US/firefox/addon/saved-password-editor/ which has a "Clone" menu option. 2. Edit/Change the cloned login entry from https to http. 3. Go to the https version of the site 4. Enter whatever login you might think of (can be wrong) 5. Say "Update" when ask if you want to update this login AR: Password is not updated. Have a look in the password manager ER: Password should be updated. If you delete the old http entry in the password manager, then it works - I could verify with my original example.com entry.
Assignee | ||
Updated•7 years ago
|
Flags: needinfo?(MattN+bmo)
Priority: -- → P1
Comment 1•7 years ago
|
||
Same issue at a friend's computer who was wondering why https://example.com does not update the password. I checked the stored entries, removed the HTTP entries and changed the passwords under settings in the same step. The new passwords were not saved. Only when I restarted and did it again the new passwords where saved.
Assignee | ||
Comment 2•7 years ago
|
||
Additional logging and the core issue was found in bug 1378448. (In reply to Eric Rescorla (:ekr) from bug 1378448 comment #3) > (In reply to Matthew N. [:MattN] from bug 1378448 comment #2) > > Can you set the pref signon.debug=True and see if there are any errors or > > warnings in your log using the instructions from > > https://wiki.mozilla.org/Toolkit:Password_Manager/Debugging. > > I'm guessing this: > Unexpected match of multiple logins. nsLoginManagerPrompter.js:961 > persistData > jar:file:///Applications/FirefoxNightly.app/Contents/Resources/omni.ja!/ > components/nsLoginManagerPrompter.js:961:9 > callback > jar:file:///Applications/FirefoxNightly.app/Contents/Resources/omni.ja!/ > components/nsLoginManagerPrompter.js:975:9 > _onButtonEvent resource://gre/modules/PopupNotifications.jsm:1461:9 > oncommand chrome://browser/content/browser.xul:1:1 Yep, that's the issue. It comes from https://dxr.mozilla.org/mozilla-central/rev/6fec4855b5345eb63fef57089e61829b88f5f4eb/toolkit/components/passwordmgr/nsLoginManagerPrompter.js#961 We will need to make this logic smarter since the HTTPS upgrade support landed. This is likely a regression from bug 667233. > > Services.logins.getAllLogins().filter(login => > > login.hostname.endsWith("united.com")).map(login => `${login.hostname} > > ${login.formSubmitURL}`).join("\n") > > "http://www.united.com https://www.united.com > https://www.united.com https://www.united.com" Note that one is http=>https and the other is https=>https
status-firefox54:
--- → affected
status-firefox55:
--- → affected
status-firefox56:
--- → affected
Flags: needinfo?(MattN+bmo)
Keywords: regression
OS: Unspecified → All
Hardware: Unspecified → All
Comment 4•7 years ago
|
||
This is not a new regression in 54. Mark 54 won't fix.
Assignee | ||
Updated•7 years ago
|
Severity: normal → critical
This is a P1 bug without an assignee. P1 are bugs which are being worked on for the current release cycle/iteration/sprint. If the bug is not assigned by Monday, 28 August, the bug's priority will be reset to '--'.
Keywords: stale-bug
Updated•7 years ago
|
Assignee | ||
Updated•7 years ago
|
Assignee | ||
Updated•6 years ago
|
Priority: P3 → P1
Assignee | ||
Updated•5 years ago
|
Assignee: nobody → MattN+bmo
Status: NEW → ASSIGNED
status-firefox64:
--- → wontfix
status-firefox65:
--- → affected
status-firefox66:
--- → affected
status-firefox-esr60:
--- → wontfix
Assignee | ||
Comment 9•5 years ago
|
||
If we have both http: and https: then we need to dedupe them to decide which one to update. This uses the same logic as for HTTP auth https://searchfox.org/mozilla-central/rev/76fe4bb385348d3f45bbebcf69ba8c7283dfcec7/toolkit/components/passwordmgr/nsLoginManagerPrompter.js#554-558
Assignee | ||
Comment 10•5 years ago
|
||
Depends on D15884
Comment 11•5 years ago
|
||
Pushed by mozilla@noorenberghe.ca: https://hg.mozilla.org/integration/autoland/rev/1898dd0aeab9 Dedupe logins to update upon password change. r=johannh https://hg.mozilla.org/integration/autoland/rev/8268418fa252 Test that password changes are saved when both http: and https: for the same username exist. r=johannh
Comment 12•5 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/1898dd0aeab9
https://hg.mozilla.org/mozilla-central/rev/8268418fa252
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla66
Updated•5 years ago
|
Flags: in-testsuite+
Hi Matt, since 65 is affected, should we uplift this to Beta?
Flags: needinfo?(MattN+bmo)
Assignee | ||
Comment 14•5 years ago
|
||
Ideally yes, but I think it should bake a little more on m-c.
Flags: needinfo?(MattN+bmo)
Thanks Matt. Tracking for 65 with the hope that we can take it in b11 (Tues) or b12 (Thurs) next week.
tracking-firefox65:
--- → +
Assignee | ||
Comment 16•5 years ago
|
||
I think it's safer to ride the trains at this point.
Updated•5 years ago
|
tracking-firefox65:
+ → ---
You need to log in
before you can comment on or make changes to this bug.
Description
•