Change the default value for the authority host option to be read fro…

…m 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.

.gitignore

@@ -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/**

sdk/identity/azure-identity/CHANGELOG.md

@@ -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)

sdk/identity/azure-identity/inc/azure/identity/client_certificate_credential.hpp

@@ -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

sdk/identity/azure-identity/inc/azure/identity/client_secret_credential.hpp

@@ -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

sdk/identity/azure-identity/inc/azure/identity/detail/client_credential_core.hpp

@@ -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,

sdk/identity/azure-identity/inc/azure/identity/workload_identity_credential.hpp

@@ -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

sdk/identity/azure-identity/src/client_credential_core.cpp

@@ -6,24 +6,29 @@
 #include "private/tenant_id_resolver.hpp"
 #include "private/token_credential_impl.hpp"
 
-#include <utility>
-
 using Azure::Identity::_detail::ClientCredentialCore;
 
 using Azure::Core::Url;
 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))
 {
 }
 

sdk/identity/azure-identity/src/environment_credential.cpp

@@ -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,15 +109,15 @@ 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[] = {
           {AzureTenantIdEnvVarName, !tenantId.empty()},
           {AzureClientIdEnvVarName, !clientId.empty()},
           {AzureClientSecretEnvVarName, !clientSecret.empty()},
           {AzureClientCertificatePathEnvVarName, !clientCertificatePath.empty()},
-          {AzureAuthorityHostEnvVarName, !authority.empty()},
+          {_detail::AzureAuthorityHostEnvVarName, !authority.empty()},
       };
       for (auto const& status : envVarStatus)
       {

sdk/identity/azure-identity/src/workload_identity_credential.cpp

@@ -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>());

sdk/identity/azure-identity/test/ut/client_certificate_credential_test.cpp

@@ -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(

sdk/identity/azure-identity/test/ut/client_secret_credential_test.cpp

@@ -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(