1548857 - Save a generated login immediately if there are no saved logins for the site

Status: VERIFIED FIXED

(Toolkit :: Password Manager, enhancement, P2)

Description

[Matthew N. [:MattN]]

When we fill a newly generated password for a user, we should save it to persistent login storage if there are no saved logins for the site.

Rationale: Since we can only save one password per username on a site, if there are saved logins for the site we risk overwriting them when the user may still need them to enter their old password.

Comment 1

[Matthew N. [:MattN]]

Attachment: Bug 1548857 - Make the 'aUsernameField' and 'aPasswordField' nsILoginInfo fields optional. r=sfoster

I'm not removing them now since that will break Thunderbird.

Comment 2

[Matthew N. [:MattN]]

Attachment: Bug 1548857 - Save a generated login immediately if there are no saved logins for the site. r=sfoster

Depends on D32426

Comment 3

[Matthew N. [:MattN]]

Attachment: Bug 1548857 - Don't highlight username fields that we didn't actually change. r=sfoster

Depends on D32426

Comment 4

[Matthew N. [:MattN]]

Attachment: Bug 1548857 - Rename `looseActionOriginMatch` to `ignoreActionAndRealm` to be more correct. r=sfoster

A loose actionOrigin match is achieved using "" for the formSubmitURL whereas the option also includes HTTP auth logins with a null formSubmitURL.

Depends on D32607

Comment 5

[Phabricator Automation]

Comment on attachment 9067248 [details]
Bug 1548857 - Make the 'aUsernameField' and 'aPasswordField' nsILoginInfo fields optional. r=sfoster

Revision D32426 was moved to bug 1555152. Setting attachment 9067248 [details] to obsolete.

Comment 6

[Phabricator Automation]

Comment on attachment 9067590 [details]
Bug 1548857 - Rename looseActionOriginMatch to ignoreActionAndRealm to be more correct. r=sfoster

Revision D32608 was moved to bug 1555152. Setting attachment 9067590 [details] to obsolete.

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/8dcae2b6d89b
Save a generated login immediately if there are no saved logins for the site. r=sfoster

Comment 8

[Narcis Beleuzu [:NarcisB]]

https://hg.mozilla.org/mozilla-central/rev/8dcae2b6d89b

Comment 9

[Peter Magyari (Desktop QA)]

Hi Matthew,

I am having trouble verifying this issue, could you help me with some steps, maybe a website on which I could verify this?

Thank you!

Comment 10

[Daniel Bodea [:danibodea]]

Yes, more info in bug 1548381. I am taking this to verify myself.

• These are the steps I took to verify this implementation, case where no password is being saved for the site in question:

1. Open browser with a new profile and change prefs in about:config :
   "signon.generation.enabled" is the user pref to enable/disable the feature from about:preferences (not implemented yet).
   "signon.generation.available" controls whether the feature is available for users (e.g. if the about:preferences UI should show).
2. Restart browser (not sure if it's required, but I'm not taking any chances)
3. Log into Yahoo (one of the sites where the feature works).
4. On the door-hanger, choose "Don't save" the credentials.
5. Reach the page where the password can be changed (Account Info/ Account Security/ Change Password).
6. Double click inside the "New Password" / "Confirm Password" fields;
7. On the password manager's drop-down, select the generated password in both fields and change the password.
8. Go to Preferences/Privacy and Security/Login and Passwords/Saved Logins...
   Notice that the generated password is saved inside the Saved Logins list (in the case of Yahoo, only the password is saved, not the username).
   In this case, the username will need to be added manually in the Saved Logins modal.

• A more edgy case, where a password is already saved:

1. Open browser with a new profile and change prefs in about:config :
   "signon.generation.enabled" is the user pref to enable/disable the feature from about:preferences (not implemented yet).
   "signon.generation.available" controls whether the feature is available for users (e.g. if the about:preferences UI should show).
2. Restart browser (not sure if it's required, but I'm not taking any chances)
3. Log into Yahoo (one of the sites where the feature works).
4. On the door-hanger, choose to "Save" the credentials.
5. Reach the page where the password can be changed (Account Info/ Account Security/ Change Password).
6. Double click inside the "New Password" / "Confirm Password" fields;
7. On the password manager's drop-down, select the generated password in both fields and change the password.
   NOTICE that the Password Manager door-hanger appears and offers to save the generated password, but with no username.
8. Go to Preferences/Privacy and Security/Login and Passwords/Saved Logins...
   Notice that the generated password is saved as a new credential, the username will need to be added manually in the Saved Logins modal.

• An even more edgy case, where a generated password is already saved:

1. Open browser with a new profile and change prefs in about:config :
   "signon.generation.enabled" is the user pref to enable/disable the feature from about:preferences (not implemented yet).
   "signon.generation.available" controls whether the feature is available for users (e.g. if the about:preferences UI should show).
2. Restart browser (not sure if it's required, but I'm not taking any chances)
3. Log into Yahoo (one of the sites where the feature works).
4. On the door-hanger, choose to "Save" the credentials.
5. Reach the page where the password can be changed (Account Info/ Account Security/ Change Password).
6. Double click inside the "New Password" / "Confirm Password" fields;
   NOTICE that the generated password is the same it was last time the user used a generated password.
   NOTICE that the Password Manager door-hanger appears and offers to save the generated password, but with no username.
   I believe that this case will be fixed when a different password will be generated every time the user attempts to change his password.

Matt, How do you think we should proceed in this case? Which case is unacceptable?
Should I also test other sites in order to validate this issue? See bug 1548381 for top 10 sites that allow for the password generator feature to work. Thanks.

Comment 11

[Daniel Bodea [:danibodea]]

I am reverifying this implementation now after some other changes were made:

