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

chore: update golang-jwt/jwt and other modules #1261

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
8 changes: 4 additions & 4 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -12,13 +12,13 @@ jobs:
runs-on: ubuntu-latest
strategy:
matrix: # Support latest and one minor back
go: ["1.17", "1.18", "1.19"]
go: ["1.19", "1.20"]
env:
GOFLAGS: -mod=readonly

services:
etcd:
image: gcr.io/etcd-development/etcd:v3.5.0
image: gcr.io/etcd-development/etcd:v3.5.8
ports:
- 2379
env:
Expand All @@ -27,12 +27,12 @@ jobs:
options: --health-cmd "ETCDCTL_API=3 etcdctl --endpoints http://localhost:2379 endpoint health" --health-interval 10s --health-timeout 5s --health-retries 5

consul:
image: consul:1.10
image: consul:1.15
ports:
- 8500

zk:
image: zookeeper:3.5
image: zookeeper:3.8
ports:
- 2181

Expand Down
8 changes: 4 additions & 4 deletions auth/jwt/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ will be added to the context via the `jwt.JWTClaimsContextKey`.

```go
import (
stdjwt "github.com/golang-jwt/jwt/v4"
stdjwt "github.com/golang-jwt/jwt/v5"

"github.com/go-kit/kit/auth/jwt"
"github.com/go-kit/kit/endpoint"
Expand All @@ -23,7 +23,7 @@ func main() {
{
kf := func(token *stdjwt.Token) (interface{}, error) { return []byte("SigningString"), nil }
exampleEndpoint = MakeExampleEndpoint(service)
exampleEndpoint = jwt.NewParser(kf, stdjwt.SigningMethodHS256, jwt.StandardClaimsFactory)(exampleEndpoint)
exampleEndpoint = jwt.NewParser(kf, stdjwt.SigningMethodHS256, jwt.RegisteredClaimsFactory)(exampleEndpoint)
}
}
```
Expand All @@ -34,7 +34,7 @@ the token string and add it to the context via the `jwt.JWTContextKey`.

