vimalloc / flask-jwt-extended

An open source Flask extension that provides JWT support (with batteries included)!
http://flask-jwt-extended.readthedocs.io/en/stable/
MIT License
1.56k stars 239 forks source link

PyJWT 2.10.0 Enforces `sub` #561

Closed jlucier closed 1 week ago

jlucier commented 1 week ago

Recently the version of the PyJWT dependency was upgraded to 2.10.0, which adds new token verification behavior. In this version PyJWT enforces that the sub claim in the JWT be a string, and deems the token invalid if that verification fails (by default). This issue in PyJWT has a discussion about the change, and ways to work around it: https://github.com/jpadilla/pyjwt/issues/1017

I have only just now become am aware that in the JWT spec the sub claim is indeed supposed to be a string, and have been using an integer in that field for a long time. This upgrade naturally has caught me out, since any tokens I issue in this way are now deemed invalid and result in a rejection. I don't believe I am alone in this situation, as multiple people piled onto the aforementioned Github issue being caught by the change.

To rectify the situation, it's clear that my application should start using strings for our sub claim. Unfortunately though, we can't simply make a hard cut over. We need preexisting tokens which haven't expired to continue to be valid. This means we need to be able to verify and parse older tokens that will have sub as an int. Right now, I'm not seeing any way to accomplish this besides pinning PyJWT to 2.9.0.

The recommended way (from PyJWT contributors) to navigate this seems to be specifying verify_sub=False as an option to their jwt.decode function per this comment. As a user of flask-jwt-extended, the high level functions verify_jwt_in_request and jwt_required don't seem to provide any way in the current API to control options like verify_sub (or any other low level options) in the underlying calls to PyJWT.

I took a look at the code, and I can see many possible avenues for adding such functionality (if desired). Before putting together a PR or anything, I figured I'd first ask:

  1. If this functionality is something you'd be interested in having or not
  2. If you have opinions on exactly how this kind of thing should be implemented if it were to exist.

Let me know what you think, thanks in advance for the feedback!

raj-patra commented 1 week ago

Came here to create this exact issue. I was very confused when this started to occur just today in my work environment.

vimalloc commented 1 week ago

I can see how not doing a hard cutoff would be a problem. I'm not opposed to a adding a way to set verify_sub=False for this use case. For brevity, I also want to mention another approach that might work depending on how your app is setup:

  1. Pin PyJWT to 2.9.0
  2. Update your application code to start using strings for newly created tokens
  3. (if needed) Write a wrapper around get_jwt_identity (or wherever you happen to be using the sub claim), so that it can handle both the old non-string data and the new string data in a uniformed way.
  4. Wait for all of the existing tokens tokens to naturally expire (typically whatever you have set for JWT_ACCESS_TOKEN_EXPIRES, or JWT_REFRESH_TOKEN_EXPIRES if you are using refresh tokens)
  5. Once that amount of time has passed and all the existing tokens are now using the new string data format, you can remove the wrapper around get_jwt_identity and upgrade PyJWT to >=2.10.0.

This would not be feasible if you have super long lived tokens, or if you are using allow_expired=True option for decode_token for whatever reason in your application, but otherwise might be a way to pretty easily make this change without causing any disruption for your users.

In terms of allowing verify_sub=False to be used in Flask-JWT-Extended, If we need to add that, I would like that to take the same approach that we did for the verify_aud configuration option. If we do need this functionality and you want to write a PR for it, by all means! Otherwise I am happy to get that PR added in the next day or two.

Let me know!

vimalloc commented 1 week ago

Also I'm going to link this comment I wrote in another ticket that outlines some ways that you could go about migrating sub to a string, in case it is helpful for anyone who sees this issue: https://github.com/vimalloc/flask-jwt-extended/issues/556#issuecomment-2486220755

jlucier commented 1 week ago

Good call on waiting the tokens out. Sadly though, that isn't a realistic option for my particular circumstance. I went ahead and made a PR to add the functionality.

vimalloc commented 1 week ago

Thanks for the PR. It has been released in version 4.7.1.

Cheers!