Bug 1433290 - Remove Block parameter for InstallAddons and Popup policies. r=bytesized
authorFelipe Gomes <felipc@gmail.com>
Mon, 26 Feb 2018 17:09:43 -0300
changeset 760286 1d5ae4b0b826feea18dfb162d5b03bfbd652f547
parent 760285 4777312d11cdfa44ec7ced4514e6be676076a5b9
child 760287 f9658f61fbe2b6b46647f8fed78269627e60b856
push id100603
push userjcristau@mozilla.com
push dateTue, 27 Feb 2018 11:10:29 +0000
reviewersbytesized
bugs1433290
milestone60.0a1
Bug 1433290 - Remove Block parameter for InstallAddons and Popup policies. r=bytesized The Block parameters for the InstallAddons policy doesn't make sense because the addons install process has two behaviors: - Allow - Ask So a website that is not explictly in the allow list will always ask for permission before installing an addon. If a sysadmin wants to fully block addon install they can use a different policy. Similarly, for Popups it's the same thing. There is a conceptually valid use case in that someone can change the _default_ behavior to always allow popups, and then the Block list becomes the allow exceptions to Ask. But that's a corner case that we don't need to support now, since allowing popups globally on the web is a pretty crazy choice. MozReview-Commit-ID: EzclfLNDgUo
browser/components/enterprisepolicies/Policies.jsm
browser/components/enterprisepolicies/schemas/policies-schema.json
browser/components/enterprisepolicies/tests/browser/browser_policies_popups_cookies_addons_flash.js
browser/components/enterprisepolicies/tests/browser/config_popups_cookies_addons_flash.json
--- a/browser/components/enterprisepolicies/Policies.jsm
+++ b/browser/components/enterprisepolicies/Policies.jsm
@@ -207,23 +207,23 @@ var Policies = {
   "FlashPlugin": {
     onBeforeUIStartup(manager, param) {
       addAllowDenyPermissions("plugin:flash", param.Allow, param.Block);
     }
   },
 
   "InstallAddons": {
     onBeforeUIStartup(manager, param) {
-      addAllowDenyPermissions("install", param.Allow, param.Block);
+      addAllowDenyPermissions("install", param.Allow, null);
     }
   },
 
   "Popups": {
     onBeforeUIStartup(manager, param) {
-      addAllowDenyPermissions("popup", param.Allow, param.Block);
+      addAllowDenyPermissions("popup", param.Allow, null);
     }
   },
 
   "RememberPasswords": {
     onBeforeUIStartup(manager, param) {
       setAndLockPref("signon.rememberSignons", param);
     }
   },
--- a/browser/components/enterprisepolicies/schemas/policies-schema.json
+++ b/browser/components/enterprisepolicies/schemas/policies-schema.json
@@ -205,45 +205,31 @@
 
       "type": "object",
       "properties": {
         "Allow": {
           "type": "array",
           "items": {
             "type": "origin"
           }
-        },
-
-        "Block": {
-          "type": "array",
-          "items": {
-            "type": "origin"
-          }
         }
       }
     },
 
     "Popups": {
       "description": "Allow or deny popup usage.",
       "first_available": "60.0",
 
       "type": "object",
       "properties": {
         "Allow": {
           "type": "array",
           "items": {
             "type": "origin"
           }
-        },
-
-        "Block": {
-          "type": "array",
-          "items": {
-            "type": "origin"
-          }
         }
       }
     },
 
     "RememberPasswords": {
       "description": "Enforces the setting to allow Firefox to remember saved logins and passwords. Both true and false values are accepted.",
       "first_available": "60.0",
 
--- a/browser/components/enterprisepolicies/tests/browser/browser_policies_popups_cookies_addons_flash.js
+++ b/browser/components/enterprisepolicies/tests/browser/browser_policies_popups_cookies_addons_flash.js
@@ -5,25 +5,19 @@
 
 function URI(str) {
   return Services.io.newURI(str);
 }
 
 add_task(async function test_setup_preexisting_permissions() {
   // Pre-existing ALLOW permissions that should be overriden
   // with DENY.
-  Services.perms.add(URI("https://www.pre-existing-allow.com"),
-                     "popup",
-                     Ci.nsIPermissionManager.ALLOW_ACTION,
-                     Ci.nsIPermissionManager.EXPIRE_SESSION);
 
-  Services.perms.add(URI("https://www.pre-existing-allow.com"),
-                     "install",
-                     Ci.nsIPermissionManager.ALLOW_ACTION,
-                     Ci.nsIPermissionManager.EXPIRE_SESSION);
+  // No ALLOW -> DENY override for popup and install permissions,
+  // because their policies only supports the Allow parameter.
 
   Services.perms.add(URI("https://www.pre-existing-allow.com"),
                      "cookie",
                      Ci.nsIPermissionManager.ALLOW_ACTION,
                      Ci.nsIPermissionManager.EXPIRE_SESSION);
 
   Services.perms.add(URI("https://www.pre-existing-allow.com"),
                      "plugin:flash",
@@ -70,47 +64,50 @@ function checkPermission(url, expected, 
     let permission = Services.perms.getPermissionObjectForURI(
       uri, permissionName, true);
     ok(permission, "Permission object exists");
     is(permission.expireType, Ci.nsIPermissionManager.EXPIRE_POLICY,
        "Permission expireType is correct");
   }
 }
 
-function checkAllPermissionsForType(type) {
+function checkAllPermissionsForType(type, typeSupportsDeny = true) {
   checkPermission("allow.com", "ALLOW", type);
-  checkPermission("deny.com", "DENY", type);
   checkPermission("unknown.com", "UNKNOWN", type);
-  checkPermission("pre-existing-allow.com", "DENY", type);
   checkPermission("pre-existing-deny.com", "ALLOW", type);
+
+  if (typeSupportsDeny) {
+    checkPermission("deny.com", "DENY", type);
+    checkPermission("pre-existing-allow.com", "DENY", type);
+  }
 }
 
 add_task(async function test_popups_policy() {
-  checkAllPermissionsForType("popup");
+  checkAllPermissionsForType("popup", false);
 });
 
 add_task(async function test_webextensions_policy() {
-  checkAllPermissionsForType("install");
+  checkAllPermissionsForType("install", false);
 });
 
 add_task(async function test_cookies_policy() {
   checkAllPermissionsForType("cookie");
 });
 
 add_task(async function test_flash_policy() {
   checkAllPermissionsForType("plugin:flash");
 });
 
 add_task(async function test_change_permission() {
   // Checks that changing a permission will still retain the
   // value set through the engine.
-  Services.perms.add(URI("https://www.allow.com"), "popup",
+  Services.perms.add(URI("https://www.allow.com"), "cookie",
                      Ci.nsIPermissionManager.DENY_ACTION,
                      Ci.nsIPermissionManager.EXPIRE_SESSION);
 
-  checkPermission("allow.com", "ALLOW", "popup");
+  checkPermission("allow.com", "ALLOW", "cookie");
 
   // Also change one un-managed permission to make sure it doesn't
   // cause any problems to the policy engine or the permission manager.
-  Services.perms.add(URI("https://www.unmanaged.com"), "popup",
+  Services.perms.add(URI("https://www.unmanaged.com"), "cookie",
                    Ci.nsIPermissionManager.DENY_ACTION,
                    Ci.nsIPermissionManager.EXPIRE_SESSION);
 });
--- a/browser/components/enterprisepolicies/tests/browser/config_popups_cookies_addons_flash.json
+++ b/browser/components/enterprisepolicies/tests/browser/config_popups_cookies_addons_flash.json
@@ -1,19 +1,14 @@
 {
   "policies": {
     "Popups": {
       "Allow": [
         "https://www.allow.com",
         "https://www.pre-existing-deny.com"
-      ],
-
-      "Block": [
-        "https://www.deny.com",
-        "https://www.pre-existing-allow.com"
       ]
     },
 
     "Cookies": {
       "Allow": [
         "https://www.allow.com",
         "https://www.pre-existing-deny.com"
       ],
@@ -23,21 +18,16 @@
         "https://www.pre-existing-allow.com"
       ]
     },
 
     "InstallAddons": {
       "Allow": [
         "https://www.allow.com",
         "https://www.pre-existing-deny.com"
-      ],
-
-      "Block": [
-        "https://www.deny.com",
-        "https://www.pre-existing-allow.com"
       ]
     },
 
     "FlashPlugin": {
       "Allow": [
         "https://www.allow.com",
         "https://www.pre-existing-deny.com"
       ],