```go
import (
stdjwt "github.com/golang-jwt/jwt/v4"
stdjwt "github.com/golang-jwt/jwt/v5"

"github.com/go-kit/kit/auth/jwt"
"github.com/go-kit/kit/endpoint"
Expand Down Expand Up @@ -65,7 +65,7 @@ Example of use in a client:

```go
import (
stdjwt "github.com/golang-jwt/jwt/v4"
stdjwt "github.com/golang-jwt/jwt/v5"

grpctransport "github.com/go-kit/kit/transport/grpc"
"github.com/go-kit/kit/auth/jwt"
Expand Down
37 changes: 15 additions & 22 deletions auth/jwt/middleware.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import (
"errors"

"github.com/go-kit/kit/endpoint"
"github.com/golang-jwt/jwt/v4"
"github.com/golang-jwt/jwt/v5"
)

type contextKey string
Expand Down Expand Up @@ -79,10 +79,10 @@ func MapClaimsFactory() jwt.Claims {
return jwt.MapClaims{}
}

// StandardClaimsFactory is a ClaimsFactory that returns
// an empty jwt.StandardClaims.
func StandardClaimsFactory() jwt.Claims {
return &jwt.StandardClaims{}
// RegisteredClaimsFactory is a ClaimsFactory that returns
// an empty jwt.RegisteredClaims.
func RegisteredClaimsFactory() jwt.Claims {
return &jwt.RegisteredClaims{}
Comment on lines -82 to +85
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In case naming consistency is not so critical, we can consider to keep StandardClaimsFactory so to avoid a breaking change in go kit.
From my understanding jwt.RegisteredClaims{} is mostly backwards compatible with jwt.StandardClaims{}.

Any feedback is appreciated

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changing public API identifiers like this should probably be avoided if at all possible.

}

// NewParser creates a new JWT parsing middleware, specifying a
Expand Down Expand Up @@ -113,23 +113,16 @@ func NewParser(keyFunc jwt.Keyfunc, method jwt.SigningMethod, newClaims ClaimsFa
return keyFunc(token)
})
if err != nil {
if e, ok := err.(*jwt.ValidationError); ok {
switch {
case e.Errors&jwt.ValidationErrorMalformed != 0:
// Token is malformed
return nil, ErrTokenMalformed
case e.Errors&jwt.ValidationErrorExpired != 0:
// Token is expired
return nil, ErrTokenExpired
case e.Errors&jwt.ValidationErrorNotValidYet != 0:
// Token is not active yet
return nil, ErrTokenNotActive
case e.Inner != nil:
// report e.Inner
return nil, e.Inner
}
// We have a ValidationError but have no specific Go kit error for it.
// Fall through to return original error.
switch {
case errors.Is(err, jwt.ErrTokenMalformed):
// Token is malformed
return nil, ErrTokenMalformed
case errors.Is(err, jwt.ErrTokenExpired):
// Token is expired
return nil, ErrTokenExpired
case errors.Is(err, jwt.ErrTokenNotValidYet):
// Token is not active yet
return nil, ErrTokenNotActive
}
return nil, err
}
Expand Down
63 changes: 32 additions & 31 deletions auth/jwt/middleware_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,40 +2,41 @@ package jwt

import (
"context"
"errors"
"sync"
"testing"
"time"

"crypto/subtle"

"github.com/go-kit/kit/endpoint"
"github.com/golang-jwt/jwt/v4"
"github.com/golang-jwt/jwt/v5"
)

type customClaims struct {
MyProperty string `json:"my_property"`
jwt.StandardClaims
jwt.RegisteredClaims
}

func (c customClaims) VerifyMyProperty(p string) bool {
return subtle.ConstantTimeCompare([]byte(c.MyProperty), []byte(p)) != 0
}

var (
kid = "kid"
key = []byte("test_signing_key")
myProperty = "some value"
method = jwt.SigningMethodHS256
invalidMethod = jwt.SigningMethodRS256
mapClaims = jwt.MapClaims{"user": "go-kit"}
standardClaims = jwt.StandardClaims{Audience: "go-kit"}
myCustomClaims = customClaims{MyProperty: myProperty, StandardClaims: standardClaims}
kid = "kid"
key = []byte("test_signing_key")
myProperty = "some value"
method = jwt.SigningMethodHS256
invalidMethod = jwt.SigningMethodRS256
mapClaims = jwt.MapClaims{"user": "go-kit"}
registeredClaims = jwt.RegisteredClaims{Audience: []string{"go-kit"}}
myCustomClaims = customClaims{MyProperty: myProperty, RegisteredClaims: registeredClaims}
// Signed tokens generated at https://jwt.io/
signedKey = "eyJhbGciOiJIUzI1NiIsImtpZCI6ImtpZCIsInR5cCI6IkpXVCJ9.eyJ1c2VyIjoiZ28ta2l0In0.14M2VmYyApdSlV_LZ88ajjwuaLeIFplB8JpyNy0A19E"
standardSignedKey = "eyJhbGciOiJIUzI1NiIsImtpZCI6ImtpZCIsInR5cCI6IkpXVCJ9.eyJhdWQiOiJnby1raXQifQ.L5ypIJjCOOv3jJ8G5SelaHvR04UJuxmcBN5QW3m_aoY"
customSignedKey = "eyJhbGciOiJIUzI1NiIsImtpZCI6ImtpZCIsInR5cCI6IkpXVCJ9.eyJteV9wcm9wZXJ0eSI6InNvbWUgdmFsdWUiLCJhdWQiOiJnby1raXQifQ.s8F-IDrV4WPJUsqr7qfDi-3GRlcKR0SRnkTeUT_U-i0"
invalidKey = "eyJhbGciOiJSUzI1NiIsInR5cCI6IkpXVCJ9.e30.vKVCKto-Wn6rgz3vBdaZaCBGfCBDTXOENSo_X2Gq7qA"
malformedKey = "malformed.jwt.token"
signedKey = "eyJhbGciOiJIUzI1NiIsImtpZCI6ImtpZCIsInR5cCI6IkpXVCJ9.eyJ1c2VyIjoiZ28ta2l0In0.14M2VmYyApdSlV_LZ88ajjwuaLeIFplB8JpyNy0A19E"
registeredSignedKey = "eyJhbGciOiJIUzI1NiIsImtpZCI6ImtpZCIsInR5cCI6IkpXVCJ9.eyJhdWQiOlsiZ28ta2l0Il19.vqB-qPpEqKyEYqNsDsM7ZrWYG7ZEhJLwBXMzR0H3ajo"
customSignedKey = "eyJhbGciOiJIUzI1NiIsImtpZCI6ImtpZCIsInR5cCI6IkpXVCJ9.eyJteV9wcm9wZXJ0eSI6InNvbWUgdmFsdWUiLCJhdWQiOlsiZ28ta2l0Il19.Yus4v91ScNgx6_zgLJVYofo2vpZziA_vds7WPWwwgbE"
invalidKey = "eyJhbGciOiJSUzI1NiIsInR5cCI6IkpXVCJ9.e30.vKVCKto-Wn6rgz3vBdaZaCBGfCBDTXOENSo_X2Gq7qA"
malformedKey = "malformed.jwt.token"
)

func signingValidator(t *testing.T, signer endpoint.Endpoint, expectedKey string) {
Expand All @@ -60,8 +61,8 @@ func TestNewSigner(t *testing.T) {
signer := NewSigner(kid, key, method, mapClaims)(e)
signingValidator(t, signer, signedKey)

signer = NewSigner(kid, key, method, standardClaims)(e)
signingValidator(t, signer, standardSignedKey)
signer = NewSigner(kid, key, method, registeredClaims)(e)
signingValidator(t, signer, registeredSignedKey)

signer = NewSigner(kid, key, method, myCustomClaims)(e)
signingValidator(t, signer, customSignedKey)
Expand Down Expand Up @@ -101,7 +102,7 @@ func TestJWTParser(t *testing.T) {
t.Error("Parser should have returned an error")
}

if err != ErrUnexpectedSigningMethod {
if !errors.Is(err, ErrUnexpectedSigningMethod) {
t.Errorf("unexpected error returned, expected: %s got: %s", ErrUnexpectedSigningMethod, err)
}

Expand Down Expand Up @@ -134,16 +135,16 @@ func TestJWTParser(t *testing.T) {
}

// Test for malformed token error response
parser = NewParser(keys, method, StandardClaimsFactory)(e)
parser = NewParser(keys, method, RegisteredClaimsFactory)(e)
ctx = context.WithValue(context.Background(), JWTContextKey, malformedKey)
ctx1, err = parser(ctx, struct{}{})
if want, have := ErrTokenMalformed, err; want != have {
t.Fatalf("Expected %+v, got %+v", want, have)
}

// Test for expired token error response
parser = NewParser(keys, method, StandardClaimsFactory)(e)
expired := jwt.NewWithClaims(method, jwt.StandardClaims{ExpiresAt: time.Now().Unix() - 100})
parser = NewParser(keys, method, RegisteredClaimsFactory)(e)
expired := jwt.NewWithClaims(method, jwt.RegisteredClaims{ExpiresAt: jwt.NewNumericDate(time.Now().Add(-time.Second * 100))})
token, err := expired.SignedString(key)
if err != nil {
t.Fatalf("Unable to Sign Token: %+v", err)
Expand All @@ -155,8 +156,8 @@ func TestJWTParser(t *testing.T) {
}

// Test for not activated token error response
parser = NewParser(keys, method, StandardClaimsFactory)(e)
notactive := jwt.NewWithClaims(method, jwt.StandardClaims{NotBefore: time.Now().Unix() + 100})
parser = NewParser(keys, method, RegisteredClaimsFactory)(e)
notactive := jwt.NewWithClaims(method, jwt.RegisteredClaims{NotBefore: jwt.NewNumericDate(time.Now().Add(time.Second * 100))})
token, err = notactive.SignedString(key)
if err != nil {
t.Fatalf("Unable to Sign Token: %+v", err)
Expand All @@ -167,19 +168,19 @@ func TestJWTParser(t *testing.T) {
t.Fatalf("Expected %+v, got %+v", want, have)
}

// test valid standard claims token
parser = NewParser(keys, method, StandardClaimsFactory)(e)
ctx = context.WithValue(context.Background(), JWTContextKey, standardSignedKey)
// test valid registered claims token
parser = NewParser(keys, method, RegisteredClaimsFactory)(e)
ctx = context.WithValue(context.Background(), JWTContextKey, registeredSignedKey)
ctx1, err = parser(ctx, struct{}{})
if err != nil {
t.Fatalf("Parser returned error: %s", err)
}
stdCl, ok := ctx1.(context.Context).Value(JWTClaimsContextKey).(*jwt.StandardClaims)
regCl, ok := ctx1.(context.Context).Value(JWTClaimsContextKey).(*jwt.RegisteredClaims)
if !ok {
t.Fatal("Claims were not passed into context correctly")
}
if !stdCl.VerifyAudience("go-kit", true) {
t.Fatalf("JWT jwt.StandardClaims.Audience did not match: expecting %s got %s", standardClaims.Audience, stdCl.Audience)
if len(regCl.Audience) != 1 || regCl.Audience[0] != registeredClaims.Audience[0] {
t.Fatalf("JWT jwt.RegisteredClaims.Audience did not match: expecting %s got %s", registeredClaims.Audience, regCl.Audience)
}

// test valid customized claims token
Expand All @@ -193,8 +194,8 @@ func TestJWTParser(t *testing.T) {
if !ok {
t.Fatal("Claims were not passed into context correctly")
}
if !custCl.VerifyAudience("go-kit", true) {
t.Fatalf("JWT customClaims.Audience did not match: expecting %s got %s", standardClaims.Audience, custCl.Audience)
if len(custCl.Audience) != 1 || custCl.Audience[0] != registeredClaims.Audience[0] {
t.Fatalf("JWT customClaims.Audience did not match: expecting %s got %s", registeredClaims.Audience, custCl.Audience)
}
if !custCl.VerifyMyProperty(myProperty) {
t.Fatalf("JWT customClaims.MyProperty did not match: expecting %s got %s", myProperty, custCl.MyProperty)
Expand Down