Skip to content

Commit 64b9f7a

Browse files
authored
fix(storage): cache legacy credentials (#11333)
I think we missed the caching feature during the multiple refactorings. The test is (maybe) not ideal, but the API is not easy to test, and we do not want to change it. This is a backport from `main`, with changes to work without `IsOkAndHolds()`.
1 parent 1d03aed commit 64b9f7a

File tree

3 files changed

+51
-7
lines changed

3 files changed

+51
-7
lines changed

google/cloud/storage/oauth2/service_account_credentials.cc

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616
#include "google/cloud/storage/internal/make_jwt_assertion.h"
1717
#include "google/cloud/storage/internal/openssl_util.h"
1818
#include "google/cloud/internal/absl_str_join_quiet.h"
19+
#include "google/cloud/internal/oauth2_cached_credentials.h"
1920
#include "google/cloud/internal/oauth2_service_account_credentials.h"
2021
#include <nlohmann/json.hpp>
2122
#include <openssl/err.h>
@@ -122,12 +123,13 @@ ServiceAccountCredentials<storage::internal::CurlRequestBuilder,
122123
std::chrono::system_clock>::
123124
ServiceAccountCredentials(ServiceAccountCredentialsInfo info,
124125
ChannelOptions const& options)
125-
: impl_(std::make_unique<oauth2_internal::ServiceAccountCredentials>(
126-
internal::MapServiceAccountCredentialsInfo(std::move(info)),
127-
Options{}.set<CARootsFilePathOption>(options.ssl_root_path()),
128-
[](Options const& o) {
129-
return rest_internal::MakeDefaultRestClient(std::string{}, o);
130-
})) {}
126+
: impl_(std::make_unique<oauth2_internal::CachedCredentials>(
127+
std::make_unique<oauth2_internal::ServiceAccountCredentials>(
128+
internal::MapServiceAccountCredentialsInfo(std::move(info)),
129+
Options{}.set<CARootsFilePathOption>(options.ssl_root_path()),
130+
[](Options const& o) {
131+
return rest_internal::MakeDefaultRestClient(std::string{}, o);
132+
}))) {}
131133

132134
} // namespace oauth2
133135

google/cloud/storage/oauth2/service_account_credentials.h

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -243,7 +243,12 @@ class ServiceAccountCredentials<storage::internal::CurlRequestBuilder,
243243
std::string KeyId() const override { return impl_->KeyId(); }
244244

245245
private:
246-
std::unique_ptr<oauth2_internal::ServiceAccountCredentials> impl_;
246+
friend struct ServiceAccountCredentialsTester;
247+
StatusOr<std::string> AuthorizationHeaderForTesting(
248+
std::chrono::system_clock::time_point tp) {
249+
return oauth2_internal::AuthorizationHeaderJoined(*impl_, tp);
250+
}
251+
std::unique_ptr<oauth2_internal::Credentials> impl_;
247252
};
248253

249254
/// @copydoc ServiceAccountCredentials

google/cloud/storage/oauth2/service_account_credentials_test.cc

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,16 @@ namespace cloud {
3636
namespace storage {
3737
GOOGLE_CLOUD_CPP_INLINE_NAMESPACE_BEGIN
3838
namespace oauth2 {
39+
40+
// Define a helper to test the specialization.
41+
struct ServiceAccountCredentialsTester {
42+
static StatusOr<std::string> Header(
43+
ServiceAccountCredentials<>& tested,
44+
std::chrono::system_clock::time_point tp) {
45+
return tested.AuthorizationHeaderForTesting(tp);
46+
}
47+
};
48+
3949
namespace {
4050

4151
using ::google::cloud::internal::SignUsingSha256;
@@ -821,6 +831,33 @@ TEST_F(ServiceAccountCredentialsTest, UseOauth) {
821831
}
822832
}
823833

834+
TEST_F(ServiceAccountCredentialsTest, CachingWithSelfSignedJwt) {
835+
auto clear = ScopedEnvironment(
836+
"GOOGLE_CLOUD_CPP_EXPERIMENTAL_DISABLE_SELF_SIGNED_JWT", absl::nullopt);
837+
auto info = ParseServiceAccountCredentials(MakeTestContents(), "test");
838+
ASSERT_STATUS_OK(info);
839+
840+
ServiceAccountCredentials<> tested(*info);
841+
auto tp = std::chrono::system_clock::now();
842+
auto initial = ServiceAccountCredentialsTester::Header(tested, tp);
843+
ASSERT_STATUS_OK(initial);
844+
845+
auto cached = ServiceAccountCredentialsTester::Header(
846+
tested, tp + std::chrono::seconds(30));
847+
ASSERT_STATUS_OK(cached);
848+
EXPECT_THAT(*cached, *initial);
849+
850+
cached = ServiceAccountCredentialsTester::Header(
851+
tested, tp + std::chrono::seconds(300));
852+
ASSERT_STATUS_OK(cached);
853+
EXPECT_THAT(*cached, *initial);
854+
855+
auto uncached = ServiceAccountCredentialsTester::Header(
856+
tested, tp + std::chrono::hours(2));
857+
ASSERT_STATUS_OK(uncached);
858+
EXPECT_NE(*initial, *uncached);
859+
}
860+
824861
} // namespace
825862
} // namespace oauth2
826863
GOOGLE_CLOUD_CPP_INLINE_NAMESPACE_END

0 commit comments

Comments
 (0)