Incorrect documentation / breaking change lead to failed build

[truescotian]

Description

Provide a clear and concise description of the issue, including what you expected to happen.

It seems there is incorrect documentation for Auth0 to address the most recent commit 1c6db3c. When providing options to github.com/auth0/go-jwt-middleware, specifically ValidationKeyGetter, we've always used github.com/dgrijalva/jwt-go *jwt.Token, which is currently invalid and throwns the shown error:

cannot use func literal (type func(*"github.com/dgrijalva/jwt-go".Token) (interface {}, error)) as type "github.com/form3tech-oss/jwt-go".Keyfunc in field value

I thought that I could just look at the documentation again to build auth (https://auth0.com/docs/quickstart/backend/golang/01-authorization) for an updated solution, but it is still using the old github.com/dgrijalva/jwt-go package.

I'm not sure why this was deployed to master branch without updating documentation on Auth0's website. If I'm incorrect on this and missing some information, my apologies. Please let me know.

Reproduction

Detail the steps taken to reproduce this error, what was expected, and whether this issue can be reproduced consistently or if it is intermittent.

I built my package and it failed. I expected it not to fail. This is a consistent issue. If you would like more information please let me know, otherwise I have followed the Auth0 docs and have not updated my package in over a year.

Environment

Please provide the following:

• Version of this library used: Master branch
• Other relevant versions (language, server software, OS, browser): Golang, Ubuntu
• Other modules/plugins/libraries that might be involved:
  github.com/auth0/go-jwt-middleware
github.com/dgrijalva/jwt-go
https://github.com/urfave/negroni

[grounded042]

Hey @truescotian, my apologies for this. If you look at #69 you can see what lead to this breaking change. The decision was made to have this breaking change in order to fix a security vulnerability. The documentation issue is an oversight on our part and I'll make sure that gets fixed up.

A contributing factor to this issue is that up until this point this package has not been versioned.

[grounded042]

Actually, it looks like the documentation issue has already been fixed and is awaiting merge: kylekampy/docs@9cdddf6

[truescotian]

Hi @grounded042, thanks for the quick response! Glad to see that the docs have been updated. However, I just wanted you to be aware of two potential issues:
form3tech-oss/jwt-go#7
form3tech-oss/jwt-go#5

I seem to be running into similar issues. I hope I go in to enough detail for you:

1. My JWT's aud is [http://xxxxx.xxx.xxxxx.com https://xxxxx-xxxx.auth0.com/xxx]

2. To confirm, I print this out: fmt.Println(token.Claims.(jwt.MapClaims)["aud"]). Output is the same as the previous line aud

3. I kept getting invalid audience using this aud. Not sure why at this point.

4. After looking into the form3tech-oss/jwt-go package, specifically func(m MapClaims) VerifyAudience(cmp string, req bool) bool {, I noticed aud, ok := m["aud"].([]string) , ok was always false.

5. So I thought there might be some type issue, such as the one in Issue validating Audience due to type problem form3tech-oss/jwt-go#7. To check this out, before VerifyAudience(AUDIENCE_HERE, false) I just set the aud to test with token.Claims.(jwt.MapClaims)["aud"] = "test", and then called checkAud := token.Claims.(jwt.MapClaims).VerifyAudience("test", false). This worked successfully. Super weird. Thought it might be because originally I was using a []string, and this one uses a string, which shouldn't even happen since RFC states both should be supported. So I tried to then set the token's aud to a []string, and verify that audience, which also worked.

6. So I thought it was a good time to check what my original aud type is according to form3tech-oss/jwt-go within the VerifyAudience method. Here's what I used:

     aud, ok := m["aud"].([]string)
     fmt.Println(fmt.Sprintf("%T", m["aud"]))

Output: []interface {}

So my aud is not being asserted to []string it seems.

At this point, I'm not really sure where things went wrong when parsing jwt.MapClaims have you run into this issue yet? It seems at least some must be.

[grounded042]

In the code snippet

aud, ok := m["aud"].([]string)
fmt.Println(fmt.Sprintf("%T", m["aud"]))

it makes sense with what you are seeing. You would need to do

aud, ok := m["aud"].([]string)
fmt.Println(fmt.Sprintf("%T", aud))

to have it show up as []string.

EDIT: actually, this is a little strange based on: https://play.golang.org/p/botHkNRY-PG

[grounded042]

@truescotian Thinking about this some more I think this has to do with how JSON is parsed into the map claims. I don't have time to dig deeper right now, but that's where I would start looking.

[dschreij]

Same here; to make things work again, I had to explicitly convert the []interface{} to []string by adding the following segment to the ValidationKeyGetter code:

jwtmiddleware.New(jwtmiddleware.Options{
	ValidationKeyGetter: func(token *jwt.Token) (interface{}, error) {
		claims, ok := token.Claims.(jwt.MapClaims)
		if !ok {
			return token, errors.New("invalid claims type")
		}

		if audienceList, ok := claims["aud"].([]interface{}); ok{
			auds := make([]string, len(audienceList))
			for _, aud := range(audienceList) {
				audStr, ok := aud.(string)
				if !ok {
					return token, errors.New("invalid audience type")
				}
				auds = append(auds, audStr)
			}
			claims["aud"] = auds
		}
                
                // Verify 'aud' claim
		checkAud := claims.VerifyAudience(auth0Aud, false)
		if !checkAud {
			return token, errors.New("invalid audience")
		}
                // ... etc

Not a big problem perse, but sadly it makes the function way more verbose.

Is there a shorter way to cast []interface{} to []string? I understand this is not as easy as you'd expect by doing some searching on this.

[truescotian]

Is there a shorter way to cast []interface{} to []string? I understand this is not as easy as you'd expect by doing some searching on this.

Unfortunately not: https://stackoverflow.com/questions/44027826/convert-interface-to-string-in-golang

[aaronprice00]

Regarding form3tech-oss/jwt-go
jwt.Parse looks like it was planned to be phased out or kept for backwards compatibility.
By default jwt.Parse calls jwt.ParseWithClaims and passes in MapClaims{} which is of type map[string]interface{}
Take a look at: https://github.com/form3tech-oss/jwt-go/blob/master/MIGRATION_GUIDE.md
But you can instead use ParseWithClaims with custom or StandardClaims and have direct access to claims.Audience (type []string) much nicer and supported by VerifyAudience!

Also, by implementing Custom Claims you could kill 3 issues at once
##58
##53

const issuedAtLeewaySecs = 10

// MyClaims custom claim type to provide leeway support.
type MyClaims struct {
	*jwt.StandardClaims
}

// Valid validates custom claim and also standard claim
func (c *MyClaims) Valid() error {
	fmt.Println("Custom Claim validation...")
	c.StandardClaims.IssuedAt -= issuedAtLeewaySecs
	err := c.StandardClaims.Valid()
	c.StandardClaims.IssuedAt += issuedAtLeewaySecs
	return err
}

// Now parse the token with Custom Claim

parsedToken, err := jwt.ParseWithClaims(token, &MyClaims{}, m.Options.ValidationKeyGetter)

go-jwt-middleware/jwtmiddleware.go

Line 204 in 1c6db3c

parsedToken, err := jwt.Parse(token, m.Options.ValidationKeyGetter)

[aaronprice00]

Here's my full solution hopefully it helps someone.

main.go:

import "github.com/form3tech-oss/jwt-go/"
...
// V4 of jwt-go should include leeway support, until then we need to customize the claims to provide a small window of time for differences in clocks. Error: "Token used before issued"
// Custom Claims also fixes the claim["aud"] issue where you get back multiple values of type []interface{}
const leeway = 10

// MyClaims custom claim type to provide leeway support.
type MyClaims struct {
	*jwt.StandardClaims
}

// Valid validates custom claim and also standard claim
func (c *MyClaims) Valid() error {
	c.StandardClaims.IssuedAt -= leeway  // subtract our leeway value
	err := c.StandardClaims.Valid()              // Check Standard claims validation
	c.StandardClaims.IssuedAt += leeway  // reset to original
	return err
}

func main() {
	jwtMiddleware := jwtmiddleware.New(jwtmiddleware.Options{
		ValidationKeyGetter: func(token *jwt.Token) (interface{}, error) {
			claims, ok := token.Claims.(*MyClaims) // Custom-Claims allows leeway support
			// claims, ok := token.Claims.(*jwt.StandardClaims) // Using StandardClaims also works
			if !ok {
				return token, errors.New("invalid claims type")
			}

			// verify 'audience' claim
			auth0Aud := "https://api.youraudience.com"
			checkAud := claims.VerifyAudience(auth0Aud, false)
			if !checkAud {
				return token, errors.New("invalid audience")
			}

			// Verify 'issuer' claim
			iss := "https://wia.us.auth0.com/"
			checkIss := claims.VerifyIssuer(iss, false)
			if !checkIss {
				return token, errors.New("invalid issuer")
			}

			cert, err := getPemCert(token)
			if err != nil {
				panic(err.Error())
			}

			result, _ := jwt.ParseRSAPublicKeyFromPEM([]byte(cert))
			return result, nil
		},
		SigningMethod: jwt.SigningMethodRS256,
		Debug:         true,
		Claims:        &MyClaims{},   // Pass in Custom Claims
	})
.....

Next, need to modify go-jwt-middleware so vendor it
$ go mod vendor

First add an option for claims
vendor/github.com/auth0/go-jwt-middleware/jwtmiddleware.go:

type Options struct {
	....
	// Optional: (custom Claims)
	// if not set we'll use StandardClaims
	// Default: nil,
	Claims jwt.Claims  // Add Custom claims support
}

Also need to modify the New() method to set default claims

func New(options ...Options) *JWTMiddleware {
        ...
	// use StandardClaims if custom Claims option not set
	if opts.Claims == nil {
		opts.Claims = &jwt.StandardClaims{}
	}
        ...
}

Finally lets use ParseWithClaims() instead of Parse() and pass in our Claims Option

func (m *JWTMiddleware) CheckJWT(w http.ResponseWriter, r *http.Request) error {
        ...
        // Now parse the token
	// Migrate to ParseWithClaims instead of old Parse method
	// ParseWithClaims allows us to use custom Claims, or StandardClaims
	parsedToken, err := jwt.ParseWithClaims(token, m.Options.Claims, m.Options.ValidationKeyGetter)
        ...

This worked for me, I'd love to get some feedback on it but it allows leeway support, custom claims support, and solves the issue with []interface{} audiences. All of this could be better solved on the go-jwt side but seems to be a lack of maintenance

[grounded042]

Hey @aaronprice00, sorry for the late reply here. Great work on putting all of that together. Looking through this it looks like a great candidate for v2. In v2 we are working on separating the logic of JWT validation. Right now the middleware provides some validation, but most people end up adding their own setup. In v2 validation should be more easily handled by a token validator which can be switched out for different implementations.

The work you've here is a major change to the contract people rely on for this package and if we were to include it we would also look at releasing a new major version of the package. In light of that I'd like to look at moving this work to v2. I also think that because we're looking at switching away from the jwt-go package in v2 we might solve some of these problems off the bat.