Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor(oauth2): prepare for API key auth #14780

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion google/cloud/internal/curl_rest_client.cc
Original file line number Diff line number Diff line change
Expand Up @@ -122,7 +122,8 @@ StatusOr<std::unique_ptr<CurlImpl>> CurlRestClient::CreateCurlImpl(
auto impl =
std::make_unique<CurlImpl>(std::move(handle), handle_factory_, options);
if (credentials_) {
auto auth_header = oauth2_internal::AuthorizationHeader(*credentials_);
auto auth_header =
credentials_->AuthenticationHeader(std::chrono::system_clock::now());
if (!auth_header.ok()) return std::move(auth_header).status();
impl->SetHeader(auth_header.value());
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ StatusOr<AccessToken> ParseAuthorizedUserRefreshResponse(
* google/cloud/credentials.h.
*
* An HTTP Authorization header, with an access token as its value,
* can be obtained by calling the AuthorizationHeader() method; if the current
* can be obtained by calling the AuthenticationHeader() method; if the current
* access token is invalid or nearing expiration, this will class will first
* obtain a new access token before returning the Authorization header string.
*
Expand Down
16 changes: 8 additions & 8 deletions google/cloud/internal/oauth2_credentials.cc
Original file line number Diff line number Diff line change
Expand Up @@ -46,21 +46,21 @@ StatusOr<std::string> Credentials::project_id(
return project_id();
}

StatusOr<std::pair<std::string, std::string>> AuthorizationHeader(
Credentials& credentials, std::chrono::system_clock::time_point tp) {
auto token = credentials.GetToken(tp);
StatusOr<std::pair<std::string, std::string>> Credentials::AuthenticationHeader(
std::chrono::system_clock::time_point tp) {
auto token = GetToken(tp);
if (!token) return std::move(token).status();
if (token->token.empty()) return std::make_pair(std::string{}, std::string{});
return std::make_pair(std::string{"Authorization"},
absl::StrCat("Bearer ", token->token));
}

StatusOr<std::string> AuthorizationHeaderJoined(
StatusOr<std::string> AuthenticationHeaderJoined(
Credentials& credentials, std::chrono::system_clock::time_point tp) {
auto token = credentials.GetToken(tp);
if (!token) return std::move(token).status();
if (token->token.empty()) return std::string{};
return absl::StrCat("Authorization: Bearer ", token->token);
auto header = credentials.AuthenticationHeader(tp);
if (!header) return std::move(header).status();
if (header->first.empty()) return std::string{};
return absl::StrCat(header->first, ": ", header->second);
}

GOOGLE_CLOUD_CPP_INLINE_NAMESPACE_END
Expand Down
36 changes: 24 additions & 12 deletions google/cloud/internal/oauth2_credentials.h
Original file line number Diff line number Diff line change
Expand Up @@ -107,23 +107,35 @@ class Credentials {

/// @copydoc project_id()
virtual StatusOr<std::string> project_id(Options const&) const;

/**
* Returns a header pair used for authentication.
*
* In most cases, this is the "Authorization" HTTP header. For API key
* credentials, it is the "X-Goog-Api-Key" header.
*
* If unable to obtain a value for the header, which could happen for
* `Credentials` that need to be periodically refreshed, the underlying
* `Status` will indicate failure details from the refresh HTTP request.
* Otherwise, the returned value will contain the header pair to be used in
* HTTP requests.
*/
virtual StatusOr<std::pair<std::string, std::string>> AuthenticationHeader(
std::chrono::system_clock::time_point tp);
};

/**
* Attempts to obtain a value for the Authorization HTTP header.
* Returns a header pair as a single string to be used for authentication.
*
* If unable to obtain a value for the Authorization header, which could
* happen for `Credentials` that need to be periodically refreshed, the
* underlying `Status` will indicate failure details from the refresh HTTP
* request. Otherwise, the returned value will contain the Authorization
* header to be used in HTTP requests.
* In most cases, this is the "Authorization" HTTP header. For API key
* credentials, it is the "X-Goog-Api-Key" header.
*
* If unable to obtain a value for the header, which could happen for
* `Credentials` that need to be periodically refreshed, the underlying `Status`
* will indicate failure details from the refresh HTTP request. Otherwise, the
* returned value will contain the header pair to be used in HTTP requests.
*/
StatusOr<std::pair<std::string, std::string>> AuthorizationHeader(
Credentials& credentials, std::chrono::system_clock::time_point tp =
std::chrono::system_clock::now());

/// @copydoc AuthorizationHeader()
StatusOr<std::string> AuthorizationHeaderJoined(
StatusOr<std::string> AuthenticationHeaderJoined(
Credentials& credentials, std::chrono::system_clock::time_point tp =
std::chrono::system_clock::now());

Expand Down
28 changes: 13 additions & 15 deletions google/cloud/internal/oauth2_credentials_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ namespace {

using ::google::cloud::internal::UnavailableError;
using ::google::cloud::testing_util::IsOk;
using ::google::cloud::testing_util::IsOkAndHolds;
using ::testing::IsEmpty;
using ::testing::Not;
using ::testing::Pair;
Expand All @@ -42,44 +43,41 @@ TEST(Credentials, AuthorizationHeaderSuccess) {
auto const expiration = now + std::chrono::seconds(3600);
EXPECT_CALL(mock, GetToken(now))
.WillOnce(Return(AccessToken{"test-token", expiration}));
auto actual = AuthorizationHeader(mock, now);
ASSERT_STATUS_OK(actual);
EXPECT_THAT(*actual, Pair("Authorization", "Bearer test-token"));
auto actual = mock.AuthenticationHeader(now);
EXPECT_THAT(actual, IsOkAndHolds(Pair("Authorization", "Bearer test-token")));
}

TEST(Credentials, AuthorizationHeaderJoinedSuccess) {
TEST(Credentials, AuthenticationHeaderJoinedSuccess) {
MockCredentials mock;
auto const now = std::chrono::system_clock::now();
auto const expiration = now + std::chrono::seconds(3600);
EXPECT_CALL(mock, GetToken(now))
.WillOnce(Return(AccessToken{"test-token", expiration}));
auto actual = AuthorizationHeaderJoined(mock, now);
ASSERT_STATUS_OK(actual);
EXPECT_THAT(*actual, "Authorization: Bearer test-token");
auto actual = AuthenticationHeaderJoined(mock, now);
EXPECT_THAT(actual, IsOkAndHolds("Authorization: Bearer test-token"));
}

TEST(Credentials, AuthorizationHeaderJoinedEmpty) {
TEST(Credentials, AuthenticationHeaderJoinedEmpty) {
MockCredentials mock;
auto const now = std::chrono::system_clock::now();
auto const expiration = now + std::chrono::seconds(3600);
EXPECT_CALL(mock, GetToken(now))
.WillOnce(Return(AccessToken{"", expiration}));
auto actual = AuthorizationHeaderJoined(mock, now);
ASSERT_STATUS_OK(actual);
EXPECT_THAT(*actual, IsEmpty());
auto actual = AuthenticationHeaderJoined(mock, now);
EXPECT_THAT(actual, IsOkAndHolds(IsEmpty()));
}

TEST(Credentials, AuthorizationHeaderError) {
TEST(Credentials, AuthenticationHeaderError) {
MockCredentials mock;
EXPECT_CALL(mock, GetToken).WillOnce(Return(UnavailableError("try-again")));
auto actual = AuthorizationHeader(mock);
auto actual = mock.AuthenticationHeader(std::chrono::system_clock::now());
EXPECT_EQ(actual.status(), UnavailableError("try-again"));
}

TEST(Credentials, AuthorizationHeaderJoinedError) {
TEST(Credentials, AuthenticationHeaderJoinedError) {
MockCredentials mock;
EXPECT_CALL(mock, GetToken).WillOnce(Return(UnavailableError("try-again")));
auto actual = AuthorizationHeaderJoined(mock);
auto actual = AuthenticationHeaderJoined(mock);
EXPECT_EQ(actual.status(), UnavailableError("try-again"));
}

Expand Down
4 changes: 2 additions & 2 deletions google/cloud/internal/oauth2_google_credentials_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,7 @@ std::string TempFileName() {
* This test only verifies the right type of object is created, the unit tests
* for `AuthorizedUserCredentials` already check that once loaded the class
* works correctly. Testing here would be redundant. Furthermore, calling
* `AuthorizationHeader()` initiates the key verification workflow, that
* `AuthenticationHeader()` initiates the key verification workflow, that
* requires valid keys and contacting Google's production servers, and would
* make this an integration test.
*/
Expand Down Expand Up @@ -187,7 +187,7 @@ TEST_F(GoogleCredentialsTest,
* This test only verifies the right type of object is created, the unit tests
* for `ServiceAccountCredentials` already check that once loaded the class
* works correctly. Testing here would be redundant. Furthermore, calling
* `AuthorizationHeader()` initiates the key verification workflow, that
* `AuthenticationHeader()` initiates the key verification workflow, that
* requires valid keys and contacting Google's production servers, and would
* make this an integration test.
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,8 @@ MinimalIamCredentialsRestStub::MinimalIamCredentialsRestStub(
StatusOr<google::cloud::AccessToken>
MinimalIamCredentialsRestStub::GenerateAccessToken(
GenerateAccessTokenRequest const& request) {
auto auth_header = AuthorizationHeader(*credentials_);
auto auth_header =
credentials_->AuthenticationHeader(std::chrono::system_clock::now());
if (!auth_header) return std::move(auth_header).status();

rest_internal::RestRequest rest_request;
Expand Down
4 changes: 2 additions & 2 deletions google/cloud/internal/oauth2_service_account_credentials.h
Original file line number Diff line number Diff line change
Expand Up @@ -174,8 +174,8 @@ StatusOr<std::string> MakeSelfSignedJWT(
* variable can be used to prefer OAuth-based access tokens.
*
* Since access tokens are relatively expensive to create this class caches the
* access tokens until they are about to expire. Use the `AuthorizationHeader()`
* to get the current access token.
* access tokens until they are about to expire. Use the
* `AuthenticationHeader()` to get the current access token.
*
* [aip/4111]: https://google.aip.dev/auth/4111
* [aip/4112]: https://google.aip.dev/auth/4112
Expand Down
2 changes: 1 addition & 1 deletion google/cloud/internal/unified_rest_credentials_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -478,7 +478,7 @@ TEST(UnifiedRestCredentialsTest, LoadSuccess) {
ScopedEnvironment env("GOOGLE_APPLICATION_CREDENTIALS", filename);

auto credentials = MapCredentials(*MakeGoogleDefaultCredentials());
// Calling AuthorizationHeader() makes RPCs which would turn this into an
// Calling AuthenticationHeader() makes RPCs which would turn this into an
// integration test, fortunately there are easier ways to verify the file was
// loaded correctly:
EXPECT_EQ(kServiceAccountEmail, credentials->AccountEmail());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ class WrapRestCredentials : public oauth2::Credentials {
: impl_(std::move(impl)) {}

StatusOr<std::string> AuthorizationHeader() override {
return oauth2_internal::AuthorizationHeaderJoined(*impl_);
return oauth2_internal::AuthenticationHeaderJoined(*impl_);
}

StatusOr<std::vector<std::uint8_t>> SignBlob(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,7 @@ TEST_F(UnifiedRestCredentialsTest, LoadSuccess) {
ScopedEnvironment env("GOOGLE_APPLICATION_CREDENTIALS", filename);

auto credentials = MapCredentials(*MakeGoogleDefaultCredentials());
// Calling AuthorizationHeader() makes RPCs which would turn this into an
// Calling AuthenticationHeader() makes RPCs which would turn this into an
// integration test, fortunately there are easier ways to verify the file was
// loaded correctly:
EXPECT_EQ(kClientEmail, credentials->AccountEmail());
Expand Down
4 changes: 2 additions & 2 deletions google/cloud/storage/oauth2/authorized_user_credentials.h
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,7 @@ class AuthorizedUserCredentials<storage::internal::CurlRequestBuilder,
ChannelOptions const& channel_options = {});

StatusOr<std::string> AuthorizationHeader() override {
return oauth2_internal::AuthorizationHeaderJoined(*impl_);
return oauth2_internal::AuthenticationHeaderJoined(*impl_);
}

private:
Expand All @@ -130,7 +130,7 @@ class AuthorizedUserCredentials<storage::internal::CurlRequestBuilder,

StatusOr<std::string> AuthorizationHeaderForTesting(
std::chrono::system_clock::time_point tp) {
return oauth2_internal::AuthorizationHeaderJoined(*impl_, tp);
return oauth2_internal::AuthenticationHeaderJoined(*impl_, tp);
}

std::shared_ptr<google::cloud::oauth2_internal::Credentials> impl_;
Expand Down
4 changes: 2 additions & 2 deletions google/cloud/storage/oauth2/compute_engine_credentials.h
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,7 @@ class ComputeEngineCredentials<storage::internal::CurlRequestBuilder,
explicit ComputeEngineCredentials(std::string service_account_email);

StatusOr<std::string> AuthorizationHeader() override {
return oauth2_internal::AuthorizationHeaderJoined(*cached_);
return oauth2_internal::AuthenticationHeaderJoined(*cached_);
}

std::string AccountEmail() const override { return impl_->AccountEmail(); }
Expand Down Expand Up @@ -143,7 +143,7 @@ class ComputeEngineCredentials<storage::internal::CurlRequestBuilder,

StatusOr<std::string> AuthorizationHeaderForTesting(
std::chrono::system_clock::time_point tp) {
return oauth2_internal::AuthorizationHeaderJoined(*cached_, tp);
return oauth2_internal::AuthenticationHeaderJoined(*cached_, tp);
}

std::shared_ptr<oauth2_internal::ComputeEngineCredentials> impl_;
Expand Down
4 changes: 2 additions & 2 deletions google/cloud/storage/oauth2/service_account_credentials.h
Original file line number Diff line number Diff line change
Expand Up @@ -221,7 +221,7 @@ class ServiceAccountCredentials<storage::internal::CurlRequestBuilder,
ChannelOptions const& options);

StatusOr<std::string> AuthorizationHeader() override {
return oauth2_internal::AuthorizationHeaderJoined(*impl_);
return oauth2_internal::AuthenticationHeaderJoined(*impl_);
}

/**
Expand Down Expand Up @@ -253,7 +253,7 @@ class ServiceAccountCredentials<storage::internal::CurlRequestBuilder,
friend struct ServiceAccountCredentialsTester;
StatusOr<std::string> AuthorizationHeaderForTesting(
std::chrono::system_clock::time_point tp) {
return oauth2_internal::AuthorizationHeaderJoined(*impl_, tp);
return oauth2_internal::AuthenticationHeaderJoined(*impl_, tp);
}
std::unique_ptr<oauth2_internal::Credentials> impl_;
};
Expand Down
Loading