From 79737f14738b60617404e69a2ac503066ad2afe7 Mon Sep 17 00:00:00 2001 From: Ahson Khan Date: Tue, 12 Sep 2023 23:18:05 -0700 Subject: [PATCH] Make several params to WorkloadIdentityCredential optional and read them from the environment instead. (#4893) * Make several params to WorkloadIdentityCredential optional and read them from the environment instead. * Fix typo in calling the ClientCredentialCore ctor * Add changelog entry. * Address PR feedback, avoid creating many WIC options instances. * Update ctor impl and add options test. * Set locals if the options are non-empty, and void unused variable in tests. * Fixup the sample since the customer no longer needs to explicitly pass in those params. --- sdk/identity/azure-identity/CHANGELOG.md | 2 + .../identity/workload_identity_credential.hpp | 46 +++--- .../samples/workload_identity_credential.cpp | 16 +- .../src/workload_identity_credential.cpp | 147 +++++++++++++----- .../ut/workload_identity_credential_test.cpp | 65 ++++++-- 5 files changed, 178 insertions(+), 98 deletions(-) diff --git a/sdk/identity/azure-identity/CHANGELOG.md b/sdk/identity/azure-identity/CHANGELOG.md index 8efd8c4c0..24ba6bc0b 100644 --- a/sdk/identity/azure-identity/CHANGELOG.md +++ b/sdk/identity/azure-identity/CHANGELOG.md @@ -4,6 +4,8 @@ ### Features Added +- Add support for reading the tenant id, client id, and the token file path for `WorkloadIdentityCredential` from the environment variables. + ### Breaking Changes ### Bugs Fixed diff --git a/sdk/identity/azure-identity/inc/azure/identity/workload_identity_credential.hpp b/sdk/identity/azure-identity/inc/azure/identity/workload_identity_credential.hpp index 9da4c4a68..2dc3613f6 100644 --- a/sdk/identity/azure-identity/inc/azure/identity/workload_identity_credential.hpp +++ b/sdk/identity/azure-identity/inc/azure/identity/workload_identity_credential.hpp @@ -27,15 +27,34 @@ namespace Azure { namespace Identity { */ struct WorkloadIdentityCredentialOptions final : public Core::Credentials::TokenCredentialOptions { + /** + * @brief The TenantID of the service principal. Defaults to the value of the environment + * variable AZURE_TENANT_ID. + */ + std::string TenantId; + + /** + * @brief The ClientID of the service principal. Defaults to the value of the environment + * variable AZURE_CLIENT_ID. + */ + std::string ClientId; + /** * @brief Authentication authority URL. - * @note Default value is Azure AD global authority (https://login.microsoftonline.com/). + * @note Defaults to the value of the environment variable AZURE_AUTHORITY_HOST. If that's not + * set, the default value is Azure AD global authority (https://login.microsoftonline.com/). * * @note Example of an authority host string: "https://login.microsoftonline.us/". See national * clouds' Azure AD authentication endpoints: * https://docs.microsoft.com/azure/active-directory/develop/authentication-national-cloud. */ - std::string AuthorityHost = _detail::ClientCredentialCore::AadGlobalAuthority; + std::string AuthorityHost; + + /** + * @brief The path of a file containing a Kubernetes service account token. Defaults to the + * value of the environment variable AZURE_FEDERATED_TOKEN_FILE. + */ + std::string TokenFilePath; /** * @brief For multi-tenant applications, specifies additional tenants for which the credential @@ -60,43 +79,22 @@ namespace Azure { namespace Identity { std::string m_requestBody; std::string m_tokenFilePath; - explicit WorkloadIdentityCredential( - std::string tenantId, - std::string const& clientId, - std::string const& tokenFilePath, - std::string const& authorityHost, - std::vector additionallyAllowedTenants, - Core::Credentials::TokenCredentialOptions const& options); - public: /** * @brief Constructs a Workload Identity Credential. * - * @param tenantId Tenant ID. - * @param clientId Client ID. - * @param tokenFilePath Path of a file containing a Kubernetes service account token. * @param options Options for token retrieval. */ explicit WorkloadIdentityCredential( - std::string tenantId, - std::string const& clientId, - std::string const& tokenFilePath, Core::Credentials::TokenCredentialOptions const& options = Core::Credentials::TokenCredentialOptions()); /** * @brief Constructs a Workload Identity Credential. * - * @param tenantId Tenant ID. - * @param clientId Client ID. - * @param tokenFilePath Path of a file containing a Kubernetes service account token. * @param options Options for token retrieval. */ - explicit WorkloadIdentityCredential( - std::string tenantId, - std::string const& clientId, - std::string const& tokenFilePath, - WorkloadIdentityCredentialOptions const& options); + explicit WorkloadIdentityCredential(WorkloadIdentityCredentialOptions const& options); /** * @brief Destructs `%WorkloadIdentityCredential`. diff --git a/sdk/identity/azure-identity/samples/workload_identity_credential.cpp b/sdk/identity/azure-identity/samples/workload_identity_credential.cpp index c46683f90..350765444 100644 --- a/sdk/identity/azure-identity/samples/workload_identity_credential.cpp +++ b/sdk/identity/azure-identity/samples/workload_identity_credential.cpp @@ -10,26 +10,14 @@ // * AZURE_TENANT_ID: Tenant ID for the Azure account. // * AZURE_CLIENT_ID: The Client ID to authenticate the request. // * AZURE_FEDERATED_TOKEN_FILE: The path of a file containing a Kubernetes service account token. -std::string GetTenantId() { return std::getenv("AZURE_TENANT_ID"); } -std::string GetClientId() { return std::getenv("AZURE_CLIENT_ID"); } -std::string GetTokenFilePath() -{ - char const* const tokenFilePath{std::getenv("AZURE_FEDERATED_TOKEN_FILE")}; - if (tokenFilePath == nullptr) - { - std::cerr << "Missing environment variable AZURE_FEDERATED_TOKEN_FILE" << std::endl; - return std::string(); - } - return tokenFilePath; -} int main() { try { // Step 1: Initialize Workload Identity Credential. - auto workloadIdentityCredential = std::make_shared( - GetTenantId(), GetClientId(), GetTokenFilePath()); + auto workloadIdentityCredential + = std::make_shared(); // Step 2: Pass the credential to an Azure Service Client. Azure::Service::Client azureServiceClient("serviceUrl", workloadIdentityCredential); diff --git a/sdk/identity/azure-identity/src/workload_identity_credential.cpp b/sdk/identity/azure-identity/src/workload_identity_credential.cpp index 7284f929a..c8f8fb8e0 100644 --- a/sdk/identity/azure-identity/src/workload_identity_credential.cpp +++ b/sdk/identity/azure-identity/src/workload_identity_credential.cpp @@ -6,6 +6,8 @@ #include "private/tenant_id_resolver.hpp" #include "private/token_credential_impl.hpp" +#include + #include #include @@ -13,61 +15,120 @@ using Azure::Identity::WorkloadIdentityCredential; using Azure::Core::Context; using Azure::Core::Url; +using Azure::Core::_internal::Environment; using Azure::Core::Credentials::AccessToken; using Azure::Core::Credentials::TokenRequestContext; using Azure::Core::Http::HttpMethod; using Azure::Identity::_detail::TenantIdResolver; using Azure::Identity::_detail::TokenCredentialImpl; +namespace { +constexpr auto AzureTenantIdEnvVarName = "AZURE_TENANT_ID"; +constexpr auto AzureClientIdEnvVarName = "AZURE_CLIENT_ID"; +constexpr auto AzureAuthorityHostEnvVarName = "AZURE_AUTHORITY_HOST"; +constexpr auto AzureFederatedTokenFileEnvVarName = "AZURE_FEDERATED_TOKEN_FILE"; +} // namespace + +WorkloadIdentityCredential::WorkloadIdentityCredential( + WorkloadIdentityCredentialOptions const& options) + : TokenCredential("WorkloadIdentityCredential"), m_clientCredentialCore( + options.TenantId, + options.AuthorityHost, + options.AdditionallyAllowedTenants) +{ + std::string tenantId = options.TenantId; + std::string clientId = options.ClientId; + std::string authorityHost = options.AuthorityHost; + m_tokenFilePath = options.TokenFilePath; + + if (tenantId.empty()) + { + tenantId = Environment::GetVariable(AzureTenantIdEnvVarName); + if (tenantId.empty()) + { + throw std::runtime_error( + "No tenant ID specified. Check pod configuration or set TenantID in the options."); + } + } + if (clientId.empty()) + { + clientId = Environment::GetVariable(AzureClientIdEnvVarName); + if (clientId.empty()) + { + throw std::runtime_error( + "No client ID specified. Check pod configuration or set ClientID in the options."); + } + } + if (authorityHost.empty()) + { + authorityHost = Environment::GetVariable(AzureAuthorityHostEnvVarName); + if (authorityHost.empty()) + { + authorityHost = _detail::ClientCredentialCore::AadGlobalAuthority; + } + } + if (m_tokenFilePath.empty()) + { + m_tokenFilePath = Environment::GetVariable(AzureFederatedTokenFileEnvVarName); + if (m_tokenFilePath.empty()) + { + throw std::runtime_error( + "No token file specified. Check pod configuration or set TokenFilePath in the options."); + } + } + + m_clientCredentialCore = Azure::Identity::_detail::ClientCredentialCore( + tenantId, authorityHost, options.AdditionallyAllowedTenants); + m_tokenCredentialImpl = std::make_unique(options); + m_requestBody + = std::string( + "grant_type=client_credentials" + "&client_assertion_type=" + "urn%3Aietf%3Aparams%3Aoauth%3Aclient-assertion-type%3Ajwt-bearer" // cspell:disable-line + "&client_id=") + + Url::Encode(clientId); +} + WorkloadIdentityCredential::WorkloadIdentityCredential( - std::string tenantId, - std::string const& clientId, - std::string const& tokenFilePath, - std::string const& authorityHost, - std::vector additionallyAllowedTenants, Core::Credentials::TokenCredentialOptions const& options) : TokenCredential("WorkloadIdentityCredential"), - m_clientCredentialCore(tenantId, authorityHost, additionallyAllowedTenants), - m_tokenCredentialImpl(std::make_unique(options)), - m_requestBody( - std::string( - "grant_type=client_credentials" - "&client_assertion_type=" - "urn%3Aietf%3Aparams%3Aoauth%3Aclient-assertion-type%3Ajwt-bearer" // cspell:disable-line - "&client_id=") - + Url::Encode(clientId)), - m_tokenFilePath(tokenFilePath) + m_clientCredentialCore("", "", std::vector()) { -} + std::string tenantId = Environment::GetVariable(AzureTenantIdEnvVarName); + if (tenantId.empty()) + { + throw std::runtime_error( + "No tenant ID specified. Check pod configuration or set TenantID in the options."); + } + std::string clientId = Environment::GetVariable(AzureClientIdEnvVarName); + if (clientId.empty()) + { + throw std::runtime_error( + "No client ID specified. Check pod configuration or set ClientID in the options."); + }; + std::string authorityHost = Environment::GetVariable(AzureAuthorityHostEnvVarName); + if (authorityHost.empty()) + { + authorityHost = _detail::ClientCredentialCore::AadGlobalAuthority; + } -WorkloadIdentityCredential::WorkloadIdentityCredential( - std::string tenantId, - std::string const& clientId, - std::string const& tokenFilePath, - WorkloadIdentityCredentialOptions const& options) - : WorkloadIdentityCredential( - tenantId, - clientId, - tokenFilePath, - options.AuthorityHost, - options.AdditionallyAllowedTenants, - options) -{ -} + m_tokenFilePath = Environment::GetVariable(AzureFederatedTokenFileEnvVarName); + if (m_tokenFilePath.empty()) + { + throw std::runtime_error( + "No token file specified. Check pod configuration or set TokenFilePath in the options."); + } -WorkloadIdentityCredential::WorkloadIdentityCredential( - std::string tenantId, - std::string const& clientId, - std::string const& tokenFilePath, - Core::Credentials::TokenCredentialOptions const& options) - : WorkloadIdentityCredential( - tenantId, - clientId, - tokenFilePath, - WorkloadIdentityCredentialOptions{}.AuthorityHost, - WorkloadIdentityCredentialOptions{}.AdditionallyAllowedTenants, - options) -{ + m_clientCredentialCore = Azure::Identity::_detail::ClientCredentialCore( + tenantId, authorityHost, std::vector()); + m_tokenCredentialImpl = std::make_unique(options); + m_requestBody + = std::string( + "grant_type=client_credentials" + "&client_assertion_type=" + "urn%3Aietf%3Aparams%3Aoauth%3Aclient-assertion-type%3Ajwt-bearer" // cspell:disable-line + "&client_id=") + + Url::Encode(clientId); } WorkloadIdentityCredential::~WorkloadIdentityCredential() = default; diff --git a/sdk/identity/azure-identity/test/ut/workload_identity_credential_test.cpp b/sdk/identity/azure-identity/test/ut/workload_identity_credential_test.cpp index 3484c2467..ac53d22bd 100644 --- a/sdk/identity/azure-identity/test/ut/workload_identity_credential_test.cpp +++ b/sdk/identity/azure-identity/test/ut/workload_identity_credential_test.cpp @@ -26,14 +26,45 @@ struct TempCertFile final TEST(WorkloadIdentityCredential, GetCredentialName) { TempCertFile const tempCertFile; - WorkloadIdentityCredential const cred( - "01234567-89ab-cdef-fedc-ba8976543210", - "fedcba98-7654-3210-0123-456789abcdef", - TempCertFile::Path); + WorkloadIdentityCredentialOptions options; + options.TenantId = "01234567-89ab-cdef-fedc-ba8976543210"; + options.ClientId = "fedcba98-7654-3210-0123-456789abcdef"; + options.TokenFilePath = TempCertFile::Path; + + WorkloadIdentityCredential const cred(options); EXPECT_EQ(cred.GetCredentialName(), "WorkloadIdentityCredential"); } +TEST(WorkloadIdentityCredential, GetOptionsFromEnvironment) +{ + CredentialTestHelper::EnvironmentOverride const env( + {{"AZURE_TENANT_ID", "01234567-89ab-cdef-fedc-ba8976543210"}, + {"AZURE_CLIENT_ID", "fedcba98-7654-3210-0123-456789abcdef"}, + {"AZURE_AUTHORITY_HOST", ""}, + {"AZURE_FEDERATED_TOKEN_FILE", TempCertFile::Path}}); + + WorkloadIdentityCredential const credDefault; + EXPECT_EQ(credDefault.GetCredentialName(), "WorkloadIdentityCredential"); + + WorkloadIdentityCredentialOptions options; + WorkloadIdentityCredential const cred(options); + EXPECT_EQ(cred.GetCredentialName(), "WorkloadIdentityCredential"); +} + +TEST(WorkloadIdentityCredential, GetOptionsFromEnvironmentInvalid) +{ + CredentialTestHelper::EnvironmentOverride const env( + {{"AZURE_TENANT_ID", ""}, + {"AZURE_CLIENT_ID", ""}, + {"AZURE_AUTHORITY_HOST", ""}, + {"AZURE_FEDERATED_TOKEN_FILE", ""}}); + + EXPECT_THROW((void)WorkloadIdentityCredential{}, std::runtime_error); + WorkloadIdentityCredentialOptions options; + EXPECT_THROW((void)WorkloadIdentityCredential{options}, std::runtime_error); +} + TEST(WorkloadIdentityCredential, Regular) { TempCertFile const tempCertFile; @@ -42,12 +73,11 @@ TEST(WorkloadIdentityCredential, Regular) [](auto transport) { WorkloadIdentityCredentialOptions options; options.Transport.Transport = transport; + options.TenantId = "01234567-89ab-cdef-fedc-ba8976543210"; + options.ClientId = "fedcba98-7654-3210-0123-456789abcdef"; + options.TokenFilePath = TempCertFile::Path; - return std::make_unique( - "01234567-89ab-cdef-fedc-ba8976543210", - "fedcba98-7654-3210-0123-456789abcdef", - TempCertFile::Path, - options); + return std::make_unique(options); }, {{{"https://azure.com/.default"}}, {{}}}, std::vector{ @@ -130,9 +160,11 @@ TEST(WorkloadIdentityCredential, AzureStack) [](auto transport) { WorkloadIdentityCredentialOptions options; options.Transport.Transport = transport; + options.TenantId = "adfs"; + options.ClientId = "fedcba98-7654-3210-0123-456789abcdef"; + options.TokenFilePath = TempCertFile::Path; - return std::make_unique( - "adfs", "fedcba98-7654-3210-0123-456789abcdef", TempCertFile::Path, options); + return std::make_unique(options); }, {{{"https://azure.com/.default"}}, {{}}}, std::vector{ @@ -210,14 +242,13 @@ TEST(WorkloadIdentityCredential, Authority) auto const actual = CredentialTestHelper::SimulateTokenRequest( [](auto transport) { WorkloadIdentityCredentialOptions options; - options.AuthorityHost = "https://microsoft.com/"; options.Transport.Transport = transport; + options.TenantId = "01234567-89ab-cdef-fedc-ba8976543210"; + options.ClientId = "fedcba98-7654-3210-0123-456789abcdef"; + options.AuthorityHost = "https://microsoft.com/"; + options.TokenFilePath = TempCertFile::Path; - return std::make_unique( - "01234567-89ab-cdef-fedc-ba8976543210", - "fedcba98-7654-3210-0123-456789abcdef", - TempCertFile::Path, - options); + return std::make_unique(options); }, {{{"https://azure.com/.default"}}, {{}}}, std::vector{