Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix DefaultAzureCredential exception issues #8936 #9038

Closed
wants to merge 9 commits into from
13 changes: 2 additions & 11 deletions sdk/identity/Azure.Identity/src/AuthenticationFailedException.cs
Original file line number Diff line number Diff line change
Expand Up @@ -34,22 +34,13 @@ public AuthenticationFailedException(string message, Exception innerException)

internal static AuthenticationFailedException CreateAggregateException(string message, ReadOnlyMemory<object> credentials, IList<Exception> innerExceptions)
{
StringBuilder exStr = new StringBuilder(message).AppendLine();
StringBuilder exStr = new StringBuilder(message);

for (int i = 0; i < credentials.Length; i++)
{
if (innerExceptions[i] is CredentialUnavailableException)
{
exStr.AppendLine($" {credentials.Span[i].GetType().Name} is unavailable {innerExceptions[i].Message}.");
}
else
{
exStr.AppendLine($" {credentials.Span[i].GetType().Name} failed with {innerExceptions[i].Message}.");
}
exStr.Append($" {credentials.Span[i].GetType().Name}: {innerExceptions[i].Message}.");
}

exStr.Append("See inner exception for more detail.");

return new AuthenticationFailedException(exStr.ToString(), new AggregateException(message, innerExceptions.ToArray()));
}
}
Expand Down
4 changes: 2 additions & 2 deletions sdk/identity/Azure.Identity/src/ChainedTokenCredential.cs
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ public override AccessToken GetToken(TokenRequestContext requestContext, Cancell
{
exceptions.Add(e);

throw AuthenticationFailedException.CreateAggregateException(AggregateCredentialFailedErrorMessage + e.Message, new ReadOnlyMemory<object>(_sources, 0, i + 1), exceptions);
throw AuthenticationFailedException.CreateAggregateException(AggregateCredentialFailedErrorMessage, new ReadOnlyMemory<object>(_sources, 0, i + 1), exceptions);
}
}

Expand Down Expand Up @@ -101,7 +101,7 @@ public override async ValueTask<AccessToken> GetTokenAsync(TokenRequestContext r
{
exceptions.Add(e);

throw AuthenticationFailedException.CreateAggregateException(AggregateCredentialFailedErrorMessage + e.Message, new ReadOnlyMemory<object>(_sources, 0, i + 1), exceptions);
throw AuthenticationFailedException.CreateAggregateException(AggregateCredentialFailedErrorMessage, new ReadOnlyMemory<object>(_sources, 0, i + 1), exceptions);
}
}

Expand Down
2 changes: 1 addition & 1 deletion sdk/identity/Azure.Identity/src/Constants.cs
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ internal class Constants
// TODO: Currently this is piggybacking off the Azure CLI client ID, but needs to be switched once the Developer Sign On application is available
public const string DeveloperSignOnClientId = "04b07795-8ddb-461a-bbee-02f9e1bf7b46";

public const string AuthenticationUnhandledExceptionMessage = "The authentication request failed due to an unhandled exception. See inner exception for details.";
public const string AuthenticationUnhandledExceptionMessage = "The authentication request failed due to an unhandled exception. See inner exception for details";

public static string SharedTokenCacheFilePath { get { return Path.Combine(DefaultCacheDirectory, "msal.cache"); } }

