Closed Bug 1322673 Opened 3 years ago Closed 9 months ago

Login/Password not updated if "Saved Logins" has stored http and https for this site

Categories

(Toolkit :: Password Manager, defect, P1, critical)

defect

Tracking

()

RESOLVED FIXED
mozilla66
Tracking Status
firefox-esr60 --- wontfix
firefox54 --- wontfix
firefox55 --- wontfix
firefox56 --- wontfix
firefox57 --- wontfix
firefox64 --- wontfix
firefox65 --- wontfix
firefox66 --- fixed

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.
Flags: needinfo?(MattN+bmo)
Priority: -- → P1
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.
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
Flags: needinfo?(MattN+bmo)
Keywords: regression
OS: Unspecified → All
Hardware: Unspecified → All
Duplicate of this bug: 1378448
This is not a new regression in 54. Mark 54 won't fix.
Duplicate of this bug: 1380889
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
Duplicate of this bug: 1406944
Moving to p3 because no activity for at least 24 weeks.
Priority: P1 → P3
Priority: P3 → P1
Assignee: nobody → MattN+bmo
Status: NEW → ASSIGNED
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
Status: ASSIGNED → RESOLVED
Closed: 9 months ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla66
Flags: in-testsuite+

Hi Matt, since 65 is affected, should we uplift this to Beta?

Flags: needinfo?(MattN+bmo)

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.

I think it's safer to ride the trains at this point.

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