Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Undocument ChainedCred usage by DefaultAzCred & remove friend and private ctor #4447

Merged
merged 2 commits into from
Mar 16, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions sdk/identity/azure-identity/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,7 @@ set(

set(
AZURE_IDENTITY_SOURCE
src/private/chained_token_credential_impl.hpp
src/private/managed_identity_source.hpp
src/private/package_version.hpp
src/private/token_credential_impl.hpp
Expand Down
2 changes: 1 addition & 1 deletion sdk/identity/azure-identity/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ The `DefaultAzureCredential` attempts to authenticate via the following mechanis
1. **Azure CLI** - If the developer has authenticated an account via the Azure CLI `az login` command, the `DefaultAzureCredential` will authenticate with that account.
1. **Managed Identity** - If the application is deployed to an Azure host with Managed Identity enabled, the `DefaultAzureCredential` will authenticate with that account.

`DefaultAzureCredential` uses [`ChainedTokenCredential`](#chained-token-credential) that consists of a chain of `EnvironmentCredential`, `AzureCliCredential`, and `ManagedIdentityCredential`. Implementation, including the order in which credentials are applied is documented, but it may change from release to release.
Even though the credentials being used and their order is documented, it may change from release to release.

`DefaultAzureCredential` intends to provide a credential that "just works out of the box and without requiring any information", if only the environment is set up sufficiently for the credential to work.
Therefore, it could be simple to use, but since it uses a chain of credentials, it could be a bit complicated to diagnose if the environment setup is not sufficient.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,9 @@
#include <vector>

namespace Azure { namespace Identity {
class DefaultAzureCredential;
namespace _detail {
class ChainedTokenCredentialImpl;
}

/**
* @brief Chained Token Credential provides a token credential implementation which chains
Expand All @@ -24,10 +26,6 @@ namespace Azure { namespace Identity {
*
*/
class ChainedTokenCredential final : public Core::Credentials::TokenCredential {
// Friend declaration is needed for DefaultAzureCredential to access ChainedTokenCredential's
// private constructor built to be used specifically by it.
friend class DefaultAzureCredential;

public:
/**
* @brief A container type to store the ordered chain of credentials.
Expand Down Expand Up @@ -62,10 +60,7 @@ namespace Azure { namespace Identity {
Core::Context const& context) const override;

private:
explicit ChainedTokenCredential(Sources sources, std::string const& enclosingCredential);

Sources m_sources;
std::string m_logPrefix;
std::unique_ptr<_detail::ChainedTokenCredentialImpl> m_impl;
};

}} // namespace Azure::Identity
Original file line number Diff line number Diff line change
Expand Up @@ -8,21 +8,23 @@

#pragma once

#include <azure/core/credentials/credentials.hpp>
#include <azure/core/credentials/token_credential_options.hpp>

#include <azure/identity/chained_token_credential.hpp>

#include <memory>

namespace Azure { namespace Identity {
namespace _detail {
class ChainedTokenCredentialImpl;
}

/**
* @brief Default Azure Credential combines multiple credentials that depend on the setup
* environment and require no parameters into a single chain. If the environment is set up
* sufficiently for at least one of such credentials to work, `DefaultAzureCredential` will work
* as well.
*
* @details This credential is using the #ChainedTokenCredential of 3 credentials in the order:
* @details This credential is using several credentials in the following order:
* #EnvironmentCredential, #AzureCliCredential, and #ManagedIdentityCredential. Even though the
* credentials being used and their order is documented, it may be changed in the future versions
* of the SDK, potentially bringing breaking changes in its behavior.
Expand Down Expand Up @@ -68,7 +70,7 @@ namespace Azure { namespace Identity {
Core::Context const& context) const override;

private:
std::shared_ptr<ChainedTokenCredential> m_credentials;
std::unique_ptr<_detail::ChainedTokenCredentialImpl> m_impl;
};

}} // namespace Azure::Identity
64 changes: 33 additions & 31 deletions sdk/identity/azure-identity/src/chained_token_credential.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2,36 +2,42 @@
// 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 <utility>

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;

namespace {
constexpr auto IdentityPrefix = "Identity: ";
} // namespace

ChainedTokenCredential::ChainedTokenCredential(ChainedTokenCredential::Sources sources)
: ChainedTokenCredential(sources, {})
: TokenCredential("ChainedTokenCredential"),
m_impl(std::make_unique<ChainedTokenCredentialImpl>(GetCredentialName(), std::move(sources)))
{
}

ChainedTokenCredential::ChainedTokenCredential(
ChainedTokenCredential::Sources sources,
std::string const& enclosingCredential)
: TokenCredential("ChainedTokenCredential"), m_sources(std::move(sources))
ChainedTokenCredential::~ChainedTokenCredential() = default;

AccessToken ChainedTokenCredential::GetToken(
TokenRequestContext const& tokenRequestContext,
Context const& context) const
{
m_logPrefix = IdentityPrefix
+ (enclosingCredential.empty() ? GetCredentialName()
: (enclosingCredential + " -> " + GetCredentialName()))
+ ": ";
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))
{
Expand All @@ -54,19 +60,12 @@ ChainedTokenCredential::ChainedTokenCredential(
credSourceDetails += '.';
}

Log::Write(
logLevel,
IdentityPrefix
+ (enclosingCredential.empty()
? (GetCredentialName() + ": Created")
: (enclosingCredential + ": Created " + GetCredentialName()))
+ credSourceDetails);
Log::Write(logLevel, IdentityPrefix + credentialName + ": Created" + credSourceDetails);
}
}

ChainedTokenCredential::~ChainedTokenCredential() = default;

AccessToken ChainedTokenCredential::GetToken(
AccessToken ChainedTokenCredentialImpl::GetToken(
std::string const& credentialName,
TokenRequestContext const& tokenRequestContext,
Context const& context) const
{
Expand All @@ -78,7 +77,9 @@ AccessToken ChainedTokenCredential::GetToken(
if (Log::ShouldWrite(logLevel))
{
Log::Write(
logLevel, m_logPrefix + "Authentication did not succeed: List of sources is empty.");
logLevel,
IdentityPrefix + credentialName
+ ": Authentication did not succeed: List of sources is empty.");
}
}
else
Expand All @@ -95,8 +96,8 @@ AccessToken ChainedTokenCredential::GetToken(
{
Log::Write(
logLevel,
m_logPrefix + "Successfully got token from " + m_sources[i]->GetCredentialName()
+ '.');
IdentityPrefix + credentialName + ": Successfully got token from "
+ m_sources[i]->GetCredentialName() + '.');
}
}

Expand All @@ -110,8 +111,8 @@ AccessToken ChainedTokenCredential::GetToken(
{
Log::Write(
logLevel,
m_logPrefix + "Failed to get token from " + m_sources[i]->GetCredentialName() + ": "
+ e.what());
IdentityPrefix + credentialName + ": Failed to get token from "
+ m_sources[i]->GetCredentialName() + ": " + e.what());
}
}

Expand All @@ -122,12 +123,13 @@ AccessToken ChainedTokenCredential::GetToken(
{
Log::Write(
logLevel,
m_logPrefix + "Didn't succeed to get a token from any credential in the chain.");
IdentityPrefix + credentialName
+ ": Didn't succeed to get a token from any credential in the chain.");
}
}
}
}
}

throw AuthenticationException("Failed to get token from " + GetCredentialName() + '.');
throw AuthenticationException("Failed to get token from " + credentialName + '.');
}
13 changes: 6 additions & 7 deletions sdk/identity/azure-identity/src/default_azure_credential.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
#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"

Expand All @@ -31,8 +32,7 @@ DefaultAzureCredential::DefaultAzureCredential(TokenCredentialOptions const& opt
Log::Write(
logLevel,
std::string(IdentityPrefix) + "Creating " + GetCredentialName()
+ " which combines mutiple parameterless credentials "
"into a single one (by using ChainedTokenCredential).\n"
+ " 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."
Expand All @@ -48,10 +48,9 @@ DefaultAzureCredential::DefaultAzureCredential(TokenCredentialOptions const& opt
auto const azCliCred = std::make_shared<AzureCliCredential>(options);
auto const managedIdentityCred = std::make_shared<ManagedIdentityCredential>(options);

// Using the ChainedTokenCredential's private constructor for more detailed log messages.
m_credentials.reset(new ChainedTokenCredential(
ChainedTokenCredential::Sources{envCred, azCliCred, managedIdentityCred},
GetCredentialName())); // extra arg for the ChainedTokenCredential's private constructor.
m_impl = std::make_unique<_detail::ChainedTokenCredentialImpl>(
GetCredentialName(),
ChainedTokenCredential::Sources{envCred, azCliCred, managedIdentityCred});
}

DefaultAzureCredential::~DefaultAzureCredential() = default;
Expand All @@ -62,7 +61,7 @@ AccessToken DefaultAzureCredential::GetToken(
{
try
{
return m_credentials->GetToken(tokenRequestContext, context);
return m_impl->GetToken(GetCredentialName(), tokenRequestContext, context);
}
catch (AuthenticationException const&)
{
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
// Copyright (c) Microsoft Corporation. All rights reserved.
// SPDX-License-Identifier: MIT

#pragma once

#include "azure/identity/chained_token_credential.hpp"

namespace Azure { namespace Identity { namespace _detail {

class ChainedTokenCredentialImpl final {
public:
ChainedTokenCredentialImpl(
std::string const& credentialName,
ChainedTokenCredential::Sources&& sources);

Core::Credentials::AccessToken GetToken(
std::string const& credentialName,
Core::Credentials::TokenRequestContext const& tokenRequestContext,
Core::Context const& context) const;

private:
ChainedTokenCredential::Sources m_sources;
};

}}} // namespace Azure::Identity::_detail
Original file line number Diff line number Diff line change
Expand Up @@ -74,9 +74,8 @@ TEST(DefaultAzureCredential, LogMessages)
EXPECT_EQ(log[0].first, Logger::Level::Verbose);
EXPECT_EQ(
log[0].second,
"Identity: Creating DefaultAzureCredential which combines mutiple parameterless "
"credentials "
"into a single one (by using ChainedTokenCredential)."
"Identity: Creating DefaultAzureCredential which combines "
"mutiple parameterless credentials into a single one."
"\nDefaultAzureCredential 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 "
Expand Down Expand Up @@ -131,8 +130,7 @@ TEST(DefaultAzureCredential, LogMessages)
EXPECT_EQ(log[8].first, Logger::Level::Informational);
EXPECT_EQ(
log[8].second,
"Identity: DefaultAzureCredential: Created ChainedTokenCredential "
"with the following credentials: "
"Identity: DefaultAzureCredential: Created with the following credentials: "
"EnvironmentCredential, AzureCliCredential, ManagedIdentityCredential.");

log.clear();
Expand All @@ -149,8 +147,7 @@ TEST(DefaultAzureCredential, LogMessages)
EXPECT_EQ(log[3].first, Logger::Level::Informational);
EXPECT_EQ(
log[3].second,
"Identity: DefaultAzureCredential -> ChainedTokenCredential: "
"Successfully got token from EnvironmentCredential.");
"Identity: DefaultAzureCredential: Successfully got token from EnvironmentCredential.");

Logger::SetListener(nullptr);
}