diff --git a/src/Microsoft.Identity.Web.TokenAcquisition/Constants.cs b/src/Microsoft.Identity.Web.TokenAcquisition/Constants.cs index 2c8145a0a..d237125a3 100644 --- a/src/Microsoft.Identity.Web.TokenAcquisition/Constants.cs +++ b/src/Microsoft.Identity.Web.TokenAcquisition/Constants.cs @@ -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"; @@ -158,6 +160,15 @@ public static class Constants CertificateWasRevoked, // AADSTS7000277 - Certificate was revoked }; + /// + /// Error codes indicating permanent configuration errors that should not trigger retry. + /// + internal static readonly HashSet s_nonRetryableConfigErrorCodes = new (StringComparer.OrdinalIgnoreCase) + { + InvalidClientSecret, // AADSTS7000215 - Wrong client secret + ApplicationNotFound, // AADSTS700016 - Application with identifier not found + }; + /// /// Key for client assertion in token acquisition options. /// diff --git a/src/Microsoft.Identity.Web.TokenAcquisition/PublicAPI/net10.0/InternalAPI.Unshipped.txt b/src/Microsoft.Identity.Web.TokenAcquisition/PublicAPI/net10.0/InternalAPI.Unshipped.txt index a238605bc..ec57a0a8d 100644 --- a/src/Microsoft.Identity.Web.TokenAcquisition/PublicAPI/net10.0/InternalAPI.Unshipped.txt +++ b/src/Microsoft.Identity.Web.TokenAcquisition/PublicAPI/net10.0/InternalAPI.Unshipped.txt @@ -1,7 +1,10 @@ #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! Microsoft.Identity.Web.TokenAcquisitionExtensionOptions.InvokeOnBeforeTokenAcquisitionForOnBehalfOf(Microsoft.Identity.Client.AcquireTokenOnBehalfOfParameterBuilder! builder, Microsoft.Identity.Abstractions.AcquireTokenOptions? acquireTokenOptions, System.Security.Claims.ClaimsPrincipal! user) -> void Microsoft.Identity.Web.TokenAcquisitionExtensionOptions.InvokeOnBeforeTokenAcquisitionForOnBehalfOfAsync(Microsoft.Identity.Client.AcquireTokenOnBehalfOfParameterBuilder! builder, Microsoft.Identity.Abstractions.AcquireTokenOptions? acquireTokenOptions, System.Security.Claims.ClaimsPrincipal! user) -> System.Threading.Tasks.Task! static Microsoft.Identity.Web.ConfidentialClientApplicationBuilderExtension.Logger.UsingCertThumbprint(Microsoft.Extensions.Logging.ILogger! logger, string? certThumbprint) -> void static readonly Microsoft.Identity.Web.Constants.s_certificateRelatedErrorCodes -> System.Collections.Generic.HashSet! +static readonly Microsoft.Identity.Web.Constants.s_nonRetryableConfigErrorCodes -> System.Collections.Generic.HashSet! diff --git a/src/Microsoft.Identity.Web.TokenAcquisition/PublicAPI/net462/InternalAPI.Unshipped.txt b/src/Microsoft.Identity.Web.TokenAcquisition/PublicAPI/net462/InternalAPI.Unshipped.txt index a238605bc..ec57a0a8d 100644 --- a/src/Microsoft.Identity.Web.TokenAcquisition/PublicAPI/net462/InternalAPI.Unshipped.txt +++ b/src/Microsoft.Identity.Web.TokenAcquisition/PublicAPI/net462/InternalAPI.Unshipped.txt @@ -1,7 +1,10 @@ #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! Microsoft.Identity.Web.TokenAcquisitionExtensionOptions.InvokeOnBeforeTokenAcquisitionForOnBehalfOf(Microsoft.Identity.Client.AcquireTokenOnBehalfOfParameterBuilder! builder, Microsoft.Identity.Abstractions.AcquireTokenOptions? acquireTokenOptions, System.Security.Claims.ClaimsPrincipal! user) -> void Microsoft.Identity.Web.TokenAcquisitionExtensionOptions.InvokeOnBeforeTokenAcquisitionForOnBehalfOfAsync(Microsoft.Identity.Client.AcquireTokenOnBehalfOfParameterBuilder! builder, Microsoft.Identity.Abstractions.AcquireTokenOptions? acquireTokenOptions, System.Security.Claims.ClaimsPrincipal! user) -> System.Threading.Tasks.Task! static Microsoft.Identity.Web.ConfidentialClientApplicationBuilderExtension.Logger.UsingCertThumbprint(Microsoft.Extensions.Logging.ILogger! logger, string? certThumbprint) -> void static readonly Microsoft.Identity.Web.Constants.s_certificateRelatedErrorCodes -> System.Collections.Generic.HashSet! +static readonly Microsoft.Identity.Web.Constants.s_nonRetryableConfigErrorCodes -> System.Collections.Generic.HashSet! diff --git a/src/Microsoft.Identity.Web.TokenAcquisition/PublicAPI/net472/InternalAPI.Unshipped.txt b/src/Microsoft.Identity.Web.TokenAcquisition/PublicAPI/net472/InternalAPI.Unshipped.txt index a238605bc..ec57a0a8d 100644 --- a/src/Microsoft.Identity.Web.TokenAcquisition/PublicAPI/net472/InternalAPI.Unshipped.txt +++ b/src/Microsoft.Identity.Web.TokenAcquisition/PublicAPI/net472/InternalAPI.Unshipped.txt @@ -1,7 +1,10 @@ #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! Microsoft.Identity.Web.TokenAcquisitionExtensionOptions.InvokeOnBeforeTokenAcquisitionForOnBehalfOf(Microsoft.Identity.Client.AcquireTokenOnBehalfOfParameterBuilder! builder, Microsoft.Identity.Abstractions.AcquireTokenOptions? acquireTokenOptions, System.Security.Claims.ClaimsPrincipal! user) -> void Microsoft.Identity.Web.TokenAcquisitionExtensionOptions.InvokeOnBeforeTokenAcquisitionForOnBehalfOfAsync(Microsoft.Identity.Client.AcquireTokenOnBehalfOfParameterBuilder! builder, Microsoft.Identity.Abstractions.AcquireTokenOptions? acquireTokenOptions, System.Security.Claims.ClaimsPrincipal! user) -> System.Threading.Tasks.Task! static Microsoft.Identity.Web.ConfidentialClientApplicationBuilderExtension.Logger.UsingCertThumbprint(Microsoft.Extensions.Logging.ILogger! logger, string? certThumbprint) -> void static readonly Microsoft.Identity.Web.Constants.s_certificateRelatedErrorCodes -> System.Collections.Generic.HashSet! +static readonly Microsoft.Identity.Web.Constants.s_nonRetryableConfigErrorCodes -> System.Collections.Generic.HashSet! diff --git a/src/Microsoft.Identity.Web.TokenAcquisition/PublicAPI/net8.0/InternalAPI.Unshipped.txt b/src/Microsoft.Identity.Web.TokenAcquisition/PublicAPI/net8.0/InternalAPI.Unshipped.txt index a238605bc..ec57a0a8d 100644 --- a/src/Microsoft.Identity.Web.TokenAcquisition/PublicAPI/net8.0/InternalAPI.Unshipped.txt +++ b/src/Microsoft.Identity.Web.TokenAcquisition/PublicAPI/net8.0/InternalAPI.Unshipped.txt @@ -1,7 +1,10 @@ #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! Microsoft.Identity.Web.TokenAcquisitionExtensionOptions.InvokeOnBeforeTokenAcquisitionForOnBehalfOf(Microsoft.Identity.Client.AcquireTokenOnBehalfOfParameterBuilder! builder, Microsoft.Identity.Abstractions.AcquireTokenOptions? acquireTokenOptions, System.Security.Claims.ClaimsPrincipal! user) -> void Microsoft.Identity.Web.TokenAcquisitionExtensionOptions.InvokeOnBeforeTokenAcquisitionForOnBehalfOfAsync(Microsoft.Identity.Client.AcquireTokenOnBehalfOfParameterBuilder! builder, Microsoft.Identity.Abstractions.AcquireTokenOptions? acquireTokenOptions, System.Security.Claims.ClaimsPrincipal! user) -> System.Threading.Tasks.Task! static Microsoft.Identity.Web.ConfidentialClientApplicationBuilderExtension.Logger.UsingCertThumbprint(Microsoft.Extensions.Logging.ILogger! logger, string? certThumbprint) -> void static readonly Microsoft.Identity.Web.Constants.s_certificateRelatedErrorCodes -> System.Collections.Generic.HashSet! +static readonly Microsoft.Identity.Web.Constants.s_nonRetryableConfigErrorCodes -> System.Collections.Generic.HashSet! diff --git a/src/Microsoft.Identity.Web.TokenAcquisition/PublicAPI/net9.0/InternalAPI.Unshipped.txt b/src/Microsoft.Identity.Web.TokenAcquisition/PublicAPI/net9.0/InternalAPI.Unshipped.txt index a238605bc..ec57a0a8d 100644 --- a/src/Microsoft.Identity.Web.TokenAcquisition/PublicAPI/net9.0/InternalAPI.Unshipped.txt +++ b/src/Microsoft.Identity.Web.TokenAcquisition/PublicAPI/net9.0/InternalAPI.Unshipped.txt @@ -1,7 +1,10 @@ #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! Microsoft.Identity.Web.TokenAcquisitionExtensionOptions.InvokeOnBeforeTokenAcquisitionForOnBehalfOf(Microsoft.Identity.Client.AcquireTokenOnBehalfOfParameterBuilder! builder, Microsoft.Identity.Abstractions.AcquireTokenOptions? acquireTokenOptions, System.Security.Claims.ClaimsPrincipal! user) -> void Microsoft.Identity.Web.TokenAcquisitionExtensionOptions.InvokeOnBeforeTokenAcquisitionForOnBehalfOfAsync(Microsoft.Identity.Client.AcquireTokenOnBehalfOfParameterBuilder! builder, Microsoft.Identity.Abstractions.AcquireTokenOptions? acquireTokenOptions, System.Security.Claims.ClaimsPrincipal! user) -> System.Threading.Tasks.Task! static Microsoft.Identity.Web.ConfidentialClientApplicationBuilderExtension.Logger.UsingCertThumbprint(Microsoft.Extensions.Logging.ILogger! logger, string? certThumbprint) -> void static readonly Microsoft.Identity.Web.Constants.s_certificateRelatedErrorCodes -> System.Collections.Generic.HashSet! +static readonly Microsoft.Identity.Web.Constants.s_nonRetryableConfigErrorCodes -> System.Collections.Generic.HashSet! diff --git a/src/Microsoft.Identity.Web.TokenAcquisition/PublicAPI/netstandard2.0/InternalAPI.Unshipped.txt b/src/Microsoft.Identity.Web.TokenAcquisition/PublicAPI/netstandard2.0/InternalAPI.Unshipped.txt index a238605bc..ec57a0a8d 100644 --- a/src/Microsoft.Identity.Web.TokenAcquisition/PublicAPI/netstandard2.0/InternalAPI.Unshipped.txt +++ b/src/Microsoft.Identity.Web.TokenAcquisition/PublicAPI/netstandard2.0/InternalAPI.Unshipped.txt @@ -1,7 +1,10 @@ #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! Microsoft.Identity.Web.TokenAcquisitionExtensionOptions.InvokeOnBeforeTokenAcquisitionForOnBehalfOf(Microsoft.Identity.Client.AcquireTokenOnBehalfOfParameterBuilder! builder, Microsoft.Identity.Abstractions.AcquireTokenOptions? acquireTokenOptions, System.Security.Claims.ClaimsPrincipal! user) -> void Microsoft.Identity.Web.TokenAcquisitionExtensionOptions.InvokeOnBeforeTokenAcquisitionForOnBehalfOfAsync(Microsoft.Identity.Client.AcquireTokenOnBehalfOfParameterBuilder! builder, Microsoft.Identity.Abstractions.AcquireTokenOptions? acquireTokenOptions, System.Security.Claims.ClaimsPrincipal! user) -> System.Threading.Tasks.Task! static Microsoft.Identity.Web.ConfidentialClientApplicationBuilderExtension.Logger.UsingCertThumbprint(Microsoft.Extensions.Logging.ILogger! logger, string? certThumbprint) -> void static readonly Microsoft.Identity.Web.Constants.s_certificateRelatedErrorCodes -> System.Collections.Generic.HashSet! +static readonly Microsoft.Identity.Web.Constants.s_nonRetryableConfigErrorCodes -> System.Collections.Generic.HashSet! diff --git a/src/Microsoft.Identity.Web.TokenAcquisition/TokenAcquisition.cs b/src/Microsoft.Identity.Web.TokenAcquisition/TokenAcquisition.cs index 060737a84..3c8a163a1 100644 --- a/src/Microsoft.Identity.Web.TokenAcquisition/TokenAcquisition.cs +++ b/src/Microsoft.Identity.Web.TokenAcquisition/TokenAcquisition.cs @@ -54,8 +54,7 @@ class OAuthConstants private readonly ConcurrentDictionary _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; @@ -125,6 +124,13 @@ public TokenAcquisition( #endif public async Task AddAccountToCacheFromAuthorizationCodeAsync( AuthCodeRedemptionParameters authCodeRedemptionParameters) + { + return await AddAccountToCacheFromAuthorizationCodeInternalAsync(authCodeRedemptionParameters, retryCount: 0).ConfigureAwait(false); + } + + private async Task AddAccountToCacheFromAuthorizationCodeInternalAsync( + AuthCodeRedemptionParameters authCodeRedemptionParameters, + int retryCount) { _ = Throws.IfNull(authCodeRedemptionParameters.Scopes); MergedOptions mergedOptions = _tokenAcquisitionHost.GetOptions(authCodeRedemptionParameters.AuthenticationScheme, out string effectiveAuthenticationScheme); @@ -191,26 +197,26 @@ public async Task 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; - } } /// @@ -257,6 +263,25 @@ public async Task 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 GetAuthenticationResultForUserInternalAsync( + IEnumerable scopes, + string? authenticationScheme, + string? tenantId, + string? userFlow, + ClaimsPrincipal? user, + TokenAcquisitionOptions? tokenAcquisitionOptions, + int retryCount) { _ = Throws.IfNull(scopes); @@ -318,22 +343,27 @@ public async Task 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) { @@ -344,10 +374,6 @@ public async Task 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. @@ -544,6 +570,21 @@ public async Task GetAuthenticationResultForAppAsync( string? authenticationScheme = null, string? tenant = null, TokenAcquisitionOptions? tokenAcquisitionOptions = null) + { + return await GetAuthenticationResultForAppInternalAsync( + scope, + authenticationScheme, + tenant, + tokenAcquisitionOptions, + retryCount: 0).ConfigureAwait(false); + } + + private async Task GetAuthenticationResultForAppInternalAsync( + string scope, + string? authenticationScheme, + string? tenant, + TokenAcquisitionOptions? tokenAcquisitionOptions, + int retryCount) { _ = Throws.IfNull(scope); @@ -718,20 +759,25 @@ public async Task 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) { @@ -740,10 +786,6 @@ public async Task GetAuthenticationResultForAppAsync( Logger.TokenAcquisitionError(_logger, ex.Message, ex); throw; } - finally - { - _retryClientCertificate = false; - } } private void AddExtraBodyParametersIfNeeded(TokenAcquisitionOptions tokenAcquisitionOptions, AcquireTokenForClientParameterBuilder builder) @@ -924,8 +966,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; } diff --git a/tests/E2E Tests/AgentApplications/AgentUserIdentityTestscs.cs b/tests/E2E Tests/AgentApplications/AgentUserIdentityTestscs.cs index f1fed66d1..a28f84234 100644 --- a/tests/E2E Tests/AgentApplications/AgentUserIdentityTestscs.cs +++ b/tests/E2E Tests/AgentApplications/AgentUserIdentityTestscs.cs @@ -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; @@ -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( + 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()!; + 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() { diff --git a/tests/Microsoft.Identity.Web.Test/CertificateReloadLogicTests.cs b/tests/Microsoft.Identity.Web.Test/CertificateReloadLogicTests.cs index c32abac3c..395bda1f4 100644 --- a/tests/Microsoft.Identity.Web.Test/CertificateReloadLogicTests.cs +++ b/tests/Microsoft.Identity.Web.Test/CertificateReloadLogicTests.cs @@ -65,27 +65,6 @@ public void IsInvalidClientCertificateOrSignedAssertionError_ReturnsFalseWhenErr Assert.False(result, "Should not trigger reload for non-invalid_client errors"); } - [Fact] - public void IsInvalidClientCertificateOrSignedAssertionError_ReturnsFalseWhenRetryAlreadyInProgress() - { - // Arrange - var tokenAcquisition = CreateTokenAcquisition(); - - // Set _retryClientCertificate to true to simulate retry in progress - var retryField = typeof(TokenAcquisition).GetField("_retryClientCertificate", - BindingFlags.NonPublic | BindingFlags.Instance); - retryField!.SetValue(tokenAcquisition, true); - - var responseBody = $"{{\"error\":\"invalid_client\",\"error_description\":\"Error {Constants.InvalidKeyError}: Test error\"}}"; - var exception = CreateMsalServiceException(InvalidClientErrorCode, responseBody); - - // Act - bool result = InvokeIsInvalidClientCertificateOrSignedAssertionError(tokenAcquisition, exception); - - // Assert - Assert.False(result, "Should not trigger reload when retry is already in progress"); - } - [Fact] public void IsInvalidClientCertificateOrSignedAssertionError_ReturnsFalseForEmptyResponseBody() { diff --git a/tests/Microsoft.Identity.Web.Test/CertificateRetryCounterTests.cs b/tests/Microsoft.Identity.Web.Test/CertificateRetryCounterTests.cs new file mode 100644 index 000000000..f2bb31139 --- /dev/null +++ b/tests/Microsoft.Identity.Web.Test/CertificateRetryCounterTests.cs @@ -0,0 +1,306 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +using System; +using System.Collections.Generic; +using System.Linq; +using System.Reflection; +using System.Security.Claims; +using System.Threading.Tasks; +using Microsoft.Extensions.DependencyInjection; +using Microsoft.Identity.Abstractions; +using Microsoft.Identity.Client; +using Microsoft.Identity.Web.Test.Common; +using Microsoft.Identity.Web.TestOnly; +using Xunit; + +namespace Microsoft.Identity.Web.Test +{ + /// + /// Tests for the certificate retry counter logic to prevent infinite retry loops. + /// This addresses the regression where misconfigured credentials (wrong ClientID/Secret) + /// caused infinite retries when using WithAgentIdentities(). + /// + [Collection(nameof(TokenAcquirerFactorySingletonProtection))] + public class CertificateRetryCounterTests + { + private const string InvalidClientErrorCode = "invalid_client"; + private const int MaxCertificateRetries = 1; + + #region Certificate Error Detection Tests + + [Theory] + [InlineData("AADSTS700016")] // Application not found (wrong ClientID) + [InlineData("AADSTS7000215")] // Invalid client secret + public void IsInvalidClientCertificateOrSignedAssertionError_ReturnsFalseForNonRetryableConfigErrors(string errorCode) + { + // Arrange + var tokenAcquisition = CreateTokenAcquisition(); + var responseBody = $"{{\"error\":\"invalid_client\",\"error_description\":\"Error {errorCode}: Config error\"}}"; + var exception = CreateMsalServiceException(InvalidClientErrorCode, responseBody); + + // Act + bool result = InvokeIsInvalidClientCertificateOrSignedAssertionError(tokenAcquisition, exception); + + // Assert + Assert.False(result, $"Should NOT retry for non-retryable config error: {errorCode}"); + } + + [Theory] + [InlineData("AADSTS700027")] // InvalidKeyError + [InlineData("AADSTS700024")] // SignedAssertionInvalidTimeRange + [InlineData("AADSTS7000214")] // CertificateHasBeenRevoked + [InlineData("AADSTS1000502")] // CertificateIsOutsideValidityWindow + [InlineData("AADSTS7000274")] // ClientAssertionContainsInvalidSignature + [InlineData("AADSTS7000277")] // CertificateWasRevoked + public void IsInvalidClientCertificateOrSignedAssertionError_ReturnsTrueForCertificateErrors(string errorCode) + { + // Arrange + var tokenAcquisition = CreateTokenAcquisition(); + var responseBody = $"{{\"error\":\"invalid_client\",\"error_description\":\"Error {errorCode}: Cert error\"}}"; + var exception = CreateMsalServiceException(InvalidClientErrorCode, responseBody); + + // Act + bool result = InvokeIsInvalidClientCertificateOrSignedAssertionError(tokenAcquisition, exception); + + // Assert + Assert.True(result, $"Should retry for certificate error: {errorCode}"); + } + + #endregion + + #region Regression Test: Infinite Loop Prevention + + /// + /// Regression test: Verifies that bad certificate/config does NOT cause infinite retry loop. + /// This test simulates the exact scenario reported in the bug where .WithAgentIdentities() + /// with wrong ClientID caused the application to hang indefinitely. + /// + [Fact] + public async Task GetAuthenticationResultForAppInternalAsync_DoesNotRetryInfinitelyOnBadConfig() + { + // Arrange + var tokenAcquisition = CreateTokenAcquisition(); + + // Mock MSAL to always throw "Application Not Found" error + var mockMsalException = CreateMsalServiceException( + InvalidClientErrorCode, + "{\"error\":\"invalid_client\",\"error_description\":\"AADSTS700016: Application with identifier 'bad-client-id' was not found.\"}"); + + // Act & Assert + try + { + // This should NOT hang and should throw after MaxCertificateRetries + await Task.Run(async () => + { + // Use reflection to call GetAuthenticationResultForAppInternalAsync with retryCount = 0 + var method = typeof(TokenAcquisition).GetMethod( + "GetAuthenticationResultForAppInternalAsync", + BindingFlags.NonPublic | BindingFlags.Instance); + + if (method == null) + { + throw new InvalidOperationException("Could not find GetAuthenticationResultForAppInternalAsync method"); + } + + // This will throw because we don't have a real MSAL setup, but we're testing the retry logic + // The test is mainly to verify it doesn't hang + var task = method.Invoke(tokenAcquisition, new object?[] + { + "https://graph.microsoft.com/.default", + null, + null, + null, + 0 // Initial retryCount + }) as Task; + + if (task != null) + { + await task; + } + }); + + // If we get here without exception, something is wrong with the test setup + Assert.Fail("Expected an exception to be thrown"); + } + catch (TargetInvocationException ex) when (ex.InnerException != null) + { + // Expected: should throw after max retries + // Verify it's not hanging and completes quickly + Assert.NotNull(ex.InnerException); + // The actual exception type depends on the implementation + } + catch (Exception ex) + { + // Also acceptable - any exception is fine as long as it doesn't hang + Assert.NotNull(ex); + } + + // If we reach here, the method completed (didn't hang) + } + + /// + /// Regression test: Simulates the Agent Identities scenario where nested token acquisitions + /// with bad config should fail quickly, not hang. + /// + [Fact(Timeout = 5000)] // 5 second timeout - if it takes longer, it's likely hanging + public async Task GetAuthenticationResultForAppAsync_WithBadClientId_CompletesQuickly() + { + // Arrange + var tokenAcquisition = CreateTokenAcquisitionWithBadClientId(); + + // Act & Assert + await Assert.ThrowsAnyAsync(async () => + { + // This simulates the Agent Identities scenario + // Should fail quickly, not hang indefinitely + var result = await InvokeGetAuthenticationResultForAppAsync( + tokenAcquisition, + "https://graph.microsoft.com/.default"); + }); + + // If we reach here within the timeout, test passes + Assert.True(true, "Completed without hanging"); + } + + #endregion + + #region Helper Methods + + /// + /// Creates a TokenAcquisition instance for testing. + /// + private TokenAcquisition CreateTokenAcquisition() + { + TokenAcquirerFactoryTesting.ResetTokenAcquirerFactoryInTest(); + var tokenAcquirerFactory = TokenAcquirerFactory.GetDefaultInstance(); + tokenAcquirerFactory.Services.Configure(options => + { + options.Instance = "https://login.microsoftonline.com/"; + options.TenantId = "f645ad92-e38d-4d1a-b510-d1b09a74a8ca"; + options.ClientId = "idu773ld-e38d-jud3-45lk-d1b09a74a8ca"; + options.ClientCredentials = [new CredentialDescription() + { + SourceType = CredentialSource.ClientSecret, + ClientSecret = "someSecret" + }]; + }); + + var serviceProvider = tokenAcquirerFactory.Build(); + + var tokenAcquisition = serviceProvider.GetService(typeof(ITokenAcquisitionInternal)) as TokenAcquisition; + + if (tokenAcquisition == null) + { + throw new InvalidOperationException("Failed to create TokenAcquisition instance for testing"); + } + + return tokenAcquisition; + } + + /// + /// Creates a TokenAcquisition instance with a bad ClientID to simulate the regression scenario. + /// + private TokenAcquisition CreateTokenAcquisitionWithBadClientId() + { + TokenAcquirerFactoryTesting.ResetTokenAcquirerFactoryInTest(); + var tokenAcquirerFactory = TokenAcquirerFactory.GetDefaultInstance(); + tokenAcquirerFactory.Services.Configure(options => + { + options.Instance = "https://login.microsoftonline.com/"; + options.TenantId = "f645ad92-e38d-4d1a-b510-d1b09a74a8ca"; + options.ClientId = "bad-client-id-does-not-exist"; // Invalid ClientID + options.ClientCredentials = [new CredentialDescription() + { + SourceType = CredentialSource.ClientSecret, + ClientSecret = "someSecret" + }]; + }); + + var serviceProvider = tokenAcquirerFactory.Build(); + + var tokenAcquisition = serviceProvider.GetService(typeof(ITokenAcquisitionInternal)) as TokenAcquisition; + + if (tokenAcquisition == null) + { + throw new InvalidOperationException("Failed to create TokenAcquisition instance for testing"); + } + + return tokenAcquisition; + } + + /// + /// Creates a MsalServiceException for testing. + /// + private MsalServiceException CreateMsalServiceException(string errorCode, string responseBody) + { + var exception = new MsalServiceException(errorCode, $"Test exception: {errorCode}"); + + // Set the ResponseBody property using reflection + var responseBodyField = typeof(MsalServiceException).GetProperty("ResponseBody"); + if (responseBodyField != null && responseBodyField.CanWrite) + { + responseBodyField.SetValue(exception, responseBody); + } + else + { + var backingField = typeof(MsalServiceException).GetField("k__BackingField", + BindingFlags.NonPublic | BindingFlags.Instance); + if (backingField != null) + { + backingField.SetValue(exception, responseBody); + } + } + + return exception; + } + + /// + /// Invokes the private IsInvalidClientCertificateOrSignedAssertionError method using reflection. + /// + private bool InvokeIsInvalidClientCertificateOrSignedAssertionError( + TokenAcquisition tokenAcquisition, + MsalServiceException exception) + { + var method = typeof(TokenAcquisition).GetMethod( + "IsInvalidClientCertificateOrSignedAssertionError", + BindingFlags.NonPublic | BindingFlags.Instance); + + if (method == null) + { + throw new InvalidOperationException("Could not find IsInvalidClientCertificateOrSignedAssertionError method"); + } + + var result = method.Invoke(tokenAcquisition, new object[] { exception }); + return (bool)result!; + } + + /// + /// Invokes GetAuthenticationResultForAppAsync using reflection for testing. + /// + private async Task InvokeGetAuthenticationResultForAppAsync( + TokenAcquisition tokenAcquisition, + string scope) + { + var method = typeof(TokenAcquisition).GetMethod( + "GetAuthenticationResultForAppAsync", + BindingFlags.Public | BindingFlags.Instance); + + if (method == null) + { + throw new InvalidOperationException("Could not find GetAuthenticationResultForAppAsync method"); + } + + var task = method.Invoke(tokenAcquisition, new object?[] { scope, null, null, null }) as Task; + + if (task == null) + { + throw new InvalidOperationException("Method did not return a Task"); + } + + return await task; + } + + #endregion + } +}