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

Incapsulate all the defaults for options values such as authority host in a helper class and add authority host url validation. (#5155)

Browse Source 
* Incapsulate all the defaults for options values such as authority host
in a helper class and add authority host url validation.

* Address PR feedback.
This commit is contained in:
Ahson Khan 2023-11-10 21:00:14 -08:00 committed by GitHub
parent 3b3795f3df
commit f2bd22794d
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
11 changed files with 135 additions and 68 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

3
sdk/core/azure-core/test/ut/url_test.cpp
View File

@ -293,6 +293,9 @@ namespace Azure { namespace Core { namespace Test {
{
Core::Url url;
EXPECT_EQ(url.GetAbsoluteUrl(), std::string());
Core::Url empty("");
EXPECT_EQ(empty.GetAbsoluteUrl(), std::string());
}
TEST(URL, AppendPathSlash)

2
sdk/identity/azure-identity/CHANGELOG.md
View File

@ -12,10 +12,12 @@
- Harden checks for the tenant ID.
- Disallow space character when validating tenant id and scopes as input for `AzureCliCredential`.
- Add authority host url validation to reject non-HTTPS schemes.
### Other Changes
- Create separate lists of characters that are allowed within tenant ids and scopes in `AzureCliCredential`.
- Add default values to some `WorkloadIdentityCredentialOptions` fields such as authority host by reading them from the environment.
- Add logging to `WorkloadIdentityCredential` to help with debugging.
## 1.6.0-beta.3 (2023-10-12)

4
sdk/identity/azure-identity/inc/azure/identity/client_certificate_credential.hpp
View File

@ -13,7 +13,6 @@
#include <azure/core/credentials/credentials.hpp>
#include <azure/core/credentials/token_credential_options.hpp>
#include <azure/core/internal/environment.hpp>
#include <azure/core/internal/unique_handle.hpp>
#include <azure/core/url.hpp>
@ -53,8 +52,7 @@ namespace Azure { namespace Identity {
* clouds' Microsoft Entra authentication endpoints:
* https://learn.microsoft.com/azure/active-directory/develop/authentication-national-cloud.
*/
std::string AuthorityHost
= Azure::Core::_internal::Environment::GetVariable(_detail::AzureAuthorityHostEnvVarName);
std::string AuthorityHost = _detail::DefaultOptionValues::GetAuthorityHost();
/**
* @brief For multi-tenant applications, specifies additional tenants for which the credential

4
sdk/identity/azure-identity/inc/azure/identity/client_secret_credential.hpp
View File

@ -13,7 +13,6 @@
#include <azure/core/credentials/credentials.hpp>
#include <azure/core/credentials/token_credential_options.hpp>
#include <azure/core/internal/environment.hpp>
#include <azure/core/url.hpp>
#include <memory>
@ -41,8 +40,7 @@ namespace Azure { namespace Identity {
* clouds' Microsoft Entra authentication endpoints:
* https://learn.microsoft.com/azure/active-directory/develop/authentication-national-cloud.
*/
std::string AuthorityHost
= Azure::Core::_internal::Environment::GetVariable(_detail::AzureAuthorityHostEnvVarName);
std::string AuthorityHost = _detail::DefaultOptionValues::GetAuthorityHost();
/**
* @brief For multi-tenant applications, specifies additional tenants for which the credential

34
sdk/identity/azure-identity/inc/azure/identity/detail/client_credential_core.hpp
View File

@ -6,6 +6,7 @@
#include "azure/identity/dll_import_export.hpp"
#include <azure/core/credentials/credentials.hpp>
#include <azure/core/internal/environment.hpp>
#include <azure/core/url.hpp>
#include <string>
@ -13,6 +14,39 @@
namespace Azure { namespace Identity { namespace _detail {
constexpr auto AzureAuthorityHostEnvVarName = "AZURE_AUTHORITY_HOST";
constexpr auto AzureTenantIdEnvVarName = "AZURE_TENANT_ID";
constexpr auto AzureClientIdEnvVarName = "AZURE_CLIENT_ID";
constexpr auto AzureFederatedTokenFileEnvVarName = "AZURE_FEDERATED_TOKEN_FILE";
const std::string AadGlobalAuthority = "https://login.microsoftonline.com/";
class DefaultOptionValues final {
DefaultOptionValues() = delete;
~DefaultOptionValues() = delete;
public:
static std::string GetAuthorityHost()
{
const std::string envAuthHost
= Core::_internal::Environment::GetVariable(AzureAuthorityHostEnvVarName);
return envAuthHost.empty() ? AadGlobalAuthority : envAuthHost;
}
static std::string GetTenantId()
{
return Core::_internal::Environment::GetVariable(AzureTenantIdEnvVarName);
}
static std::string GetClientId()
{
return Core::_internal::Environment::GetVariable(AzureClientIdEnvVarName);
}
static std::string GetFederatedTokenFile()
{
return Core::_internal::Environment::GetVariable(AzureFederatedTokenFileEnvVarName);
}
};
class ClientCredentialCore final {
std::vector<std::string> m_additionallyAllowedTenants;

8
sdk/identity/azure-identity/inc/azure/identity/workload_identity_credential.hpp
View File

@ -31,13 +31,13 @@ namespace Azure { namespace Identity {
* @brief The TenantID of the service principal. Defaults to the value of the environment
* variable AZURE_TENANT_ID.
*/
std::string TenantId;
std::string TenantId = _detail::DefaultOptionValues::GetTenantId();
/**
* @brief The ClientID of the service principal. Defaults to the value of the environment
* variable AZURE_CLIENT_ID.
*/
std::string ClientId;
std::string ClientId = _detail::DefaultOptionValues::GetClientId();
/**
* @brief Authentication authority URL.
@ -49,13 +49,13 @@ namespace Azure { namespace Identity {
* clouds' Microsoft Entra authentication endpoints:
* https://learn.microsoft.com/azure/active-directory/develop/authentication-national-cloud.
*/
std::string AuthorityHost;
std::string AuthorityHost = _detail::DefaultOptionValues::GetAuthorityHost();
/**
* @brief The path of a file containing a Kubernetes service account token. Defaults to the
* value of the environment variable AZURE_FEDERATED_TOKEN_FILE.
*/
std::string TokenFilePath;
std::string TokenFilePath = _detail::DefaultOptionValues::GetFederatedTokenFile();
/**
* @brief For multi-tenant applications, specifies additional tenants for which the credential

15
sdk/identity/azure-identity/src/client_credential_core.cpp
View File

@ -13,27 +13,28 @@ using Azure::Core::Credentials::TokenRequestContext;
using Azure::Identity::_detail::TenantIdResolver;
using Azure::Identity::_detail::TokenCredentialImpl;
namespace {
const std::string AadGlobalAuthority = "https://login.microsoftonline.com/";
}
// The authority host used by the credentials is in the following order of precedence:
// 1. AuthorityHost option set/overriden by the user.
// 2. The value of AZURE_AUTHORITY_HOST environment variable, which is the default value of the
// option.
// 3. If the option is empty, use Azure Public Cloud.
// 3. If that environment variable isn't set or is empty, use Azure Public Cloud.
ClientCredentialCore::ClientCredentialCore(
std::string tenantId,
std::string const& authorityHost,
std::vector<std::string> additionallyAllowedTenants)
: m_additionallyAllowedTenants(std::move(additionallyAllowedTenants)),
m_authorityHost(Url(authorityHost.empty() ? AadGlobalAuthority : authorityHost)),
m_tenantId(std::move(tenantId))
m_authorityHost(Url(authorityHost)), m_tenantId(std::move(tenantId))
{
}
Url ClientCredentialCore::GetRequestUrl(std::string const& tenantId) const
{
if (m_authorityHost.GetScheme() != "https")
{
throw Azure::Core::Credentials::AuthenticationException(
"Authority host must be a TLS protected (https) endpoint.");
}
auto requestUrl = m_authorityHost;
requestUrl.AppendPath(tenantId);
requestUrl.AppendPath(TenantIdResolver::IsAdfs(tenantId) ? "oauth2/token" : "oauth2/v2.0/token");

31
sdk/identity/azure-identity/src/workload_identity_credential.cpp
View File

@ -25,12 +25,6 @@ using Azure::Identity::_detail::IdentityLog;
using Azure::Identity::_detail::TenantIdResolver;
using Azure::Identity::_detail::TokenCredentialImpl;
namespace {
constexpr auto AzureTenantIdEnvVarName = "AZURE_TENANT_ID";
constexpr auto AzureClientIdEnvVarName = "AZURE_CLIENT_ID";
constexpr auto AzureFederatedTokenFileEnvVarName = "AZURE_FEDERATED_TOKEN_FILE";
} // namespace
WorkloadIdentityCredential::WorkloadIdentityCredential(
WorkloadIdentityCredentialOptions const& options)
: TokenCredential("WorkloadIdentityCredential"), m_clientCredentialCore(
@ -43,23 +37,6 @@ WorkloadIdentityCredential::WorkloadIdentityCredential(
std::string authorityHost = options.AuthorityHost;
m_tokenFilePath = options.TokenFilePath;
if (tenantId.empty())
{
tenantId = Environment::GetVariable(AzureTenantIdEnvVarName);
}
if (clientId.empty())
{
clientId = Environment::GetVariable(AzureClientIdEnvVarName);
}
if (authorityHost.empty())
{
authorityHost = Environment::GetVariable(_detail::AzureAuthorityHostEnvVarName);
}
if (m_tokenFilePath.empty())
{
m_tokenFilePath = Environment::GetVariable(AzureFederatedTokenFileEnvVarName);
}
if (!tenantId.empty() && !clientId.empty() && !m_tokenFilePath.empty())
{
m_clientCredentialCore = Azure::Identity::_detail::ClientCredentialCore(
@ -90,13 +67,13 @@ WorkloadIdentityCredential::WorkloadIdentityCredential(
: TokenCredential("WorkloadIdentityCredential"),
m_clientCredentialCore("", "", std::vector<std::string>())
{
std::string tenantId = Environment::GetVariable(AzureTenantIdEnvVarName);
std::string clientId = Environment::GetVariable(AzureClientIdEnvVarName);
m_tokenFilePath = Environment::GetVariable(AzureFederatedTokenFileEnvVarName);
std::string const tenantId = _detail::DefaultOptionValues::GetTenantId();
std::string const clientId = _detail::DefaultOptionValues::GetClientId();
m_tokenFilePath = _detail::DefaultOptionValues::GetFederatedTokenFile();
if (!tenantId.empty() && !clientId.empty() && !m_tokenFilePath.empty())
{
std::string authorityHost = Environment::GetVariable(_detail::AzureAuthorityHostEnvVarName);
std::string const authorityHost = _detail::DefaultOptionValues::GetAuthorityHost();
m_clientCredentialCore = Azure::Identity::_detail::ClientCredentialCore(
tenantId, authorityHost, std::vector<std::string>());

2
sdk/identity/azure-identity/test/ut/client_certificate_credential_test.cpp
View File

@ -155,7 +155,7 @@ TEST(ClientCertificateCredential, GetOptionsFromEnvironment)
CredentialTestHelper::EnvironmentOverride const env(envVars);
ClientCertificateCredentialOptions options;
EXPECT_EQ(options.AuthorityHost, "");
EXPECT_EQ(options.AuthorityHost, "https://login.microsoftonline.com/");
}
{

2
sdk/identity/azure-identity/test/ut/client_secret_credential_test.cpp
View File

@ -28,7 +28,7 @@ TEST(ClientSecretCredential, GetOptionsFromEnvironment)
CredentialTestHelper::EnvironmentOverride const env(envVars);
ClientSecretCredentialOptions options;
EXPECT_EQ(options.AuthorityHost, "");
EXPECT_EQ(options.AuthorityHost, "https://login.microsoftonline.com/");
}
{

98
sdk/identity/azure-identity/test/ut/workload_identity_credential_test.cpp
View File

@ -40,36 +40,90 @@ TEST(WorkloadIdentityCredential, GetCredentialName)
TEST(WorkloadIdentityCredential, GetOptionsFromEnvironment)
{
CredentialTestHelper::EnvironmentOverride const env(
{{"AZURE_TENANT_ID", "01234567-89ab-cdef-fedc-ba8976543210"},
{"AZURE_CLIENT_ID", "fedcba98-7654-3210-0123-456789abcdef"},
{"AZURE_AUTHORITY_HOST", ""},
{"AZURE_FEDERATED_TOKEN_FILE", TempCertFile::Path}});
{
CredentialTestHelper::EnvironmentOverride const env(
{{"AZURE_TENANT_ID", "01234567-89ab-cdef-fedc-ba8976543210"},
{"AZURE_CLIENT_ID", "fedcba98-7654-3210-0123-456789abcdef"},
{"AZURE_AUTHORITY_HOST", ""},
{"AZURE_FEDERATED_TOKEN_FILE", TempCertFile::Path}});
WorkloadIdentityCredential const credDefault;
EXPECT_EQ(credDefault.GetCredentialName(), "WorkloadIdentityCredential");
WorkloadIdentityCredential const credDefault;
EXPECT_EQ(credDefault.GetCredentialName(), "WorkloadIdentityCredential");
WorkloadIdentityCredentialOptions options;
WorkloadIdentityCredential const cred(options);
EXPECT_EQ(cred.GetCredentialName(), "WorkloadIdentityCredential");
WorkloadIdentityCredentialOptions options;
WorkloadIdentityCredential const cred(options);
EXPECT_EQ(cred.GetCredentialName(), "WorkloadIdentityCredential");
EXPECT_EQ(options.TenantId, "01234567-89ab-cdef-fedc-ba8976543210");
EXPECT_EQ(options.ClientId, "fedcba98-7654-3210-0123-456789abcdef");
EXPECT_EQ(options.AuthorityHost, "https://login.microsoftonline.com/");
EXPECT_EQ(options.TokenFilePath, TempCertFile::Path);
}
{
std::map<std::string, std::string> envVars = {{"AZURE_AUTHORITY_HOST", "foo"}};
CredentialTestHelper::EnvironmentOverride const env(envVars);
WorkloadIdentityCredentialOptions options;
options.AuthorityHost = "bar";
EXPECT_EQ(options.AuthorityHost, "bar");
}
{
std::map<std::string, std::string> envVars
= {{"AZURE_AUTHORITY_HOST", "https://microsoft.com/"}};
CredentialTestHelper::EnvironmentOverride const env(envVars);
WorkloadIdentityCredentialOptions options;
EXPECT_EQ(options.AuthorityHost, "https://microsoft.com/");
}
}
TEST(WorkloadIdentityCredential, GetOptionsFromEnvironmentInvalid)
{
CredentialTestHelper::EnvironmentOverride const env(
{{"AZURE_TENANT_ID", ""},
{"AZURE_CLIENT_ID", ""},
{"AZURE_AUTHORITY_HOST", ""},
{"AZURE_FEDERATED_TOKEN_FILE", ""}});
{
CredentialTestHelper::EnvironmentOverride const env(
{{"AZURE_TENANT_ID", ""},
{"AZURE_CLIENT_ID", ""},
{"AZURE_AUTHORITY_HOST", ""},
{"AZURE_FEDERATED_TOKEN_FILE", ""}});
TokenRequestContext trc;
trc.Scopes.push_back("https://storage.azure.com/.default");
TokenRequestContext trc;
trc.Scopes.push_back("https://storage.azure.com/.default");
WorkloadIdentityCredential const credDefault;
EXPECT_THROW(credDefault.GetToken(trc, {}), AuthenticationException);
WorkloadIdentityCredentialOptions options;
WorkloadIdentityCredential const cred(options);
EXPECT_THROW(cred.GetToken(trc, {}), AuthenticationException);
WorkloadIdentityCredential const credDefault;
EXPECT_THROW(credDefault.GetToken(trc, {}), AuthenticationException);
WorkloadIdentityCredentialOptions options;
WorkloadIdentityCredential const cred(options);
EXPECT_THROW(cred.GetToken(trc, {}), AuthenticationException);
}
// The http scheme is not supported.
{
CredentialTestHelper::EnvironmentOverride const env(
{{"AZURE_TENANT_ID", "01234567-89ab-cdef-fedc-ba8976543210"},
{"AZURE_CLIENT_ID", "fedcba98-7654-3210-0123-456789abcdef"},
{"AZURE_AUTHORITY_HOST", "http://microsoft.com/"},
{"AZURE_FEDERATED_TOKEN_FILE", TempCertFile::Path}});
TokenRequestContext trc;
trc.Scopes.push_back("https://storage.azure.com/.default");
WorkloadIdentityCredential const credDefault;
EXPECT_THROW(credDefault.GetToken(trc, {}), AuthenticationException);
WorkloadIdentityCredentialOptions options;
WorkloadIdentityCredential const cred(options);
EXPECT_THROW(cred.GetToken(trc, {}), AuthenticationException);
try
{
auto const token = cred.GetToken(trc, {});
}
catch (AuthenticationException const& e)
{
EXPECT_TRUE(std::string(e.what()).find("https") != std::string::npos) << e.what();
}
}
}
TEST(WorkloadIdentityCredential, Regular)