fix(repairs): send the session on the detail photo upload, and cover every handler
This commit is contained in:
parent
c581823990
commit
745593ee35
5 changed files with 217 additions and 25 deletions
18
AGENTS.md
18
AGENTS.md
|
|
@ -376,12 +376,18 @@ planned phone app lives at `/repairs-app`, not built yet.
|
|||
stored, and the agent is told the upload worked. Add the fields FIRST,
|
||||
then verify by round-tripping a save — do not wait for an error, there
|
||||
will not be one.
|
||||
- **The repairs REST family has no session check** (pre-existing; found
|
||||
2026-11-06). All six routes under `/api/v1/repairs/` read and write
|
||||
through the PocketBase superuser client with no credential check. The
|
||||
`(app)` layout gates the PAGES client-side; the API is not gated. The
|
||||
`repairs.*` MCP tools ARE gated by `assertAgentAuth`. Operator
|
||||
decision pending on adding a session check to all six routes together.
|
||||
- **The repairs REST family requires a browser session** (closed
|
||||
2026-11-06). Every handler under `/api/v1/repairs/` checks
|
||||
`assertBrowserSession` first and answers **401** without one — they
|
||||
read and write through the PocketBase superuser client, so the check is
|
||||
the only thing standing between an anonymous caller and a customer's
|
||||
address, serial number and fault description. The `repairs` slice of
|
||||
the `/api/v1/files/` proxy is gated the same way (photos, the Repair
|
||||
Request PDF, the supplier quote PDF **with the pricing**); every other
|
||||
collection's files are unaffected. That slice also accepts bearer
|
||||
`AGENT_API_KEY`, so the MCP `files.fetch` tool keeps working. The
|
||||
`repairs.*` MCP tools are gated by `assertAgentAuth` and call the
|
||||
service layer directly, so they are unaffected.
|
||||
- Phase 1 of the repairs work landed on `main` 2026-11-06 (`9358b37`):
|
||||
the shared vocabulary, the service layer (`supplier_scope`,
|
||||
`signed_request`, `getRepairStatusCounts`, `linkRepairToOrder`),
|
||||
|
|
|
|||
|
|
@ -91,7 +91,7 @@ Do not try to base64 the photos into the create call.
|
|||
- Test: `src/app/api/v1/repairs/repair-routes.test.ts` (extend)
|
||||
|
||||
**Interfaces:**
|
||||
- Consumes: `assertBrowserSession(req)` from `@/lib/server/browser-session`; `useAuth()` from `@/components/auth-provider`.
|
||||
- Consumes: `assertBrowserSession(req)` from `@/lib/server/browser-session`; the browser token from `getPocketBase().authStore.token` (see "How the browser token is obtained" above — `useAuth()` does NOT expose a token).
|
||||
- Produces: every repairs route returns **401 without a browser session**, and the office screens keep working.
|
||||
|
||||
**Why this task exists.** The reviews proved the repairs API is anonymously readable on production: repair records (customer addresses, serial numbers, fault descriptions) and files (photos, the repair request PDF, the supplier quote PDF *with pricing*) all return HTTP 200 to a caller with no credentials. The MCP tools are correctly gated by `assertAgentAuth` — this is a REST-only hole, and the phone app would sit right on top of it.
|
||||
|
|
@ -192,7 +192,7 @@ Use the real repairs collection id from the constant block at the top of that fi
|
|||
|
||||
In `src/app/(app)/repairs/page.tsx` and `src/app/(app)/repairs/[id]/page.tsx`, every fetch to `/api/v1/repairs*` must carry the browser token.
|
||||
|
||||
Read the token from `useAuth()` — the same hook `MobileAuthGate` uses. Add a small helper in each page, matching the repo's existing shape:
|
||||
Read the token from the PocketBase auth store — `useAuth()` does NOT expose a `token` (see "How the browser token is obtained" above). Add a small helper in each page, matching the repo's existing shape:
|
||||
|
||||
```ts
|
||||
const token = getPocketBase().authStore.token;
|
||||
|
|
@ -229,7 +229,7 @@ Expected: PASS. The three original route tests plus the new 401 cases.
|
|||
|
||||
Run: `npx tsc --noEmit`
|
||||
|
||||
Expected: 0 errors. If `useAuth()` does not expose a `token`, read `src/components/auth-provider.tsx` and use the field it does expose.
|
||||
Expected: 0 errors. The token comes from `getPocketBase().authStore.token` — `useAuth()` exposes no `token` field, so never write `useAuth().token` (it is `undefined` and the route answers 401).
|
||||
|
||||
- [ ] **Step 8: Commit**
|
||||
|
||||
|
|
@ -528,7 +528,7 @@ git commit -m "feat(repairs-app): the phone app shell, manifest and service work
|
|||
- Test: `src/app/(repairs-app)/repairs-app/repair-board.test.tsx`
|
||||
|
||||
**Interfaces:**
|
||||
- Consumes: `REPAIR_STATUS`, `REPAIR_STATUS_LABEL`, `REPAIR_STATUS_VARIANT` from `@/lib/repair-status`; `useAuth()` for the session header; `GET /api/v1/repairs` and `GET /api/v1/repairs/status-counts`.
|
||||
- Consumes: `REPAIR_STATUS`, `REPAIR_STATUS_LABEL`, `REPAIR_STATUS_VARIANT` from `@/lib/repair-status`; the session header built from `getPocketBase().authStore.token` (NOT `useAuth()` — it exposes no token); `GET /api/v1/repairs` and `GET /api/v1/repairs/status-counts`.
|
||||
- Produces: the board screen at `/repairs-app`, and a `<RepairBoard />` client component.
|
||||
|
||||
- [ ] **Step 1: Write the failing test**
|
||||
|
|
@ -898,7 +898,7 @@ git commit -m "feat(repairs-app): the status board with reconciling counts"
|
|||
- Test: `src/app/(repairs-app)/repairs-app/book-in-form.test.tsx`
|
||||
|
||||
**Interfaces:**
|
||||
- Consumes: `CustomerSearch`, `SupplierSearch`, `useAuth()`, `POST /api/v1/repairs`, `POST /api/v1/repairs/[id]/files/photos`.
|
||||
- Consumes: `CustomerSearch`, `SupplierSearch`, the session header built from `getPocketBase().authStore.token` (NOT `useAuth()` — it exposes no token), `POST /api/v1/repairs`, `POST /api/v1/repairs/[id]/files/photos`.
|
||||
- Produces: the booking screen at `/repairs-app/book-in`.
|
||||
|
||||
- [ ] **Step 1: Write the failing test**
|
||||
|
|
@ -1251,7 +1251,7 @@ git commit -m "feat(repairs-app): book a machine in, with photos"
|
|||
- Test: `src/app/(repairs-app)/repairs-app/[id]/repair-detail.test.tsx`
|
||||
|
||||
**Interfaces:**
|
||||
- Consumes: `nextStepFor`, `REPAIR_STATUS`, `REPAIR_STATUS_LABEL`, `REPAIR_STATUS_VARIANT` from `@/lib/repair-status`; `useAuth()`; `GET /api/v1/repairs/[id]`, `PATCH /api/v1/repairs/[id]`, `POST /api/v1/repairs/[id]/files/quote`, `POST /api/v1/repairs/[id]/generate-repair-request`.
|
||||
- Consumes: `nextStepFor`, `REPAIR_STATUS`, `REPAIR_STATUS_LABEL`, `REPAIR_STATUS_VARIANT` from `@/lib/repair-status`; the session header built from `getPocketBase().authStore.token` (NOT `useAuth()` — it exposes no token); `GET /api/v1/repairs/[id]`, `PATCH /api/v1/repairs/[id]`, `POST /api/v1/repairs/[id]/files/quote`, `POST /api/v1/repairs/[id]/generate-repair-request`.
|
||||
- Produces: the detail screen at `/repairs-app/[id]`.
|
||||
|
||||
- [ ] **Step 1: Write the failing test**
|
||||
|
|
|
|||
|
|
@ -104,29 +104,38 @@ function repairFileUrl(repairId: string, filename: string): string {
|
|||
}
|
||||
|
||||
/**
|
||||
* Fetch the repair's photos and return a filename -> blob URL map.
|
||||
* Fetch the repair's photos and return a filename -> blob URL map, plus
|
||||
* the filenames that could not be loaded.
|
||||
*
|
||||
* The file proxy requires a session and an `<img src>` cannot carry an
|
||||
* Authorization header, so the bytes are fetched with the session and
|
||||
* the browser renders them from a blob URL.
|
||||
*
|
||||
* A failed photo is reported by name rather than skipped: a silent skip
|
||||
* would leave its tile reading "Loading…" for ever, so the operator
|
||||
* would never learn the photo did not load.
|
||||
*/
|
||||
async function loadPhotoUrls(
|
||||
repairId: string,
|
||||
files: string[],
|
||||
): Promise<Record<string, string>> {
|
||||
): Promise<{ urls: Record<string, string>; failed: string[] }> {
|
||||
const urls: Record<string, string> = {};
|
||||
const failed: string[] = [];
|
||||
for (const name of files) {
|
||||
try {
|
||||
const res = await fetch(repairFileUrl(repairId, name), {
|
||||
headers: authHeaders(),
|
||||
});
|
||||
if (!res.ok) continue;
|
||||
if (!res.ok) {
|
||||
failed.push(name);
|
||||
continue;
|
||||
}
|
||||
urls[name] = URL.createObjectURL(await res.blob());
|
||||
} catch {
|
||||
// A photo that will not load is simply not shown.
|
||||
failed.push(name);
|
||||
}
|
||||
}
|
||||
return urls;
|
||||
return { urls, failed };
|
||||
}
|
||||
|
||||
type RepairDetail = Repair & {
|
||||
|
|
@ -201,6 +210,8 @@ export default function RepairDetailPage({
|
|||
* and shown from a blob URL instead.
|
||||
*/
|
||||
const [photoUrls, setPhotoUrls] = useState<Record<string, string>>({});
|
||||
/** Photos that could not be loaded — shown as an error, not a spinner. */
|
||||
const [failedPhotos, setFailedPhotos] = useState<string[]>([]);
|
||||
|
||||
const [quoteDialogOpen, setQuoteDialogOpen] = useState(false);
|
||||
const [quoteDraft, setQuoteDraft] = useState({
|
||||
|
|
@ -238,7 +249,9 @@ export default function RepairDetailPage({
|
|||
}
|
||||
const data: RepairDetail = await res.json();
|
||||
setRepair(data);
|
||||
setPhotoUrls(await loadPhotoUrls(data.id, data.photos ?? []));
|
||||
const photos = await loadPhotoUrls(data.id, data.photos ?? []);
|
||||
setPhotoUrls(photos.urls);
|
||||
setFailedPhotos(photos.failed);
|
||||
setEdit({
|
||||
customer: data.customer ?? "",
|
||||
company: data.company ?? "",
|
||||
|
|
@ -448,7 +461,9 @@ export default function RepairDetailPage({
|
|||
fd.append("photos", p.file, p.name);
|
||||
const res = await fetch(
|
||||
`/api/v1/repairs/${repairId}/files/photos`,
|
||||
{ method: "POST", body: fd },
|
||||
// Multipart: the browser sets the Content-Type boundary, so
|
||||
// only the session header goes here.
|
||||
{ method: "POST", headers: authHeaders(), body: fd },
|
||||
);
|
||||
if (res.ok) {
|
||||
uploaded++;
|
||||
|
|
@ -919,6 +934,10 @@ export default function RepairDetailPage({
|
|||
alt={f}
|
||||
className="h-full w-full object-cover"
|
||||
/>
|
||||
) : failedPhotos.includes(f) ? (
|
||||
<span className="flex h-full w-full items-center justify-center px-1 text-center text-[10px] text-destructive">
|
||||
Could not load this photo
|
||||
</span>
|
||||
) : (
|
||||
<span className="flex h-full w-full items-center justify-center text-[10px] text-muted-foreground">
|
||||
Loading…
|
||||
|
|
|
|||
|
|
@ -122,7 +122,10 @@ export async function GET(
|
|||
if (collection === "repairs" || collection === "pbc_2000000004") {
|
||||
try {
|
||||
try {
|
||||
assertAgentAuth(req);
|
||||
// Awaited even though it is synchronous today: if it is ever
|
||||
// made async, an un-awaited rejection would skip the browser
|
||||
// session check below and let the request through unauthenticated.
|
||||
await assertAgentAuth(req);
|
||||
} catch {
|
||||
await assertBrowserSession(req);
|
||||
}
|
||||
|
|
|
|||
|
|
@ -3,11 +3,30 @@ import { describe, it, expect, beforeEach, vi } from "vitest";
|
|||
import { NextRequest } from "next/server";
|
||||
import {
|
||||
createRepair,
|
||||
uploadRepairFile,
|
||||
createCustomerQuoteFromRepair,
|
||||
generateRepairRequest,
|
||||
getRepair,
|
||||
getRepairStatusCounts,
|
||||
listCustomersForRepairForm,
|
||||
listRepairs,
|
||||
listSuppliersForRepairForm,
|
||||
removeRepairFile,
|
||||
updateRepair,
|
||||
uploadRepairFile,
|
||||
} from "@/lib/server/repairs-service";
|
||||
import { POST as createRoute } from "./route";
|
||||
import { POST as filesRoute } from "./[id]/files/[field]/route";
|
||||
import { GET as listRoute, POST as createRoute } from "./route";
|
||||
import {
|
||||
GET as getRepairRoute,
|
||||
PATCH as patchRepairRoute,
|
||||
DELETE as deleteRepairRoute,
|
||||
} from "./[id]/route";
|
||||
import {
|
||||
POST as filesRoute,
|
||||
DELETE as deleteFileRoute,
|
||||
} from "./[id]/files/[field]/route";
|
||||
import { POST as createQuoteRoute } from "./[id]/create-customer-quote/route";
|
||||
import { POST as generateRequestRoute } from "./[id]/generate-repair-request/route";
|
||||
import { GET as formDataRoute } from "./form-data/route";
|
||||
import { GET as statusCountsRoute } from "./status-counts/route";
|
||||
import { GET as fileProxyRoute } from "../files/[collection]/[recordId]/[filename]/route";
|
||||
|
||||
|
|
@ -22,6 +41,9 @@ vi.mock("@/lib/server/agent-auth", () => ({
|
|||
assertAgentAuth: (...args: unknown[]) => agentAuthMock(...args),
|
||||
}));
|
||||
|
||||
// Every service function the routes touch is mocked, so a route that
|
||||
// skips its session check reaches a stubbed success instead of the
|
||||
// network — that is what makes each 401 assertion load-bearing.
|
||||
vi.mock("@/lib/server/repairs-service", async () => {
|
||||
const actual = await vi.importActual<
|
||||
typeof import("@/lib/server/repairs-service")
|
||||
|
|
@ -29,8 +51,16 @@ vi.mock("@/lib/server/repairs-service", async () => {
|
|||
return {
|
||||
...actual,
|
||||
createRepair: vi.fn(),
|
||||
uploadRepairFile: vi.fn(),
|
||||
createCustomerQuoteFromRepair: vi.fn(),
|
||||
generateRepairRequest: vi.fn(),
|
||||
getRepair: vi.fn(),
|
||||
getRepairStatusCounts: vi.fn(),
|
||||
listCustomersForRepairForm: vi.fn(),
|
||||
listRepairs: vi.fn(),
|
||||
listSuppliersForRepairForm: vi.fn(),
|
||||
removeRepairFile: vi.fn(),
|
||||
updateRepair: vi.fn(),
|
||||
uploadRepairFile: vi.fn(),
|
||||
};
|
||||
});
|
||||
|
||||
|
|
@ -134,17 +164,53 @@ describe("repairs REST surface", () => {
|
|||
});
|
||||
});
|
||||
|
||||
/**
|
||||
* Every repairs handler requires a browser session.
|
||||
*
|
||||
* One case per handler, and each one is load-bearing: with the session
|
||||
* check removed the handler reaches its (stubbed) service and answers
|
||||
* 200/201 instead of 401, so the assertion fails. The service stubs
|
||||
* are therefore part of the test, not decoration.
|
||||
*/
|
||||
describe("repairs routes require a session", () => {
|
||||
beforeEach(() => {
|
||||
vi.clearAllMocks();
|
||||
sessionMock.mockRejectedValue(
|
||||
Object.assign(new Error("Not authenticated."), { status: 401 }),
|
||||
);
|
||||
|
||||
// If a handler forgets its check it must still land on a success
|
||||
// status, so every service below resolves.
|
||||
vi.mocked(listRepairs).mockResolvedValue({
|
||||
items: [],
|
||||
totalItems: 0,
|
||||
totalPages: 1,
|
||||
page: 1,
|
||||
perPage: 20,
|
||||
} as never);
|
||||
vi.mocked(getRepair).mockResolvedValue({ id: "rep1" } as never);
|
||||
vi.mocked(updateRepair).mockResolvedValue({ id: "rep1" } as never);
|
||||
vi.mocked(createRepair).mockResolvedValue({ id: "rep1" } as never);
|
||||
vi.mocked(uploadRepairFile).mockResolvedValue({ id: "rep1" } as never);
|
||||
vi.mocked(removeRepairFile).mockResolvedValue({ id: "rep1" } as never);
|
||||
vi.mocked(generateRepairRequest).mockResolvedValue({
|
||||
filename: "RR-1.pdf",
|
||||
} as never);
|
||||
vi.mocked(createCustomerQuoteFromRepair).mockResolvedValue({
|
||||
quote: { id: "q1" },
|
||||
} as never);
|
||||
vi.mocked(listCustomersForRepairForm).mockResolvedValue([] as never);
|
||||
vi.mocked(listSuppliersForRepairForm).mockResolvedValue([] as never);
|
||||
vi.mocked(getRepairStatusCounts).mockResolvedValue({
|
||||
total: 0,
|
||||
byStatus: {},
|
||||
} as never);
|
||||
});
|
||||
|
||||
it("rejects a list request with no session", async () => {
|
||||
const res = await createRoute(req("http://localhost/api/v1/repairs"));
|
||||
const res = await listRoute(req("http://localhost/api/v1/repairs"));
|
||||
expect(res.status).toBe(401);
|
||||
expect(listRepairs).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("rejects a create request with no session", async () => {
|
||||
|
|
@ -159,6 +225,104 @@ describe("repairs routes require a session", () => {
|
|||
}),
|
||||
);
|
||||
expect(res.status).toBe(401);
|
||||
expect(createRepair).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("rejects a read of one repair with no session", async () => {
|
||||
const res = await getRepairRoute(req("http://localhost/api/v1/repairs/rep1"), {
|
||||
params: Promise.resolve({ id: "rep1" }),
|
||||
});
|
||||
expect(res.status).toBe(401);
|
||||
expect(getRepair).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("rejects an update of one repair with no session", async () => {
|
||||
const res = await patchRepairRoute(
|
||||
req("http://localhost/api/v1/repairs/rep1", {
|
||||
method: "PATCH",
|
||||
headers: { "content-type": "application/json" },
|
||||
body: JSON.stringify({ notes: "x" }),
|
||||
}),
|
||||
{ params: Promise.resolve({ id: "rep1" }) },
|
||||
);
|
||||
expect(res.status).toBe(401);
|
||||
expect(updateRepair).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("rejects a delete of one repair with no session", async () => {
|
||||
const res = await deleteRepairRoute(
|
||||
req("http://localhost/api/v1/repairs/rep1", { method: "DELETE" }),
|
||||
{ params: Promise.resolve({ id: "rep1" }) },
|
||||
);
|
||||
expect(res.status).toBe(401);
|
||||
});
|
||||
|
||||
it("rejects a file upload with no session", async () => {
|
||||
const form = new FormData();
|
||||
form.append(
|
||||
"photos",
|
||||
new Blob([new Uint8Array([1, 2, 3])], { type: "image/jpeg" }),
|
||||
"machine.jpg",
|
||||
);
|
||||
const res = await filesRoute(
|
||||
req("http://localhost/api/v1/repairs/rep1/files/photos", {
|
||||
method: "POST",
|
||||
body: form,
|
||||
}),
|
||||
{ params: Promise.resolve({ id: "rep1", field: "photos" }) },
|
||||
);
|
||||
expect(res.status).toBe(401);
|
||||
expect(uploadRepairFile).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("rejects a file delete with no session", async () => {
|
||||
const res = await deleteFileRoute(
|
||||
req("http://localhost/api/v1/repairs/rep1/files/photos?filename=machine.jpg", {
|
||||
method: "DELETE",
|
||||
}),
|
||||
{ params: Promise.resolve({ id: "rep1", field: "photos" }) },
|
||||
);
|
||||
expect(res.status).toBe(401);
|
||||
expect(removeRepairFile).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("rejects a customer quote build with no session", async () => {
|
||||
const res = await createQuoteRoute(
|
||||
req("http://localhost/api/v1/repairs/rep1/create-customer-quote", {
|
||||
method: "POST",
|
||||
headers: { "content-type": "application/json" },
|
||||
body: JSON.stringify({ items: [{ description: "Part" }] }),
|
||||
}),
|
||||
{ params: Promise.resolve({ id: "rep1" }) },
|
||||
);
|
||||
expect(res.status).toBe(401);
|
||||
expect(createCustomerQuoteFromRepair).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("rejects a repair request build with no session", async () => {
|
||||
const res = await generateRequestRoute(
|
||||
req("http://localhost/api/v1/repairs/rep1/generate-repair-request", {
|
||||
method: "POST",
|
||||
}),
|
||||
{ params: Promise.resolve({ id: "rep1" }) },
|
||||
);
|
||||
expect(res.status).toBe(401);
|
||||
expect(generateRepairRequest).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("rejects the form lookups with no session", async () => {
|
||||
const res = await formDataRoute(req("http://localhost/api/v1/repairs/form-data"));
|
||||
expect(res.status).toBe(401);
|
||||
expect(listCustomersForRepairForm).not.toHaveBeenCalled();
|
||||
expect(listSuppliersForRepairForm).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("rejects the status counts with no session", async () => {
|
||||
const res = await statusCountsRoute(
|
||||
req("http://localhost/api/v1/repairs/status-counts"),
|
||||
);
|
||||
expect(res.status).toBe(401);
|
||||
expect(getRepairStatusCounts).not.toHaveBeenCalled();
|
||||
});
|
||||
});
|
||||
|
||||
|
|
|
|||
Loading…
Reference in a new issue