From b683ebfdbfa1c33b382be9acb4c7f5e148855f3d Mon Sep 17 00:00:00 2001 From: Ahson Khan Date: Wed, 19 Jun 2024 19:27:42 -0700 Subject: [PATCH 01/23] Add AzurePipelinesCredential for authenticating an Azure Pipelines service connection with workload identity federation. --- sdk/identity/azure-identity/CHANGELOG.md | 2 + sdk/identity/azure-identity/CMakeLists.txt | 2 + sdk/identity/azure-identity/README.md | 1 + .../azure-identity/inc/azure/identity.hpp | 1 + .../identity/azure_pipelines_credential.hpp | 111 ++++++++ .../detail/client_credential_core.hpp | 6 + .../src/azure_pipelines_credential.cpp | 253 ++++++++++++++++++ 7 files changed, 376 insertions(+) create mode 100644 sdk/identity/azure-identity/inc/azure/identity/azure_pipelines_credential.hpp create mode 100644 sdk/identity/azure-identity/src/azure_pipelines_credential.cpp diff --git a/sdk/identity/azure-identity/CHANGELOG.md b/sdk/identity/azure-identity/CHANGELOG.md index c89c8380c8..a919ee9d50 100644 --- a/sdk/identity/azure-identity/CHANGELOG.md +++ b/sdk/identity/azure-identity/CHANGELOG.md @@ -4,6 +4,8 @@ ### Features Added +- Added `AzurePipelinesCredential` for authenticating an Azure Pipelines service connection with workload identity federation. + ### Breaking Changes ### Bugs Fixed diff --git a/sdk/identity/azure-identity/CMakeLists.txt b/sdk/identity/azure-identity/CMakeLists.txt index 50df76415d..709f212bad 100644 --- a/sdk/identity/azure-identity/CMakeLists.txt +++ b/sdk/identity/azure-identity/CMakeLists.txt @@ -48,6 +48,7 @@ set( AZURE_IDENTITY_HEADER inc/azure/identity.hpp inc/azure/identity/azure_cli_credential.hpp + inc/azure/identity/azure_pipelines_credential.hpp inc/azure/identity/chained_token_credential.hpp inc/azure/identity/client_certificate_credential.hpp inc/azure/identity/client_secret_credential.hpp @@ -64,6 +65,7 @@ set( set( AZURE_IDENTITY_SOURCE src/azure_cli_credential.cpp + src/azure_pipelines_credential.cpp src/chained_token_credential.cpp src/client_certificate_credential.cpp src/client_credential_core.cpp diff --git a/sdk/identity/azure-identity/README.md b/sdk/identity/azure-identity/README.md index 6b925bf1f6..57de82c28a 100644 --- a/sdk/identity/azure-identity/README.md +++ b/sdk/identity/azure-identity/README.md @@ -134,6 +134,7 @@ Configuration is attempted in the above order. For example, if values for a clie ### Authenticate service principals |Credential | Usage |-|- +|`AzurePipelinesCredential`|Supports [Microsoft Entra Workload ID](https://learn.microsoft.com/azure/devops/pipelines/release/configure-workload-identity?view=azure-devops) on Azure Pipelines. |`ClientSecretCredential`|Authenticates a service principal [using a secret](https://learn.microsoft.com/entra/identity-platform/app-objects-and-service-principals). |`ClientCertificateCredential`|Authenticates a service principal [using a certificate](https://learn.microsoft.com/entra/identity-platform/app-objects-and-service-principals). diff --git a/sdk/identity/azure-identity/inc/azure/identity.hpp b/sdk/identity/azure-identity/inc/azure/identity.hpp index a258b213d6..d86546611a 100644 --- a/sdk/identity/azure-identity/inc/azure/identity.hpp +++ b/sdk/identity/azure-identity/inc/azure/identity.hpp @@ -9,6 +9,7 @@ #pragma once #include "azure/identity/azure_cli_credential.hpp" +#include "azure/identity/azure_pipelines_credential.hpp" #include "azure/identity/chained_token_credential.hpp" #include "azure/identity/client_certificate_credential.hpp" #include "azure/identity/client_secret_credential.hpp" diff --git a/sdk/identity/azure-identity/inc/azure/identity/azure_pipelines_credential.hpp b/sdk/identity/azure-identity/inc/azure/identity/azure_pipelines_credential.hpp new file mode 100644 index 0000000000..e18ca6d46d --- /dev/null +++ b/sdk/identity/azure-identity/inc/azure/identity/azure_pipelines_credential.hpp @@ -0,0 +1,111 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +/** + * @file + * @brief Azure Pipelines Credential and options. + */ + +#pragma once + +#include "azure/identity/detail/client_credential_core.hpp" +#include "azure/identity/detail/token_cache.hpp" + +#include +#include +#include + +#include +#include + +namespace Azure { namespace Identity { + namespace _detail { + class TokenCredentialImpl; + } // namespace _detail + + /** + * @brief Options for azure pipelines credential. + * + */ + struct AzurePipelinesCredentialOptions final : public Core::Credentials::TokenCredentialOptions + { + /** + * @brief Authentication authority URL. + * @note Defaults to the value of the environment variable 'AZURE_AUTHORITY_HOST'. If that's not + * set, the default value is Microsoft Entra global authority + * (https://login.microsoftonline.com/). + * + * @note Example of an authority host string: "https://login.microsoftonline.us/". See national + * clouds' Microsoft Entra authentication endpoints: + * https://learn.microsoft.com/entra/identity-platform/authentication-national-cloud. + */ + std::string AuthorityHost = _detail::DefaultOptionValues::GetAuthorityHost(); + + /** + * @brief For multi-tenant applications, specifies additional tenants for which the credential + * may acquire tokens. Add the wildcard value `"*"` to allow the credential to acquire tokens + * for any tenant in which the application is installed. + */ + std::vector AdditionallyAllowedTenants; + }; + + /** + * @brief Credential which authenticates using an Azure Pipelines service connection. + * + */ + class AzurePipelinesCredential final : public Core::Credentials::TokenCredential { + private: + _detail::TokenCache m_tokenCache; + _detail::ClientCredentialCore m_clientCredentialCore; + std::unique_ptr<_detail::TokenCredentialImpl> m_tokenCredentialImpl; + std::string m_requestBody; + std::string m_systemAccessToken; + std::string m_serviceConnectionId; + std::string m_oidcRequestUrl; + Azure::Core::Http::_internal::HttpPipeline m_httpPipeline; + + Azure::Core::Http::Request CreateOidcRequestMessage() const; + std::string GetOidcTokenResponse( + std::unique_ptr const& response, + std::string responseBody) const; + + public: + /** + * @brief Constructs an Azure Pipelines Credential. + * + * @param tenantId The tenant ID for the service connection. + * @param clientId The client ID for the service connection. + * @param serviceConnectionId The service connection Id for the service connection associated + * with the pipeline. + * @param systemAccessToken The pipeline's System.AccessToken value. See + * https://learn.microsoft.com/azure/devops/pipelines/build/variables?view=azure-devops%26tabs=yaml#systemaccesstoken + * for more details. + * @param options Options for token retrieval. + */ + explicit AzurePipelinesCredential( + std::string tenantId, + std::string clientId, + std::string serviceConnectionId, + std::string systemAccessToken, + AzurePipelinesCredentialOptions const& options = {}); + + /** + * @brief Destructs `%AzurePipelinesCredential`. + * + */ + ~AzurePipelinesCredential() override; + + /** + * @brief Gets an authentication token. + * + * @param tokenRequestContext A context to get the token in. + * @param context A context to control the request lifetime. + * + * @throw Azure::Core::Credentials::AuthenticationException Authentication error occurred. + */ + Core::Credentials::AccessToken GetToken( + Core::Credentials::TokenRequestContext const& tokenRequestContext, + Core::Context const& context) const override; + }; + +}} // namespace Azure::Identity diff --git a/sdk/identity/azure-identity/inc/azure/identity/detail/client_credential_core.hpp b/sdk/identity/azure-identity/inc/azure/identity/detail/client_credential_core.hpp index cef582422c..40fbf92bee 100644 --- a/sdk/identity/azure-identity/inc/azure/identity/detail/client_credential_core.hpp +++ b/sdk/identity/azure-identity/inc/azure/identity/detail/client_credential_core.hpp @@ -17,6 +17,7 @@ namespace Azure { namespace Identity { namespace _detail { constexpr auto AzureTenantIdEnvVarName = "AZURE_TENANT_ID"; constexpr auto AzureClientIdEnvVarName = "AZURE_CLIENT_ID"; constexpr auto AzureFederatedTokenFileEnvVarName = "AZURE_FEDERATED_TOKEN_FILE"; + constexpr auto OidcRequestUrlEnvVarName = "SYSTEM_OIDCREQUESTURI"; const std::string AadGlobalAuthority = "https://login.microsoftonline.com/"; class DefaultOptionValues final { @@ -46,6 +47,11 @@ namespace Azure { namespace Identity { namespace _detail { { return Core::_internal::Environment::GetVariable(AzureFederatedTokenFileEnvVarName); } + + static std::string GetOidcRequestUrl() + { + return Core::_internal::Environment::GetVariable(OidcRequestUrlEnvVarName); + } }; class ClientCredentialCore final { diff --git a/sdk/identity/azure-identity/src/azure_pipelines_credential.cpp b/sdk/identity/azure-identity/src/azure_pipelines_credential.cpp new file mode 100644 index 0000000000..8411319700 --- /dev/null +++ b/sdk/identity/azure-identity/src/azure_pipelines_credential.cpp @@ -0,0 +1,253 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +#include "azure/identity/azure_pipelines_credential.hpp" + +#include "private/identity_log.hpp" +#include "private/package_version.hpp" +#include "private/tenant_id_resolver.hpp" +#include "private/token_credential_impl.hpp" + +#include + +using Azure::Identity::AzurePipelinesCredential; +using Azure::Identity::AzurePipelinesCredentialOptions; + +using Azure::Core::Context; +using Azure::Core::Url; +using Azure::Core::Credentials::AccessToken; +using Azure::Core::Credentials::AuthenticationException; +using Azure::Core::Credentials::TokenRequestContext; +using Azure::Core::Http::HttpMethod; +using Azure::Core::Http::HttpStatusCode; +using Azure::Core::Http::RawResponse; +using Azure::Core::Http::Request; +using Azure::Core::Http::_internal::HttpPipeline; +using Azure::Core::Json::_internal::json; +using Azure::Identity::_detail::IdentityLog; +using Azure::Identity::_detail::PackageVersion; +using Azure::Identity::_detail::TenantIdResolver; +using Azure::Identity::_detail::TokenCredentialImpl; + +namespace { +bool IsValidTenantId(std::string const& tenantId) +{ + const std::string allowedChars = ".-"; + if (tenantId.empty()) + { + return false; + } + for (auto const c : tenantId) + { + if (allowedChars.find(c) != std::string::npos) + { + continue; + } + if (!StringExtensions::IsAlphaNumeric(c)) + { + return false; + } + } + return true; +} +} // namespace + +AzurePipelinesCredential::AzurePipelinesCredential( + std::string tenantId, + std::string clientId, + std::string serviceConnectionId, + std::string systemAccessToken, + AzurePipelinesCredentialOptions const& options) + : TokenCredential("AzurePipelinesCredential"), m_serviceConnectionId(serviceConnectionId), + m_systemAccessToken(systemAccessToken), + m_clientCredentialCore(tenantId, options.AuthorityHost, options.AdditionallyAllowedTenants), + m_httpPipeline(HttpPipeline(options, "identity", PackageVersion::ToString(), {}, {})) +{ + m_oidcRequestUrl = _detail::DefaultOptionValues::GetOidcRequestUrl(); + + bool isTenantIdValid = IsValidTenantId(tenantId); + if (!isTenantIdValid) + { + IdentityLog::Write( + IdentityLog::Level::Warning, + "Invalid tenant ID provided for " + GetCredentialName() + + ". The tenant ID must be a non-empty string containing only alphanumeric characters, " + "periods, or hyphens. You can locate your tenantID by following the instructions " + "listed here: https://learn.microsoft.com/partner-center/find-ids-and-domain-names"); + } + if (clientId.empty()) + { + IdentityLog::Write( + IdentityLog::Level::Warning, "No client ID specified for " + GetCredentialName() + "."); + } + if (serviceConnectionId.empty()) + { + IdentityLog::Write( + IdentityLog::Level::Warning, + "No service connection ID specified for " + GetCredentialName() + "."); + } + if (systemAccessToken.empty()) + { + IdentityLog::Write( + IdentityLog::Level::Warning, + "No system access token specified for " + GetCredentialName() + "."); + } + if (m_oidcRequestUrl.empty()) + { + IdentityLog::Write( + IdentityLog::Level::Warning, + "No value for environment variable '" + OidcRequestUrlEnvVarName + "' needed by " + + GetCredentialName() + ". This should be set by Azure Pipelines.".); + } + + if (isTenantIdValid && !clientId.empty() && !serviceConnectionId.empty() + && !systemAccessToken.empty() && !m_oidcRequestUrl.empty()) + { + 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); + + IdentityLog::Write( + IdentityLog::Level::Informational, GetCredentialName() + " was created successfully."); + } + else + { + IdentityLog::Write( + IdentityLog::Level::Warning, + "Azure Pipelines environment is not set up for the " + GetCredentialName() + + " credential to work."); + } +} + +Request AzurePipelinesCredential::CreateOidcRequestMessage() const +{ + const std::string oidcApiVersion = "7.1"; + + Url requestUrl = Url( + Url::Encode(m_oidcRequestUrl) + "?api-version=" + Url::Encode(oidcApiVersion) + + "&serviceConnectionId=" + Url::Encode(m_serviceConnectionId)); + Request request = Request(HttpMethod::Post, requestUrl); + request.SetHeader("content-type", "application/json"); + request.SetHeader("authorization", "Bearer " + m_systemAccessToken); + + return request; +} + +std::string AzurePipelinesCredential::GetOidcTokenResponse( + std::unique_ptr const& response, + std::string responseBody) const +{ + auto const statusCode = response->GetStatusCode(); + if (statusCode != HttpStatusCode::Ok) + { + // Include the response because its body, if any, probably contains an error message. + // OK responses aren't included with errors because they probably contain secrets. + + std::string message = GetCredentialName() + " : " + + std::to_string(static_cast::type>(statusCode)) + + " response from the OIDC endpoint. Check service connection ID and Pipeline " + "configuration." + + response->GetReasonPhrase() + "\n\n" + responseBody; + IdentityLog::Write(IdentityLog::Level::Verbose, message); + + throw AuthenticationException(message); + } + + json parsedJson; + try + { + parsedJson = Azure::Core::Json::_internal::json::parse(responseBody); + } + catch (json::exception const&) + { + std::string message + = GetCredentialName() + " : " + "Cannot parse the string '" + responseBody + "' as JSON."; + IdentityLog::Write(IdentityLog::Level::Verbose, message); + + throw AuthenticationException(message); + } + + const std::string oidcTokenPropertyName = "oidcToken"; + if (!parsedJson.contains(oidcTokenPropertyName) || !parsedJson[oidcTokenPropertyName].is_string()) + { + std::string message = GetCredentialName() + + " : OIDC token not found in response. \nSee Azure::Core::Diagnostics::Logger for details " + "(https://aka.ms/azsdk/cpp/identity/troubleshooting)."; + IdentityLog::Write(IdentityLog::Level::Verbose, message); + throw AuthenticationException(message); + } + return parsedJson[oidcTokenPropertyName].get(); +} + +AzurePipelinesCredential::~AzurePipelinesCredential() = default; + +AccessToken AzurePipelinesCredential::GetToken( + TokenRequestContext const& tokenRequestContext, + Context const& context) const +{ + if (!m_tokenCredentialImpl) + { + auto const AuthUnavailable = GetCredentialName() + " authentication unavailable. "; + + IdentityLog::Write( + IdentityLog::Level::Warning, + AuthUnavailable + "See earlier " + GetCredentialName() + " log messages for details."); + + throw AuthenticationException( + AuthUnavailable + "Azure Pipelines environment is not set up correctly."); + } + + auto const tenantId = TenantIdResolver::Resolve( + m_clientCredentialCore.GetTenantId(), + tokenRequestContext, + m_clientCredentialCore.GetAdditionallyAllowedTenants()); + + auto const scopesStr + = m_clientCredentialCore.GetScopesString(tenantId, tokenRequestContext.Scopes); + + Azure::Core::Http::Request oidcRequest = CreateOidcRequestMessage(); + std::unique_ptr response = m_httpPipeline.Send(oidcRequest, context); + + if (!response) + { + throw AuthenticationException( + GetCredentialName() + " couldn't send OIDC token request: null response."); + } + + auto const bodyStream = response->ExtractBodyStream(); + auto const bodyVec = bodyStream ? bodyStream->ReadToEnd(context) : response->GetBody(); + auto const responseBody + = std::string(reinterpret_cast(bodyVec.data()), bodyVec.size()); + + std::string assertion = GetOidcTokenResponse(response, responseBody); + + // TokenCache::GetToken() and m_tokenCredentialImpl->GetToken() can only use the lambda + // argument when they are being executed. They are not supposed to keep a reference to lambda + // argument to call it later. Therefore, any capture made here will outlive the possible time + // frame when the lambda might get called. + return m_tokenCache.GetToken(scopesStr, tenantId, tokenRequestContext.MinimumExpiration, [&]() { + return m_tokenCredentialImpl->GetToken(context, false, [&]() { + auto body = m_requestBody; + if (!scopesStr.empty()) + { + body += "&scope=" + scopesStr; + } + + auto const requestUrl = m_clientCredentialCore.GetRequestUrl(tenantId); + + body += "&client_assertion=" + Azure::Core::Url::Encode(assertion); + + auto request + = std::make_unique(HttpMethod::Post, requestUrl, body); + + request->HttpRequest.SetHeader("Host", requestUrl.GetHost()); + + return request; + }); + }); +} From d39a861efb42fde5227b85a75f7de5007cd7877b Mon Sep 17 00:00:00 2001 From: Ahson Khan Date: Thu, 20 Jun 2024 13:25:40 -0700 Subject: [PATCH 02/23] Add unit tests. --- .../detail/client_credential_core.hpp | 4 +- .../src/azure_pipelines_credential.cpp | 12 +- .../src/token_credential_impl.cpp | 11 +- .../azure-identity/test/ut/CMakeLists.txt | 1 + .../ut/azure_pipelines_credential_test.cpp | 491 ++++++++++++++++++ 5 files changed, 510 insertions(+), 9 deletions(-) create mode 100644 sdk/identity/azure-identity/test/ut/azure_pipelines_credential_test.cpp diff --git a/sdk/identity/azure-identity/inc/azure/identity/detail/client_credential_core.hpp b/sdk/identity/azure-identity/inc/azure/identity/detail/client_credential_core.hpp index 40fbf92bee..244cd86ab9 100644 --- a/sdk/identity/azure-identity/inc/azure/identity/detail/client_credential_core.hpp +++ b/sdk/identity/azure-identity/inc/azure/identity/detail/client_credential_core.hpp @@ -17,7 +17,7 @@ namespace Azure { namespace Identity { namespace _detail { constexpr auto AzureTenantIdEnvVarName = "AZURE_TENANT_ID"; constexpr auto AzureClientIdEnvVarName = "AZURE_CLIENT_ID"; constexpr auto AzureFederatedTokenFileEnvVarName = "AZURE_FEDERATED_TOKEN_FILE"; - constexpr auto OidcRequestUrlEnvVarName = "SYSTEM_OIDCREQUESTURI"; + const std::string OidcRequestUrlEnvVarName = "SYSTEM_OIDCREQUESTURI"; const std::string AadGlobalAuthority = "https://login.microsoftonline.com/"; class DefaultOptionValues final { @@ -50,7 +50,7 @@ namespace Azure { namespace Identity { namespace _detail { static std::string GetOidcRequestUrl() { - return Core::_internal::Environment::GetVariable(OidcRequestUrlEnvVarName); + return Core::_internal::Environment::GetVariable(OidcRequestUrlEnvVarName.c_str()); } }; diff --git a/sdk/identity/azure-identity/src/azure_pipelines_credential.cpp b/sdk/identity/azure-identity/src/azure_pipelines_credential.cpp index 8411319700..e33ebc35d8 100644 --- a/sdk/identity/azure-identity/src/azure_pipelines_credential.cpp +++ b/sdk/identity/azure-identity/src/azure_pipelines_credential.cpp @@ -15,6 +15,7 @@ using Azure::Identity::AzurePipelinesCredentialOptions; using Azure::Core::Context; using Azure::Core::Url; +using Azure::Core::_internal::StringExtensions; using Azure::Core::Credentials::AccessToken; using Azure::Core::Credentials::AuthenticationException; using Azure::Core::Credentials::TokenRequestContext; @@ -96,8 +97,8 @@ AzurePipelinesCredential::AzurePipelinesCredential( { IdentityLog::Write( IdentityLog::Level::Warning, - "No value for environment variable '" + OidcRequestUrlEnvVarName + "' needed by " - + GetCredentialName() + ". This should be set by Azure Pipelines.".); + "No value for environment variable '" + Azure::Identity::_detail::OidcRequestUrlEnvVarName + + "' needed by " + GetCredentialName() + ". This should be set by Azure Pipelines."); } if (isTenantIdValid && !clientId.empty() && !serviceConnectionId.empty() @@ -129,7 +130,7 @@ Request AzurePipelinesCredential::CreateOidcRequestMessage() const const std::string oidcApiVersion = "7.1"; Url requestUrl = Url( - Url::Encode(m_oidcRequestUrl) + "?api-version=" + Url::Encode(oidcApiVersion) + m_oidcRequestUrl + "?api-version=" + Url::Encode(oidcApiVersion) + "&serviceConnectionId=" + Url::Encode(m_serviceConnectionId)); Request request = Request(HttpMethod::Post, requestUrl); request.SetHeader("content-type", "application/json"); @@ -151,7 +152,7 @@ std::string AzurePipelinesCredential::GetOidcTokenResponse( std::string message = GetCredentialName() + " : " + std::to_string(static_cast::type>(statusCode)) + " response from the OIDC endpoint. Check service connection ID and Pipeline " - "configuration." + "configuration. " + response->GetReasonPhrase() + "\n\n" + responseBody; IdentityLog::Write(IdentityLog::Level::Verbose, message); @@ -165,8 +166,7 @@ std::string AzurePipelinesCredential::GetOidcTokenResponse( } catch (json::exception const&) { - std::string message - = GetCredentialName() + " : " + "Cannot parse the string '" + responseBody + "' as JSON."; + std::string message = GetCredentialName() + " : Cannot parse the response string as JSON."; IdentityLog::Write(IdentityLog::Level::Verbose, message); throw AuthenticationException(message); diff --git a/sdk/identity/azure-identity/src/token_credential_impl.cpp b/sdk/identity/azure-identity/src/token_credential_impl.cpp index 516b7e1c07..8be2cc9553 100644 --- a/sdk/identity/azure-identity/src/token_credential_impl.cpp +++ b/sdk/identity/azure-identity/src/token_credential_impl.cpp @@ -419,6 +419,7 @@ AccessToken TokenCredentialImpl::ParseToken( } auto const tzOffsetStr = TimeZoneOffsetAsString(utcDiffSeconds); + bool successfulParse = false; if (expiresOn.is_string()) { auto const expiresOnAsString = expiresOn.get(); @@ -448,13 +449,21 @@ AccessToken TokenCredentialImpl::ParseToken( try { accessToken.ExpiresOn = parse(expiresOnAsString); - return accessToken; + // Workaround for Warning C26800 - Use of a moved from object: 'accessToken' + // (lifetime.1) on MSVC version 14.40.33807+. + // Returning accessToken here directly causes the warning. + successfulParse = true; + break; } catch (std::exception const&) { // parse() has thrown, we may throw later. } } + if (successfulParse) + { + return accessToken; + } } } } diff --git a/sdk/identity/azure-identity/test/ut/CMakeLists.txt b/sdk/identity/azure-identity/test/ut/CMakeLists.txt index 2b8f17ca2b..171655754d 100644 --- a/sdk/identity/azure-identity/test/ut/CMakeLists.txt +++ b/sdk/identity/azure-identity/test/ut/CMakeLists.txt @@ -17,6 +17,7 @@ add_compile_definitions(AZURE_TEST_RECORDING_DIR="${CMAKE_CURRENT_LIST_DIR}") add_executable ( azure-identity-test azure_cli_credential_test.cpp + azure_pipelines_credential_test.cpp chained_token_credential_test.cpp client_certificate_credential_test.cpp client_secret_credential_test.cpp diff --git a/sdk/identity/azure-identity/test/ut/azure_pipelines_credential_test.cpp b/sdk/identity/azure-identity/test/ut/azure_pipelines_credential_test.cpp new file mode 100644 index 0000000000..66d9f6d5bb --- /dev/null +++ b/sdk/identity/azure-identity/test/ut/azure_pipelines_credential_test.cpp @@ -0,0 +1,491 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +#include "azure/identity/azure_pipelines_credential.hpp" +#include "credential_test_helper.hpp" + +#include +#include + +#include + +using Azure::Core::Credentials::AuthenticationException; +using Azure::Core::Credentials::TokenRequestContext; +using Azure::Core::Http::HttpMethod; +using Azure::Identity::AzurePipelinesCredential; +using Azure::Identity::AzurePipelinesCredentialOptions; +using Azure::Identity::Test::_detail::CredentialTestHelper; + +TEST(AzurePipelinesCredential, GetCredentialName) +{ + std::string tenantId = "01234567-89ab-cdef-fedc-ba8976543210"; + std::string clientId = "fedcba98-7654-3210-0123-456789abcdef"; + std::string serviceConnectionId = "abc"; + std::string systemAccessToken = "123"; + + AzurePipelinesCredential const cred(tenantId, clientId, serviceConnectionId, systemAccessToken); + + EXPECT_EQ(cred.GetCredentialName(), "AzurePipelinesCredential"); +} + +TEST(AzurePipelinesCredential, GetOptionsFromEnvironment) +{ + std::string tenantId = "01234567-89ab-cdef-fedc-ba8976543210"; + std::string clientId = "fedcba98-7654-3210-0123-456789abcdef"; + std::string serviceConnectionId = "abc"; + std::string systemAccessToken = "123"; + + { + CredentialTestHelper::EnvironmentOverride const env({{"AZURE_AUTHORITY_HOST", ""}}); + + AzurePipelinesCredentialOptions options; + AzurePipelinesCredential const cred( + tenantId, clientId, serviceConnectionId, systemAccessToken, options); + EXPECT_EQ(cred.GetCredentialName(), "AzurePipelinesCredential"); + + EXPECT_EQ(options.AuthorityHost, "https://login.microsoftonline.com/"); + } + + { + std::map envVars = {{"AZURE_AUTHORITY_HOST", "foo"}}; + CredentialTestHelper::EnvironmentOverride const env(envVars); + + AzurePipelinesCredentialOptions options; + options.AuthorityHost = "bar"; + EXPECT_EQ(options.AuthorityHost, "bar"); + } + + { + std::map envVars + = {{"AZURE_AUTHORITY_HOST", "https://microsoft.com/"}}; + CredentialTestHelper::EnvironmentOverride const env(envVars); + + AzurePipelinesCredentialOptions options; + EXPECT_EQ(options.AuthorityHost, "https://microsoft.com/"); + } +} + +TEST(AzurePipelinesCredential, InvalidArgs) +{ + std::string tenantId = "01234567-89ab-cdef-fedc-ba8976543210"; + std::string clientId = "fedcba98-7654-3210-0123-456789abcdef"; + std::string serviceConnectionId = "abc"; + std::string systemAccessToken = "123"; + + // Empty Oidc Request Uri + { + CredentialTestHelper::EnvironmentOverride const env({{"SYSTEM_OIDCREQUESTURI", ""}}); + + TokenRequestContext trc; + trc.Scopes.push_back("https://storage.azure.com/.default"); + + AzurePipelinesCredential const cred(tenantId, clientId, serviceConnectionId, systemAccessToken); + EXPECT_THROW(cred.GetToken(trc, {}), AuthenticationException); + AzurePipelinesCredentialOptions options; + AzurePipelinesCredential const credWithOptions( + tenantId, clientId, serviceConnectionId, systemAccessToken, options); + EXPECT_THROW(credWithOptions.GetToken(trc, {}), AuthenticationException); + } + + // Empty Tenant ID + { + CredentialTestHelper::EnvironmentOverride const env( + {{"SYSTEM_OIDCREQUESTURI", "https://localhost/instance"}}); + + TokenRequestContext trc; + trc.Scopes.push_back("https://storage.azure.com/.default"); + + AzurePipelinesCredential const cred("", clientId, serviceConnectionId, systemAccessToken); + EXPECT_THROW(cred.GetToken(trc, {}), AuthenticationException); + } + + // Invalid Tenant ID + { + CredentialTestHelper::EnvironmentOverride const env( + {{"SYSTEM_OIDCREQUESTURI", "https://localhost/instance"}}); + + TokenRequestContext trc; + trc.Scopes.push_back("https://storage.azure.com/.default"); + + AzurePipelinesCredential const cred( + "!=invalidTenantId=!", clientId, serviceConnectionId, systemAccessToken); + EXPECT_THROW(cred.GetToken(trc, {}), AuthenticationException); + } + + // Empty client ID + { + CredentialTestHelper::EnvironmentOverride const env( + {{"SYSTEM_OIDCREQUESTURI", "https://localhost/instance"}}); + + TokenRequestContext trc; + trc.Scopes.push_back("https://storage.azure.com/.default"); + + AzurePipelinesCredential const cred(tenantId, "", serviceConnectionId, systemAccessToken); + EXPECT_THROW(cred.GetToken(trc, {}), AuthenticationException); + } + + // Empty service connection ID + { + CredentialTestHelper::EnvironmentOverride const env( + {{"SYSTEM_OIDCREQUESTURI", "https://localhost/instance"}}); + + TokenRequestContext trc; + trc.Scopes.push_back("https://storage.azure.com/.default"); + + AzurePipelinesCredential const cred(tenantId, clientId, "", systemAccessToken); + EXPECT_THROW(cred.GetToken(trc, {}), AuthenticationException); + } + + // Empty system access token + { + CredentialTestHelper::EnvironmentOverride const env( + {{"SYSTEM_OIDCREQUESTURI", "https://localhost/instance"}}); + + TokenRequestContext trc; + trc.Scopes.push_back("https://storage.azure.com/.default"); + + AzurePipelinesCredential const cred(tenantId, clientId, serviceConnectionId, ""); + EXPECT_THROW(cred.GetToken(trc, {}), AuthenticationException); + } +} + +TEST(AzurePipelinesCredential, Regular) +{ + CredentialTestHelper::EnvironmentOverride const env( + {{"SYSTEM_OIDCREQUESTURI", "https://localhost/instance"}}); + + auto const actual = CredentialTestHelper::SimulateTokenRequest( + [](auto transport) { + AzurePipelinesCredentialOptions options; + options.Transport.Transport = transport; + + std::string tenantId = "01234567-89ab-cdef-fedc-ba8976543210"; + std::string clientId = "fedcba98-7654-3210-0123-456789abcdef"; + std::string serviceConnectionId = "a/bc"; + std::string systemAccessToken = "123"; + + return std::make_unique( + tenantId, clientId, serviceConnectionId, systemAccessToken, options); + }, + {{{"https://azure.com/.default"}}}, + std::vector{ + "{\"oidcToken\":\"abc/d\"}", "{\"expires_in\":3600, \"access_token\":\"ACCESSTOKEN1\"}"}); + + EXPECT_EQ(actual.Requests.size(), 2U); + EXPECT_EQ(actual.Responses.size(), 1U); + + auto const& request0 = actual.Requests.at(0); + auto const& request1 = actual.Requests.at(1); + + auto const& response0 = actual.Responses.at(0); + + EXPECT_EQ(request0.HttpMethod, HttpMethod::Post); + EXPECT_EQ(request1.HttpMethod, HttpMethod::Post); + + EXPECT_EQ( + request0.AbsoluteUrl, + "https://localhost/instance?api-version=7.1&serviceConnectionId=a%2Fbc"); + + EXPECT_EQ( + request1.AbsoluteUrl, + "https://login.microsoftonline.com/01234567-89ab-cdef-fedc-ba8976543210/oauth2/v2.0/token"); + + { + constexpr char expectedBodyStart1[] // cspell:disable + = "grant_type=client_credentials" + "&client_assertion_type=urn%3Aietf%3Aparams%3Aoauth%3Aclient-assertion-type%3Ajwt-" + "bearer" + "&client_id=fedcba98-7654-3210-0123-456789abcdef" + "&scope=https%3A%2F%2Fazure.com%2F.default" + "&client_assertion=abc%2Fd"; // cspell:enable + + EXPECT_EQ(request0.Body.size(), 0); + EXPECT_EQ(request1.Body.size(), (sizeof(expectedBodyStart1) - 1)); + + EXPECT_EQ(request1.Body, expectedBodyStart1); + + EXPECT_EQ(request0.Headers.find("Content-Length"), request0.Headers.end()); + + EXPECT_NE(request1.Headers.find("Content-Length"), request1.Headers.end()); + EXPECT_EQ( + std::stoi(request1.Headers.at("Content-Length")), + static_cast(sizeof(expectedBodyStart1) - 1)); + } + + EXPECT_NE(request0.Headers.find("Content-Type"), request0.Headers.end()); + EXPECT_EQ(request0.Headers.at("Content-Type"), "application/json"); + + EXPECT_NE(request1.Headers.find("Content-Type"), request1.Headers.end()); + EXPECT_EQ(request1.Headers.at("Content-Type"), "application/x-www-form-urlencoded"); + + EXPECT_EQ(response0.AccessToken.Token, "ACCESSTOKEN1"); + + using namespace std::chrono_literals; + EXPECT_GE(response0.AccessToken.ExpiresOn, response0.EarliestExpiration + 3600s); + EXPECT_LE(response0.AccessToken.ExpiresOn, response0.LatestExpiration + 3600s); +} + +TEST(AzurePipelinesCredential, AzureStack) +{ + CredentialTestHelper::EnvironmentOverride const env( + {{"SYSTEM_OIDCREQUESTURI", "https://localhost/instance"}}); + + auto const actual = CredentialTestHelper::SimulateTokenRequest( + [](auto transport) { + AzurePipelinesCredentialOptions options; + options.Transport.Transport = transport; + + std::string tenantId = "adfs"; + std::string clientId = "fedcba98-7654-3210-0123-456789abcdef"; + std::string serviceConnectionId = "a/bc"; + std::string systemAccessToken = "123"; + + return std::make_unique( + tenantId, clientId, serviceConnectionId, systemAccessToken, options); + }, + {{{"https://azure.com/.default"}}}, + std::vector{ + "{\"oidcToken\":\"abc/d\"}", "{\"expires_in\":3600, \"access_token\":\"ACCESSTOKEN1\"}"}); + + EXPECT_EQ(actual.Requests.size(), 2U); + EXPECT_EQ(actual.Responses.size(), 1U); + + auto const& request0 = actual.Requests.at(0); + auto const& request1 = actual.Requests.at(1); + + auto const& response0 = actual.Responses.at(0); + + EXPECT_EQ(request0.HttpMethod, HttpMethod::Post); + EXPECT_EQ(request1.HttpMethod, HttpMethod::Post); + + EXPECT_EQ( + request0.AbsoluteUrl, + "https://localhost/instance?api-version=7.1&serviceConnectionId=a%2Fbc"); + + EXPECT_EQ(request1.AbsoluteUrl, "https://login.microsoftonline.com/adfs/oauth2/token"); + + { + constexpr char expectedBodyStart1[] // cspell:disable + = "grant_type=client_credentials" + "&client_assertion_type=urn%3Aietf%3Aparams%3Aoauth%3Aclient-assertion-type%3Ajwt-" + "bearer" + "&client_id=fedcba98-7654-3210-0123-456789abcdef" + "&scope=https%3A%2F%2Fazure.com" + "&client_assertion=abc%2Fd"; // cspell:enable + + EXPECT_EQ(request0.Body.size(), 0); + EXPECT_EQ(request1.Body.size(), (sizeof(expectedBodyStart1) - 1)); + + EXPECT_EQ(request1.Body, expectedBodyStart1); + + EXPECT_EQ(request0.Headers.find("Content-Length"), request0.Headers.end()); + + EXPECT_NE(request1.Headers.find("Content-Length"), request1.Headers.end()); + EXPECT_EQ( + std::stoi(request1.Headers.at("Content-Length")), + static_cast(sizeof(expectedBodyStart1) - 1)); + } + + EXPECT_NE(request0.Headers.find("Content-Type"), request0.Headers.end()); + EXPECT_EQ(request0.Headers.at("Content-Type"), "application/json"); + + EXPECT_NE(request1.Headers.find("Content-Type"), request1.Headers.end()); + EXPECT_EQ(request1.Headers.at("Content-Type"), "application/x-www-form-urlencoded"); + + EXPECT_EQ(response0.AccessToken.Token, "ACCESSTOKEN1"); + + using namespace std::chrono_literals; + EXPECT_GE(response0.AccessToken.ExpiresOn, response0.EarliestExpiration + 3600s); + EXPECT_LE(response0.AccessToken.ExpiresOn, response0.LatestExpiration + 3600s); +} + +TEST(AzurePipelinesCredential, Authority) +{ + CredentialTestHelper::EnvironmentOverride const env( + {{"SYSTEM_OIDCREQUESTURI", "https://localhost/instance"}, + {"AZURE_AUTHORITY_HOST", "https://microsoft.com/"}}); + + auto const actual = CredentialTestHelper::SimulateTokenRequest( + [](auto transport) { + AzurePipelinesCredentialOptions options; + options.Transport.Transport = transport; + + std::string tenantId = "01234567-89ab-cdef-fedc-ba8976543210"; + std::string clientId = "fedcba98-7654-3210-0123-456789abcdef"; + std::string serviceConnectionId = "a/bc"; + std::string systemAccessToken = "123"; + + return std::make_unique( + tenantId, clientId, serviceConnectionId, systemAccessToken, options); + }, + {{{"https://azure.com/.default"}}}, + std::vector{ + "{\"oidcToken\":\"abc/d\"}", "{\"expires_in\":3600, \"access_token\":\"ACCESSTOKEN1\"}"}); + + EXPECT_EQ(actual.Requests.size(), 2U); + EXPECT_EQ(actual.Responses.size(), 1U); + + auto const& request0 = actual.Requests.at(0); + auto const& request1 = actual.Requests.at(1); + + auto const& response0 = actual.Responses.at(0); + + EXPECT_EQ(request0.HttpMethod, HttpMethod::Post); + EXPECT_EQ(request1.HttpMethod, HttpMethod::Post); + + EXPECT_EQ( + request0.AbsoluteUrl, + "https://localhost/instance?api-version=7.1&serviceConnectionId=a%2Fbc"); + + EXPECT_EQ( + request1.AbsoluteUrl, + "https://microsoft.com/01234567-89ab-cdef-fedc-ba8976543210/oauth2/v2.0/token"); + + { + constexpr char expectedBodyStart1[] // cspell:disable + = "grant_type=client_credentials" + "&client_assertion_type=urn%3Aietf%3Aparams%3Aoauth%3Aclient-assertion-type%3Ajwt-" + "bearer" + "&client_id=fedcba98-7654-3210-0123-456789abcdef" + "&scope=https%3A%2F%2Fazure.com%2F.default" + "&client_assertion=abc%2Fd"; // cspell:enable + + EXPECT_EQ(request0.Body.size(), 0); + EXPECT_EQ(request1.Body.size(), (sizeof(expectedBodyStart1) - 1)); + + EXPECT_EQ(request1.Body, expectedBodyStart1); + + EXPECT_EQ(request0.Headers.find("Content-Length"), request0.Headers.end()); + + EXPECT_NE(request1.Headers.find("Content-Length"), request1.Headers.end()); + EXPECT_EQ( + std::stoi(request1.Headers.at("Content-Length")), + static_cast(sizeof(expectedBodyStart1) - 1)); + } + + EXPECT_NE(request0.Headers.find("Content-Type"), request0.Headers.end()); + EXPECT_EQ(request0.Headers.at("Content-Type"), "application/json"); + + EXPECT_NE(request1.Headers.find("Content-Type"), request1.Headers.end()); + EXPECT_EQ(request1.Headers.at("Content-Type"), "application/x-www-form-urlencoded"); + + EXPECT_EQ(response0.AccessToken.Token, "ACCESSTOKEN1"); + + using namespace std::chrono_literals; + EXPECT_GE(response0.AccessToken.ExpiresOn, response0.EarliestExpiration + 3600s); + EXPECT_LE(response0.AccessToken.ExpiresOn, response0.LatestExpiration + 3600s); +} + +TEST(AzurePipelinesCredential, HttpSchemeNotSupported) +{ + CredentialTestHelper::EnvironmentOverride const env( + {{"SYSTEM_OIDCREQUESTURI", "https://localhost/instance"}, + {"AZURE_AUTHORITY_HOST", "http://microsoft.com/"}}); + + try + { + auto const actual = CredentialTestHelper::SimulateTokenRequest( + [](auto transport) { + AzurePipelinesCredentialOptions options; + options.Transport.Transport = transport; + + std::string tenantId = "01234567-89ab-cdef-fedc-ba8976543210"; + std::string clientId = "fedcba98-7654-3210-0123-456789abcdef"; + std::string serviceConnectionId = "a/bc"; + std::string systemAccessToken = "123"; + + return std::make_unique( + tenantId, clientId, serviceConnectionId, systemAccessToken, options); + }, + {{{"https://azure.com/.default"}}}, + std::vector{ + "{\"oidcToken\":\"abc\"}", "{\"expires_in\":3600, \"access_token\":\"ACCESSTOKEN1\"}"}); + } + catch (AuthenticationException const& e) + { + EXPECT_TRUE(std::string(e.what()).find("https") != std::string::npos) << e.what(); + } +} + +TEST(AzurePipelinesCredential, InvalidOidcResponse) +{ + CredentialTestHelper::EnvironmentOverride const env( + {{"SYSTEM_OIDCREQUESTURI", "https://localhost/instance"}}); + + std::string tenantId = "01234567-89ab-cdef-fedc-ba8976543210"; + std::string clientId = "fedcba98-7654-3210-0123-456789abcdef"; + std::string serviceConnectionId = "a/bc"; + std::string systemAccessToken = "123"; + + // Non-OK response + try + { + using Azure::Core::Http::HttpStatusCode; + std::vector const testScopes; + CredentialTestHelper::TokenRequestSimulationServerResponse testResponse; + testResponse.StatusCode = HttpStatusCode::BadRequest; + testResponse.Body = "Invalid response body"; + + static_cast(CredentialTestHelper::SimulateTokenRequest( + [&](auto transport) { + AzurePipelinesCredentialOptions options; + options.Transport.Transport = transport; + + return std::make_unique( + tenantId, clientId, serviceConnectionId, systemAccessToken, options); + }, + {testScopes}, + {testResponse})); + + EXPECT_TRUE(!"AzurePipelinesCredential should throw given the response above."); + } + catch (AuthenticationException const& ex) + { + std::string expectedMessage + = "AzurePipelinesCredential : 400 response from the OIDC endpoint. Check service " + "connection ID and Pipeline configuration. Test\n\nInvalid response body"; + EXPECT_EQ(ex.what(), expectedMessage) << ex.what(); + } + + // Invalid JSON + EXPECT_THROW( + CredentialTestHelper::SimulateTokenRequest( + [&](auto transport) { + AzurePipelinesCredentialOptions options; + options.Transport.Transport = transport; + + return std::make_unique( + tenantId, clientId, serviceConnectionId, systemAccessToken, options); + }, + {{{"https://azure.com/.default"}}}, + std::vector{"{\"oidc\":\"abc\"]", ""}), + AuthenticationException); + + // Missing token + EXPECT_THROW( + CredentialTestHelper::SimulateTokenRequest( + [&](auto transport) { + AzurePipelinesCredentialOptions options; + options.Transport.Transport = transport; + + return std::make_unique( + tenantId, clientId, serviceConnectionId, systemAccessToken, options); + }, + {{{"https://azure.com/.default"}}}, + std::vector{"{\"oidc\":\"abc\"}", ""}), + AuthenticationException); + + // Incorrect token type + EXPECT_THROW( + CredentialTestHelper::SimulateTokenRequest( + [&](auto transport) { + AzurePipelinesCredentialOptions options; + options.Transport.Transport = transport; + + return std::make_unique( + tenantId, clientId, serviceConnectionId, systemAccessToken, options); + }, + {{{"https://azure.com/.default"}}}, + std::vector{"{\"oidcToken\":5}", ""}), + AuthenticationException); +} From 19581b3b491b8ddd3d7fb2a53c7d971e4e486390 Mon Sep 17 00:00:00 2001 From: Ahson Khan Date: Thu, 20 Jun 2024 13:31:15 -0700 Subject: [PATCH 03/23] Add comment about not throwing in the ctor, but rather deferring it. --- .../azure-identity/src/azure_pipelines_credential.cpp | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/sdk/identity/azure-identity/src/azure_pipelines_credential.cpp b/sdk/identity/azure-identity/src/azure_pipelines_credential.cpp index e33ebc35d8..4610fa11d3 100644 --- a/sdk/identity/azure-identity/src/azure_pipelines_credential.cpp +++ b/sdk/identity/azure-identity/src/azure_pipelines_credential.cpp @@ -118,6 +118,10 @@ AzurePipelinesCredential::AzurePipelinesCredential( } else { + // Rather than throwing an exception in the ctor, following the pattern in existing credentials + // to log the errors, and defer throwing an exception to the first call of GetToken(). This is + // primarily needed for credentials that are part of the DefaultAzureCredential, which this + // credential is not intended for. IdentityLog::Write( IdentityLog::Level::Warning, "Azure Pipelines environment is not set up for the " + GetCredentialName() From 010e46eeeab5bc57a2a8da615a3fa39b64eea088 Mon Sep 17 00:00:00 2001 From: Ahson Khan Date: Thu, 20 Jun 2024 13:57:13 -0700 Subject: [PATCH 04/23] Order field in order of initialization and fix cspell. --- .vscode/cspell.json | 3 +++ .../inc/azure/identity/azure_pipelines_credential.hpp | 10 +++++----- 2 files changed, 8 insertions(+), 5 deletions(-) diff --git a/.vscode/cspell.json b/.vscode/cspell.json index 4a88b7b534..633cad9cbf 100644 --- a/.vscode/cspell.json +++ b/.vscode/cspell.json @@ -188,7 +188,10 @@ "Oaep", "odata", "ofstad", + "oidc", + "Oidc", "OIDC", + "OIDCREQUESTURI", "okhttp", "opentelemetry", "Osterman", diff --git a/sdk/identity/azure-identity/inc/azure/identity/azure_pipelines_credential.hpp b/sdk/identity/azure-identity/inc/azure/identity/azure_pipelines_credential.hpp index e18ca6d46d..ce3c944404 100644 --- a/sdk/identity/azure-identity/inc/azure/identity/azure_pipelines_credential.hpp +++ b/sdk/identity/azure-identity/inc/azure/identity/azure_pipelines_credential.hpp @@ -55,14 +55,14 @@ namespace Azure { namespace Identity { */ class AzurePipelinesCredential final : public Core::Credentials::TokenCredential { private: - _detail::TokenCache m_tokenCache; + std::string m_serviceConnectionId; + std::string m_systemAccessToken; _detail::ClientCredentialCore m_clientCredentialCore; + Azure::Core::Http::_internal::HttpPipeline m_httpPipeline; + std::string m_oidcRequestUrl; std::unique_ptr<_detail::TokenCredentialImpl> m_tokenCredentialImpl; std::string m_requestBody; - std::string m_systemAccessToken; - std::string m_serviceConnectionId; - std::string m_oidcRequestUrl; - Azure::Core::Http::_internal::HttpPipeline m_httpPipeline; + _detail::TokenCache m_tokenCache; Azure::Core::Http::Request CreateOidcRequestMessage() const; std::string GetOidcTokenResponse( From deb75494839fb7dce41455297767aacc3ec28d6b Mon Sep 17 00:00:00 2001 From: Ahson Khan Date: Thu, 20 Jun 2024 14:05:59 -0700 Subject: [PATCH 05/23] Fix ambiguous call to EnvironmentOverride in tests. --- .../ut/azure_pipelines_credential_test.cpp | 39 ++++++++++--------- 1 file changed, 21 insertions(+), 18 deletions(-) diff --git a/sdk/identity/azure-identity/test/ut/azure_pipelines_credential_test.cpp b/sdk/identity/azure-identity/test/ut/azure_pipelines_credential_test.cpp index 66d9f6d5bb..3a693d7f9c 100644 --- a/sdk/identity/azure-identity/test/ut/azure_pipelines_credential_test.cpp +++ b/sdk/identity/azure-identity/test/ut/azure_pipelines_credential_test.cpp @@ -36,7 +36,8 @@ TEST(AzurePipelinesCredential, GetOptionsFromEnvironment) std::string systemAccessToken = "123"; { - CredentialTestHelper::EnvironmentOverride const env({{"AZURE_AUTHORITY_HOST", ""}}); + std::map envVars = {{"AZURE_AUTHORITY_HOST", ""}}; + CredentialTestHelper::EnvironmentOverride const env(envVars); AzurePipelinesCredentialOptions options; AzurePipelinesCredential const cred( @@ -72,9 +73,13 @@ TEST(AzurePipelinesCredential, InvalidArgs) std::string serviceConnectionId = "abc"; std::string systemAccessToken = "123"; + std::map validEnvVars + = {{"SYSTEM_OIDCREQUESTURI", "https://localhost/instance"}}; + // Empty Oidc Request Uri { - CredentialTestHelper::EnvironmentOverride const env({{"SYSTEM_OIDCREQUESTURI", ""}}); + std::map invalidEnvVars = {{"SYSTEM_OIDCREQUESTURI", ""}}; + CredentialTestHelper::EnvironmentOverride const env(invalidEnvVars); TokenRequestContext trc; trc.Scopes.push_back("https://storage.azure.com/.default"); @@ -89,8 +94,7 @@ TEST(AzurePipelinesCredential, InvalidArgs) // Empty Tenant ID { - CredentialTestHelper::EnvironmentOverride const env( - {{"SYSTEM_OIDCREQUESTURI", "https://localhost/instance"}}); + CredentialTestHelper::EnvironmentOverride const env(validEnvVars); TokenRequestContext trc; trc.Scopes.push_back("https://storage.azure.com/.default"); @@ -101,8 +105,7 @@ TEST(AzurePipelinesCredential, InvalidArgs) // Invalid Tenant ID { - CredentialTestHelper::EnvironmentOverride const env( - {{"SYSTEM_OIDCREQUESTURI", "https://localhost/instance"}}); + CredentialTestHelper::EnvironmentOverride const env(validEnvVars); TokenRequestContext trc; trc.Scopes.push_back("https://storage.azure.com/.default"); @@ -114,8 +117,7 @@ TEST(AzurePipelinesCredential, InvalidArgs) // Empty client ID { - CredentialTestHelper::EnvironmentOverride const env( - {{"SYSTEM_OIDCREQUESTURI", "https://localhost/instance"}}); + CredentialTestHelper::EnvironmentOverride const env(validEnvVars); TokenRequestContext trc; trc.Scopes.push_back("https://storage.azure.com/.default"); @@ -126,8 +128,7 @@ TEST(AzurePipelinesCredential, InvalidArgs) // Empty service connection ID { - CredentialTestHelper::EnvironmentOverride const env( - {{"SYSTEM_OIDCREQUESTURI", "https://localhost/instance"}}); + CredentialTestHelper::EnvironmentOverride const env(validEnvVars); TokenRequestContext trc; trc.Scopes.push_back("https://storage.azure.com/.default"); @@ -138,8 +139,7 @@ TEST(AzurePipelinesCredential, InvalidArgs) // Empty system access token { - CredentialTestHelper::EnvironmentOverride const env( - {{"SYSTEM_OIDCREQUESTURI", "https://localhost/instance"}}); + CredentialTestHelper::EnvironmentOverride const env(validEnvVars); TokenRequestContext trc; trc.Scopes.push_back("https://storage.azure.com/.default"); @@ -151,8 +151,9 @@ TEST(AzurePipelinesCredential, InvalidArgs) TEST(AzurePipelinesCredential, Regular) { - CredentialTestHelper::EnvironmentOverride const env( - {{"SYSTEM_OIDCREQUESTURI", "https://localhost/instance"}}); + std::map validEnvVars + = {{"SYSTEM_OIDCREQUESTURI", "https://localhost/instance"}}; + CredentialTestHelper::EnvironmentOverride const env(validEnvVars); auto const actual = CredentialTestHelper::SimulateTokenRequest( [](auto transport) { @@ -227,8 +228,9 @@ TEST(AzurePipelinesCredential, Regular) TEST(AzurePipelinesCredential, AzureStack) { - CredentialTestHelper::EnvironmentOverride const env( - {{"SYSTEM_OIDCREQUESTURI", "https://localhost/instance"}}); + std::map validEnvVars + = {{"SYSTEM_OIDCREQUESTURI", "https://localhost/instance"}}; + CredentialTestHelper::EnvironmentOverride const env(validEnvVars); auto const actual = CredentialTestHelper::SimulateTokenRequest( [](auto transport) { @@ -409,8 +411,9 @@ TEST(AzurePipelinesCredential, HttpSchemeNotSupported) TEST(AzurePipelinesCredential, InvalidOidcResponse) { - CredentialTestHelper::EnvironmentOverride const env( - {{"SYSTEM_OIDCREQUESTURI", "https://localhost/instance"}}); + std::map validEnvVars + = {{"SYSTEM_OIDCREQUESTURI", "https://localhost/instance"}}; + CredentialTestHelper::EnvironmentOverride const env(validEnvVars); std::string tenantId = "01234567-89ab-cdef-fedc-ba8976543210"; std::string clientId = "fedcba98-7654-3210-0123-456789abcdef"; From 0c1579bc55fbbad183b9f0952cee77491ae3dffc Mon Sep 17 00:00:00 2001 From: Ahson Khan Date: Thu, 20 Jun 2024 14:28:31 -0700 Subject: [PATCH 06/23] Add a live test to AzurePipelinesCredential. --- .../ut/azure_pipelines_credential_test.cpp | 33 +++++++++++++++++++ 1 file changed, 33 insertions(+) diff --git a/sdk/identity/azure-identity/test/ut/azure_pipelines_credential_test.cpp b/sdk/identity/azure-identity/test/ut/azure_pipelines_credential_test.cpp index 3a693d7f9c..5d447fda4d 100644 --- a/sdk/identity/azure-identity/test/ut/azure_pipelines_credential_test.cpp +++ b/sdk/identity/azure-identity/test/ut/azure_pipelines_credential_test.cpp @@ -9,6 +9,8 @@ #include +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; @@ -492,3 +494,34 @@ TEST(AzurePipelinesCredential, InvalidOidcResponse) std::vector{"{\"oidcToken\":5}", ""}), AuthenticationException); } + +TEST(AzurePipelinesCredential, RegularLive_LIVEONLY_) +{ + std::string clientId = Environment::GetVariable("AZURE_SERVICE_CONNECTION_CLIENT_ID"); + std::string serviceConnectionId = Environment::GetVariable("AZURE_SERVICE_CONNECTION_CLIENT_ID"); + std::string systemAccessToken = Environment::GetVariable("SYSTEM_ACCESSTOKEN"); + std::string tenantId = Environment::GetVariable("AZURE_SERVICE_CONNECTION_TENANT_ID"); + + if (clientId.empty() || serviceConnectionId.empty() || systemAccessToken.empty() + || tenantId.empty()) + { + GTEST_SKIP_("Set AZURE_SERVICE_CONNECTION_CLIENT_ID, AZURE_SERVICE_CONNECTION_ID, " + "AZURE_SERVICE_CONNECTION_TENANT_ID, and SYSTEM_ACCESSTOKEN to run this " + "AzurePipelinesCredential test."); + } + + AzurePipelinesCredential const cred(tenantId, clientId, serviceConnectionId, systemAccessToken); + + TokenRequestContext trc; + trc.Scopes.push_back("https://vault.azure.net/.default"); + + AccessToken token = cred.GetToken(trc, {}); + EXPECT_NE(token.Token, "") << "GetToken returned an invalid token."; + + EXPECT_TRUE(token.ExpiresOn >= std::chrono::system_clock::now()) + << "GetToken returned an invalid expiration time."; + + AccessToken token2 = cred.GetToken(trc, {}); + EXPECT_TRUE(token.Token == token2.Token && token.ExpiresOn == token2.ExpiresOn) + << "Expected a cached token."; +} From 04462a5ff5c603f9dfcbf548c042e7ef438f1908 Mon Sep 17 00:00:00 2001 From: Ahson Khan Date: Thu, 20 Jun 2024 15:33:23 -0700 Subject: [PATCH 07/23] Add invalid test cases and output response. --- .../src/azure_pipelines_credential.cpp | 26 ++-- .../ut/azure_pipelines_credential_test.cpp | 111 +++++++++++++++++- 2 files changed, 122 insertions(+), 15 deletions(-) diff --git a/sdk/identity/azure-identity/src/azure_pipelines_credential.cpp b/sdk/identity/azure-identity/src/azure_pipelines_credential.cpp index 4610fa11d3..28c2077f24 100644 --- a/sdk/identity/azure-identity/src/azure_pipelines_credential.cpp +++ b/sdk/identity/azure-identity/src/azure_pipelines_credential.cpp @@ -101,18 +101,18 @@ AzurePipelinesCredential::AzurePipelinesCredential( + "' needed by " + GetCredentialName() + ". This should be set by Azure Pipelines."); } + 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 (isTenantIdValid && !clientId.empty() && !serviceConnectionId.empty() && !systemAccessToken.empty() && !m_oidcRequestUrl.empty()) { - 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); - IdentityLog::Write( IdentityLog::Level::Informational, GetCredentialName() + " was created successfully."); } @@ -228,6 +228,14 @@ AccessToken AzurePipelinesCredential::GetToken( auto const responseBody = std::string(reinterpret_cast(bodyVec.data()), bodyVec.size()); + std::cout << "OIDC_RESPONSE_STATUSCODE: " << static_cast(response->GetStatusCode()) + << std::endl; + std::cout << "OIDC_RESPONSE_ReasonPhrase: " << response->GetReasonPhrase() << std::endl; + for (const auto& header : response->GetHeaders()) + { + std::cout << "OIDC_RESPONSE_HEADER: " << header.first << ": " << header.second << std::endl; + } + std::cout << "OIDC_RESPONSE: " << responseBody << std::endl; std::string assertion = GetOidcTokenResponse(response, responseBody); // TokenCache::GetToken() and m_tokenCredentialImpl->GetToken() can only use the lambda diff --git a/sdk/identity/azure-identity/test/ut/azure_pipelines_credential_test.cpp b/sdk/identity/azure-identity/test/ut/azure_pipelines_credential_test.cpp index 5d447fda4d..8f3f853c42 100644 --- a/sdk/identity/azure-identity/test/ut/azure_pipelines_credential_test.cpp +++ b/sdk/identity/azure-identity/test/ut/azure_pipelines_credential_test.cpp @@ -497,17 +497,20 @@ TEST(AzurePipelinesCredential, InvalidOidcResponse) TEST(AzurePipelinesCredential, RegularLive_LIVEONLY_) { + std::string tenantId = Environment::GetVariable("AZURE_SERVICE_CONNECTION_TENANT_ID"); std::string clientId = Environment::GetVariable("AZURE_SERVICE_CONNECTION_CLIENT_ID"); std::string serviceConnectionId = Environment::GetVariable("AZURE_SERVICE_CONNECTION_CLIENT_ID"); std::string systemAccessToken = Environment::GetVariable("SYSTEM_ACCESSTOKEN"); - std::string tenantId = Environment::GetVariable("AZURE_SERVICE_CONNECTION_TENANT_ID"); - if (clientId.empty() || serviceConnectionId.empty() || systemAccessToken.empty() - || tenantId.empty()) + if (tenantId.empty() || clientId.empty() || serviceConnectionId.empty() + || systemAccessToken.empty()) { - GTEST_SKIP_("Set AZURE_SERVICE_CONNECTION_CLIENT_ID, AZURE_SERVICE_CONNECTION_ID, " - "AZURE_SERVICE_CONNECTION_TENANT_ID, and SYSTEM_ACCESSTOKEN to run this " - "AzurePipelinesCredential test."); + std::string message = "Set AZURE_SERVICE_CONNECTION_CLIENT_ID, AZURE_SERVICE_CONNECTION_ID, " + "AZURE_SERVICE_CONNECTION_TENANT_ID, and SYSTEM_ACCESSTOKEN to run this " + "AzurePipelinesCredential test. " + + tenantId + " : " + clientId + " : " + serviceConnectionId + " : " + systemAccessToken + + "."; + GTEST_SKIP_(message.c_str()); } AzurePipelinesCredential const cred(tenantId, clientId, serviceConnectionId, systemAccessToken); @@ -525,3 +528,99 @@ TEST(AzurePipelinesCredential, RegularLive_LIVEONLY_) EXPECT_TRUE(token.Token == token2.Token && token.ExpiresOn == token2.ExpiresOn) << "Expected a cached token."; } + +TEST(AzurePipelinesCredential, InvalidServiceConnectionId_LIVEONLY_) +{ + std::string tenantId = Environment::GetVariable("AZURE_SERVICE_CONNECTION_TENANT_ID"); + std::string clientId = Environment::GetVariable("AZURE_SERVICE_CONNECTION_CLIENT_ID"); + std::string systemAccessToken = Environment::GetVariable("SYSTEM_ACCESSTOKEN"); + + std::string serviceConnectionId = "invalidServiceConnectionId"; + + /*if (tenantId.empty() || clientId.empty() || systemAccessToken.empty()) + { + std::string message = "Set AZURE_SERVICE_CONNECTION_CLIENT_ID, AZURE_SERVICE_CONNECTION_ID, " + "AZURE_SERVICE_CONNECTION_TENANT_ID, and SYSTEM_ACCESSTOKEN to run this " + "AzurePipelinesCredential test."; + GTEST_SKIP_(message.c_str()); + }*/ + + AzurePipelinesCredential const cred(tenantId, clientId, serviceConnectionId, systemAccessToken); + + TokenRequestContext trc; + trc.Scopes.push_back("https://vault.azure.net/.default"); + + try + { + AccessToken token = cred.GetToken(trc, {}); + } + catch (AuthenticationException const& ex) + { + std::string expectedMessage = ""; + EXPECT_EQ(ex.what(), expectedMessage) << ex.what(); + } +} + +TEST(AzurePipelinesCredential, InvalidClientId_LIVEONLY_) +{ + std::string tenantId = Environment::GetVariable("AZURE_SERVICE_CONNECTION_TENANT_ID"); + std::string serviceConnectionId = Environment::GetVariable("AZURE_SERVICE_CONNECTION_CLIENT_ID"); + std::string systemAccessToken = Environment::GetVariable("SYSTEM_ACCESSTOKEN"); + + std::string clientId = "invalidClientId"; + + /*if (tenantId.empty() || serviceConnectionId.empty() || systemAccessToken.empty()) + { + std::string message = "Set AZURE_SERVICE_CONNECTION_CLIENT_ID, AZURE_SERVICE_CONNECTION_ID, " + "AZURE_SERVICE_CONNECTION_TENANT_ID, and SYSTEM_ACCESSTOKEN to run this " + "AzurePipelinesCredential test."; + GTEST_SKIP_(message.c_str()); + }*/ + + AzurePipelinesCredential const cred(tenantId, clientId, serviceConnectionId, systemAccessToken); + + TokenRequestContext trc; + trc.Scopes.push_back("https://vault.azure.net/.default"); + + try + { + AccessToken token = cred.GetToken(trc, {}); + } + catch (AuthenticationException const& ex) + { + std::string expectedMessage = ""; + EXPECT_EQ(ex.what(), expectedMessage) << ex.what(); + } +} + +TEST(AzurePipelinesCredential, InvalidSystemAccessToken_LIVEONLY_) +{ + std::string tenantId = Environment::GetVariable("AZURE_SERVICE_CONNECTION_TENANT_ID"); + std::string clientId = Environment::GetVariable("SYSTEM_ACCESSTOKEN"); + std::string serviceConnectionId = Environment::GetVariable("AZURE_SERVICE_CONNECTION_CLIENT_ID"); + + std::string systemAccessToken = "invalidSystemAccessToken"; + + /*if (tenantId.empty() || clientId.empty() || serviceConnectionId.empty()) + { + std::string message = "Set AZURE_SERVICE_CONNECTION_CLIENT_ID, AZURE_SERVICE_CONNECTION_ID, " + "AZURE_SERVICE_CONNECTION_TENANT_ID, and SYSTEM_ACCESSTOKEN to run this " + "AzurePipelinesCredential test."; + GTEST_SKIP_(message.c_str()); + }*/ + + AzurePipelinesCredential const cred(tenantId, clientId, serviceConnectionId, systemAccessToken); + + TokenRequestContext trc; + trc.Scopes.push_back("https://vault.azure.net/.default"); + + try + { + AccessToken token = cred.GetToken(trc, {}); + } + catch (AuthenticationException const& ex) + { + std::string expectedMessage = ""; + EXPECT_EQ(ex.what(), expectedMessage) << ex.what(); + } +} \ No newline at end of file From bbfd830063f1d80bf6cefe97002056ae69be428d Mon Sep 17 00:00:00 2001 From: Ahson Khan Date: Thu, 20 Jun 2024 17:16:27 -0700 Subject: [PATCH 08/23] Add access token env var in ci.yml. --- sdk/identity/ci.yml | 2 ++ 1 file changed, 2 insertions(+) diff --git a/sdk/identity/ci.yml b/sdk/identity/ci.yml index 2fcb520b3b..c9cc9d5da1 100644 --- a/sdk/identity/ci.yml +++ b/sdk/identity/ci.yml @@ -38,6 +38,8 @@ extends: - Name: azure-identity Path: azure-identity VcpkgPortName: azure-identity-cpp + EnvVars: + SYSTEM_ACCESSTOKEN: $(System.AccessToken) TestEnv: # Tenant ID should use the uniqueID format for playback recordings - Name: AZURE_TENANT_ID From ee6ca438a296550f4a851bbf00e0f1f09e19ee0c Mon Sep 17 00:00:00 2001 From: Ahson Khan Date: Thu, 20 Jun 2024 18:11:08 -0700 Subject: [PATCH 09/23] Add identity yml files and EnvVars. --- .../templates/jobs/archetype-sdk-client.yml | 4 ++++ eng/pipelines/templates/jobs/ci.tests.yml | 7 +++++++ .../templates/stages/archetype-sdk-client.yml | 5 +++++ .../templates/stages/archetype-sdk-tests.yml | 4 ++++ .../managed-identity-matrix.json | 14 +++++++++++++ sdk/identity/azure-identity/tests.yml | 20 +++++++++++++++++++ sdk/identity/ci.yml | 17 ++++++++++++++++ 7 files changed, 71 insertions(+) create mode 100644 sdk/identity/azure-identity/managed-identity-matrix.json create mode 100644 sdk/identity/azure-identity/tests.yml diff --git a/eng/pipelines/templates/jobs/archetype-sdk-client.yml b/eng/pipelines/templates/jobs/archetype-sdk-client.yml index 8b50977a7a..131036b5f8 100644 --- a/eng/pipelines/templates/jobs/archetype-sdk-client.yml +++ b/eng/pipelines/templates/jobs/archetype-sdk-client.yml @@ -50,6 +50,9 @@ parameters: - name: RunMetaJobs type: boolean default: true + - name: EnvVars + type: object + default: {} jobs: - template: /eng/common/pipelines/templates/jobs/generate-job-matrix.yml @@ -75,6 +78,7 @@ jobs: LineCoverageTarget: ${{ parameters.LineCoverageTarget }} BranchCoverageTarget: ${{ parameters.BranchCoverageTarget }} TestEnv: ${{ parameters.TestEnv }} + EnvVars: ${{ parameters.EnvVars }} PreTestSteps: ${{ parameters.PreTestSteps }} PostTestSteps: ${{ parameters.PostTestSteps }} diff --git a/eng/pipelines/templates/jobs/ci.tests.yml b/eng/pipelines/templates/jobs/ci.tests.yml index 3d5b2eb4a5..b477bde056 100644 --- a/eng/pipelines/templates/jobs/ci.tests.yml +++ b/eng/pipelines/templates/jobs/ci.tests.yml @@ -58,6 +58,9 @@ parameters: - name: OSName type: string default: '' + - name: EnvVars + type: object + default: {} jobs: - job: ${{ parameters.DisplayName }}_${{ parameters.OSName }} @@ -103,6 +106,10 @@ jobs: - ${{ each testEnvVar in parameters.TestEnv }}: - name: ${{ testEnvVar.Name }} value: ${{ testEnvVar.Value }} + + env: + ${{ each var in parameters.EnvVars }}: + ${{ var.key }}: ${{ var.value }} steps: - template: /eng/common/pipelines/templates/steps/verify-agent-os.yml diff --git a/eng/pipelines/templates/stages/archetype-sdk-client.yml b/eng/pipelines/templates/stages/archetype-sdk-client.yml index 5d78403196..524b32ff14 100644 --- a/eng/pipelines/templates/stages/archetype-sdk-client.yml +++ b/eng/pipelines/templates/stages/archetype-sdk-client.yml @@ -83,6 +83,9 @@ parameters: - name: CMakeGenerationTimeoutInMinutes type: number default: 120 + - name: EnvVars + type: object + default: {} extends: ${{ if eq(variables['System.TeamProject'], 'internal') }}: @@ -147,6 +150,7 @@ extends: ${{ if eq(parameters.ServiceDirectory, 'template') }}: TestPipeline: true TestEnv: ${{ parameters.TestEnv }} + EnvVars: ${{ parameters.EnvVars }} PreTestSteps: ${{ parameters.PreTestSteps }} PostTestSteps: ${{ parameters.PostTestSteps }} RunMetaJobs: true @@ -172,6 +176,7 @@ extends: ${{ if eq(parameters.ServiceDirectory, 'template') }}: TestPipeline: true TestEnv: ${{ parameters.TestEnv }} + EnvVars: ${{ parameters.EnvVars }} PreTestSteps: ${{ parameters.PreTestSteps }} PostTestSteps: ${{ parameters.PostTestSteps }} RunMetaJobs: false diff --git a/eng/pipelines/templates/stages/archetype-sdk-tests.yml b/eng/pipelines/templates/stages/archetype-sdk-tests.yml index adbe4cada1..6a0aaa5f76 100644 --- a/eng/pipelines/templates/stages/archetype-sdk-tests.yml +++ b/eng/pipelines/templates/stages/archetype-sdk-tests.yml @@ -35,6 +35,9 @@ parameters: - name: PostTestSteps type: stepList default: [] +- name: EnvVars + type: object + default: {} stages: - ${{ each cloud in parameters.CloudConfig }}: @@ -67,3 +70,4 @@ stages: TimeoutInMinutes: ${{ parameters.TimeoutInMinutes}} PreTestSteps: ${{ parameters.PreTestSteps }} PostTestSteps: ${{ parameters.PostTestSteps }} + EnvVars: ${{ parameters.EnvVars }} diff --git a/sdk/identity/azure-identity/managed-identity-matrix.json b/sdk/identity/azure-identity/managed-identity-matrix.json new file mode 100644 index 0000000000..eb55b3f5e8 --- /dev/null +++ b/sdk/identity/azure-identity/managed-identity-matrix.json @@ -0,0 +1,14 @@ +{ + "include": [ + { + "Agent": { + "msi_image": { + "ArmTemplateParameters": "@{deployResources = $true}", + "OSVmImage": "env:LINUXNEXTVMIMAGE", + "Pool": "env:LINUXPOOL" + } + }, + "IDENTITY_IMDS_AVAILABLE": "1" + } + ] +} diff --git a/sdk/identity/azure-identity/tests.yml b/sdk/identity/azure-identity/tests.yml new file mode 100644 index 0000000000..84fb79017c --- /dev/null +++ b/sdk/identity/azure-identity/tests.yml @@ -0,0 +1,20 @@ +trigger: none + +extends: + template: /eng/pipelines/templates/stages/archetype-sdk-tests.yml + parameters: + PackageName: azure-identity-cpp + ServiceDirectory: identity/azure-identity + TimeoutInMinutes: 120 + SupportedClouds: 'Public,UsGov,China,Canary' + CloudConfig: + Public: + SubscriptionConfigurations: + - $(sub-config-azure-cloud-test-resources) + - $(sub-config-identity-test-resources) + ServiceConnection: azure-sdk-tests + EnvVars: + AZURE_CLIENT_ID: $(IDENTITY_CLIENT_ID) + AZURE_CLIENT_SECRET: $(IDENTITY_CLIENT_SECRET) + AZURE_TENANT_ID: $(IDENTITY_TENANT_ID) + SYSTEM_ACCESSTOKEN: $(System.AccessToken) diff --git a/sdk/identity/ci.yml b/sdk/identity/ci.yml index c9cc9d5da1..22aa9f2232 100644 --- a/sdk/identity/ci.yml +++ b/sdk/identity/ci.yml @@ -38,8 +38,25 @@ extends: - Name: azure-identity Path: azure-identity VcpkgPortName: azure-identity-cpp + CloudConfig: + Public: + SubscriptionConfigurations: + - $(sub-config-azure-cloud-test-resources) + - $(sub-config-identity-test-resources) + ServiceConnection: azure-sdk-tests EnvVars: SYSTEM_ACCESSTOKEN: $(System.AccessToken) + RunLiveTests: true + ServiceDirectory: identity/azure-identity + UsePipelineProxy: false + MatrixConfigs: + - Name: managed_identity_matrix + GenerateVMJobs: true + Path: sdk/identity/azure-identity/managed-identity-matrix.json + Selection: sparse + MatrixReplace: + - Pool=.*LINUXPOOL.*/azsdk-pool-mms-ubuntu-2204-identitymsi + - OSVmImage=.*LINUXNEXTVMIMAGE.*/azsdk-pool-mms-ubuntu-2204-1espt TestEnv: # Tenant ID should use the uniqueID format for playback recordings - Name: AZURE_TENANT_ID From 34a9798dbfb58785546ba64398ceb292b854431f Mon Sep 17 00:00:00 2001 From: Ahson Khan Date: Thu, 20 Jun 2024 21:15:33 -0700 Subject: [PATCH 10/23] Fix merge conflicts and print out the oidc response. --- .../azure-identity/src/azure_pipelines_credential.cpp | 10 ++++++++++ .../azure-identity/src/token_credential_impl.cpp | 1 - 2 files changed, 10 insertions(+), 1 deletion(-) diff --git a/sdk/identity/azure-identity/src/azure_pipelines_credential.cpp b/sdk/identity/azure-identity/src/azure_pipelines_credential.cpp index 60d70fc004..a64837ad90 100644 --- a/sdk/identity/azure-identity/src/azure_pipelines_credential.cpp +++ b/sdk/identity/azure-identity/src/azure_pipelines_credential.cpp @@ -207,6 +207,16 @@ std::string AzurePipelinesCredential::GetAssertion(Context const& context) const auto const responseBody = std::string(reinterpret_cast(bodyVec.data()), bodyVec.size()); + std::cout << "OIDC_RESPONSE_STATUSCODE: " << static_cast(response->GetStatusCode()) + << std::endl; + std::cout << "OIDC_RESPONSE_ReasonPhrase: " << response->GetReasonPhrase() << std::endl; + for (const auto& header : response->GetHeaders()) + { + std::cout << "OIDC_RESPONSE_HEADER: " << header.first << ": " << header.second << std::endl; + } + std::cout << "OIDC_RESPONSE: " << responseBody << std::endl; + std::string assertion = GetOidcTokenResponse(response, responseBody); + return GetOidcTokenResponse(response, responseBody); } diff --git a/sdk/identity/azure-identity/src/token_credential_impl.cpp b/sdk/identity/azure-identity/src/token_credential_impl.cpp index bab5478b94..00a6104e2f 100644 --- a/sdk/identity/azure-identity/src/token_credential_impl.cpp +++ b/sdk/identity/azure-identity/src/token_credential_impl.cpp @@ -419,7 +419,6 @@ AccessToken TokenCredentialImpl::ParseToken( } auto const tzOffsetStr = TimeZoneOffsetAsString(utcDiffSeconds); - bool successfulParse = false; if (expiresOn.is_string()) { bool successfulParse = false; From ef32990106c5951b453bbb853a022bfd6da1a075 Mon Sep 17 00:00:00 2001 From: Ahson Khan Date: Thu, 20 Jun 2024 21:28:21 -0700 Subject: [PATCH 11/23] Remove duplicate definition of ServiceDirectory and remove env. --- eng/pipelines/templates/jobs/ci.tests.yml | 4 ---- sdk/identity/ci.yml | 1 - 2 files changed, 5 deletions(-) diff --git a/eng/pipelines/templates/jobs/ci.tests.yml b/eng/pipelines/templates/jobs/ci.tests.yml index b477bde056..358d264c41 100644 --- a/eng/pipelines/templates/jobs/ci.tests.yml +++ b/eng/pipelines/templates/jobs/ci.tests.yml @@ -106,10 +106,6 @@ jobs: - ${{ each testEnvVar in parameters.TestEnv }}: - name: ${{ testEnvVar.Name }} value: ${{ testEnvVar.Value }} - - env: - ${{ each var in parameters.EnvVars }}: - ${{ var.key }}: ${{ var.value }} steps: - template: /eng/common/pipelines/templates/steps/verify-agent-os.yml diff --git a/sdk/identity/ci.yml b/sdk/identity/ci.yml index 22aa9f2232..1e6aa96783 100644 --- a/sdk/identity/ci.yml +++ b/sdk/identity/ci.yml @@ -47,7 +47,6 @@ extends: EnvVars: SYSTEM_ACCESSTOKEN: $(System.AccessToken) RunLiveTests: true - ServiceDirectory: identity/azure-identity UsePipelineProxy: false MatrixConfigs: - Name: managed_identity_matrix From 44914b4c12d21cfa42af24fb832f632622921735 Mon Sep 17 00:00:00 2001 From: Ahson Khan Date: Fri, 21 Jun 2024 10:48:13 -0700 Subject: [PATCH 12/23] Revert CI/infra changes. --- .../templates/jobs/archetype-sdk-client.yml | 4 ---- eng/pipelines/templates/jobs/ci.tests.yml | 3 --- .../templates/stages/archetype-sdk-client.yml | 5 ----- .../templates/stages/archetype-sdk-tests.yml | 4 ---- .../managed-identity-matrix.json | 14 ------------- sdk/identity/azure-identity/tests.yml | 20 ------------------- sdk/identity/ci.yml | 18 ----------------- 7 files changed, 68 deletions(-) delete mode 100644 sdk/identity/azure-identity/managed-identity-matrix.json delete mode 100644 sdk/identity/azure-identity/tests.yml diff --git a/eng/pipelines/templates/jobs/archetype-sdk-client.yml b/eng/pipelines/templates/jobs/archetype-sdk-client.yml index 131036b5f8..8b50977a7a 100644 --- a/eng/pipelines/templates/jobs/archetype-sdk-client.yml +++ b/eng/pipelines/templates/jobs/archetype-sdk-client.yml @@ -50,9 +50,6 @@ parameters: - name: RunMetaJobs type: boolean default: true - - name: EnvVars - type: object - default: {} jobs: - template: /eng/common/pipelines/templates/jobs/generate-job-matrix.yml @@ -78,7 +75,6 @@ jobs: LineCoverageTarget: ${{ parameters.LineCoverageTarget }} BranchCoverageTarget: ${{ parameters.BranchCoverageTarget }} TestEnv: ${{ parameters.TestEnv }} - EnvVars: ${{ parameters.EnvVars }} PreTestSteps: ${{ parameters.PreTestSteps }} PostTestSteps: ${{ parameters.PostTestSteps }} diff --git a/eng/pipelines/templates/jobs/ci.tests.yml b/eng/pipelines/templates/jobs/ci.tests.yml index 358d264c41..3d5b2eb4a5 100644 --- a/eng/pipelines/templates/jobs/ci.tests.yml +++ b/eng/pipelines/templates/jobs/ci.tests.yml @@ -58,9 +58,6 @@ parameters: - name: OSName type: string default: '' - - name: EnvVars - type: object - default: {} jobs: - job: ${{ parameters.DisplayName }}_${{ parameters.OSName }} diff --git a/eng/pipelines/templates/stages/archetype-sdk-client.yml b/eng/pipelines/templates/stages/archetype-sdk-client.yml index 524b32ff14..5d78403196 100644 --- a/eng/pipelines/templates/stages/archetype-sdk-client.yml +++ b/eng/pipelines/templates/stages/archetype-sdk-client.yml @@ -83,9 +83,6 @@ parameters: - name: CMakeGenerationTimeoutInMinutes type: number default: 120 - - name: EnvVars - type: object - default: {} extends: ${{ if eq(variables['System.TeamProject'], 'internal') }}: @@ -150,7 +147,6 @@ extends: ${{ if eq(parameters.ServiceDirectory, 'template') }}: TestPipeline: true TestEnv: ${{ parameters.TestEnv }} - EnvVars: ${{ parameters.EnvVars }} PreTestSteps: ${{ parameters.PreTestSteps }} PostTestSteps: ${{ parameters.PostTestSteps }} RunMetaJobs: true @@ -176,7 +172,6 @@ extends: ${{ if eq(parameters.ServiceDirectory, 'template') }}: TestPipeline: true TestEnv: ${{ parameters.TestEnv }} - EnvVars: ${{ parameters.EnvVars }} PreTestSteps: ${{ parameters.PreTestSteps }} PostTestSteps: ${{ parameters.PostTestSteps }} RunMetaJobs: false diff --git a/eng/pipelines/templates/stages/archetype-sdk-tests.yml b/eng/pipelines/templates/stages/archetype-sdk-tests.yml index 6a0aaa5f76..adbe4cada1 100644 --- a/eng/pipelines/templates/stages/archetype-sdk-tests.yml +++ b/eng/pipelines/templates/stages/archetype-sdk-tests.yml @@ -35,9 +35,6 @@ parameters: - name: PostTestSteps type: stepList default: [] -- name: EnvVars - type: object - default: {} stages: - ${{ each cloud in parameters.CloudConfig }}: @@ -70,4 +67,3 @@ stages: TimeoutInMinutes: ${{ parameters.TimeoutInMinutes}} PreTestSteps: ${{ parameters.PreTestSteps }} PostTestSteps: ${{ parameters.PostTestSteps }} - EnvVars: ${{ parameters.EnvVars }} diff --git a/sdk/identity/azure-identity/managed-identity-matrix.json b/sdk/identity/azure-identity/managed-identity-matrix.json deleted file mode 100644 index eb55b3f5e8..0000000000 --- a/sdk/identity/azure-identity/managed-identity-matrix.json +++ /dev/null @@ -1,14 +0,0 @@ -{ - "include": [ - { - "Agent": { - "msi_image": { - "ArmTemplateParameters": "@{deployResources = $true}", - "OSVmImage": "env:LINUXNEXTVMIMAGE", - "Pool": "env:LINUXPOOL" - } - }, - "IDENTITY_IMDS_AVAILABLE": "1" - } - ] -} diff --git a/sdk/identity/azure-identity/tests.yml b/sdk/identity/azure-identity/tests.yml deleted file mode 100644 index 84fb79017c..0000000000 --- a/sdk/identity/azure-identity/tests.yml +++ /dev/null @@ -1,20 +0,0 @@ -trigger: none - -extends: - template: /eng/pipelines/templates/stages/archetype-sdk-tests.yml - parameters: - PackageName: azure-identity-cpp - ServiceDirectory: identity/azure-identity - TimeoutInMinutes: 120 - SupportedClouds: 'Public,UsGov,China,Canary' - CloudConfig: - Public: - SubscriptionConfigurations: - - $(sub-config-azure-cloud-test-resources) - - $(sub-config-identity-test-resources) - ServiceConnection: azure-sdk-tests - EnvVars: - AZURE_CLIENT_ID: $(IDENTITY_CLIENT_ID) - AZURE_CLIENT_SECRET: $(IDENTITY_CLIENT_SECRET) - AZURE_TENANT_ID: $(IDENTITY_TENANT_ID) - SYSTEM_ACCESSTOKEN: $(System.AccessToken) diff --git a/sdk/identity/ci.yml b/sdk/identity/ci.yml index 1e6aa96783..2fcb520b3b 100644 --- a/sdk/identity/ci.yml +++ b/sdk/identity/ci.yml @@ -38,24 +38,6 @@ extends: - Name: azure-identity Path: azure-identity VcpkgPortName: azure-identity-cpp - CloudConfig: - Public: - SubscriptionConfigurations: - - $(sub-config-azure-cloud-test-resources) - - $(sub-config-identity-test-resources) - ServiceConnection: azure-sdk-tests - EnvVars: - SYSTEM_ACCESSTOKEN: $(System.AccessToken) - RunLiveTests: true - UsePipelineProxy: false - MatrixConfigs: - - Name: managed_identity_matrix - GenerateVMJobs: true - Path: sdk/identity/azure-identity/managed-identity-matrix.json - Selection: sparse - MatrixReplace: - - Pool=.*LINUXPOOL.*/azsdk-pool-mms-ubuntu-2204-identitymsi - - OSVmImage=.*LINUXNEXTVMIMAGE.*/azsdk-pool-mms-ubuntu-2204-1espt TestEnv: # Tenant ID should use the uniqueID format for playback recordings - Name: AZURE_TENANT_ID From dc61ad51f32cf89c90810440e48a2b3dc4fc000f Mon Sep 17 00:00:00 2001 From: Ahson Khan Date: Fri, 21 Jun 2024 10:49:30 -0700 Subject: [PATCH 13/23] Include engsys changes to add federated auth support. --- eng/pipelines/templates/jobs/live.tests.yml | 157 +++++++++++++----- .../templates/stages/archetype-sdk-client.yml | 16 +- .../templates/stages/archetype-sdk-tests.yml | 9 + 3 files changed, 137 insertions(+), 45 deletions(-) diff --git a/eng/pipelines/templates/jobs/live.tests.yml b/eng/pipelines/templates/jobs/live.tests.yml index 3c3d25c0d9..760f22785c 100644 --- a/eng/pipelines/templates/jobs/live.tests.yml +++ b/eng/pipelines/templates/jobs/live.tests.yml @@ -42,6 +42,12 @@ parameters: - name: OSName type: string default: '' +- name: EnvVars + type: object + default: {} +- name: UseFederatedAuth + type: boolean + default: false jobs: - job: @@ -154,12 +160,16 @@ jobs: parameters: SubscriptionConfiguration: ${{ parameters.CloudConfig.SubscriptionConfiguration }} SubscriptionConfigurations: ${{ parameters.CloudConfig.SubscriptionConfigurations }} + SubscriptionConfigurationFilePaths: ${{ parameters.CloudConfig.SubscriptionConfigurationFilePaths }} + EnvVars: ${{ parameters.EnvVars }} - template: /eng/common/TestResources/deploy-test-resources.yml parameters: ServiceDirectory: ${{ parameters.ServiceDirectory }} Location: ${{ coalesce(parameters.Location, parameters.CloudConfig.Location) }} SubscriptionConfiguration: $(SubscriptionConfiguration) + UseFederatedAuth: ${{ parameters.UseFederatedAuth }} + EnvVars: ${{ parameters.EnvVars }} - template: /eng/common/testproxy/test-proxy-tool.yml parameters: @@ -167,25 +177,49 @@ jobs: - ${{ parameters.PreTestSteps }} - # For non multi-config generator use the same build configuration to run tests - # We don't need to set it to invoke ctest - # Visual Studio generator used in CI is a multi-config generator. - # As such, it requires the configuration argument for building and invoking ctest - - bash: | - export AZURE_CLIENT_ID=$(${{parameters.ServiceDirectory}}_CLIENT_ID) - export AZURE_TENANT_ID=$(${{parameters.ServiceDirectory}}_TENANT_ID) - export AZURE_CLIENT_SECRET=$(${{parameters.ServiceDirectory}}_CLIENT_SECRET) + - ${{ if UseFederatedAuth }}: + - task: AzurePowerShell@5 + displayName: ctest + # Runs only if test-resources are happily deployed. + # unit-tests runs for those configs where samples are not ran. + # This enables to run tests and samples at the same time as different matrix configuration. + # Then unit-tests runs, samples should not run. + condition: and(succeeded(), ne(variables['RunSamples'], '1')) + inputs: + azureSubscription: ${{ parameters.CloudConfig.ServiceConnection }} + azurePowerShellVersion: LatestVersion + ScriptType: InlineScript + Inline: | + $account = (Get-AzContext).Account + $env:AZURESUBSCRIPTION_CLIENT_ID = $account.Id + $env:AZURESUBSCRIPTION_TENANT_ID = $account.Tenants - ctest $(WindowsCtestConfig) -V --tests-regex "${{ parameters.CtestRegex }}" --no-compress-output -T Test - workingDirectory: build - displayName: ctest - # Runs only if test-resources are happily deployed. - # unit-tests runs for those configs where samples are not ran. - # This enables to run tests and samples at the same time as different matrix configuration. - # Then unit-tests runs, samples should not run. - condition: and( - succeeded(), - ne(variables['RunSamples'], '1')) + ctest $(WindowsCtestConfig) -V --tests-regex "${{ parameters.CtestRegex }}" --no-compress-output -T Test + workingDirectory: build + env: + SYSTEM_ACCESSTOKEN: $(System.AccessToken) + ${{ insert }}: ${{ parameters.EnvVars }} + + - ${{ else }}: + # For non multi-config generator use the same build configuration to run tests + # We don't need to set it to invoke ctest + # Visual Studio generator used in CI is a multi-config generator. + # As such, it requires the configuration argument for building and invoking ctest + - bash: | + export AZURE_CLIENT_ID=$(${{parameters.ServiceDirectory}}_CLIENT_ID) + export AZURE_TENANT_ID=$(${{parameters.ServiceDirectory}}_TENANT_ID) + export AZURE_CLIENT_SECRET=$(${{parameters.ServiceDirectory}}_CLIENT_SECRET) + + ctest $(WindowsCtestConfig) -V --tests-regex "${{ parameters.CtestRegex }}" --no-compress-output -T Test + workingDirectory: build + displayName: ctest + # Runs only if test-resources are happily deployed. + # unit-tests runs for those configs where samples are not ran. + # This enables to run tests and samples at the same time as different matrix configuration. + # Then unit-tests runs, samples should not run. + condition: and(succeeded(), ne(variables['RunSamples'], '1')) + env: + ${{ insert }}: ${{ parameters.EnvVars }} - ${{ parameters.PostTestSteps }} @@ -201,32 +235,66 @@ jobs: # this step only makes sense when ctest has run condition: and(succeededOrFailed(), ne(variables['RunSamples'], '1')) - # Running Samples step. - # Will run samples described on a file name [service]-samples.txt within the build directory. - # For example keyvault-samples.txt. - # The file is written by CMake during configuration when building samples. - - bash: | - IFS=$'\n' - if [[ -f "./${{ parameters.ServiceDirectory }}-samples.txt" ]]; then - for sample in `cat ./${{ parameters.ServiceDirectory }}-samples.txt` - do - export AZURE_CLIENT_ID=$(${{parameters.ServiceDirectory}}_CLIENT_ID) - export AZURE_TENANT_ID=$(${{parameters.ServiceDirectory}}_TENANT_ID) - export AZURE_CLIENT_SECRET=$(${{parameters.ServiceDirectory}}_CLIENT_SECRET) - echo "**********Running sample: ${sample}" - bash -c "$sample" - status=$? - if [[ $status -eq 0 ]]; then - echo "*********Sample completed*********" - else - echo "*Sample returned a failed code: $status" - exit 1 - fi - done - fi - workingDirectory: build - displayName: "Run Samples for : ${{ parameters.ServiceDirectory }}" - condition: and(succeeded(), eq(variables['RunSamples'], '1')) + + - ${{ if UseFederatedAuth }}: + # Running Samples step. + # Will run samples described on a file name [service]-samples.txt within the build directory. + # For example keyvault-samples.txt. + # The file is written by CMake during configuration when building samples. + - bash: | + IFS=$'\n' + if [[ -f "./${{ parameters.ServiceDirectory }}-samples.txt" ]]; then + for sample in `cat ./${{ parameters.ServiceDirectory }}-samples.txt` + do + export AZURE_CLIENT_ID=$(${{parameters.ServiceDirectory}}_CLIENT_ID) + export AZURE_TENANT_ID=$(${{parameters.ServiceDirectory}}_TENANT_ID) + export AZURE_CLIENT_SECRET=$(${{parameters.ServiceDirectory}}_CLIENT_SECRET) + echo "**********Running sample: ${sample}" + bash -c "$sample" + status=$? + if [[ $status -eq 0 ]]; then + echo "*********Sample completed*********" + else + echo "*Sample returned a failed code: $status" + exit 1 + fi + done + fi + workingDirectory: build + displayName: "Run Samples for : ${{ parameters.ServiceDirectory }}" + condition: and(succeeded(), eq(variables['RunSamples'], '1')) + env: + ${{ insert }}: ${{ parameters.EnvVars }} + + - ${{ else }}: + - task: AzurePowerShell@5 + displayName: "Run Samples for : ${{ parameters.ServiceDirectory }}" + condition: and(succeeded(), eq(variables['RunSamples'], '1')) + inputs: + azureSubscription: ${{ parameters.CloudConfig.ServiceConnection }} + azurePowerShellVersion: LatestVersion + ScriptType: InlineScript + Inline: | + $account = (Get-AzContext).Account + $env:AZURESUBSCRIPTION_CLIENT_ID = $account.Id + $env:AZURESUBSCRIPTION_TENANT_ID = $account.Tenants + + if (Test-Path -Path "${{ parameters.ServiceDirectory }}-samples.txt") { + $samples = Get-Content "${{ parameters.ServiceDirectory }}-samples.txt" + foreach ($sample in $samples) { + Write-Host "**********Running sample: $sample" + & "$sample" + if ($LASTEXITCODE) { + Write-Host "Sample failed with exit code $LASTEXITCODE" + exit 1 + } + Write-Host "**********Sample completed" + } + } + workingDirectory: build + env: + SYSTEM_ACCESSTOKEN: $(System.AccessToken) + ${{ insert }}: ${{ parameters.EnvVars }} # Make coverage targets (specified in coverage_targets.txt) and assemble # coverage report @@ -248,3 +316,4 @@ jobs: parameters: ServiceDirectory: ${{ parameters.ServiceDirectory }} SubscriptionConfiguration: $(SubscriptionConfiguration) + EnvVars: ${{ parameters.EnvVars }} diff --git a/eng/pipelines/templates/stages/archetype-sdk-client.yml b/eng/pipelines/templates/stages/archetype-sdk-client.yml index 5d78403196..56ea64296d 100644 --- a/eng/pipelines/templates/stages/archetype-sdk-client.yml +++ b/eng/pipelines/templates/stages/archetype-sdk-client.yml @@ -56,15 +56,22 @@ parameters: default: Public: SubscriptionConfiguration: $(sub-config-azure-cloud-test-resources) + ServiceConnection: azure-sdk-tests + SubscriptionConfigurationFilePaths: + - eng/common/TestResources/sub-config/AzurePublicMsft.json Preview: SubscriptionConfiguration: $(sub-config-azure-cloud-test-resources-preview) + ServiceConnection: azure-sdk-tests Canary: SubscriptionConfiguration: $(sub-config-azure-cloud-test-resources) + ServiceConnection: azure-sdk-tests Location: 'eastus2euap' UsGov: SubscriptionConfiguration: $(sub-config-gov-test-resources) + ServiceConnection: usgov_azure-sdk-tests China: SubscriptionConfiguration: $(sub-config-cn-test-resources) + ServiceConnection: china_azure-sdk-tests - name: Clouds type: string default: Public @@ -83,6 +90,12 @@ parameters: - name: CMakeGenerationTimeoutInMinutes type: number default: 120 + - name: EnvVars + type: object + default: {} + - name: UseFederatedAuth + type: boolean + default: false extends: ${{ if eq(variables['System.TeamProject'], 'internal') }}: @@ -190,6 +203,8 @@ extends: UnsupportedClouds: ${{ parameters.UnsupportedClouds }} PreTestSteps: ${{ parameters.PreTestSteps }} PostTestSteps: ${{ parameters.PostTestSteps }} + UseFederatedAuth: ${{ parameters.UseFederatedAuth }} + EnvVars: ${{ parameters.EnvVars }} - ${{ if and(eq(variables['System.TeamProject'], 'internal'), not(endsWith(variables['Build.DefinitionName'], ' - tests'))) }}: - template: archetype-cpp-release.yml@self @@ -211,4 +226,3 @@ extends: ArtifactName: packages ${{ if eq(parameters.ServiceDirectory, 'template') }}: TestPipeline: true - diff --git a/eng/pipelines/templates/stages/archetype-sdk-tests.yml b/eng/pipelines/templates/stages/archetype-sdk-tests.yml index adbe4cada1..66f0d63845 100644 --- a/eng/pipelines/templates/stages/archetype-sdk-tests.yml +++ b/eng/pipelines/templates/stages/archetype-sdk-tests.yml @@ -35,6 +35,12 @@ parameters: - name: PostTestSteps type: stepList default: [] +- name: EnvVars + type: object + default: {} +- name: UseFederatedAuth + type: boolean + default: false stages: - ${{ each cloud in parameters.CloudConfig }}: @@ -58,6 +64,7 @@ stages: SubscriptionConfigurations: ${{ cloud.value.SubscriptionConfigurations }} Location: ${{ coalesce(parameters.Location, cloud.value.Location) }} Cloud: ${{ cloud.key }} + SubscriptionConfigurationFilePaths: ${{ cloud.value.SubscriptionConfigurationFilePaths }} AdditionalParameters: Location: ${{ parameters.Location}} ServiceDirectory: ${{ parameters.ServiceDirectory}} @@ -67,3 +74,5 @@ stages: TimeoutInMinutes: ${{ parameters.TimeoutInMinutes}} PreTestSteps: ${{ parameters.PreTestSteps }} PostTestSteps: ${{ parameters.PostTestSteps }} + EnvVars: ${{ parameters.EnvVars }} + UseFederatedAuth: ${{ parameters.UseFederatedAuth }} From ad3a175aa4217a64122b5a1d7b9539afa49d2ffd Mon Sep 17 00:00:00 2001 From: Ahson Khan Date: Fri, 21 Jun 2024 10:54:46 -0700 Subject: [PATCH 14/23] Update environment variables used. --- .../ut/azure_pipelines_credential_test.cpp | 33 +++++++++++-------- 1 file changed, 20 insertions(+), 13 deletions(-) diff --git a/sdk/identity/azure-identity/test/ut/azure_pipelines_credential_test.cpp b/sdk/identity/azure-identity/test/ut/azure_pipelines_credential_test.cpp index 7650533c3d..9b80a64369 100644 --- a/sdk/identity/azure-identity/test/ut/azure_pipelines_credential_test.cpp +++ b/sdk/identity/azure-identity/test/ut/azure_pipelines_credential_test.cpp @@ -497,19 +497,24 @@ TEST(AzurePipelinesCredential, InvalidOidcResponse) TEST(AzurePipelinesCredential, RegularLive_LIVEONLY_) { - std::string tenantId = Environment::GetVariable("AZURE_SERVICE_CONNECTION_TENANT_ID"); - std::string clientId = Environment::GetVariable("AZURE_SERVICE_CONNECTION_CLIENT_ID"); - std::string serviceConnectionId = Environment::GetVariable("AZURE_SERVICE_CONNECTION_CLIENT_ID"); + std::string tenantId = Environment::GetVariable("AZURESUBSCRIPTION_TENANT_ID"); + std::string clientId = Environment::GetVariable("AZURESUBSCRIPTION_CLIENT_ID"); + std::string serviceConnectionId + = Environment::GetVariable("AZURESUBSCRIPTION_SERVICE_CONNECTION_ID"); std::string systemAccessToken = Environment::GetVariable("SYSTEM_ACCESSTOKEN"); if (tenantId.empty() || clientId.empty() || serviceConnectionId.empty() || systemAccessToken.empty()) { - std::string message = "Set AZURE_SERVICE_CONNECTION_CLIENT_ID, AZURE_SERVICE_CONNECTION_ID, " - "AZURE_SERVICE_CONNECTION_TENANT_ID, and SYSTEM_ACCESSTOKEN to run this " - "AzurePipelinesCredential test. " + std::string message = "Set AZURESUBSCRIPTION_TENANT_ID, AZURESUBSCRIPTION_CLIENT_ID, " + "AZURESUBSCRIPTION_SERVICE_CONNECTION_ID, and SYSTEM_ACCESSTOKEN to run " + "this AzurePipelinesCredential test. " + tenantId + " : " + clientId + " : " + serviceConnectionId + " : " + systemAccessToken + "."; + message += "\n " + Environment::GetVariable("AZURE_SERVICE_CONNECTION_TENANT_ID") + " : " + + Environment::GetVariable("AZURE_SERVICE_CONNECTION_CLIENT_ID") + " : " + + Environment::GetVariable("AZURE_SERVICE_CONNECTION_ID") + "."; + std::cout << message << std::endl; GTEST_SKIP_(message.c_str()); } @@ -531,8 +536,8 @@ TEST(AzurePipelinesCredential, RegularLive_LIVEONLY_) TEST(AzurePipelinesCredential, InvalidServiceConnectionId_LIVEONLY_) { - std::string tenantId = Environment::GetVariable("AZURE_SERVICE_CONNECTION_TENANT_ID"); - std::string clientId = Environment::GetVariable("AZURE_SERVICE_CONNECTION_CLIENT_ID"); + std::string tenantId = Environment::GetVariable("AZURESUBSCRIPTION_TENANT_ID"); + std::string clientId = Environment::GetVariable("AZURESUBSCRIPTION_CLIENT_ID"); std::string systemAccessToken = Environment::GetVariable("SYSTEM_ACCESSTOKEN"); std::string serviceConnectionId = "invalidServiceConnectionId"; @@ -563,8 +568,9 @@ TEST(AzurePipelinesCredential, InvalidServiceConnectionId_LIVEONLY_) TEST(AzurePipelinesCredential, InvalidClientId_LIVEONLY_) { - std::string tenantId = Environment::GetVariable("AZURE_SERVICE_CONNECTION_TENANT_ID"); - std::string serviceConnectionId = Environment::GetVariable("AZURE_SERVICE_CONNECTION_CLIENT_ID"); + std::string tenantId = Environment::GetVariable("AZURESUBSCRIPTION_TENANT_ID"); + std::string serviceConnectionId + = Environment::GetVariable("AZURESUBSCRIPTION_SERVICE_CONNECTION_ID"); std::string systemAccessToken = Environment::GetVariable("SYSTEM_ACCESSTOKEN"); std::string clientId = "invalidClientId"; @@ -595,9 +601,10 @@ TEST(AzurePipelinesCredential, InvalidClientId_LIVEONLY_) TEST(AzurePipelinesCredential, InvalidSystemAccessToken_LIVEONLY_) { - std::string tenantId = Environment::GetVariable("AZURE_SERVICE_CONNECTION_TENANT_ID"); - std::string clientId = Environment::GetVariable("SYSTEM_ACCESSTOKEN"); - std::string serviceConnectionId = Environment::GetVariable("AZURE_SERVICE_CONNECTION_CLIENT_ID"); + std::string tenantId = Environment::GetVariable("AZURESUBSCRIPTION_TENANT_ID"); + std::string clientId = Environment::GetVariable("AZURESUBSCRIPTION_CLIENT_ID"); + std::string serviceConnectionId + = Environment::GetVariable("AZURESUBSCRIPTION_SERVICE_CONNECTION_ID"); std::string systemAccessToken = "invalidSystemAccessToken"; From dbd5497068e39f1e67ba8b271cba2e15077cde53 Mon Sep 17 00:00:00 2001 From: Ahson Khan Date: Fri, 21 Jun 2024 13:02:56 -0700 Subject: [PATCH 15/23] Sync recent engsys changes. --- eng/pipelines/templates/jobs/live.tests.yml | 7 +++++-- eng/pipelines/templates/stages/archetype-sdk-tests.yml | 1 + sdk/identity/ci.yml | 2 ++ sdk/identity/test-resources.json | 10 ---------- 4 files changed, 8 insertions(+), 12 deletions(-) diff --git a/eng/pipelines/templates/jobs/live.tests.yml b/eng/pipelines/templates/jobs/live.tests.yml index 760f22785c..c4078d31bd 100644 --- a/eng/pipelines/templates/jobs/live.tests.yml +++ b/eng/pipelines/templates/jobs/live.tests.yml @@ -170,6 +170,7 @@ jobs: SubscriptionConfiguration: $(SubscriptionConfiguration) UseFederatedAuth: ${{ parameters.UseFederatedAuth }} EnvVars: ${{ parameters.EnvVars }} + ServiceConnection: ${{ parameters.CloudConfig.ServiceConnection }} - template: /eng/common/testproxy/test-proxy-tool.yml parameters: @@ -177,7 +178,7 @@ jobs: - ${{ parameters.PreTestSteps }} - - ${{ if UseFederatedAuth }}: + - ${{ if parameters.UseFederatedAuth }}: - task: AzurePowerShell@5 displayName: ctest # Runs only if test-resources are happily deployed. @@ -236,7 +237,7 @@ jobs: condition: and(succeededOrFailed(), ne(variables['RunSamples'], '1')) - - ${{ if UseFederatedAuth }}: + - ${{ if parameters.UseFederatedAuth }}: # Running Samples step. # Will run samples described on a file name [service]-samples.txt within the build directory. # For example keyvault-samples.txt. @@ -316,4 +317,6 @@ jobs: parameters: ServiceDirectory: ${{ parameters.ServiceDirectory }} SubscriptionConfiguration: $(SubscriptionConfiguration) + UseFederatedAuth: ${{ parameters.UseFederatedAuth }} EnvVars: ${{ parameters.EnvVars }} + ServiceConnection: ${{ parameters.CloudConfig.ServiceConnection }} diff --git a/eng/pipelines/templates/stages/archetype-sdk-tests.yml b/eng/pipelines/templates/stages/archetype-sdk-tests.yml index 66f0d63845..1df9ae070e 100644 --- a/eng/pipelines/templates/stages/archetype-sdk-tests.yml +++ b/eng/pipelines/templates/stages/archetype-sdk-tests.yml @@ -65,6 +65,7 @@ stages: Location: ${{ coalesce(parameters.Location, cloud.value.Location) }} Cloud: ${{ cloud.key }} SubscriptionConfigurationFilePaths: ${{ cloud.value.SubscriptionConfigurationFilePaths }} + ServiceConnection: ${{ cloud.value.ServiceConnection }} AdditionalParameters: Location: ${{ parameters.Location}} ServiceDirectory: ${{ parameters.ServiceDirectory}} diff --git a/sdk/identity/ci.yml b/sdk/identity/ci.yml index 2fcb520b3b..c24cede395 100644 --- a/sdk/identity/ci.yml +++ b/sdk/identity/ci.yml @@ -30,6 +30,8 @@ extends: LiveTestCtestRegex: azure-identity. LineCoverageTarget: 95 BranchCoverageTarget: 56 + # TODO: Revert before merge + UseFederatedAuth: true Artifacts: - Name: azure-identity Path: azure-identity diff --git a/sdk/identity/test-resources.json b/sdk/identity/test-resources.json index b7d84ff613..b71f8c95d4 100644 --- a/sdk/identity/test-resources.json +++ b/sdk/identity/test-resources.json @@ -14,12 +14,6 @@ "metadata": { "description": "The application client ID used to run tests." } - }, - "testApplicationSecret": { - "type": "string", - "metadata": { - "description": "The application client secret used to run tests." - } } }, "resources": [], @@ -31,10 +25,6 @@ "AZURE_CLIENT_ID": { "type": "string", "value": "[parameters('testApplicationId')]" - }, - "AZURE_CLIENT_SECRET": { - "type": "string", - "value": "[parameters('testApplicationSecret')]" } } } From bea1ac4e824cf382b930d4bfde5252da084ba329 Mon Sep 17 00:00:00 2001 From: Ahson Khan Date: Fri, 21 Jun 2024 14:41:18 -0700 Subject: [PATCH 16/23] Add invalid tenant id test and re-order them. --- .../ut/azure_pipelines_credential_test.cpp | 59 +++++++++++++++---- 1 file changed, 46 insertions(+), 13 deletions(-) diff --git a/sdk/identity/azure-identity/test/ut/azure_pipelines_credential_test.cpp b/sdk/identity/azure-identity/test/ut/azure_pipelines_credential_test.cpp index 9b80a64369..c9d6bfe11d 100644 --- a/sdk/identity/azure-identity/test/ut/azure_pipelines_credential_test.cpp +++ b/sdk/identity/azure-identity/test/ut/azure_pipelines_credential_test.cpp @@ -503,18 +503,18 @@ TEST(AzurePipelinesCredential, RegularLive_LIVEONLY_) = Environment::GetVariable("AZURESUBSCRIPTION_SERVICE_CONNECTION_ID"); std::string systemAccessToken = Environment::GetVariable("SYSTEM_ACCESSTOKEN"); + std::string message = "Set AZURESUBSCRIPTION_TENANT_ID, AZURESUBSCRIPTION_CLIENT_ID, " + "AZURESUBSCRIPTION_SERVICE_CONNECTION_ID, and SYSTEM_ACCESSTOKEN to run " + "this AzurePipelinesCredential test. " + + tenantId + " : " + clientId + " : " + serviceConnectionId + " : " + systemAccessToken + "."; + message += "\n " + Environment::GetVariable("AZURE_SERVICE_CONNECTION_TENANT_ID") + " : " + + Environment::GetVariable("AZURE_SERVICE_CONNECTION_CLIENT_ID") + " : " + + Environment::GetVariable("AZURE_SERVICE_CONNECTION_ID") + "."; + std::cout << message << std::endl; + if (tenantId.empty() || clientId.empty() || serviceConnectionId.empty() || systemAccessToken.empty()) { - std::string message = "Set AZURESUBSCRIPTION_TENANT_ID, AZURESUBSCRIPTION_CLIENT_ID, " - "AZURESUBSCRIPTION_SERVICE_CONNECTION_ID, and SYSTEM_ACCESSTOKEN to run " - "this AzurePipelinesCredential test. " - + tenantId + " : " + clientId + " : " + serviceConnectionId + " : " + systemAccessToken - + "."; - message += "\n " + Environment::GetVariable("AZURE_SERVICE_CONNECTION_TENANT_ID") + " : " - + Environment::GetVariable("AZURE_SERVICE_CONNECTION_CLIENT_ID") + " : " - + Environment::GetVariable("AZURE_SERVICE_CONNECTION_ID") + "."; - std::cout << message << std::endl; GTEST_SKIP_(message.c_str()); } @@ -534,15 +534,16 @@ TEST(AzurePipelinesCredential, RegularLive_LIVEONLY_) << "Expected a cached token."; } -TEST(AzurePipelinesCredential, InvalidServiceConnectionId_LIVEONLY_) +TEST(AzurePipelinesCredential, InvalidTenantId_LIVEONLY_) { - std::string tenantId = Environment::GetVariable("AZURESUBSCRIPTION_TENANT_ID"); std::string clientId = Environment::GetVariable("AZURESUBSCRIPTION_CLIENT_ID"); + std::string serviceConnectionId + = Environment::GetVariable("AZURESUBSCRIPTION_SERVICE_CONNECTION_ID"); std::string systemAccessToken = Environment::GetVariable("SYSTEM_ACCESSTOKEN"); - std::string serviceConnectionId = "invalidServiceConnectionId"; + std::string tenantId = "invalidtenantId"; - /*if (tenantId.empty() || clientId.empty() || systemAccessToken.empty()) + /*if (tenantId.empty() || serviceConnectionId.empty() || systemAccessToken.empty()) { std::string message = "Set AZURE_SERVICE_CONNECTION_CLIENT_ID, AZURE_SERVICE_CONNECTION_ID, " "AZURE_SERVICE_CONNECTION_TENANT_ID, and SYSTEM_ACCESSTOKEN to run this " @@ -599,6 +600,38 @@ TEST(AzurePipelinesCredential, InvalidClientId_LIVEONLY_) } } +TEST(AzurePipelinesCredential, InvalidServiceConnectionId_LIVEONLY_) +{ + std::string tenantId = Environment::GetVariable("AZURESUBSCRIPTION_TENANT_ID"); + std::string clientId = Environment::GetVariable("AZURESUBSCRIPTION_CLIENT_ID"); + std::string systemAccessToken = Environment::GetVariable("SYSTEM_ACCESSTOKEN"); + + std::string serviceConnectionId = "invalidServiceConnectionId"; + + /*if (tenantId.empty() || clientId.empty() || systemAccessToken.empty()) + { + std::string message = "Set AZURE_SERVICE_CONNECTION_CLIENT_ID, AZURE_SERVICE_CONNECTION_ID, " + "AZURE_SERVICE_CONNECTION_TENANT_ID, and SYSTEM_ACCESSTOKEN to run this " + "AzurePipelinesCredential test."; + GTEST_SKIP_(message.c_str()); + }*/ + + AzurePipelinesCredential const cred(tenantId, clientId, serviceConnectionId, systemAccessToken); + + TokenRequestContext trc; + trc.Scopes.push_back("https://vault.azure.net/.default"); + + try + { + AccessToken token = cred.GetToken(trc, {}); + } + catch (AuthenticationException const& ex) + { + std::string expectedMessage = ""; + EXPECT_EQ(ex.what(), expectedMessage) << ex.what(); + } +} + TEST(AzurePipelinesCredential, InvalidSystemAccessToken_LIVEONLY_) { std::string tenantId = Environment::GetVariable("AZURESUBSCRIPTION_TENANT_ID"); From 2b57ba54eb2a010bf70ebd769926735fc953c657 Mon Sep 17 00:00:00 2001 From: Ahson Khan Date: Tue, 25 Jun 2024 16:44:48 -0700 Subject: [PATCH 17/23] Fail the live test pipeline if a test fails. --- eng/pipelines/templates/jobs/live.tests.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/eng/pipelines/templates/jobs/live.tests.yml b/eng/pipelines/templates/jobs/live.tests.yml index 499724560c..c38c5d14ba 100644 --- a/eng/pipelines/templates/jobs/live.tests.yml +++ b/eng/pipelines/templates/jobs/live.tests.yml @@ -196,6 +196,7 @@ jobs: $env:AZURESUBSCRIPTION_TENANT_ID = $account.Tenants ctest $(WindowsCtestConfig) -V --tests-regex "${{ parameters.CtestRegex }}" --no-compress-output -T Test + exit $LASTEXITCODE workingDirectory: build env: SYSTEM_ACCESSTOKEN: $(System.AccessToken) From 1811b65df5cfe76a0205d273381778814f79be0e Mon Sep 17 00:00:00 2001 From: Ahson Khan Date: Tue, 25 Jun 2024 18:37:54 -0700 Subject: [PATCH 18/23] Update tests and revert source changes. --- .../src/azure_pipelines_credential.cpp | 28 ++---- .../ut/azure_pipelines_credential_test.cpp | 94 +++++++++++-------- 2 files changed, 62 insertions(+), 60 deletions(-) diff --git a/sdk/identity/azure-identity/src/azure_pipelines_credential.cpp b/sdk/identity/azure-identity/src/azure_pipelines_credential.cpp index a64837ad90..2fb8f0757a 100644 --- a/sdk/identity/azure-identity/src/azure_pipelines_credential.cpp +++ b/sdk/identity/azure-identity/src/azure_pipelines_credential.cpp @@ -101,18 +101,18 @@ AzurePipelinesCredential::AzurePipelinesCredential( + "' needed by " + GetCredentialName() + ". This should be set by Azure Pipelines."); } - 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 (isTenantIdValid && !clientId.empty() && !serviceConnectionId.empty() && !systemAccessToken.empty() && !m_oidcRequestUrl.empty()) { + 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); + IdentityLog::Write( IdentityLog::Level::Informational, GetCredentialName() + " was created successfully."); } @@ -207,16 +207,6 @@ std::string AzurePipelinesCredential::GetAssertion(Context const& context) const auto const responseBody = std::string(reinterpret_cast(bodyVec.data()), bodyVec.size()); - std::cout << "OIDC_RESPONSE_STATUSCODE: " << static_cast(response->GetStatusCode()) - << std::endl; - std::cout << "OIDC_RESPONSE_ReasonPhrase: " << response->GetReasonPhrase() << std::endl; - for (const auto& header : response->GetHeaders()) - { - std::cout << "OIDC_RESPONSE_HEADER: " << header.first << ": " << header.second << std::endl; - } - std::cout << "OIDC_RESPONSE: " << responseBody << std::endl; - std::string assertion = GetOidcTokenResponse(response, responseBody); - return GetOidcTokenResponse(response, responseBody); } diff --git a/sdk/identity/azure-identity/test/ut/azure_pipelines_credential_test.cpp b/sdk/identity/azure-identity/test/ut/azure_pipelines_credential_test.cpp index c9d6bfe11d..02f110e33b 100644 --- a/sdk/identity/azure-identity/test/ut/azure_pipelines_credential_test.cpp +++ b/sdk/identity/azure-identity/test/ut/azure_pipelines_credential_test.cpp @@ -495,6 +495,25 @@ TEST(AzurePipelinesCredential, InvalidOidcResponse) AuthenticationException); } +constexpr auto TenantIdEnvVar = "AZURESUBSCRIPTION_TENANT_ID"; +constexpr auto ClientIdEnvVar = "AZURESUBSCRIPTION_CLIENT_ID"; +constexpr auto ServiceConnectionIdEnvVar = "AZURESUBSCRIPTION_SERVICE_CONNECTION_ID"; +constexpr auto SystemAccessTokenEnv = "SYSTEM_ACCESSTOKEN"; + +static std::string GetSkipTestMessage( + std::string tenantId, + std::string clientId, + std::string serviceConnectionId, + std::string systemAccessToken) +{ + std::string message = "Set " + std::string(TenantIdEnvVar) + ", " + ClientIdEnvVar + ", " + + ServiceConnectionIdEnvVar + ", and " + SystemAccessTokenEnv + + " to run this AzurePipelinesCredential test. Tenant ID - '" + tenantId + "', Client ID - '" + + clientId + "', Service Connection ID - '" + serviceConnectionId + + "', and System Access Token size : " + std::to_string(systemAccessToken.size()) + "."; + return message; +} + TEST(AzurePipelinesCredential, RegularLive_LIVEONLY_) { std::string tenantId = Environment::GetVariable("AZURESUBSCRIPTION_TENANT_ID"); @@ -503,18 +522,11 @@ TEST(AzurePipelinesCredential, RegularLive_LIVEONLY_) = Environment::GetVariable("AZURESUBSCRIPTION_SERVICE_CONNECTION_ID"); std::string systemAccessToken = Environment::GetVariable("SYSTEM_ACCESSTOKEN"); - std::string message = "Set AZURESUBSCRIPTION_TENANT_ID, AZURESUBSCRIPTION_CLIENT_ID, " - "AZURESUBSCRIPTION_SERVICE_CONNECTION_ID, and SYSTEM_ACCESSTOKEN to run " - "this AzurePipelinesCredential test. " - + tenantId + " : " + clientId + " : " + serviceConnectionId + " : " + systemAccessToken + "."; - message += "\n " + Environment::GetVariable("AZURE_SERVICE_CONNECTION_TENANT_ID") + " : " - + Environment::GetVariable("AZURE_SERVICE_CONNECTION_CLIENT_ID") + " : " - + Environment::GetVariable("AZURE_SERVICE_CONNECTION_ID") + "."; - std::cout << message << std::endl; - if (tenantId.empty() || clientId.empty() || serviceConnectionId.empty() || systemAccessToken.empty()) { + std::string message + = GetSkipTestMessage(tenantId, clientId, serviceConnectionId, systemAccessToken); GTEST_SKIP_(message.c_str()); } @@ -541,15 +553,14 @@ TEST(AzurePipelinesCredential, InvalidTenantId_LIVEONLY_) = Environment::GetVariable("AZURESUBSCRIPTION_SERVICE_CONNECTION_ID"); std::string systemAccessToken = Environment::GetVariable("SYSTEM_ACCESSTOKEN"); - std::string tenantId = "invalidtenantId"; + const std::string tenantId = "invalidtenantId"; - /*if (tenantId.empty() || serviceConnectionId.empty() || systemAccessToken.empty()) + if (clientId.empty() || serviceConnectionId.empty() || systemAccessToken.empty()) { - std::string message = "Set AZURE_SERVICE_CONNECTION_CLIENT_ID, AZURE_SERVICE_CONNECTION_ID, " - "AZURE_SERVICE_CONNECTION_TENANT_ID, and SYSTEM_ACCESSTOKEN to run this " - "AzurePipelinesCredential test."; + std::string message + = GetSkipTestMessage(tenantId, clientId, serviceConnectionId, systemAccessToken); GTEST_SKIP_(message.c_str()); - }*/ + } AzurePipelinesCredential const cred(tenantId, clientId, serviceConnectionId, systemAccessToken); @@ -559,11 +570,12 @@ TEST(AzurePipelinesCredential, InvalidTenantId_LIVEONLY_) try { AccessToken token = cred.GetToken(trc, {}); + GTEST_FAIL() << "GetToken should have thrown an exception due to an invalid tenant ID."; } catch (AuthenticationException const& ex) { - std::string expectedMessage = ""; - EXPECT_EQ(ex.what(), expectedMessage) << ex.what(); + EXPECT_TRUE(std::string(ex.what()).find("400 Bad Request") != std::string::npos) << ex.what(); + EXPECT_TRUE(std::string(ex.what()).find("AADSTS900023") != std::string::npos) << ex.what(); } } @@ -574,15 +586,14 @@ TEST(AzurePipelinesCredential, InvalidClientId_LIVEONLY_) = Environment::GetVariable("AZURESUBSCRIPTION_SERVICE_CONNECTION_ID"); std::string systemAccessToken = Environment::GetVariable("SYSTEM_ACCESSTOKEN"); - std::string clientId = "invalidClientId"; + const std::string clientId = "invalidClientId"; - /*if (tenantId.empty() || serviceConnectionId.empty() || systemAccessToken.empty()) + if (tenantId.empty() || serviceConnectionId.empty() || systemAccessToken.empty()) { - std::string message = "Set AZURE_SERVICE_CONNECTION_CLIENT_ID, AZURE_SERVICE_CONNECTION_ID, " - "AZURE_SERVICE_CONNECTION_TENANT_ID, and SYSTEM_ACCESSTOKEN to run this " - "AzurePipelinesCredential test."; + std::string message + = GetSkipTestMessage(tenantId, clientId, serviceConnectionId, systemAccessToken); GTEST_SKIP_(message.c_str()); - }*/ + } AzurePipelinesCredential const cred(tenantId, clientId, serviceConnectionId, systemAccessToken); @@ -592,11 +603,12 @@ TEST(AzurePipelinesCredential, InvalidClientId_LIVEONLY_) try { AccessToken token = cred.GetToken(trc, {}); + GTEST_FAIL() << "GetToken should have thrown an exception due to an invalid client ID."; } catch (AuthenticationException const& ex) { - std::string expectedMessage = ""; - EXPECT_EQ(ex.what(), expectedMessage) << ex.what(); + EXPECT_TRUE(std::string(ex.what()).find("400 Bad Request") != std::string::npos) << ex.what(); + EXPECT_TRUE(std::string(ex.what()).find("AADSTS700016") != std::string::npos) << ex.what(); } } @@ -606,15 +618,14 @@ TEST(AzurePipelinesCredential, InvalidServiceConnectionId_LIVEONLY_) std::string clientId = Environment::GetVariable("AZURESUBSCRIPTION_CLIENT_ID"); std::string systemAccessToken = Environment::GetVariable("SYSTEM_ACCESSTOKEN"); - std::string serviceConnectionId = "invalidServiceConnectionId"; + const std::string serviceConnectionId = "invalidServiceConnectionId"; - /*if (tenantId.empty() || clientId.empty() || systemAccessToken.empty()) + if (tenantId.empty() || clientId.empty() || systemAccessToken.empty()) { - std::string message = "Set AZURE_SERVICE_CONNECTION_CLIENT_ID, AZURE_SERVICE_CONNECTION_ID, " - "AZURE_SERVICE_CONNECTION_TENANT_ID, and SYSTEM_ACCESSTOKEN to run this " - "AzurePipelinesCredential test."; + std::string message + = GetSkipTestMessage(tenantId, clientId, serviceConnectionId, systemAccessToken); GTEST_SKIP_(message.c_str()); - }*/ + } AzurePipelinesCredential const cred(tenantId, clientId, serviceConnectionId, systemAccessToken); @@ -624,11 +635,12 @@ TEST(AzurePipelinesCredential, InvalidServiceConnectionId_LIVEONLY_) try { AccessToken token = cred.GetToken(trc, {}); + GTEST_FAIL() + << "GetToken should have thrown an exception due to an invalid service connection ID."; } catch (AuthenticationException const& ex) { - std::string expectedMessage = ""; - EXPECT_EQ(ex.what(), expectedMessage) << ex.what(); + EXPECT_TRUE(std::string(ex.what()).find("404 (Not Found)") != std::string::npos) << ex.what(); } } @@ -639,15 +651,14 @@ TEST(AzurePipelinesCredential, InvalidSystemAccessToken_LIVEONLY_) std::string serviceConnectionId = Environment::GetVariable("AZURESUBSCRIPTION_SERVICE_CONNECTION_ID"); - std::string systemAccessToken = "invalidSystemAccessToken"; + const std::string systemAccessToken = "invalidSystemAccessToken"; - /*if (tenantId.empty() || clientId.empty() || serviceConnectionId.empty()) + if (tenantId.empty() || clientId.empty() || serviceConnectionId.empty()) { - std::string message = "Set AZURE_SERVICE_CONNECTION_CLIENT_ID, AZURE_SERVICE_CONNECTION_ID, " - "AZURE_SERVICE_CONNECTION_TENANT_ID, and SYSTEM_ACCESSTOKEN to run this " - "AzurePipelinesCredential test."; + std::string message + = GetSkipTestMessage(tenantId, clientId, serviceConnectionId, systemAccessToken); GTEST_SKIP_(message.c_str()); - }*/ + } AzurePipelinesCredential const cred(tenantId, clientId, serviceConnectionId, systemAccessToken); @@ -657,10 +668,11 @@ TEST(AzurePipelinesCredential, InvalidSystemAccessToken_LIVEONLY_) try { AccessToken token = cred.GetToken(trc, {}); + GTEST_FAIL() + << "GetToken should have thrown an exception due to an invalid system access token."; } catch (AuthenticationException const& ex) { - std::string expectedMessage = ""; - EXPECT_EQ(ex.what(), expectedMessage) << ex.what(); + EXPECT_TRUE(std::string(ex.what()).find("302 (Found)") != std::string::npos) << ex.what(); } } From eba669cd39baf2f0f4aef7ead04c945f2e0d0a0b Mon Sep 17 00:00:00 2001 From: Ahson Khan Date: Tue, 25 Jun 2024 19:50:51 -0700 Subject: [PATCH 19/23] Debug failing TokenCredentialTest in new live test environment. --- .../test/ut/token_credential_test.cpp | 24 +++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/sdk/identity/azure-identity/test/ut/token_credential_test.cpp b/sdk/identity/azure-identity/test/ut/token_credential_test.cpp index 319486c253..f2c9247464 100644 --- a/sdk/identity/azure-identity/test/ut/token_credential_test.cpp +++ b/sdk/identity/azure-identity/test/ut/token_credential_test.cpp @@ -53,8 +53,25 @@ namespace Azure { namespace Identity { namespace Test { using namespace Azure::Identity::Test; using namespace Azure::Identity; +static void OutputString(std::string envarName, std::string envVarValue) +{ + std::cout << envarName << ": " << envVarValue << std::endl; + if (!envVarValue.empty()) + { + std::cout << "PARTIAL (first.last): " << envVarValue.at(0) << " . " + << envVarValue.at(envVarValue.size() - 1) << std::endl; + } +} + TEST_F(TokenCredentialTest, ClientSecret) { + OutputString("AZURE_TENANT_ID", GetEnv("AZURE_TENANT_ID")); + OutputString("AZURE_CLIENT_ID", GetEnv("AZURE_CLIENT_ID")); + OutputString("AZURE_CLIENT_SECRET", GetEnv("AZURE_CLIENT_SECRET")); + OutputString("AZURE_SERVICE_DIRECTORY", GetEnv("AZURE_SERVICE_DIRECTORY")); + OutputString("AZURE_CLIENT_CERTIFICATE_PATH", GetEnv("AZURE_CLIENT_CERTIFICATE_PATH")); + OutputString("AZURE_AUTHORITY_HOST", GetEnv("AZURE_AUTHORITY_HOST")); + std::string const testName(GetTestName()); auto const clientSecretCredential = GetClientSecretCredential(testName); @@ -71,6 +88,13 @@ TEST_F(TokenCredentialTest, ClientSecret) TEST_F(TokenCredentialTest, EnvironmentCredential) { + OutputString("AZURE_TENANT_ID", GetEnv("AZURE_TENANT_ID")); + OutputString("AZURE_CLIENT_ID", GetEnv("AZURE_CLIENT_ID")); + OutputString("AZURE_CLIENT_SECRET", GetEnv("AZURE_CLIENT_SECRET")); + OutputString("AZURE_SERVICE_DIRECTORY", GetEnv("AZURE_SERVICE_DIRECTORY")); + OutputString("AZURE_CLIENT_CERTIFICATE_PATH", GetEnv("AZURE_CLIENT_CERTIFICATE_PATH")); + OutputString("AZURE_AUTHORITY_HOST", GetEnv("AZURE_AUTHORITY_HOST")); + std::string const testName(GetTestName()); auto const clientSecretCredential = GetEnvironmentCredential(testName); From fa3250c6541ef1afddba82901ff5fa9b8f091136 Mon Sep 17 00:00:00 2001 From: Ahson Khan Date: Tue, 25 Jun 2024 20:02:27 -0700 Subject: [PATCH 20/23] Dont fail test on missing env var. --- .../test/ut/token_credential_test.cpp | 28 +++++++++++-------- 1 file changed, 16 insertions(+), 12 deletions(-) diff --git a/sdk/identity/azure-identity/test/ut/token_credential_test.cpp b/sdk/identity/azure-identity/test/ut/token_credential_test.cpp index f2c9247464..2c7a0951e7 100644 --- a/sdk/identity/azure-identity/test/ut/token_credential_test.cpp +++ b/sdk/identity/azure-identity/test/ut/token_credential_test.cpp @@ -63,14 +63,17 @@ static void OutputString(std::string envarName, std::string envVarValue) } } +using Azure::Core::_internal::Environment; + TEST_F(TokenCredentialTest, ClientSecret) { - OutputString("AZURE_TENANT_ID", GetEnv("AZURE_TENANT_ID")); - OutputString("AZURE_CLIENT_ID", GetEnv("AZURE_CLIENT_ID")); - OutputString("AZURE_CLIENT_SECRET", GetEnv("AZURE_CLIENT_SECRET")); - OutputString("AZURE_SERVICE_DIRECTORY", GetEnv("AZURE_SERVICE_DIRECTORY")); - OutputString("AZURE_CLIENT_CERTIFICATE_PATH", GetEnv("AZURE_CLIENT_CERTIFICATE_PATH")); - OutputString("AZURE_AUTHORITY_HOST", GetEnv("AZURE_AUTHORITY_HOST")); + OutputString("AZURE_TENANT_ID", Environment::GetVariable("AZURE_TENANT_ID")); + OutputString("AZURE_CLIENT_ID", Environment::GetVariable("AZURE_CLIENT_ID")); + OutputString("AZURE_CLIENT_SECRET", Environment::GetVariable("AZURE_CLIENT_SECRET")); + OutputString("AZURE_SERVICE_DIRECTORY", Environment::GetVariable("AZURE_SERVICE_DIRECTORY")); + OutputString( + "AZURE_CLIENT_CERTIFICATE_PATH", Environment::GetVariable("AZURE_CLIENT_CERTIFICATE_PATH")); + OutputString("AZURE_AUTHORITY_HOST", Environment::GetVariable("AZURE_AUTHORITY_HOST")); std::string const testName(GetTestName()); auto const clientSecretCredential = GetClientSecretCredential(testName); @@ -88,12 +91,13 @@ TEST_F(TokenCredentialTest, ClientSecret) TEST_F(TokenCredentialTest, EnvironmentCredential) { - OutputString("AZURE_TENANT_ID", GetEnv("AZURE_TENANT_ID")); - OutputString("AZURE_CLIENT_ID", GetEnv("AZURE_CLIENT_ID")); - OutputString("AZURE_CLIENT_SECRET", GetEnv("AZURE_CLIENT_SECRET")); - OutputString("AZURE_SERVICE_DIRECTORY", GetEnv("AZURE_SERVICE_DIRECTORY")); - OutputString("AZURE_CLIENT_CERTIFICATE_PATH", GetEnv("AZURE_CLIENT_CERTIFICATE_PATH")); - OutputString("AZURE_AUTHORITY_HOST", GetEnv("AZURE_AUTHORITY_HOST")); + OutputString("AZURE_TENANT_ID", Environment::GetVariable("AZURE_TENANT_ID")); + OutputString("AZURE_CLIENT_ID", Environment::GetVariable("AZURE_CLIENT_ID")); + OutputString("AZURE_CLIENT_SECRET", Environment::GetVariable("AZURE_CLIENT_SECRET")); + OutputString("AZURE_SERVICE_DIRECTORY", Environment::GetVariable("AZURE_SERVICE_DIRECTORY")); + OutputString( + "AZURE_CLIENT_CERTIFICATE_PATH", Environment::GetVariable("AZURE_CLIENT_CERTIFICATE_PATH")); + OutputString("AZURE_AUTHORITY_HOST", Environment::GetVariable("AZURE_AUTHORITY_HOST")); std::string const testName(GetTestName()); auto const clientSecretCredential = GetEnvironmentCredential(testName); From a834ce962780c71d4a96c00a47c4eba87adbb7b0 Mon Sep 17 00:00:00 2001 From: Ahson Khan Date: Tue, 25 Jun 2024 20:27:34 -0700 Subject: [PATCH 21/23] Disable federated auth in ci.yml and add back client secret env var. --- .../test/ut/token_credential_test.cpp | 19 +++++++++++++++---- sdk/identity/ci.yml | 2 -- sdk/identity/test-resources.json | 10 ++++++++++ 3 files changed, 25 insertions(+), 6 deletions(-) diff --git a/sdk/identity/azure-identity/test/ut/token_credential_test.cpp b/sdk/identity/azure-identity/test/ut/token_credential_test.cpp index 2c7a0951e7..dabb104ef5 100644 --- a/sdk/identity/azure-identity/test/ut/token_credential_test.cpp +++ b/sdk/identity/azure-identity/test/ut/token_credential_test.cpp @@ -53,13 +53,24 @@ namespace Azure { namespace Identity { namespace Test { using namespace Azure::Identity::Test; using namespace Azure::Identity; -static void OutputString(std::string envarName, std::string envVarValue) +static void OutputString(std::string enVarName, std::string envVarValue) { - std::cout << envarName << ": " << envVarValue << std::endl; + std::cout << enVarName << ": " << envVarValue << std::endl; if (!envVarValue.empty()) { - std::cout << "PARTIAL (first.last): " << envVarValue.at(0) << " . " - << envVarValue.at(envVarValue.size() - 1) << std::endl; + if (envVarValue.size() > 6) + { + std::cout << "PARTIAL (first...last): " << envVarValue.size() << " | " << envVarValue.at(0) + << " . " << envVarValue.at(1) << " . " << envVarValue.at(2) + << envVarValue.at(envVarValue.size() - 3) << " . " + << envVarValue.at(envVarValue.size() - 2) << " . " + << envVarValue.at(envVarValue.size() - 1) << std::endl; + } + else + { + std::cout << "PARTIAL (first.last): " << envVarValue.size() << " | " << envVarValue.at(0) + << " . " << envVarValue.at(envVarValue.size() - 1) << std::endl; + } } } diff --git a/sdk/identity/ci.yml b/sdk/identity/ci.yml index c24cede395..2fcb520b3b 100644 --- a/sdk/identity/ci.yml +++ b/sdk/identity/ci.yml @@ -30,8 +30,6 @@ extends: LiveTestCtestRegex: azure-identity. LineCoverageTarget: 95 BranchCoverageTarget: 56 - # TODO: Revert before merge - UseFederatedAuth: true Artifacts: - Name: azure-identity Path: azure-identity diff --git a/sdk/identity/test-resources.json b/sdk/identity/test-resources.json index b71f8c95d4..b7d84ff613 100644 --- a/sdk/identity/test-resources.json +++ b/sdk/identity/test-resources.json @@ -14,6 +14,12 @@ "metadata": { "description": "The application client ID used to run tests." } + }, + "testApplicationSecret": { + "type": "string", + "metadata": { + "description": "The application client secret used to run tests." + } } }, "resources": [], @@ -25,6 +31,10 @@ "AZURE_CLIENT_ID": { "type": "string", "value": "[parameters('testApplicationId')]" + }, + "AZURE_CLIENT_SECRET": { + "type": "string", + "value": "[parameters('testApplicationSecret')]" } } } From d31ffb05c7b577570bf003472118d643ab2075fa Mon Sep 17 00:00:00 2001 From: Ahson Khan Date: Tue, 25 Jun 2024 21:09:39 -0700 Subject: [PATCH 22/23] Remove test application secret. --- sdk/identity/test-resources.json | 10 ---------- 1 file changed, 10 deletions(-) diff --git a/sdk/identity/test-resources.json b/sdk/identity/test-resources.json index b7d84ff613..b71f8c95d4 100644 --- a/sdk/identity/test-resources.json +++ b/sdk/identity/test-resources.json @@ -14,12 +14,6 @@ "metadata": { "description": "The application client ID used to run tests." } - }, - "testApplicationSecret": { - "type": "string", - "metadata": { - "description": "The application client secret used to run tests." - } } }, "resources": [], @@ -31,10 +25,6 @@ "AZURE_CLIENT_ID": { "type": "string", "value": "[parameters('testApplicationId')]" - }, - "AZURE_CLIENT_SECRET": { - "type": "string", - "value": "[parameters('testApplicationSecret')]" } } } From 547216220dfd63aac9d3f1077b336ed7d1c4dfd9 Mon Sep 17 00:00:00 2001 From: Ahson Khan Date: Tue, 25 Jun 2024 21:30:47 -0700 Subject: [PATCH 23/23] Revert other changes related to infra. --- eng/pipelines/templates/jobs/live.tests.yml | 163 +++++------------- .../templates/stages/archetype-sdk-client.yml | 16 +- .../templates/stages/archetype-sdk-tests.yml | 10 -- .../test/ut/token_credential_test.cpp | 39 ----- sdk/identity/test-resources.json | 10 ++ 5 files changed, 57 insertions(+), 181 deletions(-) diff --git a/eng/pipelines/templates/jobs/live.tests.yml b/eng/pipelines/templates/jobs/live.tests.yml index c38c5d14ba..a3c0f13f46 100644 --- a/eng/pipelines/templates/jobs/live.tests.yml +++ b/eng/pipelines/templates/jobs/live.tests.yml @@ -42,12 +42,6 @@ parameters: - name: OSName type: string default: '' -- name: EnvVars - type: object - default: {} -- name: UseFederatedAuth - type: boolean - default: false jobs: - job: @@ -160,17 +154,14 @@ jobs: parameters: SubscriptionConfiguration: ${{ parameters.CloudConfig.SubscriptionConfiguration }} SubscriptionConfigurations: ${{ parameters.CloudConfig.SubscriptionConfigurations }} - SubscriptionConfigurationFilePaths: ${{ parameters.CloudConfig.SubscriptionConfigurationFilePaths }} - EnvVars: ${{ parameters.EnvVars }} - template: /eng/common/TestResources/deploy-test-resources.yml parameters: ServiceDirectory: ${{ parameters.ServiceDirectory }} Location: ${{ coalesce(parameters.Location, parameters.CloudConfig.Location) }} SubscriptionConfiguration: $(SubscriptionConfiguration) - UseFederatedAuth: ${{ parameters.UseFederatedAuth }} - EnvVars: ${{ parameters.EnvVars }} - ServiceConnection: ${{ parameters.CloudConfig.ServiceConnection }} + EnvVars: + Pool: $(Pool) - template: /eng/common/testproxy/test-proxy-tool.yml parameters: @@ -178,50 +169,25 @@ jobs: - ${{ parameters.PreTestSteps }} - - ${{ if parameters.UseFederatedAuth }}: - - task: AzurePowerShell@5 - displayName: ctest - # Runs only if test-resources are happily deployed. - # unit-tests runs for those configs where samples are not ran. - # This enables to run tests and samples at the same time as different matrix configuration. - # Then unit-tests runs, samples should not run. - condition: and(succeeded(), ne(variables['RunSamples'], '1')) - inputs: - azureSubscription: ${{ parameters.CloudConfig.ServiceConnection }} - azurePowerShellVersion: LatestVersion - ScriptType: InlineScript - Inline: | - $account = (Get-AzContext).Account - $env:AZURESUBSCRIPTION_CLIENT_ID = $account.Id - $env:AZURESUBSCRIPTION_TENANT_ID = $account.Tenants - - ctest $(WindowsCtestConfig) -V --tests-regex "${{ parameters.CtestRegex }}" --no-compress-output -T Test - exit $LASTEXITCODE - workingDirectory: build - env: - SYSTEM_ACCESSTOKEN: $(System.AccessToken) - ${{ insert }}: ${{ parameters.EnvVars }} - - - ${{ else }}: - # For non multi-config generator use the same build configuration to run tests - # We don't need to set it to invoke ctest - # Visual Studio generator used in CI is a multi-config generator. - # As such, it requires the configuration argument for building and invoking ctest - - bash: | - export AZURE_CLIENT_ID=$(${{parameters.ServiceDirectory}}_CLIENT_ID) - export AZURE_TENANT_ID=$(${{parameters.ServiceDirectory}}_TENANT_ID) - export AZURE_CLIENT_SECRET=$(${{parameters.ServiceDirectory}}_CLIENT_SECRET) + # For non multi-config generator use the same build configuration to run tests + # We don't need to set it to invoke ctest + # Visual Studio generator used in CI is a multi-config generator. + # As such, it requires the configuration argument for building and invoking ctest + - bash: | + export AZURE_CLIENT_ID=$(${{parameters.ServiceDirectory}}_CLIENT_ID) + export AZURE_TENANT_ID=$(${{parameters.ServiceDirectory}}_TENANT_ID) + export AZURE_CLIENT_SECRET=$(${{parameters.ServiceDirectory}}_CLIENT_SECRET) - ctest $(WindowsCtestConfig) -V --tests-regex "${{ parameters.CtestRegex }}" --no-compress-output -T Test - workingDirectory: build - displayName: ctest - # Runs only if test-resources are happily deployed. - # unit-tests runs for those configs where samples are not ran. - # This enables to run tests and samples at the same time as different matrix configuration. - # Then unit-tests runs, samples should not run. - condition: and(succeeded(), ne(variables['RunSamples'], '1')) - env: - ${{ insert }}: ${{ parameters.EnvVars }} + ctest $(WindowsCtestConfig) -V --tests-regex "${{ parameters.CtestRegex }}" --no-compress-output -T Test + workingDirectory: build + displayName: ctest + # Runs only if test-resources are happily deployed. + # unit-tests runs for those configs where samples are not ran. + # This enables to run tests and samples at the same time as different matrix configuration. + # Then unit-tests runs, samples should not run. + condition: and( + succeeded(), + ne(variables['RunSamples'], '1')) - ${{ parameters.PostTestSteps }} @@ -237,66 +203,32 @@ jobs: # this step only makes sense when ctest has run condition: and(succeededOrFailed(), ne(variables['RunSamples'], '1')) - - - ${{ if parameters.UseFederatedAuth }}: - # Running Samples step. - # Will run samples described on a file name [service]-samples.txt within the build directory. - # For example keyvault-samples.txt. - # The file is written by CMake during configuration when building samples. - - bash: | - IFS=$'\n' - if [[ -f "./${{ parameters.ServiceDirectory }}-samples.txt" ]]; then - for sample in `cat ./${{ parameters.ServiceDirectory }}-samples.txt` - do - export AZURE_CLIENT_ID=$(${{parameters.ServiceDirectory}}_CLIENT_ID) - export AZURE_TENANT_ID=$(${{parameters.ServiceDirectory}}_TENANT_ID) - export AZURE_CLIENT_SECRET=$(${{parameters.ServiceDirectory}}_CLIENT_SECRET) - echo "**********Running sample: ${sample}" - bash -c "$sample" - status=$? - if [[ $status -eq 0 ]]; then - echo "*********Sample completed*********" - else - echo "*Sample returned a failed code: $status" - exit 1 - fi - done - fi - workingDirectory: build - displayName: "Run Samples for : ${{ parameters.ServiceDirectory }}" - condition: and(succeeded(), eq(variables['RunSamples'], '1')) - env: - ${{ insert }}: ${{ parameters.EnvVars }} - - - ${{ else }}: - - task: AzurePowerShell@5 - displayName: "Run Samples for : ${{ parameters.ServiceDirectory }}" - condition: and(succeeded(), eq(variables['RunSamples'], '1')) - inputs: - azureSubscription: ${{ parameters.CloudConfig.ServiceConnection }} - azurePowerShellVersion: LatestVersion - ScriptType: InlineScript - Inline: | - $account = (Get-AzContext).Account - $env:AZURESUBSCRIPTION_CLIENT_ID = $account.Id - $env:AZURESUBSCRIPTION_TENANT_ID = $account.Tenants - - if (Test-Path -Path "${{ parameters.ServiceDirectory }}-samples.txt") { - $samples = Get-Content "${{ parameters.ServiceDirectory }}-samples.txt" - foreach ($sample in $samples) { - Write-Host "**********Running sample: $sample" - & "$sample" - if ($LASTEXITCODE) { - Write-Host "Sample failed with exit code $LASTEXITCODE" - exit 1 - } - Write-Host "**********Sample completed" - } - } - workingDirectory: build - env: - SYSTEM_ACCESSTOKEN: $(System.AccessToken) - ${{ insert }}: ${{ parameters.EnvVars }} + # Running Samples step. + # Will run samples described on a file name [service]-samples.txt within the build directory. + # For example keyvault-samples.txt. + # The file is written by CMake during configuration when building samples. + - bash: | + IFS=$'\n' + if [[ -f "./${{ parameters.ServiceDirectory }}-samples.txt" ]]; then + for sample in `cat ./${{ parameters.ServiceDirectory }}-samples.txt` + do + export AZURE_CLIENT_ID=$(${{parameters.ServiceDirectory}}_CLIENT_ID) + export AZURE_TENANT_ID=$(${{parameters.ServiceDirectory}}_TENANT_ID) + export AZURE_CLIENT_SECRET=$(${{parameters.ServiceDirectory}}_CLIENT_SECRET) + echo "**********Running sample: ${sample}" + bash -c "$sample" + status=$? + if [[ $status -eq 0 ]]; then + echo "*********Sample completed*********" + else + echo "*Sample returned a failed code: $status" + exit 1 + fi + done + fi + workingDirectory: build + displayName: "Run Samples for : ${{ parameters.ServiceDirectory }}" + condition: and(succeeded(), eq(variables['RunSamples'], '1')) # Make coverage targets (specified in coverage_targets.txt) and assemble # coverage report @@ -318,6 +250,3 @@ jobs: parameters: ServiceDirectory: ${{ parameters.ServiceDirectory }} SubscriptionConfiguration: $(SubscriptionConfiguration) - UseFederatedAuth: ${{ parameters.UseFederatedAuth }} - EnvVars: ${{ parameters.EnvVars }} - ServiceConnection: ${{ parameters.CloudConfig.ServiceConnection }} diff --git a/eng/pipelines/templates/stages/archetype-sdk-client.yml b/eng/pipelines/templates/stages/archetype-sdk-client.yml index 56ea64296d..5d78403196 100644 --- a/eng/pipelines/templates/stages/archetype-sdk-client.yml +++ b/eng/pipelines/templates/stages/archetype-sdk-client.yml @@ -56,22 +56,15 @@ parameters: default: Public: SubscriptionConfiguration: $(sub-config-azure-cloud-test-resources) - ServiceConnection: azure-sdk-tests - SubscriptionConfigurationFilePaths: - - eng/common/TestResources/sub-config/AzurePublicMsft.json Preview: SubscriptionConfiguration: $(sub-config-azure-cloud-test-resources-preview) - ServiceConnection: azure-sdk-tests Canary: SubscriptionConfiguration: $(sub-config-azure-cloud-test-resources) - ServiceConnection: azure-sdk-tests Location: 'eastus2euap' UsGov: SubscriptionConfiguration: $(sub-config-gov-test-resources) - ServiceConnection: usgov_azure-sdk-tests China: SubscriptionConfiguration: $(sub-config-cn-test-resources) - ServiceConnection: china_azure-sdk-tests - name: Clouds type: string default: Public @@ -90,12 +83,6 @@ parameters: - name: CMakeGenerationTimeoutInMinutes type: number default: 120 - - name: EnvVars - type: object - default: {} - - name: UseFederatedAuth - type: boolean - default: false extends: ${{ if eq(variables['System.TeamProject'], 'internal') }}: @@ -203,8 +190,6 @@ extends: UnsupportedClouds: ${{ parameters.UnsupportedClouds }} PreTestSteps: ${{ parameters.PreTestSteps }} PostTestSteps: ${{ parameters.PostTestSteps }} - UseFederatedAuth: ${{ parameters.UseFederatedAuth }} - EnvVars: ${{ parameters.EnvVars }} - ${{ if and(eq(variables['System.TeamProject'], 'internal'), not(endsWith(variables['Build.DefinitionName'], ' - tests'))) }}: - template: archetype-cpp-release.yml@self @@ -226,3 +211,4 @@ extends: ArtifactName: packages ${{ if eq(parameters.ServiceDirectory, 'template') }}: TestPipeline: true + diff --git a/eng/pipelines/templates/stages/archetype-sdk-tests.yml b/eng/pipelines/templates/stages/archetype-sdk-tests.yml index 1df9ae070e..adbe4cada1 100644 --- a/eng/pipelines/templates/stages/archetype-sdk-tests.yml +++ b/eng/pipelines/templates/stages/archetype-sdk-tests.yml @@ -35,12 +35,6 @@ parameters: - name: PostTestSteps type: stepList default: [] -- name: EnvVars - type: object - default: {} -- name: UseFederatedAuth - type: boolean - default: false stages: - ${{ each cloud in parameters.CloudConfig }}: @@ -64,8 +58,6 @@ stages: SubscriptionConfigurations: ${{ cloud.value.SubscriptionConfigurations }} Location: ${{ coalesce(parameters.Location, cloud.value.Location) }} Cloud: ${{ cloud.key }} - SubscriptionConfigurationFilePaths: ${{ cloud.value.SubscriptionConfigurationFilePaths }} - ServiceConnection: ${{ cloud.value.ServiceConnection }} AdditionalParameters: Location: ${{ parameters.Location}} ServiceDirectory: ${{ parameters.ServiceDirectory}} @@ -75,5 +67,3 @@ stages: TimeoutInMinutes: ${{ parameters.TimeoutInMinutes}} PreTestSteps: ${{ parameters.PreTestSteps }} PostTestSteps: ${{ parameters.PostTestSteps }} - EnvVars: ${{ parameters.EnvVars }} - UseFederatedAuth: ${{ parameters.UseFederatedAuth }} diff --git a/sdk/identity/azure-identity/test/ut/token_credential_test.cpp b/sdk/identity/azure-identity/test/ut/token_credential_test.cpp index dabb104ef5..319486c253 100644 --- a/sdk/identity/azure-identity/test/ut/token_credential_test.cpp +++ b/sdk/identity/azure-identity/test/ut/token_credential_test.cpp @@ -53,39 +53,8 @@ namespace Azure { namespace Identity { namespace Test { using namespace Azure::Identity::Test; using namespace Azure::Identity; -static void OutputString(std::string enVarName, std::string envVarValue) -{ - std::cout << enVarName << ": " << envVarValue << std::endl; - if (!envVarValue.empty()) - { - if (envVarValue.size() > 6) - { - std::cout << "PARTIAL (first...last): " << envVarValue.size() << " | " << envVarValue.at(0) - << " . " << envVarValue.at(1) << " . " << envVarValue.at(2) - << envVarValue.at(envVarValue.size() - 3) << " . " - << envVarValue.at(envVarValue.size() - 2) << " . " - << envVarValue.at(envVarValue.size() - 1) << std::endl; - } - else - { - std::cout << "PARTIAL (first.last): " << envVarValue.size() << " | " << envVarValue.at(0) - << " . " << envVarValue.at(envVarValue.size() - 1) << std::endl; - } - } -} - -using Azure::Core::_internal::Environment; - TEST_F(TokenCredentialTest, ClientSecret) { - OutputString("AZURE_TENANT_ID", Environment::GetVariable("AZURE_TENANT_ID")); - OutputString("AZURE_CLIENT_ID", Environment::GetVariable("AZURE_CLIENT_ID")); - OutputString("AZURE_CLIENT_SECRET", Environment::GetVariable("AZURE_CLIENT_SECRET")); - OutputString("AZURE_SERVICE_DIRECTORY", Environment::GetVariable("AZURE_SERVICE_DIRECTORY")); - OutputString( - "AZURE_CLIENT_CERTIFICATE_PATH", Environment::GetVariable("AZURE_CLIENT_CERTIFICATE_PATH")); - OutputString("AZURE_AUTHORITY_HOST", Environment::GetVariable("AZURE_AUTHORITY_HOST")); - std::string const testName(GetTestName()); auto const clientSecretCredential = GetClientSecretCredential(testName); @@ -102,14 +71,6 @@ TEST_F(TokenCredentialTest, ClientSecret) TEST_F(TokenCredentialTest, EnvironmentCredential) { - OutputString("AZURE_TENANT_ID", Environment::GetVariable("AZURE_TENANT_ID")); - OutputString("AZURE_CLIENT_ID", Environment::GetVariable("AZURE_CLIENT_ID")); - OutputString("AZURE_CLIENT_SECRET", Environment::GetVariable("AZURE_CLIENT_SECRET")); - OutputString("AZURE_SERVICE_DIRECTORY", Environment::GetVariable("AZURE_SERVICE_DIRECTORY")); - OutputString( - "AZURE_CLIENT_CERTIFICATE_PATH", Environment::GetVariable("AZURE_CLIENT_CERTIFICATE_PATH")); - OutputString("AZURE_AUTHORITY_HOST", Environment::GetVariable("AZURE_AUTHORITY_HOST")); - std::string const testName(GetTestName()); auto const clientSecretCredential = GetEnvironmentCredential(testName); diff --git a/sdk/identity/test-resources.json b/sdk/identity/test-resources.json index b71f8c95d4..b7d84ff613 100644 --- a/sdk/identity/test-resources.json +++ b/sdk/identity/test-resources.json @@ -14,6 +14,12 @@ "metadata": { "description": "The application client ID used to run tests." } + }, + "testApplicationSecret": { + "type": "string", + "metadata": { + "description": "The application client secret used to run tests." + } } }, "resources": [], @@ -25,6 +31,10 @@ "AZURE_CLIENT_ID": { "type": "string", "value": "[parameters('testApplicationId')]" + }, + "AZURE_CLIENT_SECRET": { + "type": "string", + "value": "[parameters('testApplicationSecret')]" } } }