Skip to content

GitLab

T
tangled
Commit 81d41d48 authored by mark a. foltz's avatar mark a. foltz Committed by Commit Bot
Browse files
Options

Revert "[Media Router] Consolidate origin checks for using Cast streaming apps." 

This reverts commit 34fae769.

Reason for revert: Seems to have broken tab mirroring.

Original change's description:
> [Media Router] Consolidate origin checks for using Cast streaming apps.
>
> Previously, the origin check was only done for sink availability and not
> at route creation.  It also did some brittle string parsing of the
> media source ID.
>
> The updated check covers all streaming app IDs, adds the meet.google.com
> origin, and checks origins for both availability and route creation.
>
> Follow up changes will add additional checks to prevent opaque origins
> from also launching streaming apps (see bug for details).
>
> Bug: 1047834,1081503
> Change-Id: I081822796524ec2c72af29c544382cb4c9d9542d
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2095928
> Commit-Queue: mark a. foltz <mfoltz@chromium.org>
> Reviewed-by: Takumi Fujimoto <takumif@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#769512}

TBR=mfoltz@chromium.org,takumif@chromium.org,jophba@chromium.org

# Not skipping CQ checks because original CL landed > 1 day ago.

Bug: 1047834, 1081503, 1084643
Change-Id: I328fe04b0d0541a86649e84f06a44a30e128051b
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2207748
Commit-Queue: mark a. foltz <mfoltz@chromium.org>
Reviewed-by: default avatarmark a. foltz <mfoltz@chromium.org>
Reviewed-by: default avatarTakumi Fujimoto <takumif@chromium.org>
Cr-Commit-Position: refs/heads/master@{#770231}
parent 1bd32aee
Hide whitespace changes
Inline Side-by-side
Showing with 32 additions and 152 deletions
chrome/browser/media/router/providers/cast/cast_media_route_provider.cc
View file @ 81d41d48
......@@ -9,7 +9,6 @@
#include "base/bind.h"
#include "base/stl_util.h"
#include "base/strings/strcat.h"
#include "base/task/post_task.h"
#include "chrome/browser/media/router/providers/cast/cast_activity_manager.h"
#include "chrome/browser/media/router/providers/cast/cast_internal_message_util.h"
......@@ -23,6 +22,25 @@
namespace media_router {
namespace {
// Returns a list of origins that are valid for |source_id|. An empty list
// means all origins are valid.
std::vector<url::Origin> GetOrigins(const MediaSource::Id& source_id) {
// Use of the mirroring app as a Cast URL is permitted for Slides as a
// temporary workaround only. The eventual goal is to support their usecase
// using generic Presentation API.
// See also cast_media_source.cc.
static const char kMirroringAppPrefix[] = "cast:0F5096E8";
return base::StartsWith(source_id, kMirroringAppPrefix,
base::CompareCase::SENSITIVE)
? std::vector<url::Origin>(
{url::Origin::Create(GURL("https://docs.google.com"))})
: std::vector<url::Origin>();
}
} // namespace
CastMediaRouteProvider::CastMediaRouteProvider(
mojo::PendingReceiver<mojom::MediaRouteProvider> receiver,
mojo::PendingRemote<mojom::MediaRouter> media_router,
......@@ -94,23 +112,16 @@ void CastMediaRouteProvider::CreateRoute(const std::string& source_id,
return;
}
const auto cast_source = CastMediaSource::FromMediaSourceId(source_id);
std::unique_ptr<CastMediaSource> cast_source =
CastMediaSource::FromMediaSourceId(source_id);
if (!cast_source) {
DVLOG(2) << "CreateRoute: invalid source";
std::move(callback).Run(
base::nullopt, nullptr, base::StrCat({"Invalid source ", source_id}),
base::nullopt, nullptr, std::string("Invalid source"),
RouteRequestResult::ResultCode::NO_SUPPORTED_PROVIDER);
return;
}
if (!cast_source->IsAllowedOrigin(origin)) {
std::move(callback).Run(
base::nullopt, nullptr,
base::StrCat({"Invalid origin ", origin.Serialize()}),
RouteRequestResult::ResultCode::INVALID_ORIGIN);
return;
}
activity_manager_->LaunchSession(*cast_source, *sink, presentation_id, origin,
tab_id, incognito, std::move(callback));
}
......@@ -122,10 +133,11 @@ void CastMediaRouteProvider::JoinRoute(const std::string& media_source,
base::TimeDelta timeout,
bool incognito,
JoinRouteCallback callback) {
const auto cast_source = CastMediaSource::FromMediaSourceId(media_source);
std::unique_ptr<CastMediaSource> cast_source =
CastMediaSource::FromMediaSourceId(media_source);
if (!cast_source) {
std::move(callback).Run(
base::nullopt, nullptr, base::StrCat({"Invalid source ", media_source}),
base::nullopt, nullptr, std::string("Invalid source"),
RouteRequestResult::ResultCode::NO_SUPPORTED_PROVIDER);
return;
}
......@@ -175,7 +187,8 @@ void CastMediaRouteProvider::StartObservingMediaSinks(
if (base::Contains(sink_queries_, media_source))
return;
const auto cast_source = CastMediaSource::FromMediaSourceId(media_source);
std::unique_ptr<CastMediaSource> cast_source =
CastMediaSource::FromMediaSourceId(media_source);
if (!cast_source)
return;
......@@ -278,11 +291,8 @@ void CastMediaRouteProvider::OnSinkQueryUpdated(
const std::vector<MediaSinkInternal>& sinks) {
DVLOG(1) << __func__ << ", source_id: " << source_id
<< ", #sinks: " << sinks.size();
const auto cast_source = CastMediaSource::FromMediaSourceId(source_id);
if (cast_source) {
media_router_->OnSinksReceived(MediaRouteProviderId::CAST, source_id, sinks,
cast_source->GetAllowedOrigins());
}
media_router_->OnSinksReceived(MediaRouteProviderId::CAST, source_id, sinks,
GetOrigins(source_id));
}
void CastMediaRouteProvider::BroadcastMessageToSinks(
......
chrome/browser/media/router/providers/cast/cast_media_route_provider_unittest.cc
View file @ 81d41d48
......@@ -6,7 +6,6 @@
#include "base/bind.h"
#include "base/run_loop.h"
#include "base/strings/strcat.h"
#include "base/test/test_simple_task_runner.h"
#include "base/threading/sequenced_task_runner_handle.h"
#include "chrome/browser/media/router/providers/cast/cast_session_tracker.h"
......@@ -90,17 +89,6 @@ class CastMediaRouteProviderTest : public testing::Test {
route_ = std::make_unique<MediaRoute>(*route);
}
void ExpectCreateRouteSuccessWithoutConnections(
const base::Optional<MediaRoute>& route,
mojom::RoutePresentationConnectionPtr presentation_connections,
const base::Optional<std::string>& error,
RouteRequestResult::ResultCode result) {
EXPECT_TRUE(route);
EXPECT_FALSE(presentation_connections);
EXPECT_FALSE(error);
EXPECT_EQ(RouteRequestResult::ResultCode::OK, result);
}
void ExpectCreateRouteFailure(
RouteRequestResult::ResultCode expected_result,
const base::Optional<MediaRoute>& route,
......@@ -195,66 +183,6 @@ TEST_F(CastMediaRouteProviderTest, CreateRouteFailsInvalidSource) {
RouteRequestResult::ResultCode::NO_SUPPORTED_PROVIDER));
}
TEST_F(CastMediaRouteProviderTest, CreateRouteForStreamingFailsInvalidOrigin) {
MediaSinkInternal sink = CreateCastSink(1);
media_sink_service_.AddOrUpdateSink(sink);
const auto streaming_source = CastMediaSource::FromAppId(kCastStreamingAppId);
// origin_ == https://www.youtube.com is not allowed to launch streaming apps.
provider_->CreateRoute(
streaming_source->source_id(), sink.sink().id(), kPresentationId, origin_,
kTabId, kRouteTimeout, /* incognito */ false,
base::BindOnce(&CastMediaRouteProviderTest::ExpectCreateRouteFailure,
base::Unretained(this),
RouteRequestResult::ResultCode::INVALID_ORIGIN));
}
TEST_F(CastMediaRouteProviderTest,
CreateRouteForStreamingSucceedsForWhitelistedOrigin) {
MediaSinkInternal sink = CreateCastSink(1);
media_sink_service_.AddOrUpdateSink(sink);
const auto streaming_source = CastMediaSource::FromMediaSourceId(
base::StrCat({"cast:", kCastStreamingAppId, "?clientId=12345"}));
EXPECT_CALL(message_handler_, LaunchSession(sink.cast_data().cast_channel_id,
kCastStreamingAppId,
kDefaultLaunchTimeout, _, _, _));
// Whitelisted origins are allowed to launch streaming apps.
provider_->CreateRoute(
streaming_source->source_id(), sink.sink().id(), kPresentationId,
url::Origin::Create(GURL("https://meet.google.com")), kTabId,
kRouteTimeout, /* incognito */ false,
base::BindOnce(
&CastMediaRouteProviderTest::ExpectCreateRouteSuccessAndSetRoute,
base::Unretained(this)));
}
TEST_F(CastMediaRouteProviderTest,
CreateRouteForTabMirroringSucceedsForEmptyOrigin) {
MediaSinkInternal sink = CreateCastSink(1);
media_sink_service_.AddOrUpdateSink(sink);
const auto tab_mirroring_source =
CastMediaSource::FromMediaSource(MediaSource::ForTab(kTabId));
EXPECT_CALL(message_handler_, LaunchSession(sink.cast_data().cast_channel_id,
kCastStreamingAppId,
kDefaultLaunchTimeout, _, _, _));
// Empty origins, passed by the browser UI, are allowed to initiate tab
// mirroring.
// TODO(crbug.com/1047834): Check a specific origin for browser requested
// mirroring.
provider_->CreateRoute(
tab_mirroring_source->source_id(), sink.sink().id(), kPresentationId,
url::Origin::Create(GURL()), kTabId, kRouteTimeout, /* incognito */ false,
base::BindOnce(&CastMediaRouteProviderTest::
ExpectCreateRouteSuccessWithoutConnections,
base::Unretained(this)));
}
TEST_F(CastMediaRouteProviderTest, CreateRoute) {
MediaSinkInternal sink = CreateCastSink(1);
media_sink_service_.AddOrUpdateSink(sink);
......
chrome/common/media_router/media_source.h
View file @ 81d41d48
......@@ -11,9 +11,6 @@
#include "base/hash/hash.h"
#include "url/gurl.h"
// TODO(takumif): Move all Chromecast-specific constants out of this file, since
// they are not directly related to MediaSource.
namespace media_router {
// URL schemes used by Presentation URLs for Cast and DIAL.
......@@ -31,6 +28,9 @@ constexpr char kLegacyCastPresentationUrlPrefix[] =
constexpr char kMirroringAppUri[] = "cast:0F5096E8";
// Strings used in presentation IDs by the Cast SDK implementation.
// TODO(takumif): Move them out of this file, since they are not directly
// related to MediaSource.
//
// This value must be the same as |chrome.cast.AUTO_JOIN_PRESENTATION_ID| in the
// component extension.
constexpr char kAutoJoinPresentationId[] = "auto-join";
......
chrome/common/media_router/providers/cast/cast_media_source.cc
View file @ 81d41d48
......@@ -23,9 +23,6 @@ using cast_channel::BroadcastRequest;
using cast_channel::CastDeviceCapability;
using cast_channel::ReceiverAppType;
constexpr char kGoogleDocsOrigin[] = "https://docs.google.com";
constexpr char kGoogleMeetOrigin[] = "https://meet.google.com";
namespace cast_util {
using media_router::AutoJoinPolicy;
......@@ -426,26 +423,6 @@ std::vector<std::string> CastMediaSource::GetAppIds() const {
return app_ids;
}
std::vector<url::Origin> CastMediaSource::GetAllowedOrigins() const {
// Initiation of tab mirroring via a cast: URL is permitted for Slides and
// Meet until web APIs can meet their needs.
return ContainsStreamingApp()
? std::vector<url::Origin>(
{url::Origin::Create(GURL(kGoogleDocsOrigin)),
url::Origin::Create(GURL(kGoogleMeetOrigin))})
: std::vector<url::Origin>();
}
bool CastMediaSource::IsAllowedOrigin(const url::Origin& origin) const {
// TODO(crbug.com/1047834): Check a specific origin for browser requested
// streaming.
if (ContainsStreamingApp() && origin.scheme() == url::kHttpsScheme) {
return base::Contains(GetAllowedOrigins(), origin);
} else {
return true;
}
}
void CastMediaSource::set_supported_app_types(
const std::vector<ReceiverAppType>& types) {
DCHECK(!types.empty());
......
chrome/common/media_router/providers/cast/cast_media_source.h
View file @ 81d41d48
......@@ -16,7 +16,6 @@
#include "chrome/common/media_router/media_source.h"
#include "components/cast_channel/cast_message_util.h"
#include "components/cast_channel/cast_socket.h"
#include "url/origin.h"
using cast_channel::ReceiverAppType;
......@@ -147,14 +146,6 @@ class CastMediaSource {
// Returns a list of App IDs in this CastMediaSource.
std::vector<std::string> GetAppIds() const;
// Returns a list of origins that are allowed to use this source. An empty
// list means any origin is allowed. For now, non-empty origins are websites
// allowed to use this source via the Presentation API.
std::vector<url::Origin> GetAllowedOrigins() const;
// Returns true if |origin| is allowed according to GetAllowedOrigins().
bool IsAllowedOrigin(const url::Origin& origin) const;
const MediaSource::Id& source_id() const { return source_id_; }
const std::vector<CastAppInfo>& app_infos() const { return app_infos_; }
const std::string& client_id() const { return client_id_; }
......
chrome/common/media_router/providers/cast/cast_media_source_unittest.cc
View file @ 81d41d48
......@@ -161,30 +161,4 @@ TEST(CastMediaSourceTest, FromInvalidSource) {
"https://google.com/cast#__castAppId__=/param=foo"));
}
TEST(CastMediaSourceTest, TestIsAllowedOrigin) {
const auto cast_app_source = CastMediaSource::FromAppId("ABCDEFAB");
ASSERT_TRUE(cast_app_source);
ASSERT_FALSE(cast_app_source->ContainsStreamingApp());
const auto cast_streaming_source =
CastMediaSource::FromAppId(kCastStreamingAppId);
ASSERT_TRUE(cast_streaming_source);
ASSERT_TRUE(cast_streaming_source->ContainsStreamingApp());
const url::Origin empty_origin = url::Origin::Create(GURL());
const url::Origin site_origin =
url::Origin::Create(GURL("https://www.example.com"));
const url::Origin docs_origin =
url::Origin::Create(GURL("https://docs.google.com"));
EXPECT_TRUE(cast_app_source->IsAllowedOrigin(empty_origin));
EXPECT_TRUE(cast_app_source->IsAllowedOrigin(site_origin));
EXPECT_TRUE(cast_app_source->IsAllowedOrigin(docs_origin));
// Cast streaming apps are only allowed on whitelisted origins.
EXPECT_FALSE(cast_streaming_source->IsAllowedOrigin(site_origin));
EXPECT_TRUE(cast_streaming_source->IsAllowedOrigin(empty_origin));
EXPECT_TRUE(cast_streaming_source->IsAllowedOrigin(docs_origin));
}
} // namespace media_router
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment