Linux pairing registry delegate implementation

BUG=156182

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

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

6 files changed, 170 insertions, 109 deletions

diff --git a/remoting/protocol/negotiating_authenticator_unittest.cc b/remoting/protocol/negotiating_authenticator_unittest.cc
index fb7ed34..b746432 100644
--- a/remoting/protocol/negotiating_authenticator_unittest.cc
+++ b/remoting/protocol/negotiating_authenticator_unittest.cc
@@ -85,14 +85,14 @@ class NegotiatingAuthenticatorTest : public AuthenticatorTestBase {
 
   void CreatePairingRegistry(bool with_paired_client) {
     mock_delegate_ = new MockPairingRegistryDelegate;
+    pairing_registry_ = new PairingRegistry(
+        scoped_ptr<PairingRegistry::Delegate>(mock_delegate_));
     if (with_paired_client) {
       PairingRegistry::Pairing pairing(
           base::Time(), kTestClientName, kTestClientId, kTestPairedSecret);
-      mock_delegate_->AddPairing(pairing,
-                                 PairingRegistry::AddPairingCallback());
+      pairing_registry_->AddPairing(pairing);
+      mock_delegate_->RunCallback();
     }
-    pairing_registry_ = new PairingRegistry(
-        scoped_ptr<PairingRegistry::Delegate>(mock_delegate_));
   }
 
   static void FetchSecret(
diff --git a/remoting/protocol/pairing_registry.cc b/remoting/protocol/pairing_registry.cc
index 6c9cc66..558c302 100644
--- a/remoting/protocol/pairing_registry.cc
+++ b/remoting/protocol/pairing_registry.cc
@@ -7,8 +7,18 @@
 #include "base/base64.h"
 #include "base/bind.h"
 #include "base/guid.h"
+#include "base/json/json_string_value_serializer.h"
+#include "base/strings/string_number_conversions.h"
+#include "base/values.h"
 #include "crypto/random.h"
 
+namespace {
+const char kCreatedTimeKey[] = "created-time";
+const char kClientIdKey[] = "client-id";
+const char kClientNameKey[] = "client-name";
+const char kSharedSecretKey[] = "shared-secret";
+}  // namespace
+
 namespace remoting {
 namespace protocol {
 
@@ -68,30 +78,101 @@ PairingRegistry::Pairing PairingRegistry::CreatePairing(
     const std::string& client_name) {
   DCHECK(CalledOnValidThread());
   Pairing result = Pairing::Create(client_name);
-  delegate_->AddPairing(result, AddPairingCallback());
+  AddPairing(result);
   return result;
 }
 
 void PairingRegistry::GetPairing(const std::string& client_id,
                                  const GetPairingCallback& callback) {
   DCHECK(CalledOnValidThread());
-  delegate_->GetPairing(client_id, callback);
+  delegate_->Load(
+      base::Bind(&PairingRegistry::DoGetPairing, this, client_id, callback));
+}
+
+void PairingRegistry::AddPairing(const Pairing& pairing) {
+  delegate_->Load(
+      base::Bind(&PairingRegistry::MergePairingAndSave, this, pairing));
+}
+
+void PairingRegistry::MergePairingAndSave(const Pairing& pairing,
+                                          const std::string& pairings_json) {
+  DCHECK(CalledOnValidThread());
+  PairedClients clients = DecodeJson(pairings_json);
+  clients[pairing.client_id()] = pairing;
+  std::string new_pairings_json = EncodeJson(clients);
+  delegate_->Save(new_pairings_json, SaveCallback());
 }
 
-void NotImplementedPairingRegistryDelegate::AddPairing(
-    const PairingRegistry::Pairing& new_paired_client,
-    const PairingRegistry::AddPairingCallback& callback) {
-  NOTIMPLEMENTED();
-  if (!callback.is_null()) {
-    callback.Run(false);
+void PairingRegistry::DoGetPairing(const std::string& client_id,
+                                   const GetPairingCallback& callback,
+                                   const std::string& pairings_json) {
+  PairedClients clients = DecodeJson(pairings_json);
+  Pairing result = clients[client_id];
+  callback.Run(result);
+}
+
+PairingRegistry::PairedClients PairingRegistry::DecodeJson(
+    const std::string& pairings_json) {
+  PairedClients result;
+
+  if (pairings_json.empty()) {
+    return result;
+  }
+
+  JSONStringValueSerializer registry(pairings_json);
+  int error_code;
+  std::string error_message;
+  scoped_ptr<base::Value> root(
+      registry.Deserialize(&error_code, &error_message));
+  if (!root) {
+    LOG(ERROR) << "Failed to load paired clients: " << error_message
+               << " (" << error_code << ").";
+    return result;
   }
+
+  base::ListValue* root_list = NULL;
+  if (!root->GetAsList(&root_list)) {
+    LOG(ERROR) << "Failed to load paired clients: root node is not a list.";
+    return result;
+  }
+
+  for (size_t i = 0; i < root_list->GetSize(); ++i) {
+    base::DictionaryValue* pairing = NULL;
+    std::string client_name, client_id, shared_secret;
+    double created_time_value;
+    if (root_list->GetDictionary(i, &pairing) &&
+        pairing->GetDouble(kCreatedTimeKey, &created_time_value) &&
+        pairing->GetString(kClientNameKey, &client_name) &&
+        pairing->GetString(kClientIdKey, &client_id) &&
+        pairing->GetString(kSharedSecretKey, &shared_secret)) {
+      base::Time created_time = base::Time::FromJsTime(created_time_value);
+      result[client_id] = Pairing(
+          created_time, client_name, client_id, shared_secret);
+    } else {
+      LOG(ERROR) << "Paired client " << i << " has unexpected format.";
+    }
+  }
+
+  return result;
 }
 
-void NotImplementedPairingRegistryDelegate::GetPairing(
-    const std::string& client_id,
-    const PairingRegistry::GetPairingCallback& callback) {
-  NOTIMPLEMENTED();
-  callback.Run(PairingRegistry::Pairing());
+std::string PairingRegistry::EncodeJson(const PairedClients& clients) {
+  base::ListValue root;
+  for (PairedClients::const_iterator i = clients.begin();
+       i != clients.end(); ++i) {
+    base::DictionaryValue* pairing = new base::DictionaryValue();
+    pairing->SetDouble(kCreatedTimeKey, i->second.created_time().ToJsTime());
+    pairing->SetString(kClientNameKey, i->second.client_name());
+    pairing->SetString(kClientIdKey, i->second.client_id());
+    pairing->SetString(kSharedSecretKey, i->second.shared_secret());
+    root.Append(pairing);
+  }
+
+  std::string result;
+  JSONStringValueSerializer serializer(&result);
+  serializer.Serialize(root);
+
+  return result;
 }
 
 }  // namespace protocol
diff --git a/remoting/protocol/pairing_registry.h b/remoting/protocol/pairing_registry.h
index 3544524..0938d4e 100644
--- a/remoting/protocol/pairing_registry.h
+++ b/remoting/protocol/pairing_registry.h
@@ -17,9 +17,6 @@
 namespace remoting {
 namespace protocol {
 
-// TODO(jamiewalch): This class is little more than a wrapper around the
-// Pairing and Delegate classes. Refactor it away.
-
 // PairingRegistry holds information about paired clients to support
 // PIN-less authentication. For each paired client, the registry holds
 // the following information:
@@ -62,62 +59,63 @@ class PairingRegistry : public base::RefCountedThreadSafe<PairingRegistry>,
   typedef std::map<std::string, Pairing> PairedClients;
 
   // Delegate callbacks.
-  typedef base::Callback<void(Pairing client_information)> GetPairingCallback;
-  typedef base::Callback<void(bool success)> AddPairingCallback;
+  typedef base::Callback<void(const std::string& pairings_json)> LoadCallback;
+  typedef base::Callback<void(bool success)> SaveCallback;
+  typedef base::Callback<void(Pairing pariring)> GetPairingCallback;
 
   // Interface representing the persistent storage back-end.
   class Delegate {
    public:
     virtual ~Delegate() {}
 
-    // Add pairing information to persistent storage. If a non-NULL callback
-    // is provided, invoke it on completion to indicate success or failure.
-    // Must not block.
-    //
-    // TODO(jamiewalch): Plumb the callback into the RequestPairing flow so
-    // that the client isn't sent the pairing information until it has been
-    // saved.
-    virtual void AddPairing(const Pairing& new_paired_client,
-                            const AddPairingCallback& callback) = 0;
-
-    // Retrieve the Pairing for the specified client id. If none is found,
-    // invoke the callback with a default pairing. Must not block.
-    virtual void GetPairing(const std::string& client_id,
-                            const GetPairingCallback& callback) = 0;
+    // Save JSON-encoded pairing information to persistent storage. If
+    // a non-NULL callback is provided, invoke it on completion to
+    // indicate success or failure. Must not block.
+    virtual void Save(const std::string& pairings_json,
+                      const SaveCallback& callback) = 0;
+
+    // Retrieve the JSON-encoded pairing information from persistent
+    // storage. Must not block.
+    virtual void Load(const LoadCallback& callback) = 0;
   };
 
   explicit PairingRegistry(scoped_ptr<Delegate> delegate);
 
-  // Create a pairing for a new client and save it to disk.
+  // Creates a pairing for a new client and saves it to disk.
+  //
+  // TODO(jamiewalch): Plumb the Save callback into the RequestPairing flow
+  // so that the client isn't sent the pairing information until it has been
+  // saved.
   Pairing CreatePairing(const std::string& client_name);
 
-  // Get the pairing for the specified client id. See the corresponding
-  // Delegate method for details.
+  // Gets the pairing for the specified client id. See the corresponding
+  // Delegate method for details. If none is found, the callback is invoked
+  // with an invalid Pairing.
   void GetPairing(const std::string& client_id,
                   const GetPairingCallback& callback);
 
  private:
+  FRIEND_TEST_ALL_PREFIXES(PairingRegistryTest, AddPairing);
+  friend class NegotiatingAuthenticatorTest;
   friend class base::RefCountedThreadSafe<PairingRegistry>;
 
   virtual ~PairingRegistry();
 
+  void AddPairing(const Pairing& pairing);;
+  void MergePairingAndSave(const Pairing& pairing,
+                           const std::string& pairings_json);
+  void DoGetPairing(const std::string& client_id,
+                    const GetPairingCallback& callback,
+                    const std::string& pairings_json);
+
+  static PairedClients DecodeJson(const std::string& pairings_json);
+  static std::string EncodeJson(const PairedClients& clients);
+
   scoped_ptr<Delegate> delegate_;
 
   DISALLOW_COPY_AND_ASSIGN(PairingRegistry);
 };
 
-// Temporary delegate that just logs NOTIMPLEMENTED for Load/Save.
-// TODO(jamiewalch): Delete once Delegates are implemented for all platforms.
-class NotImplementedPairingRegistryDelegate : public PairingRegistry::Delegate {
- public:
-  virtual void AddPairing(
-      const PairingRegistry::Pairing& paired_clients,
-      const PairingRegistry::AddPairingCallback& callback) OVERRIDE;
-  virtual void GetPairing(
-      const std::string& client_id,
-      const PairingRegistry::GetPairingCallback& callback) OVERRIDE;
-};
-
 }  // namespace protocol
 }  // namespace remoting
 
diff --git a/remoting/protocol/pairing_registry_unittest.cc b/remoting/protocol/pairing_registry_unittest.cc
index a5d8bf2..460acbe 100644
--- a/remoting/protocol/pairing_registry_unittest.cc
+++ b/remoting/protocol/pairing_registry_unittest.cc
@@ -17,57 +17,47 @@ namespace protocol {
 
 class PairingRegistryTest : public testing::Test {
  public:
-  virtual void SetUp() OVERRIDE {
-    got_secret_ = false;
-  }
-
   void CompareSecret(const std::string& expected,
                      PairingRegistry::Pairing actual) {
-    EXPECT_EQ(actual.shared_secret(), expected);
+    EXPECT_EQ(expected, actual.shared_secret());
+    secret_ = actual.shared_secret();
     got_secret_ = true;
   }
 
  protected:
+  std::string secret_;
   bool got_secret_;
 };
 
-TEST_F(PairingRegistryTest, GetPairing) {
-  PairingRegistry::Pairing client_info =
-      PairingRegistry::Pairing::Create("client_name");
+TEST_F(PairingRegistryTest, CreateAndGetPairings) {
   MockPairingRegistryDelegate* mock_delegate =
       new MockPairingRegistryDelegate();
-  mock_delegate->AddPairing(client_info, PairingRegistry::AddPairingCallback());
   scoped_ptr<PairingRegistry::Delegate> delegate(mock_delegate);
 
   scoped_refptr<PairingRegistry> registry(new PairingRegistry(delegate.Pass()));
+  PairingRegistry::Pairing pairing_1 = registry->CreatePairing("client_name");
+  mock_delegate->RunCallback();
+  PairingRegistry::Pairing pairing_2 = registry->CreatePairing("client_name");
+  mock_delegate->RunCallback();
 
-  registry->GetPairing(client_info.client_id(),
+  registry->GetPairing(pairing_1.client_id(),
                        base::Bind(&PairingRegistryTest::CompareSecret,
                                   base::Unretained(this),
-                                  client_info.shared_secret()));
+                                  pairing_1.shared_secret()));
+  got_secret_ = false;
   mock_delegate->RunCallback();
   EXPECT_TRUE(got_secret_);
-}
-
-TEST_F(PairingRegistryTest, AddPairing) {
-  MockPairingRegistryDelegate* mock_delegate =
-      new MockPairingRegistryDelegate();
-  scoped_ptr<PairingRegistry::Delegate> delegate(mock_delegate);
-
-  scoped_refptr<PairingRegistry> registry(new PairingRegistry(delegate.Pass()));
-
-  // Verify that we can create pairings from two clients with the same name, but
-  // that they aren't allocated the same client id.
-  PairingRegistry::Pairing pairing_1 = registry->CreatePairing("client_name");
-  PairingRegistry::Pairing pairing_2 = registry->CreatePairing("client_name");
+  std::string secret_1 = secret_;
 
-  const PairingRegistry::PairedClients& clients =
-      mock_delegate->paired_clients();
-  ASSERT_EQ(clients.size(), 2u);
-  PairingRegistry::Pairing first = clients.begin()->second;
-  PairingRegistry::Pairing second = (++clients.begin())->second;
-  EXPECT_EQ(first.client_name(), second.client_name());
-  EXPECT_NE(first.client_id(), second.client_id());
+  // Check that the second client is paired with a different shared secret.
+  registry->GetPairing(pairing_2.client_id(),
+                       base::Bind(&PairingRegistryTest::CompareSecret,
+                                  base::Unretained(this),
+                                  pairing_2.shared_secret()));
+  got_secret_ = false;
+  mock_delegate->RunCallback();
+  EXPECT_TRUE(got_secret_);
+  EXPECT_NE(secret_, secret_1);
 }
 
 }  // namespace protocol
diff --git a/remoting/protocol/protocol_mock_objects.cc b/remoting/protocol/protocol_mock_objects.cc
index 1902d83..f558769 100644
--- a/remoting/protocol/protocol_mock_objects.cc
+++ b/remoting/protocol/protocol_mock_objects.cc
@@ -54,31 +54,24 @@ MockPairingRegistryDelegate::MockPairingRegistryDelegate() {
 MockPairingRegistryDelegate::~MockPairingRegistryDelegate() {
 }
 
-void MockPairingRegistryDelegate::AddPairing(
-    const PairingRegistry::Pairing& new_paired_client,
-    const PairingRegistry::AddPairingCallback& callback) {
-  paired_clients_[new_paired_client.client_id()] = new_paired_client;
+void MockPairingRegistryDelegate::Save(
+    const std::string& pairings_json,
+    const PairingRegistry::SaveCallback& callback) {
+  pairings_json_ = pairings_json;
   if (!callback.is_null()) {
     callback.Run(true);
   }
 }
 
-void MockPairingRegistryDelegate::GetPairing(
-    const std::string& client_id,
-    const PairingRegistry::GetPairingCallback& callback) {
-  PairingRegistry::Pairing result;
-  PairingRegistry::PairedClients::const_iterator i =
-      paired_clients_.find(client_id);
-  if (i != paired_clients_.end()) {
-    result = i->second;
-  }
-  saved_callback_ = base::Bind(base::Bind(callback), result);
+void MockPairingRegistryDelegate::Load(
+    const PairingRegistry::LoadCallback& callback) {
+  load_callback_ = base::Bind(base::Bind(callback), pairings_json_);
 }
 
 void MockPairingRegistryDelegate::RunCallback() {
-  DCHECK(!saved_callback_.is_null());
-  saved_callback_.Run();
-  saved_callback_.Reset();
+  DCHECK(!load_callback_.is_null());
+  load_callback_.Run();
+  load_callback_.Reset();
 }
 
 }  // namespace protocol
diff --git a/remoting/protocol/protocol_mock_objects.h b/remoting/protocol/protocol_mock_objects.h
index 0f1e24d..84960fd 100644
--- a/remoting/protocol/protocol_mock_objects.h
+++ b/remoting/protocol/protocol_mock_objects.h
@@ -211,23 +211,22 @@ class MockPairingRegistryDelegate : public PairingRegistry::Delegate {
   MockPairingRegistryDelegate();
   virtual ~MockPairingRegistryDelegate();
 
-  const PairingRegistry::PairedClients& paired_clients() const {
-    return paired_clients_;
+  const std::string& pairings_json() const {
+    return pairings_json_;
   }
 
   // PairingRegistry::Delegate implementation.
-  virtual void AddPairing(
-      const PairingRegistry::Pairing& new_paired_client,
-      const PairingRegistry::AddPairingCallback& callback) OVERRIDE;
-  virtual void GetPairing(
-      const std::string& client_id,
-      const PairingRegistry::GetPairingCallback& callback) OVERRIDE;
+  virtual void Save(
+      const std::string& pairings_json,
+      const PairingRegistry::SaveCallback& callback) OVERRIDE;
+  virtual void Load(
+      const PairingRegistry::LoadCallback& callback) OVERRIDE;
 
   void RunCallback();
 
  private:
-  base::Closure saved_callback_;
-  PairingRegistry::PairedClients paired_clients_;
+  base::Closure load_callback_;
+  std::string pairings_json_;
 };
 
 }  // namespace protocol