diff --git a/backend/auth/jwt.go b/backend/auth/jwt.go index b44a46d..da59133 100644 --- a/backend/auth/jwt.go +++ b/backend/auth/jwt.go @@ -38,35 +38,35 @@ func generateJTI() (string, error) { b[0:4], b[4:6], b[6:8], b[8:10], b[10:16]), nil } -// RevokeJTI adds a JTI to the revoked set in PostgreSQL -func RevokeJTI(ctx context.Context, jti string, expiresAt time.Time) { +// RevokeJTI adds a JTI to the revoked set in PostgreSQL. +// Returns an error if the operation fails. +func RevokeJTI(ctx context.Context, jti string, expiresAt time.Time) error { if db.Conn == nil { - return + return fmt.Errorf("revoke JTI: db.Conn is nil") } tx, err := db.Conn.Begin(ctx) if err != nil { - log.Printf("WARN: Failed to begin transaction for JTI revocation: %v", err) - return + return fmt.Errorf("revoke JTI: begin transaction: %w", err) } defer func() { - if err := tx.Rollback(ctx); err != nil && err.Error() != "tx is closed" { - slog.Error("failed to rollback transaction", "err", err) - } -}() + if err := tx.Rollback(ctx); err != nil && err.Error() != "tx is closed" { + slog.Error("failed to rollback transaction", "err", err) + } + }() _, err = tx.Exec(ctx, `INSERT INTO revoked_jtis (jti, expires_at) VALUES ($1, $2) ON CONFLICT (jti) DO NOTHING`, jti, expiresAt) if err != nil { - // Log but don't fail - this is best effort - log.Printf("WARN: Failed to revoke JTI %s: %v", jti, err) - return + return fmt.Errorf("revoke JTI %s: %w", jti, err) } if err := tx.Commit(ctx); err != nil { - log.Printf("WARN: Failed to commit transaction for JTI revocation: %v", err) + return fmt.Errorf("revoke JTI: commit transaction: %w", err) } + + return nil } // IsJTIRevoked checks if a JTI is in the revoked set via PostgreSQL. diff --git a/backend/auth/jwt_test.go b/backend/auth/jwt_test.go index aa2bd82..25ae544 100644 --- a/backend/auth/jwt_test.go +++ b/backend/auth/jwt_test.go @@ -165,7 +165,9 @@ func TestVerifyToken_RevokedJTI(t *testing.T) { t.Fatalf("GenerateToken() failed: %v", err) } - RevokeJTI(ctx, jti, clock.Now().Add(30*24*time.Hour)) + if err := RevokeJTI(ctx, jti, clock.Now().Add(30*24*time.Hour)); err != nil { + t.Fatalf("RevokeJTI() failed: %v", err) + } _, _, _, err = VerifyToken(token, ctx) if err == nil { @@ -225,7 +227,9 @@ func TestRevokeJTI_AddsToSet(t *testing.T) { t.Fatal("JTI should not be revoked before calling RevokeJTI") } - RevokeJTI(ctx, jti, clock.Now().Add(30*24*time.Hour)) + if err := RevokeJTI(ctx, jti, clock.Now().Add(30*24*time.Hour)); err != nil { + t.Fatalf("RevokeJTI() failed: %v", err) + } if !IsJTIRevoked(ctx, jti) { t.Error("expected IsJTIRevoked to return true after RevokeJTI") @@ -255,7 +259,9 @@ func TestCleanupRevokedJTIs_RemovesExpired(t *testing.T) { } // Add with future expiry so IsJTIRevoked sees it - RevokeJTI(ctx, jti, clock.Now().Add(1*time.Hour)) + if err := RevokeJTI(ctx, jti, clock.Now().Add(1*time.Hour)); err != nil { + t.Fatalf("RevokeJTI() failed: %v", err) + } if !IsJTIRevoked(ctx, jti) { t.Fatal("JTI should be in revoked set after RevokeJTI") @@ -285,7 +291,9 @@ func TestCleanupRevokedJTIs_KeepsValid(t *testing.T) { } // Add with future expiry - RevokeJTI(ctx, jti, clock.Now().Add(30*24*time.Hour)) + if err := RevokeJTI(ctx, jti, clock.Now().Add(30*24*time.Hour)); err != nil { + t.Fatalf("RevokeJTI() failed: %v", err) + } if !IsJTIRevoked(ctx, jti) { t.Fatal("JTI should be in revoked set before cleanup") @@ -491,8 +499,10 @@ func TestRevokeJTI_NilConn(t *testing.T) { db.Conn = nil t.Cleanup(func() { db.Conn = savedConn }) - // Should not panic when db.Conn is nil - RevokeJTI(context.Background(), "test-jti", time.Now()) + // Should return an error when db.Conn is nil + if err := RevokeJTI(context.Background(), "test-jti", time.Now()); err == nil { + t.Error("expected error when db.Conn is nil, got nil") + } } // TestIsJTIRevoked_NilConn verifies that IsJTIRevoked returns false when db.Conn is nil. diff --git a/backend/handlers/auth/auth_test.go b/backend/handlers/auth/auth_test.go index 87ecffe..7bd38fd 100644 --- a/backend/handlers/auth/auth_test.go +++ b/backend/handlers/auth/auth_test.go @@ -1731,7 +1731,9 @@ func TestJTI_Revocation_PostgreSQL(t *testing.T) { t.Fatalf("token should be valid before revocation: %v", err) } - auth.RevokeJTI(ctx, jti, clock.Now().Add(1*time.Hour)) + if err := auth.RevokeJTI(ctx, jti, clock.Now().Add(1*time.Hour)); err != nil { + t.Fatalf("RevokeJTI() failed: %v", err) + } if !auth.IsJTIRevoked(ctx, jti) { t.Error("JTI should be revoked after RevokeJTI call") diff --git a/backend/handlers/auth/local.go b/backend/handlers/auth/local.go index f1d215f..7036582 100644 --- a/backend/handlers/auth/local.go +++ b/backend/handlers/auth/local.go @@ -485,7 +485,10 @@ func RefreshTokenHandler(w http.ResponseWriter, r *http.Request) { // Revoke the old token's JTI before issuing a new one (rotation) if oldJTI != "" { - auth.RevokeJTI(r.Context(), oldJTI, clock.Now().Add(90*24*time.Hour)) // match refresh token lifetime + // Best-effort revocation: log the error but continue with the refresh + if err := auth.RevokeJTI(r.Context(), oldJTI, clock.Now().Add(90*24*time.Hour)); err != nil { + slog.Error("refresh: failed to revoke old JTI", "oldJTI", oldJTI, "err", err) + } } // Generate new token @@ -510,7 +513,11 @@ func LogoutHandler(w http.ResponseWriter, r *http.Request) { } // Revoke the JTI — match the access token lifetime (1 hour) - auth.RevokeJTI(r.Context(), jti, clock.Now().Add(1*time.Hour)) + if err := auth.RevokeJTI(r.Context(), jti, clock.Now().Add(1*time.Hour)); err != nil { + slog.Error("logout: failed to revoke JTI", "err", err) + http.Error(w, "Failed to revoke token. Please try again.", http.StatusInternalServerError) + return + } _ = json.NewEncoder(w).Encode(map[string]bool{"success": true}) }