golang-jwt / jwt

Go implementation of JSON Web Tokens (JWT).
https://golang-jwt.github.io/jwt/
MIT License
6.98k stars 335 forks source link

Permit only certain errors on parsing #395

Open JonasDoe opened 2 months ago

JonasDoe commented 2 months ago

My scenario is, that for example I want skip the validation under certain circumstances. To achieve that, I invoke jwt.ParseWithClaims(...) and want to check afterward whether it was the signature check which failed. I understand that I could achieve most of that with errors.Is(myParsingErr, jwt.ErrTokenSignatureInvalid)

My gripe with that solution is that I'ld implicitly accept other errors wrapped in myParsingErr - as long as my one permitted error is amongst those -, and I'm not sure whether this could be exploited, e.g. when ErrTokenInvalidClaims "hides" an invalid signature.

My workaround for now is:

var allJWTErrs = [...]error{
    jwt.ErrInvalidKey, jwt.ErrInvalidKeyType, jwt.ErrHashUnavailable, jwt.ErrTokenMalformed, jwt.ErrTokenUnverifiable,
    jwt.ErrTokenSignatureInvalid, jwt.ErrTokenRequiredClaimMissing, jwt.ErrTokenInvalidAudience, jwt.ErrTokenExpired,
    jwt.ErrTokenUsedBeforeIssued, jwt.ErrTokenInvalidIssuer, jwt.ErrTokenInvalidSubject, jwt.ErrTokenNotValidYet,
    jwt.ErrTokenInvalidId, jwt.ErrTokenInvalidClaims, jwt.ErrInvalidType,
}

// isAtMostOneOfTheseJWTErrs check whether the given error is no jwt error, apart from the exceptions
func isAtMostOneOfTheseJWTErrs(toCheck error, jwtErrExceptions ...error) bool {
    for _, knownErr := range allJWTErrs {
        if !slices.ContainsFunc(jwtErrExceptions, func(exception error) bool {
            return errors.Is(toCheck, exception)
        }) {
            if errors.Is(toCheck, knownErr) {
                return false
            }
        }
    }
    return true
}

But this is logic must be checked/maintained whenever a new minor version of the jwt library gets released, to ensure all possible errors are covered. Therefore, it would be nice if all possible errors - so basically the array I'm creating myself atm - would be exposed by the library. Or if there was a check for that provided by the jwt library itself.

mfridman commented 1 week ago

I wonder if a type ParseError struct{} would help here.

Although wouldn't you still need to errors.As and then ignore certain classes of errors?

I'm not sure we want to expose a list of errors, that's not something I've observed in the wild. But the standard solution is to expose an error type.

JonasDoe commented 1 week ago

Hm, a ParseError struct might be nice b/c it could come with some method receivers which cover that logic:

func (err *ParseError) ToOtherErrThan(jwtErrExeptions ...errror)(remaining error) //result allows run-out-the-mill if err != nil check.