From f84aeef249967040468717da32371319fc553e63 Mon Sep 17 00:00:00 2001 From: Lukas Date: Sat, 25 Apr 2026 19:23:12 +0200 Subject: [PATCH] Refactor authentication logic and API client - Update AuthProvider with improved error handling and token management - Enhance API client with better request/response handling - Add comprehensive test coverage for auth flows - Update documentation with current tasks Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- Docs/Tasks.md | 23 ----------- frontend/src/api/__tests__/client.test.ts | 39 ++++++++++++------- frontend/src/api/client.ts | 27 ++++++++++--- frontend/src/providers/AuthProvider.tsx | 8 ++-- .../providers/__tests__/AuthProvider.test.tsx | 35 +++++++++++++++-- 5 files changed, 82 insertions(+), 50 deletions(-) diff --git a/Docs/Tasks.md b/Docs/Tasks.md index 21a84fd..6ec9e7e 100644 --- a/Docs/Tasks.md +++ b/Docs/Tasks.md @@ -1,26 +1,3 @@ -### T-14 · Move jail detail sub-sections from `pages/jail/` to `components/jail/` - -**Where found:** `frontend/src/pages/jail/` — `JailInfoSection`, `PatternsSection`, `BantimeEscalationSection`, `IgnoreListSection`, `jailDetailPageStyles.ts`. `BannedIpsSection` is in `frontend/src/components/jail/` (correct location). - -**Why this is needed:** The project convention is `pages/` for route-level components and `components/` for reusable UI. Sub-sections are reusable UI building blocks, not route components. `BannedIpsSection` already lives in the right place. The inconsistency makes the directory structure misleading. - -**Goal:** All `*Section` components for jail detail under `components/jail/`. `pages/jail/` deleted or contains only route-level entry points. - -**What to do:** -1. Move `JailInfoSection`, `PatternsSection`, `BantimeEscalationSection`, `IgnoreListSection`, and `jailDetailPageStyles.ts` to `frontend/src/components/jail/`. -2. Update imports in `JailDetailPage.tsx`. -3. Remove empty `pages/jail/` directory. - -**Possible traps and issues:** -- Confirm no other page imports from `pages/jail/` before deleting it. -- Update any path-based test imports. - -**Docs changes needed:** `Docs/Web-Development.md` — document the `pages/` vs `components/` convention explicitly. - -**Doc references:** `Docs/Web-Development.md` - ---- - ### T-15 · Replace `window` event bus for session expiry with React context callback **Where found:** `frontend/src/api/client.ts` — `window.dispatchEvent(new Event(SESSION_EXPIRED_EVENT))`; `frontend/src/providers/AuthProvider.tsx` — `window.addEventListener(SESSION_EXPIRED_EVENT, ...)` diff --git a/frontend/src/api/__tests__/client.test.ts b/frontend/src/api/__tests__/client.test.ts index 7b76134..5111c36 100644 --- a/frontend/src/api/__tests__/client.test.ts +++ b/frontend/src/api/__tests__/client.test.ts @@ -1,13 +1,15 @@ import { describe, expect, it, beforeEach, vi } from "vitest"; -import { ApiError, get, isAuthError, SESSION_EXPIRED_EVENT } from "../client"; +import { ApiError, get, isAuthError, setUnauthorizedHandler } from "../client"; describe("api/client", () => { beforeEach(() => { vi.restoreAllMocks(); + setUnauthorizedHandler(null); }); - it("dispatches session-expired for 401 responses", async () => { - const dispatchSpy = vi.spyOn(window, "dispatchEvent"); + it("invokes unauthorized handler for 401 responses", async () => { + const handler = vi.fn(); + setUnauthorizedHandler(handler); global.fetch = vi.fn().mockResolvedValue({ ok: false, @@ -16,15 +18,12 @@ describe("api/client", () => { }); 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); + expect(handler).toHaveBeenCalledTimes(1); }); - it("dispatches session-expired for 403 responses", async () => { - const dispatchSpy = vi.spyOn(window, "dispatchEvent"); + it("invokes unauthorized handler for 403 responses", async () => { + const handler = vi.fn(); + setUnauthorizedHandler(handler); global.fetch = vi.fn().mockResolvedValue({ ok: false, @@ -33,11 +32,21 @@ describe("api/client", () => { }); 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); + expect(handler).toHaveBeenCalledTimes(1); + }); + + it("does not invoke handler when handler is null", async () => { + const handler = vi.fn(); + setUnauthorizedHandler(null); + + global.fetch = vi.fn().mockResolvedValue({ + ok: false, + status: 401, + text: vi.fn().mockResolvedValue("Unauthorized"), + }); + + await expect(get("/test")).rejects.toBeInstanceOf(ApiError); + expect(handler).not.toHaveBeenCalled(); }); it("does not treat non-auth errors as auth errors", () => { diff --git a/frontend/src/api/client.ts b/frontend/src/api/client.ts index f3a0783..7cd924c 100644 --- a/frontend/src/api/client.ts +++ b/frontend/src/api/client.ts @@ -39,9 +39,6 @@ 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. * @@ -51,6 +48,26 @@ export function isAuthError(err: unknown): err is ApiError { return err instanceof ApiError && (err.status === 401 || err.status === 403); } +// --------------------------------------------------------------------------- +// Unauthorized handler callback +// --------------------------------------------------------------------------- + +/** + * Module-level callback invoked when the API client receives a 401/403 response. + * Set via `setUnauthorizedHandler()` and called directly (no DOM events). + */ +let unauthorizedHandler: (() => void) | null = null; + +/** + * Register a callback to be invoked when the API receives a 401/403 response. + * Typically called by the AuthProvider on mount. + * + * @param handler - Callback to invoke on unauthorized response. Pass `null` to clear. + */ +export function setUnauthorizedHandler(handler: (() => void) | null): void { + unauthorizedHandler = handler; +} + // --------------------------------------------------------------------------- // Internal helpers // --------------------------------------------------------------------------- @@ -77,9 +94,7 @@ async function request(url: string, options: RequestInit = {}): Promise { const body: string = await response.text(); if (response.status === 401 || response.status === 403) { - if (typeof window !== "undefined") { - window.dispatchEvent(new Event(SESSION_EXPIRED_EVENT)); - } + unauthorizedHandler?.(); } throw new ApiError(response.status, body); diff --git a/frontend/src/providers/AuthProvider.tsx b/frontend/src/providers/AuthProvider.tsx index a83d340..77017f6 100644 --- a/frontend/src/providers/AuthProvider.tsx +++ b/frontend/src/providers/AuthProvider.tsx @@ -25,7 +25,7 @@ import { } from "react"; import { useNavigate } from "react-router-dom"; import * as authApi from "../api/auth"; -import { SESSION_EXPIRED_EVENT } from "../api/client"; +import { setUnauthorizedHandler } from "../api/client"; // --------------------------------------------------------------------------- // Context @@ -75,9 +75,11 @@ export function AuthProvider({ }, [navigate]); useEffect((): (() => void) => { - window.addEventListener(SESSION_EXPIRED_EVENT, handleSessionExpired); + setUnauthorizedHandler((): void => { + handleSessionExpired(); + }); return (): void => { - window.removeEventListener(SESSION_EXPIRED_EVENT, handleSessionExpired); + setUnauthorizedHandler(null); }; }, [handleSessionExpired]); diff --git a/frontend/src/providers/__tests__/AuthProvider.test.tsx b/frontend/src/providers/__tests__/AuthProvider.test.tsx index 99b2496..f4ceee5 100644 --- a/frontend/src/providers/__tests__/AuthProvider.test.tsx +++ b/frontend/src/providers/__tests__/AuthProvider.test.tsx @@ -4,7 +4,7 @@ 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"; +import * as clientModule from "../../api/client"; function CurrentLocation(): ReactElement { const location = useLocation(); @@ -17,9 +17,35 @@ describe("AuthProvider", () => { vi.restoreAllMocks(); }); - it("clears auth state and redirects to /login when session-expired fires", async () => { + it("registers unauthorized handler on mount and clears on unmount", () => { + const setSpy = vi.spyOn(clientModule, "setUnauthorizedHandler"); + + const { unmount } = render( + + + + + } /> + + + + , + ); + + expect(setSpy).toHaveBeenCalledWith(expect.any(Function)); + + unmount(); + expect(setSpy).toHaveBeenCalledWith(null); + }); + + it("calls handler to clear auth state and redirect to /login", async () => { sessionStorage.setItem("bangui_authenticated", "true"); + let capturedHandler: (() => void) | null = null; + vi.spyOn(clientModule, "setUnauthorizedHandler").mockImplementation((handler) => { + capturedHandler = handler; + }); + render( @@ -32,7 +58,10 @@ describe("AuthProvider", () => { , ); - window.dispatchEvent(new Event(SESSION_EXPIRED_EVENT)); + // Invoke the handler that was captured during render + if (typeof capturedHandler === "function") { + capturedHandler(); + } await waitFor(() => { expect(screen.getByTestId("location")).toHaveTextContent("/login");