Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 11 additions & 0 deletions src/Microsoft.Identity.Web.TokenAcquisition/Constants.cs
Original file line number Diff line number Diff line change
Expand Up @@ -136,6 +136,8 @@ public static class Constants
internal const string CertificateIsOutsideValidityWindow = "AADSTS1000502";
internal const string ClientAssertionContainsInvalidSignature = "AADSTS7000274";
internal const string CertificateWasRevoked = "AADSTS7000277";
internal const string ApplicationNotFound = "AADSTS700016";
internal const string InvalidClientSecret = "AADSTS7000215";
internal const string CiamAuthoritySuffix = ".ciamlogin.com";
internal const string TestSlice = "dc";
internal const string ExtensionOptionsServiceProviderKey = "ID_WEB_INTERNAL_SERVICE_PROVIDER";
Expand All @@ -158,6 +160,15 @@ public static class Constants
CertificateWasRevoked, // AADSTS7000277 - Certificate was revoked
};

/// <summary>
/// Error codes indicating permanent configuration errors that should not trigger retry.
/// </summary>
internal static readonly HashSet<string> s_nonRetryableConfigErrorCodes = new (StringComparer.OrdinalIgnoreCase)
{
InvalidClientSecret, // AADSTS7000215 - Wrong client secret
ApplicationNotFound, // AADSTS700016 - Application with identifier not found
};

