From 476148c4f00987b17016408805c70405b187e7f6 Mon Sep 17 00:00:00 2001 From: monte Date: Wed, 22 Jul 2026 12:39:55 +0200 Subject: [PATCH 1/2] feat(frontend): add banned-IPs admin UI Surfaces the banned-IP data from the security router: a dashboard StatCard, a dedicated /banned-ips page (table + client-side pagination and search), and manual unban action, admin-gated. --- src/frontend/src/app/router.tsx | 5 + src/frontend/src/app/routes.test.ts | 1 + src/frontend/src/app/routes.ts | 1 + src/frontend/src/components/layout/NavBar.tsx | 1 + .../src/features/banned-ips/UnbanIpDialog.tsx | 70 +++++++ .../src/features/banned-ips/api.test.ts | 55 ++++++ src/frontend/src/features/banned-ips/api.ts | 14 ++ src/frontend/src/features/banned-ips/types.ts | 19 ++ .../src/features/banned-ips/use-banned-ips.ts | 98 ++++++++++ .../features/dashboard/use-dashboard-stats.ts | 12 +- .../pages/banned-ips/BannedIpsPage.test.tsx | 145 +++++++++++++++ .../src/pages/banned-ips/BannedIpsPage.tsx | 175 ++++++++++++++++++ .../pages/dashboard/DashboardPage.test.tsx | 22 ++- .../src/pages/dashboard/DashboardPage.tsx | 13 +- 14 files changed, 627 insertions(+), 4 deletions(-) create mode 100644 src/frontend/src/features/banned-ips/UnbanIpDialog.tsx create mode 100644 src/frontend/src/features/banned-ips/api.test.ts create mode 100644 src/frontend/src/features/banned-ips/api.ts create mode 100644 src/frontend/src/features/banned-ips/types.ts create mode 100644 src/frontend/src/features/banned-ips/use-banned-ips.ts create mode 100644 src/frontend/src/pages/banned-ips/BannedIpsPage.test.tsx create mode 100644 src/frontend/src/pages/banned-ips/BannedIpsPage.tsx diff --git a/src/frontend/src/app/router.tsx b/src/frontend/src/app/router.tsx index dc0927b..74b74d0 100644 --- a/src/frontend/src/app/router.tsx +++ b/src/frontend/src/app/router.tsx @@ -3,6 +3,7 @@ import { createBrowserRouter, Navigate } from "react-router-dom"; import { appRoutes } from "@/app/routes"; import { ProtectedRoute, PublicOnlyRoute } from "@/features/auth/protected-route"; import { AppLayout } from "@/layouts/AppLayout"; +import { BannedIpsPage } from "@/pages/banned-ips/BannedIpsPage"; import { DashboardPage } from "@/pages/dashboard/DashboardPage"; import { ForbiddenPage } from "@/pages/forbidden/ForbiddenPage"; import { LoginPage } from "@/pages/login/LoginPage"; @@ -62,6 +63,10 @@ export const router = createBrowserRouter([ path: appRoutes.logs, element: , }, + { + path: appRoutes.bannedIps, + element: , + }, ], }, ], diff --git a/src/frontend/src/app/routes.test.ts b/src/frontend/src/app/routes.test.ts index 8f19eab..91a2386 100644 --- a/src/frontend/src/app/routes.test.ts +++ b/src/frontend/src/app/routes.test.ts @@ -8,6 +8,7 @@ describe("appRoutes", () => { expect(appRoutes.dashboard).toBe("/dashboard"); expect(appRoutes.vhosts).toBe("/vhosts"); expect(appRoutes.policies).toBe("/policies"); + expect(appRoutes.bannedIps).toBe("/banned-ips"); }); it("builds vhost detail paths", () => { diff --git a/src/frontend/src/app/routes.ts b/src/frontend/src/app/routes.ts index 7af81d8..f81e944 100644 --- a/src/frontend/src/app/routes.ts +++ b/src/frontend/src/app/routes.ts @@ -6,6 +6,7 @@ export const appRoutes = { vhosts: "/vhosts", policies: "/policies", logs: "/logs", + bannedIps: "/banned-ips", } as const; export function getVHostDetailPath(vhostId: string | number) { diff --git a/src/frontend/src/components/layout/NavBar.tsx b/src/frontend/src/components/layout/NavBar.tsx index b8f12c0..4c9c373 100644 --- a/src/frontend/src/components/layout/NavBar.tsx +++ b/src/frontend/src/components/layout/NavBar.tsx @@ -17,6 +17,7 @@ const navigation = [ { to: appRoutes.vhosts, label: "VHosts" }, { to: appRoutes.policies, label: "Policies" }, { to: appRoutes.logs, label: "Logs" }, + { to: appRoutes.bannedIps, label: "Banned IPs" }, ]; function navLinkClass(isActive: boolean, pill = false) { diff --git a/src/frontend/src/features/banned-ips/UnbanIpDialog.tsx b/src/frontend/src/features/banned-ips/UnbanIpDialog.tsx new file mode 100644 index 0000000..e128ec5 --- /dev/null +++ b/src/frontend/src/features/banned-ips/UnbanIpDialog.tsx @@ -0,0 +1,70 @@ +import { useState } from "react"; + +import { Modal } from "@/components/shared/Modal"; +import { Alert } from "@/components/ui/alert"; +import { Button } from "@/components/ui/button"; +import { useAuth } from "@/hooks/use-auth"; +import { ApiError } from "@/lib/api-client"; + +import { unbanIp } from "./api"; +import type { BannedIp } from "./types"; + +type UnbanIpDialogProps = { + bannedIp: BannedIp; + onSuccess: () => void; + onClose: () => void; +}; + +export function UnbanIpDialog({ bannedIp, onSuccess, onClose }: UnbanIpDialogProps) { + const { accessToken } = useAuth(); + const [submitting, setSubmitting] = useState(false); + const [serverError, setServerError] = useState(null); + + async function handleUnban() { + if (!accessToken) return; + setSubmitting(true); + setServerError(null); + try { + await unbanIp(accessToken, bannedIp.ip); + onSuccess(); + } catch (err) { + setServerError( + err instanceof ApiError ? err.detail : "An unexpected error occurred", + ); + setSubmitting(false); + } + } + + return ( + + + + + } + > + {serverError && ( + + {serverError} + + )} +

+ Are you sure you want to unban{" "} + {bannedIp.ip}{" "} + on {bannedIp.domain}? +

+ + ); +} diff --git a/src/frontend/src/features/banned-ips/api.test.ts b/src/frontend/src/features/banned-ips/api.test.ts new file mode 100644 index 0000000..e1f32b6 --- /dev/null +++ b/src/frontend/src/features/banned-ips/api.test.ts @@ -0,0 +1,55 @@ +import { afterEach, describe, expect, it, vi } from "vitest"; + +import { listBannedIps, unbanIp } from "./api"; + +function jsonResponse(body: unknown) { + return new Response(JSON.stringify(body), { + status: 200, + headers: { "content-type": "application/json" }, + }); +} + +describe("banned-ips API", () => { + afterEach(() => { + vi.restoreAllMocks(); + vi.unstubAllEnvs(); + }); + + it("fetches the banned-ips list", async () => { + vi.stubEnv("VITE_API_BASE_URL", ""); + const fetchMock = vi.spyOn(globalThis, "fetch").mockResolvedValueOnce( + jsonResponse({ + items: [ + { + ip: "203.0.113.10", + vhost_id: 1, + domain: "app.example.com", + gpc0: 12, + ban_threshold: 10, + banned: true, + expires_in_seconds: 120, + }, + ], + total: 1, + }), + ); + + const response = await listBannedIps("token"); + + expect(response.items[0]?.ip).toBe("203.0.113.10"); + expect(fetchMock.mock.calls[0]?.[0]).toBe("/api/v1/security/banned-ips"); + }); + + it("sends a DELETE request with the IP URI-encoded in the path", async () => { + vi.stubEnv("VITE_API_BASE_URL", ""); + const fetchMock = vi + .spyOn(globalThis, "fetch") + .mockResolvedValueOnce(jsonResponse({ ip: "203.0.113.10", cleared: 1 })); + + const response = await unbanIp("token", "203.0.113.10"); + + expect(response).toEqual({ ip: "203.0.113.10", cleared: 1 }); + expect(fetchMock.mock.calls[0]?.[0]).toBe("/api/v1/security/banned-ips/203.0.113.10"); + expect(fetchMock.mock.calls[0]?.[1]?.method).toBe("DELETE"); + }); +}); diff --git a/src/frontend/src/features/banned-ips/api.ts b/src/frontend/src/features/banned-ips/api.ts new file mode 100644 index 0000000..8cb0410 --- /dev/null +++ b/src/frontend/src/features/banned-ips/api.ts @@ -0,0 +1,14 @@ +import { apiRequest } from "@/lib/api-client"; + +import type { BannedIpListResponse, UnbanResponse } from "./types"; + +export function listBannedIps(token: string, signal?: AbortSignal) { + return apiRequest("/security/banned-ips", { token, signal }); +} + +export function unbanIp(token: string, ip: string) { + return apiRequest(`/security/banned-ips/${encodeURIComponent(ip)}`, { + method: "DELETE", + token, + }); +} diff --git a/src/frontend/src/features/banned-ips/types.ts b/src/frontend/src/features/banned-ips/types.ts new file mode 100644 index 0000000..5a84c07 --- /dev/null +++ b/src/frontend/src/features/banned-ips/types.ts @@ -0,0 +1,19 @@ +export type BannedIp = { + ip: string; + vhost_id: number; + domain: string; + gpc0: number; + ban_threshold: number; + banned: boolean; + expires_in_seconds: number; +}; + +export type BannedIpListResponse = { + items: BannedIp[]; + total: number; +}; + +export type UnbanResponse = { + ip: string; + cleared: number; +}; diff --git a/src/frontend/src/features/banned-ips/use-banned-ips.ts b/src/frontend/src/features/banned-ips/use-banned-ips.ts new file mode 100644 index 0000000..7eab9e5 --- /dev/null +++ b/src/frontend/src/features/banned-ips/use-banned-ips.ts @@ -0,0 +1,98 @@ +import { useCallback, useEffect, useRef, useState } from "react"; + +import { useAuth } from "@/hooks/use-auth"; + +import { listBannedIps } from "./api"; +import type { BannedIp } from "./types"; + +const PAGE_SIZE = 50; + +type BannedIpsState = { + items: BannedIp[]; + total: number; + page: number; + pageSize: number; + searchQuery: string; + isLoading: boolean; + error: string | null; + setPage: (page: number) => void; + setSearchQuery: (query: string) => void; + refresh: () => void; +}; + +export function useBannedIps(): BannedIpsState { + const { accessToken } = useAuth(); + const [bannedIps, setBannedIps] = useState([]); + const [page, setPageState] = useState(1); + const [searchQuery, setSearchQueryState] = useState(""); + const [isLoading, setIsLoading] = useState(true); + const [error, setError] = useState(null); + const refreshCountRef = useRef(0); + + const fetch = useCallback(() => { + if (!accessToken) return; + + const controller = new AbortController(); + const generation = ++refreshCountRef.current; + + setIsLoading(true); + setError(null); + + listBannedIps(accessToken, controller.signal) + .then((response) => { + if (generation !== refreshCountRef.current) return; + setBannedIps(response.items.filter((item) => item.banned)); + setIsLoading(false); + }) + .catch((err: unknown) => { + if (generation !== refreshCountRef.current) return; + if (err instanceof Error && err.name === "AbortError") return; + setError(err instanceof Error ? err.message : "Failed to load banned IPs"); + setIsLoading(false); + }); + + return () => { + controller.abort(); + }; + }, [accessToken]); + + useEffect(() => { + const cleanup = fetch(); + return cleanup; + }, [fetch]); + + const refresh = useCallback(() => { + fetch(); + }, [fetch]); + + const setPage = useCallback((nextPage: number) => { + setPageState(nextPage); + }, []); + + const setSearchQuery = useCallback((query: string) => { + setPageState(1); + setSearchQueryState(query); + }, []); + + const trimmedQuery = searchQuery.trim().toLowerCase(); + const filtered = trimmedQuery + ? bannedIps.filter((item) => item.ip.toLowerCase().includes(trimmedQuery)) + : bannedIps; + + const total = filtered.length; + const start = (page - 1) * PAGE_SIZE; + const items = filtered.slice(start, start + PAGE_SIZE); + + return { + items, + total, + page, + pageSize: PAGE_SIZE, + searchQuery, + isLoading, + error, + setPage, + setSearchQuery, + refresh, + }; +} diff --git a/src/frontend/src/features/dashboard/use-dashboard-stats.ts b/src/frontend/src/features/dashboard/use-dashboard-stats.ts index 7e9fab7..f4921f7 100644 --- a/src/frontend/src/features/dashboard/use-dashboard-stats.ts +++ b/src/frontend/src/features/dashboard/use-dashboard-stats.ts @@ -1,5 +1,6 @@ import { useCallback, useEffect, useRef, useState } from "react"; +import { listBannedIps } from "@/features/banned-ips/api"; import { fetchLogTotal } from "@/features/logs/api"; import type { LogAction, LogSeverity } from "@/features/logs/types"; import { listAllVHosts } from "@/features/vhosts/api"; @@ -17,6 +18,7 @@ type DashboardStatsState = { policies: StatValue; blocked: StatValue; alerts: StatValue; + bannedIps: StatValue; refresh: () => void; }; @@ -40,6 +42,7 @@ export function useDashboardStats(): DashboardStatsState { const [policies, setPolicies] = useState(LOADING); const [blocked, setBlocked] = useState(LOADING); const [alerts, setAlerts] = useState(LOADING); + const [bannedIps, setBannedIps] = useState(LOADING); const generationRef = useRef(0); const load = useCallback(() => { @@ -52,6 +55,7 @@ export function useDashboardStats(): DashboardStatsState { setPolicies(LOADING); setBlocked(LOADING); setAlerts(LOADING); + setBannedIps(LOADING); // Each card resolves independently so fast cards aren't blocked by slow ones. // Aborted fetches are silently dropped (not surfaced as errors). @@ -82,6 +86,12 @@ export function useDashboardStats(): DashboardStatsState { .then((total) => guard(setAlerts)(ok(total))) .catch((e) => { if (!isAbortError(e)) guard(setAlerts)(failed()); }); + listBannedIps(accessToken, controller.signal) + .then((response) => + guard(setBannedIps)(ok(response.items.filter((item) => item.banned).length)), + ) + .catch((e) => { if (!isAbortError(e)) guard(setBannedIps)(failed()); }); + return () => controller.abort(); }, [accessToken]); @@ -92,5 +102,5 @@ export function useDashboardStats(): DashboardStatsState { const refresh = useCallback(() => { load(); }, [load]); - return { vhosts, policies, blocked, alerts, refresh }; + return { vhosts, policies, blocked, alerts, bannedIps, refresh }; } diff --git a/src/frontend/src/pages/banned-ips/BannedIpsPage.test.tsx b/src/frontend/src/pages/banned-ips/BannedIpsPage.test.tsx new file mode 100644 index 0000000..fea6c57 --- /dev/null +++ b/src/frontend/src/pages/banned-ips/BannedIpsPage.test.tsx @@ -0,0 +1,145 @@ +import { render, screen, waitFor, within } from "@testing-library/react"; +import userEvent from "@testing-library/user-event"; +import { describe, expect, it, vi } from "vitest"; + +import { AuthContext } from "@/features/auth/auth-context.shared"; +import type { AuthContextValue } from "@/features/auth/auth-context.types"; +import * as bannedIpsApi from "@/features/banned-ips/api"; + +import { BannedIpsPage } from "./BannedIpsPage"; + +vi.mock("@/features/banned-ips/api"); + +function makeAuthContext(overrides: Partial = {}): AuthContextValue { + return { + user: null, + role: "admin", + accessToken: "test-token", + isAuthenticated: true, + isLoading: false, + loginError: null, + hasRole: vi.fn().mockReturnValue(true), + signIn: vi.fn(), + signOut: vi.fn(), + refreshCurrentUser: vi.fn(), + ...overrides, + }; +} + +const mockListResponse = { + items: [ + { + ip: "203.0.113.10", + vhost_id: 1, + domain: "app.example.com", + gpc0: 12, + ban_threshold: 10, + banned: true, + expires_in_seconds: 90, + }, + { + ip: "203.0.113.20", + vhost_id: 2, + domain: "api.example.com", + gpc0: 4, + ban_threshold: 10, + banned: false, + expires_in_seconds: 30, + }, + ], + total: 2, +}; + +const emptyListResponse = { items: [], total: 0 }; + +function renderPage(authOverrides: Partial = {}) { + return render( + + + , + ); +} + +describe("BannedIpsPage", () => { + it("shows loading state initially", () => { + vi.mocked(bannedIpsApi.listBannedIps).mockReturnValue(new Promise(() => undefined)); + + renderPage(); + expect(screen.getByText(/loading banned ips/i)).toBeInTheDocument(); + }); + + it("renders only actively banned IPs, excluding merely-tracked entries", async () => { + vi.mocked(bannedIpsApi.listBannedIps).mockResolvedValue(mockListResponse); + + renderPage(); + + await waitFor(() => expect(screen.getByText("203.0.113.10")).toBeInTheDocument()); + expect(screen.getByText("app.example.com")).toBeInTheDocument(); + expect(screen.getByText("12 / 10")).toBeInTheDocument(); + expect(screen.queryByText("203.0.113.20")).not.toBeInTheDocument(); + }); + + it("shows error state and retry button on failure", async () => { + vi.mocked(bannedIpsApi.listBannedIps).mockRejectedValue(new Error("Network error")); + + renderPage(); + + await waitFor(() => + expect(screen.getByText(/failed to load banned ips/i)).toBeInTheDocument(), + ); + expect(screen.getByRole("button", { name: /retry/i })).toBeInTheDocument(); + }); + + it("shows empty state when there are no banned IPs", async () => { + vi.mocked(bannedIpsApi.listBannedIps).mockResolvedValue(emptyListResponse); + + renderPage(); + + await waitFor(() => expect(screen.getByText(/no banned ips/i)).toBeInTheDocument()); + }); + + it("admin sees the Unban action, viewer does not", async () => { + vi.mocked(bannedIpsApi.listBannedIps).mockResolvedValue(mockListResponse); + + renderPage({ hasRole: vi.fn().mockReturnValue(false) }); + await waitFor(() => expect(screen.getByText("203.0.113.10")).toBeInTheDocument()); + expect(screen.queryByRole("button", { name: /unban/i })).not.toBeInTheDocument(); + }); + + it("filters the list client-side by IP search", async () => { + vi.mocked(bannedIpsApi.listBannedIps).mockResolvedValue(mockListResponse); + + renderPage(); + + await waitFor(() => expect(screen.getByText("203.0.113.10")).toBeInTheDocument()); + await userEvent.type(screen.getByLabelText(/search banned ips/i), "999.999"); + + await waitFor(() => + expect(screen.queryByText("203.0.113.10")).not.toBeInTheDocument(), + ); + expect(screen.getByText(/no banned ips/i)).toBeInTheDocument(); + }); + + it("unbanning an IP calls the API and refreshes the list", async () => { + vi.mocked(bannedIpsApi.listBannedIps).mockResolvedValue(mockListResponse); + vi.mocked(bannedIpsApi.unbanIp).mockResolvedValue({ ip: "203.0.113.10", cleared: 1 }); + + renderPage(); + + await waitFor(() => expect(screen.getByText("203.0.113.10")).toBeInTheDocument()); + const callsBeforeUnban = vi.mocked(bannedIpsApi.listBannedIps).mock.calls.length; + await userEvent.click(screen.getByRole("button", { name: /unban/i })); + + const dialog = screen.getByRole("dialog"); + await userEvent.click(within(dialog).getByRole("button", { name: /^unban$/i })); + + await waitFor(() => + expect(bannedIpsApi.unbanIp).toHaveBeenCalledWith("test-token", "203.0.113.10"), + ); + await waitFor(() => + expect(vi.mocked(bannedIpsApi.listBannedIps).mock.calls.length).toBe( + callsBeforeUnban + 1, + ), + ); + }); +}); diff --git a/src/frontend/src/pages/banned-ips/BannedIpsPage.tsx b/src/frontend/src/pages/banned-ips/BannedIpsPage.tsx new file mode 100644 index 0000000..69bf968 --- /dev/null +++ b/src/frontend/src/pages/banned-ips/BannedIpsPage.tsx @@ -0,0 +1,175 @@ +import { useState } from "react"; + +import type { DataTableColumn } from "@/components/shared/DataTable"; +import { DataTable } from "@/components/shared/DataTable"; +import { ErrorState } from "@/components/shared/ErrorState"; +import { LoadingState } from "@/components/shared/LoadingState"; +import { PageHeader } from "@/components/shared/PageHeader"; +import { SectionCard } from "@/components/shared/SectionCard"; +import { Button } from "@/components/ui/button"; +import { Input } from "@/components/ui/input"; +import { UnbanIpDialog } from "@/features/banned-ips/UnbanIpDialog"; +import { useBannedIps } from "@/features/banned-ips/use-banned-ips"; +import type { BannedIp } from "@/features/banned-ips/types"; +import { useAuth } from "@/hooks/use-auth"; + +function formatExpiresIn(seconds: number) { + if (seconds <= 0) return "Expiring"; + if (seconds < 60) return `${seconds}s`; + const minutes = Math.ceil(seconds / 60); + if (minutes < 60) return `${minutes}m`; + const hours = Math.ceil(minutes / 60); + return `${hours}h`; +} + +export function BannedIpsPage() { + const { hasRole } = useAuth(); + const { + items, + total, + page, + pageSize, + searchQuery, + isLoading, + error, + setPage, + setSearchQuery, + refresh, + } = useBannedIps(); + const [unbanTarget, setUnbanTarget] = useState(null); + const isAdmin = hasRole("admin"); + const totalPages = Math.max(1, Math.ceil(total / pageSize)); + + function closeAndRefresh() { + setUnbanTarget(null); + refresh(); + } + + const columns: DataTableColumn[] = [ + { + key: "ip", + header: "IP address", + cell: (row) => {row.ip}, + }, + { + key: "domain", + header: "Domain", + cell: (row) => row.domain, + }, + { + key: "violations", + header: "Violations", + cell: (row) => `${row.gpc0} / ${row.ban_threshold}`, + }, + { + key: "expires", + header: "Expires in", + cell: (row) => formatExpiresIn(row.expires_in_seconds), + }, + ...(isAdmin + ? [ + { + key: "actions", + header: "", + className: "w-px whitespace-nowrap", + cell: (row: BannedIp) => ( +
event.stopPropagation()} + > + +
+ ), + } satisfies DataTableColumn, + ] + : []), + ]; + + return ( +
+ + + setSearchQuery(event.target.value)} + /> + } + > + {isLoading ? ( + + ) : error ? ( + + Retry + + } + /> + ) : ( + <> + row.ip} + emptyTitle="No banned IPs" + emptyDescription="No source IPs are currently banned." + /> + + {total > 0 && ( +
+ + Page {page} of {totalPages} · {total} banned IPs + +
+ + +
+
+ )} + + )} + + + {unbanTarget && ( + setUnbanTarget(null)} + /> + )} +
+ ); +} diff --git a/src/frontend/src/pages/dashboard/DashboardPage.test.tsx b/src/frontend/src/pages/dashboard/DashboardPage.test.tsx index 1a31b1d..4ff2418 100644 --- a/src/frontend/src/pages/dashboard/DashboardPage.test.tsx +++ b/src/frontend/src/pages/dashboard/DashboardPage.test.tsx @@ -3,12 +3,14 @@ import { describe, expect, it, vi } from "vitest"; import { AuthContext } from "@/features/auth/auth-context.shared"; import type { AuthContextValue } from "@/features/auth/auth-context.types"; +import * as bannedIpsApi from "@/features/banned-ips/api"; import * as logsApi from "@/features/logs/api"; import * as policiesApi from "@/features/policies/api"; import * as vhostsApi from "@/features/vhosts/api"; import { DashboardPage } from "./DashboardPage"; +vi.mock("@/features/banned-ips/api"); vi.mock("@/features/logs/api"); vi.mock("@/features/policies/api"); vi.mock("@/features/vhosts/api"); @@ -48,6 +50,16 @@ const mockPolicies = [ { id: 10, is_active: true }, { id: 11, is_active: false }, ]; +// 3 banned entries, plus one merely-tracked (not yet banned) that must be excluded. +const mockBannedIpsResponse = { + items: [ + { ip: "203.0.113.1", banned: true }, + { ip: "203.0.113.2", banned: true }, + { ip: "203.0.113.3", banned: true }, + { ip: "203.0.113.4", banned: false }, + ], + total: 4, +}; function renderPage() { return render( @@ -62,16 +74,18 @@ describe("DashboardPage StatCards", () => { vi.mocked(vhostsApi.listAllVHosts).mockReturnValue(new Promise(() => undefined)); vi.mocked(policiesApi.listAllPolicies).mockReturnValue(new Promise(() => undefined)); vi.mocked(logsApi.fetchLogTotal).mockReturnValue(new Promise(() => undefined)); + vi.mocked(bannedIpsApi.listBannedIps).mockReturnValue(new Promise(() => undefined)); renderPage(); - expect(screen.getAllByRole("status", { name: /loading/i })).toHaveLength(4); + expect(screen.getAllByRole("status", { name: /loading/i })).toHaveLength(5); }); it("renders real counts after data loads", async () => { vi.mocked(vhostsApi.listAllVHosts).mockResolvedValue(mockVHosts as never); vi.mocked(policiesApi.listAllPolicies).mockResolvedValue(mockPolicies as never); vi.mocked(logsApi.fetchLogTotal).mockResolvedValue(42); + vi.mocked(bannedIpsApi.listBannedIps).mockResolvedValue(mockBannedIpsResponse as never); renderPage(); @@ -81,12 +95,14 @@ describe("DashboardPage StatCards", () => { expect(screen.getByText("2")).toBeInTheDocument(); // vhosts expect(screen.getByText("1")).toBeInTheDocument(); // policies expect(screen.getAllByText("42")).toHaveLength(2); // blocked + alerts + expect(screen.getByText("3")).toBeInTheDocument(); // banned IPs (excludes not-yet-banned) }); it("shows — only for the card whose endpoint fails, real counts for others", async () => { vi.mocked(vhostsApi.listAllVHosts).mockResolvedValue(mockVHosts as never); vi.mocked(policiesApi.listAllPolicies).mockRejectedValue(new Error("Network error")); vi.mocked(logsApi.fetchLogTotal).mockResolvedValue(7); + vi.mocked(bannedIpsApi.listBannedIps).mockResolvedValue(mockBannedIpsResponse as never); renderPage(); @@ -96,18 +112,20 @@ describe("DashboardPage StatCards", () => { expect(screen.getByText("2")).toBeInTheDocument(); // vhosts resolved expect(screen.getByText("—")).toBeInTheDocument(); // policies failed expect(screen.getAllByText("7")).toHaveLength(2); // blocked + alerts resolved + expect(screen.getByText("3")).toBeInTheDocument(); // banned IPs resolved }); it("shows — for all cards when all endpoints fail", async () => { vi.mocked(vhostsApi.listAllVHosts).mockRejectedValue(new Error("down")); vi.mocked(policiesApi.listAllPolicies).mockRejectedValue(new Error("down")); vi.mocked(logsApi.fetchLogTotal).mockRejectedValue(new Error("down")); + vi.mocked(bannedIpsApi.listBannedIps).mockRejectedValue(new Error("down")); renderPage(); await waitFor(() => expect(screen.queryAllByRole("status", { name: /loading/i })).toHaveLength(0), ); - expect(screen.getAllByText("—")).toHaveLength(4); + expect(screen.getAllByText("—")).toHaveLength(5); }); }); diff --git a/src/frontend/src/pages/dashboard/DashboardPage.tsx b/src/frontend/src/pages/dashboard/DashboardPage.tsx index ee8ffbb..a2fdd37 100644 --- a/src/frontend/src/pages/dashboard/DashboardPage.tsx +++ b/src/frontend/src/pages/dashboard/DashboardPage.tsx @@ -34,7 +34,7 @@ export function DashboardPage() { } /> -
+
} isLoading={stats.alerts.isLoading} /> + } + isLoading={stats.bannedIps.isLoading} + />
From 89d44bb767a53922890ce1af42133a428072340c Mon Sep 17 00:00:00 2001 From: monte Date: Wed, 22 Jul 2026 13:25:30 +0200 Subject: [PATCH 2/2] fix(frontend): address banned-IPs UI review feedback - Gate the /banned-ips route, nav entry, and dashboard StatCard to admins (RequireAdmin guard), matching the admin-only backend endpoint. - Use a composite (vhost_id:ip) row key since an IP can be banned per vhost. - Reword the unban dialog to reflect that DELETE clears the IP across all virtual hosts, not just the row's domain. - Count unique source IPs in the StatCard; label the list footer as entries. - Clamp the client-side page so a shrinking list can't strand an empty page. - Add a distinct BanIcon and an IPv6 unban-encoding test. --- src/frontend/src/app/router.tsx | 11 ++++-- src/frontend/src/components/icons/index.tsx | 17 +++++++++ src/frontend/src/components/layout/NavBar.tsx | 18 +++++++--- .../features/auth/protected-route.test.tsx | 33 ++++++++++++++++- .../src/features/auth/protected-route.tsx | 10 ++++++ .../src/features/banned-ips/UnbanIpDialog.tsx | 4 ++- .../src/features/banned-ips/api.test.ts | 13 +++++++ .../src/features/banned-ips/use-banned-ips.ts | 8 +++-- .../features/dashboard/use-dashboard-stats.ts | 22 ++++++++---- .../pages/banned-ips/BannedIpsPage.test.tsx | 9 +++-- .../src/pages/banned-ips/BannedIpsPage.tsx | 5 +-- .../src/pages/dashboard/DashboardPage.tsx | 36 ++++++++++++------- 12 files changed, 149 insertions(+), 37 deletions(-) diff --git a/src/frontend/src/app/router.tsx b/src/frontend/src/app/router.tsx index 74b74d0..8361dd7 100644 --- a/src/frontend/src/app/router.tsx +++ b/src/frontend/src/app/router.tsx @@ -1,7 +1,7 @@ import { createBrowserRouter, Navigate } from "react-router-dom"; import { appRoutes } from "@/app/routes"; -import { ProtectedRoute, PublicOnlyRoute } from "@/features/auth/protected-route"; +import { ProtectedRoute, PublicOnlyRoute, RequireAdmin } from "@/features/auth/protected-route"; import { AppLayout } from "@/layouts/AppLayout"; import { BannedIpsPage } from "@/pages/banned-ips/BannedIpsPage"; import { DashboardPage } from "@/pages/dashboard/DashboardPage"; @@ -64,8 +64,13 @@ export const router = createBrowserRouter([ element: , }, { - path: appRoutes.bannedIps, - element: , + element: , + children: [ + { + path: appRoutes.bannedIps, + element: , + }, + ], }, ], }, diff --git a/src/frontend/src/components/icons/index.tsx b/src/frontend/src/components/icons/index.tsx index a670596..c981020 100644 --- a/src/frontend/src/components/icons/index.tsx +++ b/src/frontend/src/components/icons/index.tsx @@ -148,6 +148,23 @@ export function AlertTriangleIcon(props: IconProps) { ); } +export function BanIcon(props: IconProps) { + return ( + + + + + ); +} + export function PulseIcon(props: IconProps) { return ( !item.adminOnly || isAdmin); const runtimeStatus = useRuntimeStatus(); const { showNotice } = useApplyNotice(); const [mobileOpen, setMobileOpen] = useState(false); @@ -117,7 +125,7 @@ export function NavBar() {
- {navigation.map((item) => ( + {visibleNavigation.map((item) => ( = {}, @@ -73,4 +73,35 @@ describe("route guards", () => { expect(screen.getByText("Dashboard screen")).toBeInTheDocument(); }); + + function renderAdminGuard(hasAdminRole: boolean) { + render( + + + + }> + Admin-only screen
} /> + + Forbidden screen} /> + + + , + ); + } + + it("lets admins into admin-only routes", () => { + renderAdminGuard(true); + expect(screen.getByText("Admin-only screen")).toBeInTheDocument(); + }); + + it("redirects non-admins from admin-only routes to forbidden", () => { + renderAdminGuard(false); + expect(screen.getByText("Forbidden screen")).toBeInTheDocument(); + }); }); diff --git a/src/frontend/src/features/auth/protected-route.tsx b/src/frontend/src/features/auth/protected-route.tsx index c644550..f18bc81 100644 --- a/src/frontend/src/features/auth/protected-route.tsx +++ b/src/frontend/src/features/auth/protected-route.tsx @@ -20,6 +20,16 @@ export function ProtectedRoute() { return ; } +export function RequireAdmin() { + const { hasRole } = useAuth(); + + if (!hasRole("admin")) { + return ; + } + + return ; +} + export function PublicOnlyRoute() { const { isAuthenticated, isLoading } = useAuth(); diff --git a/src/frontend/src/features/banned-ips/UnbanIpDialog.tsx b/src/frontend/src/features/banned-ips/UnbanIpDialog.tsx index e128ec5..625015a 100644 --- a/src/frontend/src/features/banned-ips/UnbanIpDialog.tsx +++ b/src/frontend/src/features/banned-ips/UnbanIpDialog.tsx @@ -63,7 +63,9 @@ export function UnbanIpDialog({ bannedIp, onSuccess, onClose }: UnbanIpDialogPro

Are you sure you want to unban{" "} {bannedIp.ip}{" "} - on {bannedIp.domain}? + across all virtual hosts? + This clears the address from every active ban table, not just{" "} + {bannedIp.domain}.

); diff --git a/src/frontend/src/features/banned-ips/api.test.ts b/src/frontend/src/features/banned-ips/api.test.ts index e1f32b6..b7e4230 100644 --- a/src/frontend/src/features/banned-ips/api.test.ts +++ b/src/frontend/src/features/banned-ips/api.test.ts @@ -52,4 +52,17 @@ describe("banned-ips API", () => { expect(fetchMock.mock.calls[0]?.[0]).toBe("/api/v1/security/banned-ips/203.0.113.10"); expect(fetchMock.mock.calls[0]?.[1]?.method).toBe("DELETE"); }); + + it("URI-encodes IPv6 literals in the delete path", async () => { + vi.stubEnv("VITE_API_BASE_URL", ""); + const fetchMock = vi + .spyOn(globalThis, "fetch") + .mockResolvedValueOnce(jsonResponse({ ip: "2001:db8::1", cleared: 1 })); + + await unbanIp("token", "2001:db8::1"); + + expect(fetchMock.mock.calls[0]?.[0]).toBe( + "/api/v1/security/banned-ips/2001%3Adb8%3A%3A1", + ); + }); }); diff --git a/src/frontend/src/features/banned-ips/use-banned-ips.ts b/src/frontend/src/features/banned-ips/use-banned-ips.ts index 7eab9e5..17fb07e 100644 --- a/src/frontend/src/features/banned-ips/use-banned-ips.ts +++ b/src/frontend/src/features/banned-ips/use-banned-ips.ts @@ -80,13 +80,17 @@ export function useBannedIps(): BannedIpsState { : bannedIps; const total = filtered.length; - const start = (page - 1) * PAGE_SIZE; + const totalPages = Math.max(1, Math.ceil(total / PAGE_SIZE)); + // Clamp the page so a shrinking list (after unban/expiry) can't strand the + // user on an out-of-range page showing the empty state. + const clampedPage = Math.min(page, totalPages); + const start = (clampedPage - 1) * PAGE_SIZE; const items = filtered.slice(start, start + PAGE_SIZE); return { items, total, - page, + page: clampedPage, pageSize: PAGE_SIZE, searchQuery, isLoading, diff --git a/src/frontend/src/features/dashboard/use-dashboard-stats.ts b/src/frontend/src/features/dashboard/use-dashboard-stats.ts index f4921f7..4868fe3 100644 --- a/src/frontend/src/features/dashboard/use-dashboard-stats.ts +++ b/src/frontend/src/features/dashboard/use-dashboard-stats.ts @@ -37,7 +37,8 @@ function isAbortError(e: unknown): boolean { } export function useDashboardStats(): DashboardStatsState { - const { accessToken } = useAuth(); + const { accessToken, hasRole } = useAuth(); + const isAdmin = hasRole("admin"); const [vhosts, setVHosts] = useState(LOADING); const [policies, setPolicies] = useState(LOADING); const [blocked, setBlocked] = useState(LOADING); @@ -86,14 +87,21 @@ export function useDashboardStats(): DashboardStatsState { .then((total) => guard(setAlerts)(ok(total))) .catch((e) => { if (!isAbortError(e)) guard(setAlerts)(failed()); }); - listBannedIps(accessToken, controller.signal) - .then((response) => - guard(setBannedIps)(ok(response.items.filter((item) => item.banned).length)), - ) - .catch((e) => { if (!isAbortError(e)) guard(setBannedIps)(failed()); }); + // The banned-IPs endpoint is admin-only, so only admins fetch and see this + // card. Count unique source IPs (an IP can be banned on several vhosts). + if (isAdmin) { + listBannedIps(accessToken, controller.signal) + .then((response) => { + const uniqueBanned = new Set( + response.items.filter((item) => item.banned).map((item) => item.ip), + ); + guard(setBannedIps)(ok(uniqueBanned.size)); + }) + .catch((e) => { if (!isAbortError(e)) guard(setBannedIps)(failed()); }); + } return () => controller.abort(); - }, [accessToken]); + }, [accessToken, isAdmin]); useEffect(() => { const cleanup = load(); diff --git a/src/frontend/src/pages/banned-ips/BannedIpsPage.test.tsx b/src/frontend/src/pages/banned-ips/BannedIpsPage.test.tsx index fea6c57..f47147d 100644 --- a/src/frontend/src/pages/banned-ips/BannedIpsPage.test.tsx +++ b/src/frontend/src/pages/banned-ips/BannedIpsPage.test.tsx @@ -98,12 +98,15 @@ describe("BannedIpsPage", () => { await waitFor(() => expect(screen.getByText(/no banned ips/i)).toBeInTheDocument()); }); - it("admin sees the Unban action, viewer does not", async () => { + // Viewer access to this page is blocked at the route level (RequireAdmin), + // covered by protected-route.test.tsx. Here we only assert the admin sees the + // action column exposed once they reach the page. + it("admin sees the Unban action", async () => { vi.mocked(bannedIpsApi.listBannedIps).mockResolvedValue(mockListResponse); - renderPage({ hasRole: vi.fn().mockReturnValue(false) }); + renderPage(); await waitFor(() => expect(screen.getByText("203.0.113.10")).toBeInTheDocument()); - expect(screen.queryByRole("button", { name: /unban/i })).not.toBeInTheDocument(); + expect(screen.getByRole("button", { name: /unban/i })).toBeInTheDocument(); }); it("filters the list client-side by IP search", async () => { diff --git a/src/frontend/src/pages/banned-ips/BannedIpsPage.tsx b/src/frontend/src/pages/banned-ips/BannedIpsPage.tsx index 69bf968..d4ce15a 100644 --- a/src/frontend/src/pages/banned-ips/BannedIpsPage.tsx +++ b/src/frontend/src/pages/banned-ips/BannedIpsPage.tsx @@ -129,7 +129,7 @@ export function BannedIpsPage() { row.ip} + getRowKey={(row) => `${row.vhost_id}:${row.ip}`} emptyTitle="No banned IPs" emptyDescription="No source IPs are currently banned." /> @@ -137,7 +137,8 @@ export function BannedIpsPage() { {total > 0 && (
- Page {page} of {totalPages} · {total} banned IPs + Page {page} of {totalPages} · {total}{" "} + {total === 1 ? "ban entry" : "ban entries"}