Bug 1058438 - Remove disabledHost APIs from storage and use permission manager directly in LoginManager. r=MattN draft
authorSaad Quadri <saad@saadquadri.com>
Tue, 09 Aug 2016 10:14:46 -0700
changeset 398741 82f4cdb2e5a445208e654adb4a54331bf4f2c2d3
parent 395624 6608e5864780589b25d5421c3d3673ab30c4c318
child 398742 12eeefc6057850f34e98b84b13bba64b193845b3
push id25613
push usersaad@saadquadri.com
push dateTue, 09 Aug 2016 17:16:57 +0000
reviewersMattN
bugs1058438
milestone51.0a1
Bug 1058438 - Remove disabledHost APIs from storage and use permission manager directly in LoginManager. r=MattN MozReview-Commit-ID: AheLazqkLoK
toolkit/components/passwordmgr/LoginHelper.jsm
toolkit/components/passwordmgr/nsILoginManagerStorage.idl
toolkit/components/passwordmgr/nsLoginManager.js
toolkit/components/passwordmgr/storage-json.js
toolkit/components/passwordmgr/storage-mozStorage.js
toolkit/forgetaboutsite/ForgetAboutSite.jsm
--- a/toolkit/components/passwordmgr/LoginHelper.jsm
+++ b/toolkit/components/passwordmgr/LoginHelper.jsm
@@ -685,15 +685,35 @@ this.LoginHelper = {
                    getService(Ci.nsIPKCS11ModuleDB);
     let slot = secmodDB.findSlotByName("");
     if (!slot) {
       return false;
     }
     let hasMP = slot.status != Ci.nsIPKCS11Slot.SLOT_UNINITIALIZED &&
                 slot.status != Ci.nsIPKCS11Slot.SLOT_READY;
     return hasMP;
+  },
+
+  /**
+   * Send a notification when stored data is changed.
+   */
+  notifyStorageChanged(changeType, data) {
+    let dataObject = data;
+    // Can't pass a raw JS string or array though notifyObservers(). :-(
+    if (Array.isArray(data)) {
+      dataObject = Cc["@mozilla.org/array;1"].
+                   createInstance(Ci.nsIMutableArray);
+      for (let i = 0; i < data.length; i++) {
+        dataObject.appendElement(data[i], false);
+      }
+    } else if (typeof(data) == "string") {
+      dataObject = Cc["@mozilla.org/supports-string;1"].
+                   createInstance(Ci.nsISupportsString);
+      dataObject.data = data;
+    }
+    Services.obs.notifyObservers(dataObject, "passwordmgr-storage-changed", changeType);
   }
 };
 
 XPCOMUtils.defineLazyGetter(this, "log", () => {
   let logger = LoginHelper.createLogger("LoginHelper");
   return logger;
 });
