From 13f1759accdd3d0b0130e2d31b3011afb130ba48 Mon Sep 17 00:00:00 2001 From: Ahson Khan Date: Wed, 13 Sep 2023 16:57:24 -0700 Subject: [PATCH] Do not throw an exception during Credential construction, but rather delay it on GetToken call. (#4951) --- .../src/workload_identity_credential.cpp | 97 ++++++++----------- .../ut/workload_identity_credential_test.cpp | 11 ++- 2 files changed, 50 insertions(+), 58 deletions(-) diff --git a/sdk/identity/azure-identity/src/workload_identity_credential.cpp b/sdk/identity/azure-identity/src/workload_identity_credential.cpp index c8f8fb8e0..14f90f84a 100644 --- a/sdk/identity/azure-identity/src/workload_identity_credential.cpp +++ b/sdk/identity/azure-identity/src/workload_identity_credential.cpp @@ -17,6 +17,7 @@ using Azure::Core::Context; using Azure::Core::Url; using Azure::Core::_internal::Environment; using Azure::Core::Credentials::AccessToken; +using Azure::Core::Credentials::AuthenticationException; using Azure::Core::Credentials::TokenRequestContext; using Azure::Core::Http::HttpMethod; using Azure::Identity::_detail::TenantIdResolver; @@ -44,20 +45,10 @@ WorkloadIdentityCredential::WorkloadIdentityCredential( 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()) { @@ -70,23 +61,21 @@ WorkloadIdentityCredential::WorkloadIdentityCredential( 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); + if (!tenantId.empty() && !clientId.empty() && !m_tokenFilePath.empty()) + { + 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( @@ -95,40 +84,28 @@ WorkloadIdentityCredential::WorkloadIdentityCredential( 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; - } - 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, 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); + if (!tenantId.empty() && !clientId.empty() && !m_tokenFilePath.empty()) + { + std::string authorityHost = Environment::GetVariable(AzureAuthorityHostEnvVarName); + if (authorityHost.empty()) + { + authorityHost = _detail::ClientCredentialCore::AadGlobalAuthority; + } + + 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; @@ -137,6 +114,14 @@ AccessToken WorkloadIdentityCredential::GetToken( TokenRequestContext const& tokenRequestContext, Context const& context) const { + if (!m_tokenCredentialImpl) + { + auto const AuthUnavailable = GetCredentialName() + " authentication unavailable. "; + + throw AuthenticationException( + AuthUnavailable + "Environment variables are not fully configured."); + } + auto const tenantId = TenantIdResolver::Resolve( m_clientCredentialCore.GetTenantId(), tokenRequestContext, 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 ac53d22bd..96db871b5 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 @@ -9,6 +9,8 @@ #include +using Azure::Core::Credentials::AuthenticationException; +using Azure::Core::Credentials::TokenRequestContext; using Azure::Core::Http::HttpMethod; using Azure::Identity::WorkloadIdentityCredential; using Azure::Identity::WorkloadIdentityCredentialOptions; @@ -60,9 +62,14 @@ TEST(WorkloadIdentityCredential, GetOptionsFromEnvironmentInvalid) {"AZURE_AUTHORITY_HOST", ""}, {"AZURE_FEDERATED_TOKEN_FILE", ""}}); - EXPECT_THROW((void)WorkloadIdentityCredential{}, std::runtime_error); + TokenRequestContext trc; + trc.Scopes.push_back("https://storage.azure.com/.default"); + + WorkloadIdentityCredential const credDefault; + EXPECT_THROW(credDefault.GetToken(trc, {}), AuthenticationException); WorkloadIdentityCredentialOptions options; - EXPECT_THROW((void)WorkloadIdentityCredential{options}, std::runtime_error); + WorkloadIdentityCredential const cred(options); + EXPECT_THROW(cred.GetToken(trc, {}), AuthenticationException); } TEST(WorkloadIdentityCredential, Regular)