diff --git a/sdk/core/azure-core-test/inc/azure/core/test/test_proxy_manager.hpp b/sdk/core/azure-core-test/inc/azure/core/test/test_proxy_manager.hpp index 6fae5bc81..f1097c9bb 100644 --- a/sdk/core/azure-core-test/inc/azure/core/test/test_proxy_manager.hpp +++ b/sdk/core/azure-core-test/inc/azure/core/test/test_proxy_manager.hpp @@ -31,6 +31,8 @@ namespace Azure { namespace Core { namespace Test { class TestNonExpiringCredential final : public Core::Credentials::TokenCredential { public: + TestNonExpiringCredential() : TokenCredential("TestNonExpiringCredential") {} + Core::Credentials::AccessToken GetToken( Core::Credentials::TokenRequestContext const& tokenRequestContext, Core::Context const& context) const override diff --git a/sdk/core/azure-core/CHANGELOG.md b/sdk/core/azure-core/CHANGELOG.md index 4ecb1b52a..cca5d146d 100644 --- a/sdk/core/azure-core/CHANGELOG.md +++ b/sdk/core/azure-core/CHANGELOG.md @@ -6,6 +6,7 @@ - Added the ability to ignore invalid certificate common name for TLS connections in WinHTTP transport. - Added `DisableTlsCertificateValidation` in `TransportOptions`. +- Added `TokenCredential::GetCredentialName()` to be utilized in diagnostic messages. If you have any custom implementations of `TokenCredential`, it is recommended to pass the name of your credential to `TokenCredential` constructor. The old parameterless constructor is deprecated. ### Breaking Changes diff --git a/sdk/core/azure-core/inc/azure/core/credentials/credentials.hpp b/sdk/core/azure-core/inc/azure/core/credentials/credentials.hpp index 27a9e62d3..710c192b9 100644 --- a/sdk/core/azure-core/inc/azure/core/credentials/credentials.hpp +++ b/sdk/core/azure-core/inc/azure/core/credentials/credentials.hpp @@ -60,6 +60,9 @@ namespace Azure { namespace Core { namespace Credentials { * @brief A base type of credential that uses Azure::Core::AccessToken to authenticate requests. */ class TokenCredential { + private: + std::string m_credentialName; + public: /** * @brief Gets an authentication token. @@ -75,6 +78,12 @@ namespace Azure { namespace Core { namespace Credentials { TokenRequestContext const& tokenRequestContext, Context const& context) const = 0; + /** + * @brief Gets the name of the credential. + * + */ + std::string const& GetCredentialName() const { return m_credentialName; } + /** * @brief Destructs `%TokenCredential`. * @@ -82,11 +91,25 @@ namespace Azure { namespace Core { namespace Credentials { virtual ~TokenCredential() = default; protected: + /** + * @brief Constructs an instance of `%TokenCredential`. + * + * @param credentialName Name of the credential for diagnostic messages. + */ + TokenCredential(std::string const& credentialName) + : m_credentialName(credentialName.empty() ? "Custom Credential" : credentialName) + { + } + /** * @brief Constructs a default instance of `%TokenCredential`. * + * @deprecated Use the constructor with parameter. */ - TokenCredential() {} + [[deprecated("Use the constructor with parameter.")]] TokenCredential() + : TokenCredential(std::string{}) + { + } private: /** diff --git a/sdk/core/azure-core/test/ut/bearer_token_authentication_policy_test.cpp b/sdk/core/azure-core/test/ut/bearer_token_authentication_policy_test.cpp index 48bacd8fd..c38c6aa19 100644 --- a/sdk/core/azure-core/test/ut/bearer_token_authentication_policy_test.cpp +++ b/sdk/core/azure-core/test/ut/bearer_token_authentication_policy_test.cpp @@ -16,7 +16,7 @@ private: public: explicit TestTokenCredential( std::shared_ptr accessToken) - : m_accessToken(accessToken) + : TokenCredential("TestTokenCredential"), m_accessToken(accessToken) { } diff --git a/sdk/identity/azure-identity/CHANGELOG.md b/sdk/identity/azure-identity/CHANGELOG.md index de58b0bcc..b8143d12d 100644 --- a/sdk/identity/azure-identity/CHANGELOG.md +++ b/sdk/identity/azure-identity/CHANGELOG.md @@ -10,6 +10,8 @@ ### Other Changes +- Improved diagnostics to utilize `Azure::Core::Credentials::TokenCredential::GetCredentialName()`. + ## 1.5.0-beta.1 (2023-03-07) ### Features Added diff --git a/sdk/identity/azure-identity/inc/azure/identity/azure_cli_credential.hpp b/sdk/identity/azure-identity/inc/azure/identity/azure_cli_credential.hpp index 946c2293a..53aec4f32 100644 --- a/sdk/identity/azure-identity/inc/azure/identity/azure_cli_credential.hpp +++ b/sdk/identity/azure-identity/inc/azure/identity/azure_cli_credential.hpp @@ -58,6 +58,8 @@ namespace Azure { namespace Identity { DateTime::duration cliProcessTimeout, Core::Credentials::TokenCredentialOptions const& options); + void ThrowIfNotSafeCmdLineInput(std::string const& input, std::string const& description) const; + public: /** * @brief Constructs an Azure CLI Credential. diff --git a/sdk/identity/azure-identity/inc/azure/identity/chained_token_credential.hpp b/sdk/identity/azure-identity/inc/azure/identity/chained_token_credential.hpp index 726735c2e..042942cb1 100644 --- a/sdk/identity/azure-identity/inc/azure/identity/chained_token_credential.hpp +++ b/sdk/identity/azure-identity/inc/azure/identity/chained_token_credential.hpp @@ -62,13 +62,9 @@ namespace Azure { namespace Identity { Core::Context const& context) const override; private: - explicit ChainedTokenCredential( - Sources sources, - std::string const& enclosingCredential, - std::vector sourcesFriendlyNames); + explicit ChainedTokenCredential(Sources sources, std::string const& enclosingCredential); Sources m_sources; - std::vector m_sourcesFriendlyNames; std::string m_logPrefix; }; diff --git a/sdk/identity/azure-identity/src/azure_cli_credential.cpp b/sdk/identity/azure-identity/src/azure_cli_credential.cpp index 83e3e4b05..f2c631adc 100644 --- a/sdk/identity/azure-identity/src/azure_cli_credential.cpp +++ b/sdk/identity/azure-identity/src/azure_cli_credential.cpp @@ -51,9 +51,12 @@ using Azure::Identity::_detail::TokenCache; using Azure::Identity::_detail::TokenCredentialImpl; namespace { -std::string const MsgPrefix = "Identity: AzureCliCredential"; +constexpr auto IdentityPrefix = "Identity: "; +} -void ThrowIfNotSafeCmdLineInput(std::string const& input, std::string const& description) +void AzureCliCredential::ThrowIfNotSafeCmdLineInput( + std::string const& input, + std::string const& description) const { for (auto const c : input) { @@ -71,20 +74,21 @@ void ThrowIfNotSafeCmdLineInput(std::string const& input, std::string const& des if (!std::isalnum(c, std::locale::classic())) { throw AuthenticationException( - MsgPrefix + ": Unsafe command line input found in " + description + ": " + input); + IdentityPrefix + GetCredentialName() + ": Unsafe command line input found in " + + description + ": " + input); } } } } -} // namespace - AzureCliCredential::AzureCliCredential( std::string tenantId, DateTime::duration cliProcessTimeout, Core::Credentials::TokenCredentialOptions const& options) - : m_tenantId(std::move(tenantId)), m_cliProcessTimeout(std::move(cliProcessTimeout)) + : TokenCredential("AzureCliCredential"), m_tenantId(std::move(tenantId)), + m_cliProcessTimeout(std::move(cliProcessTimeout)) { static_cast(options); + ThrowIfNotSafeCmdLineInput(m_tenantId, "TenantID"); auto const logLevel = Logger::Level::Informational; @@ -92,7 +96,7 @@ AzureCliCredential::AzureCliCredential( { Log::Write( logLevel, - MsgPrefix + IdentityPrefix + GetCredentialName() + " created.\n" "Successful creation does not guarantee further successful token retrieval."); } @@ -161,7 +165,8 @@ AccessToken AzureCliCredential::GetToken( } catch (std::exception const& e) { - auto const errorMsg = MsgPrefix + " didn't get the token: \"" + e.what() + '\"'; + auto const errorMsg + = IdentityPrefix + GetCredentialName() + " didn't get the token: \"" + e.what() + '\"'; auto const logLevel = Logger::Level::Warning; if (Log::ShouldWrite(logLevel)) diff --git a/sdk/identity/azure-identity/src/chained_token_credential.cpp b/sdk/identity/azure-identity/src/chained_token_credential.cpp index 3a63bea9c..116270b43 100644 --- a/sdk/identity/azure-identity/src/chained_token_credential.cpp +++ b/sdk/identity/azure-identity/src/chained_token_credential.cpp @@ -4,7 +4,6 @@ #include "azure/identity/chained_token_credential.hpp" #include "azure/core/internal/diagnostics/log.hpp" -#include #include @@ -15,63 +14,53 @@ using Azure::Core::Diagnostics::Logger; using Azure::Core::Diagnostics::_internal::Log; namespace { -std::string const IdentityPrefix = "Identity: "; -} +constexpr auto IdentityPrefix = "Identity: "; +} // namespace ChainedTokenCredential::ChainedTokenCredential(ChainedTokenCredential::Sources sources) - : ChainedTokenCredential(sources, {}, {}) + : ChainedTokenCredential(sources, {}) { } ChainedTokenCredential::ChainedTokenCredential( ChainedTokenCredential::Sources sources, - std::string const& enclosingCredential, - std::vector sourcesFriendlyNames) - : m_sources(std::move(sources)), m_sourcesFriendlyNames(std::move(sourcesFriendlyNames)) + std::string const& enclosingCredential) + : TokenCredential("ChainedTokenCredential"), m_sources(std::move(sources)) { - // LCOV_EXCL_START - AZURE_ASSERT(m_sourcesFriendlyNames.empty() || m_sourcesFriendlyNames.size() == m_sources.size()); - // LCOV_EXCL_STOP + m_logPrefix = IdentityPrefix + + (enclosingCredential.empty() ? GetCredentialName() + : (enclosingCredential + " -> " + GetCredentialName())) + + ": "; auto const logLevel = m_sources.empty() ? Logger::Level::Warning : Logger::Level::Informational; if (Log::ShouldWrite(logLevel)) { - std::string credentialsList; - if (!m_sourcesFriendlyNames.empty()) + std::string credSourceDetails = " with EMPTY chain of credentials."; + if (!m_sources.empty()) { - auto const sourceDescriptionsSize = m_sourcesFriendlyNames.size(); - for (size_t i = 0; i < (sourceDescriptionsSize - 1); ++i) + credSourceDetails = " with the following credentials: "; + + auto const sourcesSize = m_sources.size(); + for (size_t i = 0; i < sourcesSize; ++i) { - credentialsList += m_sourcesFriendlyNames[i] + ", "; + if (i != 0) + { + credSourceDetails += ", "; + } + + credSourceDetails += m_sources[i]->GetCredentialName(); } - credentialsList += m_sourcesFriendlyNames.back(); + credSourceDetails += '.'; } Log::Write( logLevel, IdentityPrefix + (enclosingCredential.empty() - ? "ChainedTokenCredential: Created" - : (enclosingCredential + ": Created ChainedTokenCredential")) - + " with " - + (m_sourcesFriendlyNames.empty() - ? (std::to_string(m_sources.size()) + " credentials.") - : (std::string("the following credentials: ") + credentialsList + '.'))); - } - - m_logPrefix = IdentityPrefix - + (enclosingCredential.empty() ? "ChainedTokenCredential" - : (enclosingCredential + " -> ChainedTokenCredential")) - + ": "; - - if (m_sourcesFriendlyNames.empty()) - { - auto const sourcesSize = m_sources.size(); - for (size_t i = 1; i <= sourcesSize; ++i) - { - m_sourcesFriendlyNames.push_back(std::string("credential #") + std::to_string(i)); - } + ? (GetCredentialName() + ": Created") + : (enclosingCredential + ": Created " + GetCredentialName())) + + credSourceDetails); } } @@ -106,7 +95,8 @@ AccessToken ChainedTokenCredential::GetToken( { Log::Write( logLevel, - m_logPrefix + "Successfully got token from " + m_sourcesFriendlyNames[i] + '.'); + m_logPrefix + "Successfully got token from " + m_sources[i]->GetCredentialName() + + '.'); } } @@ -120,7 +110,7 @@ AccessToken ChainedTokenCredential::GetToken( { Log::Write( logLevel, - m_logPrefix + "Failed to get token from " + m_sourcesFriendlyNames[i] + ": " + m_logPrefix + "Failed to get token from " + m_sources[i]->GetCredentialName() + ": " + e.what()); } } @@ -139,5 +129,5 @@ AccessToken ChainedTokenCredential::GetToken( } } - throw AuthenticationException("Failed to get token from ChainedTokenCredential."); + throw AuthenticationException("Failed to get token from " + GetCredentialName() + '.'); } diff --git a/sdk/identity/azure-identity/src/client_certificate_credential.cpp b/sdk/identity/azure-identity/src/client_certificate_credential.cpp index 7f41773fb..9ffd31fbd 100644 --- a/sdk/identity/azure-identity/src/client_certificate_credential.cpp +++ b/sdk/identity/azure-identity/src/client_certificate_credential.cpp @@ -80,7 +80,8 @@ ClientCertificateCredential::ClientCertificateCredential( std::string const& clientCertificatePath, std::string const& authorityHost, TokenCredentialOptions const& options) - : m_clientCredentialCore(tenantId, authorityHost), + : TokenCredential("ClientCertificateCredential"), + m_clientCredentialCore(tenantId, authorityHost), m_tokenCredentialImpl(std::make_unique(options)), m_requestBody( std::string( diff --git a/sdk/identity/azure-identity/src/client_secret_credential.cpp b/sdk/identity/azure-identity/src/client_secret_credential.cpp index 42fcedfcb..c0e8355f6 100644 --- a/sdk/identity/azure-identity/src/client_secret_credential.cpp +++ b/sdk/identity/azure-identity/src/client_secret_credential.cpp @@ -21,7 +21,7 @@ ClientSecretCredential::ClientSecretCredential( std::string const& clientSecret, std::string const& authorityHost, TokenCredentialOptions const& options) - : m_clientCredentialCore(tenantId, authorityHost), + : TokenCredential("ClientSecretCredential"), m_clientCredentialCore(tenantId, authorityHost), m_tokenCredentialImpl(std::make_unique(options)), m_requestBody( std::string("grant_type=client_credentials&client_id=") + Url::Encode(clientId) diff --git a/sdk/identity/azure-identity/src/default_azure_credential.cpp b/sdk/identity/azure-identity/src/default_azure_credential.cpp index 85d0eead9..3a77ebd10 100644 --- a/sdk/identity/azure-identity/src/default_azure_credential.cpp +++ b/sdk/identity/azure-identity/src/default_azure_credential.cpp @@ -17,10 +17,11 @@ using Azure::Core::Diagnostics::Logger; using Azure::Core::Diagnostics::_internal::Log; namespace { -std::string const IdentityPrefix = "Identity: "; -} +constexpr auto IdentityPrefix = "Identity: "; +} // namespace DefaultAzureCredential::DefaultAzureCredential(TokenCredentialOptions const& options) + : TokenCredential("DefaultAzureCredential") { // Initializing m_credential below and not in the member initializer list to have a specific order // of log messages. @@ -29,13 +30,16 @@ DefaultAzureCredential::DefaultAzureCredential(TokenCredentialOptions const& opt { Log::Write( logLevel, - IdentityPrefix - + "Creating DefaultAzureCredential which combines mutiple parameterless credentials " - "into a single one (by using ChainedTokenCredential)." - "\nDefaultAzureCredential is only recommended for the early stages of development, " + std::string(IdentityPrefix) + "Creating " + GetCredentialName() + + " which combines mutiple parameterless credentials " + "into a single one (by using ChainedTokenCredential).\n" + + GetCredentialName() + + " is only recommended for the early stages of development, " "and not for usage in production environment." - "\nOnce the developer focuses on the Credentials and Authentication aspects of their " - "application, DefaultAzureCredential needs to be replaced with the credential that " + "\nOnce the developer focuses on the Credentials and Authentication aspects " + "of their application, " + + GetCredentialName() + + " needs to be replaced with the credential that " "is the better fit for the application."); } @@ -47,9 +51,7 @@ DefaultAzureCredential::DefaultAzureCredential(TokenCredentialOptions const& opt // Using the ChainedTokenCredential's private constructor for more detailed log messages. m_credentials.reset(new ChainedTokenCredential( ChainedTokenCredential::Sources{envCred, azCliCred, managedIdentityCred}, - "DefaultAzureCredential", // extra args for the ChainedTokenCredential's private constructor. - std::vector{ - "EnvironmentCredential", "AzureCliCredential", "ManagedIdentityCredential"})); + GetCredentialName())); // extra arg for the ChainedTokenCredential's private constructor. } DefaultAzureCredential::~DefaultAzureCredential() = default; @@ -64,9 +66,10 @@ AccessToken DefaultAzureCredential::GetToken( } catch (AuthenticationException const&) { - throw AuthenticationException("Failed to get token from DefaultAzureCredential." - "\nSee Azure::Core::Diagnostics::Logger for details " - "(https://github.com/Azure/azure-sdk-for-cpp/tree/main/sdk/" - "identity/azure-identity#troubleshooting)."); + throw AuthenticationException( + "Failed to get token from " + GetCredentialName() + + ".\nSee Azure::Core::Diagnostics::Logger for details " + "(https://github.com/Azure/azure-sdk-for-cpp/tree/main/sdk/" + "identity/azure-identity#troubleshooting)."); } } diff --git a/sdk/identity/azure-identity/src/environment_credential.cpp b/sdk/identity/azure-identity/src/environment_credential.cpp index 2d6eb2479..f6dd76414 100644 --- a/sdk/identity/azure-identity/src/environment_credential.cpp +++ b/sdk/identity/azure-identity/src/environment_credential.cpp @@ -30,15 +30,19 @@ constexpr auto AzureClientSecretEnvVarName = "AZURE_CLIENT_SECRET"; constexpr auto AzureAuthorityHostEnvVarName = "AZURE_AUTHORITY_HOST"; constexpr auto AzureClientCertificatePathEnvVarName = "AZURE_CLIENT_CERTIFICATE_PATH"; -std::string const LogMsgPrefix = "Identity: EnvironmentCredential"; +constexpr auto IdentityPrefix = "Identity: "; void PrintCredentialCreationLogMessage( + std::string const& logMsgPrefix, std::vector> const& envVarsToParams, char const* credThatGetsCreated); } // namespace EnvironmentCredential::EnvironmentCredential(TokenCredentialOptions options) + : TokenCredential("EnvironmentCredential") { + auto const logMsgPrefix = IdentityPrefix + GetCredentialName(); + auto tenantId = Environment::GetVariable(AzureTenantIdEnvVarName); auto clientId = Environment::GetVariable(AzureClientIdEnvVarName); @@ -54,6 +58,7 @@ EnvironmentCredential::EnvironmentCredential(TokenCredentialOptions options) if (!authority.empty()) { PrintCredentialCreationLogMessage( + logMsgPrefix, { {AzureTenantIdEnvVarName, "tenantId"}, {AzureClientIdEnvVarName, "clientId"}, @@ -72,6 +77,7 @@ EnvironmentCredential::EnvironmentCredential(TokenCredentialOptions options) else { PrintCredentialCreationLogMessage( + logMsgPrefix, { {AzureTenantIdEnvVarName, "tenantId"}, {AzureClientIdEnvVarName, "clientId"}, @@ -88,6 +94,7 @@ EnvironmentCredential::EnvironmentCredential(TokenCredentialOptions options) if (!authority.empty()) { PrintCredentialCreationLogMessage( + logMsgPrefix, { {AzureTenantIdEnvVarName, "tenantId"}, {AzureClientIdEnvVarName, "clientId"}, @@ -106,6 +113,7 @@ EnvironmentCredential::EnvironmentCredential(TokenCredentialOptions options) else { PrintCredentialCreationLogMessage( + logMsgPrefix, { {AzureTenantIdEnvVarName, "tenantId"}, {AzureClientIdEnvVarName, "clientId"}, @@ -124,7 +132,7 @@ EnvironmentCredential::EnvironmentCredential(TokenCredentialOptions options) auto const logLevel = Logger::Level::Warning; if (Log::ShouldWrite(logLevel)) { - auto const basicMessage = LogMsgPrefix + " was not initialized with underlying credential"; + auto const basicMessage = logMsgPrefix + " was not initialized with underlying credential"; if (!Log::ShouldWrite(Logger::Level::Verbose)) { @@ -163,7 +171,8 @@ AccessToken EnvironmentCredential::GetToken( { if (!m_credentialImpl) { - auto const AuthUnavailable = LogMsgPrefix + " authentication unavailable. "; + auto const AuthUnavailable + = IdentityPrefix + GetCredentialName() + " authentication unavailable. "; { auto const logLevel = Logger::Level::Warning; @@ -171,7 +180,7 @@ AccessToken EnvironmentCredential::GetToken( { Log::Write( logLevel, - AuthUnavailable + "See earlier EnvironmentCredential log messages for details."); + AuthUnavailable + "See earlier " + GetCredentialName() + " log messages for details."); } } @@ -184,6 +193,7 @@ AccessToken EnvironmentCredential::GetToken( namespace { void PrintCredentialCreationLogMessage( + std::string const& logMsgPrefix, std::vector> const& envVarsToParams, char const* credThatGetsCreated) { @@ -193,7 +203,7 @@ void PrintCredentialCreationLogMessage( { Log::Write( Logger::Level::Informational, - LogMsgPrefix + " gets created with " + credThatGetsCreated + '.'); + logMsgPrefix + " gets created with " + credThatGetsCreated + '.'); } return; @@ -224,7 +234,7 @@ void PrintCredentialCreationLogMessage( Log::Write( Logger::Level::Verbose, - LogMsgPrefix + ": " + envVars + " environment variables are set, so " + credThatGetsCreated + logMsgPrefix + ": " + envVars + " environment variables are set, so " + credThatGetsCreated + " with corresponding " + credParams + " gets created."); } } // namespace diff --git a/sdk/identity/azure-identity/src/managed_identity_credential.cpp b/sdk/identity/azure-identity/src/managed_identity_credential.cpp index 209dfb8b4..7b64abcaf 100644 --- a/sdk/identity/azure-identity/src/managed_identity_credential.cpp +++ b/sdk/identity/azure-identity/src/managed_identity_credential.cpp @@ -8,13 +8,16 @@ using namespace Azure::Identity; namespace { std::unique_ptr<_detail::ManagedIdentitySource> CreateManagedIdentitySource( + std::string const& credentialName, std::string const& clientId, Azure::Core::Credentials::TokenCredentialOptions const& options) { using namespace Azure::Core::Credentials; using namespace Azure::Identity::_detail; static std::unique_ptr (*managedIdentitySourceCreate[])( - std::string const& clientId, TokenCredentialOptions const& options) + std::string const& credName, + std::string const& clientId, + TokenCredentialOptions const& options) = {AppServiceV2019ManagedIdentitySource::Create, AppServiceV2017ManagedIdentitySource::Create, CloudShellManagedIdentitySource::Create, @@ -25,7 +28,7 @@ std::unique_ptr<_detail::ManagedIdentitySource> CreateManagedIdentitySource( // For that reason, it is not possible to cover that execution branch in tests. for (auto create : managedIdentitySourceCreate) // LCOV_EXCL_LINE { - if (auto source = create(clientId, options)) + if (auto source = create(credentialName, clientId, options)) { return source; } @@ -33,7 +36,7 @@ std::unique_ptr<_detail::ManagedIdentitySource> CreateManagedIdentitySource( // LCOV_EXCL_START throw AuthenticationException( - "ManagedIdentityCredential authentication unavailable. No Managed Identity endpoint found."); + credentialName + " authentication unavailable. No Managed Identity endpoint found."); // LCOV_EXCL_STOP } } // namespace @@ -43,8 +46,9 @@ ManagedIdentityCredential::~ManagedIdentityCredential() = default; ManagedIdentityCredential::ManagedIdentityCredential( std::string const& clientId, Azure::Core::Credentials::TokenCredentialOptions const& options) - : m_managedIdentitySource(CreateManagedIdentitySource(clientId, options)) + : TokenCredential("ManagedIdentityCredential") { + m_managedIdentitySource = CreateManagedIdentitySource(GetCredentialName(), clientId, options); } ManagedIdentityCredential::ManagedIdentityCredential( diff --git a/sdk/identity/azure-identity/src/managed_identity_source.cpp b/sdk/identity/azure-identity/src/managed_identity_source.cpp index de7e8e6c3..89dfa0a77 100644 --- a/sdk/identity/azure-identity/src/managed_identity_source.cpp +++ b/sdk/identity/azure-identity/src/managed_identity_source.cpp @@ -19,28 +19,28 @@ using Azure::Core::Diagnostics::Logger; using Azure::Core::Diagnostics::_internal::Log; namespace { -std::string const IdentityPrefix = "Identity: "; -constexpr auto CredPrefix = "ManagedIdentityCredential"; +constexpr auto IdentityPrefix = "Identity: "; std::string WithSourceMessage(std::string const& credSource) { return " with " + credSource + " source"; } -void PrintEnvNotSetUpMessage(std::string const& credSource) +void PrintEnvNotSetUpMessage(std::string const& credName, std::string const& credSource) { auto const logLevel = Logger::Level::Verbose; if (Log::ShouldWrite(logLevel)) { Log::Write( logLevel, - IdentityPrefix + CredPrefix + ": Environment is not set up for the credential to be created" + IdentityPrefix + credName + ": Environment is not set up for the credential to be created" + WithSourceMessage(credSource) + '.'); } } } // namespace Azure::Core::Url ManagedIdentitySource::ParseEndpointUrl( + std::string const& credName, std::string const& url, char const* envVarName, std::string const& credSource) @@ -57,7 +57,7 @@ Azure::Core::Url ManagedIdentitySource::ParseEndpointUrl( { Log::Write( logLevel, - IdentityPrefix + CredPrefix + " will be created" + WithSourceMessage(credSource) + '.'); + IdentityPrefix + credName + " will be created" + WithSourceMessage(credSource) + '.'); } return endpointUrl; @@ -69,7 +69,7 @@ Azure::Core::Url ManagedIdentitySource::ParseEndpointUrl( { } - auto const errorMessage = CredPrefix + WithSourceMessage(credSource) + auto const errorMessage = credName + WithSourceMessage(credSource) + ": Failed to create: The environment variable \'" + envVarName + "\' contains an invalid URL."; @@ -84,6 +84,7 @@ Azure::Core::Url ManagedIdentitySource::ParseEndpointUrl( template std::unique_ptr AppServiceManagedIdentitySource::Create( + std::string const& credName, std::string const& clientId, Azure::Core::Credentials::TokenCredentialOptions const& options, char const* endpointVarName, @@ -98,10 +99,13 @@ std::unique_ptr AppServiceManagedIdentitySource::Create( if (!msiEndpoint.empty() && !msiSecret.empty()) { return std::unique_ptr(new T( - clientId, options, ParseEndpointUrl(msiEndpoint, endpointVarName, credSource), msiSecret)); + clientId, + options, + ParseEndpointUrl(credName, msiEndpoint, endpointVarName, credSource), + msiSecret)); } - PrintEnvNotSetUpMessage(credSource); + PrintEnvNotSetUpMessage(credName, credSource); return nullptr; } @@ -163,22 +167,25 @@ Azure::Core::Credentials::AccessToken AppServiceManagedIdentitySource::GetToken( } std::unique_ptr AppServiceV2017ManagedIdentitySource::Create( + std::string const& credName, std::string const& clientId, Core::Credentials::TokenCredentialOptions const& options) { return AppServiceManagedIdentitySource::Create( - clientId, options, "MSI_ENDPOINT", "MSI_SECRET", "2017"); + credName, clientId, options, "MSI_ENDPOINT", "MSI_SECRET", "2017"); } std::unique_ptr AppServiceV2019ManagedIdentitySource::Create( + std::string const& credName, std::string const& clientId, Core::Credentials::TokenCredentialOptions const& options) { return AppServiceManagedIdentitySource::Create( - clientId, options, "IDENTITY_ENDPOINT", "IDENTITY_HEADER", "2019"); + credName, clientId, options, "IDENTITY_ENDPOINT", "IDENTITY_HEADER", "2019"); } std::unique_ptr CloudShellManagedIdentitySource::Create( + std::string const& credName, std::string const& clientId, Azure::Core::Credentials::TokenCredentialOptions const& options) { @@ -190,10 +197,10 @@ std::unique_ptr CloudShellManagedIdentitySource::Create( if (!msiEndpoint.empty()) { return std::unique_ptr(new CloudShellManagedIdentitySource( - clientId, options, ParseEndpointUrl(msiEndpoint, EndpointVarName, CredSource))); + clientId, options, ParseEndpointUrl(credName, msiEndpoint, EndpointVarName, CredSource))); } - PrintEnvNotSetUpMessage(CredSource); + PrintEnvNotSetUpMessage(credName, CredSource); return nullptr; } @@ -252,6 +259,7 @@ Azure::Core::Credentials::AccessToken CloudShellManagedIdentitySource::GetToken( } std::unique_ptr AzureArcManagedIdentitySource::Create( + std::string const& credName, std::string const& clientId, Azure::Core::Credentials::TokenCredentialOptions const& options) { @@ -264,7 +272,7 @@ std::unique_ptr AzureArcManagedIdentitySource::Create( if (identityEndpoint.empty() || Environment::GetVariable("IMDS_ENDPOINT").empty()) { - PrintEnvNotSetUpMessage(credSource); + PrintEnvNotSetUpMessage(credName, credSource); return nullptr; } @@ -277,7 +285,7 @@ std::unique_ptr AzureArcManagedIdentitySource::Create( } return std::unique_ptr(new AzureArcManagedIdentitySource( - options, ParseEndpointUrl(identityEndpoint, EndpointVarName, credSource))); + options, ParseEndpointUrl(credName, identityEndpoint, EndpointVarName, credSource))); } AzureArcManagedIdentitySource::AzureArcManagedIdentitySource( @@ -373,6 +381,7 @@ Azure::Core::Credentials::AccessToken AzureArcManagedIdentitySource::GetToken( } std::unique_ptr ImdsManagedIdentitySource::Create( + std::string const& credName, std::string const& clientId, Azure::Core::Credentials::TokenCredentialOptions const& options) { @@ -381,7 +390,7 @@ std::unique_ptr ImdsManagedIdentitySource::Create( { Log::Write( logLevel, - IdentityPrefix + CredPrefix + " will be created" + IdentityPrefix + credName + " will be created" + WithSourceMessage("Azure Instance Metadata Service") + ".\nSuccessful creation does not guarantee further successful token retrieval."); } diff --git a/sdk/identity/azure-identity/src/private/managed_identity_source.hpp b/sdk/identity/azure-identity/src/private/managed_identity_source.hpp index 9972ce904..146da297b 100644 --- a/sdk/identity/azure-identity/src/private/managed_identity_source.hpp +++ b/sdk/identity/azure-identity/src/private/managed_identity_source.hpp @@ -30,6 +30,7 @@ namespace Azure { namespace Identity { namespace _detail { _detail::TokenCache m_tokenCache; static Core::Url ParseEndpointUrl( + std::string const& credName, std::string const& url, char const* envVarName, std::string const& credSource); @@ -63,6 +64,7 @@ namespace Azure { namespace Identity { namespace _detail { template static std::unique_ptr Create( + std::string const& credName, std::string const& clientId, Core::Credentials::TokenCredentialOptions const& options, char const* endpointVarName, @@ -97,6 +99,7 @@ namespace Azure { namespace Identity { namespace _detail { public: static std::unique_ptr Create( + std::string const& credName, std::string const& clientId, Core::Credentials::TokenCredentialOptions const& options); }; @@ -123,6 +126,7 @@ namespace Azure { namespace Identity { namespace _detail { public: static std::unique_ptr Create( + std::string const& credName, std::string const& clientId, Core::Credentials::TokenCredentialOptions const& options); }; @@ -139,6 +143,7 @@ namespace Azure { namespace Identity { namespace _detail { public: static std::unique_ptr Create( + std::string const& credName, std::string const& clientId, Core::Credentials::TokenCredentialOptions const& options); @@ -157,6 +162,7 @@ namespace Azure { namespace Identity { namespace _detail { public: static std::unique_ptr Create( + std::string const& credName, std::string const& clientId, Core::Credentials::TokenCredentialOptions const& options); @@ -175,6 +181,7 @@ namespace Azure { namespace Identity { namespace _detail { public: static std::unique_ptr Create( + std::string const& credName, std::string const& clientId, Core::Credentials::TokenCredentialOptions const& options); diff --git a/sdk/identity/azure-identity/test/ut/azure_cli_credential_test.cpp b/sdk/identity/azure-identity/test/ut/azure_cli_credential_test.cpp index 31b740494..5e143493c 100644 --- a/sdk/identity/azure-identity/test/ut/azure_cli_credential_test.cpp +++ b/sdk/identity/azure-identity/test/ut/azure_cli_credential_test.cpp @@ -158,6 +158,12 @@ TEST(AzureCliCredential, Error) Logger::SetListener(nullptr); } +TEST(AzureCliCredential, GetCredentialName) +{ + AzureCliTestCredential const cred(EmptyOutputCommand); + EXPECT_EQ(cred.GetCredentialName(), "AzureCliCredential"); +} + TEST(AzureCliCredential, EmptyOutput) { AzureCliTestCredential const azCliCred(EmptyOutputCommand); diff --git a/sdk/identity/azure-identity/test/ut/chained_token_credential_test.cpp b/sdk/identity/azure-identity/test/ut/chained_token_credential_test.cpp index fa8297e24..3b767572f 100644 --- a/sdk/identity/azure-identity/test/ut/chained_token_credential_test.cpp +++ b/sdk/identity/azure-identity/test/ut/chained_token_credential_test.cpp @@ -25,7 +25,7 @@ private: std::string m_token; public: - TestCredential(std::string token = "") : m_token(token) {} + TestCredential(std::string token = "") : TokenCredential("TestCredential"), m_token(token) {} mutable bool WasInvoked = false; @@ -45,6 +45,12 @@ public: }; } // namespace +TEST(ChainedTokenCredential, GetCredentialName) +{ + ChainedTokenCredential const cred(ChainedTokenCredential::Sources{}); + EXPECT_EQ(cred.GetCredentialName(), "ChainedTokenCredential"); +} + TEST(ChainedTokenCredential, Success) { auto c1 = std::make_shared("Token1"); @@ -110,7 +116,9 @@ TEST(ChainedTokenCredential, Logging) ChainedTokenCredential cred({}); EXPECT_EQ(log.size(), LogMsgVec::size_type(1)); EXPECT_EQ(log[0].first, Logger::Level::Warning); - EXPECT_EQ(log[0].second, "Identity: ChainedTokenCredential: Created with 0 credentials."); + EXPECT_EQ( + log[0].second, + "Identity: ChainedTokenCredential: Created with EMPTY chain of credentials."); log.clear(); EXPECT_THROW(static_cast(cred.GetToken({}, {})), AuthenticationException); @@ -128,7 +136,10 @@ TEST(ChainedTokenCredential, Logging) ChainedTokenCredential cred({c}); EXPECT_EQ(log.size(), LogMsgVec::size_type(1)); EXPECT_EQ(log[0].first, Logger::Level::Informational); - EXPECT_EQ(log[0].second, "Identity: ChainedTokenCredential: Created with 1 credentials."); + EXPECT_EQ( + log[0].second, + "Identity: ChainedTokenCredential: Created with the following credentials: " + "TestCredential."); log.clear(); EXPECT_FALSE(c->WasInvoked); @@ -141,7 +152,7 @@ TEST(ChainedTokenCredential, Logging) EXPECT_EQ(log[0].first, Logger::Level::Verbose); EXPECT_EQ( log[0].second, - "Identity: ChainedTokenCredential: Failed to get token from credential #1: " + "Identity: ChainedTokenCredential: Failed to get token from TestCredential: " "Test Error"); EXPECT_EQ(log[1].first, Logger::Level::Warning); @@ -158,7 +169,10 @@ TEST(ChainedTokenCredential, Logging) ChainedTokenCredential cred({c1, c2}); EXPECT_EQ(log.size(), LogMsgVec::size_type(1)); EXPECT_EQ(log[0].first, Logger::Level::Informational); - EXPECT_EQ(log[0].second, "Identity: ChainedTokenCredential: Created with 2 credentials."); + EXPECT_EQ( + log[0].second, + "Identity: ChainedTokenCredential: Created with the following credentials: " + "TestCredential, TestCredential."); log.clear(); EXPECT_FALSE(c1->WasInvoked); @@ -175,13 +189,13 @@ TEST(ChainedTokenCredential, Logging) EXPECT_EQ(log[0].first, Logger::Level::Verbose); EXPECT_EQ( log[0].second, - "Identity: ChainedTokenCredential: Failed to get token from credential #1: " + "Identity: ChainedTokenCredential: Failed to get token from TestCredential: " "Test Error"); EXPECT_EQ(log[1].first, Logger::Level::Informational); EXPECT_EQ( log[1].second, - "Identity: ChainedTokenCredential: Successfully got token from credential #2."); + "Identity: ChainedTokenCredential: Successfully got token from TestCredential."); } Logger::SetListener(nullptr); diff --git a/sdk/identity/azure-identity/test/ut/client_certificate_credential_test.cpp b/sdk/identity/azure-identity/test/ut/client_certificate_credential_test.cpp index b7ff3a3b4..f67963ab3 100644 --- a/sdk/identity/azure-identity/test/ut/client_certificate_credential_test.cpp +++ b/sdk/identity/azure-identity/test/ut/client_certificate_credential_test.cpp @@ -30,9 +30,20 @@ std::vector SplitString(const std::string& s, char separator); std::string ToString(std::vector const& vec); } // namespace +TEST(ClientCertificateCredential, GetCredentialName) +{ + TempCertFile const tempCertFile; + ClientCertificateCredential const cred( + "01234567-89ab-cdef-fedc-ba8976543210", + "fedcba98-7654-3210-0123-456789abcdef", + TempCertFile::Path); + + EXPECT_EQ(cred.GetCredentialName(), "ClientCertificateCredential"); +} + TEST(ClientCertificateCredential, Regular) { - TempCertFile tempCertFile; + TempCertFile const tempCertFile; auto const actual = CredentialTestHelper::SimulateTokenRequest( [](auto transport) { @@ -167,7 +178,7 @@ TEST(ClientCertificateCredential, Regular) TEST(ClientCertificateCredential, AzureStack) { - TempCertFile tempCertFile; + TempCertFile const tempCertFile; auto const actual = CredentialTestHelper::SimulateTokenRequest( [](auto transport) { @@ -294,7 +305,7 @@ TEST(ClientCertificateCredential, AzureStack) TEST(ClientCertificateCredential, Authority) { - TempCertFile tempCertFile; + TempCertFile const tempCertFile; auto const actual = CredentialTestHelper::SimulateTokenRequest( [](auto transport) { diff --git a/sdk/identity/azure-identity/test/ut/client_secret_credential_test.cpp b/sdk/identity/azure-identity/test/ut/client_secret_credential_test.cpp index 7625bc8e6..81d354bf1 100644 --- a/sdk/identity/azure-identity/test/ut/client_secret_credential_test.cpp +++ b/sdk/identity/azure-identity/test/ut/client_secret_credential_test.cpp @@ -12,6 +12,16 @@ using Azure::Identity::ClientSecretCredential; using Azure::Identity::ClientSecretCredentialOptions; using Azure::Identity::Test::_detail::CredentialTestHelper; +TEST(ClientSecretCredential, GetCredentialName) +{ + ClientSecretCredential const cred( + "01234567-89ab-cdef-fedc-ba8976543210", + "fedcba98-7654-3210-0123-456789abcdef", + "CLIENTSECRET"); + + EXPECT_EQ(cred.GetCredentialName(), "ClientSecretCredential"); +} + TEST(ClientSecretCredential, Regular) { auto const actual = CredentialTestHelper::SimulateTokenRequest( diff --git a/sdk/identity/azure-identity/test/ut/default_azure_credential_test.cpp b/sdk/identity/azure-identity/test/ut/default_azure_credential_test.cpp index 0b7826f7e..ba9a4afa5 100644 --- a/sdk/identity/azure-identity/test/ut/default_azure_credential_test.cpp +++ b/sdk/identity/azure-identity/test/ut/default_azure_credential_test.cpp @@ -17,6 +17,28 @@ using Azure::Identity::Test::_detail::CredentialTestHelper; namespace { } // namespace +TEST(DefaultAzureCredential, GetCredentialName) +{ + CredentialTestHelper::EnvironmentOverride const env({ + {"AZURE_TENANT_ID", "01234567-89ab-cdef-fedc-ba8976543210"}, + {"AZURE_CLIENT_ID", "fedcba98-7654-3210-0123-456789abcdef"}, + {"AZURE_CLIENT_SECRET", "CLIENTSECRET"}, + {"AZURE_AUTHORITY_HOST", ""}, + {"AZURE_USERNAME", ""}, + {"AZURE_PASSWORD", ""}, + {"AZURE_CLIENT_CERTIFICATE_PATH", ""}, + {"MSI_ENDPOINT", ""}, + {"MSI_SECRET", ""}, + {"IDENTITY_ENDPOINT", "https://visualstudio.com/"}, + {"IMDS_ENDPOINT", ""}, + {"IDENTITY_HEADER", "CLIENTSECRET"}, + {"IDENTITY_SERVER_THUMBPRINT", ""}, + }); + + DefaultAzureCredential const cred; + EXPECT_EQ(cred.GetCredentialName(), "DefaultAzureCredential"); +} + TEST(DefaultAzureCredential, LogMessages) { using LogMsgVec = std::vector>; diff --git a/sdk/identity/azure-identity/test/ut/environment_credential_test.cpp b/sdk/identity/azure-identity/test/ut/environment_credential_test.cpp index 5388b2189..61066e7c2 100644 --- a/sdk/identity/azure-identity/test/ut/environment_credential_test.cpp +++ b/sdk/identity/azure-identity/test/ut/environment_credential_test.cpp @@ -13,6 +13,22 @@ using Azure::Core::Http::HttpMethod; using Azure::Identity::EnvironmentCredential; using Azure::Identity::Test::_detail::CredentialTestHelper; +TEST(EnvironmentCredential, GetCredentialName) +{ + CredentialTestHelper::EnvironmentOverride const env({ + {"AZURE_TENANT_ID", "01234567-89ab-cdef-fedc-ba8976543210"}, + {"AZURE_CLIENT_ID", "fedcba98-7654-3210-0123-456789abcdef"}, + {"AZURE_CLIENT_SECRET", "CLIENTSECRET"}, + {"AZURE_AUTHORITY_HOST", ""}, + {"AZURE_USERNAME", ""}, + {"AZURE_PASSWORD", ""}, + {"AZURE_CLIENT_CERTIFICATE_PATH", ""}, + }); + + EnvironmentCredential const cred; + EXPECT_EQ(cred.GetCredentialName(), "EnvironmentCredential"); +} + TEST(EnvironmentCredential, RegularClientSecretCredential) { using Azure::Core::Diagnostics::Logger; diff --git a/sdk/identity/azure-identity/test/ut/managed_identity_credential_test.cpp b/sdk/identity/azure-identity/test/ut/managed_identity_credential_test.cpp index 2923f9134..31deb38bb 100644 --- a/sdk/identity/azure-identity/test/ut/managed_identity_credential_test.cpp +++ b/sdk/identity/azure-identity/test/ut/managed_identity_credential_test.cpp @@ -16,6 +16,21 @@ using Azure::Core::Http::HttpStatusCode; using Azure::Identity::ManagedIdentityCredential; using Azure::Identity::Test::_detail::CredentialTestHelper; +TEST(ManagedIdentityCredential, GetCredentialName) +{ + CredentialTestHelper::EnvironmentOverride const env({ + {"MSI_ENDPOINT", ""}, + {"MSI_SECRET", ""}, + {"IDENTITY_ENDPOINT", "https://visualstudio.com/"}, + {"IMDS_ENDPOINT", ""}, + {"IDENTITY_HEADER", "CLIENTSECRET"}, + {"IDENTITY_SERVER_THUMBPRINT", ""}, + }); + + ManagedIdentityCredential const cred; + EXPECT_EQ(cred.GetCredentialName(), "ManagedIdentityCredential"); +} + TEST(ManagedIdentityCredential, AppServiceV2019) { using Azure::Core::Diagnostics::Logger; diff --git a/sdk/identity/azure-identity/test/ut/token_credential_impl_test.cpp b/sdk/identity/azure-identity/test/ut/token_credential_impl_test.cpp index 6189fb98e..567bba73a 100644 --- a/sdk/identity/azure-identity/test/ut/token_credential_impl_test.cpp +++ b/sdk/identity/azure-identity/test/ut/token_credential_impl_test.cpp @@ -34,15 +34,16 @@ public: HttpMethod httpMethod, Url url, TokenCredentialOptions const& options) - : m_httpMethod(std::move(httpMethod)), m_url(std::move(url)), - m_tokenCredentialImpl(new TokenCredentialImpl(options)) + : TokenCredential("TokenCredentialImplTester"), m_httpMethod(std::move(httpMethod)), + m_url(std::move(url)), m_tokenCredentialImpl(new TokenCredentialImpl(options)) { } explicit TokenCredentialImplTester( std::function throwingFunction, TokenCredentialOptions const& options) - : m_throwingFunction(std::move(throwingFunction)), + : TokenCredential("TokenCredentialImplTester"), + m_throwingFunction(std::move(throwingFunction)), m_tokenCredentialImpl(new TokenCredentialImpl(options)) { } @@ -63,8 +64,41 @@ public: }); } }; + +// Disable deprecation warning +#if defined(_MSC_VER) +#pragma warning(push) +#pragma warning(disable : 4996) +#elif defined(__clang__) +#pragma clang diagnostic push +#pragma clang diagnostic ignored "-Wdeprecated-declarations" +#elif defined(__GNUC__) +#pragma GCC diagnostic push +#pragma GCC diagnostic ignored "-Wdeprecated-declarations" +#endif +// This credential is needed to test the default behavior when the customer has custom credential +// they implemented while using earlier versions of the SDK which didn't have a constructor with +// credentialName. +class CustomTokenCredential : public TokenCredential { +public: + AccessToken GetToken(TokenRequestContext const&, Context const&) const override { return {}; } +}; +#if defined(_MSC_VER) +#pragma warning(pop) +#elif defined(__clang__) +#pragma clang diagnostic pop +#elif defined(__GNUC__) +#pragma GCC diagnostic pop +#endif // _MSC_VER + } // namespace +TEST(CustomTokenCredential, GetCredentialName) +{ + CustomTokenCredential cred; + EXPECT_EQ(cred.GetCredentialName(), "Custom Credential"); +} + TEST(TokenCredentialImpl, Normal) { auto const actual = CredentialTestHelper::SimulateTokenRequest(