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

Make the jti claim optional as per RFC 7519 Section 4.1.7 #63

Closed
wants to merge 1 commit into from

Conversation

ajhodges
Copy link
Contributor

RFC 7519 Section 4.1.7 states that the jti claim is optional. It's fine for this library to be opinionated and always include a jti on generated tokens, but it should not require the claim to be present when validating tokens. This PR removes that check.

The unit tests on this PR will fail until #61 is merged.

@davesque
Copy link
Member

Is there a reason you don't want that claim to be present? The purpose of the claim in this library is to provide a key into the token blacklist.

@ajhodges
Copy link
Contributor Author

I don't use simplejwt to create my tokens (I actually use https://github.com/juanifioren/django-oidc-provider), I'm using simplejwt more as a substitute for this unmaintained library: jpadilla/django-rest-framework-jwt#449

And I suspect others will be looking into this library for the same reasons as well.

@davesque
Copy link
Member

davesque commented Feb 20, 2019

@ajhodges

Cool, good to know. It's making me realize that I had originally designed this library as a self-contained solution for a particular project on which I was working but I neglected to consider how it might interact with other JWT systems.

Off the top of my head, I believe removing the jti requirement would have implications for other parts of the library that deal with token revocation. But I'll have to look over everything again to be sure. Feel free to poke around yourself and summarize whatever you find out here.

@ajhodges
Copy link
Contributor Author

When I made this change I did a quick search through all the references to jti and didn't see anything that really stuck out to me (but we also are not revoking tokens atm). For whatever it's worth we've been using our fork with this change for the past month or so, and the authentication class/tokenuser functionality has been working great for us in multiple projects.

@ajhodges
Copy link
Contributor Author

This has been addressed via #115

@ajhodges ajhodges closed this Sep 19, 2019
@ajhodges ajhodges deleted the optional-jti branch September 19, 2019 16:19
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

Successfully merging this pull request may close these issues.

2 participants