More consistent way of handling composite errors

Author: oxisto

errors.go

@@ -10,9 +10,10 @@ var (
 	ErrInvalidKeyType  = errors.New("key is of invalid type")
 	ErrHashUnavailable = errors.New("the requested hash function is unavailable")
 
-	ErrTokenMalformed        = errors.New("token is malformed")
-	ErrTokenUnverifiable     = errors.New("token is unverifiable")
-	ErrTokenSignatureInvalid = errors.New("token signature is invalid")
+	ErrTokenMalformed            = errors.New("token is malformed")
+	ErrTokenUnverifiable         = errors.New("token is unverifiable")
+	ErrTokenRequiredClaimMissing = errors.New("a required claim is missing")
+	ErrTokenSignatureInvalid     = errors.New("token signature is invalid")
 
 	ErrTokenInvalidAudience  = errors.New("token has invalid audience")
 	ErrTokenExpired          = errors.New("token is expired")
@@ -43,6 +44,7 @@ const (
 	ValidationErrorClaimsInvalid // Generic claims validation error
 )
 
+/*
 // NewValidationError is a helper for constructing a ValidationError with a string error message
 func NewValidationError(errorText string, errorFlags uint32) *ValidationError {
 	return &ValidationError{
@@ -119,3 +121,4 @@ func (e *ValidationError) Is(err error) bool {
 
 	return false
 }
+*/

go.mod

@@ -1,3 +1,3 @@
 module github.com/golang-jwt/jwt/v5
 
-go 1.16
+go 1.20

none.go

@@ -1,5 +1,7 @@
 package jwt
 
+import "fmt"
+
 // SigningMethodNone implements the none signing method.  This is required by the spec
 // but you probably should never use it.
 var SigningMethodNone *signingMethodNone
@@ -13,7 +15,7 @@ type unsafeNoneMagicConstant string
 
 func init() {
 	SigningMethodNone = &signingMethodNone{}
-	NoneSignatureTypeDisallowedError = NewValidationError("'none' signature type is not allowed", ValidationErrorSignatureInvalid)
+	NoneSignatureTypeDisallowedError = fmt.Errorf("%w: 'none' signature type is not allowed", ErrTokenUnverifiable)
 
 	RegisterSigningMethod(SigningMethodNone.Alg(), func() SigningMethod {
 		return SigningMethodNone
@@ -33,10 +35,7 @@ func (m *signingMethodNone) Verify(signingString, signature string, key interfac
 	}
 	// If signing method is none, signature must be an empty string
 	if signature != "" {
-		return NewValidationError(
-			"'none' signing method with non-empty signature",
-			ValidationErrorSignatureInvalid,
-		)
+		return fmt.Errorf("%w: 'none' signing method with non-empty signature", ErrTokenUnverifiable)
 	}
 
 	// Accept 'none' signing method.

parser.go

@@ -65,25 +65,25 @@ func (p *Parser) ParseWithClaims(tokenString string, claims Claims, keyFunc Keyf
 		}
 		if !signingMethodValid {
 			// signing method is not in the listed set
-			return token, NewValidationError(fmt.Sprintf("signing method %v is invalid", alg), ValidationErrorSignatureInvalid)
+			return token, fmt.Errorf("%w: signing method %v is invalid", ErrTokenSignatureInvalid, err)
 		}
 	}
 
 	// Lookup key
 	var key interface{}
 	if keyFunc == nil {
 		// keyFunc was not provided.  short circuiting validation
-		return token, NewValidationError("no Keyfunc was provided.", ValidationErrorUnverifiable)
+		return token, fmt.Errorf("%w: no keyfunc was provided", ErrTokenUnverifiable)
 	}
 	if key, err = keyFunc(token); err != nil {
-		// keyFunc returned an error
-		if ve, ok := err.(*ValidationError); ok {
-			return token, ve
-		}
-		return token, &ValidationError{Inner: err, Errors: ValidationErrorUnverifiable}
+		return token, fmt.Errorf("%w: error while executing keyfunc: %w", ErrTokenUnverifiable, err)
 	}
 
-	vErr := &ValidationError{}
+	// Perform signature validation
+	token.Signature = parts[2]
+	if err = token.Method.Verify(strings.Join(parts[0:2], "."), token.Signature, key); err != nil {
+		return token, fmt.Errorf("%w: %w", ErrTokenSignatureInvalid, err)
+	}
 
 	// Validate Claims
 	if !p.skipClaimsValidation {
@@ -93,29 +93,14 @@ func (p *Parser) ParseWithClaims(tokenString string, claims Claims, keyFunc Keyf
 		}
 
 		if err := p.validator.Validate(claims); err != nil {
-			// If the Claims Valid returned an error, check if it is a validation error,
-			// If it was another error type, create a ValidationError with a generic ClaimsInvalid flag set
-			if e, ok := err.(*ValidationError); !ok {
-				vErr = &ValidationError{Inner: err, Errors: ValidationErrorClaimsInvalid}
-			} else {
-				vErr = e
-			}
+			return token, err
 		}
 	}
 
-	// Perform validation
-	token.Signature = parts[2]
-	if err = token.Method.Verify(strings.Join(parts[0:2], "."), token.Signature, key); err != nil {
-		vErr.Inner = err
-		vErr.Errors |= ValidationErrorSignatureInvalid
-	}
-
-	if vErr.valid() {
-		token.Valid = true
-		return token, nil
-	}
+	// No errors so far, token is valid.
+	token.Valid = true
 
-	return token, vErr
+	return token, nil
 }
 
 // ParseUnverified parses the token but doesn't validate the signature.
@@ -127,7 +112,7 @@ func (p *Parser) ParseWithClaims(tokenString string, claims Claims, keyFunc Keyf
 func (p *Parser) ParseUnverified(tokenString string, claims Claims) (token *Token, parts []string, err error) {
 	parts = strings.Split(tokenString, ".")
 	if len(parts) != 3 {
-		return nil, parts, NewValidationError("token contains an invalid number of segments", ValidationErrorMalformed)
+		return nil, parts, fmt.Errorf("%w: token contains an invalid number of segments", ErrTokenMalformed)
 	}
 
 	token = &Token{Raw: tokenString}
@@ -136,20 +121,20 @@ func (p *Parser) ParseUnverified(tokenString string, claims Claims) (token *Toke
 	var headerBytes []byte
 	if headerBytes, err = DecodeSegment(parts[0]); err != nil {
 		if strings.HasPrefix(strings.ToLower(tokenString), "bearer ") {
-			return token, parts, NewValidationError("tokenstring should not contain 'bearer '", ValidationErrorMalformed)
+			return token, parts, fmt.Errorf("%w: tokenstring should not contain 'bearer '", ErrTokenMalformed)
 		}
-		return token, parts, &ValidationError{Inner: err, Errors: ValidationErrorMalformed}
+		return token, parts, fmt.Errorf("%w: %w", ErrTokenMalformed, err)
 	}
 	if err = json.Unmarshal(headerBytes, &token.Header); err != nil {
-		return token, parts, &ValidationError{Inner: err, Errors: ValidationErrorMalformed}
+		return token, parts, fmt.Errorf("%w: %w", ErrTokenMalformed, err)
 	}
 
 	// parse Claims
 	var claimBytes []byte
 	token.Claims = claims
 
 	if claimBytes, err = DecodeSegment(parts[1]); err != nil {
-		return token, parts, &ValidationError{Inner: err, Errors: ValidationErrorMalformed}
+		return token, parts, fmt.Errorf("%w: %w", ErrTokenMalformed, err)
 	}
 	dec := json.NewDecoder(bytes.NewBuffer(claimBytes))
 	if p.useJSONNumber {
@@ -163,16 +148,16 @@ func (p *Parser) ParseUnverified(tokenString string, claims Claims) (token *Toke
 	}
 	// Handle decode error
 	if err != nil {
-		return token, parts, &ValidationError{Inner: err, Errors: ValidationErrorMalformed}
+		return token, parts, fmt.Errorf("%w: %w", ErrTokenMalformed, err)
 	}
 
 	// Lookup signature method
 	if method, ok := token.Header["alg"].(string); ok {
 		if token.Method = GetSigningMethod(method); token.Method == nil {
-			return token, parts, NewValidationError("signing method (alg) is unavailable.", ValidationErrorUnverifiable)
+			return token, parts, fmt.Errorf("%w: signing method (alg) is unavailable", ErrTokenUnverifiable)
 		}
 	} else {
-		return token, parts, NewValidationError("signing method (alg) is unspecified.", ValidationErrorUnverifiable)
+		return token, parts, fmt.Errorf("%w: signing method (alg) is unspecified", ErrTokenUnverifiable)
 	}
 
 	return token, parts, nil

parser_test.go

@@ -359,7 +359,7 @@ func TestParser_Parse(t *testing.T) {
 
 			// Parse the token
 			var token *jwt.Token
-			var ve *jwt.ValidationError
+			//var ve *jwt.ValidationError
 			var err error
 			var parser = data.parser
 			if parser == nil {
@@ -390,7 +390,7 @@ func TestParser_Parse(t *testing.T) {
 				t.Errorf("[%v] Inconsistent behavior between returned error and token.Valid", data.name)
 			}
 
-			if data.errors != 0 {
+			/*if data.errors != 0 {
 				if err == nil {
 					t.Errorf("[%v] Expecting error. Didn't get one.", data.name)
 				} else {
@@ -405,7 +405,7 @@ func TestParser_Parse(t *testing.T) {
 						}
 					}
 				}
-			}
+			}*/
 
 			if data.err != nil {
 				if err == nil {

validator.go

@@ -2,6 +2,7 @@ package jwt
 
 import (
 	"crypto/subtle"
+	"errors"
 	"time"
 )
 
@@ -56,8 +57,10 @@ func newValidator(opts ...ParserOption) *validator {
 // Validate validates the given claims. It will also perform any custom
 // validation if claims implements the CustomValidator interface.
 func (v *validator) Validate(claims Claims) error {
-	var now time.Time
-	vErr := new(ValidationError)
+	var (
+		now  time.Time
+		errs []error = make([]error, 0)
+	)
 
 	// Check, if we have a time func
 	if v.timeFunc != nil {
@@ -69,56 +72,49 @@ func (v *validator) Validate(claims Claims) error {
 	// We always need to check the expiration time, but usage of the claim
 	// itself is OPTIONAL
 	if !v.VerifyExpiresAt(claims, now, false) {
-		vErr.Inner = ErrTokenExpired
-		vErr.Errors |= ValidationErrorExpired
+		errs = append(errs, ErrTokenExpired)
 	}
 
 	// We always need to check not-before, but usage of the claim itself is
 	// OPTIONAL
 	if !v.VerifyNotBefore(claims, now, false) {
-		vErr.Inner = ErrTokenNotValidYet
-		vErr.Errors |= ValidationErrorNotValidYet
+		errs = append(errs, ErrTokenNotValidYet)
 	}
 
 	// Check issued-at if the option is enabled
 	if v.verifyIat && !v.VerifyIssuedAt(claims, now, false) {
-		vErr.Inner = ErrTokenUsedBeforeIssued
-		vErr.Errors |= ValidationErrorIssuedAt
+		errs = append(errs, ErrTokenUsedBeforeIssued)
 	}
 
 	// If we have an expected audience, we also require the audience claim
 	if v.expectedAud != "" && !v.VerifyAudience(claims, v.expectedAud, true) {
-		vErr.Inner = ErrTokenInvalidAudience
-		vErr.Errors |= ValidationErrorAudience
+		errs = append(errs, ErrTokenInvalidAudience)
 	}
 
 	// If we have an expected issuer, we also require the issuer claim
 	if v.expectedIss != "" && !v.VerifyIssuer(claims, v.expectedIss, true) {
-		vErr.Inner = ErrTokenInvalidIssuer
-		vErr.Errors |= ValidationErrorIssuer
+		errs = append(errs, ErrTokenInvalidIssuer)
 	}
 
 	// If we have an expected subject, we also require the subject claim
 	if v.expectedSub != "" && !v.VerifySubject(claims, v.expectedSub, true) {
-		vErr.Inner = ErrTokenInvalidSubject
-		vErr.Errors |= ValidationErrorSubject
+		errs = append(errs, ErrTokenInvalidSubject)
 	}
 
 	// Finally, we want to give the claim itself some possibility to do some
 	// additional custom validation based on a custom function
 	cvt, ok := claims.(customClaims)
 	if ok {
 		if err := cvt.CustomValidation(); err != nil {
-			vErr.Inner = err
-			vErr.Errors |= ValidationErrorClaimsInvalid
+			errs = append(errs, err)
 		}
 	}
 
-	if vErr.valid() {
+	if len(errs) == 0 {
 		return nil
 	}
 
-	return vErr
+	return errors.Join(errs...)
 }
 
 // VerifyExpiresAt compares the exp claim in claims against cmp. This function