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

Better documentation on revoking request tokens #453

Closed
tgross35 opened this issue Oct 13, 2021 · 7 comments
Closed

Better documentation on revoking request tokens #453

tgross35 opened this issue Oct 13, 2021 · 7 comments

Comments

@tgross35
Copy link
Contributor

Hey all,

I came across #53 which talks about how to revoke a refresh token. It is kind of unclear what exactly this comment references #53 (comment) and revoking refresh tokens isn't mentioned anywhere else. The only place with some documentation is this random site: https://darksun-flask-jwt-extended.readthedocs.io/en/latest/blacklist_and_token_revoking/ which does not appear to be official, or even tell you what version it's talking about, and it doesn't work anyway.

Basically I'd just like a blurb to be added to the refresh token page saying that they can be revoked in the same way as auth tokens as long as verify_jwt_in_request(refresh=True) (or the equivalent decorator) is included. It's just too easy to slip a developer's mind that refresh tokens do indeed need to be revoked, to prevent generating a new access token after one was invalidated.

I think there's also a possible implementation of allowing tokens to be tied a session cookie, then invalidating all refresh/access tokens for a specific session via a logout endpoint. But that's complicated above my use case.

@vimalloc
Copy link
Owner

Sounds reasonable to me. Want to take a stab at updating the documentation? Pull requests are always welcome! 👍

@tgross35
Copy link
Contributor Author

Sure, I'll see what I can come up with. No promises on timeliness but this isn't too bad:)

@Chu-Te-Ethan-Chen
Copy link

@tgross35 Hi, I want to revoke the refresh token but find no examples. I'm wondering if you can share your examples if you have time. Thanks!!

@tgross35
Copy link
Contributor Author

tgross35 commented Dec 2, 2021

Hi Jordan,

Guess I never got around to creating a PR lol. Anyway it's easy, same as revoking an access token but you just need to verify it is a refresh token instead. In my case, using methodviews and a table called TokenBlocklist, it looks like this:

def delete(self):
    """Block the given Refresh token"""
    verify_jwt_in_request(refresh=True)

    jti = get_jwt()["jti"]
    TokenBlocklist.create(jti=jti)

    return {"message": "Refresh token revoked"}, HTTPStatus.NO_CONTENT

which is identical to my regular revoke, except refresh=True is added. Hope it helps!

@Chu-Te-Ethan-Chen
Copy link

Chu-Te-Ethan-Chen commented Dec 2, 2021

Hi @tgross35,
Thanks for your blazing fast response. Your example indeed revokes a refresh token. It helps a lot lol !!!
However, I ran into another problem. This delete method does not work for an access token.
How do you handle this situation, by providing two endpoints to revoke tokens, one for access token and the other for the refresh token?
Is it possible to revoke access token or refresh token in only one endpoint ?

@tgross35
Copy link
Contributor Author

tgross35 commented Dec 2, 2021

@Jordon-Chen you're lucky to have caught me in the midst of a long docker build :)

My current method is like you said: I have /auth/access and /auth/refresh routes that accept POST to generate and DELETE to revoke access and refresh tokens, respectively.

You could theoretically have one endpoint that does it all using verify_jwt_in_request(optional=True) with and without refresh=True and seeing which one returns, but that seemed more effort than it's worth.

tgross35 added a commit to tgross35/flask-jwt-extended that referenced this issue Jan 8, 2022
@tgross35
Copy link
Contributor Author

tgross35 commented Jan 8, 2022

@vimalloc I just wrapped up changes for this in #460, take a look when you get a chance

I think this will play nicely with #441. The new argument I added verify_type currently accepts a bool - this could be expanded to accept a string or iterable of a custom type. If you OK #460, I’ll look at bringe #441 in.

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

3 participants