These are the steps I took to verify this implementation, case where no password is being saved for the site in question:

1. Open browser;
2. Log into Yahoo/Google/Pinterest.
3. On the door-hanger, choose "Don't save" the credentials.
4. Reach the page where the password can be changed (Account Info/ Account Security/ Change Password).
5. Fill in the old password manually. (No logins saved at this time)
6. Click inside the "New Password" field;
   Observe: The Password Manager drop-down is displayed with the option to generate a password and the option to "View Saved Logins".
7. On the password manager's drop-down, select the generated password.
   Observe: "Password saved!" message is displayed, the generated password is instantly auto-saved, but without a username (can be checked in the "about:logins" page) and a dismissed pop-up is shown in the bar (a key-like icon displayed in the address bar).
8. Click inside the "Confirm Password" field;
   Observe: The Password Manager drop-down is displayed with the option to fill in the previously saved password (in step 7), the option to generate a password (the same one in the same session in the same site) and the option to "View Saved Logins".
9. On the password manager's drop-down, select the previously saved login or generated password (it's the same one).
   Observe: The Confirm Password field is correctly filled.
10. Click on the "key-like" icon from the address bar.
    Observe: For Google/Pinterest The username is filled inside the dismissed door hanger along with the generated password and the option to "Update" is available, while in the case of Yahoo, the username has to be filled in manually.
11. Click on the "Update" button to add the username to the saved generated password.
    -> Google/Pinterest: Having to update (somewhat) manually by opening the dismissed door hanger and choosing to update is a bit annoying but the functionality is fine. Usability is the issue because some people will not open the dismissed door hanger to update the username!
    In the case of Yahoo, the username has to be typed in manually, which is worse. <-
12. The password change process can be finished correctly.
    This is the whole happy flow of this case when a set of credentials hasn't already been saved before the password change.

A more probable case, where a password is already saved:

1. Open browser.
2. Log into Yahoo/Pinterest/Google.
3. On the door-hanger, choose to "Save" the credentials.
4. Reach the page where the password can be changed (Account Info/ Account Security/ Change Password).
5. Fill in the old password manually or it gets auto-filled.
6. Click inside the "New Password" field;
   Observe: The Password Manager drop-down is displayed with the option to fill in the previously saved password (when logging in), the option to generate a password and the option to "View Saved Logins".
7. On the password manager's drop-down, select the generated password.
   Observe: "Password saved!" message is displayed (->IS THIS INTENDED?<-), the generated password is instantly auto-saved as a NEW ENTRY, but without a username (can be checked in the "about:logins" page) and a dismissed pop-up is shown in the bar (a key-like icon displayed in the address bar).
8. Click inside the "Confirm Password" field;
   Observe: The Password Manager drop-down is displayed with the option to fill in the previously saved password (in step 7) without a username, the "old" login that has a username, the option to generate a password (the same one in the same session in the same site) and the option to "View Saved Logins".
9. On the password manager's drop-down, select the previously saved login or generated password (it's the same one).
   Observe: The Confirm Password field is correctly filled.
10. Click on the "key-like" icon from the address bar.
    Observe: For Google/Pinterest The username is filled inside the dismissed door hanger along with the generated password and the option to "Update" is available, while in the case of Yahoo, the username has to be filled in manually, before clicking "Update".
11. Click on the "Update" button to add the username to the saved generated password.
    -> THE USERNAME HAS NOT BEEN UPDATED, NOR ADDED TO THE NEWLY CREATED CREDENTIAL!!! <-
12. The password change process can be finished correctly.
    This is the whole happy flow of this case when a set of credentials has already been saved before the password change. In the end, the user has 2 logins (the old password and the right username, and the new password with no username).

In conclusion, the functionality has been implemented but it has some problems:

1. When generating a password, the password is instantly saved, but with a blank username. (bug 1569554)
2. After generating a password, the "key-like" icon from the address bar can be clicked so the dismissed pop-up can be displayed and the username can be added to the saved credential set. If the user does not click the icon, the username will not be updated.
3. For some sites, when the dismissed pop-up is opened, it can be observed that the username has not been filled in, and the user has to manually fill it in (occurs on Yahoo, not on Google or Pinterest).
4. THE USERNAME HAS NOT BEEN UPDATED, NOR ADDED TO THE NEWLY CREATED CREDENTIAL!!! after step 11.

Matt, How do you think we should proceed in this case? What's already covered and what's not?
Thanks.

Comment 12

[Adrian Florinescu [:aflorinescu]]

Although this has been landed in Fx69, the Password Generation feature for which this enhancement applies is only targeted for Fx70 and thus disabled by default on Fx69. As the plan from the beginning was to target Fx70, I will mark only 70 as verified and Fx69 as disabled, since IMO that reflects best the current status.

On the verification part, the conclusion from comment 11 is correct and it functions as expected (at least with the Skyline MVP in mind) with the exception of point4:

THE USERNAME HAS NOT BEEN UPDATED, NOR ADDED TO THE NEWLY CREATED CREDENTIAL!!! after step 11.

which has been fixed meanwhile by bug 1560042.

All in all, this has been verified on Ubuntu 16.04/Windows 10 on Nighttly Fx70 2019-08-30.

*Note: Leaving the NI for Matt for feedback purposes on the flag updates.

Comment 13

[Matthew N. [:MattN]]

(In reply to Adrian Florinescu [:adrian_sv] from comment #12)

Although this has been landed in Fx69, the Password Generation feature for which this enhancement applies is only targeted for Fx70 and thus disabled by default on Fx69. As the plan from the beginning was to target Fx70, I will mark only 70 as verified and Fx69 as disabled, since IMO that reflects best the current status.

Sounds good