fix(search): send the PB auth token, and a second instance of the same bug
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.
This commit is contained in:
parent
60ccfa8122
commit
f355895c75
4 changed files with 261 additions and 3 deletions
221
src/app/(app)/search/page.test.tsx
Normal file
221
src/app/(app)/search/page.test.tsx
Normal file
|
|
@ -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;
|
||||
}) => (
|
||||
<a href={href} {...props}>
|
||||
{children}
|
||||
</a>
|
||||
),
|
||||
}));
|
||||
|
||||
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(<UniversalSearchPage />);
|
||||
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(<UniversalSearchPage />);
|
||||
|
||||
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(<UniversalSearchPage />);
|
||||
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(<UniversalSearchPage />);
|
||||
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(<UniversalSearchPage />);
|
||||
await type("truco");
|
||||
|
||||
await waitFor(() =>
|
||||
expect(screen.getByText(/session has expired/i)).toBeInTheDocument(),
|
||||
);
|
||||
});
|
||||
});
|
||||
|
|
@ -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<Response> {
|
||||
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<SearchResult | null>(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);
|
||||
}, []);
|
||||
|
|
|
|||
|
|
@ -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(),
|
||||
|
|
|
|||
|
|
@ -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 ?? []);
|
||||
|
|
|
|||
Loading…
Reference in a new issue