diff options
author | jamiewalch@chromium.org <jamiewalch@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2013-06-23 14:10:30 +0000 |
---|---|---|
committer | jamiewalch@chromium.org <jamiewalch@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2013-06-23 14:10:30 +0000 |
commit | b1ae901f706f60fb1628d6f2254869265a97a2b3 (patch) | |
tree | 633d8e62314fa6593f634b39b8942976430c5271 /remoting | |
parent | 2885f9cbf2dc9b5217373147b5e2b61319362449 (diff) | |
download | chromium_src-b1ae901f706f60fb1628d6f2254869265a97a2b3.zip chromium_src-b1ae901f706f60fb1628d6f2254869265a97a2b3.tar.gz chromium_src-b1ae901f706f60fb1628d6f2254869265a97a2b3.tar.bz2 |
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
Diffstat (limited to 'remoting')
-rw-r--r-- | remoting/host/pairing_registry_delegate.h | 23 | ||||
-rw-r--r-- | remoting/host/pairing_registry_delegate_linux.cc | 132 | ||||
-rw-r--r-- | remoting/host/pairing_registry_delegate_linux.h | 73 | ||||
-rw-r--r-- | remoting/host/pairing_registry_delegate_linux_unittest.cc | 76 | ||||
-rw-r--r-- | remoting/host/pairing_registry_delegate_mac.cc | 18 | ||||
-rw-r--r-- | remoting/host/pairing_registry_delegate_win.cc | 18 | ||||
-rw-r--r-- | remoting/host/remoting_me2me_host.cc | 10 | ||||
-rw-r--r-- | remoting/protocol/negotiating_authenticator_unittest.cc | 8 | ||||
-rw-r--r-- | remoting/protocol/pairing_registry.cc | 107 | ||||
-rw-r--r-- | remoting/protocol/pairing_registry.h | 66 | ||||
-rw-r--r-- | remoting/protocol/pairing_registry_unittest.cc | 52 | ||||
-rw-r--r-- | remoting/protocol/protocol_mock_objects.cc | 27 | ||||
-rw-r--r-- | remoting/protocol/protocol_mock_objects.h | 19 | ||||
-rw-r--r-- | remoting/remoting.gyp | 6 |
14 files changed, 520 insertions, 115 deletions
diff --git a/remoting/host/pairing_registry_delegate.h b/remoting/host/pairing_registry_delegate.h new file mode 100644 index 0000000..95a4907 --- /dev/null +++ b/remoting/host/pairing_registry_delegate.h @@ -0,0 +1,23 @@ +// Copyright 2013 The Chromium Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#ifndef REMOTING_HOST_PAIRING_REGISTRY_DELEGATE_H_ +#define REMOTING_HOST_PAIRING_REGISTRY_DELEGATE_H_ + +#include "base/memory/scoped_ptr.h" +#include "remoting/protocol/pairing_registry.h" + +namespace base { +class TaskRunner; +} // namespace base + +namespace remoting { +// Returns a platform-specific pairing registry delegate that will save to +// permanent storage using the specified TaskRunner. Returns NULL on platforms +// that don't support pairing. +scoped_ptr<protocol::PairingRegistry::Delegate> +CreatePairingRegistryDelegate(scoped_refptr<base::TaskRunner> task_runner); +} // namespace remoting + +#endif // REMOTING_HOST_PAIRING_REGISTRY_DELEGATE_H_ diff --git a/remoting/host/pairing_registry_delegate_linux.cc b/remoting/host/pairing_registry_delegate_linux.cc new file mode 100644 index 0000000..bca9b4f --- /dev/null +++ b/remoting/host/pairing_registry_delegate_linux.cc @@ -0,0 +1,132 @@ +// Copyright 2013 The Chromium Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#include "remoting/host/pairing_registry_delegate_linux.h" + +#include "base/bind.h" +#include "base/file_util.h" +#include "base/files/important_file_writer.h" +#include "base/location.h" +#include "base/single_thread_task_runner.h" +#include "base/thread_task_runner_handle.h" +#include "remoting/host/branding.h" + +namespace { +const char kRegistryFilename[] = "paired-clients.json"; +} // namespace + +namespace remoting { + +using protocol::PairingRegistry; + +PairingRegistryDelegateLinux::PairingRegistryDelegateLinux( + scoped_refptr<base::TaskRunner> task_runner) + : task_runner_(task_runner), + weak_factory_(this) { +} + +PairingRegistryDelegateLinux::~PairingRegistryDelegateLinux() { +} + +void PairingRegistryDelegateLinux::Save( + const std::string& pairings_json, + const PairingRegistry::SaveCallback& callback) { + // If a callback was supplied, wrap it in a helper function that will + // run it on this thread. + PairingRegistry::SaveCallback run_callback_on_this_thread; + if (!callback.is_null()) { + run_callback_on_this_thread = + base::Bind(&PairingRegistryDelegateLinux::RunSaveCallbackOnThread, + base::ThreadTaskRunnerHandle::Get(), + callback); + } + task_runner_->PostTask( + FROM_HERE, + base::Bind(&PairingRegistryDelegateLinux::DoSave, + weak_factory_.GetWeakPtr(), + pairings_json, + run_callback_on_this_thread)); +} + +void PairingRegistryDelegateLinux::Load( + const PairingRegistry::LoadCallback& callback) { + // Wrap the callback in a helper function that will run it on this thread. + // Note that, unlike AddPairing, the GetPairing callback is mandatory. + PairingRegistry::LoadCallback run_callback_on_this_thread = + base::Bind(&PairingRegistryDelegateLinux::RunLoadCallbackOnThread, + base::ThreadTaskRunnerHandle::Get(), + callback); + task_runner_->PostTask( + FROM_HERE, + base::Bind(&PairingRegistryDelegateLinux::DoLoad, + weak_factory_.GetWeakPtr(), + run_callback_on_this_thread)); +} + +void PairingRegistryDelegateLinux::RunSaveCallbackOnThread( + scoped_refptr<base::TaskRunner> task_runner, + const PairingRegistry::SaveCallback& callback, + bool success) { + task_runner->PostTask(FROM_HERE, base::Bind(callback, success)); +} + +void PairingRegistryDelegateLinux::RunLoadCallbackOnThread( + scoped_refptr<base::TaskRunner> task_runner, + const PairingRegistry::LoadCallback& callback, + const std::string& pairings_json) { + task_runner->PostTask(FROM_HERE, base::Bind(callback, pairings_json)); +} + +void PairingRegistryDelegateLinux::DoSave( + const std::string& pairings_json, + const PairingRegistry::SaveCallback& callback) { + base::FilePath registry_path = GetRegistryFilePath(); + base::FilePath parent_directory = registry_path.DirName(); + base::PlatformFileError error; + if (!file_util::CreateDirectoryAndGetError(parent_directory, &error)) { + LOG(ERROR) << "Could not create pairing registry directory: " << error; + return; + } + if (!base::ImportantFileWriter::WriteFileAtomically(registry_path, + pairings_json)) { + LOG(ERROR) << "Could not save pairing registry."; + } + + if (!callback.is_null()) { + callback.Run(true); + } +} + +void PairingRegistryDelegateLinux::DoLoad( + const PairingRegistry::LoadCallback& callback) { + base::FilePath registry_path = GetRegistryFilePath(); + std::string result; + if (!file_util::ReadFileToString(registry_path, &result)) { + LOG(ERROR) << "Load failed."; + } + callback.Run(result); +} + +base::FilePath PairingRegistryDelegateLinux::GetRegistryFilePath() { + if (!filename_for_testing_.empty()) { + return filename_for_testing_; + } + + base::FilePath config_dir = remoting::GetConfigDir(); + return config_dir.Append(kRegistryFilename); +} + +void PairingRegistryDelegateLinux::SetFilenameForTesting( + const base::FilePath &filename) { + filename_for_testing_ = filename; +} + + +scoped_ptr<PairingRegistry::Delegate> CreatePairingRegistryDelegate( + scoped_refptr<base::TaskRunner> task_runner) { + return scoped_ptr<PairingRegistry::Delegate>( + new PairingRegistryDelegateLinux(task_runner)); +} + +} // namespace remoting diff --git a/remoting/host/pairing_registry_delegate_linux.h b/remoting/host/pairing_registry_delegate_linux.h new file mode 100644 index 0000000..08c073b --- /dev/null +++ b/remoting/host/pairing_registry_delegate_linux.h @@ -0,0 +1,73 @@ +// Copyright 2013 The Chromium Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#ifndef REMOTING_PROTOCOL_PAIRING_REGISTRY_DELEGATE_LINUX_H_ +#define REMOTING_PROTOCOL_PAIRING_REGISTRY_DELEGATE_LINUX_H_ + +#include "remoting/protocol/pairing_registry.h" + +#include "base/files/file_path.h" +#include "base/memory/weak_ptr.h" + +namespace base { +class ListValue; +class TaskRunner; +} // namespace base + +namespace remoting { + +class PairingRegistryDelegateLinux + : public protocol::PairingRegistry::Delegate { + public: + explicit PairingRegistryDelegateLinux( + scoped_refptr<base::TaskRunner> task_runner); + virtual ~PairingRegistryDelegateLinux(); + + // PairingRegistry::Delegate interface + virtual void Save( + const std::string& pairings_json, + const protocol::PairingRegistry::SaveCallback& callback) OVERRIDE; + virtual void Load( + const protocol::PairingRegistry::LoadCallback& callback) OVERRIDE; + + private: + FRIEND_TEST_ALL_PREFIXES(PairingRegistryDelegateLinuxTest, SaveAndLoad); + + // Blocking helper methods run using the TaskRunner passed to the ctor. + void DoSave(const std::string& pairings_json, + const protocol::PairingRegistry::SaveCallback& callback); + void DoLoad(const protocol::PairingRegistry::LoadCallback& callback); + + // Run the delegate callbacks on their original thread. + static void RunSaveCallbackOnThread( + scoped_refptr<base::TaskRunner> task_runner, + const protocol::PairingRegistry::SaveCallback& callback, + bool success); + static void RunLoadCallbackOnThread( + scoped_refptr<base::TaskRunner> task_runner, + const protocol::PairingRegistry::LoadCallback& callback, + const std::string& pairings_json); + + // Helper methods to load and save the pairing registry. + protocol::PairingRegistry::PairedClients LoadPairings(); + void SavePairings( + const protocol::PairingRegistry::PairedClients& paired_clients); + + // Return the path to the file to use for loading and saving paired clients. + base::FilePath GetRegistryFilePath(); + + // For testing purposes, set the path returned by |GetRegistryFilePath|. + void SetFilenameForTesting(const base::FilePath &filename); + + scoped_refptr<base::TaskRunner> task_runner_; + base::FilePath filename_for_testing_; + + base::WeakPtrFactory<PairingRegistryDelegateLinux> weak_factory_; + + DISALLOW_COPY_AND_ASSIGN(PairingRegistryDelegateLinux); +}; + +} // namespace remoting + +#endif // REMOTING_PROTOCOL_PAIRING_REGISTRY_DELEGATE_LINUX_H_ diff --git a/remoting/host/pairing_registry_delegate_linux_unittest.cc b/remoting/host/pairing_registry_delegate_linux_unittest.cc new file mode 100644 index 0000000..4387f64 --- /dev/null +++ b/remoting/host/pairing_registry_delegate_linux_unittest.cc @@ -0,0 +1,76 @@ +// Copyright 2013 The Chromium Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#include "remoting/host/pairing_registry_delegate_linux.h" + +#include "base/file_util.h" +#include "base/message_loop.h" +#include "base/run_loop.h" +#include "base/task_runner.h" +#include "base/thread_task_runner_handle.h" +#include "base/timer.h" +#include "testing/gtest/include/gtest/gtest.h" + +namespace remoting { + +using protocol::PairingRegistry; + +class PairingRegistryDelegateLinuxTest : public testing::Test { + public: + void SaveComplete(PairingRegistry::Delegate* delegate, + const std::string& expected_json, + bool success) { + EXPECT_TRUE(success); + // Load the pairings again to make sure we get what we've just written. + delegate->Load( + base::Bind(&PairingRegistryDelegateLinuxTest::VerifyLoad, + base::Unretained(this), + expected_json)); + } + + void VerifyLoad(const std::string& expected, + const std::string& actual) { + EXPECT_EQ(actual, expected); + base::MessageLoop::current()->Quit(); + } +}; + +TEST_F(PairingRegistryDelegateLinuxTest, SaveAndLoad) { + base::MessageLoop message_loop; + base::RunLoop run_loop; + + // Create a temporary directory in order to get a unique name and use a + // subdirectory to ensure that the AddPairing method creates the parent + // directory if it doesn't exist. + base::FilePath temp_dir; + file_util::CreateNewTempDirectory("chromoting-test", &temp_dir); + base::FilePath temp_file = temp_dir.Append("dir").Append("registry.json"); + + scoped_refptr<base::TaskRunner> task_runner = + base::ThreadTaskRunnerHandle::Get(); + scoped_ptr<PairingRegistryDelegateLinux> save_delegate( + new PairingRegistryDelegateLinux(task_runner)); + scoped_ptr<PairingRegistryDelegateLinux> load_delegate( + new PairingRegistryDelegateLinux(task_runner)); + save_delegate->SetFilenameForTesting(temp_file); + load_delegate->SetFilenameForTesting(temp_file); + + // Save the pairings, then load them using a different delegate to ensure + // that the test isn't passing due to cached values. Note that the delegate + // doesn't require that the strings it loads and saves are valid JSON, so + // we can simplify the test a bit. + std::string test_data = "test data"; + save_delegate->Save( + test_data, + base::Bind(&PairingRegistryDelegateLinuxTest::SaveComplete, + base::Unretained(this), + load_delegate.get(), + test_data)); + + run_loop.Run(); + + file_util::Delete(temp_dir, true); +}; + +} // namespace remoting diff --git a/remoting/host/pairing_registry_delegate_mac.cc b/remoting/host/pairing_registry_delegate_mac.cc new file mode 100644 index 0000000..602a068 --- /dev/null +++ b/remoting/host/pairing_registry_delegate_mac.cc @@ -0,0 +1,18 @@ +// Copyright 2013 The Chromium Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#include "remoting/host/pairing_registry_delegate.h" + +#include "base/task_runner.h" + +namespace remoting { + +using protocol::PairingRegistry; + +scoped_ptr<PairingRegistry::Delegate> CreatePairingRegistryDelegate( + scoped_refptr<base::TaskRunner> task_runner) { + return scoped_ptr<PairingRegistry::Delegate>(); +} + +} // namespace remoting diff --git a/remoting/host/pairing_registry_delegate_win.cc b/remoting/host/pairing_registry_delegate_win.cc new file mode 100644 index 0000000..602a068 --- /dev/null +++ b/remoting/host/pairing_registry_delegate_win.cc @@ -0,0 +1,18 @@ +// Copyright 2013 The Chromium Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#include "remoting/host/pairing_registry_delegate.h" + +#include "base/task_runner.h" + +namespace remoting { + +using protocol::PairingRegistry; + +scoped_ptr<PairingRegistry::Delegate> CreatePairingRegistryDelegate( + scoped_refptr<base::TaskRunner> task_runner) { + return scoped_ptr<PairingRegistry::Delegate>(); +} + +} // namespace remoting diff --git a/remoting/host/remoting_me2me_host.cc b/remoting/host/remoting_me2me_host.cc index 9df3657..d684a86 100644 --- a/remoting/host/remoting_me2me_host.cc +++ b/remoting/host/remoting_me2me_host.cc @@ -57,6 +57,7 @@ #include "remoting/host/log_to_server.h" #include "remoting/host/logging.h" #include "remoting/host/me2me_desktop_environment.h" +#include "remoting/host/pairing_registry_delegate.h" #include "remoting/host/policy_hack/policy_watcher.h" #include "remoting/host/service_urls.h" #include "remoting/host/session_manager_factory.h" @@ -478,12 +479,9 @@ void HostProcess::CreateAuthenticatorFactory() { // TODO(jamiewalch): Add a pairing registry here once all the code // is committed. - scoped_refptr<remoting::protocol::PairingRegistry> pairing_registry; - //scoped_refptr<protocol::PairingRegistry> pairing_registry( - // new protocol::PairingRegistry( - // scoped_ptr<protocol::PairingRegistry::Delegate>( - // new protocol::NotImplementedPairingRegistryDelegate), - // protocol::PairingRegistry::PairedClients())); + scoped_refptr<remoting::protocol::PairingRegistry> pairing_registry( + new remoting::protocol::PairingRegistry( + CreatePairingRegistryDelegate(NULL))); scoped_ptr<protocol::AuthenticatorFactory> factory; 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 diff --git a/remoting/remoting.gyp b/remoting/remoting.gyp index f442e6e..6a3f099 100644 --- a/remoting/remoting.gyp +++ b/remoting/remoting.gyp @@ -413,6 +413,11 @@ 'host/me2me_desktop_environment.h', 'host/mouse_clamping_filter.cc', 'host/mouse_clamping_filter.h', + 'host/pairing_registry_delegate.h', + 'host/pairing_registry_delegate_linux.cc', + 'host/pairing_registry_delegate_linux.h', + 'host/pairing_registry_delegate_mac.cc', + 'host/pairing_registry_delegate_win.cc', 'host/pam_authorization_factory_posix.cc', 'host/pam_authorization_factory_posix.h', 'host/pin_hash.cc', @@ -2647,6 +2652,7 @@ 'host/linux/x_server_clipboard_unittest.cc', 'host/local_input_monitor_unittest.cc', 'host/log_to_server_unittest.cc', + 'host/pairing_registry_delegate_linux_unittest.cc', 'host/pin_hash_unittest.cc', 'host/policy_hack/fake_policy_watcher.cc', 'host/policy_hack/fake_policy_watcher.h', |