Skip to content

Commit

Permalink
Check Feature Policy for Mic/Camera requests
Browse files Browse the repository at this point in the history
Mic/Camera requests don't currently go through the permissions service so we
need to implement additional checks. These happen in
MediaDevicesPermissionChecker. Tests have also been added for these.

BUG=689802

Review-Url: https://codereview.chromium.org/2931783002
Cr-Commit-Position: refs/heads/master@{#479177}
  • Loading branch information
raymes authored and Commit Bot committed Jun 13, 2017
1 parent c3ae0fb commit 7938f5d
Show file tree
Hide file tree
Showing 3 changed files with 156 additions and 10 deletions.
37 changes: 27 additions & 10 deletions content/browser/media/media_devices_permission_checker.cc
Original file line number Diff line number Diff line change
Expand Up @@ -9,12 +9,14 @@

#include "base/bind.h"
#include "base/command_line.h"
#include "base/feature_list.h"
#include "content/browser/frame_host/render_frame_host_delegate.h"
#include "content/browser/frame_host/render_frame_host_impl.h"
#include "content/common/media/media_devices.h"
#include "content/public/browser/browser_context.h"
#include "content/public/browser/browser_thread.h"
#include "content/public/browser/render_process_host.h"
#include "content/public/common/content_features.h"
#include "content/public/common/content_switches.h"
#include "url/gurl.h"
#include "url/origin.h"
Expand All @@ -38,21 +40,36 @@ MediaDevicesManager::BoolDeviceTypes DoCheckPermissionsOnUIThread(
RenderFrameHostDelegate* delegate = frame_host->delegate();
GURL origin = frame_host->GetLastCommittedOrigin().GetURL();

// Currently, the MEDIA_DEVICE_AUDIO_CAPTURE permission is used for
// both audio input and output.
MediaDevicesManager::BoolDeviceTypes result;
bool audio_permission =
delegate->CheckMediaAccessPermission(origin, MEDIA_DEVICE_AUDIO_CAPTURE);
bool mic_feature_policy = true;
bool camera_feature_policy = true;
if (base::FeatureList::IsEnabled(features::kUseFeaturePolicyForPermissions)) {
mic_feature_policy = frame_host->IsFeatureEnabled(
blink::WebFeaturePolicyFeature::kMicrophone);
camera_feature_policy =
frame_host->IsFeatureEnabled(blink::WebFeaturePolicyFeature::kCamera);
}

// Speakers.
// TODO(guidou): use specific permission for audio output when it becomes
// available. See http://crbug.com/556542.
bool has_audio_permission =
(requested_device_types[MEDIA_DEVICE_TYPE_AUDIO_INPUT] ||
requested_device_types[MEDIA_DEVICE_TYPE_AUDIO_OUTPUT]) &&
delegate->CheckMediaAccessPermission(origin, MEDIA_DEVICE_AUDIO_CAPTURE);
result[MEDIA_DEVICE_TYPE_AUDIO_OUTPUT] =
requested_device_types[MEDIA_DEVICE_TYPE_AUDIO_OUTPUT] &&
audio_permission;

MediaDevicesManager::BoolDeviceTypes result;
result[MEDIA_DEVICE_TYPE_AUDIO_INPUT] = has_audio_permission;
result[MEDIA_DEVICE_TYPE_AUDIO_OUTPUT] = has_audio_permission;
// Mic.
result[MEDIA_DEVICE_TYPE_AUDIO_INPUT] =
requested_device_types[MEDIA_DEVICE_TYPE_AUDIO_INPUT] &&
audio_permission && mic_feature_policy;

// Camera.
result[MEDIA_DEVICE_TYPE_VIDEO_INPUT] =
requested_device_types[MEDIA_DEVICE_TYPE_VIDEO_INPUT] &&
delegate->CheckMediaAccessPermission(origin, MEDIA_DEVICE_VIDEO_CAPTURE);
delegate->CheckMediaAccessPermission(origin,
MEDIA_DEVICE_VIDEO_CAPTURE) &&
camera_feature_policy;

return result;
}
Expand Down
128 changes: 128 additions & 0 deletions content/browser/media/media_devices_permission_checker_unittest.cc
Original file line number Diff line number Diff line change
@@ -0,0 +1,128 @@
// 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/media/media_devices_permission_checker.h"

#include "base/run_loop.h"
#include "base/test/scoped_feature_list.h"
#include "content/public/browser/render_frame_host.h"
#include "content/public/browser/render_process_host.h"
#include "content/public/browser/web_contents.h"
#include "content/public/browser/web_contents_delegate.h"
#include "content/public/common/content_features.h"
#include "content/public/common/media_stream_request.h"
#include "content/public/test/test_renderer_host.h"
#include "content/test/test_render_view_host.h"
#include "content/test/test_web_contents.h"
#include "third_party/WebKit/public/platform/WebFeaturePolicy.h"
#include "url/origin.h"

namespace content {

namespace {

class TestWebContentsDelegate : public content::WebContentsDelegate {
public:
~TestWebContentsDelegate() override {}

bool CheckMediaAccessPermission(WebContents* web_contents,
const GURL& security_origin,
MediaStreamType type) override {
return true;
}
};

} // namespace

class MediaDevicesPermissionCheckerTest : public RenderViewHostImplTestHarness {
public:
MediaDevicesPermissionCheckerTest()
: origin_(GURL("https://www.google.com")),
callback_run_(false),
callback_result_(false) {}

void SetUp() override {
RenderViewHostImplTestHarness::SetUp();
NavigateAndCommit(origin_.GetURL());
contents()->SetDelegate(&delegate_);
}

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);
}

bool CheckPermission(MediaDeviceType device_type) {
base::RunLoop run_loop;
quit_closure_ = run_loop.QuitClosure();
checker_.CheckPermission(
device_type, main_rfh()->GetProcess()->GetID(),
main_rfh()->GetRoutingID(),
base::Bind(&MediaDevicesPermissionCheckerTest::CheckPermissionCallback,
base::Unretained(this)));
run_loop.Run();

EXPECT_TRUE(callback_run_);
callback_run_ = false;
return callback_result_;
}

private:
void CheckPermissionCallback(bool result) {
callback_run_ = true;
callback_result_ = result;
quit_closure_.Run();
}

url::Origin origin_;

base::Closure quit_closure_;

bool callback_run_;
bool callback_result_;

MediaDevicesPermissionChecker checker_;
TestWebContentsDelegate delegate_;
};

