Closed Bug 1579539 Opened 5 months ago Closed 4 months ago

Always pass the generated password guid to promptToChangePassword when it is available

Categories

(Toolkit :: Password Manager, defect, P1)

defect

Tracking

()

VERIFIED FIXED
mozilla71
Tracking Status
firefox69 --- unaffected
firefox70 --- verified
firefox71 --- verified

People

(Reporter: sfoster, Assigned: sfoster)

References

Details

(Whiteboard: [passwords:generation] [skyline])

Attachments

(2 files, 1 obsolete file)

As it is possible to change both the username and password from the password-change doorhanger, we should always pass the autoSavedLoginGuid param to promptToChangePassword, when an autosaved (generated-password) login on that origin exists.

This bug was found during verification of bug 1560042, see comments there for more context.

STR:

  • From https://twitter.com/login, save a new login by entering "user1" as username and password "password123" in the form,
  • Click "Log in" and confirm the save-password prompt that appears
  • Load https://twitter.com/login again, clear out any autofilled fields
  • Focus the password field and enter a generated password from the context menu (right-click, "Fill Password" -> "Use a securely generated password"
  • (a confirmation hint shows up confirming the auto-saving of a new login with this password)
  • Type in "user1" into the username field and submit the form
  • When the "Update this login" doorhanger appears, confirm it with no changes to username or password values in the prompt fields.
  • Load about:logins to examine the outcome

ER:

  • A single login for twitter.com, with username "user1" and the generated password value

AR:

  • A "user1" login with unchanged password "password123"
  • A "" (no username) login with the generated password
  • In the console a "Unexpected match of multiple logins" error is logged
Assignee: nobody → sfoster
Status: NEW → ASSIGNED
Priority: -- → P1

Comment on attachment 9091543 [details]
Bug 1579540 - Part2: Always provide autoSavedLoginGuid to promptToChangePassword when available. r?MattN

Revision D45239 was moved to bug 1579540. Setting attachment 9091543 [details] to obsolete.

Attachment #9091543 - Attachment is obsolete: true
Attachment #9091543 - Attachment description: Bug 1579539 - Always provide autoSavedLoginGuid to promptToChangePassword when available. r?MattN → Bug 1579540 - Part2: Always provide autoSavedLoginGuid to promptToChangePassword when available. r?MattN
Attachment #9091543 - Attachment is obsolete: false
Attachment #9091543 - Attachment is obsolete: true
Attachment #9091542 - Attachment description: Bug 1579539 - Part 1: Share some form test helpers → Bug 1579539 - Part 1: Share some form test helpers. r?MattN
Attachment #9091570 - Attachment description: Bug 1579539 - Part2: Always provide autoSavedLoginGuid to promptToChangePassword when available. r?MattN → Bug 1579539 - Part 2: Always provide autoSavedLoginGuid to promptToChangePassword when available. r?MattN
Pushed by sfoster@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/d52c05e448ff
Part 1: Share some form test helpers. r=MattN
https://hg.mozilla.org/integration/autoland/rev/f57e70feeea0
Part 2: Always provide autoSavedLoginGuid to promptToChangePassword when available. r=MattN
Status: ASSIGNED → RESOLVED
Closed: 4 months ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla71

Comment on attachment 9091542 [details]
Bug 1579539 - Part 1: Share some form test helpers. r?MattN

Beta/Release Uplift Approval Request

  • User impact if declined: A user's username changes in the save login doorhanger could be lost
  • Is this code covered by automated tests?: Yes
  • Has the fix been verified in Nightly?: No
  • Needs manual test from QE?: Yes
  • If yes, steps to reproduce: Should be verified using steps in comment 0.
  • List of other uplifts needed: None
  • Risk to taking this patch: Low
  • Why is the change risky/not risky? (and alternatives if risky): Good test coverage
  • String changes made/needed: None
Attachment #9091542 - Flags: approval-mozilla-beta?

Comment on attachment 9091542 [details]
Bug 1579539 - Part 1: Share some form test helpers. r?MattN

Support for Skyline feature, has test coverage, OK for uplift for beta 7.

Attachment #9091542 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Attachment #9091570 - Flags: approval-mozilla-beta? → approval-mozilla-beta+

I have managed to reproduce this issue using Firefox 71.0a1 (BuildId:20190906215007) on Windows 10 64bit.

This issue is verified fixed using Firefox 71.0a1 (BuildId:20190915214245) and Firefox 70.0b7 (provided in comment 9) on Windows 10 64bit, Ubuntu 18.04 64bit and macOS 10.13.6

Status: RESOLVED → VERIFIED
Flags: qe-verify+

Added a test case for this scenario in our Password Manager & Form autofill suite.

Flags: in-qa-testsuite+
You need to log in before you can comment on or make changes to this bug.