-
Notifications
You must be signed in to change notification settings - Fork 363
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
Revert "feat: port clockskew support (#139)" #184
Conversation
This reverts commit d489c99.
If this is merged, we'll push a |
// be used to fine-tune the validation. The type used for the options is intentionally | ||
// un-exported, since its API and its naming is subject to change. | ||
Valid(opts ...validationOption) error | ||
Valid() error |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In order to maintain backwards compatibility, a ValidWithOptions(...)
could be added to the public Interface.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Even adding a new function will break the interface, as someone who implements their own Claims
structure, would need to implement ValidWithOptions
as well. I think we are stuck with this new feature until v5 :(
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Even adding a new function will break the interface, as someone who implements their own
Claims
structure, would need to implementValidWithOptions
as well. I think we are stuck with this new feature until v5 :(
Right. What you think about creating a new interface to extend Claims
?
type Claims interface {
Valid() bool
}
type ClaimsExtended interface {
Claims
ValidWithOptions() bool
}
type RegisteredClaims struct {
}
func (r *RegisteredClaims) Valid() bool {
fmt.Println("RegisteredClaims::Valid")
return true
}
type RegisteredClaimsExtended struct {
RegisteredClaims
}
func (r *RegisteredClaimsExtended) ValidWithOptions() bool {
fmt.Println("RegisteredClaimsExtended::ValidWithOptions")
return true
}
// call function accordingly to interface type (https://github.com/golang-jwt/jwt/blob/1096e506e671d6d6fe134cc997bbd475937392c8/parser.go#L87)
func Verify(claims Claims) bool {
// is this claims extended?
if claimsExt, ok := claims.(ClaimsExtended); ok {
return claimsExt.ValidWithOptions()
}
return claims.Valid()
}
func main() {
claims := &RegisteredClaims{}
fmt.Println(Verify(claims))
claimsExt := &RegisteredClaimsExtended{}
fmt.Println(Verify(claimsExt))
}
This reverts commit d489c99.
This reverts commit d489c99.
This reverts commit d489c99.
Fix #183
Reason: this is a breaking change within a major version (v4). Users might rely on the public interface and the change in signature may cause issues.