Skip to content
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

sub must be a string now? #1017

Open
ChronoDK opened this issue Nov 18, 2024 · 11 comments
Open

sub must be a string now? #1017

ChronoDK opened this issue Nov 18, 2024 · 11 comments

Comments

@ChronoDK
Copy link

It seems there is a breaking change in 2.10.0 - sub must be a string now. Is this intentional? Not sure how to handle this change for now, except keep using 2.9.0.

@WolfgangFellger
Copy link

#1005 most likely.

We also stumbled over having "sub": 1234 in our unit test token this morning. If this is illegal it should fail on jwt_encode already, which still happily produces these tokens.

Setting options={"verify_sub": False} prevents the InvalidSubjectError, if that happens to help you.

@vintas
Copy link

vintas commented Nov 18, 2024

I'm also facing this issue with my flask website with Flask-JWT-Extended.
The @jwt_required endpoint returns

{
    "msg": "Subject must be a string"
}

Downgraded to pip install PyJWT==2.9.0 for now.

PS: wrote very verbosely for others facing the same issues and searching around.

@umutseven92
Copy link

We just ran into this issue, thankfully our tests caught it before pushing to production. Pinning to 2.9.0 fixed it- I assume this is not a planned breaking change.

@M3te0r
Copy link

M3te0r commented Nov 18, 2024

Hello, We had this breaking change as well, but adopted the 2.10 version nonetheless as it was simple to fix

@yoch
Copy link

yoch commented Nov 18, 2024

We are affected by this change as well via Flask-JWT-Extended.

@mlanlazc
Copy link

mlanlazc commented Nov 18, 2024

This change caused all our protected requests to fail with 401. We caught it on dev environment, luckily.
Nonetheless, minor version bump should not introduce a breaking change.

@mathieufloury
Copy link

is it normal that a csrf field now appears in the web token from 2.10.0 whereas it was not there before ?

@msserpa
Copy link

msserpa commented Nov 19, 2024

Same issue here. Downgraded to 2.90

@benvdh-incentro
Copy link
Contributor

benvdh-incentro commented Nov 19, 2024

@ChronoDK The "sub" claim if present should indeed be a string according to the JWT RTF:

The "sub" value is a case-sensitive string containing a StringOrURI value. Use of this claim is OPTIONAL. SOURCE

So in that sense the validation logic is correct to throw an error. As for all other JWT claim validations, these validations are turned on by default (see #1005), so I can imagine it might have broken some code. In case no sub claim is provided in the JWT payload, the validator should still pass though.

I'm not author that built this functionality, but I was suggesting the 2.10.0 version number in my PR, to get some of the new functionality available through an official release. So my apologies if the version number created the wrong expectations, and broke any code. To go back to the old behaviour of 2.9.0 and below, just add the following as @WolfgangFellger already suggested:

my_jwt = "JWTGoesHere"
jwt.decode(my_jwt, options={"verify_sub": False, "verify_jti": False})

Or start using the new validation functionality:

my_jwt = "JWTGoesHere"
my_expected_subject = "someSubject"
jwt.decode(my_jwt, subject=my_expected_subject)

@Divan009
Copy link
Contributor

Hi everyone,

I appreciate the feedback regarding the sub claim validation introduced in version 2.10.0. My intention was to ensure compliance with the JWT specification, which states that the sub claim should be a string.

I understand that this change may have caused issues for some users whose JWTs contain non-string sub values or who were not expecting this validation to be enforced by default.

To address this you can follow the steps mentioned by @benvdh-incentro or @WolfgangFellger or refer to the solution provided here

Thank you for bringing this to my attention.

@amotl
Copy link

amotl commented Nov 22, 2024

Hi. FWIW, having been tripped on Apache Superset, we have been able to followup on your reference to flask-jwt-extended. Thank you very much, @Divan009.

Coming from there, and after receiving support on apache/superset#30995, the right solution for us was to update to flask-jwt-extended 4.7.1 released two days ago, and configure JWT_VERIFY_SUB = False.

wmfgerrit pushed a commit to wikimedia/pywikibot that referenced this issue Nov 30, 2024
- PyJWT < 2.10.0 is required due to
  jpadilla/pyjwt#1017
- increase leeway for mwoauth.identify function because the default
  value of 10.0 seconds is too low sometimes
- add __repr__ method for LoginManager

Bug: T380270
Change-Id: Ie7fadb26f5a3947343feb302f58098d266af2fee
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests