From fee62c895d8eeb3d987a15e0a98754114e45f097 Mon Sep 17 00:00:00 2001 From: Flea Flicker <22+gb_flea@noreply.git.farh.net> Date: Mon, 1 Jun 2026 18:16:29 +0000 Subject: [PATCH 1/4] =?UTF-8?q?fix(api):=20GRO-2014=20=E2=80=94=20profile-?= =?UTF-8?q?summary=20500=20=E2=86=92=20404/401/JSON-500=20(#137)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- UAT_PLAYBOOK.md | 3 + src/__tests__/petProfileSummary.test.ts | 285 ++++++++++++++++++++++++ src/routes/pets.ts | 35 ++- 3 files changed, 322 insertions(+), 1 deletion(-) create mode 100644 src/__tests__/petProfileSummary.test.ts diff --git a/UAT_PLAYBOOK.md b/UAT_PLAYBOOK.md index f0e1037..5860fbc 100644 --- a/UAT_PLAYBOOK.md +++ b/UAT_PLAYBOOK.md @@ -125,6 +125,9 @@ CUSTOMER=$(kubectl get secret seed-uat-passwords -n groombook-uat \ | TC-API-3.17 | Get pet profile summary — groomer restricted | GET /api/pets/{id}/profile-summary as groomer with no pet linkage | 403 Forbidden | | TC-API-3.18 | Get pet profile summary — visitCount returns full count | GET /api/pets/{id}/profile-summary with 2+ completed appointments | visitCount >= 2 (not capped at 1) | | TC-API-3.19 | Get pet profile summary — upcomingAppointment excludes past | GET /api/pets/{id}/profile-summary with a past confirmed/scheduled appointment | upcomingAppointment is null (past appointments filtered by startTime >= now) | +| TC-API-3.29 | Get pet profile summary — unknown UUID returns 404 (GRO-2014) | GET /api/pets/00000000-0000-0000-0000-000000000001/profile-summary while authenticated (any role) | 404 Not Found with body `{"error":"Not found"}` (was empty-body 500 in GRO-2014) | +| TC-API-3.30 | Get pet profile summary — malformed UUID returns 404 (GRO-2014) | GET /api/pets/not-a-uuid/profile-summary while authenticated | 404 Not Found with body `{"error":"Not found"}` (was empty-body 500 in GRO-2014 — Postgres uuid cast failure) | +| TC-API-3.31 | Get pet profile summary — never empty-body 500 (GRO-2014) | GET /api/pets/{anyId}/profile-summary across the test sweep | No response has status 500 with an empty body. Any 500 must include a JSON body `{"error":"Internal Server Error"}` | #### Seed Data Verification (GRO-1898) diff --git a/src/__tests__/petProfileSummary.test.ts b/src/__tests__/petProfileSummary.test.ts new file mode 100644 index 0000000..8a17d43 --- /dev/null +++ b/src/__tests__/petProfileSummary.test.ts @@ -0,0 +1,285 @@ +/** + * GET /pets/:id/profile-summary tests + * + * GRO-2014 regression coverage: + * - Empty-body 500 must never escape the route — the onError handler + * converts unhandled errors into a structured JSON 500. + * - Malformed UUIDs must return 404 (not 500 via a Postgres uuid cast). + * - Missing staff context must return 401 (not TypeError on staffRow.id). + * - Pet not found must return 404. + * - Groomer with no appointment linkage must return 403. + * - Manager and groomer with linkage must receive the summary body. + */ +import { describe, it, expect, vi, beforeEach } from "vitest"; +import { Hono } from "hono"; +import type { AppEnv, StaffRow } from "../middleware/rbac.js"; + +// ─── Fixtures ──────────────────────────────────────────────────────────────── + +const MANAGER: StaffRow = { + id: "00000000-0000-0000-0000-0000000000aa", + oidcSub: "oidc-manager-sub", + userId: null, + role: "manager", + isSuperUser: true, + name: "Manager McManager", + email: "manager@example.com", + active: true, + icalToken: null, + createdAt: new Date(), + updatedAt: new Date(), +}; + +const GROOMER: StaffRow = { + ...MANAGER, + id: "00000000-0000-0000-0000-0000000000bb", + oidcSub: "oidc-groomer-sub", + role: "groomer", + isSuperUser: false, + name: "Groomer Gary", + email: "groomer@example.com", +}; + +const PET_UUID = "11111111-1111-1111-1111-111111111111"; +const CLIENT_UUID = "22222222-2222-2222-2222-222222222222"; +const UNKNOWN_PET_UUID = "00000000-0000-0000-0000-000000000001"; + +const PET_ROW = { + id: PET_UUID, + clientId: CLIENT_UUID, + name: "Biscuit", + species: "dog", + breed: "Beagle", + coatType: "short", + petSizeCategory: "medium", + weightKg: "12.50", + dateOfBirth: new Date("2020-01-01"), +}; + +// ─── Mutable DB state ───────────────────────────────────────────────────────── + +interface DbState { + petRow: typeof PET_ROW | null; + linkageRow: { id: string } | null; + recentHistory: Array>; + visitCount: number; + upcoming: Record | null; + throwOnPetSelect: boolean; +} + +let dbState: DbState; + +function resetDb() { + dbState = { + petRow: { ...PET_ROW }, + linkageRow: { id: "appt-link" }, + recentHistory: [], + visitCount: 0, + upcoming: null, + throwOnPetSelect: false, + }; +} + +// ─── @groombook/db mock ────────────────────────────────────────────────────── +// +// Each select chain needs to know which table it's targeting and which columns +// it's projecting so we can return the right mocked rows. We thread that state +// through a per-call object whose chain methods all return `this`. The chain +// is also `then`-able so any `await` position resolves to the rows. + +vi.mock("@groombook/db", () => { + const namedTable = (name: string) => + new Proxy( + { _name: name }, + { + get(_t, p) { + if (p === "_name") return name; + return { table: name, column: p }; + }, + } + ); + + const pets = namedTable("pets"); + const appointments = namedTable("appointments"); + const services = namedTable("services"); + const staff = namedTable("staff"); + + // The full chain interface is intentionally loose — only `then` is exposed + // with a typed signature so vitest's await resolves to the right shape. + interface ChainLike { + from: (table: { _name: string }) => ChainLike; + where: (...args: unknown[]) => ChainLike; + innerJoin: (...args: unknown[]) => ChainLike; + leftJoin: (...args: unknown[]) => ChainLike; + orderBy: (...args: unknown[]) => ChainLike; + limit: (...args: unknown[]) => ChainLike; + then: ( + onfulfilled?: ((value: unknown[]) => T | PromiseLike) | null + ) => Promise; + } + + function buildSelect(projection?: Record): ChainLike { + let targetTable = ""; + + const resolveRows = (): unknown[] => { + if (targetTable === "pets") { + if (dbState.throwOnPetSelect) { + throw new Error("simulated postgres uuid cast failure"); + } + return dbState.petRow ? [dbState.petRow] : []; + } + if (targetTable === "appointments") { + const keys = projection ? Object.keys(projection) : []; + if (projection && keys.length === 1 && keys[0] === "id") { + return dbState.linkageRow ? [dbState.linkageRow] : []; + } + if (projection && keys.includes("count")) { + return [{ count: dbState.visitCount }]; + } + if (projection && keys.includes("confirmationStatus")) { + return dbState.upcoming ? [dbState.upcoming] : []; + } + return dbState.recentHistory; + } + return []; + }; + + const chain: ChainLike = { + from(table) { + targetTable = table._name; + return chain; + }, + where() { + return chain; + }, + innerJoin() { + return chain; + }, + leftJoin() { + return chain; + }, + orderBy() { + return chain; + }, + limit() { + return chain; + }, + then(onfulfilled) { + return Promise.resolve(resolveRows()).then(onfulfilled ?? undefined); + }, + }; + + return chain; + } + + return { + getDb: () => ({ + select: (projection?: Record) => buildSelect(projection), + }), + pets, + appointments, + services, + staff, + and: vi.fn(() => ({ _op: "and" })), + or: vi.fn(() => ({ _op: "or" })), + eq: vi.fn(() => ({ _op: "eq" })), + desc: vi.fn((arg: unknown) => arg), + exists: vi.fn((arg: unknown) => arg), + sql: Object.assign( + () => ({ _op: "sql" }), + { [Symbol.toPrimitive]: () => "sql" } + ), + }; +}); + +vi.mock("../lib/s3.js", () => ({ + getPresignedUploadUrl: vi.fn().mockResolvedValue("https://example.com/put"), + getPresignedGetUrl: vi.fn().mockResolvedValue("https://example.com/get"), + deleteObject: vi.fn().mockResolvedValue(undefined), +})); + +const { petsRouter } = await import("../routes/pets.js"); + +// ─── App builder ───────────────────────────────────────────────────────────── + +function buildApp(staffRow: StaffRow | null) { + const app = new Hono(); + app.use("*", async (c, next) => { + if (staffRow) c.set("staff", staffRow); + await next(); + }); + app.route("/pets", petsRouter); + return app; +} + +beforeEach(() => { + resetDb(); + vi.clearAllMocks(); +}); + +// ─── Tests ─────────────────────────────────────────────────────────────────── + +describe("GET /pets/:id/profile-summary — GRO-2014 error handling", () => { + it("returns 404 (not 500) for a malformed UUID path param", async () => { + const app = buildApp(MANAGER); + const res = await app.request("/pets/not-a-uuid/profile-summary"); + expect(res.status).toBe(404); + const body = (await res.json()) as { error: string }; + expect(body.error).toBe("Not found"); + }); + + it("returns 401 when staff context is missing (defense in depth)", async () => { + const app = buildApp(null); + const res = await app.request(`/pets/${UNKNOWN_PET_UUID}/profile-summary`); + expect(res.status).toBe(401); + const body = (await res.json()) as { error: string }; + expect(body.error).toBe("Unauthorized"); + }); + + it("returns 404 when authenticated and pet does not exist", async () => { + dbState.petRow = null; + const app = buildApp(MANAGER); + const res = await app.request(`/pets/${UNKNOWN_PET_UUID}/profile-summary`); + expect(res.status).toBe(404); + const body = (await res.json()) as { error: string }; + expect(body.error).toBe("Not found"); + }); + + it("returns 403 when groomer has no appointment linkage to the pet's client", async () => { + dbState.linkageRow = null; + const app = buildApp(GROOMER); + const res = await app.request(`/pets/${PET_UUID}/profile-summary`); + expect(res.status).toBe(403); + const body = (await res.json()) as { error: string }; + expect(body.error).toBe("Forbidden"); + }); + + it("returns 200 with summary for a manager (no groomer linkage check)", async () => { + const app = buildApp(MANAGER); + const res = await app.request(`/pets/${PET_UUID}/profile-summary`); + expect(res.status).toBe(200); + const body = (await res.json()) as Record; + expect(body.id).toBe(PET_UUID); + expect(body.name).toBe("Biscuit"); + expect(body.visitCount).toBe(0); + expect(body.upcomingAppointment).toBeNull(); + expect(body.recentGroomingHistory).toEqual([]); + }); + + it("returns 200 with summary for a groomer with linkage", async () => { + const app = buildApp(GROOMER); + const res = await app.request(`/pets/${PET_UUID}/profile-summary`); + expect(res.status).toBe(200); + const body = (await res.json()) as Record; + expect(body.id).toBe(PET_UUID); + }); + + it("returns a JSON envelope (not empty body) when a downstream query throws", async () => { + dbState.throwOnPetSelect = true; + const app = buildApp(MANAGER); + const res = await app.request(`/pets/${PET_UUID}/profile-summary`); + expect(res.status).toBe(500); + const body = (await res.json()) as { error: string }; + expect(body.error).toBe("Internal Server Error"); + }); +}); diff --git a/src/routes/pets.ts b/src/routes/pets.ts index ffe494c..9695af7 100644 --- a/src/routes/pets.ts +++ b/src/routes/pets.ts @@ -23,6 +23,23 @@ import { export const petsRouter = new Hono(); +// Convert Zod validation errors from 422 to 400 and ensure any thrown error +// returns a structured JSON body rather than Hono's default empty-body 500. +// GRO-2014: profile-summary previously bubbled unhandled errors and produced +// an empty-body 500. Mirror the onError pattern already used in invoices.ts +// and reports.ts so every error has a JSON envelope. +petsRouter.onError((err, c) => { + if (err instanceof z.ZodError) { + return c.json({ error: "Validation failed", issues: err.issues }, 400); + } + console.error("[pets] unhandled error", err); + return c.json({ error: "Internal Server Error" }, 500); +}); + +// UUID format used by all pet routes — guards path params against malformed +// values before they hit Drizzle / Postgres uuid columns (which would throw). +const uuidSchema = z.string().uuid(); + const createPetSchema = z.object({ clientId: z.string().uuid(), name: z.string().min(1).max(200), @@ -112,8 +129,24 @@ petsRouter.get("/:id", async (c) => { petsRouter.get("/:id/profile-summary", async (c) => { const db = getDb(); const petId = c.req.param("id"); + + // GRO-2014: validate UUID format before hitting Postgres. Passing a non-UUID + // string to a uuid column makes the driver throw, which previously surfaced + // as an empty-body 500 to clients. + const parsedId = uuidSchema.safeParse(petId); + if (!parsedId.success) { + return c.json({ error: "Not found" }, 404); + } + + // Defense in depth: resolveStaffMiddleware should always populate `staff` + // for protected routes (or short-circuit with 401/403 of its own). Guard + // anyway so a misconfigured route mount can't trigger a TypeError on + // staffRow.id when the linkage check runs. const staffRow = c.get("staff"); - const isGroomer = staffRow?.role === "groomer"; + if (!staffRow) { + return c.json({ error: "Unauthorized" }, 401); + } + const isGroomer = staffRow.role === "groomer"; // Fetch the pet const [pet] = await db.select().from(pets).where(eq(pets.id, petId)); From 9903b519319bdfb7735f1dbcb86973de8badfad8 Mon Sep 17 00:00:00 2001 From: The Dogfather <20+gb_dogfather@noreply.git.farh.net> Date: Mon, 1 Jun 2026 18:40:25 +0000 Subject: [PATCH 2/4] fix(pets): customer can view own pet profile summary (GRO-2013) (#135) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds an owner-bypass in the profile-summary handler for customers signed in via Better Auth, using the existing X-Impersonation-Session-Id portal session header. When a groomer-role staff row carries a valid impersonation session whose clientId matches the pet's clientId, skip groomerLinkageCheck and serve the summary. Otherwise fall through to the existing linkage check. Resolves a 403 Forbidden where the customer (auto-provisioned by resolveStaffMiddleware as a 'groomer' staff row with no appointment linkage) could not read their own pet's profile. Scope: GRO-2013 profile-summary endpoint only — no rbac.ts/schema/Dockerfile changes. Tests: 6 new cases (owner-bypass, no-header, cross-tenant, expired, manager regression, linked-groomer regression); 294/294 pass. UAT_PLAYBOOK.md: TC-API-3.19a/b/c. Closes GRO-2013. Co-authored-by: The Dogfather <20+gb_dogfather@noreply.git.farh.net> Co-committed-by: The Dogfather <20+gb_dogfather@noreply.git.farh.net> --- UAT_PLAYBOOK.md | 3 + .../src/__tests__/petProfileSummary.test.ts | 115 ++++++++++++++++++ apps/api/src/routes/pets.ts | 38 +++++- 3 files changed, 155 insertions(+), 1 deletion(-) diff --git a/UAT_PLAYBOOK.md b/UAT_PLAYBOOK.md index 5860fbc..cf80f3f 100644 --- a/UAT_PLAYBOOK.md +++ b/UAT_PLAYBOOK.md @@ -125,6 +125,9 @@ CUSTOMER=$(kubectl get secret seed-uat-passwords -n groombook-uat \ | TC-API-3.17 | Get pet profile summary — groomer restricted | GET /api/pets/{id}/profile-summary as groomer with no pet linkage | 403 Forbidden | | TC-API-3.18 | Get pet profile summary — visitCount returns full count | GET /api/pets/{id}/profile-summary with 2+ completed appointments | visitCount >= 2 (not capped at 1) | | TC-API-3.19 | Get pet profile summary — upcomingAppointment excludes past | GET /api/pets/{id}/profile-summary with a past confirmed/scheduled appointment | upcomingAppointment is null (past appointments filtered by startTime >= now) | +| TC-API-3.19a | Get pet profile summary — customer owner-bypass (GRO-2013) | Sign in as `uat-customer@groombook.dev`; `POST /api/portal/session-from-auth`; then `GET /api/pets/{ownPetId}/profile-summary` with header `X-Impersonation-Session-Id: {sessionId}` for either of the customer's seeded pets (`c0000001-0000-0000-0000-000000000002` UAT Pup Alpha, `c0000001-0000-0000-0000-000000000003` UAT Pup Beta) | 200 OK, aggregated profile returned (owner-bypass: customer with valid portal session for pet's clientId is allowed even though rbac.ts auto-provisions them as a `groomer` staff row with no appointment linkage) | +| TC-API-3.19b | Get pet profile summary — customer cross-tenant blocked (GRO-2013) | Sign in as `uat-customer@groombook.dev`; reuse the customer's sessionId from TC-API-3.19a; `GET /api/pets/{otherClientPetId}/profile-summary` for a pet owned by a different client (`c0000002-...` or any non-customer pet) | 403 Forbidden (owner-bypass requires session.clientId === pet.clientId) | +| TC-API-3.19c | Get pet profile summary — customer without portal session header | Same as TC-API-3.19a but omit the `X-Impersonation-Session-Id` header | 403 Forbidden (no owner-bypass without valid portal session) | | TC-API-3.29 | Get pet profile summary — unknown UUID returns 404 (GRO-2014) | GET /api/pets/00000000-0000-0000-0000-000000000001/profile-summary while authenticated (any role) | 404 Not Found with body `{"error":"Not found"}` (was empty-body 500 in GRO-2014) | | TC-API-3.30 | Get pet profile summary — malformed UUID returns 404 (GRO-2014) | GET /api/pets/not-a-uuid/profile-summary while authenticated | 404 Not Found with body `{"error":"Not found"}` (was empty-body 500 in GRO-2014 — Postgres uuid cast failure) | | TC-API-3.31 | Get pet profile summary — never empty-body 500 (GRO-2014) | GET /api/pets/{anyId}/profile-summary across the test sweep | No response has status 500 with an empty body. Any 500 must include a JSON body `{"error":"Internal Server Error"}` | diff --git a/apps/api/src/__tests__/petProfileSummary.test.ts b/apps/api/src/__tests__/petProfileSummary.test.ts index f7e5686..91fe3bb 100644 --- a/apps/api/src/__tests__/petProfileSummary.test.ts +++ b/apps/api/src/__tests__/petProfileSummary.test.ts @@ -44,6 +44,7 @@ interface MockState { groomingLogs: Record[]; staffMembers: Record[]; services: Record[]; + impersonationSessions: Record[]; } let mock: MockState; @@ -168,6 +169,19 @@ function resetMock() { { id: "service-1", name: "Full Groom", description: null, basePriceCents: 6000, durationMinutes: 120, active: true, createdAt: new Date(), updatedAt: new Date() }, { id: "service-2", name: "Bath & Brush", description: null, basePriceCents: 4000, durationMinutes: 60, active: true, createdAt: new Date(), updatedAt: new Date() }, ], + impersonationSessions: [ + { + id: "sess-owner", + staffId: "staff-groomer-id", + clientId: CLIENT_ID, + reason: "sso-bridge", + status: "active", + startedAt: new Date("2024-11-01"), + endedAt: null, + expiresAt: new Date("2099-01-01T00:00:00Z"), + createdAt: new Date("2024-11-01"), + }, + ], }; } @@ -177,6 +191,7 @@ vi.mock("../db/index.js", () => { const groomingVisitLogs = new Proxy({ _name: "groomingVisitLogs" }, { get: (t, p) => p === "_name" ? "groomingVisitLogs" : {} }); const staff = new Proxy({ _name: "staff" }, { get: (t, p) => p === "_name" ? "staff" : {} }); const services = new Proxy({ _name: "services" }, { get: (t, p) => p === "_name" ? "services" : {} }); + const impersonationSessions = new Proxy({ _name: "impersonationSessions" }, { get: (t, p) => p === "_name" ? "impersonationSessions" : {} }); // Tracks { [tableName]: { [alias]: SQLExpression } } for the current select() call let selectedColumns: Record> = {}; @@ -248,6 +263,7 @@ vi.mock("../db/index.js", () => { if (name === "groomingVisitLogs") return makeChainable(mock.groomingLogs); if (name === "staff") return makeChainable(mock.staffMembers); if (name === "services") return makeChainable(mock.services); + if (name === "impersonationSessions") return makeChainable(mock.impersonationSessions); return makeChainable([]); }, }; @@ -261,6 +277,7 @@ vi.mock("../db/index.js", () => { groomingVisitLogs, staff, services, + impersonationSessions, and: vi.fn((a: unknown, b: unknown) => [a, b]), desc: vi.fn((c: unknown) => c), eq: vi.fn((_col: unknown, _val: unknown) => ({ col: _col, val: _val })), @@ -399,4 +416,102 @@ describe("GET /:id/profile-summary — empty history", () => { expect(body.recentGroomingHistory).toEqual([]); expect(body.lastVisitDate).toBeNull(); }); +}); + +describe("GET /:id/profile-summary — owner-bypass via X-Impersonation-Session-Id (GRO-2013)", () => { + beforeEach(resetMock); + + // Simulates the rbac.ts auto-provisioned "groomer" that a customer gets on first login: + // role=groomer, no linkage to any appointment. + const CUSTOMER_STAFF: StaffRow = { + id: "staff-customer-id", + oidcSub: null, + userId: "user-customer-id", + role: "groomer", + isSuperUser: false, + name: "UAT Customer", + email: "uat-customer@groombook.dev", + active: true, + icalToken: null, + createdAt: new Date(), + updatedAt: new Date(), + }; + + it("customer with valid portal session for pet's client returns 200 (owner-bypass)", async () => { + const app = makeApp(CUSTOMER_STAFF); + // Groomer has no appointment linkage — proves the bypass is via portal session, not linkage. + mock.appointments = []; + const res = await app.request(`/pets/${PET_ID}/profile-summary`, { + headers: { "X-Impersonation-Session-Id": "sess-owner" }, + }); + expect(res.status).toBe(200); + const body = await res.json(); + expect(body.id).toBe(PET_ID); + expect(body.name).toBe("Biscuit"); + expect(body.clientId).toBe(CLIENT_ID); + }); + + it("customer without X-Impersonation-Session-Id header still gets 403 (no bypass)", async () => { + const app = makeApp(CUSTOMER_STAFF); + mock.appointments = []; + const res = await app.request(`/pets/${PET_ID}/profile-summary`); + expect(res.status).toBe(403); + }); + + it("customer with portal session for a DIFFERENT client gets 403 (cross-tenant blocked)", async () => { + const app = makeApp(CUSTOMER_STAFF); + mock.appointments = []; + mock.impersonationSessions = [ + { + id: "sess-other-client", + staffId: "staff-customer-id", + clientId: "00000000-0000-0000-0000-000000000099", // different from CLIENT_ID + reason: "sso-bridge", + status: "active", + startedAt: new Date("2024-11-01"), + endedAt: null, + expiresAt: new Date("2099-01-01T00:00:00Z"), + createdAt: new Date("2024-11-01"), + }, + ]; + const res = await app.request(`/pets/${PET_ID}/profile-summary`, { + headers: { "X-Impersonation-Session-Id": "sess-other-client" }, + }); + expect(res.status).toBe(403); + }); + + it("customer with expired portal session still gets 403", async () => { + const app = makeApp(CUSTOMER_STAFF); + mock.appointments = []; + mock.impersonationSessions = [ + { + id: "sess-expired", + staffId: "staff-customer-id", + clientId: CLIENT_ID, + reason: "sso-bridge", + status: "active", + startedAt: new Date("2024-01-01"), + endedAt: null, + expiresAt: new Date("2024-02-01T00:00:00Z"), // expired long ago + createdAt: new Date("2024-01-01"), + }, + ]; + const res = await app.request(`/pets/${PET_ID}/profile-summary`, { + headers: { "X-Impersonation-Session-Id": "sess-expired" }, + }); + expect(res.status).toBe(403); + }); + + it("manager does NOT need the impersonation header (existing role check still works)", async () => { + const app = makeApp(MANAGER); + const res = await app.request(`/pets/${PET_ID}/profile-summary`); + expect(res.status).toBe(200); + }); + + it("groomer with linkage to pet's client still works (regression — no regression from bypass)", async () => { + const app = makeApp(GROOMER); + // GROOMER fixture has appointments linked to staff-groomer-id in the mock state + const res = await app.request(`/pets/${PET_ID}/profile-summary`); + expect(res.status).toBe(200); + }); }); \ No newline at end of file diff --git a/apps/api/src/routes/pets.ts b/apps/api/src/routes/pets.ts index f8b6440..d678f6f 100644 --- a/apps/api/src/routes/pets.ts +++ b/apps/api/src/routes/pets.ts @@ -1,7 +1,7 @@ import { Hono } from "hono"; import { zValidator } from "@hono/zod-validator"; import { z } from "zod/v3"; -import { and, desc, eq, exists, getDb, gte, groomingVisitLogs, or, pets, appointments, staff, services, sql } from "../db/index.js"; +import { and, desc, eq, exists, getDb, gte, groomingVisitLogs, impersonationSessions, or, pets, appointments, staff, services, sql } from "../db/index.js"; import type { AppEnv } from "../middleware/rbac.js"; import { getPresignedUploadUrl, @@ -307,10 +307,38 @@ async function groomerLinkageCheck( return !!linkage; } +/** + * Resolves the clientId from the X-Impersonation-Session-Id header, if present and active. + * Used by staff routes to allow a customer (auto-provisioned as a `groomer` staff row + * by rbac.ts) to access their own pet's data when they are the rightful owner. + * + * Returns null when the header is missing, the session is unknown/expired/ended, or the + * session exists but has no clientId — callers should treat null as "no owner-bypass". + */ +async function resolveImpersonationClientId( + db: ReturnType, + c: { req: { header: (name: string) => string | undefined } } +): Promise { + const sessionId = c.req.header("X-Impersonation-Session-Id"); + if (!sessionId) return null; + const [session] = await db + .select({ clientId: impersonationSessions.clientId, status: impersonationSessions.status, expiresAt: impersonationSessions.expiresAt }) + .from(impersonationSessions) + .where(eq(impersonationSessions.id, sessionId)) + .limit(1); + if (!session) return null; + if (session.status !== "active") return null; + if (session.expiresAt <= new Date()) return null; + return session.clientId; +} + /** * GET /:id/profile-summary * Returns aggregated profile: basic pet fields + grooming history + visit stats + upcoming appointment. * Groomer RBAC: same visibility rules as GET /:id. + * Owner-bypass (GRO-2013): a customer who supplies a valid X-Impersonation-Session-Id + * for the pet's owning client may read their own pet's summary, even though rbac.ts + * auto-provisions them as a `groomer` staff row with no appointment linkage. */ petsRouter.get("/:id/profile-summary", async (c) => { const db = getDb(); @@ -321,7 +349,15 @@ petsRouter.get("/:id/profile-summary", async (c) => { const [row] = await db.select().from(pets).where(eq(pets.id, petId)); if (!row) return c.json({ error: "Not found" }, 404); + // Owner-bypass: customer with a valid portal session for this pet's client + // is allowed to view their own pet's profile summary (GRO-2013). + let isOwner = false; if (isGroomer) { + const ownerClientId = await resolveImpersonationClientId(db, c); + isOwner = !!ownerClientId && ownerClientId === row.clientId; + } + + if (isGroomer && !isOwner) { const hasLinkage = await groomerLinkageCheck(db, row.clientId, staffRow); if (!hasLinkage) return c.json({ error: "Forbidden" }, 403); } From 27accb9b39f4dec174b89b360454999e1c84653f Mon Sep 17 00:00:00 2001 From: Paperclip Date: Mon, 1 Jun 2026 19:36:22 +0000 Subject: [PATCH 3/4] fix(db): re-register 0034/0036 schema changes via idempotent 0039/0040 (GRO-2033) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Prod cumulative promotion 2026.06.01-7667288 (PR #596) revealed that 0034_extend_pet_profile_columns (temperament_score + 3 jsonb cols) and 0036_add_missing_coat_type_values (short/medium/silky) were silently skipped on the prod database, leaving the seed/reset path with: Seed failed: PostgresError: column "temperament_score" does not exist ## Root cause: drizzle high-water-mark, same shape as GRO-1999 drizzle-orm@0.38.4 `pg-core/dialect.js#migrate` only applies a journal entry when its `folderMillis` is strictly greater than the most recent `__drizzle_migrations.created_at`: if (!lastDbMigration || Number(lastDbMigration.created_at) < migration.folderMillis) { // apply SQL + record hash } `packages/db/migrations/meta/_journal.json` has 0033's when at 1779500000000 (2026-05-23) — but 0034 was registered with when 1751140800000 (2025-06-28) and 0036 with 1751480000000 (2025-07-02). Both are below the 0033 watermark, so on the prod DB (whose newest applied migration was 0033) drizzle silently skipped 0034 and 0036. 0038 (when 1780000000000) was above the watermark, so it applied — and the migrate Job exits 0 with 'migrations applied successfully!'. The schema didn't change. GRO-1999 documented the same bug for 0037 → 0038. UAT/dev are unaffected because their watermarks were already below the 0034/0036 entries when those originally ran. ## Fix Add two new idempotent migrations with monotonic 'when': - 0039_extend_pet_profile_columns_idempotent.sql, when 1780000000001: ALTER TABLE pets ADD COLUMN IF NOT EXISTS temperament_score integer; -- + temperament_flags jsonb, medical_alerts jsonb, preferred_cuts jsonb - 0040_register_missing_coat_type_values.sql, when 1780000000002: ALTER TYPE coat_type ADD VALUE IF NOT EXISTS 'short'; -- + 'medium', 'silky' Both are 'IF NOT EXISTS' — safe no-ops on UAT/dev where 0034/0036 applied normally, and effective forward-fix on prod where they were skipped. Do NOT modify 0034/0036 in place (per the GRO-1999 pattern): UAT/dev have already applied them and re-running would fail. ## Verification - packages/db/migrations/meta/_journal.json now has 41 entries with idx 39 and 40 strictly monotonic in 'when'. - python3 -c 'import json; json.load(open(...))' parses cleanly. - ALTER TYPE ADD VALUE IF NOT EXISTS is permitted inside a tx on PostgreSQL 18.3 (prod cluster image confirmed via CNPG status). ## UAT Playbook No user-visible behaviour change — schema only. Existing TC-API-3.8 / 3.9 / 3.11 / 3.13 (extended pet profile) and 3.19a (profile summary) continue to pass and now ALSO act as smoke tests after the prod image roll-forward. ## Refs - Issue: GRO-2033 - Same-shape prior bug: GRO-1999 (0037 → 0038), commit 423d4bf - Mitigation: groombook/infra PR #597 (suspend prod reset-demo-data CronJob while this lands) Co-Authored-By: Paperclip --- ..._extend_pet_profile_columns_idempotent.sql | 27 +++++++++++++++++++ ...0040_register_missing_coat_type_values.sql | 26 ++++++++++++++++++ packages/db/migrations/meta/_journal.json | 14 ++++++++++ 3 files changed, 67 insertions(+) create mode 100644 packages/db/migrations/0039_extend_pet_profile_columns_idempotent.sql create mode 100644 packages/db/migrations/0040_register_missing_coat_type_values.sql diff --git a/packages/db/migrations/0039_extend_pet_profile_columns_idempotent.sql b/packages/db/migrations/0039_extend_pet_profile_columns_idempotent.sql new file mode 100644 index 0000000..f38869b --- /dev/null +++ b/packages/db/migrations/0039_extend_pet_profile_columns_idempotent.sql @@ -0,0 +1,27 @@ +-- Migration: 0039_extend_pet_profile_columns_idempotent.sql +-- GRO-2033: re-register the temperament/medical/preferred-cuts columns from +-- 0034 with an idempotent ADD COLUMN IF NOT EXISTS + a monotonic journal +-- `when` (1780000000001), above the 0033 high-water mark (1779500000000) +-- and above the most recent applied migration 0038 (1780000000000). +-- +-- 0034_extend_pet_profile_columns.sql was authored on 2026-05-28 with +-- `when` = 1751140800000 (2025-06-28) — *below* the 0033 high-water mark +-- of 1779500000000 (2026-05-23). drizzle-orm@0.38.4 +-- (pg-core/dialect.js#migrate) only applies a migration when +-- `migration.folderMillis > lastDbMigration.created_at`, so on prod — +-- whose last applied entry was 0033 at created_at=1779500000000 — 0034 +-- was silently skipped, leaving `pets.temperament_score` (and friends) +-- missing. The migrate Job still exits 0 ("migrations applied +-- successfully!") because the journal high watermark *was* advanced by +-- 0038, but no schema change ever ran for 0034. Seed/reset then crash on: +-- PostgresError: column "temperament_score" does not exist (42703) +-- +-- Same pattern as GRO-1999 (0037 → 0038): do NOT modify 0034 in-place +-- (UAT/dev have already applied it via their lower watermarks). Add a +-- new idempotent migration with a monotonic `when` instead so existing +-- DBs apply it cleanly and fresh DBs are a no-op-after-no-op. + +ALTER TABLE "pets" ADD COLUMN IF NOT EXISTS "temperament_score" integer; +ALTER TABLE "pets" ADD COLUMN IF NOT EXISTS "temperament_flags" jsonb DEFAULT '[]'; +ALTER TABLE "pets" ADD COLUMN IF NOT EXISTS "medical_alerts" jsonb DEFAULT '[]'; +ALTER TABLE "pets" ADD COLUMN IF NOT EXISTS "preferred_cuts" jsonb DEFAULT '[]'; diff --git a/packages/db/migrations/0040_register_missing_coat_type_values.sql b/packages/db/migrations/0040_register_missing_coat_type_values.sql new file mode 100644 index 0000000..88ff7ec --- /dev/null +++ b/packages/db/migrations/0040_register_missing_coat_type_values.sql @@ -0,0 +1,26 @@ +-- Migration: 0040_register_missing_coat_type_values.sql +-- GRO-2033: re-register the 'short' / 'medium' / 'silky' coat_type enum +-- values that 0036 added with `when` = 1751480000000 — *below* the 0033 +-- high-water mark of 1779500000000. drizzle-orm@0.38.4 +-- (pg-core/dialect.js#migrate) silently skipped 0036 on prod for the same +-- reason it skipped 0034 (see 0039). 0036 itself was idempotent +-- (`ADD VALUE IF NOT EXISTS`), but its journal entry was never applied, +-- so the values are not in the prod enum. +-- +-- Same pattern as GRO-1999 (0037 → 0038) and 0039: do NOT modify 0036 in +-- place. Add a new entry with a monotonic `when` (1780000000002) so +-- existing prod re-applies it; UAT/dev are a safe no-op because the +-- statements are `IF NOT EXISTS` and the values are already there. +-- +-- Postgres restriction: `ALTER TYPE ... ADD VALUE` cannot run inside a +-- transaction block, so we emit individual auto-commit DDL statements +-- (no BEGIN/COMMIT). drizzle-kit migrate executes inside a tx; with +-- `ADD VALUE IF NOT EXISTS` Postgres is permissive and treats it as a +-- regular DDL statement that *can* run inside a tx in 9.6+ when no new +-- value is actually added. If you ever rename this to add a value that +-- doesn't exist on every target DB, lift it out of the journal +-- transaction (single-statement file) — see GRO-1999 commit 423d4bf. + +ALTER TYPE "coat_type" ADD VALUE IF NOT EXISTS 'short'; +ALTER TYPE "coat_type" ADD VALUE IF NOT EXISTS 'medium'; +ALTER TYPE "coat_type" ADD VALUE IF NOT EXISTS 'silky'; diff --git a/packages/db/migrations/meta/_journal.json b/packages/db/migrations/meta/_journal.json index 0645748..47e54de 100644 --- a/packages/db/migrations/meta/_journal.json +++ b/packages/db/migrations/meta/_journal.json @@ -267,6 +267,20 @@ "when": 1780000000000, "tag": "0038_register_extra_large_pet_size_category", "breakpoints": true + }, + { + "idx": 39, + "version": "7", + "when": 1780000000001, + "tag": "0039_extend_pet_profile_columns_idempotent", + "breakpoints": true + }, + { + "idx": 40, + "version": "7", + "when": 1780000000002, + "tag": "0040_register_missing_coat_type_values", + "breakpoints": true } ] } From a2b09ba502e268c3081bbe434ad69b0573e87865 Mon Sep 17 00:00:00 2001 From: The Dogfather <20+gb_dogfather@noreply.git.farh.net> Date: Mon, 1 Jun 2026 20:06:24 +0000 Subject: [PATCH 4/4] fix(pets): port owner-bypass into deployed tree (GRO-2013) (#139) --- src/__tests__/petProfileSummary.test.ts | 594 +++++++++++++++++------- src/routes/pets.ts | 42 +- 2 files changed, 480 insertions(+), 156 deletions(-) diff --git a/src/__tests__/petProfileSummary.test.ts b/src/__tests__/petProfileSummary.test.ts index 8a17d43..069d1cb 100644 --- a/src/__tests__/petProfileSummary.test.ts +++ b/src/__tests__/petProfileSummary.test.ts @@ -1,23 +1,37 @@ /** - * GET /pets/:id/profile-summary tests + * Pet Profile Summary Tests * - * GRO-2014 regression coverage: - * - Empty-body 500 must never escape the route — the onError handler - * converts unhandled errors into a structured JSON 500. - * - Malformed UUIDs must return 404 (not 500 via a Postgres uuid cast). - * - Missing staff context must return 401 (not TypeError on staffRow.id). - * - Pet not found must return 404. - * - Groomer with no appointment linkage must return 403. - * - Manager and groomer with linkage must receive the summary body. + * Covers GET /api/pets/:id/profile-summary in the deployed tree (root src/). + * + * Two suites share one mock harness: + * + * 1. GRO-2013 owner-bypass (the deployed-tree port of #135): + * A customer who is auto-provisioned as a `groomer` staff row by rbac.ts + * (with no appointment linkage) may still read their own pet's summary + * when they supply a valid X-Impersonation-Session-Id whose clientId + * matches the pet's clientId. + * + * 2. GRO-2014 error handling (deployed tree): + * - Empty-body 500 must never escape the route — the onError handler + * converts unhandled errors into a structured JSON 500. + * - Malformed UUIDs must return 404 (not 500 via a Postgres uuid cast). + * - Missing staff context must return 401 (not TypeError on staffRow.id). + * - Pet not found must return 404. + * - Groomer with no appointment linkage must return 403. + * - Manager and groomer with linkage must receive the summary body. + * + * Deployed tree handler: src/routes/pets.ts. The mock queries the + * `appointments` table (the live schema) for visit history, not + * `groomingVisitLogs`. */ import { describe, it, expect, vi, beforeEach } from "vitest"; import { Hono } from "hono"; import type { AppEnv, StaffRow } from "../middleware/rbac.js"; -// ─── Fixtures ──────────────────────────────────────────────────────────────── +// ─── Staff fixtures ────────────────────────────────────────────────────────── const MANAGER: StaffRow = { - id: "00000000-0000-0000-0000-0000000000aa", + id: "staff-manager-id", oidcSub: "oidc-manager-sub", userId: null, role: "manager", @@ -32,7 +46,7 @@ const MANAGER: StaffRow = { const GROOMER: StaffRow = { ...MANAGER, - id: "00000000-0000-0000-0000-0000000000bb", + id: "staff-groomer-id", oidcSub: "oidc-groomer-sub", role: "groomer", isSuperUser: false, @@ -40,186 +54,274 @@ const GROOMER: StaffRow = { email: "groomer@example.com", }; -const PET_UUID = "11111111-1111-1111-1111-111111111111"; -const CLIENT_UUID = "22222222-2222-2222-2222-222222222222"; -const UNKNOWN_PET_UUID = "00000000-0000-0000-0000-000000000001"; - -const PET_ROW = { - id: PET_UUID, - clientId: CLIENT_UUID, - name: "Biscuit", - species: "dog", - breed: "Beagle", - coatType: "short", - petSizeCategory: "medium", - weightKg: "12.50", - dateOfBirth: new Date("2020-01-01"), +/** + * Mirrors the auto-provisioned "groomer" staff row rbac.ts creates for an + * OIDC user (e.g. uat-customer@groombook.dev) on first login: role=groomer, + * no appointment linkage. + */ +const CUSTOMER_STAFF: StaffRow = { + ...MANAGER, + id: "staff-customer-id", + oidcSub: null, + userId: "user-customer-id", + role: "groomer", + name: "UAT Customer", + email: "uat-customer@groombook.dev", }; -// ─── Mutable DB state ───────────────────────────────────────────────────────── +// ─── Mutable mock state ───────────────────────────────────────────────────── -interface DbState { - petRow: typeof PET_ROW | null; - linkageRow: { id: string } | null; - recentHistory: Array>; - visitCount: number; - upcoming: Record | null; - throwOnPetSelect: boolean; -} +const CLIENT_ID = "c0000001-0000-0000-0000-000000000001"; +const PET_ID = "c0000001-0000-0000-0000-000000000002"; +const OTHER_CLIENT_PET_ID = "c0000002-0000-0000-0000-000000000099"; +const UNKNOWN_PET_UUID = "00000000-0000-0000-0000-000000000001"; -let dbState: DbState; +const futureDate = () => new Date(Date.now() + 30 * 60_000); +const pastDate = () => new Date(Date.now() - 5 * 60_000); -function resetDb() { - dbState = { - petRow: { ...PET_ROW }, - linkageRow: { id: "appt-link" }, - recentHistory: [], - visitCount: 0, - upcoming: null, - throwOnPetSelect: false, +function makePet(overrides: Record = {}) { + return { + id: PET_ID, + clientId: CLIENT_ID, + name: "Biscuit", + species: "dog", + breed: "Golden Retriever", + weightKg: "30.00", + dateOfBirth: null, + healthAlerts: null, + groomingNotes: null, + cutStyle: null, + shampooPreference: null, + specialCareNotes: null, + customFields: {}, + petSizeCategory: "large", + coatType: "double", + photoKey: null, + photoUploadedAt: null, + createdAt: new Date("2024-01-01"), + updatedAt: new Date("2024-01-01"), + ...overrides, }; } -// ─── @groombook/db mock ────────────────────────────────────────────────────── +function makeAppointment(overrides: Record = {}) { + return { + id: "appt-1", + clientId: CLIENT_ID, + petId: PET_ID, + serviceId: "service-1", + staffId: GROOMER.id, + batherStaffId: null, + status: "completed", + startTime: new Date("2024-06-01T09:00:00Z"), + endTime: new Date("2024-06-01T11:00:00Z"), + notes: null, + priceCents: 6000, + seriesId: null, + seriesIndex: null, + groupId: null, + confirmationStatus: "confirmed", + confirmedAt: null, + cancelledAt: null, + confirmationToken: null, + customerNotes: null, + createdAt: new Date("2024-05-15"), + updatedAt: new Date("2024-05-15"), + ...overrides, + }; +} + +function makeService(overrides: Record = {}) { + return { + id: "service-1", + name: "Full Groom", + description: null, + basePriceCents: 6000, + durationMinutes: 120, + active: true, + createdAt: new Date(), + updatedAt: new Date(), + ...overrides, + }; +} + +function makeSession(overrides: Record = {}) { + return { + id: "sess-owner", + staffId: CUSTOMER_STAFF.id, + clientId: CLIENT_ID, + reason: "sso-bridge", + status: "active", + startedAt: new Date(), + endedAt: null, + expiresAt: futureDate(), + createdAt: new Date(), + ...overrides, + }; +} + +// ─── DB mock state ────────────────────────────────────────────────────────── + +let petsTable: Record[]; +let appointmentsTable: Record[]; +let servicesTable: Record[]; +let sessionsTable: Record[]; + +// selectQueue: queries resolve in FIFO order. Each .from(table) result +// returns a chain that resolves to the next queued row set on a terminal +// call (.where()/.orderBy()/.limit()). // -// Each select chain needs to know which table it's targeting and which columns -// it's projecting so we can return the right mocked rows. We thread that state -// through a per-call object whose chain methods all return `this`. The chain -// is also `then`-able so any `await` position resolves to the rows. +// A queued entry of `{ table: "pets", rows: null, throw: "..." }` tells the +// mock to throw instead of returning rows — used by the GRO-2014 "JSON +// envelope on downstream error" test. Any other queued entry with `rows` +// resolves to those rows. An entry with `rows: []` returns an empty array +// (no rows, no throw). +let selectQueue: Array<{ + table: string; + rows: unknown[] | null; + throw?: string; +}> = []; + +function enqueue(table: string, rows: unknown[] = []) { + selectQueue.push({ table, rows }); +} + +function enqueueThrow(table: string, message: string) { + selectQueue.push({ table, rows: null, throw: message }); +} + +function resetMock() { + petsTable = [makePet()]; + appointmentsTable = [makeAppointment()]; + servicesTable = [makeService()]; + sessionsTable = [makeSession()]; + selectQueue = []; +} + +// ─── Module mocks ─────────────────────────────────────────────────────────── vi.mock("@groombook/db", () => { - const namedTable = (name: string) => - new Proxy( + function makeTable(name: string) { + return new Proxy( { _name: name }, { - get(_t, p) { - if (p === "_name") return name; - return { table: name, column: p }; + get(target, prop) { + if (prop === "_name") return name; + if (prop === "$inferSelect") return {}; + return { table: name, column: prop }; }, } ); - - const pets = namedTable("pets"); - const appointments = namedTable("appointments"); - const services = namedTable("services"); - const staff = namedTable("staff"); - - // The full chain interface is intentionally loose — only `then` is exposed - // with a typed signature so vitest's await resolves to the right shape. - interface ChainLike { - from: (table: { _name: string }) => ChainLike; - where: (...args: unknown[]) => ChainLike; - innerJoin: (...args: unknown[]) => ChainLike; - leftJoin: (...args: unknown[]) => ChainLike; - orderBy: (...args: unknown[]) => ChainLike; - limit: (...args: unknown[]) => ChainLike; - then: ( - onfulfilled?: ((value: unknown[]) => T | PromiseLike) | null - ) => Promise; } - function buildSelect(projection?: Record): ChainLike { - let targetTable = ""; - - const resolveRows = (): unknown[] => { - if (targetTable === "pets") { - if (dbState.throwOnPetSelect) { - throw new Error("simulated postgres uuid cast failure"); - } - return dbState.petRow ? [dbState.petRow] : []; - } - if (targetTable === "appointments") { - const keys = projection ? Object.keys(projection) : []; - if (projection && keys.length === 1 && keys[0] === "id") { - return dbState.linkageRow ? [dbState.linkageRow] : []; - } - if (projection && keys.includes("count")) { - return [{ count: dbState.visitCount }]; - } - if (projection && keys.includes("confirmationStatus")) { - return dbState.upcoming ? [dbState.upcoming] : []; - } - return dbState.recentHistory; - } - return []; + function sqlMock(_strings: TemplateStringsArray, ..._params: unknown[]) { + const queryString = _strings[0]; + return { + queryChunks: [queryString], + as: (alias: string) => ({ + queryChunks: [queryString], + fieldAlias: alias, + getSQL() { return this.queryChunks; }, + }), }; + } - const chain: ChainLike = { - from(table) { - targetTable = table._name; - return chain; - }, - where() { - return chain; - }, - innerJoin() { - return chain; - }, - leftJoin() { - return chain; - }, - orderBy() { - return chain; - }, - limit() { - return chain; - }, - then(onfulfilled) { - return Promise.resolve(resolveRows()).then(onfulfilled ?? undefined); - }, - }; + function takeQueuedRows(tableName: string): unknown[] { + const next = selectQueue.shift(); + if (next && next.table === tableName) { + if (next.throw) { + throw new Error(next.throw); + } + return next.rows ?? []; + } + return []; + } - return chain; + // Wrap a finalised result in a Proxy that exposes chainable methods + // and the resolved rows. Each call to a chainable method (where/orderBy/ + // limit/...) returns the SAME rows so the route's natural await on the + // chain resolves to the queued data. + function wrapRows(rows: unknown[]): unknown { + return new Proxy(rows, { + get(target, prop: string | symbol) { + if (prop === "where" || prop === "orderBy" || prop === "limit" + || prop === "leftJoin" || prop === "innerJoin" || prop === "from") { + return () => wrapRows(rows); + } + if (prop === "then") { + return (onFulfilled?: (v: unknown) => unknown, onRejected?: (e: unknown) => unknown) => + Promise.resolve(rows).then(onFulfilled, onRejected); + } + if (prop === Symbol.iterator) { + return function* () { for (const v of target) yield v; }; + } + if (prop === Symbol.asyncIterator) { + return async function* () { for (const v of target) yield v; }; + } + // @ts-expect-error proxy access + return target[prop]; + }, + }); } return { getDb: () => ({ - select: (projection?: Record) => buildSelect(projection), + select: (_cols?: Record) => ({ + from: (table: { _name?: string }) => wrapRows(takeQueuedRows(table._name ?? "")), + }), + insert: () => ({ values: () => ({ returning: () => [{}] }) }), + update: () => ({ set: () => ({ where: () => ({ returning: () => [{}] }) }) }), + delete: () => ({ where: () => ({ returning: () => [{}] }) }), }), - pets, - appointments, - services, - staff, - and: vi.fn(() => ({ _op: "and" })), - or: vi.fn(() => ({ _op: "or" })), - eq: vi.fn(() => ({ _op: "eq" })), - desc: vi.fn((arg: unknown) => arg), - exists: vi.fn((arg: unknown) => arg), - sql: Object.assign( - () => ({ _op: "sql" }), - { [Symbol.toPrimitive]: () => "sql" } - ), + pets: makeTable("pets"), + appointments: makeTable("appointments"), + staff: makeTable("staff"), + services: makeTable("services"), + impersonationSessions: makeTable("impersonationSessions"), + and: vi.fn((..._args: unknown[]) => ({})), + desc: vi.fn((c: unknown) => c), + eq: vi.fn((_a: unknown, _b: unknown) => ({})), + exists: vi.fn(() => true), + or: vi.fn((..._args: unknown[]) => ({})), + sql: sqlMock, }; }); vi.mock("../lib/s3.js", () => ({ - getPresignedUploadUrl: vi.fn().mockResolvedValue("https://example.com/put"), - getPresignedGetUrl: vi.fn().mockResolvedValue("https://example.com/get"), - deleteObject: vi.fn().mockResolvedValue(undefined), + getPresignedUploadUrl: vi.fn(), + getPresignedGetUrl: vi.fn(), + deleteObject: vi.fn(), })); +// ─── Import after mocks are set up ────────────────────────────────────────── + const { petsRouter } = await import("../routes/pets.js"); -// ─── App builder ───────────────────────────────────────────────────────────── +// ─── App builder ──────────────────────────────────────────────────────────── function buildApp(staffRow: StaffRow | null) { const app = new Hono(); app.use("*", async (c, next) => { - if (staffRow) c.set("staff", staffRow); + if (staffRow) { + c.set("jwtPayload", { sub: staffRow.oidcSub ?? staffRow.userId ?? "" }); + c.set("staff", staffRow); + } await next(); }); app.route("/pets", petsRouter); return app; } +// ─── Reset before each test ───────────────────────────────────────────────── + beforeEach(() => { - resetDb(); + resetMock(); vi.clearAllMocks(); }); -// ─── Tests ─────────────────────────────────────────────────────────────────── +// ─── GRO-2014 error-handling suite ────────────────────────────────────────── -describe("GET /pets/:id/profile-summary — GRO-2014 error handling", () => { +describe("GET /:id/profile-summary — GRO-2014 error handling", () => { it("returns 404 (not 500) for a malformed UUID path param", async () => { const app = buildApp(MANAGER); const res = await app.request("/pets/not-a-uuid/profile-summary"); @@ -237,7 +339,7 @@ describe("GET /pets/:id/profile-summary — GRO-2014 error handling", () => { }); it("returns 404 when authenticated and pet does not exist", async () => { - dbState.petRow = null; + enqueue("pets", []); const app = buildApp(MANAGER); const res = await app.request(`/pets/${UNKNOWN_PET_UUID}/profile-summary`); expect(res.status).toBe(404); @@ -246,40 +348,222 @@ describe("GET /pets/:id/profile-summary — GRO-2014 error handling", () => { }); it("returns 403 when groomer has no appointment linkage to the pet's client", async () => { - dbState.linkageRow = null; + enqueue("pets", petsTable); + enqueue("appointments", []); // linkage check returns empty → 403 const app = buildApp(GROOMER); - const res = await app.request(`/pets/${PET_UUID}/profile-summary`); + const res = await app.request(`/pets/${PET_ID}/profile-summary`); expect(res.status).toBe(403); const body = (await res.json()) as { error: string }; expect(body.error).toBe("Forbidden"); }); it("returns 200 with summary for a manager (no groomer linkage check)", async () => { + enqueue("pets", petsTable); + enqueue("appointments", appointmentsTable); // history + enqueue("appointments", [{ count: 1 }]); // visit count + enqueue("appointments", []); // upcoming (none) const app = buildApp(MANAGER); - const res = await app.request(`/pets/${PET_UUID}/profile-summary`); + const res = await app.request(`/pets/${PET_ID}/profile-summary`); expect(res.status).toBe(200); const body = (await res.json()) as Record; - expect(body.id).toBe(PET_UUID); + expect(body.id).toBe(PET_ID); expect(body.name).toBe("Biscuit"); - expect(body.visitCount).toBe(0); + expect(body.visitCount).toBe(1); expect(body.upcomingAppointment).toBeNull(); - expect(body.recentGroomingHistory).toEqual([]); + expect(body.recentGroomingHistory).toBeInstanceOf(Array); }); - it("returns 200 with summary for a groomer with linkage", async () => { + it("returns 200 with summary for a groomer with appointment linkage", async () => { + enqueue("pets", petsTable); + enqueue("appointments", [{ id: "appt-1" }]); // linkage found + enqueue("appointments", appointmentsTable); // history + enqueue("appointments", [{ count: 1 }]); // visit count + enqueue("appointments", []); // upcoming const app = buildApp(GROOMER); - const res = await app.request(`/pets/${PET_UUID}/profile-summary`); + const res = await app.request(`/pets/${PET_ID}/profile-summary`); expect(res.status).toBe(200); const body = (await res.json()) as Record; - expect(body.id).toBe(PET_UUID); + expect(body.id).toBe(PET_ID); }); it("returns a JSON envelope (not empty body) when a downstream query throws", async () => { - dbState.throwOnPetSelect = true; + enqueueThrow("pets", "simulated postgres uuid cast failure"); const app = buildApp(MANAGER); - const res = await app.request(`/pets/${PET_UUID}/profile-summary`); + const res = await app.request(`/pets/${PET_ID}/profile-summary`); expect(res.status).toBe(500); const body = (await res.json()) as { error: string }; expect(body.error).toBe("Internal Server Error"); }); }); + +// ─── GRO-2013 owner-bypass suite ──────────────────────────────────────────── + +describe("GET /:id/profile-summary — owner-bypass (GRO-2013)", () => { + it("returns 404 when the pet does not exist", async () => { + enqueue("pets", []); + const app = buildApp(MANAGER); + const res = await app.request(`/pets/${PET_ID}/profile-summary`); + expect(res.status).toBe(404); + }); + + it("returns 200 with aggregated profile for a manager", async () => { + enqueue("pets", petsTable); + enqueue("appointments", appointmentsTable); + enqueue("appointments", [{ count: 1 }]); + enqueue("appointments", []); + + const app = buildApp(MANAGER); + const res = await app.request(`/pets/${PET_ID}/profile-summary`); + expect(res.status).toBe(200); + const body = await res.json(); + expect(body.id).toBe(PET_ID); + expect(body.name).toBe("Biscuit"); + expect(body.recentGroomingHistory).toBeInstanceOf(Array); + expect(body.visitCount).toBe(1); + expect(body.upcomingAppointment).toBeNull(); + }); + + it("returns 200 for a groomer with appointment linkage to the pet's client", async () => { + enqueue("pets", petsTable); + enqueue("appointments", [{ id: "appt-1" }]); // linkage found + enqueue("appointments", appointmentsTable); + enqueue("appointments", [{ count: 1 }]); + enqueue("appointments", []); + + const app = buildApp(GROOMER); + const res = await app.request(`/pets/${PET_ID}/profile-summary`); + expect(res.status).toBe(200); + }); + + it("returns 403 for a groomer with no appointment linkage and no bypass header", async () => { + enqueue("pets", petsTable); + enqueue("appointments", []); // no linkage + + const app = buildApp(GROOMER); + const res = await app.request(`/pets/${PET_ID}/profile-summary`); + expect(res.status).toBe(403); + }); + + it("customer-as-groomer with valid active session for pet's client returns 200", async () => { + enqueue("pets", petsTable); + enqueue("impersonationSessions", sessionsTable); // active session found + enqueue("appointments", appointmentsTable); + enqueue("appointments", [{ count: 1 }]); + enqueue("appointments", []); + + const app = buildApp(CUSTOMER_STAFF); + const res = await app.request(`/pets/${PET_ID}/profile-summary`, { + headers: { "X-Impersonation-Session-Id": "sess-owner" }, + }); + expect(res.status).toBe(200); + const body = await res.json(); + expect(body.id).toBe(PET_ID); + }); + + it("customer-as-groomer with no header still gets 403 (no bypass)", async () => { + enqueue("pets", petsTable); + + const app = buildApp(CUSTOMER_STAFF); + const res = await app.request(`/pets/${PET_ID}/profile-summary`); + expect(res.status).toBe(403); + }); + + it("customer-as-groomer with session for a DIFFERENT client gets 403 (cross-tenant blocked)", async () => { + // Session exists but clientId !== pet.clientId → bypass does not apply + // → falls through to groomer linkage check → no linkage → 403 + enqueue("pets", petsTable); + enqueue("impersonationSessions", [ + makeSession({ + id: "sess-other-client", + clientId: "c0000000-0000-0000-0000-000000000099", // different from CLIENT_ID + }), + ]); + enqueue("appointments", []); // no linkage → 403 + + const app = buildApp(CUSTOMER_STAFF); + const res = await app.request(`/pets/${PET_ID}/profile-summary`, { + headers: { "X-Impersonation-Session-Id": "sess-other-client" }, + }); + expect(res.status).toBe(403); + }); + + it("customer-as-groomer with expired session still gets 403", async () => { + enqueue("pets", petsTable); + enqueue("impersonationSessions", [ + makeSession({ id: "sess-expired", expiresAt: pastDate() }), + ]); + enqueue("appointments", []); // no linkage → 403 + + const app = buildApp(CUSTOMER_STAFF); + const res = await app.request(`/pets/${PET_ID}/profile-summary`, { + headers: { "X-Impersonation-Session-Id": "sess-expired" }, + }); + expect(res.status).toBe(403); + }); + + it("customer-as-groomer with ended (status != active) session still gets 403", async () => { + enqueue("pets", petsTable); + enqueue("impersonationSessions", [ + makeSession({ id: "sess-ended", status: "ended" }), + ]); + enqueue("appointments", []); // no linkage → 403 + + const app = buildApp(CUSTOMER_STAFF); + const res = await app.request(`/pets/${PET_ID}/profile-summary`, { + headers: { "X-Impersonation-Session-Id": "sess-ended" }, + }); + expect(res.status).toBe(403); + }); + + it("customer-as-groomer with unknown session id still gets 403", async () => { + enqueue("pets", petsTable); + enqueue("impersonationSessions", []); // session not found + enqueue("appointments", []); // no linkage → 403 + + const app = buildApp(CUSTOMER_STAFF); + const res = await app.request(`/pets/${PET_ID}/profile-summary`, { + headers: { "X-Impersonation-Session-Id": "sess-unknown" }, + }); + expect(res.status).toBe(403); + }); + + it("manager does NOT need the impersonation header (existing role check still works)", async () => { + enqueue("pets", petsTable); + enqueue("appointments", appointmentsTable); + enqueue("appointments", [{ count: 1 }]); + enqueue("appointments", []); + + const app = buildApp(MANAGER); + const res = await app.request(`/pets/${PET_ID}/profile-summary`); + expect(res.status).toBe(200); + }); + + it("groomer with linkage to pet's client still works (regression — no regression from bypass)", async () => { + enqueue("pets", petsTable); + enqueue("appointments", [{ id: "appt-1" }]); // linkage found + enqueue("appointments", appointmentsTable); + enqueue("appointments", [{ count: 1 }]); + enqueue("appointments", []); + + const app = buildApp(GROOMER); + const res = await app.request(`/pets/${PET_ID}/profile-summary`); + expect(res.status).toBe(200); + }); + + it("owner-bypass: customer cannot view another client's pet (cross-tenant block)", async () => { + // The customer has a valid session for CLIENT_ID, but the pet belongs + // to a different client → isOwner=false → falls through to groomer + // linkage check → 403. + enqueue("pets", [ + makePet({ id: OTHER_CLIENT_PET_ID, clientId: "c0000002-0000-0000-0000-000000000002" }), + ]); + enqueue("impersonationSessions", sessionsTable); // valid session, but for CLIENT_ID + enqueue("appointments", []); // no linkage → 403 + + const app = buildApp(CUSTOMER_STAFF); + const res = await app.request(`/pets/${OTHER_CLIENT_PET_ID}/profile-summary`, { + headers: { "X-Impersonation-Session-Id": "sess-owner" }, + }); + expect(res.status).toBe(403); + }); +}); diff --git a/src/routes/pets.ts b/src/routes/pets.ts index 9695af7..45dcde2 100644 --- a/src/routes/pets.ts +++ b/src/routes/pets.ts @@ -7,6 +7,7 @@ import { eq, exists, getDb, + impersonationSessions, or, pets, appointments, @@ -126,6 +127,35 @@ petsRouter.get("/:id", async (c) => { return c.json(row); }); +/** + * Resolves the clientId from the X-Impersonation-Session-Id header, if present and active. + * Used by staff routes to allow a customer (auto-provisioned as a `groomer` staff row + * by rbac.ts) to access their own pet's data when they are the rightful owner. + * + * Returns null when the header is missing, the session is unknown/expired/ended, or the + * session exists but has no clientId — callers should treat null as "no owner-bypass". + */ +async function resolveImpersonationClientId( + db: ReturnType, + c: { req: { header: (name: string) => string | undefined } } +): Promise { + const sessionId = c.req.header("X-Impersonation-Session-Id"); + if (!sessionId) return null; + const [session] = await db + .select({ + clientId: impersonationSessions.clientId, + status: impersonationSessions.status, + expiresAt: impersonationSessions.expiresAt, + }) + .from(impersonationSessions) + .where(eq(impersonationSessions.id, sessionId)) + .limit(1); + if (!session) return null; + if (session.status !== "active") return null; + if (session.expiresAt <= new Date()) return null; + return session.clientId; +} + petsRouter.get("/:id/profile-summary", async (c) => { const db = getDb(); const petId = c.req.param("id"); @@ -152,8 +182,18 @@ petsRouter.get("/:id/profile-summary", async (c) => { const [pet] = await db.select().from(pets).where(eq(pets.id, petId)); if (!pet) return c.json({ error: "Not found" }, 404); - // Groomer RBAC: check appointment linkage to this pet's client + // Owner-bypass (GRO-2013): a customer who supplies a valid + // X-Impersonation-Session-Id for the pet's owning client may read their + // own pet's summary, even though rbac.ts auto-provisions them as a + // `groomer` staff row with no appointment linkage. + let isOwner = false; if (isGroomer) { + const ownerClientId = await resolveImpersonationClientId(db, c); + isOwner = !!ownerClientId && ownerClientId === pet.clientId; + } + + // Groomer RBAC: check appointment linkage to this pet's client + if (isGroomer && !isOwner) { const [linkage] = await db .select({ id: appointments.id }) .from(appointments)