Preference dis/allowing supervised users creation is now available as owner setting, not only as de…

chrome/browser/chromeos/settings/device_settings_provider.cc

 // Copyright (c) 2012 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/settings/device_settings_provider.h"
 
 #include "base/bind.h"
 #include "base/bind_helpers.h"
 #include "base/callback.h"
 #include "base/logging.h"
@@ skipping 198 matching lines @@
     else
       NOTREACHED();
   } else if (prop == kAccountsPrefAllowGuest) {
     em::GuestModeEnabledProto* guest =
         device_settings_.mutable_guest_mode_enabled();
     bool guest_value;
     if (value->GetAsBoolean(&guest_value))
       guest->set_guest_mode_enabled(guest_value);
     else
       NOTREACHED();
+  } else if (prop == kAccountsPrefSupervisedUsersEnabled) {
+    em::SupervisedUsersSettingsProto* supervised =
+        device_settings_.mutable_supervised_users_settings();
+    bool supervised_value;
+    if (value->GetAsBoolean(&supervised_value))
+      supervised->set_supervised_users_enabled(supervised_value);
+    else
+      NOTREACHED();
   } else if (prop == kAccountsPrefShowUserNamesOnSignIn) {
     em::ShowUserNamesOnSigninProto* show =
         device_settings_.mutable_show_user_names();
     bool show_value;
     if (value->GetAsBoolean(&show_value))
       show->set_show_user_names(show_value);
     else
       NOTREACHED();
   } else if (prop == kAccountsPrefDeviceLocalAccounts) {
     em::DeviceLocalAccountsProto* device_local_accounts =
@@ skipping 151 matching lines @@
         device_settings_.mutable_attestation_settings();
     bool setting_enabled;
     if (value->GetAsBoolean(&setting_enabled)) {
       attestation_settings->set_content_protection_enabled(setting_enabled);
     } else {
       NOTREACHED();
     }
   } else {
     // The remaining settings don't support Set(), since they are not
     // intended to be customizable by the user:
-    //   kAccountsPrefSupervisedUsersEnabled
     //   kAppPack
     //   kDeviceAttestationEnabled
     //   kDeviceOwner
     //   kIdleLogoutTimeout
     //   kIdleLogoutWarningDuration
     //   kReleaseChannelDelegated
     //   kReportDeviceActivityTimes
     //   kReportDeviceBootMode
     //   kReportDeviceLocation
     //   kReportDeviceVersionInfo
@@ skipping 29 matching lines @@
   }
 }
 
 void DeviceSettingsProvider::DecodeLoginPolicies(
     const em::ChromeDeviceSettingsProto& policy,
     PrefValueMap* new_values_cache) const {
   // For all our boolean settings the following is applicable:
   // true is default permissive value and false is safe prohibitive value.
   // Exceptions:
   //   kAccountsPrefEphemeralUsersEnabled has a default value of false.
+  //   kAccountsPrefSupervisedUsersEnabled has a default value of false
+  //     for enterprise devices and true for consumer devices.
   if (policy.has_allow_new_users() &&
       policy.allow_new_users().has_allow_new_users()) {
     if (policy.allow_new_users().allow_new_users()) {
       // New users allowed, user whitelist ignored.
       new_values_cache->SetBoolean(kAccountsPrefAllowNewUser, true);
     } else {
       // New users not allowed, enforce user whitelist if present.
       new_values_cache->SetBoolean(kAccountsPrefAllowNewUser,
                                    !policy.has_user_whitelist());
     }
   } else {
     // No configured allow-new-users value, enforce whitelist if non-empty.
     new_values_cache->SetBoolean(
         kAccountsPrefAllowNewUser,
         policy.user_whitelist().user_whitelist_size() == 0);
   }
 
   new_values_cache->SetBoolean(
       kAccountsPrefAllowGuest,
       !policy.has_guest_mode_enabled() ||
       !policy.guest_mode_enabled().has_guest_mode_enabled() ||
       policy.guest_mode_enabled().guest_mode_enabled());
 
+  if (g_browser_process->is_browser_policy_connector_created()) {

[jam, 2014/05/13 21:09:11]

why this check? it's not deterministic when you're in this method whether this
object is created or not?

if it's always created, then this check isn't needed.
if it's not created, can its constructor be made cheap, and any expensive work
can be done lazily?

[merkulova, 2014/05/14 06:30:10]

It's a check that covers unit tests where browser is not created. Implicit
creation at this step led to tests failing due to wrong Shutdown order.

Had a discussion with Mattias on it. Comments #55 and #59 contain the info that
might help understand the case.

[jam, 2014/05/14 17:29:46]

Can you find another way of doing this without adding yet another method to
BrowserProcess? why can't this unit test check be localized to policy code
without adding another getter to the global BrowserProcess? 

[merkulova, 2014/05/23 09:27:15]

Done.

[jam, 2014/05/28 17:38:40]

thanks, removing myself as reviewer

+    policy::BrowserPolicyConnectorChromeOS* connector =
+        g_browser_process->platform_part()->browser_policy_connector_chromeos();
+    bool supervised_users_enabled = false;
+    if (connector->IsEnterpriseManaged()) {
+      supervised_users_enabled =
+          policy.has_supervised_users_settings() &&
+          policy.supervised_users_settings().has_supervised_users_enabled() &&
+          policy.supervised_users_settings().supervised_users_enabled();
+    } else {
+      supervised_users_enabled =
+          !policy.has_supervised_users_settings() ||
+          !policy.supervised_users_settings().has_supervised_users_enabled() ||
+          policy.supervised_users_settings().supervised_users_enabled();
+    }
+    new_values_cache->SetBoolean(
+        kAccountsPrefSupervisedUsersEnabled, supervised_users_enabled);
+    } else {
+      // For unit tests.
+      new_values_cache->SetBoolean(kAccountsPrefSupervisedUsersEnabled, true);
+  }
+
   new_values_cache->SetBoolean(
       kAccountsPrefShowUserNamesOnSignIn,
       !policy.has_show_user_names() ||
       !policy.show_user_names().has_show_user_names() ||
       policy.show_user_names().show_user_names());
 
   new_values_cache->SetBoolean(
       kAccountsPrefEphemeralUsersEnabled,
       policy.has_ephemeral_users_enabled() &&
       policy.ephemeral_users_enabled().has_ephemeral_users_enabled() &&
       policy.ephemeral_users_enabled().ephemeral_users_enabled());
 
-  new_values_cache->SetBoolean(
-      kAccountsPrefSupervisedUsersEnabled,
-      policy.has_supervised_users_settings() &&
-      policy.supervised_users_settings().supervised_users_enabled());
-
   base::ListValue* list = new base::ListValue();
   const em::UserWhitelistProto& whitelist_proto = policy.user_whitelist();
   const RepeatedPtrField<std::string>& whitelist =
       whitelist_proto.user_whitelist();
   for (RepeatedPtrField<std::string>::const_iterator it = whitelist.begin();
        it != whitelist.end(); ++it) {
     list->Append(new base::StringValue(*it));
   }
   new_values_cache->SetValue(kAccountsPrefUsers, list);
 
@@ skipping 483 matching lines @@
 void DeviceSettingsProvider::AttemptMigration() {
   if (device_settings_service_->HasPrivateOwnerKey()) {
     PrefValueMap::const_iterator i;
     for (i = migration_values_.begin(); i != migration_values_.end(); ++i)
       DoSet(i->first, *i->second);
     migration_values_.Clear();
   }
 }
 
 }  // namespace chromeos