chrome.webRequest support for ExtensionSettings

chrome/browser/extensions/api/web_request/web_request_apitest.cc

Index: chrome/browser/extensions/api/web_request/web_request_apitest.cc
diff --git a/chrome/browser/extensions/api/web_request/web_request_apitest.cc b/chrome/browser/extensions/api/web_request/web_request_apitest.cc
index 689edf0961772cf262738164502912176a36e5bb..719eb62b2522cc763948e709cf9a8585ea36b36d 100644
--- a/chrome/browser/extensions/api/web_request/web_request_apitest.cc
+++ b/chrome/browser/extensions/api/web_request/web_request_apitest.cc
@@ -15,6 +15,7 @@
 #include "chrome/browser/extensions/extension_action_runner.h"
 #include "chrome/browser/extensions/extension_apitest.h"
 #include "chrome/browser/extensions/extension_service.h"
+#include "chrome/browser/extensions/extension_with_management_policy_apitest.h"
 #include "chrome/browser/extensions/tab_helper.h"
 #include "chrome/browser/extensions/test_extension_dir.h"
 #include "chrome/browser/profiles/profile.h"
@@ -46,6 +47,7 @@
 #include "extensions/test/result_catcher.h"
 #include "net/dns/mock_host_resolver.h"
 #include "net/test/embedded_test_server/embedded_test_server.h"
+#include "net/test/embedded_test_server/http_request.h"
 #include "net/test/test_data_directory.h"
 #include "net/traffic_annotation/network_traffic_annotation_test_helper.h"
 #include "net/url_request/test_url_fetcher_factory.h"
@@ -187,10 +189,10 @@ class TestURLFetcherDelegate : public net::URLFetcherDelegate {
 
 }  // namespace
 
