buffet: Expose RegistrationStatus over DBus
This new property lets applications monitor Buffet's connection to
cloud services.
BUG=brillo:16
TEST=Unittests, buffet_Registration has been expanded appropriately.
CQ-DEPEND=CL:*199337
Change-Id: I30253e8199cb65068a74dd8b780a8ab0954bf9fa
Reviewed-on: https://chromium-review.googlesource.com/250011
Reviewed-by: Alex Vakulenko <avakulenko@chromium.org>
Tested-by: Christopher Wiley <wiley@chromium.org>
Commit-Queue: Christopher Wiley <wiley@chromium.org>
(cherry picked from commit d51132a3768ab7bbeb96241a9a63ddf338d8e45d)
Reviewed-on: https://chromium-review.googlesource.com/255923
Reviewed-by: Christopher Wiley <wiley@chromium.org>
Commit-Queue: Patrick Sosinski <sosinski@google.com>
Tested-by: Patrick Sosinski <sosinski@google.com>
diff --git a/buffet/buffet.gyp b/buffet/buffet.gyp
index 6628f4d..51312e5 100644
--- a/buffet/buffet.gyp
+++ b/buffet/buffet.gyp
@@ -37,6 +37,7 @@
'dbus_bindings/org.chromium.Buffet.Manager.xml',
'dbus_constants.cc',
'manager.cc',
+ 'registration_status.cc',
'storage_impls.cc',
'states/error_codes.cc',
'states/state_change_queue.cc',
diff --git a/buffet/buffet_client.cc b/buffet/buffet_client.cc
index 10efa6e..5a8a72a 100644
--- a/buffet/buffet_client.cc
+++ b/buffet/buffet_client.cc
@@ -146,7 +146,12 @@
return return_code;
object_manager_.reset(new org::chromium::Buffet::ObjectManagerProxy{bus_});
- manager_proxy_.reset(new org::chromium::Buffet::ManagerProxy{bus_});
+ auto manager_instances = object_manager_->GetManagerInstances();
+ if (manager_instances.empty()) {
+ fprintf(stderr, "Buffet daemon was offline.");
+ return EX_UNAVAILABLE;
+ }
+ manager_proxy_ = manager_instances.front();
auto args = CommandLine::ForCurrentProcess()->GetArgs();
@@ -361,8 +366,8 @@
}
std::unique_ptr<org::chromium::Buffet::ObjectManagerProxy> object_manager_;
- std::unique_ptr<org::chromium::Buffet::ManagerProxy> manager_proxy_;
- int exit_code_ = EX_OK;
+ org::chromium::Buffet::ManagerProxy* manager_proxy_{nullptr};
+ int exit_code_{EX_OK};
DISALLOW_COPY_AND_ASSIGN(Daemon);
};
diff --git a/buffet/dbus_bindings/org.chromium.Buffet.Manager.xml b/buffet/dbus_bindings/org.chromium.Buffet.Manager.xml
index daf1458..ea5e574 100644
--- a/buffet/dbus_bindings/org.chromium.Buffet.Manager.xml
+++ b/buffet/dbus_bindings/org.chromium.Buffet.Manager.xml
@@ -41,5 +41,26 @@
<arg name="echoed_message" type="s" direction="out"/>
<annotation name="org.chromium.DBus.Method.Kind" value="simple"/>
</method>
+
+ <property name="Status" type="s" access="read">
+ <tp:docstring>
+ State of Buffet's cloud registration.
+ Possible values include:
+
+ "offline": Buffet has credentials, but no connectivity to the Internet.
+ "cloud_error": Buffet has credentials, but cannot contact cloud services
+ to verify their validity. This could indicate that cloud
+ services are down, or that DNS is not resolving.
+ "invalid_credentials": Buffet has credentials, but they are no longer
+ valid.
+ "unregistered": Buffet has no credentials, either from an out of
+ box state, or because those credentials have been
+ rejected by the cloud service. Note that we
+ can unregistered with or without Internet connectivity.
+ "registering": Buffet has been provided with credentials, and is
+ registering with the cloud.
+ "registered": Buffet is online and registered with cloud services.
+ </tp:docstring>
+ </property>
</interface>
</node>
diff --git a/buffet/device_registration_info.cc b/buffet/device_registration_info.cc
index 3e8fd00..16f576a 100644
--- a/buffet/device_registration_info.cc
+++ b/buffet/device_registration_info.cc
@@ -146,12 +146,14 @@
const std::shared_ptr<StateManager>& state_manager,
std::unique_ptr<chromeos::KeyValueStore> config_store,
const std::shared_ptr<chromeos::http::Transport>& transport,
- const std::shared_ptr<StorageInterface>& state_store)
+ const std::shared_ptr<StorageInterface>& state_store,
+ const StatusHandler& status_handler)
: transport_{transport},
storage_{state_store},
command_manager_{command_manager},
state_manager_{state_manager},
- config_store_{std::move(config_store)} {
+ config_store_{std::move(config_store)},
+ registration_status_handler_{status_handler} {
}
DeviceRegistrationInfo::~DeviceRegistrationInfo() = default;
@@ -251,6 +253,9 @@
description_ = description;
location_ = location;
+ if (HaveRegistrationCredentials(nullptr))
+ SetRegistrationStatus(RegistrationStatus::kOffline);
+
return true;
}
@@ -286,19 +291,23 @@
}
bool DeviceRegistrationInfo::CheckRegistration(chromeos::ErrorPtr* error) {
- LOG(INFO) << "Checking device registration record.";
- if (refresh_token_.empty() ||
- device_id_.empty() ||
- device_robot_account_.empty()) {
- LOG(INFO) << "No valid device registration record found.";
+ return HaveRegistrationCredentials(error) &&
+ ValidateAndRefreshAccessToken(error);
+}
+
+bool DeviceRegistrationInfo::HaveRegistrationCredentials(
+ chromeos::ErrorPtr* error) {
+ const bool have_credentials = !refresh_token_.empty() &&
+ !device_id_.empty() &&
+ !device_robot_account_.empty();
+
+ VLOG(1) << "Device registration record "
+ << ((have_credentials) ? "found" : "not found.");
+ if (!have_credentials)
chromeos::Error::AddTo(error, FROM_HERE, kErrorDomainGCD,
"device_not_registered",
"No valid device registration record found");
- return false;
- }
-
- LOG(INFO) << "Device registration record found.";
- return ValidateAndRefreshAccessToken(error);
+ return have_credentials;
}
bool DeviceRegistrationInfo::ValidateAndRefreshAccessToken(
@@ -543,6 +552,7 @@
// We're going to respond with our success immediately and we'll StartDevice
// shortly after.
ScheduleStartDevice(base::TimeDelta::FromSeconds(0));
+ SetRegistrationStatus(RegistrationStatus::kRegistered);
return device_id_;
}
@@ -704,7 +714,6 @@
void DeviceRegistrationInfo::StartDevice(chromeos::ErrorPtr* error) {
if (!CheckRegistration(error))
return;
-
base::Bind(
&DeviceRegistrationInfo::UpdateDeviceResource,
base::Unretained(this),
@@ -820,6 +829,10 @@
base::Bind(&DeviceRegistrationInfo::PublishStateUpdates,
base::Unretained(this)),
base::TimeDelta::FromSeconds(7));
+ // TODO(wiley): This is the very bare minimum of state to report to local
+ // services interested in our GCD state. Build a more
+ // robust model of our state with respect to the server.
+ SetRegistrationStatus(RegistrationStatus::kRegistered);
}
void DeviceRegistrationInfo::PublishCommands(const base::ListValue& commands) {
@@ -915,4 +928,14 @@
return false;
}
+void DeviceRegistrationInfo::SetRegistrationStatus(
+ RegistrationStatus new_status) {
+ if (new_status == registration_status_)
+ return;
+ VLOG(1) << "Changing registration status to " << StatusToString(new_status);
+ registration_status_ = new_status;
+ if (!registration_status_handler_.is_null())
+ registration_status_handler_.Run(registration_status_);
+}
+
} // namespace buffet
diff --git a/buffet/device_registration_info.h b/buffet/device_registration_info.h
index 4409d25..95c8b0d 100644
--- a/buffet/device_registration_info.h
+++ b/buffet/device_registration_info.h
@@ -19,6 +19,7 @@
#include <chromeos/errors/error.h>
#include <chromeos/http/http_transport.h>
+#include "buffet/registration_status.h"
#include "buffet/storage_interface.h"
#include "buffet/xmpp/xmpp_client.h"
@@ -44,13 +45,15 @@
public:
// This is a helper class for unit testing.
class TestHelper;
+ using StatusHandler = base::Callback<void(RegistrationStatus)>;
DeviceRegistrationInfo(
const std::shared_ptr<CommandManager>& command_manager,
const std::shared_ptr<StateManager>& state_manager,
std::unique_ptr<chromeos::KeyValueStore> config_store,
const std::shared_ptr<chromeos::http::Transport>& transport,
- const std::shared_ptr<StorageInterface>& state_store);
+ const std::shared_ptr<StorageInterface>& state_store,
+ const StatusHandler& status_handler);
~DeviceRegistrationInfo() override;
@@ -60,6 +63,11 @@
LOG(FATAL) << "No write watcher is configured";
}
+ // Returns our current best known registration status.
+ RegistrationStatus GetRegistrationStatus() const {
+ return registration_status_;
+ }
+
// Returns the authorization HTTP header that can be used to talk
// to GCD server for authenticated device communication.
// Make sure ValidateAndRefreshAccessToken() is called before this call.
@@ -136,6 +144,9 @@
// Saves the device registration to cache.
bool Save() const;
+ // Checks whether we have credentials generated during registration.
+ bool HaveRegistrationCredentials(chromeos::ErrorPtr* error);
+
// Makes sure the access token is available and up-to-date.
bool ValidateAndRefreshAccessToken(chromeos::ErrorPtr* error);
@@ -191,6 +202,8 @@
std::unique_ptr<base::DictionaryValue> BuildDeviceResource(
chromeos::ErrorPtr* error);
+ void SetRegistrationStatus(RegistrationStatus new_status);
+
std::unique_ptr<XmppClient> xmpp_client_;
base::MessageLoopForIO::FileDescriptorWatcher fd_watcher_;
@@ -225,6 +238,10 @@
// Buffet configuration.
std::unique_ptr<chromeos::KeyValueStore> config_store_;
+ // Tracks our current registration status.
+ RegistrationStatus registration_status_{RegistrationStatus::kUnregistered};
+ StatusHandler registration_status_handler_;
+
friend class TestHelper;
base::WeakPtrFactory<DeviceRegistrationInfo> weak_factory_{this};
DISALLOW_COPY_AND_ASSIGN(DeviceRegistrationInfo);
diff --git a/buffet/device_registration_info_unittest.cc b/buffet/device_registration_info_unittest.cc
index ee49b0a..dee5614 100644
--- a/buffet/device_registration_info_unittest.cc
+++ b/buffet/device_registration_info_unittest.cc
@@ -189,12 +189,18 @@
config_store->SetString("model_id", "AAA");
config_store->SetString("oauth_url", test_data::kOAuthURL);
config_store->SetString("service_url", test_data::kServiceURL);
+ auto mock_callback = base::Bind(
+ &DeviceRegistrationInfoTest::OnRegistrationStatusChange,
+ base::Unretained(this));
dev_reg_ = std::unique_ptr<DeviceRegistrationInfo>(
new DeviceRegistrationInfo(command_manager_, state_manager_,
std::move(config_store),
- transport_, storage_));
+ transport_, storage_,
+ mock_callback));
}
+ MOCK_METHOD1(OnRegistrationStatusChange, void(RegistrationStatus));
+
base::DictionaryValue data_;
std::shared_ptr<MemStorage> storage_;
std::shared_ptr<chromeos::http::fake::Transport> transport_;
@@ -425,6 +431,7 @@
EXPECT_EQ(1,
storage_->save_count()); // The device info must have been saved.
EXPECT_EQ(3, transport_->GetRequestCount());
+ EXPECT_EQ(RegistrationStatus::kRegistered, dev_reg_->GetRegistrationStatus());
// Validate the device info saved to storage...
auto storage_data = storage_->Load();
@@ -449,4 +456,17 @@
EXPECT_EQ(test_data::kServiceURL, value);
}
+TEST_F(DeviceRegistrationInfoTest, OOBRegistrationStatus) {
+ // After we've been initialized, we should be either offline or unregistered,
+ // depending on whether or not we've found credentials.
+ EXPECT_TRUE(dev_reg_->Load());
+ EXPECT_EQ(RegistrationStatus::kUnregistered,
+ dev_reg_->GetRegistrationStatus());
+ // Put some credentials into our state, make sure we call that offline.
+ SetDefaultDeviceRegistration(&data_);
+ storage_->Save(&data_);
+ EXPECT_TRUE(dev_reg_->Load());
+ EXPECT_EQ(RegistrationStatus::kOffline, dev_reg_->GetRegistrationStatus());
+}
+
} // namespace buffet
diff --git a/buffet/manager.cc b/buffet/manager.cc
index a1457c4..2e96fbc 100644
--- a/buffet/manager.cc
+++ b/buffet/manager.cc
@@ -63,12 +63,16 @@
// TODO(avakulenko): Figure out security implications of storing
// device info state data unencrypted.
device_info_ = std::unique_ptr<DeviceRegistrationInfo>(
- new DeviceRegistrationInfo(command_manager_,
- state_manager_,
- std::move(config_store),
- chromeos::http::Transport::CreateDefault(),
- std::move(state_store)));
+ new DeviceRegistrationInfo(
+ command_manager_,
+ state_manager_,
+ std::move(config_store),
+ chromeos::http::Transport::CreateDefault(),
+ std::move(state_store),
+ base::Bind(&Manager::OnRegistrationStatusChange,
+ base::Unretained(this))));
device_info_->Load();
+ OnRegistrationStatusChange(device_info_->GetRegistrationStatus());
// Wait a significant amount of time for local daemons to publish their
// state to Buffet before publishing it to the cloud.
// TODO(wiley) We could do a lot of things here to either expose this
@@ -218,4 +222,8 @@
return message;
}
+void Manager::OnRegistrationStatusChange(RegistrationStatus status) {
+ dbus_adaptor_.SetStatus(StatusToString(status));
+}
+
} // namespace buffet
diff --git a/buffet/manager.h b/buffet/manager.h
index d7d71bf..1c891ec 100644
--- a/buffet/manager.h
+++ b/buffet/manager.h
@@ -75,6 +75,8 @@
// Handles calls to org.chromium.Buffet.Manager.Test()
std::string TestMethod(const std::string& message) override;
+ void OnRegistrationStatusChange(RegistrationStatus status);
+
org::chromium::Buffet::ManagerAdaptor dbus_adaptor_{this};
chromeos::dbus_utils::DBusObject dbus_object_;
diff --git a/buffet/registration_status.cc b/buffet/registration_status.cc
new file mode 100644
index 0000000..4fb1721
--- /dev/null
+++ b/buffet/registration_status.cc
@@ -0,0 +1,25 @@
+// Copyright 2015 The Chromium OS 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 "buffet/registration_status.h"
+
+namespace buffet {
+
+std::string StatusToString(RegistrationStatus status) {
+ switch (status) {
+ case RegistrationStatus::kOffline:
+ return "offline";
+ case RegistrationStatus::kCloudError:
+ return "cloud_error";
+ case RegistrationStatus::kUnregistered:
+ return "unregistered";
+ case RegistrationStatus::kRegistering:
+ return "registering";
+ case RegistrationStatus::kRegistered:
+ return "registered";
+ }
+ return "unknown";
+}
+
+} // namespace buffet
diff --git a/buffet/registration_status.h b/buffet/registration_status.h
new file mode 100644
index 0000000..fa2e2cb
--- /dev/null
+++ b/buffet/registration_status.h
@@ -0,0 +1,25 @@
+// Copyright 2015 The Chromium OS Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#ifndef BUFFET_REGISTRATION_STATUS_H_
+#define BUFFET_REGISTRATION_STATUS_H_
+
+#include <string>
+
+namespace buffet {
+
+// See the DBus interface XML file for complete descriptions of these states.
+enum class RegistrationStatus {
+ kOffline, // We have credentials but are offline.
+ kCloudError, // We're online, but can't talk to cloud services.
+ kUnregistered, // We have no credentials.
+ kRegistering, // We've just been given credentials.
+ kRegistered, // We're registered and online.
+};
+
+std::string StatusToString(RegistrationStatus status);
+
+} // namespace buffet
+
+#endif // BUFFET_REGISTRATION_STATUS_H_
diff --git a/privetd/cloud_delegate.cc b/privetd/cloud_delegate.cc
index 7009e4d..f6ebde1 100644
--- a/privetd/cloud_delegate.cc
+++ b/privetd/cloud_delegate.cc
@@ -26,47 +26,31 @@
const int kMaxSetupRetries = 5;
const int kFirstRetryTimeoutMs = 100;
+const char kBuffetStatus[] = "Status";
class CloudDelegateImpl : public CloudDelegate {
public:
CloudDelegateImpl(const scoped_refptr<dbus::Bus>& bus,
DeviceDelegate* device,
const base::Closure& on_changed)
- : manager_proxy_{bus}, device_{device}, on_changed_{on_changed} {}
+ : object_manager_{bus}, device_{device}, on_changed_{on_changed} {
+ object_manager_.SetManagerAddedCallback(
+ base::Bind(&CloudDelegateImpl::OnManagerAdded,
+ weak_factory_.GetWeakPtr()));
+ object_manager_.SetManagerRemovedCallback(
+ base::Bind(&CloudDelegateImpl::OnManagerRemoved,
+ weak_factory_.GetWeakPtr()));
+ }
~CloudDelegateImpl() override = default;
- bool Init() {
- // TODO(vitalybuka): check if buffet available and return false if missing.
- ErrorPtr error;
- // TODO(vitalybuka): monitor registration status.
- if (!manager_proxy_.CheckDeviceRegistered(&cloud_id_, &error)) {
- LOG(ERROR) << "CheckDeviceRegistered failed:" << error->GetMessage();
- state_ = ConnectionState(ConnectionState::kUnconfigured);
- return true;
- }
-
- std::string json;
- // TODO(vitalybuka): monitor device status using state of GCD notification
- // channel.
- if (!manager_proxy_.GetDeviceInfo(&json, &error)) {
- LOG(ERROR) << "GetDeviceInfo failed:" << error->GetMessage();
- state_ = ConnectionState(ConnectionState::kOffline);
- return true;
- }
-
- LOG(ERROR) << "GetDeviceInfo:" << json;
-
- state_ = ConnectionState(ConnectionState::kOnline);
- return true;
- }
-
ConnectionState GetConnectionState() const override { return state_; }
SetupState GetSetupState() const override { return setup_state_; }
bool Setup(const std::string& ticket_id, const std::string& user) override {
- if (setup_state_.status == SetupState::kInProgress)
+ if (setup_state_.status == SetupState::kInProgress ||
+ GetManagerProxy() == nullptr)
return false;
VLOG(1) << "GCD Setup started. ticket_id: " << ticket_id
<< ", user:" << user;
@@ -85,7 +69,75 @@
std::string GetCloudId() const override { return cloud_id_; }
private:
+ org::chromium::Buffet::ManagerProxy* GetManagerProxy() {
+ if (!manager_path_.IsValid())
+ return nullptr;
+ return object_manager_.GetManagerProxy(manager_path_);
+ }
+
+ void OnManagerAdded(org::chromium::Buffet::ManagerProxy* manager) {
+ manager_path_ = manager->GetObjectPath();
+ manager->SetPropertyChangedCallback(
+ base::Bind(&CloudDelegateImpl::OnManagerPropertyChanged,
+ weak_factory_.GetWeakPtr()));
+ OnManagerPropertyChanged(manager, kBuffetStatus);
+ // TODO(wiley) Get the device id (cloud_id_) here if we're online
+ }
+
+ void OnManagerPropertyChanged(org::chromium::Buffet::ManagerProxy* manager,
+ const std::string& property_name) {
+ if (property_name != kBuffetStatus)
+ return;
+ std::string status{manager->status()};
+ if (status == "offline")
+ state_ = ConnectionState(ConnectionState::kOffline);
+ else if (status == "cloud_error")
+ state_ = ConnectionState(ConnectionState::kError);
+ else if (status == "unregistered")
+ state_ = ConnectionState(ConnectionState::kUnconfigured);
+ else if (status == "registering")
+ state_ = ConnectionState(ConnectionState::kConnecting);
+ else if (status == "registered")
+ state_ = ConnectionState(ConnectionState::kOnline);
+ else
+ state_ = ConnectionState(ConnectionState::kError);
+ on_changed_.Run();
+ }
+
+ void OnManagerRemoved(const dbus::ObjectPath& path) {
+ state_ = ConnectionState(ConnectionState::kOffline);
+ manager_path_ = dbus::ObjectPath();
+ on_changed_.Run();
+ }
+
+ void RetryRegister(const std::string& ticket_id,
+ int retries,
+ chromeos::Error* error) {
+ if (retries >= kMaxSetupRetries) {
+ setup_state_ = SetupState(Error::kServerError);
+ return;
+ }
+ base::MessageLoop::current()->PostDelayedTask(
+ FROM_HERE,
+ base::Bind(&CloudDelegateImpl::CallManagerRegisterDevice,
+ setup_weak_factory_.GetWeakPtr(), ticket_id, retries + 1),
+ base::TimeDelta::FromSeconds(kFirstRetryTimeoutMs * (1 << retries)));
+ }
+
+ void OnRegisterSuccess(const std::string& device_id) {
+ VLOG(1) << "Device registered: " << device_id;
+ cloud_id_ = device_id;
+ setup_state_ = SetupState(SetupState::kSuccess);
+ on_changed_.Run();
+ }
+
void CallManagerRegisterDevice(const std::string& ticket_id, int retries) {
+ auto manager_proxy = GetManagerProxy();
+ if (!manager_proxy) {
+ LOG(ERROR) << "Couldn't register because Buffet was offline.";
+ RetryRegister(ticket_id, retries, nullptr);
+ return;
+ }
VariantDictionary params{
{"ticket_id", ticket_id},
{"display_name", device_->GetName()},
@@ -93,29 +145,18 @@
{"location", device_->GetLocation()},
{"model_id", device_->GetModelId()},
};
-
- ErrorPtr error;
- // TODO(vitalybuka): async call with updating setup_state_ from result.
- if (!manager_proxy_.RegisterDevice(params, &cloud_id_, &error)) {
- LOG(ERROR) << "Failed to receive a response:" << error->GetMessage();
- if (retries >= kMaxSetupRetries) {
- setup_state_ = SetupState(Error::kServerError);
- return;
- }
- base::MessageLoop::current()->PostDelayedTask(
- FROM_HERE,
- base::Bind(&CloudDelegateImpl::CallManagerRegisterDevice,
- setup_weak_factory_.GetWeakPtr(), ticket_id, retries + 1),
- base::TimeDelta::FromSeconds(kFirstRetryTimeoutMs * (1 << retries)));
- return;
- }
- VLOG(1) << "Device registered: " << cloud_id_ << std::endl;
- setup_state_ = SetupState(SetupState::kSuccess);
-
- on_changed_.Run();
+ manager_proxy->RegisterDeviceAsync(
+ params,
+ base::Bind(&CloudDelegateImpl::OnRegisterSuccess,
+ setup_weak_factory_.GetWeakPtr()),
+ base::Bind(&CloudDelegateImpl::RetryRegister,
+ setup_weak_factory_.GetWeakPtr(),
+ ticket_id,
+ retries));
}
- org::chromium::Buffet::ManagerProxy manager_proxy_;
+ org::chromium::Buffet::ObjectManagerProxy object_manager_;
+ dbus::ObjectPath manager_path_;
DeviceDelegate* device_;
@@ -130,7 +171,11 @@
// Cloud ID if device is registered.
std::string cloud_id_;
+ // |setup_weak_factory_| tracks the lifetime of callbacks used in connection
+ // with a particular invocation of Setup().
base::WeakPtrFactory<CloudDelegateImpl> setup_weak_factory_{this};
+ // |weak_factory_| tracks the lifetime of |this|.
+ base::WeakPtrFactory<CloudDelegateImpl> weak_factory_{this};
};
} // namespace
@@ -146,11 +191,8 @@
const scoped_refptr<dbus::Bus>& bus,
DeviceDelegate* device,
const base::Closure& on_changed) {
- std::unique_ptr<CloudDelegateImpl> gcd(
+ return std::unique_ptr<CloudDelegateImpl>(
new CloudDelegateImpl(bus, device, on_changed));
- if (!gcd->Init())
- gcd.reset();
- return std::move(gcd);
}
} // namespace privetd
