Fix up Owner settings on first load

chrome/browser/chromeos/ownership/owner_settings_service_chromeos.cc

 // Copyright 2014 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 "chrome/browser/chromeos/ownership/owner_settings_service_chromeos.h"
 
+#include <algorithm>
 #include <string>
 
 #include "base/bind.h"
 #include "base/bind_helpers.h"
 #include "base/callback.h"
 #include "base/command_line.h"
 #include "base/prefs/pref_service.h"
 #include "base/threading/thread_checker.h"
 #include "chrome/browser/chrome_notification_types.h"
 #include "chrome/browser/chromeos/ownership/owner_settings_service_chromeos_factory.h"
 #include "chrome/browser/chromeos/profiles/profile_helper.h"
-#include "chrome/browser/chromeos/profiles/profile_helper.h"
 #include "chrome/browser/chromeos/settings/cros_settings.h"
 #include "chrome/browser/chromeos/settings/device_settings_provider.h"
 #include "chrome/browser/chromeos/settings/session_manager_operation.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chromeos/dbus/dbus_thread_manager.h"
 #include "chromeos/tpm/tpm_token_loader.h"
 #include "components/ownership/owner_key_util.h"
 #include "components/user_manager/user.h"
 #include "content/public/browser/browser_thread.h"
 #include "content/public/browser/notification_details.h"
@@ skipping 138 matching lines @@
       return false;
 
     case policy::MANAGEMENT_MODE_CONSUMER_MANAGED:
       // For consumer management unenrollment.
       return new_mode == policy::MANAGEMENT_MODE_LOCAL_OWNER;
   }
 
   NOTREACHED();
   return false;
 }
-
 }  // namespace
 
 OwnerSettingsServiceChromeOS::ManagementSettings::ManagementSettings() {
 }
 
 OwnerSettingsServiceChromeOS::ManagementSettings::~ManagementSettings() {
 }
 
 OwnerSettingsServiceChromeOS::OwnerSettingsServiceChromeOS(
     DeviceSettingsService* device_settings_service,
@@ skipping 219 matching lines @@
       base::Bind(base::IgnoreResult(&crypto::InitializeNSSForChromeOSUser),
                  user_hash,
                  ProfileHelper::GetProfilePathByUserIdHash(user_hash)),
       base::Bind(&DoesPrivateKeyExistAsync, owner_key_util, callback));
 }
 
 // static
 scoped_ptr<em::PolicyData> OwnerSettingsServiceChromeOS::AssemblePolicy(
     const std::string& user_id,
     const em::PolicyData* policy_data,
-    const em::ChromeDeviceSettingsProto* settings) {
+    em::ChromeDeviceSettingsProto* settings) {
   scoped_ptr<em::PolicyData> policy(new em::PolicyData());
   if (policy_data) {
     // Preserve management settings.
     if (policy_data->has_management_mode())
       policy->set_management_mode(policy_data->management_mode());
     if (policy_data->has_request_token())
       policy->set_request_token(policy_data->request_token());
     if (policy_data->has_device_id())
       policy->set_device_id(policy_data->device_id());
   } else {
     // If there's no previous policy data, this is the first time the device
     // setting is set. We set the management mode to LOCAL_OWNER initially.
     policy->set_management_mode(em::PolicyData::LOCAL_OWNER);
   }
+  // Perform fixups required to ensure sensical local-owner device policy:
+  //  1) The owner must be in the username field,
+  //  2) user whitelisting must be explicitly allowed or disallowed, and
+  //  3) the owner user must be on the whitelist, if it's enforced.
+  // We can enforce the first two here, but need to check the whitelist before
+  // modifying it, so that will be taken care of in a separate class.

[Mattias Nissler (ping if slow), 2015/03/18 08:36:42]

stale comment

[Chris Masone, 2015/03/24 20:53:36]

Done.

+  if (policy->management_mode() == em::PolicyData::LOCAL_OWNER)

[Mattias Nissler (ping if slow), 2015/03/18 08:36:41]

I think you want to do this both for LOCAL_OWNER and CONSUMER_MANAGED, as the
owner key is controlled by a local account in both cases.

[Chris Masone, 2015/03/24 20:53:36]

Done.

[Chris Masone, 2015/03/24 20:53:36]

Done.

+    FixupLocalOwnerPolicy(user_id, settings);
   policy->set_policy_type(policy::dm_protocol::kChromeDevicePolicyType);
   policy->set_timestamp(
       (base::Time::Now() - base::Time::UnixEpoch()).InMilliseconds());
   policy->set_username(user_id);
   if (!settings->SerializeToString(policy->mutable_policy_value()))
     return scoped_ptr<em::PolicyData>();
 
   return policy.Pass();
 }
 
 // static
+void OwnerSettingsServiceChromeOS::FixupLocalOwnerPolicy(
+    const std::string& user_id,
+    enterprise_management::ChromeDeviceSettingsProto* settings) {
+  if (!settings->has_allow_new_users())
+    settings->mutable_allow_new_users()->set_allow_new_users(true);
+
+  em::UserWhitelistProto* whitelist_proto = settings->mutable_user_whitelist();
+  if (whitelist_proto->user_whitelist().end() ==
+      std::find(whitelist_proto->user_whitelist().begin(),
+                whitelist_proto->user_whitelist().end(), user_id)) {
+    whitelist_proto->add_user_whitelist(user_id);
+  }
+}
+
+// static
 void OwnerSettingsServiceChromeOS::UpdateDeviceSettings(
     const std::string& path,
     const base::Value& value,
     enterprise_management::ChromeDeviceSettingsProto& settings) {
   if (path == kAccountsPrefAllowNewUser) {
     em::AllowNewUsersProto* allow = settings.mutable_allow_new_users();
     bool allow_value;
     if (value.GetAsBoolean(&allow_value)) {
       allow->set_allow_new_users(allow_value);
     } else {
@@ skipping 273 matching lines @@
     if (pending_management_settings_.request_token.empty())
       policy->clear_request_token();
     else
       policy->set_request_token(pending_management_settings_.request_token);
 
     if (pending_management_settings_.device_id.empty())
       policy->clear_device_id();
     else
       policy->set_device_id(pending_management_settings_.device_id);
   }
   has_pending_management_settings_ = false;

[Mattias Nissler (ping if slow), 2015/03/18 08:36:41]

I'd put the call to FixupLocalOwnerPolicy here, because the management_mode
updating code above may change the mode, and you only want to handle the local
ownership case. Then again, management mode changes should never cause a
transition that changes your decision, but it still seems cleaner to put the
code here.

[Chris Masone, 2015/03/24 20:53:36]

By this point, the policy is already assembled, so my changes to |settings| are
ignored. I've moved all these management settings and such into AssemblePolicy,
along with my own changes to the policy. This way it all happens in one place. 

 
   bool rv = AssembleAndSignPolicyAsync(
       content::BrowserThread::GetBlockingPool(), policy.Pass(),
       base::Bind(&OwnerSettingsServiceChromeOS::OnPolicyAssembledAndSigned,
                  store_settings_factory_.GetWeakPtr()));
   if (!rv)
     ReportStatusAndContinueStoring(false /* success */);
 }
 
 void OwnerSettingsServiceChromeOS::OnPolicyAssembledAndSigned(
@@ skipping 25 matching lines @@
   std::vector<OnManagementSettingsSetCallback> callbacks;
   pending_management_settings_callbacks_.swap(callbacks);
   for (const auto& callback : callbacks) {
     if (!callback.is_null())
       callback.Run(success);
   }
   StorePendingChanges();
 }
 
 }  // namespace chromeos