Closed oxisto closed 3 years ago
Hey @oxisto thanks for this! I'm a little swamped at the moment, but I'm carving out some time on Friday to review this.
Merging #91 (fe44742) into v2 (665e7da) will increase coverage by
1.85%
. The diff coverage is100.00%
.
@@ Coverage Diff @@
## v2 #91 +/- ##
==========================================
+ Coverage 90.56% 92.42% +1.85%
==========================================
Files 2 3 +1
Lines 106 132 +26
==========================================
+ Hits 96 122 +26
Misses 9 9
Partials 1 1
Impacted Files | Coverage Δ | |
---|---|---|
validate/jwt-go/jwtgo.go | 100.00% <100.00%> (ø) |
Continue to review full report at Codecov.
Legend - Click here to learn more
Δ = absolute <relative> (impact)
,ø = not affected
,? = missing data
Powered by Codecov. Last update 665e7da...fe44742. Read the comment docs.
Overall this looks great and thank you for the contribution! I left a couple of comments and nits on cleanups.
It follows largely the same style as the jose one, with a few exceptions:
- Clock skew is not supported
- Expected claims can not be set
Not supporting clock skew isn't a blocker, but it is a downside here. One of the features we want to support is clock skew. Is it possible that
golang-jwt/jwt
will support this in the future?Not supporting expected claims is no problem as that's a setup from the other package.
Thanks for the review, I will look at the smaller changes. I am planning to add clock skew among other validation features from the old v4
to golang-jwt/jwt
in a backwards compatible way (see https://github.com/golang-jwt/jwt/issues/16). Unfortunately, I do not have a dedicate time-frame for this, yet. June is quite busy here.
Awesome! Knowing it's on the backlock is a big help!
https://github.com/golang-jwt/jwt will serve as a long-term replacement to https://github.com/dgrijalva/jwt-go. Following the ideas discussed in #73, I have added a validator that uses the new repository.
It follows largely the same style as the jose one, with a few exceptions:
I want to hold of on marking this "ready" until there is a first
v3.2.1
release of https://github.com/golang-jwt/jwt, which will happened soon.Additionally, it would probably make sense to put the validators in their own go module, so that only the validators have a dependency to the actual jwt libraries, not the middleware itself.