From 715da40e51b26829f5dad71e8f18905d5c657fab Mon Sep 17 00:00:00 2001 From: Joel Rebello Date: Wed, 27 Mar 2024 10:25:55 +0100 Subject: [PATCH 1/3] jwt/NewAuthMiddleware: verify Audience and Issuer is defined in config When auth is enabled, we'd like to make sure this returns an error if its a misconfiguration --- ginjwt/jwt.go | 9 +++++++++ ginjwt/jwt_test.go | 24 ++++++++++++++++++++++++ 2 files changed, 33 insertions(+) diff --git a/ginjwt/jwt.go b/ginjwt/jwt.go index 541e4f0..382b9ed 100644 --- a/ginjwt/jwt.go +++ b/ginjwt/jwt.go @@ -8,6 +8,7 @@ import ( "time" "github.com/gin-gonic/gin" + "github.com/pkg/errors" "golang.org/x/net/context" "gopkg.in/square/go-jose.v2" "gopkg.in/square/go-jose.v2/jwt" @@ -73,6 +74,14 @@ func NewAuthMiddleware(cfg AuthConfig) (*Middleware, error) { return mw, nil } + if cfg.Audience == "" { + return nil, errors.Wrap(ErrInvalidAudience, "empty value") + } + + if cfg.Issuer == "" { + return nil, errors.Wrap(ErrInvalidIssuer, "empty value") + } + uriProvided := (cfg.JWKSURI != "") jwksProvided := len(cfg.JWKS.Keys) > 0 diff --git a/ginjwt/jwt_test.go b/ginjwt/jwt_test.go index 2e58886..e9563a5 100644 --- a/ginjwt/jwt_test.go +++ b/ginjwt/jwt_test.go @@ -735,6 +735,30 @@ func TestAuthMiddlewareConfig(t *testing.T) { assert.ErrorIs(t, err, ginjwt.ErrInvalidAuthConfig) }, }, + { + name: "MissingAudience", + input: ginjwt.AuthConfig{ + Enabled: true, + Audience: "", + Issuer: "example-iss", + RoleValidationStrategy: "all", + }, + checkFn: func(t *testing.T, mw ginauth.GenericAuthMiddleware, err error) { + assert.ErrorIs(t, err, ginjwt.ErrInvalidAudience) + }, + }, + { + name: "MissingIssuer", + input: ginjwt.AuthConfig{ + Enabled: true, + Audience: "example-aud", + Issuer: "", + RoleValidationStrategy: "all", + }, + checkFn: func(t *testing.T, mw ginauth.GenericAuthMiddleware, err error) { + assert.ErrorIs(t, err, ginjwt.ErrInvalidIssuer) + }, + }, } for _, tc := range testCases { From c05e5e15b9f5b66e2ea02e650b7de9c948069599 Mon Sep 17 00:00:00 2001 From: Joel Rebello Date: Wed, 27 Mar 2024 10:48:46 +0100 Subject: [PATCH 2/3] ginjwt/multitoken: return error if configuration is not defined --- ginjwt/multitokenmiddleware.go | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/ginjwt/multitokenmiddleware.go b/ginjwt/multitokenmiddleware.go index aa8ffe0..1a4b2ed 100644 --- a/ginjwt/multitokenmiddleware.go +++ b/ginjwt/multitokenmiddleware.go @@ -1,9 +1,16 @@ package ginjwt -import "go.hollow.sh/toolbox/ginauth" +import ( + "github.com/pkg/errors" + "go.hollow.sh/toolbox/ginauth" +) // NewMultiTokenMiddlewareFromConfigs builds a MultiTokenMiddleware object from multiple AuthConfigs. func NewMultiTokenMiddlewareFromConfigs(cfgs ...AuthConfig) (*ginauth.MultiTokenMiddleware, error) { + if len(cfgs) == 0 { + return nil, errors.Wrap(ErrInvalidAuthConfig, "configuration empty") + } + mtm := &ginauth.MultiTokenMiddleware{} for _, cfg := range cfgs { From 252ab4f5e23a969284ef6540e45ff372ea29c6dc Mon Sep 17 00:00:00 2001 From: Joel Rebello Date: Wed, 27 Mar 2024 11:26:48 +0100 Subject: [PATCH 3/3] make lint --- events/nats_config_test.go | 2 ++ ginjwt/jwt_test.go | 10 ++++++---- ginjwt/multitokenmiddleware.go | 1 + 3 files changed, 9 insertions(+), 4 deletions(-) diff --git a/events/nats_config_test.go b/events/nats_config_test.go index 4c2da05..85190a3 100644 --- a/events/nats_config_test.go +++ b/events/nats_config_test.go @@ -66,6 +66,7 @@ func TestNatsOptions_ValidatePrereqs(t *testing.T) { CredsFile: tt.fields.CredsFile, ConnectTimeout: tt.fields.ConnectTimeout, } + err := o.validatePrereqs() if tt.errorContains != "" { assert.True(t, errors.Is(err, ErrNatsConfig)) @@ -180,6 +181,7 @@ func TestNatsConsumerOptions_Validate(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { c := &NatsConsumerOptions{Name: tt.fields.Name} + err := c.validate() if tt.errorContains != "" { assert.True(t, errors.Is(err, ErrNatsConfig)) diff --git a/ginjwt/jwt_test.go b/ginjwt/jwt_test.go index e9563a5..52cc5ec 100644 --- a/ginjwt/jwt_test.go +++ b/ginjwt/jwt_test.go @@ -220,7 +220,9 @@ func TestMiddlewareValidatesTokensWithScopes(t *testing.T) { for _, tt := range testCases { t.Run(tt.testName, func(t *testing.T) { var jwksURI string + var jwks jose.JSONWebKeySet + if tt.jwksFromURI { jwksURI = ginjwt.TestHelperJWKSProvider(ginjwt.TestPrivRSAKey1ID, ginjwt.TestPrivRSAKey2ID) } else { @@ -719,7 +721,7 @@ func TestAuthMiddlewareConfig(t *testing.T) { JWKS: jwks, RoleValidationStrategy: "all", }, - checkFn: func(t *testing.T, mw ginauth.GenericAuthMiddleware, err error) { + checkFn: func(t *testing.T, _ ginauth.GenericAuthMiddleware, err error) { assert.ErrorIs(t, err, ginjwt.ErrInvalidAuthConfig) }, }, @@ -731,7 +733,7 @@ func TestAuthMiddlewareConfig(t *testing.T) { Issuer: "example-iss", RoleValidationStrategy: "all", }, - checkFn: func(t *testing.T, mw ginauth.GenericAuthMiddleware, err error) { + checkFn: func(t *testing.T, _ ginauth.GenericAuthMiddleware, err error) { assert.ErrorIs(t, err, ginjwt.ErrInvalidAuthConfig) }, }, @@ -743,7 +745,7 @@ func TestAuthMiddlewareConfig(t *testing.T) { Issuer: "example-iss", RoleValidationStrategy: "all", }, - checkFn: func(t *testing.T, mw ginauth.GenericAuthMiddleware, err error) { + checkFn: func(t *testing.T, _ ginauth.GenericAuthMiddleware, err error) { assert.ErrorIs(t, err, ginjwt.ErrInvalidAudience) }, }, @@ -755,7 +757,7 @@ func TestAuthMiddlewareConfig(t *testing.T) { Issuer: "", RoleValidationStrategy: "all", }, - checkFn: func(t *testing.T, mw ginauth.GenericAuthMiddleware, err error) { + checkFn: func(t *testing.T, _ ginauth.GenericAuthMiddleware, err error) { assert.ErrorIs(t, err, ginjwt.ErrInvalidIssuer) }, }, diff --git a/ginjwt/multitokenmiddleware.go b/ginjwt/multitokenmiddleware.go index 1a4b2ed..b42a9be 100644 --- a/ginjwt/multitokenmiddleware.go +++ b/ginjwt/multitokenmiddleware.go @@ -2,6 +2,7 @@ package ginjwt import ( "github.com/pkg/errors" + "go.hollow.sh/toolbox/ginauth" )