Skip to content

Commit

Permalink
refactor(oauth2): prepare for API key auth (#14780)
Browse files Browse the repository at this point in the history
  • Loading branch information
dbolduc authored Oct 11, 2024
1 parent 67b10ea commit b6e899a
Show file tree
Hide file tree
Showing 14 changed files with 63 additions and 51 deletions.
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
2 changes: 1 addition & 1 deletion google/cloud/internal/oauth2_authorized_user_credentials.h
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
3 changes: 2 additions & 1 deletion google/cloud/internal/oauth2_minimal_iam_credentials_rest.cc
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
2 changes: 1 addition & 1 deletion google/cloud/storage/internal/unified_rest_credentials.cc
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

0 comments on commit b6e899a

Please sign in to comment.