diff --git a/google/cloud/credentials.cc b/google/cloud/credentials.cc index 69a757c62b2ea..993ef39e364b6 100644 --- a/google/cloud/credentials.cc +++ b/google/cloud/credentials.cc @@ -48,13 +48,14 @@ std::shared_ptr MakeImpersonateServiceAccountCredentials( std::shared_ptr MakeServiceAccountCredentials( std::string json_object, Options opts) { return std::make_shared( - std::move(json_object), absl::nullopt, std::move(opts)); + internal::ServiceAccountJsonObject{std::move(json_object)}, + std::move(opts)); } std::shared_ptr MakeServiceAccountCredentialsFromFile( std::string const& file_path, Options opts) { return std::make_shared( - absl::nullopt, file_path, std::move(opts)); + internal::ServiceAccountFilePath{file_path}, std::move(opts)); } std::shared_ptr MakeExternalAccountCredentials( diff --git a/google/cloud/internal/credentials_impl.cc b/google/cloud/internal/credentials_impl.cc index f3a4b8ee9e515..3fdd1af55c3ed 100644 --- a/google/cloud/internal/credentials_impl.cc +++ b/google/cloud/internal/credentials_impl.cc @@ -87,11 +87,14 @@ std::vector const& ImpersonateServiceAccountConfig::delegates() return options_.get(); } -ServiceAccountConfig::ServiceAccountConfig( - absl::optional json_object, - absl::optional file_path, Options opts) - : json_object_(std::move(json_object)), - file_path_(std::move(file_path)), +ServiceAccountConfig::ServiceAccountConfig(ServiceAccountFilePath path, + Options opts) + : file_path_(std::move(path.value)), + options_(PopulateAuthOptions(std::move(opts))) {} + +ServiceAccountConfig::ServiceAccountConfig(ServiceAccountJsonObject json, + Options opts) + : json_object_(std::move(json.value)), options_(PopulateAuthOptions(std::move(opts))) {} ExternalAccountConfig::ExternalAccountConfig(std::string json_object, diff --git a/google/cloud/internal/credentials_impl.h b/google/cloud/internal/credentials_impl.h index 4b7eea36ab21e..ae90fb90b2b49 100644 --- a/google/cloud/internal/credentials_impl.h +++ b/google/cloud/internal/credentials_impl.h @@ -142,13 +142,22 @@ class ImpersonateServiceAccountConfig : public Credentials { Options options_; }; +struct ServiceAccountFilePath { + std::string value; + explicit ServiceAccountFilePath(std::string p) : value(std::move(p)) {} +}; + +struct ServiceAccountJsonObject { + std::string value; + explicit ServiceAccountJsonObject(std::string j) : value(std::move(j)) {} +}; + class ServiceAccountConfig : public Credentials { public: - // Only one of json_object or file_path should have a value. - // TODO(#15886): Use the C++ type system to make better constructors that - // enforces this comment. - ServiceAccountConfig(absl::optional json_object, - absl::optional file_path, Options opts); + // Just declarations ending with a semicolon + explicit ServiceAccountConfig(ServiceAccountFilePath path, Options opts); + + explicit ServiceAccountConfig(ServiceAccountJsonObject json, Options opts); absl::optional const& json_object() const { return json_object_; diff --git a/google/cloud/internal/credentials_impl_test.cc b/google/cloud/internal/credentials_impl_test.cc index d74aab145cf97..10e210b65d296 100644 --- a/google/cloud/internal/credentials_impl_test.cc +++ b/google/cloud/internal/credentials_impl_test.cc @@ -97,11 +97,14 @@ TEST(Credentials, ImpersonateServiceAccountCredentialsDefaultWithOptions) { } TEST(Credentials, ServiceAccount) { + // If MakeServiceAccountCredentials treats this string as a JSON object: auto credentials = MakeServiceAccountCredentials("test-only-invalid"); TestCredentialsVisitor visitor; CredentialsVisitor::dispatch(*credentials, visitor); ASSERT_EQ("ServiceAccountConfig", visitor.name); + // Update: Ensure the correct field is populated and the other is empty EXPECT_EQ("test-only-invalid", visitor.json_object); + EXPECT_EQ(absl::nullopt, visitor.file_path); } TEST(Credentials, ExternalAccount) { diff --git a/google/cloud/internal/unified_rest_credentials_test.cc b/google/cloud/internal/unified_rest_credentials_test.cc index b3d769bf462f0..dfd15f022ec54 100644 --- a/google/cloud/internal/unified_rest_credentials_test.cc +++ b/google/cloud/internal/unified_rest_credentials_test.cc @@ -404,8 +404,9 @@ TEST(UnifiedRestCredentialsTest, ServiceAccount) { MockClientFactory client_factory; EXPECT_CALL(client_factory, Call).Times(0); - auto const config = - internal::ServiceAccountConfig(contents.dump(), absl::nullopt, Options{}); + auto const config = internal::ServiceAccountConfig( + internal::ServiceAccountJsonObject{contents.dump()}, Options{}); + auto credentials = MapCredentials(config, client_factory.AsStdFunction()); auto access_token = credentials->GetToken(now); ASSERT_STATUS_OK(access_token);