-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
Parser throws exception when token is expired #440
Comments
This is due to the JWT specification: https://tools.ietf.org/html/rfc7519#section-4.1.4 Quote:
(Bold emphasis is mine). A parser rejecting the jws (i.e. MUST NOT be accepted) is a pretty clear-cut-and-dry adherence to the RFC specification IMO. What are you trying to do that requires that you trust an expired JWS? Maybe context will help me understand what's trying to be accomplished and a best practice for that. |
Closing as duplicate of #148 . If you want to violate the specification, you'll be able to by getting the JWT from the exception. Due to the security implications however, we won't be altering the exception-throwing design of JJWT. |
Parsing and processing are not the same thing, and not in the spirit of the point of the spec. I think your interpretation of the spec needs to be peer reviewed, on a more regular basis. This is not the first time I've noticed this library making assumptions like that, in fact I think it's the 3rd time I have personally noticed that the interpretation of the spec was misapplied in a significantly detrimental way. Which is why you may read into my response, some annoyance. Please consider enlisting some support, because unilateral anti-pattern decisions like this, cause compatibility issues when they are corrected. |
If you start using an expired JWT immediately after the parser returned quietly - as you would have it do - what would you call your usage? Once you start reading it and interpreting it and making logic decisions based on it - is that not processing? And consequently in direct violation of the spec?
According to whom? How do you know we haven't spoken with JWT RFC specification committee members? I would happily discuss with you or any other contributors on this project the merit of what you claim if you can provide some credible authoritative evidence (e.g. spec committee members or contributors or security experts) to the contrary. I genuinely mean this and I'm happy to oblige.
It has been - many, many, many times. That is why I'm using 'we' and not 'I'. Most of us agree with JJWT's stance on secure parsing because the pitfalls without it are significant. The only people I've seen disagree are you and some others in some of the GH issues here (and even some of them have changed their mind in favor of JJWT's current approach). I respectfully submit this is not enough evidence to overturn what we and most others consider a best practice.
It is no assumption, I can assure you. It was a carefully crafted, very calculated behavioral choice based on the exact wording in the RFC specification as well as the significant security attack vectors that can happen with JWTs if library users are not careful. Believe me - we agonize over this stuff. Every line of code in this project has a very clear intent for what is going on. It is why the entire codebase has 100% line and branch/conditional test coverage. We do our very best to make JJWT insanely easy to use, but sometimes we choose to favor security over usability.
Again, not unilateral (peer reviewed and discussed with many), and definitely not an anti-pattern according to those we've spoken with security backgrounds. And we've had great support so far, and more people lend a hand all the time. Finally, I will end this by saying that we're all human and fallible, and I could be wrong. But so could you. As much as I respect your desire and passion behind what you are advocating, you've presented only conjecture and opinion as to why JJWT's security stance should change. Unfortunately that's just not enough to override what I, @dogeared, and the other significant contributors believe to be the correct direction for JJWT's design. If you'd like to provide evidence from spec committee members or credible security experts that our design is flawed, I'll be the first person to work with you to design a better approach. Cheers! |
Ok, real world software happening here. Think through the logic of throwing an exception, and preventing any more information from being gleaned from the token, just because it's expired. Ok, so the token is expired. In a simple approach, the client would simply ask for a new token. My problem with this, and with every other time I have had to raise issues, is that the library doesn't take into account what software really has to do. The library makes far too many assumptions about how it's being used, that only match a very narrow profile. The library developer can't possibly know what is the right thing to do at the time it's being executed, and tries to impose its will on the calling code, via an exception. To make things worse, it isn't even a checked exception! You can imagine how frustrating it is, when a library does something contrary to the "principle of least astonishment", and actively prevents normal flow. In this case, the anti-pattern is pretty much control flow by exception. |
While I get the intention behind strictly rejecting expired tokens, an option to manually override that behavior and allow a token to be expired for parsing can be a real-world requirement. There are use-cases where you want to parse JWTs even when they have expired. Although admittedly, those use-cases might be rare. In my specific use-case, I am working on an OAuth2/OpenID Connect Provider (OP) and am currently implementing the OpenID Connect RP-Initiated Logout 1.0 specification. I need to parse the
It's an edge-case of course, because I still need to to the signature check but am accepting expired tokens. I will probably need to implement my own parser on this route, that's not that big of a deal. But would have been nice if I could also use the jjwt library for that 😄 Edit: I just realized that my use-case was already mentioned in a comment in #148 |
When parsing a token that has expired, the parser should not throw an exception, as was implemented in #6, because then the rest of the data in the token is inaccessible.
This appears to have been brought up before, and ignored in #379.
This exact case is happening to me now. I'm literally parsing the token to check if it has expired, but before I can evaluate it, I get an exception. Ok, so now I know its expired, but I can't get any other data out of it to determine how I should refresh it.
The text was updated successfully, but these errors were encountered: