Bug 1811947 - Add permission warning for declarativeNetRequestFeedbac…

…k r=rpl,geckoview-reviewers,calu ... and make the permission available by default, not gated on the extra pref, to make sure that the permission warning shows up. In the future, the DNR API will also use this permission even for non-debugging APIs (e.g. bug 1745772), as mentioned in the bug. Differential Revision: https://phabricator.services.mozilla.com/D169213
iaxat · Feb 14, 2023 · 8270a1a · 8270a1a
1 parent 011ebfa
commit 8270a1a
Show file tree

Hide file tree

Showing 8 changed files with 73 additions and 28 deletions.
diff --git a/browser/locales/en-US/chrome/browser/browser.properties b/browser/locales/en-US/chrome/browser/browser.properties
@@ -142,6 +142,7 @@ webextPerms.description.browsingData=Clear recent browsing history, cookies, and
 webextPerms.description.clipboardRead=Get data from the clipboard
 webextPerms.description.clipboardWrite=Input data to the clipboard
 webextPerms.description.declarativeNetRequest=Block content on any page
+webextPerms.description.declarativeNetRequestFeedback=Read your browsing history
 webextPerms.description.devtools=Extend developer tools to access your data in open tabs
 webextPerms.description.downloads=Download files and read and modify the browser’s download history
 webextPerms.description.downloads.open=Open files downloaded to your computer

diff --git a/mobile/android/locales/en-US/chrome/browser.properties b/mobile/android/locales/en-US/chrome/browser.properties
@@ -50,6 +50,7 @@ webextPerms.description.browsingData=Clear recent browsing history, cookies, and
 webextPerms.description.clipboardRead=Get data from the clipboard
 webextPerms.description.clipboardWrite=Input data to the clipboard
 webextPerms.description.declarativeNetRequest=Block content on any page
+webextPerms.description.declarativeNetRequestFeedback=Read your browsing history
 webextPerms.description.devtools=Extend developer tools to access your data in open tabs
 webextPerms.description.downloads=Download files and read and modify the browser’s download history
 webextPerms.description.downloads.open=Open files downloaded to your computer

diff --git a/toolkit/components/extensions/Extension.jsm b/toolkit/components/extensions/Extension.jsm
@@ -219,7 +219,6 @@ if (
 }
 
 const PREF_DNR_ENABLED = "extensions.dnr.enabled";
-const PREF_DNR_FEEDBACK = "extensions.dnr.feedback";
 
 // Message included in warnings and errors related to privileged permissions and
 // privileged manifest properties. Provides a link to the firefox-source-docs.mozilla.org
@@ -283,14 +282,6 @@ function isDNRPermissionAllowed(perm) {
     return false;
   }
 
-  // APIs tied to declarativeNetRequestFeedback are for debugging purposes and
-  // are only supposed to be available when the (add-on dev) user opts in.
-  if (
-    perm === "declarativeNetRequestFeedback" &&
-    !Services.prefs.getBoolPref(PREF_DNR_FEEDBACK, false)
-  ) {
-    return false;
-  }
   return true;
 }
 

