Added support for custom JWT types #441
Conversation
|
Sorry it's taking me so long to look at this, had a bunch of stuff going on. I'll try to get a proper look at this in the next few days. |
| def verify_jwt_in_request( | ||
| optional=False, fresh=False, refresh=False, locations=None, token_type="access" | ||
| ): |
There was a problem hiding this comment.
I'm not sure I like having refresh and token_type in here, that seems confusing to understand from a top level API point of view. I'm not entirely sue how that could be handled without a breaking change though. Hmm... 🤔 🤔 🤔
| # Test refresh token access to jwt_required | ||
| response = test_client.get(url, headers=make_headers(refresh_token)) | ||
| assert response.status_code == 422 | ||
| assert response.get_json() == {"msg": "Token of type refresh is not allowed"} |
There was a problem hiding this comment.
Not a big deal, but I wonder if the error message should say what token type is allowed instead of this specific token is now allowed? Might make for more discoverable errors?
| elif not refresh and decoded_token["type"] != token_type: | ||
| raise WrongTokenError(f"Token of type { decoded_token['type'] } is not allowed") |
There was a problem hiding this comment.
I think this might be a breaking change. I believe some people are using JWTs from other sources that set the type to other fields, and we implictly treat those as access tokens right now.
I haven't fully thought this out, but I wonder if we instead set token_type to null in the view decorators and verify_jwt_in_request, and then we preserve the old logic here unless a token_type is actually specified?
|
This would be a nice to have feature. Will it be merged? |
Not as is. The breaking change would need to be addressed, and I’m still not sure I like how the API looks for this change, but would be open to feedback on that front. If anyone wants to continue working on this I’m not necessarily opposed to it 👍 |
|
I agree this would be nice to have. Found this PR while looking into making a "registration" type token to build out a registration flow using JWT. Based on @tgross35's comments in an other PR, I agree it probably won't be the most used feature and I think it's reasonable to make it slightly less accessible. |
Added the ability to create tokens with types other than "access" and "refresh", among some other intuitive changes. This is done in such a way that will not introduce any breaking changes. This is useful in such scenarios such as password reset emails, where a JWT needs to be provided to authenticate the user, but an access token and refresh token don't fulfill that purpose.
Summary of changes
create_custom_tokenwith an additional parametertoken_typeverify_jwt_in_request/jwt_requiredto allow for the additional specification of atoken_typeverify_token_typeto check for custom token types, and modified the resulting error messages accordingly.expires_deltais not provided, instead of the refresh token expiry time as before.