dgrijalva / jwt-go

ARCHIVE - Golang implementation of JSON Web Tokens (JWT). This project is now maintained at:
https://github.com/golang-jwt/jwt
MIT License
10.78k stars 997 forks source link

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 closed 3 years ago

softwarespot commented 3 years ago

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 commented 3 years ago

I am closing this issue, because this https://github.com/lcobucci/jwt/pull/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