Repository: lestrrat-go/jwx
Pull request #699 (2022-04-16 09:53:08)

Change jwt.Validator API to return jwt.ValidationError

codecov-commenter: # [Codecov](https://codecov.io/gh/lestrrat-go/jwx/pull/699?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=lestrrat-go) Report
> Merging [#699](https://codecov.io/gh/lestrrat-go/jwx/pull/699?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=lestrrat-go) (bb800b4) into [develop/v2](https://codecov.io/gh/lestrrat-go/jwx/commit/07a6852c94b0ea14e3ef4a7fa59dd51deac668cf?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=lestrrat-go) (07a6852) will **decrease** coverage by `0.02%`.
> The diff coverage is `100.00%`.

```diff
@@              Coverage Diff               @@
##           develop/v2     #699      +/-   ##
==============================================
- Coverage       71.56%   71.53%   -0.03%     
==============================================
  Files              90       89       -1     
  Lines            9740     9731       -9     
==============================================
- Hits             6970     6961       -9     
  Misses           1969     1969              
  Partials          801      801              
```


| [Impacted Files](https://codecov.io/gh/lestrrat-go/jwx/pull/699?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=lestrrat-go) | Coverage Δ | |
|---|---|---|
| [jwt/validate.go](https://codecov.io/gh/lestrrat-go/jwx/pull/699/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=lestrrat-go#diff-and0L3ZhbGlkYXRlLmdv) | `91.60% <100.00%> (+0.05%)` | :arrow_up: |
| [internal/json/goccy.go](https://codecov.io/gh/lestrrat-go/jwx/pull/699/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=lestrrat-go#diff-aW50ZXJuYWwvanNvbi9nb2NjeS5nbw==) | | |

------

[Continue to review full report at Codecov](https://codecov.io/gh/lestrrat-go/jwx/pull/699?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=lestrrat-go).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=lestrrat-go)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/lestrrat-go/jwx/pull/699?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=lestrrat-go). Last update [07a6852...bb800b4](https://codecov.io/gh/lestrrat-go/jwx/pull/699?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=lestrrat-go). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=lestrrat-go).


Patch:
diff --git a/Changes-v2.md b/Changes-v2.md
index 864ae4ce..22e12618 100644
--- a/Changes-v2.md
+++ b/Changes-v2.md
@@ -306,6 +306,9 @@ jws.Parse(serialized,
   The rest of the arguments are treated as options passed to the
   `(jwk.Fetcher).Fetch()` function.
 
+* Remove `jws.WithPayloadSigner()`. This should be completely repleceable
+  using `jws.WithKey()`
+
 * jws.WithKeyProvider() has been added to specify arbitrary
   code to specify which keys to try.
 
@@ -341,6 +344,10 @@ jws.Parse(serialized,
 * `jwt.WithVerify()` has been renamed to `jwt.WithKey()`. The option can
   be used for signing, encryption, and parsing.
 
+* `jwt.Validator` has been changed to return `jwt.ValidationError`.
+  If you provide a custom validator, you should wrap the error with
+  `jwt.NewValidationError()`
+
 * `jwt.UseDefault()` has been removed. You should use `jws.WithUseDefault()`
   as a suboption in the `jwt.WithKeySet()` option.
 
@@ -369,11 +376,6 @@ case ...:
 }
 ```
 
-* Remove `jws.WithPayloadSigner()`. This should be completely repleceable
-  using `jws.WithKey()`
-
-* `jwt.Sign()` has been changed so that it works more like the new `jws.Sign()`
-
 * `jwt.WithHeaders` and `jwt.WithJwsHeaders` have been removed.
   You should be able to use the new `jwt.WithKey` option to pass headers
 
@@ -383,3 +385,6 @@ case ...:
 
 * `jwt.ReadFile` now supports the option `jwt.WithFS` which allows you to
   read data from arbitrary `fs.FS` objects
+
+* `jwt.Sign()` has been changed so that it works more like the new `jws.Sign()`
+
diff --git a/jwk/interface.go b/jwk/interface.go
index a8a3763f..31b2f07e 100644
--- a/jwk/interface.go
+++ b/jwk/interface.go
@@ -54,9 +54,8 @@ type Set interface {
 	AddKey(Key) error
 
 	// Clear resets the list of keys associated with this set, emptying the
-	// internal list of `jwk.Key`s
-	// This method will be changed in the future to clear all contents in the
-	// `jwk.Set` instead of just the keys.
+	// internal list of `jwk.Key`s, as well as clearing any other non-key
+	// fields
 	Clear() error
 
 	// Get returns the key at index `idx`. If the index is out of range,
diff --git a/jwt/validate.go b/jwt/validate.go
index 750b165c..d8148d5e 100644
--- a/jwt/validate.go
+++ b/jwt/validate.go
@@ -119,7 +119,7 @@ func MinDeltaIs(c1, c2 string, dur time.Duration) Validator {
 	}
 }
 
-func (iitr *isInTimeRange) Validate(ctx context.Context, t Token) error {
+func (iitr *isInTimeRange) Validate(ctx context.Context, t Token) ValidationError {
 	clock := ValidationCtxClock(ctx) // MUST be populated
 	skew := ValidationCtxSkew(ctx)   // MUST be populated
 	// We don't check if the claims already exist, because we already did that
@@ -142,6 +142,7 @@ func (iitr *isInTimeRange) Validate(ctx context.Context, t Token) error {
 type ValidationError interface {
 	error
 	isValidationError()
+	Unwrap() error
 }
 
 func NewValidationError(err error) ValidationError {
@@ -154,6 +155,9 @@ type validationError struct {
 }
 
 func (validationError) isValidationError() {}
+func (err *validationError) Unwrap() error {
+	return err.error
+}
 
 var errTokenExpired = NewValidationError(fmt.Errorf(`"exp" not satisfied`))
 var errInvalidIssuedAt = NewValidationError(fmt.Errorf(`"iat" not satisfied`))
@@ -161,34 +165,31 @@ var errTokenNotYetValid = NewValidationError(fmt.Errorf(`"nbf" not satisfied`))
 
 // ErrTokenExpired returns the immutable error used when `exp` claim
 // is not satisfied
-func ErrTokenExpired() error {
+func ErrTokenExpired() ValidationError {
 	return errTokenExpired
 }
 
 // ErrInvalidIssuedAt returns the immutable error used when `iat` claim
 // is not satisfied
-func ErrInvalidIssuedAt() error {
+func ErrInvalidIssuedAt() ValidationError {
 	return errInvalidIssuedAt
 }
 
-func ErrTokenNotYetValid() error {
+func ErrTokenNotYetValid() ValidationError {
 	return errTokenNotYetValid
 }
 
 // Validator describes interface to validate a Token.
 type Validator interface {
 	// Validate should return an error if a required conditions is not met.
-	// This method will be changed in the next major release to return
-	// jwt.ValidationError instead of error to force users to return
-	// a validation error even for user-specified validators
-	Validate(context.Context, Token) error
+	Validate(context.Context, Token) ValidationError
 }
 
 // ValidatorFunc is a type of Validator that does not have any
 // state, that is implemented as a function
-type ValidatorFunc func(context.Context, Token) error
+type ValidatorFunc func(context.Context, Token) ValidationError
 
-func (vf ValidatorFunc) Validate(ctx context.Context, tok Token) error {
+func (vf ValidatorFunc) Validate(ctx context.Context, tok Token) ValidationError {
 	return vf(ctx, tok)
 }
 
@@ -227,7 +228,7 @@ func IsExpirationValid() Validator {
 	return ValidatorFunc(isExpirationValid)
 }
 
-func isExpirationValid(ctx context.Context, t Token) error {
+func isExpirationValid(ctx context.Context, t Token) ValidationError {
 	if tv := t.Expiration(); !tv.IsZero() && tv.Unix() != 0 {
 		clock := ValidationCtxClock(ctx) // MUST be populated
 		now := clock.Now().Truncate(time.Second)
@@ -251,7 +252,7 @@ func IsIssuedAtValid() Validator {
 	return ValidatorFunc(isIssuedAtValid)
 }
 
-func isIssuedAtValid(ctx context.Context, t Token) error {
+func isIssuedAtValid(ctx context.Context, t Token) ValidationError {
 	if tv := t.IssuedAt(); !tv.IsZero() && tv.Unix() != 0 {
 		clock := ValidationCtxClock(ctx) // MUST be populated
 		now := clock.Now().Truncate(time.Second)
@@ -275,7 +276,7 @@ func IsNbfValid() Validator {
 	return ValidatorFunc(isNbfValid)
 }
 
-func isNbfValid(ctx context.Context, t Token) error {
+func isNbfValid(ctx context.Context, t Token) ValidationError {
 	if tv := t.NotBefore(); !tv.IsZero() && tv.Unix() != 0 {
 		clock := ValidationCtxClock(ctx) // MUST be populated
 		now := clock.Now().Truncate(time.Second)
@@ -319,7 +320,7 @@ func IsValidationError(err error) bool {
 	}
 }
 
-func (ccs claimContainsString) Validate(_ context.Context, t Token) error {
+func (ccs claimContainsString) Validate(_ context.Context, t Token) ValidationError {
 	v, ok := t.Get(ccs.name)
 	if !ok {
 		return NewValidationError(fmt.Errorf(`claim %q not found`, ccs.name))
@@ -356,7 +357,7 @@ func ClaimValueIs(name string, value interface{}) Validator {
 	return &claimValueIs{name: name, value: value}
 }
 
-func (cv *claimValueIs) Validate(_ context.Context, t Token) error {
+func (cv *claimValueIs) Validate(_ context.Context, t Token) ValidationError {
 	v, ok := t.Get(cv.name)
 	if !ok {
 		return NewValidationError(fmt.Errorf(`%q not satisfied: claim %q does not exist`, cv.name, cv.name))
@@ -375,7 +376,7 @@ func IsRequired(name string) Validator {
 
 type isRequired string
 
-func (ir isRequired) Validate(_ context.Context, t Token) error {
+func (ir isRequired) Validate(_ context.Context, t Token) ValidationError {
 	_, ok := t.Get(string(ir))
 	if !ok {
 		return NewValidationError(fmt.Errorf(`%q not satisfied: required claim not found`, string(ir)))


Test patch:
diff --git a/examples/jwt_validate_validator_example_test.go b/examples/jwt_validate_validator_example_test.go
index d1124706..9813ee07 100644
--- a/examples/jwt_validate_validator_example_test.go
+++ b/examples/jwt_validate_validator_example_test.go
@@ -10,7 +10,7 @@ import (
 )
 
 func ExampleJWT_ValidateValidator() {
-	validator := jwt.ValidatorFunc(func(_ context.Context, t jwt.Token) error {
+	validator := jwt.ValidatorFunc(func(_ context.Context, t jwt.Token) jwt.ValidationError {
 		if t.IssuedAt().Month() != 8 {
 			return jwt.NewValidationError(errors.New(`tokens are only valid if issued during August!`))
 		}
diff --git a/jwt/validate_test.go b/jwt/validate_test.go
index c64ae851..6c5873ee 100644
--- a/jwt/validate_test.go
+++ b/jwt/validate_test.go
@@ -397,10 +397,10 @@ func TestClaimValidator(t *testing.T) {
 	t.Parallel()
 	const myClaim = "my-claim"
 	err0 := errors.New(myClaim + " does not exist")
-	v := jwt.ValidatorFunc(func(_ context.Context, tok jwt.Token) error {
+	v := jwt.ValidatorFunc(func(_ context.Context, tok jwt.Token) jwt.ValidationError {
 		_, ok := tok.Get(myClaim)
 		if !ok {
-			return err0
+			return jwt.NewValidationError(err0)
 		}
 		return nil
 	})