DNR: Ensure page whitelisting API can't be called by extensions with no rulesets.

This CL ensures that all the page whitelisting API functions
(chrome.declarativeNetRequest.*) can't be called by extensions which don't have
a declarative ruleset. It doesn't make sense for extensions to whitelist page
patterns if they don't have a registered declarative ruleset.

BUG=811460, 696822
Doc=https://docs.google.com/document/d/1Fhm-t0JCc3dcmwyBX_i7TGyuJ2QjqDechv1-VCRBo5o/edit?usp=sharing

Change-Id: Id2822af5f258a32889e9217a3c63e13879984312
Reviewed-on: https://chromium-review.googlesource.com/987314
Commit-Queue: Karan Bhatia <karandeepb@chromium.org>
Reviewed-by: Istiaque Ahmed <lazyboy@chromium.org>
Cr-Commit-Position: refs/heads/master@{#547636}

chrome/browser/extensions/api/declarative_net_request/declarative_net_request_apitest.cc

@@ -6,21 +6,48 @@
 
 #include "base/files/file_path.h"
 #include "base/files/file_util.h"
+#include "base/files/scoped_temp_dir.h"
+#include "base/macros.h"
 #include "base/threading/thread_restrictions.h"
 
-IN_PROC_BROWSER_TEST_F(ExtensionApiTest, DNRPageWhitelisting) {
-  base::FilePath extension_path =
-      test_data_dir_.AppendASCII("declarative_net_request");
-
-  // Copy the extension directory to a temporary location. We do this to ensure
-  // that the temporary kMetadata folder created as a result of loading the
-  // extension is not written to the src directory and is automatically removed.
-  base::ScopedAllowBlockingForTesting allow_blocking;
-  base::ScopedTempDir temp_dir;
-  ASSERT_TRUE(temp_dir.CreateUniqueTempDir());
-  base::CopyDirectory(extension_path, temp_dir.GetPath(), true /*recursive*/);
-
-  // Override the path used for loading the extension.
-  test_data_dir_ = temp_dir.GetPath();
-  ASSERT_TRUE(RunExtensionTest("declarative_net_request")) << message_;
+namespace {
+
+class DeclarativeNetRequestAPItest : public ExtensionApiTest {
+ public:
+  DeclarativeNetRequestAPItest() {}
+
+ protected:
+  // ExtensionApiTest override.
+  void SetUpOnMainThread() override {
+    ExtensionApiTest::SetUpOnMainThread();
+
+    base::FilePath test_data_dir =
+        test_data_dir_.AppendASCII("declarative_net_request");
+
+    // Copy the |test_data_dir| to a temporary location. We do this to ensure
+    // that the temporary kMetadata folder created as a result of loading the
+    // extension is not written to the src directory and is automatically
+    // removed.
+    base::ScopedAllowBlockingForTesting allow_blocking;
+    ASSERT_TRUE(temp_dir_.CreateUniqueTempDir());
+    base::CopyDirectory(test_data_dir, temp_dir_.GetPath(), true /*recursive*/);
+
+    // Override the path used for loading the extension.
+    test_data_dir_ = temp_dir_.GetPath().AppendASCII("declarative_net_request");
+  }
+
+ private:
+  base::ScopedTempDir temp_dir_;
+
+  DISALLOW_COPY_AND_ASSIGN(DeclarativeNetRequestAPItest);
+};
+
+IN_PROC_BROWSER_TEST_F(DeclarativeNetRequestAPItest, PageWhitelistingAPI) {
+  ASSERT_TRUE(RunExtensionTest("page_whitelisting_api")) << message_;
 }
+
+IN_PROC_BROWSER_TEST_F(DeclarativeNetRequestAPItest, ExtensionWithNoRuleset) {
+  ASSERT_TRUE(RunExtensionTest("extension_with_no_ruleset")) << message_;
+}
+
+}  // namespace

chrome/test/data/extensions/api_test/declarative_net_request/extension_with_no_ruleset/background.js

+// Copyright 2018 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+var callbackFail = chrome.test.callbackFail;
+var expectedError =
+    'The extension must have a ruleset in order to call this function.';
+
+chrome.test.runTests([
+  // The extension shouldn't be able to call any of the page whitelisting
+  // functions from chrome.declarativeNetRequest.* since it does not have a
+  // declarative ruleset.
+  function addWhitelistedPages() {
+    chrome.declarativeNetRequest.addWhitelistedPages(
+        ['https://www.google.com/'], callbackFail(expectedError));
+  },
+
+  function removeWhitelistedPages() {
+    chrome.declarativeNetRequest.removeWhitelistedPages(
+        ['https://www.google.com/'], callbackFail(expectedError));
+  },
+
+  function getWhitelistedPages() {
+    chrome.declarativeNetRequest.getWhitelistedPages(
+        callbackFail(expectedError));
+  }
+]);

chrome/test/data/extensions/api_test/declarative_net_request/extension_with_no_ruleset/manifest.json

+{
+  "name": "Test extension with no declarative ruleset.",
+  "manifest_version": 2,
+  "permissions": [
+    "declarativeNetRequest"
+  ],
+  "version": "1.0",
+  "background": {
+    "scripts": [
+      "background.js"
+    ]
+  }
+}

extensions/browser/api/declarative_net_request/declarative_net_request_api.cc

@@ -8,6 +8,7 @@
 
 #include "base/bind.h"
 #include "content/public/browser/browser_thread.h"
+#include "extensions/browser/api/declarative_net_request/rules_monitor_service.h"
 #include "extensions/browser/api/declarative_net_request/ruleset_manager.h"
 #include "extensions/browser/extension_prefs.h"
 #include "extensions/browser/extension_system.h"
@@ -19,6 +20,26 @@
 
 namespace extensions {
 
+namespace {
+
+// Returns true if the given |extension| has a registered ruleset. If it
+// doesn't, returns false and populates |error|.
+bool HasRegisteredRuleset(content::BrowserContext* context,
+                          const Extension* extension,
+                          std::string* error) {
+  const auto* rules_monitor_service = BrowserContextKeyedAPIFactory<
+      declarative_net_request::RulesMonitorService>::Get(context);
+  DCHECK(rules_monitor_service);
+
+  if (rules_monitor_service->HasRegisteredRuleset(extension))
+    return true;
+
+  *error = "The extension must have a ruleset in order to call this function.";
+  return false;
+}
+
+}  // namespace
+
 DeclarativeNetRequestUpdateWhitelistedPagesFunction::
     DeclarativeNetRequestUpdateWhitelistedPagesFunction() = default;
 DeclarativeNetRequestUpdateWhitelistedPagesFunction::
@@ -62,6 +83,12 @@ DeclarativeNetRequestUpdateWhitelistedPagesFunction::UpdateWhitelistedPages(
   return RespondNow(NoArguments());
 }
 
+bool DeclarativeNetRequestUpdateWhitelistedPagesFunction::PreRunValidation(
+    std::string* error) {
+  return UIThreadExtensionFunction::PreRunValidation(error) &&
+         HasRegisteredRuleset(browser_context(), extension(), error);
+}
+
 DeclarativeNetRequestAddWhitelistedPagesFunction::
     DeclarativeNetRequestAddWhitelistedPagesFunction() = default;
 DeclarativeNetRequestAddWhitelistedPagesFunction::
@@ -97,6 +124,12 @@ DeclarativeNetRequestGetWhitelistedPagesFunction::
 DeclarativeNetRequestGetWhitelistedPagesFunction::
     ~DeclarativeNetRequestGetWhitelistedPagesFunction() = default;
 
+bool DeclarativeNetRequestGetWhitelistedPagesFunction::PreRunValidation(
+    std::string* error) {
+  return UIThreadExtensionFunction::PreRunValidation(error) &&
+         HasRegisteredRuleset(browser_context(), extension(), error);
+}
+
 ExtensionFunction::ResponseAction
 DeclarativeNetRequestGetWhitelistedPagesFunction::Run() {
   const ExtensionPrefs* prefs = ExtensionPrefs::Get(browser_context());

extensions/browser/api/declarative_net_request/declarative_net_request_api.h

@@ -29,6 +29,9 @@ class DeclarativeNetRequestUpdateWhitelistedPagesFunction
       const std::vector<std::string>& patterns,
       Action action);
 
+  // ExtensionFunction override:
+  bool PreRunValidation(std::string* error) override;
+
  private:
   DISALLOW_COPY_AND_ASSIGN(DeclarativeNetRequestUpdateWhitelistedPagesFunction);
 };
@@ -83,7 +86,8 @@ class DeclarativeNetRequestGetWhitelistedPagesFunction
  protected:
   ~DeclarativeNetRequestGetWhitelistedPagesFunction() override;
 
-  // ExtensionFunction override:
+  // ExtensionFunction overrides:
+  bool PreRunValidation(std::string* error) override;
   ExtensionFunction::ResponseAction Run() override;
 
  private:

extensions/browser/api/declarative_net_request/rules_monitor_service.cc

@@ -100,6 +100,12 @@ bool RulesMonitorService::HasAnyRegisteredRulesets() const {
   return !extensions_with_rulesets_.empty();
 }
 
+bool RulesMonitorService::HasRegisteredRuleset(
+    const Extension* extension) const {
+  return extensions_with_rulesets_.find(extension) !=
+         extensions_with_rulesets_.end();
+}
+
 RulesMonitorService::RulesMonitorService(
     content::BrowserContext* browser_context)
     : registry_observer_(this),

extensions/browser/api/declarative_net_request/rules_monitor_service.h

@@ -37,6 +37,9 @@ class RulesMonitorService : public BrowserContextKeyedAPI,
 
   bool HasAnyRegisteredRulesets() const;
 
+  // Returns true if the given |extension| has a registered declarative ruleset.
+  bool HasRegisteredRuleset(const Extension* extension) const;
+
  private:
   friend class BrowserContextKeyedAPIFactory<RulesMonitorService>;