Updated SessionManagerClient to use the multi-profile user policy calls.

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 <map>
+
 #include "base/bind.h"
 #include "base/callback.h"
+#include "base/file_util.h"
+#include "base/files/file_path.h"
+#include "base/location.h"
+#include "base/path_service.h"
 #include "base/string_util.h"
+#include "base/threading/worker_pool.h"
+#include "chromeos/chromeos_paths.h"
 #include "dbus/bus.h"
 #include "dbus/message.h"
 #include "dbus/object_path.h"
 #include "dbus/object_proxy.h"
 #include "third_party/cros_system_api/dbus/service_constants.h"
 
 namespace chromeos {
 
 // The SessionManagerClient implementation used in production.
 class SessionManagerClientImpl : public SessionManagerClient {
@@ skipping 152 matching lines @@
         login_manager::kSessionManagerUnlockScreen);
   }
 
   virtual void NotifyLockScreenDismissed() OVERRIDE {
     SimpleMethodCallToSessionManager(
         login_manager::kSessionManagerHandleLockScreenDismissed);
   }
 
   virtual void RetrieveDevicePolicy(
       const RetrievePolicyCallback& callback) OVERRIDE {
-    CallRetrievePolicy(login_manager::kSessionManagerRetrievePolicy,
-                       callback);
+    dbus::MethodCall method_call(login_manager::kSessionManagerInterface,
+                                 login_manager::kSessionManagerRetrievePolicy);
+    session_manager_proxy_->CallMethod(
+        &method_call,
+        dbus::ObjectProxy::TIMEOUT_USE_DEFAULT,
+        base::Bind(&SessionManagerClientImpl::OnRetrievePolicy,
+                   weak_ptr_factory_.GetWeakPtr(),
+                   login_manager::kSessionManagerRetrievePolicy,
+                   callback));
   }
 
-  virtual void RetrieveUserPolicy(
+  virtual void RetrievePolicyForUser(
+      const std::string& username,
       const RetrievePolicyCallback& callback) OVERRIDE {
-    CallRetrievePolicy(login_manager::kSessionManagerRetrieveUserPolicy,
-                       callback);
+    CallRetrievePolicyForUsername(
+        login_manager::kSessionManagerRetrievePolicyForUser,
+        username,
+        callback);
   }
 
   virtual void RetrieveDeviceLocalAccountPolicy(
       const std::string& account_name,
       const RetrievePolicyCallback& callback) OVERRIDE {
-    dbus::MethodCall method_call(
-        login_manager::kSessionManagerInterface,
-        login_manager::kSessionManagerRetrieveDeviceLocalAccountPolicy);
-    dbus::MessageWriter writer(&method_call);
-    writer.AppendString(account_name);
-    session_manager_proxy_->CallMethod(
-        &method_call,
-        dbus::ObjectProxy::TIMEOUT_USE_DEFAULT,
-        base::Bind(
-            &SessionManagerClientImpl::OnRetrievePolicy,
-            weak_ptr_factory_.GetWeakPtr(),
-            login_manager::kSessionManagerRetrieveDeviceLocalAccountPolicy,
-            callback));
+    CallRetrievePolicyForUsername(
+        login_manager::kSessionManagerRetrieveDeviceLocalAccountPolicy,
+        account_name,
+        callback);
   }
 
   virtual void StoreDevicePolicy(const std::string& policy_blob,
                                  const StorePolicyCallback& callback) OVERRIDE {
-    CallStorePolicy(login_manager::kSessionManagerStorePolicy,
-                    policy_blob, callback);
+    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*>(policy_blob.data()), policy_blob.size());
+    session_manager_proxy_->CallMethod(
+        &method_call,
+        dbus::ObjectProxy::TIMEOUT_USE_DEFAULT,
+        base::Bind(&SessionManagerClientImpl::OnStorePolicy,
+                   weak_ptr_factory_.GetWeakPtr(),
+                   login_manager::kSessionManagerStorePolicy,
+                   callback));
   }
 
