From 5fd4f52e3506f8d0d04258e49f22b454d08a28ce Mon Sep 17 00:00:00 2001 From: andres-portainer <91705312+andres-portainer@users.noreply.github.com> Date: Tue, 17 Sep 2024 18:23:33 -0300 Subject: [PATCH] fix(jwt): fix handling of non-expiring JWT tokens BE-11242 (#12220) --- api/http/security/bouncer.go | 4 +++- api/http/security/bouncer_test.go | 16 ++++++++++++++-- api/jwt/jwt.go | 4 ++++ api/jwt/jwt_kubeconfig_test.go | 25 +++++++++++++++++++------ 4 files changed, 40 insertions(+), 9 deletions(-) diff --git a/api/http/security/bouncer.go b/api/http/security/bouncer.go index 2b740de4c..11b7ee745 100644 --- a/api/http/security/bouncer.go +++ b/api/http/security/bouncer.go @@ -381,7 +381,9 @@ func (bouncer *RequestBouncer) RevokeJWT(token string) { func (bouncer *RequestBouncer) cleanUpExpiredJWTPass() { bouncer.revokedJWT.Range(func(key, value any) bool { - if time.Now().After(value.(time.Time)) { + if t := value.(time.Time); t.IsZero() { + return true + } else if time.Now().After(t) { bouncer.revokedJWT.Delete(key) } diff --git a/api/http/security/bouncer_test.go b/api/http/security/bouncer_test.go index 7b0e5e0a4..9553b90c5 100644 --- a/api/http/security/bouncer_test.go +++ b/api/http/security/bouncer_test.go @@ -473,6 +473,17 @@ func TestJWTRevocation(t *testing.T) { token, _, err := jwtService.GenerateToken(&portainer.TokenData{ID: 1}) require.NoError(t, err) + settings, err := store.Settings().Settings() + require.NoError(t, err) + + settings.KubeconfigExpiry = "0" + + err = store.Settings().UpdateSettings(settings) + require.NoError(t, err) + + kubeToken, err := jwtService.GenerateTokenForKubeconfig(&portainer.TokenData{ID: 1}) + require.NoError(t, err) + apiKeyService := apikey.NewAPIKeyService(nil, nil) bouncer := NewRequestBouncer(store, jwtService, apiKeyService) @@ -491,6 +502,7 @@ func TestJWTRevocation(t *testing.T) { require.NoError(t, err) bouncer.RevokeJWT(token) + bouncer.RevokeJWT(kubeToken) revokeLen := func() (l int) { bouncer.revokedJWT.Range(func(key, value any) bool { @@ -501,7 +513,7 @@ func TestJWTRevocation(t *testing.T) { return l } - require.Equal(t, 1, revokeLen()) + require.Equal(t, 2, revokeLen()) _, err = bouncer.JWTAuthLookup(r) require.Error(t, err) @@ -513,5 +525,5 @@ func TestJWTRevocation(t *testing.T) { bouncer.cleanUpExpiredJWTPass() - require.Equal(t, 0, revokeLen()) + require.Equal(t, 1, revokeLen()) } diff --git a/api/jwt/jwt.go b/api/jwt/jwt.go index 337a51901..520a95e48 100644 --- a/api/jwt/jwt.go +++ b/api/jwt/jwt.go @@ -137,6 +137,10 @@ func (service *Service) ParseAndVerifyToken(token string) (*portainer.TokenData, return nil, "", time.Time{}, errInvalidJWTToken } + if cl.ExpiresAt == nil { + cl.ExpiresAt = &jwt.NumericDate{} + } + return &portainer.TokenData{ ID: portainer.UserID(cl.UserID), Username: cl.Username, diff --git a/api/jwt/jwt_kubeconfig_test.go b/api/jwt/jwt_kubeconfig_test.go index 07727dead..8f79aaa60 100644 --- a/api/jwt/jwt_kubeconfig_test.go +++ b/api/jwt/jwt_kubeconfig_test.go @@ -3,14 +3,20 @@ package jwt import ( "testing" - "github.com/golang-jwt/jwt/v4" portainer "github.com/portainer/portainer/api" "github.com/portainer/portainer/api/dataservices" - i "github.com/portainer/portainer/api/internal/testhelpers" + "github.com/portainer/portainer/api/datastore" + + "github.com/golang-jwt/jwt/v4" "github.com/stretchr/testify/assert" ) func TestService_GenerateTokenForKubeconfig(t *testing.T) { + _, store := datastore.MustNewTestStore(t, true, false) + + err := store.User().Create(&portainer.User{ID: 1}) + assert.NoError(t, err) + type fields struct { userSessionTimeout string dataStore dataservices.DataStore @@ -20,13 +26,17 @@ func TestService_GenerateTokenForKubeconfig(t *testing.T) { data *portainer.TokenData } - mySettings := &portainer.Settings{ - KubeconfigExpiry: "0", - } + settings, err := store.Settings().Settings() + assert.NoError(t, err) + + settings.KubeconfigExpiry = "0" + + err = store.Settings().UpdateSettings(settings) + assert.NoError(t, err) myFields := fields{ userSessionTimeout: "24h", - dataStore: i.NewDatastore(i.WithSettingsService(mySettings)), + dataStore: store, } myTokenData := &portainer.TokenData{ @@ -66,6 +76,9 @@ func TestService_GenerateTokenForKubeconfig(t *testing.T) { return } + _, _, _, err = service.ParseAndVerifyToken(got) + assert.NoError(t, err) + parsedToken, err := jwt.ParseWithClaims(got, &claims{}, func(token *jwt.Token) (any, error) { return service.secrets[kubeConfigScope], nil })