Implement origin specific Permissions Blacklisting.

chrome/browser/permissions/permission_context_base_unittest.cc

 // Copyright 2014 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.
 
 #include "chrome/browser/permissions/permission_context_base.h"
 
 #include <map>
+#include <set>
 #include <string>
 #include <utility>
 #include <vector>
 
 #include "base/bind.h"
 #include "base/feature_list.h"
 #include "base/macros.h"
 #include "base/memory/ptr_util.h"
 #include "base/metrics/field_trial.h"
+#include "base/run_loop.h"
 #include "base/test/histogram_tester.h"
 #include "base/test/mock_entropy_provider.h"
 #include "base/test/scoped_feature_list.h"
 #include "build/build_config.h"
 #include "chrome/browser/content_settings/host_content_settings_map_factory.h"
 #include "chrome/browser/infobars/infobar_service.h"
 #include "chrome/browser/permissions/permission_decision_auto_blocker.h"
 #include "chrome/browser/permissions/permission_queue_controller.h"
 #include "chrome/browser/permissions/permission_request_id.h"
 #include "chrome/browser/permissions/permission_util.h"
 #include "chrome/common/chrome_features.h"
 #include "chrome/common/chrome_switches.h"
 #include "chrome/test/base/chrome_render_view_host_test_harness.h"
 #include "chrome/test/base/testing_profile.h"
 #include "components/content_settings/core/browser/host_content_settings_map.h"
 #include "components/content_settings/core/common/content_settings.h"
 #include "components/content_settings/core/common/content_settings_types.h"
+#include "components/safe_browsing_db/database_manager.h"
+#include "components/safe_browsing_db/test_database_manager.h"
 #include "components/variations/variations_associated_data.h"
+#include "content/public/browser/browser_thread.h"
 #include "content/public/browser/permission_type.h"
 #include "content/public/browser/render_frame_host.h"
 #include "content/public/browser/web_contents.h"
 #include "content/public/test/mock_render_process_host.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
 #if !defined(OS_ANDROID)
 #include "chrome/browser/permissions/permission_request_manager.h"
 #endif
 
 const char* const kPermissionsKillSwitchFieldStudy =
     PermissionContextBase::kPermissionsKillSwitchFieldStudy;
 const char* const kPermissionsKillSwitchBlockedValue =
     PermissionContextBase::kPermissionsKillSwitchBlockedValue;
 const char kPermissionsKillSwitchTestGroup[] = "TestGroup";
 const char* const kPromptGroupName = kPermissionsKillSwitchTestGroup;
 const char kPromptTrialName[] = "PermissionPromptsUX";
 
