dvsekhvalnov / jose-jwt

Ultimate Javascript Object Signing and Encryption (JOSE), JSON Web Token (JWT) and Json Web Keys (JWK) Implementation for .NET and .NET Core
MIT License
936 stars 184 forks source link

Automatically include kid in encoded JWT if kid is available #212

Closed Smoovsky closed 1 year ago

Smoovsky commented 1 year ago

Here there,

I'm not sure whether this is intended behavior but it seems the JWT encoded by the library does not include kid in its headers even if it's available in the key used to encode unless you explicitly include the kid in extraHeaders.

Example:

    var key0 = RSA.Create(3072);
    var key1 = RSA.Create(3072);

    var privateJwks = new JwkSet()
    {
        new Jwk(key0)
        {
            KeyId = "0", // kid provided here
            Use = Jwk.KeyUsage.Signature
        },
        new Jwk(key1)
        {
            KeyId = "1", // kid provided here
            Use = Jwk.KeyUsage.Signature
        },
    };

    var payload = System.Text.Json.JsonSerializer.Serialize(new { iss = "1", sub = "2" });

    var jwt = JWT.Encode(
        payload,
        privateJwks.First(),
        JwsAlgorithm.PS256);

    // jwt's headers will not have kid unless it's encoded with
    // var jwt = JWT.Encode(
    //  payload,
    //  privateJwks.First(),
    //  JwsAlgorithm.PS256,
    //  extraHeaders: new Dictionary<string, object>() { { "kid", privateJwks.First().KeyId } });

If it's not intentional, please consider automatically adding kid in the encoded JWT as an improvement.

dvsekhvalnov commented 1 year ago

Hi @Smoovsky ,

well, yes, it's kind of intentional:

  1. JWK is not the only way to pass signing keys into library. And other methods don't have notion of Kid within them.
  2. It is still low level lib, doesn't trying to make assumptions about how you wanna use it or what kind of content, headers you want to express with your payloads.
Smoovsky commented 1 year ago

Hi @Smoovsky ,

well, yes, it's kind of intentional:

  1. JWK is not the only way to pass signing keys into library. And other methods don't have notion of Kid within them.
  2. It is still low level lib, doesn't trying to make assumptions about how you wanna use it or what kind of content, headers you want to express with your payloads.

Well, I guess it's kinda anti-instinctive if you pass in a key type with kid but in the output, it's just not there. But I'm fine with the current behavior, probably adding some notes in the documentation will help latecomers.