fix(bouncer): add support for JWT revocation BE-11179 (#12164)

api/http/handler/auth/authenticate.go

@@ -154,7 +154,6 @@ func (handler *Handler) persistAndWriteToken(w http.ResponseWriter, tokenData *p
 	security.AddAuthCookie(w, token, expirationTime)
 
 	return response.JSON(w, &authenticateResponse{JWT: token})
-
 }
 
 func (handler *Handler) syncUserTeamsWithLDAPGroups(user *portainer.User, settings *portainer.LDAPSettings) error {
@@ -179,20 +178,18 @@ func (handler *Handler) syncUserTeamsWithLDAPGroups(user *portainer.User, settin
 	}
 
 	for _, team := range teams {
-		if teamExists(team.Name, userGroups) {
+		if !teamExists(team.Name, userGroups) || teamMembershipExists(team.ID, userMemberships) {
-			if teamMembershipExists(team.ID, userMemberships) {
+			continue
-				continue
+		}
-			}
 
-			membership := &portainer.TeamMembership{
+		membership := &portainer.TeamMembership{
-				UserID: user.ID,
+			UserID: user.ID,
-				TeamID: team.ID,
+			TeamID: team.ID,
-				Role:   portainer.TeamMember,
+			Role:   portainer.TeamMember,
-			}
+		}
 
-			if err := handler.DataStore.TeamMembership().Create(membership); err != nil {
+		if err := handler.DataStore.TeamMembership().Create(membership); err != nil {
-				return err
+			return err
-			}
 		}
 	}
 

api/http/handler/auth/logout.go

@@ -28,5 +28,7 @@ func (handler *Handler) logout(w http.ResponseWriter, r *http.Request) *httperro
 
 	security.RemoveAuthCookie(w)
 
+	handler.bouncer.RevokeJWT(tokenData.Token)
+
 	return response.Empty(w)
 }

api/http/security/bouncer.go

@@ -4,6 +4,7 @@ import (
 	"fmt"
 	"net/http"
 	"strings"
+	"sync"
 	"time"
 
 	portainer "github.com/portainer/portainer/api"
@@ -16,6 +17,9 @@ import (
 	"github.com/rs/zerolog/log"
 )
 
+const apiKeyHeader = "X-API-KEY"
+const jwtTokenHeader = "Authorization"
+
 type (
 	BouncerService interface {
 		PublicAccess(http.Handler) http.Handler
@@ -27,9 +31,10 @@ type (
 
 		AuthorizedEndpointOperation(*http.Request, *portainer.Endpoint) error
 		AuthorizedEdgeEndpointOperation(*http.Request, *portainer.Endpoint) error
-		TrustedEdgeEnvironmentAccess(dataservices.DataStoreTx, *portainer.Endpoint) error
 		CookieAuthLookup(*http.Request) (*portainer.TokenData, error)
 		JWTAuthLookup(*http.Request) (*portainer.TokenData, error)
+		TrustedEdgeEnvironmentAccess(dataservices.DataStoreTx, *portainer.Endpoint) error
+		RevokeJWT(string)
 	}
 
 	// RequestBouncer represents an entity that manages API request accesses
@@ -37,6 +42,7 @@ type (
 		dataStore     dataservices.DataStore
 		jwtService    portainer.JWTService
 		apiKeyService apikey.APIKeyService
+		revokedJWT    sync.Map
 	}
 
 	// RestrictedRequestContext is a data structure containing information
@@ -52,16 +58,22 @@ type (
 	tokenLookup func(*http.Request) (*portainer.TokenData, error)
 )
 
-const apiKeyHeader = "X-API-KEY"
+var (
-const jwtTokenHeader = "Authorization"
+	ErrInvalidKey = errors.New("Invalid API key")
+	ErrRevokedJWT = errors.New("the JWT has been revoked")
+)
 
 // NewRequestBouncer initializes a new RequestBouncer
 func NewRequestBouncer(dataStore dataservices.DataStore, jwtService portainer.JWTService, apiKeyService apikey.APIKeyService) *RequestBouncer {
-	return &RequestBouncer{
+	b := &RequestBouncer{
 		dataStore:     dataStore,
 		jwtService:    jwtService,
 		apiKeyService: apiKeyService,
 	}
+
+	go b.cleanUpExpiredJWT()
+
+	return b
 }
 
 // PublicAccess defines a security check for public API endpoints.
@@ -80,6 +92,7 @@ func (bouncer *RequestBouncer) AdminAccess(h http.Handler) http.Handler {
 	h = bouncer.mwUpgradeToRestrictedRequest(h)
 	h = bouncer.mwCheckPortainerAuthorizations(h, true)
 	h = bouncer.mwAuthenticatedUser(h)
+
 	return h
 }
 
@@ -92,6 +105,7 @@ func (bouncer *RequestBouncer) RestrictedAccess(h http.Handler) http.Handler {
 	h = bouncer.mwUpgradeToRestrictedRequest(h)
 	h = bouncer.mwCheckPortainerAuthorizations(h, false)
 	h = bouncer.mwAuthenticatedUser(h)
+
 	return h
 }
 
@@ -105,6 +119,7 @@ func (bouncer *RequestBouncer) TeamLeaderAccess(h http.Handler) http.Handler {
 	h = bouncer.mwIsTeamLeader(h)
 	h = bouncer.mwUpgradeToRestrictedRequest(h)
 	h = bouncer.mwAuthenticatedUser(h)
+
 	return h
 }
 
@@ -116,6 +131,7 @@ func (bouncer *RequestBouncer) TeamLeaderAccess(h http.Handler) http.Handler {
 func (bouncer *RequestBouncer) AuthenticatedAccess(h http.Handler) http.Handler {
 	h = bouncer.mwUpgradeToRestrictedRequest(h)
 	h = bouncer.mwAuthenticatedUser(h)
+
 	return h
 }
 
@@ -197,6 +213,7 @@ func (bouncer *RequestBouncer) mwAuthenticatedUser(h http.Handler) http.Handler
 		bouncer.JWTAuthLookup,
 	}, h)
 	h = mwSecureHeaders(h)
+
 	return h
 }
 
@@ -284,23 +301,27 @@ func (bouncer *RequestBouncer) mwAuthenticateFirst(tokenLookups []tokenLookup, n
 			resultToken, err := lookup(r)
 			if err != nil {
 				httperror.WriteError(w, http.StatusUnauthorized, "Invalid JWT token", httperrors.ErrUnauthorized)
+
 				return
 			}
 
 			if resultToken != nil {
 				token = resultToken
+
 				break
 			}
 		}
 
 		if token == nil {
 			httperror.WriteError(w, http.StatusUnauthorized, "A valid authorization token is missing", httperrors.ErrUnauthorized)
+
 			return
 		}
 
 		user, _ := bouncer.dataStore.User().Read(token.ID)
 		if user == nil {
 			httperror.WriteError(w, http.StatusUnauthorized, "An authorization token is invalid", httperrors.ErrUnauthorized)
+
 			return
 		}
 
@@ -317,11 +338,15 @@ func (bouncer *RequestBouncer) CookieAuthLookup(r *http.Request) (*portainer.Tok
 		return nil, nil
 	}
 
-	tokenData, err := bouncer.jwtService.ParseAndVerifyToken(token)
+	tokenData, jti, _, err := bouncer.jwtService.ParseAndVerifyToken(token)
 	if err != nil {
 		return nil, err
 	}
 
+	if _, ok := bouncer.revokedJWT.Load(jti); ok {
+		return nil, ErrRevokedJWT
+	}
+
 	return tokenData, nil
 }
 
@@ -333,15 +358,44 @@ func (bouncer *RequestBouncer) JWTAuthLookup(r *http.Request) (*portainer.TokenD
 		return nil, nil
 	}
 
-	tokenData, err := bouncer.jwtService.ParseAndVerifyToken(token)
+	tokenData, jti, _, err := bouncer.jwtService.ParseAndVerifyToken(token)
 	if err != nil {
 		return nil, err
 	}
 
+	if _, ok := bouncer.revokedJWT.Load(jti); ok {
+		return nil, ErrRevokedJWT
+	}
+
 	return tokenData, nil
 }
 
-var ErrInvalidKey = errors.New("Invalid API key")
+func (bouncer *RequestBouncer) RevokeJWT(token string) {
+	_, jti, exp, err := bouncer.jwtService.ParseAndVerifyToken(token)
+	if err != nil {
+		return
+	}
+
+	bouncer.revokedJWT.Store(jti, exp)
+}
+
+func (bouncer *RequestBouncer) cleanUpExpiredJWTPass() {
+	bouncer.revokedJWT.Range(func(key, value any) bool {
+		if time.Now().After(value.(time.Time)) {
+			bouncer.revokedJWT.Delete(key)
+		}
+
+		return true
+	})
+}
+
+func (bouncer *RequestBouncer) cleanUpExpiredJWT() {
+	ticker := time.NewTicker(time.Hour)
+
+	for range ticker.C {
+		bouncer.cleanUpExpiredJWTPass()
+	}
+}
 
 // apiKeyLookup looks up an verifies an api-key by:
 // - computing the digest of the raw api-key
@@ -392,6 +446,7 @@ func extractBearerToken(r *http.Request) (string, bool) {
 	if token != "" {
 		query.Del("token")
 		r.URL.RawQuery = query.Encode()
+
 		return token, true
 	}
 
@@ -505,11 +560,13 @@ func (bouncer *RequestBouncer) EdgeComputeOperation(next http.Handler) http.Hand
 		settings, err := bouncer.dataStore.Settings().Settings()
 		if err != nil {
 			httperror.WriteError(w, http.StatusServiceUnavailable, "Unable to retrieve settings", err)
+
 			return
 		}
 
 		if !settings.EnableEdgeComputeFeatures {
 			httperror.WriteError(w, http.StatusServiceUnavailable, "Edge compute features are disabled", errors.New("Edge compute features are disabled"))
+
 			return
 		}
 

api/http/security/bouncer_test.go

@@ -5,6 +5,7 @@ import (
 	"net/http"
 	"net/http/httptest"
 	"testing"
+	"time"
 
 	portainer "github.com/portainer/portainer/api"
 	"github.com/portainer/portainer/api/apikey"
@@ -14,6 +15,7 @@ import (
 	"github.com/portainer/portainer/api/jwt"
 
 	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
 )
 
 // testHandler200 is a simple handler which returns HTTP status 200 OK
@@ -39,7 +41,6 @@ func tokenLookupEmpty(r *http.Request) (*portainer.TokenData, error) {
 }
 
 func Test_mwAuthenticateFirst(t *testing.T) {
-
 	_, store := datastore.MustNewTestStore(t, true, true)
 
 	jwtService, err := jwt.NewService("1h", store)
@@ -154,7 +155,6 @@ func Test_extractKeyFromCookie(t *testing.T) {
 }
 
 func Test_extractBearerToken(t *testing.T) {
-
 	tt := []struct {
 		name               string
 		requestHeader      string
@@ -384,7 +384,6 @@ func Test_apiKeyLookup(t *testing.T) {
 }
 
 func Test_ShouldSkipCSRFCheck(t *testing.T) {
-
 	tt := []struct {
 		name                     string
 		cookieValue              string
@@ -459,3 +458,60 @@ func Test_ShouldSkipCSRFCheck(t *testing.T) {
 		})
 	}
 }
+
+func TestJWTRevocation(t *testing.T) {
+	_, store := datastore.MustNewTestStore(t, true, true)
+
+	jwtService, err := jwt.NewService("1h", store)
+	require.NoError(t, err)
+
+	err = store.User().Create(&portainer.User{ID: 1})
+	require.NoError(t, err)
+
+	jwtService.SetUserSessionDuration(time.Second)
+
+	token, _, err := jwtService.GenerateToken(&portainer.TokenData{ID: 1})
+	require.NoError(t, err)
+
+	apiKeyService := apikey.NewAPIKeyService(nil, nil)
+
+	bouncer := NewRequestBouncer(store, jwtService, apiKeyService)
+
+	r, err := http.NewRequest(http.MethodGet, "url", nil)
+	require.NoError(t, err)
+
+	r.Header.Add(jwtTokenHeader, "Bearer "+token)
+
+	r.AddCookie(&http.Cookie{Name: portainer.AuthCookieKey, Value: token})
+
+	_, err = bouncer.JWTAuthLookup(r)
+	require.NoError(t, err)
+
+	_, err = bouncer.CookieAuthLookup(r)
+	require.NoError(t, err)
+
+	bouncer.RevokeJWT(token)
+
+	revokeLen := func() (l int) {
+		bouncer.revokedJWT.Range(func(key, value any) bool {
+			l++
+
+			return true
+		})
+
+		return l
+	}
+	require.Equal(t, 1, revokeLen())
+
+	_, err = bouncer.JWTAuthLookup(r)
+	require.Error(t, err)
+
+	_, err = bouncer.CookieAuthLookup(r)
+	require.Error(t, err)
+
+	time.Sleep(time.Second)
+
+	bouncer.cleanUpExpiredJWTPass()
+
+	require.Equal(t, 0, revokeLen())
+}

api/internal/testhelpers/request_bouncer.go

@@ -58,6 +58,8 @@ func (testRequestBouncer) JWTAuthLookup(r *http.Request) (*portainer.TokenData,
 	return nil, nil
 }
 
+func (testRequestBouncer) RevokeJWT(jti string) {}
+
 // AddTestSecurityCookie adds a security cookie to the request
 func AddTestSecurityCookie(r *http.Request, jwt string) {
 	r.AddCookie(&http.Cookie{

api/jwt/jwt.go

@@ -13,7 +13,14 @@ import (
 	"github.com/rs/zerolog/log"
 )
 
-const year = time.Hour * 24 * 365
+const (
+	year = time.Hour * 24 * 365
+
+	keyLen = 32
+
+	defaultScope    = scope("default")
+	kubeConfigScope = scope("kubeconfig")
+)
 
 // scope represents JWT scopes that are supported in JWT claims.
 type scope string
@@ -35,13 +42,8 @@ type claims struct {
 }
 
 var (
-	errSecretGeneration = errors.New("Unable to generate secret key")
+	errSecretGeneration = errors.New("unable to generate secret key")
-	errInvalidJWTToken  = errors.New("Invalid JWT token")
+	errInvalidJWTToken  = errors.New("invalid JWT token")
-)
-
-const (
-	defaultScope    = scope("default")
-	kubeConfigScope = scope("kubeconfig")
 )
 
 // NewService initializes a new service. It will generate a random key that will be used to sign JWT tokens.
@@ -51,7 +53,7 @@ func NewService(userSessionDuration string, dataStore dataservices.DataStore) (*
 		return nil, err
 	}
 
-	secret := apikey.GenerateRandomKey(32)
+	secret := apikey.GenerateRandomKey(keyLen)
 	if secret == nil {
 		return nil, errSecretGeneration
 	}
@@ -61,16 +63,14 @@ func NewService(userSessionDuration string, dataStore dataservices.DataStore) (*
 		return nil, err
 	}
 
-	service := &Service{
+	return &Service{
 		map[scope][]byte{
 			defaultScope:    secret,
 			kubeConfigScope: kubeSecret,
 		},
 		userSessionTimeout,
 		dataStore,
-	}
+	}, nil
-
-	return service, nil
 }
 
 func getOrCreateKubeSecret(dataStore dataservices.DataStore) ([]byte, error) {
@@ -80,17 +80,19 @@ func getOrCreateKubeSecret(dataStore dataservices.DataStore) ([]byte, error) {
 	}
 
 	kubeSecret := settings.OAuthSettings.KubeSecretKey
+	if kubeSecret != nil {
+		return kubeSecret, nil
+	}
+
+	kubeSecret = apikey.GenerateRandomKey(keyLen)
 	if kubeSecret == nil {
-		kubeSecret = apikey.GenerateRandomKey(32)
+		return nil, errSecretGeneration
-		if kubeSecret == nil {
+	}
-			return nil, errSecretGeneration
-		}
 
-		settings.OAuthSettings.KubeSecretKey = kubeSecret
+	settings.OAuthSettings.KubeSecretKey = kubeSecret
 
-		if err := dataStore.Settings().UpdateSettings(settings); err != nil {
+	if err := dataStore.Settings().UpdateSettings(settings); err != nil {
-			return nil, err
+		return nil, err
-		}
 	}
 
 	return kubeSecret, nil
@@ -104,53 +106,54 @@ func (service *Service) defaultExpireAt() time.Time {
 func (service *Service) GenerateToken(data *portainer.TokenData) (string, time.Time, error) {
 	expiryTime := service.defaultExpireAt()
 	token, err := service.generateSignedToken(data, expiryTime, defaultScope)
+
 	return token, expiryTime, err
 }
 
 // ParseAndVerifyToken parses a JWT token and verify its validity. It returns an error if token is invalid.
-func (service *Service) ParseAndVerifyToken(token string) (*portainer.TokenData, error) {
+func (service *Service) ParseAndVerifyToken(token string) (*portainer.TokenData, string, time.Time, error) {
 	scope := parseScope(token)
 	secret := service.secrets[scope]
+
 	parsedToken, err := jwt.ParseWithClaims(token, &claims{}, func(token *jwt.Token) (any, error) {
 		if _, ok := token.Method.(*jwt.SigningMethodHMAC); !ok {
-			msg := fmt.Errorf("unexpected signing method: %v", token.Header["alg"])
+			return nil, fmt.Errorf("unexpected signing method: %v", token.Header["alg"])
-			return nil, msg
 		}
+
 		return secret, nil
 	})
-
+	if err != nil || parsedToken == nil {
-	if err == nil && parsedToken != nil {
+		return nil, "", time.Time{}, errInvalidJWTToken
-		if cl, ok := parsedToken.Claims.(*claims); ok && parsedToken.Valid {
-
-			user, err := service.dataStore.User().Read(portainer.UserID(cl.UserID))
-			if err != nil {
-				return nil, errInvalidJWTToken
-			}
-			if user.TokenIssueAt > cl.RegisteredClaims.IssuedAt.Unix() {
-				return nil, errInvalidJWTToken
-			}
-
-			return &portainer.TokenData{
-				ID:                  portainer.UserID(cl.UserID),
-				Username:            cl.Username,
-				Role:                portainer.UserRole(cl.Role),
-				Token:               token,
-				ForceChangePassword: cl.ForceChangePassword,
-			}, nil
-		}
 	}
-	return nil, errInvalidJWTToken
+
+	cl, ok := parsedToken.Claims.(*claims)
+	if !ok || !parsedToken.Valid {
+		return nil, "", time.Time{}, errInvalidJWTToken
+	}
+
+	user, err := service.dataStore.User().Read(portainer.UserID(cl.UserID))
+	if err != nil || user.TokenIssueAt > cl.RegisteredClaims.IssuedAt.Unix() {
+		return nil, "", time.Time{}, errInvalidJWTToken
+	}
+
+	return &portainer.TokenData{
+		ID:                  portainer.UserID(cl.UserID),
+		Username:            cl.Username,
+		Role:                portainer.UserRole(cl.Role),
+		Token:               token,
+		ForceChangePassword: cl.ForceChangePassword,
+	}, cl.ID, cl.ExpiresAt.Time, nil
 }
 
 // parse a JWT token, fallback to defaultScope if no scope is present in the JWT
 func parseScope(token string) scope {
 	unverifiedToken, _, _ := new(jwt.Parser).ParseUnverified(token, &claims{})
-	if unverifiedToken != nil {
+	if unverifiedToken == nil {
-		if cl, ok := unverifiedToken.Claims.(*claims); ok {
+		return defaultScope
-			if cl.Scope == kubeConfigScope {
+	}
-				return kubeConfigScope
+
-			}
+	if cl, ok := unverifiedToken.Claims.(*claims); ok && cl.Scope == kubeConfigScope {
-		}
+		return kubeConfigScope
 	}
 
 	return defaultScope
@@ -173,9 +176,8 @@ func (service *Service) generateSignedToken(data *portainer.TokenData, expiresAt
 	}
 
 	if settings.IsDockerDesktopExtension {
-		// Set expiration to 99 years for docker desktop extension.
 		log.Info().Msg("detected docker desktop extension mode")
-		expiresAt = time.Now().Add(year * 99)
+		expiresAt = time.Now().Add(99 * year)
 	}
 
 	cl := claims{
@@ -196,10 +198,6 @@ func (service *Service) generateSignedToken(data *portainer.TokenData, expiresAt
 	}
 
 	token := jwt.NewWithClaims(jwt.SigningMethodHS256, cl)
-	signedToken, err := token.SignedString(secret)
-	if err != nil {
-		return "", err
-	}
 
-	return signedToken, nil
+	return token.SignedString(secret)
 }

api/jwt/jwt_test.go

@@ -4,14 +4,17 @@ import (
 	"testing"
 	"time"
 
-	"github.com/golang-jwt/jwt/v4"
 	portainer "github.com/portainer/portainer/api"
-	i "github.com/portainer/portainer/api/internal/testhelpers"
+	"github.com/portainer/portainer/api/datastore"
+	"github.com/portainer/portainer/api/internal/testhelpers"
+
+	"github.com/golang-jwt/jwt/v4"
 	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
 )
 
 func TestGenerateSignedToken(t *testing.T) {
-	dataStore := i.NewDatastore(i.WithSettingsService(&portainer.Settings{}))
+	dataStore := testhelpers.NewDatastore(testhelpers.WithSettingsService(&portainer.Settings{}))
 	svc, err := NewService("24h", dataStore)
 	assert.NoError(t, err, "failed to create a copy of service")
 
@@ -40,7 +43,7 @@ func TestGenerateSignedToken(t *testing.T) {
 }
 
 func TestGenerateSignedToken_InvalidScope(t *testing.T) {
-	dataStore := i.NewDatastore(i.WithSettingsService(&portainer.Settings{}))
+	dataStore := testhelpers.NewDatastore(testhelpers.WithSettingsService(&portainer.Settings{}))
 	svc, err := NewService("24h", dataStore)
 	assert.NoError(t, err, "failed to create a copy of service")
 
@@ -55,3 +58,56 @@ func TestGenerateSignedToken_InvalidScope(t *testing.T) {
 	assert.Error(t, err)
 	assert.Equal(t, "invalid scope: testing", err.Error())
 }
+
+func TestGenerationAndParsing(t *testing.T) {
+	_, store := datastore.MustNewTestStore(t, true, false)
+
+	err := store.User().Create(&portainer.User{ID: 1})
+	require.NoError(t, err)
+
+	service, err := NewService("1h", store)
+	require.NoError(t, err)
+
+	expectedToken := &portainer.TokenData{
+		Username: "User",
+		ID:       1,
+		Role:     1,
+	}
+
+	tokenString, _, err := service.GenerateToken(expectedToken)
+	require.NoError(t, err)
+
+	expectedToken.Token = tokenString
+
+	token, _, _, err := service.ParseAndVerifyToken(tokenString)
+	require.NoError(t, err)
+	require.Equal(t, expectedToken, token)
+}
+
+func TestExpiration(t *testing.T) {
+	_, store := datastore.MustNewTestStore(t, true, false)
+
+	err := store.User().Create(&portainer.User{ID: 1})
+	require.NoError(t, err)
+
+	service, err := NewService("1h", store)
+	require.NoError(t, err)
+
+	expectedToken := &portainer.TokenData{
+		Username: "User",
+		ID:       1,
+		Role:     1,
+	}
+
+	service.SetUserSessionDuration(time.Second)
+
+	tokenString, _, err := service.GenerateToken(expectedToken)
+	require.NoError(t, err)
+
+	expectedToken.Token = tokenString
+
+	time.Sleep(2 * time.Second)
+
+	_, _, _, err = service.ParseAndVerifyToken(tokenString)
+	require.Error(t, err)
+}

api/portainer.go

@@ -1458,7 +1458,7 @@ type (
 	JWTService interface {
 		GenerateToken(data *TokenData) (string, time.Time, error)
 		GenerateTokenForKubeconfig(data *TokenData) (string, error)
-		ParseAndVerifyToken(token string) (*TokenData, error)
+		ParseAndVerifyToken(token string) (*TokenData, string, time.Time, error)
 		SetUserSessionDuration(userSessionDuration time.Duration)
 	}