diff --git a/sdk/identity/azure-identity/CHANGELOG.md b/sdk/identity/azure-identity/CHANGELOG.md index c8658bb32..8658986cb 100644 --- a/sdk/identity/azure-identity/CHANGELOG.md +++ b/sdk/identity/azure-identity/CHANGELOG.md @@ -8,6 +8,7 @@ ### Bugs Fixed +- Harden checks for the tenant ID. - Disallow space character when validating tenant id and scopes as input for `AzureCliCredential`. ### Other Changes diff --git a/sdk/identity/azure-identity/src/azure_cli_credential.cpp b/sdk/identity/azure-identity/src/azure_cli_credential.cpp index 87f8f7707..911fee683 100644 --- a/sdk/identity/azure-identity/src/azure_cli_credential.cpp +++ b/sdk/identity/azure-identity/src/azure_cli_credential.cpp @@ -116,7 +116,7 @@ std::string AzureCliCredential::GetAzCommand(std::string const& scopes, std::str // The OAuth 2.0 RFC (https://datatracker.ietf.org/doc/html/rfc6749#section-3.3) allows space as // well for a list of scopes, but that isn't currently required. ThrowIfNotSafeCmdLineInput(scopes, ".-:/_", "Scopes"); - ThrowIfNotSafeCmdLineInput(m_tenantId, ".-", "TenantID"); + ThrowIfNotSafeCmdLineInput(tenantId, ".-", "TenantID"); std::string command = "az account get-access-token --output json --scope \"" + scopes + "\""; if (!tenantId.empty()) @@ -141,6 +141,7 @@ AccessToken AzureCliCredential::GetToken( auto const scopes = TokenCredentialImpl::FormatScopes(tokenRequestContext.Scopes, false, false); auto const tenantId = TenantIdResolver::Resolve(m_tenantId, tokenRequestContext, m_additionallyAllowedTenants); + auto const command = GetAzCommand(scopes, tenantId); // TokenCache::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 @@ -148,8 +149,7 @@ AccessToken AzureCliCredential::GetToken( return m_tokenCache.GetToken(scopes, tenantId, tokenRequestContext.MinimumExpiration, [&]() { try { - auto const azCliResult - = RunShellCommand(GetAzCommand(scopes, tenantId), m_cliProcessTimeout, context); + auto const azCliResult = RunShellCommand(command, m_cliProcessTimeout, context); try { diff --git a/sdk/identity/azure-identity/test/ut/azure_cli_credential_test.cpp b/sdk/identity/azure-identity/test/ut/azure_cli_credential_test.cpp index 670b6d01c..da4853882 100644 --- a/sdk/identity/azure-identity/test/ut/azure_cli_credential_test.cpp +++ b/sdk/identity/azure-identity/test/ut/azure_cli_credential_test.cpp @@ -353,6 +353,7 @@ TEST_P(ParameterizedTestForDisallowedChars, DisallowedCharsForScopeAndTenantId) { std::string const InvalidValue = GetParam(); + // Tenant ID test via AzureCliCredentialOptions directly. { AzureCliCredentialOptions options; options.TenantId = "01234567-89AB-CDEF-0123-456789ABCDEF"; @@ -373,6 +374,50 @@ TEST_P(ParameterizedTestForDisallowedChars, DisallowedCharsForScopeAndTenantId) } } + // Tenant ID test via TokenRequestContext, using a wildcard for AdditionallyAllowedTenants. + { + AzureCliCredentialOptions options; + options.CliProcessTimeout = std::chrono::hours(24); + options.AdditionallyAllowedTenants.push_back("*"); + AzureCliCredential azCliCred(options); + + TokenRequestContext trc; + trc.Scopes.push_back(std::string("https://storage.azure.com/.default")); + trc.TenantId = InvalidValue; + EXPECT_THROW(static_cast(azCliCred.GetToken(trc, {})), AuthenticationException); + + try + { + auto const token = azCliCred.GetToken(trc, {}); + } + catch (AuthenticationException const& e) + { + EXPECT_TRUE(std::string(e.what()).find("Unsafe") != std::string::npos) << e.what(); + } + } + + // Tenant ID test via TokenRequestContext, using a specific AdditionallyAllowedTenants value. + { + AzureCliCredentialOptions options; + options.AdditionallyAllowedTenants.push_back(InvalidValue); + AzureCliCredential azCliCred(options); + + TokenRequestContext trc; + trc.Scopes.push_back(std::string("https://storage.azure.com/.default")); + trc.TenantId = InvalidValue; + EXPECT_THROW(static_cast(azCliCred.GetToken(trc, {})), AuthenticationException); + + try + { + auto const token = azCliCred.GetToken(trc, {}); + } + catch (AuthenticationException const& e) + { + EXPECT_TRUE(std::string(e.what()).find("Unsafe") != std::string::npos) << e.what(); + } + } + + // Scopes test via TokenRequestContext. { AzureCliCredentialOptions options; options.CliProcessTimeout = std::chrono::hours(24);