Expand Down
8 changes: 3 additions & 5 deletions sdk/identity/Azure.Identity/src/DefaultAzureCredential.cs
Original file line number Diff line number Diff line change
Expand Up @@ -28,8 +28,8 @@ namespace Azure.Identity
/// </remarks>
public class DefaultAzureCredential : TokenCredential
{
private const string DefaultExceptionMessage = "The DefaultAzureCredential failed to retrieve a token from the included credentials.";
private const string UnhandledExceptionMessage = "The DefaultAzureCredential failed due to an unhandled exception: ";
private const string DefaultExceptionMessage = "DefaultAzureCredential was unable to retrieve a token. Please review the Azure Identity documentation for instructions on how to configure each credential.";
private const string UnhandledExceptionMessage = "DefaultAzureCredential failed due to an unhandled exception: ";
private static readonly IExtendedTokenCredential[] s_defaultCredentialChain = GetDefaultAzureCredentialChain(new DefaultAzureCredentialFactory(CredentialPipeline.GetInstance(null)), new DefaultAzureCredentialOptions());

private readonly IExtendedTokenCredential[] _sources;
Expand Down Expand Up @@ -106,16 +106,14 @@ private async Task<AccessToken> GetTokenAsync(bool isAsync, TokenRequestContext
{
return scope.Succeeded(exToken.AccessToken);
}

if (exToken.Exception is CredentialUnavailableException)
{
exceptions.Add(exToken.Exception);
}
else
{
exceptions.Add(exToken.Exception);

throw scope.Failed(AuthenticationFailedException.CreateAggregateException($"{UnhandledExceptionMessage} {_sources[i].GetType().Name} failed with unhandled exception {exToken.Exception.Message}.", new ReadOnlyMemory<object>(_sources, 0, i + 1), exceptions));
throw scope.Failed(AuthenticationFailedException.CreateAggregateException($"{UnhandledExceptionMessage}", new ReadOnlyMemory<object>(_sources, 0, i + 1), exceptions));
}
}

Expand Down
42 changes: 3 additions & 39 deletions sdk/identity/Azure.Identity/src/EnvironmentCredential.cs
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ public class EnvironmentCredential : TokenCredential, IExtendedTokenCredential
{
private readonly CredentialPipeline _pipeline;
private readonly TokenCredential _credential;
private readonly string _unavailbleErrorMessage;
private const string UnavailableErrorMessage = "Environment variables not set";

/// <summary>
/// Creates an instance of the EnvironmentCredential class and reads client secret details from environment variables.
Expand Down Expand Up @@ -77,42 +77,6 @@ internal EnvironmentCredential(CredentialPipeline pipeline)
_credential = new AuthFileCredential(sdkAuthLocation);
}

if (_credential is null)
{
StringBuilder builder = new StringBuilder("Environment variables not fully configured. AZURE_TENANT_ID and AZURE_CLIENT_ID must be set, along with either AZURE_CLIENT_SECRET or AZURE_USERNAME and AZURE_PASSWORD. Alternately, AZURE_AUTH_LOCATION ca be set. Currently set variables [");

if (tenantId != null)
{
builder.Append(" AZURE_TENANT_ID");
}

if (clientId != null)
{
builder.Append(" AZURE_CLIENT_ID");
}

if (clientSecret != null)
{
builder.Append(" AZURE_CLIENT_SECRET");
}

if (username != null)
{
builder.Append(" AZURE_USERNAME");
}

if (password != null)
{
builder.Append(" AZURE_PASSWORD");
}

if (sdkAuthLocation != null)
{
builder.Append(" AZURE_AUTH_LOCATION");
}

_unavailbleErrorMessage = builder.Append(" ]").ToString();
}
}

