refactoring-backend #3

Merged
lukas.pupkalipinski merged 403 commits from refactoring-backend into main 2026-05-20 20:23:46 +02:00
8 changed files with 164 additions and 1 deletions
Showing only changes of commit cc8c71906f - Show all commits

View File

@@ -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.

View File

@@ -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.

View File

@@ -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);
});
});

View File

@@ -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<T>(url: string, options: RequestInit = {}): Promise<T> {
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);
}

View File

@@ -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<boolean>(() => {
if (!auth.token || !auth.expiresAt) return false;

View File

@@ -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 <div data-testid="location">{location.pathname}</div>;
}
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(
<FluentProvider theme={webLightTheme}>
<MemoryRouter initialEntries={["/private"]}>
<AuthProvider>
<Routes>
<Route path="*" element={<CurrentLocation />} />
</Routes>
</AuthProvider>
</MemoryRouter>
</FluentProvider>,
);
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();
});
});

View File

@@ -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");
});
});

View File

@@ -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);
}