Bug 1348791 - Add a timeout to master password prompt when searching logins. r=MattN draft
authorJohann Hofmann <jhofmann@mozilla.com>
Mon, 08 May 2017 06:32:09 -0400
changeset 574092 6f4d2c59360963f53972b812d999756637434415
parent 573811 17d8a1e278a9c54a6fdda9d390abce4077e55b20
child 574093 9c7aa29301e06816a7d9fcf91a7511d46418bb1f
push id57587
push userbmo:jhofmann@mozilla.com
push dateMon, 08 May 2017 10:32:42 +0000
reviewersMattN
bugs1348791
milestone55.0a1
Bug 1348791 - Add a timeout to master password prompt when searching logins. r=MattN The login manager searching logins for autofill may trap the user in an infinite loop of master password prompts until the user enters the correct master password. To prevent that, we're adding a timeout to showing the master password prompt for autofill after it was last cancelled. MozReview-Commit-ID: JcmTDU6CKKA
modules/libpref/init/all.js
toolkit/components/passwordmgr/LoginManagerParent.jsm
--- a/modules/libpref/init/all.js
+++ b/modules/libpref/init/all.js
@@ -4429,16 +4429,18 @@ pref("signon.rememberSignons.visibilityT
 pref("signon.autofillForms",                true);
 pref("signon.autofillForms.http",           false);
 pref("signon.autologin.proxy",              false);
 pref("signon.formlessCapture.enabled",      true);
 pref("signon.storeWhenAutocompleteOff",     true);
 pref("signon.debug",                        false);
 pref("signon.recipes.path",                 "chrome://passwordmgr/content/recipes.json");
 pref("signon.schemeUpgrades",               false);
+// This temporarily prevents the master password to reprompt for autocomplete.
+pref("signon.masterPasswordReprompt.timeout_ms", 900000); // 15 Minutes
 
 // Satchel (Form Manager) prefs
 pref("browser.formfill.debug",            false);
 pref("browser.formfill.enable",           true);
 pref("browser.formfill.expire_days",      180);
 pref("browser.formfill.saveHttpsForms",   true);
 pref("browser.formfill.agedWeight",       2);
 pref("browser.formfill.bucketSize",       1);
--- a/toolkit/components/passwordmgr/LoginManagerParent.jsm
+++ b/toolkit/components/passwordmgr/LoginManagerParent.jsm
@@ -31,16 +31,47 @@ var LoginManagerParent = {
    * synchronous access. This is a temporary hack and new consumers should yield on
    * recipeParentPromise instead.
    *
    * @type LoginRecipesParent
    * @deprecated
    */
   _recipeManager: null,
 
+  // Tracks the last time the user cancelled the master password prompt,
+  // to avoid spamming master password prompts on autocomplete searches.
+  _lastMPLoginCancelled: Math.NEGATIVE_INFINITY,
+
+  _searchAndDedupeLogins(formOrigin, actionOrigin) {
+    let logins;
+    try {
+      logins = LoginHelper.searchLoginsWithObject({
+        hostname: formOrigin,
+        formSubmitURL: actionOrigin,
+        schemeUpgrades: LoginHelper.schemeUpgrades,
+      });
+    } catch (e) {
+      // Record the last time the user cancelled the MP prompt
+      // to avoid spamming them with MP prompts for autocomplete.
+      if (e.result == Cr.NS_ERROR_ABORT) {
+        log("User cancelled master password prompt.");
+        this._lastMPLoginCancelled = Date.now();
+        return [];
+      }
+      throw e;
+    }
+
+    // Dedupe so the length checks below still make sense with scheme upgrades.
+    let resolveBy = [
+      "scheme",
+      "timePasswordChanged",
+    ];
+    return LoginHelper.dedupeLogins(logins, ["username"], resolveBy, formOrigin);
+  },
+
   // This should only be called on Android. Listeners are added in
   // nsBrowserGlue.js on desktop. Please make sure that the list of
   // listeners added here stays in sync with the listeners added in
   // nsBrowserGlue when you change either.
   init() {
     let mm = Cc["@mozilla.org/globalmessagemanager;1"]
                .getService(Ci.nsIMessageListenerManager);
     // PLEASE KEEP THIS LIST IN SYNC WITH THE LISTENERS ADDED IN nsBrowserGlue
@@ -196,26 +227,18 @@ var LoginManagerParent = {
       // never return). We should guarantee that at least one of these
       // will fire.
       // See bug XXX.
       Services.obs.addObserver(observer, "passwordmgr-crypto-login");
       Services.obs.addObserver(observer, "passwordmgr-crypto-loginCanceled");
       return;
     }
 
-    let logins = LoginHelper.searchLoginsWithObject({
-      formSubmitURL: actionOrigin,
-      hostname: formOrigin,
-      schemeUpgrades: LoginHelper.schemeUpgrades,
-    });
-    let resolveBy = [
-      "scheme",
-      "timePasswordChanged",
-    ];
-    logins = LoginHelper.dedupeLogins(logins, ["username"], resolveBy, formOrigin);
+    let logins = this._searchAndDedupeLogins(formOrigin, actionOrigin);
+
     log("sendLoginDataToChild:", logins.length, "deduped logins");
     // Convert the array of nsILoginInfo to vanilla JS objects since nsILoginInfo
     // doesn't support structured cloning.
     var jsLogins = LoginHelper.loginsToVanillaObjects(logins);
     target.sendAsyncMessage("RemoteLogins:loginsFound", {
       requestId,
       logins: jsLogins,
       recipes,
@@ -224,39 +247,45 @@ var LoginManagerParent = {
 
   doAutocompleteSearch({ formOrigin, actionOrigin,
                          searchString, previousResult,
                          rect, requestId, isSecure, isPasswordField,
                        }, target) {
     // Note: previousResult is a regular object, not an
     // nsIAutoCompleteResult.
 
+    // Cancel if we unsuccessfully prompted for the master password too recently.
+    if (!Services.logins.isLoggedIn) {
+      let timeDiff = Date.now() - this._lastMPLoginCancelled;
+      if (timeDiff < this._repromptTimeout) {
+        log("Not searching logins for autocomplete since the master password " +
+            `prompt was last cancelled ${Math.round(timeDiff / 1000)} seconds ago.`);
+        // Send an empty array to make LoginManagerContent clear the
+        // outstanding request it has temporarily saved.
+        target.messageManager.sendAsyncMessage("RemoteLogins:loginsAutoCompleted", {
+          requestId,
+          logins: [],
+        });
+        return;
+      }
+    }
+
     let searchStringLower = searchString.toLowerCase();
     let logins;
     if (previousResult &&
         searchStringLower.startsWith(previousResult.searchString.toLowerCase())) {
       log("Using previous autocomplete result");
 
       // We have a list of results for a shorter search string, so just
       // filter them further based on the new search string.
       logins = LoginHelper.vanillaObjectsToLogins(previousResult.logins);
     } else {
       log("Creating new autocomplete search result.");
 
-      // Grab the logins from the database.
-      logins = LoginHelper.searchLoginsWithObject({
-        formSubmitURL: actionOrigin,
-        hostname: formOrigin,
-        schemeUpgrades: LoginHelper.schemeUpgrades,
-      });
-      let resolveBy = [
-        "scheme",
-        "timePasswordChanged",
-      ];
-      logins = LoginHelper.dedupeLogins(logins, ["username"], resolveBy, formOrigin);
+      logins = this._searchAndDedupeLogins(formOrigin, actionOrigin);
     }
 
     let matchingLogins = logins.filter(function(fullMatch) {
       let match = fullMatch.username;
 
       // Remove results that are too short, or have different prefix.
       // Also don't offer empty usernames as possible results except
       // for password field.
@@ -305,30 +334,19 @@ var LoginManagerParent = {
     var formLogin = Cc["@mozilla.org/login-manager/loginInfo;1"].
                     createInstance(Ci.nsILoginInfo);
     formLogin.init(hostname, formSubmitURL, null,
                    (usernameField ? usernameField.value : ""),
                    newPasswordField.value,
                    (usernameField ? usernameField.name : ""),
                    newPasswordField.name);
 
-    let logins = LoginHelper.searchLoginsWithObject({
-      formSubmitURL,
-      hostname,
-      schemeUpgrades: LoginHelper.schemeUpgrades,
-    });
-
-    // Dedupe so the length checks below still make sense with scheme upgrades.
     // Below here we have one login per hostPort + action + username with the
     // matching scheme being preferred.
-    let resolveBy = [
-      "scheme",
-      "timePasswordChanged",
-    ];
-    logins = LoginHelper.dedupeLogins(logins, ["username"], resolveBy, hostname);
+    let logins = this._searchAndDedupeLogins(hostname, formSubmitURL);
 
     // If we didn't find a username field, but seem to be changing a
     // password, allow the user to select from a list of applicable
     // logins to update the password for.
     if (!usernameField && oldPasswordField && logins.length > 0) {
       var prompter = getPrompter();
 
       if (logins.length == 1) {
@@ -478,8 +496,11 @@ var LoginManagerParent = {
 
 XPCOMUtils.defineLazyGetter(LoginManagerParent, "recipeParentPromise", function() {
   const { LoginRecipesParent } = Cu.import("resource://gre/modules/LoginRecipes.jsm", {});
   this._recipeManager = new LoginRecipesParent({
     defaults: Services.prefs.getStringPref("signon.recipes.path"),
   });
   return this._recipeManager.initializationPromise;
 });
+
+XPCOMUtils.defineLazyPreferenceGetter(LoginManagerParent, "_repromptTimeout",
+  "signon.masterPasswordReprompt.timeout_ms", 900000); // 15 Minutes