+class MockSafeBrowsingDatabaseManager
+    : public safe_browsing::TestSafeBrowsingDatabaseManager {
+ public:
+  explicit MockSafeBrowsingDatabaseManager(bool perform_callback)
+      : perform_callback_(perform_callback) {}
+
+  bool CheckApiBlacklistUrl(
+      const GURL& url,
+      safe_browsing::SafeBrowsingDatabaseManager::Client* client) override {
+    if (perform_callback_) {
+      safe_browsing::ThreatMetadata metadata;
+      std::map<GURL, std::set<std::string>>::iterator blacklisted_permissions =

[raymes, 2017/01/09 06:28:18]

nit: It would be fine to use "auto" here. We also usually would use a const
reference here, e.g.
const auto& blacklisted_permissions = 

[meredithl, 2017/01/10 02:24:57]

Done. Still getting used to C++11 :)

+          permissions_blacklist_.find(url);
+      if (blacklisted_permissions != permissions_blacklist_.end())
+        metadata.api_permissions = blacklisted_permissions->second;
+      client->OnCheckApiBlacklistUrlResult(url, metadata);
+    }
+    // Returns false if scheme is HTTP/HTTPS and able to be checked.
+    return false;
+  }
+
+  bool CancelApiCheck(Client* client) override {
+    if (perform_callback_)

[raymes, 2017/01/09 06:28:17]

nit: can have DCHECK(!perform_callback_) here to avoid the if statement

[meredithl, 2017/01/10 02:24:57]

Done.

+      NOTREACHED();
+    // Returns true when client check could be stopped.
+    return true;
+  }
+
+  void BlacklistUrlPermissions(const GURL& url,
+                               const std::set<std::string> permissions) {
+    permissions_blacklist_[url] = permissions;
+  }
+
+ protected:
+  ~MockSafeBrowsingDatabaseManager() override {}
+
+ private:
+  bool perform_callback_;
+  std::map<GURL, std::set<std::string>> permissions_blacklist_;
+
+  DISALLOW_COPY_AND_ASSIGN(MockSafeBrowsingDatabaseManager);
+};
+
 class TestPermissionContext : public PermissionContextBase {
  public:
   TestPermissionContext(Profile* profile,
                         const content::PermissionType permission_type,
                         const ContentSettingsType content_settings_type)
       : PermissionContextBase(profile, permission_type, content_settings_type),
         tab_context_updated_(false) {}
 
   ~TestPermissionContext() override {}
 
 #if defined(OS_ANDROID)
   PermissionQueueController* GetInfoBarController() {
     return GetQueueController();
   }
 #endif
 
   const std::vector<ContentSetting>& decisions() const { return decisions_; }
 
   bool tab_context_updated() const { return tab_context_updated_; }
 
+  // Once a decision for the requested permission has been made, run the
+  // callback.
   void TrackPermissionDecision(ContentSetting content_setting) {
     decisions_.push_back(content_setting);
+    content::BrowserThread::PostTask(content::BrowserThread::UI, FROM_HERE,
+                                     quit_closure_);

[raymes, 2017/01/09 06:28:17]

Can we just call quit_closure.Run() here instead of posting?

[meredithl, 2017/01/10 02:24:57]

It appears we can! These test methods have been hacked into place, so this must
have been a hangover from how we were previously doing it.

   }
 
   ContentSetting GetContentSettingFromMap(const GURL& url_a,
                                           const GURL& url_b) {
     auto* map = HostContentSettingsMapFactory::GetForProfile(profile());
     return map->GetContentSetting(url_a.GetOrigin(), url_b.GetOrigin(),
                                   content_settings_type(), std::string());
   }
 
+  void RequestPermission(content::WebContents* web_contents,
+                         const PermissionRequestID& id,
+                         const GURL& requesting_frame,
+                         bool user_gesture,
+                         const BrowserPermissionCallback& callback) override {
+    base::RunLoop run_loop;
+    quit_closure_ = run_loop.QuitClosure();
+    PermissionContextBase::RequestPermission(web_contents, id, requesting_frame,
+                                             true /* user_gesture */, callback);
+    run_loop.Run();
+  }
+
+  void DecidePermission(content::WebContents* web_contents,
+                        const PermissionRequestID& id,
+                        const GURL& requesting_origin,
+                        const GURL& embedding_origin,
+                        bool user_gesture,
+                        const BrowserPermissionCallback& callback) override {
+    PermissionContextBase::DecidePermission(web_contents, id, requesting_origin,
+                                            embedding_origin, user_gesture,
+                                            callback);
+    if (respond_permission_)
+      respond_permission_.Run();

[raymes, 2017/01/09 06:28:17]

Same here, we probably want to reset this after running it

[meredithl, 2017/01/10 02:24:57]

Done.

+    else
+      quit_closure_.Run();

[raymes, 2017/01/09 06:28:18]

I wonder if we can just remove this line?

[meredithl, 2017/01/10 02:24:57]

respond_permission_ only gets set in the TestParallelRequests* methods after the
first request is made, which means for the first request the run loop just spins
indefinitely, because the quit_closure_ only gets run from the callback. I'll
add a comment explaining this, unless there is a better way to achieve the same
behaviour. It is a bit messy :/

+  }
+
+  // Permission request will need to be responded to, so pass a callback to be
+  // run once the request has completed and the decision has been made.
+  void SetRespondPermissionCallback(base::Closure callback) {
+    respond_permission_ = callback;
+  }
+
  protected:
   void UpdateTabContext(const PermissionRequestID& id,
                         const GURL& requesting_origin,
                         bool allowed) override {
     tab_context_updated_ = true;
   }
 
   bool IsRestrictedToSecureOrigins() const override {
     return false;
   }
 
  private:
   std::vector<ContentSetting> decisions_;
   bool tab_context_updated_;
-
+  base::Closure quit_closure_;
+  // Callback for responding to a permission once the request has been completed
+  // (valid URL, kill switch disabled, not blacklisted)
+  base::Closure respond_permission_;
   DISALLOW_COPY_AND_ASSIGN(TestPermissionContext);
 };
 
 class TestKillSwitchPermissionContext : public TestPermissionContext {
  public:
   TestKillSwitchPermissionContext(
       Profile* profile,
       const content::PermissionType permission_type,
       const ContentSettingsType content_settings_type)
       : TestPermissionContext(profile, permission_type, content_settings_type),
@@ skipping 64 matching lines @@
     TestPermissionContext permission_context(profile(), permission,
                                              content_settings_type);
     GURL url("https://www.google.com");
     NavigateAndCommit(url);
     base::HistogramTester histograms;
 
     const PermissionRequestID id(
         web_contents()->GetRenderProcessHost()->GetID(),
         web_contents()->GetMainFrame()->GetRoutingID(),
         -1);
+    permission_context.SetRespondPermissionCallback(
+        base::Bind(&PermissionContextBaseTests::RespondToPermission,
+                   base::Unretained(this), &permission_context, id, url,
+                   persist, decision));
     permission_context.RequestPermission(
         web_contents(),
         id, url, true /* user_gesture */,
         base::Bind(&TestPermissionContext::TrackPermissionDecision,
                    base::Unretained(&permission_context)));
-
-    RespondToPermission(&permission_context, id, url, persist, decision);
     ASSERT_EQ(1u, permission_context.decisions().size());
     EXPECT_EQ(decision, permission_context.decisions()[0]);
     EXPECT_TRUE(permission_context.tab_context_updated());
 
     std::string decision_string;
     if (decision == CONTENT_SETTING_ALLOW)
       decision_string = "Accepted";
     else if (decision == CONTENT_SETTING_BLOCK)
       decision_string = "Denied";
     else if (decision == CONTENT_SETTING_ASK)
@@ skipping 30 matching lines @@
     // from dismiss to block, and hence change the persisted content setting.
     for (uint32_t i = 0; i < iterations; ++i) {
       TestPermissionContext permission_context(
           profile(), permission_type, content_settings_type);
       ContentSetting expected =
           (i < (iterations - 1)) ? CONTENT_SETTING_ASK : CONTENT_SETTING_BLOCK;
 
       const PermissionRequestID id(
           web_contents()->GetRenderProcessHost()->GetID(),
           web_contents()->GetMainFrame()->GetRoutingID(), i);
+
+      permission_context.SetRespondPermissionCallback(
+          base::Bind(&PermissionContextBaseTests::RespondToPermission,
+                     base::Unretained(this), &permission_context, id, url,
+                     false, CONTENT_SETTING_ASK));
+
       permission_context.RequestPermission(
           web_contents(), id, url, true /* user_gesture */,
           base::Bind(&TestPermissionContext::TrackPermissionDecision,
                     base::Unretained(&permission_context)));
-
-      RespondToPermission(&permission_context, id, url, false, /* persist */
-                          CONTENT_SETTING_ASK);
       histograms.ExpectTotalCount(
           "Permissions.Prompt.Dismissed.PriorDismissCount." +
               PermissionUtil::GetPermissionString(permission_type),
           i + 1);
       histograms.ExpectBucketCount(
           "Permissions.Prompt.Dismissed.PriorDismissCount." +
               PermissionUtil::GetPermissionString(permission_type),
           i, 1);
 
       ASSERT_EQ(1u, permission_context.decisions().size());
@@ skipping 17 matching lines @@
 
     // First, ensure that > 3 dismissals behaves correctly.
     for (uint32_t i = 0; i < 4; ++i) {
       TestPermissionContext permission_context(
           profile(), content::PermissionType::GEOLOCATION,
           CONTENT_SETTINGS_TYPE_GEOLOCATION);
 
       const PermissionRequestID id(
           web_contents()->GetRenderProcessHost()->GetID(),
           web_contents()->GetMainFrame()->GetRoutingID(), i);
+
+      permission_context.SetRespondPermissionCallback(
+          base::Bind(&PermissionContextBaseTests::RespondToPermission,
+                     base::Unretained(this), &permission_context, id, url,
+                     false, CONTENT_SETTING_ASK));
       permission_context.RequestPermission(
           web_contents(), id, url, true /* user_gesture */,
           base::Bind(&TestPermissionContext::TrackPermissionDecision,
                     base::Unretained(&permission_context)));
-
-      RespondToPermission(&permission_context, id, url, false, /* persist */
-                          CONTENT_SETTING_ASK);
       histograms.ExpectTotalCount(
           "Permissions.Prompt.Dismissed.PriorDismissCount.Geolocation",
           i + 1);
       histograms.ExpectBucketCount(
           "Permissions.Prompt.Dismissed.PriorDismissCount.Geolocation", i, 1);
       ASSERT_EQ(1u, permission_context.decisions().size());
       EXPECT_EQ(CONTENT_SETTING_ASK, permission_context.decisions()[0]);
       EXPECT_TRUE(permission_context.tab_context_updated());
       EXPECT_EQ(CONTENT_SETTING_ASK,
                 permission_context.GetContentSettingFromMap(url, url));
@@ skipping 57 matching lines @@
     for (uint32_t i = 0; i < 5; ++i) {
       TestPermissionContext permission_context(
           profile(), content::PermissionType::MIDI_SYSEX,
           CONTENT_SETTINGS_TYPE_MIDI_SYSEX);
 
       ContentSetting expected =
           (i < 4) ? CONTENT_SETTING_ASK : CONTENT_SETTING_BLOCK;
       const PermissionRequestID id(
           web_contents()->GetRenderProcessHost()->GetID(),
           web_contents()->GetMainFrame()->GetRoutingID(), i);
+      permission_context.SetRespondPermissionCallback(
+          base::Bind(&PermissionContextBaseTests::RespondToPermission,
+                     base::Unretained(this), &permission_context, id, url,
+                     false, CONTENT_SETTING_ASK));
       permission_context.RequestPermission(
           web_contents(), id, url, true /* user_gesture */,
           base::Bind(&TestPermissionContext::TrackPermissionDecision,
-                    base::Unretained(&permission_context)));
+                     base::Unretained(&permission_context)));
 
-      RespondToPermission(&permission_context, id, url, false, /* persist */
-                          CONTENT_SETTING_ASK);
       EXPECT_EQ(1u, permission_context.decisions().size());
       ASSERT_EQ(expected, permission_context.decisions()[0]);
       EXPECT_TRUE(permission_context.tab_context_updated());
       EXPECT_EQ(expected,
                 permission_context.GetContentSettingFromMap(url, url));
 
       histograms.ExpectTotalCount(
           "Permissions.Prompt.Dismissed.PriorDismissCount.MidiSysEx", i + 1);
       histograms.ExpectBucketCount(
           "Permissions.Prompt.Dismissed.PriorDismissCount.MidiSysEx", i, 1);
@@ skipping 39 matching lines @@
                                       ContentSetting expected_default) {
     TestPermissionContext permission_context(profile(), permission_type,
                                              content_settings_type);
     GURL url("https://www.google.com");
     NavigateAndCommit(url);
 
     const PermissionRequestID id(
         web_contents()->GetRenderProcessHost()->GetID(),
         web_contents()->GetMainFrame()->GetRoutingID(),
         -1);
+    permission_context.SetRespondPermissionCallback(
+        base::Bind(&PermissionContextBaseTests::RespondToPermission,
+                   base::Unretained(this), &permission_context, id, url, true,
+                   CONTENT_SETTING_ALLOW));
+
     permission_context.RequestPermission(
         web_contents(),
         id, url, true /* user_gesture */,
         base::Bind(&TestPermissionContext::TrackPermissionDecision,
                    base::Unretained(&permission_context)));
 
-    RespondToPermission(&permission_context, id, url, true, /* persist */
-                        CONTENT_SETTING_ALLOW);
     ASSERT_EQ(1u, permission_context.decisions().size());
     EXPECT_EQ(CONTENT_SETTING_ALLOW, permission_context.decisions()[0]);
     EXPECT_TRUE(permission_context.tab_context_updated());
     EXPECT_EQ(CONTENT_SETTING_ALLOW,
               permission_context.GetContentSettingFromMap(url, url));
 
     // Try to reset permission.
     permission_context.ResetPermission(url.GetOrigin(), url.GetOrigin());
     ContentSetting setting_after_reset =
         permission_context.GetContentSettingFromMap(url, url);
@@ skipping 31 matching lines @@
     GURL url("http://www.google.com");
     NavigateAndCommit(url);
 
     const PermissionRequestID id0(
         web_contents()->GetRenderProcessHost()->GetID(),
         web_contents()->GetMainFrame()->GetRoutingID(), 0);
     const PermissionRequestID id1(
         web_contents()->GetRenderProcessHost()->GetID(),
         web_contents()->GetMainFrame()->GetRoutingID(), 1);
 
+    bool persist = (response == CONTENT_SETTING_ALLOW ||
+                    response == CONTENT_SETTING_BLOCK);
+
+    // Request a permission without setting the callback to DecidePermission.
     permission_context.RequestPermission(
         web_contents(), id0, url, true /* user_gesture */,
         base::Bind(&TestPermissionContext::TrackPermissionDecision,
                    base::Unretained(&permission_context)));
+
+    EXPECT_EQ(0u, permission_context.decisions().size());
+
+    // Set the callback, and make a second permission request.
+    permission_context.SetRespondPermissionCallback(
+        base::Bind(&PermissionContextBaseTests::RespondToPermission,
+                   base::Unretained(this), &permission_context, id0, url,
+                   persist, response));
     permission_context.RequestPermission(
         web_contents(), id1, url, true /* user_gesture */,
         base::Bind(&TestPermissionContext::TrackPermissionDecision,
                    base::Unretained(&permission_context)));
 
-    EXPECT_EQ(0u, permission_context.decisions().size());
-
-    bool persist = (response == CONTENT_SETTING_ALLOW ||
-                    response == CONTENT_SETTING_BLOCK);
-    RespondToPermission(&permission_context, id0, url, persist, response);
-
     ASSERT_EQ(2u, permission_context.decisions().size());
     EXPECT_EQ(response, permission_context.decisions()[0]);
     EXPECT_EQ(response, permission_context.decisions()[1]);
     EXPECT_TRUE(permission_context.tab_context_updated());
 
     EXPECT_EQ(response, permission_context.GetContentSettingFromMap(url, url));
   }
 
+  void TestPermissionsBlacklisting(
+      content::PermissionType permission_type,
+      ContentSettingsType content_settings_type,
+      scoped_refptr<safe_browsing::SafeBrowsingDatabaseManager> db_manager,
+      const GURL& url,
+      int timeout,
+      ContentSetting response) {
+    NavigateAndCommit(url);
+    base::test::ScopedFeatureList scoped_feature_list;
+    scoped_feature_list.InitAndEnableFeature(features::kPermissionsBlacklist);
+    TestPermissionContext permission_context(profile(), permission_type,
+                                             content_settings_type);
+    permission_context.SetSafeBrowsingDatabaseManagerAndTimeoutForTest(
+        db_manager, timeout);
+    const PermissionRequestID id(
+        web_contents()->GetRenderProcessHost()->GetID(),
+        web_contents()->GetMainFrame()->GetRoutingID(), -1);
+    permission_context.SetRespondPermissionCallback(base::Bind(
+        &PermissionContextBaseTests::RespondToPermission,
+        base::Unretained(this), &permission_context, id, url, false, response));

[raymes, 2017/01/10 02:57:15]

Is this needed?

[meredithl, 2017/01/10 03:40:46]

Turns out for simplicity it is, because we also want to test that a decision can
be made when a) the site isn't blacklisted and b) safe browsing times out.

+    permission_context.RequestPermission(
+        web_contents(), id, url, true /* user_gesture */,
+        base::Bind(&TestPermissionContext::TrackPermissionDecision,
+                   base::Unretained(&permission_context)));
+
+    ASSERT_EQ(1u, permission_context.decisions().size());
+    EXPECT_EQ(response, permission_context.decisions()[0]);
+  }
+
  private:
   // ChromeRenderViewHostTestHarness:
   void SetUp() override {
     ChromeRenderViewHostTestHarness::SetUp();
 #if defined(OS_ANDROID)
     InfoBarService::CreateForWebContents(web_contents());
 #else
     PermissionRequestManager::CreateForWebContents(web_contents());
 #endif
   }
