Use access token for ICE Config requests

I've recently uppdated our service API to allow authenticated requests
for fetching the ICE Config. In the long-term, this will help prevent
abuse scenarios. In the short-term, we will no longer need to figure
out which API key is being used (Chrome or Chromoting) on ChromeOS
since both workflows will provide an access token (or in the case of
delegated signaling, the ice_config itself).

Note that this CL updates the host codepaths but I am leaving the client
codepaths as-is to minimize changes in them.

Bug: NO_BUG
Change-Id: I4d914daadc51161f02fb76c5746c951bcfcc0c9f
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3017296
Reviewed-by: Yuwei Huang <yuweih@chromium.org>
Commit-Queue: Joe Downing <joedow@chromium.org>
Cr-Commit-Position: refs/heads/master@{#900118}

remoting/client/chromoting_session.cc

@@ -539,6 +539,7 @@ void ChromotingSession::Core::ConnectOnNetworkThread() {
       new protocol::TransportContext(
           std::make_unique<protocol::ChromiumPortAllocatorFactory>(),
           runtime_->url_loader_factory(),
+          /* oauth_token_getter= */ nullptr,
           protocol::NetworkSettings(
               protocol::NetworkSettings::NAT_TRAVERSAL_FULL),
           protocol::TransportRole::CLIENT);

remoting/host/it2me/it2me_host.cc

@@ -19,6 +19,7 @@
 #include "net/url_request/url_request_context_getter.h"
 #include "remoting/base/auto_thread.h"
 #include "remoting/base/logging.h"
+#include "remoting/base/oauth_token_getter.h"
 #include "remoting/base/rsa_key_pair.h"
 #include "remoting/base/service_urls.h"
 #include "remoting/host/chromoting_host.h"
@@ -143,6 +144,7 @@ void It2MeHost::ConnectOnNetworkThread(
   auto connection_context = std::move(create_context).Run(host_context_.get());
   log_to_server_ = std::move(connection_context->log_to_server);
   signal_strategy_ = std::move(connection_context->signal_strategy);
+  oauth_token_getter_ = std::move(connection_context->oauth_token_getter);
   DCHECK(log_to_server_);
   DCHECK(signal_strategy_);
 
@@ -213,8 +215,8 @@ void It2MeHost::ConnectOnNetworkThread(
   scoped_refptr<protocol::TransportContext> transport_context =
       new protocol::TransportContext(
           std::make_unique<protocol::ChromiumPortAllocatorFactory>(),
-          host_context_->url_loader_factory(), network_settings,
-          protocol::TransportRole::SERVER);
+          host_context_->url_loader_factory(), oauth_token_getter_.get(),
+          network_settings, protocol::TransportRole::SERVER);
   if (!ice_config.is_null()) {
     transport_context->set_turn_ice_config(ice_config);
   }

remoting/host/it2me/it2me_host.h

@@ -36,6 +36,7 @@ class FtlSignalingConnector;
 class HostEventLogger;
 class HostStatusLogger;
 class LogToServer;
+class OAuthTokenGetter;
 class RegisterSupportHostRequest;
 class RsaKeyPair;
 
@@ -54,6 +55,7 @@ class It2MeHost : public base::RefCountedThreadSafe<It2MeHost>,
     std::unique_ptr<LogToServer> log_to_server;
     std::unique_ptr<RegisterSupportHostRequest> register_request;
     std::unique_ptr<SignalStrategy> signal_strategy;
+    std::unique_ptr<OAuthTokenGetter> oauth_token_getter;
 
     // Since the deferred context only provides an interface* for the signal
     // strategy, we use this boolean to indicate whether the host process should
@@ -98,9 +100,6 @@ class It2MeHost : public base::RefCountedThreadSafe<It2MeHost>,
   // Methods called by the script object, from the plugin thread.
 
   // Creates It2Me host structures and starts the host.
-  //
-  // XmppLogToServer cannot be created and used in different sequence, so pass
-  // in a factory callback instead.
   virtual void Connect(
       std::unique_ptr<ChromotingHostContext> context,
       std::unique_ptr<base::DictionaryValue> policies,
@@ -185,6 +184,7 @@ class It2MeHost : public base::RefCountedThreadSafe<It2MeHost>,
   std::unique_ptr<SignalStrategy> signal_strategy_;
   std::unique_ptr<FtlSignalingConnector> ftl_signaling_connector_;
   std::unique_ptr<LogToServer> log_to_server_;
+  std::unique_ptr<OAuthTokenGetter> oauth_token_getter_;
 
   It2MeHostState state_ = It2MeHostState::kDisconnected;
 

remoting/host/it2me/it2me_native_messaging_host.cc

@@ -316,6 +316,9 @@ void It2MeNativeMessagingHost::ProcessConnect(
                     std::make_unique<PassthroughOAuthTokenGetter>(username,
                                                                   access_token),
                     host_context->url_loader_factory());
+            connection_context->oauth_token_getter =
+                std::make_unique<PassthroughOAuthTokenGetter>(username,
+                                                              access_token);
             return connection_context;
           },
           username, access_token);

remoting/host/remoting_me2me_host.cc

@@ -1557,8 +1557,8 @@ void HostProcess::StartHost() {
   scoped_refptr<protocol::TransportContext> transport_context =
       new protocol::TransportContext(
           std::make_unique<protocol::ChromiumPortAllocatorFactory>(),
-          context_->url_loader_factory(), network_settings,
-          protocol::TransportRole::SERVER);
+          context_->url_loader_factory(), oauth_token_getter_.get(),
+          network_settings, protocol::TransportRole::SERVER);
   std::unique_ptr<protocol::SessionManager> session_manager(
       new protocol::JingleSessionManager(signal_strategy_.get()));
 

remoting/protocol/BUILD.gn

@@ -234,6 +234,7 @@ static_library("protocol") {
     "//jingle:webrtc_glue",
     "//net",
     "//remoting/base",
+    "//remoting/base:authorization",
     "//remoting/codec:decoder",
     "//remoting/proto/remoting/v1:network_traversal_proto",
     "//remoting/signaling",

remoting/protocol/ice_transport_unittest.cc

@@ -119,7 +119,8 @@ class IceTransportTest : public testing::Test {
 
     host_transport_ = std::make_unique<IceTransport>(
         new TransportContext(std::make_unique<ChromiumPortAllocatorFactory>(),
-                             nullptr, network_settings_, TransportRole::SERVER),
+                             nullptr, nullptr, network_settings_,
+                             TransportRole::SERVER),
         &host_event_handler_);
     if (!host_authenticator_) {
       host_authenticator_ =
@@ -128,7 +129,8 @@ class IceTransportTest : public testing::Test {
 
     client_transport_ = std::make_unique<IceTransport>(
         new TransportContext(std::make_unique<ChromiumPortAllocatorFactory>(),
-                             nullptr, network_settings_, TransportRole::CLIENT),
+                             nullptr, nullptr, network_settings_,
+                             TransportRole::CLIENT),
         &client_event_handler_);
     if (!client_authenticator_) {
       client_authenticator_ =

remoting/protocol/remoting_ice_config_request.cc

@@ -57,10 +57,15 @@ constexpr char kGetIceConfigPath[] = "/v1/networktraversal:geticeconfig";
 }  // namespace
 
 RemotingIceConfigRequest::RemotingIceConfigRequest(
-    scoped_refptr<network::SharedURLLoaderFactory> url_loader_factory)
+    scoped_refptr<network::SharedURLLoaderFactory> url_loader_factory,
+    OAuthTokenGetter* oauth_token_getter)
     : http_client_(ServiceUrls::GetInstance()->remoting_server_endpoint(),
-                   nullptr,
-                   url_loader_factory) {}
+                   oauth_token_getter,
+                   url_loader_factory) {
+  // |oauth_token_getter| is allowed to be null if the caller wants the request
+  // to be unauthenticated.
+  make_authenticated_requests_ = oauth_token_getter != nullptr;
+}
 
 RemotingIceConfigRequest::~RemotingIceConfigRequest() = default;
 
@@ -72,19 +77,15 @@ void RemotingIceConfigRequest::Send(OnIceConfigCallback callback) {
 
   auto request_config =
       std::make_unique<ProtobufHttpRequestConfig>(kTrafficAnnotation);
-  request_config->authenticated = false;
   request_config->path = kGetIceConfigPath;
   request_config->request_message =
       std::make_unique<apis::v1::GetIceConfigRequest>();
-#if BUILDFLAG(IS_CHROMEOS_ASH)
-  // Use the default Chrome API key for ChromeOS as the only host instance
-  // which runs there is used for the ChromeOS Enterprise Kiosk mode
-  // scenario.  If we decide to implement a remote access host for ChromeOS,
-  // then we will need a way for the caller to provide an API key.
-  request_config->api_key = google_apis::GetAPIKey();
-#else
-  request_config->api_key = google_apis::GetRemotingAPIKey();
-#endif
+  if (!make_authenticated_requests_) {
+    // TODO(joedow): Remove this after we no longer have any clients/hosts which
+    // call this API in an unauthenticated fashion.
+    request_config->authenticated = false;
+    request_config->api_key = google_apis::GetRemotingAPIKey();
+  }
   auto request =
       std::make_unique<ProtobufHttpRequest>(std::move(request_config));
   request->SetResponseCallback(base::BindOnce(

remoting/protocol/remoting_ice_config_request.h

@@ -26,6 +26,7 @@ class GetIceConfigResponse;
 }  // namespace apis
 
 class ProtobufHttpStatus;
+class OAuthTokenGetter;
 
 namespace protocol {
 
@@ -33,8 +34,9 @@ namespace protocol {
 // service.
 class RemotingIceConfigRequest final : public IceConfigRequest {
  public:
-  explicit RemotingIceConfigRequest(
-      scoped_refptr<network::SharedURLLoaderFactory> url_loader_factory);
+  RemotingIceConfigRequest(
+      scoped_refptr<network::SharedURLLoaderFactory> url_loader_factory,
+      OAuthTokenGetter* oauth_token_getter);
   ~RemotingIceConfigRequest() override;
 
   // IceConfigRequest implementation.
@@ -46,6 +48,7 @@ class RemotingIceConfigRequest final : public IceConfigRequest {
   void OnResponse(const ProtobufHttpStatus& status,
                   std::unique_ptr<apis::v1::GetIceConfigResponse> response);
 
+  bool make_authenticated_requests_ = false;
   OnIceConfigCallback on_ice_config_callback_;
   ProtobufHttpClient http_client_;
 

remoting/protocol/remoting_ice_config_request_unittest.cc

@@ -42,7 +42,8 @@ class RemotingIceConfigRequestTest : public testing::Test {
 
   base::test::TaskEnvironment task_environment_;
   ProtobufHttpTestResponder test_responder_;
-  RemotingIceConfigRequest request_{test_responder_.GetUrlLoaderFactory()};
+  RemotingIceConfigRequest request_{test_responder_.GetUrlLoaderFactory(),
+                                    nullptr};
 };
 
 TEST_F(RemotingIceConfigRequestTest, SuccessfulRequest) {

remoting/protocol/transport_context.cc

@@ -15,6 +15,7 @@
 #include "jingle/glue/thread_wrapper.h"
 #include "net/url_request/url_request_context_getter.h"
 #include "remoting/base/logging.h"
+#include "remoting/base/oauth_token_getter.h"
 #include "remoting/protocol/chromium_port_allocator_factory.h"
 #include "remoting/protocol/port_allocator_factory.h"
 #include "remoting/protocol/remoting_ice_config_request.h"
@@ -62,6 +63,7 @@ scoped_refptr<TransportContext> TransportContext::ForTests(TransportRole role) {
   jingle_glue::JingleThreadWrapper::EnsureForCurrentMessageLoop();
   return new protocol::TransportContext(
       std::make_unique<protocol::ChromiumPortAllocatorFactory>(), nullptr,
+      nullptr,
       protocol::NetworkSettings(
           protocol::NetworkSettings::NAT_TRAVERSAL_OUTGOING),
       role);
@@ -70,10 +72,12 @@ scoped_refptr<TransportContext> TransportContext::ForTests(TransportRole role) {
 TransportContext::TransportContext(
     std::unique_ptr<PortAllocatorFactory> port_allocator_factory,
     scoped_refptr<network::SharedURLLoaderFactory> url_loader_factory,
+    OAuthTokenGetter* oauth_token_getter,
     const NetworkSettings& network_settings,
     TransportRole role)
     : port_allocator_factory_(std::move(port_allocator_factory)),
       url_loader_factory_(url_loader_factory),
+      oauth_token_getter_(oauth_token_getter),
       network_settings_(network_settings),
       role_(role) {}
 
@@ -118,8 +122,8 @@ void TransportContext::EnsureFreshIceConfig() {
 
   if (base::Time::Now() >
       (last_request_completion_time_ + kIceConfigRequestCooldown)) {
-    ice_config_request_ =
-        std::make_unique<RemotingIceConfigRequest>(url_loader_factory_);
+    ice_config_request_ = std::make_unique<RemotingIceConfigRequest>(
+        url_loader_factory_, oauth_token_getter_);
     ice_config_request_->Send(
         base::BindOnce(&TransportContext::OnIceConfig, base::Unretained(this)));
   } else {

remoting/protocol/transport_context.h

@@ -26,6 +26,8 @@ class NetworkManager;
 
 namespace remoting {
 
+class OAuthTokenGetter;
+
 namespace protocol {
 
 class PortAllocatorFactory;
@@ -44,6 +46,7 @@ class TransportContext : public base::RefCountedThreadSafe<TransportContext> {
   TransportContext(
       std::unique_ptr<PortAllocatorFactory> port_allocator_factory,
       scoped_refptr<network::SharedURLLoaderFactory> url_loader_factory,
+      OAuthTokenGetter* oauth_token_getter,
       const NetworkSettings& network_settings,
       TransportRole role);
 
@@ -96,6 +99,7 @@ class TransportContext : public base::RefCountedThreadSafe<TransportContext> {
 
   std::unique_ptr<PortAllocatorFactory> port_allocator_factory_;
   scoped_refptr<network::SharedURLLoaderFactory> url_loader_factory_;
+  OAuthTokenGetter* oauth_token_getter_ = nullptr;
   NetworkSettings network_settings_;
   TransportRole role_;
 

remoting/test/ftl_signaling_playground.cc

@@ -252,8 +252,8 @@ void FtlSignalingPlayground::InitializeTransport() {
       protocol::NetworkSettings::NAT_TRAVERSAL_FULL);
   auto transport_context = base::MakeRefCounted<protocol::TransportContext>(
       std::make_unique<protocol::ChromiumPortAllocatorFactory>(),
-      url_loader_factory_owner_->GetURLLoaderFactory(), network_settings,
-      transport_role_);
+      url_loader_factory_owner_->GetURLLoaderFactory(), nullptr,
+      network_settings, transport_role_);
   auto close_callback =
       base::BindOnce(&FtlSignalingPlayground::AsyncTearDownAndRunCallback,
                      base::Unretained(this));

remoting/test/protocol_perftest.cc

@@ -294,7 +294,7 @@ class ProtocolPerfTest
         GetParam().out_of_order_rate);
     scoped_refptr<protocol::TransportContext> transport_context(
         new protocol::TransportContext(std::move(port_allocator_factory),
-                                       nullptr, network_settings,
+                                       nullptr, nullptr, network_settings,
                                        protocol::TransportRole::SERVER));
     std::unique_ptr<protocol::SessionManager> session_manager(
         new protocol::JingleSessionManager(host_signaling_.get()));
@@ -361,7 +361,7 @@ class ProtocolPerfTest
         GetParam().out_of_order_rate);
     scoped_refptr<protocol::TransportContext> transport_context(
         new protocol::TransportContext(std::move(port_allocator_factory),
-                                       nullptr, network_settings,
+                                       nullptr, nullptr, network_settings,
                                        protocol::TransportRole::CLIENT));
 
     protocol::ClientAuthenticationConfig client_auth_config;