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

Incorrect documentation / breaking change lead to failed build #72

Closed
truescotian opened this issue Jan 8, 2021 · 26 comments
Closed

Incorrect documentation / breaking change lead to failed build #72

truescotian opened this issue Jan 8, 2021 · 26 comments
Assignees

Comments

@truescotian
Copy link

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
Copy link
Contributor

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
Copy link
Contributor

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

@truescotian
Copy link
Author

truescotian commented Jan 9, 2021

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
Copy link
Contributor

grounded042 commented Jan 11, 2021

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
Copy link
Contributor

@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.

@grounded042 grounded042 self-assigned this Jan 11, 2021
@dschreij
Copy link

dschreij commented Jan 13, 2021

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
Copy link
Author

truescotian commented Jan 13, 2021

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
Copy link

aaronprice00 commented Feb 16, 2021

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)

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

@aaronprice00
Copy link

aaronprice00 commented Mar 1, 2021

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
Copy link
Contributor

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.

@urbantrout
Copy link

@aaronprice00 Thanks for your solution. Unfortunately I get the following error:

interface conversion: jwt.Claims is *main.MyClaims, not jwt.MapClaims

go 1.16

@grounded042
Copy link
Contributor

@urbantrout can you show use the code for the error you are hitting?

@DeamonMV
Copy link

DeamonMV commented Jun 9, 2021

I think problem solved. I tried those versions of modules

github.com/auth0/go-jwt-middleware v1.0.0
github.com/form3tech-oss/jwt-go v3.2.3+incompatible

And with them was no need to convert interface into string.

@urbantrout
Copy link

I was using

github.com/auth0/go-jwt-middleware v1.0.0
github.com/form3tech-oss/jwt-go v3.2.2+incompatible

Upgrading to

github.com/auth0/go-jwt-middleware v1.0.0
github.com/form3tech-oss/jwt-go v3.2.3+incompatible

did not solve the problem.

I think it has something to do with this line of code:

claims, ok := token.Claims.(jwt.MapClaims)

which according to #72 (comment) I have to change to

claims, ok := token.Claims.(*MyClaims) // Custom-Claims allows leeway support

@hspens
Copy link

hspens commented Jun 22, 2021

I'm experiencing the same issue and have a work-around for it. I've implemented a custom json parser for my CustomClaims which supports both string & []string. However, in my opinion, the underlying jwt.StandardClaims should implement aud as type []string instead of string.

type CustomClaims struct {
	jwt.StandardClaims
	Permissions []string `json:"permissions"`
	Audience    []string `json:"aud,omitempty"`
}

func (t *CustomClaims) UnmarshalJSON(data []byte) error {

	type Alias CustomClaims
	aux := &struct {
		Audience interface{} `json:"aud"`
		*Alias
	}{
		Alias: (*Alias)(t),
	}

	if err := json.Unmarshal(data, &aux); err != nil {
		return err
	}

	aud, ok := aux.Audience.(string)

	if ok {
		t.Audience = []string{aud}
		return nil
	}

	audList, ok := aux.Audience.([]interface{})

	if !ok {
		errors.Errorf("failed to unmarshal audience of type: %T, into CustomClaims", aux.Audience)
		return nil
	}

	t.Audience = make([]string, len(audList))

	for i, v := range audList {
		aud, ok = v.(string)
		if !ok {
			return errors.Errorf("expected audience of type string in list, found type: %T", v)
		}
		t.Audience[i] = aud
	}

	return nil
}

I'm using the following packages:

github.com/auth0/go-jwt-middleware v0.0.0-20201030150249-d783b5c46b39
github.com/dgrijalva/jwt-go v3.2.0+incompatible

@DeamonMV
Copy link

@hspens
try to use those versions

github.com/auth0/go-jwt-middleware v1.0.0
github.com/form3tech-oss/jwt-go v3.2.3+incompatible

@sergiught
Copy link
Contributor

We just released the v2.0.0-beta 🥳 !

You can start testing it by running go get github.com/auth0/go-jwt-middleware/v2@v2.0.0-beta.

In case of issues fetching the v2 you might want to try go clean --modcache first before doing go get.

I'm closing this issue as now this is part of v2, but feel free to reopen if needed.

@ashish-nikalje
Copy link

go get github.com/auth0/go-jwt-middleware/v2@v2.0.0-beta

getting error
go: github.com/auth0/go-jwt-middleware/v2/v2@v2.0.0-beta: reading github.com/auth0/go-jwt-middleware/v2/v2/go.mod at revision v2/v2.0.0-beta: unknown revision v2/v2.0.0-beta

