Bug 1566568 - Don't auto-save username along with generated passwords. r=MattN
authorTim Nguyen <ntim.bugs@gmail.com>
Fri, 19 Jul 2019 23:45:49 +0000
changeset 483603 8a7b8a8a93daa4160805c37823fc353bb2b5f968
parent 483602 a62403f0bb88179aeef39d2c172dd2711b61005b
child 483604 74908c04ec0ed6756ce2163e9392d18be0c8c3d2
push id90485
push userntim.bugs@gmail.com
push dateFri, 19 Jul 2019 23:47:08 +0000
treeherderautoland@8a7b8a8a93da [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersMattN
bugs1566568
milestone70.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1566568 - Don't auto-save username along with generated passwords. r=MattN Differential Revision: https://phabricator.services.mozilla.com/D38250
toolkit/components/passwordmgr/LoginManagerParent.jsm
toolkit/components/passwordmgr/test/browser/browser_generated_password_doorhangers.js
--- a/toolkit/components/passwordmgr/LoginManagerParent.jsm
+++ b/toolkit/components/passwordmgr/LoginManagerParent.jsm
@@ -4,16 +4,22 @@
 
 "use strict";
 
 const { XPCOMUtils } = ChromeUtils.import(
   "resource://gre/modules/XPCOMUtils.jsm"
 );
 const { Services } = ChromeUtils.import("resource://gre/modules/Services.jsm");
 
+const LoginInfo = new Components.Constructor(
+  "@mozilla.org/login-manager/loginInfo;1",
+  Ci.nsILoginInfo,
+  "init"
+);
+
 XPCOMUtils.defineLazyGlobalGetters(this, ["URL"]);
 
 ChromeUtils.defineModuleGetter(
   this,
   "AutoCompletePopup",
   "resource://gre/modules/AutoCompletePopup.jsm"
 );
 ChromeUtils.defineModuleGetter(
@@ -518,20 +524,17 @@ this.LoginManagerParent = {
       Services.logins.modifyLogin(login, propBag);
     }
 
     if (!Services.logins.getLoginSavingEnabled(origin)) {
       log("(form submission ignored -- saving is disabled for:", origin, ")");
       return;
     }
 
-    let formLogin = Cc["@mozilla.org/login-manager/loginInfo;1"].createInstance(
-      Ci.nsILoginInfo
-    );
-    formLogin.init(
+    let formLogin = new LoginInfo(
       origin,
       formActionOrigin,
       null,
       usernameField ? usernameField.value : "",
       newPasswordField.value,
       usernameField ? usernameField.name : "",
       newPasswordField.name
     );
@@ -696,20 +699,33 @@ this.LoginManagerParent = {
       log("_onGeneratedPasswordFilledOrEdited: The password field is empty");
       return;
     }
     if (passwordInField != password) {
       // The user edited the field after generation to a non-empty value.
       log("The field containing the generated password has changed");
       return;
     }
-    let formLogin = Cc["@mozilla.org/login-manager/loginInfo;1"].createInstance(
-      Ci.nsILoginInfo
+
+    let formLogin = new LoginInfo(
+      formOrigin,
+      formActionOrigin,
+      null,
+      username,
+      password
     );
-    formLogin.init(formOrigin, formActionOrigin, null, username, password);
+
+    let formLoginWithoutUsername = new LoginInfo(
+      formOrigin,
+      formActionOrigin,
+      null,
+      "",
+      password
+    );
+
     let shouldSaveLogin = true;
 
     // This will throw if we can't look up the entry in the password/origin map
     if (!generatedPW.filled) {
       // record first use of this generated password
       Services.telemetry.recordEvent(
         "pwmgr",
         "autocomplete_field",
@@ -737,40 +753,40 @@ this.LoginManagerParent = {
     });
 
     if (logins.length > 0) {
       log(
         "_onGeneratedPasswordFilledOrEdited: Login already saved for this site"
       );
       shouldSaveLogin = false;
       for (let login of logins) {
-        if (formLogin.matches(login, false)) {
+        if (formLoginWithoutUsername.matches(login, false)) {
           // This login is already saved so show no new UI.
           log(
             "_onGeneratedPasswordFilledOrEdited: Matching login already saved"
           );
           return;
         }
       }
     }
 
     if (shouldSaveLogin) {
-      Services.logins.addLogin(formLogin);
+      Services.logins.addLogin(formLoginWithoutUsername);
     }
     log(
       "_onGeneratedPasswordFilledOrEdited: show dismissed save-password notification"
     );
     let browser = browsingContext.top.embedderElement;
     let prompter = this._getPrompter(browser, openerTopWindowID);
 
     if (shouldSaveLogin) {
       // If we auto-saved the login then show a change doorhanger to allow
       // modifying it e.g. adding a username.
       prompter.promptToChangePassword(
-        formLogin,
+        formLoginWithoutUsername,
         formLogin,
         true, // dimissed prompt
         shouldSaveLogin // notifySaved
       );
       return;
     }
     prompter.promptToSavePassword(
       formLogin,
--- a/toolkit/components/passwordmgr/test/browser/browser_generated_password_doorhangers.js
+++ b/toolkit/components/passwordmgr/test/browser/browser_generated_password_doorhangers.js
@@ -133,16 +133,21 @@ add_task(async function autocomplete_gen
           // We'll reuse the form_basic.html, but ensure we'll get the generated password autocomplete option
           passwordInput.setAttribute("autocomplete", "new-password");
           passwordInput.value = "";
           let usernameInput = content.document.querySelector(usernameSelector);
           usernameInput.setUserInput("user1");
         }
       );
 
+      let storageChangedPromise = TestUtils.topicObserved(
+        "passwordmgr-storage-changed",
+        (_, data) => data == "addLogin"
+      );
+
       let confirmationHint = document.getElementById("confirmation-hint");
       let hintPromiseShown = BrowserTestUtils.waitForEvent(
         confirmationHint,
         "popupshown"
       );
       await fillGeneratedPasswordFromACPopup(browser, passwordInputSelector);
       await ContentTask.spawn(
         browser,
@@ -152,31 +157,36 @@ add_task(async function autocomplete_gen
           is(
             passwordInput.value.length,
             15,
             "Password field was filled with generated password"
           );
         }
       );
 
+      let [{ username, password }] = await storageChangedPromise;
       // Make sure confirmation hint was shown
       await hintPromiseShown;
 
       Assert.equal(
         confirmationHint.anchorNode.id,
         "password-notification-icon",
         "Hint should be anchored on the password notification icon"
       );
 
       let hintPromiseHidden = BrowserTestUtils.waitForEvent(
         confirmationHint,
         "popuphidden"
       );
       await hintPromiseHidden;
 
+      // Check properties of the newly auto-saved login
+      is(username, "", "Saved login should have no username");
+      is(password.length, 15, "Saved login should have generated password");
+
       // check a dismissed prompt was shown with extraAttr attribute
       let notif = getCaptureDoorhanger("password-change");
       ok(notif && notif.dismissed, "Dismissed notification was created");
       is(
         notif.anchorElement.getAttribute("extraAttr"),
         "attention",
         "Check if icon has the extraAttr attribute"
       );