Skip to content
This repository has been archived by the owner on May 21, 2022. It is now read-only.

iat, nbf and exp which have a decimal value, are considered as strings and not float64 or json.Number (when UseJSONNumber is true) #454

Closed
softwarespot opened this issue Feb 21, 2021 · 1 comment

Comments

@softwarespot
Copy link

softwarespot commented Feb 21, 2021

I have experienced issues with tokens that are created with latest https://github.com/laravel/passport. For example "exp" had the value "1644948329.528239" which is a valid float64 and json.Number. Previous this would have been "1644948329", but developers of passport decided to add additional precision.

In the end I solved this with the code at the end of this issue, but felt it would be good to report this.

What I noticed, was that when I was trying VerifyExpiresAt - https://github.com/dgrijalva/jwt-go/blob/master/map_claims.go#L23, the type was neither float64 nor json.Number, but instead string. I also created my own instance of the parser and defined "UseJSONNumber" to be true, but then my output was json.Number for those exps which were a valid float64 from before and still string for those which had the decimal precision.

exp as valid float64 (or json.Number) - 1631112778
exp as string - 1644948329.528239 

After checking the claims, is that exp is stored as a string and not a number (implementation they are using . https://github.com/lcobucci/jwt, https://github.com/lcobucci/jwt/blob/2f533837091d0b76a89a059e7ed2b2732b2f459e/src/Encoding/MicrosecondBasedDateConversion.php#L29), which would make perfect sense. Also jwt.io doesn't recognise this either as it shows "Invalid date"

jwtParser := jwt.Parser{
     UseJSONNumber: true,
}

Before

// expire gets the expiration time for the token content
func expire(content string) (time.Time, error) {
	tkn, _ := jwt.Parse(content, func(token *jwt.Token) (interface{}, error) {
		// Ignore the secret key, which is the first return value of this function.
		// We are only interested in the expiry and not the validity of the token
		return nil, nil
	})

	claims, ok := tkn.Claims.(jwt.MapClaims)
	if !ok {
		return time.Time{}, errors.New("invalid claims type")
	}

	exp, ok := claims["exp"].(float64)
	if !ok {
		return time.Time{}, errors.Errorf(`invalid expiration type, expected float64 for "%v"`, exp)
	}
	return time.Unix(int64(exp), 0), nil
} 

After

func expire(content string) (time.Time, error) {
	tkn, _ := jwt.Parse(content, func(token *jwt.Token) (interface{}, error) {
		// Ignore the secret key, which is the first return value of this function.
		// We are only interested in the expiry and not the validity of the token
		return nil, nil
	})

	claims, ok := tkn.Claims.(jwt.MapClaims)
	if !ok {
		return time.Time{}, errors.New("invalid claims type")
	}

	var expiry int64
	switch exp := claims["exp"].(type) {
	case float64:
		expiry = int64(exp)
	case string:
		v, err := strconv.ParseFloat(exp, 64)
		if err != nil {
			return time.Time{}, errors.Wrapf(err, `invalid expiration type, expected float64 for "%s"`, exp)
		}
		expiry = int64(v)
	default:
		return time.Time{}, errors.Errorf(`invalid expiration type, expected float64 or string for "%v"`, exp)
	}
	return time.Unix(expiry, 0), nil
}
****
@softwarespot
Copy link
Author

I am closing this issue, because this lcobucci/jwt#618 pretty much states that a claims formatter should be used instead (which I did above). Ideally it should have never been a string and so I am sorry for the noise. Hopefully others can learn from this too

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant