Bug 1888975 - Move common optional permissions validation to ExtensionPermissions.sys.mjs r=willdurand

Differential Revision: https://phabricator.services.mozilla.com/D206722

toolkit/components/extensions/ExtensionPermissions.sys.mjs

@@ -13,6 +13,7 @@ const lazy = {};
 ChromeUtils.defineESModuleGetters(lazy, {
   AddonManager: "resource://gre/modules/AddonManager.sys.mjs",
   AddonManagerPrivate: "resource://gre/modules/AddonManager.sys.mjs",
+  Extension: "resource://gre/modules/Extension.sys.mjs",
   ExtensionParent: "resource://gre/modules/ExtensionParent.sys.mjs",
   FileUtils: "resource://gre/modules/FileUtils.sys.mjs",
   JSONFile: "resource://gre/modules/JSONFile.sys.mjs",
@@ -347,6 +348,50 @@ export var ExtensionPermissions = {
     return this._getCached(extensionId);
   },
 
+  /**
+   * Validate and normalize passed in `perms`, including a fixup to
+   * include all possible "all sites" permissions when appropriate.
+   *
+   * @throws if an origin or permission is not part of optional permissions.
+   *
+   * @typedef {object} Perms
+   * @property {string[]} origins
+   * @property {string[]} permissions
+   *
+   * @param {Perms} perms api permissions and origins to be added/removed.
+   * @param {Perms} optional permissions and origins from the manifest.
+   * @returns {Perms} normalized
+   */
+  normalizeOptional(perms, optional) {
+    let allSites = false;
+    let patterns = new MatchPatternSet(optional.origins, { ignorePath: true });
+    let normalized = Object.assign({}, perms);
+
+    for (let o of perms.origins) {
+      if (!patterns.subsumes(new MatchPattern(o))) {
+        throw new Error(`${o} was not declared in the manifest`);
+      }
+      // If this is one of the "all sites" permissions
+      allSites ||= lazy.Extension.isAllSitesPermission(o);
+    }
+
+    if (allSites) {
+      // Grant/revoke ALL "all sites" optional permissions from the manifest.
+      let origins = perms.origins.concat(
+        optional.origins.filter(o => lazy.Extension.isAllSitesPermission(o))
+      );
+      normalized.origins = Array.from(new Set(origins));
+    }
+
+    for (let p of perms.permissions) {
+      if (!optional.permissions.includes(p)) {
+        throw new Error(`${p} was not declared in optional_permissions`);
+      }
+    }
+
+    return normalized;
+  },
+
   _fixupAllUrlsPerms(perms) {
     // Unfortunately, we treat <all_urls> as an API permission as well.
     // If it is added to either, ensure it is added to both.
@@ -362,7 +407,7 @@ export var ExtensionPermissions = {
    * in the format that is passed to browser.permissions.request().
    *
    * @param {string} extensionId The extension id
-   * @param {object} perms Object with permissions and origins array.
+   * @param {Perms} perms Object with permissions and origins array.
    * @param {EventEmitter} [emitter] optional object implementing emitter interfaces
    */
   async add(extensionId, perms, emitter) {
@@ -401,7 +446,7 @@ export var ExtensionPermissions = {
    * in the format that is passed to browser.permissions.request().
    *
    * @param {string} extensionId The extension id
-   * @param {object} perms Object with permissions and origins array.
+   * @param {Perms} perms Object with permissions and origins array.
    * @param {EventEmitter} [emitter] optional object implementing emitter interfaces
    */
   async remove(extensionId, perms, emitter) {

toolkit/components/extensions/test/xpcshell/test_ext_permissions.js

@@ -1059,3 +1059,47 @@ add_task(async function test_internal_permissions() {
 
   await extension.unload();
 });
+
+add_task(function test_normalizeOptional() {
+  const optional1 = {
+    origins: ["*://site.com/", "*://*.domain.com/"],
+    permissions: ["downloads", "tabs"],
+  };
+
+  function normalize(perms, optional) {
+    perms = { origins: [], permissions: [], ...perms };
+    optional = { origins: [], permissions: [], ...optional };
+    return ExtensionPermissions.normalizeOptional(perms, optional);
+  }
+
+  normalize({ origins: ["http://site.com/"] }, optional1);
+  normalize({ origins: ["https://site.com/"] }, optional1);
+  normalize({ origins: ["*://blah.domain.com/"] }, optional1);
+  normalize({ permissions: ["downloads", "tabs"] }, optional1);
+
+  Assert.throws(
+    () => normalize({ origins: ["http://www.example.com/"] }, optional1),
+    /was not declared in the manifest/
+  );
+  Assert.throws(
+    () => normalize({ permissions: ["proxy"] }, optional1),
+    /was not declared in optional_permissions/
+  );
+
+  const optional2 = {
+    origins: ["<all_urls>", "*://*/*"],
+    permissions: ["idle", "clipboardWrite"],
+  };
+
+  normalize({ origins: ["http://site.com/"] }, optional2);
+  normalize({ origins: ["https://site.com/"] }, optional2);
+  normalize({ origins: ["*://blah.domain.com/"] }, optional2);
+  normalize({ permissions: ["idle", "clipboardWrite"] }, optional2);
+
+  let perms = normalize({ origins: ["<all_urls>"] }, optional2);
+  equal(
+    perms.origins.sort().join(),
+    optional2.origins.sort().join(),
+    `Expect both "all sites" permissions`
+  );
+});

toolkit/mozapps/extensions/content/aboutaddons.js

@@ -2451,45 +2451,27 @@ class AddonCard extends HTMLElement {
 
   async setAddonPermission(permission, type, action) {
     let { addon } = this;
-    let origins = [],
-      permissions = [];
+    let perms = { origins: [], permissions: [] };
+
     if (!["add", "remove"].includes(action)) {
       throw new Error("invalid action for permission change");
     }
-    if (type == "permission") {
-      if (
-        action == "add" &&
-        !addon.optionalPermissions.permissions.includes(permission)
-      ) {
-        throw new Error("permission missing from manifest");
-      }
-      permissions = [permission];
-    } else if (type == "origin") {
-      if (action === "add") {
-        let { origins } = addon.optionalPermissions;
-        let patternSet = new MatchPatternSet(origins, { ignorePath: true });
-        if (!patternSet.subsumes(new MatchPattern(permission))) {
-          throw new Error("origin missing from manifest");
-        }
-      }
-      origins = [permission];
 
-      // If this is one of the "all sites" permissions
-      if (Extension.isAllSitesPermission(permission)) {
-        // Grant/revoke ALL "all sites" optional permissions from the manifest.
-        origins = addon.optionalPermissions.origins.filter(perm =>
-          Extension.isAllSitesPermission(perm)
-        );
-      }
+    if (type === "permission") {
+      perms.permissions = [permission];
+    } else if (type === "origin") {
+      perms.origins = [permission];
     } else {
       throw new Error("unknown permission type changed");
     }
-    let policy = WebExtensionPolicy.getByID(addon.id);
-    ExtensionPermissions[action](
-      addon.id,
-      { origins, permissions },
-      policy?.extension
+
+    let normalized = ExtensionPermissions.normalizeOptional(
+      perms,
+      addon.optionalPermissions
     );
+
+    let policy = WebExtensionPolicy.getByID(addon.id);
+    ExtensionPermissions[action](addon.id, normalized, policy?.extension);
   }
 
   async handleEvent(e) {

toolkit/mozapps/extensions/test/browser/browser_html_detail_permissions.js

@@ -613,7 +613,7 @@ add_task(async function testPermissionsViewStates() {
   let card = getAddonCard(view, addon.id);
   await Assert.rejects(
     card.setAddonPermission("webRequest", "permission", "add"),
-    /permission missing from manifest/,
+    /was not declared in optional_permissions/,
     "unable to set the addon permission"
   );