/// <summary>
/// Key for client assertion in token acquisition options.
/// </summary>
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,7 @@
#nullable enable
const Microsoft.Identity.Web.Constants.ClientAssertionContainsInvalidSignature = "AADSTS7000274" -> string!
const Microsoft.Identity.Web.Constants.ApplicationNotFound = "AADSTS700016" -> string!
const Microsoft.Identity.Web.Constants.CertificateWasRevoked = "AADSTS7000277" -> string!
const Microsoft.Identity.Web.Constants.ClientAssertionContainsInvalidSignature = "AADSTS7000274" -> string!
const Microsoft.Identity.Web.Constants.InvalidClientSecret = "AADSTS7000215" -> string!
static readonly Microsoft.Identity.Web.Constants.s_certificateRelatedErrorCodes -> System.Collections.Generic.HashSet<string!>!
static readonly Microsoft.Identity.Web.Constants.s_nonRetryableConfigErrorCodes -> System.Collections.Generic.HashSet<string!>!
Original file line number Diff line number Diff line change
@@ -1,4 +1,7 @@
#nullable enable
const Microsoft.Identity.Web.Constants.ClientAssertionContainsInvalidSignature = "AADSTS7000274" -> string!
const Microsoft.Identity.Web.Constants.ApplicationNotFound = "AADSTS700016" -> string!
const Microsoft.Identity.Web.Constants.CertificateWasRevoked = "AADSTS7000277" -> string!
const Microsoft.Identity.Web.Constants.ClientAssertionContainsInvalidSignature = "AADSTS7000274" -> string!
const Microsoft.Identity.Web.Constants.InvalidClientSecret = "AADSTS7000215" -> string!
static readonly Microsoft.Identity.Web.Constants.s_certificateRelatedErrorCodes -> System.Collections.Generic.HashSet<string!>!
static readonly Microsoft.Identity.Web.Constants.s_nonRetryableConfigErrorCodes -> System.Collections.Generic.HashSet<string!>!
Original file line number Diff line number Diff line change
@@ -1,4 +1,7 @@
#nullable enable
const Microsoft.Identity.Web.Constants.ClientAssertionContainsInvalidSignature = "AADSTS7000274" -> string!
const Microsoft.Identity.Web.Constants.ApplicationNotFound = "AADSTS700016" -> string!
const Microsoft.Identity.Web.Constants.CertificateWasRevoked = "AADSTS7000277" -> string!
const Microsoft.Identity.Web.Constants.ClientAssertionContainsInvalidSignature = "AADSTS7000274" -> string!
const Microsoft.Identity.Web.Constants.InvalidClientSecret = "AADSTS7000215" -> string!
static readonly Microsoft.Identity.Web.Constants.s_certificateRelatedErrorCodes -> System.Collections.Generic.HashSet<string!>!
static readonly Microsoft.Identity.Web.Constants.s_nonRetryableConfigErrorCodes -> System.Collections.Generic.HashSet<string!>!
Original file line number Diff line number Diff line change
@@ -1,4 +1,7 @@
#nullable enable
const Microsoft.Identity.Web.Constants.ClientAssertionContainsInvalidSignature = "AADSTS7000274" -> string!
const Microsoft.Identity.Web.Constants.ApplicationNotFound = "AADSTS700016" -> string!
const Microsoft.Identity.Web.Constants.CertificateWasRevoked = "AADSTS7000277" -> string!
const Microsoft.Identity.Web.Constants.ClientAssertionContainsInvalidSignature = "AADSTS7000274" -> string!
const Microsoft.Identity.Web.Constants.InvalidClientSecret = "AADSTS7000215" -> string!
static readonly Microsoft.Identity.Web.Constants.s_certificateRelatedErrorCodes -> System.Collections.Generic.HashSet<string!>!
static readonly Microsoft.Identity.Web.Constants.s_nonRetryableConfigErrorCodes -> System.Collections.Generic.HashSet<string!>!
Original file line number Diff line number Diff line change
@@ -1,4 +1,7 @@
#nullable enable
const Microsoft.Identity.Web.Constants.ClientAssertionContainsInvalidSignature = "AADSTS7000274" -> string!
const Microsoft.Identity.Web.Constants.ApplicationNotFound = "AADSTS700016" -> string!
const Microsoft.Identity.Web.Constants.CertificateWasRevoked = "AADSTS7000277" -> string!
const Microsoft.Identity.Web.Constants.ClientAssertionContainsInvalidSignature = "AADSTS7000274" -> string!
const Microsoft.Identity.Web.Constants.InvalidClientSecret = "AADSTS7000215" -> string!
static readonly Microsoft.Identity.Web.Constants.s_certificateRelatedErrorCodes -> System.Collections.Generic.HashSet<string!>!
static readonly Microsoft.Identity.Web.Constants.s_nonRetryableConfigErrorCodes -> System.Collections.Generic.HashSet<string!>!
Original file line number Diff line number Diff line change
@@ -1,4 +1,7 @@
#nullable enable
const Microsoft.Identity.Web.Constants.ClientAssertionContainsInvalidSignature = "AADSTS7000274" -> string!
const Microsoft.Identity.Web.Constants.ApplicationNotFound = "AADSTS700016" -> string!
const Microsoft.Identity.Web.Constants.CertificateWasRevoked = "AADSTS7000277" -> string!
const Microsoft.Identity.Web.Constants.ClientAssertionContainsInvalidSignature = "AADSTS7000274" -> string!
const Microsoft.Identity.Web.Constants.InvalidClientSecret = "AADSTS7000215" -> string!
static readonly Microsoft.Identity.Web.Constants.s_certificateRelatedErrorCodes -> System.Collections.Generic.HashSet<string!>!
static readonly Microsoft.Identity.Web.Constants.s_nonRetryableConfigErrorCodes -> System.Collections.Generic.HashSet<string!>!
114 changes: 78 additions & 36 deletions src/Microsoft.Identity.Web.TokenAcquisition/TokenAcquisition.cs
Original file line number Diff line number Diff line change
Expand Up @@ -53,8 +53,7 @@ class OAuthConstants
private readonly ConcurrentDictionary<string, SemaphoreSlim> _appSemaphores = new();

private const string TokenBindingParameterName = "IsTokenBinding";

