feat(auth,security,scheduling): JWT revocation, S3 fix, notes validation, docs, tests

- JWT revocation with JTI (UUID v4): in-memory tracking, POST /api/logout, refresh handler revokes old JTI, RequireAuth rejects revoked tokens - Fix extractKey for S3 portfolio deletion: extracts full key path from URLs instead of just filename, preventing orphaned storage files - Notes validation: max=1000000 on all 13 Notes fields across 4 booking structs - CharCounter: grapheme-aware counter (Intl.Segmenter), threshold 750K, color-coded, integrated into 6 booking/admin components - loginInProgress: timestamp-based tracking, 30s staleness, 20-entry cap (429), ticker cleanup for stuck entries - Profile picture 15MB client-side limit, portfolio 20MB backend limit - Exceptional scheduling: expand query start to Monday of week - TodayCalendar: week-range fetching, closing time indicator, short-day lunch skip - NavBar: link reorder, mobile burger badge, slide transition, backdrop - ImageUpload: 20MB limit with visual feedback - formatDateISO: shared YYYY-MM-DD utility, shouldApplyLunchProtection helper - Update README.md and all Obsidian docs (Overview, Technical, Admin, Future Work) - Add 28 new tests: JWT (11), auth handlers (7), portfolio extractKey (5), notes validation (5). go build + go vet clean with test,dev tags
2026-06-03 11:17:41 +01:00
parent 169d7dc6e3
commit bffb984ebb
30 changed files with 1016 additions and 62 deletions
@@ -31,6 +31,7 @@ import (
 	"testing"
 	"time"

+	"crussell/auth"
 	"crussell/db"
 	"crussell/internal/dav"
 	"crussell/mw"
@@ -38,6 +39,7 @@ import (
 	"crussell/testutils/jwt"
 	"crussell/testutils/testdb"

+	"github.com/go-chi/chi/v5"
 	"github.com/jackc/pgx/v5/pgxpool"
 )

@@ -1105,5 +1107,291 @@ func TestRegister_WithInvalidReferralCodeFormat(t *testing.T) {
 	}
 }

+// =============================================================================
+// Logout Handler Tests
+// =============================================================================
+
+// TestLogoutHandler_Success verifies that a valid logout request returns
+// 200 OK with {"success": true} and revokes the JTI.
+func TestLogoutHandler_Success(t *testing.T) {
+	resetTestData(t)
+
+	// Create a test user
+	userID, err := fixtures.CreateTestUser(db.DB)
+	if err != nil {
+		t.Fatalf("failed to create test user: %v", err)
+	}
+	defer fixtures.DeleteUser(db.DB, userID)
+
+	// Generate a token and get its JTI
+	token, jti, err := auth.GenerateToken(userID, "verified_email")
+	if err != nil {
+		t.Fatalf("failed to generate token: %v", err)
+	}
+
+	// Create request with JTI in context (simulating RequireAuth middleware)
+	req := httptest.NewRequest("POST", "/api/logout", nil)
+	req.Header.Set("Authorization", "Bearer "+token)
+	ctx := context.WithValue(req.Context(), mw.JTIKey, jti)
+	req = req.WithContext(ctx)
+	w := httptest.NewRecorder()
+
+	LogoutHandler(w, req)
+
+	if w.Code != http.StatusOK {
+		t.Errorf("expected status 200, got %d. body: %s", w.Code, w.Body.String())
+	}
+
+	var resp struct {
+		Success bool `json:"success"`
+	}
+	if err := parseResponseBody(w, &resp); err != nil {
+		t.Errorf("failed to parse response: %v", err)
+	}
+	if !resp.Success {
+		t.Error("expected success=true in response")
+	}
+
+	// Verify JTI was revoked
+	if !auth.IsJTIRevoked(jti) {
+		t.Error("expected JTI to be revoked after logout")
+	}
+}
+
+// TestLogoutHandler_RevokesJTI verifies that after logout, the token's JTI is
+// revoked and the token can no longer be used with authenticated endpoints.
+func TestLogoutHandler_RevokesJTI(t *testing.T) {
+	resetTestData(t)
+
+	// Create a test user
+	userID, err := fixtures.CreateTestUser(db.DB)
+	if err != nil {
+		t.Fatalf("failed to create test user: %v", err)
+	}
+	defer fixtures.DeleteUser(db.DB, userID)
+
+	// Generate a token and get its JTI
+	token, jti, err := auth.GenerateToken(userID, "verified_email")
+	if err != nil {
+		t.Fatalf("failed to generate token: %v", err)
+	}
+
+	// Call logout with JTI in context
+	req := httptest.NewRequest("POST", "/api/logout", nil)
+	req.Header.Set("Authorization", "Bearer "+token)
+	ctx := context.WithValue(req.Context(), mw.JTIKey, jti)
+	req = req.WithContext(ctx)
+	w := httptest.NewRecorder()
+	LogoutHandler(w, req)
+
+	if w.Code != http.StatusOK {
+		t.Fatalf("logout failed: %d. body: %s", w.Code, w.Body.String())
+	}
+
+	// Try to use the same token through RequireAuth middleware
+	router := chi.NewRouter()
+	router.With(mw.RequireAuth).Get("/api/protected", func(w http.ResponseWriter, r *http.Request) {
+		w.WriteHeader(http.StatusOK)
+	})
+
+	req2 := httptest.NewRequest("GET", "/api/protected", nil)
+	req2.Header.Set("Authorization", "Bearer "+token)
+	w2 := httptest.NewRecorder()
+	router.ServeHTTP(w2, req2)
+
+	if w2.Code != http.StatusUnauthorized {
+		t.Errorf("expected 401 for revoked token, got %d", w2.Code)
+	}
+}
+
+// TestLogoutHandler_NoToken verifies that calling logout without an
+// Authorization header returns 401.
+func TestLogoutHandler_NoToken(t *testing.T) {
+	resetTestData(t)
+
+	req := httptest.NewRequest("POST", "/api/logout", nil)
+	w := httptest.NewRecorder()
+
+	// Without JTI in context, handler returns 401
+	LogoutHandler(w, req)
+
+	if w.Code != http.StatusUnauthorized {
+		t.Errorf("expected status 401, got %d. body: %s", w.Code, w.Body.String())
+	}
+}
+
+// TestLogoutHandler_InvalidToken verifies that calling logout with an empty
+// JTI returns 401.
+func TestLogoutHandler_InvalidToken(t *testing.T) {
+	resetTestData(t)
+
+	req := httptest.NewRequest("POST", "/api/logout", nil)
+	ctx := context.WithValue(req.Context(), mw.JTIKey, "")
+	req = req.WithContext(ctx)
+	w := httptest.NewRecorder()
+
+	LogoutHandler(w, req)
+
+	if w.Code != http.StatusUnauthorized {
+		t.Errorf("expected status 401, got %d. body: %s", w.Code, w.Body.String())
+	}
+}
+
+// =============================================================================
+// Refresh Token JTI Tests
+// =============================================================================
+
+// TestRefreshToken_RevokesOldJTI verifies that refreshing a token revokes the
+// old JTI and issues a new one. The old token becomes invalid after refresh.
+func TestRefreshToken_RevokesOldJTI(t *testing.T) {
+	resetTestData(t)
+
+	// Create a test user
+	userID, err := fixtures.CreateTestUser(db.DB)
+	if err != nil {
+		t.Fatalf("failed to create test user: %v", err)
+	}
+	defer fixtures.DeleteUser(db.DB, userID)
+
+	// Generate initial token and JTI
+	oldToken, oldJTI, err := auth.GenerateToken(userID, "verified_email")
+	if err != nil {
+		t.Fatalf("failed to generate old token: %v", err)
+	}
+
+	// Verify old JTI is not yet revoked
+	if auth.IsJTIRevoked(oldJTI) {
+		t.Fatal("old JTI should not be revoked before refresh")
+	}
+
+	// Call refresh handler with old JTI in context
+	req := httptest.NewRequest("POST", "/api/refresh-token", nil)
+	req.Header.Set("Authorization", "Bearer "+oldToken)
+	ctx := req.Context()
+	ctx = context.WithValue(ctx, mw.UserIDKey, userID)
+	ctx = context.WithValue(ctx, mw.UserRoleKey, "verified_email")
+	ctx = context.WithValue(ctx, mw.JTIKey, oldJTI)
+	req = req.WithContext(ctx)
+	w := httptest.NewRecorder()
+
+	RefreshTokenHandler(w, req)
+
+	if w.Code != http.StatusOK {
+		t.Fatalf("refresh failed: %d. body: %s", w.Code, w.Body.String())
+	}
+
+	// Verify old JTI was revoked
+	if !auth.IsJTIRevoked(oldJTI) {
+		t.Error("expected old JTI to be revoked after refresh")
+	}
+
+	// Verify old token is rejected by middleware
+	router := chi.NewRouter()
+	router.With(mw.RequireAuth).Get("/api/protected", func(w http.ResponseWriter, r *http.Request) {
+		w.WriteHeader(http.StatusOK)
+	})
+
+	req2 := httptest.NewRequest("GET", "/api/protected", nil)
+	req2.Header.Set("Authorization", "Bearer "+oldToken)
+	w2 := httptest.NewRecorder()
+	router.ServeHTTP(w2, req2)
+
+	if w2.Code != http.StatusUnauthorized {
+		t.Errorf("expected 401 for revoked token after refresh, got %d", w2.Code)
+	}
+}
+
+// =============================================================================
+// Login Response JTI Tests
+// =============================================================================
+
+// TestLoginResponse_IncludesJTI verifies that the login response includes both
+// "token" and "jti" fields, both non-empty.
+func TestLoginResponse_IncludesJTI(t *testing.T) {
+	resetTestData(t)
+
+	handler := http.HandlerFunc(LoginHandler)
+
+	// Create a test user with known email
+	userID, err := fixtures.CreateTestUserWithEmail(db.DB, "jti-test@test.com", "verified_email")
+	if err != nil {
+		t.Fatalf("failed to create test user: %v", err)
+	}
+	defer fixtures.DeleteUser(db.DB, userID)
+
+	body := LoginRequest{
+		Email:    "jti-test@test.com",
+		Password: "testpassword123",
+	}
+
+	w := makeRequest(handler, "POST", "/api/login", body)
+
+	if w.Code != http.StatusOK {
+		t.Errorf("expected status 200, got %d. body: %s", w.Code, w.Body.String())
+	}
+
+	var resp struct {
+		Token string `json:"token"`
+		JTI   string `json:"jti"`
+	}
+	if err := parseResponseBody(w, &resp); err != nil {
+		t.Errorf("failed to parse response: %v", err)
+	}
+
+	if resp.Token == "" {
+		t.Error("expected non-empty token in login response")
+	}
+	if resp.JTI == "" {
+		t.Error("expected non-empty jti in login response")
+	}
+}
+
+// =============================================================================
+// LoginInProgress Rate Limiting Tests
+// =============================================================================
+
+// TestLoginInProgress_Cap verifies that when the loginInProgress map is full
+// (20 concurrent logins), the 21st attempt returns HTTP 429 Too Many Requests.
+func TestLoginInProgress_Cap(t *testing.T) {
+	resetTestData(t)
+
+	handler := http.HandlerFunc(LoginHandler)
+
+	// Create a test user
+	userID, err := fixtures.CreateTestUserWithEmail(db.DB, "ratelimit@test.com", "verified_email")
+	if err != nil {
+		t.Fatalf("failed to create test user: %v", err)
+	}
+	defer fixtures.DeleteUser(db.DB, userID)
+
+	// Fill the loginInProgress map with 20 entries
+	loginStateMu.Lock()
+	for i := 0; i < maxLoginInProgress; i++ {
+		loginInProgress[fmt.Sprintf("stale-user-%d", i)] = time.Now()
+	}
+	loginStateMu.Unlock()
+
+	defer func() {
+		// Clean up
+		loginStateMu.Lock()
+		for i := 0; i < maxLoginInProgress; i++ {
+			delete(loginInProgress, fmt.Sprintf("stale-user-%d", i))
+		}
+		loginStateMu.Unlock()
+	}()
+
+	// Attempt login - should get 429
+	body := LoginRequest{
+		Email:    "ratelimit@test.com",
+		Password: "testpassword123",
+	}
+
+	w := makeRequest(handler, "POST", "/api/login", body)
+
+	if w.Code != http.StatusTooManyRequests {
+		t.Errorf("expected status 429, got %d. body: %s", w.Code, w.Body.String())
+	}
+}
+
 // Ensure test compilation - import pgxpool to avoid unused import
 var _ = func() *pgxpool.Pool { return nil }
