From 4ddf5c094e56fcef73ad2fe8f2f3324d5fc0e6c2 Mon Sep 17 00:00:00 2001 From: yiekheng Date: Sun, 10 May 2026 18:30:58 +0800 Subject: [PATCH] feat(web): admin nav entry + role-aware AppShell - Add an Admin nav item (key 'admin', href /settings/users) with visibleTo=['admin'] so signed-in users with role='user' don't see it. - nav-config exposes navItemsForRole(role) helper that filters NAV_ITEMS by visibleTo. - Root layout fetches getCurrentUser() and forwards role into AppShell. AppShell narrows the role gate to the rendered nav (sidebar + drawer); /login still short-circuits to the bare header. Unknown role falls back to 'user' visibility (defense-in-depth). - Settings page renders an admin-only card linking to Users so admins have a discoverable in-app entry point too. Tests: - nav-config: navItemsForRole admin/user matrix + admin entry shape. - app-shell: admin link visible for admin, hidden for user, hidden for null/unauthenticated, /login bare header strips nav entirely. - actions/auth: cookie payload encodes role=user, unknown role rejected, AUTH_SECRET-unset path, whitespace-only username rejected, rate-limit key contains client IP, unknown-user path still hits DB+bcrypt. 440 tests now (was 423). Co-Authored-By: Claude Opus 4.7 (1M context) --- apps/web/src/actions/auth.test.ts | 73 +++++++++++++++ apps/web/src/app/layout.tsx | 10 +- apps/web/src/app/settings/page.tsx | 28 ++++++ apps/web/src/components/app-shell.test.tsx | 101 ++++++++++++++++++--- apps/web/src/components/app-shell.tsx | 32 +++++-- apps/web/src/components/nav-config.test.ts | 33 +++++++ apps/web/src/components/nav-config.ts | 26 +++++- 7 files changed, 278 insertions(+), 25 deletions(-) create mode 100644 apps/web/src/components/nav-config.test.ts diff --git a/apps/web/src/actions/auth.test.ts b/apps/web/src/actions/auth.test.ts index 2307795..1ea5785 100644 --- a/apps/web/src/actions/auth.test.ts +++ b/apps/web/src/actions/auth.test.ts @@ -209,4 +209,77 @@ describe("logoutAction", () => { expect(cookiesDeleteMock).toHaveBeenCalledWith("session"); expect(redirectMock).toHaveBeenCalledWith("/login"); }); + + it("is idempotent — clears the cookie even when no session exists", async () => { + // Real-world: a user with an expired cookie clicks Sign out. cookies.delete + // doesn't care about pre-existing state and we still issue the redirect. + cookiesDeleteMock.mockReset(); + await logoutAction().catch(() => {}); + expect(cookiesDeleteMock).toHaveBeenCalledTimes(1); + expect(cookiesDeleteMock).toHaveBeenCalledWith("session"); + }); +}); + +describe("loginAction — additional cases", () => { + it("issues a cookie with role='user' encoded in the payload for a non-admin user", async () => { + findUserMock.mockResolvedValue({ ...ADMIN_ROW, role: "user" }); + await loginAction(fd({ username: "alice", password: "correct-horse" })).catch(() => {}); + expect(cookiesSetMock).toHaveBeenCalledTimes(1); + const [, cookieValue] = cookiesSetMock.mock.calls[0]!; + // Cookie shape: .. Decode payload and + // assert the role round-trips, so the middleware/AppShell role gate gets + // accurate data without us having to import auth-cookie. + const [payloadEnc] = (cookieValue as string).split("."); + const json = Buffer.from( + payloadEnc!.replace(/-/g, "+").replace(/_/g, "/") + + "=".repeat((4 - (payloadEnc!.length % 4)) % 4), + "base64", + ).toString("utf8"); + const decoded = JSON.parse(json); + expect(decoded.role).toBe("user"); + }); + + it("rejects when the user row has an unrecognised role string", async () => { + findUserMock.mockResolvedValue({ ...ADMIN_ROW, role: "robot" }); + const r = await loginAction(fd({ username: "admin", password: "correct-horse" })); + expect(r).toEqual({ ok: false, error: "Account is not enabled." }); + expect(cookiesSetMock).not.toHaveBeenCalled(); + }); + + it("returns ok:false when AUTH_SECRET is unset (server misconfig)", async () => { + findUserMock.mockResolvedValue(ADMIN_ROW); + const prev = process.env.AUTH_SECRET; + delete process.env.AUTH_SECRET; + try { + const r = await loginAction(fd({ username: "admin", password: "correct-horse" })); + expect(r).toEqual({ ok: false, error: "Server is not configured for sign-in." }); + expect(cookiesSetMock).not.toHaveBeenCalled(); + } finally { + process.env.AUTH_SECRET = prev; + } + }); + + it("treats whitespace-only username as missing input", async () => { + const r = await loginAction(fd({ username: " ", password: "x" })); + expect(r).toEqual({ ok: false, error: "Username and password are required." }); + expect(findUserMock).not.toHaveBeenCalled(); + }); + + it("rate limit key includes the client IP so a hostile IP can't lock everyone out", async () => { + findUserMock.mockResolvedValue(ADMIN_ROW); + headersGetMock.mockReturnValue("198.51.100.42"); + await loginAction(fd({ username: "admin", password: "correct-horse" })).catch(() => {}); + const [key] = checkRateLimitMock.mock.calls[0]!; + expect(key).toContain("198.51.100.42"); + }); + + it("still hits the DB and bcrypt on the unknown-user path so timing equivalence holds", async () => { + findUserMock.mockResolvedValue(undefined); + const cmpSpy = vi.spyOn(bcrypt, "compare"); + await loginAction(fd({ username: "ghost", password: "anything" })); + // findFirst was called even though we know the user doesn't exist. + expect(findUserMock).toHaveBeenCalledTimes(1); + expect(cmpSpy).toHaveBeenCalled(); + cmpSpy.mockRestore(); + }); }); diff --git a/apps/web/src/app/layout.tsx b/apps/web/src/app/layout.tsx index cca40f8..bb07fd7 100644 --- a/apps/web/src/app/layout.tsx +++ b/apps/web/src/app/layout.tsx @@ -4,6 +4,7 @@ import { ThemeProvider } from "@/components/theme-provider"; import { AppShell } from "@/components/app-shell"; import { NotificationManager } from "@/components/notification-manager"; import { Toaster } from "@/components/ui/sonner"; +import { getCurrentUser } from "@/lib/auth"; import "./globals.css"; export const metadata: Metadata = { @@ -33,7 +34,12 @@ export const viewport: Viewport = { ], }; -export default function RootLayout({ children }: { children: React.ReactNode }) { +export default async function RootLayout({ children }: { children: React.ReactNode }) { + // Pass the role into AppShell so the nav can hide admin-only entries + // for the 'user' role. On /login getCurrentUser returns null and + // AppShell short-circuits to the bare header anyway. + const me = await getCurrentUser(); + const role = me?.role ?? null; return ( // `suppressHydrationWarning` here is for *attribute* differences only. // Two sources legitimately mutate / attributes after the @@ -46,7 +52,7 @@ export default function RootLayout({ children }: { children: React.ReactNode }) - {children} + {children} {/* SSE → browser notification bridge. Renders no DOM. */} diff --git a/apps/web/src/app/settings/page.tsx b/apps/web/src/app/settings/page.tsx index 06e24d9..47daa2f 100644 --- a/apps/web/src/app/settings/page.tsx +++ b/apps/web/src/app/settings/page.tsx @@ -1,3 +1,5 @@ +import Link from "next/link"; +import { ShieldCheckIcon, ChevronRightIcon } from "lucide-react"; import { getSeededOperator } from "@/lib/operator"; import { Card, CardContent, CardHeader, CardTitle, CardDescription } from "@/components/ui/card"; import { Separator } from "@/components/ui/separator"; @@ -7,6 +9,7 @@ import { PageShell } from "@/components/page-shell"; export default async function SettingsPage() { const op = await getSeededOperator(); + const isAdmin = op.role === "admin"; return ( @@ -24,6 +27,31 @@ export default async function SettingsPage() { + {isAdmin && ( + + + + + Admin + + + Manage which usernames can sign in and what role each + one has. Visible to admins only. + + + + + Users + + + + + )} + Notifications diff --git a/apps/web/src/components/app-shell.test.tsx b/apps/web/src/components/app-shell.test.tsx index 2a1d412..fa99f7b 100644 --- a/apps/web/src/components/app-shell.test.tsx +++ b/apps/web/src/components/app-shell.test.tsx @@ -55,7 +55,7 @@ describe("AppShell — mobile header (SSR)", () => { it("renders a fixed top header that hides on sm+ breakpoints", () => { const html = renderToStaticMarkup( - +
page
, ); @@ -66,7 +66,7 @@ describe("AppShell — mobile header (SSR)", () => { it("brand mark on the left links to /", () => { const html = renderToStaticMarkup( - +
, ); @@ -90,7 +90,7 @@ describe("AppShell — mobile header (SSR)", () => { for (const c of cases) { pathnameMock.mockReturnValue(c.path); const html = renderToStaticMarkup( - +
, ); @@ -104,7 +104,7 @@ describe("AppShell — mobile header (SSR)", () => { it("falls back to 'WhatsApp Bot' when the path doesn't match any nav item", () => { pathnameMock.mockReturnValue("/unknown-route"); const html = renderToStaticMarkup( - +
, ); @@ -115,7 +115,7 @@ describe("AppShell — mobile header (SSR)", () => { it("menu button on the right uses aria-label='Open menu'", () => { const html = renderToStaticMarkup( - +
, ); @@ -134,7 +134,7 @@ describe("AppShell — menu drawer contents (SSR via transparent Sheet mock)", ( it("renders one nav link per NAV_ITEM, in order", () => { const html = renderToStaticMarkup( - +
, ); @@ -157,7 +157,7 @@ describe("AppShell — menu drawer contents (SSR via transparent Sheet mock)", ( it("marks the active route's link with aria-current='page'", () => { pathnameMock.mockReturnValue("/reminders"); const html = renderToStaticMarkup( - +
, ); @@ -174,7 +174,7 @@ describe("AppShell — menu drawer contents (SSR via transparent Sheet mock)", ( // every page. The header uses an exact-match check for "/". pathnameMock.mockReturnValue("/accounts"); const html = renderToStaticMarkup( - +
, ); @@ -185,7 +185,7 @@ describe("AppShell — menu drawer contents (SSR via transparent Sheet mock)", ( it("does NOT include a theme toggle in the mobile drawer (per request)", () => { const html = renderToStaticMarkup( - +
, ); @@ -195,7 +195,7 @@ describe("AppShell — menu drawer contents (SSR via transparent Sheet mock)", ( it("drawer header carries the brand wording and a screen-reader description", () => { const html = renderToStaticMarkup( - +
, ); @@ -221,7 +221,7 @@ describe("AppShell — desktop sidebar (SSR)", () => { it("renders the sidebar nav with every NAV_ITEM", () => { const html = renderToStaticMarkup( - +
, ); @@ -234,7 +234,7 @@ describe("AppShell — desktop sidebar (SSR)", () => { it("keeps the theme toggle in the sidebar footer", () => { const html = renderToStaticMarkup( - +
, ); @@ -246,7 +246,7 @@ describe("AppShell — desktop sidebar (SSR)", () => { it("sidebar brand header is a link to / with a 'Go to dashboard' aria-label", () => { pathnameMock.mockReturnValue("/accounts"); const html = renderToStaticMarkup( - +
, ); @@ -264,7 +264,7 @@ describe("AppShell — desktop sidebar (SSR)", () => { // reader users on a wide-window split-screen don't hear two // identical announcements when both are visible. const html = renderToStaticMarkup( - +
, ); @@ -273,6 +273,79 @@ describe("AppShell — desktop sidebar (SSR)", () => { }); }); +// --------------------------------------------------------------------------- +// Role-gated nav (admin panel) +// --------------------------------------------------------------------------- +describe("AppShell — role-based nav filtering", () => { + beforeEach(() => { + pathnameMock.mockReset(); + pathnameMock.mockReturnValue("/"); + }); + + it("shows the Admin entry in BOTH the sidebar and drawer when role=admin", () => { + const html = renderToStaticMarkup( + +
+ , + ); + expect(html).toContain('href="/settings/users"'); + // A label appears in both the sidebar and the drawer; either way the + // count must be >=2 (sidebar copy + drawer copy). + const occurrences = (html.match(/href="\/settings\/users"/g) ?? []).length; + expect(occurrences).toBeGreaterThanOrEqual(2); + }); + + it("hides the Admin entry from BOTH surfaces when role=user", () => { + const html = renderToStaticMarkup( + +
+ , + ); + expect(html).not.toContain('href="/settings/users"'); + }); + + it("hides the Admin entry when role=null (defense-in-depth: unauthenticated)", () => { + pathnameMock.mockReturnValue("/accounts"); + const html = renderToStaticMarkup( + +
+ , + ); + expect(html).not.toContain('href="/settings/users"'); + }); + + it("does NOT hide entries that have no visibleTo restriction for either role", () => { + const adminHtml = renderToStaticMarkup( + +
+ , + ); + const userHtml = renderToStaticMarkup( + +
+ , + ); + for (const item of NAV_ITEMS) { + if (item.visibleTo) continue; + expect(adminHtml).toContain(`href="${item.href}"`); + expect(userHtml).toContain(`href="${item.href}"`); + } + }); + + it("/login route renders the bare header — no nav, no drawer, regardless of role", () => { + pathnameMock.mockReturnValue("/login"); + const html = renderToStaticMarkup( + +
+ , + ); + expect(html).not.toContain(" href === "/" ? pathname === "/" : pathname.startsWith(href), ); @@ -92,7 +101,7 @@ function MobileHeader() { aria-label="Primary navigation" className="flex flex-col gap-0.5 p-2 flex-1" > - {NAV_ITEMS.map(({ key, href, label, icon: Icon }) => { + {items.map(({ key, href, label, icon: Icon }) => { const active = href === "/" ? pathname === "/" : pathname.startsWith(href); return ( - {NAV_ITEMS.map(({ key, href, label, icon: Icon }) => { + {items.map(({ key, href, label, icon: Icon }) => { const active = href === "/" ? pathname === "/" : pathname.startsWith(href); return ( {/* Desktop sidebar */} - + {/* Mobile header (single row: brand · title · menu) */} - + {/* Main content Mobile: push down for the h-14 header (56px) plus a small gap diff --git a/apps/web/src/components/nav-config.test.ts b/apps/web/src/components/nav-config.test.ts new file mode 100644 index 0000000..3507b84 --- /dev/null +++ b/apps/web/src/components/nav-config.test.ts @@ -0,0 +1,33 @@ +import { describe, it, expect } from "vitest"; +import { NAV_ITEMS, navItemsForRole } from "./nav-config"; + +describe("navItemsForRole", () => { + it("includes every NAV_ITEM for an admin", () => { + const items = navItemsForRole("admin"); + expect(items).toHaveLength(NAV_ITEMS.length); + for (const original of NAV_ITEMS) { + expect(items.find((i) => i.key === original.key)).toBeDefined(); + } + }); + + it("hides admin-only entries for the 'user' role", () => { + const items = navItemsForRole("user"); + const keys = items.map((i) => i.key); + expect(keys).not.toContain("admin"); + }); + + it("returns the un-restricted entries (no visibleTo) for the 'user' role", () => { + const items = navItemsForRole("user"); + const keys = items.map((i) => i.key); + expect(keys).toEqual( + expect.arrayContaining(["dashboard", "accounts", "reminders", "activity", "settings"]), + ); + }); + + it("admin nav entry routes to /settings/users", () => { + const admin = NAV_ITEMS.find((i) => i.key === "admin"); + expect(admin).toBeDefined(); + expect(admin!.href).toBe("/settings/users"); + expect(admin!.visibleTo).toEqual(["admin"]); + }); +}); diff --git a/apps/web/src/components/nav-config.ts b/apps/web/src/components/nav-config.ts index 05dcc03..d71b29c 100644 --- a/apps/web/src/components/nav-config.ts +++ b/apps/web/src/components/nav-config.ts @@ -1,11 +1,22 @@ -import { Home, Smartphone, Calendar, Activity, Settings } from "lucide-react"; +import { + Home, + Smartphone, + Calendar, + Activity, + Settings, + ShieldCheck, +} from "lucide-react"; import type { LucideIcon } from "lucide-react"; +export type NavRole = "admin" | "user"; + export interface NavItem { key: string; href: string; label: string; icon: LucideIcon; + /** When set, only roles listed here will see this nav entry. */ + visibleTo?: NavRole[]; } export const NAV_ITEMS: NavItem[] = [ @@ -13,5 +24,18 @@ export const NAV_ITEMS: NavItem[] = [ { key: "accounts", href: "/accounts", label: "Accounts", icon: Smartphone }, { key: "reminders", href: "/reminders", label: "Reminders", icon: Calendar }, { key: "activity", href: "/activity", label: "Activity", icon: Activity }, + { + key: "admin", + href: "/settings/users", + label: "Admin", + icon: ShieldCheck, + visibleTo: ["admin"], + }, { key: "settings", href: "/settings", label: "Settings", icon: Settings }, ]; + +export function navItemsForRole(role: NavRole): NavItem[] { + return NAV_ITEMS.filter( + (item) => item.visibleTo === undefined || item.visibleTo.includes(role), + ); +}