feat: port clockskew support #139
Conversation
The approach is similar to that used by the golang mongodb driver to maintain backwards compatibility
Thanks for the contribution, very much welcome, this is a feature that was on the wish list for a long time. This is definitely something I also wanted to explore as part of the new validator helper approach. I promise that I will find some time on the upcoming weekend to have a look at this, unfortunately we are quite in a end-of-the-year rush here. I also want to align this with the work I have previously started as well. |
|
Agree with much of what @oxisto said. (I'm also a bit swamped) My only 2 cents is we should take our time and think through whether this is the correct API and ensure this is a backwards compatible change. (not saying it's not, just that we think through this). We should also use functional options instead of a variadic struct pointer. |
|
Agree, we would prefer to have functional options for those of us stuck on the v4 preview in the old library we are quite happy with them. As we evaluated our options on whether to port to a different library with skew support or this, this was a trade-off that we thought could be adequate until functional options are made available and there could be far easier ways as well now that I consider it e.g. an environment variable. We fully support whatever approach is decided on. |
# Conflicts: # claims.go # map_claims.go # parser_option.go
|
@oxisto this is now all functional style |
ksegun
force-pushed
the
clockskew-support
branch
from
90bfa5e
to
64c4dc6
Compare
Nice! I will have a detailed look on this in the upcoming days. Thanks for the improvements |
There was a problem hiding this comment.
LGMT. This is / should be 100 % backwards compatible and fixes a long-wanted feature. Furthermore, it allows us to go into the direction of a validation helper that fine-tunes validation of the parser, also completely backwards compatible and incrementally. All new types are unexported, so we are free to change / stabilize the API. All function signature changes are variadic.
I definitely want to get some more votes on this before we merge though
cc @golang-jwt/maintainers
|
@mfridman I have tweaked the documentation. I think we can merge it then? We are still free to change the internal names in the future. |
Ideally, we already started them in a way that we are happy with so not too many breaking changes are necessary. That is the main reason why I wanted to keep most of the validation structs and functions unexpected, so we can easily change them again internally. |
|
ksegun:clockskew-support |
Co-authored-by: Kolawole Segun <Kolawole.Segun@kyndryl.com> Co-authored-by: Christian Banse <oxisto@aybaze.com>
Co-authored-by: Kolawole Segun <Kolawole.Segun@kyndryl.com> Co-authored-by: Christian Banse <oxisto@aybaze.com>
This reverts commit d489c99.
Co-authored-by: Kolawole Segun <Kolawole.Segun@kyndryl.com> Co-authored-by: Christian Banse <oxisto@aybaze.com>
This reverts commit d489c99.
The approach is similar to that used by the golang mongodb driver to maintain backwards compatibility
This should mitigate #16