jjwt
jjwt copied to clipboard
jwtk
Add ignore validation options and static methods for validation of claims independent to token parsing
This pull request contains the features requested in #80 and additionally code refactoring to reuse claim validation codes in static validation methods.
With these changes we can:
- Parse token by ignoring expiration, notbefore claim values or signature problems.
- Do these validations explicitly and separately by calling static validation methods.
Use case: As part of our token expiration strategy, we create tokens with short life spans and re-create new tokens if needed. To do this in more convenient way, we need aforementioned ignore methods while parsing. This way we can do additional security checks like XSRF token validation or check JWT integrity by validating signature seperately, even if the JWT token expired.
P.S.: I must add new unit test as Java code, since I have no idea how to write code in groovy. Additionally I made some code refactoring and splitted huge methods into smaller. I hope the names of new methods are appropriate to existing (if any) naming conventions.
Thanks.
Coverage decreased (-0.6%) to 99.369% when pulling 0f4a6c130b8f50cb2129518b5092a0d4d5fb610c on selimok:master into 48dae365b18aadf6323b32f89f63960cca3a3f47 on jwtk:master.
Please write the tests in Groovy - we have standardized on it for testing and it's even easier than Java. We won't accept a PR with Java tests or without 100% coverage.
Actually, we can convert the Java to Groovy - I don't want to discourage your PR. If you can though, please try!
Coverage decreased (-0.6%) to 99.369% when pulling e97430785f624d290d14c331a76d07ebf8bba181 on selimok:master into 48dae365b18aadf6323b32f89f63960cca3a3f47 on jwtk:master.
Coverage decreased (-0.09%) to 99.91% when pulling 8ebd92c978014898f870115bb571ddacba5d1b63 on selimok:master into 48dae365b18aadf6323b32f89f63960cca3a3f47 on jwtk:master.
Coverage decreased (-0.09%) to 99.91% when pulling 8ae456fcda9924480a6f800d7f65c3c7faa9d6dd on selimok:master into 48dae365b18aadf6323b32f89f63960cca3a3f47 on jwtk:master.
Do you have an idea how can fix that https://coveralls.io/jobs/18754160/source_files/1580589642#L15 ?
Thanks.
I added a code review - the use of the static class is why you're getting code coverage failures. HTH!
Coverage decreased (-2.5%) to 97.484% when pulling 735934d61972fe4f6c2eafc35fd032c1a2b7ffb3 on selimok:master into 48dae365b18aadf6323b32f89f63960cca3a3f47 on jwtk:master.
Coverage remained the same at 100.0% when pulling 80758c256f61d1bf0d8d4405dacba95a1c7f66de on selimok:master into 48dae365b18aadf6323b32f89f63960cca3a3f47 on jwtk:master.
Coverage remained the same at 100.0% when pulling a0e4b693239fd86f3d9406ff7116ab4a7bc8f148 on selimok:master into 48dae365b18aadf6323b32f89f63960cca3a3f47 on jwtk:master.
Coverage remained the same at 100.0% when pulling e4ae4c522dae89c1c2c2283854892f8c6a31c518 on selimok:master into 48dae365b18aadf6323b32f89f63960cca3a3f47 on jwtk:master.
If my PR less likely to be accepted, I would open a new feature request for setting clock skew seperately for notBefore and expiry claims. So that I can at least explicitly set a great amount of clock skew to simulate ignore expiry behaviour. Currently if I set the clock skew value, it affects both nb and exp claim validation.
Thanks.
Hi @selimok - sorry - we just haven't had time to review this due to conferences (Gartner, re:Invent) and holiday/end-of-quarter deadlines. We'll review this as soon as we can!
Hi @lhazlewood, no problem. Sorry for my impatience. Thanks in advance for your effort.
@selimok No worries! I'm sorry for the delay! We really appreciate your patience, and we're grateful for the PR!
Hey, what's the status of this pr? We are also trying to accomplish a validation without expiration date even though it's contained in the jwt.
Wow @selimok delivers a PR and all the requested changes but 6 months later there is still no feedback from the devs. 6 months! This is a great example how not to handle an open source project. If you have no time to support it anymore please add it to the README.MD so that others can decide if they really want to integrate this or look for another solution.
Years after the PR, all the quick fix @selimok done. still not merged... now I bet too much conflicts to merge... contribute they said...
@jonalbertini we're taking a different route with a proper Validator interface/builder with chaining methods and such for a future release. This one slipped through the cracks because it was too specialized and not flexible enough for custom validation. We're going with a different approach.
See #474
Closing just as a matter of housekeeping in favor of the work that will be done via #474. If you feel otherwise, please let us know and we can discuss.