From 4b63b6a640f3adf61f67155225d073d1dcf5b7d4 Mon Sep 17 00:00:00 2001 From: Dominique St-Amand Date: Fri, 3 Jan 2025 09:45:50 -0500 Subject: [PATCH] Add checks to protect the internal claims used by MIW. Ref: issue #2968 (#3131) * 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 * Add unit tests for the modifications * Add check for claim equality * Change string comparison to OrdinalIgnoreCase * Improvements based on feedback from PR * PR changes post discussion * Update src/Microsoft.Identity.Web.OWIN/AppBuilderExtension.cs Update the description based on PR review Co-authored-by: Bogdan Gavril * Refactor to test each claim separately. * Change the logic to match --------- Co-authored-by: Bogdan Gavril --- .../AppBuilderExtension.cs | 60 ++++- .../Microsoft.Identity.Web.OWIN.xml | 9 + .../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 + ...softIdentityWebAppAuthenticationBuilder.cs | 35 ++- .../TestHelpers/HttpContextUtilities.cs | 10 + .../WebAppExtensionsTests.cs | 230 +++++++++++++++++- 13 files changed, 340 insertions(+), 12 deletions(-) diff --git a/src/Microsoft.Identity.Web.OWIN/AppBuilderExtension.cs b/src/Microsoft.Identity.Web.OWIN/AppBuilderExtension.cs index 55083dbc6..c8d85174b 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; @@ -173,10 +176,23 @@ public static IAppBuilder AddMicrosoftIdentityWebApp( { ClientInfo? clientInfoFromServer = ClientInfo.CreateFromJson(clientInfo); - if (clientInfoFromServer != null && clientInfoFromServer.UniqueTenantIdentifier != null && clientInfoFromServer.UniqueObjectIdentifier != null) + if (clientInfoFromServer != null) { - context.AuthenticationTicket.Identity.AddClaim(new Claim(ClaimConstants.UniqueTenantIdentifier, clientInfoFromServer.UniqueTenantIdentifier)); - context.AuthenticationTicket.Identity.AddClaim(new Claim(ClaimConstants.UniqueObjectIdentifier, clientInfoFromServer.UniqueObjectIdentifier)); + if (clientInfoFromServer.UniqueTenantIdentifier != null) + { + RejectInternalClaims(context.AuthenticationTicket.Identity, ClaimConstants.UniqueTenantIdentifier, clientInfoFromServer.UniqueTenantIdentifier); + } + + 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); } @@ -185,10 +201,23 @@ public static IAppBuilder AddMicrosoftIdentityWebApp( { ClientInfo? clientInfoFromServer = ClientInfo.CreateFromJson(clientInfo); - if (clientInfoFromServer != null && clientInfoFromServer.UniqueTenantIdentifier != null && clientInfoFromServer.UniqueObjectIdentifier != null) + if (clientInfoFromServer != null) { - context.AuthenticationTicket.Identity.AddClaim(new Claim(ClaimConstants.UniqueTenantIdentifier, clientInfoFromServer.UniqueTenantIdentifier)); - context.AuthenticationTicket.Identity.AddClaim(new Claim(ClaimConstants.UniqueObjectIdentifier, clientInfoFromServer.UniqueObjectIdentifier)); + if (clientInfoFromServer.UniqueTenantIdentifier != null) + { + RejectInternalClaims(context.AuthenticationTicket.Identity, ClaimConstants.UniqueTenantIdentifier, clientInfoFromServer.UniqueTenantIdentifier); + } + + 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); } @@ -238,5 +267,24 @@ public static IAppBuilder AddMicrosoftIdentityWebApp( return app.UseOpenIdConnectAuthentication(options); } + + /// + /// 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 claim type + /// The claim value + /// The contains internal claims that are used internal use by this library + private static void RejectInternalClaims(ClaimsIdentity identity, string claimType, string claimValue) + { + 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, claimType)); + } + } } } 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..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 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/WebAppExtensions/MicrosoftIdentityWebAppAuthenticationBuilder.cs b/src/Microsoft.Identity.Web/WebAppExtensions/MicrosoftIdentityWebAppAuthenticationBuilder.cs index d43edc207..dafd1d250 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; @@ -177,10 +179,37 @@ internal static void WebAppCallsWebApiImplementation( { ClientInfo? clientInfoFromServer = ClientInfo.CreateFromJson(clientInfo); - if (clientInfoFromServer != null && clientInfoFromServer.UniqueTenantIdentifier != null && clientInfoFromServer.UniqueObjectIdentifier != null) + if (clientInfoFromServer != 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) + { + if (clientInfoFromServer.UniqueTenantIdentifier != null) + { + 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 (clientInfoFromServer.UniqueObjectIdentifier != null) + { + 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; + } + } + + if (clientInfoFromServer.UniqueTenantIdentifier != null && clientInfoFromServer.UniqueObjectIdentifier != null) + { + identity.AddClaim(new Claim(ClaimConstants.UniqueTenantIdentifier, clientInfoFromServer.UniqueTenantIdentifier)); + identity.AddClaim(new Claim(ClaimConstants.UniqueObjectIdentifier, clientInfoFromServer.UniqueObjectIdentifier)); + } + } } } await onTokenValidatedHandler(context).ConfigureAwait(false); 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..7f89c0941 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,110 @@ public async Task AddMicrosoftIdentityWebAppCallsWebApi_WithConfigNameParameters await AddMicrosoftIdentityWebAppCallsWebApi_TestRedirectToIdentityProviderForSignOutEventAsync(provider, oidcOptions, redirectFuncMock, tokenAcquisitionMock); } + [Theory] + [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(); + 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.False(tokenValidatedContext.Result.Succeeded); + Assert.NotNull(tokenValidatedContext.Result.Failure); + Assert.IsType(tokenValidatedContext.Result.Failure); + }); + await AddMicrosoftIdentityWebAppCallsWebApi_TestRedirectToIdentityProviderForSignOutEventAsync(provider, oidcOptions, redirectFuncMock, tokenAcquisitionMock); + } + + [Theory] + [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(); + 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); + } + [Fact] public async Task AddMicrosoftIdentityWebAppCallsWebApi_WithConfigActionParametersAsync() { @@ -405,6 +508,110 @@ public async Task AddMicrosoftIdentityWebAppCallsWebApi_WithConfigActionParamete await AddMicrosoftIdentityWebAppCallsWebApi_TestRedirectToIdentityProviderForSignOutEventAsync(provider, oidcOptions, redirectFuncMock, tokenAcquisitionMock); } + [Theory] + [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(); + 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.False(tokenValidatedContext.Result.Succeeded); + Assert.NotNull(tokenValidatedContext.Result.Failure); + Assert.IsType(tokenValidatedContext.Result.Failure); + }); + await AddMicrosoftIdentityWebAppCallsWebApi_TestRedirectToIdentityProviderForSignOutEventAsync(provider, oidcOptions, redirectFuncMock, tokenAcquisitionMock); + } + + [Theory] + [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(); + 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); + } + [Fact] public void AddMicrosoftIdentityWebAppCallsWebApi_NoScopes() { @@ -853,6 +1060,23 @@ 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, Action assertions) + { + 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); + assertions(tokenValidatedContext); + } + private async Task AddMicrosoftIdentityWebAppCallsWebApi_TestRedirectToIdentityProviderForSignOutEventAsync( IServiceProvider provider, OpenIdConnectOptions oidcOptions, @@ -868,9 +1092,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));