@sergiught
Copy link
Contributor

Hey @ashish-scalent ! It seems you're trying to do /v2/v2 twice in your go get:) It should be just go get github.com/auth0/go-jwt-middleware/v2@v2.0.0-beta.

@ashish-nikalje
Copy link

previously we are fetching tokens from requests after jwt middle handler validate the token set in the req context
but now CheckJwt middleware set that as

// No err means we have a valid token, so set
// it into the context and continue onto next.
r = r.Clone(context.WithValue(r.Context(), ContextKey{}, validToken))

so in this case if we retrieve the ctxToken from req context something like
ctxToken := r.Context().Value(jwtmiddleware.ContextKey{})

so token is interface having type string actually so when I tried to retrieve (*jwt.Token) to map with claims it will cause an nil pointer error

	token := ctxToken.(*jwt.Token)
	claims := token.Claims.(jwt.MapClaims)

is anyone have any idea about this?

@sergiught
Copy link
Contributor

Hey @ashish-scalent , you need to do something like this as described in the README.md:

claims := r.Context().Value(jwtmiddleware.ContextKey{}).(*validator.ValidatedClaims)

as now the value will be of type validator.ValidatedClaims.

@ashish-nikalje
Copy link

r.Context().Value(jwtmiddleware.ContextKey{}).(*validator.ValidatedClaims)

PANIC: interface conversion: interface {} is string, not *validator.ValidatedClaims

getting panic in that way I forgot to mention that previously

@sergiught
Copy link
Contributor

@ashish-scalent Could you please open a separate issue describing in full detail the issue following the template we provide please?:) It would be great to provide a reproducible of your setup code.

@ashish-nikalje
Copy link

@ashish-scalent Could you please open a separate issue describing in full detail the issue following the template we provide please?:) It would be great to provide a reproducible of your setup code.

Hey Sorry my bad you were right I actually forgot to pass *validator.ValidateToken function in middleware

no issue from my side, thanks for the instant support

@sergiught
Copy link
Contributor

@ashish-scalent Happy to help!:) Glad you fixed it!

@noamApps
Copy link

noamApps commented Feb 1, 2022

Hi everyone, we also encountered this problem, I'm putting our solution here in case it will useful for someone.

This solution doesn't require using the auth0 v2 sdk which was in beta until recently (if you are implementing from scratch I suggest using it), and does not require vendoring the jwt package to your repository and modify it, and does not require github.com/form3tech-oss/jwt-go which is no longer maintained.

The problem as we understand it relies in golang-jwt v3.X which only have a StandardClaims struct that allows a single Audience, so when unmarshaling a jwt with multiple audience, it fails.

type StandardClaims struct {
	Audience  string `json:"aud,omitempty"`
	ExpiresAt int64  `json:"exp,omitempty"`
	Id        string `json:"jti,omitempty"`
	IssuedAt  int64  `json:"iat,omitempty"`
	Issuer    string `json:"iss,omitempty"`
	NotBefore int64  `json:"nbf,omitempty"`
	Subject   string `json:"sub,omitempty"`
}

However, this was solved in golang-jwt v4, which features a RegisteredClaims that allows multiple audience (ClaimStrings is []string)

type RegisteredClaims struct {
	Issuer string `json:"iss,omitempty"`
	Subject string `json:"sub,omitempty"`
	Audience ClaimStrings `json:"aud,omitempty"`
	ExpiresAt *NumericDate `json:"exp,omitempty"`
	NotBefore *NumericDate `json:"nbf,omitempty"`
	IssuedAt *NumericDate `json:"iat,omitempty"`
	ID string `json:"jti,omitempty"`
}

So we decided to import both versions (v3.2.1 and v4) of golang-jwt (unfortunately using only v4 breaks the currently required jwt-go version).
Then, we used the v4 version for the CustomClaim struct, and the old version for the rest of the validation procedures as stated in the docs

import (
	"github.com/golang-jwt/jwt"
	jwt_modern_claims "github.com/golang-jwt/jwt/v4"
)

type CustomClaims struct {
	Scope            string `json:"scope"`
	jwt_modern_claims.RegisteredClaims
}

...

After that, everything worked as expected, no unmarshaling errors and we were able to implement functions such as

func (c CustomClaims) verifyAudience() bool {
	for _, aud := range c.Audience {
		if aud == c.expectedAudience {
			return true
		}
	}
	return false
}

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

10 participants