Bug 793204 - Add remove() API to PermissionSettings. r=sicking

caps/idl/nsIPrincipal.idl

@@ -108,7 +108,7 @@ interface nsIPrincipal : nsISerializable
      * principals are not allowed to load anything. This is changed slightly
      * by the optional flag allowIfInheritsPrincipal (which defaults to false)
      * which allows the load of a data: URI (which inherits the principal of
-     * its loader) or a URI with the same principal as its loader (eg. a 
+     * its loader) or a URI with the same principal as its loader (eg. a
      * Blob URI).
      * In these cases, with allowIfInheritsPrincipal set to true, the URI can
      * be loaded by a null principal.
@@ -159,7 +159,7 @@ interface nsIPrincipal : nsISerializable
      * non-hierarchical schemes correctly.
      */
     readonly attribute ACString baseDomain;
-    
+
     const short APP_STATUS_NOT_INSTALLED = 0;
     const short APP_STATUS_INSTALLED     = 1;
     const short APP_STATUS_PRIVILEGED    = 2;
@@ -229,22 +229,22 @@ interface nsIPrincipal : nsISerializable
 };
 
 /**
- * If nsSystemPrincipal is too risky to use, but we want a principal to access 
- * more than one origin, nsExpandedPrincipals letting us define an array of 
+ * If nsSystemPrincipal is too risky to use, but we want a principal to access
+ * more than one origin, nsExpandedPrincipals letting us define an array of
  * principals it subsumes. So script with an nsExpandedPrincipals will gain
- * same origin access when at least one of its principals it contains gained 
+ * same origin access when at least one of its principals it contains gained
  * sameorigin acccess. An nsExpandedPrincipal will be subsumed by the system
  * principal, and by another nsExpandedPrincipal that has all its principals.
  * It is added for jetpack content-scripts to let them interact with the
- * content and a well defined set of other domains, without the risk of 
+ * content and a well defined set of other domains, without the risk of
  * leaking out a system principal to the content. See: Bug 734891
  */
 [uuid(f3e177Df-6a5e-489f-80a7-2dd1481471d8)]
 interface nsIExpandedPrincipal : nsISupports
 {
   /**
    * An array of principals that the expanded principal subsumes.
-   * Note: this list is not reference counted, it is shared, so 
+   * Note: this list is not reference counted, it is shared, so
    * should not be changed and should only be used ephemerally.
    */
   [noscript] readonly attribute PrincipalArray whiteList;

dom/apps/src/PermissionsInstaller.jsm

@@ -90,8 +90,10 @@ this.PermissionsInstaller = {
                 continue;
               }
               // Remove the deprecated permission
-              // TODO: use PermSettings.remove, see bug 793204
-              this._setPermission(permName, "unknown", aApp);
+              PermissionSettingsModule.removePermission(permName,
+                                                        aApp.manifestURL,
+                                                        aApp.origin,
+                                                        false);
             }
           }
         }

dom/interfaces/permission/nsIDOMPermissionSettings.idl

@@ -6,12 +6,16 @@
 
 interface nsIDOMDOMRequest;
 
-[scriptable, uuid(b3e3894e-b24e-4174-9c80-08115709615b)]
+[scriptable, uuid(18390770-02ab-11e2-a21f-0800200c9a66)]
 interface nsIDOMPermissionSettings : nsISupports
 {
   DOMString get(in DOMString permission, in DOMString manifestURI, in DOMString origin, in bool browserFlag);
 
   void set(in DOMString permission, in DOMString value, in DOMString manifestURI, in DOMString origin, in bool browserFlag);
 
   bool isExplicit(in DOMString permission, in DOMString manifestURI, in DOMString origin, in bool browserFlag);
+
+  // Removing a permission is only allowed for pages with a different origin than the app
+  // and pages that have browserFlag=true, so remove() doesn't have a browserFlag parameter.
+  void remove(in DOMString permission, in DOMString manifestURI, in DOMString origin);
 };

dom/permission/PermissionSettings.js

