Comparison of token parsing errors

[fresonn]

My parsing of token:

token, err := jwt.ParseWithClaims(tokenString, &accessToken{}, func(token *jwt.Token) (interface{}, error) {
	return []byte(t.secretKey), nil
})

I took an example of how to distinguish between errors from the documentation

if err != nil {
	if err.(*jwt.ValidationError).Errors&jwt.ValidationErrorExpired != 0 {
		return &accessToken{
			UserId: token.Claims.(*accessToken).UserId,
		}, ErrTokenExpired
	}
	return nil, err
}

And here's what I noticed. This patch can fire even when the error is not a token expiration error.

if err.(*jwt.ValidationError).Errors&jwt.ValidationErrorExpired != 0 { ...

For example, this condition will work even on the "signature is invalid" error. But only if the token has expired and it has been changed. In other words:

Token	Condition
Token has been changed AND has not expired yet	doesn't work
Token changed AND expired	It works

But the condition should not work on an error like "signature is invalid"?

P.S Can I compare errors like this?

if err.(*jwt.ValidationError).Errors&jwt.ValidationErrorExpired == jwt.ValidationErrorExpired { ...

[fresonn]

UPD I supplemented the check:

if err.(*jwt.ValidationError).Errors&jwt.ValidationErrorExpired != 0 && err.(*jwt.ValidationError).Errors&jwt.ValidationErrorSignatureInvalid == 0

It seems to work! Or tell me what is better to do?

[oxisto]

We implemented go 1.13 error style checking a while ago, you can see in the example how it works

https://github.com/golang-jwt/jwt/blob/6de17d3b3e986289b9b32d4febae39899bd838e2/example_test.go#L97-L117

[mfridman]

I wonder if we can capture this in a README, or publish lightweight docs capturing the various use-cases and types of errors.

[qeq66]

我也遇到了该问题，目前依然无法实现过期判断

[johakoch]

We implemented go 1.13 error style checking a while ago, you can see in the example how it works

https://github.com/golang-jwt/jwt/blob/6de17d3b3e986289b9b32d4febae39899bd838e2/example_test.go#L97-L117

If you remove some characters from the end of the token string, e.g.

	var tokenString = "eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJmb28iOiJiYXIiLCJleHAiOjE1MDAwLCJpc3MiOiJ0ZXN0In0.HE7fK0xOQwFEr4WDgRWj4teRPZ6i3GLwD5YC"

you still get the message "Timing is everything", which on one side is correct, because the token is still expired. But the more critical problem now is IMO the invalid signature.

This probably happens because claim validation is done first, followed by the signature check, which only ORs a ValidationErrorSignatureInvalid to the existing expiration error.

[oxisto]

We implemented go 1.13 error style checking a while ago, you can see in the example how it works https://github.com/golang-jwt/jwt/blob/6de17d3b3e986289b9b32d4febae39899bd838e2/example_test.go#L97-L117

If you remove some characters from the end of the token string, e.g.

	var tokenString = "eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJmb28iOiJiYXIiLCJleHAiOjE1MDAwLCJpc3MiOiJ0ZXN0In0.HE7fK0xOQwFEr4WDgRWj4teRPZ6i3GLwD5YC"

you still get the message "Timing is everything", which on one side is correct, because the token is still expired. But the more critical problem now is IMO the invalid signature.

Ah, I get your point now. We could / should adjust the example to the following then:

if token.Valid {
		fmt.Println("You look nice today")
	} else if errors.Is(err, jwt.ErrTokenMalformed) {
		fmt.Println("That's not even a token")
	} else if errors.Is(err, jwt.ErrTokenSignatureInvalid) {
		// Invalid signature
		fmt.Println("Invalid signature")
	} else if errors.Is(err, jwt.ErrTokenExpired) || errors.Is(err, jwt.ErrTokenNotValidYet) {
		// Token is either expired or not active yet
		fmt.Println("Timing is everything")
	} else {
		fmt.Println("Couldn't handle this token:", err)
	}

This is a basic problem of checking multiple wrapped errors. Maybe even the else/if construct is not the best, but it's basically just there to demonstrate the different errors. In a real application I highly suggest aborting whatever you are doing once you encounter any kind of error from Parse and/or if .Valid is false.

This probably happens because claim validation is done first, followed by the signature check, which only ORs a ValidationErrorSignatureInvalid to the existing expiration error.

Sort of; the issue is that we want to keep all the errors and not really have a hierarchy of errors, e.g. one that overrides all of them. One could however argue that if we encounter either a malformed token or an invalid signature we basically skip setting the rest of the errors.

[oxisto]

Fixed by #234