-class ExtensionWebRequestApiTest : public ExtensionApiTest {
+class ExtensionWebRequestApiTest : public ExtensionApiTestWithManagementPolicy {

[Devlin, 2017/05/25 20:38:32]

Let's not have all web request API tests inherit from this, but rather only the
ones that use it.

[nrpeter, 2017/05/26 02:46:52]

Done.

  public:
   void SetUpInProcessBrowserTestFixture() override {
-    ExtensionApiTest::SetUpInProcessBrowserTestFixture();
+    ExtensionApiTestWithManagementPolicy::SetUpInProcessBrowserTestFixture();
     host_resolver()->AddRule("*", "127.0.0.1");
   }
 
@@ -954,4 +956,215 @@ IN_PROC_BROWSER_TEST_F(ExtensionWebRequestApiTest,
   }
 }
 
+// Test that web requests made by a webpage protected by the

[Devlin, 2017/05/25 20:38:32]

test comments should be descriptive, not imperative.

[nrpeter, 2017/05/26 02:46:52]

Done.

+// runtime_blocked_hosts of the ExtensionsSettings policy are not modifyable.

[Devlin, 2017/05/25 20:38:32]

typo: modifiable

[nrpeter, 2017/05/26 02:46:52]

Done.

+IN_PROC_BROWSER_TEST_F(ExtensionWebRequestApiTest, policyBlockByOrigin) {

[Devlin, 2017/05/25 20:38:32]

Tests should be TitleCase

[nrpeter, 2017/05/26 02:46:52]

Done.

+  // Browse to protected example.com page which uses javascript to
+  // include a script from another domain example2.com. We use the
+  // chrome.webRequest API on the extension's background page to listen for
+  // requests to example2.com. In this first part of the test, we want to
+  // make sure our extension successfully detects requests to example2.com with
+  // the webRequest API. Additionally this tests that this protection only
+  // applies for domains listed in the policy.
+
+  // Host that we navigate to in Chrome.
+  const std::string example_com = "example.com";
+
+  // Domain that hosts javascript file referenced by example_com.
+  const std::string example2_com = "example2.com";
+
+  // URL within the test extension we navigate to to initiate the requests.
+  const std::string extension_test_url =
+      "/extensions/api_test/webrequest/policy_blocked/ref_remote_js.html";
+
+  // Extension communicates back using this listener name.
+  const std::string listener_name = "protected_origin";
+
+  // We expect that no webRequest will be hidden or modification blocked. This
+  // means that the request to example.com will be seen by the extension.
+  {
+    ExtensionManagementPolicyUpdater pref(&policy_provider_);
+    pref.AddRuntimeBlockedHost("*", "*://not_example.com");

[Devlin, 2017/05/25 20:38:32]

Underscores aren't allowed in domain names by the spec, so let's keep this
valid.

[nrpeter, 2017/05/26 02:46:51]

Done.

+  }
+  // Set auto confirm UI flag.
+  PermissionsRequestFunction::SetAutoConfirmForTests(true);

[Devlin, 2017/05/25 20:38:32]

Why do we need this?

[nrpeter, 2017/05/26 02:46:51]

Done.

+  PermissionsRequestFunction::SetIgnoreUserGestureForTests(true);
+
+  ASSERT_TRUE(StartEmbeddedTestServer());
+
+  LoadExtension(test_data_dir_.AppendASCII("webrequest/policy_blocked"));
+
+  // Listen to verify extension sees the web request.
+  ExtensionTestMessageListener before_request_listener(listener_name, false);
+
+  // Wait until all remote Javascript files have been blocked / pulled down.
+  ui_test_utils::NavigateToURLWithDisposition(
+      browser(),
+      embedded_test_server()->GetURL(example_com, extension_test_url),

[Devlin, 2017/05/25 20:38:32]

instead of const'ing example.com and the file, we should cache a GURL and use
that.

[nrpeter, 2017/05/26 02:46:52]

Made a const GURL as suggested. However I kept example.com const'd since it is
referenced more than once. Also went through other tests and const'd strings
that were repeated.

+      WindowOpenDisposition::CURRENT_TAB,
+      ui_test_utils::BROWSER_TEST_WAIT_FOR_NAVIGATION);
+
+  // The server saw a request for the remote Javascript file.
+  EXPECT_TRUE(BrowsedTo(example2_com));
+
+  // The webRequest was seen by the extension.
+  EXPECT_TRUE(before_request_listener.was_satisfied());
+
+  // Clear the list of domains the server has seen.
+  ClearRequestLog();
+
+  // Make sure we've cleared the embedded server history.
+  EXPECT_FALSE(BrowsedTo(example2_com));
+
+  // Now lets set the policy to hide webRequests to example.com or any resource

[Devlin, 2017/05/25 20:38:32]

In comments like this, omit words like "let's", "we'll", etc.  So here, just:
// Set the policy to hide requests to example.com...

[nrpeter, 2017/05/26 02:46:51]

Think I got all of these. If you notice any I missed I'm happy to fix.

+  // it includes. We expect that in this test, the webRequest to example2.com
+  // will not be seen by the extension.
+  {
+    ExtensionManagementPolicyUpdater pref(&policy_provider_);
+    pref.AddRuntimeBlockedHost("*", "*://" + example_com);
+  }
+
+  // Listen in case extension sees the web requst.
+  ExtensionTestMessageListener before_request_listener2(listener_name, false);
+
+  // Wait until all remote Javascript files have been pulled down.
+  ui_test_utils::NavigateToURLWithDisposition(
+      browser(),
+      embedded_test_server()->GetURL(example_com, extension_test_url),
+      WindowOpenDisposition::CURRENT_TAB,
+      ui_test_utils::BROWSER_TEST_WAIT_FOR_NAVIGATION);
+
+  // The server saw a request for the remote Javascript file.
+  EXPECT_TRUE(BrowsedTo(example2_com));
+
+  // The webRequest was hidden from the extension.
+  EXPECT_FALSE(before_request_listener2.was_satisfied());
+}
+
+// Test that web requests made to a resource protected by the
+// runtime_blocked_hosts of the ExtensionsSettings policy are not visible.
+IN_PROC_BROWSER_TEST_F(ExtensionWebRequestApiTest, policyBlockByUrl) {
+  // We'll set policy to block webRequest events for example.com domains. Then
+  // navigate to example.com and make sure our extension doesn't see the event.
+  // We then navigate to a website not protected by policy and make sure that
+  // our extension can see the event.
+
+  // Set example.com as protected using runtime_blocked_hosts from the
+  // ExtensionSettings policy.
+  {
+    ExtensionManagementPolicyUpdater pref(&policy_provider_);
+    pref.AddRuntimeBlockedHost("*", "*://example.com");
+  }
+
+  // Set auto confirm UI flag.
+  PermissionsRequestFunction::SetAutoConfirmForTests(true);
+  PermissionsRequestFunction::SetIgnoreUserGestureForTests(true);
+
+  ASSERT_TRUE(StartEmbeddedTestServer());
+
+  LoadExtension(test_data_dir_.AppendASCII("webrequest/policy_blocked"));
+
+  // Listen in case extension sees the web requst.
+  ExtensionTestMessageListener before_request_listener("protected_url", false);
+
+  // Navigate to the protected domain and wait until page fully loads.
+  ui_test_utils::NavigateToURLWithDisposition(
+      browser(),
+      embedded_test_server()->GetURL(
+          "example.com",
+          "/extensions/api_test/webrequest/policy_blocked/protected_url.html"),
+      WindowOpenDisposition::CURRENT_TAB,
+      ui_test_utils::BROWSER_TEST_WAIT_FOR_NAVIGATION);
+
+  // The server saw a request for the protected site.
+  EXPECT_TRUE(BrowsedTo("example.com"));
+
+  // The webRequest was hidden from the extension.
+  EXPECT_FALSE(before_request_listener.was_satisfied());
+
+  // Now we'll test browsing to a non-protected website where we expect the
+  // extension to see the webrequest event.
+  ui_test_utils::NavigateToURLWithDisposition(
+      browser(),
+      embedded_test_server()->GetURL(
+          "not_blocked_example.com",
+          "/extensions/api_test/webrequest/policy_blocked/protected_url.html"),
+      WindowOpenDisposition::CURRENT_TAB,
+      ui_test_utils::BROWSER_TEST_WAIT_FOR_NAVIGATION);
+
+  // The server saw a request for the non-protected site.
+  EXPECT_TRUE(BrowsedTo("not_blocked_example.com"));
+
+  // The webRequest was visible from the extension.
+  EXPECT_TRUE(before_request_listener.was_satisfied());
+}
+
+// Test that no webRequest events are seen for a protected host during normal
+// navigation. This replicates most of the tests from
+// WebRequestWithWithheldPermissions with a protected host. Granting a tab
+// specific permission shouldn't bypass our policy.
+IN_PROC_BROWSER_TEST_F(ExtensionWebRequestApiTest,
+                       WebRequestProtectedByPolicy) {
+  FeatureSwitch::ScopedOverride enable_scripts_require_action(
+      FeatureSwitch::scripts_require_action(), true);
+  {
+    ExtensionManagementPolicyUpdater pref(&policy_provider_);
+    pref.AddRuntimeBlockedHost("*", "*://example.com");
+  }
+  extensions::PermissionsRequestFunction::SetIgnoreUserGestureForTests(true);
+  extensions::PermissionsRequestFunction::SetAutoConfirmForTests(true);
+  ASSERT_TRUE(StartEmbeddedTestServer());
+
+  ExtensionTestMessageListener listener("ready", false);
+  const Extension* extension =
+      LoadExtension(test_data_dir_.AppendASCII("webrequest_activetab"));
+  ASSERT_TRUE(extension) << message_;
+  EXPECT_TRUE(listener.WaitUntilSatisfied());
+
+  // Navigate the browser to a page in a new tab.
+  const std::string kHost = "example.com";
+  GURL url = embedded_test_server()->GetURL(kHost, "/empty.html");
+  chrome::NavigateParams params(browser(), url, ui::PAGE_TRANSITION_LINK);
+  params.disposition = WindowOpenDisposition::NEW_FOREGROUND_TAB;
+  ui_test_utils::NavigateToURL(&params);
+
+  content::WebContents* web_contents =
+      browser()->tab_strip_model()->GetActiveWebContents();
+  ASSERT_TRUE(web_contents);
+  ExtensionActionRunner* runner =
+      ExtensionActionRunner::GetForWebContents(web_contents);
+  ASSERT_TRUE(runner);
+
+  int port = embedded_test_server()->port();
+  const std::string kXhrPath = "simple.html";
+
+  // The extension shouldn't have currently received any webRequest events,
+  // since it doesn't have permission (and shouldn't receive any from an XHR).
+  EXPECT_EQ(0, GetWebRequestCountFromBackgroundPage(extension, profile()));
+  PerformXhrInFrame(web_contents->GetMainFrame(), kHost, port, kXhrPath);
+  EXPECT_EQ(0, GetWebRequestCountFromBackgroundPage(extension, profile()));
+
+  // Grant activeTab permission, and perform another XHR. The extension should
+  // still be blocked due to ExtensionSettings policy on example.com.
+  // Only records ACCESS_WITHHELD, not ACCESS_DENIED, this is why we match
+  // BLOCKED_ACTION_NONE.
+  EXPECT_EQ(BLOCKED_ACTION_NONE, runner->GetBlockedActions(extension));
+  runner->set_default_bubble_close_action_for_testing(
+      base::WrapUnique(new ToolbarActionsBarBubbleDelegate::CloseAction(
+          ToolbarActionsBarBubbleDelegate::CLOSE_EXECUTE)));
+  runner->RunAction(extension, true);
+  base::RunLoop().RunUntilIdle();
+  EXPECT_TRUE(content::WaitForLoadStop(web_contents));
+  // The runner will have refreshed the page...
+  EXPECT_EQ(BLOCKED_ACTION_NONE, runner->GetBlockedActions(extension));
+  int xhr_count = GetWebRequestCountFromBackgroundPage(extension, profile());
+  // ... which means that we should have a non-zero xhr count if the policy
+  // didn't block the events.
+  EXPECT_EQ(xhr_count, 0);
+  // And the extension should also block future events.
+  PerformXhrInFrame(web_contents->GetMainFrame(), kHost, port, kXhrPath);
+  EXPECT_EQ(xhr_count,
+            GetWebRequestCountFromBackgroundPage(extension, profile()));
+}
+
 }  // namespace extensions