Bug 1016051 - Support adding a username to a password-only login upon capture. r=MattN
authorBernardo P. Rittmeyer <bernardo@rittme.com>
Wed, 16 Sep 2015 01:04:00 +0200
changeset 295246 105633f695fae3769f9c72832c8b1d19a5941c97
parent 295245 dbfddd6efb5d646d4906d660989bc9d6ed509ab1
child 295247 8d3e78909a643f9a31dd03cdae4a40984591b7e0
push id5245
push userraliiev@mozilla.com
push dateThu, 29 Oct 2015 11:30:51 +0000
treeherdermozilla-beta@dac831dc1bd0 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersMattN
bugs1016051
milestone43.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 1016051 - Support adding a username to a password-only login upon capture. r=MattN
toolkit/components/passwordmgr/LoginManagerParent.jsm
toolkit/components/passwordmgr/nsILoginManagerPrompter.idl
toolkit/components/passwordmgr/nsILoginMetaInfo.idl
toolkit/components/passwordmgr/nsLoginManagerPrompter.js
toolkit/components/passwordmgr/test/notification_common.js
toolkit/components/passwordmgr/test/test_notifications.html
--- a/toolkit/components/passwordmgr/LoginManagerParent.jsm
+++ b/toolkit/components/passwordmgr/LoginManagerParent.jsm
@@ -518,16 +518,20 @@ var LoginManagerParent = {
     if (existingLogin) {
       log("Found an existing login matching this form submission");
 
       // Change password if needed.
       if (existingLogin.password != formLogin.password) {
         log("...passwords differ, prompting to change.");
         prompter = getPrompter();
         prompter.promptToChangePassword(existingLogin, formLogin);
+      } else if (!existingLogin.username && formLogin.username) {
+        log("...empty username update, prompting to change.");
+        prompter = getPrompter();
+        prompter.promptToChangePassword(existingLogin, formLogin);
       } else {
         recordLoginUse(existingLogin);
       }
 
       return;
     }
 
 
--- a/toolkit/components/passwordmgr/nsILoginManagerPrompter.idl
+++ b/toolkit/components/passwordmgr/nsILoginManagerPrompter.idl
@@ -36,18 +36,18 @@ interface nsILoginManagerPrompter : nsIS
    * Ask the user if they want to save a login (Yes, Never, Not Now)
    *
    * @param aLogin
    *        The login to be saved.
    */
   void promptToSavePassword(in nsILoginInfo aLogin);
 
   /**
-   * Ask the user if they want to change a login's password. If the
-   * user consents, modifyLogin() will be called.
+   * Ask the user if they want to change a login's password or username.
+   * If the user consents, modifyLogin() will be called.
    *
    * @param aOldLogin
    *        The existing login (with the old password).
    * @param aNewLogin
    *        The new login.
    */
   void promptToChangePassword(in nsILoginInfo aOldLogin,
                               in nsILoginInfo aNewLogin);
--- a/toolkit/components/passwordmgr/nsILoginMetaInfo.idl
+++ b/toolkit/components/passwordmgr/nsILoginMetaInfo.idl
@@ -35,18 +35,20 @@ interface nsILoginMetaInfo : nsISupports
 
   /**
    * The time, in Unix Epoch milliseconds, when the login was last submitted
    * in a form or used to begin an HTTP auth session.
    */
   attribute unsigned long long timeLastUsed;
 
   /**
-   * The time, in Unix Epoch milliseconds, when the login's password was
-   * last modified.
+   * The time, in Unix Epoch milliseconds, when the login was last modified.
+   *
+   * Contrary to what the name may suggest, this attribute takes into account
+   * not only the password but also the username attribute.
    */
   attribute unsigned long long timePasswordChanged;
 
   /**
    * The number of times the login was submitted in a form or used to begin
    * an HTTP auth session.
    */
   attribute unsigned long timesUsed;
--- a/toolkit/components/passwordmgr/nsLoginManagerPrompter.js
+++ b/toolkit/components/passwordmgr/nsLoginManagerPrompter.js
@@ -372,28 +372,28 @@ LoginManagerPrompter.prototype = {
 
     // XXX We can't prompt with multiple logins yet (bug 227632), so
     // the entered login might correspond to an existing login
     // other than the one we originally selected.
     selectedLogin = this._repickSelectedLogin(foundLogins, aUsername.value);
 
     // If we didn't find an existing login, or if the username
     // changed, save as a new login.
+    let newLogin = Cc["@mozilla.org/login-manager/loginInfo;1"].
+                   createInstance(Ci.nsILoginInfo);
+    newLogin.init(hostname, null, realm,
+                  aUsername.value, aPassword.value, "", "");
     if (!selectedLogin) {
       // add as new
       this.log("New login seen for " + realm);
-      var newLogin = Cc["@mozilla.org/login-manager/loginInfo;1"].
-                     createInstance(Ci.nsILoginInfo);
-      newLogin.init(hostname, null, realm,
-                    aUsername.value, aPassword.value, "", "");
       this._pwmgr.addLogin(newLogin);
     } else if (aPassword.value != selectedLogin.password) {
       // update password
       this.log("Updating password for  " + realm);
-      this._updateLogin(selectedLogin, aPassword.value);
+      this._updateLogin(selectedLogin, newLogin);
     } else {
       this.log("Login unchanged, no further action needed.");
       this._updateLogin(selectedLogin);
     }
 
     return ok;
   },
 
@@ -599,41 +599,40 @@ LoginManagerPrompter.prototype = {
 
       // XXX We can't prompt with multiple logins yet (bug 227632), so
       // the entered login might correspond to an existing login
       // other than the one we originally selected.
       selectedLogin = this._repickSelectedLogin(foundLogins, username);
 
       // If we didn't find an existing login, or if the username
       // changed, save as a new login.
+      let newLogin = Cc["@mozilla.org/login-manager/loginInfo;1"].
+                     createInstance(Ci.nsILoginInfo);
+      newLogin.init(hostname, null, httpRealm,
+                    username, password, "", "");
       if (!selectedLogin) {
         this.log("New login seen for " + username +
                  " @ " + hostname + " (" + httpRealm + ")");
 
-        var newLogin = Cc["@mozilla.org/login-manager/loginInfo;1"].
-                       createInstance(Ci.nsILoginInfo);
-        newLogin.init(hostname, null, httpRealm,
-                      username, password, "", "");
         var notifyObj = this._getPopupNote() || notifyBox;
         if (notifyObj)
           this._showSaveLoginNotification(notifyObj, newLogin);
         else
           this._pwmgr.addLogin(newLogin);
 
       } else if (password != selectedLogin.password) {
 
         this.log("Updating password for " + username +
                  " @ " + hostname + " (" + httpRealm + ")");
         var notifyObj = this._getPopupNote() || notifyBox;
         if (notifyObj)
           this._showChangeLoginNotification(notifyObj,
-                                            selectedLogin, password);
+                                            selectedLogin, newLogin);
         else
-          this._updateLogin(selectedLogin, password);
-
+          this._updateLogin(selectedLogin, newLogin);
       } else {
         this.log("Login unchanged, no further action needed.");
         this._updateLogin(selectedLogin);
       }
     } catch (e) {
       Components.utils.reportError("LoginManagerPrompter: " +
           "Fail2 in promptAuth: " + e + "\n");
     }