diff --git a/toolkit/components/extensions/parent/ext-declarativeNetRequest.js b/toolkit/components/extensions/parent/ext-declarativeNetRequest.js
@@ -12,6 +12,22 @@ ChromeUtils.defineESModuleGetters(this, {
 
 var { ExtensionError } = ExtensionUtils;
 
+const PREF_DNR_FEEDBACK = "extensions.dnr.feedback";
+XPCOMUtils.defineLazyPreferenceGetter(
+  this,
+  "dnrFeedbackEnabled",
+  PREF_DNR_FEEDBACK,
+  false
+);
+
+function ensureDNRFeedbackEnabled(apiName) {
+  if (!dnrFeedbackEnabled) {
+    throw new ExtensionError(
+      `${apiName} is only available when the "${PREF_DNR_FEEDBACK}" preference is set to true.`
+    );
+  }
+}
+
 this.declarativeNetRequest = class extends ExtensionAPI {
   onManifestEntry(entryName) {
     if (entryName === "declarative_net_request") {
@@ -87,6 +103,7 @@ this.declarativeNetRequest = class extends ExtensionAPI {
         },
 
         async testMatchOutcome(request, options) {
+          ensureDNRFeedbackEnabled("declarativeNetRequest.testMatchOutcome");
           let { url, initiator, ...req } = request;
           req.requestURI = Services.io.newURI(url);
           if (initiator) {

diff --git a/toolkit/components/extensions/schemas/declarative_net_request.json b/toolkit/components/extensions/schemas/declarative_net_request.json
@@ -10,15 +10,15 @@
         "$extend": "Permission",
         "choices": [{
           "type": "string",
-          "enum": ["declarativeNetRequest"],
+          "enum": ["declarativeNetRequest", "declarativeNetRequestFeedback"],
           "min_manifest_version": 3
         }]
-      }, 
+      },
       {
         "$extend": "PermissionNoPrompt",
         "choices": [{
           "type": "string",
-          "enum": ["declarativeNetRequestFeedback", "declarativeNetRequestWithHostAccess"],
+          "enum": ["declarativeNetRequestWithHostAccess"],
           "min_manifest_version": 3
         }]
       },

diff --git a/toolkit/components/extensions/test/xpcshell/test_ext_dnr_api.js b/toolkit/components/extensions/test/xpcshell/test_ext_dnr_api.js
@@ -11,26 +11,57 @@ const PREF_DNR_FEEDBACK_DEFAULT_VALUE = Services.prefs.getBoolPref(
   false
 );
 
+// To distinguish from testMatchOutcome, undefined vs resolving vs rejecting.
+const kTestMatchOutcomeNotAllowed = "kTestMatchOutcomeNotAllowed";
+
 async function testAvailability({
   allowDNRFeedback = false,
   testExpectations,
   ...extensionData
 }) {
-  function background(testExpectations) {
+  async function background(testExpectations) {
     let {
+      // declarativeNetRequest should be available if "declarativeNetRequest" or
+      // "declarativeNetRequestWithHostAccess" permission is requested.
+      //  (and always unavailable when "extensions.dnr.enabled" pref is false)
       declarativeNetRequest_available = false,
+      // testMatchOutcome is available when the "declarativeNetRequestFeedback"
+      // permission is granted AND the "extensions.dnr.feedback" pref is true.
+      //  (and always unavailable when "extensions.dnr.enabled" pref is false)
+      // testMatchOutcome_available: true - permission granted + pref true.
+      // testMatchOutcome_available: false - no permission, pref doesn't matter.
+      // testMatchOutcome_available: kTestMatchOutcomeNotAllowed - permission
+      //   granted, but pref is false.
       testMatchOutcome_available = false,
     } = testExpectations;
     browser.test.assertEq(
       declarativeNetRequest_available,
       !!browser.declarativeNetRequest,
       "declarativeNetRequest API namespace availability"
     );
-    browser.test.assertEq(
-      testMatchOutcome_available,
-      !!browser.declarativeNetRequest?.testMatchOutcome,
-      "declarativeNetRequest.testMatchOutcome availability"
-    );
+
+    // Dummy param for testMatchOutcome:
+    const dummyRequest = { url: "https://example.com/", type: "other" };
+
+    if (!testMatchOutcome_available) {
+      browser.test.assertEq(
+        undefined,
+        browser.declarativeNetRequest?.testMatchOutcome,
+        "declarativeNetRequest.testMatchOutcome availability"
+      );
+    } else if (testMatchOutcome_available === "kTestMatchOutcomeNotAllowed") {
+      await browser.test.assertRejects(
+        browser.declarativeNetRequest.testMatchOutcome(dummyRequest),
+        `declarativeNetRequest.testMatchOutcome is only available when the "extensions.dnr.feedback" preference is set to true.`,
+        "declarativeNetRequest.testMatchOutcome is unavailable"
+      );
+    } else {
+      browser.test.assertDeepEq(
+        { matchedRules: [] },
+        await browser.declarativeNetRequest.testMatchOutcome(dummyRequest),
+        "declarativeNetRequest.testMatchOutcome is available"
+      );
+    }
     browser.test.sendMessage("done");
   }
   let extension = ExtensionTestUtils.loadExtension({
@@ -106,6 +137,7 @@ add_task(async function dnr_feedback_apis_disabled_by_default() {
       allowDNRFeedback: PREF_DNR_FEEDBACK_DEFAULT_VALUE,
       testExpectations: {
         declarativeNetRequest_available: true,
+        testMatchOutcome_available: kTestMatchOutcomeNotAllowed,
       },
       manifest: {
         permissions: [
@@ -118,15 +150,13 @@ add_task(async function dnr_feedback_apis_disabled_by_default() {
   });
 
   AddonTestUtils.checkMessages(messages, {
-    expected: [
-      {
-        message: /Reading manifest: Invalid extension permission: declarativeNetRequestFeedback/,
-      },
-    ],
     forbidden: [
       {
         message: /Reading manifest: Invalid extension permission: declarativeNetRequest$/,
       },
+      {
+        message: /Reading manifest: Invalid extension permission: declarativeNetRequestFeedback/,
+      },
       {
         message: /Reading manifest: Invalid extension permission: declarativeNetRequestWithHostAccess/,
       },
@@ -251,7 +281,7 @@ add_task(async function declarativeNetRequestFeedback_without_feature() {
     testExpectations: {
       declarativeNetRequest_available: true,
       // all permissions set, but DNR feedback feature not allowed.
-      testMatchOutcome_available: false,
+      testMatchOutcome_available: kTestMatchOutcomeNotAllowed,
     },
     manifest: {
       permissions: ["declarativeNetRequest", "declarativeNetRequestFeedback"],

diff --git a/toolkit/components/extensions/test/xpcshell/test_ext_permission_warnings.js b/toolkit/components/extensions/test/xpcshell/test_ext_permission_warnings.js
@@ -475,7 +475,7 @@ add_task(async function declarativeNetRequest_unavailable_by_default() {
   let manifestPermissions = await getManifestPermissions({
     manifest: {
       manifest_version: 3,
-      permissions: ["declarativeNetRequest"],
+      permissions: ["declarativeNetRequest", "declarativeNetRequestFeedback"],
     },
   });
   deepEqual(
@@ -491,13 +491,16 @@ add_task(
     let manifestPermissions = await getManifestPermissions({
       manifest: {
         manifest_version: 3,
-        permissions: ["declarativeNetRequest"],
+        permissions: ["declarativeNetRequest", "declarativeNetRequestFeedback"],
       },
     });
 
     deepEqual(
       manifestPermissions,
-      { origins: [], permissions: ["declarativeNetRequest"] },
+      {
+        origins: [],
+        permissions: ["declarativeNetRequest", "declarativeNetRequestFeedback"],
+      },
       "Expected origins and permissions"
     );
 
@@ -507,6 +510,9 @@ add_task(
         bundle.GetStringFromName(
           "webextPerms.description.declarativeNetRequest"
         ),
+        bundle.GetStringFromName(
+          "webextPerms.description.declarativeNetRequestFeedback"
+        ),
       ],
       "Expected warnings"
     );

diff --git a/toolkit/components/extensions/test/xpcshell/test_ext_permissions.js b/toolkit/components/extensions/test/xpcshell/test_ext_permissions.js
@@ -626,7 +626,6 @@ const GRANTED_WITHOUT_USER_PROMPT = [
   "contextMenus",
   "contextualIdentities",
   "cookies",
-  "declarativeNetRequestFeedback",
   "declarativeNetRequestWithHostAccess",
   "dns",
   "geckoProfiler",