From f4dfea11030845a94e1e44f93c2501b18ac153c3 Mon Sep 17 00:00:00 2001 From: ykargeee-bit Date: Sat, 17 Oct 2026 21:49:57 +0100 Subject: [PATCH] Cookie-based session model, admin role gate and 401 handling (ykargeee-bit) - Adopt the backend's HttpOnly accessToken cookie as the single session store: every fetch sends credentials: "include", the js-cookie mirror and all bearer-header/token threading are removed from the client, and the session store keeps only the in-memory copy of the login token plus the user (Closes #1975) - Add GET /auth/me (backend) and useSessionBootstrap so a reload restores who is signed in without reading cookies from JavaScript, fixing the hydration-mismatch class of bugs at the source - Gate /admin routes by role: middleware decodes the verified admin claim and redirects members to a new /admin/forbidden page; RequireAdmin is rewritten as a session-driven client backstop with a not-authorised state (Closes #1973) - React to rejected sessions centrally: expireSession() clears the store, toasts once and redirects to /login?returnTo=... with loop protection, wired into every *-api helper (Closes #1977) - Add logout() (POST /auth/logout with credentials), a shared useSignOut hook and a UserMenu on wallet, payments, usage, credits and admin headers so sign-out reaches the backend everywhere (Closes #1974) Notes for reviewers: - Backend change is limited to the new GET /auth/me endpoint reading the guarded request user; login/register/logout are untouched. - The api modules' tests keep the same behavior coverage: the "Bearer" header assertions were dropped (no client-side bearer) and the wallet credentials expectation moved from same-origin to include. - Keep an eye on merge order with #1976 (shared lib/http.ts by devrabiuhussain): both PRs touch the five *-api modules and their callers. #1976 keeps an optional accessToken param for compatibility; this PR removes the parameter and headers entirely, so resolve in favor of this one's cookie session semantics. - The query-client side of the 401 reaction (skip retries for 401/403) lands with the providers consolidation in #1980 to avoid both PRs editing the same provider files. --- backend/src/auth/auth.controller.ts | 14 +- backend/src/auth/auth.service.ts | 17 ++- frontend/app/admin/accounts/page.tsx | 18 +-- frontend/app/admin/charge/page.tsx | 7 - frontend/app/admin/forbidden/page.tsx | 35 +++++ frontend/app/admin/integrity/page.tsx | 10 +- frontend/app/admin/layout.tsx | 10 +- frontend/app/admin/payments/page.tsx | 9 +- frontend/app/admin/refund/page.tsx | 13 +- frontend/app/admin/settlements/page.tsx | 12 +- frontend/app/admin/splits/page.tsx | 16 +-- frontend/app/credits/page.tsx | 33 +++-- frontend/app/payments/__tests__/page.test.tsx | 76 ++++++++++ frontend/app/payments/new/page.tsx | 7 - frontend/app/payments/page.tsx | 29 +++- frontend/app/payments/return/page.tsx | 23 ++-- frontend/app/usage/page.tsx | 19 ++- frontend/app/wallet/page.tsx | 6 +- .../admin/__tests__/require-admin.test.tsx | 104 ++++++++++++-- frontend/components/admin/require-admin.tsx | 61 ++++++-- frontend/components/auth/auth-form.test.tsx | 13 +- frontend/components/auth/auth-form.tsx | 6 +- frontend/components/auth/use-sign-out.ts | 24 ++++ frontend/components/auth/user-menu.tsx | 48 +++++++ .../components/wallet/wallet-status-card.tsx | 6 +- frontend/lib/__tests__/api-401.test.ts | 130 ++++++++++++++++++ frontend/lib/__tests__/auth-api.test.ts | 105 ++++++++++++++ frontend/lib/__tests__/credits-api.test.ts | 10 +- .../__tests__/payment-return-page.test.tsx | 18 ++- frontend/lib/__tests__/payments-api.test.ts | 6 +- frontend/lib/__tests__/session-expiry.test.ts | 91 ++++++++++++ frontend/lib/__tests__/wallet-api.test.ts | 2 +- frontend/lib/admin-api.ts | 50 +++---- frontend/lib/auth-api.ts | 84 ++++++++++- frontend/lib/auth-types.ts | 14 ++ frontend/lib/credits-api.ts | 19 ++- frontend/lib/payments-api.ts | 40 +++--- frontend/lib/session-expiry.ts | 39 ++++++ frontend/lib/stores/session-store.ts | 44 +++--- frontend/lib/stores/use-session-bootstrap.ts | 39 ++++++ frontend/lib/wallet-api.ts | 10 +- frontend/middleware.test.ts | 82 +++++++++++ frontend/middleware.ts | 40 ++++++ 43 files changed, 1202 insertions(+), 237 deletions(-) create mode 100644 frontend/app/admin/forbidden/page.tsx create mode 100644 frontend/app/payments/__tests__/page.test.tsx create mode 100644 frontend/components/auth/use-sign-out.ts create mode 100644 frontend/components/auth/user-menu.tsx create mode 100644 frontend/lib/__tests__/api-401.test.ts create mode 100644 frontend/lib/__tests__/auth-api.test.ts create mode 100644 frontend/lib/__tests__/session-expiry.test.ts create mode 100644 frontend/lib/auth-types.ts create mode 100644 frontend/lib/session-expiry.ts create mode 100644 frontend/lib/stores/use-session-bootstrap.ts create mode 100644 frontend/middleware.test.ts diff --git a/backend/src/auth/auth.controller.ts b/backend/src/auth/auth.controller.ts index f01da287..333c07fd 100644 --- a/backend/src/auth/auth.controller.ts +++ b/backend/src/auth/auth.controller.ts @@ -1,6 +1,7 @@ import { Body, Controller, + Get, Param, ParseUUIDPipe, Patch, @@ -14,7 +15,7 @@ import { RegisterDto } from './dto/register.dto'; import { LoginDto } from './dto/login.dto'; import { ChangePasswordDto } from './dto/change-password.dto'; import { UpdateUserRoleDto } from './dto/update-user-role.dto'; -import { AuthResponseDto } from './dto/auth-response.dto'; +import { AuthResponseDto, AuthUserDto } from './dto/auth-response.dto'; import { Response, CookieOptions } from 'express'; import { JwtAuthGuard } from './guards/jwt-auth.guard'; import { RolesGuard } from './guards/roles.guard'; @@ -58,6 +59,16 @@ export class AuthController { return result; } + @Get('me') + @UseGuards(JwtAuthGuard) + @ApiOperation({ + summary: + 'Return the authenticated user (id, email, role) so the client can restore a session from the HttpOnly cookie without exposing it to JavaScript', + }) + async me(@CurrentUser() currentUser: RequestUser): Promise { + return this.authService.me(currentUser); + } + @Post('logout') @UseGuards(JwtAuthGuard) @ApiOperation({ @@ -115,3 +126,4 @@ export class AuthController { return { id: user.id, role: user.role }; } } + diff --git a/backend/src/auth/auth.service.ts b/backend/src/auth/auth.service.ts index 4aa60e06..258940eb 100644 --- a/backend/src/auth/auth.service.ts +++ b/backend/src/auth/auth.service.ts @@ -13,7 +13,8 @@ import { UserRole } from './enums/user-role.enum'; import { RegisterDto } from './dto/register.dto'; import { LoginDto } from './dto/login.dto'; import { ChangePasswordDto } from './dto/change-password.dto'; -import { AuthResponseDto } from './dto/auth-response.dto'; +import { AuthResponseDto, AuthUserDto } from './dto/auth-response.dto'; +import { RequestUser } from './interfaces/authenticated-request.interface'; /** * Cost factor every password hash in this service uses (issue #1964). Both @@ -164,6 +165,20 @@ export class AuthService { return this.users.save(user); } + /** + * Restores the session client-side (issue #1973): the HttpOnly cookie + * cannot be read by the frontend, so `GET /auth/me` is the only way to + * learn who is signed in and with which role. A revoked token never gets + * this far (the guard's verifier rejects it), so a hit means "logged in". + */ + async me(currentUser: RequestUser): Promise { + const user = await this.users.findOne({ where: { id: currentUser.id } }); + if (!user) { + throw new NotFoundException('User not found'); + } + return { id: user.id, email: user.email, role: user.role }; + } + private async bumpTokenVersion(userId: string): Promise { await this.users.increment({ id: userId }, 'tokenVersion', 1); } diff --git a/frontend/app/admin/accounts/page.tsx b/frontend/app/admin/accounts/page.tsx index 4d70b33c..a9014048 100644 --- a/frontend/app/admin/accounts/page.tsx +++ b/frontend/app/admin/accounts/page.tsx @@ -16,7 +16,7 @@ import { SecondaryButton, inputClass, } from "@/components/admin/ui"; -import { RequireAdmin, useAdminToken } from "@/components/admin/require-admin"; +import { RequireAdmin } from "@/components/admin/require-admin"; const KINDS: LedgerAccountKind[] = [ "USER", @@ -28,27 +28,25 @@ const KINDS: LedgerAccountKind[] = [ ]; export default function AccountsPage() { - const token = useAdminToken(); - return ( - + ); } -function AccountsInner({ token }: { token: string }) { +function AccountsInner() { const queryClient = useQueryClient(); const [showCreate, setShowCreate] = useState(false); const accounts = useQuery({ queryKey: ["ledger-accounts"], - queryFn: () => listAccounts(token), + queryFn: () => listAccounts(), }); const freeze = useMutation({ mutationFn: ({ id, frozen }: { id: string; frozen: boolean }) => - updateAccount(token, id, { frozen }), + updateAccount(id, { frozen }), onSuccess: () => { queryClient.invalidateQueries({ queryKey: ["ledger-accounts"] }); toast.success("Account policy updated"); @@ -74,7 +72,7 @@ function AccountsInner({ token }: { token: string }) { {showCreate && ( - setShowCreate(false)} /> + setShowCreate(false)} /> )} @@ -176,10 +174,8 @@ function AccountRow({ } function CreateAccountForm({ - token, onDone, }: { - token: string; onDone: () => void; }) { const queryClient = useQueryClient(); @@ -201,7 +197,7 @@ function CreateAccountForm({ const create = useMutation({ mutationFn: () => - createAccount(token, { + createAccount({ kind: form.kind, ownerId: form.ownerId || undefined, currency: form.currency, diff --git a/frontend/app/admin/charge/page.tsx b/frontend/app/admin/charge/page.tsx index 3c75b425..9930b92f 100644 --- a/frontend/app/admin/charge/page.tsx +++ b/frontend/app/admin/charge/page.tsx @@ -9,7 +9,6 @@ import { } from "@/components/app-ui"; import { RequireAdmin } from "@/components/admin/require-admin"; import { chargeCredits, type ChargeCreditsResponse } from "@/lib/payments-api"; -import Cookies from "js-cookie"; import { useState } from "react"; import { toast } from "sonner"; @@ -24,15 +23,9 @@ export default function AdminChargePage() { async function onSubmit(e: React.FormEvent) { e.preventDefault(); - const token = Cookies.get("accessToken"); - if (!token) { - toast.error("Sign in as an administrator."); - return; - } setBusy(true); try { const res = await chargeCredits( - token, { userId, amount: Number(amount), diff --git a/frontend/app/admin/forbidden/page.tsx b/frontend/app/admin/forbidden/page.tsx new file mode 100644 index 00000000..85c5830e --- /dev/null +++ b/frontend/app/admin/forbidden/page.tsx @@ -0,0 +1,35 @@ +import Link from "next/link"; +import { ShieldAlert } from "lucide-react"; + +export const metadata = { + title: "Not authorised", +}; + +/** + * Landing page for member-role tokens that try to open /admin/* routes + * (issue #1973). The middleware redirects non-admin sessions here; the + * backend still enforces authorization on every admin endpoint. + */ +export default function AdminForbiddenPage() { + return ( +
+
+ +

