azure-activedirectory-identitymodel-extensions-for-dotnet

azure-activedirectory-identitymodel-extensions-for-dotnet copied to clipboard

Which version of Microsoft.IdentityModel are you using? 7.0.0-preview3

Where is the issue?

• [ ] M.IM.JsonWebTokens
• [ ] M.IM.KeyVaultExtensions
• [ ] M.IM.Logging
• [ ] M.IM.ManagedKeyVaultSecurityKey
• [ ] M.IM.Protocols
• [ ] M.IM.Protocols.OpenIdConnect
• [ ] M.IM.Protocols.SignedHttpRequest
• [ ] M.IM.Protocols.WsFederation
• [ ] M.IM.TestExtensions
• [x] M.IM.Tokens
• [ ] M.IM.Tokens.Saml
• [ ] M.IM.Validators
• [ ] M.IM.Xml
• [ ] S.IM.Tokens.Jwt
• Other (please describe)

Is this a new or an existing app?

Repro

var jwks = new JsonWebKeySet();

var key = new X509SecurityKey(cert);
var jwk = JsonWebKeyConverter.ConvertFromX509SecurityKey(key, true);
jwk.Use = "sig";

jwks.Keys.Add(jwk);

{
  "keys": [
    {
      "e": "AQAB",
      "key_ops": [],
      "kid": "7f4...",
      "kty": "RSA",
      "n": "v1L8m2wgy0ddn....",
      "oth": [],
      "use": "sig",
      "x5c": []
    }
  ]
}

Expected behavior key_ops, oth and x5c should be omitted since they were unset, not show up as empty lists

Actual behavior List fields of array/list type are returned as empty lists

Possible solution The getters seem to swap nulls with empty arrays before returning, which means [JsonIgnore(Condition = JsonIgnoreCondition.WhenWritingNull)] has no effect.

Additional context / logs / screenshots / links to code

Add any other context about the problem here, such as logs and screenshots or links to code.

@polys what are you using to Serialize? Newtonsoft? System.Text.Json?

@polys what are you using to Serialize?

System.Text.Json in a modern out-of-the-box ASP.NET project on .NET 7

@polys the issue here is key_ops, oth, x5c are collections that are non-null by design. This is a change from 6x where these properties had a get/set pair and could be null.

We have a JsonWebKeySetSerializer that is used internally to skip the empty arrays. Adding JsonWebKeySet.Write() would not be a solution for derived types.

I will discuss with the team.

We have a JsonWebKeySetSerializer that is used internally to skip the empty arrays.

Is JsonWebKeySetSerializer something we can use as well?

All I'm trying to do is have a simple API endpoint returning my public JWKS, similar to how the OIDC /.well-known/openid-configuration endpoint points to a jwks_uri.

I could of course serialise manually, or remove the extra fields manually, but if feels like a hack.

Adding JsonWebKeySet.Write() would not be a solution for derived types.

Is there a way you can narrow the scope or constrain the expectations somehow so this would not be an issue?

For example, saying something like Write RFC7517 only promises RFC7517-compliant serialisation, as opposed to Write that suggests complete serialisation, even for derived types.

Some options that come to mind:

• Add a WriteRfc7517() method on JsonWebKeySet (and potentially JsonWebKey), although I feel the name doesn't sound very .net-iny.
• Add a Write(options) method, which options allowing for RFC7517-compliant serialisation or custom. This feels more idiomatic and extensible.
• Expose your internal JsonWebKeySetSerializer but, similar to the above, use a name that doesn't suggest complete serialisation of every possible scenario.

I'm really quite excited and appreciative of the work you're all doing on version 7. Thank you :)

@polys we are up against a hard deadline to coordinate with asp.net 8. Opening up the JsonWebKeySetSerializer may make sense, but it will need some work for derived types.

In the past we have used controls such as 'bool strict' for compliance. Rfc7517, states parameter names are case-sensitive, the 'kty' parameter MUST exist, etc. As our 6x release was not strict, we decided to release 7 as lax.

I agree a bool is not enough, something similar to JsonSerializerOptions but scoped to JsonWebKey.

It would be best to design this with extensibility of OpenIdConnectConfiguration also as the two chain together.

We can improve this in .net 8.

This even causes a regression in our frontend because the JS crypto API complains about it:

DOMException: The JWK "key_ops" member was inconsistent with that specified by the Web Crypto call. The JWK usage must be a superset of those requested

We worked around this by special-casing JsonWebKey in our JsonTypeInfoResolver to ignore empty list properties:

// Workaround https://github.com/AzureAD/azure-activedirectory-identitymodel-extensions-for-dotnet/issues/2254
if (type == typeof(JsonWebKey) && jsonProperty.PropertyType.IsGenericType && jsonProperty.PropertyType.GetGenericTypeDefinition() == typeof(IList<>))
{
  jsonProperty.ShouldSerialize = (obj, _) =>
  {
    PropertyInfo propertyInfo = (PropertyInfo)jsonProperty.AttributeProvider;
    System.Collections.IList? list = (System.Collections.IList?)propertyInfo.GetValue(obj);
    return list != null && list.Count > 0;
  };
}

@polys @mus65 I think opening up the serializers for: JsonWebToken, JsonWebKey, JsonWebKey, OpenIdConnectConfiguration with extensibility seems to be a good solution.

Marked this as an enhancement.