Bug 846200 - Support for granting settings permissions on a per-permission basis; r=bent
☠☠ backed out by 7c97c5f7a05e ☠ ☠
authorKyle Machulis <kyle@nonpolynomial.com>
Wed, 27 Aug 2014 21:01:29 -0700
changeset 223755 b7a7dfbe4e3faba12323a7350b11eb635541dcd0
parent 223754 c6f54d821c11ee5b2d71281cb8a37d680a32618e
child 223756 b89c84a850f936bf4436030162c68d2860dca74b
push id3979
push userraliiev@mozilla.com
push dateMon, 13 Oct 2014 16:35:44 +0000
treeherdermozilla-beta@30f2cc610691 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbent
bugs846200
milestone34.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 846200 - Support for granting settings permissions on a per-permission basis; r=bent
dom/apps/src/PermissionsInstaller.jsm
dom/apps/src/PermissionsTable.jsm
dom/base/Navigator.cpp
dom/settings/SettingsManager.js
--- a/dom/apps/src/PermissionsInstaller.jsm
+++ b/dom/apps/src/PermissionsInstaller.jsm
@@ -25,30 +25,16 @@ const READCREATE = "readcreate";
 const READWRITE = "readwrite";
 
 const PERM_TO_STRING = ["unknown", "allow", "deny", "prompt"];
 
 function debug(aMsg) {
   //dump("-*-*- PermissionsInstaller.jsm : " + aMsg + "\n");
 }
 
