Abandon user sign in when policy is retrieved before session started

chrome/browser/chromeos/settings/session_manager_operation.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/session_manager_operation.h"
 
 #include <utility>
 
 #include "base/bind.h"
 #include "base/bind_helpers.h"
@@ skipping 121 matching lines @@
 }
 
 void SessionManagerOperation::RetrieveDeviceSettings() {
   session_manager_client()->RetrieveDevicePolicy(
       base::Bind(&SessionManagerOperation::ValidateDeviceSettings,
                  weak_factory_.GetWeakPtr()));
 }
 
 void SessionManagerOperation::BlockingRetrieveDeviceSettings() {
   ValidateDeviceSettings(
-      session_manager_client()->BlockingRetrieveDevicePolicy());
+      session_manager_client()->BlockingRetrieveDevicePolicy(),
+      SessionManagerClient::SUCCESS);

[emaxx, 2017/04/18 15:03:17]

Same comment as was above for DeviceLocalAccountPolicyStore: it's better to
obtain the actual response type instead of injecting the possibly untrue
"SUCCESS" flag.

[igorcov, 2017/04/20 14:52:29]

Done.

 }
 
 void SessionManagerOperation::ValidateDeviceSettings(
-    const std::string& policy_blob) {
+    const std::string& policy_blob,
+    const chromeos::SessionManagerClient::RetrievePolicyResponseType&
+        response_type) {
   std::unique_ptr<em::PolicyFetchResponse> policy(
       new em::PolicyFetchResponse());
-  if (policy_blob.empty()) {
+  if (policy_blob.empty() && response_type == SessionManagerClient::SUCCESS) {
     ReportResult(DeviceSettingsService::STORE_NO_POLICY);
     return;
   }
 
-  if (!policy->ParseFromString(policy_blob) ||
-      !policy->IsInitialized()) {
+  if (!policy->ParseFromString(policy_blob) || !policy->IsInitialized() ||
+      response_type != SessionManagerClient::SUCCESS) {

[emaxx, 2017/04/18 15:03:17]

nit: Maybe move the response_type comparison into the beginning of the
condition, so that the parsing is skipped completely in case of errors?

[igorcov, 2017/04/20 14:52:29]

Didn't notice that, thanks.

     ReportResult(DeviceSettingsService::STORE_INVALID_POLICY);
     return;
   }
 
   base::SequencedWorkerPool* pool =
       content::BrowserThread::GetBlockingPool();
   scoped_refptr<base::SequencedTaskRunner> background_task_runner =
       pool->GetSequencedTaskRunnerWithShutdownBehavior(
           pool->GetSequenceToken(),
           base::SequencedWorkerPool::SKIP_ON_SHUTDOWN);
@@ skipping 107 matching lines @@
 }
 
 void StoreSettingsOperation::HandleStoreResult(bool success) {
   if (!success)
     ReportResult(DeviceSettingsService::STORE_OPERATION_FAILED);
   else
     StartLoading();
 }
 
 }  // namespace chromeos