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

Bring over ValidationHelper functionality from v4 branch #16

Closed
oxisto opened this issue May 28, 2021 · 10 comments · Fixed by #236
Closed

Bring over ValidationHelper functionality from v4 branch #16

oxisto opened this issue May 28, 2021 · 10 comments · Fixed by #236
Milestone

Comments

@oxisto
Copy link
Collaborator

oxisto commented May 28, 2021

The v4 branch has a nice validation helper struct which combines a lot of the functionality of the Validate functions. It might be nice to re-implement that and slowly move existing functionality to it.

This would include things that have been on the wishlist in other forks, such as having leeway when validating timestamps (see form3tech-oss/jwt-go#12).

@mcquackers
Copy link

I was wondering about the implementation of the leeway:
Why does the incorporation of leeway depend on the validating server instead of the authorizing client?
To me, it seems like it would be cleaner for the JWT to be the source of truth, with leeway built into the iat/exp

@dgurns
Copy link

dgurns commented Oct 13, 2021

Just ran into the "token used before issued" error today for the first time, after things had been working fine previously.

The discussion at #98 referenced this issue. Are there any plans to implement it and disregard the pre-IAT check?

@oxisto
Copy link
Collaborator Author

oxisto commented Oct 13, 2021

Just ran into the "token used before issued" error today for the first time, after things had been working fine previously.

The discussion at #98 referenced this issue. Are there any plans to implement it and disregard the pre-IAT check?

In a world with infinite time, yes :) Currently I am waiting for #108 to be merged. This one will be next.

@dgurns
Copy link

dgurns commented Oct 15, 2021

Awesome, appreciate your work!

@miparnisari
Copy link

Currently I am waiting for #108 to be merged.

it's been merged 👀 :)

@oxisto
Copy link
Collaborator Author

oxisto commented Nov 21, 2021

I had a quick look yesterday and I am not sure if we can port this over in a backwards compatible manner, since the interface of Claims needs to be changed. Especially the Valid function needs to have an additional parameter for the ValidationHelper. I guess I will first focus on the functionality and then see whether we can port this to v4 or if this will then introduce a v5 branch with breaking changes.

@ksegun
Copy link
Contributor

ksegun commented Dec 6, 2021

This is something we relied on from the old library as we ran on the preview version and would help us move to adopt this port.

@arinto
Copy link

arinto commented Dec 10, 2021

Yes, we also rely in this functionality from the old library :)

@oxisto , regarding your statement below:

I guess I will first focus on the functionality and then see whether we can port this to v4 or if this will then introduce a v5 branch with breaking changes.

So, what are the intended functionality that are targeted in v4.x?

@oxisto
Copy link
Collaborator Author

oxisto commented May 28, 2022

Please see #211 for an ongoing discussion how to proceed.

@oxisto oxisto linked a pull request Aug 27, 2022 that will close this issue
5 tasks
@oxisto oxisto modified the milestones: v4.1.0, v5.0.0 Aug 27, 2022
@oxisto oxisto mentioned this issue Aug 28, 2022
@oxisto
Copy link
Collaborator Author

oxisto commented Feb 21, 2023

Fixed by #234

@oxisto oxisto closed this as completed Feb 21, 2023
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 a pull request may close this issue.

6 participants