-
Notifications
You must be signed in to change notification settings - Fork 671
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Updating PyJWT to 2.10.0 broke login #831
Comments
I had the same issue today. |
ahh ok yeah I've just run into this issue as well, also using flask_jwt_extended and found this issue today. good to see I'm not the only one, I guess we enforce the previous version for now until this is fixed? btw @nagasiNR I found that 2.9.0 works still so maybe you can try that one. but as soon as you hit 2.10.0 or 2.10.1 it breaks. |
There was a PR #837 that tried to ignore the sub claim by @grayver . I believe a proper fix would be to stringify sub claim. A proper migration path forward would be:
The fix involves: upgrading SimpleJWT by a minor version, capping PyJWT to <2.10 due to this issue, then bumping SimpleJWT to a major version and removing the PyJWT version cap. What are everyone's thoughts here on this migration path forward? |
Since RFC defines "sub" claim as a string, it should be string. But current version of SimpleJWT makes it integer out of the box, so stringifying it breaks back compatibility.
No I would consider the following patching way:
|
Why should we ignore sub claim if PyJWT < 2.10? It's always been working on <2.10; even if it was silently failing, people could be relying on it. Removing stuff unnecessarily could be a breaking change too. We should simply cap the version. Lmk if I'm misunderstanding. |
Andrew.
I haven’t checked the standard to see if there is any ambiguity on the sub
is a string so assuming it’s clear and unambiguous for this email.
It seems to me if the project is creating invalid JWT’s then if libraries
are catching these are being tougher on this the user experience will get
worse over time as peoples systems might expand and they are interoperating
with anything else, rolling their own, doing their own validations.
The initial step would seem to me starting with pinning the older version
of the PyJWT library to prevent the silent failure people suffer.
But then I would suggest the long term solution should be to see if we can
transparently handle the situations while emitting a deprecation warning.
Eventually escalating to a breaking change with a good log entry.
Do you think this is a good path forward?
Regards
Alexander
Alexander Neilson
Neilson Productions Limited
***@***.***
021 329 681
022 456 2326
…On Sun, 5 Jan 2025 at 15:24, Andrew Chen Wang ***@***.***> wrote:
Why should we ignore sub claim if PyJWT < 2.10? It's always been working
on <2.10; even if it was silently failing, people could be relying on it.
Removing stuff unnecessarily could be a breaking change too. We should
simply cap the version. Lmk if I'm misunderstanding.
—
Reply to this email directly, view it on GitHub
<#831 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/ADISKLGS2NFBLNHYK5Z5O332JCJXDAVCNFSM6AAAAABSDGCYJCVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDKNZRGQ3TIOBYGU>
.
You are receiving this because you are subscribed to this thread.Message
ID: ***@***.***
com>
|
We shouldn't. I proposed either pin PyJWT version to <2.10 or ignore sub claim check by PyJWT. I agree that pinning PyJWT looks cleaner solution, so we can go this way in the next minor release. But remove this pin in the next major release, where sub claim becomes a string. |
Yes exactly. This is what I'm assuming happened as well for the sub field. A similar change was made during the PyJWT 1.7.1 to PyJWT 2.0 transition as well where they required a string rather than any serializable type for many of their parameters.
Perfect thanks for the review @grayver Let's go with this path. |
I don't usually write issues, so bear with me. We noticed our dev environment suddenly wasn't letting anyone log in. It turns out it's because PyJWT updated to 2.10.0, and that began enforcing that
subject
be a string.In this section, the code only sometimes converts the id to a string
That returns a seemingly fine token. However, when trying to use it, downstream from here
vv
All the way down in
it blows up
because
sub
is3
and not"3"
, i guess.Maybe if simplejwt passes
verify_sub: False
when it calls jwt.decode in/usr/local/lib/python3.11/site-packages/rest_framework_simplejwt/backends.py(139)
it would work? PyJWT seems to merge optionsThis seems to stem from the changes added in jpadilla/pyjwt#1005
The text was updated successfully, but these errors were encountered: