Closed Bug 1554005 Opened 3 months ago Closed 3 months ago

Don't offer generated passwords on sites where the user disabled password saving

Categories

(Toolkit :: Password Manager, defect, P2)

defect

Tracking

()

VERIFIED FIXED
mozilla69
Tracking Status
firefox69 --- verified

People

(Reporter: MattN, Assigned: sfoster)

References

(Blocks 1 open bug, )

Details

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

Attachments

(1 file)

If the user disabled login saving for a site, we shouldn't offer a generated password for them as it won't get persisted for them to log in when they return to the site.

We can probably add a check of Services.logins.getLoginSavingEnabled(…) at https://searchfox.org/mozilla-central/rev/662de518b1686c4769320d6b8825ce4864c4eda0/toolkit/components/passwordmgr/LoginManagerParent.jsm#316-317

Flags: qe-verify+

Do we want to check Services.logins.getLoginSavingEnabled for the form document origin, the form action origin, or both?

Flags: needinfo?(MattN+bmo)
Assignee: nobody → sfoster
Status: NEW → ASSIGNED

(In reply to Sam Foster [:sfoster] (he/him) from comment #1)

Do we want to check Services.logins.getLoginSavingEnabled for the form document origin, the form action origin, or both?

We only only ever use that API on the form/document origin so I would keep doing the same.

Flags: needinfo?(MattN+bmo)
Pushed by sfoster@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/8486ebd0eb20
Check login-saving is enabled before offering a generated password for a form. r=MattN
Status: ASSIGNED → RESOLVED
Closed: 3 months ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla69

Hi guys!
Do you have any steps to reproduce this issue? Do we need to change some variables in about:config?

Flags: needinfo?(MattN+bmo)

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

These are the steps I took to verify this implementation:

  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 to save the credentials, select "Never save" to block the door-hanger on this website.
  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 Password Manager's drop-down is not displayed at all.

A more edgy case:

  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 Password Manager door-hanger, choose to save the password.
  5. Go to browser's Preferences/Privacy and Security/Logins and Passwords/Exceptions... and add "https://login.yahoo.com" to the exceptions list and close the Exceptions modal by the "Save Changes" button.
  6. Reach the page where the password can be changed (Account Info/ Account Security/ Change Password).
  7. Double click inside the "New Password" / "Confirm Password" fields;
    Notice that the Password Manged drop-down offers the option to fill the previously saved password, but it does NOT offer the option to input a generated password.

Matt, do you think this verification suffices? Thanks.

Hi Bodea Daniel [:danibodea]!

Thank you for the information.
Besides that and in order to be sure about fixed, could you please add in which version this happened and what was the actual result.
Maybe you can add some screenshots or video recorder in order to see how the issue was.

Flags: needinfo?(daniel.bodea)

(In reply to Bodea Daniel [:danibodea] from comment #7)

  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 to save the credentials, select "Never save" to block the door-hanger on this website.
  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 Password Manager's drop-down is not displayed at all.

This is a good verification of the patch.

A more edgy case:

  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 Password Manager door-hanger, choose to save the password.
  5. Go to browser's Preferences/Privacy and Security/Logins and Passwords/Exceptions... and add "https://login.yahoo.com" to the exceptions list and close the Exceptions modal by the "Save Changes" button.
  6. Reach the page where the password can be changed (Account Info/ Account Security/ Change Password).
  7. Double click inside the "New Password" / "Confirm Password" fields;y sh
    Notice that the Password Manged drop-down offers the option to fill the previously saved password, but it does NOT offer the option to input a generated password.

We wouldn't expect to see the generated password option here as there is an existing matching login. But, yes, it does confirm that we only offer the generated password when appropriate, not just for every autocomplete="new-password" field. Thanks!

Flags: needinfo?(MattN+bmo)

I have reproduced the issue in Nightly v69.0a1 (2019-05-23) with the following steps:

  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 to save the credentials, select "Never save" to block the door-hanger on this website.
  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 Password Manager's drop-down IS displayed.

I have also reproduced the issue in Nightly v69.0a1 (2019-05-23) with these more edgy steps:

  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 Password Manager door-hanger, choose to save the password.
  5. Go to browser's Preferences/Privacy and Security/Logins and Passwords/Exceptions... and add "https://login.yahoo.com" to the exceptions list and close the Exceptions modal by the "Save Changes" button.
  6. Reach the page where the password can be changed (Account Info/ Account Security/ Change Password).
  7. Double click inside the "New Password" / "Confirm Password" fields;
    NOTICE that the Password Manged drop-down offers the option to fill the previously saved password AND the option to input a generated password.

The initial issue was that the password generator feature UI would appear when the user would block the use of the password manager on a website. It is now fixed.
I'll be frank, I did not waste time on reproducing it the first time because I was sure the issue would reproduce otherwise the bug would not have been logged. I have verified the fix with comment 7, on Nightly v69.0a1 (2019-06-03).

I deem this issue verified. Thank you.

Status: RESOLVED → VERIFIED
Flags: qe-verify+
Flags: needinfo?(daniel.bodea)
Whiteboard: [passwords:generation] [skyline]
You need to log in before you can comment on or make changes to this bug.