fix(pets): customer can view own pet profile summary (GRO-2013)
When a customer (e.g. uat-customer@groombook.dev) signs in via Better Auth and calls GET /api/pets/{ownPetId}/profile-summary with their portal session header, the staff RBAC middleware auto-provisions a 'groomer' staff row for them (rbac.ts) and the profile-summary route's groomerLinkageCheck then denies the request with 403 Forbidden, because the auto-provisioned customer-as-groomer has no appointment linkage. This adds an owner-bypass: when a groomer-role staff row is making the request with a valid X-Impersonation-Session-Id header, and the resolved impersonation session's clientId matches the pet's clientId, we treat the caller as the pet's owner and skip the groomerLinkageCheck. The bypass is intentionally scoped to the profile-summary endpoint and to the existing portal session mechanism (no new roles, no staff-row shape changes). Cross-tenant access is still blocked because the bypass requires session.clientId === pet.clientId. Co-Authored-By: Paperclip <noreply@paperclip.ing>
This commit is contained in:
@@ -125,6 +125,9 @@ CUSTOMER=$(kubectl get secret seed-uat-passwords -n groombook-uat \
|
|||||||
| TC-API-3.17 | Get pet profile summary — groomer restricted | GET /api/pets/{id}/profile-summary as groomer with no pet linkage | 403 Forbidden |
|
| TC-API-3.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.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.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) |
|
||||||
|
|
||||||
#### Seed Data Verification (GRO-1898)
|
#### Seed Data Verification (GRO-1898)
|
||||||
|
|
||||||
|
|||||||
@@ -44,6 +44,7 @@ interface MockState {
|
|||||||
groomingLogs: Record<string, unknown>[];
|
groomingLogs: Record<string, unknown>[];
|
||||||
staffMembers: Record<string, unknown>[];
|
staffMembers: Record<string, unknown>[];
|
||||||
services: Record<string, unknown>[];
|
services: Record<string, unknown>[];
|
||||||
|
impersonationSessions: Record<string, unknown>[];
|
||||||
}
|
}
|
||||||
|
|
||||||
let mock: MockState;
|
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-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() },
|
{ 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 groomingVisitLogs = new Proxy({ _name: "groomingVisitLogs" }, { get: (t, p) => p === "_name" ? "groomingVisitLogs" : {} });
|
||||||
const staff = new Proxy({ _name: "staff" }, { get: (t, p) => p === "_name" ? "staff" : {} });
|
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 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
|
// Tracks { [tableName]: { [alias]: SQLExpression } } for the current select() call
|
||||||
let selectedColumns: Record<string, Record<string, unknown>> = {};
|
let selectedColumns: Record<string, Record<string, unknown>> = {};
|
||||||
@@ -248,6 +263,7 @@ vi.mock("../db/index.js", () => {
|
|||||||
if (name === "groomingVisitLogs") return makeChainable(mock.groomingLogs);
|
if (name === "groomingVisitLogs") return makeChainable(mock.groomingLogs);
|
||||||
if (name === "staff") return makeChainable(mock.staffMembers);
|
if (name === "staff") return makeChainable(mock.staffMembers);
|
||||||
if (name === "services") return makeChainable(mock.services);
|
if (name === "services") return makeChainable(mock.services);
|
||||||
|
if (name === "impersonationSessions") return makeChainable(mock.impersonationSessions);
|
||||||
return makeChainable([]);
|
return makeChainable([]);
|
||||||
},
|
},
|
||||||
};
|
};
|
||||||
@@ -261,6 +277,7 @@ vi.mock("../db/index.js", () => {
|
|||||||
groomingVisitLogs,
|
groomingVisitLogs,
|
||||||
staff,
|
staff,
|
||||||
services,
|
services,
|
||||||
|
impersonationSessions,
|
||||||
and: vi.fn((a: unknown, b: unknown) => [a, b]),
|
and: vi.fn((a: unknown, b: unknown) => [a, b]),
|
||||||
desc: vi.fn((c: unknown) => c),
|
desc: vi.fn((c: unknown) => c),
|
||||||
eq: vi.fn((_col: unknown, _val: unknown) => ({ col: _col, val: _val })),
|
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.recentGroomingHistory).toEqual([]);
|
||||||
expect(body.lastVisitDate).toBeNull();
|
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);
|
||||||
|
});
|
||||||
});
|
});
|
||||||
@@ -1,7 +1,7 @@
|
|||||||
import { Hono } from "hono";
|
import { Hono } from "hono";
|
||||||
import { zValidator } from "@hono/zod-validator";
|
import { zValidator } from "@hono/zod-validator";
|
||||||
import { z } from "zod/v3";
|
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 type { AppEnv } from "../middleware/rbac.js";
|
||||||
import {
|
import {
|
||||||
getPresignedUploadUrl,
|
getPresignedUploadUrl,
|
||||||
@@ -307,10 +307,38 @@ async function groomerLinkageCheck(
|
|||||||
return !!linkage;
|
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<typeof getDb>,
|
||||||
|
c: { req: { header: (name: string) => string | undefined } }
|
||||||
|
): Promise<string | null> {
|
||||||
|
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
|
* GET /:id/profile-summary
|
||||||
* Returns aggregated profile: basic pet fields + grooming history + visit stats + upcoming appointment.
|
* Returns aggregated profile: basic pet fields + grooming history + visit stats + upcoming appointment.
|
||||||
* Groomer RBAC: same visibility rules as GET /:id.
|
* 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) => {
|
petsRouter.get("/:id/profile-summary", async (c) => {
|
||||||
const db = getDb();
|
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));
|
const [row] = await db.select().from(pets).where(eq(pets.id, petId));
|
||||||
if (!row) return c.json({ error: "Not found" }, 404);
|
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) {
|
if (isGroomer) {
|
||||||
|
const ownerClientId = await resolveImpersonationClientId(db, c);
|
||||||
|
isOwner = !!ownerClientId && ownerClientId === row.clientId;
|
||||||
|
}
|
||||||
|
|
||||||
|
if (isGroomer && !isOwner) {
|
||||||
const hasLinkage = await groomerLinkageCheck(db, row.clientId, staffRow);
|
const hasLinkage = await groomerLinkageCheck(db, row.clientId, staffRow);
|
||||||
if (!hasLinkage) return c.json({ error: "Forbidden" }, 403);
|
if (!hasLinkage) return c.json({ error: "Forbidden" }, 403);
|
||||||
}
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user