Added example and docs for revoking both tokens #187
Conversation
|
Hello @tecoholic! Thanks for updating the PR. Cheers ! There are no PEP8 issues in this Pull Request. 🍻 Comment last updated on January 01, 2019 at 11:13 Hours UTC |
vimalloc
left a comment
vimalloc
left a comment
There was a problem hiding this comment.
I've got a few minor nits here, but all and all this looks good 👍
examples/backlist_both_tokens.py
Outdated
| if username != 'test' or password != 'test': | ||
| return jsonify({"msg": "Bad username or password"}), 401 | ||
|
|
||
| user = {id: 1, username: 'test'} |
There was a problem hiding this comment.
I think this would be more idomatic if we passed it in via the @jwt.user_claims_loader and accessing it later via get_jwt_claims()
There was a problem hiding this comment.
But the claims loader function takes only one argument. I need to pass both the username and the refresh_jti when creating the access token. So had to do it manually
examples/backlist_both_tokens.py
Outdated
| @jwt_required | ||
| def logout(): | ||
| access_jti = get_raw_jwt()["jti"] | ||
| refresh_jti = get_raw_jwt()["identity"]["refresh_jti"] |
There was a problem hiding this comment.
Related to above, I think it would be more idiomatic if we pulled the token out of the get_jwt_claims() instead of the identity.
There was a problem hiding this comment.
Question, how do i go about using black list loader with a database like postgres?
| def refresh(): | ||
| current_user = get_jwt_identity() | ||
| refresh_jti = get_raw_jwt()['jti'] | ||
| current_user['refresh_jti'] = refresh_jti |
There was a problem hiding this comment.
Same thing here regarding the user_claims_loader
|
I have made a few more changes to the example. Kindly review |
| refresh_token = create_refresh_token(identity=user) | ||
|
|
||
| # Embed the refresh token's jti in the access_token | ||
| user["refresh_jti"] = get_jti(refresh_token) |
There was a problem hiding this comment.
I don't really like having having this data stored in what will become the current_user for the JWT. It complicates things, especially when using the complex objects from token and vice versa features in this extension. Could we use the @jwt.user_claims_loader instead to put the data into the JWT, then access it via get_jwt_claims() instead of current_user bellow?
|
I left some comments on your pull request, if you could take a look at them 👍 |
|
If this PR is still alive let me notice that the filename should be fixed: blacklist_both_tokens.py. |
|
|
||
| .. literalinclude:: ../examples/blacklist.py | ||
|
|
||
| Sometimes there will be situations where we would want to backlist both the access token and the refresh token in the logout call. It could be done by |
| we could write the login, logout and refresh functions as: | ||
|
|
||
|
|
||
| .. literalinclude:: ../examples/backlist_both_tokens.py |
|
I am closing this down. It has been a year and I haven't found time to do it the right way. Will try again with new code base sometime later. |
6 participants
No description provided.