-  virtual void StoreUserPolicy(const std::string& policy_blob,
-                               const StorePolicyCallback& callback) OVERRIDE {
-    CallStorePolicy(login_manager::kSessionManagerStoreUserPolicy,
-                    policy_blob, callback);
+  virtual void StorePolicyForUser(
+      const std::string& username,
+      const std::string& policy_blob,
+      const std::string& ignored_policy_key,
+      const StorePolicyCallback& callback) OVERRIDE {
+    CallStorePolicyForUsername(login_manager::kSessionManagerStorePolicyForUser,
+                               username,
+                               policy_blob,
+                               callback);
   }
 
   virtual void StoreDeviceLocalAccountPolicy(
       const std::string& account_name,
       const std::string& policy_blob,
       const StorePolicyCallback& callback) OVERRIDE {
-    dbus::MethodCall method_call(
-        login_manager::kSessionManagerInterface,
-        login_manager::kSessionManagerStoreDeviceLocalAccountPolicy);
-    dbus::MessageWriter writer(&method_call);
-    writer.AppendString(account_name);
-    // static_cast does not work due to signedness.
-    writer.AppendArrayOfBytes(
-        reinterpret_cast<const uint8*>(policy_blob.data()), policy_blob.size());
-    session_manager_proxy_->CallMethod(
-        &method_call,
-        dbus::ObjectProxy::TIMEOUT_USE_DEFAULT,
-        base::Bind(
-            &SessionManagerClientImpl::OnStorePolicy,
-            weak_ptr_factory_.GetWeakPtr(),
-            login_manager::kSessionManagerStoreDeviceLocalAccountPolicy,
-            callback));
+    CallStorePolicyForUsername(
+        login_manager::kSessionManagerStoreDeviceLocalAccountPolicy,
+        account_name,
+        policy_blob,
+        callback);
   }
 
  private:
   // Makes a method call to the session manager with no arguments and no
   // response.
   void SimpleMethodCallToSessionManager(const std::string& method_name) {
     dbus::MethodCall method_call(login_manager::kSessionManagerInterface,
                                  method_name);
     session_manager_proxy_->CallMethod(
         &method_call,
         dbus::ObjectProxy::TIMEOUT_USE_DEFAULT,
         dbus::ObjectProxy::EmptyResponseCallback());
   }
 
-  // Helper for Retrieve{User,Device}Policy.
-  virtual void CallRetrievePolicy(const std::string& method_name,
-                                  const RetrievePolicyCallback& callback) {
+  // Helper for RetrieveDeviceLocalAccountPolicy and RetrievePolicyForUser.
+  void CallRetrievePolicyForUsername(const std::string& method_name,

[Mattias Nissler (ping if slow), 2013/05/10 12:40:15]

Can we rename to CallRetrievePolicyByUsername in order to use a name that is
maximum different from RetrievePolicyForUser?

[Joao da Silva, 2013/05/13 09:39:23]

Done.

+                                     const std::string& username,
+                                     const RetrievePolicyCallback& callback) {
     dbus::MethodCall method_call(login_manager::kSessionManagerInterface,
                                  method_name);
+    dbus::MessageWriter writer(&method_call);
+    writer.AppendString(username);
     session_manager_proxy_->CallMethod(
         &method_call,
         dbus::ObjectProxy::TIMEOUT_USE_DEFAULT,
-        base::Bind(&SessionManagerClientImpl::OnRetrievePolicy,
-                   weak_ptr_factory_.GetWeakPtr(),
-                   method_name,
-                   callback));
+        base::Bind(
+            &SessionManagerClientImpl::OnRetrievePolicy,
+            weak_ptr_factory_.GetWeakPtr(),
+            method_name,
+            callback));
   }
 
-  // Helper for Store{User,Device}Policy.
-  virtual void CallStorePolicy(const std::string& method_name,
-                               const std::string& policy_blob,
-                               const StorePolicyCallback& callback) {
+  void CallStorePolicyForUsername(const std::string& method_name,

[Mattias Nissler (ping if slow), 2013/05/10 12:40:15]

Ditto re nameing, i.e. CallStorePolicyByUsername

[Joao da Silva, 2013/05/13 09:39:23]

Done.

+                                  const std::string& username,
+                                  const std::string& policy_blob,
+                                  const StorePolicyCallback& callback) {
     dbus::MethodCall method_call(login_manager::kSessionManagerInterface,
                                  method_name);
     dbus::MessageWriter writer(&method_call);
+    writer.AppendString(username);
     // static_cast does not work due to signedness.
     writer.AppendArrayOfBytes(
         reinterpret_cast<const uint8*>(policy_blob.data()), policy_blob.size());
     session_manager_proxy_->CallMethod(
         &method_call,
         dbus::ObjectProxy::TIMEOUT_USE_DEFAULT,
-        base::Bind(&SessionManagerClientImpl::OnStorePolicy,
-                   weak_ptr_factory_.GetWeakPtr(),
-                   method_name,
-                   callback));
+        base::Bind(
+            &SessionManagerClientImpl::OnStorePolicy,
+            weak_ptr_factory_.GetWeakPtr(),
+            method_name,
+            callback));
   }
 
   // Called when kSessionManagerRestartJob method is complete.
   void OnRestartJob(dbus::Response* response) {
     LOG_IF(ERROR, !response)
         << "Failed to call "
         << login_manager::kSessionManagerRestartJob;
   }
 
   // Called when kSessionManagerStartSession method is complete.
@@ skipping 11 matching lines @@
   }
 
   // Called when kSessionManagerStopSession method is complete.
   void OnDeviceWipe(dbus::Response* response) {
     LOG_IF(ERROR, !response)
         << "Failed to call "
         << login_manager::kSessionManagerStartDeviceWipe;
   }
 
   // Called when kSessionManagerRetrievePolicy or
