From 3605bb24d357c8dca0f65b1610e780c66c93ccce Mon Sep 17 00:00:00 2001 From: Yao Cui Date: Fri, 11 Oct 2024 17:28:50 -0400 Subject: [PATCH] cleanup(oauth2): MinimalIamCredentialsRestStub use universe domain in endpoint (#14781) * cleanup(oauth2): MinimalIamCredentialsRestStub use universe domain in endpoint * test * cleanup * split unit tests * cleanup * fix win build * fix msan-pr --- .../oauth2_minimal_iam_credentials_rest.cc | 11 ++-- .../oauth2_minimal_iam_credentials_rest.h | 2 +- ...auth2_minimal_iam_credentials_rest_test.cc | 62 ++++++++++++++++--- 3 files changed, 60 insertions(+), 15 deletions(-) diff --git a/google/cloud/internal/oauth2_minimal_iam_credentials_rest.cc b/google/cloud/internal/oauth2_minimal_iam_credentials_rest.cc index b7d646e0b85cd..8a4b415d35b31 100644 --- a/google/cloud/internal/oauth2_minimal_iam_credentials_rest.cc +++ b/google/cloud/internal/oauth2_minimal_iam_credentials_rest.cc @@ -72,12 +72,11 @@ MinimalIamCredentialsRestStub::GenerateAccessToken( } std::string MinimalIamCredentialsRestStub::MakeRequestPath( - GenerateAccessTokenRequest const& request) { - // TODO(#13422): Do not use hardcoded IAM endpoint. Use Universe Domain - // to build endpoint name. - return absl::StrCat( - "https://iamcredentials.googleapis.com/v1/projects/-/serviceAccounts/", - request.service_account, ":generateAccessToken"); + GenerateAccessTokenRequest const& request) const { + auto ud = universe_domain(Options{}); + return absl::StrCat("https://iamcredentials.", ud ? *ud : "googleapis.com", + "/v1/projects/-/serviceAccounts/", + request.service_account, ":generateAccessToken"); } MinimalIamCredentialsRestLogging::MinimalIamCredentialsRestLogging( diff --git a/google/cloud/internal/oauth2_minimal_iam_credentials_rest.h b/google/cloud/internal/oauth2_minimal_iam_credentials_rest.h index 18d2dd565f0a6..0c1a8e241c5f9 100644 --- a/google/cloud/internal/oauth2_minimal_iam_credentials_rest.h +++ b/google/cloud/internal/oauth2_minimal_iam_credentials_rest.h @@ -83,7 +83,7 @@ class MinimalIamCredentialsRestStub : public MinimalIamCredentialsRest { } private: - static std::string MakeRequestPath(GenerateAccessTokenRequest const& request); + std::string MakeRequestPath(GenerateAccessTokenRequest const& request) const; std::shared_ptr credentials_; Options options_; diff --git a/google/cloud/internal/oauth2_minimal_iam_credentials_rest_test.cc b/google/cloud/internal/oauth2_minimal_iam_credentials_rest_test.cc index 9ddf5a05fcf96..25f60f96585a2 100644 --- a/google/cloud/internal/oauth2_minimal_iam_credentials_rest_test.cc +++ b/google/cloud/internal/oauth2_minimal_iam_credentials_rest_test.cc @@ -15,6 +15,7 @@ #include "google/cloud/internal/oauth2_minimal_iam_credentials_rest.h" #include "google/cloud/internal/absl_str_cat_quiet.h" #include "google/cloud/internal/http_payload.h" +#include "google/cloud/internal/make_status.h" #include "google/cloud/internal/rest_request.h" #include "google/cloud/internal/rest_response.h" #include "google/cloud/testing_util/chrono_output.h" @@ -215,7 +216,6 @@ TEST(MinimalIamCredentialsRestTest, GenerateAccessTokenSuccess) { std::string response = R"""({ "accessToken": "my_access_token", "expireTime": "2022-10-12T07:20:50.52Z"})"""; - MockHttpClientFactory mock_client_factory; EXPECT_CALL(mock_client_factory, Call).WillOnce([=](Options const&) { auto client = std::make_unique(); @@ -238,17 +238,14 @@ TEST(MinimalIamCredentialsRestTest, GenerateAccessTokenSuccess) { "projects/-/serviceAccounts/", service_account, ":generateAccessToken"))); std::string str_payload(payload[0].begin(), payload[0].end()); + EXPECT_THAT(str_payload, HasSubstr("\"lifetime\":\"3600s\"")); + EXPECT_THAT(str_payload, HasSubstr("\"scope\":[\"my_scope\"]")); EXPECT_THAT(str_payload, - testing::HasSubstr("\"lifetime\":\"3600s\"")); - EXPECT_THAT(str_payload, - testing::HasSubstr("\"scope\":[\"my_scope\"]")); - EXPECT_THAT(str_payload, - testing::HasSubstr("\"delegates\":[\"my_delegate\"]")); + HasSubstr("\"delegates\":[\"my_delegate\"]")); return std::unique_ptr(std::move(mock_response)); }); return std::unique_ptr(std::move(client)); }); - auto mock_credentials = std::make_shared(); EXPECT_CALL(*mock_credentials, GetToken).WillOnce([lifetime](auto tp) { return AccessToken{"test-token", tp + lifetime}; @@ -262,12 +259,61 @@ TEST(MinimalIamCredentialsRestTest, GenerateAccessTokenSuccess) { request.lifetime = lifetime; request.scopes.push_back(scope); request.delegates.push_back(delegate); - auto access_token = stub.GenerateAccessToken(request); EXPECT_THAT(access_token, IsOk()); EXPECT_THAT(access_token->token, Eq("my_access_token")); } +TEST(MinimalIamCredentialsRestTest, GenerateAccessTokenWithUniverseDomain) { + std::string universe_domain = "my-ud.net"; + std::string service_account = "foo@somewhere.com"; + std::chrono::seconds lifetime(3600); + std::string response = R"""({ + "accessToken": "my_access_token", + "expireTime": "2022-10-12T07:20:50.52Z"})"""; + MockHttpClientFactory mock_client_factory; + EXPECT_CALL(mock_client_factory, Call).WillOnce([=](Options const&) { + auto client = std::make_unique(); + EXPECT_CALL(*client, + Post(_, _, A> const&>())) + .WillOnce([response, service_account, universe_domain]( + RestContext&, RestRequest const& request, + std::vector> const&) { + auto mock_response = std::make_unique(); + EXPECT_CALL(*mock_response, StatusCode) + .WillRepeatedly(Return(rest_internal::HttpStatusCode::kOk)); + EXPECT_CALL(std::move(*mock_response), ExtractPayload) + .WillOnce([response] { + return testing_util::MakeMockHttpPayloadSuccess(response); + }); + + EXPECT_THAT( + request.path(), + Eq(absl::StrCat("https://iamcredentials.", universe_domain, + "/v1/projects/-/serviceAccounts/", + service_account, ":generateAccessToken"))); + return std::unique_ptr(std::move(mock_response)); + }); + return std::unique_ptr(std::move(client)); + }); + auto mock_credentials = std::make_shared(); + EXPECT_CALL(*mock_credentials, GetToken).WillOnce([](auto tp) { + return AccessToken{"test-token", tp}; + }); + EXPECT_CALL(*mock_credentials, universe_domain) + .WillOnce([&](Options const&) -> StatusOr { + return universe_domain; + }); + + auto stub = + MinimalIamCredentialsRestStub(std::move(mock_credentials), Options{}, + mock_client_factory.AsStdFunction()); + GenerateAccessTokenRequest request; + request.lifetime = lifetime; + request.service_account = service_account; + stub.GenerateAccessToken(request); +} + TEST(MinimalIamCredentialsRestTest, GenerateAccessTokenCredentialFailure) { auto mock_credentials = std::make_shared(); EXPECT_CALL(*mock_credentials, GetToken).WillOnce([] {