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);