Adding a JSON claim to the JWT/Userinfo endpoint is a bit unintuitive because of different valueType constants

[raff-run]

When searching on how to add a JSON claim to a JWT, you'll come across answers like this one, which use System.IdentityModel.Tokens.Jwt.JsonClaimValueTypes.Json to specify the valueType, and that works with JwtSecurityTokenHandler.WriteToken.

But since ClaimsExtensions.cs checks for claim.ValueType == IdentityServerConstants.ClaimValueTypes.Json and IdentityServerConstants.ClaimValueTypes.Json is "json" while System.IdentityModel.Tokens.Jwt.JsonClaimValueTypes.Json is "JSON", that means that adding a claim like this: context.IssuedClaims.Add(new Claim("json", "{\"sample\":{\"sample\":\"value\"}}", System.IdentityModel.Tokens.Jwt.JsonClaimValueTypes.Json))

will make the claim an escaped JSON in the JWT instead of actual JSON.

[AndersAbel]

What version of Duende.IdentityServer are you using? What version of Asp.Net Core? What version of the Microsoft.IdentityModel libraries?

[josephdecock]

@brockallen do you think we should relax our logic here and allow either constant? I feel like this has come up in the past and don't quite remember if there is some wrinkle.

[raff-run]

I am using the latest version of each, this fiddle here shows how IdentityServer's constant is not capitalized: https://dotnetfiddle.net/qniKJj

And this link shows how ClaimsExtensions.cs checks the valueType https://github.com/DuendeSoftware/IdentityServer/blob/237ffcae018893954ed678a9137f0ebd501d7921/src/IdentityServer/Extensions/ClaimsExtensions.cs#L87

From what I've searched in the repo, the constant itself could be updated without breaking anything since it's only used in 4 places, instead of changing the logic.

[brockallen]

IOW, why can't we do this:

if (claim.ValueType == IdentityServerConstants.ClaimValueTypes.Json || 
    claim.ValueType == System.IdentityModel.Tokens.Jwt.JsonClaimValueTypes.Json)

I guess that would make sense. @leastprivilege any recollection around this topic and thought why this would not be ok?

[leastprivilege]

By the time we added this, the JWT handler only had very limited JSON serialization capabilities. Would be also interesting to see if they now can serialize complex objects or lists...

[josephdecock]

Transferring to the IdentityServer repo to consider for a future release.

[runegri]

By the time we added this, the JWT handler only had very limited JSON serialization capabilities. Would be also interesting to see if they now can serialize complex objects or lists...

Things look better now. We have seen the same issue on our side (HelseID) but are investigating if we can remove the workarounds. I can update here if things look good.