// Basic tests for feature policy checks through the
// MediaDevicesPermissionChecker. 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(MediaDevicesPermissionCheckerTest, CheckPermissionWithFeaturePolicy) {
base::test::ScopedFeatureList feature_list;
feature_list.InitAndEnableFeature(features::kUseFeaturePolicyForPermissions);
// Mic and Camera should be enabled by default for a frame (if permission is
// granted).
EXPECT_TRUE(CheckPermission(MEDIA_DEVICE_TYPE_AUDIO_INPUT));
EXPECT_TRUE(CheckPermission(MEDIA_DEVICE_TYPE_VIDEO_INPUT));

RefreshPageAndSetHeaderPolicy(blink::WebFeaturePolicyFeature::kMicrophone,
/*enabled=*/false);
EXPECT_FALSE(CheckPermission(MEDIA_DEVICE_TYPE_AUDIO_INPUT));
EXPECT_TRUE(CheckPermission(MEDIA_DEVICE_TYPE_VIDEO_INPUT));

RefreshPageAndSetHeaderPolicy(blink::WebFeaturePolicyFeature::kCamera,
/*enabled=*/false);
EXPECT_TRUE(CheckPermission(MEDIA_DEVICE_TYPE_AUDIO_INPUT));
EXPECT_FALSE(CheckPermission(MEDIA_DEVICE_TYPE_VIDEO_INPUT));

// Ensure that the policy is ignored if kUseFeaturePolicyForPermissions is
// disabled.
base::test::ScopedFeatureList empty_feature_list;
empty_feature_list.Init();
EXPECT_TRUE(CheckPermission(MEDIA_DEVICE_TYPE_AUDIO_INPUT));
EXPECT_TRUE(CheckPermission(MEDIA_DEVICE_TYPE_VIDEO_INPUT));
}

} // namespace
1 change: 1 addition & 0 deletions content/test/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -1214,6 +1214,7 @@ test("content_unittests") {
"../browser/media/capture/audio_mirroring_manager_unittest.cc",
"../browser/media/capture/web_contents_audio_input_stream_unittest.cc",
"../browser/media/cdm_registry_impl_unittest.cc",
"../browser/media/media_devices_permission_checker_unittest.cc",
"../browser/media/media_internals_unittest.cc",
"../browser/media/midi_host_unittest.cc",
"../browser/media/session/audio_focus_manager_unittest.cc",
Expand Down

0 comments on commit 7938f5d

Please sign in to comment.