Move NegotiatingClientAuthentication creation to ChromotingClient.

For the new SPAKE2 authenticator we need to pass client_jid
to the authenticator. This wasn't possible previously because
NegotiatingClientAuthenticator was created before signaling is
connected. Moved NegotiatingClientAuthentication creation to
ChromotingClient.

BUG=589698

Committed: https://crrev.com/279bf7c76a3e0bfbbd28748d0d02fcead88f4436
Cr-Commit-Position: refs/heads/master@{#380779}

Review URL: https://codereview.chromium.org/1778023002

Cr-Commit-Position: refs/heads/master@{#380831}

5 files changed, 48 insertions, 64 deletions

diff --git a/remoting/client/chromoting_client.cc b/remoting/client/chromoting_client.cc
index 9239a35..71d020f 100644
--- a/remoting/client/chromoting_client.cc
+++ b/remoting/client/chromoting_client.cc
@@ -53,7 +53,7 @@ void ChromotingClient::SetConnectionToHostForTests(
 
 void ChromotingClient::Start(
     SignalStrategy* signal_strategy,
-    scoped_ptr<protocol::Authenticator> authenticator,
+    const protocol::ClientAuthenticationConfig& client_auth_config,
     scoped_refptr<protocol::TransportContext> transport_context,
     const std::string& host_jid,
     const std::string& capabilities) {
@@ -89,7 +89,7 @@ void ChromotingClient::Start(
   session_manager_.reset(new protocol::JingleSessionManager(signal_strategy));
   session_manager_->set_protocol_config(std::move(protocol_config_));
 
-  authenticator_ = std::move(authenticator);
+  client_auth_config_ = client_auth_config;
   transport_context_ = transport_context;
 
   signal_strategy_ = signal_strategy;
@@ -212,7 +212,10 @@ bool ChromotingClient::OnSignalStrategyIncomingStanza(
 void ChromotingClient::StartConnection() {
   DCHECK(thread_checker_.CalledOnValidThread());
   connection_->Connect(
-      session_manager_->Connect(host_jid_, std::move(authenticator_)),
+      session_manager_->Connect(
+          host_jid_,
+          make_scoped_ptr(new protocol::NegotiatingClientAuthenticator(
+              client_auth_config_))),
       transport_context_, this);
 }
 
diff --git a/remoting/client/chromoting_client.h b/remoting/client/chromoting_client.h
index 48cd5f9..6563720 100644
--- a/remoting/client/chromoting_client.h
+++ b/remoting/client/chromoting_client.h
@@ -16,6 +16,7 @@
 #include "remoting/protocol/clipboard_stub.h"
 #include "remoting/protocol/connection_to_host.h"
 #include "remoting/protocol/input_stub.h"
+#include "remoting/protocol/negotiating_client_authenticator.h"
 #include "remoting/protocol/performance_tracker.h"
 #include "remoting/protocol/session_config.h"
 #include "remoting/protocol/video_stub.h"
@@ -63,7 +64,7 @@ class ChromotingClient : public SignalStrategy::Listener,
   // Start the client. Must be called on the main thread. |signal_strategy|
   // must outlive the client.
   void Start(SignalStrategy* signal_strategy,
-             scoped_ptr<protocol::Authenticator> authenticator,
+             const protocol::ClientAuthenticationConfig& client_auth_config,
              scoped_refptr<protocol::TransportContext> transport_context,
              const std::string& host_jid,
              const std::string& capabilities);
@@ -121,7 +122,7 @@ class ChromotingClient : public SignalStrategy::Listener,
   SignalStrategy* signal_strategy_ = nullptr;
 
   std::string host_jid_;
-  scoped_ptr<protocol::Authenticator> authenticator_;
+  protocol::ClientAuthenticationConfig client_auth_config_;
   scoped_refptr<protocol::TransportContext> transport_context_;
 
   scoped_ptr<protocol::SessionManager> session_manager_;
diff --git a/remoting/client/jni/chromoting_jni_instance.cc b/remoting/client/jni/chromoting_jni_instance.cc
index d1602110..00066ed 100644
--- a/remoting/client/jni/chromoting_jni_instance.cc
+++ b/remoting/client/jni/chromoting_jni_instance.cc
@@ -57,11 +57,8 @@ ChromotingJniInstance::ChromotingJniInstance(ChromotingJniRuntime* jni_runtime,
                                              const std::string& capabilities,
                                              const std::string& flags)
     : jni_runtime_(jni_runtime),
-      host_id_(host_id),
       host_jid_(host_jid),
       flags_(flags),
-      create_pairing_(false),
-      stats_logging_enabled_(false),
       capabilities_(capabilities),
       weak_factory_(this) {
   DCHECK(jni_runtime_->ui_task_runner()->BelongsToCurrentThread());
@@ -73,13 +70,14 @@ ChromotingJniInstance::ChromotingJniInstance(ChromotingJniRuntime* jni_runtime,
   xmpp_config_.username = username;
   xmpp_config_.auth_token = auth_token;
 
-  // Initialize |authenticator_|.
-  authenticator_.reset(new protocol::NegotiatingClientAuthenticator(
-      pairing_id, pairing_secret, host_id_,
-      base::Bind(&ChromotingJniInstance::FetchSecret,
-                 weak_factory_.GetWeakPtr()),
+  client_auth_config_.host_id = host_id;
+  client_auth_config_.pairing_client_id = pairing_id;
+  client_auth_config_.pairing_secret = pairing_secret;
+  client_auth_config_.fetch_secret_callback = base::Bind(
+      &ChromotingJniInstance::FetchSecret, weak_factory_.GetWeakPtr());
+  client_auth_config_.fetch_third_party_token_callback =
       base::Bind(&ChromotingJniInstance::FetchThirdPartyToken,
-                 weak_factory_.GetWeakPtr(), host_pubkey)));
+                 weak_factory_.GetWeakPtr(), host_pubkey);
 
   // Post a task to start connection
   jni_runtime_->network_task_runner()->PostTask(
@@ -94,7 +92,6 @@ ChromotingJniInstance::~ChromotingJniInstance() {
   DCHECK(!view_);
   DCHECK(!client_context_);
   DCHECK(!video_renderer_);
-  DCHECK(!authenticator_);
   DCHECK(!client_);
   DCHECK(!signaling_);
   DCHECK(!client_status_logger_);
@@ -108,8 +105,6 @@ void ChromotingJniInstance::Disconnect() {
     return;
   }
 
-  host_id_.clear();
-
   stats_logging_enabled_ = false;
 
   // |client_| must be torn down before |signaling_|.
@@ -117,7 +112,6 @@ void ChromotingJniInstance::Disconnect() {
   client_status_logger_.reset();
   video_renderer_.reset();
   view_.reset();
-  authenticator_.reset();
   signaling_.reset();
   perf_tracker_.reset();
   client_context_.reset();
@@ -333,30 +327,25 @@ void ChromotingJniInstance::OnRouteChanged(
 
 void ChromotingJniInstance::SetCapabilities(const std::string& capabilities) {
   jni_runtime_->ui_task_runner()->PostTask(
-      FROM_HERE,
-      base::Bind(&ChromotingJniRuntime::SetCapabilities,
-                 base::Unretained(jni_runtime_),
-                 capabilities));
+      FROM_HERE, base::Bind(&ChromotingJniRuntime::SetCapabilities,
+                            base::Unretained(jni_runtime_), capabilities));
 }
 
 void ChromotingJniInstance::SetPairingResponse(
     const protocol::PairingResponse& response) {
-
   jni_runtime_->ui_task_runner()->PostTask(
       FROM_HERE,
       base::Bind(&ChromotingJniRuntime::CommitPairingCredentials,
-                 base::Unretained(jni_runtime_),
-                 host_id_, response.client_id(), response.shared_secret()));
+                 base::Unretained(jni_runtime_), client_auth_config_.host_id,
+                 response.client_id(), response.shared_secret()));
 }
 
 void ChromotingJniInstance::DeliverHostMessage(
     const protocol::ExtensionMessage& message) {
   jni_runtime_->ui_task_runner()->PostTask(
-      FROM_HERE,
-      base::Bind(&ChromotingJniRuntime::HandleExtensionMessage,
-                 base::Unretained(jni_runtime_),
-                 message.type(),
-                 message.data()));
+      FROM_HERE, base::Bind(&ChromotingJniRuntime::HandleExtensionMessage,
+                            base::Unretained(jni_runtime_), message.type(),
+                            message.data()));
 }
 
 protocol::ClipboardStub* ChromotingJniInstance::GetClipboardStub() {
@@ -398,16 +387,15 @@ void ChromotingJniInstance::ConnectToHostOnNetworkThread() {
   video_renderer_.reset(new SoftwareVideoRenderer(
       client_context_->decode_task_runner(), view_.get(), perf_tracker_.get()));
 
-  client_.reset(new ChromotingClient(
-      client_context_.get(), this, video_renderer_.get(), nullptr));
+  client_.reset(new ChromotingClient(client_context_.get(), this,
+                                     video_renderer_.get(), nullptr));
 
-  signaling_.reset(new XmppSignalStrategy(
-      net::ClientSocketFactory::GetDefaultFactory(),
-      jni_runtime_->url_requester(), xmpp_config_));
+  signaling_.reset(
+      new XmppSignalStrategy(net::ClientSocketFactory::GetDefaultFactory(),
+                             jni_runtime_->url_requester(), xmpp_config_));
 
   client_status_logger_.reset(
-      new ClientStatusLogger(ServerLogEntry::ME2ME,
-                             signaling_.get(),
+      new ClientStatusLogger(ServerLogEntry::ME2ME, signaling_.get(),
                              ServiceUrls::GetInstance()->directory_bot_jid()));
 
   scoped_refptr<protocol::TransportContext> transport_context =
@@ -429,7 +417,7 @@ void ChromotingJniInstance::ConnectToHostOnNetworkThread() {
     client_->set_protocol_config(std::move(protocol_config));
   }
 
-  client_->Start(signaling_.get(), std::move(authenticator_), transport_context,
+  client_->Start(signaling_.get(), client_auth_config_, transport_context,
                  host_jid_, capabilities_);
 }
 
@@ -444,7 +432,7 @@ void ChromotingJniInstance::FetchSecret(
   }
 
   // Delete pairing credentials if they exist.
-  jni_runtime_->CommitPairingCredentials(host_id_, "", "");
+  jni_runtime_->CommitPairingCredentials(client_auth_config_.host_id, "", "");
 
   pin_callback_ = callback;
   jni_runtime_->DisplayAuthenticationPrompt(pairable);
diff --git a/remoting/client/jni/chromoting_jni_instance.h b/remoting/client/jni/chromoting_jni_instance.h
index 9eab8d9..ae2c862 100644
--- a/remoting/client/jni/chromoting_jni_instance.h
+++ b/remoting/client/jni/chromoting_jni_instance.h
@@ -147,9 +147,10 @@ class ChromotingJniInstance
   ChromotingJniRuntime* jni_runtime_;
 
   // ID of the host we are connecting to.
-  std::string host_id_;
   std::string host_jid_;
 
+  protocol::ClientAuthenticationConfig client_auth_config_;
+
   std::string flags_;
 
   // This group of variables is to be used on the network thread.
@@ -157,7 +158,6 @@ class ChromotingJniInstance
   scoped_ptr<protocol::PerformanceTracker> perf_tracker_;
   scoped_ptr<JniFrameConsumer> view_;
   scoped_ptr<protocol::VideoRenderer> video_renderer_;
-  scoped_ptr<protocol::Authenticator> authenticator_;
   scoped_ptr<ChromotingClient> client_;
   XmppSignalStrategy::XmppServerConfig xmpp_config_;
   scoped_ptr<XmppSignalStrategy> signaling_;  // Must outlive client_
@@ -172,7 +172,7 @@ class ChromotingJniInstance
   // modified in ProvideSecret(), but thereafter to be used only from the
   // network thread. (This is safe because ProvideSecret() is invoked at most
   // once per run, and always before any reference to this flag.)
-  bool create_pairing_;
+  bool create_pairing_ = false;
 
   // The device name to appear in the paired-clients list. Accessed on the
   // network thread.
@@ -180,7 +180,7 @@ class ChromotingJniInstance
 
   // If this is true, performance statistics will be periodically written to
   // the Android log. Used on the network thread.
-  bool stats_logging_enabled_;
+  bool stats_logging_enabled_ = false;
 
   // The set of capabilities supported by the client. Accessed on the network
   // thread. Once SetCapabilities() is called, this will contain the negotiated
diff --git a/remoting/client/plugin/chromoting_instance.cc b/remoting/client/plugin/chromoting_instance.cc
index 1365821..e0ca0d5 100644
--- a/remoting/client/plugin/chromoting_instance.cc
+++ b/remoting/client/plugin/chromoting_instance.cc
@@ -579,26 +579,24 @@ void ChromotingInstance::SetCursorShape(
 }
 
 void ChromotingInstance::HandleConnect(const base::DictionaryValue& data) {
+  protocol::ClientAuthenticationConfig client_auth_config;
+
   std::string local_jid;
   std::string host_jid;
   std::string host_public_key;
-  std::string authentication_tag;
   if (!data.GetString("hostJid", &host_jid) ||
       !data.GetString("hostPublicKey", &host_public_key) ||
       !data.GetString("localJid", &local_jid) ||
-      !data.GetString("authenticationTag", &authentication_tag)) {
+      !data.GetString("hostId", &client_auth_config.host_id)) {
     LOG(ERROR) << "Invalid connect() data.";
     return;
   }
 
-  std::string client_pairing_id;
-  data.GetString("clientPairingId", &client_pairing_id);
-  std::string client_paired_secret;
-  data.GetString("clientPairedSecret", &client_paired_secret);
+  data.GetString("clientPairingId", &client_auth_config.pairing_client_id);
+  data.GetString("clientPairedSecret", &client_auth_config.pairing_secret);
 
-  protocol::FetchSecretCallback fetch_secret_callback;
   if (use_async_pin_dialog_) {
-    fetch_secret_callback = base::Bind(
+    client_auth_config.fetch_secret_callback = base::Bind(
         &ChromotingInstance::FetchSecretFromDialog, weak_factory_.GetWeakPtr());
   } else {
     std::string shared_secret;
@@ -606,10 +604,14 @@ void ChromotingInstance::HandleConnect(const base::DictionaryValue& data) {
       LOG(ERROR) << "sharedSecret not specified in connect().";
       return;
     }
-    fetch_secret_callback =
+    client_auth_config.fetch_secret_callback =
         base::Bind(&ChromotingInstance::FetchSecretFromString, shared_secret);
   }
 
+  client_auth_config.fetch_third_party_token_callback =
+      base::Bind(&ChromotingInstance::FetchThirdPartyToken,
+                 weak_factory_.GetWeakPtr(), host_public_key);
+
   // Read the list of capabilities, if any.
   std::string capabilities;
   if (data.HasKey("capabilities")) {
@@ -695,16 +697,6 @@ void ChromotingInstance::HandleConnect(const base::DictionaryValue& data) {
               protocol::NetworkSettings::NAT_TRAVERSAL_FULL),
           protocol::TransportRole::CLIENT));
 
-  // Create Authenticator.
-  protocol::FetchThirdPartyTokenCallback fetch_third_party_token_callback =
-      base::Bind(&ChromotingInstance::FetchThirdPartyToken,
-                 weak_factory_.GetWeakPtr(), host_public_key);
-
-  scoped_ptr<protocol::Authenticator> authenticator(
-      new protocol::NegotiatingClientAuthenticator(
-          client_pairing_id, client_paired_secret, authentication_tag,
-          fetch_secret_callback, fetch_third_party_token_callback));
-
   scoped_ptr<protocol::CandidateSessionConfig> config =
       protocol::CandidateSessionConfig::CreateDefault();
   if (std::find(experiments_list.begin(), experiments_list.end(), "vp9") !=
@@ -714,8 +706,8 @@ void ChromotingInstance::HandleConnect(const base::DictionaryValue& data) {
   client_->set_protocol_config(std::move(config));
 
   // Kick off the connection.
-  client_->Start(signal_strategy_.get(), std::move(authenticator),
-                 transport_context, host_jid, capabilities);
+  client_->Start(signal_strategy_.get(), client_auth_config, transport_context,
+                 host_jid, capabilities);
 
   // Connect the input pipeline to the protocol stub.
   mouse_input_filter_.set_input_stub(client_->input_stub());