Chromad: added AD Join ui, authpolicy_client

chromeos/dbus/authpolicy_client.cc

+// Copyright (c) 2016 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/authpolicy_client.h"
+
+#include <map>

[hashimoto, 2016/10/24 03:01:18]

Please remove unneeded include.

[Roman Sorokin (ftl), 2016/10/24 16:50:34]

Done.

+#include <memory>
+#include <utility>
+
+#include "base/bind.h"
+#include "base/location.h"
+#include "base/macros.h"
+#include "base/memory/weak_ptr.h"
+#include "base/message_loop/message_loop.h"
+#include "base/stl_util.h"
+#include "base/strings/stringprintf.h"
+#include "base/values.h"
+#include "dbus/bus.h"
+#include "dbus/message.h"
+#include "dbus/object_proxy.h"
+#include "dbus/values_util.h"
+#include "third_party/cros_system_api/dbus/service_constants.h"
+
+namespace chromeos {
+
+namespace {
+
+// TODO(rsorokin): switch to service constants when it's landed.

[hashimoto, 2016/10/24 03:01:18]

URL to the bug?

Please follow the naming rule
https://google.github.io/styleguide/cppguide.html#Constant_Names

[Roman Sorokin (ftl), 2016/10/24 16:50:34]

Done.

+const char* objPath = "/org/chromium/authpolicy/Domain";

[hashimoto, 2016/10/24 03:01:18]

This should be const char[].

[Roman Sorokin (ftl), 2016/10/24 16:50:34]

Done.

+const char* authInt = "org.chromium.authpolicy";
+const char* domainInt = "org.chromium.authpolicy.Domain";
+const char* joinMethod = "Join";

[hashimoto, 2016/10/24 03:01:18]

Add a blank line.

[Roman Sorokin (ftl), 2016/10/24 16:50:34]

Done.

+// AuthpolicyClient is used to communicate with the org.chromium.authpolicy
+// service.
+class AuthpolicyClientImpl : public AuthpolicyClient {
+ public:
+  AuthpolicyClientImpl() : bus_(NULL), weak_ptr_factory_(this) {}

[hashimoto, 2016/10/24 03:01:18]

nullptr instead of NULL.

[Roman Sorokin (ftl), 2016/10/24 16:50:34]

Done.

+
+  ~AuthpolicyClientImpl() override {}
+
+  void Join(const std::string& machine_name,
+            const std::string& user,
+            const std::string& password,
+            const JoinCallback& callback) override {
+    // TODO(rsorokin): make proper callbacks with errors.
+    int fd[2];
+    if (pipe(fd) < 0) {

[hashimoto, 2016/10/24 03:01:18]

Please move this pipe creation/preparation logic out of this class.

[Roman Sorokin (ftl), 2016/10/24 16:50:34]

Done.

+      LOG(ERROR) << "Join: pipes fail: " << strerror(errno);
+      return;
+    }
+
+    if (write(fd[1], password.c_str(), password.size()) <= 0) {
+      LOG(ERROR) << "Join: write fail";
+      return;
+    }
+
+    if (close(fd[1])) {

[hashimoto, 2016/10/24 03:01:18]

Use ScopedFD.

[Roman Sorokin (ftl), 2016/10/24 16:50:34]

Done.

+      LOG(ERROR) << "Join: close failed";
+    }
+
+    dbus::ObjectPath objectPath(objPath);
+    dbus::ObjectProxy* proxy = bus_->GetObjectProxy(authInt, objectPath);
+    dbus::MethodCall method_call(domainInt, joinMethod);
+    dbus::MessageWriter writer(&method_call);
+    writer.AppendString(machine_name);
+    writer.AppendString(user);
+    writer.AppendFileDescriptor(fd[0]);
+    proxy->CallMethod(&method_call, dbus::ObjectProxy::TIMEOUT_USE_DEFAULT,
+                      base::Bind(&AuthpolicyClientImpl::HandleJoinCallback,
+                                 weak_ptr_factory_.GetWeakPtr(), callback));
+    if (close(fd[0])) {
+      LOG(ERROR) << "Join: close failed";
+    }
+  }
+
+ protected:
+  void Init(dbus::Bus* bus) override { bus_ = bus; }
+
+ private:
+  void HandleJoinCallback(const JoinCallback& callback,
+                          dbus::Response* response) {
+    if (!response) {
+      LOG(ERROR) << "Join: Couldn't call to authpolicy";
+      callback.Run(1);

[hashimoto, 2016/10/24 03:01:18]

What does this "1" mean?

[Roman Sorokin (ftl), 2016/10/24 16:50:34]

Have not defined error codes yet. For now it's just |not success|
Added TODO

+      return;
+    }
+
+    dbus::MessageReader reader(response);
+    dbus::ObjectPath connection_settings_path;

[hashimoto, 2016/10/24 03:01:18]

These variables are unused.

[Roman Sorokin (ftl), 2016/10/24 16:50:34]

Done.

+    dbus::ObjectPath active_connection_path;
+    int res;
+    if (!reader.PopInt32(&res)) {
+      LOG(ERROR) << "Join: Couldn't get an error from the response";
+      callback.Run(1);
+      return;
+    }
+
+    callback.Run(res);
+  }
+
+  dbus::Bus* bus_;
+
+  // Note: This should remain the last member so it'll be destroyed and
+  // invalidate its weak pointers before any other members are destroyed.
+  base::WeakPtrFactory<AuthpolicyClientImpl> weak_ptr_factory_;
+
+  DISALLOW_COPY_AND_ASSIGN(AuthpolicyClientImpl);
+};
+
+}  // namespace
+
+AuthpolicyClient::AuthpolicyClient() {}
+
+AuthpolicyClient::~AuthpolicyClient() {}
+
+// static
+AuthpolicyClient* AuthpolicyClient::Create() {
+  return new AuthpolicyClientImpl();
+}
+
+}  // namespace chromeos