Claims with multiple values are incorrectly serialized in 7.0.0-preview3

[kevinchalet]

There's a serious regression in 7.0.0-preview3 that affects the serialization of identities containing multiple claims of the same type:

var identity = new ClaimsIdentity("Test");
identity.AddClaim(new Claim("a", "value1"));
identity.AddClaim(new Claim("a", "value2"));
identity.AddClaim(new Claim("a", "value3"));
identity.AddClaim(new Claim("a", "value4"));

var descriptor = new SecurityTokenDescriptor
{
    SigningCredentials = new SigningCredentials(new RsaSecurityKey(RSA.Create(2048)), SecurityAlgorithms.RsaSha256),
    Subject = identity
};

var handler = new JsonWebTokenHandler();

var token = handler.CreateToken(descriptor);

7.0.0-preview creates a token with the expected JSON payload:

{
  "a": [
    "value1",
    "value2",
    "value3",
    "value4"
  ],
  "exp": 1692705694,
  "iat": 1692702094,
  "nbf": 1692702094
}

7.0.0-preview3 doesn't: two values are missing 😱

{
  "a": [
    "value1",
    "value2"
  ],
  "exp": 1692705632,
  "iat": 1692702032,
  "nbf": 1692702032
}

/cc @brentschmaltz @jennyf19 it's a serious regression that may have security implications as it can lead to incorrect authorization decisions. I suggest marking it as a priority bug and unlisting 7.0.0-preview3 until it's fixed 😃

[brentschmaltz]

@kevinchalet I have to say I am really surprised by this.
Keegan will fix it :-)

[kevinchalet]

Hahaha, no need to say I was surprised too 😄

[brentschmaltz]

@kevinchalet issue #2246 with the dictionary is different, preview1 was using our internal newtonsoft which has a very generous deserialization engine.

[kevinchalet]

Sadly, this will break existing versions of OpenIddict 😭

If you can't fix that, is there at least a way to get a JsonElement representing the JWT payload?
If so, is the JSON serializer replaceable in Wilson 7.0? (in this case, is there a guarantee a JsonElement instance will always be available?)

[brentschmaltz]

@kevinchalet we will fix #2246 by supporting a number of collections such as List, List, Collection, string[], object[], etc.

The question we are debating is how may such types do we support?
what about int[], double[], ...

[brentschmaltz]

fixed by: #2248

[kevinchalet]

The question we are debating is how may such types do we support?
what about int[], double[], ...

With AOT in mind, not wanting to support every possible combination is certainly understandable.

I'm personally - not at all - opposed to using JsonElement, but I need some answers to these questions 👇🏻 😄

If so, is the JSON serializer replaceable in Wilson 7.0? (in this case, is there a guarantee a JsonElement instance will always be available?)

[jmprieur]

@kevinchalet : would this work for you: #2255 ?

[kevinchalet]

It could certainly be an option, but I personally don't need "full serialization" support (i.e being able to project the JSON representation to any arbitrary CLR type). I actually just need a stable way to access the JSON tree, so the low-level JsonElement option is perfect.

I just need to know whether TryGetPayloadValue<JsonElement>(...) will be guaranteed to always return true in Wilson 7.x and higher. E.g if you plan on allowing to replace the JSON lib used under the hood, the answer will be "no", as JsonElement is a System.Text.Json-specific type.

[sebastienros]

I am not expert here but it looks like exposing something like this

public JsonElement HeaderElement => Header.RootElement; 

would provide all the flexibility @kevinchalet is asking for. And probably would be a solution to anyone who need some custom deserialization. Assuming that using STJ is here to stay, or at least do it for the platforms that have it in their runtime.

[kevinchalet]

@sebastienros yup, that would be the best option (assuming tying JsonWebToken to STJ is not against the design goals for 7.x+).

I opened #2260 earlier today to discuss that.