From 2a39a3422b1ffaceea8ccf73dcc3e807ebf814b8 Mon Sep 17 00:00:00 2001 From: Anton Kolesnyk <41349689+antkmsft@users.noreply.github.com> Date: Wed, 22 Mar 2023 14:46:32 -0700 Subject: [PATCH] Organize applying Identity log prefix (#4459) * Organize applying Identity log prefix * logLevel * Cosmetic changes --------- Co-authored-by: Anton Kolesnyk --- sdk/identity/azure-identity/CMakeLists.txt | 1 + .../src/azure_cli_credential.cpp | 25 ++++------ .../src/chained_token_credential.cpp | 41 ++++++++-------- .../src/default_azure_credential.cpp | 16 +++--- .../src/environment_credential.cpp | 49 +++++++++---------- .../src/managed_identity_source.cpp | 30 +++++------- .../src/private/identity_log.hpp | 29 +++++++++++ 7 files changed, 101 insertions(+), 90 deletions(-) create mode 100644 sdk/identity/azure-identity/src/private/identity_log.hpp diff --git a/sdk/identity/azure-identity/CMakeLists.txt b/sdk/identity/azure-identity/CMakeLists.txt index 6d6e4ad7c..25169d9ef 100644 --- a/sdk/identity/azure-identity/CMakeLists.txt +++ b/sdk/identity/azure-identity/CMakeLists.txt @@ -63,6 +63,7 @@ set( set( AZURE_IDENTITY_SOURCE src/private/chained_token_credential_impl.hpp + src/private/identity_log.hpp src/private/managed_identity_source.hpp src/private/package_version.hpp src/private/token_credential_impl.hpp diff --git a/sdk/identity/azure-identity/src/azure_cli_credential.cpp b/sdk/identity/azure-identity/src/azure_cli_credential.cpp index ceb73e535..443088770 100644 --- a/sdk/identity/azure-identity/src/azure_cli_credential.cpp +++ b/sdk/identity/azure-identity/src/azure_cli_credential.cpp @@ -3,9 +3,9 @@ #include "azure/identity/azure_cli_credential.hpp" +#include "private/identity_log.hpp" #include "private/token_credential_impl.hpp" -#include #include #include #include @@ -45,16 +45,11 @@ using Azure::Core::Credentials::AccessToken; using Azure::Core::Credentials::AuthenticationException; using Azure::Core::Credentials::TokenCredentialOptions; using Azure::Core::Credentials::TokenRequestContext; -using Azure::Core::Diagnostics::Logger; -using Azure::Core::Diagnostics::_internal::Log; using Azure::Identity::AzureCliCredentialOptions; +using Azure::Identity::_detail::IdentityLog; using Azure::Identity::_detail::TokenCache; using Azure::Identity::_detail::TokenCredentialImpl; -namespace { -constexpr auto IdentityPrefix = "Identity: "; -} - void AzureCliCredential::ThrowIfNotSafeCmdLineInput( std::string const& input, std::string const& description) const @@ -75,8 +70,8 @@ void AzureCliCredential::ThrowIfNotSafeCmdLineInput( if (!StringExtensions::IsAlphaNumeric(c)) { throw AuthenticationException( - IdentityPrefix + GetCredentialName() + ": Unsafe command line input found in " - + description + ": " + input); + GetCredentialName() + ": Unsafe command line input found in " + description + ": " + + input); } } } @@ -92,9 +87,9 @@ AzureCliCredential::AzureCliCredential( ThrowIfNotSafeCmdLineInput(m_tenantId, "TenantID"); - Log::Write( - Logger::Level::Informational, - IdentityPrefix + GetCredentialName() + IdentityLog::Write( + IdentityLog::Level::Informational, + GetCredentialName() + " created.\n" "Successful creation does not guarantee further successful token retrieval."); } @@ -162,10 +157,8 @@ AccessToken AzureCliCredential::GetToken( } catch (std::exception const& e) { - auto const errorMsg - = IdentityPrefix + GetCredentialName() + " didn't get the token: \"" + e.what() + '\"'; - - Log::Write(Logger::Level::Warning, errorMsg); + auto const errorMsg = GetCredentialName() + " didn't get the token: \"" + e.what() + '\"'; + IdentityLog::Write(IdentityLog::Level::Warning, errorMsg); throw AuthenticationException(errorMsg); } }); diff --git a/sdk/identity/azure-identity/src/chained_token_credential.cpp b/sdk/identity/azure-identity/src/chained_token_credential.cpp index e30774e35..c7b441bac 100644 --- a/sdk/identity/azure-identity/src/chained_token_credential.cpp +++ b/sdk/identity/azure-identity/src/chained_token_credential.cpp @@ -2,8 +2,11 @@ // SPDX-License-Identifier: MIT #include "azure/identity/chained_token_credential.hpp" -#include "azure/core/internal/diagnostics/log.hpp" + #include "private/chained_token_credential_impl.hpp" +#include "private/identity_log.hpp" + +#include "azure/core/internal/diagnostics/log.hpp" #include @@ -11,8 +14,7 @@ using namespace Azure::Identity; using namespace Azure::Identity::_detail; using namespace Azure::Core::Credentials; using Azure::Core::Context; -using Azure::Core::Diagnostics::Logger; -using Azure::Core::Diagnostics::_internal::Log; +using Azure::Identity::_detail::IdentityLog; ChainedTokenCredential::ChainedTokenCredential(ChainedTokenCredential::Sources sources) : TokenCredential("ChainedTokenCredential"), @@ -29,17 +31,15 @@ AccessToken ChainedTokenCredential::GetToken( return m_impl->GetToken(GetCredentialName(), tokenRequestContext, context); } -namespace { -constexpr auto IdentityPrefix = "Identity: "; -} // namespace - ChainedTokenCredentialImpl::ChainedTokenCredentialImpl( std::string const& credentialName, ChainedTokenCredential::Sources&& sources) : m_sources(std::move(sources)) { - auto const logLevel = m_sources.empty() ? Logger::Level::Warning : Logger::Level::Informational; - if (Log::ShouldWrite(logLevel)) + auto const logLevel + = m_sources.empty() ? IdentityLog::Level::Warning : IdentityLog::Level::Informational; + + if (IdentityLog::ShouldWrite(logLevel)) { std::string credSourceDetails = " with EMPTY chain of credentials."; if (!m_sources.empty()) @@ -60,7 +60,7 @@ ChainedTokenCredentialImpl::ChainedTokenCredentialImpl( credSourceDetails += '.'; } - Log::Write(logLevel, IdentityPrefix + credentialName + ": Created" + credSourceDetails); + IdentityLog::Write(logLevel, credentialName + ": Created" + credSourceDetails); } } @@ -75,25 +75,24 @@ AccessToken ChainedTokenCredentialImpl::GetToken( { auto token = source->GetToken(tokenRequestContext, context); - Log::Write( - Logger::Level::Informational, - IdentityPrefix + credentialName + ": Successfully got token from " - + source->GetCredentialName() + '.'); + IdentityLog::Write( + IdentityLog::Level::Informational, + credentialName + ": Successfully got token from " + source->GetCredentialName() + '.'); return token; } catch (AuthenticationException const& e) { - Log::Write( - Logger::Level::Verbose, - IdentityPrefix + credentialName + ": Failed to get token from " - + source->GetCredentialName() + ": " + e.what()); + IdentityLog::Write( + IdentityLog::Level::Verbose, + credentialName + ": Failed to get token from " + source->GetCredentialName() + ": " + + e.what()); } } - Log::Write( - Logger::Level::Warning, - IdentityPrefix + credentialName + IdentityLog::Write( + IdentityLog::Level::Warning, + credentialName + (m_sources.empty() ? ": Authentication did not succeed: List of sources is empty." : ": Didn't succeed to get a token from any credential in the chain.")); diff --git a/sdk/identity/azure-identity/src/default_azure_credential.cpp b/sdk/identity/azure-identity/src/default_azure_credential.cpp index d50c2a6e7..79bebb997 100644 --- a/sdk/identity/azure-identity/src/default_azure_credential.cpp +++ b/sdk/identity/azure-identity/src/default_azure_credential.cpp @@ -6,20 +6,16 @@ #include "azure/identity/azure_cli_credential.hpp" #include "azure/identity/environment_credential.hpp" #include "azure/identity/managed_identity_credential.hpp" -#include "private/chained_token_credential_impl.hpp" -#include "azure/core/internal/diagnostics/log.hpp" +#include "private/chained_token_credential_impl.hpp" +#include "private/identity_log.hpp" using namespace Azure::Identity; using namespace Azure::Core::Credentials; using Azure::Core::Context; using Azure::Core::Diagnostics::Logger; -using Azure::Core::Diagnostics::_internal::Log; - -namespace { -constexpr auto IdentityPrefix = "Identity: "; -} // namespace +using Azure::Identity::_detail::IdentityLog; DefaultAzureCredential::DefaultAzureCredential(TokenCredentialOptions const& options) : TokenCredential("DefaultAzureCredential") @@ -27,9 +23,9 @@ DefaultAzureCredential::DefaultAzureCredential(TokenCredentialOptions const& opt // Initializing m_credential below and not in the member initializer list to have a specific order // of log messages. - Log::Write( - Logger::Level::Verbose, - std::string(IdentityPrefix) + "Creating " + GetCredentialName() + IdentityLog::Write( + IdentityLog::Level::Verbose, + "Creating " + GetCredentialName() + " which combines mutiple parameterless credentials into a single one.\n" + GetCredentialName() + " is only recommended for the early stages of development, " diff --git a/sdk/identity/azure-identity/src/environment_credential.cpp b/sdk/identity/azure-identity/src/environment_credential.cpp index 7d96727da..80dc948ff 100644 --- a/sdk/identity/azure-identity/src/environment_credential.cpp +++ b/sdk/identity/azure-identity/src/environment_credential.cpp @@ -5,8 +5,9 @@ #include "azure/identity/client_certificate_credential.hpp" #include "azure/identity/client_secret_credential.hpp" +#include "private/identity_log.hpp" + #include -#include #include #include @@ -20,8 +21,7 @@ using Azure::Core::Credentials::AccessToken; using Azure::Core::Credentials::AuthenticationException; using Azure::Core::Credentials::TokenCredentialOptions; using Azure::Core::Credentials::TokenRequestContext; -using Azure::Core::Diagnostics::Logger; -using Azure::Core::Diagnostics::_internal::Log; +using Azure::Identity::_detail::IdentityLog; namespace { constexpr auto AzureTenantIdEnvVarName = "AZURE_TENANT_ID"; @@ -30,8 +30,6 @@ constexpr auto AzureClientSecretEnvVarName = "AZURE_CLIENT_SECRET"; constexpr auto AzureAuthorityHostEnvVarName = "AZURE_AUTHORITY_HOST"; constexpr auto AzureClientCertificatePathEnvVarName = "AZURE_CLIENT_CERTIFICATE_PATH"; -constexpr auto IdentityPrefix = "Identity: "; - void PrintCredentialCreationLogMessage( std::string const& logMsgPrefix, std::vector> const& envVarsToParams, @@ -41,8 +39,6 @@ void PrintCredentialCreationLogMessage( EnvironmentCredential::EnvironmentCredential(TokenCredentialOptions options) : TokenCredential("EnvironmentCredential") { - auto const logMsgPrefix = IdentityPrefix + GetCredentialName(); - auto tenantId = Environment::GetVariable(AzureTenantIdEnvVarName); auto clientId = Environment::GetVariable(AzureClientIdEnvVarName); @@ -58,7 +54,7 @@ EnvironmentCredential::EnvironmentCredential(TokenCredentialOptions options) if (!authority.empty()) { PrintCredentialCreationLogMessage( - logMsgPrefix, + GetCredentialName(), { {AzureTenantIdEnvVarName, "tenantId"}, {AzureClientIdEnvVarName, "clientId"}, @@ -77,7 +73,7 @@ EnvironmentCredential::EnvironmentCredential(TokenCredentialOptions options) else { PrintCredentialCreationLogMessage( - logMsgPrefix, + GetCredentialName(), { {AzureTenantIdEnvVarName, "tenantId"}, {AzureClientIdEnvVarName, "clientId"}, @@ -94,7 +90,7 @@ EnvironmentCredential::EnvironmentCredential(TokenCredentialOptions options) if (!authority.empty()) { PrintCredentialCreationLogMessage( - logMsgPrefix, + GetCredentialName(), { {AzureTenantIdEnvVarName, "tenantId"}, {AzureClientIdEnvVarName, "clientId"}, @@ -113,7 +109,7 @@ EnvironmentCredential::EnvironmentCredential(TokenCredentialOptions options) else { PrintCredentialCreationLogMessage( - logMsgPrefix, + GetCredentialName(), { {AzureTenantIdEnvVarName, "tenantId"}, {AzureClientIdEnvVarName, "clientId"}, @@ -129,13 +125,14 @@ EnvironmentCredential::EnvironmentCredential(TokenCredentialOptions options) if (!m_credentialImpl) { - Log::Write( - Logger::Level::Warning, logMsgPrefix + " was not initialized with underlying credential."); + IdentityLog::Write( + IdentityLog::Level::Warning, + GetCredentialName() + " was not initialized with underlying credential."); - auto const logLevel = Logger::Level::Verbose; - if (Log::ShouldWrite(logLevel)) + auto const logLevel = IdentityLog::Level::Verbose; + if (IdentityLog::ShouldWrite(logLevel)) { - auto logMsg = logMsgPrefix + ": Both '" + AzureTenantIdEnvVarName + "' and '" + auto logMsg = GetCredentialName() + ": Both '" + AzureTenantIdEnvVarName + "' and '" + AzureClientIdEnvVarName + "', and at least one of '" + AzureClientSecretEnvVarName + "', '" + AzureClientCertificatePathEnvVarName + "' needs to be set. Additionally, '" + AzureAuthorityHostEnvVarName @@ -154,7 +151,7 @@ EnvironmentCredential::EnvironmentCredential(TokenCredentialOptions options) + "set\n"; } - Log::Write(logLevel, logMsg); + IdentityLog::Write(logLevel, logMsg); } } } @@ -165,11 +162,10 @@ AccessToken EnvironmentCredential::GetToken( { if (!m_credentialImpl) { - auto const AuthUnavailable - = IdentityPrefix + GetCredentialName() + " authentication unavailable. "; + auto const AuthUnavailable = GetCredentialName() + " authentication unavailable. "; - Log::Write( - Logger::Level::Warning, + IdentityLog::Write( + IdentityLog::Level::Warning, AuthUnavailable + "See earlier " + GetCredentialName() + " log messages for details."); throw AuthenticationException( @@ -185,11 +181,12 @@ void PrintCredentialCreationLogMessage( std::vector> const& envVarsToParams, char const* credThatGetsCreated) { - Log::Write( - Logger::Level::Informational, + IdentityLog::Write( + IdentityLog::Level::Informational, logMsgPrefix + " gets created with " + credThatGetsCreated + '.'); - if (!Log::ShouldWrite(Logger::Level::Verbose)) + auto const logLevel = IdentityLog::Level::Verbose; + if (!IdentityLog::ShouldWrite(logLevel)) { return; } @@ -217,8 +214,8 @@ void PrintCredentialCreationLogMessage( envVars += And + Tick + envVarsToParams.back().first + Tick; credParams += And + envVarsToParams.back().second; - Log::Write( - Logger::Level::Verbose, + IdentityLog::Write( + logLevel, logMsgPrefix + ": " + envVars + " environment variables are set, so " + credThatGetsCreated + " with corresponding " + credParams + " gets created."); } diff --git a/sdk/identity/azure-identity/src/managed_identity_source.cpp b/sdk/identity/azure-identity/src/managed_identity_source.cpp index daea912fe..e3001b9c6 100644 --- a/sdk/identity/azure-identity/src/managed_identity_source.cpp +++ b/sdk/identity/azure-identity/src/managed_identity_source.cpp @@ -3,9 +3,9 @@ #include "private/managed_identity_source.hpp" -#include +#include "private/identity_log.hpp" -#include +#include #include #include @@ -15,12 +15,9 @@ using namespace Azure::Identity::_detail; using Azure::Core::_internal::Environment; -using Azure::Core::Diagnostics::Logger; -using Azure::Core::Diagnostics::_internal::Log; +using Azure::Identity::_detail::IdentityLog; namespace { -constexpr auto IdentityPrefix = "Identity: "; - std::string WithSourceMessage(std::string const& credSource) { return " with " + credSource + " source"; @@ -28,9 +25,9 @@ std::string WithSourceMessage(std::string const& credSource) void PrintEnvNotSetUpMessage(std::string const& credName, std::string const& credSource) { - Log::Write( - Logger::Level::Verbose, - IdentityPrefix + credName + ": Environment is not set up for the credential to be created" + IdentityLog::Write( + IdentityLog::Level::Verbose, + credName + ": Environment is not set up for the credential to be created" + WithSourceMessage(credSource) + '.'); } } // namespace @@ -48,9 +45,9 @@ Azure::Core::Url ManagedIdentitySource::ParseEndpointUrl( { auto const endpointUrl = Url(url); - Log::Write( - Logger::Level::Informational, - IdentityPrefix + credName + " will be created" + WithSourceMessage(credSource) + '.'); + IdentityLog::Write( + IdentityLog::Level::Informational, + credName + " will be created" + WithSourceMessage(credSource) + '.'); return endpointUrl; } @@ -65,7 +62,7 @@ Azure::Core::Url ManagedIdentitySource::ParseEndpointUrl( + ": Failed to create: The environment variable \'" + envVarName + "\' contains an invalid URL."; - Log::Write(Logger::Level::Warning, IdentityPrefix + errorMessage); + IdentityLog::Write(IdentityLog::Level::Warning, errorMessage); throw AuthenticationException(errorMessage); } @@ -372,10 +369,9 @@ std::unique_ptr ImdsManagedIdentitySource::Create( std::string const& clientId, Azure::Core::Credentials::TokenCredentialOptions const& options) { - Log::Write( - Logger::Level::Informational, - IdentityPrefix + credName + " will be created" - + WithSourceMessage("Azure Instance Metadata Service") + IdentityLog::Write( + IdentityLog::Level::Informational, + credName + " will be created" + WithSourceMessage("Azure Instance Metadata Service") + ".\nSuccessful creation does not guarantee further successful token retrieval."); return std::unique_ptr(new ImdsManagedIdentitySource(clientId, options)); diff --git a/sdk/identity/azure-identity/src/private/identity_log.hpp b/sdk/identity/azure-identity/src/private/identity_log.hpp new file mode 100644 index 000000000..ee33661de --- /dev/null +++ b/sdk/identity/azure-identity/src/private/identity_log.hpp @@ -0,0 +1,29 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// SPDX-License-Identifier: MIT + +#pragma once + +#include + +namespace Azure { namespace Identity { namespace _detail { + + class IdentityLog final { + public: + using Level = Core::Diagnostics::Logger::Level; + + static void Write(Level level, std::string const& message) + { + Core::Diagnostics::_internal::Log::Write(level, "Identity: " + message); + } + + static bool ShouldWrite(Level level) + { + return Core::Diagnostics::_internal::Log::ShouldWrite(level); + } + + private: + IdentityLog() = delete; + ~IdentityLog() = delete; + }; + +}}} // namespace Azure::Identity::_detail