fix(storage): cache all credential types (#11447)

Applications may call `AuthorizedHeader()` and that can generate calls
to the GCE metadata service or Google's Security Token Service.

Author: coryan

google/cloud/storage/oauth2/authorized_user_credentials.cc

@@ -78,13 +78,23 @@ AuthorizedUserCredentials<storage::internal::CurlRequestBuilder,
     AuthorizedUserCredentials(
         google::cloud::oauth2_internal::AuthorizedUserCredentialsInfo info,
         ChannelOptions const& channel_options)
-    : impl_(
+    : AuthorizedUserCredentials(
           std::move(info),
           Options{}.set<CARootsFilePathOption>(channel_options.ssl_root_path()),
           [](Options const& o) {
             return rest_internal::MakeDefaultRestClient(std::string{}, o);
           }) {}
 
+AuthorizedUserCredentials<storage::internal::CurlRequestBuilder,
+                          std::chrono::system_clock>::
+    AuthorizedUserCredentials(
+        google::cloud::oauth2_internal::AuthorizedUserCredentialsInfo info,
+        Options options, oauth2_internal::HttpClientFactory client_factory)
+    : impl_(std::make_shared<oauth2_internal::CachedCredentials>(
+          std::make_shared<oauth2_internal::AuthorizedUserCredentials>(
+              std::move(info), std::move(options),
+              std::move(client_factory)))) {}
+
 }  // namespace oauth2
 GOOGLE_CLOUD_CPP_INLINE_NAMESPACE_END
 }  // namespace storage

google/cloud/storage/oauth2/authorized_user_credentials.h

@@ -23,6 +23,7 @@
 #include "google/cloud/storage/oauth2/refreshing_credentials_wrapper.h"
 #include "google/cloud/storage/version.h"
 #include "google/cloud/internal/oauth2_authorized_user_credentials.h"
+#include "google/cloud/internal/oauth2_cached_credentials.h"
 #include "google/cloud/internal/oauth2_credential_constants.h"
 #include "google/cloud/status.h"
 #include <chrono>
@@ -114,11 +115,21 @@ class AuthorizedUserCredentials<storage::internal::CurlRequestBuilder,
       ChannelOptions const& channel_options = {});
 
   StatusOr<std::string> AuthorizationHeader() override {
-    return oauth2_internal::AuthorizationHeaderJoined(impl_);
+    return oauth2_internal::AuthorizationHeaderJoined(*impl_);
   }
 
  private:
-  google::cloud::oauth2_internal::AuthorizedUserCredentials impl_;
+  friend struct AuthorizedUserCredentialsTester;
+  AuthorizedUserCredentials(
+      google::cloud::oauth2_internal::AuthorizedUserCredentialsInfo,
+      Options options, oauth2_internal::HttpClientFactory client_factory);
+
+  StatusOr<std::string> AuthorizationHeaderForTesting(
+      std::chrono::system_clock::time_point tp) {
+    return oauth2_internal::AuthorizationHeaderJoined(*impl_, tp);
+  }
+
+  std::shared_ptr<google::cloud::oauth2_internal::Credentials> impl_;
 };
 
 /// @copydoc AuthorizedUserCredentials

google/cloud/storage/oauth2/authorized_user_credentials_test.cc

@@ -16,6 +16,9 @@
 #include "google/cloud/storage/oauth2/credential_constants.h"
 #include "google/cloud/storage/testing/mock_http_request.h"
 #include "google/cloud/testing_util/mock_fake_clock.h"
+#include "google/cloud/testing_util/mock_http_payload.h"
+#include "google/cloud/testing_util/mock_rest_client.h"
+#include "google/cloud/testing_util/mock_rest_response.h"
 #include "google/cloud/testing_util/status_matchers.h"
 #include <gmock/gmock.h>
 #include <nlohmann/json.hpp>
