diff options
author | pastarmovj@chromium.org <pastarmovj@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2011-11-22 14:37:19 +0000 |
---|---|---|
committer | pastarmovj@chromium.org <pastarmovj@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2011-11-22 14:37:19 +0000 |
commit | 4cdc26a740d1e00bee47bf820d5a1ba7b6d768b6 (patch) | |
tree | daedafed9abd5afa19e843b45a3b4eb852901a65 | |
parent | b97f3c2e2c5ba4cdce59124efd6d3a34b53565f4 (diff) | |
download | chromium_src-4cdc26a740d1e00bee47bf820d5a1ba7b6d768b6.zip chromium_src-4cdc26a740d1e00bee47bf820d5a1ba7b6d768b6.tar.gz chromium_src-4cdc26a740d1e00bee47bf820d5a1ba7b6d768b6.tar.bz2 |
Removed whitelist special ops.
BUG=chromium-os:14054
TEST=unit_tests:*Signed*, browser_tests
Review URL: http://codereview.chromium.org/8163011
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@111154 0039d316-1c4b-4281-b951-d872f2087c98
10 files changed, 24 insertions, 686 deletions
diff --git a/chrome/browser/chromeos/login/mock_signed_settings_helper.h b/chrome/browser/chromeos/login/mock_signed_settings_helper.h index 5a5b551..628b552 100644 --- a/chrome/browser/chromeos/login/mock_signed_settings_helper.h +++ b/chrome/browser/chromeos/login/mock_signed_settings_helper.h @@ -17,10 +17,6 @@ class MockSignedSettingsHelper : public chromeos::SignedSettingsHelper { MockSignedSettingsHelper(); virtual ~MockSignedSettingsHelper(); - MOCK_METHOD2(StartCheckWhitelistOp, - void(const std::string& email, Callback*)); - MOCK_METHOD3(StartWhitelistOp, - void(const std::string&, bool, Callback*)); MOCK_METHOD3(StartStorePropertyOp, void(const std::string&, const base::Value&, Callback*)); MOCK_METHOD2(StartRetrieveProperty, diff --git a/chrome/browser/chromeos/login/signed_settings.cc b/chrome/browser/chromeos/login/signed_settings.cc index 6fa426c..31921cf 100644 --- a/chrome/browser/chromeos/login/signed_settings.cc +++ b/chrome/browser/chromeos/login/signed_settings.cc @@ -83,83 +83,6 @@ SignedSettings::ReturnCode SignedSettings::MapKeyOpCode( KEY_UNAVAILABLE : BAD_SIGNATURE); } -// static -bool SignedSettings::EnumerateWhitelist(std::vector<std::string>* whitelisted) { - OwnershipService* service = OwnershipService::GetSharedInstance(); - if (!service->has_cached_policy()) - return false; - em::ChromeDeviceSettingsProto pol; - pol.ParseFromString(service->cached_policy().policy_value()); - if (!pol.has_user_whitelist()) - return false; - - const RepeatedPtrField<std::string>& whitelist = - pol.user_whitelist().user_whitelist(); - for (RepeatedPtrField<std::string>::const_iterator it = whitelist.begin(); - it != whitelist.end(); - ++it) { - whitelisted->push_back(*it); - } - return true; -} - -class CheckWhitelistOp : public SignedSettings { - public: - CheckWhitelistOp(const std::string& email, - SignedSettings::Delegate<bool>* d); - virtual ~CheckWhitelistOp(); - void Execute(); - void Fail(SignedSettings::ReturnCode code); - void Succeed(bool value); - // Implementation of OwnerManager::Delegate - void OnKeyOpComplete(const OwnerManager::KeyOpCode return_code, - const std::vector<uint8>& payload); - - private: - bool LookUpInPolicy(const std::string& email); - // Always call d_->OnSettingOpCompleted() via this call. - // It guarantees that the callback will not be triggered until _after_ - // Execute() returns, which is implicitly assumed by SignedSettingsHelper - // in some cases. - void PerformCallback(SignedSettings::ReturnCode code, bool value); - - const std::string email_; - SignedSettings::Delegate<bool>* d_; -}; - -class WhitelistOp : public SignedSettings, - public SignedSettings::Delegate<bool> { - public: - WhitelistOp(const std::string& email, - bool add_to_whitelist, - SignedSettings::Delegate<bool>* d); - virtual ~WhitelistOp(); - void Execute(); - void Fail(SignedSettings::ReturnCode code); - void Succeed(bool value); - // Implementation of OwnerManager::Delegate - void OnKeyOpComplete(const OwnerManager::KeyOpCode return_code, - const std::vector<uint8>& payload); - // Implementation of SignedSettings::Delegate - void OnSettingsOpCompleted(ReturnCode code, bool value); - - private: - void ModifyWhitelist(const std::string& email, - bool add_to_whitelist, - em::UserWhitelistProto* whitelist_proto); - // Always call d_->OnSettingOpCompleted() via this call. - // It guarantees that the callback will not be triggered until _after_ - // Execute() returns, which is implicitly assumed by SignedSettingsHelper - // in some cases. - void PerformCallback(SignedSettings::ReturnCode code, bool value); - - const std::string email_; - const bool add_to_whitelist_; - SignedSettings::Delegate<bool>* d_; - em::PolicyFetchResponse to_store_; - scoped_refptr<SignedSettings> store_op_; -}; - class StorePropertyOp : public SignedSettings, public SignedSettings::Delegate<bool> { public: @@ -272,25 +195,6 @@ class RetrievePolicyOp : public SignedSettings { }; // static -SignedSettings* SignedSettings::CreateCheckWhitelistOp( - const std::string& email, - SignedSettings::Delegate<bool>* d) { - DCHECK(d != NULL); - return new CheckWhitelistOp(Authenticator::Canonicalize(email), d); -} - -// static -SignedSettings* SignedSettings::CreateWhitelistOp( - const std::string& email, - bool add_to_whitelist, - SignedSettings::Delegate<bool>* d) { - DCHECK(d != NULL); - return new WhitelistOp(Authenticator::Canonicalize(email), - add_to_whitelist, - d); -} - -// static SignedSettings* SignedSettings::CreateStorePropertyOp( const std::string& name, const base::Value& value, @@ -323,200 +227,6 @@ SignedSettings* SignedSettings::CreateRetrievePolicyOp( return new RetrievePolicyOp(d); } -CheckWhitelistOp::CheckWhitelistOp(const std::string& email, - SignedSettings::Delegate<bool>* d) - : email_(email), - d_(d) { -} - -CheckWhitelistOp::~CheckWhitelistOp() {} - -void CheckWhitelistOp::Execute() { - std::vector<uint8> sig; - std::string email_to_check = email_; - if (!service_->has_cached_policy()) { - TryToFetchPolicyAndCallBack(); - return; - } - if (LookUpInPolicy(email_to_check)) { - VLOG(2) << "Whitelist check was successful for " << email_to_check; - Succeed(true); - return; - } - // If the exact match was not found try to match against a wildcard entry - // where the domain only matches (e.g. *@example.com). In theory we should - // always have correctly formated mail address here but a little precaution - // does no harm. - if (email_.find('@') != std::string::npos) { - email_to_check = std::string("*").append(email_.substr(email_.find('@'))); - if (LookUpInPolicy(email_to_check)) { - VLOG(2) << "Whitelist check was successful for " << email_to_check; - Succeed(true); - return; - } - } - Fail(NOT_FOUND); - return; -} - -void CheckWhitelistOp::Fail(SignedSettings::ReturnCode code) { - BrowserThread::PostTask( - BrowserThread::UI, FROM_HERE, - base::Bind(&CheckWhitelistOp::PerformCallback, this, code, false)); -} - -void CheckWhitelistOp::Succeed(bool value) { - BrowserThread::PostTask( - BrowserThread::UI, FROM_HERE, - base::Bind(&CheckWhitelistOp::PerformCallback, this, SUCCESS, value)); -} - -void CheckWhitelistOp::OnKeyOpComplete( - const OwnerManager::KeyOpCode return_code, - const std::vector<uint8>& payload) { - NOTREACHED(); - // Ensure we're on the UI thread, due to the need to send DBus traffic. - if (!BrowserThread::CurrentlyOn(BrowserThread::UI)) { - BrowserThread::PostTask( - BrowserThread::UI, FROM_HERE, - base::Bind(&CheckWhitelistOp::OnKeyOpComplete, this, return_code, - payload)); - return; - } - if (return_code == OwnerManager::SUCCESS) { - VLOG(2) << "Whitelist check was successful."; - Succeed(true); - } else { - VLOG(2) << "Whitelist check failed."; - Fail(SignedSettings::MapKeyOpCode(return_code)); - } -} - -bool CheckWhitelistOp::LookUpInPolicy(const std::string& email) { - em::ChromeDeviceSettingsProto pol; - pol.ParseFromString(service_->cached_policy().policy_value()); - if (!pol.has_user_whitelist()) - return false; - - const RepeatedPtrField<std::string>& whitelist = - pol.user_whitelist().user_whitelist(); - for (RepeatedPtrField<std::string>::const_iterator it = whitelist.begin(); - it != whitelist.end(); - ++it) { - if (email == *it) - return true; - } - return false; -} - -void CheckWhitelistOp::PerformCallback(SignedSettings::ReturnCode code, - bool value) { - d_->OnSettingsOpCompleted(code, value); -} - -WhitelistOp::WhitelistOp(const std::string& email, - bool add_to_whitelist, - SignedSettings::Delegate<bool>* d) - : email_(email), - add_to_whitelist_(add_to_whitelist), - d_(d) { -} - -WhitelistOp::~WhitelistOp() {} - -void WhitelistOp::Execute() { - if (!service_->has_cached_policy()) { - TryToFetchPolicyAndCallBack(); - return; - } - em::PolicyData to_sign; - to_sign.CheckTypeAndMergeFrom(service_->cached_policy()); - em::ChromeDeviceSettingsProto pol; - pol.ParseFromString(to_sign.policy_value()); - em::UserWhitelistProto* whitelist_proto = pol.mutable_user_whitelist(); - ModifyWhitelist(email_, add_to_whitelist_, whitelist_proto); - to_sign.set_policy_value(pol.SerializeAsString()); - to_store_.set_policy_data(to_sign.SerializeAsString()); - service_->StartSigningAttempt(to_store_.policy_data(), this); -} - -void WhitelistOp::Fail(SignedSettings::ReturnCode code) { - BrowserThread::PostTask( - BrowserThread::UI, FROM_HERE, - base::Bind(&WhitelistOp::PerformCallback, this, code, false)); -} - -void WhitelistOp::Succeed(bool value) { - BrowserThread::PostTask( - BrowserThread::UI, FROM_HERE, - base::Bind(&WhitelistOp::PerformCallback, this, SUCCESS, value)); -} - -void WhitelistOp::OnKeyOpComplete(const OwnerManager::KeyOpCode return_code, - const std::vector<uint8>& sig) { - // Ensure we're on the UI thread, due to the need to send DBus traffic. - if (!BrowserThread::CurrentlyOn(BrowserThread::UI)) { - BrowserThread::PostTask( - BrowserThread::UI, FROM_HERE, - base::Bind(&WhitelistOp::OnKeyOpComplete, this, return_code, sig)); - return; - } - VLOG(2) << "WhitelistOp::OnKeyOpComplete return_code = " << return_code; - // Now, sure we're on the UI thread. - if (return_code == OwnerManager::SUCCESS) { - to_store_.set_policy_data_signature( - std::string(reinterpret_cast<const char*>(&sig[0]), sig.size())); - store_op_ = CreateStorePolicyOp(&to_store_, this); - // d_->OnSettingsOpCompleted() will be called by this call. - store_op_->Execute(); - } else { - Fail(SignedSettings::MapKeyOpCode(return_code)); - } -} - -void WhitelistOp::OnSettingsOpCompleted(ReturnCode code, bool value) { - if (value && to_store_.has_policy_data()) { - em::PolicyData poldata; - poldata.ParseFromString(to_store_.policy_data()); - service_->set_cached_policy(poldata); - Succeed(value); - return; - } - Fail(NOT_FOUND); -} - -void WhitelistOp::ModifyWhitelist(const std::string& email, - bool add_to_whitelist, - em::UserWhitelistProto* whitelist_proto) { - int i = 0; - const RepeatedPtrField<string>& whitelist = whitelist_proto->user_whitelist(); - for (RepeatedPtrField<string>::const_iterator it = whitelist.begin(); - it != whitelist.end(); - ++it, ++i) { - if (email == *it) - break; - } - // |i| contains the index of |email|, if it is in |whitelist|. - if (add_to_whitelist) { - if (i >= whitelist.size()) // |email| was not in |whitelist|, we must add. - whitelist_proto->add_user_whitelist(email); - return; - } else { - if (i < whitelist.size()) { // |email| was in |whitelist|, we must remove. - RepeatedPtrField<string>* change_list = - whitelist_proto->mutable_user_whitelist(); - change_list->SwapElements(i, whitelist.size() - 1); // Move to end. - change_list->RemoveLast(); - } - return; - } - LOG(WARNING) << "Whitelist modification no-op: " << email; -} - -void WhitelistOp::PerformCallback(SignedSettings::ReturnCode code, bool value) { - d_->OnSettingsOpCompleted(code, value); -} - StorePropertyOp::StorePropertyOp(const std::string& name, const base::Value& value, SignedSettings::Delegate<bool>* d) diff --git a/chrome/browser/chromeos/login/signed_settings.h b/chrome/browser/chromeos/login/signed_settings.h index a483962..43e6f05 100644 --- a/chrome/browser/chromeos/login/signed_settings.h +++ b/chrome/browser/chromeos/login/signed_settings.h @@ -64,16 +64,6 @@ class SignedSettings : public base::RefCountedThreadSafe<SignedSettings>, SignedSettings(); virtual ~SignedSettings(); - // These are both "whitelist" operations, and only one instance of - // one type can be in flight at a time. - static SignedSettings* CreateCheckWhitelistOp( - const std::string& email, - SignedSettings::Delegate<bool>* d); - - static SignedSettings* CreateWhitelistOp(const std::string& email, - bool add_to_whitelist, - SignedSettings::Delegate<bool>* d); - // These are both "property" operations, and only one instance of // one type can be in flight at a time. static SignedSettings* CreateStorePropertyOp( @@ -94,8 +84,6 @@ class SignedSettings : public base::RefCountedThreadSafe<SignedSettings>, static SignedSettings* CreateRetrievePolicyOp( SignedSettings::Delegate<const em::PolicyFetchResponse&>* d); - static bool EnumerateWhitelist(std::vector<std::string>* whitelisted); - static ReturnCode MapKeyOpCode(OwnerManager::KeyOpCode code); virtual void Execute() = 0; diff --git a/chrome/browser/chromeos/login/signed_settings_helper.cc b/chrome/browser/chromeos/login/signed_settings_helper.cc index e5ea6fb..e61cdb1 100644 --- a/chrome/browser/chromeos/login/signed_settings_helper.cc +++ b/chrome/browser/chromeos/login/signed_settings_helper.cc @@ -101,72 +101,6 @@ class OpContext { SignedSettingsHelper::Callback* callback_; }; -class WhitelistOpContext : public SignedSettings::Delegate<bool>, - public OpContext { - public: - enum Type { - CHECK, - ADD, - REMOVE, - }; - - WhitelistOpContext(Type type, - const std::string& email, - SignedSettingsHelper::Callback* callback, - OpContext::Delegate* delegate) - : OpContext(callback, delegate), - type_(type), - email_(email) { - } - - // chromeos::SignedSettings::Delegate implementation - virtual void OnSettingsOpCompleted(SignedSettings::ReturnCode code, - bool value) OVERRIDE { - if (callback_) { - switch (type_) { - case CHECK: - callback_->OnCheckWhitelistCompleted(code, email_); - break; - case ADD: - callback_->OnWhitelistCompleted(code, email_); - break; - case REMOVE: - callback_->OnUnwhitelistCompleted(code, email_); - break; - default: - LOG(ERROR) << "Unknown WhitelistOpContext type " << type_; - break; - } - } - OnOpCompleted(); - } - - protected: - // OpContext implemenetation - virtual void CreateOp() OVERRIDE { - switch (type_) { - case CHECK: - op_ = SignedSettings::CreateCheckWhitelistOp(email_, this); - break; - case ADD: - op_ = SignedSettings::CreateWhitelistOp(email_, true, this); - break; - case REMOVE: - op_ = SignedSettings::CreateWhitelistOp(email_, false, this); - break; - default: - LOG(ERROR) << "Unknown WhitelistOpContext type " << type_; - break; - } - } - - private: - Type type_; - std::string email_; - - DISALLOW_COPY_AND_ASSIGN(WhitelistOpContext); -}; - class StorePropertyOpContext : public SignedSettings::Delegate<bool>, public OpContext { public: @@ -298,11 +232,6 @@ class SignedSettingsHelperImpl : public SignedSettingsHelper, public OpContext::Delegate { public: // SignedSettingsHelper implementation - virtual void StartCheckWhitelistOp(const std::string& email, - Callback* callback) OVERRIDE; - virtual void StartWhitelistOp(const std::string& email, - bool add_to_whitelist, - Callback* callback) OVERRIDE; virtual void StartStorePropertyOp(const std::string& name, const base::Value& value, Callback* callback) OVERRIDE; @@ -345,27 +274,6 @@ SignedSettingsHelperImpl::~SignedSettingsHelperImpl() { } } -void SignedSettingsHelperImpl::StartCheckWhitelistOp( - const std::string&email, - SignedSettingsHelper::Callback* callback) { - AddOpContext(new WhitelistOpContext( - WhitelistOpContext::CHECK, - email, - callback, - this)); -} - -void SignedSettingsHelperImpl::StartWhitelistOp( - const std::string&email, - bool add_to_whitelist, - SignedSettingsHelper::Callback* callback) { - AddOpContext(new WhitelistOpContext( - add_to_whitelist ? WhitelistOpContext::ADD : WhitelistOpContext::REMOVE, - email, - callback, - this)); -} - void SignedSettingsHelperImpl::StartStorePropertyOp( const std::string& name, const base::Value& value, diff --git a/chrome/browser/chromeos/login/signed_settings_helper.h b/chrome/browser/chromeos/login/signed_settings_helper.h index 1d1411c..61ed873 100644 --- a/chrome/browser/chromeos/login/signed_settings_helper.h +++ b/chrome/browser/chromeos/login/signed_settings_helper.h @@ -28,20 +28,6 @@ class SignedSettingsHelper { public: class Callback { public: - // Callback of CheckWhitelistOp. |success| indicates whether the op succeeds - // or not. |email| is the email that is checked against. - virtual void OnCheckWhitelistCompleted( - SignedSettings::ReturnCode code, - const std::string& email) {} - - // Callback of WhitelistOp that adds |email| to the whitelist. - virtual void OnWhitelistCompleted( - SignedSettings::ReturnCode code, const std::string& email) {} - - // Callback of WhitelistOp that removes |email| to the whitelist. - virtual void OnUnwhitelistCompleted( - SignedSettings::ReturnCode code, const std::string& email) {} - // Callback of StorePropertyOp. virtual void OnStorePropertyCompleted( SignedSettings::ReturnCode code, @@ -68,11 +54,6 @@ class SignedSettingsHelper { static SignedSettingsHelper* Get(); // Functions to start signed settings ops. - virtual void StartCheckWhitelistOp(const std::string& email, - Callback* callback) = 0; - virtual void StartWhitelistOp(const std::string& email, - bool add_to_whitelist, - Callback* callback) = 0; virtual void StartStorePropertyOp(const std::string& name, const base::Value& value, Callback* callback) = 0; diff --git a/chrome/browser/chromeos/login/signed_settings_helper_unittest.cc b/chrome/browser/chromeos/login/signed_settings_helper_unittest.cc index 7b5ad5b..a808d77 100644 --- a/chrome/browser/chromeos/login/signed_settings_helper_unittest.cc +++ b/chrome/browser/chromeos/login/signed_settings_helper_unittest.cc @@ -32,12 +32,8 @@ namespace chromeos { class MockSignedSettingsHelperCallback : public SignedSettingsHelper::Callback { public: - MOCK_METHOD2(OnCheckWhitelistCompleted, void( - SignedSettings::ReturnCode code, const std::string& email)); - MOCK_METHOD2(OnWhitelistCompleted, void( - SignedSettings::ReturnCode code, const std::string& email)); - MOCK_METHOD2(OnUnwhitelistCompleted, void( - SignedSettings::ReturnCode code, const std::string& email)); + virtual ~MockSignedSettingsHelperCallback() {} + MOCK_METHOD3(OnStorePropertyCompleted, void( SignedSettings::ReturnCode code, const std::string& name, @@ -119,42 +115,27 @@ TEST_F(SignedSettingsHelperTest, SerializedOps) { .Times(2) .WillRepeatedly(Return(OwnershipService::OWNERSHIP_TAKEN)); EXPECT_CALL(m_, has_cached_policy()) - .Times(5) + .Times(2) .WillRepeatedly(Return(true)); em::PolicyData fake_pol = BuildPolicyData(); EXPECT_CALL(m_, cached_policy()) - .Times(5) + .Times(2) .WillRepeatedly(ReturnRef(fake_pol)); EXPECT_CALL(m_, set_cached_policy(A<const em::PolicyData&>())) - .Times(3) + .Times(1) .WillRepeatedly(SaveArg<0>(&fake_pol)); InSequence s; EXPECT_CALL(m_, StartSigningAttempt(_, A<OwnerManager::Delegate*>())) .WillOnce(WithArg<1>(Invoke(&SignedSettingsHelperTest::OnKeyOpComplete))); - EXPECT_CALL(cb, OnWhitelistCompleted(SignedSettings::SUCCESS, _)) - .Times(1); - - EXPECT_CALL(cb, OnCheckWhitelistCompleted(SignedSettings::SUCCESS, _)) - .Times(1); - - EXPECT_CALL(m_, StartSigningAttempt(_, A<OwnerManager::Delegate*>())) - .WillOnce(WithArg<1>(Invoke(&SignedSettingsHelperTest::OnKeyOpComplete))); - EXPECT_CALL(cb, OnUnwhitelistCompleted(SignedSettings::SUCCESS, _)) - .Times(1); - - EXPECT_CALL(m_, StartSigningAttempt(_, A<OwnerManager::Delegate*>())) - .WillOnce(WithArg<1>(Invoke(&SignedSettingsHelperTest::OnKeyOpComplete))); EXPECT_CALL(cb, OnStorePropertyCompleted(SignedSettings::SUCCESS, _, _)) .Times(1); EXPECT_CALL(cb, OnRetrievePropertyCompleted(SignedSettings::SUCCESS, _, _)) .Times(1); - pending_ops_ = 5; - SignedSettingsHelper::Get()->StartWhitelistOp(fake_email_, true, &cb); - SignedSettingsHelper::Get()->StartCheckWhitelistOp(fake_email_, &cb); - SignedSettingsHelper::Get()->StartWhitelistOp(fake_email_, false, &cb); + + pending_ops_ = 2; SignedSettingsHelper::Get()->StartStorePropertyOp(fake_prop_, fake_value_, &cb); SignedSettingsHelper::Get()->StartRetrieveProperty(fake_prop_, &cb); @@ -166,53 +147,34 @@ TEST_F(SignedSettingsHelperTest, CanceledOps) { MockSignedSettingsHelperCallback cb; EXPECT_CALL(m_, GetStatus(_)) - .Times(2) + .Times(3) .WillRepeatedly(Return(OwnershipService::OWNERSHIP_TAKEN)); EXPECT_CALL(m_, has_cached_policy()) - .Times(6) + .Times(3) .WillRepeatedly(Return(true)); em::PolicyData fake_pol = BuildPolicyData(); EXPECT_CALL(m_, cached_policy()) - .Times(7) + .Times(3) .WillRepeatedly(ReturnRef(fake_pol)); EXPECT_CALL(m_, set_cached_policy(A<const em::PolicyData&>())) - .Times(3) + .Times(1) .WillRepeatedly(SaveArg<0>(&fake_pol)); InSequence s; - EXPECT_CALL(m_, StartSigningAttempt(_, A<OwnerManager::Delegate*>())) - .WillOnce(WithArg<1>(Invoke(&SignedSettingsHelperTest::OnKeyOpComplete))); - EXPECT_CALL(cb, OnWhitelistCompleted(SignedSettings::SUCCESS, _)) - .Times(1); - - EXPECT_CALL(cb, OnCheckWhitelistCompleted(SignedSettings::SUCCESS, _)) - .Times(1); - - EXPECT_CALL(m_, StartSigningAttempt(_, A<OwnerManager::Delegate*>())) - .WillOnce(WithArg<1>(Invoke(&SignedSettingsHelperTest::OnKeyOpComplete))); - EXPECT_CALL(cb, OnUnwhitelistCompleted(SignedSettings::SUCCESS, _)) - .Times(1); - - // CheckWhitelistOp for cb_to_be_canceled still gets executed but callback + // RetrievePropertyOp for cb_to_be_canceled still gets executed but callback // does not happen. - EXPECT_CALL(m_, StartSigningAttempt(_, A<OwnerManager::Delegate*>())) .WillOnce(WithArg<1>(Invoke(&SignedSettingsHelperTest::OnKeyOpComplete))); EXPECT_CALL(cb, OnStorePropertyCompleted(SignedSettings::SUCCESS, _, _)) .Times(1); - EXPECT_CALL(cb, OnRetrievePropertyCompleted(SignedSettings::SUCCESS, _, _)) .Times(1); - pending_ops_ = 6; - SignedSettingsHelper::Get()->StartWhitelistOp(fake_email_, true, &cb); - SignedSettingsHelper::Get()->StartCheckWhitelistOp(fake_email_, &cb); - SignedSettingsHelper::Get()->StartWhitelistOp(fake_email_, false, &cb); - + pending_ops_ = 3; MockSignedSettingsHelperCallback cb_to_be_canceled; - SignedSettingsHelper::Get()->StartCheckWhitelistOp(fake_email_, - &cb_to_be_canceled); + SignedSettingsHelper::Get()->StartRetrieveProperty(fake_prop_, + &cb_to_be_canceled); SignedSettingsHelper::Get()->CancelCallback(&cb_to_be_canceled); SignedSettingsHelper::Get()->StartStorePropertyOp(fake_prop_, fake_value_, diff --git a/chrome/browser/chromeos/login/signed_settings_unittest.cc b/chrome/browser/chromeos/login/signed_settings_unittest.cc index e6ca1f1..36896cd 100644 --- a/chrome/browser/chromeos/login/signed_settings_unittest.cc +++ b/chrome/browser/chromeos/login/signed_settings_unittest.cc @@ -177,39 +177,6 @@ class SignedSettingsTest : public testing::Test { poldata->set_policy_value(pol.SerializeAsString()); } - bool CheckWhitelist(const std::string& email, const em::PolicyData& poldata) { - if (!poldata.has_policy_value()) - return false; - em::ChromeDeviceSettingsProto pol; - pol.ParseFromString(poldata.policy_value()); - if (!pol.has_user_whitelist()) - return false; - - const RepeatedPtrField<std::string>& whitelist = - pol.user_whitelist().user_whitelist(); - for (RepeatedPtrField<std::string>::const_iterator it = whitelist.begin(); - it != whitelist.end(); - ++it) { - if (email == *it) - return true; - } - return false; - } - - void ExpectWhitelistOp(SignedSettings* s, - em::PolicyData* fake_pol, - em::PolicyData* out_pol) { - mock_service(s, &m_); - EXPECT_CALL(m_, StartSigningAttempt(_, _)) - .Times(1); - EXPECT_CALL(m_, has_cached_policy()) - .WillOnce(Return(true)); - EXPECT_CALL(m_, cached_policy()) - .WillOnce(ReturnRef(*fake_pol)); - EXPECT_CALL(m_, set_cached_policy(A<const em::PolicyData&>())) - .WillOnce(SaveArg<0>(out_pol)); - } - void FailingStorePropertyOp(const OwnerManager::KeyOpCode return_code) { NormalDelegate<bool> d(false); scoped_refptr<SignedSettings> s( @@ -314,160 +281,6 @@ ACTION_P(Retrieve, policy_blob) { arg0.Run(policy_blob); } ACTION_P(Store, success) { arg1.Run(success); } ACTION_P(FinishKeyOp, s) { arg2->OnKeyOpComplete(OwnerManager::SUCCESS, s); } -TEST_F(SignedSettingsTest, CheckWhitelist) { - NormalDelegate<bool> d(true); - d.expect_success(); - scoped_refptr<SignedSettings> s( - SignedSettings::CreateCheckWhitelistOp(fake_email_, &d)); - - mock_service(s.get(), &m_); - EXPECT_CALL(m_, has_cached_policy()) - .WillOnce(Return(true)); - - std::vector<std::string> whitelist(1, fake_email_); - whitelist.push_back(fake_email_ + "m"); - em::PolicyData fake_pol = BuildPolicyData(whitelist); - EXPECT_CALL(m_, cached_policy()) - .WillOnce(ReturnRef(fake_pol)); - - s->Execute(); - message_loop_.RunAllPending(); -} - -TEST_F(SignedSettingsTest, CheckWhitelistWildcards) { - NormalDelegate<bool> d(true); - d.expect_success(); - scoped_refptr<SignedSettings> s( - SignedSettings::CreateCheckWhitelistOp(fake_email_, &d)); - - mock_service(s.get(), &m_); - EXPECT_CALL(m_, has_cached_policy()) - .WillOnce(Return(true)); - - std::vector<std::string> whitelist(1, fake_domain_); - whitelist.push_back(fake_email_ + "m"); - em::PolicyData fake_pol = BuildPolicyData(whitelist); - EXPECT_CALL(m_, cached_policy()) - .WillOnce(ReturnRef(fake_pol)) - .WillOnce(ReturnRef(fake_pol)); - - s->Execute(); - message_loop_.RunAllPending(); -} - -TEST_F(SignedSettingsTest, CheckWhitelistNotFound) { - NormalDelegate<bool> d(true); - scoped_refptr<SignedSettings> s( - SignedSettings::CreateCheckWhitelistOp(fake_email_, &d)); - d.expect_failure(SignedSettings::NOT_FOUND); - - mock_service(s.get(), &m_); - EXPECT_CALL(m_, has_cached_policy()) - .WillOnce(Return(true)); - - std::vector<std::string> whitelist(1, fake_email_ + "m"); - em::PolicyData fake_pol = BuildPolicyData(whitelist); - EXPECT_CALL(m_, cached_policy()) - .WillOnce(ReturnRef(fake_pol)) - .WillOnce(ReturnRef(fake_pol)); - - s->Execute(); - message_loop_.RunAllPending(); -} - -TEST_F(SignedSettingsTest, Whitelist) { - NormalDelegate<bool> d(true); - d.expect_success(); - scoped_refptr<SignedSettings> s( - SignedSettings::CreateWhitelistOp(fake_email_, true, &d)); - em::PolicyData in_pol = BuildPolicyData(std::vector<std::string>()); - em::PolicyData out_pol; - ExpectWhitelistOp(s.get(), &in_pol, &out_pol); - - MockSessionManagerClient* client = - mock_dbus_thread_manager_->mock_session_manager_client(); - EXPECT_CALL(*client, StorePolicy(_, _)) - .WillOnce(Store(true)) - .RetiresOnSaturation(); - - s->Execute(); - s->OnKeyOpComplete(OwnerManager::SUCCESS, std::vector<uint8>()); - message_loop_.RunAllPending(); - - ASSERT_TRUE(CheckWhitelist(fake_email_, out_pol)); -} - -TEST_F(SignedSettingsTest, AddToExistingWhitelist) { - NormalDelegate<bool> d(true); - d.expect_success(); - scoped_refptr<SignedSettings> s( - SignedSettings::CreateWhitelistOp(fake_email_, true, &d)); - em::PolicyData in_pol = - BuildPolicyData(std::vector<std::string>(1, fake_domain_)); - em::PolicyData out_pol; - ExpectWhitelistOp(s.get(), &in_pol, &out_pol); - - MockSessionManagerClient* client = - mock_dbus_thread_manager_->mock_session_manager_client(); - EXPECT_CALL(*client, StorePolicy(_, _)) - .WillOnce(Store(true)) - .RetiresOnSaturation(); - - s->Execute(); - s->OnKeyOpComplete(OwnerManager::SUCCESS, std::vector<uint8>()); - message_loop_.RunAllPending(); - - ASSERT_TRUE(CheckWhitelist(fake_email_, out_pol)); -} - -TEST_F(SignedSettingsTest, Unwhitelist) { - NormalDelegate<bool> d(true); - d.expect_success(); - scoped_refptr<SignedSettings> s( - SignedSettings::CreateWhitelistOp(fake_email_, false, &d)); - em::PolicyData in_pol = - BuildPolicyData(std::vector<std::string>(1, fake_email_)); - em::PolicyData out_pol; - ExpectWhitelistOp(s.get(), &in_pol, &out_pol); - - MockSessionManagerClient* client = - mock_dbus_thread_manager_->mock_session_manager_client(); - EXPECT_CALL(*client, StorePolicy(_, _)) - .WillOnce(Store(true)) - .RetiresOnSaturation(); - - s->Execute(); - s->OnKeyOpComplete(OwnerManager::SUCCESS, std::vector<uint8>()); - message_loop_.RunAllPending(); - - ASSERT_FALSE(CheckWhitelist(fake_email_, out_pol)); -} - -TEST_F(SignedSettingsTest, RemoveFromExistingWhitelist) { - NormalDelegate<bool> d(true); - d.expect_success(); - scoped_refptr<SignedSettings> s( - SignedSettings::CreateWhitelistOp(fake_email_, false, &d)); - std::vector<std::string> whitelist(1, fake_domain_); - whitelist.push_back(fake_email_); - whitelist.push_back(fake_email_ + "m"); - em::PolicyData in_pol = BuildPolicyData(whitelist); - em::PolicyData out_pol; - ExpectWhitelistOp(s.get(), &in_pol, &out_pol); - - MockSessionManagerClient* client = - mock_dbus_thread_manager_->mock_session_manager_client(); - EXPECT_CALL(*client, StorePolicy(_, _)) - .WillOnce(Store(true)) - .RetiresOnSaturation(); - - s->Execute(); - s->OnKeyOpComplete(OwnerManager::SUCCESS, std::vector<uint8>()); - message_loop_.RunAllPending(); - - ASSERT_FALSE(CheckWhitelist(fake_email_, out_pol)); -} - TEST_F(SignedSettingsTest, StoreProperty) { NormalDelegate<bool> d(true); d.expect_success(); diff --git a/chrome/browser/chromeos/user_cros_settings_provider.cc b/chrome/browser/chromeos/user_cros_settings_provider.cc index fff561f..2eeb77a 100644 --- a/chrome/browser/chromeos/user_cros_settings_provider.cc +++ b/chrome/browser/chromeos/user_cros_settings_provider.cc @@ -534,28 +534,6 @@ bool UserCrosSettingsProvider::HandlesSetting(const std::string& path) const { } // static -void UserCrosSettingsProvider::WhitelistUser(const std::string& email) { - SignedSettingsHelper::Get()->StartWhitelistOp( - email, true, UserCrosSettingsTrust::GetInstance()); - PrefService* prefs = g_browser_process->local_state(); - ListPrefUpdate cached_whitelist_update(prefs, kAccountsPrefUsers); - cached_whitelist_update->Append(Value::CreateStringValue(email)); - prefs->ScheduleSavePersistentPrefs(); -} - -// static -void UserCrosSettingsProvider::UnwhitelistUser(const std::string& email) { - SignedSettingsHelper::Get()->StartWhitelistOp( - email, false, UserCrosSettingsTrust::GetInstance()); - - PrefService* prefs = g_browser_process->local_state(); - ListPrefUpdate cached_whitelist_update(prefs, kAccountsPrefUsers); - StringValue email_value(email); - if (cached_whitelist_update->Remove(email_value, NULL)) - prefs->ScheduleSavePersistentPrefs(); -} - -// static void UserCrosSettingsProvider::UpdateCachedOwner(const std::string& email) { base::StringValue email_value(email); UpdateCache(kDeviceOwner, &email_value, USE_VALUE_SUPPLIED); diff --git a/chrome/browser/chromeos/user_cros_settings_provider.h b/chrome/browser/chromeos/user_cros_settings_provider.h index cd4ae1f..798f2fd 100644 --- a/chrome/browser/chromeos/user_cros_settings_provider.h +++ b/chrome/browser/chromeos/user_cros_settings_provider.h @@ -41,9 +41,6 @@ class UserCrosSettingsProvider : public CrosSettingsProvider { const base::Closure& callback) const OVERRIDE; virtual bool HandlesSetting(const std::string& path) const OVERRIDE; - static void WhitelistUser(const std::string& email); - static void UnwhitelistUser(const std::string& email); - // Updates cached value of the owner. static void UpdateCachedOwner(const std::string& email); diff --git a/chrome/browser/ui/webui/options/chromeos/accounts_options_handler.cc b/chrome/browser/ui/webui/options/chromeos/accounts_options_handler.cc index 00cd210..41550f5 100644 --- a/chrome/browser/ui/webui/options/chromeos/accounts_options_handler.cc +++ b/chrome/browser/ui/webui/options/chromeos/accounts_options_handler.cc @@ -84,8 +84,10 @@ void AccountsOptionsHandler::WhitelistUser(const base::ListValue* args) { if (!args->GetString(0, &email)) { return; } - // TODO(pastarmovj): Those will change to CrosSettings ops in phase 2. - UserCrosSettingsProvider::WhitelistUser(Authenticator::Canonicalize(email)); + + scoped_ptr<base::StringValue> canonical_email( + base::Value::CreateStringValue(Authenticator::Canonicalize(email))); + CrosSettings::Get()->AppendToList(kAccountsPrefUsers, canonical_email.get()); } void AccountsOptionsHandler::UnwhitelistUser(const base::ListValue* args) { @@ -93,8 +95,11 @@ void AccountsOptionsHandler::UnwhitelistUser(const base::ListValue* args) { if (!args->GetString(0, &email)) { return; } - // TODO(pastarmovj): Those will change to CrosSettings ops in phase 2. - UserCrosSettingsProvider::UnwhitelistUser(Authenticator::Canonicalize(email)); + + scoped_ptr<base::StringValue> canonical_email( + base::Value::CreateStringValue(Authenticator::Canonicalize(email))); + CrosSettings::Get()->RemoveFromList(kAccountsPrefUsers, + canonical_email.get()); UserManager::Get()->RemoveUser(email, NULL); } |