From f8f5512a546a508c1a18f5ac166db868b17d6c8e Mon Sep 17 00:00:00 2001 From: Dominique St-Amand Date: Tue, 5 Nov 2024 15:08:06 -0500 Subject: [PATCH 1/9] Add InternalClaimDetectedException for ID Token validation Introduce InternalClaimDetectedException in Microsoft.Identity.Web and Microsoft.Identity.Web.OWIN namespaces. This exception is thrown when internal ID Token claims (UniqueTenantIdentifier, UniqueObjectIdentifier) are detected in the user's ID Token. Updated AppBuilderExtension.cs and MicrosoftIdentityWebAppAuthenticationBuilder.cs to check for these claims and throw the exception if found. The new exception class includes a property to store the invalid claim --- .../AppBuilderExtension.cs | 34 +++++++++++++++++++ .../InternalClaimDetectedException.cs | 31 +++++++++++++++++ .../InternalClaimDetectedException.cs | 31 +++++++++++++++++ ...softIdentityWebAppAuthenticationBuilder.cs | 25 ++++++++++++-- 4 files changed, 119 insertions(+), 2 deletions(-) create mode 100644 src/Microsoft.Identity.Web.OWIN/InternalClaimDetectedException.cs create mode 100644 src/Microsoft.Identity.Web/InternalClaimDetectedException.cs diff --git a/src/Microsoft.Identity.Web.OWIN/AppBuilderExtension.cs b/src/Microsoft.Identity.Web.OWIN/AppBuilderExtension.cs index 55083dbc6..5638899ce 100644 --- a/src/Microsoft.Identity.Web.OWIN/AppBuilderExtension.cs +++ b/src/Microsoft.Identity.Web.OWIN/AppBuilderExtension.cs @@ -175,6 +175,23 @@ public static IAppBuilder AddMicrosoftIdentityWebApp( if (clientInfoFromServer != null && clientInfoFromServer.UniqueTenantIdentifier != null && clientInfoFromServer.UniqueObjectIdentifier != null) { + var uniqueTenantIdentifierClaim = context.AuthenticationTicket.Identity.FindFirst(c => c.Type == ClaimConstants.UniqueTenantIdentifier); + var uniqueObjectIdentifierClaim = context.AuthenticationTicket.Identity.FindFirst(c => c.Type == ClaimConstants.UniqueObjectIdentifier); + if (uniqueTenantIdentifierClaim != null) + { + throw new InternalClaimDetectedException($"The claim \"{ClaimConstants.UniqueTenantIdentifier}\" is reserved for internal use by this library. To ensure proper functionality and avoid conflicts, please remove or rename this claim in your ID Token.") + { + Claim = uniqueTenantIdentifierClaim, + }; + } + if (uniqueObjectIdentifierClaim != null) + { + throw new InternalClaimDetectedException($"The claim \"{ClaimConstants.UniqueObjectIdentifier}\" is reserved for internal use by this library. To ensure proper functionality and avoid conflicts, please remove or rename this claim in your ID Token.") + { + Claim = uniqueObjectIdentifierClaim + }; + } + context.AuthenticationTicket.Identity.AddClaim(new Claim(ClaimConstants.UniqueTenantIdentifier, clientInfoFromServer.UniqueTenantIdentifier)); context.AuthenticationTicket.Identity.AddClaim(new Claim(ClaimConstants.UniqueObjectIdentifier, clientInfoFromServer.UniqueObjectIdentifier)); } @@ -187,6 +204,23 @@ public static IAppBuilder AddMicrosoftIdentityWebApp( if (clientInfoFromServer != null && clientInfoFromServer.UniqueTenantIdentifier != null && clientInfoFromServer.UniqueObjectIdentifier != null) { + var uniqueTenantIdentifierClaim = context.AuthenticationTicket.Identity.FindFirst(c => c.Type == ClaimConstants.UniqueTenantIdentifier); + var uniqueObjectIdentifierClaim = context.AuthenticationTicket.Identity.FindFirst(c => c.Type == ClaimConstants.UniqueObjectIdentifier); + if (uniqueTenantIdentifierClaim != null) + { + throw new InternalClaimDetectedException($"The claim \"{ClaimConstants.UniqueTenantIdentifier}\" is reserved for internal use by this library. To ensure proper functionality and avoid conflicts, please remove or rename this claim in your ID Token.") + { + Claim = uniqueTenantIdentifierClaim + }; + } + if (uniqueObjectIdentifierClaim != null) + { + throw new InternalClaimDetectedException($"The claim \"{ClaimConstants.UniqueObjectIdentifier}\" is reserved for internal use by this library. To ensure proper functionality and avoid conflicts, please remove or rename this claim in your ID Token.") + { + Claim = uniqueObjectIdentifierClaim + }; + } + context.AuthenticationTicket.Identity.AddClaim(new Claim(ClaimConstants.UniqueTenantIdentifier, clientInfoFromServer.UniqueTenantIdentifier)); context.AuthenticationTicket.Identity.AddClaim(new Claim(ClaimConstants.UniqueObjectIdentifier, clientInfoFromServer.UniqueObjectIdentifier)); } diff --git a/src/Microsoft.Identity.Web.OWIN/InternalClaimDetectedException.cs b/src/Microsoft.Identity.Web.OWIN/InternalClaimDetectedException.cs new file mode 100644 index 000000000..69ff2fb95 --- /dev/null +++ b/src/Microsoft.Identity.Web.OWIN/InternalClaimDetectedException.cs @@ -0,0 +1,31 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +using System; +using System.Security.Claims; + +namespace Microsoft.Identity.Web.OWIN +{ + /// + /// The exception that is thrown when an internal ID Token claim used by Microsoft.Identity.Web internally is detected in the user's ID Token. + /// + public class InternalClaimDetectedException : Exception + { + /// + /// Gets or sets the invalid claim. + /// + public Claim Claim { get; set; } + + public InternalClaimDetectedException() + { + } + + public InternalClaimDetectedException(string message) : base(message) + { + } + + public InternalClaimDetectedException(string message, Exception innerException) : base(message, innerException) + { + } + } +} diff --git a/src/Microsoft.Identity.Web/InternalClaimDetectedException.cs b/src/Microsoft.Identity.Web/InternalClaimDetectedException.cs new file mode 100644 index 000000000..bbd7b0908 --- /dev/null +++ b/src/Microsoft.Identity.Web/InternalClaimDetectedException.cs @@ -0,0 +1,31 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +using System; +using System.Security.Claims; + +namespace Microsoft.Identity.Web +{ + /// + /// The exception that is thrown when an internal ID Token claim used by Microsoft.Identity.Web internally is detected in the user's ID Token. + /// + public class InternalClaimDetectedException : Exception + { + /// + /// Gets or sets the invalid claim. + /// + public Claim Claim { get; set; } + + public InternalClaimDetectedException() + { + } + + public InternalClaimDetectedException(string message) : base(message) + { + } + + public InternalClaimDetectedException(string message, Exception innerException) : base(message, innerException) + { + } + } +} diff --git a/src/Microsoft.Identity.Web/WebAppExtensions/MicrosoftIdentityWebAppAuthenticationBuilder.cs b/src/Microsoft.Identity.Web/WebAppExtensions/MicrosoftIdentityWebAppAuthenticationBuilder.cs index d43edc207..565dab016 100644 --- a/src/Microsoft.Identity.Web/WebAppExtensions/MicrosoftIdentityWebAppAuthenticationBuilder.cs +++ b/src/Microsoft.Identity.Web/WebAppExtensions/MicrosoftIdentityWebAppAuthenticationBuilder.cs @@ -179,8 +179,29 @@ internal static void WebAppCallsWebApiImplementation( if (clientInfoFromServer != null && clientInfoFromServer.UniqueTenantIdentifier != null && clientInfoFromServer.UniqueObjectIdentifier != null) { - context!.Principal!.Identities.FirstOrDefault()?.AddClaim(new Claim(ClaimConstants.UniqueTenantIdentifier, clientInfoFromServer.UniqueTenantIdentifier)); - context!.Principal!.Identities.FirstOrDefault()?.AddClaim(new Claim(ClaimConstants.UniqueObjectIdentifier, clientInfoFromServer.UniqueObjectIdentifier)); + var identity = context!.Principal!.Identities.FirstOrDefault(); + if (identity != null) + { + var uniqueTenantIdentifierClaim = identity.FindFirst(c => c.Type == ClaimConstants.UniqueTenantIdentifier); + var uniqueObjectIdentifierClaim = identity.FindFirst(c => c.Type == ClaimConstants.UniqueObjectIdentifier); + if (uniqueTenantIdentifierClaim != null) + { + throw new InternalClaimDetectedException($"The claim \"{ClaimConstants.UniqueTenantIdentifier}\" is reserved for internal use by this library. To ensure proper functionality and avoid conflicts, please remove or rename this claim in your ID Token.") + { + Claim = uniqueTenantIdentifierClaim + }; + } + if (uniqueObjectIdentifierClaim != null) + { + throw new InternalClaimDetectedException($"The claim \"{ClaimConstants.UniqueObjectIdentifier}\" is reserved for internal use by this library. To ensure proper functionality and avoid conflicts, please remove or rename this claim in your ID Token.") + { + Claim = uniqueObjectIdentifierClaim + }; + } + + identity.AddClaim(new Claim(ClaimConstants.UniqueTenantIdentifier, clientInfoFromServer.UniqueTenantIdentifier)); + identity.AddClaim(new Claim(ClaimConstants.UniqueObjectIdentifier, clientInfoFromServer.UniqueObjectIdentifier)); + } } } await onTokenValidatedHandler(context).ConfigureAwait(false); From 6be209b40b6af9edf7e5050fececb4a1be8c5b51 Mon Sep 17 00:00:00 2001 From: Dominique St-Amand Date: Tue, 5 Nov 2024 18:40:15 -0500 Subject: [PATCH 2/9] Add unit tests for the modifications --- .../Microsoft.Identity.Web.OWIN.xml | 30 +++-- ...softIdentityWebAppAuthenticationBuilder.cs | 10 +- .../TestHelpers/HttpContextUtilities.cs | 10 ++ .../WebAppExtensionsTests.cs | 118 +++++++++++++++++- 4 files changed, 151 insertions(+), 17 deletions(-) diff --git a/src/Microsoft.Identity.Web.OWIN/Microsoft.Identity.Web.OWIN.xml b/src/Microsoft.Identity.Web.OWIN/Microsoft.Identity.Web.OWIN.xml index 8c0da7f69..8dd5fdc8a 100644 --- a/src/Microsoft.Identity.Web.OWIN/Microsoft.Identity.Web.OWIN.xml +++ b/src/Microsoft.Identity.Web.OWIN/Microsoft.Identity.Web.OWIN.xml @@ -79,21 +79,15 @@ - + - Gets the issuer the credentials are for. + The exception that is thrown when an internal ID Token claim used by Microsoft.Identity.Web internally is detected in the user's ID Token. - - The issuer the credentials are for. - - + - Gets all known security keys. + Gets or sets the invalid claim. - - All known security keys. - @@ -112,5 +106,21 @@ Pre-build action. Ensures that the host is an OWIN host. + + + Gets the issuer the credentials are for. + + + The issuer the credentials are for. + + + + + Gets all known security keys. + + + All known security keys. + + diff --git a/src/Microsoft.Identity.Web/WebAppExtensions/MicrosoftIdentityWebAppAuthenticationBuilder.cs b/src/Microsoft.Identity.Web/WebAppExtensions/MicrosoftIdentityWebAppAuthenticationBuilder.cs index 565dab016..b986b86b4 100644 --- a/src/Microsoft.Identity.Web/WebAppExtensions/MicrosoftIdentityWebAppAuthenticationBuilder.cs +++ b/src/Microsoft.Identity.Web/WebAppExtensions/MicrosoftIdentityWebAppAuthenticationBuilder.cs @@ -186,17 +186,19 @@ internal static void WebAppCallsWebApiImplementation( var uniqueObjectIdentifierClaim = identity.FindFirst(c => c.Type == ClaimConstants.UniqueObjectIdentifier); if (uniqueTenantIdentifierClaim != null) { - throw new InternalClaimDetectedException($"The claim \"{ClaimConstants.UniqueTenantIdentifier}\" is reserved for internal use by this library. To ensure proper functionality and avoid conflicts, please remove or rename this claim in your ID Token.") + context.Fail(new InternalClaimDetectedException($"The claim \"{ClaimConstants.UniqueTenantIdentifier}\" is reserved for internal use by this library. To ensure proper functionality and avoid conflicts, please remove or rename this claim in your ID Token.") { Claim = uniqueTenantIdentifierClaim - }; + }); + return; } if (uniqueObjectIdentifierClaim != null) { - throw new InternalClaimDetectedException($"The claim \"{ClaimConstants.UniqueObjectIdentifier}\" is reserved for internal use by this library. To ensure proper functionality and avoid conflicts, please remove or rename this claim in your ID Token.") + context.Fail(new InternalClaimDetectedException($"The claim \"{ClaimConstants.UniqueObjectIdentifier}\" is reserved for internal use by this library. To ensure proper functionality and avoid conflicts, please remove or rename this claim in your ID Token.") { Claim = uniqueObjectIdentifierClaim - }; + }); + return; } identity.AddClaim(new Claim(ClaimConstants.UniqueTenantIdentifier, clientInfoFromServer.UniqueTenantIdentifier)); diff --git a/tests/Microsoft.Identity.Web.Test.Common/TestHelpers/HttpContextUtilities.cs b/tests/Microsoft.Identity.Web.Test.Common/TestHelpers/HttpContextUtilities.cs index 7c64e72ab..87cf55e06 100644 --- a/tests/Microsoft.Identity.Web.Test.Common/TestHelpers/HttpContextUtilities.cs +++ b/tests/Microsoft.Identity.Web.Test.Common/TestHelpers/HttpContextUtilities.cs @@ -1,6 +1,7 @@ // Copyright (c) Microsoft Corporation. All rights reserved. // Licensed under the MIT License. +using System.Collections.Generic; using System.IO; using System.Security.Claims; using Microsoft.AspNetCore.Http; @@ -44,5 +45,14 @@ public static HttpContext CreateHttpContext( return httpContext; } + + public static HttpContext CreateHttpContext(IEnumerable claims) + { + var httpContext = CreateHttpContext(); + + httpContext.User = new ClaimsPrincipal(new CaseSensitiveClaimsIdentity(claims)); + + return httpContext; + } } } diff --git a/tests/Microsoft.Identity.Web.Test/WebAppExtensionsTests.cs b/tests/Microsoft.Identity.Web.Test/WebAppExtensionsTests.cs index 3a5cdc8c9..94826aacc 100644 --- a/tests/Microsoft.Identity.Web.Test/WebAppExtensionsTests.cs +++ b/tests/Microsoft.Identity.Web.Test/WebAppExtensionsTests.cs @@ -20,7 +20,6 @@ using Microsoft.Extensions.DependencyInjection.Extensions; using Microsoft.Extensions.Hosting; using Microsoft.Extensions.Hosting.Internal; -using Microsoft.Extensions.Logging; using Microsoft.Extensions.Options; using Microsoft.Graph; using Microsoft.Identity.Client; @@ -360,6 +359,53 @@ public async Task AddMicrosoftIdentityWebAppCallsWebApi_WithConfigNameParameters await AddMicrosoftIdentityWebAppCallsWebApi_TestRedirectToIdentityProviderForSignOutEventAsync(provider, oidcOptions, redirectFuncMock, tokenAcquisitionMock); } + [Theory] + [InlineData("uid","user-uid")] + [InlineData("utid","user-utid")] + public async Task AddMicrosoftIdentityWebAppCallsWebApi_WithConfigNameParametersAsync_ShouldThrowExceptionForInternalClaims(string claimType, string claimValue) + { + var configMock = Substitute.For(); + configMock.Configure().GetSection(ConfigSectionName).Returns(_configSection); + var initialScopes = new List() { "custom_scope" }; + var tokenAcquisitionMock = Substitute.For(); + var authCodeReceivedFuncMock = Substitute.For>(); + var tokenValidatedFuncMock = Substitute.For>(); + var redirectFuncMock = Substitute.For>(); + var services = new ServiceCollection(); + + services.AddSingleton((provider) => _env) + .AddSingleton(configMock); + + services.AddAuthentication() + .AddMicrosoftIdentityWebApp(configMock, ConfigSectionName, OidcScheme) + .EnableTokenAcquisitionToCallDownstreamApi(initialScopes); + services.Configure(OidcScheme, (options) => + { + options.Events ??= new OpenIdConnectEvents(); + options.Events.OnAuthorizationCodeReceived += authCodeReceivedFuncMock; + options.Events.OnTokenValidated += tokenValidatedFuncMock; + options.Events.OnRedirectToIdentityProviderForSignOut += redirectFuncMock; + }); + + services.RemoveAll(); + services.AddScoped((provider) => tokenAcquisitionMock); + + var provider = services.BuildServiceProvider(); + + // Assert config bind actions added correctly + provider.GetRequiredService>().Get(OidcScheme); + provider.GetRequiredService>().Get(OidcScheme); + + configMock.Received(1).GetSection(ConfigSectionName); + + var oidcOptions = provider.GetRequiredService>().Get(OidcScheme); + + AddMicrosoftIdentityWebAppCallsWebApi_TestCommon(services, provider, oidcOptions, initialScopes); + await AddMicrosoftIdentityWebAppCallsWebApi_TestAuthorizationCodeReceivedEventAsync(provider, oidcOptions, authCodeReceivedFuncMock, tokenAcquisitionMock); + await AddMicrosoftIdentityWebAppCallsWebApi_TestTokenValidatedEventAsync(provider, oidcOptions, new Claim[] { new Claim(claimType, claimValue) }); + await AddMicrosoftIdentityWebAppCallsWebApi_TestRedirectToIdentityProviderForSignOutEventAsync(provider, oidcOptions, redirectFuncMock, tokenAcquisitionMock); + } + [Fact] public async Task AddMicrosoftIdentityWebAppCallsWebApi_WithConfigActionParametersAsync() { @@ -405,6 +451,53 @@ public async Task AddMicrosoftIdentityWebAppCallsWebApi_WithConfigActionParamete await AddMicrosoftIdentityWebAppCallsWebApi_TestRedirectToIdentityProviderForSignOutEventAsync(provider, oidcOptions, redirectFuncMock, tokenAcquisitionMock); } + [Theory] + [InlineData("uid", "user-uid")] + [InlineData("utid", "user-utid")] + public async Task AddMicrosoftIdentityWebAppCallsWebApi_WithConfigActionParametersAsync_ShouldThrowExceptionForInternalClaims(string claimType, string claimValue) + { + var configMock = Substitute.For(); + var initialScopes = new List() { "custom_scope" }; + var tokenAcquisitionMock = Substitute.For(); + var authCodeReceivedFuncMock = Substitute.For>(); + var tokenValidatedFuncMock = Substitute.For>(); + var redirectFuncMock = Substitute.For>(); + + var services = new ServiceCollection(); + services.AddSingleton(configMock); + services.AddSingleton((provider) => _env); + + var builder = services.AddAuthentication() + .AddMicrosoftIdentityWebApp(_configureMsOptions, null, OidcScheme) + .EnableTokenAcquisitionToCallDownstreamApi(_configureAppOptions, initialScopes); + services.Configure(OidcScheme, (options) => + { + options.Events ??= new OpenIdConnectEvents(); + options.Events.OnAuthorizationCodeReceived += authCodeReceivedFuncMock; + options.Events.OnTokenValidated += tokenValidatedFuncMock; + options.Events.OnRedirectToIdentityProviderForSignOut += redirectFuncMock; + }); + + services.RemoveAll(); + services.AddScoped((provider) => tokenAcquisitionMock); + + var provider = builder.Services.BuildServiceProvider(); + + // Assert configure options actions added correctly + var configuredAppOptions = provider.GetServices>().Cast>(); + var configuredMsOptions = provider.GetServices>().Cast>(); + + Assert.Contains(configuredAppOptions, o => o.Action == _configureAppOptions); + Assert.Contains(configuredMsOptions, o => o.Action == _configureMsOptions); + + var oidcOptions = provider.GetRequiredService>().Create(OidcScheme); + + AddMicrosoftIdentityWebAppCallsWebApi_TestCommon(services, provider, oidcOptions, initialScopes); + await AddMicrosoftIdentityWebAppCallsWebApi_TestAuthorizationCodeReceivedEventAsync(provider, oidcOptions, authCodeReceivedFuncMock, tokenAcquisitionMock); + await AddMicrosoftIdentityWebAppCallsWebApi_TestTokenValidatedEventAsync(provider, oidcOptions, new Claim[] { new Claim(claimType, claimValue) }); + await AddMicrosoftIdentityWebAppCallsWebApi_TestRedirectToIdentityProviderForSignOutEventAsync(provider, oidcOptions, redirectFuncMock, tokenAcquisitionMock); + } + [Fact] public void AddMicrosoftIdentityWebAppCallsWebApi_NoScopes() { @@ -853,6 +946,25 @@ private async Task AddMicrosoftIdentityWebAppCallsWebApi_TestTokenValidatedEvent Assert.True(tokenValidatedContext?.Principal?.HasClaim(c => c.Type == ClaimConstants.UniqueObjectIdentifier)); } + private async Task AddMicrosoftIdentityWebAppCallsWebApi_TestTokenValidatedEventAsync(IServiceProvider provider, OpenIdConnectOptions oidcOptions, IEnumerable? claims) + { + var (httpContext, authScheme, authProperties) = CreateContextParameters(provider, claims); + + var tokenValidatedContext = new TokenValidatedContext(httpContext, authScheme, oidcOptions, httpContext.User, authProperties) + { + ProtocolMessage = new OpenIdConnectMessage( + new Dictionary() + { + { ClaimConstants.ClientInfo, new string[] { Base64UrlHelpers.Encode($"{{\"uid\":\"{TestConstants.Uid}\",\"utid\":\"{TestConstants.Utid}\"}}")! } }, + }), + }; + + await oidcOptions.Events.TokenValidated(tokenValidatedContext); + Assert.False(tokenValidatedContext.Result.Succeeded); + Assert.NotNull(tokenValidatedContext.Result.Failure); + Assert.IsType(tokenValidatedContext.Result.Failure); + } + private async Task AddMicrosoftIdentityWebAppCallsWebApi_TestRedirectToIdentityProviderForSignOutEventAsync( IServiceProvider provider, OpenIdConnectOptions oidcOptions, @@ -868,9 +980,9 @@ private async Task AddMicrosoftIdentityWebAppCallsWebApi_TestRedirectToIdentityP await tokenAcquisitionMock.ReceivedWithAnyArgs().RemoveAccountAsync(Arg.Any()); } - private (HttpContext, AuthenticationScheme, AuthenticationProperties) CreateContextParameters(IServiceProvider provider) + private (HttpContext, AuthenticationScheme, AuthenticationProperties) CreateContextParameters(IServiceProvider provider, IEnumerable? claims = null) { - var httpContext = HttpContextUtilities.CreateHttpContext(); + var httpContext = claims != null ? HttpContextUtilities.CreateHttpContext(claims) : HttpContextUtilities.CreateHttpContext(); httpContext.RequestServices = provider; var authScheme = new AuthenticationScheme(OpenIdConnectDefaults.AuthenticationScheme, OpenIdConnectDefaults.AuthenticationScheme, typeof(OpenIdConnectHandler)); From 06befbea9ec18e9c8c39402f111b9c1bd33fa87b Mon Sep 17 00:00:00 2001 From: Dominique St-Amand Date: Wed, 6 Nov 2024 15:27:30 -0500 Subject: [PATCH 3/9] Add check for claim equality --- .../AppBuilderExtension.cs | 8 +- ...softIdentityWebAppAuthenticationBuilder.cs | 4 +- .../WebAppExtensionsTests.cs | 128 ++++++++++++++++-- 3 files changed, 126 insertions(+), 14 deletions(-) diff --git a/src/Microsoft.Identity.Web.OWIN/AppBuilderExtension.cs b/src/Microsoft.Identity.Web.OWIN/AppBuilderExtension.cs index 5638899ce..5045268ee 100644 --- a/src/Microsoft.Identity.Web.OWIN/AppBuilderExtension.cs +++ b/src/Microsoft.Identity.Web.OWIN/AppBuilderExtension.cs @@ -177,14 +177,14 @@ public static IAppBuilder AddMicrosoftIdentityWebApp( { var uniqueTenantIdentifierClaim = context.AuthenticationTicket.Identity.FindFirst(c => c.Type == ClaimConstants.UniqueTenantIdentifier); var uniqueObjectIdentifierClaim = context.AuthenticationTicket.Identity.FindFirst(c => c.Type == ClaimConstants.UniqueObjectIdentifier); - if (uniqueTenantIdentifierClaim != null) + if (uniqueTenantIdentifierClaim != null && !string.Equals(clientInfoFromServer.UniqueTenantIdentifier, uniqueTenantIdentifierClaim.Value, StringComparison.Ordinal)) { throw new InternalClaimDetectedException($"The claim \"{ClaimConstants.UniqueTenantIdentifier}\" is reserved for internal use by this library. To ensure proper functionality and avoid conflicts, please remove or rename this claim in your ID Token.") { Claim = uniqueTenantIdentifierClaim, }; } - if (uniqueObjectIdentifierClaim != null) + if (uniqueObjectIdentifierClaim != null && !string.Equals(clientInfoFromServer.UniqueObjectIdentifier, uniqueObjectIdentifierClaim.Value, StringComparison.Ordinal)) { throw new InternalClaimDetectedException($"The claim \"{ClaimConstants.UniqueObjectIdentifier}\" is reserved for internal use by this library. To ensure proper functionality and avoid conflicts, please remove or rename this claim in your ID Token.") { @@ -206,14 +206,14 @@ public static IAppBuilder AddMicrosoftIdentityWebApp( { var uniqueTenantIdentifierClaim = context.AuthenticationTicket.Identity.FindFirst(c => c.Type == ClaimConstants.UniqueTenantIdentifier); var uniqueObjectIdentifierClaim = context.AuthenticationTicket.Identity.FindFirst(c => c.Type == ClaimConstants.UniqueObjectIdentifier); - if (uniqueTenantIdentifierClaim != null) + if (uniqueTenantIdentifierClaim != null && !string.Equals(clientInfoFromServer.UniqueTenantIdentifier, uniqueTenantIdentifierClaim.Value, StringComparison.Ordinal)) { throw new InternalClaimDetectedException($"The claim \"{ClaimConstants.UniqueTenantIdentifier}\" is reserved for internal use by this library. To ensure proper functionality and avoid conflicts, please remove or rename this claim in your ID Token.") { Claim = uniqueTenantIdentifierClaim }; } - if (uniqueObjectIdentifierClaim != null) + if (uniqueObjectIdentifierClaim != null && !string.Equals(clientInfoFromServer.UniqueObjectIdentifier, uniqueObjectIdentifierClaim.Value, StringComparison.Ordinal)) { throw new InternalClaimDetectedException($"The claim \"{ClaimConstants.UniqueObjectIdentifier}\" is reserved for internal use by this library. To ensure proper functionality and avoid conflicts, please remove or rename this claim in your ID Token.") { diff --git a/src/Microsoft.Identity.Web/WebAppExtensions/MicrosoftIdentityWebAppAuthenticationBuilder.cs b/src/Microsoft.Identity.Web/WebAppExtensions/MicrosoftIdentityWebAppAuthenticationBuilder.cs index b986b86b4..173a23035 100644 --- a/src/Microsoft.Identity.Web/WebAppExtensions/MicrosoftIdentityWebAppAuthenticationBuilder.cs +++ b/src/Microsoft.Identity.Web/WebAppExtensions/MicrosoftIdentityWebAppAuthenticationBuilder.cs @@ -184,7 +184,7 @@ internal static void WebAppCallsWebApiImplementation( { var uniqueTenantIdentifierClaim = identity.FindFirst(c => c.Type == ClaimConstants.UniqueTenantIdentifier); var uniqueObjectIdentifierClaim = identity.FindFirst(c => c.Type == ClaimConstants.UniqueObjectIdentifier); - if (uniqueTenantIdentifierClaim != null) + if (uniqueTenantIdentifierClaim != null && !string.Equals(clientInfoFromServer.UniqueTenantIdentifier, uniqueTenantIdentifierClaim.Value, StringComparison.Ordinal)) { context.Fail(new InternalClaimDetectedException($"The claim \"{ClaimConstants.UniqueTenantIdentifier}\" is reserved for internal use by this library. To ensure proper functionality and avoid conflicts, please remove or rename this claim in your ID Token.") { @@ -192,7 +192,7 @@ internal static void WebAppCallsWebApiImplementation( }); return; } - if (uniqueObjectIdentifierClaim != null) + if (uniqueObjectIdentifierClaim != null && !string.Equals(clientInfoFromServer.UniqueObjectIdentifier, uniqueObjectIdentifierClaim.Value, StringComparison.Ordinal)) { context.Fail(new InternalClaimDetectedException($"The claim \"{ClaimConstants.UniqueObjectIdentifier}\" is reserved for internal use by this library. To ensure proper functionality and avoid conflicts, please remove or rename this claim in your ID Token.") { diff --git a/tests/Microsoft.Identity.Web.Test/WebAppExtensionsTests.cs b/tests/Microsoft.Identity.Web.Test/WebAppExtensionsTests.cs index 94826aacc..32ab6bedc 100644 --- a/tests/Microsoft.Identity.Web.Test/WebAppExtensionsTests.cs +++ b/tests/Microsoft.Identity.Web.Test/WebAppExtensionsTests.cs @@ -362,7 +362,7 @@ public async Task AddMicrosoftIdentityWebAppCallsWebApi_WithConfigNameParameters [Theory] [InlineData("uid","user-uid")] [InlineData("utid","user-utid")] - public async Task AddMicrosoftIdentityWebAppCallsWebApi_WithConfigNameParametersAsync_ShouldThrowExceptionForInternalClaims(string claimType, string claimValue) + public async Task AddMicrosoftIdentityWebAppCallsWebApi_WithConfigNameParametersAsync_ShouldThrowExceptionForInternalClaims_WhenClaimsDiffer(string claimType, string claimValue) { var configMock = Substitute.For(); configMock.Configure().GetSection(ConfigSectionName).Returns(_configSection); @@ -402,7 +402,64 @@ public async Task AddMicrosoftIdentityWebAppCallsWebApi_WithConfigNameParameters AddMicrosoftIdentityWebAppCallsWebApi_TestCommon(services, provider, oidcOptions, initialScopes); await AddMicrosoftIdentityWebAppCallsWebApi_TestAuthorizationCodeReceivedEventAsync(provider, oidcOptions, authCodeReceivedFuncMock, tokenAcquisitionMock); - await AddMicrosoftIdentityWebAppCallsWebApi_TestTokenValidatedEventAsync(provider, oidcOptions, new Claim[] { new Claim(claimType, claimValue) }); + await AddMicrosoftIdentityWebAppCallsWebApi_TestTokenValidatedEventAsync(provider, oidcOptions, new Claim[] { new Claim(claimType, claimValue) }, + tokenValidatedContext => + { + Assert.False(tokenValidatedContext.Result.Succeeded); + Assert.NotNull(tokenValidatedContext.Result.Failure); + Assert.IsType(tokenValidatedContext.Result.Failure); + }); + await AddMicrosoftIdentityWebAppCallsWebApi_TestRedirectToIdentityProviderForSignOutEventAsync(provider, oidcOptions, redirectFuncMock, tokenAcquisitionMock); + } + + [Theory] + [InlineData("uid", TestConstants.Uid)] + [InlineData("utid", TestConstants.Utid)] + public async Task AddMicrosoftIdentityWebAppCallsWebApi_WithConfigNameParametersAsync_ShouldNotThrowExceptionForInternalClaims_WhenClaimsAreEqual(string claimType, string claimValue) + { + var configMock = Substitute.For(); + configMock.Configure().GetSection(ConfigSectionName).Returns(_configSection); + var initialScopes = new List() { "custom_scope" }; + var tokenAcquisitionMock = Substitute.For(); + var authCodeReceivedFuncMock = Substitute.For>(); + var tokenValidatedFuncMock = Substitute.For>(); + var redirectFuncMock = Substitute.For>(); + var services = new ServiceCollection(); + + services.AddSingleton((provider) => _env) + .AddSingleton(configMock); + + services.AddAuthentication() + .AddMicrosoftIdentityWebApp(configMock, ConfigSectionName, OidcScheme) + .EnableTokenAcquisitionToCallDownstreamApi(initialScopes); + services.Configure(OidcScheme, (options) => + { + options.Events ??= new OpenIdConnectEvents(); + options.Events.OnAuthorizationCodeReceived += authCodeReceivedFuncMock; + options.Events.OnTokenValidated += tokenValidatedFuncMock; + options.Events.OnRedirectToIdentityProviderForSignOut += redirectFuncMock; + }); + + services.RemoveAll(); + services.AddScoped((provider) => tokenAcquisitionMock); + + var provider = services.BuildServiceProvider(); + + // Assert config bind actions added correctly + provider.GetRequiredService>().Get(OidcScheme); + provider.GetRequiredService>().Get(OidcScheme); + + configMock.Received(1).GetSection(ConfigSectionName); + + var oidcOptions = provider.GetRequiredService>().Get(OidcScheme); + + AddMicrosoftIdentityWebAppCallsWebApi_TestCommon(services, provider, oidcOptions, initialScopes); + await AddMicrosoftIdentityWebAppCallsWebApi_TestAuthorizationCodeReceivedEventAsync(provider, oidcOptions, authCodeReceivedFuncMock, tokenAcquisitionMock); + await AddMicrosoftIdentityWebAppCallsWebApi_TestTokenValidatedEventAsync(provider, oidcOptions, new Claim[] { new Claim(claimType, claimValue) }, + tokenValidatedContext => + { + Assert.Null(tokenValidatedContext.Result); + }); await AddMicrosoftIdentityWebAppCallsWebApi_TestRedirectToIdentityProviderForSignOutEventAsync(provider, oidcOptions, redirectFuncMock, tokenAcquisitionMock); } @@ -454,7 +511,7 @@ public async Task AddMicrosoftIdentityWebAppCallsWebApi_WithConfigActionParamete [Theory] [InlineData("uid", "user-uid")] [InlineData("utid", "user-utid")] - public async Task AddMicrosoftIdentityWebAppCallsWebApi_WithConfigActionParametersAsync_ShouldThrowExceptionForInternalClaims(string claimType, string claimValue) + public async Task AddMicrosoftIdentityWebAppCallsWebApi_WithConfigActionParametersAsync_ShouldThrowExceptionForInternalClaims_WhenClaimsDiffer(string claimType, string claimValue) { var configMock = Substitute.For(); var initialScopes = new List() { "custom_scope" }; @@ -494,7 +551,64 @@ public async Task AddMicrosoftIdentityWebAppCallsWebApi_WithConfigActionParamete AddMicrosoftIdentityWebAppCallsWebApi_TestCommon(services, provider, oidcOptions, initialScopes); await AddMicrosoftIdentityWebAppCallsWebApi_TestAuthorizationCodeReceivedEventAsync(provider, oidcOptions, authCodeReceivedFuncMock, tokenAcquisitionMock); - await AddMicrosoftIdentityWebAppCallsWebApi_TestTokenValidatedEventAsync(provider, oidcOptions, new Claim[] { new Claim(claimType, claimValue) }); + await AddMicrosoftIdentityWebAppCallsWebApi_TestTokenValidatedEventAsync(provider, oidcOptions, new Claim[] { new Claim(claimType, claimValue) }, + tokenValidatedContext => + { + Assert.False(tokenValidatedContext.Result.Succeeded); + Assert.NotNull(tokenValidatedContext.Result.Failure); + Assert.IsType(tokenValidatedContext.Result.Failure); + }); + await AddMicrosoftIdentityWebAppCallsWebApi_TestRedirectToIdentityProviderForSignOutEventAsync(provider, oidcOptions, redirectFuncMock, tokenAcquisitionMock); + } + + [Theory] + [InlineData("uid", TestConstants.Uid)] + [InlineData("utid", TestConstants.Utid)] + public async Task AddMicrosoftIdentityWebAppCallsWebApi_WithConfigActionParametersAsync_ShouldNotThrowExceptionForInternalClaims_WhenClaimsAreEqual(string claimType, string claimValue) + { + var configMock = Substitute.For(); + var initialScopes = new List() { "custom_scope" }; + var tokenAcquisitionMock = Substitute.For(); + var authCodeReceivedFuncMock = Substitute.For>(); + var tokenValidatedFuncMock = Substitute.For>(); + var redirectFuncMock = Substitute.For>(); + + var services = new ServiceCollection(); + services.AddSingleton(configMock); + services.AddSingleton((provider) => _env); + + var builder = services.AddAuthentication() + .AddMicrosoftIdentityWebApp(_configureMsOptions, null, OidcScheme) + .EnableTokenAcquisitionToCallDownstreamApi(_configureAppOptions, initialScopes); + services.Configure(OidcScheme, (options) => + { + options.Events ??= new OpenIdConnectEvents(); + options.Events.OnAuthorizationCodeReceived += authCodeReceivedFuncMock; + options.Events.OnTokenValidated += tokenValidatedFuncMock; + options.Events.OnRedirectToIdentityProviderForSignOut += redirectFuncMock; + }); + + services.RemoveAll(); + services.AddScoped((provider) => tokenAcquisitionMock); + + var provider = builder.Services.BuildServiceProvider(); + + // Assert configure options actions added correctly + var configuredAppOptions = provider.GetServices>().Cast>(); + var configuredMsOptions = provider.GetServices>().Cast>(); + + Assert.Contains(configuredAppOptions, o => o.Action == _configureAppOptions); + Assert.Contains(configuredMsOptions, o => o.Action == _configureMsOptions); + + var oidcOptions = provider.GetRequiredService>().Create(OidcScheme); + + AddMicrosoftIdentityWebAppCallsWebApi_TestCommon(services, provider, oidcOptions, initialScopes); + await AddMicrosoftIdentityWebAppCallsWebApi_TestAuthorizationCodeReceivedEventAsync(provider, oidcOptions, authCodeReceivedFuncMock, tokenAcquisitionMock); + await AddMicrosoftIdentityWebAppCallsWebApi_TestTokenValidatedEventAsync(provider, oidcOptions, new Claim[] { new Claim(claimType, claimValue) }, + tokenValidatedContext => + { + Assert.Null(tokenValidatedContext.Result); + }); await AddMicrosoftIdentityWebAppCallsWebApi_TestRedirectToIdentityProviderForSignOutEventAsync(provider, oidcOptions, redirectFuncMock, tokenAcquisitionMock); } @@ -946,7 +1060,7 @@ private async Task AddMicrosoftIdentityWebAppCallsWebApi_TestTokenValidatedEvent Assert.True(tokenValidatedContext?.Principal?.HasClaim(c => c.Type == ClaimConstants.UniqueObjectIdentifier)); } - private async Task AddMicrosoftIdentityWebAppCallsWebApi_TestTokenValidatedEventAsync(IServiceProvider provider, OpenIdConnectOptions oidcOptions, IEnumerable? claims) + private async Task AddMicrosoftIdentityWebAppCallsWebApi_TestTokenValidatedEventAsync(IServiceProvider provider, OpenIdConnectOptions oidcOptions, IEnumerable? claims, Action assertions) { var (httpContext, authScheme, authProperties) = CreateContextParameters(provider, claims); @@ -960,9 +1074,7 @@ private async Task AddMicrosoftIdentityWebAppCallsWebApi_TestTokenValidatedEvent }; await oidcOptions.Events.TokenValidated(tokenValidatedContext); - Assert.False(tokenValidatedContext.Result.Succeeded); - Assert.NotNull(tokenValidatedContext.Result.Failure); - Assert.IsType(tokenValidatedContext.Result.Failure); + assertions(tokenValidatedContext); } private async Task AddMicrosoftIdentityWebAppCallsWebApi_TestRedirectToIdentityProviderForSignOutEventAsync( From 60e16d2a1f33cc53c8f9192bb84f46aac6d3d0ab Mon Sep 17 00:00:00 2001 From: Dominique St-Amand Date: Thu, 7 Nov 2024 14:48:19 -0500 Subject: [PATCH 4/9] Change string comparison to OrdinalIgnoreCase --- src/Microsoft.Identity.Web.OWIN/AppBuilderExtension.cs | 8 ++++---- .../MicrosoftIdentityWebAppAuthenticationBuilder.cs | 4 ++-- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/Microsoft.Identity.Web.OWIN/AppBuilderExtension.cs b/src/Microsoft.Identity.Web.OWIN/AppBuilderExtension.cs index 5045268ee..c437d8ca7 100644 --- a/src/Microsoft.Identity.Web.OWIN/AppBuilderExtension.cs +++ b/src/Microsoft.Identity.Web.OWIN/AppBuilderExtension.cs @@ -177,14 +177,14 @@ public static IAppBuilder AddMicrosoftIdentityWebApp( { var uniqueTenantIdentifierClaim = context.AuthenticationTicket.Identity.FindFirst(c => c.Type == ClaimConstants.UniqueTenantIdentifier); var uniqueObjectIdentifierClaim = context.AuthenticationTicket.Identity.FindFirst(c => c.Type == ClaimConstants.UniqueObjectIdentifier); - if (uniqueTenantIdentifierClaim != null && !string.Equals(clientInfoFromServer.UniqueTenantIdentifier, uniqueTenantIdentifierClaim.Value, StringComparison.Ordinal)) + if (uniqueTenantIdentifierClaim != null && !string.Equals(clientInfoFromServer.UniqueTenantIdentifier, uniqueTenantIdentifierClaim.Value, StringComparison.OrdinalIgnoreCase)) { throw new InternalClaimDetectedException($"The claim \"{ClaimConstants.UniqueTenantIdentifier}\" is reserved for internal use by this library. To ensure proper functionality and avoid conflicts, please remove or rename this claim in your ID Token.") { Claim = uniqueTenantIdentifierClaim, }; } - if (uniqueObjectIdentifierClaim != null && !string.Equals(clientInfoFromServer.UniqueObjectIdentifier, uniqueObjectIdentifierClaim.Value, StringComparison.Ordinal)) + if (uniqueObjectIdentifierClaim != null && !string.Equals(clientInfoFromServer.UniqueObjectIdentifier, uniqueObjectIdentifierClaim.Value, StringComparison.OrdinalIgnoreCase)) { throw new InternalClaimDetectedException($"The claim \"{ClaimConstants.UniqueObjectIdentifier}\" is reserved for internal use by this library. To ensure proper functionality and avoid conflicts, please remove or rename this claim in your ID Token.") { @@ -206,14 +206,14 @@ public static IAppBuilder AddMicrosoftIdentityWebApp( { var uniqueTenantIdentifierClaim = context.AuthenticationTicket.Identity.FindFirst(c => c.Type == ClaimConstants.UniqueTenantIdentifier); var uniqueObjectIdentifierClaim = context.AuthenticationTicket.Identity.FindFirst(c => c.Type == ClaimConstants.UniqueObjectIdentifier); - if (uniqueTenantIdentifierClaim != null && !string.Equals(clientInfoFromServer.UniqueTenantIdentifier, uniqueTenantIdentifierClaim.Value, StringComparison.Ordinal)) + if (uniqueTenantIdentifierClaim != null && !string.Equals(clientInfoFromServer.UniqueTenantIdentifier, uniqueTenantIdentifierClaim.Value, StringComparison.OrdinalIgnoreCase)) { throw new InternalClaimDetectedException($"The claim \"{ClaimConstants.UniqueTenantIdentifier}\" is reserved for internal use by this library. To ensure proper functionality and avoid conflicts, please remove or rename this claim in your ID Token.") { Claim = uniqueTenantIdentifierClaim }; } - if (uniqueObjectIdentifierClaim != null && !string.Equals(clientInfoFromServer.UniqueObjectIdentifier, uniqueObjectIdentifierClaim.Value, StringComparison.Ordinal)) + if (uniqueObjectIdentifierClaim != null && !string.Equals(clientInfoFromServer.UniqueObjectIdentifier, uniqueObjectIdentifierClaim.Value, StringComparison.OrdinalIgnoreCase)) { throw new InternalClaimDetectedException($"The claim \"{ClaimConstants.UniqueObjectIdentifier}\" is reserved for internal use by this library. To ensure proper functionality and avoid conflicts, please remove or rename this claim in your ID Token.") { diff --git a/src/Microsoft.Identity.Web/WebAppExtensions/MicrosoftIdentityWebAppAuthenticationBuilder.cs b/src/Microsoft.Identity.Web/WebAppExtensions/MicrosoftIdentityWebAppAuthenticationBuilder.cs index 173a23035..64f341d6a 100644 --- a/src/Microsoft.Identity.Web/WebAppExtensions/MicrosoftIdentityWebAppAuthenticationBuilder.cs +++ b/src/Microsoft.Identity.Web/WebAppExtensions/MicrosoftIdentityWebAppAuthenticationBuilder.cs @@ -184,7 +184,7 @@ internal static void WebAppCallsWebApiImplementation( { var uniqueTenantIdentifierClaim = identity.FindFirst(c => c.Type == ClaimConstants.UniqueTenantIdentifier); var uniqueObjectIdentifierClaim = identity.FindFirst(c => c.Type == ClaimConstants.UniqueObjectIdentifier); - if (uniqueTenantIdentifierClaim != null && !string.Equals(clientInfoFromServer.UniqueTenantIdentifier, uniqueTenantIdentifierClaim.Value, StringComparison.Ordinal)) + if (uniqueTenantIdentifierClaim != null && !string.Equals(clientInfoFromServer.UniqueTenantIdentifier, uniqueTenantIdentifierClaim.Value, StringComparison.OrdinalIgnoreCase)) { context.Fail(new InternalClaimDetectedException($"The claim \"{ClaimConstants.UniqueTenantIdentifier}\" is reserved for internal use by this library. To ensure proper functionality and avoid conflicts, please remove or rename this claim in your ID Token.") { @@ -192,7 +192,7 @@ internal static void WebAppCallsWebApiImplementation( }); return; } - if (uniqueObjectIdentifierClaim != null && !string.Equals(clientInfoFromServer.UniqueObjectIdentifier, uniqueObjectIdentifierClaim.Value, StringComparison.Ordinal)) + if (uniqueObjectIdentifierClaim != null && !string.Equals(clientInfoFromServer.UniqueObjectIdentifier, uniqueObjectIdentifierClaim.Value, StringComparison.OrdinalIgnoreCase)) { context.Fail(new InternalClaimDetectedException($"The claim \"{ClaimConstants.UniqueObjectIdentifier}\" is reserved for internal use by this library. To ensure proper functionality and avoid conflicts, please remove or rename this claim in your ID Token.") { From cb2b715a3549d19f02bae6ca91ebda009c69a15d Mon Sep 17 00:00:00 2001 From: Dominique St-Amand Date: Fri, 8 Nov 2024 12:41:21 -0500 Subject: [PATCH 5/9] Improvements based on feedback from PR --- .../AppBuilderExtension.cs | 59 +++++++++---------- .../InternalClaimDetectedException.cs | 31 ---------- .../IDWebErrorMessage.cs | 1 + .../PublicAPI/net462/InternalAPI.Shipped.txt | 1 + .../PublicAPI/net472/InternalAPI.Shipped.txt | 1 + .../PublicAPI/net6.0/InternalAPI.Shipped.txt | 1 + .../PublicAPI/net7.0/InternalAPI.Shipped.txt | 1 + .../PublicAPI/net8.0/InternalAPI.Shipped.txt | 1 + .../PublicAPI/net9.0/InternalAPI.Shipped.txt | 1 + .../netstandard2.0/InternalAPI.Shipped.txt | 1 + .../InternalClaimDetectedException.cs | 31 ---------- ...softIdentityWebAppAuthenticationBuilder.cs | 12 ++-- .../WebAppExtensionsTests.cs | 20 +++---- 13 files changed, 49 insertions(+), 112 deletions(-) delete mode 100644 src/Microsoft.Identity.Web.OWIN/InternalClaimDetectedException.cs delete mode 100644 src/Microsoft.Identity.Web/InternalClaimDetectedException.cs diff --git a/src/Microsoft.Identity.Web.OWIN/AppBuilderExtension.cs b/src/Microsoft.Identity.Web.OWIN/AppBuilderExtension.cs index c437d8ca7..5a6136c88 100644 --- a/src/Microsoft.Identity.Web.OWIN/AppBuilderExtension.cs +++ b/src/Microsoft.Identity.Web.OWIN/AppBuilderExtension.cs @@ -2,6 +2,8 @@ // Licensed under the MIT License. using System; +using System.Globalization; +using System.Security.Authentication; using System.Security.Claims; using System.Threading.Tasks; using System.Web; @@ -15,6 +17,7 @@ using Microsoft.IdentityModel.Tokens; using Microsoft.IdentityModel.Validators; using Microsoft.Owin.Security.Jwt; +using Microsoft.Owin.Security.Notifications; using Microsoft.Owin.Security.OAuth; using Microsoft.Owin.Security.OpenIdConnect; using Owin; @@ -175,22 +178,7 @@ public static IAppBuilder AddMicrosoftIdentityWebApp( if (clientInfoFromServer != null && clientInfoFromServer.UniqueTenantIdentifier != null && clientInfoFromServer.UniqueObjectIdentifier != null) { - var uniqueTenantIdentifierClaim = context.AuthenticationTicket.Identity.FindFirst(c => c.Type == ClaimConstants.UniqueTenantIdentifier); - var uniqueObjectIdentifierClaim = context.AuthenticationTicket.Identity.FindFirst(c => c.Type == ClaimConstants.UniqueObjectIdentifier); - if (uniqueTenantIdentifierClaim != null && !string.Equals(clientInfoFromServer.UniqueTenantIdentifier, uniqueTenantIdentifierClaim.Value, StringComparison.OrdinalIgnoreCase)) - { - throw new InternalClaimDetectedException($"The claim \"{ClaimConstants.UniqueTenantIdentifier}\" is reserved for internal use by this library. To ensure proper functionality and avoid conflicts, please remove or rename this claim in your ID Token.") - { - Claim = uniqueTenantIdentifierClaim, - }; - } - if (uniqueObjectIdentifierClaim != null && !string.Equals(clientInfoFromServer.UniqueObjectIdentifier, uniqueObjectIdentifierClaim.Value, StringComparison.OrdinalIgnoreCase)) - { - throw new InternalClaimDetectedException($"The claim \"{ClaimConstants.UniqueObjectIdentifier}\" is reserved for internal use by this library. To ensure proper functionality and avoid conflicts, please remove or rename this claim in your ID Token.") - { - Claim = uniqueObjectIdentifierClaim - }; - } + RejectInternalClaims(context.AuthenticationTicket.Identity, clientInfoFromServer.UniqueTenantIdentifier, clientInfoFromServer.UniqueObjectIdentifier); context.AuthenticationTicket.Identity.AddClaim(new Claim(ClaimConstants.UniqueTenantIdentifier, clientInfoFromServer.UniqueTenantIdentifier)); context.AuthenticationTicket.Identity.AddClaim(new Claim(ClaimConstants.UniqueObjectIdentifier, clientInfoFromServer.UniqueObjectIdentifier)); @@ -204,22 +192,7 @@ public static IAppBuilder AddMicrosoftIdentityWebApp( if (clientInfoFromServer != null && clientInfoFromServer.UniqueTenantIdentifier != null && clientInfoFromServer.UniqueObjectIdentifier != null) { - var uniqueTenantIdentifierClaim = context.AuthenticationTicket.Identity.FindFirst(c => c.Type == ClaimConstants.UniqueTenantIdentifier); - var uniqueObjectIdentifierClaim = context.AuthenticationTicket.Identity.FindFirst(c => c.Type == ClaimConstants.UniqueObjectIdentifier); - if (uniqueTenantIdentifierClaim != null && !string.Equals(clientInfoFromServer.UniqueTenantIdentifier, uniqueTenantIdentifierClaim.Value, StringComparison.OrdinalIgnoreCase)) - { - throw new InternalClaimDetectedException($"The claim \"{ClaimConstants.UniqueTenantIdentifier}\" is reserved for internal use by this library. To ensure proper functionality and avoid conflicts, please remove or rename this claim in your ID Token.") - { - Claim = uniqueTenantIdentifierClaim - }; - } - if (uniqueObjectIdentifierClaim != null && !string.Equals(clientInfoFromServer.UniqueObjectIdentifier, uniqueObjectIdentifierClaim.Value, StringComparison.OrdinalIgnoreCase)) - { - throw new InternalClaimDetectedException($"The claim \"{ClaimConstants.UniqueObjectIdentifier}\" is reserved for internal use by this library. To ensure proper functionality and avoid conflicts, please remove or rename this claim in your ID Token.") - { - Claim = uniqueObjectIdentifierClaim - }; - } + RejectInternalClaims(context.AuthenticationTicket.Identity, clientInfoFromServer.UniqueTenantIdentifier, clientInfoFromServer.UniqueObjectIdentifier); context.AuthenticationTicket.Identity.AddClaim(new Claim(ClaimConstants.UniqueTenantIdentifier, clientInfoFromServer.UniqueTenantIdentifier)); context.AuthenticationTicket.Identity.AddClaim(new Claim(ClaimConstants.UniqueObjectIdentifier, clientInfoFromServer.UniqueObjectIdentifier)); @@ -272,5 +245,27 @@ public static IAppBuilder AddMicrosoftIdentityWebApp( return app.UseOpenIdConnectAuthentication(options); } + + /// + /// Rejects the internal claims, if present. + /// + /// The associated to the logged-in user + /// The tenant identifier (i.e. >) + /// The object identifier (i.e. >) + /// The contains internal claims that are used internal use by this library + private static void RejectInternalClaims(ClaimsIdentity identity, string uniqueTenantIdentifier, string uniqueObjectIdentifier) + { + var uniqueTenantIdentifierClaim = identity.FindFirst(c => c.Type == ClaimConstants.UniqueTenantIdentifier); + if (uniqueTenantIdentifierClaim != null && !string.Equals(uniqueTenantIdentifier, uniqueTenantIdentifierClaim.Value, StringComparison.OrdinalIgnoreCase)) + { + throw new AuthenticationException(string.Format(CultureInfo.InvariantCulture, IDWebErrorMessage.InternalClaimDetected, ClaimConstants.UniqueTenantIdentifier)); + } + + var uniqueObjectIdentifierClaim = identity.FindFirst(c => c.Type == ClaimConstants.UniqueObjectIdentifier); + if (uniqueObjectIdentifierClaim != null && !string.Equals(uniqueObjectIdentifier, uniqueObjectIdentifierClaim.Value, StringComparison.OrdinalIgnoreCase)) + { + throw new AuthenticationException(string.Format(CultureInfo.InvariantCulture, IDWebErrorMessage.InternalClaimDetected, ClaimConstants.UniqueObjectIdentifier)); + } + } } } diff --git a/src/Microsoft.Identity.Web.OWIN/InternalClaimDetectedException.cs b/src/Microsoft.Identity.Web.OWIN/InternalClaimDetectedException.cs deleted file mode 100644 index 69ff2fb95..000000000 --- a/src/Microsoft.Identity.Web.OWIN/InternalClaimDetectedException.cs +++ /dev/null @@ -1,31 +0,0 @@ -// Copyright (c) Microsoft Corporation. All rights reserved. -// Licensed under the MIT License. - -using System; -using System.Security.Claims; - -namespace Microsoft.Identity.Web.OWIN -{ - /// - /// The exception that is thrown when an internal ID Token claim used by Microsoft.Identity.Web internally is detected in the user's ID Token. - /// - public class InternalClaimDetectedException : Exception - { - /// - /// Gets or sets the invalid claim. - /// - public Claim Claim { get; set; } - - public InternalClaimDetectedException() - { - } - - public InternalClaimDetectedException(string message) : base(message) - { - } - - public InternalClaimDetectedException(string message, Exception innerException) : base(message, innerException) - { - } - } -} diff --git a/src/Microsoft.Identity.Web.TokenAcquisition/IDWebErrorMessage.cs b/src/Microsoft.Identity.Web.TokenAcquisition/IDWebErrorMessage.cs index fc989d5b6..89963ae4f 100644 --- a/src/Microsoft.Identity.Web.TokenAcquisition/IDWebErrorMessage.cs +++ b/src/Microsoft.Identity.Web.TokenAcquisition/IDWebErrorMessage.cs @@ -37,6 +37,7 @@ internal static class IDWebErrorMessage public const string NoMetadataDocumentRetrieverProvided = "IDW10302: No metadata document retriever is provided. "; public const string IssuerDoesNotMatchValidIssuers = "IDW10303: Issuer: '{0}', does not match any of the valid issuers provided for this application. "; public const string B2CTfpIssuerNotSupported = "IDW10304: Microsoft Identity Web does not support a B2C issuer with 'tfp' in the URI. See https://aka.ms/ms-id-web/b2c-issuer for details. "; + public const string InternalClaimDetected = "IDW10305: The claim '{0}' is reserved for internal use by this library. To ensure proper functionality and avoid conflicts, please remove or rename this claim in your ID Token. "; // Protocol IDW10400 = "IDW10400:" public const string TenantIdClaimNotPresentInToken = "IDW10401: Neither `tid` nor `tenantId` claim is present in the token obtained from Microsoft identity platform. "; diff --git a/src/Microsoft.Identity.Web.TokenAcquisition/PublicAPI/net462/InternalAPI.Shipped.txt b/src/Microsoft.Identity.Web.TokenAcquisition/PublicAPI/net462/InternalAPI.Shipped.txt index 494ac733b..8f4830af7 100644 --- a/src/Microsoft.Identity.Web.TokenAcquisition/PublicAPI/net462/InternalAPI.Shipped.txt +++ b/src/Microsoft.Identity.Web.TokenAcquisition/PublicAPI/net462/InternalAPI.Shipped.txt @@ -73,6 +73,7 @@ const Microsoft.Identity.Web.IDWebErrorMessage.HttpContextAndHttpResponseAreNull const Microsoft.Identity.Web.IDWebErrorMessage.HttpContextIsNull = "IDW10001: HttpContext is null. " -> string! const Microsoft.Identity.Web.IDWebErrorMessage.IncorrectNumberOfUriSegments = "IDW10702: Number of URI segments is incorrect: {0}, URI: {1}. " -> string! const Microsoft.Identity.Web.IDWebErrorMessage.InitializeAsyncIsObsolete = "IDW10801: Use Initialize instead. See https://aka.ms/ms-id-web/1.9.0. " -> string! +const Microsoft.Identity.Web.IDWebErrorMessage.InternalClaimDetected = "IDW10305: The claim '{0}' is reserved for internal use by this library. To ensure proper functionality and avoid conflicts, please remove or rename this claim in your ID Token. " -> string! const Microsoft.Identity.Web.IDWebErrorMessage.InvalidAssertion = "IDW10504: Invalid assertion: contains unsupported character(s)." -> string! const Microsoft.Identity.Web.IDWebErrorMessage.InvalidBase64UrlString = "IDW10601: Invalid Base64URL string. " -> string! const Microsoft.Identity.Web.IDWebErrorMessage.InvalidCertificateStorePath = "IDW10703: Certificate store path must be of the form 'StoreLocation/StoreName'. StoreLocation must be one of 'CurrentUser', 'LocalMachine'. StoreName must be empty or one of '{0}'. " -> string! diff --git a/src/Microsoft.Identity.Web.TokenAcquisition/PublicAPI/net472/InternalAPI.Shipped.txt b/src/Microsoft.Identity.Web.TokenAcquisition/PublicAPI/net472/InternalAPI.Shipped.txt index 494ac733b..8f4830af7 100644 --- a/src/Microsoft.Identity.Web.TokenAcquisition/PublicAPI/net472/InternalAPI.Shipped.txt +++ b/src/Microsoft.Identity.Web.TokenAcquisition/PublicAPI/net472/InternalAPI.Shipped.txt @@ -73,6 +73,7 @@ const Microsoft.Identity.Web.IDWebErrorMessage.HttpContextAndHttpResponseAreNull const Microsoft.Identity.Web.IDWebErrorMessage.HttpContextIsNull = "IDW10001: HttpContext is null. " -> string! const Microsoft.Identity.Web.IDWebErrorMessage.IncorrectNumberOfUriSegments = "IDW10702: Number of URI segments is incorrect: {0}, URI: {1}. " -> string! const Microsoft.Identity.Web.IDWebErrorMessage.InitializeAsyncIsObsolete = "IDW10801: Use Initialize instead. See https://aka.ms/ms-id-web/1.9.0. " -> string! +const Microsoft.Identity.Web.IDWebErrorMessage.InternalClaimDetected = "IDW10305: The claim '{0}' is reserved for internal use by this library. To ensure proper functionality and avoid conflicts, please remove or rename this claim in your ID Token. " -> string! const Microsoft.Identity.Web.IDWebErrorMessage.InvalidAssertion = "IDW10504: Invalid assertion: contains unsupported character(s)." -> string! const Microsoft.Identity.Web.IDWebErrorMessage.InvalidBase64UrlString = "IDW10601: Invalid Base64URL string. " -> string! const Microsoft.Identity.Web.IDWebErrorMessage.InvalidCertificateStorePath = "IDW10703: Certificate store path must be of the form 'StoreLocation/StoreName'. StoreLocation must be one of 'CurrentUser', 'LocalMachine'. StoreName must be empty or one of '{0}'. " -> string! diff --git a/src/Microsoft.Identity.Web.TokenAcquisition/PublicAPI/net6.0/InternalAPI.Shipped.txt b/src/Microsoft.Identity.Web.TokenAcquisition/PublicAPI/net6.0/InternalAPI.Shipped.txt index 1b9a83191..e41091f82 100644 --- a/src/Microsoft.Identity.Web.TokenAcquisition/PublicAPI/net6.0/InternalAPI.Shipped.txt +++ b/src/Microsoft.Identity.Web.TokenAcquisition/PublicAPI/net6.0/InternalAPI.Shipped.txt @@ -73,6 +73,7 @@ const Microsoft.Identity.Web.IDWebErrorMessage.HttpContextAndHttpResponseAreNull const Microsoft.Identity.Web.IDWebErrorMessage.HttpContextIsNull = "IDW10001: HttpContext is null. " -> string! const Microsoft.Identity.Web.IDWebErrorMessage.IncorrectNumberOfUriSegments = "IDW10702: Number of URI segments is incorrect: {0}, URI: {1}. " -> string! const Microsoft.Identity.Web.IDWebErrorMessage.InitializeAsyncIsObsolete = "IDW10801: Use Initialize instead. See https://aka.ms/ms-id-web/1.9.0. " -> string! +const Microsoft.Identity.Web.IDWebErrorMessage.InternalClaimDetected = "IDW10305: The claim '{0}' is reserved for internal use by this library. To ensure proper functionality and avoid conflicts, please remove or rename this claim in your ID Token. " -> string! const Microsoft.Identity.Web.IDWebErrorMessage.InvalidAssertion = "IDW10504: Invalid assertion: contains unsupported character(s)." -> string! const Microsoft.Identity.Web.IDWebErrorMessage.InvalidBase64UrlString = "IDW10601: Invalid Base64URL string. " -> string! const Microsoft.Identity.Web.IDWebErrorMessage.InvalidCertificateStorePath = "IDW10703: Certificate store path must be of the form 'StoreLocation/StoreName'. StoreLocation must be one of 'CurrentUser', 'LocalMachine'. StoreName must be empty or one of '{0}'. " -> string! diff --git a/src/Microsoft.Identity.Web.TokenAcquisition/PublicAPI/net7.0/InternalAPI.Shipped.txt b/src/Microsoft.Identity.Web.TokenAcquisition/PublicAPI/net7.0/InternalAPI.Shipped.txt index 9b7c49167..9e074ac29 100644 --- a/src/Microsoft.Identity.Web.TokenAcquisition/PublicAPI/net7.0/InternalAPI.Shipped.txt +++ b/src/Microsoft.Identity.Web.TokenAcquisition/PublicAPI/net7.0/InternalAPI.Shipped.txt @@ -73,6 +73,7 @@ const Microsoft.Identity.Web.IDWebErrorMessage.HttpContextAndHttpResponseAreNull const Microsoft.Identity.Web.IDWebErrorMessage.HttpContextIsNull = "IDW10001: HttpContext is null. " -> string! const Microsoft.Identity.Web.IDWebErrorMessage.IncorrectNumberOfUriSegments = "IDW10702: Number of URI segments is incorrect: {0}, URI: {1}. " -> string! const Microsoft.Identity.Web.IDWebErrorMessage.InitializeAsyncIsObsolete = "IDW10801: Use Initialize instead. See https://aka.ms/ms-id-web/1.9.0. " -> string! +const Microsoft.Identity.Web.IDWebErrorMessage.InternalClaimDetected = "IDW10305: The claim '{0}' is reserved for internal use by this library. To ensure proper functionality and avoid conflicts, please remove or rename this claim in your ID Token. " -> string! const Microsoft.Identity.Web.IDWebErrorMessage.InvalidAssertion = "IDW10504: Invalid assertion: contains unsupported character(s)." -> string! const Microsoft.Identity.Web.IDWebErrorMessage.InvalidBase64UrlString = "IDW10601: Invalid Base64URL string. " -> string! const Microsoft.Identity.Web.IDWebErrorMessage.InvalidCertificateStorePath = "IDW10703: Certificate store path must be of the form 'StoreLocation/StoreName'. StoreLocation must be one of 'CurrentUser', 'LocalMachine'. StoreName must be empty or one of '{0}'. " -> string! diff --git a/src/Microsoft.Identity.Web.TokenAcquisition/PublicAPI/net8.0/InternalAPI.Shipped.txt b/src/Microsoft.Identity.Web.TokenAcquisition/PublicAPI/net8.0/InternalAPI.Shipped.txt index 9b7c49167..9e074ac29 100644 --- a/src/Microsoft.Identity.Web.TokenAcquisition/PublicAPI/net8.0/InternalAPI.Shipped.txt +++ b/src/Microsoft.Identity.Web.TokenAcquisition/PublicAPI/net8.0/InternalAPI.Shipped.txt @@ -73,6 +73,7 @@ const Microsoft.Identity.Web.IDWebErrorMessage.HttpContextAndHttpResponseAreNull const Microsoft.Identity.Web.IDWebErrorMessage.HttpContextIsNull = "IDW10001: HttpContext is null. " -> string! const Microsoft.Identity.Web.IDWebErrorMessage.IncorrectNumberOfUriSegments = "IDW10702: Number of URI segments is incorrect: {0}, URI: {1}. " -> string! const Microsoft.Identity.Web.IDWebErrorMessage.InitializeAsyncIsObsolete = "IDW10801: Use Initialize instead. See https://aka.ms/ms-id-web/1.9.0. " -> string! +const Microsoft.Identity.Web.IDWebErrorMessage.InternalClaimDetected = "IDW10305: The claim '{0}' is reserved for internal use by this library. To ensure proper functionality and avoid conflicts, please remove or rename this claim in your ID Token. " -> string! const Microsoft.Identity.Web.IDWebErrorMessage.InvalidAssertion = "IDW10504: Invalid assertion: contains unsupported character(s)." -> string! const Microsoft.Identity.Web.IDWebErrorMessage.InvalidBase64UrlString = "IDW10601: Invalid Base64URL string. " -> string! const Microsoft.Identity.Web.IDWebErrorMessage.InvalidCertificateStorePath = "IDW10703: Certificate store path must be of the form 'StoreLocation/StoreName'. StoreLocation must be one of 'CurrentUser', 'LocalMachine'. StoreName must be empty or one of '{0}'. " -> string! diff --git a/src/Microsoft.Identity.Web.TokenAcquisition/PublicAPI/net9.0/InternalAPI.Shipped.txt b/src/Microsoft.Identity.Web.TokenAcquisition/PublicAPI/net9.0/InternalAPI.Shipped.txt index 9b7c49167..9e074ac29 100644 --- a/src/Microsoft.Identity.Web.TokenAcquisition/PublicAPI/net9.0/InternalAPI.Shipped.txt +++ b/src/Microsoft.Identity.Web.TokenAcquisition/PublicAPI/net9.0/InternalAPI.Shipped.txt @@ -73,6 +73,7 @@ const Microsoft.Identity.Web.IDWebErrorMessage.HttpContextAndHttpResponseAreNull const Microsoft.Identity.Web.IDWebErrorMessage.HttpContextIsNull = "IDW10001: HttpContext is null. " -> string! const Microsoft.Identity.Web.IDWebErrorMessage.IncorrectNumberOfUriSegments = "IDW10702: Number of URI segments is incorrect: {0}, URI: {1}. " -> string! const Microsoft.Identity.Web.IDWebErrorMessage.InitializeAsyncIsObsolete = "IDW10801: Use Initialize instead. See https://aka.ms/ms-id-web/1.9.0. " -> string! +const Microsoft.Identity.Web.IDWebErrorMessage.InternalClaimDetected = "IDW10305: The claim '{0}' is reserved for internal use by this library. To ensure proper functionality and avoid conflicts, please remove or rename this claim in your ID Token. " -> string! const Microsoft.Identity.Web.IDWebErrorMessage.InvalidAssertion = "IDW10504: Invalid assertion: contains unsupported character(s)." -> string! const Microsoft.Identity.Web.IDWebErrorMessage.InvalidBase64UrlString = "IDW10601: Invalid Base64URL string. " -> string! const Microsoft.Identity.Web.IDWebErrorMessage.InvalidCertificateStorePath = "IDW10703: Certificate store path must be of the form 'StoreLocation/StoreName'. StoreLocation must be one of 'CurrentUser', 'LocalMachine'. StoreName must be empty or one of '{0}'. " -> string! diff --git a/src/Microsoft.Identity.Web.TokenAcquisition/PublicAPI/netstandard2.0/InternalAPI.Shipped.txt b/src/Microsoft.Identity.Web.TokenAcquisition/PublicAPI/netstandard2.0/InternalAPI.Shipped.txt index 494ac733b..8f4830af7 100644 --- a/src/Microsoft.Identity.Web.TokenAcquisition/PublicAPI/netstandard2.0/InternalAPI.Shipped.txt +++ b/src/Microsoft.Identity.Web.TokenAcquisition/PublicAPI/netstandard2.0/InternalAPI.Shipped.txt @@ -73,6 +73,7 @@ const Microsoft.Identity.Web.IDWebErrorMessage.HttpContextAndHttpResponseAreNull const Microsoft.Identity.Web.IDWebErrorMessage.HttpContextIsNull = "IDW10001: HttpContext is null. " -> string! const Microsoft.Identity.Web.IDWebErrorMessage.IncorrectNumberOfUriSegments = "IDW10702: Number of URI segments is incorrect: {0}, URI: {1}. " -> string! const Microsoft.Identity.Web.IDWebErrorMessage.InitializeAsyncIsObsolete = "IDW10801: Use Initialize instead. See https://aka.ms/ms-id-web/1.9.0. " -> string! +const Microsoft.Identity.Web.IDWebErrorMessage.InternalClaimDetected = "IDW10305: The claim '{0}' is reserved for internal use by this library. To ensure proper functionality and avoid conflicts, please remove or rename this claim in your ID Token. " -> string! const Microsoft.Identity.Web.IDWebErrorMessage.InvalidAssertion = "IDW10504: Invalid assertion: contains unsupported character(s)." -> string! const Microsoft.Identity.Web.IDWebErrorMessage.InvalidBase64UrlString = "IDW10601: Invalid Base64URL string. " -> string! const Microsoft.Identity.Web.IDWebErrorMessage.InvalidCertificateStorePath = "IDW10703: Certificate store path must be of the form 'StoreLocation/StoreName'. StoreLocation must be one of 'CurrentUser', 'LocalMachine'. StoreName must be empty or one of '{0}'. " -> string! diff --git a/src/Microsoft.Identity.Web/InternalClaimDetectedException.cs b/src/Microsoft.Identity.Web/InternalClaimDetectedException.cs deleted file mode 100644 index bbd7b0908..000000000 --- a/src/Microsoft.Identity.Web/InternalClaimDetectedException.cs +++ /dev/null @@ -1,31 +0,0 @@ -// Copyright (c) Microsoft Corporation. All rights reserved. -// Licensed under the MIT License. - -using System; -using System.Security.Claims; - -namespace Microsoft.Identity.Web -{ - /// - /// The exception that is thrown when an internal ID Token claim used by Microsoft.Identity.Web internally is detected in the user's ID Token. - /// - public class InternalClaimDetectedException : Exception - { - /// - /// Gets or sets the invalid claim. - /// - public Claim Claim { get; set; } - - public InternalClaimDetectedException() - { - } - - public InternalClaimDetectedException(string message) : base(message) - { - } - - public InternalClaimDetectedException(string message, Exception innerException) : base(message, innerException) - { - } - } -} diff --git a/src/Microsoft.Identity.Web/WebAppExtensions/MicrosoftIdentityWebAppAuthenticationBuilder.cs b/src/Microsoft.Identity.Web/WebAppExtensions/MicrosoftIdentityWebAppAuthenticationBuilder.cs index 64f341d6a..0bd0d4378 100644 --- a/src/Microsoft.Identity.Web/WebAppExtensions/MicrosoftIdentityWebAppAuthenticationBuilder.cs +++ b/src/Microsoft.Identity.Web/WebAppExtensions/MicrosoftIdentityWebAppAuthenticationBuilder.cs @@ -4,7 +4,9 @@ using System; using System.Collections.Generic; using System.Diagnostics.CodeAnalysis; +using System.Globalization; using System.Linq; +using System.Security.Authentication; using System.Security.Claims; using System.Threading.Tasks; using Microsoft.AspNetCore.Authentication.OpenIdConnect; @@ -186,18 +188,12 @@ internal static void WebAppCallsWebApiImplementation( var uniqueObjectIdentifierClaim = identity.FindFirst(c => c.Type == ClaimConstants.UniqueObjectIdentifier); if (uniqueTenantIdentifierClaim != null && !string.Equals(clientInfoFromServer.UniqueTenantIdentifier, uniqueTenantIdentifierClaim.Value, StringComparison.OrdinalIgnoreCase)) { - context.Fail(new InternalClaimDetectedException($"The claim \"{ClaimConstants.UniqueTenantIdentifier}\" is reserved for internal use by this library. To ensure proper functionality and avoid conflicts, please remove or rename this claim in your ID Token.") - { - Claim = uniqueTenantIdentifierClaim - }); + context.Fail(new AuthenticationException(string.Format(CultureInfo.InvariantCulture, IDWebErrorMessage.InternalClaimDetected, ClaimConstants.UniqueTenantIdentifier))); return; } if (uniqueObjectIdentifierClaim != null && !string.Equals(clientInfoFromServer.UniqueObjectIdentifier, uniqueObjectIdentifierClaim.Value, StringComparison.OrdinalIgnoreCase)) { - context.Fail(new InternalClaimDetectedException($"The claim \"{ClaimConstants.UniqueObjectIdentifier}\" is reserved for internal use by this library. To ensure proper functionality and avoid conflicts, please remove or rename this claim in your ID Token.") - { - Claim = uniqueObjectIdentifierClaim - }); + context.Fail(new AuthenticationException(string.Format(CultureInfo.InvariantCulture, IDWebErrorMessage.InternalClaimDetected, ClaimConstants.UniqueObjectIdentifier))); return; } diff --git a/tests/Microsoft.Identity.Web.Test/WebAppExtensionsTests.cs b/tests/Microsoft.Identity.Web.Test/WebAppExtensionsTests.cs index 32ab6bedc..7f89c0941 100644 --- a/tests/Microsoft.Identity.Web.Test/WebAppExtensionsTests.cs +++ b/tests/Microsoft.Identity.Web.Test/WebAppExtensionsTests.cs @@ -360,8 +360,8 @@ public async Task AddMicrosoftIdentityWebAppCallsWebApi_WithConfigNameParameters } [Theory] - [InlineData("uid","user-uid")] - [InlineData("utid","user-utid")] + [InlineData(ClaimConstants.UniqueObjectIdentifier, "user-uid")] + [InlineData(ClaimConstants.UniqueTenantIdentifier, "user-utid")] public async Task AddMicrosoftIdentityWebAppCallsWebApi_WithConfigNameParametersAsync_ShouldThrowExceptionForInternalClaims_WhenClaimsDiffer(string claimType, string claimValue) { var configMock = Substitute.For(); @@ -407,14 +407,14 @@ public async Task AddMicrosoftIdentityWebAppCallsWebApi_WithConfigNameParameters { Assert.False(tokenValidatedContext.Result.Succeeded); Assert.NotNull(tokenValidatedContext.Result.Failure); - Assert.IsType(tokenValidatedContext.Result.Failure); + Assert.IsType(tokenValidatedContext.Result.Failure); }); await AddMicrosoftIdentityWebAppCallsWebApi_TestRedirectToIdentityProviderForSignOutEventAsync(provider, oidcOptions, redirectFuncMock, tokenAcquisitionMock); } [Theory] - [InlineData("uid", TestConstants.Uid)] - [InlineData("utid", TestConstants.Utid)] + [InlineData(ClaimConstants.UniqueObjectIdentifier, TestConstants.Uid)] + [InlineData(ClaimConstants.UniqueTenantIdentifier, TestConstants.Utid)] public async Task AddMicrosoftIdentityWebAppCallsWebApi_WithConfigNameParametersAsync_ShouldNotThrowExceptionForInternalClaims_WhenClaimsAreEqual(string claimType, string claimValue) { var configMock = Substitute.For(); @@ -509,8 +509,8 @@ public async Task AddMicrosoftIdentityWebAppCallsWebApi_WithConfigActionParamete } [Theory] - [InlineData("uid", "user-uid")] - [InlineData("utid", "user-utid")] + [InlineData(ClaimConstants.UniqueObjectIdentifier, "user-uid")] + [InlineData(ClaimConstants.UniqueTenantIdentifier, "user-utid")] public async Task AddMicrosoftIdentityWebAppCallsWebApi_WithConfigActionParametersAsync_ShouldThrowExceptionForInternalClaims_WhenClaimsDiffer(string claimType, string claimValue) { var configMock = Substitute.For(); @@ -556,14 +556,14 @@ public async Task AddMicrosoftIdentityWebAppCallsWebApi_WithConfigActionParamete { Assert.False(tokenValidatedContext.Result.Succeeded); Assert.NotNull(tokenValidatedContext.Result.Failure); - Assert.IsType(tokenValidatedContext.Result.Failure); + Assert.IsType(tokenValidatedContext.Result.Failure); }); await AddMicrosoftIdentityWebAppCallsWebApi_TestRedirectToIdentityProviderForSignOutEventAsync(provider, oidcOptions, redirectFuncMock, tokenAcquisitionMock); } [Theory] - [InlineData("uid", TestConstants.Uid)] - [InlineData("utid", TestConstants.Utid)] + [InlineData(ClaimConstants.UniqueObjectIdentifier, TestConstants.Uid)] + [InlineData(ClaimConstants.UniqueTenantIdentifier, TestConstants.Utid)] public async Task AddMicrosoftIdentityWebAppCallsWebApi_WithConfigActionParametersAsync_ShouldNotThrowExceptionForInternalClaims_WhenClaimsAreEqual(string claimType, string claimValue) { var configMock = Substitute.For(); From a54dcaf8f5bfc4b19cbbc3a93e98aed0546388c6 Mon Sep 17 00:00:00 2001 From: Dominique St-Amand Date: Fri, 8 Nov 2024 14:14:20 -0500 Subject: [PATCH 6/9] PR changes post discussion --- .../Microsoft.Identity.Web.OWIN.xml | 39 +++++++++---------- ...softIdentityWebAppAuthenticationBuilder.cs | 32 +++++++++------ 2 files changed, 40 insertions(+), 31 deletions(-) diff --git a/src/Microsoft.Identity.Web.OWIN/Microsoft.Identity.Web.OWIN.xml b/src/Microsoft.Identity.Web.OWIN/Microsoft.Identity.Web.OWIN.xml index 8dd5fdc8a..0f91fd4c9 100644 --- a/src/Microsoft.Identity.Web.OWIN/Microsoft.Identity.Web.OWIN.xml +++ b/src/Microsoft.Identity.Web.OWIN/Microsoft.Identity.Web.OWIN.xml @@ -55,6 +55,15 @@ Configuration section in which to read the options. The app builder to chain. + + + Rejects the internal claims, if present. + + The associated to the logged-in user + The tenant identifier (i.e. >) + The object identifier (i.e. >) + The contains internal claims that are used internal use by this library + Extension methods to retrieve a Graph service client and interfaces used @@ -79,15 +88,21 @@ - + - The exception that is thrown when an internal ID Token claim used by Microsoft.Identity.Web internally is detected in the user's ID Token. + Gets the issuer the credentials are for. + + The issuer the credentials are for. + - + - Gets or sets the invalid claim. + Gets all known security keys. + + All known security keys. + @@ -106,21 +121,5 @@ Pre-build action. Ensures that the host is an OWIN host. - - - Gets the issuer the credentials are for. - - - The issuer the credentials are for. - - - - - Gets all known security keys. - - - All known security keys. - - diff --git a/src/Microsoft.Identity.Web/WebAppExtensions/MicrosoftIdentityWebAppAuthenticationBuilder.cs b/src/Microsoft.Identity.Web/WebAppExtensions/MicrosoftIdentityWebAppAuthenticationBuilder.cs index 0bd0d4378..dafd1d250 100644 --- a/src/Microsoft.Identity.Web/WebAppExtensions/MicrosoftIdentityWebAppAuthenticationBuilder.cs +++ b/src/Microsoft.Identity.Web/WebAppExtensions/MicrosoftIdentityWebAppAuthenticationBuilder.cs @@ -179,26 +179,36 @@ internal static void WebAppCallsWebApiImplementation( { ClientInfo? clientInfoFromServer = ClientInfo.CreateFromJson(clientInfo); - if (clientInfoFromServer != null && clientInfoFromServer.UniqueTenantIdentifier != null && clientInfoFromServer.UniqueObjectIdentifier != null) + if (clientInfoFromServer != null) { var identity = context!.Principal!.Identities.FirstOrDefault(); if (identity != null) { - var uniqueTenantIdentifierClaim = identity.FindFirst(c => c.Type == ClaimConstants.UniqueTenantIdentifier); - var uniqueObjectIdentifierClaim = identity.FindFirst(c => c.Type == ClaimConstants.UniqueObjectIdentifier); - if (uniqueTenantIdentifierClaim != null && !string.Equals(clientInfoFromServer.UniqueTenantIdentifier, uniqueTenantIdentifierClaim.Value, StringComparison.OrdinalIgnoreCase)) + if (clientInfoFromServer.UniqueTenantIdentifier != null) { - context.Fail(new AuthenticationException(string.Format(CultureInfo.InvariantCulture, IDWebErrorMessage.InternalClaimDetected, ClaimConstants.UniqueTenantIdentifier))); - return; + var uniqueTenantIdentifierClaim = identity.FindFirst(c => c.Type == ClaimConstants.UniqueTenantIdentifier); + if (uniqueTenantIdentifierClaim != null && !string.Equals(clientInfoFromServer.UniqueTenantIdentifier, uniqueTenantIdentifierClaim.Value, StringComparison.OrdinalIgnoreCase)) + { + context.Fail(new AuthenticationException(string.Format(CultureInfo.InvariantCulture, IDWebErrorMessage.InternalClaimDetected, ClaimConstants.UniqueTenantIdentifier))); + return; + } } - if (uniqueObjectIdentifierClaim != null && !string.Equals(clientInfoFromServer.UniqueObjectIdentifier, uniqueObjectIdentifierClaim.Value, StringComparison.OrdinalIgnoreCase)) + + if (clientInfoFromServer.UniqueObjectIdentifier != null) { - context.Fail(new AuthenticationException(string.Format(CultureInfo.InvariantCulture, IDWebErrorMessage.InternalClaimDetected, ClaimConstants.UniqueObjectIdentifier))); - return; + var uniqueObjectIdentifierClaim = identity.FindFirst(c => c.Type == ClaimConstants.UniqueObjectIdentifier); + if (uniqueObjectIdentifierClaim != null && !string.Equals(clientInfoFromServer.UniqueObjectIdentifier, uniqueObjectIdentifierClaim.Value, StringComparison.OrdinalIgnoreCase)) + { + context.Fail(new AuthenticationException(string.Format(CultureInfo.InvariantCulture, IDWebErrorMessage.InternalClaimDetected, ClaimConstants.UniqueObjectIdentifier))); + return; + } } - identity.AddClaim(new Claim(ClaimConstants.UniqueTenantIdentifier, clientInfoFromServer.UniqueTenantIdentifier)); - identity.AddClaim(new Claim(ClaimConstants.UniqueObjectIdentifier, clientInfoFromServer.UniqueObjectIdentifier)); + if (clientInfoFromServer.UniqueTenantIdentifier != null && clientInfoFromServer.UniqueObjectIdentifier != null) + { + identity.AddClaim(new Claim(ClaimConstants.UniqueTenantIdentifier, clientInfoFromServer.UniqueTenantIdentifier)); + identity.AddClaim(new Claim(ClaimConstants.UniqueObjectIdentifier, clientInfoFromServer.UniqueObjectIdentifier)); + } } } } From 9aabaed79be1903f41e9e6a817dc4e110bcf3097 Mon Sep 17 00:00:00 2001 From: Dominique St-Amand Date: Mon, 11 Nov 2024 14:15:45 -0500 Subject: [PATCH 7/9] Update src/Microsoft.Identity.Web.OWIN/AppBuilderExtension.cs Update the description based on PR review Co-authored-by: Bogdan Gavril --- src/Microsoft.Identity.Web.OWIN/AppBuilderExtension.cs | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/Microsoft.Identity.Web.OWIN/AppBuilderExtension.cs b/src/Microsoft.Identity.Web.OWIN/AppBuilderExtension.cs index 5a6136c88..b25420491 100644 --- a/src/Microsoft.Identity.Web.OWIN/AppBuilderExtension.cs +++ b/src/Microsoft.Identity.Web.OWIN/AppBuilderExtension.cs @@ -247,7 +247,11 @@ public static IAppBuilder AddMicrosoftIdentityWebApp( } /// - /// Rejects the internal claims, if present. + /// The SDK injects 2 claims named uuid and utid into the ClaimsPrincipal, from ESTS's client_info. These represent + /// the home oid and home tid and together they form the AccountId, which MSAL uses as cache key for web site + /// scenarios. In case the app owner defines claims with the same name to be added to the ID Token, this creates + /// a conflict with the reserved claims Id.Web injects, and it is better to throw a meaningful error. See issue 2968 for details. + /// /// The associated to the logged-in user /// The tenant identifier (i.e. >) From 56b255c04882cc2b84be6483223502c035f997fd Mon Sep 17 00:00:00 2001 From: Dominique St-Amand Date: Tue, 12 Nov 2024 09:32:48 -0500 Subject: [PATCH 8/9] Refactor to test each claim separately. --- .../AppBuilderExtension.cs | 38 ++++++++++--------- 1 file changed, 21 insertions(+), 17 deletions(-) diff --git a/src/Microsoft.Identity.Web.OWIN/AppBuilderExtension.cs b/src/Microsoft.Identity.Web.OWIN/AppBuilderExtension.cs index b25420491..78fec614c 100644 --- a/src/Microsoft.Identity.Web.OWIN/AppBuilderExtension.cs +++ b/src/Microsoft.Identity.Web.OWIN/AppBuilderExtension.cs @@ -176,12 +176,23 @@ public static IAppBuilder AddMicrosoftIdentityWebApp( { ClientInfo? clientInfoFromServer = ClientInfo.CreateFromJson(clientInfo); - if (clientInfoFromServer != null && clientInfoFromServer.UniqueTenantIdentifier != null && clientInfoFromServer.UniqueObjectIdentifier != null) + if (clientInfoFromServer != null) { - RejectInternalClaims(context.AuthenticationTicket.Identity, clientInfoFromServer.UniqueTenantIdentifier, clientInfoFromServer.UniqueObjectIdentifier); + if (clientInfoFromServer.UniqueTenantIdentifier != null) + { + RejectInternalClaims(context.AuthenticationTicket.Identity, ClaimConstants.UniqueTenantIdentifier, clientInfoFromServer.UniqueTenantIdentifier); + } - context.AuthenticationTicket.Identity.AddClaim(new Claim(ClaimConstants.UniqueTenantIdentifier, clientInfoFromServer.UniqueTenantIdentifier)); - context.AuthenticationTicket.Identity.AddClaim(new Claim(ClaimConstants.UniqueObjectIdentifier, clientInfoFromServer.UniqueObjectIdentifier)); + if (clientInfoFromServer.UniqueObjectIdentifier != null) + { + RejectInternalClaims(context.AuthenticationTicket.Identity, ClaimConstants.UniqueObjectIdentifier, clientInfoFromServer.UniqueObjectIdentifier); + } + + if (clientInfoFromServer.UniqueTenantIdentifier != null && clientInfoFromServer.UniqueObjectIdentifier != null) + { + context.AuthenticationTicket.Identity.AddClaim(new Claim(ClaimConstants.UniqueTenantIdentifier, clientInfoFromServer.UniqueTenantIdentifier)); + context.AuthenticationTicket.Identity.AddClaim(new Claim(ClaimConstants.UniqueObjectIdentifier, clientInfoFromServer.UniqueObjectIdentifier)); + } } context.OwinContext.Environment.Remove(ClaimConstants.ClientInfo); } @@ -251,24 +262,17 @@ public static IAppBuilder AddMicrosoftIdentityWebApp( /// the home oid and home tid and together they form the AccountId, which MSAL uses as cache key for web site /// scenarios. In case the app owner defines claims with the same name to be added to the ID Token, this creates /// a conflict with the reserved claims Id.Web injects, and it is better to throw a meaningful error. See issue 2968 for details. - /// /// The associated to the logged-in user - /// The tenant identifier (i.e. >) - /// The object identifier (i.e. >) + /// The claim type + /// The claim value /// The contains internal claims that are used internal use by this library - private static void RejectInternalClaims(ClaimsIdentity identity, string uniqueTenantIdentifier, string uniqueObjectIdentifier) + private static void RejectInternalClaims(ClaimsIdentity identity, string claimType, string claimValue) { - var uniqueTenantIdentifierClaim = identity.FindFirst(c => c.Type == ClaimConstants.UniqueTenantIdentifier); - if (uniqueTenantIdentifierClaim != null && !string.Equals(uniqueTenantIdentifier, uniqueTenantIdentifierClaim.Value, StringComparison.OrdinalIgnoreCase)) - { - throw new AuthenticationException(string.Format(CultureInfo.InvariantCulture, IDWebErrorMessage.InternalClaimDetected, ClaimConstants.UniqueTenantIdentifier)); - } - - var uniqueObjectIdentifierClaim = identity.FindFirst(c => c.Type == ClaimConstants.UniqueObjectIdentifier); - if (uniqueObjectIdentifierClaim != null && !string.Equals(uniqueObjectIdentifier, uniqueObjectIdentifierClaim.Value, StringComparison.OrdinalIgnoreCase)) + var identityClaim = identity.FindFirst(c => c.Type == claimType); + if (identityClaim != null && !string.Equals(claimValue, identityClaim.Value, StringComparison.OrdinalIgnoreCase)) { - throw new AuthenticationException(string.Format(CultureInfo.InvariantCulture, IDWebErrorMessage.InternalClaimDetected, ClaimConstants.UniqueObjectIdentifier)); + throw new AuthenticationException(string.Format(CultureInfo.InvariantCulture, IDWebErrorMessage.InternalClaimDetected, claimType)); } } } From 22705ef3618aeb94062587504b1ad10fc2608a8c Mon Sep 17 00:00:00 2001 From: Dominique St-Amand Date: Thu, 2 Jan 2025 14:03:15 -0500 Subject: [PATCH 9/9] Change the logic to match --- .../AppBuilderExtension.cs | 19 +++++++++++++++---- 1 file changed, 15 insertions(+), 4 deletions(-) diff --git a/src/Microsoft.Identity.Web.OWIN/AppBuilderExtension.cs b/src/Microsoft.Identity.Web.OWIN/AppBuilderExtension.cs index 78fec614c..c8d85174b 100644 --- a/src/Microsoft.Identity.Web.OWIN/AppBuilderExtension.cs +++ b/src/Microsoft.Identity.Web.OWIN/AppBuilderExtension.cs @@ -201,12 +201,23 @@ public static IAppBuilder AddMicrosoftIdentityWebApp( { ClientInfo? clientInfoFromServer = ClientInfo.CreateFromJson(clientInfo); - if (clientInfoFromServer != null && clientInfoFromServer.UniqueTenantIdentifier != null && clientInfoFromServer.UniqueObjectIdentifier != null) + if (clientInfoFromServer != null) { - RejectInternalClaims(context.AuthenticationTicket.Identity, clientInfoFromServer.UniqueTenantIdentifier, clientInfoFromServer.UniqueObjectIdentifier); + if (clientInfoFromServer.UniqueTenantIdentifier != null) + { + RejectInternalClaims(context.AuthenticationTicket.Identity, ClaimConstants.UniqueTenantIdentifier, clientInfoFromServer.UniqueTenantIdentifier); + } - context.AuthenticationTicket.Identity.AddClaim(new Claim(ClaimConstants.UniqueTenantIdentifier, clientInfoFromServer.UniqueTenantIdentifier)); - context.AuthenticationTicket.Identity.AddClaim(new Claim(ClaimConstants.UniqueObjectIdentifier, clientInfoFromServer.UniqueObjectIdentifier)); + if (clientInfoFromServer.UniqueObjectIdentifier != null) + { + RejectInternalClaims(context.AuthenticationTicket.Identity, ClaimConstants.UniqueObjectIdentifier, clientInfoFromServer.UniqueObjectIdentifier); + } + + if (clientInfoFromServer.UniqueTenantIdentifier != null && clientInfoFromServer.UniqueObjectIdentifier != null) + { + context.AuthenticationTicket.Identity.AddClaim(new Claim(ClaimConstants.UniqueTenantIdentifier, clientInfoFromServer.UniqueTenantIdentifier)); + context.AuthenticationTicket.Identity.AddClaim(new Claim(ClaimConstants.UniqueObjectIdentifier, clientInfoFromServer.UniqueObjectIdentifier)); + } } httpContext.Session.Remove(ClaimConstants.ClientInfo); }