impl(oauth2): use client factory for impersonation credentials

google/cloud/internal/oauth2_impersonate_service_account_credentials.cc

@@ -35,11 +35,12 @@ GenerateAccessTokenRequest MakeRequest(
 }  // namespace
 
 ImpersonateServiceAccountCredentials::ImpersonateServiceAccountCredentials(
-    google::cloud::internal::ImpersonateServiceAccountConfig const& config)
+    google::cloud::internal::ImpersonateServiceAccountConfig const& config,
+    HttpClientFactory client_factory)
     : ImpersonateServiceAccountCredentials(
-          config,
-          MakeMinimalIamCredentialsRestStub(
-              rest_internal::MapCredentials(config.base_credentials()))) {}
+          config, MakeMinimalIamCredentialsRestStub(
+                      rest_internal::MapCredentials(config.base_credentials()),
+                      config.options(), std::move(client_factory))) {}
 
 ImpersonateServiceAccountCredentials::ImpersonateServiceAccountCredentials(
     google::cloud::internal::ImpersonateServiceAccountConfig const& config,

google/cloud/internal/oauth2_impersonate_service_account_credentials.h

@@ -39,7 +39,8 @@ class ImpersonateServiceAccountCredentials
    *     time. This should generally not be overridden except for testing.
    */
   explicit ImpersonateServiceAccountCredentials(
-      google::cloud::internal::ImpersonateServiceAccountConfig const& config);
+      google::cloud::internal::ImpersonateServiceAccountConfig const& config,
+      HttpClientFactory client_factory);
   ImpersonateServiceAccountCredentials(
       google::cloud::internal::ImpersonateServiceAccountConfig const& config,
       std::shared_ptr<MinimalIamCredentialsRest> stub);

google/cloud/internal/oauth2_minimal_iam_credentials_rest.cc

@@ -32,25 +32,15 @@ namespace google {
 namespace cloud {
 namespace oauth2_internal {
 GOOGLE_CLOUD_CPP_INLINE_NAMESPACE_BEGIN
-namespace {
 
 using ::google::cloud::internal::InvalidArgumentError;
 
-auto constexpr kIamCredentialsEndpoint =
-    "https://iamcredentials.googleapis.com/v1/";
-}  // namespace
-
 MinimalIamCredentialsRestStub::MinimalIamCredentialsRestStub(
     std::shared_ptr<oauth2_internal::Credentials> credentials, Options options,
-    std::shared_ptr<rest_internal::RestClient> rest_client)
+    HttpClientFactory client_factory)
     : credentials_(std::move(credentials)),
-      rest_client_(std::move(rest_client)),
-      options_(std::move(options)) {
-  if (!rest_client_) {
-    rest_client_ =
-        rest_internal::MakeDefaultRestClient(kIamCredentialsEndpoint, options_);
-  }
-}
+      options_(std::move(options)),
+      client_factory_(std::move(client_factory)) {}
 
 StatusOr<google::cloud::internal::AccessToken>
 MinimalIamCredentialsRestStub::GenerateAccessToken(
@@ -68,7 +58,8 @@ MinimalIamCredentialsRestStub::GenerateAccessToken(
       {"lifetime", std::to_string(request.lifetime.count()) + "s"},
   };
 
-  auto response = rest_client_->Post(rest_request, {payload.dump()});
+  auto client = client_factory_(options_);
+  auto response = client->Post(rest_request, {payload.dump()});
   if (!response) return std::move(response).status();
   return ParseGenerateAccessTokenResponse(
       **response,
@@ -80,8 +71,9 @@ MinimalIamCredentialsRestStub::GenerateAccessToken(
 
 std::string MinimalIamCredentialsRestStub::MakeRequestPath(
     GenerateAccessTokenRequest const& request) {
-  return absl::StrCat("projects/-/serviceAccounts/", request.service_account,
-                      ":generateAccessToken");
+  return absl::StrCat(
+      "https://iamcredentials.googleapis.com/v1/projects/-/serviceAccounts/",
+      request.service_account, ":generateAccessToken");
 }
 
 MinimalIamCredentialsRestLogging::MinimalIamCredentialsRestLogging(
@@ -138,14 +130,15 @@ StatusOr<internal::AccessToken> ParseGenerateAccessTokenResponse(
 }
 
 std::shared_ptr<MinimalIamCredentialsRest> MakeMinimalIamCredentialsRestStub(
-    std::shared_ptr<oauth2_internal::Credentials> credentials,
-    Options options) {
+    std::shared_ptr<oauth2_internal::Credentials> credentials, Options options,
+    HttpClientFactory client_factory) {
   auto enable_logging =
       options.get<TracingComponentsOption>().count("rpc") != 0 ||
       options.get<TracingComponentsOption>().count("raw-client") != 0;
   std::shared_ptr<MinimalIamCredentialsRest> stub =
-      std::make_shared<MinimalIamCredentialsRestStub>(std::move(credentials),
-                                                      std::move(options));
+      std::make_shared<MinimalIamCredentialsRestStub>(
+          std::move(credentials), std::move(options),
+          std::move(client_factory));
   if (enable_logging) {
     stub = std::make_shared<MinimalIamCredentialsRestLogging>(std::move(stub));
   }

google/cloud/internal/oauth2_minimal_iam_credentials_rest.h

@@ -19,6 +19,7 @@
 #include "google/cloud/internal/credentials_impl.h"
 #include "google/cloud/internal/error_context.h"
 #include "google/cloud/internal/oauth2_credentials.h"
+#include "google/cloud/internal/oauth2_http_client_factory.h"
 #include "google/cloud/internal/rest_client.h"
 #include "google/cloud/options.h"
 #include "google/cloud/status_or.h"
@@ -63,14 +64,13 @@ class MinimalIamCredentialsRestStub : public MinimalIamCredentialsRest {
   /**
    * Creates an instance of MinimalIamCredentialsRestStub.
    *
-   * @param rest_client a dependency injection point. It makes it possible to
+   * @param client_factory a dependency injection point. It makes it possible to
    *     mock internal REST types. This should generally not be overridden
    *     except for testing.
    */
   MinimalIamCredentialsRestStub(
       std::shared_ptr<oauth2_internal::Credentials> credentials,
-      Options options,
-      std::shared_ptr<rest_internal::RestClient> rest_client = nullptr);
+      Options options, HttpClientFactory client_factory);
 
   StatusOr<google::cloud::internal::AccessToken> GenerateAccessToken(
       GenerateAccessTokenRequest const& request) override;
@@ -79,8 +79,8 @@ class MinimalIamCredentialsRestStub : public MinimalIamCredentialsRest {
   static std::string MakeRequestPath(GenerateAccessTokenRequest const& request);
 
   std::shared_ptr<oauth2_internal::Credentials> credentials_;
-  std::shared_ptr<rest_internal::RestClient> rest_client_;
   Options options_;
+  HttpClientFactory client_factory_;
 };
 
 /**
@@ -99,8 +99,8 @@ class MinimalIamCredentialsRestLogging : public MinimalIamCredentialsRest {
 };
 
 std::shared_ptr<MinimalIamCredentialsRest> MakeMinimalIamCredentialsRestStub(
-    std::shared_ptr<oauth2_internal::Credentials> credentials,
-    Options options = {});
+    std::shared_ptr<oauth2_internal::Credentials> credentials, Options options,
+    HttpClientFactory client_factory);
 
 GOOGLE_CLOUD_CPP_INLINE_NAMESPACE_END
 }  // namespace oauth2_internal

google/cloud/internal/oauth2_minimal_iam_credentials_rest_test.cc

@@ -51,16 +51,9 @@ class MockCredentials : public google::cloud::oauth2_internal::Credentials {
               (std::chrono::system_clock::time_point), (override));
 };
 
-class MinimalIamCredentialsRestTest : public ::testing::Test {
- protected:
-  void SetUp() override {
-    mock_rest_client_ = std::make_shared<MockRestClient>();
-    mock_credentials_ = std::make_shared<MockCredentials>();
-  }
-
-  std::shared_ptr<MockRestClient> mock_rest_client_;
-  std::shared_ptr<MockCredentials> mock_credentials_;
-};
+using MockHttpClientFactory =
+    ::testing::MockFunction<std::unique_ptr<rest_internal::RestClient>(
+        Options const&)>;
 
 TEST(ParseGenerateAccessTokenResponse, Success) {
   auto const response = std::string{R"""({
@@ -210,46 +203,56 @@ TEST(ParseGenerateAccessTokenResponse, InvalidExpireTimeFormat) {
                        HasSubstr("invalid format for `expireTime` field")));
 }
 
-TEST_F(MinimalIamCredentialsRestTest, GenerateAccessTokenSuccess) {
+TEST(MinimalIamCredentialsRestTest, GenerateAccessTokenSuccess) {
   std::string service_account = "foo@somewhere.com";
   std::chrono::seconds lifetime(3600);
   std::string scope = "my_scope";
   std::string delegate = "my_delegate";
-
-  EXPECT_CALL(*mock_credentials_, GetToken).WillOnce([lifetime](auto tp) {
-    return internal::AccessToken{"test-token", tp + lifetime};
-  });
-
   std::string response = R"""({
-  "accessToken": "my_access_token",
-  "expireTime": "2022-10-12T07:20:50.52Z"
-})""";
-
-  auto* mock_response = new MockRestResponse();
-  EXPECT_CALL(*mock_response, StatusCode)
-      .WillRepeatedly(Return(rest_internal::HttpStatusCode::kOk));
-  EXPECT_CALL(std::move(*mock_response), ExtractPayload).WillOnce([&] {
-    return testing_util::MakeMockHttpPayloadSuccess(response);
+    "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 = absl::make_unique<MockRestClient>();
+    EXPECT_CALL(*client,
+                Post(_, A<std::vector<absl::Span<char const>> const&>()))
+        .WillOnce([response, service_account](
+                      RestRequest const& request,
+                      std::vector<absl::Span<char const>> const& payload) {
+          auto mock_response = absl::make_unique<MockRestResponse>();
+          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.googleapis.com/v1/",
+                              "projects/-/serviceAccounts/", service_account,
+                              ":generateAccessToken")));
+          std::string str_payload(payload[0].begin(), payload[0].end());
+          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\"]"));
+          return std::unique_ptr<RestResponse>(std::move(mock_response));
+        });
+    return std::unique_ptr<rest_internal::RestClient>(std::move(client));
   });
 
-  EXPECT_CALL(*mock_rest_client_,
-              Post(_, A<std::vector<absl::Span<char const>> const&>()))
-      .WillOnce([&](RestRequest const& request,
-                    std::vector<absl::Span<char const>> const& payload) {
-        EXPECT_THAT(request.path(),
-                    Eq(absl::StrCat("projects/-/serviceAccounts/",
-                                    service_account, ":generateAccessToken")));
-        std::string str_payload(payload[0].begin(), payload[0].end());
-        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\"]"));
-        return std::unique_ptr<RestResponse>(std::move(mock_response));
-      });
+  auto mock_credentials = std::make_shared<MockCredentials>();
+  EXPECT_CALL(*mock_credentials, GetToken).WillOnce([lifetime](auto tp) {
+    return internal::AccessToken{"test-token", tp + lifetime};
+  });
 
-  auto stub = MinimalIamCredentialsRestStub(std::move(mock_credentials_), {},
-                                            std::move(mock_rest_client_));
+  auto stub =
+      MinimalIamCredentialsRestStub(std::move(mock_credentials), Options{},
+                                    mock_client_factory.AsStdFunction());
   GenerateAccessTokenRequest request;
   request.service_account = service_account;
   request.lifetime = lifetime;
@@ -261,12 +264,15 @@ TEST_F(MinimalIamCredentialsRestTest, GenerateAccessTokenSuccess) {
   EXPECT_THAT(access_token->token, Eq("my_access_token"));
 }
 
-TEST_F(MinimalIamCredentialsRestTest, GenerateAccessTokenCredentialFailure) {
-  EXPECT_CALL(*mock_credentials_, GetToken).WillOnce([] {
+TEST(MinimalIamCredentialsRestTest, GenerateAccessTokenCredentialFailure) {
+  auto mock_credentials = std::make_shared<MockCredentials>();
+  EXPECT_CALL(*mock_credentials, GetToken).WillOnce([] {
     return Status(StatusCode::kPermissionDenied, "Permission Denied");
   });
-  auto stub = MinimalIamCredentialsRestStub(std::move(mock_credentials_), {},
-                                            std::move(mock_rest_client_));
+  MockHttpClientFactory mock_client_factory;
+  EXPECT_CALL(mock_client_factory, Call).Times(0);
+  auto stub = MinimalIamCredentialsRestStub(
+      std::move(mock_credentials), {}, mock_client_factory.AsStdFunction());
   GenerateAccessTokenRequest request;
   auto access_token = stub.GenerateAccessToken(request);
   EXPECT_THAT(access_token, StatusIs(StatusCode::kPermissionDenied));

google/cloud/internal/unified_rest_credentials.cc

@@ -92,7 +92,8 @@ std::shared_ptr<oauth2_internal::Credentials> MapCredentials(
 
     void visit(ImpersonateServiceAccountConfig& cfg) override {
       result = std::make_shared<
-          oauth2_internal::ImpersonateServiceAccountCredentials>(cfg);
+          oauth2_internal::ImpersonateServiceAccountCredentials>(
+          cfg, [](Options const& o) { return MakeDefaultRestClient("", o); });
       result = Decorate(std::move(result), cfg.options());
     }
 

google/cloud/internal/unified_rest_credentials_integration_test.cc

@@ -108,8 +108,9 @@ TEST(UnifiedRestCredentialsIntegrationTest, AccessTokenCredentials) {
       "GOOGLE_APPLICATION_CREDENTIALS", key_file);
   auto default_creds = oauth2_internal::GoogleDefaultCredentials();
   ASSERT_THAT(default_creds, IsOk());
-  auto iam_creds =
-      oauth2_internal::MakeMinimalIamCredentialsRestStub(*default_creds);
+  auto iam_creds = oauth2_internal::MakeMinimalIamCredentialsRestStub(
+      *default_creds, Options{},
+      [](Options const& o) { return MakeDefaultRestClient("", o); });
   oauth2_internal::GenerateAccessTokenRequest request;
   request.lifetime = std::chrono::hours(1);
   request.service_account = std::move(*service_account);