Skip to content

Commit

Permalink
Add checks to protect the internal claims used by MIW. Ref: issue #2968
Browse files Browse the repository at this point in the history
… (#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 <[email protected]>

* Refactor to test each claim separately.

* Change the logic to match

---------

Co-authored-by: Bogdan Gavril <[email protected]>
  • Loading branch information
DOMZE and bgavrilMS authored Jan 3, 2025
1 parent 28a5af1 commit 4b63b6a
Show file tree
Hide file tree
Showing 13 changed files with 340 additions and 12 deletions.
60 changes: 54 additions & 6 deletions src/Microsoft.Identity.Web.OWIN/AppBuilderExtension.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
Expand Down Expand Up @@ -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);
}
Expand All @@ -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);
}
Expand Down Expand Up @@ -238,5 +267,24 @@ public static IAppBuilder AddMicrosoftIdentityWebApp(

return app.UseOpenIdConnectAuthentication(options);
}

/// <summary>
/// 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.
/// </summary>
/// <param name="identity">The <see cref="ClaimsIdentity"/> associated to the logged-in user</param>
/// <param name="claimType">The claim type</param>
/// <param name="claimValue">The claim value</param>
/// <exception cref="AuthenticationException">The <see cref="ClaimsIdentity"/> contains internal claims that are used internal use by this library</exception>
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));
}
}
}
}

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Original file line number Diff line number Diff line change
Expand Up @@ -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. ";
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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!
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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!
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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!
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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!
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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!
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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!
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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!
Expand Down
Loading

0 comments on commit 4b63b6a

Please sign in to comment.