github/azure-sdk-for-cpp
1
0
Fork 0
Code Issues Pull Requests Actions 1 Packages Projects Releases Wiki Activity

Simpler identity logging (#4455)

Browse Source 
* Simpler identity logging

* Even simpler

* Remove refactoring artifact

* Cosmetic change

* foreach

---------

Co-authored-by: Anton Kolesnyk <antkmsft@users.noreply.github.com>
This commit is contained in:
Anton Kolesnyk 2023-03-16 12:50:15 -07:00 committed by GitHub
parent 14677e92be
commit 83f736d8ad
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
7 changed files with 158 additions and 198 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

21
sdk/identity/azure-identity/src/azure_cli_credential.cpp
View File

@ -91,15 +91,11 @@ AzureCliCredential::AzureCliCredential(
ThrowIfNotSafeCmdLineInput(m_tenantId, "TenantID");
auto const logLevel = Logger::Level::Informational;
if (Log::ShouldWrite(logLevel))
{
Log::Write(
logLevel,
IdentityPrefix + GetCredentialName()
+ " created.\n"
"Successful creation does not guarantee further successful token retrieval.");
}
Log::Write(
Logger::Level::Informational,
IdentityPrefix + GetCredentialName()
+ " created.\n"
"Successful creation does not guarantee further successful token retrieval.");
}
AzureCliCredential::AzureCliCredential(AzureCliCredentialOptions const& options)
@ -168,12 +164,7 @@ AccessToken AzureCliCredential::GetToken(
auto const errorMsg
= IdentityPrefix + GetCredentialName() + " didn't get the token: \"" + e.what() + '\"';
auto const logLevel = Logger::Level::Warning;
if (Log::ShouldWrite(logLevel))
{
Log::Write(logLevel, errorMsg);
}
Log::Write(Logger::Level::Warning, errorMsg);
throw AuthenticationException(errorMsg);
}
});

77
sdk/identity/azure-identity/src/chained_token_credential.cpp
View File

@ -69,67 +69,34 @@ AccessToken ChainedTokenCredentialImpl::GetToken(
TokenRequestContext const& tokenRequestContext,
Context const& context) const
{
auto const sourcesSize = m_sources.size();
if (sourcesSize == 0)
for (auto const& source : m_sources)
{
const auto logLevel = Logger::Level::Warning;
if (Log::ShouldWrite(logLevel))
try
{
auto token = source->GetToken(tokenRequestContext, context);
Log::Write(
Logger::Level::Informational,
IdentityPrefix + credentialName + ": Successfully got token from "
+ source->GetCredentialName() + '.');
return token;
}
catch (AuthenticationException const& e)
{
Log::Write(
logLevel,
IdentityPrefix + credentialName
+ ": Authentication did not succeed: List of sources is empty.");
Logger::Level::Verbose,
IdentityPrefix + credentialName + ": Failed to get token from "
+ source->GetCredentialName() + ": " + e.what());
}
}
else
{
for (size_t i = 0; i < sourcesSize; ++i)
{
try
{
auto token = m_sources[i]->GetToken(tokenRequestContext, context);
{
auto const logLevel = Logger::Level::Informational;
if (Log::ShouldWrite(logLevel))
{
Log::Write(
logLevel,
IdentityPrefix + credentialName + ": Successfully got token from "
+ m_sources[i]->GetCredentialName() + '.');
}
}
return token;
}
catch (AuthenticationException const& e)
{
{
auto const logLevel = Logger::Level::Verbose;
if (Log::ShouldWrite(logLevel))
{
Log::Write(
logLevel,
IdentityPrefix + credentialName + ": Failed to get token from "
+ m_sources[i]->GetCredentialName() + ": " + e.what());
}
}
if ((sourcesSize - 1) == i) // On the last credential
{
auto const logLevel = Logger::Level::Warning;
if (Log::ShouldWrite(logLevel))
{
Log::Write(
logLevel,
IdentityPrefix + credentialName
+ ": Didn't succeed to get a token from any credential in the chain.");
}
}
}
}
}
Log::Write(
Logger::Level::Warning,
IdentityPrefix + 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."));
throw AuthenticationException("Failed to get token from " + credentialName + '.');
}