@@ -820,17 +819,17 @@ LoginManagerPrompter.prototype = {
                  .classList.remove("popup-notification-invalid-input");
       }
     };
 
     let updateButtonLabel = () => {
       let foundLogins = Services.logins.findLogins({}, login.hostname,
                                                    login.formSubmitURL,
                                                    login.httpRealm);
-      let logins = foundLogins.filter(l => l.username == login.username);
+      let logins = this._filterUpdatableLogins(login, foundLogins);
       let msgNames = (logins.length == 0) ? saveMsgNames : changeMsgNames;
 
       // Update the label based on whether this will be a new login or not.
       let label = this._getLocalizedString(msgNames.buttonLabel);
       let accessKey = this._getLocalizedString(msgNames.buttonAccessKey);
 
       // Update the labels for the next time the panel is opened.
       currentNotification.mainAction.label = label;
@@ -913,33 +912,35 @@ LoginManagerPrompter.prototype = {
       }
       focusedBindingParent.blur();
     };
 
     let persistData = () => {
       let foundLogins = Services.logins.findLogins({}, login.hostname,
                                                    login.formSubmitURL,
                                                    login.httpRealm);
-      let logins = foundLogins.filter(l => l.username == login.username);
+      let logins = this._filterUpdatableLogins(login, foundLogins);
+
       if (logins.length == 0) {
         // The original login we have been provided with might have its own
         // metadata, but we don't want it propagated to the newly created one.
         Services.logins.addLogin(new LoginInfo(login.hostname,
                                                login.formSubmitURL,
                                                login.httpRealm,
                                                login.username,
                                                login.password,
                                                login.usernameField,
                                                login.passwordField));
       } else if (logins.length == 1) {
-        if (logins[0].password == login.password) {
+        if (logins[0].password == login.password &&
+            logins[0].username == login.username) {
           // We only want to touch the login's use count and last used time.
-          this._updateLogin(logins[0], null);
+          this._updateLogin(logins[0]);
         } else {
-          this._updateLogin(logins[0], login.password);
+          this._updateLogin(logins[0], login);
         }
       } else {
         Cu.reportError("Unexpected match of multiple logins.");
       }
     };
 
     // The main action is the "Remember" or "Update" button.
     let mainAction = {
@@ -1187,44 +1188,53 @@ LoginManagerPrompter.prototype = {
       this._pwmgr.addLogin(aLogin);
     } else {
       // userChoice == 1 --> just ignore the login.
       this.log("Ignoring login.");
     }
   },
 
 
-  /*
-   * promptToChangePassword
+  /**
+   * Called when we think we detect a password or username change for
+   * an existing login, when the form being submitted contains multiple
+   * password fields.
    *
-   * Called when we think we detect a password change for an existing
-   * login, when the form being submitted contains multiple password
-   * fields.
-   *
+   * @param {nsILoginInfo} aOldLogin
+   *                       The old login we may want to update.
+   * @param {nsILoginInfo} aNewLogin
+   *                       The new login from the page form.
    */
-  promptToChangePassword : function (aOldLogin, aNewLogin) {
-    var notifyObj = this._getPopupNote() || this._getNotifyBox();
+  promptToChangePassword(aOldLogin, aNewLogin) {
+    let notifyObj = this._getPopupNote() || this._getNotifyBox();
 
-    if (notifyObj)
+    if (notifyObj) {
       this._showChangeLoginNotification(notifyObj, aOldLogin,
-                                        aNewLogin.password);
-    else
-      this._showChangeLoginDialog(aOldLogin, aNewLogin.password);
+                                        aNewLogin);
+    } else {
+      this._showChangeLoginDialog(aOldLogin, aNewLogin);
+    }
   },
 
-
   /*
    * _showChangeLoginNotification
    *
    * Shows the Change Password notification bar or popup notification.
    *
    * @param aNotifyObj
    *        A notification box or a popup notification.
+   *
+   * @param aOldLogin
+   *        The stored login we want to update.
+   *
+   * @param aNewLogin
+   *        The login object with the changes we want to make.
+   *
    */
-  _showChangeLoginNotification : function (aNotifyObj, aOldLogin, aNewPassword) {
+  _showChangeLoginNotification(aNotifyObj, aOldLogin, aNewLogin) {
     var changeButtonText =
           this._getLocalizedString("notifyBarUpdateButtonText");
     var changeButtonAccessKey =
           this._getLocalizedString("notifyBarUpdateButtonAccessKey");
 
     // We reuse the existing message, even if it expects a username, until we
     // switch to the final terminology in bug 1144856.
     var displayHost = this._getShortDisplayHost(aOldLogin.hostname);
@@ -1233,31 +1243,32 @@ LoginManagerPrompter.prototype = {
 
     // The callbacks in |buttons| have a closure to access the variables
     // in scope here; set one to |this._pwmgr| so we can get back to pwmgr
     // without a getService() call.
     var self = this;
 
     // Notification is a PopupNotification
     if (aNotifyObj == this._getPopupNote()) {
-      aOldLogin.password = aNewPassword;
+      aOldLogin.password = aNewLogin.password;
+      aOldLogin.username = aNewLogin.username;
       this._showLoginCaptureDoorhanger(aOldLogin, "password-change");
     } else {
       var dontChangeButtonText =
             this._getLocalizedString("notifyBarDontChangeButtonText");
       var dontChangeButtonAccessKey =
             this._getLocalizedString("notifyBarDontChangeButtonAccessKey");
       var buttons = [
         // "Yes" button
         {
           label:     changeButtonText,
           accessKey: changeButtonAccessKey,
           popup:     null,
           callback:  function(aNotifyObj, aButton) {
-            self._updateLogin(aOldLogin, aNewPassword);
+            self._updateLogin(aOldLogin, aNewLogin);
           }
         },
 
         // "No" button
         {
           label:     dontChangeButtonText,
           accessKey: dontChangeButtonAccessKey,
           popup:     null,
@@ -1274,17 +1285,17 @@ LoginManagerPrompter.prototype = {
 
 
   /*
    * _showChangeLoginDialog
    *
    * Shows the Change Password dialog.
    *
    */
-  _showChangeLoginDialog : function (aOldLogin, aNewPassword) {
+  _showChangeLoginDialog(aOldLogin, aNewLogin) {
     const buttonFlags = Ci.nsIPrompt.STD_YES_NO_BUTTONS;
 
     var dialogText;
     if (aOldLogin.username)
       dialogText  = this._getLocalizedString(
                               "updatePasswordMsg",
                               [aOldLogin.username]);
     else
@@ -1296,17 +1307,17 @@ LoginManagerPrompter.prototype = {
 
     // returns 0 for yes, 1 for no.
     var ok = !this._promptService.confirmEx(this._window,
                             dialogTitle, dialogText, buttonFlags,
                             null, null, null,
                             null, {});
     if (ok) {
       this.log("Updating password for user " + aOldLogin.username);
-      this._updateLogin(aOldLogin, aNewPassword);
+      this._updateLogin(aOldLogin, aNewLogin);
     }
   },
 
 
   /*
    * promptToChangePasswordWithUsernames
    *
    * Called when we detect a password change in a form submission, but we
@@ -1332,37 +1343,35 @@ LoginManagerPrompter.prototype = {
     var ok = this._promptService.select(this._window,
                             dialogTitle, dialogText,
                             usernames.length, usernames,
                             selectedIndex);
     if (ok) {
       // Now that we know which login to use, modify its password.
       var selectedLogin = logins[selectedIndex.value];
       this.log("Updating password for user " + selectedLogin.username);
-      this._updateLogin(selectedLogin, aNewLogin.password);
+      this._updateLogin(selectedLogin, aNewLogin);
     }
   },
 
 
 
 
   /* ---------- Internal Methods ---------- */
 
 
 
 
-  /*
-   * _updateLogin
-   */
-  _updateLogin : function (login, newPassword) {
+  _updateLogin(login, aNewLogin = null) {
     var now = Date.now();
     var propBag = Cc["@mozilla.org/hash-property-bag;1"].
                   createInstance(Ci.nsIWritablePropertyBag);
-    if (newPassword) {
-      propBag.setProperty("password", newPassword);
+    if (aNewLogin) {
+      propBag.setProperty("password", aNewLogin.password);
+      propBag.setProperty("username", aNewLogin.username);
       // Explicitly set the password change time here (even though it would
       // be changed automatically), to ensure that it's exactly the same
       // value as timeLastUsed.
       propBag.setProperty("timePasswordChanged", now);
     }
     propBag.setProperty("timeLastUsed", now);
     propBag.setProperty("timesUsedIncrement", 1);
     this._pwmgr.modifyLogin(login, propBag);
@@ -1715,17 +1724,38 @@ LoginManagerPrompter.prototype = {
       callback: aCallback,
       context: aContext,
       cancel: function() {
         this.callback.onAuthCancelled(this.context, false);
         this.callback = null;
         this.context = null;
       }
     };
-  }
+  },
+
+  /**
+   * This function looks for existing logins that can be updated
+   * to match a submitted login, instead of creating a new one.
+   *
+   * Given a login and a loginList, it filters the login list
+   * to find every login with either the same username as aLogin
+   * or with the same password as aLogin and an empty username
+   * so the user can add a username.
+   *
+   * @param {nsILoginInfo} aLogin
+   *                       login to use as filter.
+   * @param {nsILoginInfo[]} aLoginList
+   *                         Array of logins to filter.
+   * @returns {nsILoginInfo[]} the filtered array of logins.
+   */
+  _filterUpdatableLogins(aLogin, aLoginList) {
+    return aLoginList.filter(l => l.username == aLogin.username ||
+                             (l.password == aLogin.password &&
+                              !l.username));
+  },
 
 }; // end of LoginManagerPrompter implementation
 
 XPCOMUtils.defineLazyGetter(this.LoginManagerPrompter.prototype, "log", () => {
   let logger = LoginHelper.createLogger("LoginManagerPrompter");
   return logger.log.bind(logger);
 });
 
--- a/toolkit/components/passwordmgr/test/notification_common.js
+++ b/toolkit/components/passwordmgr/test/notification_common.js
@@ -28,25 +28,32 @@ function getPopupNotifications(aWindow) 
                                  .QueryInterface(Ci.nsIDocShell)
                                  .chromeEventHandler.ownerDocument.defaultView;
 
     var popupNotifications = chromeWin.PopupNotifications;
     return popupNotifications;
 }
 
 
-/*
- * getPopup
+/**
+ * Checks if we have a password popup notification
+ * of the right type and with the right label.
  *
+ * @returns the found password popup notification.
  */
 function getPopup(aPopupNote, aKind) {
     ok(true, "Looking for " + aKind + " popup notification");
     var notification = aPopupNote.getNotification("password");
     if (notification) {
-      is(notification.options.passwordNotificationType, aKind);
+      is(notification.options.passwordNotificationType, aKind, "Notification type matches.");
+      if (aKind == "password-change") {
+        is(notification.mainAction.label, "Update", "Main action label matches update doorhanger.");
+      } else if (aKind == "password-save") {
+        is(notification.mainAction.label, "Remember", "Main action label matches save doorhanger.");
+      }
     }
     return notification;
 }
 
 
 /*
  * clickPopupButton
  *
--- a/toolkit/components/passwordmgr/test/test_notifications.html
+++ b/toolkit/components/passwordmgr/test/test_notifications.html
@@ -241,21 +241,22 @@ function checkTest() {
         popup = getPopup(popupNotifications, "password-save");
         ok(!popup, "checking for no notification popup");
 
         // Add login for the next test.
         pwmgr.addLogin(login2);
         break;
 
       case 13:
-        // Check for no notification popup when existing pw-only login matches form.
+        // Check for update popup when existing pw-only login matches form.
         is(gotUser, "notifyu1", "Checking submitted username");
         is(gotPass, "notifyp1", "Checking submitted password");
-        popup = getPopup(popupNotifications, "password-save");
-        ok(!popup, "checking for no notification popup");
+        popup = getPopup(popupNotifications, "password-change");
+        ok(popup, "checking for notification popup");
+        popup.remove();
         pwmgr.removeLogin(login2);
 
         // Add login for the next test
         pwmgr.addLogin(login1);
         break;
 
       case 14:
         // Check for no notification popup when pw-only form matches existing login.