-  // kSessionManagerRetrieveUserPolicy  method is complete.
+  // kSessionManagerRetrievePolicyForUser method is complete.
   void OnRetrievePolicy(const std::string& method_name,
                         const RetrievePolicyCallback& callback,
                         dbus::Response* response) {
     if (!response) {
       LOG(ERROR) << "Failed to call " << method_name;
       callback.Run("");
       return;
     }
     dbus::MessageReader reader(response);
     uint8* values = NULL;
     size_t length = 0;
     if (!reader.PopArrayOfBytes(&values, &length)) {
       LOG(ERROR) << "Invalid response: " << response->ToString();
       callback.Run("");
       return;
     }
     // static_cast does not work due to signedness.
     std::string serialized_proto(reinterpret_cast<char*>(values), length);
     callback.Run(serialized_proto);
   }
 
-  // Called when kSessionManagerStorePolicy or kSessionManagerStoreUserPolicy
+  // 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;
     } else {
       dbus::MessageReader reader(response);
       if (!reader.PopBool(&success))
@@ skipping 61 matching lines @@
   // invalidate its weak pointers before any other members are destroyed.
   base::WeakPtrFactory<SessionManagerClientImpl> weak_ptr_factory_;
 
   DISALLOW_COPY_AND_ASSIGN(SessionManagerClientImpl);
 };
 
 // The SessionManagerClient implementation used on Linux desktop,
 // which does nothing.
 class SessionManagerClientStubImpl : public SessionManagerClient {
  public:
-  SessionManagerClientStubImpl() {}
+  SessionManagerClientStubImpl() {
+    // Make sure that there are no keys left over from a previous browser run.
+    base::FilePath user_policy_key_dir;
+    if (PathService::Get(chromeos::DIR_USER_POLICY_KEYS,
+                         &user_policy_key_dir)) {
+      base::WorkerPool::PostTask(
+          FROM_HERE,
+          base::Bind(base::IgnoreResult(&file_util::Delete),
+                     user_policy_key_dir, true),
+          false);
+    }
+  }
   virtual ~SessionManagerClientStubImpl() {}
 
   // SessionManagerClient overrides.
   virtual void AddObserver(Observer* observer) OVERRIDE {
     observers_.AddObserver(observer);
   }
   virtual void RemoveObserver(Observer* observer) OVERRIDE {
     observers_.RemoveObserver(observer);
   }
   virtual bool HasObserver(Observer* observer) OVERRIDE {
@@ skipping 15 matching lines @@
   virtual void RequestUnlockScreen() OVERRIDE {
     FOR_EACH_OBSERVER(Observer, observers_, UnlockScreen());
   }
   virtual void NotifyLockScreenDismissed() OVERRIDE {
     FOR_EACH_OBSERVER(Observer, observers_, ScreenIsUnlocked());
   }
   virtual void RetrieveDevicePolicy(
       const RetrievePolicyCallback& callback) OVERRIDE {
     callback.Run(device_policy_);
   }
-  virtual void RetrieveUserPolicy(
+  virtual void RetrievePolicyForUser(
+      const std::string& username,
       const RetrievePolicyCallback& callback) OVERRIDE {
-    callback.Run(user_policy_);
+    callback.Run(user_policies_[username]);
   }
   virtual void RetrieveDeviceLocalAccountPolicy(
       const std::string& account_name,
       const RetrievePolicyCallback& callback) OVERRIDE {
-    callback.Run("");
+    callback.Run(user_policies_[account_name]);
   }
   virtual void StoreDevicePolicy(const std::string& policy_blob,
                                  const StorePolicyCallback& callback) OVERRIDE {
     device_policy_ = policy_blob;
     callback.Run(true);
   }
-  virtual void StoreUserPolicy(const std::string& policy_blob,
-                               const StorePolicyCallback& callback) OVERRIDE {
-    user_policy_ = policy_blob;
-    callback.Run(true);
+  virtual void StorePolicyForUser(
+      const std::string& username,
+      const std::string& policy_blob,
+      const std::string& policy_key,
+      const StorePolicyCallback& callback) OVERRIDE {
+    if (policy_key.empty()) {
+      user_policies_[username] = policy_blob;
+      callback.Run(true);
+      return;
+    }
+    // The session manager writes the user policy key to a well-known
+    // location. Do the same with the stub impl, so that user policy works and
+    // can be tested on desktop builds.

[Mattias Nissler (ping if slow), 2013/05/10 12:40:15]

If you need this, why not extract the policy key from the policy blob here?

[Joao da Silva, 2013/05/13 09:39:23]

That'd be my preferred solution, but chromeos/ can't #include the policy
protobufs. There's no good place outside chrome/ to move them to for now.

[Mattias Nissler (ping if slow), 2013/05/13 10:27:14]

Bummer. Any chance to get an exception into chromeos/DEPS for the
device_management_backend.pb.h? That's not nice either, but more honest than
just  screwing up the interface here in order to be able to implement stubs.

[Joao da Silva, 2013/05/13 10:54:19]

Can we propose that change in another CL? I'd rather try to find a better
top-level dir for the protobufs.

+    base::FilePath key_path;
+    if (!PathService::Get(chromeos::DIR_USER_POLICY_KEYS, &key_path)) {
+      callback.Run(false);
+      return;
+    }
+    // Keep this in sync with CryptohomeClientStubImpl::GetSanitizedUsername.
+    const std::string sanitized = username + "-profile";
+    key_path = key_path.AppendASCII(sanitized).AppendASCII("policy.pub");

[Mattias Nissler (ping if slow), 2013/05/10 12:40:15]

This is misleading, please name it policy_key.pub or something.

[Joao da Silva, 2013/05/13 09:39:23]

This has to be the same name that the session_manager writes to.

[Mattias Nissler (ping if slow), 2013/05/13 10:27:14]

Oh right, I guess I should have payed closer attention in the review that added
this then :)

+    // Assume that the key write is successful.
+    user_policies_[username] = policy_blob;
+    base::WorkerPool::PostTaskAndReply(
+        FROM_HERE,
+        base::Bind(&SessionManagerClientStubImpl::StoreFileInBackground,
+                   key_path, policy_key),
+        base::Bind(callback, true),
+        false);
   }
   virtual void StoreDeviceLocalAccountPolicy(
       const std::string& account_name,
       const std::string& policy_blob,
       const StorePolicyCallback& callback) OVERRIDE {
+    user_policies_[account_name] = policy_blob;
     callback.Run(true);
   }
 
+  static void StoreFileInBackground(const base::FilePath& path,
+                                    const std::string& data) {
+    const int size = static_cast<int>(data.size());
+    if (!file_util::CreateDirectory(path.DirName()) ||
+        file_util::WriteFile(path, data.data(), size) != size) {
+      LOG(WARNING) << "Failed to write policy key to " << path.value();
+    }
+  }
+
  private:
   ObserverList<Observer> observers_;
   std::string device_policy_;
-  std::string user_policy_;
+  std::map<std::string, std::string> user_policies_;
 
   DISALLOW_COPY_AND_ASSIGN(SessionManagerClientStubImpl);
 };
 
 SessionManagerClient::SessionManagerClient() {
 }
 
 SessionManagerClient::~SessionManagerClient() {
 }
 
 SessionManagerClient* SessionManagerClient::Create(
     DBusClientImplementationType type,
     dbus::Bus* bus) {
   if (type == REAL_DBUS_CLIENT_IMPLEMENTATION)
     return new SessionManagerClientImpl(bus);
   DCHECK_EQ(STUB_DBUS_CLIENT_IMPLEMENTATION, type);
   return new SessionManagerClientStubImpl();
 }
 
 }  // namespace chromeos