Protocol / client plugin changes to support interactively asking for a PIN.

This has a few special cases, to be able to deal with all sorts of combinations of Me2Me/It2Me/old-plugin/old-webapp/old-host.

The idea is: A webapp that supports asking for PINs asynchronously will explicitly notify the plugin of that. Older webapps (or It2Me) will send the passphrase directly on connect.
The negotiating authenticator, instead of immediately trying to send the first SPAKE message, first sends a message with the supported methods to the host, and only when the host replies with the specific method it tries to create the authenticator. If there is a PinFetcher interface, it tries to use a PinClientAuthenticator (a thin layer on top of V2Authenticator that takes care of asynchronously asking for PIN), otherwise it uses V2Authenticator directly with the pre-provided pass phrase.

This also adds support for authenticators that can't be created in a particular state (e.g. ones for which the first message must go in one particular direction). The NegotiatingAuthenticator takes care of sending blank messages/ignoring those messages as appropriate.

BUG=115899

Review URL: https://chromiumcodereview.appspot.com/12518027

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@190056 0039d316-1c4b-4281-b951-d872f2087c98

7 files changed, 253 insertions, 111 deletions

diff --git a/remoting/client/chromoting_client.cc b/remoting/client/chromoting_client.cc
index c0edb1b..c83002f 100644
--- a/remoting/client/chromoting_client.cc
+++ b/remoting/client/chromoting_client.cc
@@ -54,8 +54,8 @@ void ChromotingClient::Start(
 
   scoped_ptr<protocol::Authenticator> authenticator(
       protocol::NegotiatingAuthenticator::CreateForClient(
-          config_.authentication_tag,
-          config_.shared_secret, config_.authentication_methods));
+          config_.authentication_tag, config_.fetch_secret_callback,
+          config_.authentication_methods));
 
   // Create a WeakPtr to ourself for to use for all posted tasks.
   weak_ptr_ = weak_factory_.GetWeakPtr();
diff --git a/remoting/client/client_config.h b/remoting/client/client_config.h
index bf8474b..8cc67b3 100644
--- a/remoting/client/client_config.h
+++ b/remoting/client/client_config.h
@@ -10,6 +10,7 @@
 
 #include "base/basictypes.h"
 #include "remoting/protocol/authentication_method.h"
+#include "remoting/protocol/negotiating_authenticator.h"
 
 namespace remoting {
 
@@ -22,7 +23,7 @@ struct ClientConfig {
   std::string host_jid;
   std::string host_public_key;
 
-  std::string shared_secret;
+  protocol::FetchSecretCallback fetch_secret_callback;
   std::vector<protocol::AuthenticationMethod> authentication_methods;
   std::string authentication_tag;
 };
diff --git a/remoting/client/plugin/chromoting_instance.cc b/remoting/client/plugin/chromoting_instance.cc
index 9247610..1514466 100644
--- a/remoting/client/plugin/chromoting_instance.cc
+++ b/remoting/client/plugin/chromoting_instance.cc
@@ -134,7 +134,8 @@ logging::LogMessageHandlerFunction g_logging_old_handler = NULL;
 // String sent in the "hello" message to the plugin to describe features.
 const char ChromotingInstance::kApiFeatures[] =
     "highQualityScaling injectKeyEvent sendClipboardItem remapKey trapKey "
-    "notifyClientDimensions notifyClientResolution pauseVideo pauseAudio";
+    "notifyClientDimensions notifyClientResolution pauseVideo pauseAudio "
+    "asyncPin";
 
 bool ChromotingInstance::ParseAuthMethods(const std::string& auth_methods_str,
                                           ClientConfig* config) {
@@ -171,6 +172,7 @@ ChromotingInstance::ChromotingInstance(PP_Instance pp_instance)
       key_mapper_(&input_tracker_),
 #endif
       input_handler_(&key_mapper_),
+      use_async_pin_dialog_(false),
       weak_factory_(ALLOW_THIS_IN_INITIALIZER_LIST(this)) {
   RequestInputEvents(PP_INPUTEVENT_CLASS_MOUSE | PP_INPUTEVENT_CLASS_WHEEL);
   RequestFilteringInputEvents(PP_INPUTEVENT_CLASS_KEYBOARD);
@@ -270,14 +272,25 @@ void ChromotingInstance::HandleMessage(const pp::Var& message) {
     if (!data->GetString("hostJid", &config.host_jid) ||
         !data->GetString("hostPublicKey", &config.host_public_key) ||
         !data->GetString("localJid", &config.local_jid) ||
-        !data->GetString("sharedSecret", &config.shared_secret) ||
         !data->GetString("authenticationMethods", &auth_methods) ||
         !ParseAuthMethods(auth_methods, &config) ||
         !data->GetString("authenticationTag", &config.authentication_tag)) {
       LOG(ERROR) << "Invalid connect() data.";
       return;
     }
-
+    if (use_async_pin_dialog_) {
+      config.fetch_secret_callback =
+          base::Bind(&ChromotingInstance::FetchSecretFromDialog,
+                     this->AsWeakPtr());
+    } else {
+      std::string shared_secret;
+      if (!data->GetString("sharedSecret", &shared_secret)) {
+        LOG(ERROR) << "sharedSecret not specified in connect().";
+        return;
+      }
+      config.fetch_secret_callback =
+          base::Bind(&ChromotingInstance::FetchSecretFromString, shared_secret);
+    }
     Connect(config);
   } else if (method == "disconnect") {
     Disconnect();
@@ -373,6 +386,15 @@ void ChromotingInstance::HandleMessage(const pp::Var& message) {
       return;
     }
     PauseAudio(pause);
+  } else if (method == "useAsyncPinDialog") {
+    use_async_pin_dialog_ = true;
+  } else if (method == "onPinFetched") {
+    std::string pin;
+    if (!data->GetString("pin", &pin)) {
+      LOG(ERROR) << "Invalid onPinFetched.";
+      return;
+    }
+    OnPinFetched(pin);
   }
 }
 
@@ -424,6 +446,23 @@ void ChromotingInstance::OnConnectionReady(bool ready) {
   PostChromotingMessage("onConnectionReady", data.Pass());
 }
 
+void ChromotingInstance::FetchSecretFromDialog(
+    const protocol::SecretFetchedCallback& secret_fetched_callback) {
+  // Once the Session object calls this function, it won't continue the
+  // authentication until the callback is called (or connection is canceled).
+  // So, it's impossible to reach this with a callback already registered.
+  DCHECK(secret_fetched_callback_.is_null());
+  secret_fetched_callback_ = secret_fetched_callback;
+  scoped_ptr<base::DictionaryValue> data(new base::DictionaryValue());
+  PostChromotingMessage("fetchPin", data.Pass());
+}
+
+void ChromotingInstance::FetchSecretFromString(
+    const std::string& shared_secret,
+    const protocol::SecretFetchedCallback& secret_fetched_callback) {
+  secret_fetched_callback.Run(shared_secret);
+}
+
 protocol::ClipboardStub* ChromotingInstance::GetClipboardStub() {
   // TODO(sergeyu): Move clipboard handling to a separate class.
   // crbug.com/138108
@@ -650,6 +689,15 @@ void ChromotingInstance::PauseAudio(bool pause) {
   host_connection_->host_stub()->ControlAudio(audio_control);
 }
 
+void ChromotingInstance::OnPinFetched(const std::string& pin) {
+  if (!secret_fetched_callback_.is_null()) {
+    secret_fetched_callback_.Run(pin);
+    secret_fetched_callback_.Reset();
+  } else {
+    VLOG(1) << "Ignored OnPinFetched received without a pending fetch.";
+  }
+}
+
 ChromotingStats* ChromotingInstance::GetStats() {
   if (!client_.get())
     return NULL;
diff --git a/remoting/client/plugin/chromoting_instance.h b/remoting/client/plugin/chromoting_instance.h
index 9765f96..e814eaf 100644
--- a/remoting/client/plugin/chromoting_instance.h
+++ b/remoting/client/plugin/chromoting_instance.h
@@ -39,6 +39,7 @@
 #include "remoting/protocol/cursor_shape_stub.h"
 #include "remoting/protocol/input_event_tracker.h"
 #include "remoting/protocol/mouse_input_filter.h"
+#include "remoting/protocol/negotiating_authenticator.h"
 
 namespace base {
 class DictionaryValue;
@@ -120,20 +121,6 @@ class ChromotingInstance :
   void SetDesktopSize(const SkISize& size, const SkIPoint& dpi);
   void OnFirstFrameReceived();
 
-  // Message handlers for messages that come from JavaScript. Called
-  // from HandleMessage().
-  void Connect(const ClientConfig& config);
-  void Disconnect();
-  void OnIncomingIq(const std::string& iq);
-  void ReleaseAllKeys();
-  void InjectKeyEvent(const protocol::KeyEvent& event);
-  void RemapKey(uint32 in_usb_keycode, uint32 out_usb_keycode);
-  void TrapKey(uint32 usb_keycode, bool trap);
-  void SendClipboardItem(const std::string& mime_type, const std::string& item);
-  void NotifyClientResolution(int width, int height, int x_dpi, int y_dpi);
-  void PauseVideo(bool pause);
-  void PauseAudio(bool pause);
-
   // Return statistics record by ChromotingClient.
   // If no connection is currently active then NULL will be returned.
   ChromotingStats* GetStats();
@@ -163,6 +150,27 @@ class ChromotingInstance :
  private:
   FRIEND_TEST_ALL_PREFIXES(ChromotingInstanceTest, TestCaseSetup);
 
+  // Used as the |FetchSecretCallback| for IT2Me (or Me2Me from old webapps).
+  // Immediately calls |secret_fetched_callback| with |shared_secret|.
+  static void FetchSecretFromString(
+      const std::string& shared_secret,
+      const protocol::SecretFetchedCallback& secret_fetched_callback);
+
+  // Message handlers for messages that come from JavaScript. Called
+  // from HandleMessage().
+  void Connect(const ClientConfig& config);
+  void Disconnect();
+  void OnIncomingIq(const std::string& iq);
+  void ReleaseAllKeys();
+  void InjectKeyEvent(const protocol::KeyEvent& event);
+  void RemapKey(uint32 in_usb_keycode, uint32 out_usb_keycode);
+  void TrapKey(uint32 usb_keycode, bool trap);
+  void SendClipboardItem(const std::string& mime_type, const std::string& item);
+  void NotifyClientResolution(int width, int height, int x_dpi, int y_dpi);
+  void PauseVideo(bool pause);
+  void PauseAudio(bool pause);
+  void OnPinFetched(const std::string& pin);
+
   // Helper method to post messages to the webapp.
   void PostChromotingMessage(const std::string& method,
                              scoped_ptr<base::DictionaryValue> data);
@@ -183,6 +191,11 @@ class ChromotingInstance :
   // Returns true if there is a ConnectionToHost and it is connected.
   bool IsConnected();
 
+  // Used as the |FetchSecretCallback| for Me2Me connections.
+  // Uses the PIN request dialog in the webapp to obtain the shared secret.
+  void FetchSecretFromDialog(
+      const protocol::SecretFetchedCallback& secret_fetched_callback);
+
   bool initialized_;
 
   PepperPluginThreadDelegate plugin_thread_delegate_;
@@ -210,6 +223,10 @@ class ChromotingInstance :
   // connection.
   scoped_refptr<PepperXmppProxy> xmpp_proxy_;
 
+  // PIN Fetcher.
+  bool use_async_pin_dialog_;
+  protocol::SecretFetchedCallback secret_fetched_callback_;
+
   base::WeakPtrFactory<ChromotingInstance> weak_factory_;
 
   DISALLOW_COPY_AND_ASSIGN(ChromotingInstance);
diff --git a/remoting/protocol/negotiating_authenticator.cc b/remoting/protocol/negotiating_authenticator.cc
index 7b794d2..c6ecacb 100644
--- a/remoting/protocol/negotiating_authenticator.cc
+++ b/remoting/protocol/negotiating_authenticator.cc
@@ -30,20 +30,14 @@ const char kSupportedMethodsSeparator = ',';
 }  // namespace
 
 // static
-bool NegotiatingAuthenticator::IsNegotiableMessage(
-    const buzz::XmlElement* message) {
-  return message->HasAttr(kSupportedMethodsAttributeQName);
-}
-
-// static
 scoped_ptr<Authenticator> NegotiatingAuthenticator::CreateForClient(
     const std::string& authentication_tag,
-    const std::string& shared_secret,
+    const FetchSecretCallback& fetch_secret_callback,
     const std::vector<AuthenticationMethod>& methods) {
   scoped_ptr<NegotiatingAuthenticator> result(
       new NegotiatingAuthenticator(MESSAGE_READY));
   result->authentication_tag_ = authentication_tag;
-  result->shared_secret_ = shared_secret;
+  result->fetch_secret_callback_ = fetch_secret_callback;
 
   DCHECK(!methods.empty());
   for (std::vector<AuthenticationMethod>::const_iterator it = methods.begin();
@@ -75,7 +69,8 @@ NegotiatingAuthenticator::NegotiatingAuthenticator(
     Authenticator::State initial_state)
     : current_method_(AuthenticationMethod::Invalid()),
       state_(initial_state),
-      rejection_reason_(INVALID_CREDENTIALS) {
+      rejection_reason_(INVALID_CREDENTIALS),
+      weak_factory_(ALLOW_THIS_IN_INITIALIZER_LIST(this)) {
 }
 
 NegotiatingAuthenticator::~NegotiatingAuthenticator() {
@@ -145,24 +140,24 @@ void NegotiatingAuthenticator::ProcessMessage(
 
     // Drop the current message because we've chosen a different
     // method.
-    state_ = MESSAGE_READY;
-  }
-
-  DCHECK(method.is_valid());
-
-  // Replace current authenticator if the method has changed.
-  if (method != current_method_) {
     current_method_ = method;
-    CreateAuthenticator(state_);
-  }
-  if (state_ == WAITING_MESSAGE) {
-    // |current_authenticator_| is owned, so Unretained() is safe here.
-    current_authenticator_->ProcessMessage(message, base::Bind(
+    state_ = PROCESSING_MESSAGE;
+    CreateAuthenticator(MESSAGE_READY, base::Bind(
         &NegotiatingAuthenticator::UpdateState,
         base::Unretained(this), resume_callback));
-  } else {
-    UpdateState(resume_callback);
+    return;
   }
+  if (method != current_method_) {
+    current_method_ = method;
+    state_ = PROCESSING_MESSAGE;
+    // Copy the message since the authenticator may process it asynchronously.
+    CreateAuthenticator(WAITING_MESSAGE, base::Bind(
+        &NegotiatingAuthenticator::ProcessMessageInternal,
+        base::Unretained(this), base::Owned(new buzz::XmlElement(*message)),
+        resume_callback));
+    return;
+  }
+  ProcessMessageInternal(message, resume_callback);
 }
 
 void NegotiatingAuthenticator::UpdateState(
@@ -179,27 +174,12 @@ void NegotiatingAuthenticator::UpdateState(
 scoped_ptr<buzz::XmlElement> NegotiatingAuthenticator::GetNextMessage() {
   DCHECK_EQ(state(), MESSAGE_READY);
 
-  bool add_supported_methods_attr = false;
+  scoped_ptr<buzz::XmlElement> result;
 
-  // Initialize current method in case it is not initialized
-  // yet. Normally happens only on client.
+  // No method yet, just send a message with the list of supported methods.
+  // Normally happens only on client.
   if (!current_method_.is_valid()) {
-    CHECK(!methods_.empty());
-
-    // Initially try the first method.
-    current_method_ = methods_[0];
-    CreateAuthenticator(MESSAGE_READY);
-    add_supported_methods_attr = true;
-  }
-
-  scoped_ptr<buzz::XmlElement> result =
-      current_authenticator_->GetNextMessage();
-  state_ = current_authenticator_->state();
-  DCHECK_NE(state_, REJECTED);
-
-  result->AddAttr(kMethodAttributeQName, current_method_.ToString());
-
-  if (add_supported_methods_attr) {
+    result = CreateEmptyAuthenticatorMessage();
     std::stringstream supported_methods(std::stringstream::out);
     for (std::vector<AuthenticationMethod>::iterator it = methods_.begin();
          it != methods_.end(); ++it) {
@@ -208,6 +188,16 @@ scoped_ptr<buzz::XmlElement> NegotiatingAuthenticator::GetNextMessage() {
       supported_methods << it->ToString();
     }
     result->AddAttr(kSupportedMethodsAttributeQName, supported_methods.str());
+    state_ = WAITING_MESSAGE;
+  } else {
+    if (current_authenticator_->state() == MESSAGE_READY) {
+      result = current_authenticator_->GetNextMessage();
+    } else {
+      result = CreateEmptyAuthenticatorMessage();
+    }
+    state_ = current_authenticator_->state();
+    DCHECK(state_ == ACCEPTED || state_ == WAITING_MESSAGE);
+    result->AddAttr(kMethodAttributeQName, current_method_.ToString());
   }
 
   return result.Pass();
@@ -228,17 +218,44 @@ bool NegotiatingAuthenticator::is_host_side() const {
   return local_key_pair_.get() != NULL;
 }
 
-void NegotiatingAuthenticator::CreateAuthenticator(State initial_state) {
+void NegotiatingAuthenticator::CreateAuthenticator(
+    Authenticator::State preferred_initial_state,
+    const base::Closure& resume_callback) {
   if (is_host_side()) {
     current_authenticator_ = V2Authenticator::CreateForHost(
-        local_cert_, local_key_pair_, shared_secret_hash_, initial_state);
+        local_cert_, local_key_pair_, shared_secret_hash_,
+        preferred_initial_state);
+    resume_callback.Run();
   } else {
-    current_authenticator_ = V2Authenticator::CreateForClient(
-        AuthenticationMethod::ApplyHashFunction(
-            current_method_.hash_function(),
-            authentication_tag_, shared_secret_), initial_state);
+    fetch_secret_callback_.Run(base::Bind(
+        &NegotiatingAuthenticator::CreateV2AuthenticatorWithSecret,
+        weak_factory_.GetWeakPtr(), preferred_initial_state, resume_callback));
+  }
+}
+
+void NegotiatingAuthenticator::ProcessMessageInternal(
+    const buzz::XmlElement* message,
+    const base::Closure& resume_callback) {
+  if (current_authenticator_->state() == WAITING_MESSAGE) {
+    // If the message was not discarded and the authenticator is waiting for it,
+    // give it to the underlying authenticator to process.
+    // |current_authenticator_| is owned, so Unretained() is safe here.
+    current_authenticator_->ProcessMessage(message, base::Bind(
+        &NegotiatingAuthenticator::UpdateState,
+        base::Unretained(this), resume_callback));
   }
 }
 
+void NegotiatingAuthenticator::CreateV2AuthenticatorWithSecret(
+    Authenticator::State initial_state,
+    const base::Closure& resume_callback,
+    const std::string& shared_secret) {
+  current_authenticator_ = V2Authenticator::CreateForClient(
+      AuthenticationMethod::ApplyHashFunction(
+          current_method_.hash_function(), authentication_tag_, shared_secret),
+      initial_state);
+  resume_callback.Run();
+}
+
 }  // namespace protocol
 }  // namespace remoting
diff --git a/remoting/protocol/negotiating_authenticator.h b/remoting/protocol/negotiating_authenticator.h
index a87d543e..7166cf5 100644
--- a/remoting/protocol/negotiating_authenticator.h
+++ b/remoting/protocol/negotiating_authenticator.h
@@ -11,6 +11,7 @@
 #include "base/basictypes.h"
 #include "base/memory/ref_counted.h"
 #include "base/memory/scoped_ptr.h"
+#include "base/memory/weak_ptr.h"
 #include "remoting/protocol/authenticator.h"
 #include "remoting/protocol/authentication_method.h"
 
@@ -20,16 +21,57 @@ class RsaKeyPair;
 
 namespace protocol {
 
+typedef base::Callback<void(const std::string& secret)> SecretFetchedCallback;
+typedef base::Callback<void(
+    const SecretFetchedCallback& secret_fetched_callback)> FetchSecretCallback;
+
+// This class provides a meta-authenticator that allows clients and hosts that
+// support multiple authentication methods to negotiate a method to use.
+//
+// The typical flow is:
+//  * Client sends a message to host with its supported methods.
+//      (clients may additionally pick a method and send its first message).
+//  * Host picks a method and sends its first message (if any).
+//      (if a message for that method was sent by the client, it is processed).
+//  * Client creates the authenticator selected by the host. If the method
+//      starts with a message from the host, it is processed.
+//  * Client and host exchange messages until the authentication is ACCEPTED or
+//      REJECTED.
+//
+// The details:
+//  * CreateAuthenticator() may be asynchronous (i.e. require user interaction
+//      to determine initial parameters, like PIN). This happens inside
+//      ProcessMessage, so to the outside this behaves like any asynchronous
+//      message processing. Internally, CreateAuthenticator() receives a
+//      callback, that will resume the authentication once the authenticator is
+//      created. If there is already a message to be processed by the new
+//      authenticator, this callback includes a call to the underlying
+//      ProcessMessage().
+//  * Some authentication methods may have a specific starting direction (e.g.
+//      host always sends the first message), while others are versatile (e.g.
+//      SPAKE, where either side can send the first message). When an
+//      authenticator is created, it is given a preferred initial state, which
+//      the authenticator may ignore.
+//  * If the new authenticator state doesn't match the preferred one,
+//      the NegotiatingAuthenticator deals with that, by sending an empty
+//      <authenticator> stanza if the method has no message to send, and
+//      ignoring such empty messages on the receiving end.
+//  * The client may optimistically pick a method on its first message (assuming
+//      it doesn't require user interaction to start). If the host doesn't
+//      support that method, it will just discard that message, and choose
+//      another method from the client's supported methods list.
+//  * The host never sends its own supported methods back to the client, so once
+//      the host picks a method from the client's list, it's final.
+//  * Any change in this class must maintain compatibility between any version
+//      mix of webapp, client plugin and host, for both Me2Me and IT2Me.
 class NegotiatingAuthenticator : public Authenticator {
  public:
   virtual ~NegotiatingAuthenticator();
 
-  static bool IsNegotiableMessage(const buzz::XmlElement* message);
-
   // Creates a client authenticator for the given methods.
   static scoped_ptr<Authenticator> CreateForClient(
       const std::string& authentication_tag,
-      const std::string& shared_secret,
+      const FetchSecretCallback& fetch_secret_callback,
       const std::vector<AuthenticationMethod>& methods);
 
   // Creates a host authenticator, using a fixed shared secret/PIN hash.
@@ -49,12 +91,33 @@ class NegotiatingAuthenticator : public Authenticator {
       CreateChannelAuthenticator() const OVERRIDE;
 
  private:
-  NegotiatingAuthenticator(Authenticator::State initial_state);
+  explicit NegotiatingAuthenticator(Authenticator::State initial_state);
 
   void AddMethod(const AuthenticationMethod& method);
-  void CreateAuthenticator(State initial_state);
+
+  // (Asynchronously) creates an authenticator, and stores it in
+  // |current_authenticator_|. Authenticators that can be started in either
+  // state will be created in |preferred_initial_state|.
+  // |resume_callback| is called after |current_authenticator_| is set.
+  void CreateAuthenticator(Authenticator::State preferred_initial_state,
+                           const base::Closure& resume_callback);
+
+  // Calls |current_authenticator_| to process |message|, passing the supplied
+  // |resume_callback|.
+  void ProcessMessageInternal(const buzz::XmlElement* message,
+                              const base::Closure& resume_callback);
+
+  // Updates |state_| to reflect the current underlying authenticator state.
+  // |resume_callback| is called after the state is updated.
   void UpdateState(const base::Closure& resume_callback);
 
+  // Creates a V2Authenticator in state |initial_state| with the given
+  // |shared_secret|, then runs |resume_callback|.
+  void CreateV2AuthenticatorWithSecret(
+      Authenticator::State initial_state,
+      const base::Closure& resume_callback,
+      const std::string& shared_secret);
+
   bool is_host_side() const;
 
   // Used only for host authenticators.
@@ -64,7 +127,7 @@ class NegotiatingAuthenticator : public Authenticator {
 
   // Used only for client authenticators.
   std::string authentication_tag_;
-  std::string shared_secret_;
+  FetchSecretCallback fetch_secret_callback_;
 
   // Used for both host and client authenticators.
   std::vector<AuthenticationMethod> methods_;
@@ -73,6 +136,8 @@ class NegotiatingAuthenticator : public Authenticator {
   State state_;
   RejectionReason rejection_reason_;
 
+  base::WeakPtrFactory<NegotiatingAuthenticator> weak_factory_;
+
   DISALLOW_COPY_AND_ASSIGN(NegotiatingAuthenticator);
 };
 
diff --git a/remoting/protocol/negotiating_authenticator_unittest.cc b/remoting/protocol/negotiating_authenticator_unittest.cc
index 73daf83..b17fa6a 100644
--- a/remoting/protocol/negotiating_authenticator_unittest.cc
+++ b/remoting/protocol/negotiating_authenticator_unittest.cc
@@ -59,9 +59,15 @@ class NegotiatingAuthenticatorTest : public AuthenticatorTestBase {
           AuthenticationMethod::NONE));
     }
     client_ = NegotiatingAuthenticator::CreateForClient(
-        kTestHostId, client_secret, methods);
+        kTestHostId, base::Bind(&NegotiatingAuthenticatorTest::FetchSecret,
+                                client_secret), methods);
   }
 
+  static void FetchSecret(
+      const std::string& client_secret,
+      const protocol::SecretFetchedCallback& secret_fetched_callback) {
+    secret_fetched_callback.Run(client_secret);
+  }
   void VerifyRejected(Authenticator::RejectionReason reason) {
     ASSERT_TRUE((client_->state() == Authenticator::REJECTED &&
                  (client_->rejection_reason() == reason)) ||
@@ -69,6 +75,28 @@ class NegotiatingAuthenticatorTest : public AuthenticatorTestBase {
                  (host_->rejection_reason() == reason)));
   }
 
+  void VerifyAccepted() {
+    ASSERT_NO_FATAL_FAILURE(RunAuthExchange());
+
+    ASSERT_EQ(Authenticator::ACCEPTED, host_->state());
+    ASSERT_EQ(Authenticator::ACCEPTED, client_->state());
+
+    client_auth_ = client_->CreateChannelAuthenticator();
+    host_auth_ = host_->CreateChannelAuthenticator();
+    RunChannelAuth(false);
+
+    EXPECT_TRUE(client_socket_.get() != NULL);
+    EXPECT_TRUE(host_socket_.get() != NULL);
+
+    StreamConnectionTester tester(host_socket_.get(), client_socket_.get(),
+                                  kMessageSize, kMessages);
+
+    tester.Start();
+    message_loop_.Run();
+    tester.CheckResults();
+  }
+
+ private:
   DISALLOW_COPY_AND_ASSIGN(NegotiatingAuthenticatorTest);
 };
 
@@ -76,48 +104,14 @@ TEST_F(NegotiatingAuthenticatorTest, SuccessfulAuthHmac) {
   ASSERT_NO_FATAL_FAILURE(InitAuthenticators(
       kTestSharedSecret, kTestSharedSecret,
       AuthenticationMethod::HMAC_SHA256, false));
-  ASSERT_NO_FATAL_FAILURE(RunAuthExchange());
-
-  ASSERT_EQ(Authenticator::ACCEPTED, host_->state());
-  ASSERT_EQ(Authenticator::ACCEPTED, client_->state());
-
-  client_auth_ = client_->CreateChannelAuthenticator();
-  host_auth_ = host_->CreateChannelAuthenticator();
-  RunChannelAuth(false);
-
-  EXPECT_TRUE(client_socket_.get() != NULL);
-  EXPECT_TRUE(host_socket_.get() != NULL);
-
-  StreamConnectionTester tester(host_socket_.get(), client_socket_.get(),
-                                kMessageSize, kMessages);
-
-  tester.Start();
-  message_loop_.Run();
-  tester.CheckResults();
+  VerifyAccepted();
 }
 
 TEST_F(NegotiatingAuthenticatorTest, SuccessfulAuthPlain) {
   ASSERT_NO_FATAL_FAILURE(InitAuthenticators(
       kTestSharedSecret, kTestSharedSecret,
       AuthenticationMethod::NONE, false));
-  ASSERT_NO_FATAL_FAILURE(RunAuthExchange());
-
-  ASSERT_EQ(Authenticator::ACCEPTED, host_->state());
-  ASSERT_EQ(Authenticator::ACCEPTED, client_->state());
-
-  client_auth_ = client_->CreateChannelAuthenticator();
-  host_auth_ = host_->CreateChannelAuthenticator();
-  RunChannelAuth(false);
-
-  EXPECT_TRUE(client_socket_.get() != NULL);
-  EXPECT_TRUE(host_socket_.get() != NULL);
-
-  StreamConnectionTester tester(host_socket_.get(), client_socket_.get(),
-                                kMessageSize, kMessages);
-
-  tester.Start();
-  message_loop_.Run();
-  tester.CheckResults();
+  VerifyAccepted();
 }
 
 TEST_F(NegotiatingAuthenticatorTest, InvalidSecretHmac) {