--- a/toolkit/components/passwordmgr/nsILoginManagerStorage.idl
+++ b/toolkit/components/passwordmgr/nsILoginManagerStorage.idl
@@ -140,58 +140,16 @@ interface nsILoginManagerStorage : nsISu
    *       var logins = pwmgr.searchLogins({}, matchData);
    *       (|logins| is an array).
    */
   void searchLogins(out unsigned long count, in nsIPropertyBag matchData,
                     [retval, array, size_is(count)] out nsILoginInfo logins);
 
 
   /**
-   * Obtain a list of all hosts for which password saving is disabled.
-   *
-   * @param count
-   *        The number of elements in the array. JS callers can simply use
-   *        the array's .length property and omit this param.
-   * @param hostnames
-   *        An array of hostname strings, in origin URL format without a
-   *        pathname. For example: "https://www.site.com".
-   *
-   * NOTE: This can be called from JS as:
-   *       var logins = pwmgr.getAllDisabledHosts();
-   */
-  void getAllDisabledHosts([optional] out unsigned long count,
-                    [retval, array, size_is(count)] out wstring hostnames);
-
-
-  /**
-   * Check to see if saving logins has been disabled for a host.
-   *
-   * @param aHost
-   *        The hostname to check. This argument should be in the origin
-   *        URL format, without a pathname. For example: "http://foo.com".
-   */
-  boolean getLoginSavingEnabled(in AString aHost);
-
-
-  /**
-   * Disable (or enable) storing logins for the specified host. When
-   * disabled, the login manager will not prompt to store logins for
-   * that host. Existing logins are not affected.
-   *
-   * @param aHost
-   *        The hostname to set. This argument should be in the origin
-   *        URL format, without a pathname. For example: "http://foo.com".
-   * @param isEnabled
-   *        Specify if saving logins should be enabled (true) or
-   *        disabled (false)
-   */
-  void setLoginSavingEnabled(in AString aHost, in boolean isEnabled);
-
-
-  /**
    * Search for logins matching the specified criteria. Called when looking
    * for logins that might be applicable to a form or authentication request.
    *
    * @param count
    *        The number of elements in the array. JS callers can simply use
    *        the array's .length property, and supply an dummy object for
    *        this out param. For example: |findLogins({}, hostname, ...)|
    * @param aHostname
--- a/toolkit/components/passwordmgr/nsLoginManager.js
+++ b/toolkit/components/passwordmgr/nsLoginManager.js
@@ -1,16 +1,18 @@
 /* This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 "use strict";
 
 const { classes: Cc, interfaces: Ci, results: Cr, utils: Cu } = Components;
 
+const PERMISSION_SAVE_LOGINS = "login-saving";
+
 Cu.import("resource://gre/modules/AppConstants.jsm");
 Cu.import("resource://gre/modules/XPCOMUtils.jsm");
 Cu.import("resource://gre/modules/Services.jsm");
 Cu.import("resource://gre/modules/Timer.jsm");
 Cu.import("resource://gre/modules/LoginManagerContent.jsm");
 
 XPCOMUtils.defineLazyModuleGetter(this, "Promise",
                                   "resource://gre/modules/Promise.jsm");
@@ -342,24 +344,39 @@ LoginManager.prototype = {
   removeAllLogins() {
     log.debug("Removing all logins");
     this._storage.removeAllLogins();
   },
 
   /**
    * Get a list of all origins for which logins are disabled.
    *
-   * |count| is only needed for XPCOM.
+   * @param {Number} count - only needed for XPCOM.
    *
    * @return {String[]} of disabled origins. If there are no disabled origins,
    *                    the array is empty.
    */
   getAllDisabledHosts(count) {
     log.debug("Getting a list of all disabled origins");
-    return this._storage.getAllDisabledHosts(count);
+
+    let disabledHosts = [];
+    let enumerator = Services.perms.enumerator;
+
+    while (enumerator.hasMoreElements()) {
+      let perm = enumerator.getNext();
+      if (perm.type == PERMISSION_SAVE_LOGINS && perm.capability == Services.perms.DENY_ACTION) {
+        disabledHosts.push(perm.principal.URI.prePath);
+      }
+    }
+
+    if (count)
+      count.value = disabledHosts.length; // needed for XPCOM
+
+    log.debug("getAllDisabledHosts: returning", disabledHosts.length, "disabled hosts.");
+    return disabledHosts;
   },
 
 
   /**
    * Search for the known logins for entries matching the specified criteria.
    */
   findLogins(count, origin, formActionOrigin, httpRealm) {
     log.debug("Searching for logins matching origin:", origin,
@@ -418,31 +435,37 @@ LoginManager.prototype = {
    * Check to see if user has disabled saving logins for the origin.
    */
   getLoginSavingEnabled(origin) {
     log.debug("Checking if logins to", origin, "can be saved.");
     if (!this._remember) {
       return false;
     }
 
-    return this._storage.getLoginSavingEnabled(origin);
+    let uri = Services.io.newURI(origin, null, null);
+    return Services.perms.testPermission(uri, PERMISSION_SAVE_LOGINS) != Services.perms.DENY_ACTION;
   },
 
 
   /**
    * Enable or disable storing logins for the specified origin.
    */
   setLoginSavingEnabled(origin, enabled) {
-    // Nulls won't round-trip with getAllDisabledHosts().
-    if (origin.indexOf("\0") != -1) {
-      throw new Error("Invalid hostname");
+    // Throws if there are bogus values.
+    LoginHelper.checkHostnameValue(origin);
+
+    let uri = Services.io.newURI(origin, null, null);
+    if (enabled) {
+      Services.perms.remove(uri, PERMISSION_SAVE_LOGINS);
+    } else {
+      Services.perms.add(uri, PERMISSION_SAVE_LOGINS, Services.perms.DENY_ACTION);
     }
 
     log.debug("Login saving for", origin, "now enabled?", enabled);
-    return this._storage.setLoginSavingEnabled(origin, enabled);
+    LoginHelper.notifyStorageChanged(enabled ? "hostSavingEnabled" : "hostSavingDisabled", origin);
   },
 
   /**
    * Yuck. This is called directly by satchel:
    * nsFormFillController::StartSearch()
    * [toolkit/components/satchel/nsFormFillController.cpp]
    *
    * We really ought to have a simple way for code to register an
--- a/toolkit/components/passwordmgr/storage-json.js
+++ b/toolkit/components/passwordmgr/storage-json.js
@@ -145,33 +145,33 @@ this.LoginManagerStorage_json.prototype 
       timeCreated:         loginClone.timeCreated,
       timeLastUsed:        loginClone.timeLastUsed,
       timePasswordChanged: loginClone.timePasswordChanged,
       timesUsed:           loginClone.timesUsed
     });
     this._store.saveSoon();
 
     // Send a notification that a login was added.
-    this._sendNotification("addLogin", loginClone);
+    LoginHelper.notifyStorageChanged("addLogin", loginClone);
   },
 
   removeLogin(login) {
     this._store.ensureDataReady();
 
     let [idToDelete, storedLogin] = this._getIdForLogin(login);
     if (!idToDelete)
       throw new Error("No matching logins");
 
     let foundIndex = this._store.data.logins.findIndex(l => l.id == idToDelete);
     if (foundIndex != -1) {
       this._store.data.logins.splice(foundIndex, 1);
       this._store.saveSoon();
     }
 
-    this._sendNotification("removeLogin", storedLogin);
+    LoginHelper.notifyStorageChanged("removeLogin", storedLogin);
   },
 
   modifyLogin(oldLogin, newLoginData) {
     this._store.ensureDataReady();
 
     let [idToModify, oldStoredLogin] = this._getIdForLogin(oldLogin);
     if (!idToModify)
       throw new Error("No matching logins");
@@ -213,17 +213,17 @@ this.LoginManagerStorage_json.prototype 
         loginItem.timeLastUsed = newLogin.timeLastUsed;
         loginItem.timePasswordChanged = newLogin.timePasswordChanged;
         loginItem.timesUsed = newLogin.timesUsed;
         this._store.saveSoon();
         break;
       }
     }
 
-    this._sendNotification("modifyLogin", [oldStoredLogin, newLogin]);
+    LoginHelper.notifyStorageChanged("modifyLogin", [oldStoredLogin, newLogin]);
   },
 
   /**
    * @return {nsILoginInfo[]}
    */
   getAllLogins(count) {
     let [logins, ids] = this._searchLogins({});
 
@@ -371,58 +371,17 @@ this.LoginManagerStorage_json.prototype 
    */
   removeAllLogins() {
     this._store.ensureDataReady();
 
     this.log("Removing all logins");
     this._store.data.logins = [];
     this._store.saveSoon();
 
-    this._sendNotification("removeAllLogins", null);
-  },
-
-  getAllDisabledHosts(count) {
-    this._store.ensureDataReady();
-
-    let disabledHosts = this._store.data.disabledHosts.slice(0);
-
-    this.log("_getAllDisabledHosts: returning", disabledHosts.length, "disabled hosts.");
-    if (count)
-      count.value = disabledHosts.length; // needed for XPCOM
-    return disabledHosts;
-  },
-
-  getLoginSavingEnabled(hostname) {
-    this._store.ensureDataReady();
-
-    this.log("Getting login saving is enabled for", hostname);
-    return this._store.data.disabledHosts.indexOf(hostname) == -1;
-  },
-
-  setLoginSavingEnabled(hostname, enabled) {
-    this._store.ensureDataReady();
-
-    // Throws if there are bogus values.
-    LoginHelper.checkHostnameValue(hostname);
-
-    this.log("Setting login saving enabled for", hostname, "to", enabled);
-    let foundIndex = this._store.data.disabledHosts.indexOf(hostname);
-    if (enabled) {
-      if (foundIndex != -1) {
-        this._store.data.disabledHosts.splice(foundIndex, 1);
-        this._store.saveSoon();
-      }
-    } else {
-      if (foundIndex == -1) {
-        this._store.data.disabledHosts.push(hostname);
-        this._store.saveSoon();
-      }
-    }
-
-    this._sendNotification(enabled ? "hostSavingEnabled" : "hostSavingDisabled", hostname);
+    LoginHelper.notifyStorageChanged("removeAllLogins", null);
   },
 
   findLogins(count, hostname, formSubmitURL, httpRealm) {
     let loginData = {
       hostname: hostname,
       formSubmitURL: formSubmitURL,
       httpRealm: httpRealm
     };
@@ -461,35 +420,16 @@ this.LoginManagerStorage_json.prototype 
     return this._crypto.uiBusy;
   },
 
   get isLoggedIn() {
     return this._crypto.isLoggedIn;
   },
 
   /**
-   * Send a notification when stored data is changed.
-   */
-  _sendNotification(changeType, data) {
-    let dataObject = data;
-    // Can't pass a raw JS string or array though notifyObservers(). :-(
-    if (data instanceof Array) {
-      dataObject = Cc["@mozilla.org/array;1"].
-                   createInstance(Ci.nsIMutableArray);
-      for (let i = 0; i < data.length; i++)
-        dataObject.appendElement(data[i], false);
-    } else if (typeof(data) == "string") {
-      dataObject = Cc["@mozilla.org/supports-string;1"].
-                   createInstance(Ci.nsISupportsString);
-      dataObject.data = data;
-    }
-    Services.obs.notifyObservers(dataObject, "passwordmgr-storage-changed", changeType);
-  },
-
-  /**
    * Returns an array with two items: [id, login]. If the login was not
    * found, both items will be null. The returned login contains the actual
    * stored login (useful for looking at the actual nsILoginMetaInfo values).
    */
   _getIdForLogin(login) {
     let matchData = { };
     for (let field of ["hostname", "formSubmitURL", "httpRealm"])
       if (login[field] != '')
--- a/toolkit/components/passwordmgr/storage-mozStorage.js
+++ b/toolkit/components/passwordmgr/storage-mozStorage.js
@@ -280,17 +280,17 @@ LoginManagerStorage_mozStorage.prototype
       throw new Error("Couldn't write to database, login not added.");
     } finally {
       if (stmt) {
         stmt.reset();
       }
     }
 
     // Send a notification that a login was added.
-    this._sendNotification("addLogin", loginClone);
+    LoginHelper.notifyStorageChanged("addLogin", loginClone);
   },
 
 
   /*
    * removeLogin
    *
    */
   removeLogin : function (login) {
@@ -312,17 +312,17 @@ LoginManagerStorage_mozStorage.prototype
       this.log("_removeLogin failed: " + e.name + " : " + e.message);
       throw new Error("Couldn't write to database, login not removed.");
       transaction.rollback();
     } finally {
       if (stmt) {
         stmt.reset();
       }
     }
-    this._sendNotification("removeLogin", storedLogin);
+    LoginHelper.notifyStorageChanged("removeLogin", storedLogin);
   },
 
 
   /*
    * modifyLogin
    *
    */
   modifyLogin : function (oldLogin, newLoginData) {
@@ -394,17 +394,17 @@ LoginManagerStorage_mozStorage.prototype
       this.log("modifyLogin failed: " + e.name + " : " + e.message);
       throw new Error("Couldn't write to database, login not modified.");
     } finally {
       if (stmt) {
         stmt.reset();
       }
     }
 
-    this._sendNotification("modifyLogin", [oldStoredLogin, newLogin]);
+    LoginHelper.notifyStorageChanged("modifyLogin", [oldStoredLogin, newLogin]);
   },
 
 
   /*
    * getAllLogins
    *
    * Returns an array of nsILoginInfo.
    */
@@ -616,76 +616,17 @@ LoginManagerStorage_mozStorage.prototype
       transaction.rollback();
       throw new Error("Couldn't write to database");
     } finally {
       if (stmt) {
         stmt.reset();
       }
     }
 
-    this._sendNotification("removeAllLogins", null);
-  },
-
-
-  /*
-   * getAllDisabledHosts
-   *
-   */
-  getAllDisabledHosts : function (count) {
-    let disabledHosts = this._queryDisabledHosts(null);
-
-    this.log("_getAllDisabledHosts: returning " + disabledHosts.length + " disabled hosts.");
-    if (count)
-      count.value = disabledHosts.length; // needed for XPCOM
-    return disabledHosts;
-  },
-
-
-  /*
-   * getLoginSavingEnabled
-   *
-   */
-  getLoginSavingEnabled : function (hostname) {
-    this.log("Getting login saving is enabled for " + hostname);
-    return this._queryDisabledHosts(hostname).length == 0
-  },
-
-
-  /*
-   * setLoginSavingEnabled
-   *
-   */
-  setLoginSavingEnabled : function (hostname, enabled) {
-    // Throws if there are bogus values.
-    LoginHelper.checkHostnameValue(hostname);
-
-    this.log("Setting login saving enabled for " + hostname + " to " + enabled);
-    let query;
-    if (enabled)
-      query = "DELETE FROM moz_disabledHosts " +
-              "WHERE hostname = :hostname";
-    else
-      query = "INSERT INTO moz_disabledHosts " +
-              "(hostname) VALUES (:hostname)";
-    let params = { hostname: hostname };
-
-    let stmt
-    try {
-      stmt = this._dbCreateStatement(query, params);
-      stmt.execute();
-    } catch (e) {
-      this.log("setLoginSavingEnabled failed: " + e.name + " : " + e.message);
-      throw new Error("Couldn't write to database");
-    } finally {
-      if (stmt) {
-        stmt.reset();
-      }
-    }
-
-    this._sendNotification(enabled ? "hostSavingEnabled" : "hostSavingDisabled", hostname);
+    LoginHelper.notifyStorageChanged("removeAllLogins", null);
   },
 
 
   /*
    * findLogins
    *
    */
   findLogins : function (count, hostname, formSubmitURL, httpRealm) {
@@ -759,38 +700,16 @@ LoginManagerStorage_mozStorage.prototype
    * isLoggedIn
    */
   get isLoggedIn() {
     return this._crypto.isLoggedIn;
   },
 
 
   /*
-   * _sendNotification
-   *
-   * Send a notification when stored data is changed.
-   */
-  _sendNotification : function (changeType, data) {
-    let dataObject = data;
-    // Can't pass a raw JS string or array though notifyObservers(). :-(
-    if (data instanceof Array) {
-      dataObject = Cc["@mozilla.org/array;1"].
-                   createInstance(Ci.nsIMutableArray);
-      for (let i = 0; i < data.length; i++)
-        dataObject.appendElement(data[i], false);
-    } else if (typeof(data) == "string") {
-      dataObject = Cc["@mozilla.org/supports-string;1"].
-                   createInstance(Ci.nsISupportsString);
-      dataObject.data = data;
-    }
-    Services.obs.notifyObservers(dataObject, "passwordmgr-storage-changed", changeType);
-  },
-
-
-  /*
    * _getIdForLogin
    *
    * Returns an array with two items: [id, login]. If the login was not
    * found, both items will be null. The returned login contains the actual
    * stored login (useful for looking at the actual nsILoginMetaInfo values).
    */
   _getIdForLogin : function (login) {
     let matchData = { };
@@ -818,50 +737,16 @@ LoginManagerStorage_mozStorage.prototype
       break;
     }
 
     return [id, foundLogin];
   },
 
 
   /*
-   * _queryDisabledHosts
-   *
-   * Returns an array of hostnames from the database according to the
-   * criteria given in the argument. If the argument hostname is null, the
-   * result array contains all hostnames
-   */
-  _queryDisabledHosts : function (hostname) {
-    let disabledHosts = [];
-
-    let query = "SELECT hostname FROM moz_disabledHosts";
-    let params = {};
-    if (hostname) {
-      query += " WHERE hostname = :hostname";
-      params = { hostname: hostname };
-    }
-
-    let stmt;
-    try {
-      stmt = this._dbCreateStatement(query, params);
-      while (stmt.executeStep())
-        disabledHosts.push(stmt.row.hostname);
-    } catch (e) {
-      this.log("_queryDisabledHosts failed: " + e.name + " : " + e.message);
-    } finally {
-      if (stmt) {
-        stmt.reset();
-      }
-    }
-
-    return disabledHosts;
-  },
-
-
-  /*
    * _buildConditionsAndParams
    *
    * Adjusts the WHERE conditions and parameters for statements prior to the
    * statement being created. This fixes the cases where nulls are involved
    * and the empty string is supposed to be a wildcard match
    */
   _buildConditionsAndParams : function (hostname, formSubmitURL, httpRealm) {
     let conditions = [], params = {};
--- a/toolkit/forgetaboutsite/ForgetAboutSite.jsm
+++ b/toolkit/forgetaboutsite/ForgetAboutSite.jsm
@@ -139,22 +139,16 @@ this.ForgetAboutSite = {
     // XXXehsan: is there a better way to do this rather than this
     // hacky comparison?
     catch (ex) {
       if (ex.message.indexOf("User canceled Master Password entry") == -1) {
         throw ex;
       }
     }
 
-    // Clear any "do not save for this site" for this domain
-    let disabledHosts = lm.getAllDisabledHosts();
-    for (let i = 0; i < disabledHosts.length; i++)
-      if (hasRootDomain(disabledHosts[i], aDomain))
-        lm.setLoginSavingEnabled(disabledHosts, true);
-
     // Permissions
     let pm = Cc["@mozilla.org/permissionmanager;1"].
              getService(Ci.nsIPermissionManager);
     // Enumerate all of the permissions, and if one matches, remove it
     enumerator = pm.enumerator;
     while (enumerator.hasMoreElements()) {
       let perm = enumerator.getNext().QueryInterface(Ci.nsIPermission);
       try {