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

replace jwt-go #73

Closed
grounded042 opened this issue Jan 29, 2021 · 21 comments
Closed

replace jwt-go #73

grounded042 opened this issue Jan 29, 2021 · 21 comments
Milestone

Comments

@grounded042
Copy link
Contributor

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:

  • actively maintained
  • idiomatic go
  • adheres to RFC 7519
  • support best practices in RFC 8725

references

@grounded042 grounded042 added this to the v2 milestone Jan 29, 2021
@cyx
Copy link

cyx commented Jan 29, 2021

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.

@JayHelton
Copy link

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

@cyx
Copy link

cyx commented Feb 1, 2021

@JayHelton sure, the more ideas the better! 👍 I'd also like to see how many "production grade" services use the libraries as well.

@JayHelton
Copy link

JayHelton commented Feb 4, 2021

Here is a WIP PoC (i havent touched tests yet) using lestrrat-go/jwx master...JayHelton:jwx-poc-one

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.

@JayHelton
Copy link

Ive done some more stewing on the use of JWX.

  • lestrrat-go/jwx does seem to be getting favor in the community.
  • the maintainer is responsive and consistently doing work in the repo
  • the implementation is straightforward, though i would like some thoughts on how idiomatic it is (not a lot of professional go experience)

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.

@grounded042
Copy link
Contributor Author

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?

@JayHelton
Copy link

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!

@grounded042
Copy link
Contributor Author

Perfect. I'll take a look this afternoon.

@grounded042
Copy link
Contributor Author

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 JWX. I briefly passed it by @cyx and I think I'll take a spike to see what that interface might look like.

@JayHelton
Copy link

@grounded042 I like that idea!
Then servers could chose whichever impl they feel most confident about.

Im down to rubber ducky if you would like someone to bounce ideas off of.

@grounded042
Copy link
Contributor Author

☝️ 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.

@grounded042
Copy link
Contributor Author

grounded042 commented Feb 19, 2021

With this interface setup we could have some first adapters of JWX and square/go-jose.

@grounded042
Copy link
Contributor Author

I've made some good progress on go-jose: v2...jon/go-jose-token-validator

@oxisto
Copy link
Contributor

oxisto commented May 23, 2021

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 jwt-go in dgrijalva/jwt-go#462 after the original author came back from its hiatus and suggested migrating maintenance.

@grounded042
Copy link
Contributor Author

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.

@oxisto
Copy link
Contributor

oxisto commented May 29, 2021

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 jwt-go side of things, e.g. to write a validator PR similar to the one you have for jose? Or would that be something external to the project? We do have of course some interest to keep the jwt-go project alive, since it has/had quite a large user base.

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.

@grounded042
Copy link
Contributor Author

Is there anything we can do from the jwt-go side of things, e.g. to write a validator PR similar to the one you have for jose?

Yes, the contribution of a jwt-go validator via a PR would be welcome. I took a quick glance at the code you have up and you are on the right track! This is much appreciated 🙇

@jamiecuthill
Copy link

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?

@grounded042
Copy link
Contributor Author

Ideally this project would not need that PR as we have #86. v2 is feature complete IMO and just needs to be merged in. It already includes golang-jwt/jwt. As I mentioned in #86 I left Auth0 about a month ago so I am no longer maintaining this project, but I do want to see v2 released and did hand the project off before I left.

@ashish-nikalje
Copy link

ashish-nikalje commented Oct 29, 2021

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?

@sergiught
Copy link
Contributor

We just released the v2.0.0-beta 🥳 !

You can start testing it by running go get github.com/auth0/go-jwt-middleware/[email protected].

In case of issues fetching the v2 you might want to try go clean --modcache first before doing go get.

I'm closing this issue as now this is part of v2, but feel free to reopen if needed.

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

No branches or pull requests

7 participants