Add RESTORE_MODE_DISABLED to RestoreMode

This CL changes the way that device disabling is expressed in the
DeviceStateRetrievalResponse proto. Instead of being implied when
the |device_state| field is present, it is now explictly set as a
|restore_mode|.

BUG=425574
TEST=Updated unit and browser tests

Review URL: https://codereview.chromium.org/690643002

Cr-Commit-Position: refs/heads/master@{#302426}

chrome/browser/chromeos/login/screens/device_disabled_screen.cc

@@ -48,11 +48,7 @@ void DeviceDisabledScreen::Show() {
   if (!actor_ || showing_)
     return;
 
-  bool is_device_disabled = false;
-  g_browser_process->local_state()->GetDictionary(
-      prefs::kServerBackedDeviceState)->GetBoolean(policy::kDeviceStateDisabled,
-                                                   &is_device_disabled);
-  if (!is_device_disabled ||
+  if (policy::GetRestoreMode() != policy::RESTORE_MODE_DISABLED ||
       CommandLine::ForCurrentProcess()->HasSwitch(
           switches::kDisableDeviceDisabling)) {
     // Skip the screen if the device is not marked as disabled or device

chrome/browser/chromeos/login/screens/device_disabled_screen_unittest.cc

@@ -102,9 +102,13 @@ void DeviceDisabledScreenTest::HideErrorScreen(BaseScreen* parent_screen) {
 
 void DeviceDisabledScreenTest::SetDeviceDisabled(bool disabled) {
   DictionaryPrefUpdate dict(&local_state_, prefs::kServerBackedDeviceState);
-  dict->SetBoolean(policy::kDeviceStateDisabled, disabled);
-  if (disabled)
-    dict->SetString(policy::kDeviceStateDisabledMessage, kDisabledMessage);
+  if (disabled) {
+    dict->SetString(policy::kDeviceStateRestoreMode,
+                    policy::kDeviceStateRestoreModeDisabled);
+  } else {
+    dict->Remove(policy::kDeviceStateRestoreMode, nullptr);
+  }
+  dict->SetString(policy::kDeviceStateDisabledMessage, kDisabledMessage);
 }
 
 void DeviceDisabledScreenTest::SetDeviceMode(policy::DeviceMode device_mode) {

chrome/browser/chromeos/login/wizard_controller_browsertest.cc

@@ -884,7 +884,8 @@ IN_PROC_BROWSER_TEST_F(WizardControllerDeviceStateTest,
   EXPECT_EQ(GetErrorScreen(),
             WizardController::default_controller()->current_screen());
   base::DictionaryValue device_state;
-  device_state.SetBoolean(policy::kDeviceStateDisabled, true);
+  device_state.SetString(policy::kDeviceStateRestoreMode,
+                         policy::kDeviceStateRestoreModeDisabled);
   device_state.SetString(policy::kDeviceStateDisabledMessage, kDisabledMessage);
   g_browser_process->local_state()->Set(prefs::kServerBackedDeviceState,
                                         device_state);

chrome/browser/chromeos/policy/auto_enrollment_client.cc

@@ -74,6 +74,8 @@ std::string ConvertRestoreMode(
       return kDeviceStateRestoreModeReEnrollmentRequested;
     case em::DeviceStateRetrievalResponse::RESTORE_MODE_REENROLLMENT_ENFORCED:
       return kDeviceStateRestoreModeReEnrollmentEnforced;
+    case em::DeviceStateRetrievalResponse::RESTORE_MODE_DISABLED:
+      return kDeviceStateRestoreModeDisabled;
   }
 
   // Return is required to avoid compiler warning.
@@ -233,13 +235,10 @@ void AutoEnrollmentClient::NextStep() {
     // Protocol finished successfully, report result.
     bool trigger_enrollment = false;
     if (retrieve_device_state_) {
-      const base::DictionaryValue* device_state_dict =
-          local_state_->GetDictionary(prefs::kServerBackedDeviceState);
-      std::string restore_mode;
-      device_state_dict->GetString(kDeviceStateRestoreMode, &restore_mode);
+      const RestoreMode restore_mode = GetRestoreMode();
       trigger_enrollment =
-          (restore_mode == kDeviceStateRestoreModeReEnrollmentRequested ||
-           restore_mode == kDeviceStateRestoreModeReEnrollmentEnforced);
+          (restore_mode == RESTORE_MODE_REENROLLMENT_REQUESTED ||
+           restore_mode == RESTORE_MODE_REENROLLMENT_ENFORCED);
     } else {
       trigger_enrollment = has_server_state_;
     }
@@ -411,11 +410,6 @@ bool AutoEnrollmentClient::OnDeviceStateRequestCompletion(
                  !restore_mode.empty(),
                  new base::StringValue(restore_mode));
 
-      UpdateDict(dict.Get(),
-                 kDeviceStateDisabled,
-                 true /* set_or_clear */,
-                 new base::FundamentalValue(
-                     state_response.has_disabled_state()));
       UpdateDict(dict.Get(),
                  kDeviceStateDisabledMessage,
                  state_response.has_disabled_state(),

chrome/browser/chromeos/policy/auto_enrollment_client_unittest.cc

@@ -118,17 +118,14 @@ class AutoEnrollmentClientTest : public testing::Test {
   void ServerWillSendState(
       const std::string& management_domain,
       em::DeviceStateRetrievalResponse::RestoreMode restore_mode,
-      scoped_ptr<std::string> device_disabled_message) {
+      const std::string& device_disabled_message) {
     em::DeviceManagementResponse response;
     em::DeviceStateRetrievalResponse* state_response =
         response.mutable_device_state_retrieval_response();
     state_response->set_restore_mode(restore_mode);
     state_response->set_management_domain(management_domain);
-    if (device_disabled_message) {
-      em::DisabledState* disabled_state =
-          state_response->mutable_disabled_state();
-      disabled_state->set_message(*device_disabled_message);
-    }
+    state_response->mutable_disabled_state()->set_message(
+        device_disabled_message);
     EXPECT_CALL(
         *service_,
         CreateJob(DeviceManagementRequestJob::TYPE_DEVICE_STATE_RETRIEVAL, _))
@@ -163,7 +160,7 @@ class AutoEnrollmentClientTest : public testing::Test {
   void VerifyServerBackedState(
       const std::string& expected_management_domain,
       const std::string& expected_restore_mode,
-      scoped_ptr<std::string> expected_disabled_message) {
+      const std::string& expected_disabled_message) {
     const base::Value* state =
         local_state_->GetUserPref(prefs::kServerBackedDeviceState);
     ASSERT_TRUE(state);
@@ -184,20 +181,10 @@ class AutoEnrollmentClientTest : public testing::Test {
       EXPECT_FALSE(state_dict->HasKey(kDeviceStateRestoreMode));
     }
 
-    const bool expected_disabled_state = expected_disabled_message;
-    bool actual_disabled_state = false;
-    EXPECT_TRUE(state_dict->GetBoolean(kDeviceStateDisabled,
-                                       &actual_disabled_state));
-    EXPECT_EQ(expected_disabled_state, actual_disabled_state);
-
-    if (expected_disabled_message) {
-      std::string actual_disabled_message;
-      EXPECT_TRUE(state_dict->GetString(kDeviceStateDisabledMessage,
-                                        &actual_disabled_message));
-      EXPECT_EQ(*expected_disabled_message, actual_disabled_message);
-    } else {
-      EXPECT_FALSE(state_dict->HasKey(kDeviceStateDisabledMessage));
-    }
+    std::string actual_disabled_message;
+    EXPECT_TRUE(state_dict->GetString(kDeviceStateDisabledMessage,
+                                      &actual_disabled_message));
+    EXPECT_EQ(expected_disabled_message, actual_disabled_message);
   }
 
   const em::DeviceAutoEnrollmentRequest& auto_enrollment_request() {
@@ -272,13 +259,13 @@ TEST_F(AutoEnrollmentClientTest, AskForLess) {
   ServerWillSendState(
       "example.com",
       em::DeviceStateRetrievalResponse::RESTORE_MODE_REENROLLMENT_ENFORCED,
-      make_scoped_ptr(new std::string(kDisabledMessage)));
+      kDisabledMessage);
   client_->Start();
   EXPECT_EQ(AUTO_ENROLLMENT_STATE_TRIGGER_ENROLLMENT, state_);
   VerifyCachedResult(true, 8);
   VerifyServerBackedState("example.com",
                           kDeviceStateRestoreModeReEnrollmentEnforced,
-                          make_scoped_ptr(new std::string(kDisabledMessage)));
+                          kDisabledMessage);
 }
 
 TEST_F(AutoEnrollmentClientTest, AskForSame) {
@@ -288,13 +275,13 @@ TEST_F(AutoEnrollmentClientTest, AskForSame) {
   ServerWillSendState(
       "example.com",
       em::DeviceStateRetrievalResponse::RESTORE_MODE_REENROLLMENT_ENFORCED,
-      make_scoped_ptr(new std::string(kDisabledMessage)));
+      kDisabledMessage);
   client_->Start();
   EXPECT_EQ(AUTO_ENROLLMENT_STATE_TRIGGER_ENROLLMENT, state_);
   VerifyCachedResult(true, 8);
   VerifyServerBackedState("example.com",
                           kDeviceStateRestoreModeReEnrollmentEnforced,
-                          make_scoped_ptr(new std::string(kDisabledMessage)));
+                          kDisabledMessage);
 }
 
 TEST_F(AutoEnrollmentClientTest, AskForSameTwice) {
@@ -347,13 +334,13 @@ TEST_F(AutoEnrollmentClientTest, ForcedReEnrollment) {
   ServerWillSendState(
       "example.com",
       em::DeviceStateRetrievalResponse::RESTORE_MODE_REENROLLMENT_ENFORCED,
-      nullptr);
+      kDisabledMessage);
   client_->Start();
   EXPECT_EQ(AUTO_ENROLLMENT_STATE_TRIGGER_ENROLLMENT, state_);
   VerifyCachedResult(true, 8);
   VerifyServerBackedState("example.com",
                           kDeviceStateRestoreModeReEnrollmentEnforced,
-                          nullptr);
+                          kDisabledMessage);
 
   // Network changes don't trigger retries after obtaining a response from
   // the server.
@@ -366,27 +353,27 @@ TEST_F(AutoEnrollmentClientTest, RequestedReEnrollment) {
   ServerWillSendState(
       "example.com",
       em::DeviceStateRetrievalResponse::RESTORE_MODE_REENROLLMENT_REQUESTED,
-      nullptr);
+      kDisabledMessage);
   client_->Start();
   EXPECT_EQ(AUTO_ENROLLMENT_STATE_TRIGGER_ENROLLMENT, state_);
   VerifyCachedResult(true, 8);
   VerifyServerBackedState("example.com",
                           kDeviceStateRestoreModeReEnrollmentRequested,
-                          nullptr);
+                          kDisabledMessage);
 }
 
 TEST_F(AutoEnrollmentClientTest, DeviceDisabled) {
   ServerWillReply(-1, true, true);
   ServerWillSendState(
       "example.com",
-      em::DeviceStateRetrievalResponse::RESTORE_MODE_NONE,
-      make_scoped_ptr(new std::string(kDisabledMessage)));
+      em::DeviceStateRetrievalResponse::RESTORE_MODE_DISABLED,
+      kDisabledMessage);
   client_->Start();
   EXPECT_EQ(AUTO_ENROLLMENT_STATE_NO_ENROLLMENT, state_);
   VerifyCachedResult(true, 8);
   VerifyServerBackedState("example.com",
-                          "",
-                          make_scoped_ptr(new std::string(kDisabledMessage)));
+                          kDeviceStateRestoreModeDisabled,
+                          kDisabledMessage);
 }
 
 TEST_F(AutoEnrollmentClientTest, NoSerial) {
@@ -435,13 +422,13 @@ TEST_F(AutoEnrollmentClientTest, MoreThan32BitsUploaded) {
   ServerWillSendState(
       "example.com",
       em::DeviceStateRetrievalResponse::RESTORE_MODE_REENROLLMENT_ENFORCED,
-      make_scoped_ptr(new std::string(kDisabledMessage)));
+      kDisabledMessage);
   client_->Start();
   EXPECT_EQ(AUTO_ENROLLMENT_STATE_TRIGGER_ENROLLMENT, state_);
   VerifyCachedResult(true, 37);
   VerifyServerBackedState("example.com",
                           kDeviceStateRestoreModeReEnrollmentEnforced,
-                          make_scoped_ptr(new std::string(kDisabledMessage)));
+                          kDisabledMessage);
 }
 
 TEST_F(AutoEnrollmentClientTest, ReuseCachedDecision) {
@@ -453,12 +440,12 @@ TEST_F(AutoEnrollmentClientTest, ReuseCachedDecision) {
   ServerWillSendState(
       "example.com",
       em::DeviceStateRetrievalResponse::RESTORE_MODE_REENROLLMENT_ENFORCED,
-      make_scoped_ptr(new std::string(kDisabledMessage)));
+      kDisabledMessage);
   client_->Start();
   EXPECT_EQ(AUTO_ENROLLMENT_STATE_TRIGGER_ENROLLMENT, state_);
   VerifyServerBackedState("example.com",
                           kDeviceStateRestoreModeReEnrollmentEnforced,
-                          make_scoped_ptr(new std::string(kDisabledMessage)));
+                          kDisabledMessage);
   AutoEnrollmentClient::CancelAutoEnrollment();
   client_->Start();
   EXPECT_EQ(AUTO_ENROLLMENT_STATE_NO_ENROLLMENT, state_);
@@ -475,12 +462,12 @@ TEST_F(AutoEnrollmentClientTest, RetryIfPowerLargerThanCached) {
   ServerWillSendState(
       "example.com",
       em::DeviceStateRetrievalResponse::RESTORE_MODE_REENROLLMENT_ENFORCED,
-      make_scoped_ptr(new std::string(kDisabledMessage)));
+      kDisabledMessage);
   client_->Start();
   EXPECT_EQ(AUTO_ENROLLMENT_STATE_TRIGGER_ENROLLMENT, state_);
   VerifyServerBackedState("example.com",
                           kDeviceStateRestoreModeReEnrollmentEnforced,
-                          make_scoped_ptr(new std::string(kDisabledMessage)));
+                          kDisabledMessage);
 }
 
 TEST_F(AutoEnrollmentClientTest, NetworkChangeRetryAfterErrors) {
@@ -502,13 +489,13 @@ TEST_F(AutoEnrollmentClientTest, NetworkChangeRetryAfterErrors) {
   ServerWillSendState(
       "example.com",
       em::DeviceStateRetrievalResponse::RESTORE_MODE_REENROLLMENT_ENFORCED,
-      make_scoped_ptr(new std::string(kDisabledMessage)));
+      kDisabledMessage);
   client_->OnNetworkChanged(net::NetworkChangeNotifier::CONNECTION_ETHERNET);
   EXPECT_EQ(AUTO_ENROLLMENT_STATE_TRIGGER_ENROLLMENT, state_);
   EXPECT_TRUE(HasCachedDecision());
   VerifyServerBackedState("example.com",
                           kDeviceStateRestoreModeReEnrollmentEnforced,
-                          make_scoped_ptr(new std::string(kDisabledMessage)));
+                          kDisabledMessage);
 
   // Subsequent network changes don't trigger retries.
   client_->OnNetworkChanged(net::NetworkChangeNotifier::CONNECTION_NONE);
@@ -517,7 +504,7 @@ TEST_F(AutoEnrollmentClientTest, NetworkChangeRetryAfterErrors) {
   EXPECT_TRUE(HasCachedDecision());
   VerifyServerBackedState("example.com",
                           kDeviceStateRestoreModeReEnrollmentEnforced,
-                          make_scoped_ptr(new std::string(kDisabledMessage)));
+                          kDisabledMessage);
 }
 
 TEST_F(AutoEnrollmentClientTest, CancelAndDeleteSoonWithPendingRequest) {
@@ -576,12 +563,12 @@ TEST_F(AutoEnrollmentClientTest, CancelAndDeleteSoonAfterCompletion) {
   ServerWillSendState(
       "example.com",
       em::DeviceStateRetrievalResponse::RESTORE_MODE_REENROLLMENT_ENFORCED,
-      make_scoped_ptr(new std::string(kDisabledMessage)));
+      kDisabledMessage);
   client_->Start();
   EXPECT_EQ(AUTO_ENROLLMENT_STATE_TRIGGER_ENROLLMENT, state_);
   VerifyServerBackedState("example.com",
                           kDeviceStateRestoreModeReEnrollmentEnforced,
-                          make_scoped_ptr(new std::string(kDisabledMessage)));
+                          kDisabledMessage);
 
   // The client will delete itself immediately if there are no pending
   // requests.
@@ -626,7 +613,7 @@ TEST_F(AutoEnrollmentClientTest, NetworkFailureThenRequireUpdatedModulus) {
   ServerWillSendState(
       "example.com",
       em::DeviceStateRetrievalResponse::RESTORE_MODE_REENROLLMENT_ENFORCED,
-      make_scoped_ptr(new std::string(kDisabledMessage)));
+      kDisabledMessage);
   EXPECT_CALL(*service_, StartJob(_, _, _, _, _, _, _));
 
   // Trigger a network change event.
@@ -635,7 +622,7 @@ TEST_F(AutoEnrollmentClientTest, NetworkFailureThenRequireUpdatedModulus) {
   EXPECT_TRUE(HasCachedDecision());
   VerifyServerBackedState("example.com",
                           kDeviceStateRestoreModeReEnrollmentEnforced,
-                          make_scoped_ptr(new std::string(kDisabledMessage)));
+                          kDisabledMessage);
   Mock::VerifyAndClearExpectations(service_.get());
 }
 

chrome/browser/chromeos/policy/device_cloud_policy_initializer.cc

@@ -135,9 +135,9 @@ void DeviceCloudPolicyInitializer::StartEnrollment(
 }
 
 bool DeviceCloudPolicyInitializer::ShouldAutoStartEnrollment() const {
-  std::string restore_mode = GetRestoreMode();
-  if (restore_mode == kDeviceStateRestoreModeReEnrollmentRequested ||
-      restore_mode == kDeviceStateRestoreModeReEnrollmentEnforced) {
+  const RestoreMode restore_mode = GetRestoreMode();
+  if (restore_mode == RESTORE_MODE_REENROLLMENT_REQUESTED ||
+      restore_mode == RESTORE_MODE_REENROLLMENT_ENFORCED) {
     return true;
   }
 
@@ -163,7 +163,7 @@ std::string DeviceCloudPolicyInitializer::GetEnrollmentRecoveryDomain() const {
 }
 
 bool DeviceCloudPolicyInitializer::CanExitEnrollment() const {
-  if (GetRestoreMode() == kDeviceStateRestoreModeReEnrollmentEnforced)
+  if (GetRestoreMode() == RESTORE_MODE_REENROLLMENT_ENFORCED)
     return false;
 
   if (local_state_->HasPrefPath(prefs::kDeviceEnrollmentCanExit))
@@ -254,12 +254,4 @@ void DeviceCloudPolicyInitializer::StartConnection(
   }
 }
 
-std::string DeviceCloudPolicyInitializer::GetRestoreMode() const {
-  const base::DictionaryValue* device_state_dict =
-      local_state_->GetDictionary(prefs::kServerBackedDeviceState);
-  std::string restore_mode;
-  device_state_dict->GetString(kDeviceStateRestoreMode, &restore_mode);
-  return restore_mode;
-}
-
 }  // namespace policy

chrome/browser/chromeos/policy/device_cloud_policy_initializer.h

@@ -110,9 +110,6 @@ class DeviceCloudPolicyInitializer : public CloudPolicyStore::Observer {
   void TryToCreateClient();
   void StartConnection(scoped_ptr<CloudPolicyClient> client);
 
-  // Gets the device restore mode as stored in |local_state_|.
-  std::string GetRestoreMode() const;
-
   PrefService* local_state_;
   DeviceManagementService* enterprise_service_;
   DeviceManagementService* consumer_service_;

chrome/browser/chromeos/policy/server_backed_device_state.cc

@@ -4,16 +4,43 @@
 
 #include "chrome/browser/chromeos/policy/server_backed_device_state.h"
 
+#include <string>
+
+#include "base/logging.h"
+#include "base/prefs/pref_service.h"
+#include "base/values.h"
+#include "chrome/browser/browser_process.h"
+#include "chrome/browser/browser_process_platform_part.h"
+#include "chrome/common/pref_names.h"
+
 namespace policy {
 
 const char kDeviceStateManagementDomain[] = "management_domain";
 const char kDeviceStateRestoreMode[] = "device_mode";
-const char kDeviceStateDisabled[] = "disabled";
 const char kDeviceStateDisabledMessage[] = "disabled_message";
 
 const char kDeviceStateRestoreModeReEnrollmentRequested[] =
     "re-enrollment-requested";
 const char kDeviceStateRestoreModeReEnrollmentEnforced[] =
     "re-enrollment-enforced";
+const char kDeviceStateRestoreModeDisabled[] = "disabled";
+
+RestoreMode GetRestoreMode() {
+  std::string restore_mode;
+  g_browser_process->local_state()->GetDictionary(
+      prefs::kServerBackedDeviceState)->GetString(kDeviceStateRestoreMode,
+                                                  &restore_mode);
+  if (restore_mode.empty())
+    return RESTORE_MODE_NONE;
+  if (restore_mode == kDeviceStateRestoreModeReEnrollmentRequested)
+    return RESTORE_MODE_REENROLLMENT_REQUESTED;
+  if (restore_mode == kDeviceStateRestoreModeReEnrollmentEnforced)
+    return RESTORE_MODE_REENROLLMENT_ENFORCED;
+  if (restore_mode == kDeviceStateRestoreModeDisabled)
+    return RESTORE_MODE_DISABLED;
+
+  NOTREACHED();
+  return RESTORE_MODE_NONE;
+}
 
 }  // namespace policy

chrome/browser/chromeos/policy/server_backed_device_state.h

@@ -10,12 +10,30 @@ namespace policy {
 // Dictionary key constants for prefs::kServerBackedDeviceState.
 extern const char kDeviceStateManagementDomain[];
 extern const char kDeviceStateRestoreMode[];
-extern const char kDeviceStateDisabled[];
 extern const char kDeviceStateDisabledMessage[];
 
-// Values for kDeviceStateRestoreMode.
-extern const char kDeviceStateRestoreModeReEnrollmentEnforced[];
+// String constants used to persist the restorative action in the
+// kDeviceStateRestoreMode dictionary entry.
 extern const char kDeviceStateRestoreModeReEnrollmentRequested[];
+extern const char kDeviceStateRestoreModeReEnrollmentEnforced[];
+extern const char kDeviceStateRestoreModeDisabled[];
+
+// Restorative action to take after device reset.
+enum RestoreMode {
+  // No state restoration.
+  RESTORE_MODE_NONE = 0,
+  // Enterprise enrollment requested, but user may skip.
+  RESTORE_MODE_REENROLLMENT_REQUESTED = 1,
+  // Enterprise enrollment is enforced and cannot be skipped.
+  RESTORE_MODE_REENROLLMENT_ENFORCED = 2,
+  // The device has been disabled by its owner. The device will show a warning
+  // screen and prevent the user from proceeding further.
+  RESTORE_MODE_DISABLED = 3,
+};
+
+// Parses the contents of the kDeviceStateRestoreMode dictionary entry and
+// returns it as a RestoreMode.
+RestoreMode GetRestoreMode();
 
 }  // namespace policy
 

components/policy/proto/device_management_backend.proto

@@ -619,6 +619,9 @@ message DeviceStateRetrievalResponse {
     RESTORE_MODE_REENROLLMENT_REQUESTED = 1;
     // Enterprise enrollment is enforced and cannot be skipped.
     RESTORE_MODE_REENROLLMENT_ENFORCED = 2;
+    // The device has been disabled by its owner. The device will show a warning
+    // screen and prevent the user from proceeding further.
+    RESTORE_MODE_DISABLED = 3;
   };
   // The server-indicated restore mode.
   optional RestoreMode restore_mode = 1 [default = RESTORE_MODE_NONE];
@@ -626,7 +629,8 @@ message DeviceStateRetrievalResponse {
   // Primary domain the device is associated with.
   optional string management_domain = 2;
 
-  // The device is disabled and no logins are possible when this is set.
+  // State that is relevant only when the |restore_mode| is
+  // |RESTORE_MODE_DISABLED|.
   optional DisabledState disabled_state = 3;
 }