From f355895c753faf7c1692b5dc4ca60be4ddf3091b Mon Sep 17 00:00:00 2001 From: Dave Date: Wed, 14 Oct 2026 11:49:30 +0200 Subject: [PATCH] fix(search): send the PB auth token, and a second instance of the same bug MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Universal search returned 401 "Not authenticated." on every query. Cause: dashboard auth is a PocketBase token in the `Authorization` header, NOT a cookie — `assertBrowserSession` reads the header and refreshes it against PB. The page used a bare `fetch()`, which sends neither. The service underneath was always correct. Worth being explicit about what was missed: the service was verified end-to-end against live PB, and the route compiles, but curl against the dev server returns the same auth-gated shell as /orders does — so the authenticated path was never actually exercised until a human opened it in a browser. Green tests plus a working backend is not the same as a working feature. The cache-warm call had the same defect, which mattered more than it looks: an unauthenticated warm is a server no-op, so the first real search would have quietly paid the full ~4.5s cold build every session. SECOND INSTANCE, same root cause `customer-form.tsx` fetched `/api/v1/customers/sites` — a guarded route — with no Authorization header. The `.catch` swallowed the 401, so the site-location autocomplete has always silently returned an empty list, which is indistinguishable from "no sites in use yet". Fixed in the same pass. Audited the rest: cross-referenced every route that calls assertBrowserSession against every client caller, and this is the only other real one. The `x-wfdash-auth` header installed by `installAuditFetch` is attribution for the audit trail, not auth, which is probably what makes this easy to get wrong. New: 5 tests on the search page covering the auth header on both the search and the warm call, the truncated-group "showing N of TOTAL" rendering, the failed-source row, and session expiry. Verified the two auth tests fail when the header is removed while the other three keep passing. Also completed the customer-form pocketbase test mock, which was missing `authStore` — the real SDK always has one. --- src/app/(app)/search/page.test.tsx | 221 ++++++++++++++++++++++++++ src/app/(app)/search/page.tsx | 30 +++- src/components/customer-form.test.tsx | 3 + src/components/customer-form.tsx | 10 +- 4 files changed, 261 insertions(+), 3 deletions(-) create mode 100644 src/app/(app)/search/page.test.tsx diff --git a/src/app/(app)/search/page.test.tsx b/src/app/(app)/search/page.test.tsx new file mode 100644 index 0000000..627a688 --- /dev/null +++ b/src/app/(app)/search/page.test.tsx @@ -0,0 +1,221 @@ +import { cleanup, fireEvent, render, screen, waitFor } from "@testing-library/react"; +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; + +import UniversalSearchPage from "./page"; + +/** + * The page talks to `/api/v1/search`, which is gated by + * `assertBrowserSession`. That helper reads a PocketBase token from the + * `Authorization` request header — dashboard auth is NOT a cookie. + * + * This file exists because the first version of this page used a bare + * `fetch()`, so every search returned 401 "Not authenticated." while the + * service underneath was working perfectly. Green tests and a live + * backend, and the feature was still unusable — the failure was only + * visible in a real browser. + */ +const { fetchMock } = vi.hoisted(() => ({ fetchMock: vi.fn() })); + +// Stub the GLOBAL fetch — the page calls it via `authedFetch`, which +// resolves `fetch` at call time. Assigning to `globalThis.fetch` is what +// makes the mock observable; without this jsdom/undici tries to parse the +// relative URL and the test fails for the wrong reason. +vi.stubGlobal("fetch", fetchMock); + +vi.mock("@/lib/pocketbase", () => ({ + default: () => ({ authStore: { token: "test-pb-token" } }), +})); + +vi.mock("next/link", () => ({ + default: ({ + href, + children, + ...props + }: { + href: string; + children: React.ReactNode; + }) => ( + + {children} + + ), +})); + +function jsonResponse(body: unknown, ok = true, status = 200) { + return { + ok, + status, + json: async () => body, + } as Response; +} + +const EMPTY_RESULT = { + query: "", + groups: [], + totalHits: 0, + tookMs: 0, + windowDays: 90, +}; + +const RESULT = { + query: "truco", + groups: [ + { + type: "order", + label: "Orders", + hits: [{ id: "o1", title: "ORD-1", href: "/orders/o1", matchedOn: "supplier" }], + total: 1, + truncated: false, + tookMs: 5, + status: "ok", + }, + ], + totalHits: 1, + tookMs: 12, + windowDays: 90, +}; + +describe("UniversalSearchPage — auth", () => { + beforeEach(() => { + fetchMock.mockReset(); + vi.useFakeTimers({ shouldAdvanceTime: true }); + }); + + afterEach(() => { + cleanup(); + vi.useRealTimers(); + }); + + /** Type into the box and let the debounce fire. */ + async function type(value: string) { + const input = screen.getByPlaceholderText(/Order no, quote no/i); + fireEvent.change(input, { target: { value } }); + await vi.advanceTimersByTimeAsync(400); + } + + it("sends the PocketBase token as Authorization on every search", async () => { + fetchMock.mockResolvedValue(jsonResponse(RESULT)); + + render(); + await type("truco"); + + await waitFor(() => expect(fetchMock).toHaveBeenCalled()); + + const searchCalls = fetchMock.mock.calls.filter((c) => + String(c[0]).includes("q=truco"), + ); + expect(searchCalls.length).toBeGreaterThan(0); + + const [, init] = searchCalls[0]; + expect(init?.headers).toMatchObject({ Authorization: "test-pb-token" }); + }); + + it("authenticates the cache-warm call too", async () => { + // An unauthenticated warm is a server no-op, so the first real + // search would silently pay the full ~4.5s cold build. + fetchMock.mockResolvedValue(jsonResponse(EMPTY_RESULT)); + + render(); + + await waitFor(() => expect(fetchMock).toHaveBeenCalled()); + + const warmCall = fetchMock.mock.calls.find((c) => + String(c[0]).endsWith("q="), + ); + expect(warmCall).toBeDefined(); + expect(warmCall?.[1]?.headers).toMatchObject({ + Authorization: "test-pb-token", + }); + }); +}); + +describe("UniversalSearchPage — the never-hide-a-cap invariant", () => { + beforeEach(() => { + fetchMock.mockReset(); + vi.useFakeTimers({ shouldAdvanceTime: true }); + }); + + afterEach(() => { + cleanup(); + vi.useRealTimers(); + }); + + async function type(value: string) { + const input = screen.getByPlaceholderText(/Order no, quote no/i); + fireEvent.change(input, { target: { value } }); + await vi.advanceTimersByTimeAsync(400); + } + + it("shows 'showing N of TOTAL' when a group is truncated", async () => { + // Regression guard for the whole point of the `total` field: a cap + // that renders as a plain short list is indistinguishable from a + // complete answer, which is how the /orders 500-cap bug hid 110 + // orders with no error anywhere. + fetchMock.mockResolvedValue( + jsonResponse({ + ...RESULT, + totalHits: 1240, + groups: [ + { + ...RESULT.groups[0], + hits: [{ id: "o1", title: "ORD-1", href: "/orders/o1" }], + total: 1240, + truncated: true, + }, + ], + }), + ); + + render(); + await type("truco"); + + await waitFor(() => + expect(screen.getByText(/showing 1 of 1240/i)).toBeInTheDocument(), + ); + expect(screen.getByText(/refine to see the rest/i)).toBeInTheDocument(); + }); + + it("surfaces a failed source as an explicit row, not an empty group", async () => { + fetchMock.mockResolvedValue( + jsonResponse({ + ...RESULT, + totalHits: 0, + groups: [ + { + type: "quote", + label: "Quotes", + hits: [], + total: 0, + truncated: false, + tookMs: 6000, + status: "timeout", + error: "timed out after 6000ms", + }, + ], + }), + ); + + render(); + await type("truco"); + + // "Couldn't search quotes" must never look like "no quotes matched". + await waitFor(() => + expect(screen.getByText(/couldn't search quotes/i)).toBeInTheDocument(), + ); + expect(screen.getByText(/other sources are unaffected/i)).toBeInTheDocument(); + }); + + it("reports a session expiry in plain language", async () => { + // No token in the store — the helper throws before fetching. + fetchMock.mockResolvedValue(jsonResponse(RESULT)); + const mod = await import("@/lib/pocketbase"); + vi.spyOn(mod, "default").mockReturnValue({ authStore: { token: "" } } as never); + + render(); + await type("truco"); + + await waitFor(() => + expect(screen.getByText(/session has expired/i)).toBeInTheDocument(), + ); + }); +}); \ No newline at end of file diff --git a/src/app/(app)/search/page.tsx b/src/app/(app)/search/page.tsx index cd5faf2..84fecbf 100644 --- a/src/app/(app)/search/page.tsx +++ b/src/app/(app)/search/page.tsx @@ -5,6 +5,7 @@ import Link from "next/link"; import { Search as SearchIcon, X, Loader2, AlertTriangle, ArrowUpRight } from "lucide-react"; import { Input } from "@/components/ui/input"; import { Badge } from "@/components/ui/badge"; +import getPocketBase from "@/lib/pocketbase"; /** * Universal search — one box across orders, quotes, POs, suppliers, @@ -57,6 +58,27 @@ const MIN_QUERY_CHARS = 2; /** Matches the orders page's debounce so the two feel the same. */ const DEBOUNCE_MS = 300; +/** + * Authenticated fetch against `/api/*`. + * + * Dashboard auth is a PocketBase token held in the client auth store, + * NOT a cookie — `assertBrowserSession` reads it from the + * `Authorization` header and refreshes it against PB. A bare `fetch()` + * sends neither, so the route answers 401 "Not authenticated." and the + * page looks broken. + * + * Note this is NOT the `x-wfdash-auth` header installed by + * `installAuditFetch` — that one is attribution for the audit trail, + * and the server does not treat it as authentication. + */ +async function authedFetch(url: string): Promise { + const token = getPocketBase().authStore.token; + if (!token) { + throw new Error("Your session has expired. Please sign in again."); + } + return fetch(url, { headers: { Authorization: token } }); +} + export default function UniversalSearchPage() { const [input, setInput] = useState(""); const [result, setResult] = useState(null); @@ -81,7 +103,9 @@ export default function UniversalSearchPage() { setLoading(true); setFailed(null); try { - const res = await fetch(`/api/v1/search?q=${encodeURIComponent(q)}`); + const res = await authedFetch( + `/api/v1/search?q=${encodeURIComponent(q)}`, + ); const data = await res.json(); if (seq !== requestSeq.current) return; // superseded if (!res.ok || data.error) { @@ -133,7 +157,9 @@ export default function UniversalSearchPage() { // doesn't pay the ~4.5s cold build. The empty-query request is the // documented warm trigger. useEffect(() => { - fetch("/api/v1/search?q=") + // Authenticated like every other call — an unauthenticated warm is + // a no-op on the server, which would quietly skip the pre-build. + authedFetch("/api/v1/search?q=") .then(() => undefined) .catch(() => undefined); }, []); diff --git a/src/components/customer-form.test.tsx b/src/components/customer-form.test.tsx index 92d5ded..eb1eb73 100644 --- a/src/components/customer-form.test.tsx +++ b/src/components/customer-form.test.tsx @@ -16,6 +16,9 @@ const { vi.mock("@/lib/pocketbase", () => ({ __esModule: true, default: () => ({ + // The real SDK always exposes `authStore`; the component reads + // `authStore.token` to authenticate its `/api/v1/*` calls. + authStore: { token: "test-pb-token" }, collection: () => ({ create: createMock, update: vi.fn(), diff --git a/src/components/customer-form.tsx b/src/components/customer-form.tsx index aef7845..bffce5f 100644 --- a/src/components/customer-form.tsx +++ b/src/components/customer-form.tsx @@ -147,7 +147,15 @@ export function CustomerForm({ open, onOpenChange, customer, onSaved }: Customer useEffect(() => { if (!open || siteLocationSuggestions.length > 0) return; let cancelled = false; - fetch("/api/v1/customers/sites", { cache: "no-store" }) + fetch("/api/v1/customers/sites", { + // `/api/v1/*` browser routes are gated by `assertBrowserSession`, + // which reads the PocketBase token from `Authorization` — auth + // here is NOT a cookie. Without it the route 401s, the `.catch` + // below swallows it, and the autocomplete silently degrades to an + // empty list, which looks exactly like "no sites in use yet". + headers: { Authorization: getPocketBase().authStore.token }, + cache: "no-store", + }) .then((r) => (r.ok ? r.json() : Promise.reject(new Error(`HTTP ${r.status}`)))) .then((data: { items?: { site_location: string; customerCount: number }[] }) => { if (!cancelled) setSiteLocationSuggestions(data.items ?? []);