Revert "Abandon user sign in when policy is retrieved before session started."

chromeos/dbus/session_manager_client.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 "chromeos/dbus/session_manager_client.h"
 
 #include <stddef.h>
 #include <stdint.h>
 
 #include <memory>
 
 #include "base/bind.h"
 #include "base/callback.h"
 #include "base/files/file_path.h"
 #include "base/files/file_util.h"
 #include "base/location.h"
 #include "base/macros.h"
-#include "base/metrics/histogram_macros.h"
 #include "base/path_service.h"
 #include "base/strings/string_number_conversions.h"
 #include "base/strings/string_util.h"
 #include "base/task_scheduler/post_task.h"
 #include "base/threading/thread_task_runner_handle.h"
 #include "chromeos/chromeos_paths.h"
 #include "chromeos/cryptohome/cryptohome_parameters.h"
 #include "chromeos/dbus/blocking_method_caller.h"
 #include "chromeos/dbus/cryptohome_client.h"
 #include "components/policy/proto/device_management_backend.pb.h"
 #include "crypto/sha2.h"
 #include "dbus/bus.h"
 #include "dbus/message.h"
 #include "dbus/object_path.h"
 #include "dbus/object_proxy.h"
-#include "dbus/scoped_dbus_error.h"
 #include "third_party/cros_system_api/dbus/service_constants.h"
 
 namespace chromeos {
 
 namespace {
 
-using RetrievePolicyResponseType =
-    SessionManagerClient::RetrievePolicyResponseType;
+// TODO(hidehiko): Share the constant between Chrome and ChromeOS.
+constexpr char kArcLowDiskError[] =
+    "org.chromium.SessionManagerInterface.LowFreeDisk";
 
 constexpr char kStubPolicyFile[] = "stub_policy";
 constexpr char kStubDevicePolicyFile[] = "stub_device_policy";
 constexpr char kStubStateKeysFile[] = "stub_state_keys";
 
 // Returns a location for |file| that is specific to the given |cryptohome_id|.
 // These paths will be relative to DIR_USER_POLICY_KEYS, and can be used only
 // to store stub files.
 base::FilePath GetUserFilePath(const cryptohome::Identification& cryptohome_id,
                                const char* file) {
@@ skipping 44 matching lines @@
   }
   return state_keys;
 }
 
 // Turn pass-by-value into pass-by-reference as expected by StateKeysCallback.
 void RunStateKeysCallbackStub(SessionManagerClient::StateKeysCallback callback,
                               std::vector<std::string> state_keys) {
   callback.Run(state_keys);
 }
 
-// Helper to notify the callback with SUCCESS, to be used by the stub.
-void NotifyOnRetrievePolicySuccess(
-    const SessionManagerClient::RetrievePolicyCallback& callback,
-    const std::string& protobuf) {
-  callback.Run(protobuf, RetrievePolicyResponseType::SUCCESS);
-}
-
-// Helper to get the enum type of RetrievePolicyResponseType based on error
-// name.
-RetrievePolicyResponseType GetResponseTypeBasedOnError(
-    base::StringPiece error_name) {
-  if (error_name == login_manager::dbus_error::kNone) {
-    return RetrievePolicyResponseType::SUCCESS;
-  } else if (error_name == login_manager::dbus_error::kSessionDoesNotExist) {
-    return RetrievePolicyResponseType::SESSION_DOES_NOT_EXIST;
-  } else if (error_name == login_manager::dbus_error::kSigEncodeFail) {
-    return RetrievePolicyResponseType::POLICY_ENCODE_ERROR;
-  }
-  return RetrievePolicyResponseType::OTHER_ERROR;
-}
-
-// Logs UMA stat for retrieve policy request, corresponding to D-Bus method name
-// used.
-void LogPolicyResponseUma(base::StringPiece method_name,
-                          RetrievePolicyResponseType response) {
-  if (method_name == login_manager::kSessionManagerRetrievePolicy) {
-    UMA_HISTOGRAM_ENUMERATION("Enterprise.RetrievePolicyResponse.Device",
-                              response, RetrievePolicyResponseType::COUNT);
-  } else if (method_name ==
-             login_manager::kSessionManagerRetrieveDeviceLocalAccountPolicy) {
-    UMA_HISTOGRAM_ENUMERATION(
-        "Enterprise.RetrievePolicyResponse.DeviceLocalAccount", response,
-        RetrievePolicyResponseType::COUNT);
-  } else if (method_name ==
-             login_manager::kSessionManagerRetrievePolicyForUser) {
-    UMA_HISTOGRAM_ENUMERATION("Enterprise.RetrievePolicyResponse.User",
-                              response, RetrievePolicyResponseType::COUNT);
-  } else {
-    LOG(ERROR) << "Invalid method_name: " << method_name;
-  }
-}
-
 }  // namespace
 
 // The SessionManagerClient implementation used in production.
 class SessionManagerClientImpl : public SessionManagerClient {
  public:
   SessionManagerClientImpl()
       : session_manager_proxy_(NULL),
         screen_is_locked_(false),
         weak_ptr_factory_(this) {}
 
@@ skipping 108 matching lines @@
         dbus::ObjectProxy::TIMEOUT_USE_DEFAULT,
         base::Bind(&SessionManagerClientImpl::OnRetrieveActiveSessions,
                    weak_ptr_factory_.GetWeakPtr(),
                    login_manager::kSessionManagerRetrieveActiveSessions,
                    callback));
   }
 
   void RetrieveDevicePolicy(const RetrievePolicyCallback& callback) override {
     dbus::MethodCall method_call(login_manager::kSessionManagerInterface,
                                  login_manager::kSessionManagerRetrievePolicy);
-    session_manager_proxy_->CallMethodWithErrorCallback(
-        &method_call, dbus::ObjectProxy::TIMEOUT_USE_DEFAULT,
-        base::Bind(&SessionManagerClientImpl::OnRetrievePolicySuccess,
+    session_manager_proxy_->CallMethod(
+        &method_call,
+        dbus::ObjectProxy::TIMEOUT_USE_DEFAULT,
+        base::Bind(&SessionManagerClientImpl::OnRetrievePolicy,
                    weak_ptr_factory_.GetWeakPtr(),
-                   login_manager::kSessionManagerRetrievePolicy, callback),
-        base::Bind(&SessionManagerClientImpl::OnRetrievePolicyError,
-                   weak_ptr_factory_.GetWeakPtr(),
-                   login_manager::kSessionManagerRetrievePolicy, callback));
+                   login_manager::kSessionManagerRetrievePolicy,
+                   callback));
   }
 
