diff --git a/e2e/rbac.spec.ts b/e2e/rbac.spec.ts index b82730e..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", { @@ -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/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/(auth)/login/page.tsx b/src/app/(auth)/login/page.tsx index 38bba21..7c8924e 100644 --- a/src/app/(auth)/login/page.tsx +++ b/src/app/(auth)/login/page.tsx @@ -2,17 +2,50 @@ import { useState } from "react" import { useRouter } from "next/navigation" +import { Loader2, ShieldCheck } from "lucide-react" import { signIn, twoFactor } from "@/lib/auth/client" import { Button } from "@/components/ui/button" +// 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 ( +
+ +
+ + Opening your workspace... +
+
+ ) +} + export default function LoginPage() { const [error, setError] = useState(null) - const [loading, setLoading] = useState(false) + // Which action is in flight, not merely whether one is: the spinner belongs on + // the button that was actually clicked, and the passkey button must keep its + // 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) @@ -26,30 +59,30 @@ export default function LoginPage() { } async function handlePasskey() { - setLoading(true) + setPending("passkey") setError(null) try { const res = await signIn.passkey() if (res?.error) { setError("Passkey sign-in failed or was cancelled") + setPending(null) return } - router.push("/dashboard") + enterWorkspace() } catch { setError("Passkey sign-in failed or was cancelled") - } finally { - setLoading(false) + setPending(null) } } async function handleSendOtp() { - setLoading(true) + setPending("otp") setError(null) const { error } = await twoFactor.sendOtp() - setLoading(false) + setPending(null) if (error) { // 403 when the company's policy does not allow EMAIL_OTP; the server hook @@ -67,7 +100,7 @@ export default function LoginPage() { async function handleEmailVerify(e: React.FormEvent) { e.preventDefault() - setLoading(true) + setPending("otp") setError(null) const form = new FormData(e.currentTarget) @@ -75,19 +108,18 @@ export default function LoginPage() { code: String(form.get("code")), }) - setLoading(false) - if (error) { + setPending(null) setError("Invalid code") return } - router.push("/dashboard") + enterWorkspace() } async function handlePassword(e: React.FormEvent) { e.preventDefault() - setLoading(true) + setPending("password") setError(null) const form = new FormData(e.currentTarget) @@ -96,24 +128,25 @@ export default function LoginPage() { password: String(form.get("password")), }) - setLoading(false) - if (error) { + setPending(null) setError("Invalid email or password") return } if (data && "twoFactorRedirect" in data && data.twoFactorRedirect) { + // Stays on the page to ask for the second factor, so hand control back. + setPending(null) setNeedsTotp(true) return } - router.push("/dashboard") + enterWorkspace() } async function handleTotp(e: React.FormEvent) { e.preventDefault() - setLoading(true) + setPending("totp") setError(null) const form = new FormData(e.currentTarget) @@ -121,16 +154,17 @@ export default function LoginPage() { code: String(form.get("code")), }) - setLoading(false) - if (error) { + setPending(null) setError("Invalid code") return } - router.push("/dashboard") + enterWorkspace() } + if (entering) return + return (
@@ -306,7 +340,7 @@ export default function LoginPage() { className="w-full" size="lg" > - {loading ? "Signing in..." : "Sign in"} + {pending === "password" ? "Signing in..." : "Sign in"}
diff --git a/src/app/(dashboard)/access/page.tsx b/src/app/(dashboard)/access/page.tsx index 6129129..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() @@ -18,28 +19,28 @@ 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.

+
+
+

Access

+

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

-
-

Roles

- -
- {canManageUsers && ( -
-

People

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

Audit trail

- -
- )} -
+ + }, + ...(canManageUsers + ? [{ id: "people", label: "People", panel: }] + : []), + ...(canReadAudit ? [{ id: "audit", label: "Audit trail", panel: }] : []), + ]} + /> +
) } 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) => ( -
- ))} -
-
-
- ) -} 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/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) 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/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/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..d99f0e5 100644 --- a/src/components/rbac/AuditTrail.tsx +++ b/src/components/rbac/AuditTrail.tsx @@ -1,22 +1,108 @@ "use client" import { useEffect, useState } from "react" +import { ChevronLeft, ChevronRight, ArrowRight } 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, 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 = 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)) + 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 +} + 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 +115,77 @@ 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?.()} diff --git a/src/lib/auth/server.ts b/src/lib/auth/server.ts index e384c41..d78f0cf 100644 --- a/src/lib/auth/server.ts +++ b/src/lib/auth/server.ts @@ -123,12 +123,25 @@ 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, + // 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), }, }, 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) } 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),