fix(apiserver): revoke the token on logout (bug-1946)

Cause: Logout only deleted tokens/<user>/token, a file that token
validation (ValidateHandler / GetUsernameFromToken) never reads, so a
logged-out token stayed valid until its expiry (24 h by default).

Fix: an in-memory revocation set (controllers/token_revocation.go) keyed
by the token's jti (generateJWT now sets a random uuid jti, so two logons
in the same second give different tokens) or, for tokens without a jti,
by SHA-256 of the token; each entry is kept until the token's expiry and
expired entries are purged on every revocation. Logout revokes the token;
the shared ParseTokenClaims (used by ValidateHandler, RequirePermission
and handlers via GetUsernameFromToken) rejects revoked tokens. A missing
token file no longer fails logout. Scope: per process, not persisted and
not shared between apiservers.

Tests: TestLogout_RevokesToken (middleware), TestTokenRevocations_*,
TestIntegration_PostLogoutTokenRejected (replaces the design-gap test).

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

src/frontend/apiserver/controllers/auth.go

@@ -16,6 +16,7 @@ import (
 
 	"github.com/gin-gonic/gin"
 	"github.com/golang-jwt/jwt/v4"
+	"github.com/google/uuid"
 	"go.uber.org/zap"
 )
 
@@ -48,6 +49,8 @@ func generateJWT(username string, IP string) (string, error) {
 		Username: username,
 		IP:       IP,
 		RegisteredClaims: jwt.RegisteredClaims{
+			// unique token id: logout revokes by it (bug-1946)
+			ID:        uuid.NewString(),
 			ExpiresAt: jwt.NewNumericDate(time.Now().Add(time.Duration(utils.AuthenticationYamlConfig.Token.TokenExpireTime) * time.Second)),
 			IssuedAt:  jwt.NewNumericDate(time.Now()),
 			NotBefore: jwt.NewNumericDate(time.Now()),
@@ -99,11 +102,13 @@ func storeToken(token string, username string) error {
 	return nil
 }
 
-// removeToken removes the JWT file for a user.
+// removeToken removes the JWT file for a user. A missing file is not an
+// error: the file only mirrors the last token issued, and revocation does
+// not depend on it.
 func removeToken(username string) error {
 	tokenDir := filepath.Join(utils.Global_TopDir, "tokens", username)
 	tokenFile := filepath.Join(tokenDir, "token")
-	if err := os.Remove(tokenFile); err != nil {
+	if err := os.Remove(tokenFile); err != nil && !os.IsNotExist(err) {
 		return err
 	}
 	// Remove user directory if empty
@@ -175,15 +180,22 @@ func (a AuthController) Logout(ctx *gin.Context) {
 	token := ctx.GetHeader("Authorization")
 
 	// get username from token
-	var err error
-	username, err := GetUsernameFromToken(strings.TrimPrefix(token, "Bearer "))
+	claims, err := ParseTokenClaims(token)
 	if err != nil {
 		logger.Error(common.ERROR_GET_USERNAME_MSG, zap.Error(err))
 		ReturnError(ctx, common.ERROR_CODE, common.ERROR_GET_USERNAME_MSG+": "+err.Error(), nil)
 		return
 	}
+	username := claims.Username
 	logger.Debug("username", zap.String("username", username))
 
+	// Revoke the token until its expiry: validation rejects it from now on.
+	expiresAt := time.Now().Add(time.Duration(utils.AuthenticationYamlConfig.Token.TokenExpireTime) * time.Second)
+	if claims.ExpiresAt != nil {
+		expiresAt = claims.ExpiresAt.Time
+	}
+	RevokeToken(token, claims.ID, expiresAt)
+
 	// Remove token file from server-managed directory
 	if err := removeToken(username); err != nil {
 		logger.Error(common.ERROR_REMOVE_TOKEN_MSG, zap.Error(err))
@@ -198,19 +210,33 @@ func (a AuthController) Logout(ctx *gin.Context) {
 	ReturnSuccess(ctx, common.SUCCESS_CODE, common.SUCCESS_MSG, response)
 }
 
-func GetUsernameFromToken(tokenString string) (string, error) {
-	token, err := jwt.Parse(strings.TrimPrefix(tokenString, "Bearer "), JWTKeyFunc, JWTValidMethods())
+// ParseTokenClaims parses and verifies a token ("Bearer " prefix optional)
+// and rejects a token revoked by logout.
+func ParseTokenClaims(tokenString string) (*models.JWTClaims, error) {
+	tokenString = strings.TrimPrefix(tokenString, "Bearer ")
+	claims := &models.JWTClaims{}
+	token, err := jwt.ParseWithClaims(tokenString, claims, JWTKeyFunc, JWTValidMethods())
+	if err != nil {
+		return nil, err
+	}
+	if !token.Valid {
+		return nil, fmt.Errorf("invalid token")
+	}
+	if IsTokenRevoked(tokenString, claims.ID) {
+		return nil, fmt.Errorf("token has been revoked")
+	}
+	return claims, nil
+}
 
+func GetUsernameFromToken(tokenString string) (string, error) {
+	claims, err := ParseTokenClaims(tokenString)
 	if err != nil {
 		return "", err
 	}
-
-	if claims, ok := token.Claims.(jwt.MapClaims); ok && token.Valid {
-		if username, ok := claims["username"].(string); ok {
-			return username, nil
-		}
+	if claims.Username == "" {
+		return "", fmt.Errorf("invalid token")
 	}
-	return "", fmt.Errorf("invalid token")
+	return claims.Username, nil
 }
 
 // getAccessQueues returns the list of queues the user can access for the given permission.

src/frontend/apiserver/controllers/integration_test.go

@@ -591,9 +591,10 @@ users:
 	}
 }
 
-// --- 6.7 P1: Post-logout token reuse (design gap documentation) ---
-// JWTs are stateless — no JTI blacklist. A valid token remains usable until expiry.
-func TestIntegration_DesignGap_PostLogoutTokenReuse(t *testing.T) {
+// --- 6.7 P1: Post-logout token reuse is rejected (bug-1946) ---
+// Logout revokes the token until its expiry (in-memory, per process). This
+// token has no jti, so it is revoked by its hash.
+func TestIntegration_PostLogoutTokenRejected(t *testing.T) {
 	resetIntegrationState()
 	initTestAuthConfig()
 
@@ -602,14 +603,17 @@ func TestIntegration_DesignGap_PostLogoutTokenReuse(t *testing.T) {
 		ctx.JSON(http.StatusOK, gin.H{"status": "ok"})
 	})
 
-	// Create a valid long-lived token
-	token := signTestJWT("user1", "basic_user", time.Now().Add(time.Hour))
+	expiresAt := time.Now().Add(time.Hour)
+	token := signTestJWT("user1", "basic_user", expiresAt)
+	// No RBAC roles are loaded here, so the token passes authentication and
+	// fails authorization (403); only the 401 matters.
+	if w := doRequest(router, "GET", "/xce/v1/hosts", token); w.Code == http.StatusUnauthorized {
+		t.Fatalf("before logout: status = 401, the token must authenticate")
+	}
 
-	// "Logout" is a no-op in stateless JWT — the token is still valid
-	w := doRequest(router, "GET", "/xce/v1/hosts", token)
-	if w.Code == http.StatusUnauthorized {
-		t.Error("post-logout token reuse: got 401, but JWTs are stateless and no blacklist exists — " +
-			"if this fails, a JTI blacklist has been implemented and this test should be updated")
+	RevokeToken(token, "", expiresAt)
+	if w := doRequest(router, "GET", "/xce/v1/hosts", token); w.Code != http.StatusUnauthorized {
+		t.Errorf("after logout: status = %d, want 401", w.Code)
 	}
 }
 

src/frontend/apiserver/controllers/token_revocation.go

@@ -0,0 +1,73 @@
+package controllers
+
+import (
+	"crypto/sha256"
+	"encoding/hex"
+	"strings"
+	"sync"
+	"time"
+)
+
+// tokenRevocations is the in-memory set of logged-out tokens (bug-1946).
+// Each entry is kept until the token's own expiry, after which the token is
+// rejected anyway; expired entries are purged on every revocation, so the set
+// holds at most the tokens logged out within one token lifetime.
+//
+// Scope: per process. The set is not persisted and not shared, so a revoked
+// token is accepted again by another apiserver sharing XCE_JWT_SECRET, or by
+// this one after a restart with the same XCE_JWT_SECRET.
+type tokenRevocations struct {
+	mu      sync.Mutex
+	expires map[string]time.Time // revocation key -> token expiry
+}
+
+var revokedTokens = &tokenRevocations{expires: make(map[string]time.Time)}
+
+// tokenRevocationKey identifies a token: its jti when present (every token
+// generateJWT issues has one), otherwise a SHA-256 of the raw token string.
+func tokenRevocationKey(tokenString, jti string) string {
+	if jti != "" {
+		return "jti:" + jti
+	}
+	sum := sha256.Sum256([]byte(strings.TrimPrefix(tokenString, "Bearer ")))
+	return "sha256:" + hex.EncodeToString(sum[:])
+}
+
+// revoke records key as revoked until expiresAt and purges expired entries.
+func (r *tokenRevocations) revoke(key string, expiresAt time.Time, now time.Time) {
+	r.mu.Lock()
+	defer r.mu.Unlock()
+	for k, exp := range r.expires {
+		if !exp.After(now) {
+			delete(r.expires, k)
+		}
+	}
+	if expiresAt.After(now) {
+		r.expires[key] = expiresAt
+	}
+}
+
+// isRevoked reports whether key was revoked and has not expired yet.
+func (r *tokenRevocations) isRevoked(key string, now time.Time) bool {
+	r.mu.Lock()
+	defer r.mu.Unlock()
+	exp, ok := r.expires[key]
+	return ok && exp.After(now)
+}
+
+// size returns the number of entries kept (for tests).
+func (r *tokenRevocations) size() int {
+	r.mu.Lock()
+	defer r.mu.Unlock()
+	return len(r.expires)
+}
+
+// RevokeToken revokes a token until its expiry.
+func RevokeToken(tokenString, jti string, expiresAt time.Time) {
+	revokedTokens.revoke(tokenRevocationKey(tokenString, jti), expiresAt, time.Now())
+}
+
+// IsTokenRevoked reports whether a token was revoked by logout.
+func IsTokenRevoked(tokenString, jti string) bool {
+	return revokedTokens.isRevoked(tokenRevocationKey(tokenString, jti), time.Now())
+}

src/frontend/apiserver/controllers/token_revocation_test.go

@@ -0,0 +1,45 @@
+package controllers
+
+import (
+	"testing"
+	"time"
+)
+
+// bug-1946: revocation entries live until the token's expiry and are purged
+// afterwards, so memory stays bounded.
+func TestTokenRevocations_ExpiryAndPurge(t *testing.T) {
+	r := &tokenRevocations{expires: make(map[string]time.Time)}
+	now := time.Unix(1_000_000, 0)
+
+	r.revoke("a", now.Add(time.Hour), now)
+	r.revoke("b", now.Add(2*time.Hour), now)
+	r.revoke("already-expired", now.Add(-time.Second), now)
+	if r.size() != 2 {
+		t.Fatalf("size = %d, want 2 (an already expired token is not stored)", r.size())
+	}
+	if !r.isRevoked("a", now) || !r.isRevoked("b", now) {
+		t.Fatal("revoked tokens must be reported as revoked")
+	}
+	if r.isRevoked("c", now) {
+		t.Error("a token never revoked must not be reported as revoked")
+	}
+
+	later := now.Add(90 * time.Minute)
+	if r.isRevoked("a", later) {
+		t.Error("an entry past its token expiry must not count")
+	}
+	r.revoke("c", later.Add(time.Hour), later)
+	if r.size() != 2 {
+		t.Errorf("size = %d, want 2: the expired entry a must be purged", r.size())
+	}
+}
+
+func TestTokenRevocationKey(t *testing.T) {
+	if got := tokenRevocationKey("tok", "id-1"); got != "jti:id-1" {
+		t.Errorf("with jti: %q", got)
+	}
+	a, b := tokenRevocationKey("Bearer tok", ""), tokenRevocationKey("tok", "")
+	if a != b || a == "" || a == "jti:" {
+		t.Errorf("without jti the key must be the token hash, Bearer prefix ignored: %q %q", a, b)
+	}
+}

src/frontend/apiserver/middleware/validatehandler.go

@@ -3,7 +3,6 @@ package middleware
 import (
 	"apiserver/common"
 	"apiserver/controllers"
-	"apiserver/models"
 	logger "apiserver/utils"
 	"bytes"
 	"encoding/json"
@@ -13,7 +12,6 @@ import (
 	"strings"
 
 	"github.com/gin-gonic/gin"
-	"github.com/golang-jwt/jwt/v4"
 	"go.uber.org/zap"
 )
 
@@ -98,20 +96,14 @@ func validateHeader() gin.HandlerFunc {
 	}
 }
 
+// isValidToken verifies the token (signature, HS256 only, expiry, not
+// revoked by logout) and its IP binding.
 func isValidToken(tokenString string, IP string) (bool, error) {
-	token, err := jwt.ParseWithClaims(tokenString, &models.JWTClaims{},
-		controllers.JWTKeyFunc, controllers.JWTValidMethods())
-
+	claims, err := controllers.ParseTokenClaims(tokenString)
 	if err != nil {
 		return false, err
 	}
-
-	if claims, ok := token.Claims.(*models.JWTClaims); ok && token.Valid {
-		if claims.IP == IP {
-			return true, nil
-		}
-	}
-	return false, nil
+	return claims.IP == IP, nil
 }
 
 func validateRequestBody(ctx *gin.Context) (bool, error) {

src/frontend/apiserver/middleware/validatehandler_test.go

@@ -1,11 +1,18 @@
 package middleware
 
 import (
+	"apiserver/backend/auth"
+	"apiserver/controllers"
 	"apiserver/models"
 	"apiserver/utils"
+	"encoding/json"
+	"net/http"
+	"net/http/httptest"
+	"strings"
 	"testing"
 	"time"
 
+	"github.com/gin-gonic/gin"
 	"github.com/golang-jwt/jwt/v4"
 	"go.uber.org/zap"
 )
@@ -45,6 +52,81 @@ func signTestToken(t *testing.T, method jwt.SigningMethod, username, ip string)
 	return signed
 }
 
+// newAuthFlowRouter wires logon, logout and one protected route behind
+// ValidateHandler, with a config_file provider holding alice/s3cret.
+func newAuthFlowRouter(t *testing.T) *gin.Engine {
+	t.Helper()
+	setTestAuthConfig(t)
+	prevTop, prevProvider := utils.Global_TopDir, auth.GlobalProvider
+	t.Cleanup(func() { utils.Global_TopDir, auth.GlobalProvider = prevTop, prevProvider })
+	utils.Global_TopDir = t.TempDir()
+	provider, err := auth.NewConfigFileProvider([]byte(
+		"provider: config_file\nconfig_file:\n  users:\n    - username: alice\n      password: s3cret\n"))
+	if err != nil {
+		t.Fatalf("NewConfigFileProvider: %v", err)
+	}
+	auth.GlobalProvider = provider
+
+	gin.SetMode(gin.TestMode)
+	r := gin.New()
+	r.Use(ValidateHandler())
+	authHandler := &controllers.AuthController{}
+	r.POST("/xce/v1/auth/logon", authHandler.Logon)
+	r.POST("/xce/v1/auth/logout", authHandler.Logout)
+	r.GET("/xce/v1/hosts", func(ctx *gin.Context) {
+		controllers.ReturnSuccess(ctx, http.StatusOK, "success", nil)
+	})
+	return r
+}
+
+func serve(r *gin.Engine, method, path, token, body string) *httptest.ResponseRecorder {
+	req := httptest.NewRequest(method, path, strings.NewReader(body))
+	req.RemoteAddr = "10.0.0.1:4000"
+	if body != "" {
+		req.Header.Set("Content-Type", "application/json")
+	}
+	if token != "" {
+		req.Header.Set("Authorization", "Bearer "+token)
+	}
+	w := httptest.NewRecorder()
+	r.ServeHTTP(w, req)
+	return w
+}
+
+func logon(t *testing.T, r *gin.Engine) string {
+	t.Helper()
+	w := serve(r, http.MethodPost, "/xce/v1/auth/logon", "", `{"username":"alice","password":"s3cret"}`)
+	var resp struct {
+		Data models.LogonResponse `json:"data"`
+	}
+	if err := json.Unmarshal(w.Body.Bytes(), &resp); err != nil || resp.Data.Token == "" {
+		t.Fatalf("logon failed: status %d body %s", w.Code, w.Body.String())
+	}
+	return resp.Data.Token
+}
+
+// bug-1946: after logout the token is rejected; a new logon (even in the
+// same second) gets a working token.
+func TestLogout_RevokesToken(t *testing.T) {
+	r := newAuthFlowRouter(t)
+
+	token := logon(t, r)
+	if w := serve(r, http.MethodGet, "/xce/v1/hosts", token, ""); w.Code != http.StatusOK {
+		t.Fatalf("before logout: status = %d, want 200 (%s)", w.Code, w.Body.String())
+	}
+	if w := serve(r, http.MethodPost, "/xce/v1/auth/logout", token, ""); w.Code != http.StatusOK {
+		t.Fatalf("logout: status = %d, want 200 (%s)", w.Code, w.Body.String())
+	}
+	if w := serve(r, http.MethodGet, "/xce/v1/hosts", token, ""); w.Code != http.StatusUnauthorized {
+		t.Errorf("after logout: status = %d, want 401 (%s)", w.Code, w.Body.String())
+	}
+
+	fresh := logon(t, r)
+	if w := serve(r, http.MethodGet, "/xce/v1/hosts", fresh, ""); w.Code != http.StatusOK {
+		t.Errorf("token from a new logon: status = %d, want 200 (%s)", w.Code, w.Body.String())
+	}
+}
+
 // bug-1950: only the HMAC algorithm used for signing (HS256) is accepted.
 func TestIsValidToken_PinsSigningAlgorithm(t *testing.T) {
 	setTestAuthConfig(t)