-
Notifications
You must be signed in to change notification settings - Fork 206
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
replace jwt-go #73
Comments
I'd like to explore using square/go-jose and try to fill in the gaps when it's not adhering to best practices that we at least care about for the purposes of the middleware. |
lestrrat-go/jwx would be another option to look at. It is ultimately a more opinionated package than go-jose though. I can throw up a small poc for it. I have a couple ideas, if that is cool. JWX provides variadic params for parsing and validating claims on the token, so id like to see if that could be leveraged well, but if we still want to provide a user defined validation func as an option, which i personally like, we can ignore those options |
@JayHelton sure, the more ideas the better! 👍 I'd also like to see how many "production grade" services use the libraries as well. |
Here is a WIP PoC (i havent touched tests yet) using Currently it takes in an array of ValidateOptions which are a JWX interface from the JWT module, and then just the Key as a Option. We can also bring back the ValidationKeyGetter and provide a more free form token check, which Im going to PoC on a different branch. I do want to point out that the JWX module does bring a bit of bloat in the modules with it. |
Ive done some more stewing on the use of JWX.
At first i was skeptical about wanting to change the middlewares interface to take in validation options as an array, instead of exposing a validation func that the token runs through. Ultimately we can still provide that functionality so the user of the middleware has a nice hook where a user can do whatever they want. Logging, complex validations, etc. If there are no strong arguments against it, im going to progress more on the PoC above to include some more ideas. |
Thanks for taking a stab at this for JWX @JayHelton. I looked over your code and have a couple of comments. Do you think you could open it up as a draft PR here so I (and others) can add the comments inline? |
Done! #76 Thank you! |
Perfect. I'll take a look this afternoon. |
Nice job @JayHelton - I left some comments. I also had a thought while looking over your code: what if instead of depending on a specific package we built an interface? We could then provide some common implementations such as using |
@grounded042 I like that idea! Im down to rubber ducky if you would like someone to bounce ideas off of. |
☝️ that's the first pass - sorry I didn't see the message sooner otherwise I would have been down to pair. Feedback in there is appreciated. If it looks like something we want to explore more I can take a pass at a first implementation. |
With this interface setup we could have some first adapters of JWX and square/go-jose. |
I've made some good progress on go-jose: v2...jon/go-jose-token-validator |
Just to be sure that you are also aware of this. There is currently a discussion going on if (or more how) the community is going to pickup maintaining |
Thanks @oxisto! I think we are set in this regard - we've built the replacement in such a way that you can switch out providers like jwt-go with a small amount of code. |
Understood. Is there anything we can do from the Update: I have looked into the interface you have built and spun up a quick implementation of this in https://github.com/oxisto/go-jwt-middleware/tree/jwt-go-validator. It is not yet finished and also still depends on the actual first release of https://github.com/golang-jwt/jwt. I can prepare a PR once that is done. |
Yes, the contribution of a |
What else needs to be done to switch over to github.com/golang-jwt/jwt? It appears the linked PR by @mohdrasbi needs to be approved. How can we get this moving? |
Ideally this project would not need that PR as we have #86. |
according to this GHSA-w73w-5m7g-f7qc jwt library updated import path but that import path is still not updated in the master. I saw the PR was out for that any idea how we can use that? |
We just released the v2.0.0-beta 🥳 ! You can start testing it by running In case of issues fetching the v2 you might want to try I'm closing this issue as now this is part of v2, but feel free to reopen if needed. |
We need to determine a replacement for the jwt-go dependency. Currently github.com/dgrijalva/jwt-go is not actively maintained, and we switched to https://github.com/form3tech-oss/jwt-go. However, the later also seems to have a lack of maintenance which leads us to seek a replacement.
We need to choose a package. In this issue I'd like to track discussing a replacement, doing some PoC work, and the actual replacement work.
So far I've been looking at the following criteria for replacement:
references
The text was updated successfully, but these errors were encountered: