From 876cb60b13610cdc089d9c864a3b8dcd182fdbe4 Mon Sep 17 00:00:00 2001 From: WhiteMuush Date: Tue, 4 Aug 2026 21:26:11 +0200 Subject: [PATCH 01/13] feat(rbac): show ghost placeholders while the access lists load The three lists on /access rendered an empty bordered box until their fetch came back, so the page looked finished and wrong for a beat, then jumped as rows appeared. Extract the idiom the dashboard route skeleton already uses (a pulsing muted block) into a Skeleton primitive plus a SkeletonRows list, sized like the real rows so nothing shifts when the data lands. Roles and People clear the flag once and never raise it again: the reloads that follow a mutation keep the list on screen rather than flashing placeholders over rows the user is reading. The audit trail is the exception and re-arms on every page change, because paging swaps the whole set and the placeholders are the feedback that the page turned. Its Prev/Next are disabled mid-fetch so a fast double click cannot outrun the response. --- src/components/rbac/AuditTrail.tsx | 38 +++++++++++++++++----- src/components/rbac/RolesManager.tsx | 21 ++++++++++-- src/components/rbac/UserRoleAssignment.tsx | 18 ++++++++-- src/components/ui/Skeleton.tsx | 23 +++++++++++++ 4 files changed, 86 insertions(+), 14 deletions(-) create mode 100644 src/components/ui/Skeleton.tsx diff --git a/src/components/rbac/AuditTrail.tsx b/src/components/rbac/AuditTrail.tsx index a51a218..6582324 100644 --- a/src/components/rbac/AuditTrail.tsx +++ b/src/components/rbac/AuditTrail.tsx @@ -1,6 +1,7 @@ "use client" import { useEffect, useState } from "react" +import { SkeletonRows } from "@/components/ui/Skeleton" type Entry = { id: string @@ -17,20 +18,32 @@ export function AuditTrail() { const [entries, setEntries] = useState([]) const [total, setTotal] = useState(0) const [skip, setSkip] = useState(0) + // Unlike the other two lists this one is re-armed on every page change, + // because paging swaps the whole set: showing placeholders is the feedback + // that the page actually turned. + const [loading, setLoading] = useState(true) useEffect(() => { void (async () => { - const res = await fetch(`/api/audit?take=${PAGE}&skip=${skip}`) - if (res.ok) { - const data = (await res.json()) as { entries: Entry[]; total: number } - setEntries(data.entries) - setTotal(data.total) + setLoading(true) + try { + const res = await fetch(`/api/audit?take=${PAGE}&skip=${skip}`) + if (res.ok) { + const data = (await res.json()) as { entries: Entry[]; total: number } + setEntries(data.entries) + setTotal(data.total) + } + } finally { + setLoading(false) } })() }, [skip]) return (
+ {loading ? ( + + ) : (
    {entries.map((e) => (
  • @@ -41,13 +54,22 @@ export function AuditTrail() {
    • ))}
+ )}
- {total} event(s) + {loading ? "Loading events..." : `${total} event(s)`}
- -
diff --git a/src/components/rbac/RolesManager.tsx b/src/components/rbac/RolesManager.tsx index 4110d48..c4a84e4 100644 --- a/src/components/rbac/RolesManager.tsx +++ b/src/components/rbac/RolesManager.tsx @@ -3,6 +3,7 @@ import { useEffect, useMemo, useState } from "react" import { PermissionEditor } from "./PermissionEditor" import { StepUpDialog } from "./StepUpDialog" +import { SkeletonRows } from "@/components/ui/Skeleton" type Role = { id: string @@ -27,10 +28,18 @@ export function RolesManager() { const [name, setName] = useState("") const [error, setError] = useState(null) const [stepUpRetry, setStepUpRetry] = useState void)>(null) + // Only ever cleared, never set back to true: the skeleton covers the first + // paint, while the reloads that follow a mutation keep the list on screen + // instead of flashing placeholders over data the user is already reading. + const [loading, setLoading] = useState(true) async function load() { - const res = await fetch("/api/roles") - if (res.ok) setRoles((await res.json()).roles) + try { + const res = await fetch("/api/roles") + if (res.ok) setRoles((await res.json()).roles) + } finally { + setLoading(false) + } } useEffect(() => { void load() @@ -126,6 +135,9 @@ export function RolesManager() {
+ {loading ? ( + + ) : (
    {pageRoles.map((r) => (
  • @@ -149,10 +161,13 @@ export function RolesManager() {
    • ))}
+ )}
- {filtered.length} role(s), page {page + 1} of {Math.max(1, Math.ceil(filtered.length / PAGE_SIZE))} + {loading + ? "Loading roles..." + : `${filtered.length} role(s), page ${page + 1} of ${Math.max(1, Math.ceil(filtered.length / PAGE_SIZE))}`}
-
diff --git a/src/components/rbac/RolesManager.tsx b/src/components/rbac/RolesManager.tsx index c4a84e4..4110d48 100644 --- a/src/components/rbac/RolesManager.tsx +++ b/src/components/rbac/RolesManager.tsx @@ -3,7 +3,6 @@ import { useEffect, useMemo, useState } from "react" import { PermissionEditor } from "./PermissionEditor" import { StepUpDialog } from "./StepUpDialog" -import { SkeletonRows } from "@/components/ui/Skeleton" type Role = { id: string @@ -28,18 +27,10 @@ export function RolesManager() { const [name, setName] = useState("") const [error, setError] = useState(null) const [stepUpRetry, setStepUpRetry] = useState void)>(null) - // Only ever cleared, never set back to true: the skeleton covers the first - // paint, while the reloads that follow a mutation keep the list on screen - // instead of flashing placeholders over data the user is already reading. - const [loading, setLoading] = useState(true) async function load() { - try { - const res = await fetch("/api/roles") - if (res.ok) setRoles((await res.json()).roles) - } finally { - setLoading(false) - } + const res = await fetch("/api/roles") + if (res.ok) setRoles((await res.json()).roles) } useEffect(() => { void load() @@ -135,9 +126,6 @@ export function RolesManager() {
- {loading ? ( - - ) : (
    {pageRoles.map((r) => (
  • @@ -161,13 +149,10 @@ export function RolesManager() {
    • ))}
- )}
- {loading - ? "Loading roles..." - : `${filtered.length} role(s), page ${page + 1} of ${Math.max(1, Math.ceil(filtered.length / PAGE_SIZE))}`} + {filtered.length} role(s), page {page + 1} of {Math.max(1, Math.ceil(filtered.length / PAGE_SIZE))}
@@ -323,7 +341,7 @@ export default function LoginPage() { className="w-full" size="lg" > - Sign in with a passkey + {pending === "passkey" ? : "Sign in with a passkey"} )} From 331272657efbf305bfe460bc17f42fd22cb672b4 Mon Sep 17 00:00:00 2001 From: WhiteMuush Date: Tue, 4 Aug 2026 21:33:20 +0200 Subject: [PATCH 04/13] feat(auth): show a transition screen when entering the workspace Scope the animation to the one moment that had no feedback: the stretch between credentials being accepted and the dashboard painting. Nothing covered it, because the pending state was released as soon as the auth call returned, so the button read "Sign in" again while the navigation was still running. A full-screen shield and spinner now replace the form once authentication has succeeded, on all four paths that navigate: password, passkey, TOTP and email code. The credential check itself keeps its plain label, and error paths hand control straight back since they stay on the page. Amends the previous commit, which put spinners on the buttons themselves. That animated the wrong moment. --- src/app/(auth)/login/page.tsx | 56 ++++++++++++++++++++++------------- 1 file changed, 36 insertions(+), 20 deletions(-) diff --git a/src/app/(auth)/login/page.tsx b/src/app/(auth)/login/page.tsx index 3ee463e..7c8924e 100644 --- a/src/app/(auth)/login/page.tsx +++ b/src/app/(auth)/login/page.tsx @@ -2,18 +2,27 @@ import { useState } from "react" import { useRouter } from "next/navigation" -import { Loader2 } from "lucide-react" +import { Loader2, ShieldCheck } from "lucide-react" import { signIn, twoFactor } from "@/lib/auth/client" import { Button } from "@/components/ui/button" -// Spinner plus label. The icon is aria-hidden so the button keeps a stable -// accessible name while it spins. -function Pending({ label }: { label: string }) { +// Covers the gap between "credentials accepted" and "dashboard painted". That +// stretch is the slowest part of signing in and used to show nothing at all: +// the button had already snapped back to its idle label. Rendered only once +// authentication has actually succeeded, never during the check itself. +function EnteringWorkspace() { return ( - - - {label} - +
+ +
+ + Opening your workspace... +
+
) } @@ -24,11 +33,19 @@ export default function LoginPage() { // own label so its accessible name never collides with "Sign in". const [pending, setPending] = useState(null) const loading = pending !== null + const [entering, setEntering] = useState(false) const [needsTotp, setNeedsTotp] = useState(false) const [emailMode, setEmailMode] = useState(false) const [otpSent, setOtpSent] = useState(false) const router = useRouter() + // Every successful path funnels through here so the transition screen is the + // single thing standing between authentication and the dashboard. + function enterWorkspace() { + setEntering(true) + router.push("/dashboard") + } + function chooseEmail() { setError(null) setEmailMode(true) @@ -52,10 +69,7 @@ export default function LoginPage() { setPending(null) return } - // Deliberately stays pending: the spinner has to survive until the - // dashboard paints, otherwise the button snaps back to its idle label - // while the navigation is still in flight. - router.push("/dashboard") + enterWorkspace() } catch { setError("Passkey sign-in failed or was cancelled") setPending(null) @@ -100,7 +114,7 @@ export default function LoginPage() { return } - router.push("/dashboard") + enterWorkspace() } async function handlePassword(e: React.FormEvent) { @@ -127,7 +141,7 @@ export default function LoginPage() { return } - router.push("/dashboard") + enterWorkspace() } async function handleTotp(e: React.FormEvent) { @@ -146,9 +160,11 @@ export default function LoginPage() { return } - router.push("/dashboard") + enterWorkspace() } + if (entering) return + return (
@@ -195,7 +211,7 @@ export default function LoginPage() { )}
@@ -341,7 +357,7 @@ export default function LoginPage() { className="w-full" size="lg" > - {pending === "passkey" ? : "Sign in with a passkey"} + Sign in with a passkey )} From b0c7d608e055a0eb685527d3a2edc5aa9ec9fe5e Mon Sep 17 00:00:00 2001 From: WhiteMuush Date: Tue, 4 Aug 2026 21:37:49 +0200 Subject: [PATCH 05/13] feat(dashboard): drop the skeleton shown between navigation entries Moving between Employees, Alerts, Reports and the rest flashed a placeholder grid before the real page. Routes are already prefetched, so the pause it covered is short, and a shape that briefly pretends to be content reads worse than a beat of nothing. Loading feedback now exists in exactly one place: entering the workspace after signing in, which is the only wait long enough to need it. Without a loading.tsx, Next keeps the current page on screen until the next one is ready rather than swapping in a fallback. --- src/app/(dashboard)/loading.tsx | 31 ------------------------------- 1 file changed, 31 deletions(-) delete mode 100644 src/app/(dashboard)/loading.tsx diff --git a/src/app/(dashboard)/loading.tsx b/src/app/(dashboard)/loading.tsx deleted file mode 100644 index 500922a..0000000 --- a/src/app/(dashboard)/loading.tsx +++ /dev/null @@ -1,31 +0,0 @@ -// Shown instantly while a dashboard page's server data loads, so navigation -// feels immediate. Routes are also prefetched (see RoutePrefetcher), so only -// the dynamic data fetch remains, and this skeleton covers it. -export default function DashboardLoading() { - return ( -
-
-
-
-
- -
- {Array.from({ length: 4 }).map((_, i) => ( -
-
-
-
- ))} -
- -
-
-
- {Array.from({ length: 6 }).map((_, i) => ( -
- ))} -
-
-
- ) -} From 1d057763f68c98bda32835bc6495ef29de9daa68 Mon Sep 17 00:00:00 2001 From: WhiteMuush Date: Tue, 4 Aug 2026 21:46:44 +0200 Subject: [PATCH 06/13] fix(rbac): give the access page its own scroll container Opening the permission editor made /access unusable on any viewport shorter than the content: the panel could not be closed and the page would not scroll. The dashboard shell is h-screen with overflow-hidden at every level, so each page owns its scrolling. Every other page opens with "h-full overflow-y-auto p-6" and puts its max-width on an inner wrapper. This one opened straight into "mx-auto max-w-4xl space-y-6 p-6", so the shell clipped it: measured at a 620px viewport, 2233px of content shown through 572px with no way to reach the rest. It also nested a second main inside the shell's own. The regression test walks out from the form and fails on any ancestor clipping content it is too short to show. Asserting the Cancel button's position instead would pass or fail on how many roles happen to be seeded, and Playwright can force a click on a clipped node where a user cannot. It runs at 620px because the editor does fit in the 720px default. --- e2e/rbac.spec.ts | 38 ++++++++++++++++++++++++ src/app/(dashboard)/access/page.tsx | 46 ++++++++++++++++------------- 2 files changed, 64 insertions(+), 20 deletions(-) diff --git a/e2e/rbac.spec.ts b/e2e/rbac.spec.ts index b82730e..1bd8997 100644 --- a/e2e/rbac.spec.ts +++ b/e2e/rbac.spec.ts @@ -30,6 +30,44 @@ test("a viewer can view roles but not create one", async ({ page }) => { expect(res.status()).toBe(403) }) +// The dashboard shell is h-screen with overflow-hidden at every level, so each +// page owns its scrolling. Opening the permission editor makes /access taller +// than the viewport, and without a scroll container the Cancel button sits below +// the fold with no way to reach it: the panel cannot be closed. +test("the permission editor stays reachable once it overflows the viewport", async ({ page }) => { + // Laptop height, not the 1280x720 default: the editor fits in 720 and the + // clipping only shows up on a shorter viewport. + await page.setViewportSize({ width: 1280, height: 620 }) + await login(page, ADMIN) + await page.goto("/access") + + await page.getByRole("button", { name: "Edit" }).first().click() + await expect(page.getByPlaceholder("Role name")).toBeVisible() + + // Walk out from the form and fail on any ancestor that clips content it is + // too short to show. Asserting the Cancel button's position instead would + // pass or fail on how many roles happen to be seeded, and Playwright can + // force a click on a clipped node where a user cannot. + const clipped = await page.evaluate(() => { + let el: Element | null = document.querySelector('input[placeholder="Role name"]') + const bad: string[] = [] + while (el && el !== document.documentElement) { + const overflowY = getComputedStyle(el).overflowY + if ((overflowY === "hidden" || overflowY === "clip") && el.scrollHeight > el.clientHeight + 1) { + bad.push(`${el.tagName.toLowerCase()} clips ${el.scrollHeight}px into ${el.clientHeight}px`) + } + el = el.parentElement + } + return bad + }) + expect(clipped).toEqual([]) + + const cancel = page.getByRole("button", { name: "Cancel" }) + await cancel.scrollIntoViewIfNeeded() + await cancel.click() + await expect(page.getByPlaceholder("Role name")).toBeHidden() +}) + // An admin creates a plain role through the UI. The crown-jewel step-up path is // covered at the API level by the Task 8/9 integration tests. // The name is unique per run and deleted afterwards: a fixed name would survive diff --git a/src/app/(dashboard)/access/page.tsx b/src/app/(dashboard)/access/page.tsx index 6129129..b357535 100644 --- a/src/app/(dashboard)/access/page.tsx +++ b/src/app/(dashboard)/access/page.tsx @@ -18,28 +18,34 @@ export default async function AccessPage() { const canManageUsers = authorize(perms, "users:manage") const canReadAudit = authorize(perms, "audit:read") + // The shell is h-screen with overflow-hidden at every level, so a page owns its + // own scrolling. Without the outer container the permission editor pushes the + // rest of the page past the fold with no way to reach it, and the panel can no + // longer be closed. Same shape as every other dashboard page. return ( -
-
-

Access management

-

Roles, assignments, and the audit trail.

-
-
-

Roles

- -
- {canManageUsers && ( -
-

People

- -
- )} - {canReadAudit && ( +
+
+
+

Access management

+

Roles, assignments, and the audit trail.

+
-

Audit trail

- +

Roles

+
- )} -
+ {canManageUsers && ( +
+

People

+ +
+ )} + {canReadAudit && ( +
+

Audit trail

+ +
+ )} +
+
) } From 98f230714fd03edac2cba7b9a66442195173305f Mon Sep 17 00:00:00 2001 From: WhiteMuush Date: Tue, 4 Aug 2026 21:58:03 +0200 Subject: [PATCH 07/13] fix(rbac): let the access page use the full width It was the only page capped at max-w-4xl. The convention splits on content: list pages run full width (alerts, dashboard, employees, register, reports), settings pages sit at max-w-3xl (data-api, data-sources, notifications). This page holds three lists, so it belongs with the first group. The cap came from the plan and matched neither. --- src/app/(dashboard)/access/page.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/app/(dashboard)/access/page.tsx b/src/app/(dashboard)/access/page.tsx index b357535..ecfe6f1 100644 --- a/src/app/(dashboard)/access/page.tsx +++ b/src/app/(dashboard)/access/page.tsx @@ -24,7 +24,7 @@ export default async function AccessPage() { // longer be closed. Same shape as every other dashboard page. return (
-
+

Access management

Roles, assignments, and the audit trail.

From 85f7fbcee5e8f93a678bdb0af49efefcb2533cac Mon Sep 17 00:00:00 2001 From: WhiteMuush Date: Tue, 4 Aug 2026 22:16:34 +0200 Subject: [PATCH 08/13] feat(rbac): rebuild the access page on the app's own design language The page read as a prototype next to a finished app. It used none of the vocabulary the rest of the product already has: bare ul lists instead of the card-wrapped tables every other list page uses, an unstyled search input where EmployeeTable insets a Search icon at max-w-sm, pagination floating outside any container, and stacked h2 headings instead of the page-header shape. Three lists stacked on one page also ran past 2000px. They move behind a plain underlined tab strip, no pills and no colour, so each fits a viewport. Every surface now reuses the existing classes: rounded-xl border-border/60 bg-card, thead on bg-muted/30 with uppercase tracked labels, rows on hover:bg-muted/40, empty states at py-12 centered, pagination in a bordered footer with size-7 icon buttons. Row actions reveal on hover, the pattern PresetTab and SetupGuides already use. The audit trail finally shows what it recorded. It listed only action, actor and date, so a row read "role.update" with no way to tell which role or what changed, while the API had been returning targetId, before and after all along. It now resolves ids to names and renders a real diff: "+alerts:close, -reports:export" for a permission change, "Viewer -> Security Manager" for an assignment. Name lookups degrade to a short id when the viewer holds audit:read without roles:read or users:read. --- e2e/rbac.spec.ts | 2 +- src/app/(dashboard)/access/page.tsx | 37 ++--- src/components/rbac/AccessTabs.tsx | 38 +++++ src/components/rbac/AuditTrail.tsx | 143 +++++++++++++--- src/components/rbac/RolesManager.tsx | 179 ++++++++++++++------- src/components/rbac/UserRoleAssignment.tsx | 96 +++++++---- 6 files changed, 372 insertions(+), 123 deletions(-) create mode 100644 src/components/rbac/AccessTabs.tsx diff --git a/e2e/rbac.spec.ts b/e2e/rbac.spec.ts index 1bd8997..ded5e0c 100644 --- a/e2e/rbac.spec.ts +++ b/e2e/rbac.spec.ts @@ -20,7 +20,7 @@ async function login(page: import("@playwright/test").Page, u: { email: string; test("a viewer can view roles but not create one", async ({ page }) => { await login(page, MEMBER) await page.goto("/access") - await expect(page.getByRole("heading", { name: "Access management" })).toBeVisible() + await expect(page.getByRole("heading", { name: "Access" })).toBeVisible() // Direct API create must be forbidden for a viewer. const res = await page.request.post("/api/roles", { diff --git a/src/app/(dashboard)/access/page.tsx b/src/app/(dashboard)/access/page.tsx index ecfe6f1..5e25e49 100644 --- a/src/app/(dashboard)/access/page.tsx +++ b/src/app/(dashboard)/access/page.tsx @@ -5,6 +5,7 @@ import { getUserPermissions, authorize } from "@/lib/rbac/authorize" import { RolesManager } from "@/components/rbac/RolesManager" import { UserRoleAssignment } from "@/components/rbac/UserRoleAssignment" import { AuditTrail } from "@/components/rbac/AuditTrail" +import { AccessTabs } from "@/components/rbac/AccessTabs" export default async function AccessPage() { const session = await getSession() @@ -24,28 +25,22 @@ export default async function AccessPage() { // longer be closed. Same shape as every other dashboard page. return (
-
-
-

Access management

-

Roles, assignments, and the audit trail.

-
-
-

Roles

- -
- {canManageUsers && ( -
-

People

- -
- )} - {canReadAudit && ( -
-

Audit trail

- -
- )} +
+

Access

+

+ Who can do what in this workspace, and a record of every change +

+ + }, + ...(canManageUsers + ? [{ id: "people", label: "People", panel: }] + : []), + ...(canReadAudit ? [{ id: "audit", label: "Audit trail", panel: }] : []), + ]} + />
) } diff --git a/src/components/rbac/AccessTabs.tsx b/src/components/rbac/AccessTabs.tsx new file mode 100644 index 0000000..63eacd1 --- /dev/null +++ b/src/components/rbac/AccessTabs.tsx @@ -0,0 +1,38 @@ +"use client" + +import { useState, type ReactNode } from "react" +import { cn } from "@/lib/utils" + +type Tab = { id: string; label: string; panel: ReactNode } + +// Three lists stacked on one page ran past 2000px. Tabs keep each one inside a +// single viewport without inventing a new visual language: the strip is a plain +// underlined row, no pills, no colour. +export function AccessTabs({ tabs }: { tabs: Tab[] }) { + const [active, setActive] = useState(tabs[0]?.id) + const current = tabs.find((t) => t.id === active) ?? tabs[0] + + return ( +
+
+ {tabs.map((t) => ( + + ))} +
+
{current?.panel}
+
+ ) +} diff --git a/src/components/rbac/AuditTrail.tsx b/src/components/rbac/AuditTrail.tsx index a51a218..6b685a5 100644 --- a/src/components/rbac/AuditTrail.tsx +++ b/src/components/rbac/AuditTrail.tsx @@ -1,22 +1,80 @@ "use client" import { useEffect, useState } from "react" +import { ChevronLeft, ChevronRight } from "lucide-react" + +type Snapshot = { name?: string; permissions?: string[]; roleId?: string | null } type Entry = { id: string action: string targetType: string targetId: string | null + before: Snapshot | null + after: Snapshot | null + ip: string | null createdAt: string actor: { email: string } | null } const PAGE = 20 +// Names are resolved from the roles and users endpoints when the viewer is +// allowed to read them. audit:read does not imply roles:read or users:read, so +// a denied lookup degrades to the raw id rather than blanking the column. +function useNames() { + const [names, setNames] = useState>({}) + useEffect(() => { + void (async () => { + const map: Record = {} + const [r, u] = await Promise.all([ + fetch("/api/roles").catch(() => null), + fetch("/api/users").catch(() => null), + ]) + if (r?.ok) for (const role of (await r.json()).roles as { id: string; name: string }[]) map[role.id] = role.name + if (u?.ok) for (const user of (await u.json()).users as { id: string; email: string }[]) map[user.id] = user.email + setNames(map) + })() + }, []) + return names +} + +function describeTarget(e: Entry, names: Record): string { + const named = e.after?.name ?? e.before?.name ?? (e.targetId ? names[e.targetId] : undefined) + if (named) return named + return e.targetId ? e.targetId.slice(0, 8) : "-" +} + +// What actually changed, in the terms the reader cares about: which permissions +// moved, or which role a person went to. Raw JSON would be unreadable here. +function describeChange(e: Entry, names: Record): string { + if (e.action === "user.role.assign") { + const from = e.before?.roleId ? (names[e.before.roleId] ?? "a role") : "no access" + const to = e.after?.roleId ? (names[e.after.roleId] ?? "a role") : "no access" + return `${from} -> ${to}` + } + + const before = e.before?.permissions + const after = e.after?.permissions + if (before && after) { + const added = after.filter((p) => !before.includes(p)) + const removed = before.filter((p) => !after.includes(p)) + const parts = [...added.map((p) => `+${p}`), ...removed.map((p) => `-${p}`)] + if (parts.length === 0) return "no permission change" + return parts.join(", ") + } + + // Create and delete carry a single snapshot, so there is nothing to diff. + const list = after ?? before + if (list) return `${list.length} permission${list.length === 1 ? "" : "s"}` + return "-" +} + export function AuditTrail() { const [entries, setEntries] = useState([]) const [total, setTotal] = useState(0) const [skip, setSkip] = useState(0) + const names = useNames() useEffect(() => { void (async () => { @@ -29,26 +87,75 @@ export function AuditTrail() { })() }, [skip]) + const pages = Math.max(1, Math.ceil(total / PAGE)) + return ( -
-
    - {entries.map((e) => ( -
  • - {e.action} - - {e.actor?.email ?? "system"} - {new Date(e.createdAt).toLocaleString()} - -
    • - ))} -
-
- {total} event(s) -
- -
diff --git a/src/components/rbac/RolesManager.tsx b/src/components/rbac/RolesManager.tsx index 4110d48..e431431 100644 --- a/src/components/rbac/RolesManager.tsx +++ b/src/components/rbac/RolesManager.tsx @@ -1,6 +1,7 @@ "use client" import { useEffect, useMemo, useState } from "react" +import { Search, ChevronLeft, ChevronRight } from "lucide-react" import { PermissionEditor } from "./PermissionEditor" import { StepUpDialog } from "./StepUpDialog" @@ -41,6 +42,7 @@ export function RolesManager() { [roles, query], ) const pageRoles = filtered.slice(page * PAGE_SIZE, page * PAGE_SIZE + PAGE_SIZE) + const pages = Math.max(1, Math.ceil(filtered.length / PAGE_SIZE)) function startEdit(role: Role | null) { setEditing(role) @@ -107,82 +109,149 @@ export function RolesManager() { } return ( -
-
- { - setQuery(e.target.value) - setPage(0) - }} - className="rounded-lg border border-input bg-card px-3 py-2 text-sm" - /> +
+
+
+ + { + setQuery(e.target.value) + setPage(0) + }} + className="w-full rounded-lg border border-input bg-card py-2 pl-9 pr-3 text-sm text-foreground placeholder:text-muted-foreground focus:border-ring focus:outline-none focus:ring-2 focus:ring-ring/20" + /> +
-
    - {pageRoles.map((r) => ( -
  • -
    - {r.name} - {r.isSystem && (system)} - - {r.permissions.length} permissions - -
    - {!r.isSystem && ( -
    - - -
    +
    + + + + + + + + + + {pageRoles.length === 0 ? ( + + + + ) : ( + pageRoles.map((r) => ( + + + + + + )) )} - - ))} - - -
    - - {filtered.length} role(s), page {page + 1} of {Math.max(1, Math.ceil(filtered.length / PAGE_SIZE))} - -
    - - +
    +
    + Role + + Permissions + + Actions +
    + No roles match this search +
    + {r.name} + {r.isSystem && ( + Built-in + )} + + {r.permissions.length} + + {r.isSystem ? ( + — + ) : ( +
    + + +
    + )} +
    + +
    +

    + {filtered.length} role{filtered.length === 1 ? "" : "s"} +

    +
    + + + {page + 1} / {pages} + + +
    {formOpen && ( -
    +
    +
    +

    + {editing ? `Edit ${editing.name}` : "New role"} +

    + + {perms.size} selected + +
    + setName(e.target.value)} - className="w-full rounded-lg border border-input bg-card px-3 py-2 text-sm" + className="w-full max-w-sm rounded-lg border border-input bg-card px-3 py-2 text-sm text-foreground placeholder:text-muted-foreground focus:border-ring focus:outline-none focus:ring-2 focus:ring-ring/20" /> + + {error &&

    {error}

    } -
    - -
    diff --git a/src/components/rbac/UserRoleAssignment.tsx b/src/components/rbac/UserRoleAssignment.tsx index 45dec1d..3947b77 100644 --- a/src/components/rbac/UserRoleAssignment.tsx +++ b/src/components/rbac/UserRoleAssignment.tsx @@ -1,6 +1,7 @@ "use client" import { useEffect, useMemo, useState } from "react" +import { Search } from "lucide-react" import { StepUpDialog } from "./StepUpDialog" type UserRow = { id: string; email: string; name: string; roleId: string | null; roleName: string | null } @@ -56,35 +57,74 @@ export function UserRoleAssignment() { } return ( -
    - setQuery(e.target.value)} - className="rounded-lg border border-input bg-card px-3 py-2 text-sm" - /> +
    +
    + + setQuery(e.target.value)} + className="w-full rounded-lg border border-input bg-card py-2 pl-9 pr-3 text-sm text-foreground placeholder:text-muted-foreground focus:border-ring focus:outline-none focus:ring-2 focus:ring-ring/20" + /> +
    + {error &&

    {error}

    } -
      - {filtered.map((u) => ( -
    • - {u.email} - -
      • - ))} -
    + +
    + + + + + + + + + {filtered.length === 0 ? ( + + + + ) : ( + filtered.map((u) => ( + + + + + )) + )} + +
    + Person + + Role +
    + No one matches this search +
    + {u.name || u.email} + {u.name && {u.email}} + + +
    + +
    +

    + {filtered.length} {filtered.length === 1 ? "person" : "people"} +

    +
    +
    + stepUpRetry?.()} From f5ffcba324a2647d7ea80cd6b1e8eed5a9946a64 Mon Sep 17 00:00:00 2001 From: WhiteMuush Date: Tue, 4 Aug 2026 22:19:44 +0200 Subject: [PATCH 09/13] feat(rbac): render the audit change as a diff instead of a string "Viewer -> Security Manager" read like terminal output, and gave both values the same weight when only the destination matters. The column now returns a fragment rather than a string. A reassignment steps the previous role back in muted text behind a small arrow glyph, with the new role in foreground. A permission change lists additions in foreground and struck-through removals in muted, instead of joining "+a, -b" into one line. --- src/components/rbac/AuditTrail.tsx | 60 ++++++++++++++++++++++-------- 1 file changed, 45 insertions(+), 15 deletions(-) diff --git a/src/components/rbac/AuditTrail.tsx b/src/components/rbac/AuditTrail.tsx index 6b685a5..d99f0e5 100644 --- a/src/components/rbac/AuditTrail.tsx +++ b/src/components/rbac/AuditTrail.tsx @@ -1,7 +1,7 @@ "use client" import { useEffect, useState } from "react" -import { ChevronLeft, ChevronRight } from "lucide-react" +import { ChevronLeft, ChevronRight, ArrowRight } from "lucide-react" type Snapshot = { name?: string; permissions?: string[]; roleId?: string | null } @@ -46,28 +46,56 @@ function describeTarget(e: Entry, names: Record): string { } // What actually changed, in the terms the reader cares about: which permissions -// moved, or which role a person went to. Raw JSON would be unreadable here. -function describeChange(e: Entry, names: Record): string { - if (e.action === "user.role.assign") { - const from = e.before?.roleId ? (names[e.before.roleId] ?? "a role") : "no access" - const to = e.after?.roleId ? (names[e.after.roleId] ?? "a role") : "no access" - return `${from} -> ${to}` +// moved, or which role a person went to. Raw JSON would be unreadable here, and +// so is a plain "a -> b" string: only the destination matters, so the previous +// value steps back rather than sharing the weight. +function Change({ entry, names }: { entry: Entry; names: Record }) { + if (entry.action === "user.role.assign") { + const from = entry.before?.roleId ? (names[entry.before.roleId] ?? "a role") : "No access" + const to = entry.after?.roleId ? (names[entry.after.roleId] ?? "a role") : "No access" + return ( + + {from} + + {to} + + ) } - const before = e.before?.permissions - const after = e.after?.permissions + const before = entry.before?.permissions + const after = entry.after?.permissions if (before && after) { const added = after.filter((p) => !before.includes(p)) const removed = before.filter((p) => !after.includes(p)) - const parts = [...added.map((p) => `+${p}`), ...removed.map((p) => `-${p}`)] - if (parts.length === 0) return "no permission change" - return parts.join(", ") + if (added.length === 0 && removed.length === 0) { + return No permission change + } + return ( + + {added.map((p) => ( + + + {p} + + ))} + {removed.map((p) => ( + + {p} + + ))} + + ) } // Create and delete carry a single snapshot, so there is nothing to diff. const list = after ?? before - if (list) return `${list.length} permission${list.length === 1 ? "" : "s"}` - return "-" + if (list) { + return ( + + {list.length} permission{list.length === 1 ? "" : "s"} + + ) + } + return — } export function AuditTrail() { @@ -125,7 +153,9 @@ export function AuditTrail() { {describeTarget(e, names)} - {describeChange(e, names)} + + + )) )} From b92db85230c42b0a9f6e9323d0bcd6c55f757311 Mon Sep 17 00:00:00 2001 From: WhiteMuush Date: Tue, 4 Aug 2026 22:38:47 +0200 Subject: [PATCH 10/13] feat(security): refuse outbound URLs that reach the private network Webhook targets are supplied by an admin, so from the server's side they are attacker-controlled: a request the app makes reaches whatever the network reaches. Forcing https already blocked the AWS metadata endpoint, which is http only, but nothing stopped an internal host behind TLS. parseOutboundUrl now rejects a literal private address, embedded credentials and a non-https scheme, covering loopback, RFC1918, link-local (including 169.254.169.254), carrier-grade NAT, unique-local v6, and IPv4-mapped v6 that wraps a private v4. A hostname is also resolved and every returned address checked, because a name that looks public can point into the private space. That runs at write time so a bad endpoint is refused up front, and again before each delivery so a record valid when saved cannot be repointed afterwards. Deliveries no longer follow redirects, which would otherwise route around both checks. --- src/app/api/webhooks/route.ts | 22 ++++++---- src/lib/ssrf.test.ts | 78 +++++++++++++++++++++++++++++++++++ src/lib/ssrf.ts | 74 +++++++++++++++++++++++++++++++++ src/lib/webhooks.ts | 8 ++++ 4 files changed, 173 insertions(+), 9 deletions(-) create mode 100644 src/lib/ssrf.test.ts create mode 100644 src/lib/ssrf.ts diff --git a/src/app/api/webhooks/route.ts b/src/app/api/webhooks/route.ts index 8099cb3..faa3ca3 100644 --- a/src/app/api/webhooks/route.ts +++ b/src/app/api/webhooks/route.ts @@ -4,24 +4,28 @@ import { prisma } from "@/lib/prisma" import { encryptConfig } from "@/lib/directory/crypto" import { listWebhooks, urlHint } from "@/lib/webhooks" import { isEmail } from "@/lib/validators" +import { parseOutboundUrl, resolvesToPublicHost } from "@/lib/ssrf" import { NotificationChannel, Severity } from "@prisma/client" // Resolve the encrypted delivery target and its display hint for a channel. // EMAIL targets a recipient address; every other channel targets an HTTPS URL. -function resolveTarget(channel: NotificationChannel, raw: string): { target: string; hint: string } | { error: string } { +async function resolveTarget( + channel: NotificationChannel, + raw: string, +): Promise<{ target: string; hint: string } | { error: string }> { if (channel === "EMAIL") { const addr = raw.trim().toLowerCase() if (!isEmail(addr)) return { error: "Invalid email address" } return { target: addr, hint: addr } } - let parsed: URL - try { - parsed = new URL(raw) - } catch { - return { error: "Invalid URL" } + const parsed = parseOutboundUrl(raw) + if ("error" in parsed) return parsed + // Refuse an endpoint that resolves inside the private space up front, rather + // than storing it and discovering the problem on the first delivery. + if (!(await resolvesToPublicHost(parsed.url.hostname))) { + return { error: "URL must point to a public host" } } - if (parsed.protocol !== "https:") return { error: "URL must use https" } - return { target: parsed.toString(), hint: urlHint(parsed.toString()) } + return { target: parsed.url.toString(), hint: urlHint(parsed.url.toString()) } } export async function GET() { @@ -48,7 +52,7 @@ export async function POST(req: Request) { ? (channel as NotificationChannel) : NotificationChannel.WEBHOOK - const resolved = resolveTarget(chan, url ?? "") + const resolved = await resolveTarget(chan, url ?? "") if ("error" in resolved) return NextResponse.json({ error: resolved.error }, { status: 400 }) const severity = diff --git a/src/lib/ssrf.test.ts b/src/lib/ssrf.test.ts new file mode 100644 index 0000000..fd2403e --- /dev/null +++ b/src/lib/ssrf.test.ts @@ -0,0 +1,78 @@ +import { describe, expect, it } from "vitest" +import { isPrivateAddress, parseOutboundUrl } from "./ssrf" + +describe("isPrivateAddress", () => { + it("rejects loopback", () => { + expect(isPrivateAddress("127.0.0.1")).toBe(true) + expect(isPrivateAddress("127.255.255.254")).toBe(true) + expect(isPrivateAddress("::1")).toBe(true) + }) + + it("rejects the RFC1918 ranges", () => { + expect(isPrivateAddress("10.0.0.1")).toBe(true) + expect(isPrivateAddress("172.16.0.1")).toBe(true) + expect(isPrivateAddress("172.31.255.255")).toBe(true) + expect(isPrivateAddress("192.168.1.1")).toBe(true) + }) + + it("keeps 172.15 and 172.32 public, they sit outside the /12", () => { + expect(isPrivateAddress("172.15.0.1")).toBe(false) + expect(isPrivateAddress("172.32.0.1")).toBe(false) + }) + + it("rejects link-local, including the cloud metadata address", () => { + expect(isPrivateAddress("169.254.169.254")).toBe(true) + expect(isPrivateAddress("fe80::1")).toBe(true) + }) + + it("rejects carrier-grade NAT, unspecified and unique local v6", () => { + expect(isPrivateAddress("100.64.0.1")).toBe(true) + expect(isPrivateAddress("0.0.0.0")).toBe(true) + expect(isPrivateAddress("fc00::1")).toBe(true) + expect(isPrivateAddress("fd12:3456::1")).toBe(true) + }) + + it("rejects IPv4-mapped IPv6 that wraps a private v4", () => { + expect(isPrivateAddress("::ffff:127.0.0.1")).toBe(true) + expect(isPrivateAddress("::ffff:10.0.0.1")).toBe(true) + }) + + it("accepts ordinary public addresses", () => { + expect(isPrivateAddress("1.1.1.1")).toBe(false) + expect(isPrivateAddress("93.184.216.34")).toBe(false) + expect(isPrivateAddress("2606:4700::1111")).toBe(false) + }) +}) + +describe("parseOutboundUrl", () => { + it("requires https", () => { + expect(parseOutboundUrl("http://example.com/hook")).toEqual({ error: "URL must use https" }) + }) + + it("rejects a literal private host without needing DNS", () => { + expect(parseOutboundUrl("https://127.0.0.1/hook")).toEqual({ + error: "URL must point to a public host", + }) + expect(parseOutboundUrl("https://169.254.169.254/latest/meta-data")).toEqual({ + error: "URL must point to a public host", + }) + expect(parseOutboundUrl("https://[::1]/hook")).toEqual({ + error: "URL must point to a public host", + }) + }) + + it("rejects credentials embedded in the URL", () => { + expect(parseOutboundUrl("https://user:pass@example.com/hook")).toEqual({ + error: "URL must not embed credentials", + }) + }) + + it("rejects a malformed URL", () => { + expect(parseOutboundUrl("not a url")).toEqual({ error: "Invalid URL" }) + }) + + it("accepts a normal https endpoint", () => { + const out = parseOutboundUrl("https://hooks.slack.com/services/T/B/xyz") + expect("url" in out && out.url.host).toBe("hooks.slack.com") + }) +}) diff --git a/src/lib/ssrf.ts b/src/lib/ssrf.ts new file mode 100644 index 0000000..4d6a528 --- /dev/null +++ b/src/lib/ssrf.ts @@ -0,0 +1,74 @@ +import { lookup } from "dns/promises" + +// Outbound URLs are supplied by admins (webhooks, directory endpoints), so they +// are attacker-controlled from the server's point of view: a request the app +// makes reaches whatever the network reaches, including sibling services and +// the cloud metadata endpoint. Forcing https already blocks the AWS IMDS +// address, which is http only, but not an internal host behind TLS. + +function isPrivateV4(ip: string): boolean { + const parts = ip.split(".") + if (parts.length !== 4) return false + const [a, b] = parts.map((p) => Number(p)) + if (parts.some((p) => p === "" || !/^\d+$/.test(p)) || [a, b].some(Number.isNaN)) return false + + if (a === 0) return true // "this network" + if (a === 10) return true // RFC1918 + if (a === 127) return true // loopback + if (a === 169 && b === 254) return true // link-local, covers 169.254.169.254 + if (a === 172 && b >= 16 && b <= 31) return true // RFC1918 /12 + if (a === 192 && b === 168) return true // RFC1918 + if (a === 100 && b >= 64 && b <= 127) return true // RFC6598 carrier-grade NAT + if (a >= 224) return true // multicast and reserved + return false +} + +export function isPrivateAddress(ip: string): boolean { + const addr = ip.trim().toLowerCase().replace(/^\[|\]$/g, "") + + // An IPv4-mapped v6 address reaches the same host as the v4 it wraps, so it + // has to be unwrapped rather than treated as an opaque v6 string. + const mapped = addr.match(/^::ffff:(\d+\.\d+\.\d+\.\d+)$/) + if (mapped) return isPrivateV4(mapped[1]) + + if (addr.includes(":")) { + if (addr === "::" || addr === "::1") return true + if (addr.startsWith("fe80")) return true // link-local + if (/^f[cd]/.test(addr)) return true // unique local fc00::/7 + return false + } + + return isPrivateV4(addr) +} + +export type OutboundUrl = { url: URL } | { error: string } + +// Synchronous checks only: shape, scheme, credentials, and a literal private +// address. A hostname still has to be resolved before the request is made, see +// resolvesToPublicHost. +export function parseOutboundUrl(raw: string): OutboundUrl { + let url: URL + try { + url = new URL(raw) + } catch { + return { error: "Invalid URL" } + } + if (url.protocol !== "https:") return { error: "URL must use https" } + if (url.username || url.password) return { error: "URL must not embed credentials" } + if (isPrivateAddress(url.hostname)) return { error: "URL must point to a public host" } + return { url } +} + +// A hostname that looks public can still resolve into the private space, which +// is the whole point of a DNS rebinding attack. Checked at write time so a bad +// endpoint is refused up front, and again before delivery so a record that was +// valid when saved cannot be repointed later. +export async function resolvesToPublicHost(hostname: string): Promise { + if (isPrivateAddress(hostname)) return false + try { + const records = await lookup(hostname, { all: true }) + return records.length > 0 && records.every((r) => !isPrivateAddress(r.address)) + } catch { + return false + } +} diff --git a/src/lib/webhooks.ts b/src/lib/webhooks.ts index 7082f11..ef9805d 100644 --- a/src/lib/webhooks.ts +++ b/src/lib/webhooks.ts @@ -1,4 +1,5 @@ import { prisma } from "@/lib/prisma" +import { parseOutboundUrl, resolvesToPublicHost } from "@/lib/ssrf" import { decryptConfig } from "@/lib/directory/crypto" import { emailEnabled, sendBreachAlert } from "@/lib/email" import { slackPayload, summaryLine, teamsPayload } from "@/lib/notify/payloads" @@ -41,8 +42,15 @@ export function listWebhooks(companyId: string): Promise { } async function postJson(url: string, body: unknown): Promise { + // Re-checked at delivery, not only when the row was written: DNS can be + // repointed into the private space long after an endpoint was accepted. + const parsed = parseOutboundUrl(url) + if ("error" in parsed) return false + if (!(await resolvesToPublicHost(parsed.url.hostname))) return false + try { const res = await fetch(url, { + redirect: "error", method: "POST", headers: { "Content-Type": "application/json" }, body: JSON.stringify(body), From e5c76045dd5dba832ee46f213fb7150cf0a829b3 Mon Sep 17 00:00:00 2001 From: WhiteMuush Date: Tue, 4 Aug 2026 22:39:12 +0200 Subject: [PATCH 11/13] feat(security): move rate limiting from memory to Postgres Both limiters counted in a per-process Map. On more than one instance the real ceiling was the configured limit times the number of nodes, and every deploy reset it, which makes the protection decorative anywhere but a single long-lived process. Better Auth switches to its database storage, and the application limiter behind scan, SCIM and SIEM now counts in an ApiRateLimit row. The counter is a single INSERT ON CONFLICT that rolls the window over inside the statement, so two requests arriving together cannot both read a stale count and both decide they are under the limit. An integration test drives 20 concurrent calls against a limit of 5 and asserts exactly 5 get through. The limiter helpers become async, so their callers await. The SCIM rate-limit tests move out of the unit suite into an itest: the counter lives in Postgres now, and a mocked store would only prove the mock counts. Expired rows are swept by the existing cron tick, which has no timing requirement. --- .../migration.sql | 24 ++++++ prisma/schema.prisma | 23 +++++ src/app/api/cron/route.test.ts | 7 ++ src/app/api/cron/route.ts | 11 ++- src/app/api/employees/scan/route.ts | 2 +- .../integrations/siem/[companyId]/route.ts | 2 +- .../[connectionId]/Users/[scimId]/route.ts | 4 +- .../api/scim/[connectionId]/Users/route.ts | 4 +- src/lib/auth/server.ts | 7 +- src/lib/directory/scim-auth.test.ts | 18 +--- src/lib/directory/scim-auth.ts | 6 +- src/lib/rateLimit.itest.ts | 84 +++++++++++++++++++ src/lib/rateLimit.ts | 42 ++++++---- src/lib/siem.ts | 2 +- 14 files changed, 192 insertions(+), 44 deletions(-) create mode 100644 prisma/migrations/20260804202845_add_rate_limit_tables/migration.sql create mode 100644 src/lib/rateLimit.itest.ts diff --git a/prisma/migrations/20260804202845_add_rate_limit_tables/migration.sql b/prisma/migrations/20260804202845_add_rate_limit_tables/migration.sql new file mode 100644 index 0000000..f835422 --- /dev/null +++ b/prisma/migrations/20260804202845_add_rate_limit_tables/migration.sql @@ -0,0 +1,24 @@ +-- CreateTable +CREATE TABLE "RateLimit" ( + "id" TEXT NOT NULL, + "key" TEXT NOT NULL, + "count" INTEGER NOT NULL, + "lastRequest" BIGINT NOT NULL, + + CONSTRAINT "RateLimit_pkey" PRIMARY KEY ("id") +); + +-- CreateTable +CREATE TABLE "ApiRateLimit" ( + "key" TEXT NOT NULL, + "count" INTEGER NOT NULL, + "reset" TIMESTAMP(3) NOT NULL, + + CONSTRAINT "ApiRateLimit_pkey" PRIMARY KEY ("key") +); + +-- CreateIndex +CREATE INDEX "RateLimit_key_idx" ON "RateLimit"("key"); + +-- CreateIndex +CREATE INDEX "ApiRateLimit_reset_idx" ON "ApiRateLimit"("reset"); diff --git a/prisma/schema.prisma b/prisma/schema.prisma index cac340d..15a4815 100644 --- a/prisma/schema.prisma +++ b/prisma/schema.prisma @@ -538,3 +538,26 @@ enum AlertConfidence { MEDIUM LOW } + +// Better Auth's own limiter, moved off in-memory storage so the sign-in window +// is shared by every instance and survives a deploy. Field names and types are +// dictated by Better Auth (storage: "database"). +model RateLimit { + id String @id + key String + count Int + lastRequest BigInt + + @@index([key]) +} + +// The application limiter behind scan, SCIM and SIEM. Same reasoning: a +// per-process Map divides the limit by the number of instances and resets on +// every deploy. +model ApiRateLimit { + key String @id + count Int + reset DateTime + + @@index([reset]) +} diff --git a/src/app/api/cron/route.test.ts b/src/app/api/cron/route.test.ts index 17ec528..9f66612 100644 --- a/src/app/api/cron/route.test.ts +++ b/src/app/api/cron/route.test.ts @@ -3,9 +3,13 @@ import { describe, it, expect, vi, beforeEach } from "vitest" const runDueSchedules = vi.fn() const runDueReportSchedules = vi.fn() const runDueSiemPush = vi.fn() +const pruneRateLimits = vi.fn() vi.mock("@/lib/scheduler", () => ({ runDueSchedules: () => runDueSchedules() })) vi.mock("@/lib/reportSchedules", () => ({ runDueReportSchedules: () => runDueReportSchedules() })) vi.mock("@/lib/siem", () => ({ runDueSiemPush: () => runDueSiemPush() })) +// Mocked like the rest: this one reaches Postgres, and the unit suite runs +// without a database. +vi.mock("@/lib/rateLimit", () => ({ pruneRateLimits: () => pruneRateLimits() })) import { POST } from "./route" @@ -20,6 +24,7 @@ beforeEach(() => { runDueSchedules.mockResolvedValue({ syncsEnqueued: 1, scansStarted: 0, jobsProcessed: 1 }) runDueReportSchedules.mockResolvedValue({ sent: 0 }) runDueSiemPush.mockResolvedValue({ pushed: 0 }) + pruneRateLimits.mockResolvedValue(0) }) describe("POST /api/cron", () => { @@ -48,10 +53,12 @@ describe("POST /api/cron", () => { jobsProcessed: 1, reportsSent: 0, alertsPushed: 0, + rateLimitsPruned: 0, }) expect(runDueSchedules).toHaveBeenCalled() expect(runDueReportSchedules).toHaveBeenCalled() expect(runDueSiemPush).toHaveBeenCalled() + expect(pruneRateLimits).toHaveBeenCalled() delete process.env.CRON_SECRET }) }) diff --git a/src/app/api/cron/route.ts b/src/app/api/cron/route.ts index e8323a6..b928cdc 100644 --- a/src/app/api/cron/route.ts +++ b/src/app/api/cron/route.ts @@ -3,6 +3,7 @@ import { NextResponse } from "next/server" import { runDueSchedules } from "@/lib/scheduler" import { runDueReportSchedules } from "@/lib/reportSchedules" import { runDueSiemPush } from "@/lib/siem" +import { pruneRateLimits } from "@/lib/rateLimit" function safeEqual(a: string, b: string): boolean { const bufA = Buffer.from(a) @@ -26,5 +27,13 @@ export async function POST(req: Request) { const result = await runDueSchedules() const { sent } = await runDueReportSchedules() const { pushed } = await runDueSiemPush() - return NextResponse.json({ ...result, reportsSent: sent, alertsPushed: pushed }) + // Counters whose window has closed are dead rows; sweeping them here avoids a + // dedicated job for something with no timing requirement. + const rateLimitsPruned = await pruneRateLimits() + return NextResponse.json({ + ...result, + reportsSent: sent, + alertsPushed: pushed, + rateLimitsPruned, + }) } diff --git a/src/app/api/employees/scan/route.ts b/src/app/api/employees/scan/route.ts index 2d97634..1f65328 100644 --- a/src/app/api/employees/scan/route.ts +++ b/src/app/api/employees/scan/route.ts @@ -11,7 +11,7 @@ export async function POST() { const companyId = session.user.companyId - if (!rateLimit(`scan:${companyId}`, 5, 60_000)) { + if (!(await rateLimit(`scan:${companyId}`, 5, 60_000))) { return NextResponse.json( { error: "Too many scans. Try again in a minute." }, { status: 429 } diff --git a/src/app/api/integrations/siem/[companyId]/route.ts b/src/app/api/integrations/siem/[companyId]/route.ts index 844f622..4d39083 100644 --- a/src/app/api/integrations/siem/[companyId]/route.ts +++ b/src/app/api/integrations/siem/[companyId]/route.ts @@ -9,7 +9,7 @@ const FORMATS: SiemFormat[] = ["cef", "syslog", "json"] export async function GET(req: Request, { params }: { params: Promise<{ companyId: string }> }) { const { companyId } = await params - if (!checkSiemRateLimit(companyId)) + if (!(await checkSiemRateLimit(companyId))) return NextResponse.json({ error: "Rate limit exceeded" }, { status: 429 }) if (!(await authenticateSiem(req, companyId))) diff --git a/src/app/api/scim/[connectionId]/Users/[scimId]/route.ts b/src/app/api/scim/[connectionId]/Users/[scimId]/route.ts index fc22ad9..18a7f61 100644 --- a/src/app/api/scim/[connectionId]/Users/[scimId]/route.ts +++ b/src/app/api/scim/[connectionId]/Users/[scimId]/route.ts @@ -25,7 +25,7 @@ export async function PATCH( { params }: { params: Promise<{ connectionId: string; scimId: string }> } ) { const { connectionId, scimId } = await params - if (!checkScimRateLimit(connectionId)) + if (!(await checkScimRateLimit(connectionId))) return NextResponse.json({ status: 429, detail: "Too many requests" }, { status: 429 }) const ctx = await authenticateScim(req, connectionId) if (!ctx) return NextResponse.json({ status: 401, detail: "Unauthorized" }, { status: 401 }) @@ -52,7 +52,7 @@ export async function DELETE( { params }: { params: Promise<{ connectionId: string; scimId: string }> } ) { const { connectionId, scimId } = await params - if (!checkScimRateLimit(connectionId)) + if (!(await checkScimRateLimit(connectionId))) return NextResponse.json({ status: 429, detail: "Too many requests" }, { status: 429 }) const ctx = await authenticateScim(req, connectionId) if (!ctx) return NextResponse.json({ status: 401, detail: "Unauthorized" }, { status: 401 }) diff --git a/src/app/api/scim/[connectionId]/Users/route.ts b/src/app/api/scim/[connectionId]/Users/route.ts index 8f959e8..264dd00 100644 --- a/src/app/api/scim/[connectionId]/Users/route.ts +++ b/src/app/api/scim/[connectionId]/Users/route.ts @@ -28,7 +28,7 @@ export async function POST( { params }: { params: Promise<{ connectionId: string }> } ) { const { connectionId } = await params - if (!checkScimRateLimit(connectionId)) + if (!(await checkScimRateLimit(connectionId))) return NextResponse.json({ status: 429, detail: "Too many requests" }, { status: 429 }) const ctx = await authenticateScim(req, connectionId) if (!ctx) return NextResponse.json({ status: 401, detail: "Unauthorized" }, { status: 401 }) @@ -70,7 +70,7 @@ export async function GET( { params }: { params: Promise<{ connectionId: string }> } ) { const { connectionId } = await params - if (!checkScimRateLimit(connectionId)) + if (!(await checkScimRateLimit(connectionId))) return NextResponse.json({ status: 429, detail: "Too many requests" }, { status: 429 }) const ctx = await authenticateScim(req, connectionId) if (!ctx) return NextResponse.json({ status: 401, detail: "Unauthorized" }, { status: 401 }) diff --git a/src/lib/auth/server.ts b/src/lib/auth/server.ts index e384c41..fde2611 100644 --- a/src/lib/auth/server.ts +++ b/src/lib/auth/server.ts @@ -123,7 +123,12 @@ export const auth = betterAuth({ // e2e only: the serial 2FA suite makes several sign-ins inside Better Auth's // 3-per-10s window, which would 429-flake. Gated on E2E=1 *and* a loopback // base URL, so it can never arm in production even if E2E leaks there. - ...(rateLimitDisabled ? { rateLimit: { enabled: false } } : {}), + // Persisted rather than in-memory, so the sign-in window is shared by every + // instance and survives a deploy. An in-process counter divides the real + // limit by the number of nodes and resets whenever one restarts. + ...(rateLimitDisabled + ? { rateLimit: { enabled: false } } + : { rateLimit: { enabled: true, storage: "database" as const, modelName: "rateLimit" } }), database: prismaAdapter(prisma, { provider: "postgresql" }), emailAndPassword: { enabled: true, diff --git a/src/lib/directory/scim-auth.test.ts b/src/lib/directory/scim-auth.test.ts index 1675b38..18377bf 100644 --- a/src/lib/directory/scim-auth.test.ts +++ b/src/lib/directory/scim-auth.test.ts @@ -10,7 +10,7 @@ vi.mock("@/lib/directory/crypto", () => ({ decryptConfig: (s: string) => decryptConfig(s), })) -import { authenticateScim, checkScimRateLimit } from "./scim-auth" +import { authenticateScim } from "./scim-auth" function reqWith(token?: string): Request { const headers = new Headers() @@ -60,19 +60,3 @@ describe("authenticateScim", () => { ) }) }) - -describe("checkScimRateLimit", () => { - it("allows up to 120 requests per connection then throttles", () => { - const id = `conn-${Math.random()}` - for (let i = 0; i < 120; i++) expect(checkScimRateLimit(id)).toBe(true) - expect(checkScimRateLimit(id)).toBe(false) - }) - - it("tracks connections independently", () => { - const a = `a-${Math.random()}` - const b = `b-${Math.random()}` - for (let i = 0; i < 120; i++) checkScimRateLimit(a) - expect(checkScimRateLimit(a)).toBe(false) - expect(checkScimRateLimit(b)).toBe(true) - }) -}) diff --git a/src/lib/directory/scim-auth.ts b/src/lib/directory/scim-auth.ts index d510b73..93f9680 100644 --- a/src/lib/directory/scim-auth.ts +++ b/src/lib/directory/scim-auth.ts @@ -9,9 +9,9 @@ const SCIM_RATE_WINDOW_MS = 60_000 // Throttle inbound SCIM requests per connection, before token validation, so // an unauthenticated caller cannot brute-force tokens or flood the endpoint. -// Returns true when the request is allowed. In-memory and per-instance (see -// rateLimit); move to a shared store when scaling horizontally. -export function checkScimRateLimit(connectionId: string): boolean { +// Returns true when the request is allowed. Shared across instances, see +// rateLimit. +export function checkScimRateLimit(connectionId: string): Promise { return rateLimit(`scim:${connectionId}`, SCIM_RATE_LIMIT, SCIM_RATE_WINDOW_MS) } diff --git a/src/lib/rateLimit.itest.ts b/src/lib/rateLimit.itest.ts new file mode 100644 index 0000000..052cf04 --- /dev/null +++ b/src/lib/rateLimit.itest.ts @@ -0,0 +1,84 @@ +import { afterAll, describe, expect, it } from "vitest" +import { prisma } from "@/lib/prisma" +import { rateLimit, pruneRateLimits } from "./rateLimit" +import { checkScimRateLimit } from "./directory/scim-auth" + +// Integration, not unit: the counter lives in Postgres now, which is the whole +// point. A mocked store would only prove the mock counts. + +const keys: string[] = [] +function freshKey(prefix: string): string { + const k = `${prefix}-${Math.random().toString(36).slice(2)}` + keys.push(k) + return k +} + +afterAll(async () => { + await prisma.apiRateLimit.deleteMany({ where: { key: { in: keys } } }) +}) + +describe("rateLimit", () => { + it("allows up to the limit then refuses", async () => { + const key = freshKey("unit") + for (let i = 0; i < 3; i++) expect(await rateLimit(key, 3, 60_000)).toBe(true) + expect(await rateLimit(key, 3, 60_000)).toBe(false) + expect(await rateLimit(key, 3, 60_000)).toBe(false) + }) + + it("counts each key on its own", async () => { + const a = freshKey("a") + const b = freshKey("b") + for (let i = 0; i < 3; i++) await rateLimit(a, 3, 60_000) + expect(await rateLimit(a, 3, 60_000)).toBe(false) + expect(await rateLimit(b, 3, 60_000)).toBe(true) + }) + + it("starts a new window once the old one has expired", async () => { + const key = freshKey("window") + // Long enough that the two calls below land inside the same window; a 1ms + // window expires between them and the refusal never happens. + expect(await rateLimit(key, 1, 150)).toBe(true) + expect(await rateLimit(key, 1, 150)).toBe(false) + await new Promise((r) => setTimeout(r, 200)) + expect(await rateLimit(key, 1, 60_000)).toBe(true) + }) + + // The reason the counter moved to SQL: two requests landing together must not + // both read a stale count and both conclude they are under the limit. + it("does not overshoot under concurrency", async () => { + const key = freshKey("race") + const results = await Promise.all(Array.from({ length: 20 }, () => rateLimit(key, 5, 60_000))) + expect(results.filter(Boolean)).toHaveLength(5) + }) + + it("survives a process restart, because the count is not in memory", async () => { + const key = freshKey("persist") + for (let i = 0; i < 3; i++) await rateLimit(key, 3, 60_000) + const row = await prisma.apiRateLimit.findUnique({ where: { key } }) + expect(row?.count).toBe(3) + }) +}) + +describe("pruneRateLimits", () => { + it("drops rows whose window has passed and keeps live ones", async () => { + const stale = freshKey("stale") + const live = freshKey("live") + await rateLimit(stale, 5, 50) + await rateLimit(live, 5, 60_000) + await new Promise((r) => setTimeout(r, 120)) + + await pruneRateLimits() + + expect(await prisma.apiRateLimit.findUnique({ where: { key: stale } })).toBeNull() + expect(await prisma.apiRateLimit.findUnique({ where: { key: live } })).not.toBeNull() + }) +}) + +describe("checkScimRateLimit", () => { + it("allows 120 requests per connection then throttles", async () => { + const id = freshKey("conn") + keys.push(`scim:${id}`) + for (let i = 0; i < 120; i++) expect(await checkScimRateLimit(id)).toBe(true) + expect(await checkScimRateLimit(id)).toBe(false) + }) +}) diff --git a/src/lib/rateLimit.ts b/src/lib/rateLimit.ts index f474b68..57b276c 100644 --- a/src/lib/rateLimit.ts +++ b/src/lib/rateLimit.ts @@ -1,18 +1,30 @@ -type Entry = { count: number; reset: number } +import { prisma } from "@/lib/prisma" -const buckets = new Map() +// Backed by Postgres rather than a per-process Map. In memory the limit was +// silently multiplied by the number of running instances and reset on every +// deploy, which makes it decorative on anything but a single long-lived node. +// +// One statement does the whole thing, so two concurrent requests cannot both +// read a stale count and both decide they are under the limit: the row is +// locked by the UPDATE and the window rolls over inside the same statement. +export async function rateLimit(key: string, limit: number, windowMs: number): Promise { + const reset = new Date(Date.now() + windowMs) -// Fixed-window counter. Returns false once the limit is reached for the -// current window. In-memory and per-instance, sufficient for a single-node -// deployment; move to a shared store if the app is scaled horizontally. -export function rateLimit(key: string, limit: number, windowMs: number): boolean { - const now = Date.now() - const entry = buckets.get(key) - if (!entry || now > entry.reset) { - buckets.set(key, { count: 1, reset: now + windowMs }) - return true - } - if (entry.count >= limit) return false - entry.count++ - return true + const rows = await prisma.$queryRaw<{ count: number }[]>` + INSERT INTO "ApiRateLimit" ("key", "count", "reset") + VALUES (${key}, 1, ${reset}) + ON CONFLICT ("key") DO UPDATE SET + "count" = CASE WHEN "ApiRateLimit"."reset" <= now() THEN 1 ELSE "ApiRateLimit"."count" + 1 END, + "reset" = CASE WHEN "ApiRateLimit"."reset" <= now() THEN ${reset} ELSE "ApiRateLimit"."reset" END + RETURNING "count" + ` + + return (rows[0]?.count ?? 1) <= limit +} + +// Expired rows are dead weight once their window has passed. Nothing depends on +// this running promptly, so it rides along with the existing cron sweep. +export async function pruneRateLimits(): Promise { + const { count } = await prisma.apiRateLimit.deleteMany({ where: { reset: { lte: new Date() } } }) + return count } diff --git a/src/lib/siem.ts b/src/lib/siem.ts index 2fb9fba..7d180f9 100644 --- a/src/lib/siem.ts +++ b/src/lib/siem.ts @@ -8,7 +8,7 @@ const SIEM_RATE_LIMIT = 60 const SIEM_RATE_WINDOW_MS = 60_000 const MAX_ALERTS = 1000 -export function checkSiemRateLimit(companyId: string): boolean { +export function checkSiemRateLimit(companyId: string): Promise { return rateLimit(`siem:${companyId}`, SIEM_RATE_LIMIT, SIEM_RATE_WINDOW_MS) } From f722c602ca7a3cf599ddd6c9f107dd15e447fffe Mon Sep 17 00:00:00 2001 From: WhiteMuush Date: Tue, 4 Aug 2026 22:39:46 +0200 Subject: [PATCH 12/13] fix(auth): raise bcrypt cost to 12, require 12-char passwords The production hash ran at cost 10 while every seed script used 12. That one line is the only place a real user's password is hashed, and it was the lowest. Existing hashes keep verifying unchanged: bcrypt stores its cost inside the hash, so old and new coexist and rotate naturally on the next password change. No minimum was configured either, leaving Better Auth's default of 8 with no complexity requirement. Thin for a product whose job is finding exposed credentials. The 72-byte cap is bcrypt's own truncation point, made explicit so a long passphrase is refused rather than silently cut. --- src/lib/auth/server.ts | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/src/lib/auth/server.ts b/src/lib/auth/server.ts index fde2611..d78f0cf 100644 --- a/src/lib/auth/server.ts +++ b/src/lib/auth/server.ts @@ -132,8 +132,16 @@ export const auth = betterAuth({ database: prismaAdapter(prisma, { provider: "postgresql" }), emailAndPassword: { enabled: true, + // 12 rounds, matching every seed script. This was the only place still on + // 10, and it is the only one that hashes a real user's password. Existing + // hashes keep verifying: bcrypt stores its cost inside the hash. + // The default minimum is 8, which is thin for a product whose job is + // finding exposed credentials. The cap keeps bcrypt's 72-byte truncation + // from silently ignoring the tail of a very long passphrase. + minPasswordLength: 12, + maxPasswordLength: 72, password: { - hash: (password) => bcrypt.hash(password, 10), + hash: (password) => bcrypt.hash(password, 12), verify: ({ hash, password }) => bcrypt.compare(password, hash), }, }, From 2bc6f93b25c95b2a0be720ac5adff606450b5cb1 Mon Sep 17 00:00:00 2001 From: WhiteMuush Date: Tue, 4 Aug 2026 22:47:48 +0200 Subject: [PATCH 13/13] fix(dashboard): validate the preset scope before the permission gate CodeQL flagged this as a user-controlled bypass of a security check. The alert predates this branch, the route last changed in c7b7167, but the reasoning holds: `scope` arrives as untrusted JSON while its type annotation is only a compile-time claim, so any value other than the two members skipped the `scope === "COMPANY"` guard and its permission check. That was not exploitable, because Prisma rejects an unknown enum member a few lines later, but it left an authorization decision resting on a database constraint instead of an explicit check. Narrow the value against a literal set first and answer 400 otherwise. --- src/app/api/dashboard/presets/route.ts | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/src/app/api/dashboard/presets/route.ts b/src/app/api/dashboard/presets/route.ts index e30e73b..f7ec1a1 100644 --- a/src/app/api/dashboard/presets/route.ts +++ b/src/app/api/dashboard/presets/route.ts @@ -37,7 +37,16 @@ export async function POST(req: Request) { return NextResponse.json({ error: "Name is required" }, { status: 400 }) } + // Narrowed against a literal set before it reaches the guard below. The type + // annotation on `body` is a compile-time claim about a value that arrives as + // untrusted JSON, so anything other than the two members used to slip past + // `scope === "COMPANY"` unchecked and only fail later, on Prisma's enum + // validation. That made an authorization decision depend on a database + // constraint several lines away rather than on an explicit check. const scope = body.scope ?? "PERSONAL" + if (scope !== "PERSONAL" && scope !== "COMPANY") { + return NextResponse.json({ error: "Invalid scope" }, { status: 400 }) + } if (scope === "COMPANY") { const perms = await getUserPermissions(prisma, session.user.roleId ?? null)