search

AzureAD / microsoft-identity-web

Helps creating protected web apps and web APIs with Microsoft identity platform and Azure AD B2C
MIT License
684 stars 217 forks source link

Add checks to protect the internal claims used by MIW. Ref: issue #2968 #3131

Open DOMZE opened 3 weeks ago

DOMZE commented 3 weeks ago

Add checks to protect the users to not use internal claims used by the library

Summary of the changes (Less than 80 chars)

Description

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

Fixes #2968 (in this specific format)

DOMZE commented 3 weeks ago

@bgavrilMS / @jennyf19 / @jmprieur please review.

Thank you!

DOMZE commented 3 weeks ago

@microsoft-github-policy-service agree

bgavrilMS commented 3 weeks ago

LGTM, I only have minor comments and resolving @msbw2 's comments

DOMZE commented 2 weeks ago

@bgavrilMS @jmprieur @jennyf19 @pmaytak to be able to go forward, we just need your input on claims casing match (type+value/value only) and the changes are done.

Thank you!

bgavrilMS commented 2 weeks ago

@bgavrilMS @jmprieur @jennyf19 @pmaytak to be able to go forward, we just need your input on claims casing match (type+value/value only) and the changes are done.

Thank you!

I am fine with case insensitive.

jennyf19 commented 2 weeks ago

@bgavrilMS @jmprieur @jennyf19 @pmaytak to be able to go forward, we just need your input on claims casing match (type+value/value only) and the changes are done.

Thank you!

ClaimsType should never be treated as case insensitive

pmaytak commented 2 weeks ago

I think claim names should be case sensitive at least as per JWT spec. IdentityModel also treats them as case sensstive.

pmaytak commented 2 weeks ago

Read the issue, thought about this more. When we search for a claim to validate or use, then we should be case sensitive. This way we get the exact claim we're looking for. However, if our intent is to throw an exception if the user specified any variation of utid or uid, then we should use case insensitive approach to broaden the search. This way we ensure that the token only ever has utid and uid variations of claim names.