Change the default value for the authority host option to be read from the environment variable first. (#4980)

* Change the default option for authority host to be read from the environment first.

* Update changelog.

* Update doc comment and refer to the env var correctly.

* Update doc comments and add unit tests.
This commit is contained in:
Ahson Khan 2023-10-05 11:14:22 -07:00 committed by GitHub
parent 1d4412cb8e
commit 81d95c951f
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
11 changed files with 74 additions and 29 deletions

1
.gitignore vendored
View File

@ -31,6 +31,7 @@ bld*/
[Oo]bj/
[Ll]og/
[Oo]ut/
[Tt]emp
#these are for build from source temp directories
[Ss]dk/**/[Ss]dk/
**/[Oo]ut/**

View File

@ -8,6 +8,8 @@
### Bugs Fixed
- Change the default value for the authority host option to be read from the environment variable first.
### Other Changes
## 1.6.0-beta.2 (2023-09-13)

View File

@ -13,6 +13,7 @@
#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>
@ -44,13 +45,15 @@ namespace Azure { namespace Identity {
{
/**
* @brief Authentication authority URL.
* @note Default value is Azure AD global authority (https://login.microsoftonline.com/).
* @note Defaults to the value of the environment variable 'AZURE_AUTHORITY_HOST'. If that's not
* set, the default value is Azure AD global authority (https://login.microsoftonline.com/).
*
* @note Example of an authority host string: "https://login.microsoftonline.us/". See national
* clouds' Azure AD authentication endpoints:
* https://docs.microsoft.com/azure/active-directory/develop/authentication-national-cloud.
*/
std::string AuthorityHost = _detail::ClientCredentialCore::AadGlobalAuthority;
std::string AuthorityHost
= Azure::Core::_internal::Environment::GetVariable(_detail::AzureAuthorityHostEnvVarName);
/**
* @brief For multi-tenant applications, specifies additional tenants for which the credential

View File

@ -13,6 +13,7 @@
#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>
@ -32,13 +33,15 @@ namespace Azure { namespace Identity {
{
/**
* @brief Authentication authority URL.
* @note Default value is Azure AD global authority (https://login.microsoftonline.com/).
* @note Defaults to the value of the environment variable 'AZURE_AUTHORITY_HOST'. If that's not
* set, the default value is Azure AD global authority (https://login.microsoftonline.com/).
*
* @note Example of an authority host string: "https://login.microsoftonline.us/". See national
* clouds' Azure AD authentication endpoints:
* https://docs.microsoft.com/azure/active-directory/develop/authentication-national-cloud.
*/
std::string AuthorityHost = _detail::ClientCredentialCore::AadGlobalAuthority;
std::string AuthorityHost
= Azure::Core::_internal::Environment::GetVariable(_detail::AzureAuthorityHostEnvVarName);
/**
* @brief For multi-tenant applications, specifies additional tenants for which the credential

View File

@ -12,14 +12,14 @@
#include <vector>
namespace Azure { namespace Identity { namespace _detail {
constexpr auto AzureAuthorityHostEnvVarName = "AZURE_AUTHORITY_HOST";
class ClientCredentialCore final {
std::vector<std::string> m_additionallyAllowedTenants;
Core::Url m_authorityHost;
std::string m_tenantId;
public:
AZ_IDENTITY_DLLEXPORT static std::string const AadGlobalAuthority;
explicit ClientCredentialCore(
std::string tenantId,
std::string const& authorityHost,

View File

@ -41,7 +41,7 @@ namespace Azure { namespace Identity {
/**
* @brief Authentication authority URL.
* @note Defaults to the value of the environment variable AZURE_AUTHORITY_HOST. If that's not
* @note Defaults to the value of the environment variable 'AZURE_AUTHORITY_HOST'. If that's not
* set, the default value is Azure AD global authority (https://login.microsoftonline.com/).
*
* @note Example of an authority host string: "https://login.microsoftonline.us/". See national

View File

@ -6,8 +6,6 @@
#include "private/tenant_id_resolver.hpp"
#include "private/token_credential_impl.hpp"
#include <utility>
using Azure::Identity::_detail::ClientCredentialCore;
using Azure::Core::Url;
@ -15,15 +13,22 @@ using Azure::Core::Credentials::TokenRequestContext;
using Azure::Identity::_detail::TenantIdResolver;
using Azure::Identity::_detail::TokenCredentialImpl;
decltype(ClientCredentialCore::AadGlobalAuthority) ClientCredentialCore::AadGlobalAuthority
= "https://login.microsoftonline.com/";
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.
ClientCredentialCore::ClientCredentialCore(
std::string tenantId,
std::string const& authorityHost,
std::vector<std::string> additionallyAllowedTenants)
: m_additionallyAllowedTenants(std::move(additionallyAllowedTenants)),
m_authorityHost(Url(authorityHost)), m_tenantId(std::move(tenantId))
m_authorityHost(Url(authorityHost.empty() ? AadGlobalAuthority : authorityHost)),
m_tenantId(std::move(tenantId))
{
}

View File

@ -5,6 +5,7 @@
#include "azure/identity/client_certificate_credential.hpp"
#include "azure/identity/client_secret_credential.hpp"
#include "azure/identity/detail/client_credential_core.hpp"
#include "private/identity_log.hpp"
#include <azure/core/azure_assert.hpp>
@ -28,7 +29,6 @@ namespace {
constexpr auto AzureTenantIdEnvVarName = "AZURE_TENANT_ID";
constexpr auto AzureClientIdEnvVarName = "AZURE_CLIENT_ID";
constexpr auto AzureClientSecretEnvVarName = "AZURE_CLIENT_SECRET";
constexpr auto AzureAuthorityHostEnvVarName = "AZURE_AUTHORITY_HOST";
constexpr auto AzureClientCertificatePathEnvVarName = "AZURE_CLIENT_CERTIFICATE_PATH";
void PrintCredentialCreationLogMessage(
@ -46,7 +46,7 @@ EnvironmentCredential::EnvironmentCredential(
auto clientId = Environment::GetVariable(AzureClientIdEnvVarName);
auto clientSecret = Environment::GetVariable(AzureClientSecretEnvVarName);
auto authority = Environment::GetVariable(AzureAuthorityHostEnvVarName);
auto authority = Environment::GetVariable(_detail::AzureAuthorityHostEnvVarName);
auto clientCertificatePath = Environment::GetVariable(AzureClientCertificatePathEnvVarName);
@ -65,7 +65,7 @@ EnvironmentCredential::EnvironmentCredential(
if (!authority.empty())
{
envVarsToParams.push_back({AzureAuthorityHostEnvVarName, "authorityHost"});
envVarsToParams.push_back({_detail::AzureAuthorityHostEnvVarName, "authorityHost"});
clientSecretCredentialOptions.AuthorityHost = authority;
}
@ -85,7 +85,7 @@ EnvironmentCredential::EnvironmentCredential(
if (!authority.empty())
{
envVarsToParams.push_back({AzureAuthorityHostEnvVarName, "authorityHost"});
envVarsToParams.push_back({_detail::AzureAuthorityHostEnvVarName, "authorityHost"});
clientCertificateCredentialOptions.AuthorityHost = authority;
}
@ -109,7 +109,7 @@ EnvironmentCredential::EnvironmentCredential(
auto logMsg = GetCredentialName() + ": Both '" + AzureTenantIdEnvVarName + "' and '"
+ AzureClientIdEnvVarName + "', and at least one of '" + AzureClientSecretEnvVarName
+ "', '" + AzureClientCertificatePathEnvVarName + "' needs to be set. Additionally, '"
+ AzureAuthorityHostEnvVarName
+ _detail::AzureAuthorityHostEnvVarName
+ "' could be set to override the default authority host. Currently:\n";
std::pair<char const*, bool> envVarStatus[] = {
@ -117,7 +117,7 @@ EnvironmentCredential::EnvironmentCredential(
{AzureClientIdEnvVarName, !clientId.empty()},
{AzureClientSecretEnvVarName, !clientSecret.empty()},
{AzureClientCertificatePathEnvVarName, !clientCertificatePath.empty()},
{AzureAuthorityHostEnvVarName, !authority.empty()},
{_detail::AzureAuthorityHostEnvVarName, !authority.empty()},
};
for (auto const& status : envVarStatus)
{

View File

@ -26,7 +26,6 @@ using Azure::Identity::_detail::TokenCredentialImpl;
namespace {
constexpr auto AzureTenantIdEnvVarName = "AZURE_TENANT_ID";
constexpr auto AzureClientIdEnvVarName = "AZURE_CLIENT_ID";
constexpr auto AzureAuthorityHostEnvVarName = "AZURE_AUTHORITY_HOST";
constexpr auto AzureFederatedTokenFileEnvVarName = "AZURE_FEDERATED_TOKEN_FILE";
} // namespace
@ -52,11 +51,7 @@ WorkloadIdentityCredential::WorkloadIdentityCredential(
}
if (authorityHost.empty())
{
authorityHost = Environment::GetVariable(AzureAuthorityHostEnvVarName);
if (authorityHost.empty())
{
authorityHost = _detail::ClientCredentialCore::AadGlobalAuthority;
}
authorityHost = Environment::GetVariable(_detail::AzureAuthorityHostEnvVarName);
}
if (m_tokenFilePath.empty())
{
@ -89,11 +84,7 @@ WorkloadIdentityCredential::WorkloadIdentityCredential(
if (!tenantId.empty() && !clientId.empty() && !m_tokenFilePath.empty())
{
std::string authorityHost = Environment::GetVariable(AzureAuthorityHostEnvVarName);
if (authorityHost.empty())
{
authorityHost = _detail::ClientCredentialCore::AadGlobalAuthority;
}
std::string authorityHost = Environment::GetVariable(_detail::AzureAuthorityHostEnvVarName);
m_clientCredentialCore = Azure::Identity::_detail::ClientCredentialCore(
tenantId, authorityHost, std::vector<std::string>());

View File

@ -148,6 +148,26 @@ TEST_P(GetCredentialName, )
EXPECT_EQ(cred.GetCredentialName(), "ClientCertificateCredential");
}
TEST(ClientCertificateCredential, GetOptionsFromEnvironment)
{
{
std::map<std::string, std::string> envVars = {{"AZURE_AUTHORITY_HOST", ""}};
CredentialTestHelper::EnvironmentOverride const env(envVars);
ClientCertificateCredentialOptions options;
EXPECT_EQ(options.AuthorityHost, "");
}
{
std::map<std::string, std::string> envVars
= {{"AZURE_AUTHORITY_HOST", "https://microsoft.com/"}};
CredentialTestHelper::EnvironmentOverride const env(envVars);
ClientCertificateCredentialOptions options;
EXPECT_EQ(options.AuthorityHost, "https://microsoft.com/");
}
}
TEST_P(GetToken, )
{
auto const actual = CredentialTestHelper::SimulateTokenRequest(

View File

@ -21,6 +21,26 @@ TEST(ClientSecretCredential, GetCredentialName)
EXPECT_EQ(cred.GetCredentialName(), "ClientSecretCredential");
}
TEST(ClientSecretCredential, GetOptionsFromEnvironment)
{
{
std::map<std::string, std::string> envVars = {{"AZURE_AUTHORITY_HOST", ""}};
CredentialTestHelper::EnvironmentOverride const env(envVars);
ClientSecretCredentialOptions options;
EXPECT_EQ(options.AuthorityHost, "");
}
{
std::map<std::string, std::string> envVars
= {{"AZURE_AUTHORITY_HOST", "https://microsoft.com/"}};
CredentialTestHelper::EnvironmentOverride const env(envVars);
ClientSecretCredentialOptions options;
EXPECT_EQ(options.AuthorityHost, "https://microsoft.com/");
}
}
TEST(ClientSecretCredential, Regular)
{
auto const actual = CredentialTestHelper::SimulateTokenRequest(