diff --git a/Docs/Backend-Development.md b/Docs/Backend-Development.md index e2932dc..65c7507 100644 --- a/Docs/Backend-Development.md +++ b/Docs/Backend-Development.md @@ -110,6 +110,7 @@ backend/ - Use **Depends()** for dependency injection (database sessions, services, auth). - Group endpoints into routers by feature domain (`routers/jails.py`, `routers/bans.py`, …). - Use appropriate HTTP status codes: `201` for creation, `204` for deletion with no body, `404` for not found, etc. +- Protected endpoints should return `401 Unauthorized` or `403 Forbidden` when the session is invalid or expired; the frontend treats these responses as a session-expiry event and redirects the user to `/login`. - Use **HTTPException** or custom exception handlers — never return error dicts manually. - **GET endpoints are read-only — never call `db.commit()` or execute INSERT/UPDATE/DELETE inside a GET handler.** If a GET path produces side-effects (e.g., caching resolved data), that write belongs in a background task, a scheduled flush, or a separate POST endpoint. Users and HTTP caches assume GET is idempotent and non-mutating. diff --git a/Docs/Tasks.md b/Docs/Tasks.md index 9fc9901..2030fe0 100644 --- a/Docs/Tasks.md +++ b/Docs/Tasks.md @@ -111,7 +111,7 @@ Issues are grouped by category and ordered roughly by severity. Each entry descr --- -### TASK-006 — No 401 interceptor: expired sessions show broken pages instead of redirecting +### TASK-006 — No 401 interceptor: expired sessions show broken pages instead of redirecting (done) **Where found:** `frontend/src/api/client.ts`, `request` function. All non-2xx responses including 401 are thrown as a generic `ApiError`. Consumers render "Failed to load…" messages instead of redirecting. diff --git a/frontend/src/api/__tests__/client.test.ts b/frontend/src/api/__tests__/client.test.ts new file mode 100644 index 0000000..7b76134 --- /dev/null +++ b/frontend/src/api/__tests__/client.test.ts @@ -0,0 +1,52 @@ +import { describe, expect, it, beforeEach, vi } from "vitest"; +import { ApiError, get, isAuthError, SESSION_EXPIRED_EVENT } from "../client"; + +describe("api/client", () => { + beforeEach(() => { + vi.restoreAllMocks(); + }); + + it("dispatches session-expired for 401 responses", async () => { + const dispatchSpy = vi.spyOn(window, "dispatchEvent"); + + global.fetch = vi.fn().mockResolvedValue({ + ok: false, + status: 401, + text: vi.fn().mockResolvedValue("Unauthorized"), + }); + + await expect(get("/test")).rejects.toBeInstanceOf(ApiError); + expect(dispatchSpy).toHaveBeenCalledTimes(1); + const call = dispatchSpy.mock.calls[0]; + expect(call).toBeDefined(); + const event = call?.[0] as Event; + expect(event.type).toBe(SESSION_EXPIRED_EVENT); + }); + + it("dispatches session-expired for 403 responses", async () => { + const dispatchSpy = vi.spyOn(window, "dispatchEvent"); + + global.fetch = vi.fn().mockResolvedValue({ + ok: false, + status: 403, + text: vi.fn().mockResolvedValue("Forbidden"), + }); + + await expect(get("/test")).rejects.toBeInstanceOf(ApiError); + expect(dispatchSpy).toHaveBeenCalledTimes(1); + const call = dispatchSpy.mock.calls[0]; + expect(call).toBeDefined(); + const event = call?.[0] as Event; + expect(event.type).toBe(SESSION_EXPIRED_EVENT); + }); + + it("does not treat non-auth errors as auth errors", () => { + const error = new ApiError(500, "Server error"); + expect(isAuthError(error)).toBe(false); + }); + + it("recognizes 401 and 403 ApiError as auth errors", () => { + expect(isAuthError(new ApiError(401, "Unauthorized"))).toBe(true); + expect(isAuthError(new ApiError(403, "Forbidden"))).toBe(true); + }); +}); diff --git a/frontend/src/api/client.ts b/frontend/src/api/client.ts index 8e7a49d..f3a0783 100644 --- a/frontend/src/api/client.ts +++ b/frontend/src/api/client.ts @@ -39,6 +39,18 @@ export class ApiError extends Error { } } +/** Custom event name emitted when the backend signals an expired or invalid session. */ +export const SESSION_EXPIRED_EVENT = "bangui:session-expired"; + +/** + * Returns `true` when the error represents an expired or unauthorized session. + * + * @param err - The error returned from the API client. + */ +export function isAuthError(err: unknown): err is ApiError { + return err instanceof ApiError && (err.status === 401 || err.status === 403); +} + // --------------------------------------------------------------------------- // Internal helpers // --------------------------------------------------------------------------- @@ -63,6 +75,13 @@ async function request(url: string, options: RequestInit = {}): Promise { if (!response.ok) { const body: string = await response.text(); + + if (response.status === 401 || response.status === 403) { + if (typeof window !== "undefined") { + window.dispatchEvent(new Event(SESSION_EXPIRED_EVENT)); + } + } + throw new ApiError(response.status, body); } diff --git a/frontend/src/providers/AuthProvider.tsx b/frontend/src/providers/AuthProvider.tsx index b6b7a88..fbc956e 100644 --- a/frontend/src/providers/AuthProvider.tsx +++ b/frontend/src/providers/AuthProvider.tsx @@ -10,10 +10,13 @@ import { createContext, useCallback, + useEffect, useMemo, useState, } from "react"; +import { useNavigate } from "react-router-dom"; import * as authApi from "../api/auth"; +import { SESSION_EXPIRED_EVENT } from "../api/client"; // --------------------------------------------------------------------------- // Types @@ -64,6 +67,21 @@ export function AuthProvider({ token: sessionStorage.getItem(SESSION_KEY), expiresAt: sessionStorage.getItem(SESSION_EXPIRES_KEY), })); + const navigate = useNavigate(); + + const handleSessionExpired = useCallback((): void => { + sessionStorage.removeItem(SESSION_KEY); + sessionStorage.removeItem(SESSION_EXPIRES_KEY); + setAuth({ token: null, expiresAt: null }); + navigate("/login", { replace: true }); + }, [navigate]); + + useEffect((): (() => void) => { + window.addEventListener(SESSION_EXPIRED_EVENT, handleSessionExpired); + return (): void => { + window.removeEventListener(SESSION_EXPIRED_EVENT, handleSessionExpired); + }; + }, [handleSessionExpired]); const isAuthenticated = useMemo(() => { if (!auth.token || !auth.expiresAt) return false; diff --git a/frontend/src/providers/__tests__/AuthProvider.test.tsx b/frontend/src/providers/__tests__/AuthProvider.test.tsx new file mode 100644 index 0000000..a6f2ff1 --- /dev/null +++ b/frontend/src/providers/__tests__/AuthProvider.test.tsx @@ -0,0 +1,44 @@ +import { describe, expect, it, vi, beforeEach } from "vitest"; +import { render, screen, waitFor } from "@testing-library/react"; +import { type ReactElement } from "react"; +import { MemoryRouter, Route, Routes, useLocation } from "react-router-dom"; +import { FluentProvider, webLightTheme } from "@fluentui/react-components"; +import { AuthProvider } from "../AuthProvider"; +import { SESSION_EXPIRED_EVENT } from "../../api/client"; + +function CurrentLocation(): ReactElement { + const location = useLocation(); + return
{location.pathname}
; +} + +describe("AuthProvider", () => { + beforeEach(() => { + sessionStorage.clear(); + vi.restoreAllMocks(); + }); + + it("clears auth state and redirects to /login when session-expired fires", async () => { + sessionStorage.setItem("bangui_token", "token"); + sessionStorage.setItem("bangui_expires_at", new Date(Date.now() + 10000).toISOString()); + + render( + + + + + } /> + + + + , + ); + + window.dispatchEvent(new Event(SESSION_EXPIRED_EVENT)); + + await waitFor(() => { + expect(screen.getByTestId("location")).toHaveTextContent("/login"); + }); + expect(sessionStorage.getItem("bangui_token")).toBeNull(); + expect(sessionStorage.getItem("bangui_expires_at")).toBeNull(); + }); +}); diff --git a/frontend/src/utils/__tests__/fetchError.test.ts b/frontend/src/utils/__tests__/fetchError.test.ts new file mode 100644 index 0000000..395f9bc --- /dev/null +++ b/frontend/src/utils/__tests__/fetchError.test.ts @@ -0,0 +1,23 @@ +import { describe, expect, it, vi } from "vitest"; +import { ApiError } from "../../api/client"; +import { handleFetchError } from "../fetchError"; + +describe("utils/fetchError", () => { + it("ignores AbortError errors", () => { + const setError = vi.fn(); + handleFetchError(new DOMException("Aborted", "AbortError"), setError, "fallback"); + expect(setError).not.toHaveBeenCalled(); + }); + + it("ignores auth errors", () => { + const setError = vi.fn(); + handleFetchError(new ApiError(401, "Unauthorized"), setError, "fallback"); + expect(setError).not.toHaveBeenCalled(); + }); + + it("sets fallback for normal errors", () => { + const setError = vi.fn(); + handleFetchError(new Error("Oops"), setError, "fallback"); + expect(setError).toHaveBeenCalledWith("Oops"); + }); +}); diff --git a/frontend/src/utils/fetchError.ts b/frontend/src/utils/fetchError.ts index 7cee108..093b9b0 100644 --- a/frontend/src/utils/fetchError.ts +++ b/frontend/src/utils/fetchError.ts @@ -1,3 +1,5 @@ +import { isAuthError } from "../api/client"; + /** * Normalize fetch error handling across hooks. */ @@ -10,5 +12,9 @@ export function handleFetchError( return; } + if (isAuthError(err)) { + return; + } + setError(err instanceof Error ? err.message : fallback); }