diff --git a/UAT_PLAYBOOK.md b/UAT_PLAYBOOK.md index f0e1037..ec555be 100644 --- a/UAT_PLAYBOOK.md +++ b/UAT_PLAYBOOK.md @@ -40,6 +40,24 @@ CUSTOMER=$(kubectl get secret seed-uat-passwords -n groombook-uat \ **How to apply:** at the start of every UAT run that touches TC-API-1.4 / 1.5 / 1.6 / 1.7 / 3.18 / 3.21 / 3.23, refresh these four env vars from the cluster before issuing the sign-in request. +### rbac auto-provision for Better-Auth customers (GRO-2052) + +> Applies to TC-API-3.16 / 3.19a / 3.19b / 3.19c (customer-as-owner profile-summary paths) and any future case where the test user authenticates via Better-Auth email/password and the route relies on `resolveStaffMiddleware` to resolve a `staff` row. + +**Pre-condition (rbac auto-provision):** The test user must have a row in the Better-Auth `user` table (email/password sign-in creates this automatically — see TC-API-1.6 / 1.7). On first authenticated call, `resolveStaffMiddleware` (`./src/middleware/rbac.ts`) auto-provisions a `groomer` staff row keyed by `staff.user_id = user.id` (Better-Auth branch fires before the legacy OIDC `account` branch). + +**Verify the auto-provision fired** by querying the DB after the first authenticated call: + +```sql +SELECT user_id, role FROM staff WHERE user_id = ''; +``` + +Expected: one row, `role = 'groomer'`. If zero rows return, the request hit the OIDC `account` branch and 403'd, or the user has no `user` row — fix the test sign-in path before re-running. + +**Why this matters:** without the auto-provision branch, Better-Auth email/password customers (e.g. `uat-customer@groombook.dev`) have no `account` row for the OIDC providers, so `resolveStaffMiddleware` falls through to `403 "Forbidden: no staff record found for authenticated user"` *before* `pets.ts` can run the owner-bypass added in GRO-2013. The owner-bypass code is unreachable unless the auto-provision has fired. A green TC-API-3.19a therefore implicitly proves the auto-provision worked; if 3.19a fails with the pre-fix 403, the auto-provision branch is missing from the deployed `./src` tree (see [GRO-2052](/GRO/issues/GRO-2052)). + +**How to apply:** for every run of TC-API-3.16 / 3.19a / 3.19b / 3.19c, sign in via TC-API-1.6 (email+password) first to guarantee the `user` row exists, then run the profile-summary call, then assert the `staff` row above before declaring pass. + ## Test Cases ### 4.0 Health Check @@ -125,6 +143,12 @@ 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"}` | #### Seed Data Verification (GRO-1898) 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); } 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 } ] } diff --git a/src/__tests__/petProfileSummary.test.ts b/src/__tests__/petProfileSummary.test.ts new file mode 100644 index 0000000..069d1cb --- /dev/null +++ b/src/__tests__/petProfileSummary.test.ts @@ -0,0 +1,569 @@ +/** + * Pet Profile Summary Tests + * + * 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"; + +// ─── Staff fixtures ────────────────────────────────────────────────────────── + +const MANAGER: StaffRow = { + id: "staff-manager-id", + 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: "staff-groomer-id", + oidcSub: "oidc-groomer-sub", + role: "groomer", + isSuperUser: false, + name: "Groomer Gary", + email: "groomer@example.com", +}; + +/** + * 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 mock state ───────────────────────────────────────────────────── + +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"; + +const futureDate = () => new Date(Date.now() + 30 * 60_000); +const pastDate = () => new Date(Date.now() - 5 * 60_000); + +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, + }; +} + +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()). +// +// 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", () => { + function makeTable(name: string) { + return new Proxy( + { _name: name }, + { + get(target, prop) { + if (prop === "_name") return name; + if (prop === "$inferSelect") return {}; + return { table: name, column: prop }; + }, + } + ); + } + + function sqlMock(_strings: TemplateStringsArray, ..._params: unknown[]) { + const queryString = _strings[0]; + return { + queryChunks: [queryString], + as: (alias: string) => ({ + queryChunks: [queryString], + fieldAlias: alias, + getSQL() { return this.queryChunks; }, + }), + }; + } + + 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 []; + } + + // 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: (_cols?: Record) => ({ + from: (table: { _name?: string }) => wrapRows(takeQueuedRows(table._name ?? "")), + }), + insert: () => ({ values: () => ({ returning: () => [{}] }) }), + update: () => ({ set: () => ({ where: () => ({ returning: () => [{}] }) }) }), + delete: () => ({ where: () => ({ returning: () => [{}] }) }), + }), + 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(), + getPresignedGetUrl: vi.fn(), + deleteObject: vi.fn(), +})); + +// ─── Import after mocks are set up ────────────────────────────────────────── + +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("jwtPayload", { sub: staffRow.oidcSub ?? staffRow.userId ?? "" }); + c.set("staff", staffRow); + } + await next(); + }); + app.route("/pets", petsRouter); + return app; +} + +// ─── Reset before each test ───────────────────────────────────────────────── + +beforeEach(() => { + resetMock(); + vi.clearAllMocks(); +}); + +// ─── GRO-2014 error-handling suite ────────────────────────────────────────── + +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"); + 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 () => { + enqueue("pets", []); + 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 () => { + enqueue("pets", petsTable); + enqueue("appointments", []); // linkage check returns empty → 403 + const app = buildApp(GROOMER); + 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_ID}/profile-summary`); + expect(res.status).toBe(200); + const body = (await res.json()) as Record; + expect(body.id).toBe(PET_ID); + expect(body.name).toBe("Biscuit"); + expect(body.visitCount).toBe(1); + expect(body.upcomingAppointment).toBeNull(); + expect(body.recentGroomingHistory).toBeInstanceOf(Array); + }); + + 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_ID}/profile-summary`); + expect(res.status).toBe(200); + const body = (await res.json()) as Record; + expect(body.id).toBe(PET_ID); + }); + + it("returns a JSON envelope (not empty body) when a downstream query throws", async () => { + enqueueThrow("pets", "simulated postgres uuid cast failure"); + const app = buildApp(MANAGER); + 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/__tests__/rbac.test.ts b/src/__tests__/rbac.test.ts index f975316..642ed65 100644 --- a/src/__tests__/rbac.test.ts +++ b/src/__tests__/rbac.test.ts @@ -43,42 +43,103 @@ const GROOMER: StaffRow = { // ─── Mock DB ────────────────────────────────────────────────────────────────── +// staffLookupResult drives every `from(staff)` query that doesn't go through +// the dev-mode `.limit()` shortcut. Tests that want to simulate "no staff row" +// leave it null. let staffLookupResult: StaffRow | null = null; + +// managerFallbackResult is only consumed by the dev-mode `from(staff).limit(1)` +// path (looking up the first manager when AUTH_DISABLED=true and no header). let managerFallbackResult: StaffRow | null = MANAGER; +// userLookupResult drives `from(user).limit(1)` for the Better-Auth user +// auto-provision branch (GRO-2052). Tests that simulate "no Better-Auth user" +// leave it null. +type UserRow = { id: string; name: string | null; email: string | null }; +let userLookupResult: UserRow | null = null; + +// accountLookupResult drives `from(account).limit(1)` for the legacy OIDC +// auto-provision branch. Null means "no OIDC account row". +let accountLookupResult: { id: string } | null = null; + +// insertReturningResult drives `insert(staff).values(...).returning()` for +// any auto-provision branch that actually creates a staff record. Null means +// the INSERT returned no rows (simulating a DB failure). +let insertReturningResult: StaffRow | null = null; + vi.mock("@groombook/db", () => { - const staff = new Proxy( - { _name: "staff" }, - { - get(target, prop) { - if (prop === "_name") return "staff"; - if (prop === "$inferSelect") return {}; - return { table: "staff", column: prop }; - }, - } - ); + function tableMarker(name: string) { + return new Proxy( + { _name: name }, + { + get(_target, prop) { + if (prop === "_name") return name; + if (prop === "$inferSelect") return {}; + return { table: name, column: prop }; + }, + } + ); + } + + const staff = tableMarker("staff"); + const user = tableMarker("user"); + const account = tableMarker("account"); + + function lookupFor(tableName: string) { + if (tableName === "user") return userLookupResult; + if (tableName === "account") return accountLookupResult; + return staffLookupResult; + } return { getDb: () => ({ - select: () => ({ - from: () => ({ - where: () => ({ - limit: () => { - // dev mode fallback to first manager - return managerFallbackResult ? [managerFallbackResult] : []; - }, - [Symbol.iterator]: function* () { - if (staffLookupResult) yield staffLookupResult; - }, - 0: staffLookupResult, - length: staffLookupResult ? 1 : 0, - }), + select: (_columns?: unknown) => ({ + from: (table: { _name?: string }) => { + const name = table?._name ?? "staff"; + return { + where: () => ({ + limit: () => { + // The user / account auto-provision branches always call + // `.limit(1)`; route to the per-table lookup state. + if (name === "user") + return userLookupResult ? [userLookupResult] : []; + if (name === "account") + return accountLookupResult ? [accountLookupResult] : []; + // dev-mode `from(staff).limit(1)` falls back to the first + // manager when AUTH_DISABLED is set with no header. + return managerFallbackResult ? [managerFallbackResult] : []; + }, + [Symbol.iterator]: function* () { + const row = lookupFor(name); + if (row) yield row; + }, + 0: lookupFor(name), + length: lookupFor(name) ? 1 : 0, + }), + }; + }, + }), + insert: (_table: unknown) => ({ + values: (_v: unknown) => ({ + returning: () => + insertReturningResult ? [insertReturningResult] : [], + }), + }), + update: (_table: unknown) => ({ + set: (_v: unknown) => ({ + where: () => Promise.resolve(undefined), }), }), }), staff, + user, + account, eq: vi.fn((_col: unknown, _val: unknown) => ({ col: _col, val: _val })), and: vi.fn((..._clauses: unknown[]) => ({})), + sql: Object.assign( + vi.fn((..._tpl: unknown[]) => ({})), + { raw: vi.fn(() => ({})) } + ), }; }); @@ -87,16 +148,25 @@ vi.mock("@groombook/db", () => { function resetMocks() { staffLookupResult = null; managerFallbackResult = MANAGER; + userLookupResult = null; + accountLookupResult = null; + insertReturningResult = null; } /** Build a minimal Hono app with jwtPayload pre-set, then apply a middleware. */ function buildApp( middleware: MiddlewareHandler, - handler?: (c: Context) => Response | Promise + handler?: (c: Context) => Response | Promise, + jwtOverride?: Partial<{ sub: string; email: string; name: string }> ) { const app = new Hono(); app.use("*", async (c, next) => { - c.set("jwtPayload", { sub: staffLookupResult?.userId ?? "unknown-sub" }); + const defaultSub = staffLookupResult?.userId ?? "unknown-sub"; + c.set("jwtPayload", { + sub: jwtOverride?.sub ?? defaultSub, + ...(jwtOverride?.email !== undefined ? { email: jwtOverride.email } : {}), + ...(jwtOverride?.name !== undefined ? { name: jwtOverride.name } : {}), + }); await next(); }); app.use("*", middleware); @@ -204,6 +274,125 @@ describe("resolveStaffMiddleware", () => { }); }); +// ─── Auto-provision branches (GRO-2052) ─────────────────────────────────────── +// +// Each branch creates a staff row on first authenticated request when no row +// exists yet. The Better-Auth branch (user table) is the primary path for +// email/password customers; the OIDC branch (account table) is a fallback for +// legacy authentik/google/github sessions. + +describe("resolveStaffMiddleware — auto-provision", () => { + const PROVISIONED: StaffRow = { + ...MANAGER, + id: "staff-provisioned-id", + oidcSub: null, + userId: "ba-user-customer", + role: "groomer", + isSuperUser: false, + name: "UAT Customer", + email: "uat-customer@groombook.dev", + }; + + it("Better-Auth: creates a groomer staff row when user exists but no staff record (GRO-2052)", async () => { + // No existing staff row, no OIDC account row, but a Better-Auth user row. + staffLookupResult = null; + userLookupResult = { + id: "ba-user-customer", + name: "UAT Customer", + email: "uat-customer@groombook.dev", + }; + accountLookupResult = null; + insertReturningResult = PROVISIONED; + + let capturedStaff: StaffRow | null = null; + const app = buildApp( + resolveStaffMiddleware, + (c) => { + capturedStaff = c.get("staff"); + return c.json({ ok: true }); + }, + { sub: "ba-user-customer", email: "uat-customer@groombook.dev" } + ); + + const res = await app.request("/test"); + expect(res.status).toBe(200); + expect(capturedStaff).not.toBeNull(); + expect(capturedStaff!.role).toBe("groomer"); + expect(capturedStaff!.userId).toBe("ba-user-customer"); + }); + + it("Better-Auth: returns 500 if INSERT yields no row", async () => { + staffLookupResult = null; + userLookupResult = { + id: "ba-user-customer", + name: "UAT Customer", + email: "uat-customer@groombook.dev", + }; + insertReturningResult = null; // simulate INSERT … RETURNING returning [] + + const app = buildApp(resolveStaffMiddleware, undefined, { + sub: "ba-user-customer", + email: "uat-customer@groombook.dev", + }); + + const res = await app.request("/test"); + expect(res.status).toBe(500); + const body = await res.json(); + expect(body.error).toMatch(/auto-provision failed/i); + }); + + it("Better-Auth branch runs before OIDC branch (does not require jwt.email)", async () => { + // A Better-Auth user row alone is sufficient: jwt.email is intentionally + // absent. The pre-GRO-2052 code only auto-provisioned inside `if (jwt.email)`. + staffLookupResult = null; + userLookupResult = { + id: "ba-user-customer", + name: "UAT Customer", + email: "uat-customer@groombook.dev", + }; + insertReturningResult = PROVISIONED; + + const app = buildApp(resolveStaffMiddleware, undefined, { + sub: "ba-user-customer", + }); + + const res = await app.request("/test"); + expect(res.status).toBe(200); + }); + + it("OIDC fallback: still provisions when user row is missing but account row exists", async () => { + // No staff row, no Better-Auth user, but an OIDC account row. + staffLookupResult = null; + userLookupResult = null; + accountLookupResult = { id: "oidc-account-id" }; + insertReturningResult = { ...PROVISIONED, userId: "oidc-sub" }; + + const app = buildApp(resolveStaffMiddleware, undefined, { + sub: "oidc-sub", + email: "oidc-user@example.com", + }); + + const res = await app.request("/test"); + expect(res.status).toBe(200); + }); + + it("falls through to 403 when neither Better-Auth user nor OIDC account row exists", async () => { + staffLookupResult = null; + userLookupResult = null; + accountLookupResult = null; + + const app = buildApp(resolveStaffMiddleware, undefined, { + sub: "ghost-sub", + email: "ghost@example.com", + }); + + const res = await app.request("/test"); + expect(res.status).toBe(403); + const body = await res.json(); + expect(body.error).toMatch(/no staff record/i); + }); +}); + // ─── requireRole tests ──────────────────────────────────────────────────────── describe("requireRole", () => { diff --git a/src/middleware/rbac.ts b/src/middleware/rbac.ts index de1fdec..e6fd055 100644 --- a/src/middleware/rbac.ts +++ b/src/middleware/rbac.ts @@ -1,5 +1,5 @@ import type { MiddlewareHandler } from "hono"; -import { and, eq, getDb, sql, staff, account } from "@groombook/db"; +import { and, eq, getDb, sql, staff, account, user } from "@groombook/db"; export type StaffRole = "groomer" | "receptionist" | "manager"; export type StaffRow = typeof staff.$inferSelect; @@ -111,8 +111,49 @@ export const resolveStaffMiddleware: MiddlewareHandler = async ( } } + // Auto-provision for Better-Auth users (GRO-2052): the user signed in via + // Better-Auth (email/password, magic link, etc.), so a row exists in `user` + // for jwt.sub but no `account` provider row is required. Create a minimal + // groomer staff record on first login. This is the primary auto-provision + // path; the OIDC branch below remains as a fallback for legacy accounts + // that exist in `account` but not in `user`. + const [userRow] = await db + .select({ id: user.id, name: user.name, email: user.email }) + .from(user) + .where(eq(user.id, jwt.sub)) + .limit(1); + if (userRow) { + const emailPrefix = userRow.email ? userRow.email.split("@")[0] : "Unknown"; + const name = userRow.name?.trim() || jwt.name?.trim() || emailPrefix; + + const [newStaff] = await db + .insert(staff) + .values({ + userId: jwt.sub, + email: userRow.email ?? jwt.email ?? "", + name, + role: "groomer", + isSuperUser: false, + active: true, + } as Parameters[0] extends { values: infer V } ? V : never) + .returning()!; + + if (!newStaff) { + return c.json({ error: "Forbidden: auto-provision failed" }, 500); + } + + console.log( + `[rbac] auto-provisioned staff record for Better-Auth user: ${jwt.sub} -> staff:${newStaff.id} (${name})` + ); + c.set("staff", newStaff); + await next(); + return; + } + // Auto-provision for OIDC users: check if jwt.sub has an OAuth/OIDC account - // (e.g. authentik). If so, create a groomer staff record on the fly. + // (e.g. authentik). If so, create a groomer staff record on the fly. This + // is kept for backward compatibility with legacy OIDC sessions whose user + // row may not yet exist in the Better-Auth `user` table. if (jwt.email) { const [oidcAccount] = await db .select({ id: account.id }) diff --git a/src/routes/pets.ts b/src/routes/pets.ts index ffe494c..45dcde2 100644 --- a/src/routes/pets.ts +++ b/src/routes/pets.ts @@ -7,6 +7,7 @@ import { eq, exists, getDb, + impersonationSessions, or, pets, appointments, @@ -23,6 +24,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), @@ -109,18 +127,73 @@ 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"); + + // 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)); 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)