@@ -26,18 +29,40 @@ namespace cloud {
 namespace storage {
 GOOGLE_CLOUD_CPP_INLINE_NAMESPACE_BEGIN
 namespace oauth2 {
+
+// Define a helper to test the specialization.
+struct AuthorizedUserCredentialsTester {
+  static StatusOr<std::string> Header(
+      AuthorizedUserCredentials<>& tested,
+      std::chrono::system_clock::time_point tp) {
+    return tested.AuthorizationHeaderForTesting(tp);
+  }
+
+  static AuthorizedUserCredentials<> MakeAuthorizedUserCredentials(
+      google::cloud::oauth2_internal::AuthorizedUserCredentialsInfo info,
+      oauth2_internal::HttpClientFactory factory) {
+    return AuthorizedUserCredentials<>(std::move(info), Options{},
+                                       std::move(factory));
+  }
+};
+
 namespace {
 
 using ::google::cloud::storage::internal::HttpResponse;
 using ::google::cloud::storage::testing::MockHttpRequest;
 using ::google::cloud::storage::testing::MockHttpRequestBuilder;
 using ::google::cloud::testing_util::FakeClock;
 using ::google::cloud::testing_util::IsOk;
+using ::google::cloud::testing_util::IsOkAndHolds;
+using ::google::cloud::testing_util::MakeMockHttpPayloadSuccess;
+using ::google::cloud::testing_util::MockRestClient;
+using ::google::cloud::testing_util::MockRestResponse;
 using ::google::cloud::testing_util::StatusIs;
 using ::testing::_;
 using ::testing::AllOf;
 using ::testing::An;
 using ::testing::AtLeast;
+using ::testing::ByMove;
 using ::testing::HasSubstr;
 using ::testing::Not;
 using ::testing::Return;
@@ -413,6 +438,60 @@ TEST_F(AuthorizedUserCredentialsTest, ParseAuthorizedUserRefreshResponse) {
   EXPECT_EQ(token.token, "Authorization: Type access-token-r1");
 }
 
+TEST_F(AuthorizedUserCredentialsTest, Caching) {
+  // We need to mock the Security Token Service or this would be an
+  // integration test that requires a valid user account.
+  auto make_mock_client = [](std::string const& payload) {
+    auto response = std::make_unique<MockRestResponse>();
+    EXPECT_CALL(*response, StatusCode)
+        .WillRepeatedly(
+            Return(google::cloud::rest_internal::HttpStatusCode::kOk));
+    EXPECT_CALL(std::move(*response), ExtractPayload)
+        .WillOnce(Return(ByMove(MakeMockHttpPayloadSuccess(payload))));
+    auto mock = std::make_unique<MockRestClient>();
+    using PostPayloadType = std::vector<std::pair<std::string, std::string>>;
+    EXPECT_CALL(*mock, Post(_, _, An<PostPayloadType const&>()))
+        .WillOnce(Return(ByMove(std::unique_ptr<rest_internal::RestResponse>(
+            std::move(response)))));
+    return std::unique_ptr<rest_internal::RestClient>(std::move(mock));
+  };
+
+  auto constexpr kPayload1 = R"js({
+    "access_token": "access-token-1", "id_token": "id-token-1",
+    "token_type": "Bearer", "expires_in": 3600})js";
+  auto constexpr kPayload2 = R"js({
+    "access_token": "access-token-2", "id_token": "id-token-2",
+    "token_type": "Bearer", "expires_in": 3600})js";
+
+  using MockHttpClientFactory =
+      ::testing::MockFunction<std::unique_ptr<rest_internal::RestClient>(
+          Options const&)>;
+  MockHttpClientFactory mock_factory;
+  EXPECT_CALL(mock_factory, Call)
+      .WillOnce(Return(ByMove(make_mock_client(kPayload1))))
+      .WillOnce(Return(ByMove(make_mock_client(kPayload2))));
+
+  auto tested = AuthorizedUserCredentialsTester::MakeAuthorizedUserCredentials(
+      oauth2_internal::AuthorizedUserCredentialsInfo{},
+      mock_factory.AsStdFunction());
+  auto const tp = std::chrono::system_clock::now();
+  auto initial = AuthorizedUserCredentialsTester::Header(tested, tp);
+  ASSERT_STATUS_OK(initial);
+
+  auto cached = AuthorizedUserCredentialsTester::Header(
+      tested, tp + std::chrono::seconds(30));
+  EXPECT_THAT(cached, IsOkAndHolds(*initial));
+
+  cached = AuthorizedUserCredentialsTester::Header(
+      tested, tp + std::chrono::seconds(300));
+  EXPECT_THAT(cached, IsOkAndHolds(*initial));
+
+  auto uncached = AuthorizedUserCredentialsTester::Header(
+      tested, tp + std::chrono::hours(2));
+  ASSERT_STATUS_OK(uncached);
+  EXPECT_NE(*initial, *uncached);
+}
+
 }  // namespace
 }  // namespace oauth2
 GOOGLE_CLOUD_CPP_INLINE_NAMESPACE_END

google/cloud/storage/oauth2/compute_engine_credentials.cc

@@ -13,6 +13,7 @@
 // limitations under the License.
 
 #include "google/cloud/storage/oauth2/compute_engine_credentials.h"
+#include "google/cloud/internal/oauth2_cached_credentials.h"
 #include "google/cloud/internal/oauth2_compute_engine_credentials.h"
 #include <nlohmann/json.hpp>
 
