yglukhov / nim-jwt

JWT implementation for nim-lang
MIT License
51 stars 11 forks source link

Support for public and private header values. #8

Open jdbernard opened 4 years ago

jdbernard commented 4 years ago

Looking at the JWT standard(s) the list of possible headers is open-ended, providing for public and private name/value pairs in headers, similar to claims.

In the current implementation we only support 2 header names (alg and typ)

In particular, I am currently looking at a need to support the kid header in the context of an OAuth2 implementation validating a token created by a third-party auth server. The auth server publishes a list of signing keys and adds kid to the token header to allow the verifying party to select the proper key by id from the list of published keys.

I don't mind putting up a PR if this is in scope for the project.

yglukhov commented 3 years ago

Sorry, I missed this issue.

I don't mind putting up a PR if this is in scope for the project.

Yes, PRs are welcome! :)

salexkidd commented 3 years ago

+1

timotheecour commented 3 years ago

In the current implementation we only support 2 header names (alg and typ)

are you sure?

parseJson(decodeUrlSafeAsString(token.headerB64)).pretty

shows additional fields, eg: typ, kid, alg, iss, client, signer, exp which is as expected

jdbernard commented 2 years ago

token.headerB64 is not exposed on the JWT type (and decodeUrlSafeAsString isn't either). Maybe I'm missing something, but the following doesn't compile:

import json, jwt

let token = toJWT("eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCIsImtpZCI6IjEyM2FiYyJ9.eyJzdWIiOiIxMjM0NTY3ODkwIiwibmFtZSI6IkpvaG4gRG9lIiwiaWF0IjoxNTE2MjM5MDIyfQ.D3SN-jxFLsYxjVhVQVC6tlH2ZaRYQW991GbOctg8a2E")
echo parseJson(decodeUrlSafeAsString(token.headerB64)).pretty

Even if it did, we already parse the JSON header and throw away all values except the two mentioned above. Forcing the caller to re-parse the string token seems less than ideal. It would be nice to provide direct access to all of the claims in the header, like we do for the payload. It would also be nice if token.parsed would return all of the claims originally present.

My head hasn't been in the JWT specs for a while, so I'll have to think a bit on how we can best support this. On the one hand, IIRC the spec supports a potentially arbitrary set of claims in the header, similar to the payload. However, changing the implementation to an arbitrary lookup of claims would break code dependent on direct access to, for example, token.header.alg. In practice I think there is a fairly finite set of values typically used.

@yglukhov If I have time in the next two weeks I'll try to put together a PR with some ideas. I don't have a lot of visibility into how others are using this library, and this feels like an architectural change, so I'm sure I'm not going to get it right the first time. Maybe a start to the conversation anyways.