buffet: Merge registration start and finalization.
As currently user-initiated registration flow is recommended,
it allows us to simplify registration protocol to a single method.
BUG=None
TEST=cros_workon_make buffet --test && manual testing.
Change-Id: I1937b91f4f7b34585ce5b8cb1a4c26ec3a849865
Reviewed-on: https://chromium-review.googlesource.com/223700
Commit-Queue: Anton Muhin <antonm@chromium.org>
Tested-by: Anton Muhin <antonm@chromium.org>
Reviewed-by: Anton Muhin <antonm@chromium.org>
diff --git a/buffet/buffet_client.cc b/buffet/buffet_client.cc
index f8ef56e..e32082d 100644
--- a/buffet/buffet_client.cc
+++ b/buffet/buffet_client.cc
@@ -41,9 +41,8 @@
std::cerr << " " << kManagerStartDevice << std::endl;
std::cerr << " " << kManagerCheckDeviceRegistered << std::endl;
std::cerr << " " << kManagerGetDeviceInfo << std::endl;
- std::cerr << " " << kManagerStartRegisterDevice
+ std::cerr << " " << kManagerRegisterDevice
<< " param1 = val1¶m2 = val2..." << std::endl;
- std::cerr << " " << kManagerFinishRegisterDevice << std::endl;
std::cerr << " " << kManagerAddCommand
<< " '{\"name\":\"command_name\",\"parameters\":{}}'"
<< std::endl;
@@ -160,10 +159,10 @@
return EX_OK;
}
- int CallManagerStartRegisterDevice(const CommandLine::StringVector& args) {
+ int CallManagerRegisterDevice(const CommandLine::StringVector& args) {
if (args.size() > 1) {
std::cerr << "Invalid number of arguments for "
- << "Manager." << kManagerStartRegisterDevice << std::endl;
+ << "Manager." << kManagerRegisterDevice << std::endl;
usage();
return EX_USAGE;
}
@@ -181,35 +180,8 @@
auto response = CallMethodAndBlockWithTimeout(
timeout_ms,
manager_proxy_,
- kManagerInterface, kManagerStartRegisterDevice, &error,
+ kManagerInterface, kManagerRegisterDevice, &error,
params);
- std::string info;
- if (!response ||
- !ExtractMethodCallResults(response.get(), &error, &info)) {
- std::cout << "Failed to receive a response:"
- << error->GetMessage() << std::endl;
- return EX_UNAVAILABLE;
- }
-
- std::cout << "Registration started: " << info << std::endl;
- return EX_OK;
- }
-
- int CallManagerFinishRegisterDevice(const CommandLine::StringVector& args) {
- if (args.size() > 0) {
- std::cerr << "Invalid number of arguments for "
- << "Manager." << kManagerFinishRegisterDevice << std::endl;
- usage();
- return EX_USAGE;
- }
-
- ErrorPtr error;
- std::string user_auth_code;
- static const int timeout_ms = 10000;
- auto response = CallMethodAndBlockWithTimeout(
- timeout_ms,
- manager_proxy_,
- kManagerInterface, kManagerFinishRegisterDevice, &error);
std::string device_id;
if (!response ||
!ExtractMethodCallResults(response.get(), &error, &device_id)) {
@@ -218,9 +190,7 @@
return EX_UNAVAILABLE;
}
- std::cout << "Device ID is "
- << (device_id.empty() ? std::string("<unregistered>") : device_id)
- << std::endl;
+ std::cout << "Device registered: " << device_id << std::endl;
return EX_OK;
}
@@ -328,12 +298,9 @@
} else if (command.compare(kManagerGetDeviceInfo) == 0 ||
command.compare("di") == 0) {
err = helper.CallManagerGetDeviceInfo(args);
- } else if (command.compare(kManagerStartRegisterDevice) == 0 ||
- command.compare("sr") == 0) {
- err = helper.CallManagerStartRegisterDevice(args);
- } else if (command.compare(kManagerFinishRegisterDevice) == 0 ||
- command.compare("fr") == 0) {
- err = helper.CallManagerFinishRegisterDevice(args);
+ } else if (command.compare(kManagerRegisterDevice) == 0 ||
+ command.compare("rd") == 0) {
+ err = helper.CallManagerRegisterDevice(args);
} else if (command.compare(kManagerUpdateStateMethod) == 0 ||
command.compare("us") == 0) {
err = helper.CallManagerUpdateState(args);
diff --git a/buffet/device_registration_info.cc b/buffet/device_registration_info.cc
index 2c04dcf..c67dce0 100644
--- a/buffet/device_registration_info.cc
+++ b/buffet/device_registration_info.cc
@@ -369,7 +369,7 @@
return false;
}
-std::string DeviceRegistrationInfo::StartRegistration(
+std::string DeviceRegistrationInfo::RegisterDevice(
const std::map<std::string, std::string>& params,
chromeos::ErrorPtr* error) {
GetParamValue(params, "ticket_id", &ticket_id_);
@@ -382,7 +382,6 @@
GetParamValue(params, storage_keys::kOAuthURL, &oauth_url_);
GetParamValue(params, storage_keys::kServiceURL, &service_url_);
-
std::unique_ptr<base::DictionaryValue> device_draft =
BuildDeviceResource(error);
if (!device_draft)
@@ -411,37 +410,17 @@
{"client_id", client_id_}
});
- base::DictionaryValue json;
- json.SetString("ticket_id", ticket_id_);
- json.SetString("auth_url", auth_url);
-
- std::string ret;
- base::JSONWriter::Write(&json, &ret);
- return ret;
-}
-
-bool DeviceRegistrationInfo::FinishRegistration(chromeos::ErrorPtr* error) {
- if (ticket_id_.empty()) {
- LOG(ERROR) << "Finish registration without ticket ID";
- chromeos::Error::AddTo(error, kErrorDomainBuffet,
- "registration_not_started",
- "Device registration not started");
- return false;
- }
-
- std::string url = GetServiceURL("registrationTickets/" + ticket_id_ +
- "/finalize?key=" + api_key_);
- std::unique_ptr<chromeos::http::Response> response =
- chromeos::http::PostBinary(url, nullptr, 0, transport_, error);
+ url = GetServiceURL("registrationTickets/" + ticket_id_ +
+ "/finalize?key=" + api_key_);
+ response = chromeos::http::PostBinary(url, nullptr, 0, transport_, error);
if (!response)
- return false;
- auto json_resp = chromeos::http::ParseJsonResponse(response.get(), nullptr,
- error);
+ return std::string();
+ json_resp = chromeos::http::ParseJsonResponse(response.get(), nullptr, error);
if (!json_resp)
- return false;
+ return std::string();
if (!response->IsSuccessful()) {
ParseGCDError(json_resp.get(), error);
- return false;
+ return std::string();
}
std::string auth_code;
@@ -450,7 +429,7 @@
!json_resp->GetString("deviceDraft.id", &device_id_)) {
chromeos::Error::AddTo(error, kErrorDomainGCD, "unexpected_response",
"Device account missing in response");
- return false;
+ return std::string();
}
// Now get access_token and refresh_token
@@ -463,7 +442,7 @@
{"grant_type", "authorization_code"}
}, transport_, error);
if (!response)
- return false;
+ return std::string();
json_resp = ParseOAuthResponse(response.get(), error);
int expires_in = 0;
@@ -476,14 +455,14 @@
expires_in <= 0) {
chromeos::Error::AddTo(error, kErrorDomainGCD, "unexpected_response",
"Device access_token missing in response");
- return false;
+ return std::string();
}
access_token_expiration_ = base::Time::Now() +
base::TimeDelta::FromSeconds(expires_in);
Save();
- return true;
+ return device_id_;
}
void DeviceRegistrationInfo::DoCloudRequest(
diff --git a/buffet/device_registration_info.h b/buffet/device_registration_info.h
index 8292ea6..071df46 100644
--- a/buffet/device_registration_info.h
+++ b/buffet/device_registration_info.h
@@ -95,19 +95,17 @@
// the device is not registered or communication failure.
std::unique_ptr<base::Value> GetDeviceInfo(chromeos::ErrorPtr* error);
- // Starts device registration procedure. |params| are a list of
- // key-value pairs of device information, such as client_id, client_secret,
- // and so on. If a particular key-value pair is omitted, a default value
- // is used when possible. Returns a device registration ticket ID on success.
+ // Registers the device.
+ //
+ // |params| are a list of key-value pairs of device information,
+ // such as client_id, client_secret, and so on. If a particular key-value pair
+ // is omitted, a default value is used when possible.
+ // Returns a device ID on success.
// The values are all strings for now.
- std::string StartRegistration(
+ std::string RegisterDevice(
const std::map<std::string, std::string>& params,
chromeos::ErrorPtr* error);
- // Finalizes the device registration.
- // StartRegistration must have been invoked before.
- bool FinishRegistration(chromeos::ErrorPtr* error);
-
// Start device execution.
// Device will do required start up chores and then start to listen
// to new commands.
diff --git a/buffet/device_registration_info_unittest.cc b/buffet/device_registration_info_unittest.cc
index e33630f..097039a 100644
--- a/buffet/device_registration_info_unittest.cc
+++ b/buffet/device_registration_info_unittest.cc
@@ -270,7 +270,7 @@
EXPECT_EQ(test_data::kDeviceId, id);
}
-TEST_F(DeviceRegistrationInfoTest, StartRegistration) {
+TEST_F(DeviceRegistrationInfoTest, RegisterDevice) {
EXPECT_TRUE(dev_reg_->Load());
auto update_ticket = [](const ServerRequest& request,
@@ -339,24 +339,6 @@
std::string("registrationTickets/") + test_data::kClaimTicketId),
chromeos::http::request_type::kPatch,
base::Bind(update_ticket));
- std::map<std::string, std::string> params;
- params["ticket_id"] = test_data::kClaimTicketId;
- std::string json_resp = dev_reg_->StartRegistration(params, nullptr);
- auto json = std::unique_ptr<base::Value>(base::JSONReader::Read(json_resp));
- EXPECT_NE(nullptr, json.get());
- base::DictionaryValue* dict = nullptr;
- EXPECT_TRUE(json->GetAsDictionary(&dict));
- std::string value;
- EXPECT_TRUE(dict->GetString("ticket_id", &value));
- EXPECT_EQ(test_data::kClaimTicketId, value);
-}
-
-TEST_F(DeviceRegistrationInfoTest, FinishRegistration_NoAuth) {
- // Test finalizing ticket with no user authorization token.
- // This assumes that a client would patch in their email separately.
- EXPECT_TRUE(dev_reg_->Load());
-
- // General ticket finalization handler.
std::string ticket_url =
dev_reg_->GetServiceURL("registrationTickets/" +
std::string(test_data::kClaimTicketId));
@@ -367,13 +349,17 @@
transport_->AddHandler(dev_reg_->GetOAuthURL("token"),
chromeos::http::request_type::kPost,
base::Bind(OAuth2Handler));
-
storage_->reset_save_count();
DeviceRegistrationInfo::TestHelper::SetTestTicketId(dev_reg_.get());
- EXPECT_TRUE(dev_reg_->FinishRegistration(nullptr));
+
+ std::map<std::string, std::string> params;
+ params["ticket_id"] = test_data::kClaimTicketId;
+ std::string device_id = dev_reg_->RegisterDevice(params, nullptr);
+
+ EXPECT_EQ(test_data::kDeviceId, device_id);
EXPECT_EQ(1,
storage_->save_count()); // The device info must have been saved.
- EXPECT_EQ(2, transport_->GetRequestCount());
+ EXPECT_EQ(3, transport_->GetRequestCount());
// Validate the device info saved to storage...
auto storage_data = storage_->Load();
diff --git a/buffet/libbuffet/dbus_constants.cc b/buffet/libbuffet/dbus_constants.cc
index fba4c79..16cdb36 100644
--- a/buffet/libbuffet/dbus_constants.cc
+++ b/buffet/libbuffet/dbus_constants.cc
@@ -18,8 +18,7 @@
const char kManagerStartDevice[] = "StartDevice";
const char kManagerCheckDeviceRegistered[] = "CheckDeviceRegistered";
const char kManagerGetDeviceInfo[] = "GetDeviceInfo";
-const char kManagerStartRegisterDevice[] = "StartRegisterDevice";
-const char kManagerFinishRegisterDevice[] = "FinishRegisterDevice";
+const char kManagerRegisterDevice[] = "RegisterDevice";
const char kManagerUpdateStateMethod[] = "UpdateState";
const char kManagerAddCommand[] = "AddCommand";
const char kManagerTestMethod[] = "TestMethod";
diff --git a/buffet/libbuffet/dbus_constants.h b/buffet/libbuffet/dbus_constants.h
index 0d0c63b..e0dd22a 100644
--- a/buffet/libbuffet/dbus_constants.h
+++ b/buffet/libbuffet/dbus_constants.h
@@ -25,8 +25,7 @@
LIBBUFFET_EXPORT extern const char kManagerStartDevice[];
LIBBUFFET_EXPORT extern const char kManagerCheckDeviceRegistered[];
LIBBUFFET_EXPORT extern const char kManagerGetDeviceInfo[];
-LIBBUFFET_EXPORT extern const char kManagerStartRegisterDevice[];
-LIBBUFFET_EXPORT extern const char kManagerFinishRegisterDevice[];
+LIBBUFFET_EXPORT extern const char kManagerRegisterDevice[];
LIBBUFFET_EXPORT extern const char kManagerUpdateStateMethod[];
LIBBUFFET_EXPORT extern const char kManagerAddCommand[];
LIBBUFFET_EXPORT extern const char kManagerTestMethod[];
diff --git a/buffet/manager.cc b/buffet/manager.cc
index af1632b..4648c83 100644
--- a/buffet/manager.cc
+++ b/buffet/manager.cc
@@ -45,12 +45,9 @@
itf->AddMethodHandler(dbus_constants::kManagerGetDeviceInfo,
base::Unretained(this),
&Manager::HandleGetDeviceInfo);
- itf->AddMethodHandler(dbus_constants::kManagerStartRegisterDevice,
+ itf->AddMethodHandler(dbus_constants::kManagerRegisterDevice,
base::Unretained(this),
- &Manager::HandleStartRegisterDevice);
- itf->AddMethodHandler(dbus_constants::kManagerFinishRegisterDevice,
- base::Unretained(this),
- &Manager::HandleFinishRegisterDevice);
+ &Manager::HandleRegisterDevice);
itf->AddMethodHandler(dbus_constants::kManagerUpdateStateMethod,
base::Unretained(this),
&Manager::HandleUpdateState);
@@ -108,20 +105,12 @@
return device_info_str;
}
-std::string Manager::HandleStartRegisterDevice(
+std::string Manager::HandleRegisterDevice(
chromeos::ErrorPtr* error,
const std::map<std::string, std::string>& params) {
- LOG(INFO) << "Received call to Manager.StartRegisterDevice()";
+ LOG(INFO) << "Received call to Manager.RegisterDevice()";
- return device_info_->StartRegistration(params, error);
-}
-
-std::string Manager::HandleFinishRegisterDevice(chromeos::ErrorPtr* error) {
- LOG(INFO) << "Received call to Manager.FinishRegisterDevice()";
- if (!device_info_->FinishRegistration(error))
- return std::string();
-
- return device_info_->GetDeviceId(error);
+ return device_info_->RegisterDevice(params, error);
}
void Manager::HandleUpdateState(
diff --git a/buffet/manager.h b/buffet/manager.h
index 8d7ab13..4fcf445 100644
--- a/buffet/manager.h
+++ b/buffet/manager.h
@@ -49,12 +49,10 @@
std::string HandleCheckDeviceRegistered(chromeos::ErrorPtr* error);
// Handles calls to org.chromium.Buffet.Manager.GetDeviceInfo().
std::string HandleGetDeviceInfo(chromeos::ErrorPtr* error);
- // Handles calls to org.chromium.Buffet.Manager.StartRegisterDevice().
- std::string HandleStartRegisterDevice(chromeos::ErrorPtr* error,
- const std::map<std::string,
- std::string>& params);
- // Handles calls to org.chromium.Buffet.Manager.FinishRegisterDevice().
- std::string HandleFinishRegisterDevice(chromeos::ErrorPtr* error);
+ // Handles calls to org.chromium.Buffet.Manager.RegisterDevice().
+ std::string HandleRegisterDevice(
+ chromeos::ErrorPtr* error,
+ const std::map<std::string, std::string>& params);
// Handles calls to org.chromium.Buffet.Manager.UpdateState().
void HandleUpdateState(chromeos::ErrorPtr* error,
const chromeos::VariantDictionary& property_set);