@@ -61,9 +62,19 @@ ParseComputeEngineRefreshResponse(
 ComputeEngineCredentials<storage::internal::CurlRequestBuilder,
                          std::chrono::system_clock>::
     ComputeEngineCredentials(std::string service_account_email)
-    : impl_(std::move(service_account_email), Options{}, [](Options const& o) {
-        return rest_internal::MakeDefaultRestClient(std::string{}, o);
-      }) {}
+    : ComputeEngineCredentials(
+          std::move(service_account_email), [](Options const& o) {
+            return rest_internal::MakeDefaultRestClient(std::string{}, o);
+          }) {}
+
+ComputeEngineCredentials<storage::internal::CurlRequestBuilder,
+                         std::chrono::system_clock>::
+    ComputeEngineCredentials(std::string service_account_email,
+                             oauth2_internal::HttpClientFactory client_factory)
+    : impl_(std::make_shared<oauth2_internal::ComputeEngineCredentials>(
+          std::move(service_account_email), Options{},
+          std::move(client_factory))),
+      cached_(std::make_shared<oauth2_internal::CachedCredentials>(impl_)) {}
 
 }  // namespace oauth2
 GOOGLE_CLOUD_CPP_INLINE_NAMESPACE_END

google/cloud/storage/oauth2/compute_engine_credentials.h

@@ -23,6 +23,7 @@
 #include "google/cloud/storage/oauth2/refreshing_credentials_wrapper.h"
 #include "google/cloud/storage/version.h"
 #include "google/cloud/internal/getenv.h"
+#include "google/cloud/internal/oauth2_cached_credentials.h"
 #include "google/cloud/internal/oauth2_compute_engine_credentials.h"
 #include "google/cloud/status.h"
 #include <chrono>
@@ -106,10 +107,10 @@ class ComputeEngineCredentials<storage::internal::CurlRequestBuilder,
   explicit ComputeEngineCredentials(std::string service_account_email);
 
   StatusOr<std::string> AuthorizationHeader() override {
-    return oauth2_internal::AuthorizationHeaderJoined(impl_);
+    return oauth2_internal::AuthorizationHeaderJoined(*cached_);
   }
 
-  std::string AccountEmail() const override { return impl_.AccountEmail(); }
+  std::string AccountEmail() const override { return impl_->AccountEmail(); }
 
   /**
    * Returns the email or alias of this credential's service account.
@@ -120,7 +121,7 @@ class ComputeEngineCredentials<storage::internal::CurlRequestBuilder,
    * initializing this credential, that alias is returned as this credential's
    * email address if the credential has not been refreshed yet.
    */
-  std::string service_account_email() { return impl_.service_account_email(); }
+  std::string service_account_email() { return impl_->service_account_email(); }
 
   /**
    * Returns the set of scopes granted to this credential's service account.
@@ -129,10 +130,20 @@ class ComputeEngineCredentials<storage::internal::CurlRequestBuilder,
    * server to fetch service account metadata, this method will return an empty
    * set if the credential has not been refreshed yet.
    */
-  std::set<std::string> scopes() const { return impl_.scopes(); }
+  std::set<std::string> scopes() const { return impl_->scopes(); }
 
  private:
-  google::cloud::oauth2_internal::ComputeEngineCredentials impl_;
+  friend struct ComputeEngineCredentialsTester;
+  ComputeEngineCredentials(std::string service_account_email,
+                           oauth2_internal::HttpClientFactory client_factory);
+
+  StatusOr<std::string> AuthorizationHeaderForTesting(
+      std::chrono::system_clock::time_point tp) {
+    return oauth2_internal::AuthorizationHeaderJoined(*cached_, tp);
+  }
+
+  std::shared_ptr<oauth2_internal::ComputeEngineCredentials> impl_;
+  std::shared_ptr<oauth2_internal::CachedCredentials> cached_;
 };
 
 /// @copydoc ComputeEngineCredentials

google/cloud/storage/oauth2/compute_engine_credentials_test.cc

@@ -17,6 +17,9 @@
 #include "google/cloud/storage/oauth2/credential_constants.h"
 #include "google/cloud/storage/testing/mock_http_request.h"
 #include "google/cloud/testing_util/mock_fake_clock.h"
+#include "google/cloud/testing_util/mock_http_payload.h"
+#include "google/cloud/testing_util/mock_rest_client.h"
+#include "google/cloud/testing_util/mock_rest_response.h"
 #include "google/cloud/testing_util/status_matchers.h"
 #include <gmock/gmock.h>
 #include <nlohmann/json.hpp>