internal EnvironmentCredential(CredentialPipeline pipeline, TokenCredential credential)
Expand Down Expand Up @@ -174,7 +138,7 @@ private ExtendedAccessToken GetTokenImpl(TokenRequestContext requestContext, Can

if (_credential is null)
{
return new ExtendedAccessToken(scope.Failed(new CredentialUnavailableException(_unavailbleErrorMessage)));
return new ExtendedAccessToken(scope.Failed(new CredentialUnavailableException(UnavailableErrorMessage)));
}

try
Expand All @@ -201,7 +165,7 @@ private async ValueTask<ExtendedAccessToken> GetTokenImplAsync(TokenRequestConte

if (_credential is null)
{
return new ExtendedAccessToken(scope.Failed(new CredentialUnavailableException(_unavailbleErrorMessage)));
return new ExtendedAccessToken(scope.Failed(new CredentialUnavailableException(UnavailableErrorMessage)));
}

try
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ namespace Azure.Identity
/// </summary>
public class ManagedIdentityCredential : TokenCredential, IExtendedTokenCredential
{
internal const string MsiUnavailableError = "No managed identity endpoint found.";
internal const string MsiUnavailableError = "Managed identity endpoint not found";

private readonly string _clientId;
private readonly CredentialPipeline _pipeline;
Expand Down
8 changes: 4 additions & 4 deletions sdk/identity/Azure.Identity/src/SharedTokenCacheCredential.cs
Original file line number Diff line number Diff line change
Expand Up @@ -19,10 +19,10 @@ namespace Azure.Identity
/// </summary>
public class SharedTokenCacheCredential : TokenCredential, IExtendedTokenCredential
{
internal const string NoAccountsInCacheMessage = "No accounts were found in thecache. To authenticate with the SharedTokenCacheCredential, login an account through developer tooling supporting Azure single sign on.";
internal const string MultipleAccountsInCacheMessage = "Multiple accounts were found in the cache. To authenticate with the SharedTokenCacheCredential, set the AZURE_USERNAME and AZURE_TENANT_ID environment variables to the preferred username and tenantId, or specify them to the constructor. {0}";
internal const string NoMatchingAccountsInCacheMessage = "No account matching the specified{0}{1} was found in the cache. To authenticate with the SharedTokenCacheCredential, login an account through developer tooling supporting Azure single sign on. {2}";
internal const string MultipleMatchingAccountsInCacheMessage = "Multiple accounts matching the specified{0}{1} were found in the cache. To authenticate with the SharedTokenCacheCredential, set the AZURE_USERNAME and AZURE_TENANT_ID environment variables to the preferred username and tenantId, or specify them to the constructor. {2}";
internal const string NoAccountsInCacheMessage = "Local account not found";
internal const string MultipleAccountsInCacheMessage = "Multiple local accounts were found. To authenticate with the SharedTokenCacheCredential, set the AZURE_USERNAME and AZURE_TENANT_ID environment variables to the preferred username and tenantId, or specify them to the constructor. {0}";
internal const string NoMatchingAccountsInCacheMessage = "No local account matching the specified{0}{1} was found. To authenticate with the SharedTokenCacheCredential, login an account with a development tool that supports Azure single sign on. {2}";
internal const string MultipleMatchingAccountsInCacheMessage = "Multiple local accounts matching the specified{0}{1} were found. To authenticate with the SharedTokenCacheCredential, set the AZURE_USERNAME and AZURE_TENANT_ID environment variables to the preferred username and tenantId, or specify them to the constructor. {2}";

private readonly MsalPublicClient _client;
private readonly CredentialPipeline _pipeline;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -164,7 +164,7 @@ public async Task MultipleAccountsNoTenantIdOrUsername()

var ex = Assert.ThrowsAsync<CredentialUnavailableException>(async () => await credential.GetTokenAsync(new TokenRequestContext(MockScopes.Default)));

Assert.True(ex.Message.StartsWith("Multiple accounts were found in the cache."));
Assert.True(ex.Message.StartsWith("Multiple local accounts were found."));

Assert.True(ex.Message.Contains($"username: fakeuser@fakedomain.com tenantId: {fakeuserTenantId}"));

Expand Down Expand Up @@ -192,7 +192,7 @@ public async Task NoMatchingAccountsUsernameOnly()

var ex = Assert.ThrowsAsync<CredentialUnavailableException>(async () => await credential.GetTokenAsync(new TokenRequestContext(MockScopes.Default)));

Assert.True(ex.Message.StartsWith("No account matching the specified username: mockuser@mockdomain.com was found in the cache."));
Assert.True(ex.Message.StartsWith("No local account matching the specified username: mockuser@mockdomain.com was found."));

Assert.True(ex.Message.Contains($"username: fakeuser@fakedomain.com tenantId: {fakeuserTenantId}"));

Expand Down Expand Up @@ -221,7 +221,7 @@ public async Task NoMatchingAccountsTenantIdOnly()

var ex = Assert.ThrowsAsync<CredentialUnavailableException>(async () => await credential.GetTokenAsync(new TokenRequestContext(MockScopes.Default)));

Assert.True(ex.Message.StartsWith($"No account matching the specified tenantId: {tenantId} was found in the cache."));
Assert.True(ex.Message.StartsWith($"No local account matching the specified tenantId: {tenantId} was found."));

Assert.True(ex.Message.Contains($"username: fakeuser@fakedomain.com tenantId: {fakeuserTenantId}"));

Expand Down Expand Up @@ -250,7 +250,7 @@ public async Task NoMatchingAccountsTenantIdAndUsername()

var ex = Assert.ThrowsAsync<CredentialUnavailableException>(async () => await credential.GetTokenAsync(new TokenRequestContext(MockScopes.Default)));

Assert.True(ex.Message.StartsWith($"No account matching the specified username: mockuser@mockdomain.com tenantId: {tenantId} was found in the cache."));
Assert.True(ex.Message.StartsWith($"No local account matching the specified username: mockuser@mockdomain.com tenantId: {tenantId} was found."));

Assert.True(ex.Message.Contains($"username: fakeuser@fakedomain.com tenantId: {fakeuserTenantId}"));

Expand Down Expand Up @@ -278,7 +278,7 @@ public async Task MultipleMatchingAccountsUsernameOnly()

var ex = Assert.ThrowsAsync<CredentialUnavailableException>(async () => await credential.GetTokenAsync(new TokenRequestContext(MockScopes.Default)));

Assert.True(ex.Message.StartsWith("Multiple accounts matching the specified username: mockuser@mockdomain.com were found in the cache"));
Assert.True(ex.Message.StartsWith("Multiple local accounts matching the specified username: mockuser@mockdomain.com were found"));

Assert.True(ex.Message.Contains($"username: fakeuser@fakedomain.com tenantId: {fakeuserTenantId}"));

Expand Down Expand Up @@ -308,7 +308,7 @@ public async Task MultipleMatchingAccountsTenantIdOnly()

var ex = Assert.ThrowsAsync<CredentialUnavailableException>(async () => await credential.GetTokenAsync(new TokenRequestContext(MockScopes.Default)));

Assert.True(ex.Message.StartsWith($"Multiple accounts matching the specified tenantId: {mockuserGuestTenantId} were found in the cache"));
Assert.True(ex.Message.StartsWith($"Multiple local accounts matching the specified tenantId: {mockuserGuestTenantId} were found"));

Assert.True(ex.Message.Contains($"username: fakeuser@fakedomain.com tenantId: {fakeuserTenantId}"));

Expand Down Expand Up @@ -338,7 +338,7 @@ public async Task MultipleMatchingAccountsUsernameAndTenantId()

var ex = Assert.ThrowsAsync<CredentialUnavailableException>(async () => await credential.GetTokenAsync(new TokenRequestContext(MockScopes.Default)));

Assert.True(ex.Message.StartsWith($"Multiple accounts matching the specified username: mockuser@mockdomain.com tenantId: {mockuserTenantId} were found in the cache"));
Assert.True(ex.Message.StartsWith($"Multiple local accounts matching the specified username: mockuser@mockdomain.com tenantId: {mockuserTenantId} were found"));

Assert.True(ex.Message.Contains($"username: fakeuser@fakedomain.com tenantId: {fakeuserTenantId}"));

Expand Down