@@ -21,7 +21,7 @@ var cpm = Cc["@mozilla.org/childprocessmessagemanager;1"].getService(Ci.nsISyncM
 // PermissionSettings
 
 const PERMISSIONSETTINGS_CONTRACTID = "@mozilla.org/permissionSettings;1";
-const PERMISSIONSETTINGS_CID        = Components.ID("{18390770-02ab-11e2-a21f-0800200c9a66}");
+const PERMISSIONSETTINGS_CID        = Components.ID("{cd2cf7a1-f4c1-487b-8c1b-1a71c7097431}");
 const nsIDOMPermissionSettings      = Ci.nsIDOMPermissionSettings;
 
 function PermissionSettings()
@@ -81,13 +81,13 @@ PermissionSettings.prototype = {
   set: function set(aPermName, aPermValue, aManifestURL, aOrigin,
                     aBrowserFlag) {
     debug("Set called with: " + aPermName + ", " + aManifestURL + ", " +
-          aOrigin + ",  " + aPermValue + ", " + aBrowserFlag); 
-    let currentPermValue = this.get(aPermName, aManifestURL, aOrigin, 
+          aOrigin + ",  " + aPermValue + ", " + aBrowserFlag);
+    let currentPermValue = this.get(aPermName, aManifestURL, aOrigin,
                                     aBrowserFlag);
     let action;
     // Check for invalid calls so that we throw an exception rather than get
     // killed by parent process
-    if (currentPermValue === "unknown" || 
+    if (currentPermValue === "unknown" ||
         aPermValue === "unknown" ||
         !this.isExplicit(aPermName, aManifestURL, aOrigin, aBrowserFlag)) {
       let errorMsg = "PermissionSettings.js: '" + aPermName + "'" +
@@ -106,6 +106,29 @@ PermissionSettings.prototype = {
     });
   },
 
+  remove: function remove(aPermName, aManifestURL, aOrigin) {
+    let uri = Services.io.newURI(aOrigin, null, null);
+    let appID = appsService.getAppLocalIdByManifestURL(aManifestURL);
+    let principal = secMan.getAppCodebasePrincipal(uri, appID, true);
+
+    if (principal.appStatus !== Ci.nsIPrincipal.APP_STATUS_NOT_INSTALLED) {
+      let errorMsg = "PermissionSettings.js: '" + aOrigin + "'" +
+                     " is installed or permission is implicit, cannot remove '" +
+                     aPermName + "'.";
+      Cu.reportError(errorMsg);
+      throw new Components.Exception(errorMsg);
+    }
+
+    // PermissionSettings.jsm handles delete when value is "unknown"
+    cpm.sendSyncMessage("PermissionSettings:AddPermission", {
+      type: aPermName,
+      origin: aOrigin,
+      manifestURL: aManifestURL,
+      value: "unknown",
+      browserFlag: true
+    });
+  },
+
   init: function init(aWindow) {
     debug("init");
 

dom/permission/PermissionSettings.jsm

@@ -49,21 +49,24 @@ this.PermissionSettingsModule = {
     // Bug 812289:
     // Change is allowed from a child process when all of the following
     // conditions stand true:
-    //   * the action isn't "unknown" (so the change isn't a delete)
+    //   * the action isn't "unknown" (so the change isn't a delete) if the app
+    //     is installed
     //   * the permission already exists on the database
     //   * the permission is marked as explicit on the permissions table
-    // Note that we *have* to check the first two conditions ere because
+    // Note that we *have* to check the first two conditions here because
     // permissionManager doesn't know if it's being called as a result of
     // a parent process or child process request. We could check
     // if the permission is actually explicit (and thus modifiable) or not
     // on permissionManager also but we currently don't.
     let perm =
       permissionManager.testExactPermissionFromPrincipal(aPrincipal,aPermName);
     let isExplicit = isExplicitInPermissionsTable(aPermName, aPrincipal.appStatus);
-
-    return (aAction !== "unknown") &&
-           (perm !== Ci.nsIPermissionManager.UNKNOWN_ACTION) &&
-           isExplicit;
+
+    return (aAction === "unknown" &&
+            aPrincipal.appStatus === Ci.nsIPrincipal.APP_STATUS_NOT_INSTALLED) ||
+           (aAction !== "unknown" &&
+            (perm !== Ci.nsIPermissionManager.UNKNOWN_ACTION) &&
+            isExplicit);
   },
 
   addPermission: function addPermission(aData, aCallbacks) {
@@ -132,6 +135,17 @@ this.PermissionSettingsModule = {
     }
   },
 
+  removePermission: function removePermission(aPermName, aManifestURL, aOrigin, aBrowserFlag) {
+    let data = {
+      type: aPermName,
+      origin: aOrigin,
+      manifestURL: aManifestURL,
+      value: "unknown",
+      browserFlag: aBrowserFlag
+    };
+    this._internalAddPermission(data, true);
+  },
+
   observe: function observe(aSubject, aTopic, aData) {
     ppmm.removeMessageListener("PermissionSettings:AddPermission", this);
     Services.obs.removeObserver(this, "profile-before-change");
@@ -147,11 +161,11 @@ this.PermissionSettingsModule = {
     switch (aMessage.name) {
       case "PermissionSettings:AddPermission":
         let success = false;
-        let errorMsg = 
+        let errorMsg =
               " from a content process with no 'permissions' privileges.";
         if (mm.assertPermission("permissions")) {
           success = this._internalAddPermission(msg, false);
-          if (!success) { 
+          if (!success) {
             // Just kill the calling process
             mm.assertPermission("permissions-modify-implicit");
             errorMsg = " had an implicit permission change. Child process killed.";

dom/permission/PermissionSettings.manifest

@@ -1,3 +1,3 @@
-component {18390770-02ab-11e2-a21f-0800200c9a66} PermissionSettings.js
-contract @mozilla.org/permissionSettings;1 {18390770-02ab-11e2-a21f-0800200c9a66}
+component {cd2cf7a1-f4c1-487b-8c1b-1a71c7097431} PermissionSettings.js
+contract @mozilla.org/permissionSettings;1 {cd2cf7a1-f4c1-487b-8c1b-1a71c7097431}
 category JavaScript-navigator-property mozPermissionSettings @mozilla.org/permissionSettings;1

dom/permission/tests/test_permission_basics.html

@@ -31,7 +31,7 @@
 
 SpecialPowers.addPermission("permissions", true, document);
 var comp = SpecialPowers.wrap(Components);
-SpecialPowers.pushPrefEnv({ "set": [["dom.mozPermissionSettings.enabled", true]] }, 
+SpecialPowers.pushPrefEnv({ "set": [["dom.mozPermissionSettings.enabled", true]] },
                           function() {
                             SpecialPowers.removePermission("permissions", document);
                           });
@@ -42,16 +42,21 @@
 function permissionTest() {
   // Any permission explicit for privileged and implicit for certified serves
   var testPerm = "contacts-read";
+  // Any permission explicit for privileged and certified apps
+  var explicitPerm = "geolocation";
 
-  // Simulate than the app request the permissions
+  // Simulate that the app requested the permissions
   SpecialPowers.addPermission(testPerm, true, testPrivApp);
   SpecialPowers.addPermission(testPerm, true, testCertApp);
+  SpecialPowers.addPermission(explicitPerm, true, testPrivApp);
+  SpecialPowers.addPermission(explicitPerm, true, testCertApp);
 
-  if (gPermissionssEnabled) {
+  if (gPermissionsEnabled) {
     var certAppManifest = testCertApp.manifestURL;
     var privAppManifest = testPrivApp.manifestURL;
     var originPriv = "https://aprivileged.com";
     var originCert = "https://acertified.com";
+    var originOther = "http://test";
 
     // Trying to make any change to implicit permissions should fail
     try {
@@ -64,14 +69,30 @@
     var result=mozPermissions.get(testPerm, certAppManifest, originCert, false);
     is(result, "allow", "same result");
 
-    // Erasing a permission, even an explicit one, is not allowed
+    // Removing a permission from the same origin, even an explicit one, should fail
     try {
-      mozPermissions.set(testPerm, "unknown", privAppManifest, originPriv, false);
-      ok(false, "Erase explicit permission");
+      mozPermissions.set(testPerm, "unknown", privAppManifest, originPriv);
+      ok(false, "Setting a permission to unknown");
     } catch (e) {
-      ok(true, "Erase explicit permission");
+      ok(true, "Setting a permission to unknown");
     }
 
+    // Removing an explicit permission from a different origin should work
+    var testRemove = function(aPerm, aManifest, aOrigin, aTestMsg) {
+      try {
+        mozPermissions.remove(aPerm, aManifest, aOrigin);
+        var status = mozPermissions.get(aPerm, aManifest, aOrigin, false);
+        is(status, "unknown", aTestMsg);
+      } catch (e) {
+        ok(false, aTestMsg);
+      }
+    }
+
+    testRemove(explicitPerm, privAppManifest, originOther,
+               "Remove explicit permission of privileged app");
+    testRemove(explicitPerm, certAppManifest, originOther,
+               "Remove explicit permission of certified app");
+
     mozPermissions.set(testPerm, "allow", privAppManifest, originPriv, false);
     result = mozPermissions.get(testPerm, privAppManifest, originPriv, false);
     is(result, "allow", "Set to allow");
@@ -90,7 +111,7 @@
   SpecialPowers.removePermission(testPerm, testCertApp);
 }
 
-var gPermissionssEnabled = SpecialPowers.getBoolPref("dom.mozPermissionSettings.enabled");
+var gPermissionsEnabled = SpecialPowers.getBoolPref("dom.mozPermissionSettings.enabled");
 SimpleTest.waitForExplicitFinish();
 addLoadEvent(permissionTest);