Bug 1352239 Don't prompt for optional permissions an extension already has draft
authorAndrew Swan <aswan@mozilla.com>
Sat, 01 Jul 2017 16:49:14 -0700
changeset 603323 b8fadf31cf6ba21ee79edc635f239af69524ac53
parent 602911 587daa4bdc4b40b7053f4ca3b74723ca747f3b52
child 635894 587897554bd241f6558e35276ad8136cb2481e9a
push id66748
push useraswan@mozilla.com
push dateMon, 03 Jul 2017 17:40:30 +0000
bugs1352239
milestone56.0a1
Bug 1352239 Don't prompt for optional permissions an extension already has MozReview-Commit-ID: EwyzfFB3LyS
toolkit/components/extensions/ext-permissions.js
toolkit/components/extensions/test/mochitest/test_chrome_ext_permissions.html
--- a/toolkit/components/extensions/ext-permissions.js
+++ b/toolkit/components/extensions/ext-permissions.js
@@ -31,16 +31,23 @@ this.permissions = class extends Extensi
           let optionalOrigins = context.extension.optionalOrigins;
           for (let origin of origins) {
             if (!optionalOrigins.subsumes(new MatchPattern(origin))) {
               throw new ExtensionError(`Cannot request origin permission for ${origin} since it was not declared in optional_permissions`);
             }
           }
 
           if (promptsEnabled) {
+            permissions = permissions.filter(perm => !context.extension.hasPermission(perm));
+            origins = origins.filter(origin => !context.extension.whiteListedHosts.subsumes(new MatchPattern(origin)));
+
+            if (permissions.length == 0 && origins.length == 0) {
+              return true;
+            }
+
             let browser = context.pendingEventBrowser || context.xulBrowser;
             let allow = await new Promise(resolve => {
               let subject = {
                 wrappedJSObject: {
                   browser,
                   name: context.extension.name,
                   icon: context.extension.iconURL,
                   permissions: {permissions, origins},
--- a/toolkit/components/extensions/test/mochitest/test_chrome_ext_permissions.html
+++ b/toolkit/components/extensions/test/mochitest/test_chrome_ext_permissions.html
@@ -7,91 +7,97 @@
   <script src="chrome://mochikit/content/tests/SimpleTest/ExtensionTestUtils.js"></script>
   <link rel="stylesheet" href="chrome://mochikit/content/tests/SimpleTest/test.css"/>
 </head>
 <body>
 
 <script type="text/javascript">
 "use strict";
 
-function makeTest(manifestPermissions, optionalPermissions, checkFetch = true) {
-  return async function() {
-    function pageScript() {
-      /* global PERMISSIONS */
-      /* eslint-disable mozilla/balanced-listeners */
-      window.addEventListener("keypress", () => {
-        browser.permissions.request(PERMISSIONS).then(result => {
-          browser.test.sendMessage("request.result", result);
-        }, {once: true});
+Components.utils.import("resource://gre/modules/Services.jsm");
+
+function makeExtension(manifestPermissions, optionalPermissions) {
+  function pageScript() {
+    let requestPermissions;
+    window.addEventListener("keypress", () => {
+      browser.permissions.request(requestPermissions).then(result => {
+        browser.test.sendMessage("request.result", result);
       });
-      /* eslint-enable mozilla/balanced-listeners */
+    }, {once: true});
 
-      browser.test.onMessage.addListener(async msg => {
-        if (msg == "set-cookie") {
-          try {
-            await browser.cookies.set({
-              url: "http://example.com/",
-              name: "COOKIE",
-              value: "NOM NOM",
-            });
-            browser.test.sendMessage("set-cookie.result", {success: true});
-          } catch (err) {
-            dump(`set cookie failed with ${err.message}\n`);
-            browser.test.sendMessage("set-cookie.result",
-                                     {success: false, message: err.message});
-          }
-        } else if (msg == "remove") {
-          browser.permissions.remove(PERMISSIONS).then(result => {
-            browser.test.sendMessage("remove.result", result);
+    browser.test.onMessage.addListener(async (msg, arg) => {
+      if (msg == "set-cookie") {
+        try {
+          await browser.cookies.set({
+            url: "http://example.com/",
+            name: "COOKIE",
+            value: "NOM NOM",
           });
+          browser.test.sendMessage("set-cookie.result", {success: true});
+        } catch (err) {
+          browser.test.sendMessage("set-cookie.result",
+                                   {success: false, message: err.message});
         }
-      });
+      } else if (msg == "request") {
+        requestPermissions = arg;
+        browser.test.sendMessage("request.result");
+      } else if (msg == "remove") {
+        browser.permissions.remove(arg).then(result => {
+          browser.test.sendMessage("remove.result", result);
+        });
+      }
+    });
+
+    browser.test.sendMessage("page-ready");
+  }
 
-      browser.test.sendMessage("page-ready");
-    }
+  return ExtensionTestUtils.loadExtension({
+    background() {
+      browser.test.sendMessage("ready", browser.runtime.getURL("page.html"));
+    },
+
+    manifest: {
+      permissions: manifestPermissions,
+      optional_permissions: optionalPermissions,
 
-    let extension = ExtensionTestUtils.loadExtension({
-      background() {
-        browser.test.sendMessage("ready", browser.runtime.getURL("page.html"));
+      content_scripts: [{
+        matches: ["http://mochi.test/*/file_sample.html"],
+        js: ["content_script.js"],
+      }],
+    },
+
+    files: {
+      "content_script.js": async () => {
+        let url = new URL(window.location.pathname, "http://example.com/");
+        fetch(url, {}).then(response => {
+          browser.test.sendMessage("fetch.result", response.ok);
+        }).catch(err => {
+          browser.test.sendMessage("fetch.result", false);
+        });
       },
 
-      manifest: {
-        permissions: manifestPermissions,
-        optional_permissions: [...(optionalPermissions.permissions || []),
-                               ...(optionalPermissions.origins || [])],
-
-        content_scripts: [{
-          matches: ["http://mochi.test/*/file_sample.html"],
-          js: ["content_script.js"],
-        }],
-      },
-
-      files: {
-        "content_script.js": async () => {
-          let url = new URL(window.location.pathname, "http://example.com/");
-          fetch(url, {}).then(response => {
-            browser.test.sendMessage("fetch.result", response.ok);
-          }).catch(err => {
-            browser.test.sendMessage("fetch.result", false);
-          });
-        },
-
-        "page.html": `<html><head>
+      "page.html": `<html><head>
           <script src="page.js"><\/script>
         </head></html>`,
 
-        "page.js": `const PERMISSIONS = ${JSON.stringify(optionalPermissions)}; (${pageScript})();`,
-      },
-    });
+      "page.js": pageScript,
+    },
+  });
+}
 
+function makeTest(manifestPermissions, optionalPermissions, checkFetch = true) {
+  return async function() {
+    let flatOptional = [...optionalPermissions.permissions || [],
+                        ...optionalPermissions.origins || []];
+    let extension = makeExtension(manifestPermissions, flatOptional);
     await extension.startup();
 
-    function call(method) {
-      extension.sendMessage(method);
-      return extension.awaitMessage(`${method}.result`);
+    function call(...args) {
+      extension.sendMessage(...args);
+      return extension.awaitMessage(`${args[0]}.result`);
     }
 
     let base = window.location.href.replace(/^chrome:\/\/mochitests\/content/,
                                             "http://mochi.test:8888");
     let file = new URL("file_sample.html", base);
 
     async function testContentScript() {
       let win = window.open(file);
@@ -117,33 +123,34 @@ function makeTest(manifestPermissions, o
 
     // Making a cross-origin request from a content script should fail
     if (checkFetch) {
       result = await testContentScript();
       is(result, false, "fetch() failed from content script due to lack of host permission");
     }
 
     // Request some permissions
+    await call("request", optionalPermissions);
     let winutils = SpecialPowers.getDOMWindowUtils(win);
     winutils.sendKeyEvent("keypress", KeyEvent.DOM_VK_A, 0, 0);
     result = await extension.awaitMessage("request.result");
     is(result, true, "permissions.request() succeeded");
 
     // Using the cookies API from an extension page should succeed
     result = await call("set-cookie");
     is(result.success, true, "setting cookie succeeded");
 
     // Making a cross-origin request from a content script should succeed
     if (checkFetch) {
       result = await testContentScript();
       is(result, true, "fetch() succeeded from content script due to lack of host permission");
     }
 
     // Now revoke our permissions
-    result = await call("remove");
+    result = await call("remove", optionalPermissions);
 
     // The cookies API should once again fail
     result = await call("set-cookie");
     is(result.success, false, "setting cookie failed");
 
     // As should the cross-origin request from a content script
     if (checkFetch) {
       result = await testContentScript();
@@ -165,13 +172,89 @@ const ORIGIN = "*://example.com/";
 add_task(makeTest([], {
   permissions: ["cookies"],
   origins: [ORIGIN],
 }));
 
 add_task(makeTest(["cookies"], {origins: [ORIGIN]}));
 add_task(makeTest([ORIGIN], {permissions: ["cookies"]}, false));
 
+// Test that we don't prompt for permissions an extension already has.
+add_task(async function test_alreadyGranted() {
+  // Turn on permission prompts and set up a listener
+  await SpecialPowers.pushPrefEnv({
+    set: [["extensions.webextOptionalPermissionPrompts", true]],
+  });
+
+  let sawPrompt = false;
+  function listener(subject) {
+    sawPrompt = true;
+    subject.wrappedJSObject.resolve(true);
+  }
+
+  Services.obs.addObserver(listener, "webextension-optional-permission-prompt");
+
+  const REQUIRED_PERMISSIONS = [
+    "geolocation",
+    "*://required-host.com/",
+    "*://*.required-domain.com/",
+  ];
+  const OPTIONAL_PERMISSIONS = [
+    ...REQUIRED_PERMISSIONS,
+    "clipboardRead",
+    "*://optional-host.com/",
+    "*://*.optional-domain.com/",
+  ];
+
+  let extension = makeExtension(REQUIRED_PERMISSIONS, OPTIONAL_PERMISSIONS);
+  await extension.startup();
+
+  let url = await extension.awaitMessage("ready");
+  let win = window.open(url);
+  await extension.awaitMessage("page-ready");
+
+  let winutils = SpecialPowers.getDOMWindowUtils(win);
+
+  async function checkRequest(arg, expectPrompt, msg) {
+    extension.sendMessage("request", arg);
+    await extension.awaitMessage("request.result");
+
+    sawPrompt = false;
+    winutils.sendKeyEvent("keypress", KeyEvent.DOM_VK_A, 0, 0);
+
+    let result = await extension.awaitMessage("request.result");
+    ok(result, "request() call succeeded");
+    is(sawPrompt, expectPrompt,
+       `Got ${expectPrompt ? "" : "no "}permission prompt for ${msg}`);
+  }
+
+  await checkRequest({permissions: ["geolocation"]}, false,
+                     "required permission from manifest");
+  await checkRequest({origins: ["http://required-host.com/"]}, false,
+                     "origin permission from manifest");
+  await checkRequest({origins: ["http://host.required-domain.com/"]}, false,
+                     "wildcard origin permission from manifest");
+
+  await checkRequest({permissions: ["clipboardRead"]}, true,
+                     "optional permission");
+  await checkRequest({permissions: ["clipboardRead"]}, false,
+                     "already granted optional permission");
+
+  await checkRequest({origins: ["http://optional-host.com/"]}, true,
+                     "optional origin");
+  await checkRequest({origins: ["http://optional-host.com/"]}, false,
+                     "already granted origin permission");
+
+  await checkRequest({origins: ["http://*.optional-domain.com/"]}, true,
+                     "optional wildcard origin");
+  await checkRequest({origins: ["http://*.optional-domain.com/"]}, false,
+                     "already granted optional wildcard origin");
+  await checkRequest({origins: ["http://host.optional-domain.com/"]}, false,
+                     "host matching optional wildcard origin");
+
+  await extension.unload();
+});
+
 </script>
 
 </body>
 </html>