Skip to content
Merged
Show file tree
Hide file tree
Changes from 5 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
2 changes: 2 additions & 0 deletions sdk/identity/azure-identity/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@

### Features Added

- Added `ClientAssertionCredential` to enable applications to authenticate with custom client assertions.

### Breaking Changes

### Bugs Fixed
Expand Down
2 changes: 2 additions & 0 deletions sdk/identity/azure-identity/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@ set(
inc/azure/identity/azure_cli_credential.hpp
inc/azure/identity/azure_pipelines_credential.hpp
inc/azure/identity/chained_token_credential.hpp
inc/azure/identity/client_assertion_credential.hpp
inc/azure/identity/client_certificate_credential.hpp
inc/azure/identity/client_secret_credential.hpp
inc/azure/identity/default_azure_credential.hpp
Expand All @@ -67,6 +68,7 @@ set(
src/azure_cli_credential.cpp
src/azure_pipelines_credential.cpp
src/chained_token_credential.cpp
src/client_assertion_credential.cpp
src/client_certificate_credential.cpp
src/client_credential_core.cpp
src/client_secret_credential.cpp
Expand Down
1 change: 1 addition & 0 deletions sdk/identity/azure-identity/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -135,6 +135,7 @@ Configuration is attempted in the above order. For example, if values for a clie
|Credential | Usage
|-|-
|`AzurePipelinesCredential`|Supports [Microsoft Entra Workload ID](https://learn.microsoft.com/azure/devops/pipelines/release/configure-workload-identity?view=azure-devops) on Azure Pipelines.
|`ClientAssertionCredential`|Authenticates a service principal using a signed client assertion.
|`ClientSecretCredential`|Authenticates a service principal [using a secret](https://learn.microsoft.com/entra/identity-platform/app-objects-and-service-principals).
|`ClientCertificateCredential`|Authenticates a service principal [using a certificate](https://learn.microsoft.com/entra/identity-platform/app-objects-and-service-principals).

Expand Down
1 change: 1 addition & 0 deletions sdk/identity/azure-identity/inc/azure/identity.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
#include "azure/identity/azure_cli_credential.hpp"
#include "azure/identity/azure_pipelines_credential.hpp"
#include "azure/identity/chained_token_credential.hpp"
#include "azure/identity/client_assertion_credential.hpp"
#include "azure/identity/client_certificate_credential.hpp"
#include "azure/identity/client_secret_credential.hpp"
#include "azure/identity/default_azure_credential.hpp"
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,103 @@
// Copyright (c) Microsoft Corporation.
// Licensed under the MIT License.

/**
* @file
* @brief Client Assertion Credential and options.
*/

#pragma once

#include "azure/identity/detail/client_credential_core.hpp"
#include "azure/identity/detail/token_cache.hpp"

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

#include <string>
#include <vector>

namespace Azure { namespace Identity {
namespace _detail {
class TokenCredentialImpl;
} // namespace _detail

/**
* @brief Options used to configure the Client Assertion credential.
*
*/
struct ClientAssertionCredentialOptions final : public Core::Credentials::TokenCredentialOptions
{
/**
* @brief Authentication authority URL.
* @note Defaults to the value of the environment variable 'AZURE_AUTHORITY_HOST'. If that's not
* set, the default value is Microsoft Entra global authority
* (https://login.microsoftonline.com/).
*
* @note Example of an authority host string: "https://login.microsoftonline.us/". See national
* clouds' Microsoft Entra authentication endpoints:
* https://learn.microsoft.com/entra/identity-platform/authentication-national-cloud.
*/
std::string AuthorityHost = _detail::DefaultOptionValues::GetAuthorityHost();

/**
* @brief For multi-tenant applications, specifies additional tenants for which the credential
* may acquire tokens. Add the wildcard value `"*"` to allow the credential to acquire tokens
* for any tenant in which the application is installed.
*/
std::vector<std::string> AdditionallyAllowedTenants;
};

/**
* @brief Credential which authenticates a Microsoft Entra service principal using a signed client
* assertion.
*
*/
class ClientAssertionCredential final : public Core::Credentials::TokenCredential {
private:
std::function<std::string(Core::Context const&)> m_assertionCallback;
_detail::ClientCredentialCore m_clientCredentialCore;
std::unique_ptr<_detail::TokenCredentialImpl> m_tokenCredentialImpl;
std::string m_requestBody;
_detail::TokenCache m_tokenCache;

public:
/**
* @brief Creates an instance of the Client Assertion Credential with a callback that provides a
* signed client assertion to authenticate against Microsoft Entra ID.
*
* @param tenantId The Microsoft Entra tenant (directory) ID of the service principal.
* @param clientId The client (application) ID of the service principal.
* @param assertionCallback A callback returning a valid client assertion used to authenticate
* the service principal.
* @param options Options that allow to configure the management of the requests sent to
* Microsoft Entra ID for token retrieval.
*/
explicit ClientAssertionCredential(
std::string tenantId,
std::string clientId,
std::function<std::string(Core::Context const&)> const& assertionCallback,
ClientAssertionCredentialOptions const& options = {});

/**
* @brief Destructs `%ClientAssertionCredential`.
*
*/
~ClientAssertionCredential() override;

/**
* @brief Obtains an authentication token from Microsoft Entra ID, by calling the
* assertionCallback specified when constructing the credential to obtain a client assertion for
* authentication.
*
* @param tokenRequestContext A context to get the token in.
* @param context A context to control the request lifetime.
*
* @throw Azure::Core::Credentials::AuthenticationException Authentication error occurred.
*/
Core::Credentials::AccessToken GetToken(
Core::Credentials::TokenRequestContext const& tokenRequestContext,
Core::Context const& context) const override;
};

}} // namespace Azure::Identity
160 changes: 160 additions & 0 deletions sdk/identity/azure-identity/src/client_assertion_credential.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,160 @@
// Copyright (c) Microsoft Corporation.
// Licensed under the MIT License.

#include "azure/identity/client_assertion_credential.hpp"

#include "private/identity_log.hpp"
#include "private/package_version.hpp"
#include "private/tenant_id_resolver.hpp"
#include "private/token_credential_impl.hpp"

#include <azure/core/internal/json/json.hpp>

using Azure::Identity::ClientAssertionCredential;
using Azure::Identity::ClientAssertionCredentialOptions;

using Azure::Core::Context;
using Azure::Core::Url;
using Azure::Core::_internal::StringExtensions;
using Azure::Core::Credentials::AccessToken;
using Azure::Core::Credentials::AuthenticationException;
using Azure::Core::Credentials::TokenRequestContext;
using Azure::Core::Http::HttpMethod;
using Azure::Identity::_detail::IdentityLog;
using Azure::Identity::_detail::TenantIdResolver;
using Azure::Identity::_detail::TokenCredentialImpl;

namespace {
bool IsValidTenantId(std::string const& tenantId)
Comment thread
ahsonkhan marked this conversation as resolved.
{
const std::string allowedChars = ".-";
if (tenantId.empty())
{
return false;
}
for (auto const c : tenantId)
{
if (allowedChars.find(c) != std::string::npos)
{
continue;
}
if (!StringExtensions::IsAlphaNumeric(c))
{
return false;
}
}
return true;
}
} // namespace

ClientAssertionCredential::ClientAssertionCredential(
std::string tenantId,
std::string clientId,
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This one can be const ref, and it will not be a breaking change to change it back to by-value, if at some point we will be copying and storing one.

Suggested change
std::string clientId,
const std::string& clientId,

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it's better not to design (or change) public surface area based on the implementation detail which could change.
I like the pattern for the ctor signatures being consistent with other credential types that we have.

I am not sure I see customer value by having some string params as const& and others as by-value, especially if we change those whenever the underlying implementation changes. What does the end user get out of that decision?
It's simpler to keep all params like these as std::string across all the creds.

std::function<std::string(Context const&)> const& assertionCallback,
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This one is more efficient to take by-value and move:

Suggested change
std::function<std::string(Context const&)> const& assertionCallback,
std::function<std::string(Context const&)> assertionCallback,

(part 1/4)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you share more details on this. When is this more efficient vs take by-reference and why does std::function behave better as a move? Also, are there any consequences of doing an std::move (in general)? Why don't we do that everywhere?

ClientAssertionCredentialOptions const& options)
: TokenCredential("ClientAssertionCredential"), m_assertionCallback(assertionCallback),
Comment thread
ahsonkhan marked this conversation as resolved.
Outdated
m_clientCredentialCore(tenantId, options.AuthorityHost, options.AdditionallyAllowedTenants)
{
bool isTenantIdValid = IsValidTenantId(tenantId);
if (!isTenantIdValid)
{
IdentityLog::Write(
IdentityLog::Level::Warning,
"Invalid tenant ID provided for " + GetCredentialName()
Comment thread
ahsonkhan marked this conversation as resolved.
Outdated
Comment thread
ahsonkhan marked this conversation as resolved.
Outdated
+ ". The tenant ID must be a non-empty string containing only alphanumeric characters, "
"periods, or hyphens. You can locate your tenant ID by following the instructions "
"listed here: https://learn.microsoft.com/partner-center/find-ids-and-domain-names");
}
Comment thread
ahsonkhan marked this conversation as resolved.
if (clientId.empty())
{
IdentityLog::Write(
IdentityLog::Level::Warning, "No client ID specified for " + GetCredentialName() + ".");
}
if (!assertionCallback)
Comment thread
ahsonkhan marked this conversation as resolved.
Outdated
{
IdentityLog::Write(
IdentityLog::Level::Warning,
"The assertionCallback must be a valid function that returns assertions for "
Copy link
Copy Markdown
Member

@antkmsft antkmsft Jul 13, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we can be less general, i.e. not "valid function that returns", but "not be null".

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not sure I see the value in being that specific. An "empty/no target" std::function would still fail. The invalid case isn't only if null is passed.

+ GetCredentialName() + ".");
}

if (isTenantIdValid && !clientId.empty() && assertionCallback)
Comment thread
ahsonkhan marked this conversation as resolved.
Outdated
{
m_tokenCredentialImpl = std::make_unique<TokenCredentialImpl>(options);
m_requestBody
= std::string(
"grant_type=client_credentials"
"&client_assertion_type="
"urn%3Aietf%3Aparams%3Aoauth%3Aclient-assertion-type%3Ajwt-bearer" // cspell:disable-line
"&client_id=")
+ Url::Encode(clientId);

IdentityLog::Write(
IdentityLog::Level::Informational, GetCredentialName() + " was created successfully.");
}
else
{
// Rather than throwing an exception in the ctor, following the pattern in existing credentials
// to log the errors, and defer throwing an exception to the first call of GetToken(). This is
// primarily needed for credentials that are part of the DefaultAzureCredential, which this
// credential is not intended for.
Comment thread
ahsonkhan marked this conversation as resolved.
IdentityLog::Write(
IdentityLog::Level::Warning, GetCredentialName() + " was not initialized correctly.");
Copy link
Copy Markdown
Member

@antkmsft antkmsft Jul 12, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we make the reason for failure to be listed after the message that it was not initialized correctly, plus summarize everything that wrong in a single message? We do the similar in ManagedIdentityCredential and EnvironmentCredential, and I think in some others.

WARNING: Identity: ClientAssertionCredential was not initialized correctly.
VERBOSE: Identity: ClientAssertionCredential:
  * assertionCallback is null
  * clientId is empty
  * tenantId is not valid ... see http://...

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That will require storing the errors in the class (even in cases when things eventually succeed) and result in duplicated logs being printed. To me, that's more noise than signal since the info would be redundantly present.

}
}

ClientAssertionCredential::~ClientAssertionCredential() = default;

AccessToken ClientAssertionCredential::GetToken(
TokenRequestContext const& tokenRequestContext,
Context const& context) const
{
if (!m_tokenCredentialImpl)
{
auto const AuthUnavailable = GetCredentialName() + " authentication unavailable. ";

IdentityLog::Write(
IdentityLog::Level::Warning,
AuthUnavailable + "See earlier " + GetCredentialName() + " log messages for details.");
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"See earlier ClientAssertionCredential log messages for details." - that's where that failure reason grouping and using GetCredentialName() at the start of the message will become handy, together with https://aka.ms/azsdk/cpp/identity/troubleshooting link that we include in some of our exceptions, and I think we should include it in the throw exception below too, using the similar patterns to existing credentials, they do this too.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't understand what you mean here. The pattern I used here is consistent with the existing credentials (environment, workload, etc.).

Look at this, as an example. This is almost identical to what we are doing here:

auto const AuthUnavailable = GetCredentialName() + " authentication unavailable. ";
IdentityLog::Write(
IdentityLog::Level::Warning,
AuthUnavailable + "See earlier " + GetCredentialName() + " log messages for details.");
throw AuthenticationException(
AuthUnavailable + "Environment variables are not fully configured.");


throw AuthenticationException(AuthUnavailable);
}

auto const tenantId = TenantIdResolver::Resolve(
m_clientCredentialCore.GetTenantId(),
tokenRequestContext,
m_clientCredentialCore.GetAdditionallyAllowedTenants());

auto const scopesStr
= m_clientCredentialCore.GetScopesString(tenantId, tokenRequestContext.Scopes);

// TokenCache::GetToken() and m_tokenCredentialImpl->GetToken() can only use the lambda
// argument when they are being executed. They are not supposed to keep a reference to lambda
// argument to call it later. Therefore, any capture made here will outlive the possible time
// frame when the lambda might get called.
return m_tokenCache.GetToken(scopesStr, tenantId, tokenRequestContext.MinimumExpiration, [&]() {
return m_tokenCredentialImpl->GetToken(context, false, [&]() {
auto body = m_requestBody;
if (!scopesStr.empty())
{
body += "&scope=" + scopesStr;
}

// Get the request url before calling m_assertionCallback to validate the authority host
// scheme. This is to avoid calling the assertion callback if the authority host scheme is
Comment thread
ahsonkhan marked this conversation as resolved.
Outdated
// invalid.
auto const requestUrl = m_clientCredentialCore.GetRequestUrl(tenantId);

const std::string assertion = m_assertionCallback(context);

body += "&client_assertion=" + Azure::Core::Url::Encode(assertion);

auto request
= std::make_unique<TokenCredentialImpl::TokenRequest>(HttpMethod::Post, requestUrl, body);

request->HttpRequest.SetHeader("Host", requestUrl.GetHost());

return request;
});
});
}
1 change: 1 addition & 0 deletions sdk/identity/azure-identity/test/ut/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ add_executable (
azure_cli_credential_test.cpp
azure_pipelines_credential_test.cpp
chained_token_credential_test.cpp
client_assertion_credential_test.cpp
client_certificate_credential_test.cpp
client_secret_credential_test.cpp
credential_test_helper.cpp
Expand Down
Loading