private bool _retryClientCertificate;
private const int MaxCertificateRetries = 1;
protected readonly IMsalHttpClientFactory _httpClientFactory;
protected readonly ILogger _logger;
protected readonly IServiceProvider _serviceProvider;
Expand Down Expand Up @@ -124,6 +123,13 @@ public TokenAcquisition(
#endif
public async Task<AcquireTokenResult> AddAccountToCacheFromAuthorizationCodeAsync(
AuthCodeRedemptionParameters authCodeRedemptionParameters)
{
return await AddAccountToCacheFromAuthorizationCodeInternalAsync(authCodeRedemptionParameters, retryCount: 0).ConfigureAwait(false);
}

private async Task<AcquireTokenResult> AddAccountToCacheFromAuthorizationCodeInternalAsync(
AuthCodeRedemptionParameters authCodeRedemptionParameters,
int retryCount)
{
_ = Throws.IfNull(authCodeRedemptionParameters.Scopes);
MergedOptions mergedOptions = _tokenAcquisitionHost.GetOptions(authCodeRedemptionParameters.AuthenticationScheme, out string effectiveAuthenticationScheme);
Expand Down Expand Up @@ -190,26 +196,26 @@ public async Task<AcquireTokenResult> AddAccountToCacheFromAuthorizationCodeAsyn
result.CorrelationId,
result.TokenType);
}
catch (MsalServiceException exMsal) when (IsInvalidClientCertificateOrSignedAssertionError(exMsal))
catch (MsalServiceException exMsal) when (retryCount < MaxCertificateRetries && IsInvalidClientCertificateOrSignedAssertionError(exMsal))
{
Logger.TokenAcquisitionError(
_logger,
$"Certificate error detected. Retrying with next certificate (attempt {retryCount + 1}/{MaxCertificateRetries}). {exMsal.Message}",
exMsal);

string applicationKey = GetApplicationKey(mergedOptions);
NotifyCertificateSelection(mergedOptions, application!, CerticateObserverAction.Deselected, exMsal);
DefaultCertificateLoader.ResetCertificates(mergedOptions.ClientCredentials);
_applicationsByAuthorityClientId[applicationKey] = null;

// Retry
_retryClientCertificate = true;
return await AddAccountToCacheFromAuthorizationCodeAsync(authCodeRedemptionParameters).ConfigureAwait(false);
// Retry with incremented counter
return await AddAccountToCacheFromAuthorizationCodeInternalAsync(authCodeRedemptionParameters, retryCount + 1).ConfigureAwait(false);
}
catch (MsalException ex)
{
Logger.TokenAcquisitionError(_logger, LogMessages.ExceptionOccurredWhenAddingAnAccountToTheCacheFromAuthCode, ex);
throw;
}
finally
{
_retryClientCertificate = false;
}
}


Expand Down Expand Up @@ -256,6 +262,25 @@ public async Task<AuthenticationResult> GetAuthenticationResultForUserAsync(
string? userFlow = null,
ClaimsPrincipal? user = null,
TokenAcquisitionOptions? tokenAcquisitionOptions = null)
{
return await GetAuthenticationResultForUserInternalAsync(
scopes,
authenticationScheme,
tenantId,
userFlow,
user,
tokenAcquisitionOptions,
retryCount: 0).ConfigureAwait(false);
}

