Allow passing in additional jwt headers

[janwijbrand]

What kind of change does this PR introduce? (check at least one)

• Bugfix
• Feature
• Code style update
• Refactor
• Other, please describe:

Does this PR introduce a breaking change? (check one)

• Yes
• No

---

• You consent that the copyright of your pull request source code belongs to Authlib's author.

Referencing #391

I'd like to pass in addtional JWT headers into the client assertion, such as the kid so the recipient of the token can find the corresponding key at the registered JWKS endpoint.

Describe the solution you'd like

When setting up an PrivateKeyJWT() instance to register as client auth method, I'd like to pass in additional JWT headers such as the kid.

[janwijbrand]

This PR is based on #390.

Please let me know what I can do to make the PR qualify for merge. Thanks!

[bjmc]

You may already be doing this, but if you are allowing users to specify this algorithm, make sure you are very careful about discouraging (maybe even explicitly disallowing) the use of the none algorithm, which has been a source of many vulnerabilities in JWT-related libraries.

I notice some places where None is set as a default value and special checks like if not None are involved. We need to make it hard for users or future developers of this library to accidentally shoot themselves in the foot with insecure tokens. 🙂

[janwijbrand]

You may already be doing this, but if you are allowing users to specify this algorithm, make sure you are very careful about discouraging (maybe even explicitly disallowing) the use of the none algorithm, which has been a source of many vulnerabilities in JWT-related libraries.

I notice some places where None is set as a default value and special checks like if not None are involved. We need to make it hard for users or future developers of this library to accidentally shoot themselves in the foot with insecure tokens. 🙂

Thanks for the heads-up. I wonder: the PR is for code that creates the token, but the vulnerabilities you refer to seems to be about the verification of tokens, right? I do think indeed we should help developers to the right thing.

[bjmc]

You're correct about create/verify, but this code that creates the token should make it difficult (or IMHO impossible) to create an unsecured token.

[janwijbrand]

You're correct about create/verify, but this code that creates the token should make it difficult (or IMHO impossible) to create an unsecured token.

I agree and thanks for confirming my understanding of your remark. I hope the reviewer can assess if my approach at least makes it not too easy to forget passing in an signing alg.

[lepture]

I will not allow passing alg in parameter of ClientSecretJWT. Instead, if one want to use other alg, they can subclass ClientSecretJWT:

class MyClientSecretJWT(ClientSecretJWT):
    alg = 'HS512'

[bjmc]

@lepture you say "I will not allow passing alg in parameter of ClientSecretJWT" but then #390 looks like it allows passing alg as a parameter?

[janwijbrand]

@lepture you say "I will not allow passing alg in parameter of ClientSecretJWT" but then #390 looks like it allows passing alg as a parameter?

I think that's indeed what my PR allows (and that was my intention) and that PR is merged. And I'm really happy with that (I like this better then having to subclass), but @lepture's remark is confusing.

[janwijbrand]

And thanks @lepture for accepting the PR!!

[lepture]

@bjmc @janwijbrand I've modified the code 38ac0d2

[janwijbrand]

@bjmc @janwijbrand I've modified the code 38ac0d2

Ah, that clarifies it. Out of curiosity, may I ask why you prefer subclassing here over passing in a parameter when instantiating?

[bjmc]

I can't speak for lepture, but I think it's safer because it makes it harder for a novice user to accidentally pass an insecure algorithm (especially None). If you're going to the trouble of subclassing then you probably know what you're doing and have a good reason for it.