From 9753c848073e630a995c1c9f15cd73875bdc31ca Mon Sep 17 00:00:00 2001 From: Gabriel Blandin Date: Fri, 2 Feb 2024 15:20:11 +0000 Subject: [PATCH 1/4] Update to golang-jwt V5 --- auth_jwt.go | 2 +- auth_jwt_test.go | 2 +- go.mod | 2 +- go.sum | 4 ++-- 4 files changed, 5 insertions(+), 5 deletions(-) diff --git a/auth_jwt.go b/auth_jwt.go index 1804494..fadbc97 100644 --- a/auth_jwt.go +++ b/auth_jwt.go @@ -10,7 +10,7 @@ import ( "time" "github.com/gin-gonic/gin" - "github.com/golang-jwt/jwt/v4" + "github.com/golang-jwt/jwt/v5" ) // MapClaims type that uses the map[string]interface{} for JSON decoding diff --git a/auth_jwt_test.go b/auth_jwt_test.go index b239a3a..50da185 100644 --- a/auth_jwt_test.go +++ b/auth_jwt_test.go @@ -13,7 +13,7 @@ import ( "github.com/appleboy/gofight/v2" "github.com/gin-gonic/gin" - "github.com/golang-jwt/jwt/v4" + "github.com/golang-jwt/jwt/v5" "github.com/stretchr/testify/assert" "github.com/tidwall/gjson" ) diff --git a/go.mod b/go.mod index 3e65456..4c7c9ca 100644 --- a/go.mod +++ b/go.mod @@ -5,7 +5,7 @@ go 1.18 require ( github.com/appleboy/gofight/v2 v2.1.2 github.com/gin-gonic/gin v1.9.1 - github.com/golang-jwt/jwt/v4 v4.5.0 + github.com/golang-jwt/jwt/v5 v5.2.0 github.com/stretchr/testify v1.8.4 github.com/tidwall/gjson v1.17.0 ) diff --git a/go.sum b/go.sum index 9c53ac0..e1004d8 100644 --- a/go.sum +++ b/go.sum @@ -29,8 +29,8 @@ github.com/go-playground/validator/v10 v10.15.4 h1:zMXza4EpOdooxPel5xDqXEdXG5r+W github.com/go-playground/validator/v10 v10.15.4/go.mod h1:9iXMNT7sEkjXb0I+enO7QXmzG6QCsPWY4zveKFVRSyU= github.com/goccy/go-json v0.10.2 h1:CrxCmQqYDkv1z7lO7Wbh2HN93uovUHgrECaO5ZrCXAU= github.com/goccy/go-json v0.10.2/go.mod h1:6MelG93GURQebXPDq3khkgXZkazVtN9CRI+MGFi0w8I= -github.com/golang-jwt/jwt/v4 v4.5.0 h1:7cYmW1XlMY7h7ii7UhUyChSgS5wUJEnm9uZVTGqOWzg= -github.com/golang-jwt/jwt/v4 v4.5.0/go.mod h1:m21LjoU+eqJr34lmDMbreY2eSTRJ1cv77w39/MY0Ch0= +github.com/golang-jwt/jwt/v5 v5.2.0 h1:d/ix8ftRUorsN+5eMIlF4T6J8CAt9rch3My2winC1Jw= +github.com/golang-jwt/jwt/v5 v5.2.0/go.mod h1:pqrtFR0X4osieyHYxtmOUWsAWrfe1Q5UVIyoH402zdk= github.com/golang/protobuf v1.5.0/go.mod h1:FsONVRAS9T7sI+LIUmWTfcYkHO4aIWwzhcaSAoJOfIk= github.com/google/go-cmp v0.5.5 h1:Khx7svrCpmxxtHBq5j2mp/xVjsi8hQMfNLvJFAlrGgU= github.com/google/go-cmp v0.5.5/go.mod h1:v8dTdLbMG2kIc/vJvl+f65V22dbkXbowE6jgT/gNBxE= From c0ecf09671ec01780ffba422905cdf2cf41ac0ee Mon Sep 17 00:00:00 2001 From: Gabriel Blandin Date: Fri, 2 Feb 2024 17:17:25 +0000 Subject: [PATCH 2/4] Fix Removed Errors The error types that were being used were removed in v5 --- auth_jwt.go | 17 +++++++---------- 1 file changed, 7 insertions(+), 10 deletions(-) diff --git a/auth_jwt.go b/auth_jwt.go index fadbc97..2cb966d 100644 --- a/auth_jwt.go +++ b/auth_jwt.go @@ -635,16 +635,13 @@ func (mw *GinJWTMiddleware) RefreshToken(c *gin.Context) (string, time.Time, err // CheckIfTokenExpire check if token expire func (mw *GinJWTMiddleware) CheckIfTokenExpire(c *gin.Context) (jwt.MapClaims, error) { token, err := mw.ParseToken(c) - if err != nil { - // If we receive an error, and the error is anything other than a single - // ValidationErrorExpired, we want to return the error. - // If the error is just ValidationErrorExpired, we want to continue, as we can still - // refresh the token if it's within the MaxRefresh time. - // (see https://github.com/appleboy/gin-jwt/issues/176) - validationErr, ok := err.(*jwt.ValidationError) - if !ok || validationErr.Errors != jwt.ValidationErrorExpired { - return nil, err - } + // If we receive an error, and the error is anything other than a single + // ErrTokenExpired, we want to return the error. + // If the error is just ErrTokenExpired, we want to continue, as we can still + // refresh the token if it's within the MaxRefresh time. + // (see https://github.com/appleboy/gin-jwt/issues/176) + if err != nil && !errors.Is(err, jwt.ErrTokenExpired) { + return nil, err } claims := token.Claims.(jwt.MapClaims) From 110df2f496888fe3b4bf0d12499e33d474a076cf Mon Sep 17 00:00:00 2001 From: Gabriel Blandin Date: Fri, 2 Feb 2024 20:28:21 +0000 Subject: [PATCH 3/4] Fix Tests Added default for parser option to propagate the time func Also adjusted the get claims error check around expiry Updated a related test due to changes --- auth_jwt.go | 41 ++++++++++++++++++++--------------------- auth_jwt_test.go | 6 +++--- 2 files changed, 23 insertions(+), 24 deletions(-) diff --git a/auth_jwt.go b/auth_jwt.go index 2cb966d..eb35431 100644 --- a/auth_jwt.go +++ b/auth_jwt.go @@ -2,7 +2,6 @@ package jwt import ( "crypto/rsa" - "encoding/json" "errors" "net/http" "os" @@ -153,7 +152,8 @@ type GinJWTMiddleware struct { // CookieSameSite allow use http.SameSite cookie param CookieSameSite http.SameSite - // ParseOptions allow to modify jwt's parser methods + // ParseOptions allow to modify jwt's parser methods. + // WithTimeFunc is always added to ensure the TimeFunc is propagated to the validator ParseOptions []jwt.ParserOption } @@ -406,6 +406,12 @@ func (mw *GinJWTMiddleware) MiddlewareInit() error { if mw.Key == nil { return ErrMissingSecretKey } + + if len(mw.ParseOptions) == 0 { + mw.ParseOptions = []jwt.ParserOption{} + } + mw.ParseOptions = append(mw.ParseOptions, jwt.WithTimeFunc(mw.TimeFunc)) + return nil } @@ -419,31 +425,24 @@ func (mw *GinJWTMiddleware) MiddlewareFunc() gin.HandlerFunc { func (mw *GinJWTMiddleware) middlewareImpl(c *gin.Context) { claims, err := mw.GetClaimsFromJWT(c) if err != nil { - mw.unauthorized(c, http.StatusUnauthorized, mw.HTTPStatusMessageFunc(err, c)) - return - } - - switch v := claims["exp"].(type) { - case nil: - mw.unauthorized(c, http.StatusBadRequest, mw.HTTPStatusMessageFunc(ErrMissingExpField, c)) - return - case float64: - if int64(v) < mw.TimeFunc().Unix() { + if errors.Is(err, jwt.ErrTokenExpired) { mw.unauthorized(c, http.StatusUnauthorized, mw.HTTPStatusMessageFunc(ErrExpiredToken, c)) return - } - case json.Number: - n, err := v.Int64() - if err != nil { + } else if errors.Is(err, jwt.ErrInvalidType) && strings.Contains(err.Error(), "exp is invalid") { mw.unauthorized(c, http.StatusBadRequest, mw.HTTPStatusMessageFunc(ErrWrongFormatOfExp, c)) return - } - if n < mw.TimeFunc().Unix() { - mw.unauthorized(c, http.StatusUnauthorized, mw.HTTPStatusMessageFunc(ErrExpiredToken, c)) + } else if errors.Is(err, jwt.ErrTokenRequiredClaimMissing) && strings.Contains(err.Error(), "exp claim is required") { + mw.unauthorized(c, http.StatusBadRequest, mw.HTTPStatusMessageFunc(ErrMissingExpField, c)) + return + } else { + mw.unauthorized(c, http.StatusUnauthorized, mw.HTTPStatusMessageFunc(err, c)) return } - default: - mw.unauthorized(c, http.StatusBadRequest, mw.HTTPStatusMessageFunc(ErrWrongFormatOfExp, c)) + } + + // For backwards compatibility since technically exp is not required in the spec but has been in gin-jwt + if claims["exp"] == nil { + mw.unauthorized(c, http.StatusBadRequest, mw.HTTPStatusMessageFunc(ErrMissingExpField, c)) return } diff --git a/auth_jwt_test.go b/auth_jwt_test.go index 50da185..ee1b1ab 100644 --- a/auth_jwt_test.go +++ b/auth_jwt_test.go @@ -1227,7 +1227,7 @@ func TestExpiredField(t *testing.T) { }) // wrong format - claims["exp"] = "wrongFormatForExpiryIgnoredByJwtLibrary" + claims["exp"] = "wrongFormatForExpiry" tokenString, _ = token.SignedString(key) r.GET("/auth/hello"). @@ -1237,8 +1237,8 @@ func TestExpiredField(t *testing.T) { Run(handler, func(r gofight.HTTPResponse, rq gofight.HTTPRequest) { message := gjson.Get(r.Body.String(), "message") - assert.Equal(t, ErrExpiredToken.Error(), strings.ToLower(message.String())) - assert.Equal(t, http.StatusUnauthorized, r.Code) + assert.Equal(t, ErrWrongFormatOfExp.Error(), strings.ToLower(message.String())) + assert.Equal(t, http.StatusBadRequest, r.Code) }) } From 7837aef8fa948d40f34dfbe754c44465ef9610e4 Mon Sep 17 00:00:00 2001 From: Gabriel Blandin Date: Mon, 5 Feb 2024 14:24:09 +0000 Subject: [PATCH 4/4] Add Test Case Added test case for using the expiration required parser option --- auth_jwt_test.go | 47 +++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 47 insertions(+) diff --git a/auth_jwt_test.go b/auth_jwt_test.go index ee1b1ab..d7a8134 100644 --- a/auth_jwt_test.go +++ b/auth_jwt_test.go @@ -1242,6 +1242,53 @@ func TestExpiredField(t *testing.T) { }) } +func TestExpiredFieldRequiredParserOption(t *testing.T) { + // the middleware to test + authMiddleware, _ := New(&GinJWTMiddleware{ + Realm: "test zone", + Key: key, + Timeout: time.Hour, + Authenticator: defaultAuthenticator, + ParseOptions: []jwt.ParserOption{jwt.WithExpirationRequired()}, + }) + + handler := ginHandler(authMiddleware) + + r := gofight.New() + + token := jwt.New(jwt.GetSigningMethod("HS256")) + claims := token.Claims.(jwt.MapClaims) + claims["identity"] = "admin" + claims["orig_iat"] = 0 + tokenString, _ := token.SignedString(key) + + r.GET("/auth/hello"). + SetHeader(gofight.H{ + "Authorization": "Bearer " + tokenString, + }). + Run(handler, func(r gofight.HTTPResponse, rq gofight.HTTPRequest) { + message := gjson.Get(r.Body.String(), "message") + + assert.Equal(t, ErrMissingExpField.Error(), message.String()) + assert.Equal(t, http.StatusBadRequest, r.Code) + }) + + // wrong format + claims["exp"] = "wrongFormatForExpiry" + tokenString, _ = token.SignedString(key) + + r.GET("/auth/hello"). + SetHeader(gofight.H{ + "Authorization": "Bearer " + tokenString, + }). + Run(handler, func(r gofight.HTTPResponse, rq gofight.HTTPRequest) { + message := gjson.Get(r.Body.String(), "message") + + assert.Equal(t, ErrWrongFormatOfExp.Error(), strings.ToLower(message.String())) + assert.Equal(t, http.StatusBadRequest, r.Code) + }) +} + func TestCheckTokenString(t *testing.T) { // the middleware to test authMiddleware, _ := New(&GinJWTMiddleware{