Compare commits
3 Commits
| Author | SHA1 | Date | |
|---|---|---|---|
 Paperclip
|
a9ed681726 | ||
|
Paperclip
|
7fe578aeef | ||
|
Paperclip
|
337c0e2733 |
@@ -156,32 +156,3 @@ jobs:
|
||||
${{ github.ref == 'refs/heads/main' && 'git.farh.net/groombook/reset:latest' || '' }}
|
||||
cache-from: type=registry,ref=git.farh.net/groombook/cache:reset
|
||||
cache-to: type=registry,ref=git.farh.net/groombook/cache:reset,mode=max
|
||||
|
||||
- name: Smoke test seed image (blackhole npmjs.org)
|
||||
run: |
|
||||
set -euo pipefail
|
||||
IMAGE="git.farh.net/groombook/seed:${{ steps.version.outputs.tag }}"
|
||||
docker pull "$IMAGE"
|
||||
# GRO-1985: pnpm must be a real binary, not a Corepack shim, and must
|
||||
# not try to reach registry.npmjs.org on invocation.
|
||||
docker run --rm \
|
||||
--add-host registry.npmjs.org:127.0.0.1 \
|
||||
--entrypoint="" \
|
||||
"$IMAGE" \
|
||||
sh -c 'set -e; test "$(which pnpm)" = "/usr/local/bin/pnpm"; pnpm --version'
|
||||
echo "seed image: pnpm resolves to /usr/local/bin/pnpm and runs offline ✓"
|
||||
|
||||
- name: Smoke test reset image (blackhole npmjs.org)
|
||||
run: |
|
||||
set -euo pipefail
|
||||
IMAGE="git.farh.net/groombook/reset:${{ steps.version.outputs.tag }}"
|
||||
docker pull "$IMAGE"
|
||||
# GRO-1985: pnpm must be a real binary, not a Corepack shim, and must
|
||||
# not try to reach registry.npmjs.org on invocation. Validates the
|
||||
# hard requirement from the issue: reset runs offline.
|
||||
docker run --rm \
|
||||
--add-host registry.npmjs.org:127.0.0.1 \
|
||||
--entrypoint="" \
|
||||
"$IMAGE" \
|
||||
sh -c 'set -e; test "$(which pnpm)" = "/usr/local/bin/pnpm"; echo "HOME=$HOME"; pnpm --version'
|
||||
echo "reset image: pnpm resolves to /usr/local/bin/pnpm, HOME=/tmp, runs offline ✓"
|
||||
|
||||
+1
-13
@@ -3,12 +3,8 @@ FROM node:22-alpine AS base
|
||||
# invocations of `pnpm` work without DNS access to registry.npmjs.org.
|
||||
# The corepack shim delegates to corepack, which re-validates against
|
||||
# npmjs.org on first use — that fails in air-gapped UAT seed/migrate/reset
|
||||
# Jobs. GRO-1983 / GRO-1889 / GRO-1909 / GRO-1981 / GRO-1985.
|
||||
# Jobs. GRO-1983 / GRO-1889 / GRO-1909.
|
||||
RUN npm install -g pnpm@9.15.4
|
||||
# Belt-and-braces: disable Corepack's download fallback so that even if a
|
||||
# Corepack shim is somehow invoked at runtime, it will not try to fetch
|
||||
# pnpm from registry.npmjs.org. Belt for the real-binary trousers. GRO-1985.
|
||||
ENV COREPACK_ENABLE_DOWNLOAD_FALLBACK=0
|
||||
WORKDIR /app
|
||||
|
||||
# Install deps
|
||||
@@ -30,8 +26,6 @@ RUN pnpm --filter @groombook/types build && \
|
||||
# Runtime
|
||||
FROM node:22-alpine AS runner
|
||||
RUN npm install -g pnpm@9.15.4
|
||||
# Same defence-in-depth as base: no Corepack fallback. GRO-1985.
|
||||
ENV COREPACK_ENABLE_DOWNLOAD_FALLBACK=0
|
||||
WORKDIR /app
|
||||
ENV NODE_ENV=production
|
||||
|
||||
@@ -52,18 +46,12 @@ CMD ["node", "dist/index.js"]
|
||||
|
||||
# Migrate stage — runs drizzle-kit migrate against the database
|
||||
FROM builder AS migrate
|
||||
# pnpm needs a writable HOME for any config/state it writes. With
|
||||
# readOnlyRootFilesystem: true and runAsUser: 1000, /home/node is read-only.
|
||||
# The job pods mount a writable emptyDir at /tmp; point HOME there. GRO-1985.
|
||||
ENV HOME=/tmp
|
||||
CMD ["pnpm", "--filter", "@groombook/db", "migrate"]
|
||||
|
||||
# Seed stage — populates the database with test data
|
||||
FROM builder AS seed
|
||||
ENV HOME=/tmp
|
||||
CMD ["pnpm", "--filter", "@groombook/db", "seed"]
|
||||
|
||||
# Reset stage — drops all tables, re-runs migrations, and re-seeds
|
||||
FROM builder AS reset
|
||||
ENV HOME=/tmp
|
||||
CMD ["pnpm", "--filter", "@groombook/db", "reset"]
|
||||
|
||||
@@ -40,24 +40,6 @@ 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 = '<test-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
|
||||
@@ -146,9 +128,6 @@ Expected: one row, `role = 'groomer'`. If zero rows return, the request hit the
|
||||
| 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)
|
||||
|
||||
|
||||
@@ -1,27 +0,0 @@
|
||||
-- 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 '[]';
|
||||
@@ -1,26 +0,0 @@
|
||||
-- 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';
|
||||
@@ -267,20 +267,6 @@
|
||||
"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
|
||||
}
|
||||
]
|
||||
}
|
||||
|
||||
@@ -1,34 +1,23 @@
|
||||
/**
|
||||
* Pet Profile Summary Tests
|
||||
*
|
||||
* Covers GET /api/pets/:id/profile-summary in the deployed tree (root src/).
|
||||
* Covers GET /api/pets/:id/profile-summary in the deployed tree
|
||||
* (root src/). The headline cases validate the GRO-2013 owner-bypass:
|
||||
* 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.
|
||||
*
|
||||
* 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`.
|
||||
* Deployed tree: src/routes/pets.ts. This test mirrors the live handler
|
||||
* (which queries the `appointments` table 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 ──────────────────────────────────────────────────────────
|
||||
// ─── Mock staff fixtures ──────────────────────────────────────────────────────
|
||||
|
||||
const MANAGER: StaffRow = {
|
||||
id: "staff-manager-id",
|
||||
@@ -49,7 +38,6 @@ const GROOMER: StaffRow = {
|
||||
id: "staff-groomer-id",
|
||||
oidcSub: "oidc-groomer-sub",
|
||||
role: "groomer",
|
||||
isSuperUser: false,
|
||||
name: "Groomer Gary",
|
||||
email: "groomer@example.com",
|
||||
};
|
||||
@@ -69,12 +57,11 @@ const CUSTOMER_STAFF: StaffRow = {
|
||||
email: "uat-customer@groombook.dev",
|
||||
};
|
||||
|
||||
// ─── Mutable mock state ─────────────────────────────────────────────────────
|
||||
// ─── 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);
|
||||
@@ -160,7 +147,7 @@ function makeSession(overrides: Record<string, unknown> = {}) {
|
||||
};
|
||||
}
|
||||
|
||||
// ─── DB mock state ──────────────────────────────────────────────────────────
|
||||
// ─── DB mock state ────────────────────────────────────────────────────────────
|
||||
|
||||
let petsTable: Record<string, unknown>[];
|
||||
let appointmentsTable: Record<string, unknown>[];
|
||||
@@ -170,26 +157,12 @@ let sessionsTable: Record<string, unknown>[];
|
||||
// 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;
|
||||
}> = [];
|
||||
let selectQueue: Array<{ table: string; rows: unknown[] }> = [];
|
||||
|
||||
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()];
|
||||
@@ -198,7 +171,7 @@ function resetMock() {
|
||||
selectQueue = [];
|
||||
}
|
||||
|
||||
// ─── Module mocks ───────────────────────────────────────────────────────────
|
||||
// ─── Module mocks ─────────────────────────────────────────────────────────────
|
||||
|
||||
vi.mock("@groombook/db", () => {
|
||||
function makeTable(name: string) {
|
||||
@@ -228,12 +201,7 @@ vi.mock("@groombook/db", () => {
|
||||
|
||||
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 ?? [];
|
||||
}
|
||||
if (next && next.table === tableName) return next.rows;
|
||||
return [];
|
||||
}
|
||||
|
||||
@@ -293,113 +261,35 @@ vi.mock("../lib/s3.js", () => ({
|
||||
deleteObject: vi.fn(),
|
||||
}));
|
||||
|
||||
// ─── Import after mocks are set up ──────────────────────────────────────────
|
||||
// ─── Import after mocks are set up ────────────────────────────────────────────
|
||||
|
||||
const { petsRouter } = await import("../routes/pets.js");
|
||||
|
||||
// ─── App builder ────────────────────────────────────────────────────────────
|
||||
// ─── App builder ──────────────────────────────────────────────────────────────
|
||||
|
||||
function buildApp(staffRow: StaffRow | null) {
|
||||
function buildApp(staffRow: StaffRow) {
|
||||
const app = new Hono<AppEnv>();
|
||||
app.use("*", async (c, next) => {
|
||||
if (staffRow) {
|
||||
c.set("jwtPayload", { sub: staffRow.oidcSub ?? staffRow.userId ?? "" });
|
||||
c.set("staff", 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 ─────────────────────────────────────────────────
|
||||
// ─── Reset before each test ───────────────────────────────────────────────────
|
||||
|
||||
beforeEach(() => {
|
||||
resetMock();
|
||||
vi.clearAllMocks();
|
||||
});
|
||||
|
||||
// ─── GRO-2014 error-handling suite ──────────────────────────────────────────
|
||||
// ─── Tests ────────────────────────────────────────────────────────────────────
|
||||
|
||||
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<string, unknown>;
|
||||
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<string, unknown>;
|
||||
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)", () => {
|
||||
describe("GET /:id/profile-summary — basic access", () => {
|
||||
it("returns 404 when the pet does not exist", async () => {
|
||||
petsTable = [];
|
||||
enqueue("pets", []);
|
||||
const app = buildApp(MANAGER);
|
||||
const res = await app.request(`/pets/${PET_ID}/profile-summary`);
|
||||
@@ -407,6 +297,7 @@ describe("GET /:id/profile-summary — owner-bypass (GRO-2013)", () => {
|
||||
});
|
||||
|
||||
it("returns 200 with aggregated profile for a manager", async () => {
|
||||
// Query order: pets, recent history, visit count, upcoming
|
||||
enqueue("pets", petsTable);
|
||||
enqueue("appointments", appointmentsTable);
|
||||
enqueue("appointments", [{ count: 1 }]);
|
||||
@@ -424,9 +315,10 @@ describe("GET /:id/profile-summary — owner-bypass (GRO-2013)", () => {
|
||||
});
|
||||
|
||||
it("returns 200 for a groomer with appointment linkage to the pet's client", async () => {
|
||||
// Query order: pets, linkage check, recent history, visit count, upcoming
|
||||
enqueue("pets", petsTable);
|
||||
enqueue("appointments", [{ id: "appt-1" }]); // linkage found
|
||||
enqueue("appointments", appointmentsTable);
|
||||
enqueue("appointments", appointmentsTable); // history
|
||||
enqueue("appointments", [{ count: 1 }]);
|
||||
enqueue("appointments", []);
|
||||
|
||||
@@ -436,6 +328,7 @@ describe("GET /:id/profile-summary — owner-bypass (GRO-2013)", () => {
|
||||
});
|
||||
|
||||
it("returns 403 for a groomer with no appointment linkage and no bypass header", async () => {
|
||||
// Query order: pets, linkage check (returns empty → 403)
|
||||
enqueue("pets", petsTable);
|
||||
enqueue("appointments", []); // no linkage
|
||||
|
||||
@@ -443,8 +336,14 @@ describe("GET /:id/profile-summary — owner-bypass (GRO-2013)", () => {
|
||||
const res = await app.request(`/pets/${PET_ID}/profile-summary`);
|
||||
expect(res.status).toBe(403);
|
||||
});
|
||||
});
|
||||
|
||||
// ─── GRO-2013 owner-bypass ────────────────────────────────────────────────────
|
||||
|
||||
describe("GET /:id/profile-summary — owner-bypass (GRO-2013)", () => {
|
||||
it("customer-as-groomer with valid active session for pet's client returns 200", async () => {
|
||||
// Query order: pets, session lookup (found, active, future), recent history,
|
||||
// visit count, upcoming
|
||||
enqueue("pets", petsTable);
|
||||
enqueue("impersonationSessions", sessionsTable); // active session found
|
||||
enqueue("appointments", appointmentsTable);
|
||||
@@ -461,6 +360,7 @@ describe("GET /:id/profile-summary — owner-bypass (GRO-2013)", () => {
|
||||
});
|
||||
|
||||
it("customer-as-groomer with no header still gets 403 (no bypass)", async () => {
|
||||
// Query order: pets, session lookup (header missing → returns [], 403)
|
||||
enqueue("pets", petsTable);
|
||||
|
||||
const app = buildApp(CUSTOMER_STAFF);
|
||||
|
||||
+25
-214
@@ -43,103 +43,42 @@ 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", () => {
|
||||
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;
|
||||
}
|
||||
const staff = new Proxy(
|
||||
{ _name: "staff" },
|
||||
{
|
||||
get(target, prop) {
|
||||
if (prop === "_name") return "staff";
|
||||
if (prop === "$inferSelect") return {};
|
||||
return { table: "staff", column: prop };
|
||||
},
|
||||
}
|
||||
);
|
||||
|
||||
return {
|
||||
getDb: () => ({
|
||||
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),
|
||||
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,
|
||||
}),
|
||||
}),
|
||||
}),
|
||||
}),
|
||||
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(() => ({})) }
|
||||
),
|
||||
};
|
||||
});
|
||||
|
||||
@@ -148,25 +87,16 @@ 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<AppEnv>,
|
||||
handler?: (c: Context<AppEnv>) => Response | Promise<Response>,
|
||||
jwtOverride?: Partial<{ sub: string; email: string; name: string }>
|
||||
handler?: (c: Context<AppEnv>) => Response | Promise<Response>
|
||||
) {
|
||||
const app = new Hono<AppEnv>();
|
||||
app.use("*", async (c, next) => {
|
||||
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 } : {}),
|
||||
});
|
||||
c.set("jwtPayload", { sub: staffLookupResult?.userId ?? "unknown-sub" });
|
||||
await next();
|
||||
});
|
||||
app.use("*", middleware);
|
||||
@@ -274,125 +204,6 @@ 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", () => {
|
||||
|
||||
+2
-43
@@ -1,5 +1,5 @@
|
||||
import type { MiddlewareHandler } from "hono";
|
||||
import { and, eq, getDb, sql, staff, account, user } from "@groombook/db";
|
||||
import { and, eq, getDb, sql, staff, account } from "@groombook/db";
|
||||
|
||||
export type StaffRole = "groomer" | "receptionist" | "manager";
|
||||
export type StaffRow = typeof staff.$inferSelect;
|
||||
@@ -111,49 +111,8 @@ export const resolveStaffMiddleware: MiddlewareHandler<AppEnv> = 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<typeof db.insert>[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. This
|
||||
// is kept for backward compatibility with legacy OIDC sessions whose user
|
||||
// row may not yet exist in the Better-Auth `user` table.
|
||||
// (e.g. authentik). If so, create a groomer staff record on the fly.
|
||||
if (jwt.email) {
|
||||
const [oidcAccount] = await db
|
||||
.select({ id: account.id })
|
||||
|
||||
+1
-34
@@ -24,23 +24,6 @@ import {
|
||||
|
||||
export const petsRouter = new Hono<AppEnv>();
|
||||
|
||||
// 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),
|
||||
@@ -159,24 +142,8 @@ async function resolveImpersonationClientId(
|
||||
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");
|
||||
if (!staffRow) {
|
||||
return c.json({ error: "Unauthorized" }, 401);
|
||||
}
|
||||
const isGroomer = staffRow.role === "groomer";
|
||||
const isGroomer = staffRow?.role === "groomer";
|
||||
|
||||
// Fetch the pet
|
||||
const [pet] = await db.select().from(pets).where(eq(pets.id, petId));
|
||||
|
||||
Reference in New Issue
Block a user