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

Harden checks for the tenant ID across all inputs. (#5106)

Browse Source 
* Harden checks for the tenant ID across all inputs.

* Fix clang formatting.

* Fixup unit tests.

* Fixup test comments.
This commit is contained in:
Ahson Khan 2023-11-08 17:57:00 -08:00 committed by GitHub
parent a6ea41e717
commit f816bb194b
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 49 additions and 3 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

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

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

6
sdk/identity/azure-identity/src/azure_cli_credential.cpp
View File

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

45
sdk/identity/azure-identity/test/ut/azure_cli_credential_test.cpp
View File

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