azure-activedirectory-identitymodel-extensions-for-dotnet icon indicating copy to clipboard operation
azure-activedirectory-identitymodel-extensions-for-dotnet copied to clipboard

JwtSecurityTokenHandler does not create principal with roles correctly.

Open javiercn opened this issue 4 years ago • 6 comments

If you setup a custom RoleClaimType in TokenValidationParamters the identity is created here with the custom role claim type (as expected) but then when the mappings are being applied, all the role claims get mapped to a different claim type (ClaimTypes.RoleClaimType by default) here.

That results in a broken experience as when a user configures the JwtSecurityTokenHandler to use the "role" claim from the access token, the Identity that gets created has a "role" claim type but the roles are mapped to ClaimTypes.RoleClaimType.

This breaks basic functionality like [Authorize(Role = "admin")] in ASP.NET Core and so on, and the only available workaround (AFAIK) is to modify the static DefaultJwtInboundClaimsMap property in JwtSecurityTokenHandler, which I don't think is an experience we want to push into our customers.

I think that JwtSecurityTokenHandler should account for any possible mapping when it creates the ClaimsIdentity or that it should avoid mapping the role claim all-together.

Of the two options, I believe the most appropriate one is the first one, as it preserves the current mapping behavior and simply fixes the definition on the identity.

javiercn avatar Mar 13 '20 17:03 javiercn

@javiercn any change will modify existing behavior, unless we adopt an opt-in control.

Our long term strategy was to move to Microsoft.IdentityModel.JsonWebTokenHandler which doesn't perform any mapping. This will require coordination between asp.net and IdentityModel. We desire to make this change for 5.0. This will require asp.net to make some changes in the existing token type inside.

I opened https://github.com/dotnet/aspnetcore/issues/20066 so there is an asp.net issue for tracking.

brentschmaltz avatar Mar 23 '20 03:03 brentschmaltz

@javiercn any comment?

brentschmaltz avatar Mar 24 '20 22:03 brentschmaltz

@brentschmaltz Sorry, this got missed in my swath of notifications. I'm not familiar with JsonWebTokenHandler I think we can keep this open in case we don't move away from it in the 5.0 timeframe.

The issue here is simple and ultimately what I care about is our customers having a nice E2E. Whether that is achieved by switching the handler or fixing this particular issue, I have no preference.

javiercn avatar May 31 '20 11:05 javiercn

@javiercn thanks for the update, will keep it open.

brentschmaltz avatar Jun 02 '20 17:06 brentschmaltz

@javiercn we are working on 6.x and applying only serious fixes to 5.x. Since System.IdentityModel.Tokens.Jwt is still in 6.x, we should fix this. Does this logic sound right? here

I assume you meant ClaimTypes.Role and not ClaimTypes.RoleClaimType above.

if (!_inboundClaimTypeMap.TryGetValue(jwtClaim.Type, out claimType))
{
    if (claimType == ClaimsIdentity.DefaultRoleClaim)
         claimType = identity.RoleClaimType;
}

Also with JsonWebToken, it is unlikely that IsInRole will work as expected as the ClaimsIdentity will have the default value and JsonWebTokenHandler does not map. We will have to take this into account when asking Asp.net to move to JsonWebTokens.

brentschmaltz avatar Nov 05 '21 00:11 brentschmaltz

Followed a guide for adding Roles To IdentityServer4 but [Authorize(Role = "Administrator")] did not work until I added JwtSecurityTokenHandler.DefaultInboundClaimTypeMap.Clear(); before services.AddAuthentication for .NET 6. I hope this will be fixed since it was quite time consuming to find given that the JWT token had a role when checking it manually.

https://ffimnsr.medium.com/adding-identity-roles-to-identity-server-4-in-net-core-3-1-d42b64ff6675

Ogglas avatar Jan 24 '22 21:01 Ogglas

By design. Closing.

jennyf19 avatar Sep 19 '23 21:09 jennyf19

@jennyf19 It is by design that you need to clear DefaultInboundClaimTypeMap?

Ogglas avatar Sep 20 '23 12:09 Ogglas

@Ogglas yes

brentschmaltz avatar Sep 22 '23 18:09 brentschmaltz

Followed a guide for adding Roles To IdentityServer4 but [Authorize(Role = "Administrator")] did not work until I added JwtSecurityTokenHandler.DefaultInboundClaimTypeMap.Clear(); before services.AddAuthentication for .NET 6. I hope this will be fixed since it was quite time consuming to find given that the JWT token had a role when checking it manually.

https://ffimnsr.medium.com/adding-identity-roles-to-identity-server-4-in-net-core-3-1-d42b64ff6675

This helped me!

hflexgrig avatar Dec 30 '23 18:12 hflexgrig