29
sdk/identity/azure-identity/src/default_azure_credential.cpp
View File

@ -26,22 +26,19 @@ DefaultAzureCredential::DefaultAzureCredential(TokenCredentialOptions const& opt
{
// Initializing m_credential below and not in the member initializer list to have a specific order
// of log messages.
auto const logLevel = Logger::Level::Verbose;
if (Log::ShouldWrite(logLevel))
{
Log::Write(
logLevel,
std::string(IdentityPrefix) + "Creating " + GetCredentialName()
+ " which combines mutiple parameterless credentials into a single one.\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, "
+ GetCredentialName()
+ " needs to be replaced with the credential that "
"is the better fit for the application.");
}
Log::Write(
Logger::Level::Verbose,
std::string(IdentityPrefix) + "Creating " + GetCredentialName()
+ " which combines mutiple parameterless credentials into a single one.\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, "
+ GetCredentialName()
+ " needs to be replaced with the credential that "
"is the better fit for the application.");
// Creating credentials in order to ensure the order of log messages.
auto const envCred = std::make_shared<EnvironmentCredential>(options);

69
sdk/identity/azure-identity/src/environment_credential.cpp
View File

@ -129,38 +129,32 @@ EnvironmentCredential::EnvironmentCredential(TokenCredentialOptions options)
if (!m_credentialImpl)
{
auto const logLevel = Logger::Level::Warning;
Log::Write(
Logger::Level::Warning, logMsgPrefix + " was not initialized with underlying credential.");
auto const logLevel = Logger::Level::Verbose;
if (Log::ShouldWrite(logLevel))
{
auto const basicMessage = logMsgPrefix + " was not initialized with underlying credential";
auto logMsg = logMsgPrefix + ": Both '" + AzureTenantIdEnvVarName + "' and '"
+ AzureClientIdEnvVarName + "', and at least one of '" + AzureClientSecretEnvVarName
+ "', '" + AzureClientCertificatePathEnvVarName + "' needs to be set. Additionally, '"
+ AzureAuthorityHostEnvVarName
+ "' could be set to override the default authority host. Currently:\n";
if (!Log::ShouldWrite(Logger::Level::Verbose))
std::pair<char const*, bool> envVarStatus[] = {
{AzureTenantIdEnvVarName, !tenantId.empty()},
{AzureClientIdEnvVarName, !clientId.empty()},
{AzureClientSecretEnvVarName, !clientSecret.empty()},
{AzureClientCertificatePathEnvVarName, !clientCertificatePath.empty()},
{AzureAuthorityHostEnvVarName, !authority.empty()},
};
for (auto const& status : envVarStatus)
{
Log::Write(logLevel, basicMessage + '.');
logMsg += std::string(" * '") + status.first + "' " + "is" + (status.second ? " " : " NOT ")
+ "set\n";
}
else
{
auto logMsg = basicMessage + ": Both '" + AzureTenantIdEnvVarName + "' and '"
+ AzureClientIdEnvVarName + "', and at least one of '" + AzureClientSecretEnvVarName
+ "', '" + AzureClientCertificatePathEnvVarName + "' needs to be set. Additionally, '"
+ AzureAuthorityHostEnvVarName
+ "' could be set to override the default authority host. Currently:\n";
std::pair<char const*, bool> envVarStatus[] = {
{AzureTenantIdEnvVarName, !tenantId.empty()},
{AzureClientIdEnvVarName, !clientId.empty()},
{AzureClientSecretEnvVarName, !clientSecret.empty()},
{AzureClientCertificatePathEnvVarName, !clientCertificatePath.empty()},
{AzureAuthorityHostEnvVarName, !authority.empty()},
};
for (auto const& status : envVarStatus)
{
logMsg += std::string(" * '") + status.first + "' " + "is"
+ (status.second ? " " : " NOT ") + "set\n";
}
Log::Write(logLevel, logMsg);
}
Log::Write(logLevel, logMsg);
}
}
}
@ -174,15 +168,9 @@ AccessToken EnvironmentCredential::GetToken(
auto const AuthUnavailable
= IdentityPrefix + GetCredentialName() + " authentication unavailable. ";
{
auto const logLevel = Logger::Level::Warning;
if (Log::ShouldWrite(logLevel))
{
Log::Write(
logLevel,
AuthUnavailable + "See earlier " + GetCredentialName() + " log messages for details.");
}
}
Log::Write(
Logger::Level::Warning,
AuthUnavailable + "See earlier " + GetCredentialName() + " log messages for details.");
throw AuthenticationException(
AuthUnavailable + "Environment variables are not fully configured.");
@ -197,15 +185,12 @@ void PrintCredentialCreationLogMessage(
std::vector<std::pair<char const*, char const*>> const& envVarsToParams,
char const* credThatGetsCreated)
{
Log::Write(
Logger::Level::Informational,
logMsgPrefix + " gets created with " + credThatGetsCreated + '.');
if (!Log::ShouldWrite(Logger::Level::Verbose))
{
if (Log::ShouldWrite(Logger::Level::Informational))
{
Log::Write(
Logger::Level::Informational,
logMsgPrefix + " gets created with " + credThatGetsCreated + '.');
}
return;
}

43
sdk/identity/azure-identity/src/managed_identity_source.cpp
View File

@ -28,14 +28,10 @@ std::string WithSourceMessage(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 + credName + ": Environment is not set up for the credential to be created"
+ WithSourceMessage(credSource) + '.');
}
Log::Write(
Logger::Level::Verbose,
IdentityPrefix + credName + ": Environment is not set up for the credential to be created"
+ WithSourceMessage(credSource) + '.');
}
} // namespace
@ -52,13 +48,9 @@ Azure::Core::Url ManagedIdentitySource::ParseEndpointUrl(
{
auto const endpointUrl = Url(url);
auto const logLevel = Logger::Level::Informational;
if (Log::ShouldWrite(logLevel))
{
Log::Write(
logLevel,
IdentityPrefix + credName + " will be created" + WithSourceMessage(credSource) + '.');
}
Log::Write(
Logger::Level::Informational,
IdentityPrefix + credName + " will be created" + WithSourceMessage(credSource) + '.');
return endpointUrl;
}
@ -73,12 +65,7 @@ Azure::Core::Url ManagedIdentitySource::ParseEndpointUrl(
+ ": Failed to create: The environment variable \'" + envVarName
+ "\' contains an invalid URL.";
auto const logLevel = Logger::Level::Warning;
if (Log::ShouldWrite(logLevel))
{
Log::Write(logLevel, IdentityPrefix + errorMessage);
}
Log::Write(Logger::Level::Warning, IdentityPrefix + errorMessage);
throw AuthenticationException(errorMessage);
}
@ -385,15 +372,11 @@ std::unique_ptr<ManagedIdentitySource> ImdsManagedIdentitySource::Create(
std::string const& clientId,
Azure::Core::Credentials::TokenCredentialOptions const& options)
{
auto const logLevel = Logger::Level::Informational;
if (Log::ShouldWrite(logLevel))
{
Log::Write(
logLevel,
IdentityPrefix + credName + " will be created"
+ WithSourceMessage("Azure Instance Metadata Service")
+ ".\nSuccessful creation does not guarantee further successful token retrieval.");
}
Log::Write(
Logger::Level::Informational,
IdentityPrefix + credName + " will be created"
+ WithSourceMessage("Azure Instance Metadata Service")
+ ".\nSuccessful creation does not guarantee further successful token retrieval.");
return std::unique_ptr<ManagedIdentitySource>(new ImdsManagedIdentitySource(clientId, options));
}

41
sdk/identity/azure-identity/test/ut/default_azure_credential_test.cpp
View File

@ -69,7 +69,7 @@ TEST(DefaultAzureCredential, LogMessages)
auto credential = std::make_unique<DefaultAzureCredential>(options);
EXPECT_EQ(log.size(), LogMsgVec::size_type(9));
EXPECT_EQ(log.size(), LogMsgVec::size_type(10));
EXPECT_EQ(log[0].first, Logger::Level::Verbose);
EXPECT_EQ(
@ -82,54 +82,59 @@ TEST(DefaultAzureCredential, LogMessages)
"application, DefaultAzureCredential needs to be replaced with the credential that "
"is the better fit for the application.");
EXPECT_EQ(log[1].first, Logger::Level::Verbose);
EXPECT_EQ(log[1].first, Logger::Level::Informational);
EXPECT_EQ(
log[1].second,
"Identity: EnvironmentCredential gets created with ClientSecretCredential.");
EXPECT_EQ(log[2].first, Logger::Level::Verbose);
EXPECT_EQ(
log[2].second,
"Identity: EnvironmentCredential: 'AZURE_TENANT_ID', 'AZURE_CLIENT_ID', "
"'AZURE_CLIENT_SECRET', and 'AZURE_AUTHORITY_HOST' environment variables are set, so "
"ClientSecretCredential with corresponding tenantId, clientId, clientSecret, and "
"authorityHost gets created.");
EXPECT_EQ(log[2].first, Logger::Level::Informational);
EXPECT_EQ(
log[2].second,
"Identity: AzureCliCredential created."
"\nSuccessful creation does not guarantee further successful token retrieval.");
EXPECT_EQ(log[3].first, Logger::Level::Verbose);
EXPECT_EQ(log[3].first, Logger::Level::Informational);
EXPECT_EQ(
log[3].second,
"Identity: ManagedIdentityCredential: Environment is not set up for the credential "
"to be created with App Service 2019 source.");
"Identity: AzureCliCredential created."
"\nSuccessful creation does not guarantee further successful token retrieval.");
EXPECT_EQ(log[4].first, Logger::Level::Verbose);
EXPECT_EQ(
log[4].second,
"Identity: ManagedIdentityCredential: Environment is not set up for the credential "
"to be created with App Service 2017 source.");
"to be created with App Service 2019 source.");
EXPECT_EQ(log[5].first, Logger::Level::Verbose);
EXPECT_EQ(
log[5].second,
"Identity: ManagedIdentityCredential: Environment is not set up for the credential "
"to be created with Cloud Shell source.");
"to be created with App Service 2017 source.");
EXPECT_EQ(log[6].first, Logger::Level::Verbose);
EXPECT_EQ(
log[6].second,
"Identity: ManagedIdentityCredential: Environment is not set up for the credential "
"to be created with Azure Arc source.");
"to be created with Cloud Shell source.");
EXPECT_EQ(log[7].first, Logger::Level::Informational);
EXPECT_EQ(log[7].first, Logger::Level::Verbose);
EXPECT_EQ(
log[7].second,
"Identity: ManagedIdentityCredential will be created "
"with Azure Instance Metadata Service source."
"\nSuccessful creation does not guarantee further successful token retrieval.");
"Identity: ManagedIdentityCredential: Environment is not set up for the credential "
"to be created with Azure Arc source.");
EXPECT_EQ(log[8].first, Logger::Level::Informational);
EXPECT_EQ(
log[8].second,
"Identity: ManagedIdentityCredential will be created "
"with Azure Instance Metadata Service source."
"\nSuccessful creation does not guarantee further successful token retrieval.");
EXPECT_EQ(log[9].first, Logger::Level::Informational);
EXPECT_EQ(
log[9].second,
"Identity: DefaultAzureCredential: Created with the following credentials: "
"EnvironmentCredential, AzureCliCredential, ManagedIdentityCredential.");

