Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(958)

Unified Diff: chrome/browser/chromeos/policy/device_cloud_policy_store_chromeos.cc

Issue 400623002: Be more picky in triggering enrollment recovery. (Closed) Base URL: https://chromium.googlesource.com/chromium/src.git@master
Patch Set: Address Julian's comment. Created 6 years, 5 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
« no previous file with comments | « chrome/browser/chromeos/policy/device_cloud_policy_store_chromeos.h ('k') | no next file » | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: chrome/browser/chromeos/policy/device_cloud_policy_store_chromeos.cc
diff --git a/chrome/browser/chromeos/policy/device_cloud_policy_store_chromeos.cc b/chrome/browser/chromeos/policy/device_cloud_policy_store_chromeos.cc
index 13f49924d0341885f241cab5b2d543751189915c..2136a10ccc3a6581ca7fd529266e94ec13aa32c4 100644
--- a/chrome/browser/chromeos/policy/device_cloud_policy_store_chromeos.cc
+++ b/chrome/browser/chromeos/policy/device_cloud_policy_store_chromeos.cc
@@ -26,7 +26,7 @@ DeviceCloudPolicyStoreChromeOS::DeviceCloudPolicyStoreChromeOS(
: device_settings_service_(device_settings_service),
install_attributes_(install_attributes),
background_task_runner_(background_task_runner),
- first_update_(true),
+ enrollment_validation_done_(false),
weak_factory_(this) {
device_settings_service_->AddObserver(this);
}
@@ -138,32 +138,47 @@ void DeviceCloudPolicyStoreChromeOS::UpdateFromService() {
return;
}
- // Fill UMA histogram once per session. Skip temp validation error because it
- // is not a definitive result (policy load will be retried).
+ // Once per session, validate internal consistency of enrollment state (DM
+ // token must be present on enrolled devices) and in case of failure set flag
+ // to indicate that recovery is required.
const chromeos::DeviceSettingsService::Status status =
device_settings_service_->status();
- if (first_update_ &&
- status != chromeos::DeviceSettingsService::STORE_TEMP_VALIDATION_ERROR) {
- first_update_ = false;
- const bool has_dm_token =
- status == chromeos::DeviceSettingsService::STORE_SUCCESS &&
- device_settings_service_->policy_data() &&
- device_settings_service_->policy_data()->has_request_token();
-
- // At the time LoginDisplayHostImpl decides whether enrollment flow is to be
- // started, policy hasn't been read yet, so LoginDisplayHostImpl is not in a
- // position to decide whether recovery is required. To work around this,
- // upon policy load on machines requiring recovery, a flag is stored in
- // prefs which is accessed by LoginDisplayHostImpl early during (next) boot.
- if (!has_dm_token) {
- LOG(ERROR) << "Policy read on enrolled device yields no DM token! "
- << "Status: " << status << ".";
- chromeos::StartupUtils::MarkEnrollmentRecoveryRequired();
+ switch (status) {
+ case chromeos::DeviceSettingsService::STORE_SUCCESS:
+ case chromeos::DeviceSettingsService::STORE_KEY_UNAVAILABLE:
+ case chromeos::DeviceSettingsService::STORE_NO_POLICY:
+ case chromeos::DeviceSettingsService::STORE_INVALID_POLICY:
+ case chromeos::DeviceSettingsService::STORE_VALIDATION_ERROR: {
+ if (!enrollment_validation_done_) {
+ enrollment_validation_done_ = true;
+ const bool has_dm_token =
+ status == chromeos::DeviceSettingsService::STORE_SUCCESS &&
+ device_settings_service_->policy_data() &&
+ device_settings_service_->policy_data()->has_request_token();
+
+ // At the time LoginDisplayHostImpl decides whether enrollment flow is
+ // to be started, policy hasn't been read yet. To work around this,
+ // once the need for recovery is detected upon policy load, a flag is
+ // stored in prefs which is accessed by LoginDisplayHostImpl early
+ // during (next) boot.
+ if (!has_dm_token) {
+ LOG(ERROR) << "Device policy read on enrolled device yields "
+ << "no DM token! Status: " << status << ".";
+ chromeos::StartupUtils::MarkEnrollmentRecoveryRequired();
+ }
+ UMA_HISTOGRAM_BOOLEAN("Enterprise.EnrolledPolicyHasDMToken",
+ has_dm_token);
+ }
+ break;
}
- UMA_HISTOGRAM_BOOLEAN("Enterprise.EnrolledPolicyHasDMToken", has_dm_token);
+ case chromeos::DeviceSettingsService::STORE_POLICY_ERROR:
+ case chromeos::DeviceSettingsService::STORE_OPERATION_FAILED:
+ case chromeos::DeviceSettingsService::STORE_TEMP_VALIDATION_ERROR:
+ // Do nothing for write errors or transient read errors.
+ break;
}
- switch (device_settings_service_->status()) {
+ switch (status) {
case chromeos::DeviceSettingsService::STORE_SUCCESS: {
status_ = STATUS_OK;
policy_.reset(new em::PolicyData());
« no previous file with comments | « chrome/browser/chromeos/policy/device_cloud_policy_store_chromeos.h ('k') | no next file » | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698