+ Not authorised +

+

+ Your account does not have the admin role, so the admin area is + unavailable. If you should have access, ask an administrator to + upgrade your account. +

+ + Back to your wallet + +
+
+ ); +} diff --git a/frontend/app/admin/integrity/page.tsx b/frontend/app/admin/integrity/page.tsx index 568f359c..6fc1f147 100644 --- a/frontend/app/admin/integrity/page.tsx +++ b/frontend/app/admin/integrity/page.tsx @@ -5,22 +5,20 @@ import { CheckCircle2, AlertTriangle } from "lucide-react"; import { checkIntegrity, type LedgerIntegrityReport } from "@/lib/admin-api"; import { Card, CardHeader, Button } from "@/components/admin/ui"; import { IntegrityResults } from "@/components/admin/integrity-results"; -import { RequireAdmin, useAdminToken } from "@/components/admin/require-admin"; +import { RequireAdmin } from "@/components/admin/require-admin"; export default function IntegrityPage() { - const token = useAdminToken(); - return ( - + ); } -function IntegrityInner({ token }: { token: string }) { +function IntegrityInner() { const report = useQuery({ queryKey: ["ledger-integrity"], - queryFn: () => checkIntegrity(token), + queryFn: () => checkIntegrity(), refetchOnWindowFocus: false, }); diff --git a/frontend/app/admin/layout.tsx b/frontend/app/admin/layout.tsx index dca57495..0c13f2b4 100644 --- a/frontend/app/admin/layout.tsx +++ b/frontend/app/admin/layout.tsx @@ -10,6 +10,7 @@ import { } from "lucide-react"; import { AdminProviders } from "@/lib/providers"; import { AdminBreadcrumbs } from "@/components/admin/admin-breadcrumbs"; +import { UserMenu } from "@/components/auth/user-menu"; import { cn } from "@/lib/utils"; export const metadata: Metadata = { @@ -39,9 +40,12 @@ export default function AdminLayout({ ManageHub Admin - - Back to site - +
+ + + Back to site + +
diff --git a/frontend/app/admin/payments/page.tsx b/frontend/app/admin/payments/page.tsx index 2255a467..2fc6f8d1 100644 --- a/frontend/app/admin/payments/page.tsx +++ b/frontend/app/admin/payments/page.tsx @@ -12,24 +12,23 @@ import { Badge, StatusBadge, } from "@/components/admin/ui"; -import { RequireAdmin, useAdminToken } from "@/components/admin/require-admin"; +import { RequireAdmin } from "@/components/admin/require-admin"; const PAGE_SIZE = 25; export default function AdminManualReviewPage() { - const token = useAdminToken(); return ( - + ); } -function ManualReviewInner({ token }: { token: string }) { +function ManualReviewInner() { const [offset, setOffset] = useState(0); const { data, isLoading, error } = useQuery({ queryKey: ["manual-review-payments", offset], - queryFn: () => listManualReviewPayments(token, PAGE_SIZE, offset), + queryFn: () => listManualReviewPayments(PAGE_SIZE, offset), refetchInterval: 30_000, }); diff --git a/frontend/app/admin/refund/page.tsx b/frontend/app/admin/refund/page.tsx index 39c985b2..c96c22a6 100644 --- a/frontend/app/admin/refund/page.tsx +++ b/frontend/app/admin/refund/page.tsx @@ -14,7 +14,6 @@ import { requestRefund, type Payment, } from "@/lib/payments-api"; -import Cookies from "js-cookie"; import { useQuery, useQueryClient } from "@tanstack/react-query"; import { useState } from "react"; import { toast } from "sonner"; @@ -24,14 +23,12 @@ function PaymentRow({ payment }: { payment: Payment }) { const [amount, setAmount] = useState(String(payment.amount)); const [reason, setReason] = useState(""); const [busy, setBusy] = useState(false); - const token = typeof window !== "undefined" ? Cookies.get("accessToken") : null; async function onRefund(e: React.FormEvent) { e.preventDefault(); - if (!token) return; setBusy(true); try { - await requestRefund(token, payment.id, Number(amount), reason); + await requestRefund(payment.id, Number(amount), reason); toast.success("Refund request submitted"); setReason(""); await queryClient.invalidateQueries({ queryKey: ["manual-review"] }); @@ -91,11 +88,9 @@ function PaymentRow({ payment }: { payment: Payment }) { } export default function AdminRefundPage() { - const token = typeof window !== "undefined" ? Cookies.get("accessToken") : null; const { data, isLoading, error } = useQuery({ queryKey: ["manual-review"], - queryFn: () => listManualReviewPayments(token!), - enabled: !!token, + queryFn: () => listManualReviewPayments(), }); return ( @@ -107,9 +102,7 @@ export default function AdminRefundPage() { description="Payments awaiting reconciliation or manual review can be refunded." />
- {!token ? ( -

Sign in required.

- ) : isLoading ? ( + isLoading ? (

Loading…

) : error ? (

{(error as Error).message}

diff --git a/frontend/app/admin/settlements/page.tsx b/frontend/app/admin/settlements/page.tsx index 2f7fa6c8..907136ea 100644 --- a/frontend/app/admin/settlements/page.tsx +++ b/frontend/app/admin/settlements/page.tsx @@ -13,7 +13,7 @@ import { StatusBadge, Button, } from "@/components/admin/ui"; -import { RequireAdmin, useAdminToken } from "@/components/admin/require-admin"; +import { RequireAdmin } from "@/components/admin/require-admin"; import { SettlementVolumeChart } from "@/components/admin/settlement-volume-chart"; const STATUSES: Array<{ value: SettlementBatchStatus | ""; label: string }> = [ @@ -27,16 +27,14 @@ const STATUSES: Array<{ value: SettlementBatchStatus | ""; label: string }> = [ ]; export default function SettlementsPage() { - const token = useAdminToken(); - return ( - + ); } -function SettlementsInner({ token }: { token: string }) { +function SettlementsInner() { const queryClient = useQueryClient(); const status = typeof window !== "undefined" @@ -46,11 +44,11 @@ function SettlementsInner({ token }: { token: string }) { const batches = useQuery({ queryKey: ["settlement-batches", status], queryFn: () => - listBatches(token, (status || undefined) as SettlementBatchStatus | undefined), + listBatches((status || undefined) as SettlementBatchStatus | undefined), }); const run = useMutation({ - mutationFn: () => runSettlement(token), + mutationFn: () => runSettlement(), onSuccess: () => { queryClient.invalidateQueries({ queryKey: ["settlement-batches"] }); toast.success("Settlement pass completed"); diff --git a/frontend/app/admin/splits/page.tsx b/frontend/app/admin/splits/page.tsx index 7962b732..56e36d1f 100644 --- a/frontend/app/admin/splits/page.tsx +++ b/frontend/app/admin/splits/page.tsx @@ -16,24 +16,22 @@ import { SecondaryButton, inputClass, } from "@/components/admin/ui"; -import { RequireAdmin, useAdminToken } from "@/components/admin/require-admin"; +import { RequireAdmin } from "@/components/admin/require-admin"; export default function SplitsPage() { - const token = useAdminToken(); - return ( - + ); } -function SplitsInner({ token }: { token: string }) { +function SplitsInner() { const [showCreate, setShowCreate] = useState(false); const splits = useQuery({ queryKey: ["revenue-splits"], - queryFn: () => listSplits(token), + queryFn: () => listSplits(), }); return ( @@ -55,7 +53,7 @@ function SplitsInner({ token }: { token: string }) {
{showCreate && ( - setShowCreate(false)} /> + setShowCreate(false)} /> )} @@ -104,7 +102,7 @@ function SplitsInner({ token }: { token: string }) { ); } -function CreateSplitForm({ token, onDone }: { token: string; onDone: () => void }) { +function CreateSplitForm({ onDone }: { onDone: () => void }) { const queryClient = useQueryClient(); const [name, setName] = useState(""); const [description, setDescription] = useState(""); @@ -130,7 +128,7 @@ function CreateSplitForm({ token, onDone }: { token: string; onDone: () => void const create = useMutation({ mutationFn: () => - createSplit(token, { + createSplit({ name, description: description || undefined, recipients: recipients.map((r) => ({ diff --git a/frontend/app/credits/page.tsx b/frontend/app/credits/page.tsx index 50066e8a..3777e76c 100644 --- a/frontend/app/credits/page.tsx +++ b/frontend/app/credits/page.tsx @@ -3,20 +3,28 @@ import { useState } from "react"; import { useQuery } from "@tanstack/react-query"; import { Wallet, ArrowDownLeft, ArrowUpRight, ChevronLeft, ChevronRight } from "lucide-react"; -import Cookies from "js-cookie"; +import { useSessionBootstrap } from "@/lib/stores/use-session-bootstrap"; +import { useSessionStore } from "@/lib/stores/session-store"; +import { UserMenu } from "@/components/auth/user-menu"; import { getCreditBalance, getCreditStatement } from "@/lib/credits-api"; import type { CreditStatementEntry } from "@/lib/credits-api"; const PAGE_SIZE = 20; -function useAccessToken(): string | null { - return Cookies.get("accessToken") ?? null; -} - export default function CreditsPage() { - const token = useAccessToken(); + useSessionBootstrap(); + const user = useSessionStore((state) => state.user); + const sessionStatus = useSessionStore((state) => state.status); - if (!token) { + if (sessionStatus === "unknown") { + return ( +
+

Checking your session...

+
+ ); + } + + if (!user) { return (
@@ -32,20 +40,20 @@ export default function CreditsPage() { ); } - return ; + return ; } -function CreditsInner({ token }: { token: string }) { +function CreditsInner() { const [page, setPage] = useState(1); const balance = useQuery({ queryKey: ["credit-balance"], - queryFn: () => getCreditBalance(token), + queryFn: () => getCreditBalance(), }); const statement = useQuery({ queryKey: ["credit-statement", page], - queryFn: () => getCreditStatement(token, page, PAGE_SIZE), + queryFn: () => getCreditStatement(page, PAGE_SIZE), }); const totalPages = statement.data @@ -54,6 +62,9 @@ function CreditsInner({ token }: { token: string }) { return (
+
+ +
{/* Balance card */}
diff --git a/frontend/app/payments/__tests__/page.test.tsx b/frontend/app/payments/__tests__/page.test.tsx new file mode 100644 index 00000000..74d31259 --- /dev/null +++ b/frontend/app/payments/__tests__/page.test.tsx @@ -0,0 +1,76 @@ +import { render, screen, waitFor } from "@testing-library/react"; +import { QueryClient, QueryClientProvider } from "@tanstack/react-query"; +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; + +import PaymentsPage from "../page"; +import { useSessionStore } from "@/lib/stores/session-store"; + +vi.mock("next/navigation", () => ({ + usePathname: () => "/payments", + useRouter: () => ({ push: vi.fn(), replace: vi.fn() }), +})); + +vi.mock("@/lib/stores/use-session-bootstrap", () => ({ + useSessionBootstrap: () => {}, +})); + +// Mock the API module: the query itself then simulates the rejected session +// that a real 401 produces, so the page shows the raw error instead of +// pretending the request is retryable. +vi.mock("@/lib/payments-api", () => ({ + getPayments: vi + .fn() + .mockRejectedValue(new Error("Request failed (401): Unauthorized")), +})); + +function renderWithClient(ui: React.ReactNode) { + const queryClient = new QueryClient({ + defaultOptions: { queries: { retry: false } }, + }); + return render( + {ui}, + ); +} + +beforeEach(() => { + useSessionStore.setState( + { + user: { id: "u1", email: "a@example.com", role: "user" }, + accessToken: "memory-only", + status: "authenticated", + }, + false, + ); +}); + +afterEach(() => { + useSessionStore.setState( + { user: null, accessToken: null, status: "unknown" }, + false, + ); +}); + +describe("PaymentsPage with a rejected session after reload (issue #1977)", () => { + it("surfaces the failure instead of showing cached or empty data", async () => { + // A stale user in the store (post-reload /auth/me restore) lets the query + // start; the 401 must surface as an error message, not silent emptiness. + renderWithClient(); + await waitFor(() => + expect(screen.getByText(/Unauthorized/)).toBeInTheDocument(), + ); + expect( + screen.queryByText("No payments yet."), + ).not.toBeInTheDocument(); + }); + + it("still asks members to sign in when the store knows no session", () => { + useSessionStore.setState( + { user: null, accessToken: null, status: "unauthenticated" }, + false, + ); + + renderWithClient(); + + expect(screen.getByRole("link", { name: "Sign in" })).toBeInTheDocument(); + }); +}); diff --git a/frontend/app/payments/new/page.tsx b/frontend/app/payments/new/page.tsx index c98d2536..7bc6f233 100644 --- a/frontend/app/payments/new/page.tsx +++ b/frontend/app/payments/new/page.tsx @@ -2,12 +2,10 @@ import { Button, Card, CardHeader, Field, inputClass } from "@/components/app-ui"; import { initiatePayment, type PaymentRail } from "@/lib/payments-api"; -import Cookies from "js-cookie"; import { useState } from "react"; import { toast } from "sonner"; export default function NewPaymentPage() { - const [token] = useState(() => Cookies.get("accessToken")); const [bookingId, setBookingId] = useState(""); const [amount, setAmount] = useState(""); const [currency, setCurrency] = useState("USD"); @@ -18,14 +16,9 @@ export default function NewPaymentPage() { async function onSubmit(e: React.FormEvent) { e.preventDefault(); - if (!token) { - toast.error("You must sign in to make a payment."); - return; - } setSubmitting(true); try { const payment = await initiatePayment( - token, { bookingId, amount: Number(amount), diff --git a/frontend/app/payments/page.tsx b/frontend/app/payments/page.tsx index ad9253f9..e1184ff2 100644 --- a/frontend/app/payments/page.tsx +++ b/frontend/app/payments/page.tsx @@ -3,7 +3,9 @@ import Link from "next/link"; import { useQuery } from "@tanstack/react-query"; import { getPayments } from "@/lib/payments-api"; +import { useSessionBootstrap } from "@/lib/stores/use-session-bootstrap"; import { useSessionStore } from "@/lib/stores/session-store"; +import { UserMenu } from "@/components/auth/user-menu"; import { Card, CardContent, @@ -31,19 +33,31 @@ function statusTone(status: string): string { } export default function PaymentsPage() { - const accessToken = useSessionStore((state) => state.accessToken); - + useSessionBootstrap(); + const user = useSessionStore((state) => state.user); + const sessionStatus = useSessionStore((state) => state.status); const { data: payments, isLoading, error, } = useQuery({ queryKey: ["payments", "list"], - queryFn: () => getPayments(accessToken as string), - enabled: Boolean(accessToken), + queryFn: () => getPayments(), + enabled: Boolean(user), }); - if (!accessToken) { + if (sessionStatus === "unknown") { + return ( +
+

Payments

+

+ Checking your session... +

+
+ ); + } + + if (!user) { return (

Payments

@@ -63,7 +77,10 @@ export default function PaymentsPage() { return (
-

Payments

+
+

Payments

+ +
{isLoading && ( diff --git a/frontend/app/payments/return/page.tsx b/frontend/app/payments/return/page.tsx index e15d5f29..2c41e7a6 100644 --- a/frontend/app/payments/return/page.tsx +++ b/frontend/app/payments/return/page.tsx @@ -26,6 +26,7 @@ import Link from "next/link"; import { verifyPaymentReturn, getPayment } from "@/lib/payments-api"; import type { Payment } from "@/lib/payments-api"; import { useSessionStore } from "@/lib/stores/session-store"; +import { useSessionBootstrap } from "@/lib/stores/use-session-bootstrap"; import { Card, CardContent, CardHeader, CardTitle } from "@/components/ui/card"; import { Button } from "@/components/ui/button"; import { getProviderReturnErrorMessage } from "@/lib/payment-return-errors"; @@ -82,7 +83,9 @@ function statusTone(status: string): string { export default function PaymentReturnPage() { const router = useRouter(); const searchParams = useSearchParams(); - const accessToken = useSessionStore((state) => state.accessToken); + useSessionBootstrap(); + const user = useSessionStore((state) => state.user); + const sessionStatus = useSessionStore((state) => state.status); const paymentId = searchParams.get("paymentId") ?? searchParams.get("payment_id") ?? ""; @@ -121,7 +124,7 @@ export default function PaymentReturnPage() { // -- polling loop -------------------------------------------------------- const startPolling = useCallback( - (id: string, token: string) => { + (id: string) => { let attempt = 0; setPhase("polling"); @@ -129,7 +132,7 @@ export default function PaymentReturnPage() { attempt += 1; setPollCount(attempt); - getPayment(id, token) + getPayment(id) .then((p) => { setPayment(p); if (terminalStatus(p.status)) { @@ -156,7 +159,7 @@ export default function PaymentReturnPage() { // -- fast path ----------------------------------------------------------- useEffect(() => { - if (providerErrorMessage || !paymentId || !accessToken) return; + if (providerErrorMessage || !paymentId || !user || sessionStatus !== "authenticated") return; const ctrl = new AbortController(); abortRef.current = ctrl; @@ -164,10 +167,10 @@ export default function PaymentReturnPage() { // Arm the timeout that triggers the polling fallback const timeoutId = setTimeout(() => { ctrl.abort(); - startPolling(paymentId, accessToken as string); + startPolling(paymentId); }, FAST_PATH_TIMEOUT_MS); - verifyPaymentReturn(accessToken as string, paymentId) + verifyPaymentReturn(paymentId) .then((result) => { clearTimeout(timeoutId); if (ctrl.signal.aborted) return; // timeout already fired, ignore @@ -176,7 +179,7 @@ export default function PaymentReturnPage() { setPhase("success"); } else { // Not yet in a terminal state � fall through to polling - startPolling(paymentId, accessToken as string); + startPolling(paymentId); } }) .catch((err: unknown) => { @@ -187,7 +190,7 @@ export default function PaymentReturnPage() { "[verify-return] fast path failed, falling back to polling:", err, ); - startPolling(paymentId, accessToken as string); + startPolling(paymentId); }); return () => { @@ -195,7 +198,7 @@ export default function PaymentReturnPage() { ctrl.abort(); }; // eslint-disable-next-line react-hooks/exhaustive-deps - }, [paymentId, accessToken, providerErrorMessage]); + }, [paymentId, user, sessionStatus, providerErrorMessage]); // -- render -------------------------------------------------------------- @@ -229,7 +232,7 @@ export default function PaymentReturnPage() { ); } - if (!accessToken) { + if (!user) { return (

Verifying Payment

diff --git a/frontend/app/usage/page.tsx b/frontend/app/usage/page.tsx index 2de74440..1600a1ad 100644 --- a/frontend/app/usage/page.tsx +++ b/frontend/app/usage/page.tsx @@ -3,7 +3,9 @@ import { Card, CardHeader, StatusBadge } from "@/components/app-ui"; import { listMyUsage, type MeteredUsageEvent } from "@/lib/payments-api"; import { formatCurrency } from "@/lib/utils"; -import Cookies from "js-cookie"; +import { useSessionBootstrap } from "@/lib/stores/use-session-bootstrap"; +import { useSessionStore } from "@/lib/stores/session-store"; +import { UserMenu } from "@/components/auth/user-menu"; import { useQuery } from "@tanstack/react-query"; function formatMoney(amount: number, currency: string) { @@ -11,19 +13,26 @@ function formatMoney(amount: number, currency: string) { } export default function UsagePage() { - const token = typeof window !== "undefined" ? Cookies.get("accessToken") : null; + useSessionBootstrap(); + const user = useSessionStore((state) => state.user); + const sessionStatus = useSessionStore((state) => state.status); const { data, isLoading, error } = useQuery({ queryKey: ["my-usage"], - queryFn: () => listMyUsage(token!), - enabled: !!token, + queryFn: () => listMyUsage(), + enabled: Boolean(user), }); return (
+
+ +
- {!token ? ( + {sessionStatus === "unknown" ? ( +

Checking your session...

+ ) : !user ? (

Sign in to view usage.

) : isLoading ? (

Loading…

diff --git a/frontend/app/wallet/page.tsx b/frontend/app/wallet/page.tsx index 9cb0807a..d92ca87e 100644 --- a/frontend/app/wallet/page.tsx +++ b/frontend/app/wallet/page.tsx @@ -1,11 +1,15 @@ "use client"; import { WalletStatusCard } from "@/components/wallet/wallet-status-card"; +import { UserMenu } from "@/components/auth/user-menu"; export default function WalletPage() { return (
-

Wallet

+
+

Wallet

+ +
); diff --git a/frontend/components/admin/__tests__/require-admin.test.tsx b/frontend/components/admin/__tests__/require-admin.test.tsx index faecbf4d..0647694e 100644 --- a/frontend/components/admin/__tests__/require-admin.test.tsx +++ b/frontend/components/admin/__tests__/require-admin.test.tsx @@ -1,18 +1,33 @@ import { render, screen, waitFor } from "@testing-library/react"; -import { beforeEach, describe, expect, it, vi } from "vitest"; -import Cookies from "js-cookie"; +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; import { usePathname, useRouter } from "next/navigation"; -import { RequireAdmin } from "../require-admin"; -vi.mock("js-cookie", () => ({ - default: { get: vi.fn() }, -})); +import { RequireAdmin } from "../require-admin"; +import { useSessionStore } from "@/lib/stores/session-store"; vi.mock("next/navigation", () => ({ usePathname: vi.fn(), useRouter: vi.fn(), })); +vi.mock("@/lib/stores/use-session-bootstrap", () => ({ + useSessionBootstrap: () => {}, +})); + +/** + * JWT_SECRET verification works server-side only, so the client relies on + * `GET /auth/me` to learn the signed-in user (and role). The bootstrap hook is + * mocked above, and each test seeds the real zustand store directly so the + * store's status machine drives the component. + */ +function seedSession(state: { + user: { id: string; email: string; role: string } | null; + accessToken: string | null; + status: "unknown" | "authenticated" | "unauthenticated"; +}): void { + useSessionStore.setState(state, false); +} + const replace = vi.fn(); beforeEach(() => { @@ -24,9 +39,53 @@ beforeEach(() => { } as unknown as ReturnType); }); -describe("RequireAdmin", () => { +afterEach(() => { + useSessionStore.setState( + { user: null, accessToken: null, status: "unknown" }, + false, + ); +}); + +describe("RequireAdmin role gate (issue #1973)", () => { + it("renders protected content for an admin user", () => { + seedSession({ + user: { id: "u1", email: "a@example.com", role: "admin" }, + accessToken: "not-readable-from-cookie", + status: "authenticated", + }); + + render( + +
Protected content
+
, + ); + + expect(screen.getByText("Protected content")).toBeInTheDocument(); + expect(replace).not.toHaveBeenCalled(); + }); + + it("shows a not-authorised state for a member-role user", () => { + seedSession({ + user: { id: "u2", email: "bill@example.com", role: "user" }, + accessToken: "not-readable-from-cookie", + status: "authenticated", + }); + + render( + +
Protected content
+
, + ); + + expect( + screen.getByRole("heading", { name: "Not authorised" }), + ).toBeInTheDocument(); + expect(screen.queryByText("Protected content")).not.toBeInTheDocument(); + expect(replace).not.toHaveBeenCalled(); + }); + it("redirects an unauthenticated visitor with the current destination", async () => { - vi.mocked(Cookies.get).mockReturnValue(undefined); + seedSession({ user: null, accessToken: null, status: "unauthenticated" }); render( @@ -34,23 +93,38 @@ describe("RequireAdmin", () => { , ); - const loginUrl = - "/login?returnTo=%2Fadmin%2Faccounts%3Ftab%3Dfrozen"; + const loginUrl = "/login?returnTo=%2Fadmin%2Faccounts%3Ftab%3Dfrozen"; const loginLink = screen.getByRole("link", { name: "Sign in to continue" }); await waitFor(() => expect(loginLink).toHaveAttribute("href", loginUrl)); await waitFor(() => expect(replace).toHaveBeenCalledWith(loginUrl)); }); - it("renders protected content when an access token is present", () => { - vi.mocked(Cookies.get).mockReturnValue("access-token"); + it("shows a transient checking state while the session is unknown", async () => { + // Simulate /auth/me resolving to a member shortly after mount. + seedSession({ user: null, accessToken: null, status: "unknown" }); - render( + const { rerender } = render(
Protected content
, ); - expect(screen.getByText("Protected content")).toBeInTheDocument(); - expect(replace).not.toHaveBeenCalled(); + expect(screen.getByText("Checking your session...")).toBeInTheDocument(); + + useSessionStore.setState({ + user: { id: "u2", email: "bill@example.com", role: "user" }, + accessToken: "not-readable-from-cookie", + status: "authenticated", + }); + rerender( + +
Protected content
+
, + ); + + expect( + screen.getByRole("heading", { name: "Not authorised" }), + ).toBeInTheDocument(); + expect(screen.queryByText("Protected content")).not.toBeInTheDocument(); }); }); diff --git a/frontend/components/admin/require-admin.tsx b/frontend/components/admin/require-admin.tsx index 9136e0af..b4d56724 100644 --- a/frontend/components/admin/require-admin.tsx +++ b/frontend/components/admin/require-admin.tsx @@ -1,18 +1,27 @@ "use client"; -import Cookies from "js-cookie"; import Link from "next/link"; import { useEffect, useState } from "react"; -import { ShieldAlert } from "lucide-react"; +import { Loader2, ShieldAlert } from "lucide-react"; import { usePathname, useRouter } from "next/navigation"; import { buildReturnTo, getSafeReturnTo } from "@/lib/auth-redirect"; +import { useSessionBootstrap } from "@/lib/stores/use-session-bootstrap"; +import { useSessionStore } from "@/lib/stores/session-store"; -export function useAdminToken(): string | null { - return Cookies.get("accessToken") ?? null; +/** + * An account may open ADMIN-gated API endpoints only with an admin role + * (issue #1973). The role comes from `GET /auth/me` (restored through the + * session store, never from `document.cookie`); the middleware enforces the + * same check server-side, this component is the client-side backstop. + */ +export function isAdminRole(role: string | null | undefined): boolean { + return (role ?? "").toLowerCase() === "admin"; } export function RequireAdmin({ children }: { children: React.ReactNode }) { - const token = useAdminToken(); + useSessionBootstrap(); + const user = useSessionStore((state) => state.user); + const status = useSessionStore((state) => state.status); const pathname = usePathname(); const router = useRouter(); const [returnTo, setReturnTo] = useState(pathname); @@ -23,12 +32,24 @@ export function RequireAdmin({ children }: { children: React.ReactNode }) { buildReturnTo(pathname, window.location.search.replace(/^\?/, "")), ); setReturnTo(currentReturnTo); - if (!token) { + if (status === "unauthenticated") { router.replace(`/login?returnTo=${encodeURIComponent(currentReturnTo)}`); } - }, [pathname, router, token]); + }, [pathname, router, status]); - if (!token) { + if (status === "unknown") { + return ( +
+ + Checking your session... +
+ ); + } + + if (status === "unauthenticated" || !user) { return (
@@ -37,7 +58,7 @@ export function RequireAdmin({ children }: { children: React.ReactNode }) {

These pages call ADMIN-gated API endpoints. Sign in with an admin - account so an accessToken cookie is present. + account to continue.

); } + + if (!isAdminRole(user.role)) { + return ( +
+ +

+ Not authorised +

+

+ Your account is signed in as a member, but this area requires the + admin role. Ask an administrator to upgrade your account. +

+ + Back to your wallet + +
+ ); + } + return <>{children}; } diff --git a/frontend/components/auth/auth-form.test.tsx b/frontend/components/auth/auth-form.test.tsx index 9b5009a1..0d77303e 100644 --- a/frontend/components/auth/auth-form.test.tsx +++ b/frontend/components/auth/auth-form.test.tsx @@ -5,7 +5,7 @@ import { beforeEach, describe, expect, it, vi } from "vitest"; const mocks = vi.hoisted(() => ({ searchParams: new URLSearchParams(), push: vi.fn(), - setAccessToken: vi.fn(), + setSession: vi.fn(), login: vi.fn(), register: vi.fn(), })); @@ -17,8 +17,8 @@ vi.mock("next/navigation", () => ({ vi.mock("@/lib/stores/session-store", () => ({ useSessionStore: ( - selector: (state: { setAccessToken: typeof mocks.setAccessToken }) => unknown, - ) => selector({ setAccessToken: mocks.setAccessToken }), + selector: (state: { setSession: typeof mocks.setSession }) => unknown, + ) => selector({ setSession: mocks.setSession }), })); vi.mock("@/lib/auth-api", () => ({ @@ -60,7 +60,7 @@ describe("AuthForm registration confirmation", () => { const user = userEvent.setup(); mocks.register.mockResolvedValue({ accessToken: "access-token", - user: { id: "user-1", email: "person@example.com", role: "MEMBER" }, + user: { id: "user-1", email: "person@example.com", role: "user" }, }); renderForm("register"); @@ -75,7 +75,10 @@ describe("AuthForm registration confirmation", () => { "password-one", ), ); - expect(mocks.setAccessToken).toHaveBeenCalledWith("access-token"); + expect(mocks.setSession).toHaveBeenCalledWith( + { id: "user-1", email: "person@example.com", role: "user" }, + "access-token", + ); expect(mocks.push).toHaveBeenCalledWith("/wallet"); }); }); diff --git a/frontend/components/auth/auth-form.tsx b/frontend/components/auth/auth-form.tsx index 0a85f64f..fa7065a9 100644 --- a/frontend/components/auth/auth-form.tsx +++ b/frontend/components/auth/auth-form.tsx @@ -12,7 +12,7 @@ type Mode = "login" | "register"; export function AuthForm({ mode }: { mode: Mode }) { const router = useRouter(); const searchParams = useSearchParams(); - const setAccessToken = useSessionStore((state) => state.setAccessToken); + const setSession = useSessionStore((state) => state.setSession); const [email, setEmail] = useState(""); const [password, setPassword] = useState(""); const [confirmPassword, setConfirmPassword] = useState(""); @@ -35,7 +35,9 @@ export function AuthForm({ mode }: { mode: Mode }) { const result = isLogin ? await login(email.trim(), password) : await register(email.trim(), password); - setAccessToken(result.accessToken); + // The backend already set its HttpOnly cookie for this response; the + // token surname in the body is kept in memory only (session-store). + setSession(result.user, result.accessToken); const returnTo = getSafeReturnTo( searchParams.get("returnTo") ?? searchParams.get("next"), ); diff --git a/frontend/components/auth/use-sign-out.ts b/frontend/components/auth/use-sign-out.ts new file mode 100644 index 00000000..91df99da --- /dev/null +++ b/frontend/components/auth/use-sign-out.ts @@ -0,0 +1,24 @@ +"use client"; + +import { useRouter } from "next/navigation"; +import { useQueryClient } from "@tanstack/react-query"; +import { useSessionStore } from "@/lib/stores/session-store"; +import { logout } from "@/lib/auth-api"; + +/** + * Shared sign-out flow (issue #1974): call `POST /auth/logout` with + * credentials (the backend clears the HttpOnly cookie and revokes every + * outstanding token), then drop the local session and the React Query cache + * so no signed-in data survives, and send the user to the login page. + */ +export function useSignOut(): () => Promise { + const router = useRouter(); + const queryClient = useQueryClient(); + + return async () => { + await logout(); + queryClient.clear(); + useSessionStore.getState().clearSession(); + router.replace("/login"); + }; +} diff --git a/frontend/components/auth/user-menu.tsx b/frontend/components/auth/user-menu.tsx new file mode 100644 index 00000000..d84df2ff --- /dev/null +++ b/frontend/components/auth/user-menu.tsx @@ -0,0 +1,48 @@ +"use client"; + +import { useState } from "react"; +import { LogOut } from "lucide-react"; +import { useSessionStore } from "@/lib/stores/session-store"; +import { useSignOut } from "@/components/auth/use-sign-out"; + +/** + * Small identity chip with a Sign out control (issue #1974). Rendered in the + * header of every authenticated surface - member pages and the admin layout - + * so signing out is available wherever the user is working. + */ +export function UserMenu() { + const user = useSessionStore((state) => state.user); + const signOut = useSignOut(); + const [busy, setBusy] = useState(false); + + if (!user) return null; + + const handleSignOut = async () => { + setBusy(true); + try { + await signOut(); + } finally { + setBusy(false); + } + }; + + return ( +
+ + {user.email} + + +
+ ); +} diff --git a/frontend/components/wallet/wallet-status-card.tsx b/frontend/components/wallet/wallet-status-card.tsx index d2da4137..d310af10 100644 --- a/frontend/components/wallet/wallet-status-card.tsx +++ b/frontend/components/wallet/wallet-status-card.tsx @@ -12,7 +12,7 @@ import { import { Card, CardContent, CardHeader, CardTitle } from "@/components/ui/card"; import { Button } from "@/components/ui/button"; import { WalletLinkForm } from "@/components/wallet/wallet-link-form"; -import { useSessionStore } from "@/lib/stores/session-store"; +import { useSignOut } from "@/components/auth/use-sign-out"; /** * Minor-unit decimal places per currency. `GET /credits/balance` is @@ -54,7 +54,7 @@ export function formatBalance(minorUnits: number, currency: string): string { */ export function WalletStatusCard() { const queryClient = useQueryClient(); - const setAccessToken = useSessionStore((state) => state.setAccessToken); + const signOut = useSignOut(); const [linking, setLinking] = useState(false); const [nonce, setNonce] = useState(null); @@ -140,7 +140,7 @@ export function WalletStatusCard() { Your balance - diff --git a/frontend/lib/__tests__/api-401.test.ts b/frontend/lib/__tests__/api-401.test.ts new file mode 100644 index 00000000..e83cf132 --- /dev/null +++ b/frontend/lib/__tests__/api-401.test.ts @@ -0,0 +1,130 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; +import { toast } from "sonner"; + +import { useSessionStore } from "../stores/session-store"; + +vi.mock("sonner", () => ({ + toast: { + error: vi.fn(), + }, +})); + +/** + * Issue #1977: any 401 from a protected endpoint must clear the session and + * send the user to login. Every shared helper wires this centrally via + * lib/session-expiry. Each case imports a FRESH module graph because the + * expiry redirect flag lives at module level (parallel 401s coalesce into a + * single redirect), which would otherwise mute later cases. + */ +const CASES: Array<{ + name: string; + modulePath: string; + update: (m: Record) => Promise; +}> = [ + { + name: "wallet-api", + modulePath: "../wallet-api", + update: (m) => (m as { getWalletStatus(): unknown }).getWalletStatus(), + }, + { + name: "credits-api", + modulePath: "../credits-api", + update: (m) => (m as { getCreditBalance(): unknown }).getCreditBalance(), + }, + { + name: "admin-api queries", + modulePath: "../admin-api", + update: (m) => (m as { listAccounts(): unknown }).listAccounts(), + }, + { + name: "admin-api mutations", + modulePath: "../admin-api", + update: (m) => (m as { listSplits(): unknown }).listSplits(), + }, + { + name: "payments-api queries", + modulePath: "../payments-api", + update: (m) => (m as { getPayments(): unknown }).getPayments(), + }, + { + name: "payments-api detail", + modulePath: "../payments-api", + update: (m) => (m as { getPayment(id: string): unknown }).getPayment("pay_1"), + }, + { + name: "payments-api usage", + modulePath: "../payments-api", + update: (m) => (m as { listMyUsage(): unknown }).listMyUsage(), + }, +]; + +describe("401 reactions across shared helpers (issue #1977)", () => { + const assignSpy = vi.fn(); + + beforeEach(() => { + vi.clearAllMocks(); + useSessionStore + .getState() + .setSession({ id: "u1", email: "a@example.com", role: "admin" }); + // jsdom does not implement navigation: observe instead. + Object.defineProperty(window, "location", { + writable: true, + value: { + pathname: "/wallet", + search: "", + replace: assignSpy, + }, + }); + }); + + afterEach(() => { + useSessionStore.getState().clearSession(); + vi.unstubAllGlobals(); + }); + + for (const { name, modulePath, update } of CASES) { + it(`${name} clears the session and redirects to login on 401`, async () => { + vi.resetModules(); + const api = await import(modulePath); + + const fetchMock = vi.fn().mockResolvedValue( + new Response(JSON.stringify({ message: "Unauthorized" }), { + status: 401, + headers: { "Content-Type": "application/json" }, + }), + ); + vi.stubGlobal("fetch", fetchMock); + + await expect(update(api)).rejects.toThrow(); + + expect(toast.error).toHaveBeenCalledWith( + "Your session has expired. Please sign in again.", + ); + expect(useSessionStore.getState().user).toBeNull(); + expect(useSessionStore.getState().status).toBe("unauthenticated"); + expect(assignSpy).toHaveBeenCalledTimes(1); + expect(assignSpy).toHaveBeenCalledWith("/login?returnTo=%2Fwallet"); + }, 100); + } + + it("does not react on non-401 errors", async () => { + vi.resetModules(); + const { getWalletStatus } = await import("../wallet-api"); + + vi.stubGlobal( + "fetch", + vi.fn().mockResolvedValue( + new Response(JSON.stringify({ message: "Wallet not found" }), { + status: 404, + headers: { "Content-Type": "application/json" }, + }), + ), + ); + + await expect(getWalletStatus()).rejects.toThrow("Wallet not found"); + + expect(toast.error).not.toHaveBeenCalled(); + expect(useSessionStore.getState().user).not.toBeNull(); + expect(assignSpy).not.toHaveBeenCalled(); + }, 100); +}); diff --git a/frontend/lib/__tests__/auth-api.test.ts b/frontend/lib/__tests__/auth-api.test.ts new file mode 100644 index 00000000..77550e57 --- /dev/null +++ b/frontend/lib/__tests__/auth-api.test.ts @@ -0,0 +1,105 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; +import { toast } from "sonner"; + +import { login, register, me, logout } from "../auth-api"; +import { useSessionStore } from "../stores/session-store"; + +vi.mock("sonner", () => ({ + toast: { + error: vi.fn(), + }, +})); + +function jsonResponse(status: number, body: unknown): Response { + return new Response(JSON.stringify(body), { + status, + headers: { "Content-Type": "application/json" }, + }); +} + +describe("auth-api (cookie session model, issue #1975)", () => { + let fetchMock: ReturnType; + + beforeEach(() => { + fetchMock = vi.fn(); + vi.stubGlobal("fetch", fetchMock); + useSessionStore.getState().clearSession(); + }); + + afterEach(() => { + vi.unstubAllGlobals(); + vi.clearAllMocks(); + }); + + it("sends credentials so the backend's HttpOnly cookie is set", async () => { + fetchMock.mockResolvedValue( + jsonResponse(200, { + accessToken: "tok", + user: { id: "u1", email: "a@example.com", role: "user" }, + }), + ); + + await login("a@example.com", "pw"); + + const [, init] = fetchMock.mock.calls[0]; + expect(init.credentials).toBe("include"); + expect(init.headers).toMatchObject({ "Content-Type": "application/json" }); + }); + + it("keeps the invalid-credentials copy for a login 401 without redirecting", async () => { + fetchMock.mockResolvedValue(jsonResponse(401, { message: "Unauthorized" })); + + await expect(login("a@example.com", "pw")).rejects.toThrow( + "Invalid credentials", + ); + // A 401 on the login endpoint is bad credentials, not an expired session. + expect(toast.error).not.toHaveBeenCalled(); + const [, init] = fetchMock.mock.calls[0]; + expect(init.method).toBe("POST"); + }); + + it("restores the session from GET /auth/me and is single-flight", async () => { + fetchMock.mockResolvedValue( + jsonResponse(200, { id: "u1", email: "a@example.com", role: "admin" }), + ); + + const [restored, restoredAgain] = await Promise.all([me(), me()]); + + expect(restored).toEqual({ + id: "u1", + email: "a@example.com", + role: "admin", + }); + expect(restoredAgain).toEqual(restored); + expect(fetchMock).toHaveBeenCalledTimes(1); + const [url, init] = fetchMock.mock.calls[0]; + expect(String(url)).toContain("/auth/me"); + expect(init.credentials).toBe("include"); + }); + + it("reports a rejected 401 session and clears the store for /auth/me", async () => { + fetchMock.mockResolvedValue(jsonResponse(401, { message: "Unauthorized" })); + + await expect(me()).rejects.toThrow("Unauthorized"); + + expect(useSessionStore.getState().user).toBeNull(); + expect(toast.error).toHaveBeenCalledWith( + "Your session has expired. Please sign in again.", + ); + }); + + it("logout posts to /auth/logout with credentials and never throws", async () => { + fetchMock.mockResolvedValue(jsonResponse(200, null)); + + await logout(); + await expect(logout()).resolves.toBeUndefined(); + + const calls = fetchMock.mock.calls; + for (const call of calls) { + const [url, init] = call; + expect(String(url)).toContain("/auth/logout"); + expect(init.method).toBe("POST"); + expect(init.credentials).toBe("include"); + } + }); +}); diff --git a/frontend/lib/__tests__/credits-api.test.ts b/frontend/lib/__tests__/credits-api.test.ts index 445341bf..1b26479b 100644 --- a/frontend/lib/__tests__/credits-api.test.ts +++ b/frontend/lib/__tests__/credits-api.test.ts @@ -38,7 +38,7 @@ describe("credits-api", () => { let caught: unknown; try { - await getCreditBalance("access-token"); + await getCreditBalance(); } catch (error) { caught = error; } @@ -60,7 +60,7 @@ describe("credits-api", () => { let caught: unknown; try { - await getCreditStatement("access-token", 2, 10); + await getCreditStatement(2, 10); } catch (error) { caught = error; } @@ -75,17 +75,17 @@ describe("credits-api", () => { }); }); - it("preserves the access token and statement query parameters", async () => { + it("preserves the statement query parameters", async () => { fetchMock.mockResolvedValue( jsonResponse(200, { entries: [], total: 0, page: 2, pageSize: 10 }), ); - await getCreditStatement("access-token", 2, 10); + await getCreditStatement(2, 10); const [url, init] = fetchMock.mock.calls[0]; expect(String(url)).toContain("/credits/statement?page=2&pageSize=10"); + expect(init.credentials).toBe("include"); expect(init.headers).toMatchObject({ - Authorization: "Bearer access-token", "Content-Type": "application/json", }); }); diff --git a/frontend/lib/__tests__/payment-return-page.test.tsx b/frontend/lib/__tests__/payment-return-page.test.tsx index 2cd9aecf..5ab3396b 100644 --- a/frontend/lib/__tests__/payment-return-page.test.tsx +++ b/frontend/lib/__tests__/payment-return-page.test.tsx @@ -15,9 +15,23 @@ vi.mock("next/navigation", () => ({ useSearchParams: () => mocks.searchParams, })); +vi.mock("@/lib/stores/use-session-bootstrap", () => ({ + useSessionBootstrap: () => {}, +})); + vi.mock("@/lib/stores/session-store", () => ({ - useSessionStore: (selector: (state: { accessToken: string }) => unknown) => - selector({ accessToken: "token" }), + useSessionStore: ( + selector: ( + state: { + user: { id: string; email: string; role: string }; + status: string; + }, + ) => unknown, + ) => + selector({ + user: { id: "user-1", email: "a@example.com", role: "user" }, + status: "authenticated", + }), })); vi.mock("@/lib/payments-api", () => ({ diff --git a/frontend/lib/__tests__/payments-api.test.ts b/frontend/lib/__tests__/payments-api.test.ts index a1793212..585e61b6 100644 --- a/frontend/lib/__tests__/payments-api.test.ts +++ b/frontend/lib/__tests__/payments-api.test.ts @@ -33,7 +33,7 @@ describe("payments-api retries", () => { .mockRejectedValueOnce(new TypeError("Network unavailable")) .mockResolvedValueOnce(jsonResponse(200, [])); - const resultPromise = getPayments("access-token"); + const resultPromise = getPayments(); await Promise.resolve(); await vi.advanceTimersByTimeAsync(250); @@ -42,7 +42,6 @@ describe("payments-api retries", () => { const [url, init] = fetchMock.mock.calls[0]; expect(String(url)).toContain("/payments"); expect(init.headers).toMatchObject({ - Authorization: "Bearer access-token", "Content-Type": "application/json", }); }); @@ -52,7 +51,7 @@ describe("payments-api retries", () => { .mockResolvedValueOnce(jsonResponse(503, { message: "Unavailable" })) .mockResolvedValueOnce(jsonResponse(200, [])); - const resultPromise = getPayments("access-token"); + const resultPromise = getPayments(); await Promise.resolve(); await vi.advanceTimersByTimeAsync(250); @@ -65,7 +64,6 @@ describe("payments-api retries", () => { await expect( initiatePayment( - "access-token", { bookingId: "booking-1", amount: 100, diff --git a/frontend/lib/__tests__/session-expiry.test.ts b/frontend/lib/__tests__/session-expiry.test.ts new file mode 100644 index 00000000..075c2896 --- /dev/null +++ b/frontend/lib/__tests__/session-expiry.test.ts @@ -0,0 +1,91 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; +import { toast } from "sonner"; + +import { useSessionStore } from "../stores/session-store"; + +vi.mock("sonner", () => ({ + toast: { + error: vi.fn(), + }, +})); + +/** + * The expiry flag lives at module level (parallel 401s must coalesce), so each + * test case imports a fresh instance of the module through vi.resetModules(). + */ +async function loadExpiry() { + vi.resetModules(); + return await import("../session-expiry"); +} + +describe("session expiry reactions (issue #1977)", () => { + const assignSpy = vi.fn(); + + beforeEach(() => { + vi.clearAllMocks(); + useSessionStore + .getState() + .setSession({ id: "u1", email: "a@example.com", role: "user" }); + assignSpy.mockClear(); + // jsdom does not implement navigation: observe it instead. + Object.defineProperty(window, "location", { + writable: true, + value: { + pathname: "/admin/accounts", + search: "?tab=frozen", + replace: assignSpy, + }, + }); + }); + + afterEach(() => { + useSessionStore.getState().clearSession(); + }); + + it("clears the session, shows a toast and redirects to login once", async () => { + const { expireSession } = await loadExpiry(); + + expireSession(); + + expect(useSessionStore.getState().user).toBeNull(); + expect(useSessionStore.getState().status).toBe("unauthenticated"); + expect(toast.error).toHaveBeenCalledWith( + "Your session has expired. Please sign in again.", + ); + expect(assignSpy).toHaveBeenCalledTimes(1); + expect(assignSpy).toHaveBeenCalledWith( + "/login?returnTo=%2Fadmin%2Faccounts%3Ftab%3Dfrozen", + ); + }); + + it("coalesces parallel 401s into a single redirect (no loops)", async () => { + const { expireSession } = await loadExpiry(); + + expireSession(); + expireSession(); + expireSession(); + + expect(assignSpy).toHaveBeenCalledTimes(1); + expect(toast.error).toHaveBeenCalledTimes(1); + }); + + it("does nothing extra when the user is already on an auth page", async () => { + const { expireSession } = await loadExpiry(); + Object.assign(window.location, { pathname: "/login", search: "" }); + + expireSession(); + + expect(assignSpy).not.toHaveBeenCalled(); + // The stale session is still dropped so the login page shows logged-out UI. + expect(useSessionStore.getState().user).toBeNull(); + }); + + it("redirects carry only the return path, never credentials or body data", async () => { + const { expireSession } = await loadExpiry(); + Object.assign(window.location, { pathname: "/wallet", search: "" }); + + expireSession(); + + expect(assignSpy).toHaveBeenCalledWith("/login?returnTo=%2Fwallet"); + }); +}); diff --git a/frontend/lib/__tests__/wallet-api.test.ts b/frontend/lib/__tests__/wallet-api.test.ts index 0b61094f..f511a4b4 100644 --- a/frontend/lib/__tests__/wallet-api.test.ts +++ b/frontend/lib/__tests__/wallet-api.test.ts @@ -53,7 +53,7 @@ describe("wallet-api", () => { expect(result).toEqual(body); const [url, init] = fetchMock.mock.calls[0]; expect(String(url)).toContain("/wallets/me"); - expect(init).toMatchObject({ credentials: "same-origin" }); + expect(init).toMatchObject({ credentials: "include" }); expect(init.headers).toMatchObject({ "Content-Type": "application/json" }); }); }); diff --git a/frontend/lib/admin-api.ts b/frontend/lib/admin-api.ts index fe11a63c..08526ad0 100644 --- a/frontend/lib/admin-api.ts +++ b/frontend/lib/admin-api.ts @@ -5,6 +5,7 @@ import { getRequestId, reportApiError, } from "@/lib/api-report"; +import { expireSession } from "@/lib/session-expiry"; export type LedgerAccountKind = | "USER" @@ -159,19 +160,25 @@ const API_BASE_URL = process.env.NEXT_PUBLIC_API_URL ?? ""; async function adminFetch( path: string, - accessToken: string, init?: RequestInit, ): Promise { const response = await fetch(`${API_BASE_URL}${path}`, { ...init, headers: { "Content-Type": "application/json", - Authorization: `Bearer ${accessToken}`, ...init?.headers, }, + // The backend authorizes via its HttpOnly `accessToken` cookie (issue + // #1975); no bearer header is threaded from the client. + credentials: "include", }); if (!response.ok) { + if (response.status === 401) { + // A rejected session must send the user to login, not leave the stale + // state in place (issue #1977). + expireSession(); + } const body = await response.json().catch(() => null); const error = new Error( getApiErrorMessage(body, `Admin request failed (${response.status})`), @@ -197,29 +204,26 @@ async function adminFetch( // ── ledger accounts (FE-111) ───────────────────────────────────────────── export function listAccounts( - accessToken: string, currency?: string, ): Promise { const query = currency ? `?currency=${encodeURIComponent(currency)}` : ""; - return adminFetch(`/credits/admin/accounts${query}`, accessToken); + return adminFetch(`/credits/admin/accounts${query}`); } export function createAccount( - accessToken: string, input: CreateLedgerAccountInput, ): Promise { - return adminFetch("/credits/admin/accounts", accessToken, { + return adminFetch("/credits/admin/accounts", { method: "POST", body: JSON.stringify(input), }); } export function updateAccount( - accessToken: string, id: string, input: UpdateLedgerAccountInput, ): Promise { - return adminFetch(`/credits/admin/accounts/${id}`, accessToken, { + return adminFetch(`/credits/admin/accounts/${id}`, { method: "PATCH", body: JSON.stringify(input), }); @@ -228,66 +232,58 @@ export function updateAccount( // ── ledger integrity (FE-112) ──────────────────────────────────────────── export function checkIntegrity( - accessToken: string, currency?: string, ): Promise { const query = currency ? `?currency=${encodeURIComponent(currency)}` : ""; return adminFetch( `/credits/admin/ledger/integrity${query}`, - accessToken, ); } // ── revenue splits (FE-113) ────────────────────────────────────────────── -export function listSplits(accessToken: string): Promise { - return adminFetch("/credits/admin/splits", accessToken); +export function listSplits(): Promise { + return adminFetch("/credits/admin/splits"); } export function createSplit( - accessToken: string, input: { name: string; description?: string; recipients: SplitRecipientInput[]; }, ): Promise { - return adminFetch("/credits/admin/splits", accessToken, { + return adminFetch("/credits/admin/splits", { method: "POST", body: JSON.stringify(input), }); } export function setSplitActive( - accessToken: string, id: string, active: boolean, ): Promise { return adminFetch( `/credits/admin/splits/${id}/active`, - accessToken, { method: "POST", body: JSON.stringify({ active }) }, ); } export function replaceSplitRecipients( - accessToken: string, id: string, recipients: SplitRecipientInput[], ): Promise { return adminFetch( `/credits/admin/splits/${id}/recipients`, - accessToken, { method: "PUT", body: JSON.stringify({ recipients }) }, ); } export function previewSplit( - accessToken: string, configId: string, amount: number, ): Promise { - return adminFetch("/credits/admin/splits/preview", accessToken, { + return adminFetch("/credits/admin/splits/preview", { method: "POST", body: JSON.stringify({ configId, amount }), }); @@ -295,63 +291,53 @@ export function previewSplit( // ── settlement (FE-114) ────────────────────────────────────────────────── -export function runSettlement(accessToken: string): Promise { - return adminFetch("/credits/admin/settlement/run", accessToken, { +export function runSettlement(): Promise { + return adminFetch("/credits/admin/settlement/run", { method: "POST", }); } export function listBatches( - accessToken: string, status?: SettlementBatchStatus, ): Promise { const query = status ? `?status=${encodeURIComponent(status)}` : ""; return adminFetch( `/credits/admin/settlement/batches${query}`, - accessToken, ); } export function getBatchBreakdown( - accessToken: string, id: string, ): Promise { return adminFetch( `/credits/admin/settlement/batches/${id}`, - accessToken, ); } export function executeBatch( - accessToken: string, id: string, ): Promise { return adminFetch( `/credits/admin/settlement/batches/${id}/execute`, - accessToken, { method: "POST" }, ); } export function retryBatch( - accessToken: string, id: string, ): Promise { return adminFetch( `/credits/admin/settlement/batches/${id}/retry`, - accessToken, { method: "POST" }, ); } export function abandonBatch( - accessToken: string, id: string, reason: string, ): Promise { return adminFetch( `/credits/admin/settlement/batches/${id}/abandon`, - accessToken, { method: "POST", body: JSON.stringify({ reason }) }, ); } diff --git a/frontend/lib/auth-api.ts b/frontend/lib/auth-api.ts index 381d68d0..42dd3a9b 100644 --- a/frontend/lib/auth-api.ts +++ b/frontend/lib/auth-api.ts @@ -3,14 +3,12 @@ import { getRequestId, reportApiError, } from "@/lib/api-report"; +import { expireSession } from "@/lib/session-expiry"; +import type { AuthUser } from "@/lib/auth-types"; -export type UserRole = "MEMBER" | "ADMIN" | "SUPER_ADMIN"; - -export interface AuthUser { - id: string; - email: string; - role: UserRole; -} +// Re-exported for convenience: the session store and the auth form both need +// these exact shapes, and the backend's `UserRole` enum is the source of truth. +export type { AuthUser, UserRole } from "@/lib/auth-types"; export interface AuthResponse { accessToken: string; @@ -19,6 +17,12 @@ export interface AuthResponse { const API_BASE_URL = process.env.NEXT_PUBLIC_API_URL ?? ""; +/** + * Every request includes `credentials: "include"` (issue #1975): the backend + * answers register/login with an HttpOnly `accessToken` cookie, so the session + * is owned by the cookie, not by a JavaScript-readable duplicate. The token + * returned in the response body is kept in memory only (see session-store). + */ async function authRequest( path: string, body: { email: string; password: string }, @@ -27,9 +31,12 @@ async function authRequest( method: "POST", headers: { "Content-Type": "application/json" }, body: JSON.stringify(body), + credentials: "include", }); if (!response.ok) { + // A 401 from login/register means bad credentials, not an expired + // session - resolving the session and redirecting here would be wrong. const errorBody = await response.json().catch(() => null); const fallbackMessage = response.status === 401 @@ -61,3 +68,66 @@ export function register( ): Promise { return authRequest("/auth/register", { email, password }); } + +/** + * Returns the signed-in user from the backend (issue #1973). Used to restore + * a session after a page reload without reading the HttpOnly cookie from + * JavaScript. Single-flight: concurrent callers share one request. + */ +let meInFlight: Promise | null = null; + +export function me(): Promise { + meInFlight ??= fetchMe().finally(() => { + meInFlight = null; + }); + return meInFlight; +} + +async function fetchMe(): Promise { + const response = await fetch(`${API_BASE_URL}/auth/me`, { + method: "GET", + credentials: "include", + }); + + if (!response.ok) { + if (response.status === 401) { + // A rejected session must clear the store and send the user to login + // instead of leaving a stale state behind (issue #1977). + expireSession(); + } + const errorBody = await response.json().catch(() => null); + const error = new Error( + getApiErrorMessage(errorBody, "Failed to load the current session"), + ); + + reportApiError({ + api: "auth", + method: "GET", + pathTemplate: "/auth/me", + status: response.status, + requestId: getRequestId(response), + }); + + throw error; + } + + return (await response.json()) as AuthUser; +} + +/** + * Ends the session server-side and clears the HttpOnly cookie (issue #1974). + * The backend revokes every outstanding token for the user by bumping their + * `tokenVersion`, so a copied JWT cannot outlive the logout either. Network + * or 401/403 failures must not block the local sign-out: callers always clear + * the local session afterwards, which is why errors are swallowed here. + */ +export async function logout(): Promise { + try { + await fetch(`${API_BASE_URL}/auth/logout`, { + method: "POST", + credentials: "include", + }); + } catch { + // The local session is cleared by the caller regardless. + } +} diff --git a/frontend/lib/auth-types.ts b/frontend/lib/auth-types.ts new file mode 100644 index 00000000..0b81824d --- /dev/null +++ b/frontend/lib/auth-types.ts @@ -0,0 +1,14 @@ +/** + * Shared auth types for the frontend. + * + * Mirrors the backend `UserRole` enum values (issue #1973): the backend signs + * the role as a lowercase string claim ("user" | "admin"), so the previous + * "MEMBER" | "ADMIN" | "SUPER_ADMIN" union never matched real tokens. + */ +export type UserRole = "user" | "admin"; + +export interface AuthUser { + id: string; + email: string; + role: UserRole; +} diff --git a/frontend/lib/credits-api.ts b/frontend/lib/credits-api.ts index 6a81bf3b..f1e2f752 100644 --- a/frontend/lib/credits-api.ts +++ b/frontend/lib/credits-api.ts @@ -1,4 +1,6 @@ -// lib/credits-api.ts -- member-facing credit API (FE-108) +// lib/credits-api.ts -- member-facing credit API (FE-108) + +import { expireSession } from "@/lib/session-expiry"; export interface CreditBalance { accountId: string; @@ -69,19 +71,24 @@ export class CreditsApiError extends Error { async function creditsFetch( path: string, - accessToken: string, init?: RequestInit, ): Promise { const response = await fetch(`${API_BASE_URL}${path}`, { ...init, headers: { "Content-Type": "application/json", - Authorization: `Bearer ${accessToken}`, ...init?.headers, }, + // The API authorizes every credits call through the HttpOnly session + // cookie (issue #1975); if it has been rejected, route the user to login + // instead of leaving stale state around (issue #1977). + credentials: "include", }); if (!response.ok) { + if (response.status === 401) { + expireSession(); + } const body: unknown = await response.json().catch(() => null); throw new CreditsApiError(response.status, body); } @@ -94,17 +101,15 @@ async function creditsFetch( // -- member credit balance (FE-108) ----------------------------------------------- -export function getCreditBalance(accessToken: string): Promise { - return creditsFetch("/credits/balance", accessToken); +export function getCreditBalance(): Promise { + return creditsFetch("/credits/balance"); } export function getCreditStatement( - accessToken: string, page = 1, pageSize = 20, ): Promise { return creditsFetch( `/credits/statement?page=${page}&pageSize=${pageSize}`, - accessToken, ); } diff --git a/frontend/lib/payments-api.ts b/frontend/lib/payments-api.ts index cd906612..1a9a492e 100644 --- a/frontend/lib/payments-api.ts +++ b/frontend/lib/payments-api.ts @@ -140,7 +140,6 @@ async function fetchWithRetry( async function apiFetch( path: string, - accessToken: string, init?: RequestInit, idempotencyKey?: string, ): Promise { @@ -148,14 +147,21 @@ async function apiFetch( ...init, headers: { "Content-Type": "application/json", - Authorization: `Bearer ${accessToken}`, ...(idempotencyKey ? { "Idempotency-Key": idempotencyKey } : {}), ...init?.headers, }, + // The backend authorizes via its HttpOnly `accessToken` cookie (issue + // #1975); no bearer header is threaded from the client. + credentials: "include", }; const response = await fetchWithRetry(`${API_BASE_URL}${path}`, requestInit); if (!response.ok) { + if (response.status === 401) { + // A rejected session must send the user to login, not leave the stale + // state in place (issue #1977). + expireSession(); + } const body = await response.json().catch(() => null); const error = new Error( getApiErrorMessage(body, `Request failed (${response.status})`), @@ -181,43 +187,39 @@ async function apiFetch( // ── payments (FE-102) ──────────────────────────────────────────────────── export function initiatePayment( - accessToken: string, input: InitiatePaymentInput, idempotencyKey: string, ): Promise { - return apiFetch("/payments/initiate", accessToken, { + return apiFetch("/payments/initiate", { method: "POST", body: JSON.stringify(input), }, idempotencyKey); } -export function getPayments(accessToken: string): Promise { - return apiFetch("/payments", accessToken); +export function getPayments(): Promise { + return apiFetch("/payments"); } -export function getPayment(id: string, accessToken: string): Promise { - return apiFetch(`/payments/${id}`, accessToken); +export function getPayment(id: string): Promise { + return apiFetch(`/payments/${id}`); } export function verifyPaymentReturn( - accessToken: string, id: string, ): Promise<{ payment: Payment; verified: boolean }> { return apiFetch<{ payment: Payment; verified: boolean }>( `/payments/${id}/verify-return`, - accessToken, { method: "POST" }, ); } // ── metered usage (FE-109) ─────────────────────────────────────────────── -export function listMyUsage(accessToken: string): Promise { - return apiFetch("/credits/usage", accessToken); +export function listMyUsage(): Promise { + return apiFetch("/credits/usage"); } export function listManualReviewPayments( - accessToken: string, limit = 25, offset = 0, ): Promise { @@ -227,43 +229,36 @@ export function listManualReviewPayments( }); return apiFetch( `/payments/admin/manual-review?${params.toString()}`, - accessToken, ); } // ── admin payment actions (FE-104 / FE-106) ────────────────────────────── export function forceReconcilePayment( - accessToken: string, id: string, ): Promise { return apiFetch( `/payments/admin/${id}/force-reconcile`, - accessToken, { method: "POST" }, ); } export function resolvePaymentManually( - accessToken: string, id: string, resolution: string, ): Promise { return apiFetch( `/payments/admin/${id}/resolve-manually`, - accessToken, { method: "POST", body: JSON.stringify({ resolution }) }, ); } export function voidPayment( - accessToken: string, id: string, reason: string, ): Promise { return apiFetch( `/payments/admin/${id}/void`, - accessToken, { method: "POST", body: JSON.stringify({ reason }) }, ); } @@ -271,14 +266,12 @@ export function voidPayment( // ── refunds (FE-107) ───────────────────────────────────────────────────── export function requestRefund( - accessToken: string, paymentId: string, amount: number, reason: string, ): Promise { return apiFetch( `/payments/admin/${paymentId}/refunds`, - accessToken, { method: "POST", body: JSON.stringify({ amount, reason }) }, ); } @@ -286,7 +279,6 @@ export function requestRefund( // ── charge credits (FE-110) ────────────────────────────────────────────── export function chargeCredits( - accessToken: string, input: { userId: string; amount: number; @@ -296,7 +288,7 @@ export function chargeCredits( metadata?: Record; }, ): Promise { - return apiFetch("/credits/charge", accessToken, { + return apiFetch("/credits/charge", { method: "POST", body: JSON.stringify(input), }); diff --git a/frontend/lib/session-expiry.ts b/frontend/lib/session-expiry.ts new file mode 100644 index 00000000..16ecc4f8 --- /dev/null +++ b/frontend/lib/session-expiry.ts @@ -0,0 +1,39 @@ +"use client"; + +import { toast } from "sonner"; +import { buildReturnTo, getSafeReturnTo } from "@/lib/auth-redirect"; +import { useSessionStore } from "@/lib/stores/session-store"; + +/** + * Shared reaction to a rejected session (401) - issue #1977. + * + * Any API helper that receives a 401 from a protected endpoint calls this + * once: the zustand session is cleared, the user sees a "session expired" + * toast, and the browser is sent to `/login?returnTo=`. The flag + * below coalesces parallel 401s (five failed queries must not fire five + * redirects or five toasts) and the redirect is a full `window.location` + * navigation, so after login the flag is gone and the next real expiry can + * redirect again - no loop is possible because /login never calls helpers + * that react to 401 and the login page is reachable while logged out. + */ +let redirectScheduled = false; + +export function expireSession(): void { + useSessionStore.getState().clearSession(); + + if (typeof window === "undefined") return; + + const { pathname, search } = window.location; + + // Already on an auth route: the app is heading to login anyway. + if (pathname === "/login" || pathname === "/register") return; + if (redirectScheduled) return; + + redirectScheduled = true; + toast.error("Your session has expired. Please sign in again."); + + const returnTo = getSafeReturnTo( + buildReturnTo(pathname, search.replace(/^\?/, "")), + ); + window.location.replace(`/login?returnTo=${encodeURIComponent(returnTo)}`); +} diff --git a/frontend/lib/stores/session-store.ts b/frontend/lib/stores/session-store.ts index 06c65a34..7d7da1f6 100644 --- a/frontend/lib/stores/session-store.ts +++ b/frontend/lib/stores/session-store.ts @@ -1,31 +1,35 @@ "use client"; import { create } from "zustand"; -import Cookies from "js-cookie"; +import type { AuthUser } from "@/lib/auth-types"; -const ACCESS_TOKEN_COOKIE = "accessToken"; - -function readStoredToken(): string | null { - if (typeof window === "undefined") return null; - return Cookies.get(ACCESS_TOKEN_COOKIE) ?? null; -} +/** + * The access token is kept in memory only: the backend owns the real session + * through its HttpOnly `accessToken` cookie (issue #1975), and the client must + * not keep a JavaScript-readable copy (no js-cookie mirror). A JavaScript- + * readable cookie also had to be read synchronously at store creation, which + * made server-rendered markup disagree with the first client render and + * caused hydration mismatches (issue #1983's root cause). What the client + * needs - who the user is and which role they hold - is restored from + * `GET /auth/me` inside a client effect instead. + */ +export type SessionStatus = "unknown" | "authenticated" | "unauthenticated"; interface SessionState { + user: AuthUser | null; + /** Memory-only copy of the login/register response token. Not persisted. */ accessToken: string | null; - setAccessToken: (token: string | null) => void; + status: SessionStatus; + setSession: (user: AuthUser, accessToken?: string | null) => void; + clearSession: () => void; } export const useSessionStore = create((set) => ({ - accessToken: readStoredToken(), - setAccessToken: (token) => { - if (token) { - Cookies.set(ACCESS_TOKEN_COOKIE, token, { - sameSite: "lax", - secure: window.location.protocol === "https:", - }); - } else { - Cookies.remove(ACCESS_TOKEN_COOKIE); - } - set({ accessToken: token }); - }, + user: null, + accessToken: null, + status: "unknown", + setSession: (user, accessToken = null) => + set({ user, accessToken, status: "authenticated" }), + clearSession: () => + set({ user: null, accessToken: null, status: "unauthenticated" }), })); diff --git a/frontend/lib/stores/use-session-bootstrap.ts b/frontend/lib/stores/use-session-bootstrap.ts new file mode 100644 index 00000000..ebaf31be --- /dev/null +++ b/frontend/lib/stores/use-session-bootstrap.ts @@ -0,0 +1,39 @@ +"use client"; + +import { useEffect } from "react"; +import { me } from "@/lib/auth-api"; +import { useSessionStore } from "@/lib/stores/session-store"; + +/** + * Restores the session after a page reload (issue #1975 / issue #1973). + * + * The HttpOnly `accessToken` cookie cannot be read from JavaScript, so the + * client learns who is signed in by calling `GET /auth/me` once inside a + * client effect (never during render), which keeps server-rendered markup and + * the first client render identical and avoids hydration mismatches + * (issue #1983's root cause). `auth-api.me()` is single-flight, so several + * mounted callers share one request. + */ +export function useSessionBootstrap(): void { + const status = useSessionStore((state) => state.status); + + useEffect(() => { + if (status !== "unknown") return; + + me() + .then((user) => { + // A later action (logout, expiry) may have resolved the state on a + // different instance; only set the session if we are still unknown. + if (useSessionStore.getState().status === "unknown") { + useSessionStore.getState().setSession(user); + } + }) + .catch(() => { + // me() already clears the session and redirects on a 401; anything + // else (offline, 5xx) means we simply cannot prove an active session. + if (useSessionStore.getState().status === "unknown") { + useSessionStore.getState().clearSession(); + } + }); + }, [status]); +} diff --git a/frontend/lib/wallet-api.ts b/frontend/lib/wallet-api.ts index bfe2184d..456110a6 100644 --- a/frontend/lib/wallet-api.ts +++ b/frontend/lib/wallet-api.ts @@ -3,6 +3,7 @@ import { getRequestId, reportApiError, } from "@/lib/api-report"; +import { expireSession } from "@/lib/session-expiry"; export type WalletCustodyType = "CUSTODIAL" | "EXTERNAL"; export type WalletStatus = "PENDING" | "ACTIVE" | "DISABLED"; @@ -30,10 +31,17 @@ async function walletFetch(path: string, init?: RequestInit): Promise { "Content-Type": "application/json", ...init?.headers, }, - credentials: "same-origin", + // The backend owns the session via its HttpOnly `accessToken` cookie + // (issue #1975); no bearer header is threaded from the client. + credentials: "include", }); if (!response.ok) { + if (response.status === 401) { + // A rejected session must send the user to login, not leave the stale + // state in place (issue #1977). + expireSession(); + } const body = await response.json().catch(() => null); const error = new Error( getApiErrorMessage(body, `Wallet request failed (${response.status})`), diff --git a/frontend/middleware.test.ts b/frontend/middleware.test.ts new file mode 100644 index 00000000..f2f847ee --- /dev/null +++ b/frontend/middleware.test.ts @@ -0,0 +1,82 @@ +// @vitest-environment node +import { beforeEach, describe, expect, it, vi } from "vitest"; +import { NextRequest } from "next/server"; +import { jwtVerify } from "jose"; + +import { middleware } from "../middleware"; + +vi.mock("jose", () => ({ + jwtVerify: vi.fn(), +})); + +type VerifyPayload = { role?: string }; + +function requestWithCookie(path: string): NextRequest { + return new NextRequest(`http://localhost${path}`, { + headers: { cookie: "accessToken=jwt-token" }, + }); +} + +function verifiedToken(role: string): void { + vi.mocked(jwtVerify).mockResolvedValue({ + payload: { sub: "u1", role } as VerifyPayload, + } as never); +} + +describe("middleware role gate (issue #1973)", () => { + beforeEach(() => { + vi.clearAllMocks(); + }); + + it("lets an admin token through to admin pages", async () => { + verifiedToken("admin"); + const response = await middleware(requestWithCookie("/admin/accounts")); + + expect(response.status).toBe(200); + expect(response.headers.get("location")).toBeNull(); + }); + + it("redirects a member token away from admin pages to /admin/forbidden", async () => { + verifiedToken("user"); + const response = await middleware(requestWithCookie("/admin/accounts")); + + expect(response.status).toBe(307); + expect(response.headers.get("location")).toBe("http://localhost/admin/forbidden"); + }); + + it("does not loop when rendering the forbidden page itself", async () => { + verifiedToken("user"); + const response = await middleware(requestWithCookie("/admin/forbidden")); + + expect(response.status).toBe(200); + expect(response.headers.get("location")).toBeNull(); + }); + + it("redirects unauthenticated users to /login with a returnTo", async () => { + const response = await middleware( + new NextRequest("http://localhost/credits"), + ); + + expect(response.status).toBe(307); + expect(response.headers.get("location")).toBe( + "http://localhost/login?returnTo=%2Fcredits", + ); + }); + + it("redirects authenticated users away from login to /wallet", async () => { + verifiedToken("user"); + const response = await middleware(requestWithCookie("/login")); + + expect(response.status).toBe(307); + expect(response.headers.get("location")).toBe("http://localhost/wallet"); + }); + + it("still redirects to /admin/forbidden when the token cannot be verified", async () => { + vi.mocked(jwtVerify).mockRejectedValue(new Error("bad token")); + + // Unverifiable token -> treated as unauthenticated -> login redirect. + const response = await middleware(requestWithCookie("/admin")); + expect(response.status).toBe(307); + expect(String(response.headers.get("location"))).toContain("/login"); + }); +}); diff --git a/frontend/middleware.ts b/frontend/middleware.ts index f4586137..c47f9074 100644 --- a/frontend/middleware.ts +++ b/frontend/middleware.ts @@ -44,6 +44,28 @@ async function isAuthenticated(request: NextRequest): Promise { } } +/** + * Decodes the verified token's `role` claim (issue #1973). The backend signs + * the `user`/`admin` enum values, and role changes bump `tokenVersion`, so a + * role claim that still verifies is fresh enough to gate admin pages here. + * The backend still enforces authorization on every endpoint. + */ +async function isAdminToken(request: NextRequest): Promise { + const token = getAccessToken(request); + if (!token) return false; + + const secret = process.env.JWT_SECRET; + if (!secret) return false; + + try { + const key = new TextEncoder().encode(secret); + const { payload } = await jwtVerify(token, key); + return payload.role === "admin"; + } catch { + return false; + } +} + export async function middleware(request: NextRequest) { const { pathname } = request.nextUrl; const authenticated = await isAuthenticated(request); @@ -63,6 +85,24 @@ export async function middleware(request: NextRequest) { return NextResponse.redirect(url); } + // Member-role tokens must land on a "not authorised" page instead of the + // admin shell (issue #1973). The dedicated forbidden page skips this check + // so the redirect cannot loop. + if ( + authenticated && + isProtectedRoute(pathname) && + (pathname === "/admin" || pathname.startsWith("/admin/")) && + pathname !== "/admin/forbidden" + ) { + const isAdmin = await isAdminToken(request); + if (!isAdmin) { + const forbiddenUrl = request.nextUrl.clone(); + forbiddenUrl.pathname = "/admin/forbidden"; + forbiddenUrl.search = ""; + return NextResponse.redirect(forbiddenUrl); + } + } + return NextResponse.next(); }