Add checks for Feature Policy to the mojo Permission Service

This adds Feature Policy checks to the mojo permission service and
corresponding tests. Because multiple permissions can be requested at the same
time, part of a request can be answered synchronously (by feature policy) and
the other part asynchronously (by quering the PermissionManager). Thus we need
to track the partially answered request in the PendingRequest class.

Feature policy checks are done here (in content) as they will form part of the
specifications for various features and thus it is best if other embedders of
content also benefit from these checks.

BUG=689802
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation

Review-Url: https://codereview.chromium.org/2874053003
Cr-Commit-Position: refs/heads/master@{#475832}

content/browser/permissions/permission_service_context.h

@@ -6,6 +6,7 @@
 #define CONTENT_BROWSER_PERMISSIONS_PERMISSION_SERVICE_CONTEXT_H_
 
 #include "base/macros.h"
+#include "content/common/content_export.h"
 #include "content/public/browser/permission_type.h"
 #include "content/public/browser/web_contents_observer.h"
 #include "mojo/public/cpp/bindings/strong_binding_set.h"
@@ -29,7 +30,7 @@ class RenderProcessHost;
 // There is one PermissionServiceContext per RenderFrameHost/RenderProcessHost
 // which owns it. It then owns all PermissionServiceImpl associated to their
 // owner.
-class PermissionServiceContext : public WebContentsObserver {
+class CONTENT_EXPORT PermissionServiceContext : public WebContentsObserver {
  public:
   explicit PermissionServiceContext(RenderFrameHost* render_frame_host);
   explicit PermissionServiceContext(RenderProcessHost* render_process_host);

content/browser/permissions/permission_service_impl.cc

@@ -10,10 +10,14 @@
 #include <utility>
 
 #include "base/bind.h"
+#include "base/feature_list.h"
 #include "base/memory/ptr_util.h"
 #include "content/public/browser/browser_context.h"
 #include "content/public/browser/permission_manager.h"
 #include "content/public/browser/permission_type.h"
+#include "content/public/browser/render_frame_host.h"
+#include "content/public/common/content_features.h"
+#include "third_party/WebKit/public/platform/WebFeaturePolicy.h"
 
 using blink::mojom::PermissionDescriptorPtr;
 using blink::mojom::PermissionName;
@@ -56,8 +60,56 @@ PermissionType PermissionDescriptorToPermissionType(
   return PermissionType::NUM;
 }
 
-// This function allows the usage of the the multiple request map
-// with single requests.
+blink::WebFeaturePolicyFeature PermissionTypeToFeaturePolicyFeature(
+    PermissionType type) {
+  switch (type) {
+    case PermissionType::MIDI:
+    case PermissionType::MIDI_SYSEX:
+      return blink::WebFeaturePolicyFeature::kMidiFeature;
+    case PermissionType::GEOLOCATION:
+      return blink::WebFeaturePolicyFeature::kGeolocation;
+    case PermissionType::PROTECTED_MEDIA_IDENTIFIER:
+      return blink::WebFeaturePolicyFeature::kEme;
+    case PermissionType::AUDIO_CAPTURE:
+      return blink::WebFeaturePolicyFeature::kMicrophone;
+    case PermissionType::VIDEO_CAPTURE:
+      return blink::WebFeaturePolicyFeature::kCamera;
+    case PermissionType::PUSH_MESSAGING:
+    case PermissionType::NOTIFICATIONS:
+    case PermissionType::DURABLE_STORAGE:
+    case PermissionType::BACKGROUND_SYNC:
+    case PermissionType::FLASH:
+    case PermissionType::NUM:
+      // These aren't exposed by feature policy.
+      return blink::WebFeaturePolicyFeature::kNotFound;
+  }
+
+  NOTREACHED();
+  return blink::WebFeaturePolicyFeature::kNotFound;
+}
+
+bool AllowedByFeaturePolicy(RenderFrameHost* rfh, PermissionType type) {
+  if (!base::FeatureList::IsEnabled(
+          features::kUseFeaturePolicyForPermissions)) {
+    // Default to ignoring the feature policy.
+    return true;
+  }
+
+  blink::WebFeaturePolicyFeature feature_policy_feature =
+      PermissionTypeToFeaturePolicyFeature(type);
+  if (feature_policy_feature == blink::WebFeaturePolicyFeature::kNotFound)
+    return true;
+
+  // If there is no frame, there is no policy, so disable the feature for
+  // safety.
+  if (!rfh)
+    return false;
+
+  return rfh->IsFeatureEnabled(feature_policy_feature);
+}
+
+// This function allows the usage of the the multiple request map with single
+// requests.
 void PermissionRequestResponseCallbackWrapper(
     const base::Callback<void(PermissionStatus)>& callback,
     const std::vector<PermissionStatus>& vector) {
@@ -67,20 +119,56 @@ void PermissionRequestResponseCallbackWrapper(
 
 } // anonymous namespace
 
-PermissionServiceImpl::PendingRequest::PendingRequest(
-    const RequestPermissionsCallback& callback,
-    int request_count)
-    : callback(callback),
-      request_count(request_count) {
-}
+class PermissionServiceImpl::PendingRequest {
+ public:
+  PendingRequest(std::vector<PermissionType> types,
+                 const RequestPermissionsCallback& callback)
+      : types_(types),
+        callback_(callback),
+        has_result_been_set_(types.size(), false),
+        results_(types.size(), PermissionStatus::DENIED) {}
+
+  ~PendingRequest() {
+    if (callback_.is_null())
+      return;
 
-PermissionServiceImpl::PendingRequest::~PendingRequest() {
-  if (callback.is_null())
-    return;
+    std::vector<PermissionStatus> result(types_.size(),
+                                         PermissionStatus::DENIED);
+    callback_.Run(result);
+  }
 
-  std::vector<PermissionStatus> result(request_count, PermissionStatus::DENIED);
-  callback.Run(result);
-}
+  int id() const { return id_; }
+  void set_id(int id) { id_ = id; }
+
+  size_t RequestSize() const { return types_.size(); }
+
+  void SetResult(int index, PermissionStatus result) {
+    DCHECK_EQ(false, has_result_been_set_[index]);
+    has_result_been_set_[index] = true;
+    results_[index] = result;
+  }
+
+  bool HasResultBeenSet(size_t index) const {
+    return has_result_been_set_[index];
+  }
+
+  void RunCallback() {
+    // Check that all results have been set.
+    DCHECK(std::find(has_result_been_set_.begin(), has_result_been_set_.end(),
+                     false) == has_result_been_set_.end());
+    callback_.Run(results_);
+    callback_.Reset();
+  }
+
+ private:
+  // Request ID received from the PermissionManager.
+  int id_;
+  std::vector<PermissionType> types_;
+  RequestPermissionsCallback callback_;
+
+  std::vector<bool> has_result_been_set_;
+  std::vector<PermissionStatus> results_;
+};
 
 PermissionServiceImpl::PermissionServiceImpl(PermissionServiceContext* context)
     : context_(context), weak_factory_(this) {}
@@ -96,7 +184,7 @@ PermissionServiceImpl::~PermissionServiceImpl() {
   // Cancel pending requests.
   for (RequestsMap::Iterator<PendingRequest> it(&pending_requests_);
        !it.IsAtEnd(); it.Advance()) {
-    permission_manager->CancelPermissionRequest(it.GetCurrentValue()->id);
+    permission_manager->CancelPermissionRequest(it.GetCurrentValue()->id());
   }
   pending_requests_.Clear();
 }
@@ -140,30 +228,53 @@ void PermissionServiceImpl::RequestPermissions(
   for (size_t i = 0; i < types.size(); ++i)
     types[i] = PermissionDescriptorToPermissionType(permissions[i]);
 
-  int pending_request_id = pending_requests_.Add(
-      base::MakeUnique<PendingRequest>(callback, permissions.size()));
+  std::unique_ptr<PendingRequest> pending_request =
+      base::MakeUnique<PendingRequest>(types, callback);
+  std::vector<PermissionType> request_types;
+  for (size_t i = 0; i < types.size(); ++i) {
+    // Check feature policy.
+    if (!AllowedByFeaturePolicy(context_->render_frame_host(), types[i]))
+      pending_request->SetResult(i, PermissionStatus::DENIED);
+    else
+      request_types.push_back(types[i]);
+  }
+
+  int pending_request_id = pending_requests_.Add(std::move(pending_request));
   int id = browser_context->GetPermissionManager()->RequestPermissions(
-      types, context_->render_frame_host(), origin.GetURL(), user_gesture,
+      request_types, context_->render_frame_host(), origin.GetURL(),
+      user_gesture,
       base::Bind(&PermissionServiceImpl::OnRequestPermissionsResponse,
                  weak_factory_.GetWeakPtr(), pending_request_id));
 
   // Check if the request still exists. It may have been removed by the
   // the response callback.
-  PendingRequest* pending_request = pending_requests_.Lookup(
-      pending_request_id);
-  if (!pending_request)
-      return;
-  pending_request->id = id;
+  PendingRequest* in_progress_request =
+      pending_requests_.Lookup(pending_request_id);
+  if (!in_progress_request)
+    return;
+  in_progress_request->set_id(id);
 }
 
 void PermissionServiceImpl::OnRequestPermissionsResponse(
     int pending_request_id,
-    const std::vector<PermissionStatus>& result) {
+    const std::vector<PermissionStatus>& partial_result) {
   PendingRequest* request = pending_requests_.Lookup(pending_request_id);
-  RequestPermissionsCallback callback(request->callback);
-  request->callback.Reset();
+  auto partial_result_it = partial_result.begin();
+  // Fill in the unset results in the request. Some results in the request are
+  // set synchronously because they are blocked by feature policy. Others are
+  // determined by a call to RequestPermission. All unset results will be
+  // contained in |partial_result| in the same order that they were requested.
+  // We fill in the unset results in the request with |partial_result|.
+  for (size_t i = 0; i < request->RequestSize(); ++i) {
+    if (!request->HasResultBeenSet(i)) {
+      request->SetResult(i, *partial_result_it);
+      ++partial_result_it;
+    }
+  }
+  DCHECK(partial_result.end() == partial_result_it);
+
+  request->RunCallback();
   pending_requests_.Remove(pending_request_id);
-  callback.Run(result);
 }
 
 void PermissionServiceImpl::HasPermission(
@@ -221,8 +332,10 @@ PermissionStatus PermissionServiceImpl::GetPermissionStatusFromType(
     const url::Origin& origin) {
   BrowserContext* browser_context = context_->GetBrowserContext();
   DCHECK(browser_context);
-  if (!browser_context->GetPermissionManager())
+  if (!browser_context->GetPermissionManager() ||
+      !AllowedByFeaturePolicy(context_->render_frame_host(), type)) {
     return PermissionStatus::DENIED;
+  }
 
   GURL requesting_origin(origin.Serialize());
   // If the embedding_origin is empty we'll use |origin| instead.

content/browser/permissions/permission_service_impl.h

@@ -10,6 +10,7 @@
 #include "base/macros.h"
 #include "base/memory/weak_ptr.h"
 #include "content/browser/permissions/permission_service_context.h"
+#include "content/common/content_export.h"
 #include "third_party/WebKit/public/platform/modules/permissions/permission.mojom.h"
 #include "url/origin.h"
 
@@ -24,25 +25,19 @@ enum class PermissionType;
 // to have some information about the current context. That enables the service
 // to know whether it can show UI and have knowledge of the associated
 // WebContents for example.
-class PermissionServiceImpl : public blink::mojom::PermissionService {
+class CONTENT_EXPORT PermissionServiceImpl
+    : public blink::mojom::PermissionService {
  public:
   PermissionServiceImpl(PermissionServiceContext* context);
   ~PermissionServiceImpl() override;
 
  private:
+  friend class PermissionServiceImplTest;
+
   using PermissionStatusCallback =
       base::Callback<void(blink::mojom::PermissionStatus)>;
 
-  struct PendingRequest {
-    PendingRequest(const RequestPermissionsCallback& callback,
-                   int request_count);
-    ~PendingRequest();
-
-    // Request ID received from the PermissionManager.
-    int id;
-    RequestPermissionsCallback callback;
-    int request_count;
-  };
+  class PendingRequest;
   using RequestsMap = IDMap<std::unique_ptr<PendingRequest>>;
 
   // blink::mojom::PermissionService.

content/browser/permissions/permission_service_impl_unittest.cc

+// Copyright 2017 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 "content/browser/permissions/permission_service_impl.h"
+
+#include "base/run_loop.h"
+#include "base/test/scoped_feature_list.h"
+#include "content/browser/permissions/permission_service_context.h"
+#include "content/public/browser/web_contents.h"
+#include "content/public/common/content_features.h"
+#include "content/public/test/test_browser_context.h"
+#include "content/public/test/test_renderer_host.h"
+#include "content/test/mock_permission_manager.h"
+#include "mojo/public/cpp/bindings/interface_request.h"
+#include "third_party/WebKit/public/platform/WebFeaturePolicy.h"
+#include "third_party/WebKit/public/platform/modules/permissions/permission.mojom.h"
+#include "url/origin.h"
+
+using blink::mojom::PermissionStatus;
+using blink::mojom::PermissionName;
+
+namespace content {
+
+namespace {
+
+blink::mojom::PermissionDescriptorPtr CreatePermissionDescriptor(
+    PermissionName name) {
+  auto descriptor = blink::mojom::PermissionDescriptor::New();
+  descriptor->name = name;
+  return descriptor;
+}
+
+class TestPermissionManager : public MockPermissionManager {
+ public:
+  ~TestPermissionManager() override = default;
+
+  PermissionStatus GetPermissionStatus(PermissionType permission,
+                                       const GURL& requesting_origin,
+                                       const GURL& embedding_origin) override {
+    // Always return granted.
+    return PermissionStatus::GRANTED;
+  }
+
+  int RequestPermissions(
+      const std::vector<PermissionType>& permissions,
+      RenderFrameHost* render_frame_host,
+      const GURL& requesting_origin,
+      bool user_gesture,
+      const base::Callback<void(const std::vector<PermissionStatus>&)>&
+          callback) override {
+    callback.Run(std::vector<PermissionStatus>(permissions.size(),
+                                               PermissionStatus::GRANTED));
+    return 0;
+  }
+};
+
+}  // namespace
+
+class PermissionServiceImplTest : public RenderViewHostTestHarness {
+ public:
+  PermissionServiceImplTest() : origin_(GURL("https://www.google.com")) {}
+
+  void SetUp() override {
+    RenderViewHostTestHarness::SetUp();
+    static_cast<TestBrowserContext*>(browser_context())
+        ->SetPermissionManager(base::MakeUnique<TestPermissionManager>());
+    NavigateAndCommit(origin_.GetURL());
+    service_context_.reset(new PermissionServiceContext(main_rfh()));
+    service_impl_.reset(new PermissionServiceImpl(service_context_.get()));
+  }
+
+  void TearDown() override {
+    service_impl_.reset();
+    service_context_.reset();
+    RenderViewHostTestHarness::TearDown();
+  }
+
+ protected:
+  // The header policy should only be set once on page load, so we refresh the
+  // page to simulate that.
+  void RefreshPageAndSetHeaderPolicy(blink::WebFeaturePolicyFeature feature,
+                                     bool enabled) {
+    NavigateAndCommit(origin_.GetURL());
+    std::vector<url::Origin> whitelist;
+    if (enabled)
+      whitelist.push_back(origin_);
+    RenderFrameHostTester::For(main_rfh())
+        ->SimulateFeaturePolicyHeader(feature, whitelist);
+  }
+
+  PermissionStatus HasPermission(PermissionName permission) {
+    base::Callback<void(PermissionStatus)> callback =
+        base::Bind(&PermissionServiceImplTest::PermissionStatusCallback,
+                   base::Unretained(this));
+    service_impl_->HasPermission(CreatePermissionDescriptor(permission),
+                                 origin_, callback);
+    EXPECT_EQ(1u, last_permission_statuses_.size());
+    return last_permission_statuses_[0];
+  }
+
+  std::vector<PermissionStatus> RequestPermissions(
+      const std::vector<PermissionName>& permissions) {
+    std::vector<blink::mojom::PermissionDescriptorPtr> descriptors;
+    for (PermissionName name : permissions)
+      descriptors.push_back(CreatePermissionDescriptor(name));
+    base::Callback<void(const std::vector<PermissionStatus>&)> callback =
+        base::Bind(&PermissionServiceImplTest::RequestPermissionsCallback,
+                   base::Unretained(this));
+    service_impl_->RequestPermissions(std::move(descriptors), origin_,
+                                      /*user_gesture=*/false, callback);
+    EXPECT_EQ(permissions.size(), last_permission_statuses_.size());
+    return last_permission_statuses_;
+  }
+
+ private:
+  void PermissionStatusCallback(blink::mojom::PermissionStatus status) {
+    last_permission_statuses_ = std::vector<PermissionStatus>{status};
+  }
+
+  void RequestPermissionsCallback(
+      const std::vector<PermissionStatus>& statuses) {
+    last_permission_statuses_ = statuses;
+  }
+
+  url::Origin origin_;
+
+  base::Closure quit_closure_;
+
+  std::vector<PermissionStatus> last_permission_statuses_;
+
+  std::unique_ptr<PermissionServiceImpl> service_impl_;
+  std::unique_ptr<PermissionServiceContext> service_context_;
+};
+
+// Basic tests for feature policy checks through the PermissionService.  These
+// tests are not meant to cover every edge case as the FeaturePolicy class
+// itself is tested thoroughly in feature_policy_unittest.cc and in
+// render_frame_host_feature_policy_unittest.cc.
+TEST_F(PermissionServiceImplTest, HasPermissionWithFeaturePolicy) {
+  base::test::ScopedFeatureList feature_list;
+  feature_list.InitAndEnableFeature(features::kUseFeaturePolicyForPermissions);
+  // Geolocation should be enabled by default for a frame (if permission is
+  // granted).
+  EXPECT_EQ(PermissionStatus::GRANTED,
+            HasPermission(PermissionName::GEOLOCATION));
+
+  RefreshPageAndSetHeaderPolicy(blink::WebFeaturePolicyFeature::kGeolocation,
+                                /*enabled=*/false);
+  EXPECT_EQ(PermissionStatus::DENIED,
+            HasPermission(PermissionName::GEOLOCATION));
+
+  // Midi should be allowed even though geolocation was disabled.
+  EXPECT_EQ(PermissionStatus::GRANTED, HasPermission(PermissionName::MIDI));
+
+  // Now block midi.
+  RefreshPageAndSetHeaderPolicy(blink::WebFeaturePolicyFeature::kMidiFeature,
+                                /*enabled=*/false);
+  EXPECT_EQ(PermissionStatus::DENIED, HasPermission(PermissionName::MIDI));
+
+  // Ensure that the policy is ignored if kUseFeaturePolicyForPermissions is
+  // disabled.
+  base::test::ScopedFeatureList empty_feature_list;
+  empty_feature_list.Init();
+  EXPECT_EQ(PermissionStatus::GRANTED, HasPermission(PermissionName::MIDI));
+}
+
+TEST_F(PermissionServiceImplTest, RequestPermissionsWithFeaturePolicy) {
+  base::test::ScopedFeatureList feature_list;
+  feature_list.InitAndEnableFeature(features::kUseFeaturePolicyForPermissions);
+
+  // Disable midi.
+  RefreshPageAndSetHeaderPolicy(blink::WebFeaturePolicyFeature::kMidiFeature,
+                                /*enabled=*/false);
+
+  std::vector<PermissionStatus> result =
+      RequestPermissions(std::vector<PermissionName>{PermissionName::MIDI});
+  EXPECT_EQ(1u, result.size());
+  EXPECT_EQ(PermissionStatus::DENIED, result[0]);
+
+  // Request midi along with geolocation. Geolocation should be granted.
+  result = RequestPermissions(std::vector<PermissionName>{
+      PermissionName::MIDI, PermissionName::GEOLOCATION});
+  EXPECT_EQ(2u, result.size());
+  EXPECT_EQ(PermissionStatus::DENIED, result[0]);
+  EXPECT_EQ(PermissionStatus::GRANTED, result[1]);
+}
+
+}  // namespace
\ No newline at end of file

content/test/BUILD.gn

@@ -1219,6 +1219,7 @@ test("content_unittests") {
     "../browser/payments/payment_app_content_unittest_base.h",
     "../browser/payments/payment_app_provider_impl_unittest.cc",
     "../browser/payments/payment_manager_unittest.cc",
+    "../browser/permissions/permission_service_impl_unittest.cc",
     "../browser/presentation/presentation_service_impl_unittest.cc",
     "../browser/renderer_host/clipboard_message_filter_unittest.cc",
     "../browser/renderer_host/compositor_resize_lock_unittest.cc",