@@ -30,10 +30,12 @@ var (
 	titleCaser = cases.Title(language.English)
 )

+const maxLoginInProgress = 20
+
 // Login state management
 var (
 	loginStateMu    sync.Mutex
-	loginInProgress = make(map[string]bool)
+	loginInProgress = make(map[string]time.Time)
 	loginAttempts   = make(map[string]time.Time)
 )

@@ -51,6 +53,12 @@ func init() {
 					delete(loginAttempts, userID)
 				}
 			}
+			// Clean up stuck loginInProgress entries (older than 30s)
+			for userID, startedAt := range loginInProgress {
+				if now.Sub(startedAt) > 30*time.Second {
+					delete(loginInProgress, userID)
+				}
+			}
 			loginStateMu.Unlock()
 		}
 	}()
@@ -311,12 +319,18 @@ func LoginHandler(w http.ResponseWriter, r *http.Request) {

 	// Check if user is already logging in
 	loginStateMu.Lock()
-	if loginInProgress[userID] {
+	if t, ok := loginInProgress[userID]; ok && time.Since(t) < 30*time.Second {
 		loginStateMu.Unlock()
 		http.Error(w, "login already in progress", http.StatusConflict) // 409
 		return
 	}
-	loginInProgress[userID] = true
+	// Cap the map size - drop new request if at capacity
+	if len(loginInProgress) >= maxLoginInProgress {
+		loginStateMu.Unlock()
+		http.Error(w, "server busy, try again later", http.StatusTooManyRequests)
+		return
+	}
+	loginInProgress[userID] = time.Now()
 	loginStateMu.Unlock()

 	// Always clear flag when done
@@ -363,20 +377,21 @@ func LoginHandler(w http.ResponseWriter, r *http.Request) {
 	}

 	// Generate JWT
-	tokenString, err := auth.GenerateToken(userID, role)
+	tokenString, jti, err := auth.GenerateToken(userID, role)
 	if err != nil {
 		http.Error(w, "could not generate token", http.StatusInternalServerError)
 		return
 	}

 	w.Header().Set("Content-Type", "application/json")
-	json.NewEncoder(w).Encode(auth.AuthResponse{Token: tokenString})
+	json.NewEncoder(w).Encode(auth.AuthResponse{Token: tokenString, JTI: jti})
 }

 // POST /api/refresh-token (requires auth middleware)
 func RefreshTokenHandler(w http.ResponseWriter, r *http.Request) {
 	userID, _ := mw.GetUserID(r.Context())
 	role, _ := mw.GetUserRole(r.Context())
+	oldJTI, _ := mw.GetJTI(r.Context())

 	// Verify user still exists and role hasn't changed
 	var currentRole string
@@ -395,15 +410,36 @@ func RefreshTokenHandler(w http.ResponseWriter, r *http.Request) {
 		return
 	}

+	// Revoke the old token's JTI before issuing a new one
+	if oldJTI != "" {
+		// Use a 30-day expiry from now for the revoked JTI (matching token lifetime)
+		auth.RevokeJTI(oldJTI, time.Now().Add(30*24*time.Hour))
+	}
+
 	// Generate new token
-	newToken, err := auth.GenerateToken(userID, currentRole)
+	newToken, jti, err := auth.GenerateToken(userID, currentRole)
 	if err != nil {
 		http.Error(w, "could not generate token", http.StatusInternalServerError)
 		return
 	}

 	w.Header().Set("Content-Type", "application/json")
-	json.NewEncoder(w).Encode(auth.AuthResponse{Token: newToken})
+	json.NewEncoder(w).Encode(auth.AuthResponse{Token: newToken, JTI: jti})
+}
+
+// POST /api/logout (requires auth middleware)
+func LogoutHandler(w http.ResponseWriter, r *http.Request) {
+	jti, ok := mw.GetJTI(r.Context())
+	if !ok || jti == "" {
+		http.Error(w, "invalid token", http.StatusUnauthorized)
+		return
+	}
+
+	// Revoke the JTI — it will be kept until the token's natural expiry (30 days)
+	auth.RevokeJTI(jti, time.Now().Add(30*24*time.Hour))
+
+	w.Header().Set("Content-Type", "application/json")
+	json.NewEncoder(w).Encode(map[string]bool{"success": true})
 }

 type VerificationCodeRequest struct {