@@ skipping 134 matching lines @@
   TestParallelRequests(CONTENT_SETTING_ALLOW);
 }
 
 TEST_F(PermissionContextBaseTests, TestParallelRequestsBlocked) {
   TestParallelRequests(CONTENT_SETTING_BLOCK);
 }
 
 TEST_F(PermissionContextBaseTests, TestParallelRequestsDismissed) {
   TestParallelRequests(CONTENT_SETTING_ASK);
 }
+
+// Tests a blacklisted (URL, permission) pair has had its permission request
+// blocked.
+TEST_F(PermissionContextBaseTests, TestPermissionsBlacklistingBlocked) {
+  scoped_refptr<MockSafeBrowsingDatabaseManager> db_manager =
+      new MockSafeBrowsingDatabaseManager(true);

[raymes, 2017/01/09 06:28:18]

nit: here and below, document the bool parameter that gets passed in. We always
just put the parameter name in here, so in this case:
new MockSafeBrowsingDatabaseManager(true /* perform_callback */);

[meredithl, 2017/01/10 02:24:57]

Done.

+  const GURL url("https://www.example.com");
+  std::set<std::string> blacklisted_permissions{
+      PermissionUtil::GetPermissionString(
+          content::PermissionType::GEOLOCATION)};
+  db_manager->BlacklistUrlPermissions(url, blacklisted_permissions);
+  TestPermissionsBlacklisting(content::PermissionType::GEOLOCATION,
+                              CONTENT_SETTINGS_TYPE_GEOLOCATION, db_manager,
+                              url, 2000 /* timeout */, CONTENT_SETTING_BLOCK);
+}
+
+// Tests that a URL with a blacklisted permission is permitted to request a
+// non-blacklisted permission.
+TEST_F(PermissionContextBaseTests, TestPermissionsBlacklistingAllowed) {
+  scoped_refptr<MockSafeBrowsingDatabaseManager> db_manager =
+      new MockSafeBrowsingDatabaseManager(
+          true /* run client with result method immediately */);

[raymes, 2017/01/09 06:28:18]

nit: just put perform_callback. If you want to add an additional comment, you
can do that above this statement as usual. But I don't think it's necessary in
this case.

[meredithl, 2017/01/10 02:24:57]

Done.

+  const GURL url("https://www.example.com");
+  std::set<std::string> blacklisted_permissions{
+      PermissionUtil::GetPermissionString(
+          content::PermissionType::GEOLOCATION)};
+  db_manager->BlacklistUrlPermissions(url, blacklisted_permissions);
+  TestPermissionsBlacklisting(
+      content::PermissionType::GEOLOCATION, CONTENT_SETTINGS_TYPE_GEOLOCATION,
+      db_manager, url, 2000 /* timeout in ms */, CONTENT_SETTING_BLOCK);
+  TestPermissionsBlacklisting(content::PermissionType::NOTIFICATIONS,
+                              CONTENT_SETTINGS_TYPE_NOTIFICATIONS, db_manager,
+                              url, 2000 /* timeout in ms */,
+                              CONTENT_SETTING_ASK);
+}
+
+// Tests that a URL with a blacklisted permisison is permitted to request that
+// permission if Safe Browsing has timed out.
+TEST_F(PermissionContextBaseTests, TestSafeBrowsingTimeout) {
+  scoped_refptr<MockSafeBrowsingDatabaseManager> db_manager =
+      new MockSafeBrowsingDatabaseManager(
+          false /* don't call client with result */);

[raymes, 2017/01/09 06:28:18]

nit: same thing here

[meredithl, 2017/01/10 02:24:57]

Done.

+  const GURL url("https://www.example.com");
+  std::set<std::string> blacklisted_permissions{
+      PermissionUtil::GetPermissionString(
+          content::PermissionType::GEOLOCATION)};
+  db_manager->BlacklistUrlPermissions(url, blacklisted_permissions);
+  TestPermissionsBlacklisting(content::PermissionType::GEOLOCATION,
+                              CONTENT_SETTINGS_TYPE_GEOLOCATION, db_manager,
+                              url, 0 /* timeout in ms */, CONTENT_SETTING_ASK);
+}