Update It2Me Host to allow unauthenticated support requests

This CL changes the logic around the 'userName' field such that it is
no longer required when using a delegated signal strategy.  We only
need username for the XMPP case.

While making the change I found the ProcessConnect method a bit awkward
to parse as it had several #ifdefs and long if/then based on the
signal strategy type.  I decide to clean this up a bit and move the
signal strategy creation logic into helper methods.

Lastly, in the scenario where a delegated signal strategy is used and
no user name is specified, I am assigning a hard coded value to
indicate we don't have a user name.  This value will show up in logs.

Change-Id: I5eb37c5639aa265d8b6db6652470fd85fcf1db70
Reviewed-on: https://chromium-review.googlesource.com/1155741Reviewed-by: Joe Downing <joedow@chromium.org>
Reviewed-by: Jamie Walch <jamiewalch@chromium.org>
Commit-Queue: Joe Downing <joedow@chromium.org>
Commit-Queue: Jamie Walch <jamiewalch@chromium.org>
Cr-Commit-Position: refs/heads/master@{#583099}

remoting/host/it2me/it2me_native_messaging_host.cc

@@ -66,6 +66,8 @@ const base::FilePath::CharType kElevatedHostBinaryName[] =
     FILE_PATH_LITERAL("remote_assistance_host_uiaccess.exe");
 #endif  // defined(OS_WIN)
 
+constexpr char kAnonymousUserName[] = "anonymous_user";
+
 // Helper functions to run |callback| asynchronously on the correct thread
 // using |task_runner|.
 void PolicyUpdateCallback(
@@ -227,95 +229,29 @@ void It2MeNativeMessagingHost::ProcessConnect(
     return;
   }
 
-  std::string username;
-  if (!message->GetString("userName", &username)) {
-    LOG(ERROR) << "'userName' not found in request.";
-    SendErrorAndExit(std::move(response), ErrorCode::INCOMPATIBLE_PROTOCOL);
-    return;
-  }
-
   bool use_signaling_proxy = false;
   message->GetBoolean("useSignalingProxy", &use_signaling_proxy);
 
-  const ServiceUrls* service_urls = ServiceUrls::GetInstance();
-  std::unique_ptr<SignalStrategy> signal_strategy;
-
-  if (!use_signaling_proxy) {
-    XmppSignalStrategy::XmppServerConfig xmpp_config;
-    xmpp_config.username = username;
-
-    const bool xmpp_server_valid =
-        net::ParseHostAndPort(service_urls->xmpp_server_address(),
-                              &xmpp_config.host, &xmpp_config.port);
-    DCHECK(xmpp_server_valid);
-    xmpp_config.use_tls = service_urls->xmpp_server_use_tls();
-
-    std::string auth_service_with_token;
-    if (!message->GetString("authServiceWithToken", &auth_service_with_token)) {
-      LOG(ERROR) << "'authServiceWithToken' not found in request.";
-      SendErrorAndExit(std::move(response), ErrorCode::INCOMPATIBLE_PROTOCOL);
-      return;
-    }
-
-    // For backward compatibility the webapp still passes OAuth service as part
-    // of the authServiceWithToken field. But auth service part is always
-    // expected to be set to oauth2.
-    const char kOAuth2ServicePrefix[] = "oauth2:";
-    if (!base::StartsWith(auth_service_with_token, kOAuth2ServicePrefix,
-                          base::CompareCase::SENSITIVE)) {
-      LOG(ERROR) << "Invalid 'authServiceWithToken': "
-                 << auth_service_with_token;
-      SendErrorAndExit(std::move(response), ErrorCode::INCOMPATIBLE_PROTOCOL);
-      return;
-    }
-
-    xmpp_config.auth_token =
-        auth_service_with_token.substr(strlen(kOAuth2ServicePrefix));
-
-#if !defined(NDEBUG)
-    std::string address;
-    if (!message->GetString("xmppServerAddress", &address)) {
-      LOG(ERROR) << "'xmppServerAddress' not found in request.";
-      SendErrorAndExit(std::move(response), ErrorCode::INCOMPATIBLE_PROTOCOL);
-      return;
-    }
-
-    if (!net::ParseHostAndPort(address, &xmpp_config.host, &xmpp_config.port)) {
-      LOG(ERROR) << "Invalid 'xmppServerAddress': " << address;
-      SendErrorAndExit(std::move(response), ErrorCode::INCOMPATIBLE_PROTOCOL);
-      return;
-    }
+  std::string username;
+  message->GetString("userName", &username);
 
-    if (!message->GetBoolean("xmppServerUseTls", &xmpp_config.use_tls)) {
-      LOG(ERROR) << "'xmppServerUseTls' not found in request.";
-      SendErrorAndExit(std::move(response), ErrorCode::INCOMPATIBLE_PROTOCOL);
-      return;
+  std::unique_ptr<SignalStrategy> signal_strategy;
+  if (use_signaling_proxy) {
+    if (username.empty()) {
+      // Allow unauthenticated users for the delegated signal strategy case.
+      username = kAnonymousUserName;
     }
-#endif  // !defined(NDEBUG)
-
-    signal_strategy.reset(new XmppSignalStrategy(
-        net::ClientSocketFactory::GetDefaultFactory(),
-        host_context_->url_request_context_getter(), xmpp_config));
+    signal_strategy = CreateDelegatedSignalStrategy(message.get());
   } else {
-    std::string local_jid;
-
-    if (!message->GetString("localJid", &local_jid)) {
-      LOG(ERROR) << "'localJid' not found in request.";
-      SendErrorAndExit(std::move(response), ErrorCode::INCOMPATIBLE_PROTOCOL);
-      return;
-    }
-
-    auto delegating_signal_strategy =
-        std::make_unique<DelegatingSignalStrategy>(
-            SignalingAddress(local_jid), host_context_->network_task_runner(),
-            base::Bind(&It2MeNativeMessagingHost::SendOutgoingIq,
-                       weak_factory_.GetWeakPtr()));
-    incoming_message_callback_ =
-        delegating_signal_strategy->GetIncomingMessageCallback();
-    signal_strategy = std::move(delegating_signal_strategy);
+    signal_strategy = CreateXmppSignalStrategy(username, message.get());
+  }
+  if (!signal_strategy) {
+    SendErrorAndExit(std::move(response), ErrorCode::INCOMPATIBLE_PROTOCOL);
+    return;
   }
 
-  std::string directory_bot_jid = service_urls->directory_bot_jid();
+  std::string directory_bot_jid =
+      ServiceUrls::GetInstance()->directory_bot_jid();
 
 #if !defined(NDEBUG)
   if (!message->GetString("directoryBotJid", &directory_bot_jid)) {
@@ -575,6 +511,85 @@ void It2MeNativeMessagingHost::OnPolicyError() {
   }
 }
 
+std::unique_ptr<SignalStrategy>
+It2MeNativeMessagingHost::CreateDelegatedSignalStrategy(
+    const base::DictionaryValue* message) {
+  std::string local_jid;
+  if (!message->GetString("localJid", &local_jid)) {
+    LOG(ERROR) << "'localJid' not found in request.";
+    return nullptr;
+  }
+
+  auto delegating_signal_strategy = std::make_unique<DelegatingSignalStrategy>(
+      SignalingAddress(local_jid), host_context_->network_task_runner(),
+      base::BindRepeating(&It2MeNativeMessagingHost::SendOutgoingIq,
+                          weak_factory_.GetWeakPtr()));
+  incoming_message_callback_ =
+      delegating_signal_strategy->GetIncomingMessageCallback();
+  return delegating_signal_strategy;
+}
+
+std::unique_ptr<SignalStrategy>
+It2MeNativeMessagingHost::CreateXmppSignalStrategy(
+    const std::string& username,
+    const base::DictionaryValue* message) {
+  if (username.empty()) {
+    LOG(ERROR) << "'userName' not found in request.";
+    return nullptr;
+  }
+
+  XmppSignalStrategy::XmppServerConfig xmpp_config;
+  xmpp_config.username = username;
+
+  const ServiceUrls* service_urls = ServiceUrls::GetInstance();
+  const bool xmpp_server_valid =
+      net::ParseHostAndPort(service_urls->xmpp_server_address(),
+                            &xmpp_config.host, &xmpp_config.port);
+  DCHECK(xmpp_server_valid);
+  xmpp_config.use_tls = service_urls->xmpp_server_use_tls();
+
+  std::string auth_service_with_token;
+  if (!message->GetString("authServiceWithToken", &auth_service_with_token)) {
+    LOG(ERROR) << "'authServiceWithToken' not found in request.";
+    return nullptr;
+  }
+
+  // For backward compatibility the webapp still passes OAuth service as part
+  // of the authServiceWithToken field. But auth service part is always
+  // expected to be set to oauth2.
+  const char kOAuth2ServicePrefix[] = "oauth2:";
+  if (!base::StartsWith(auth_service_with_token, kOAuth2ServicePrefix,
+                        base::CompareCase::SENSITIVE)) {
+    LOG(ERROR) << "Invalid 'authServiceWithToken': " << auth_service_with_token;
+    return nullptr;
+  }
+
+  xmpp_config.auth_token =
+      auth_service_with_token.substr(strlen(kOAuth2ServicePrefix));
+
+#if !defined(NDEBUG)
+  std::string address;
+  if (!message->GetString("xmppServerAddress", &address)) {
+    LOG(ERROR) << "'xmppServerAddress' not found in request.";
+    return nullptr;
+  }
+
+  if (!net::ParseHostAndPort(address, &xmpp_config.host, &xmpp_config.port)) {
+    LOG(ERROR) << "Invalid 'xmppServerAddress': " << address;
+    return nullptr;
+  }
+
+  if (!message->GetBoolean("xmppServerUseTls", &xmpp_config.use_tls)) {
+    LOG(ERROR) << "'xmppServerUseTls' not found in request.";
+    return nullptr;
+  }
+#endif  // !defined(NDEBUG)
+
+  return std::make_unique<XmppSignalStrategy>(
+      net::ClientSocketFactory::GetDefaultFactory(),
+      host_context_->url_request_context_getter(), xmpp_config);
+}
+
 #if defined(OS_WIN)
 
 bool It2MeNativeMessagingHost::DelegateToElevatedHost(

remoting/host/it2me/it2me_native_messaging_host.h

@@ -110,6 +110,17 @@ class It2MeNativeMessagingHost : public It2MeHost::Observer,
   // Returns whether the request was successfully sent to the elevated host.
   bool DelegateToElevatedHost(std::unique_ptr<base::DictionaryValue> message);
 
+  // Creates a delegated signal strategy from the values stored in |message|.
+  // Returns nullptr on failure.
+  std::unique_ptr<SignalStrategy> CreateDelegatedSignalStrategy(
+      const base::DictionaryValue* message);
+
+  // Creates an XMPP signal strategy from the values stored in |message| along
+  // with |user_name|.  Returns nullptr on failure.
+  std::unique_ptr<SignalStrategy> CreateXmppSignalStrategy(
+      const std::string& user_name,
+      const base::DictionaryValue* message);
+
   // Used to determine whether to create and pass messages to an elevated host.
   bool needs_elevation_ = false;