buffet: Simplify device registration.
Make device always finalize registration ticket itself.
Also minor unifications in the code.
BUG=None
TEST=cros_work_make buffet --test
Change-Id: I7aa432aee2546fd562fd7048ef320d773c32be19
Reviewed-on: https://chromium-review.googlesource.com/220380
Reviewed-by: Alex Vakulenko <avakulenko@chromium.org>
Commit-Queue: Anton Muhin <antonm@chromium.org>
Tested-by: Anton Muhin <antonm@chromium.org>
diff --git a/buffet/buffet_client.cc b/buffet/buffet_client.cc
index 635c037..bde5abe 100644
--- a/buffet/buffet_client.cc
+++ b/buffet/buffet_client.cc
@@ -42,8 +42,7 @@
std::cerr << " " << kManagerGetDeviceInfo << std::endl;
std::cerr << " " << kManagerStartRegisterDevice
<< " param1 = val1¶m2 = val2..." << std::endl;
- std::cerr << " " << kManagerFinishRegisterDevice
- << " device_id" << std::endl;
+ std::cerr << " " << kManagerFinishRegisterDevice << std::endl;
std::cerr << " " << kManagerAddCommand
<< " '{\"name\":\"command_name\",\"parameters\":{}}'"
<< std::endl;
@@ -176,7 +175,7 @@
}
int CallManagerFinishRegisterDevice(const CommandLine::StringVector& args) {
- if (args.size() > 1) {
+ if (args.size() > 0) {
std::cerr << "Invalid number of arguments for "
<< "Manager." << kManagerFinishRegisterDevice << std::endl;
usage();
@@ -185,13 +184,11 @@
ErrorPtr error;
std::string user_auth_code;
- if (!args.empty()) { user_auth_code = args.front(); }
static const int timeout_ms = 10000;
auto response = CallMethodAndBlockWithTimeout(
timeout_ms,
manager_proxy_,
- kManagerInterface, kManagerFinishRegisterDevice, &error,
- user_auth_code);
+ kManagerInterface, kManagerFinishRegisterDevice, &error);
std::string device_id;
if (!response ||
!ExtractMethodCallResults(response.get(), &error, &device_id)) {
diff --git a/buffet/device_registration_info.cc b/buffet/device_registration_info.cc
index 3e35cec..925307d 100644
--- a/buffet/device_registration_info.cc
+++ b/buffet/device_registration_info.cc
@@ -374,16 +374,15 @@
req_json.Set("deviceDraft.commandDefs", commands.release());
req_json.Set("deviceDraft.state", state.release());
- int status_code{0};
auto url = GetServiceURL("registrationTickets/" + ticket_id_,
{{"key", api_key_}});
- auto resp_json = chromeos::http::ParseJsonResponse(
- chromeos::http::PatchJson(url, &req_json, transport_, error).get(),
- &status_code, error);
- if (!resp_json)
+ std::unique_ptr<chromeos::http::Response> response =
+ chromeos::http::PatchJson(url, &req_json, transport_, error);
+ auto json_resp = chromeos::http::ParseJsonResponse(response.get(), nullptr,
+ error);
+ if (!json_resp)
return std::string();
-
- if (status_code >= chromeos::http::status_code::BadRequest)
+ if (!response->IsSuccessful())
return std::string();
std::string auth_url = GetOAuthURL("auth", {
@@ -402,8 +401,7 @@
return ret;
}
-bool DeviceRegistrationInfo::FinishRegistration(
- const std::string& user_auth_code, chromeos::ErrorPtr* error) {
+bool DeviceRegistrationInfo::FinishRegistration(chromeos::ErrorPtr* error) {
if (ticket_id_.empty()) {
LOG(ERROR) << "Finish registration without ticket ID";
chromeos::Error::AddTo(error, kErrorDomainBuffet,
@@ -412,48 +410,10 @@
return false;
}
- std::string url = GetServiceURL("registrationTickets/" + ticket_id_);
- std::unique_ptr<chromeos::http::Response> response;
- if (!user_auth_code.empty()) {
- response = chromeos::http::PostFormData(GetOAuthURL("token"), {
- {"code", user_auth_code},
- {"client_id", client_id_},
- {"client_secret", client_secret_},
- {"redirect_uri", "urn:ietf:wg:oauth:2.0:oob"},
- {"grant_type", "authorization_code"}
- }, transport_, error);
- if (!response)
- return false;
-
- auto json_resp = ParseOAuthResponse(response.get(), error);
- if (!json_resp)
- return false;
-
- std::string user_access_token;
- std::string token_type;
- if (!json_resp->GetString("access_token", &user_access_token) ||
- !json_resp->GetString("token_type", &token_type)) {
- chromeos::Error::AddTo(error, kErrorDomainOAuth2, "unexpected_response",
- "User access_token is missing in response");
- return false;
- }
-
- base::DictionaryValue user_info;
- user_info.SetString("userEmail", "me");
- response = chromeos::http::PatchJson(
- url, &user_info, {BuildAuthHeader(token_type, user_access_token)},
- transport_, error);
-
- auto json = chromeos::http::ParseJsonResponse(response.get(), nullptr,
- error);
- if (!json)
- return false;
- }
-
- std::string auth_code;
- url += "/finalize?key=" + api_key_;
- LOG(INFO) << "Sending request to: " << url;
- response = chromeos::http::PostBinary(url, nullptr, 0, transport_, error);
+ 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);
if (!response)
return false;
auto json_resp = chromeos::http::ParseJsonResponse(response.get(), nullptr,
@@ -464,6 +424,8 @@
ParseGCDError(json_resp.get(), error);
return false;
}
+
+ std::string auth_code;
if (!json_resp->GetString("robotAccountEmail", &device_robot_account_) ||
!json_resp->GetString("robotAccountAuthorizationCode", &auth_code) ||
!json_resp->GetString("deviceDraft.id", &device_id_)) {
diff --git a/buffet/device_registration_info.h b/buffet/device_registration_info.h
index 117f891..bd2159f 100644
--- a/buffet/device_registration_info.h
+++ b/buffet/device_registration_info.h
@@ -97,18 +97,15 @@
// 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 claim ID on success.
+ // is used when possible. Returns a device registration ticket ID on success.
// The values are all strings for now.
std::string StartRegistration(
const std::map<std::string, std::string>& params,
chromeos::ErrorPtr* error);
- // Finalizes the device registration. If |user_auth_code| is provided, then
- // the device record is populated with user email on user's behalf. Otherwise
- // the user is responsible to issue a PATCH request to provide a valid
- // email address before calling FinishRegistration.
- bool FinishRegistration(const std::string& user_auth_code,
- chromeos::ErrorPtr* error);
+ // Finalizes the device registration.
+ // StartRegistration must have been invoked before.
+ bool FinishRegistration(chromeos::ErrorPtr* error);
private:
// Saves the device registration to cache.
diff --git a/buffet/device_registration_info_unittest.cc b/buffet/device_registration_info_unittest.cc
index d61f502..e33630f 100644
--- a/buffet/device_registration_info_unittest.cc
+++ b/buffet/device_registration_info_unittest.cc
@@ -370,7 +370,7 @@
storage_->reset_save_count();
DeviceRegistrationInfo::TestHelper::SetTestTicketId(dev_reg_.get());
- EXPECT_TRUE(dev_reg_->FinishRegistration("", nullptr));
+ EXPECT_TRUE(dev_reg_->FinishRegistration(nullptr));
EXPECT_EQ(1,
storage_->save_count()); // The device info must have been saved.
EXPECT_EQ(2, transport_->GetRequestCount());
@@ -398,55 +398,4 @@
EXPECT_EQ(test_data::kServiceURL, value);
}
-TEST_F(DeviceRegistrationInfoTest, FinishRegistration_Auth) {
- // Test finalizing ticket with user authorization token.
- EXPECT_TRUE(dev_reg_->Load());
-
- // General ticket finalization handler.
- std::string ticket_url =
- dev_reg_->GetServiceURL("registrationTickets/" +
- std::string(test_data::kClaimTicketId));
- transport_->AddHandler(ticket_url + "/finalize",
- chromeos::http::request_type::kPost,
- base::Bind(FinalizeTicketHandler));
-
- transport_->AddHandler(dev_reg_->GetOAuthURL("token"),
- chromeos::http::request_type::kPost,
- base::Bind(OAuth2Handler));
-
- // Handle patching in the user email onto the device record.
- auto email_patch_handler = [](const ServerRequest& request,
- ServerResponse* response) {
- std::string auth_header = "Bearer ";
- auth_header += test_data::kUserAccessToken;
- EXPECT_EQ(auth_header,
- request.GetHeader(chromeos::http::request_header::kAuthorization));
- auto json = request.GetDataAsJson();
- EXPECT_NE(nullptr, json.get());
- std::string value;
- EXPECT_TRUE(json->GetString("userEmail", &value));
- EXPECT_EQ("me", value);
-
- response->ReplyJson(chromeos::http::status_code::Ok, {
- {"id", test_data::kClaimTicketId},
- {"kind", "clouddevices#registrationTicket"},
- {"oauthClientId", test_data::kClientId},
- {"userEmail", "user@email.com"},
- {"deviceDraft.id", test_data::kDeviceId},
- {"deviceDraft.kind", "clouddevices#device"},
- {"deviceDraft.channel.supportedType", "xmpp"},
- });
- };
- transport_->AddHandler(ticket_url, chromeos::http::request_type::kPatch,
- base::Bind(email_patch_handler));
-
- storage_->reset_save_count();
- DeviceRegistrationInfo::TestHelper::SetTestTicketId(dev_reg_.get());
- EXPECT_TRUE(dev_reg_->FinishRegistration(test_data::kUserAccountAuthCode,
- nullptr));
- EXPECT_EQ(1,
- storage_->save_count()); // The device info must have been saved.
- EXPECT_EQ(4, transport_->GetRequestCount());
-}
-
} // namespace buffet
diff --git a/buffet/manager.cc b/buffet/manager.cc
index c39ec52..10d36cf 100644
--- a/buffet/manager.cc
+++ b/buffet/manager.cc
@@ -107,10 +107,9 @@
return device_info_->StartRegistration(params, error);
}
-std::string Manager::HandleFinishRegisterDevice(
- chromeos::ErrorPtr* error, const std::string& user_auth_code) {
+std::string Manager::HandleFinishRegisterDevice(chromeos::ErrorPtr* error) {
LOG(INFO) << "Received call to Manager.FinishRegisterDevice()";
- if (!device_info_->FinishRegistration(user_auth_code, error))
+ if (!device_info_->FinishRegistration(error))
return std::string();
return device_info_->GetDeviceId(error);
diff --git a/buffet/manager.h b/buffet/manager.h
index a56b6c3..4648b95 100644
--- a/buffet/manager.h
+++ b/buffet/manager.h
@@ -52,8 +52,7 @@
const std::map<std::string,
std::string>& params);
// Handles calls to org.chromium.Buffet.Manager.FinishRegisterDevice().
- std::string HandleFinishRegisterDevice(chromeos::ErrorPtr* error,
- const std::string& user_auth_code);
+ std::string HandleFinishRegisterDevice(chromeos::ErrorPtr* error);
// Handles calls to org.chromium.Buffet.Manager.UpdateState().
void HandleUpdateState(chromeos::ErrorPtr* error,
const chromeos::VariantDictionary& property_set);
