pyjwt
pyjwt copied to clipboard
Remove **kwargs from encode() and decode()
I recently had a situation where I was passing algorithm instead of algorithms to jwt.decode(). Due to the **kwargs eating the algorithm parameter and the decode method looking at the encoded JWT to figure out what type of algorithm to use, everything appeared to be working the way you would expect, even though it wasn't actually doing what I wanted it to.
If there wasn't the **kwargs there, my code would have raised a TypeError instead and immediately let me know I wasn't using the API correctly. Personally, I think that is very desirable.
Would you be interested in a patch to remove **kwargs and replace them all with named arguments instead?
Glad I wasn't the only one who made that mistake :)
It looks like we don't currently allow **kwargs on encode() but I do think we should definitely remove the **kwargs from decode().
This may be a non-trivial change to make since we currently have several arguments such as audience= and issuer= which are not currently explicitly enumerated in the function definitions for decode().
Give it a shot if you'd like. 👍
This permissive function signature (along with a typo) resulted in what appears to be quite a serious vulnerability in the social-auth-core library:
https://github.com/python-social-auth/social-core/commit/eed3007c4ccdbe959b1a3ac83102fe869d261948#diff-c00186529f1572bf1eba22a82687e9d9bb4a5424a84f82a9e6f3bf41ce800e02L128
I believe this would allow someone to forge Apple ID auth tokens by using HS256 and Apple's public key, although I have not tested this.
The JWT spec's acceptance of token-specified algorithms has been the cause of a number of vulnerabilities, so there should be a good deal of caution around anything that could fail to pin the algorithms list. (I'd argue that the algorithms arg should be mandatory, in fact, but that's a ticket for another day... :-P)
[EDIT: Clarified that I meant that the JWT spec, not the pyjwt library, in last paragraph.]
This issue is stale because it has been open 60 days with no activity. Remove stale label or comment or this will be closed in 7 days
This appears to still be an issue: https://github.com/jpadilla/pyjwt/blob/a863a7329027fe1e993c866374b195785978b2a9/jwt/api_jwt.py#L72
(I'm not sure it makes sense to have stalebot running on this one.)
This issue is stale because it has been open 60 days with no activity. Remove stale label or comment or this will be closed in 7 days