private async Task<AuthenticationResult> GetAuthenticationResultForUserInternalAsync(
IEnumerable<string> scopes,
string? authenticationScheme,
string? tenantId,
string? userFlow,
ClaimsPrincipal? user,
TokenAcquisitionOptions? tokenAcquisitionOptions,
int retryCount)
{
_ = Throws.IfNull(scopes);

Expand Down Expand Up @@ -318,22 +343,27 @@ public async Task<AuthenticationResult> GetAuthenticationResultForUserAsync(
LogAuthResult(authenticationResult);
return authenticationResult;
}
catch (MsalServiceException exMsal) when (IsInvalidClientCertificateOrSignedAssertionError(exMsal))
catch (MsalServiceException exMsal) when (retryCount < MaxCertificateRetries && IsInvalidClientCertificateOrSignedAssertionError(exMsal))
{
Logger.TokenAcquisitionError(
_logger,
$"Certificate error detected. Retrying with next certificate (attempt {retryCount + 1}/{MaxCertificateRetries}). {exMsal.Message}",
exMsal);

string applicationKey = GetApplicationKey(mergedOptions);
NotifyCertificateSelection(mergedOptions, application, CerticateObserverAction.Deselected, exMsal);
DefaultCertificateLoader.ResetCertificates(mergedOptions.ClientCredentials);
_applicationsByAuthorityClientId[applicationKey] = null;

// Retry
_retryClientCertificate = true;
return await GetAuthenticationResultForUserAsync(
// Retry with incremented counter
return await GetAuthenticationResultForUserInternalAsync(
scopes,
authenticationScheme: authenticationScheme,
tenantId: tenantId,
userFlow: userFlow,
user: user,
tokenAcquisitionOptions: tokenAcquisitionOptions).ConfigureAwait(false);
authenticationScheme,
tenantId,
userFlow,
user,
tokenAcquisitionOptions,
retryCount + 1).ConfigureAwait(false);
}
catch (MsalUiRequiredException ex)
{
Expand All @@ -344,10 +374,6 @@ public async Task<AuthenticationResult> GetAuthenticationResultForUserAsync(
// AuthorizeForScopesAttribute exception filter so that the user can consent, do 2FA, etc ...
throw new MicrosoftIdentityWebChallengeUserException(ex, scopes.ToArray(), userFlow);
}
finally
{
_retryClientCertificate = false;
}
}

// This method mutate the user claims to include claims uid and utid to perform the silent flow for subsequent calls.
Expand Down Expand Up @@ -537,6 +563,21 @@ public async Task<AuthenticationResult> GetAuthenticationResultForAppAsync(
string? authenticationScheme = null,
string? tenant = null,
TokenAcquisitionOptions? tokenAcquisitionOptions = null)
{
return await GetAuthenticationResultForAppInternalAsync(
scope,
authenticationScheme,
tenant,
tokenAcquisitionOptions,
retryCount: 0).ConfigureAwait(false);
}

private async Task<AuthenticationResult> GetAuthenticationResultForAppInternalAsync(
string scope,
string? authenticationScheme,
string? tenant,
TokenAcquisitionOptions? tokenAcquisitionOptions,
int retryCount)
{
_ = Throws.IfNull(scope);

Expand Down Expand Up @@ -695,20 +736,25 @@ public async Task<AuthenticationResult> GetAuthenticationResultForAppAsync(
NotifyCertificateSelection(mergedOptions, application, CerticateObserverAction.SuccessfullyUsed, null);
return result;
}
catch (MsalServiceException exMsal) when (IsInvalidClientCertificateOrSignedAssertionError(exMsal))
catch (MsalServiceException exMsal) when (retryCount < MaxCertificateRetries && IsInvalidClientCertificateOrSignedAssertionError(exMsal))
{
Logger.TokenAcquisitionError(
_logger,
$"Certificate error detected. Retrying with next certificate (attempt {retryCount + 1}/{MaxCertificateRetries}). {exMsal.Message}",
exMsal);

string applicationKey = GetApplicationKey(mergedOptions);
NotifyCertificateSelection(mergedOptions, application, CerticateObserverAction.Deselected, exMsal);
DefaultCertificateLoader.ResetCertificates(mergedOptions.ClientCredentials);
_applicationsByAuthorityClientId[applicationKey] = null;

// Retry
_retryClientCertificate = true;
return await GetAuthenticationResultForAppAsync(
// Retry with incremented counter
return await GetAuthenticationResultForAppInternalAsync(
scope,
authenticationScheme: authenticationScheme,
tenant: tenant,
tokenAcquisitionOptions: tokenAcquisitionOptions);
authenticationScheme,
tenant,
tokenAcquisitionOptions,
retryCount + 1);
}
catch (MsalException ex)
{
Expand All @@ -717,10 +763,6 @@ public async Task<AuthenticationResult> GetAuthenticationResultForAppAsync(
Logger.TokenAcquisitionError(_logger, ex.Message, ex);
throw;
}
finally
{
_retryClientCertificate = false;
}
}

private void AddExtraBodyParametersIfNeeded(TokenAcquisitionOptions tokenAcquisitionOptions, AcquireTokenForClientParameterBuilder builder)
Expand Down Expand Up @@ -901,8 +943,8 @@ public async Task RemoveAccountAsync(

private bool IsInvalidClientCertificateOrSignedAssertionError(MsalServiceException exMsal)
{
if (_retryClientCertificate ||
!string.Equals(exMsal.ErrorCode, Constants.InvalidClient, StringComparison.OrdinalIgnoreCase))
// Only check invalid_client errors
if (!string.Equals(exMsal.ErrorCode, Constants.InvalidClient, StringComparison.OrdinalIgnoreCase))
{
return false;
}
Expand Down
74 changes: 74 additions & 0 deletions tests/E2E Tests/AgentApplications/AgentUserIdentityTestscs.cs
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
using Microsoft.Extensions.DependencyInjection;
using Microsoft.Graph;
using Microsoft.Identity.Abstractions;
using Microsoft.Identity.Client;
using Microsoft.Identity.Web;
using Microsoft.IdentityModel.Tokens;

Expand Down Expand Up @@ -251,6 +252,79 @@ public async Task AgentUserIdentityGetsTokenForGraphWithCacheAsync()
#endif
}

[Fact]
public async Task AgentUserIdentityGetsTokenForGraphAsyncInvalidCertificate()
{
IServiceCollection services = new ServiceCollection();

// Configure the information about the agent application with an INVALID secret
services.Configure<MicrosoftIdentityApplicationOptions>(
options =>
{
options.Instance = instance;
options.TenantId = tenantId;
options.ClientId = agentApplication;
options.ClientCredentials = [
new CredentialDescription
{
SourceType = CredentialSource.ClientSecret,
ClientSecret = "invalid-secret-that-will-fail-authentication"
}
];
});
IServiceProvider serviceProvider = services.ConfigureServicesForAgentIdentitiesTests();

// Get an authorization header provider
IAuthorizationHeaderProvider authorizationHeaderProvider = serviceProvider.GetService<IAuthorizationHeaderProvider>()!;
AuthorizationHeaderProviderOptions options = new AuthorizationHeaderProviderOptions().WithAgentUserIdentity(
agentApplicationId: agentIdentity,
username: userUpn
);

// Track start time to verify the request doesn't retry infinitely
var startTime = DateTime.UtcNow;

// Assert that authentication fails with invalid credentials
Exception? caughtException = null;
try
{
string authorizationHeaderWithUserToken = await authorizationHeaderProvider.CreateAuthorizationHeaderForUserAsync(
scopes: ["https://graph.microsoft.com/.default"],
options);

// If we got here, no exception was thrown - this is unexpected
Assert.Fail($"Expected authentication to fail with invalid secret, but it generated token.");
}
catch (MsalServiceException msalEx)
{
caughtException = msalEx;

// Calculate duration to ensure it doesn't hang indefinitely
var duration = DateTime.UtcNow - startTime;

// Verify it failed within a reasonable timeframe (no infinite retry loop)
Assert.True(duration.TotalSeconds < 30,
$"Authentication failure took too long ({duration.TotalSeconds} seconds), indicating possible retry loop issue");

// Verify the exception indicates authentication failure
string message = msalEx.Message;
Assert.Contains("AADSTS7000215", message, StringComparison.Ordinal);
}
catch (Exception ex)
{
caughtException = ex;

// Calculate duration
var duration = DateTime.UtcNow - startTime;

// Verify it failed within a reasonable timeframe
Assert.True(duration.TotalSeconds < 30,
$"Authentication failure took too long ({duration.TotalSeconds} seconds), indicating possible retry loop issue");
}


}

[Fact]
public async Task AgentUserIdentityGetsTokenForGraphByUserIdAsync()
{
Expand Down
Loading
Loading