diff --git a/src/Microsoft.Identity.Web.TokenAcquisition/Constants.cs b/src/Microsoft.Identity.Web.TokenAcquisition/Constants.cs index 0ed55a3cb..2c8145a0a 100644 --- a/src/Microsoft.Identity.Web.TokenAcquisition/Constants.cs +++ b/src/Microsoft.Identity.Web.TokenAcquisition/Constants.cs @@ -1,6 +1,9 @@ -// Copyright (c) Microsoft Corporation. All rights reserved. +// Copyright (c) Microsoft Corporation. All rights reserved. // Licensed under the MIT License. +using System; +using System.Collections.Generic; + namespace Microsoft.Identity.Web { /// @@ -131,6 +134,8 @@ public static class Constants internal const string SignedAssertionInvalidTimeRange = "AADSTS700024"; internal const string CertificateHasBeenRevoked = "AADSTS7000214"; internal const string CertificateIsOutsideValidityWindow = "AADSTS1000502"; + internal const string ClientAssertionContainsInvalidSignature = "AADSTS7000274"; + internal const string CertificateWasRevoked = "AADSTS7000277"; internal const string CiamAuthoritySuffix = ".ciamlogin.com"; internal const string TestSlice = "dc"; internal const string ExtensionOptionsServiceProviderKey = "ID_WEB_INTERNAL_SERVICE_PROVIDER"; @@ -140,6 +145,19 @@ public static class Constants internal const string FmiPathForClientAssertion = "IDWEB_FMI_PATH_FOR_CLIENT_ASSERTION"; internal const string MicrosoftIdentityOptionsParameter = "IDWEB_FMI_MICROSOFT_IDENTITY_OPTIONS"; + /// + /// Error codes indicating certificate or signed assertion issues that warrant retry with a new certificate. + /// + internal static readonly HashSet s_certificateRelatedErrorCodes = new (StringComparer.OrdinalIgnoreCase) + { + InvalidKeyError, // AADSTS700027 - Client assertion contains an invalid signature + SignedAssertionInvalidTimeRange, // AADSTS700024 - Signed assertion invalid time range + CertificateHasBeenRevoked, // AADSTS7000214 - Certificate has been revoked + CertificateIsOutsideValidityWindow, // AADSTS1000502 - Certificate is outside validity window + ClientAssertionContainsInvalidSignature, // AADSTS7000274 - Client assertion contains an invalid signature + CertificateWasRevoked, // AADSTS7000277 - Certificate was revoked + }; + /// /// 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 7dc5c5811..72f068746 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 +1,4 @@ #nullable enable +const Microsoft.Identity.Web.Constants.ClientAssertionContainsInvalidSignature = "AADSTS7000274" -> string! +const Microsoft.Identity.Web.Constants.CertificateWasRevoked = "AADSTS7000277" -> string! +static readonly Microsoft.Identity.Web.Constants.s_certificateRelatedErrorCodes -> 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 7dc5c5811..72f068746 100644 --- a/src/Microsoft.Identity.Web.TokenAcquisition/PublicAPI/net462/InternalAPI.Unshipped.txt +++ b/src/Microsoft.Identity.Web.TokenAcquisition/PublicAPI/net462/InternalAPI.Unshipped.txt @@ -1 +1,4 @@ #nullable enable +const Microsoft.Identity.Web.Constants.ClientAssertionContainsInvalidSignature = "AADSTS7000274" -> string! +const Microsoft.Identity.Web.Constants.CertificateWasRevoked = "AADSTS7000277" -> string! +static readonly Microsoft.Identity.Web.Constants.s_certificateRelatedErrorCodes -> 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 7dc5c5811..72f068746 100644 --- a/src/Microsoft.Identity.Web.TokenAcquisition/PublicAPI/net472/InternalAPI.Unshipped.txt +++ b/src/Microsoft.Identity.Web.TokenAcquisition/PublicAPI/net472/InternalAPI.Unshipped.txt @@ -1 +1,4 @@ #nullable enable +const Microsoft.Identity.Web.Constants.ClientAssertionContainsInvalidSignature = "AADSTS7000274" -> string! +const Microsoft.Identity.Web.Constants.CertificateWasRevoked = "AADSTS7000277" -> string! +static readonly Microsoft.Identity.Web.Constants.s_certificateRelatedErrorCodes -> 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 7dc5c5811..72f068746 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 +1,4 @@ #nullable enable +const Microsoft.Identity.Web.Constants.ClientAssertionContainsInvalidSignature = "AADSTS7000274" -> string! +const Microsoft.Identity.Web.Constants.CertificateWasRevoked = "AADSTS7000277" -> string! +static readonly Microsoft.Identity.Web.Constants.s_certificateRelatedErrorCodes -> 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 7dc5c5811..72f068746 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 +1,4 @@ #nullable enable +const Microsoft.Identity.Web.Constants.ClientAssertionContainsInvalidSignature = "AADSTS7000274" -> string! +const Microsoft.Identity.Web.Constants.CertificateWasRevoked = "AADSTS7000277" -> string! +static readonly Microsoft.Identity.Web.Constants.s_certificateRelatedErrorCodes -> 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 7dc5c5811..72f068746 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 +1,4 @@ #nullable enable +const Microsoft.Identity.Web.Constants.ClientAssertionContainsInvalidSignature = "AADSTS7000274" -> string! +const Microsoft.Identity.Web.Constants.CertificateWasRevoked = "AADSTS7000277" -> string! +static readonly Microsoft.Identity.Web.Constants.s_certificateRelatedErrorCodes -> System.Collections.Generic.HashSet! diff --git a/src/Microsoft.Identity.Web.TokenAcquisition/TokenAcquisition.cs b/src/Microsoft.Identity.Web.TokenAcquisition/TokenAcquisition.cs index 279fdef8f..55436d560 100644 --- a/src/Microsoft.Identity.Web.TokenAcquisition/TokenAcquisition.cs +++ b/src/Microsoft.Identity.Web.TokenAcquisition/TokenAcquisition.cs @@ -901,13 +901,33 @@ public async Task RemoveAccountAsync( private bool IsInvalidClientCertificateOrSignedAssertionError(MsalServiceException exMsal) { - return !_retryClientCertificate && - string.Equals(exMsal.ErrorCode, Constants.InvalidClient, StringComparison.OrdinalIgnoreCase) && - !exMsal.ResponseBody.Contains("AADSTS7000215" // No retry when wrong client secret. + if (_retryClientCertificate || + !string.Equals(exMsal.ErrorCode, Constants.InvalidClient, StringComparison.OrdinalIgnoreCase)) + { + return false; + } + + string responseBody = exMsal.ResponseBody; + #if NET6_0_OR_GREATER - , StringComparison.OrdinalIgnoreCase + foreach (var errorCode in Constants.s_certificateRelatedErrorCodes) + { + if (responseBody.Contains(errorCode, StringComparison.OrdinalIgnoreCase)) + { + return true; + } + } + return false; +#else + foreach (var errorCode in Constants.s_certificateRelatedErrorCodes) + { + if (responseBody.IndexOf(errorCode, StringComparison.OrdinalIgnoreCase) >= 0) + { + return true; + } + } + return false; #endif - ); } diff --git a/tests/Microsoft.Identity.Web.Test/CertificateReloadLogicTests.cs b/tests/Microsoft.Identity.Web.Test/CertificateReloadLogicTests.cs new file mode 100644 index 000000000..c32abac3c --- /dev/null +++ b/tests/Microsoft.Identity.Web.Test/CertificateReloadLogicTests.cs @@ -0,0 +1,218 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +using System; +using System.Collections.Generic; +using System.Reflection; +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 reload logic to ensure it only triggers on certificate-related errors. + /// This addresses the regression from PR #3430 where reload was triggered on all invalid_client errors. + /// + [Collection(nameof(TokenAcquirerFactorySingletonProtection))] + public class CertificateReloadLogicTests + { + private const string InvalidClientErrorCode = "invalid_client"; + + [Theory] + [InlineData("AADSTS700027", true)] // InvalidKeyError + [InlineData("AADSTS700024", true)] // SignedAssertionInvalidTimeRange + [InlineData("AADSTS7000214", true)] // CertificateHasBeenRevoked + [InlineData("AADSTS1000502", true)] // CertificateIsOutsideValidityWindow + [InlineData("AADSTS7000274", true)] // ClientAssertionContainsInvalidSignature + [InlineData("AADSTS7000277", true)] // CertificateWasRevoked + [InlineData("AADSTS7000215", false)] // Invalid client secret - should NOT trigger reload + [InlineData("AADSTS700016", false)] // Application not found - should NOT trigger reload + [InlineData("AADSTS7000222", false)] // Invalid client secret (expired) - should NOT trigger reload + [InlineData("AADSTS50011", false)] // Invalid reply address - should NOT trigger reload + [InlineData("AADSTS50012", false)] // Invalid client credentials - should NOT trigger reload + public void IsInvalidClientCertificateOrSignedAssertionError_ReturnsTrueOnlyForCertificateErrors( + string errorCode, + bool shouldTriggerReload) + { + // Arrange + var tokenAcquisition = CreateTokenAcquisition(); + var responseBody = $"{{\"error\":\"invalid_client\",\"error_description\":\"Error {errorCode}: Test error\"}}"; + var exception = CreateMsalServiceException(InvalidClientErrorCode, responseBody); + + // Act + bool result = InvokeIsInvalidClientCertificateOrSignedAssertionError(tokenAcquisition, exception); + + // Assert + Assert.Equal(shouldTriggerReload, result); + } + + [Fact] + public void IsInvalidClientCertificateOrSignedAssertionError_ReturnsFalseWhenErrorCodeIsNotInvalidClient() + { + // Arrange + var tokenAcquisition = CreateTokenAcquisition(); + var responseBody = "{\"error\":\"unauthorized_client\",\"error_description\":\"Test error\"}"; + var exception = CreateMsalServiceException("unauthorized_client", responseBody); + + // Act + bool result = InvokeIsInvalidClientCertificateOrSignedAssertionError(tokenAcquisition, exception); + + // Assert + 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() + { + // Arrange + var tokenAcquisition = CreateTokenAcquisition(); + var exception = CreateMsalServiceException(InvalidClientErrorCode, string.Empty); + + // Act + bool result = InvokeIsInvalidClientCertificateOrSignedAssertionError(tokenAcquisition, exception); + + // Assert + Assert.False(result, "Should not trigger reload when response body is empty"); + } + + [Theory] + [InlineData("AADSTS700027")] // Case sensitive check - should still work + [InlineData("aadsts700027")] // Lowercase + [InlineData("AaDsTs700027")] // Mixed case + public void IsInvalidClientCertificateOrSignedAssertionError_IsCaseInsensitive(string errorCodeCase) + { + // Arrange + var tokenAcquisition = CreateTokenAcquisition(); + var responseBody = $"{{\"error\":\"invalid_client\",\"error_description\":\"Error {errorCodeCase}: Test error\"}}"; + var exception = CreateMsalServiceException(InvalidClientErrorCode, responseBody); + + // Act + bool result = InvokeIsInvalidClientCertificateOrSignedAssertionError(tokenAcquisition, exception); + + // Assert + Assert.True(result, $"Should trigger reload regardless of case: {errorCodeCase}"); + } + + [Fact] + public void IsInvalidClientCertificateOrSignedAssertionError_WorksWithMultipleErrorCodesInResponse() + { + // Arrange + var tokenAcquisition = CreateTokenAcquisition(); + // Response might contain multiple error codes or descriptions + var responseBody = $"{{\"error\":\"invalid_client\",\"error_description\":\"Error {Constants.CertificateHasBeenRevoked}: Certificate has been revoked. Also note AADSTS7000215 in logs.\"}}"; + var exception = CreateMsalServiceException(InvalidClientErrorCode, responseBody); + + // Act + bool result = InvokeIsInvalidClientCertificateOrSignedAssertionError(tokenAcquisition, exception); + + // Assert + Assert.True(result, "Should trigger reload when certificate error code is present, even if other codes are also mentioned"); + } + + /// + /// 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(); + + // Get the TokenAcquisition instance from the service provider + 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) + { + // Use the MsalServiceException constructor with errorCode and errorMessage + // The ResponseBody property is internal but can be accessed via reflection + 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 + { + // Try the backing field instead + 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!; + } + } +} diff --git a/tests/Microsoft.Identity.Web.Test/CertificatesObserverTests.cs b/tests/Microsoft.Identity.Web.Test/CertificatesObserverTests.cs index 7f337c34e..9d6f82844 100644 --- a/tests/Microsoft.Identity.Web.Test/CertificatesObserverTests.cs +++ b/tests/Microsoft.Identity.Web.Test/CertificatesObserverTests.cs @@ -332,8 +332,8 @@ protected override Task SendAsync(HttpRequestMessage reques var errorResponse = new { error = "invalid_client", - error_description = $"Invalid certificate: {this.description.CachedValue}", - error_codes = new[] { 50000 }, + error_description = $"AADSTS700027: Invalid certificate: {this.description.CachedValue}", + error_codes = new[] { 700027 }, timestamp = DateTime.UtcNow, };