76
sdk/identity/azure-identity/test/ut/environment_credential_test.cpp
View File

@ -53,14 +53,21 @@ TEST(EnvironmentCredential, RegularClientSecretCredential)
auto credential = std::make_unique<EnvironmentCredential>(options);
EXPECT_EQ(log.size(), LogMsgVec::size_type(1));
EXPECT_EQ(log[0].first, Logger::Level::Verbose);
EXPECT_EQ(log.size(), LogMsgVec::size_type(2));
EXPECT_EQ(log[0].first, Logger::Level::Informational);
EXPECT_EQ(
log[0].second,
"Identity: EnvironmentCredential gets created with ClientSecretCredential.");
EXPECT_EQ(log[1].first, Logger::Level::Verbose);
EXPECT_EQ(
log[1].second,
"Identity: EnvironmentCredential: 'AZURE_TENANT_ID', 'AZURE_CLIENT_ID', "
"'AZURE_CLIENT_SECRET', and 'AZURE_AUTHORITY_HOST' environment variables are set, so "
"ClientSecretCredential with corresponding tenantId, clientId, clientSecret, and "
"authorityHost gets created.");
log.clear();
return credential;
@ -202,20 +209,26 @@ TEST(EnvironmentCredential, Unavailable)
auto credential = std::make_unique<EnvironmentCredential>(options);
EXPECT_EQ(log.size(), LogMsgVec::size_type(1));
EXPECT_EQ(log.size(), LogMsgVec::size_type(2));
EXPECT_EQ(log[0].first, Logger::Level::Warning);
EXPECT_EQ(
log[0].second,
"Identity: EnvironmentCredential was not initialized with underlying credential: Both "
"'AZURE_TENANT_ID' and 'AZURE_CLIENT_ID', and at least one of 'AZURE_CLIENT_SECRET', "
"'AZURE_CLIENT_CERTIFICATE_PATH' needs to be set. Additionally, "
"'AZURE_AUTHORITY_HOST' could be set to override the default authority host."
" Currently:\n"
"Identity: EnvironmentCredential was not initialized with underlying credential.");
EXPECT_EQ(log[1].first, Logger::Level::Verbose);
EXPECT_EQ(
log[1].second,
"Identity: EnvironmentCredential: Both 'AZURE_TENANT_ID' and 'AZURE_CLIENT_ID',"
" and at least one of 'AZURE_CLIENT_SECRET', 'AZURE_CLIENT_CERTIFICATE_PATH'"
" needs to be set. Additionally, 'AZURE_AUTHORITY_HOST' could be set"
" to override the default authority host. Currently:\n"
" * 'AZURE_TENANT_ID' is NOT set\n"
" * 'AZURE_CLIENT_ID' is NOT set\n"
" * 'AZURE_CLIENT_SECRET' is NOT set\n"
" * 'AZURE_CLIENT_CERTIFICATE_PATH' is NOT set\n"
" * 'AZURE_AUTHORITY_HOST' is NOT set\n");
log.clear();
return credential;
@ -265,13 +278,20 @@ TEST(EnvironmentCredential, ClientSecretDefaultAuthority)
auto credential = std::make_unique<EnvironmentCredential>(options);
EXPECT_EQ(log.size(), LogMsgVec::size_type(1));
EXPECT_EQ(log[0].first, Logger::Level::Verbose);
EXPECT_EQ(log.size(), LogMsgVec::size_type(2));
EXPECT_EQ(log[0].first, Logger::Level::Informational);
EXPECT_EQ(
log[0].second,
"Identity: EnvironmentCredential gets created with ClientSecretCredential.");
EXPECT_EQ(log[1].first, Logger::Level::Verbose);
EXPECT_EQ(
log[1].second,
"Identity: EnvironmentCredential: 'AZURE_TENANT_ID', 'AZURE_CLIENT_ID', and "
"'AZURE_CLIENT_SECRET' environment variables are set, so ClientSecretCredential with "
"corresponding tenantId, clientId, and clientSecret gets created.");
log.clear();
return credential;
@ -342,20 +362,26 @@ TEST(EnvironmentCredential, ClientSecretNoTenantId)
auto credential = std::make_unique<EnvironmentCredential>(options);
EXPECT_EQ(log.size(), LogMsgVec::size_type(1));
EXPECT_EQ(log.size(), LogMsgVec::size_type(2));
EXPECT_EQ(log[0].first, Logger::Level::Warning);
EXPECT_EQ(
log[0].second,
"Identity: EnvironmentCredential was not initialized with underlying credential: Both "
"'AZURE_TENANT_ID' and 'AZURE_CLIENT_ID', and at least one of 'AZURE_CLIENT_SECRET', "
"'AZURE_CLIENT_CERTIFICATE_PATH' needs to be set. Additionally, "
"'AZURE_AUTHORITY_HOST' could be set to override the default authority host."
" Currently:\n"
"Identity: EnvironmentCredential was not initialized with underlying credential.");
EXPECT_EQ(log[1].first, Logger::Level::Verbose);
EXPECT_EQ(
log[1].second,
"Identity: EnvironmentCredential: Both 'AZURE_TENANT_ID' and 'AZURE_CLIENT_ID',"
" and at least one of 'AZURE_CLIENT_SECRET', 'AZURE_CLIENT_CERTIFICATE_PATH'"
" needs to be set. Additionally, 'AZURE_AUTHORITY_HOST' could be set"
" to override the default authority host. Currently:\n"
" * 'AZURE_TENANT_ID' is NOT set\n"
" * 'AZURE_CLIENT_ID' is set\n"
" * 'AZURE_CLIENT_SECRET' is set\n"
" * 'AZURE_CLIENT_CERTIFICATE_PATH' is NOT set\n"
" * 'AZURE_AUTHORITY_HOST' is set\n");
log.clear();
return credential;
@ -466,20 +492,26 @@ TEST(EnvironmentCredential, ClientSecretNoClientSecret)
auto credential = std::make_unique<EnvironmentCredential>(options);
EXPECT_EQ(log.size(), LogMsgVec::size_type(1));
EXPECT_EQ(log.size(), LogMsgVec::size_type(2));
EXPECT_EQ(log[0].first, Logger::Level::Warning);
EXPECT_EQ(
log[0].second,
"Identity: EnvironmentCredential was not initialized with underlying credential: Both "
"'AZURE_TENANT_ID' and 'AZURE_CLIENT_ID', and at least one of 'AZURE_CLIENT_SECRET', "
"'AZURE_CLIENT_CERTIFICATE_PATH' needs to be set. Additionally, "
"'AZURE_AUTHORITY_HOST' could be set to override the default authority host."
" Currently:\n"
"Identity: EnvironmentCredential was not initialized with underlying credential.");
EXPECT_EQ(log[1].first, Logger::Level::Verbose);
EXPECT_EQ(
log[1].second,
"Identity: EnvironmentCredential: Both 'AZURE_TENANT_ID' and 'AZURE_CLIENT_ID',"
" and at least one of 'AZURE_CLIENT_SECRET', 'AZURE_CLIENT_CERTIFICATE_PATH'"
" needs to be set. Additionally, 'AZURE_AUTHORITY_HOST' could be set"
" to override the default authority host. Currently:\n"
" * 'AZURE_TENANT_ID' is set\n"
" * 'AZURE_CLIENT_ID' is set\n"
" * 'AZURE_CLIENT_SECRET' is NOT set\n"
" * 'AZURE_CLIENT_CERTIFICATE_PATH' is NOT set\n"
" * 'AZURE_AUTHORITY_HOST' is set\n");
log.clear();
return credential;