Reduce claims in ClaimsIdentity after completing OIDC protocol legs

[brentschmaltz]

OIDC is a multi-leg protocol that can have three legs ( code -> access_token -> user_info) flowing through the three endpoints. Authorize, Token and UserInfo. The IdentityProvider uses the payload to provide links between the legs. Once the sequence of messages is complete and deemed valid, some of those claims have little value, such as: nonce, at_hash, c_hash. These could be removed from the ClaimsIdentity presented to the application layer. This would reduce the size of cookies.

[Tratcher]

why are those things even claims? They're not consumed as claims in the ODIC middleware are they?

[brockallen]

These things are part of the id_token payload (which are co-mingled with the user's claims) and have been expressed as claims ever since katana.

[kevinchalet]

and have been expressed as claims ever since katana.

Because essentially, they are, as the JWT format doesn't make any distinction between protocol claims (nonce, expiration date, access token/code hashes, etc.) and user claims.

[brockallen]

Because essentially, they are, as the JWT format doesn't make any distinction between protocol claims (nonce, expiration date, access token/code hashes, etc.) and user claims.

Which is exactly what this issue is requesting. :)

[HaoK]

Would hooking up a claims transformation to strip out the unwanted claims be enough for this scenario?

[kevinchalet]

@HaoK wouldn't claims transformation come too late? The OIDC/remote middleware immediately calls the cookie middleware via SignInAsync, so I'm not sure you can plug the CT middleware here.

Personally, I'm not sure the OIDC middleware is the best place for this change. I think it would be better to handle that directly in the JWT middleware, using a specific validation options.

[HaoK]

It's probably a gap right now that we don't have any claims transformation involved in the remote authentication handler flows before sign in passes the claims to another scheme...

[HaoK]

claims transformation is always needing love :(

[kevinchalet]

claims transformation is always needing love :(

We all need love ❤️

That said, if it's a feature you like, it should be quite easy to add it. We just need to call the transformer here: https://github.com/aspnet/Security/blob/dev/src/Microsoft.AspNetCore.Authentication/ClaimsTransformationHandler.cs#L67

[Tratcher]

@PinpointTownes that only works if you put the CTM in the pipeline between cookies and OIDC.

[kevinchalet]

@Tratcher yeah, it's not ideal. That's why such a feature might be better directly in IM.

[brentschmaltz]

@PinpointTownes @Tratcher @HaoK IM has support for claims filtering (inbound and outbound) when creating / serializing the ClaimsIdentity. See https://github.com/AzureAD/azure-activedirectory-identitymodel-extensions-for-dotnet/blob/dev/src/System.IdentityModel.Tokens.Jwt/JwtSecurityTokenHandler.cs#L120 . This was the whole idea for this feature. By default, I had originally removed nonce, etc... but pushback during beta 1 (i think) had it removed. It would be simple for asp.net options to add values here.

[kevinchalet]

@brentschmaltz sounds like a good idea. Now, the hard part is listing the "protocol claims" that should be ignored and determining if they should be excluded by default and if it represents a breaking change or not.

[brentschmaltz]

@PinpointTownes nonce, c_hash, at_hash are the obvious contenders to be dropped on the floor. I can't think of any other obvious ones. SAML has separate sections (statements) for such artifacts making such calls easier.

[kevinchalet]

What about iat/exp/nbf, iss, azp and aud?

[brentschmaltz]

@PinpointTownes those are good candidates. The iss is in every Claim as the Issuer property.

[brockallen]

nonce?

[brentschmaltz]

@brockallen yes we should kick 'nonce' to the curb.

[Tratcher]

Proposal: IM won't change it's defaults, so have Asp.Net use IM's claims filtering to prevent these extra claims from being added.

[leastprivilege]

Introduce a setting - FilterProtocolClaims - and make the list of protocol claims configurable. That's how we do it in our client libraries.

I wouldn't use the JWT handler built-in maps/filters.