Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

VerifyIssuedAt in mapclaims broken #47

Closed
ggilley opened this issue Jul 31, 2021 · 6 comments
Closed

VerifyIssuedAt in mapclaims broken #47

ggilley opened this issue Jul 31, 2021 · 6 comments

Comments

@ggilley
Copy link

ggilley commented Jul 31, 2021

StandardClaims has issuedAt as an int64. That means when it gets marshaled to json, it's an int64.

MapClaims VerifyIssuedAt checks for float64 and json.Number, but not int64.

Therefore, verifyIssuedAt fails.

Seems like a recent regression?

github.com/golang-jwt/jwt v3.2.2+incompatible

@oxisto
Copy link
Collaborator

oxisto commented Jul 31, 2021

Hmm interesting point, I need to check this further. It may have always been broken, i.e. that it just takes the inverted value of req instead of properly checking, if it is an int64. A possible workaround is to use the parser option that enables JSON numbers, that should then properly unmarshal as json.Number.

I was never a big fan of those MapClaims, they are a bit annoying to handle because they fields can be "anything".

@oxisto
Copy link
Collaborator

oxisto commented Jul 31, 2021

@ggilley Can you share an example? If I parse a regular JWT that contains the payload { "iat": 1000 } it will always create a float64 field. Afaik that is the way the json package works. How did you manage to create an int64 field?

@ggilley
Copy link
Author

ggilley commented Jul 31, 2021

Hmm... Good idea. It looks like I picked up an example for custom validation that "fixed" expiredAt and not issuedAt. This code has been running for a couple of years, so something in the path changed recently.

func (c CustomClaims) Valid() error {
        err := jwt.MapClaims{"exp": float64(c.ExpiresAt), "iat": c.IssuedAt, "iss": c.Issuer}.Valid()
        if err != nil {
                return err
        }
        if c.UserID == "" {
                return errors.New("missing userid")
        }
        if c.TenantID == "" {
                return errors.New("missing tenantid")
        }
        if c.PID == "" {
                return errors.New("missing pid")
        }
        return nil
}

@ggilley
Copy link
Author

ggilley commented Jul 31, 2021

Here's the whole example:

package main

import (
    "errors"
    "fmt"
    "time"

    "github.com/golang-jwt/jwt"
)

type CustomClaims struct {
	UserID      string   `json:"userid"`
	TenantID    string	`json:"tenantid"`
	PID         string         `json:"pid"`
	Roles       []string       `json:"roles"`
	Permissions []string       `json:"permissions"`
	Parameters  []string       `json:"parameters"`
	jwt.StandardClaims
}

type CustomClaimsWrapper struct {
	CustomClaims
	Partial bool `json:"-"`
}

var JWTSecret string = ""

func main() {

	claims := CustomClaims{
		"1",
		"1",
		"1",
		[]string{"User"},
		[]string{"rtdb"},
		[]string{},
		jwt.StandardClaims{
			Issuer:    "jwt.test.org",
			ExpiresAt: time.Now().Add(15 * time.Minute).Unix(),
			IssuedAt:  time.Now().Unix(),
			Audience:  "my-audience",
		},
	}
	token := jwt.NewWithClaims(jwt.SigningMethodHS256, claims)

	tokenString, err := token.SignedString([]byte(JWTSecret))
	if err != nil {
  	    fmt.Printf("Error signing token: %v\n", err)
	}


	var parsedClaims CustomClaimsWrapper

	_, error := jwt.ParseWithClaims(tokenString, &parsedClaims, func(token *jwt.Token) (interface{}, error) {
			if _, ok := token.Method.(*jwt.SigningMethodHMAC); !ok {
				return nil, fmt.Errorf("There was an error")
			}

			return []byte(JWTSecret), nil
	})

	if error != nil {
	    fmt.Printf("error: %v\n", error);
	}

}

func (c CustomClaims) Valid() error {
	err := jwt.MapClaims{"exp": float64(c.ExpiresAt), "iat": c.IssuedAt, "iss": c.Issuer}.Valid()
	if err != nil {
		return err
	}
	if c.UserID == "" {
		return errors.New("missing userid")
	}
	if c.TenantID == "" {
		return errors.New("missing tenantid")
	}
	if c.PID == "" {
		return errors.New("missing pid")
	}
	return nil
}

@ggilley
Copy link
Author

ggilley commented Jul 31, 2021

Given that it's json, it makes more sense for me to change my code. Thanks for looking!

@ggilley ggilley closed this as completed Jul 31, 2021
@oxisto
Copy link
Collaborator

oxisto commented Jul 31, 2021

Given that it's json, it makes more sense for me to change my code. Thanks for looking!

No problem. BTW, you can make your life a lot easier if you replace the line where you reconstruct your custom claims with map claims just to validate them with the following line: err := c.StandardClaims.Valid()

Since you are embedding StandardClaims, this will just forward the Valid() to it and then run your custom code on top of it.

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

No branches or pull requests

2 participants