@@ -28,6 +31,23 @@ namespace cloud {
 namespace storage {
 GOOGLE_CLOUD_CPP_INLINE_NAMESPACE_BEGIN
 namespace oauth2 {
+
+// Define a helper to test the specialization.
+struct ComputeEngineCredentialsTester {
+  static StatusOr<std::string> Header(
+      ComputeEngineCredentials<>& tested,
+      std::chrono::system_clock::time_point tp) {
+    return tested.AuthorizationHeaderForTesting(tp);
+  }
+
+  static ComputeEngineCredentials<> MakeComputeEngineCredentials(
+      std::string service_account_email,
+      oauth2_internal::HttpClientFactory factory) {
+    return ComputeEngineCredentials<>(std::move(service_account_email),
+                                      std::move(factory));
+  }
+};
+
 namespace {
 
 using ::google::cloud::storage::internal::GceMetadataHostname;
@@ -36,8 +56,13 @@ using ::google::cloud::storage::testing::MockHttpRequest;
 using ::google::cloud::storage::testing::MockHttpRequestBuilder;
 using ::google::cloud::testing_util::FakeClock;
 using ::google::cloud::testing_util::IsOk;
+using ::google::cloud::testing_util::IsOkAndHolds;
+using ::google::cloud::testing_util::MakeMockHttpPayloadSuccess;
+using ::google::cloud::testing_util::MockRestClient;
+using ::google::cloud::testing_util::MockRestResponse;
 using ::google::cloud::testing_util::StatusIs;
 using ::testing::_;
+using ::testing::ByMove;
 using ::testing::HasSubstr;
 using ::testing::IsEmpty;
 using ::testing::Not;
@@ -408,6 +433,68 @@ TEST_F(ComputeEngineCredentialsTest, AccountEmail) {
   EXPECT_EQ(credentials.service_account_email(), refreshed_email);
 }
 
+TEST_F(ComputeEngineCredentialsTest, Caching) {
+  // We need to mock the Compute Engine Metadata Service or this would be an
+  // integration test that only runs on GCE.
+  auto make_mock_client = [](std::string const& payload) {
+    auto response = std::make_unique<MockRestResponse>();
+    EXPECT_CALL(*response, StatusCode)
+        .WillRepeatedly(
+            Return(google::cloud::rest_internal::HttpStatusCode::kOk));
+    EXPECT_CALL(std::move(*response), ExtractPayload)
+        .WillOnce(Return(ByMove(MakeMockHttpPayloadSuccess(payload))));
+    auto mock = std::make_unique<MockRestClient>();
+    EXPECT_CALL(*mock, Get)
+        .WillOnce(Return(ByMove(std::unique_ptr<rest_internal::RestResponse>(
+            std::move(response)))));
+    return std::unique_ptr<rest_internal::RestClient>(std::move(mock));
+  };
+
+  // We expect a call to the metadata service to get the service account
+  // metadata. Then one call to get the token (which is cached), and finally a
+  // call the refresh the token.
+  auto constexpr kPayload0 =
+      R"""({"email": "test-only-email", "scopes": ["s1", "s2]})""";
+  auto constexpr kPayload1 = R"""({
+            "access_token": "test-token-1",
+            "expires_in": 3600,
+            "token_type": "tokentype"
+        })""";
+  auto constexpr kPayload2 = R"""({
+            "access_token": "test-token-2",
+            "expires_in": 3600,
+            "token_type": "tokentype"
+        })""";
+
+  using MockHttpClientFactory =
+      ::testing::MockFunction<std::unique_ptr<rest_internal::RestClient>(
+          Options const&)>;
+  MockHttpClientFactory mock_factory;
+  EXPECT_CALL(mock_factory, Call)
+      .WillOnce(Return(ByMove(make_mock_client(kPayload0))))
+      .WillOnce(Return(ByMove(make_mock_client(kPayload1))))
+      .WillOnce(Return(ByMove(make_mock_client(kPayload2))));
+
+  auto tested = ComputeEngineCredentialsTester::MakeComputeEngineCredentials(
+      "test-only", mock_factory.AsStdFunction());
+  auto const tp = std::chrono::system_clock::now();
+  auto initial = ComputeEngineCredentialsTester::Header(tested, tp);
+  ASSERT_STATUS_OK(initial);
+
+  auto cached = ComputeEngineCredentialsTester::Header(
+      tested, tp + std::chrono::seconds(30));
+  EXPECT_THAT(cached, IsOkAndHolds(*initial));
+
+  cached = ComputeEngineCredentialsTester::Header(
+      tested, tp + std::chrono::seconds(300));
+  EXPECT_THAT(cached, IsOkAndHolds(*initial));
+
+  auto uncached = ComputeEngineCredentialsTester::Header(
+      tested, tp + std::chrono::hours(2));
+  ASSERT_STATUS_OK(uncached);
+  EXPECT_NE(*initial, *uncached);
+}
+
 }  // namespace
 }  // namespace oauth2
 GOOGLE_CLOUD_CPP_INLINE_NAMESPACE_END