diff options
| author | davidyu@chromium.org <davidyu@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2014-04-24 11:39:02 +0000 |
|---|---|---|
| committer | davidyu@chromium.org <davidyu@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2014-04-24 11:39:02 +0000 |
| commit | d359450d7c55630a4bb3bed7c8c8ac5d3697c500 (patch) | |
| tree | da4589d4d5b9e4e42f3ac736c428bfd21ce561e6 | |
| parent | c39e625ff5dec4b981d9c52529645d5682085fdc (diff) | |
| download | chromium_src-d359450d7c55630a4bb3bed7c8c8ac5d3697c500.zip chromium_src-d359450d7c55630a4bb3bed7c8c8ac5d3697c500.tar.gz chromium_src-d359450d7c55630a4bb3bed7c8c8ac5d3697c500.tar.bz2 | |
Check for invalid management mode transition.
BUG=353050
TEST=unit_tests
Review URL: https://codereview.chromium.org/240653004
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@265910 0039d316-1c4b-4281-b951-d872f2087c98
4 files changed, 194 insertions, 12 deletions
diff --git a/chrome/browser/chromeos/settings/device_settings_service.cc b/chrome/browser/chromeos/settings/device_settings_service.cc index 20d8256..0895080 100644 --- a/chrome/browser/chromeos/settings/device_settings_service.cc +++ b/chrome/browser/chromeos/settings/device_settings_service.cc @@ -129,6 +129,11 @@ void DeviceSettingsService::SignAndStore( scoped_ptr<em::ChromeDeviceSettingsProto> new_settings, const base::Closure& callback) { scoped_ptr<em::PolicyData> new_policy = AssemblePolicy(*new_settings); + if (!new_policy) { + HandleError(STORE_POLICY_ERROR, callback); + return; + } + Enqueue( new SignAndStoreSettingsOperation( base::Bind(&DeviceSettingsService::HandleCompletedOperation, @@ -142,13 +147,23 @@ void DeviceSettingsService::SetManagementSettings( const std::string& request_token, const std::string& device_id, const base::Closure& callback) { - // TODO(davidyu): Check for invalid management mode transition. + if (!CheckManagementModeTransition(management_mode)) { + LOG(ERROR) << "Invalid management mode transition: current mode = " + << GetManagementMode() << ", new mode = " << management_mode; + HandleError(STORE_POLICY_ERROR, callback); + return; + } + scoped_ptr<em::PolicyData> policy = AssemblePolicy(*device_settings_); - if (policy) { - policy->set_management_mode(management_mode); - policy->set_request_token(request_token); - policy->set_device_id(device_id); + if (!policy) { + HandleError(STORE_POLICY_ERROR, callback); + return; } + + policy->set_management_mode(management_mode); + policy->set_request_token(request_token); + policy->set_device_id(device_id); + Enqueue( new SignAndStoreSettingsOperation( base::Bind(&DeviceSettingsService::HandleCompletedOperation, @@ -382,8 +397,22 @@ void DeviceSettingsService::HandleCompletedOperation( StartNextOperation(); } +void DeviceSettingsService::HandleError(Status status, + const base::Closure& callback) { + store_status_ = status; + + LOG(ERROR) << "Session manager operation failed: " << status; + + FOR_EACH_OBSERVER(Observer, observers_, DeviceSettingsUpdated()); + + // The completion callback happens after the notification so clients can + // filter self-triggered updates. + if (!callback.is_null()) + callback.Run(); +} + scoped_ptr<em::PolicyData> DeviceSettingsService::AssemblePolicy( - const em::ChromeDeviceSettingsProto& settings) { + const em::ChromeDeviceSettingsProto& settings) const { scoped_ptr<em::PolicyData> policy(new em::PolicyData()); if (policy_data_) { // Preserve management settings. @@ -408,6 +437,39 @@ scoped_ptr<em::PolicyData> DeviceSettingsService::AssemblePolicy( return policy.Pass(); } +em::PolicyData::ManagementMode DeviceSettingsService::GetManagementMode() + const { + if (policy_data_ && policy_data_->has_management_mode()) + return policy_data_->management_mode(); + return em::PolicyData::NOT_MANAGED; +} + +bool DeviceSettingsService::CheckManagementModeTransition( + em::PolicyData::ManagementMode new_mode) const { + em::PolicyData::ManagementMode current_mode = GetManagementMode(); + + // Mode is not changed. + if (current_mode == new_mode) + return true; + + switch (current_mode) { + case em::PolicyData::NOT_MANAGED: + // For consumer management enrollment. + return new_mode == em::PolicyData::CONSUMER_MANAGED; + + case em::PolicyData::ENTERPRISE_MANAGED: + // Management mode cannot be set when it is currently ENTERPRISE_MANAGED. + return false; + + case em::PolicyData::CONSUMER_MANAGED: + // For consumer management unenrollment. + return new_mode == em::PolicyData::NOT_MANAGED; + } + + NOTREACHED(); + return false; +} + ScopedTestDeviceSettingsService::ScopedTestDeviceSettingsService() { DeviceSettingsService::Initialize(); } diff --git a/chrome/browser/chromeos/settings/device_settings_service.h b/chrome/browser/chromeos/settings/device_settings_service.h index 9facc45..5b05513 100644 --- a/chrome/browser/chromeos/settings/device_settings_service.h +++ b/chrome/browser/chromeos/settings/device_settings_service.h @@ -235,10 +235,21 @@ class DeviceSettingsService : public SessionManagerClient::Observer, SessionManagerOperation* operation, Status status); + // Updates status and invokes the callback immediately. + void HandleError(Status status, const base::Closure& callback); + // Assembles PolicyData based on |settings| and the current |policy_data_| // and |username_|. scoped_ptr<enterprise_management::PolicyData> AssemblePolicy( - const enterprise_management::ChromeDeviceSettingsProto& settings); + const enterprise_management::ChromeDeviceSettingsProto& settings) const; + + // Returns the current management mode. + enterprise_management::PolicyData::ManagementMode GetManagementMode() const; + + // Returns true if it is okay to transfer from the current mode to the new + // mode. This function should be called in SetManagementMode(). + bool CheckManagementModeTransition( + enterprise_management::PolicyData::ManagementMode new_mode) const; SessionManagerClient* session_manager_client_; scoped_refptr<OwnerKeyUtil> owner_key_util_; diff --git a/chrome/browser/chromeos/settings/device_settings_service_unittest.cc b/chrome/browser/chromeos/settings/device_settings_service_unittest.cc index 0f0037b..727a34f 100644 --- a/chrome/browser/chromeos/settings/device_settings_service_unittest.cc +++ b/chrome/browser/chromeos/settings/device_settings_service_unittest.cc @@ -223,6 +223,120 @@ TEST_F(DeviceSettingsServiceTest, SignAndStoreSuccess) { policy_data->username()); } +TEST_F(DeviceSettingsServiceTest, SetManagementSettingsModeTransition) { + ReloadDeviceSettings(); + EXPECT_EQ(DeviceSettingsService::STORE_SUCCESS, + device_settings_service_.status()); + + owner_key_util_->SetPrivateKey(device_policy_.GetSigningKey()); + device_settings_service_.SetUsername(device_policy_.policy_data().username()); + FlushDeviceSettings(); + + // The initial management mode should be NOT_MANAGED. + EXPECT_EQ(em::PolicyData::NOT_MANAGED, + device_settings_service_.policy_data()->management_mode()); + + // NOT_MANAGED -> CONSUMER_MANAGED: Okay. + device_settings_service_.SetManagementSettings( + em::PolicyData::CONSUMER_MANAGED, + "fake_request_token", + "fake_device_id", + base::Bind(&DeviceSettingsServiceTest::SetOperationCompleted, + base::Unretained(this))); + FlushDeviceSettings(); + + EXPECT_TRUE(operation_completed_); + EXPECT_EQ(DeviceSettingsService::STORE_SUCCESS, + device_settings_service_.status()); + EXPECT_EQ(em::PolicyData::CONSUMER_MANAGED, + device_settings_service_.policy_data()->management_mode()); + + // CONSUMER_MANAGED -> ENTERPRISE_MANAGED: Invalid. + device_settings_service_.SetManagementSettings( + em::PolicyData::ENTERPRISE_MANAGED, + "fake_request_token", + "fake_device_id", + base::Bind(&DeviceSettingsServiceTest::SetOperationCompleted, + base::Unretained(this))); + FlushDeviceSettings(); + + EXPECT_TRUE(operation_completed_); + EXPECT_EQ(DeviceSettingsService::STORE_POLICY_ERROR, + device_settings_service_.status()); + EXPECT_EQ(em::PolicyData::CONSUMER_MANAGED, + device_settings_service_.policy_data()->management_mode()); + + // CONSUMER_MANAGED -> NOT_MANAGED: Okay. + device_settings_service_.SetManagementSettings( + em::PolicyData::NOT_MANAGED, + "fake_request_token", + "fake_device_id", + base::Bind(&DeviceSettingsServiceTest::SetOperationCompleted, + base::Unretained(this))); + FlushDeviceSettings(); + + EXPECT_TRUE(operation_completed_); + EXPECT_EQ(DeviceSettingsService::STORE_SUCCESS, + device_settings_service_.status()); + EXPECT_EQ(em::PolicyData::NOT_MANAGED, + device_settings_service_.policy_data()->management_mode()); + + // NOT_MANAGED -> ENTERPRISE_MANAGED: Invalid. + device_settings_service_.SetManagementSettings( + em::PolicyData::ENTERPRISE_MANAGED, + "fake_request_token", + "fake_device_id", + base::Bind(&DeviceSettingsServiceTest::SetOperationCompleted, + base::Unretained(this))); + FlushDeviceSettings(); + + EXPECT_TRUE(operation_completed_); + EXPECT_EQ(DeviceSettingsService::STORE_POLICY_ERROR, + device_settings_service_.status()); + EXPECT_EQ(em::PolicyData::NOT_MANAGED, + device_settings_service_.policy_data()->management_mode()); + + // Inject a policy data with management mode set to ENTERPRISE_MANAGED. + device_policy_.policy_data().set_management_mode( + em::PolicyData::ENTERPRISE_MANAGED); + device_policy_.Build(); + device_settings_test_helper_.set_policy_blob(device_policy_.GetBlob()); + ReloadDeviceSettings(); + EXPECT_EQ(em::PolicyData::ENTERPRISE_MANAGED, + device_settings_service_.policy_data()->management_mode()); + + // ENTERPRISE_MANAGED -> NOT_MANAGED: Invalid. + device_settings_service_.SetManagementSettings( + em::PolicyData::NOT_MANAGED, + "fake_request_token", + "fake_device_id", + base::Bind(&DeviceSettingsServiceTest::SetOperationCompleted, + base::Unretained(this))); + FlushDeviceSettings(); + + EXPECT_TRUE(operation_completed_); + EXPECT_EQ(DeviceSettingsService::STORE_POLICY_ERROR, + device_settings_service_.status()); + EXPECT_EQ(em::PolicyData::ENTERPRISE_MANAGED, + device_settings_service_.policy_data()->management_mode()); + + // ENTERPRISE_MANAGED -> CONSUMER_MANAGED: Invalid. + device_settings_service_.SetManagementSettings( + em::PolicyData::CONSUMER_MANAGED, + "fake_request_token", + "fake_device_id", + base::Bind(&DeviceSettingsServiceTest::SetOperationCompleted, + base::Unretained(this))); + FlushDeviceSettings(); + + EXPECT_TRUE(operation_completed_); + EXPECT_EQ(DeviceSettingsService::STORE_POLICY_ERROR, + device_settings_service_.status()); + EXPECT_EQ(em::PolicyData::ENTERPRISE_MANAGED, + device_settings_service_.policy_data()->management_mode()); + +} + TEST_F(DeviceSettingsServiceTest, SetManagementSettingsSuccess) { ReloadDeviceSettings(); EXPECT_EQ(DeviceSettingsService::STORE_SUCCESS, diff --git a/chrome/browser/chromeos/settings/session_manager_operation.cc b/chrome/browser/chromeos/settings/session_manager_operation.cc index 9cab08e..b95d283 100644 --- a/chrome/browser/chromeos/settings/session_manager_operation.cc +++ b/chrome/browser/chromeos/settings/session_manager_operation.cc @@ -257,11 +257,6 @@ SignAndStoreSettingsOperation::SignAndStoreSettingsOperation( SignAndStoreSettingsOperation::~SignAndStoreSettingsOperation() {} void SignAndStoreSettingsOperation::Run() { - if (!new_policy_) { - ReportResult(DeviceSettingsService::STORE_POLICY_ERROR); - return; - } - EnsureOwnerKey(base::Bind(&SignAndStoreSettingsOperation::StartSigning, weak_factory_.GetWeakPtr())); } |