-  RetrievePolicyResponseType BlockingRetrieveDevicePolicy(
-      std::string* policy_out) override {
+  std::string BlockingRetrieveDevicePolicy() override {
     dbus::MethodCall method_call(login_manager::kSessionManagerInterface,
                                  login_manager::kSessionManagerRetrievePolicy);
-    dbus::ScopedDBusError error;
     std::unique_ptr<dbus::Response> response =
-        blocking_method_caller_->CallMethodAndBlockWithError(&method_call,
-                                                             &error);
-    RetrievePolicyResponseType result = RetrievePolicyResponseType::SUCCESS;
-    if (error.is_set() && error.name()) {
-      result = GetResponseTypeBasedOnError(error.name());
-    }
-    if (result == RetrievePolicyResponseType::SUCCESS) {
-      ExtractString(login_manager::kSessionManagerRetrievePolicy,
-                    response.get(), policy_out);
-    } else {
-      *policy_out = "";
-    }
-    LogPolicyResponseUma(login_manager::kSessionManagerRetrievePolicy, result);
-    return result;
+        blocking_method_caller_->CallMethodAndBlock(&method_call);
+    std::string policy;
+    ExtractString(login_manager::kSessionManagerRetrievePolicy, response.get(),
+                  &policy);
+    return policy;
   }
 
   void RetrievePolicyForUser(const cryptohome::Identification& cryptohome_id,
                              const RetrievePolicyCallback& callback) override {
     CallRetrievePolicyByUsername(
         login_manager::kSessionManagerRetrievePolicyForUser, cryptohome_id.id(),
         callback);
   }
 
-  RetrievePolicyResponseType BlockingRetrievePolicyForUser(
-      const cryptohome::Identification& cryptohome_id,
-      std::string* policy_out) override {
-    return BlockingRetrievePolicyByUsername(
-        login_manager::kSessionManagerRetrievePolicyForUser, cryptohome_id.id(),
-        policy_out);
+  std::string BlockingRetrievePolicyForUser(
+      const cryptohome::Identification& cryptohome_id) override {
+    dbus::MethodCall method_call(
+        login_manager::kSessionManagerInterface,
+        login_manager::kSessionManagerRetrievePolicyForUser);
+    dbus::MessageWriter writer(&method_call);
+    writer.AppendString(cryptohome_id.id());
+    std::unique_ptr<dbus::Response> response =
+        blocking_method_caller_->CallMethodAndBlock(&method_call);
+    std::string policy;
+    ExtractString(login_manager::kSessionManagerRetrievePolicyForUser,
+                  response.get(),
+                  &policy);
+    return policy;
   }
 
   void RetrieveDeviceLocalAccountPolicy(
       const std::string& account_name,
       const RetrievePolicyCallback& callback) override {
     CallRetrievePolicyByUsername(
         login_manager::kSessionManagerRetrieveDeviceLocalAccountPolicy,
-        account_name, callback);
+        account_name,
+        callback);
   }
 
-  RetrievePolicyResponseType BlockingRetrieveDeviceLocalAccountPolicy(
-      const std::string& account_name,
-      std::string* policy_out) override {
-    return BlockingRetrievePolicyByUsername(
+  std::string BlockingRetrieveDeviceLocalAccountPolicy(
+      const std::string& account_name) override {
+    dbus::MethodCall method_call(
+        login_manager::kSessionManagerInterface,
+        login_manager::kSessionManagerRetrieveDeviceLocalAccountPolicy);
+    dbus::MessageWriter writer(&method_call);
+    writer.AppendString(account_name);
+    std::unique_ptr<dbus::Response> response =
+        blocking_method_caller_->CallMethodAndBlock(&method_call);
+    std::string policy;
+    ExtractString(
         login_manager::kSessionManagerRetrieveDeviceLocalAccountPolicy,
-        account_name, policy_out);
+        response.get(), &policy);
+    return policy;
   }
 
   void StoreDevicePolicy(const std::string& policy_blob,
                          const StorePolicyCallback& callback) override {
     dbus::MethodCall method_call(login_manager::kSessionManagerInterface,
                                  login_manager::kSessionManagerStorePolicy);
     dbus::MessageWriter writer(&method_call);
     // static_cast does not work due to signedness.
     writer.AppendArrayOfBytes(
         reinterpret_cast<const uint8_t*>(policy_blob.data()),
@@ skipping 207 matching lines @@
   }
 
   // Helper for RetrieveDeviceLocalAccountPolicy and RetrievePolicyForUser.
   void CallRetrievePolicyByUsername(const std::string& method_name,
                                     const std::string& account_id,
                                     const RetrievePolicyCallback& callback) {
     dbus::MethodCall method_call(login_manager::kSessionManagerInterface,
                                  method_name);
     dbus::MessageWriter writer(&method_call);
     writer.AppendString(account_id);
-    session_manager_proxy_->CallMethodWithErrorCallback(
-        &method_call, dbus::ObjectProxy::TIMEOUT_USE_DEFAULT,
-        base::Bind(&SessionManagerClientImpl::OnRetrievePolicySuccess,
-                   weak_ptr_factory_.GetWeakPtr(), method_name, callback),
-        base::Bind(&SessionManagerClientImpl::OnRetrievePolicyError,
-                   weak_ptr_factory_.GetWeakPtr(), method_name, callback));
-  }
-
-  // Helper for blocking RetrievePolicyForUser and
-  // RetrieveDeviceLocalAccountPolicy.
-  RetrievePolicyResponseType BlockingRetrievePolicyByUsername(
-      const std::string& method_name,
-      const std::string& account_name,
-      std::string* policy_out) {
-    dbus::MethodCall method_call(login_manager::kSessionManagerInterface,
-                                 method_name);
-    dbus::MessageWriter writer(&method_call);
-    writer.AppendString(account_name);
-    dbus::ScopedDBusError error;
-    std::unique_ptr<dbus::Response> response =
-        blocking_method_caller_->CallMethodAndBlockWithError(&method_call,
-                                                             &error);
-    RetrievePolicyResponseType result = RetrievePolicyResponseType::SUCCESS;
-    if (error.is_set() && error.name()) {
-      result = GetResponseTypeBasedOnError(error.name());
-    }
-    if (result == RetrievePolicyResponseType::SUCCESS) {
-      ExtractString(method_name, response.get(), policy_out);
-    } else {
-      *policy_out = "";
-    }
-    LogPolicyResponseUma(method_name, result);
-    return result;
+    session_manager_proxy_->CallMethod(
+        &method_call,
+        dbus::ObjectProxy::TIMEOUT_USE_DEFAULT,
+        base::Bind(
+            &SessionManagerClientImpl::OnRetrievePolicy,
+            weak_ptr_factory_.GetWeakPtr(),
+            method_name,
+            callback));
   }
 
   void CallStorePolicyByUsername(const std::string& method_name,
                                  const std::string& account_id,
                                  const std::string& policy_blob,
                                  const StorePolicyCallback& callback) {
     dbus::MethodCall method_call(login_manager::kSessionManagerInterface,
                                  method_name);
     dbus::MessageWriter writer(&method_call);
     writer.AppendString(account_id);
@@ skipping 91 matching lines @@
     size_t length = 0;
     if (!reader.PopArrayOfBytes(&values, &length)) {
       LOG(ERROR) << "Invalid response: " << response->ToString();
       return;
     }
     // static_cast does not work due to signedness.
     extracted->assign(reinterpret_cast<const char*>(values), length);
   }
 
   // Called when kSessionManagerRetrievePolicy or
-  // kSessionManagerRetrievePolicyForUser method is successfully complete.
-  void OnRetrievePolicySuccess(const std::string& method_name,
-                               const RetrievePolicyCallback& callback,
-                               dbus::Response* response) {
+  // kSessionManagerRetrievePolicyForUser method is complete.
+  void OnRetrievePolicy(const std::string& method_name,
+                        const RetrievePolicyCallback& callback,
+                        dbus::Response* response) {
     std::string serialized_proto;
     ExtractString(method_name, response, &serialized_proto);
-    callback.Run(serialized_proto, RetrievePolicyResponseType::SUCCESS);
-
-    LogPolicyResponseUma(method_name, RetrievePolicyResponseType::SUCCESS);
-  }
-
-  // Called when kSessionManagerRetrievePolicy or
-  // kSessionManagerRetrievePolicyForUser method fails.
-  void OnRetrievePolicyError(const std::string& method_name,
-                             const RetrievePolicyCallback& callback,
-                             dbus::ErrorResponse* response) {
-    RetrievePolicyResponseType response_type =
-        GetResponseTypeBasedOnError(response->GetErrorName());
-    callback.Run(std::string(), response_type);
-
-    LogPolicyResponseUma(method_name, response_type);
+    callback.Run(serialized_proto);
   }
 
   // Called when kSessionManagerStorePolicy or kSessionManagerStorePolicyForUser
   // method is complete.
   void OnStorePolicy(const std::string& method_name,
                      const StorePolicyCallback& callback,
                      dbus::Response* response) {
     bool success = false;
     if (!response) {
       LOG(ERROR) << "Failed to call " << method_name;
@@ skipping 150 matching lines @@
   void OnStartArcInstanceSucceeded(const StartArcInstanceCallback& callback,
                                    dbus::Response* response) {
     if (!callback.is_null())
       callback.Run(StartArcInstanceResult::SUCCESS);
   }
 
   void OnStartArcInstanceFailed(const StartArcInstanceCallback& callback,
                                 dbus::ErrorResponse* response) {
     LOG(ERROR) << "Failed to call StartArcInstance: "
                << (response ? response->ToString() : "(null)");
-    if (!callback.is_null()) {
-      callback.Run(response && response->GetErrorName() ==
-                                   login_manager::dbus_error::kLowFreeDisk
+    if (!callback.is_null())
+      callback.Run(response && response->GetErrorName() == kArcLowDiskError
                        ? StartArcInstanceResult::LOW_FREE_DISK_SPACE
                        : StartArcInstanceResult::UNKNOWN_ERROR);
-    }
   }
 
   dbus::ObjectProxy* session_manager_proxy_;
   std::unique_ptr<BlockingMethodCaller> blocking_method_caller_;
   base::ObserverList<Observer> observers_;
 
   // Most recent screen-lock state received from session_manager.
   bool screen_is_locked_;
 
   // Note: This should remain the last member so it'll be destroyed and
@@ skipping 46 matching lines @@
   void NotifyLockScreenDismissed() override {
     screen_is_locked_ = false;
     for (auto& observer : observers_)
       observer.ScreenIsUnlocked();
   }
   void RetrieveActiveSessions(const ActiveSessionsCallback& callback) override {
   }
   void RetrieveDevicePolicy(const RetrievePolicyCallback& callback) override {
     base::FilePath owner_key_path;
     if (!PathService::Get(chromeos::FILE_OWNER_KEY, &owner_key_path)) {
-      callback.Run("", RetrievePolicyResponseType::SUCCESS);
+      callback.Run("");
       return;
     }
     base::FilePath device_policy_path =
         owner_key_path.DirName().AppendASCII(kStubDevicePolicyFile);
     base::PostTaskWithTraitsAndReplyWithResult(
-        FROM_HERE,
-        base::TaskTraits()
-            .WithShutdownBehavior(
-                base::TaskShutdownBehavior::CONTINUE_ON_SHUTDOWN)
-            .MayBlock(),
-        base::Bind(&GetFileContent, device_policy_path),
-        base::Bind(&NotifyOnRetrievePolicySuccess, callback));
+        FROM_HERE, base::TaskTraits()
+                       .WithShutdownBehavior(
+                           base::TaskShutdownBehavior::CONTINUE_ON_SHUTDOWN)
+                       .MayBlock(),
+        base::Bind(&GetFileContent, device_policy_path), callback);
   }
-  RetrievePolicyResponseType BlockingRetrieveDevicePolicy(
-      std::string* policy_out) override {
+  std::string BlockingRetrieveDevicePolicy() override {
     base::FilePath owner_key_path;
     if (!PathService::Get(chromeos::FILE_OWNER_KEY, &owner_key_path)) {
-      *policy_out = "";
-      return RetrievePolicyResponseType::SUCCESS;
+      return "";
     }
     base::FilePath device_policy_path =
         owner_key_path.DirName().AppendASCII(kStubDevicePolicyFile);
-    *policy_out = GetFileContent(device_policy_path);
-    return RetrievePolicyResponseType::SUCCESS;
+    return GetFileContent(device_policy_path);
   }
   void RetrievePolicyForUser(const cryptohome::Identification& cryptohome_id,
                              const RetrievePolicyCallback& callback) override {
     base::PostTaskWithTraitsAndReplyWithResult(
         FROM_HERE,
         base::TaskTraits()
             .WithShutdownBehavior(
                 base::TaskShutdownBehavior::CONTINUE_ON_SHUTDOWN)
             .MayBlock(),
         base::Bind(&GetFileContent,
                    GetUserFilePath(cryptohome_id, kStubPolicyFile)),
-        base::Bind(&NotifyOnRetrievePolicySuccess, callback));
+        callback);
   }
-  RetrievePolicyResponseType BlockingRetrievePolicyForUser(
-      const cryptohome::Identification& cryptohome_id,
-      std::string* policy_out) override {
-    *policy_out =
-        GetFileContent(GetUserFilePath(cryptohome_id, kStubPolicyFile));
-    return RetrievePolicyResponseType::SUCCESS;
+  std::string BlockingRetrievePolicyForUser(
+      const cryptohome::Identification& cryptohome_id) override {
+    return GetFileContent(GetUserFilePath(cryptohome_id, kStubPolicyFile));
   }
   void RetrieveDeviceLocalAccountPolicy(
       const std::string& account_id,
       const RetrievePolicyCallback& callback) override {
     RetrievePolicyForUser(cryptohome::Identification::FromString(account_id),
                           callback);
   }
-  RetrievePolicyResponseType BlockingRetrieveDeviceLocalAccountPolicy(
-      const std::string& account_id,
-      std::string* policy_out) override {
+  std::string BlockingRetrieveDeviceLocalAccountPolicy(
+      const std::string& account_id) override {
     return BlockingRetrievePolicyForUser(
-        cryptohome::Identification::FromString(account_id), policy_out);
+        cryptohome::Identification::FromString(account_id));
   }
   void StoreDevicePolicy(const std::string& policy_blob,
                          const StorePolicyCallback& callback) override {
     enterprise_management::PolicyFetchResponse response;
     base::FilePath owner_key_path;
     if (!response.ParseFromString(policy_blob) ||
         !PathService::Get(chromeos::FILE_OWNER_KEY, &owner_key_path)) {
       callback.Run(false);
       return;
     }
@@ skipping 138 matching lines @@
 
 SessionManagerClient* SessionManagerClient::Create(
     DBusClientImplementationType type) {
   if (type == REAL_DBUS_CLIENT_IMPLEMENTATION)
     return new SessionManagerClientImpl();
   DCHECK_EQ(FAKE_DBUS_CLIENT_IMPLEMENTATION, type);
   return new SessionManagerClientStubImpl();
 }
 
 }  // namespace chromeos