-// An array carring all the possible (expanded) permission names.
-let AllPossiblePermissions = [];
-for (let permName in PermissionsTable) {
-  let expandedPermNames = [];
-  if (PermissionsTable[permName].access) {
-    expandedPermNames = expandPermissions(permName, READWRITE);
-  } else {
-    expandedPermNames = expandPermissions(permName);
-  }
-  AllPossiblePermissions = AllPossiblePermissions.concat(expandedPermNames);
-  AllPossiblePermissions =
-    AllPossiblePermissions.concat(["offline-app", "pin-app"]);
-}
-
 this.PermissionsInstaller = {
   /**
    * Install permissisions or remove deprecated permissions upon re-install.
    * @param object aApp
    *        The just-installed app configuration.
    *        The properties used are manifestURL, origin and manifest.
    * @param boolean aIsReinstall
    *        Indicates the app was just re-installed
--- a/dom/apps/src/PermissionsTable.jsm
+++ b/dom/apps/src/PermissionsTable.jsm
@@ -7,17 +7,18 @@
 const Ci = Components.interfaces;
 const Cu = Components.utils;
 
 this.EXPORTED_SYMBOLS = [
   "PermissionsTable",
   "PermissionsReverseTable",
   "expandPermissions",
   "appendAccessToPermName",
-  "isExplicitInPermissionsTable"
+  "isExplicitInPermissionsTable",
+  "AllPossiblePermissions"
 ];
 
 // Permission access flags
 const READONLY = "readonly";
 const CREATEONLY = "createonly";
 const READCREATE = "readcreate";
 const READWRITE = "readwrite";
 
@@ -144,17 +145,17 @@ this.PermissionsTable =  { geolocation: 
                             privileged: ALLOW_ACTION,
                             certified: ALLOW_ACTION
                            },
                            settings: {
                              app: DENY_ACTION,
                              privileged: DENY_ACTION,
                              certified: ALLOW_ACTION,
                              access: ["read", "write"],
-                             additional: ["indexedDB-chrome-settings"]
+                             additional: ["indexedDB-chrome-settings", "settings-api"]
                            },
                            permissions: {
                              app: DENY_ACTION,
                              privileged: DENY_ACTION,
                              certified: ALLOW_ACTION
                            },
                            phonenumberservice: {
                              app: DENY_ACTION,
@@ -491,40 +492,44 @@ this.expandPermissions = function expand
     if (tableEntry.additional) {
       expandedPermNames = expandedPermNames.concat(tableEntry.additional);
     }
   }
 
   return expandedPermNames;
 };
 
-this.PermissionsReverseTable = (function () {
+this.PermissionsReverseTable = {};
+this.AllPossiblePermissions = [];
+
+(function () {
   // PermissionsTable as it is works well for direct searches, but not
   // so well for reverse ones (that is, if I get something like
   // device-storage:music-read or indexedDB-chrome-settings-read how
   // do I know which permission it really is? Hence this table is
   // born. The idea is that
   // reverseTable[device-storage:music-read] should return
   // device-storage:music
-  let reverseTable = {};
-
+  //
+  // We also need a list of all the possible permissions for things like the
+  // settingsmanager, so construct that while we're at it.
   for (let permName in PermissionsTable) {
     let permAliases;
     if (PermissionsTable[permName].access) {
       permAliases = expandPermissions(permName, "readwrite");
     } else {
       permAliases = expandPermissions(permName);
     }
     for (let i = 0; i < permAliases.length; i++) {
-      reverseTable[permAliases[i]] = permName;
+      PermissionsReverseTable[permAliases[i]] = permName;
+      AllPossiblePermissions.push(permAliases[i]);
     }
   }
-
-  return reverseTable;
-
+  AllPossiblePermissions =
+    AllPossiblePermissions.concat(["offline-app", "pin-app"]);
 })();
 
 this.isExplicitInPermissionsTable = function(aPermName, aIntStatus) {
 
   // Check to see if the 'webapp' is app/privileged/certified.
   let appStatus;
   switch (aIntStatus) {
     case Ci.nsIPrincipal.APP_STATUS_CERTIFIED:
--- a/dom/base/Navigator.cpp
+++ b/dom/base/Navigator.cpp
@@ -2104,18 +2104,18 @@ Navigator::DoNewResolve(JSContext* aCx, 
       // we don't want to define an interface on the Xray if it's disabled in
       // the target global, even if it's enabled in the Xray's global.
       if (name_struct->mConstructorEnabled &&
           !(*name_struct->mConstructorEnabled)(aCx, naviObj)) {
         return true;
       }
 
       if (name.EqualsLiteral("mozSettings")) {
-        bool hasPermission = CheckPermission("settings-read") ||
-                             CheckPermission("settings-write");
+        bool hasPermission = CheckPermission("settings-api-read") ||
+          CheckPermission("settings-api-write");
         if (!hasPermission) {
           FillPropertyDescriptor(aDesc, aObject, JS::NullValue(), false);
           return true;
         }
       }
 
       if (name.EqualsLiteral("mozDownloadManager")) {
         if (!CheckPermission("downloads")) {
--- a/dom/settings/SettingsManager.js
+++ b/dom/settings/SettingsManager.js
@@ -12,16 +12,17 @@ function debug(s) {
 const Cc = Components.classes;
 const Ci = Components.interfaces;
 const Cu = Components.utils;
 
 Cu.import("resource://gre/modules/SettingsQueue.jsm");
 Cu.import("resource://gre/modules/SettingsDB.jsm");
 Cu.import("resource://gre/modules/XPCOMUtils.jsm");
 Cu.import("resource://gre/modules/Services.jsm");
+Cu.import("resource://gre/modules/PermissionsTable.jsm");
 
 XPCOMUtils.defineLazyServiceGetter(this, "cpmm",
                                    "@mozilla.org/childprocessmessagemanager;1",
                                    "nsIMessageSender");
 XPCOMUtils.defineLazyServiceGetter(this, "mrm",
                                    "@mozilla.org/memory-reporter-manager;1",
                                    "nsIMemoryReporterManager");
 
@@ -61,17 +62,17 @@ SettingsLock.prototype = {
         case "clear":
           let clearReq = store.clear();
           clearReq.onsuccess = function() {
             this._open = true;
             Services.DOMRequest.fireSuccess(request, 0);
             this._open = false;
           }.bind(lock);
           clearReq.onerror = function() {
-            Services.DOMRequest.fireError(request, 0)
+            Services.DOMRequest.fireError(request, 0);
           };
           break;
         case "set":
           let keys = Object.getOwnPropertyNames(info.settings);
           if (keys.length) {
             lock._isBusy = true;
           }
           for (let i = 0; i < keys.length; i++) {
@@ -100,30 +101,30 @@ SettingsLock.prototype = {
                   lock._open = true;
                   Services.DOMRequest.fireSuccess(request, 0);
                   lock._open = false;
                 }
               };
 
               setReq.onerror = function() {
                 if (!request.error) {
-                  Services.DOMRequest.fireError(request, setReq.error.name)
+                  Services.DOMRequest.fireError(request, setReq.error.name);
                 }
               };
 
               if (last) {
                 lock._isBusy = false;
                 if (!lock._requests.isEmpty()) {
                   lock.process();
                 }
               }
             };
             checkKeyRequest.onerror = function(event) {
               if (!request.error) {
-                Services.DOMRequest.fireError(request, checkKeyRequest.error.name)
+                Services.DOMRequest.fireError(request, checkKeyRequest.error.name);
               }
             };
           }
           // Don't break here, instead return. Once the previous requests have
           // finished this loop will start again.
           return;
         case "get":
           let getReq = (info.name === "*") ? store.mozGetAll()
@@ -147,28 +148,28 @@ SettingsLock.prototype = {
             }
 
             this._open = true;
             Services.DOMRequest.fireSuccess(request, this._wrap(results));
             this._open = false;
           }.bind(lock);
 
           getReq.onerror = function() {
-            Services.DOMRequest.fireError(request, 0)
+            Services.DOMRequest.fireError(request, 0);
           };
           break;
       }
     }
   },
 
   createTransactionAndProcess: function() {
     if (DEBUG) debug("database opened, creating transaction");
 
     let manager = this._settingsManager;
-    let transactionType = manager.hasWritePrivileges ? "readwrite" : "readonly";
+    let transactionType = manager.hasAnyWritePrivileges ? "readwrite" : "readonly";
 
     this._transaction =
       manager._settingsDB._db.transaction(SETTINGSSTORE_NAME, transactionType);
 
     this.process();
   },
 
   maybeProcess: function() {
@@ -178,25 +179,24 @@ SettingsLock.prototype = {
   },
 
   get: function get(aName) {
     if (!this._open) {
       dump("Settings lock not open!\n");
       throw Components.results.NS_ERROR_ABORT;
     }
 
-    if (this._settingsManager.hasReadPrivileges) {
+    if (!this._settingsManager.hasReadPermission("settings:" + aName)) {
+      if (DEBUG) debug("get not allowed");
+      throw Components.results.NS_ERROR_NOT_IMPLEMENTED;
+    }
       let req = Services.DOMRequest.createRequest(this._settingsManager._window);
       this._requests.enqueue({ request: req, intent:"get", name: aName });
       this.maybeProcess();
       return req;
-    } else {
-      if (DEBUG) debug("get not allowed");
-      throw Components.results.NS_ERROR_NOT_IMPLEMENTED;
-    }
   },
 
   _serializePreservingBinaries: function _serializePreservingBinaries(aObject) {
     function needsUUID(aValue) {
       if (!aValue || !aValue.constructor) {
         return false;
       }
       return (aValue.constructor.name == "Date") || (aValue instanceof Ci.nsIDOMFile) ||
@@ -227,35 +227,38 @@ SettingsLock.prototype = {
     });
   },
 
   set: function set(aSettings) {
     if (!this._open) {
       throw "Settings lock not open";
     }
 
-    if (this._settingsManager.hasWritePrivileges) {
+    // Check each requested permission to make sure we can set it
+    let keys = Object.getOwnPropertyNames(aSettings);
+    for (let i = 0; i < keys.length; i++) {
+      if (!this._settingsManager.hasWritePermission("settings:" + keys[i])) {
+        if (DEBUG) debug("set not allowed on" + keys[i]);
+        throw "No permission to call set on " + keys[i];
+      }
+    }
       let req = Services.DOMRequest.createRequest(this._settingsManager._window);
       if (DEBUG) debug("send: " + JSON.stringify(aSettings));
       let settings = this._serializePreservingBinaries(aSettings);
       this._requests.enqueue({request: req, intent: "set", settings: settings});
       this.maybeProcess();
       return req;
-    } else {
-      if (DEBUG) debug("set not allowed");
-      throw "No permission to call set";
-    }
   },
 
   clear: function clear() {
     if (!this._open) {
       throw "Settings lock not open";
     }
-
-    if (this._settingsManager.hasWritePrivileges) {
+    // Only certified apps should be allowed to clear
+    if (this._settingsManager.hasFullWritePrivileges) {
       let req = Services.DOMRequest.createRequest(this._settingsManager._window);
       this._requests.enqueue({ request: req, intent: "clear"});
       this.maybeProcess();
       return req;
     } else {
       if (DEBUG) debug("clear not allowed");
       throw "No permission to call clear";
     }
@@ -268,16 +271,17 @@ SettingsLock.prototype = {
 
 function SettingsManager() {
   this._settingsDB = new SettingsDB();
   this._settingsDB.init();
 }
 
 SettingsManager.prototype = {
   _callbacks: null,
+  _perms: [],
 
   _wrap: function _wrap(obj) {
     return Cu.cloneInto(obj, this._window);
   },
 
   set onsettingchange(aHandler) {
     this.__DOM_IMPL__.setEventHandler("onsettingchange", aHandler);
   },
@@ -342,46 +346,72 @@ SettingsManager.prototype = {
     } else {
       this._callbacks[aName].push(aCallback);
     }
   },
 
   removeObserver: function removeObserver(aName, aCallback) {
     if (DEBUG) debug("deleteObserver " + aName);
     if (this._callbacks && this._callbacks[aName]) {
-      let index = this._callbacks[aName].indexOf(aCallback)
+      let index = this._callbacks[aName].indexOf(aCallback);
       if (index != -1) {
-        this._callbacks[aName].splice(index, 1)
+        this._callbacks[aName].splice(index, 1);
       } else {
         if (DEBUG) debug("Callback not found for: " + aName);
       }
     } else {
       if (DEBUG) debug("No observers stored for " + aName);
     }
   },
 
+  hasReadPermission: function hasPermission(aName) {
+    return this.hasFullReadPrivileges || this._perms.indexOf(aName + "-read") != -1;
+  },
+
+  hasWritePermission: function hasPermission(aName) {
+    return this.hasFullWritePrivileges || this._perms.indexOf(aName + "-write") != -1;
+  },
+
   init: function(aWindow) {
     mrm.registerStrongReporter(this);
     cpmm.addMessageListener("Settings:Change:Return:OK", this);
     cpmm.addMessageListener("Settings:Notifier:Init:OK", this);
     this._window = aWindow;
     Services.obs.addObserver(this, "inner-window-destroyed", false);
     let util = aWindow.QueryInterface(Ci.nsIInterfaceRequestor).getInterface(Ci.nsIDOMWindowUtils);
     this.innerWindowID = util.currentInnerWindowID;
 
-    let readPerm = Services.perms.testExactPermissionFromPrincipal(aWindow.document.nodePrincipal, "settings-read");
-    let writePerm = Services.perms.testExactPermissionFromPrincipal(aWindow.document.nodePrincipal, "settings-write");
-    this.hasReadPrivileges = readPerm == Ci.nsIPermissionManager.ALLOW_ACTION;
-    this.hasWritePrivileges = writePerm == Ci.nsIPermissionManager.ALLOW_ACTION;
+
+    this.hasAnyWritePrivileges = false;
 
-    if (this.hasReadPrivileges) {
+    for (let idx in AllPossiblePermissions) {
+      let permName = AllPossiblePermissions[idx];
+      //Check to see if this is a settings permission. All settings permissions
+      //begin with the word settings.
+      if (permName.indexOf("settings") != 0) {
+        continue;
+      }
+      if (Services.perms.testExactPermissionFromPrincipal(this._window.document.nodePrincipal, permName) == Ci.nsIPermissionManager.ALLOW_ACTION) {
+        if(permName.indexOf("-write") > 0) {
+          this.hasAnyWritePrivileges = true;
+        }
+        this._perms.push(permName);
+      }
+    }
+
+    this.hasFullReadPrivileges = this.hasReadPermission("settings");
+    this.hasFullWritePrivileges = this.hasWritePermission("settings");
+
+    if (this.hasFullReadPrivileges) {
       cpmm.sendAsyncMessage("Settings:RegisterForMessages");
     }
 
-    if (!this.hasReadPrivileges && !this.hasWritePrivileges) {
+    // settings-api is an additional setting on all settings permissions. This
+    // is how we can figure out whether to expose the mozSettings DOM object.
+    if (!this.hasReadPermission("settings-api") && !this.hasWritePermission("settings-api")) {
       dump("No settings permission for: " + aWindow.document.nodePrincipal.origin + "\n");
       Cu.reportError("No settings permission for: " + aWindow.document.nodePrincipal.origin);
     }
   },
 
   observe: function(aSubject, aTopic, aData) {
     if (DEBUG) debug("Topic: " + aTopic);
     if (aTopic == "inner-window-destroyed") {
@@ -430,9 +460,9 @@ SettingsManager.prototype = {
   classID: Components.ID("{c40b1c70-00fb-11e2-a21f-0800200c9a66}"),
   contractID: "@mozilla.org/settingsManager;1",
   QueryInterface: XPCOMUtils.generateQI([Ci.nsISupports,
                                          Ci.nsIDOMGlobalPropertyInitializer,
                                          Ci.nsIObserver,
                                          Ci.nsIMemoryReporter]),
 };
 
-this.NSGetFactory = XPCOMUtils.generateNSGetFactory([SettingsManager, SettingsLock])
+this.NSGetFactory = XPCOMUtils.generateNSGetFactory([SettingsManager, SettingsLock]);