Commit fc46ab79 authored by Kinuko Yasuda's avatar Kinuko Yasuda Committed by Chromium LUCI CQ

CloudPrint: Correctly set Authorization header

The change https://crrev.com/c/2577150 missed to set this value.
This also adds a unittest to check this (which fails without the fix).

R=rbpotter@chromium.org, thakis@chromium.org

Bug: 862175
Change-Id: I6548804749de242d6ccf1ec4b4c20547dfba2d9c
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2597249Reviewed-by: default avatarNico Weber <thakis@chromium.org>
Commit-Queue: Kinuko Yasuda <kinuko@chromium.org>
Cr-Commit-Position: refs/heads/master@{#838591}
parent a60172b8
...@@ -234,8 +234,8 @@ std::string GetPostDataForPrinterTags( ...@@ -234,8 +234,8 @@ std::string GetPostDataForPrinterTags(
return post_data; return post_data;
} }
std::string GetCloudPrintAuthHeader(const std::string& auth_token) { std::string GetCloudPrintAuthHeaderValue(const std::string& auth_token) {
return base::StringPrintf("Authorization: OAuth %s", auth_token.c_str()); return base::StringPrintf("OAuth %s", auth_token.c_str());
} }
} // namespace cloud_print } // namespace cloud_print
...@@ -78,8 +78,8 @@ std::string GetPostDataForPrinterTags( ...@@ -78,8 +78,8 @@ std::string GetPostDataForPrinterTags(
const std::string& proxy_tag_prefix, const std::string& proxy_tag_prefix,
const std::string& tags_hash_tag_name); const std::string& tags_hash_tag_name);
// Get the cloud print auth header from |auth_token|. // Get the cloud print auth header value from |auth_token|.
std::string GetCloudPrintAuthHeader(const std::string& auth_token); std::string GetCloudPrintAuthHeaderValue(const std::string& auth_token);
} // namespace cloud_print } // namespace cloud_print
......
...@@ -122,10 +122,9 @@ TEST(CloudPrintHelpersTest, GetPostDataForPrinterTags) { ...@@ -122,10 +122,9 @@ TEST(CloudPrintHelpersTest, GetPostDataForPrinterTags) {
std::string("__test__tagshash"))); std::string("__test__tagshash")));
} }
TEST(CloudPrintHelpersTest, GetCloudPrintAuthHeader) { TEST(CloudPrintHelpersTest, GetCloudPrintAuthHeaderValue) {
std::string test_auth("testauth"); std::string test_auth("testauth");
EXPECT_EQ("Authorization: OAuth testauth", EXPECT_EQ("OAuth testauth", GetCloudPrintAuthHeaderValue(test_auth));
GetCloudPrintAuthHeader(test_auth));
} }
} // namespace cloud_print } // namespace cloud_print
...@@ -217,10 +217,10 @@ CloudPrintURLFetcher::ResponseAction CloudPrintAuth::OnRequestAuthError() { ...@@ -217,10 +217,10 @@ CloudPrintURLFetcher::ResponseAction CloudPrintAuth::OnRequestAuthError() {
return CloudPrintURLFetcher::STOP_PROCESSING; return CloudPrintURLFetcher::STOP_PROCESSING;
} }
std::string CloudPrintAuth::GetAuthHeader() { std::string CloudPrintAuth::GetAuthHeaderValue() {
DCHECK(!client_login_token_.empty()); DCHECK(!client_login_token_.empty());
std::string header; std::string header;
header = "Authorization: GoogleLogin auth="; header = "GoogleLogin auth=";
header += client_login_token_; header += client_login_token_;
return header; return header;
} }
......
...@@ -82,7 +82,7 @@ class CloudPrintAuth : public base::RefCountedThreadSafe<CloudPrintAuth>, ...@@ -82,7 +82,7 @@ class CloudPrintAuth : public base::RefCountedThreadSafe<CloudPrintAuth>,
const base::Value& json_data, const base::Value& json_data,
bool succeeded) override; bool succeeded) override;
CloudPrintURLFetcher::ResponseAction OnRequestAuthError() override; CloudPrintURLFetcher::ResponseAction OnRequestAuthError() override;
std::string GetAuthHeader() override; std::string GetAuthHeaderValue() override;
private: private:
friend class base::RefCountedThreadSafe<CloudPrintAuth>; friend class base::RefCountedThreadSafe<CloudPrintAuth>;
...@@ -119,4 +119,3 @@ class CloudPrintAuth : public base::RefCountedThreadSafe<CloudPrintAuth>, ...@@ -119,4 +119,3 @@ class CloudPrintAuth : public base::RefCountedThreadSafe<CloudPrintAuth>,
} // namespace cloud_print } // namespace cloud_print
#endif // CHROME_SERVICE_CLOUD_PRINT_CLOUD_PRINT_AUTH_H_ #endif // CHROME_SERVICE_CLOUD_PRINT_CLOUD_PRINT_AUTH_H_
...@@ -204,7 +204,7 @@ CloudPrintURLFetcher::ResponseAction CloudPrintConnector::OnRequestAuthError() { ...@@ -204,7 +204,7 @@ CloudPrintURLFetcher::ResponseAction CloudPrintConnector::OnRequestAuthError() {
return CloudPrintURLFetcher::STOP_PROCESSING; return CloudPrintURLFetcher::STOP_PROCESSING;
} }
std::string CloudPrintConnector::GetAuthHeader() { std::string CloudPrintConnector::GetAuthHeaderValue() {
return GetCloudPrintAuthHeaderFromStore(); return GetCloudPrintAuthHeaderFromStore();
} }
......
...@@ -107,7 +107,7 @@ class CloudPrintConnector ...@@ -107,7 +107,7 @@ class CloudPrintConnector
const base::Value& json_data, const base::Value& json_data,
bool succeeded) override; bool succeeded) override;
CloudPrintURLFetcher::ResponseAction OnRequestAuthError() override; CloudPrintURLFetcher::ResponseAction OnRequestAuthError() override;
std::string GetAuthHeader() override; std::string GetAuthHeaderValue() override;
// Begin response handlers // Begin response handlers
CloudPrintURLFetcher::ResponseAction HandlePrinterListResponse( CloudPrintURLFetcher::ResponseAction HandlePrinterListResponse(
...@@ -212,4 +212,3 @@ class CloudPrintConnector ...@@ -212,4 +212,3 @@ class CloudPrintConnector
} // namespace cloud_print } // namespace cloud_print
#endif // CHROME_SERVICE_CLOUD_PRINT_CLOUD_PRINT_CONNECTOR_H_ #endif // CHROME_SERVICE_CLOUD_PRINT_CLOUD_PRINT_CONNECTOR_H_
...@@ -93,7 +93,7 @@ std::string GetCloudPrintAuthHeaderFromStore() { ...@@ -93,7 +93,7 @@ std::string GetCloudPrintAuthHeaderFromStore() {
LOG(ERROR) << "CP_PROXY: Missing OAuth token for request"; LOG(ERROR) << "CP_PROXY: Missing OAuth token for request";
return std::string(); return std::string();
} }
return GetCloudPrintAuthHeader(token_store->token()); return GetCloudPrintAuthHeaderValue(token_store->token());
} }
} // namespace cloud_print } // namespace cloud_print
...@@ -296,7 +296,10 @@ void CloudPrintURLFetcher::StartRequestHelper( ...@@ -296,7 +296,10 @@ void CloudPrintURLFetcher::StartRequestHelper(
void CloudPrintURLFetcher::SetupRequestHeaders() { void CloudPrintURLFetcher::SetupRequestHeaders() {
request_->ClearExtraRequestHeaders(); request_->ClearExtraRequestHeaders();
std::string headers = delegate_->GetAuthHeader(); std::string auth_header_value = delegate_->GetAuthHeaderValue();
if (!auth_header_value.empty()) {
request_->AddExtraRequestHeader("Authorization", auth_header_value);
}
request_->AddExtraRequestHeader(kChromeCloudPrintProxyHeaderName, request_->AddExtraRequestHeader(kChromeCloudPrintProxyHeaderName,
kChromeCloudPrintProxyHeaderValue); kChromeCloudPrintProxyHeaderValue);
if (!additional_accept_header_.empty()) { if (!additional_accept_header_.empty()) {
......
...@@ -106,7 +106,7 @@ class CloudPrintURLFetcher ...@@ -106,7 +106,7 @@ class CloudPrintURLFetcher
// Authentication information may change between retries. // Authentication information may change between retries.
// CloudPrintURLFetcher will request auth info before sending any request. // CloudPrintURLFetcher will request auth info before sending any request.
virtual std::string GetAuthHeader() = 0; virtual std::string GetAuthHeaderValue() = 0;
protected: protected:
virtual ~Delegate() {} virtual ~Delegate() {}
......
...@@ -114,7 +114,9 @@ class CloudPrintURLFetcherTest : public testing::Test, ...@@ -114,7 +114,9 @@ class CloudPrintURLFetcherTest : public testing::Test,
return CloudPrintURLFetcher::STOP_PROCESSING; return CloudPrintURLFetcher::STOP_PROCESSING;
} }
std::string GetAuthHeader() override { return std::string(); } std::string GetAuthHeaderValue() override { return auth_header_; }
void SetAuthHeaderValue(const std::string& value) { auth_header_ = value; }
scoped_refptr<base::SingleThreadTaskRunner> io_task_runner() { scoped_refptr<base::SingleThreadTaskRunner> io_task_runner() {
return io_task_runner_; return io_task_runner_;
...@@ -143,6 +145,7 @@ class CloudPrintURLFetcherTest : public testing::Test, ...@@ -143,6 +145,7 @@ class CloudPrintURLFetcherTest : public testing::Test,
base::test::SingleThreadTaskEnvironment task_environment_{ base::test::SingleThreadTaskEnvironment task_environment_{
base::test::SingleThreadTaskEnvironment::MainThreadType::IO}; base::test::SingleThreadTaskEnvironment::MainThreadType::IO};
scoped_refptr<base::SingleThreadTaskRunner> io_task_runner_; scoped_refptr<base::SingleThreadTaskRunner> io_task_runner_;
std::string auth_header_;
int max_retries_; int max_retries_;
Time start_time_; Time start_time_;
scoped_refptr<TestCloudPrintURLFetcher> fetcher_; scoped_refptr<TestCloudPrintURLFetcher> fetcher_;
...@@ -182,10 +185,12 @@ class CloudPrintURLFetcherBasicTest : public CloudPrintURLFetcherTest { ...@@ -182,10 +185,12 @@ class CloudPrintURLFetcherBasicTest : public CloudPrintURLFetcherTest {
void SetHandleRawData(bool handle_raw_data) { void SetHandleRawData(bool handle_raw_data) {
handle_raw_data_ = handle_raw_data; handle_raw_data_ = handle_raw_data;
} }
void SetExpectedData(const std::string& data) { expected_data_ = data; }
private: private:
bool handle_raw_response_; bool handle_raw_response_;
bool handle_raw_data_; bool handle_raw_data_;
std::string expected_data_;
}; };
// Version of CloudPrintURLFetcherTest that tests overload protection. // Version of CloudPrintURLFetcherTest that tests overload protection.
...@@ -278,6 +283,9 @@ CloudPrintURLFetcherBasicTest::HandleRawData( ...@@ -278,6 +283,9 @@ CloudPrintURLFetcherBasicTest::HandleRawData(
// We should never get here if we returned true in HandleRawResponse // We should never get here if we returned true in HandleRawResponse
EXPECT_FALSE(handle_raw_response_); EXPECT_FALSE(handle_raw_response_);
if (handle_raw_data_) { if (handle_raw_data_) {
if (!expected_data_.empty()) {
EXPECT_EQ(expected_data_, data);
}
std::move(quit_run_loop_).Run(); std::move(quit_run_loop_).Run();
return CloudPrintURLFetcher::STOP_PROCESSING; return CloudPrintURLFetcher::STOP_PROCESSING;
} }
...@@ -352,6 +360,19 @@ TEST_F(CloudPrintURLFetcherBasicTest, HandleRawData) { ...@@ -352,6 +360,19 @@ TEST_F(CloudPrintURLFetcherBasicTest, HandleRawData) {
run_loop_.Run(); run_loop_.Run();
} }
TEST_F(CloudPrintURLFetcherBasicTest, AuthorizationHeader) {
const char kAuthHeaderValue[] = "OAuth abcdefg";
net::EmbeddedTestServer test_server;
SetAuthHeaderValue(kAuthHeaderValue);
test_server.AddDefaultHandlers(base::FilePath(kDocRoot));
ASSERT_TRUE(test_server.Start());
SetHandleRawData(true);
SetExpectedData(kAuthHeaderValue);
CreateFetcher(test_server.GetURL("/echoheader?Authorization"), 0);
run_loop_.Run();
}
TEST_F(CloudPrintURLFetcherOverloadTest, Protect) { TEST_F(CloudPrintURLFetcherOverloadTest, Protect) {
net::EmbeddedTestServer test_server; net::EmbeddedTestServer test_server;
test_server.AddDefaultHandlers(base::FilePath(kDocRoot)); test_server.AddDefaultHandlers(base::FilePath(kDocRoot));
......
...@@ -66,8 +66,8 @@ CloudPrintURLFetcher::ResponseAction CloudPrintWipeout::OnRequestAuthError() { ...@@ -66,8 +66,8 @@ CloudPrintURLFetcher::ResponseAction CloudPrintWipeout::OnRequestAuthError() {
return CloudPrintURLFetcher::STOP_PROCESSING; return CloudPrintURLFetcher::STOP_PROCESSING;
} }
std::string CloudPrintWipeout::GetAuthHeader() { std::string CloudPrintWipeout::GetAuthHeaderValue() {
return GetCloudPrintAuthHeader(auth_token_); return GetCloudPrintAuthHeaderValue(auth_token_);
} }
} // namespace cloud_print } // namespace cloud_print
...@@ -42,7 +42,7 @@ class CloudPrintWipeout : public CloudPrintURLFetcher::Delegate { ...@@ -42,7 +42,7 @@ class CloudPrintWipeout : public CloudPrintURLFetcher::Delegate {
bool succeeded) override; bool succeeded) override;
void OnRequestGiveUp() override; void OnRequestGiveUp() override;
CloudPrintURLFetcher::ResponseAction OnRequestAuthError() override; CloudPrintURLFetcher::ResponseAction OnRequestAuthError() override;
std::string GetAuthHeader() override; std::string GetAuthHeaderValue() override;
private: private:
void UnregisterNextPrinter(); void UnregisterNextPrinter();
...@@ -66,4 +66,3 @@ class CloudPrintWipeout : public CloudPrintURLFetcher::Delegate { ...@@ -66,4 +66,3 @@ class CloudPrintWipeout : public CloudPrintURLFetcher::Delegate {
} // namespace cloud_print } // namespace cloud_print
#endif // CHROME_SERVICE_CLOUD_PRINT_CLOUD_PRINT_WIPEOUT_H_ #endif // CHROME_SERVICE_CLOUD_PRINT_CLOUD_PRINT_WIPEOUT_H_
...@@ -115,7 +115,7 @@ CloudPrintURLFetcher::ResponseAction JobStatusUpdater::OnRequestAuthError() { ...@@ -115,7 +115,7 @@ CloudPrintURLFetcher::ResponseAction JobStatusUpdater::OnRequestAuthError() {
return CloudPrintURLFetcher::STOP_PROCESSING; return CloudPrintURLFetcher::STOP_PROCESSING;
} }
std::string JobStatusUpdater::GetAuthHeader() { std::string JobStatusUpdater::GetAuthHeaderValue() {
return GetCloudPrintAuthHeaderFromStore(); return GetCloudPrintAuthHeaderFromStore();
} }
......
...@@ -52,7 +52,7 @@ class JobStatusUpdater : public base::RefCountedThreadSafe<JobStatusUpdater>, ...@@ -52,7 +52,7 @@ class JobStatusUpdater : public base::RefCountedThreadSafe<JobStatusUpdater>,
const base::Value& json_data, const base::Value& json_data,
bool succeeded) override; bool succeeded) override;
CloudPrintURLFetcher::ResponseAction OnRequestAuthError() override; CloudPrintURLFetcher::ResponseAction OnRequestAuthError() override;
std::string GetAuthHeader() override; std::string GetAuthHeaderValue() override;
base::Time start_time() const { base::Time start_time() const {
return start_time_; return start_time_;
......
...@@ -204,7 +204,7 @@ CloudPrintURLFetcher::ResponseAction PrinterJobHandler::OnRequestAuthError() { ...@@ -204,7 +204,7 @@ CloudPrintURLFetcher::ResponseAction PrinterJobHandler::OnRequestAuthError() {
return CloudPrintURLFetcher::STOP_PROCESSING; return CloudPrintURLFetcher::STOP_PROCESSING;
} }
std::string PrinterJobHandler::GetAuthHeader() { std::string PrinterJobHandler::GetAuthHeaderValue() {
return GetCloudPrintAuthHeaderFromStore(); return GetCloudPrintAuthHeaderFromStore();
} }
......
...@@ -131,7 +131,7 @@ class PrinterJobHandler : public base::RefCountedThreadSafe<PrinterJobHandler>, ...@@ -131,7 +131,7 @@ class PrinterJobHandler : public base::RefCountedThreadSafe<PrinterJobHandler>,
bool succeeded) override; bool succeeded) override;
void OnRequestGiveUp() override; void OnRequestGiveUp() override;
CloudPrintURLFetcher::ResponseAction OnRequestAuthError() override; CloudPrintURLFetcher::ResponseAction OnRequestAuthError() override;
std::string GetAuthHeader() override; std::string GetAuthHeaderValue() override;
// JobStatusUpdater::Delegate implementation // JobStatusUpdater::Delegate implementation
bool OnJobCompleted(JobStatusUpdater* updater) override; bool OnJobCompleted(JobStatusUpdater* updater) override;
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment