From 6893cad13a5032ec0622871f1dc84de25a283f90 Mon Sep 17 00:00:00 2001 From: Scrubs McBarkley Date: Fri, 20 Mar 2026 02:35:30 +0000 Subject: [PATCH] fix: add authorization + expiry checks to impersonation endpoints, add tests Security: Add ownership verification (resolveStaff + staffId check) to GET /sessions/:id, POST /sessions/:id/log, and GET /sessions/:id/audit-log endpoints that were previously unprotected. Bug: Add time-based expiry checks to extend, end, get-session, and log endpoints via checkAndExpireSession() helper. Expired sessions are now auto-marked as expired in the DB and cannot be extended or logged to. Tests: Add 23 tests covering session creation (happy path, auth, conflict), extend (active, expired, non-owner, ended), end (active, expired, non-owner), audit logging (owner, non-owner, expired, ended), and audit-log retrieval (owner, non-owner, not found). Addresses QA review on PR #75 (GRO-66). Co-Authored-By: Claude Opus 4.6 --- apps/api/src/__tests__/impersonation.test.ts | 577 +++++++++++++++++++ apps/api/src/routes/impersonation.ts | 61 ++ 2 files changed, 638 insertions(+) create mode 100644 apps/api/src/__tests__/impersonation.test.ts diff --git a/apps/api/src/__tests__/impersonation.test.ts b/apps/api/src/__tests__/impersonation.test.ts new file mode 100644 index 0000000..8eb6217 --- /dev/null +++ b/apps/api/src/__tests__/impersonation.test.ts @@ -0,0 +1,577 @@ +import { describe, it, expect, vi, beforeEach } from "vitest"; +import { Hono } from "hono"; +import type { JwtPayload } from "../middleware/auth.js"; + +// ─── Mock data ─────────────────────────────────────────────────────────────── + +const MANAGER_STAFF = { + id: "staff-manager-id", + oidcSub: "oidc-manager-sub", + role: "manager", + name: "Manager", +}; + +const GROOMER_STAFF = { + id: "staff-groomer-id", + oidcSub: "oidc-groomer-sub", + role: "groomer", + name: "Groomer", +}; + +const CLIENT = { id: "aabbccdd-1111-2222-3333-444444444444", name: "Fido Owner" }; + +const futureDate = () => new Date(Date.now() + 30 * 60_000); +const pastDate = () => new Date(Date.now() - 5 * 60_000); + +function makeSession(overrides: Record = {}) { + return { + id: "session-uuid-1", + staffId: MANAGER_STAFF.id, + clientId: CLIENT.id, + reason: "Testing portal", + status: "active" as string, + startedAt: new Date(), + endedAt: null as Date | null, + expiresAt: futureDate(), + createdAt: new Date(), + ...overrides, + }; +} + +function makeAuditLog(overrides: Record = {}) { + return { + id: "audit-uuid-1", + sessionId: "session-uuid-1", + action: "session_started", + pageVisited: null, + metadata: null, + createdAt: new Date(), + ...overrides, + }; +} + +// ─── Queue-based mock DB ───────────────────────────────────────────────────── + +let selectQueue: unknown[][] = []; +let insertedValues: Array<{ table: string; vals: unknown }> = []; +let updatedValues: Array<{ table: string; set: Record }> = []; + +function resetMock() { + selectQueue = []; + insertedValues = []; + updatedValues = []; +} + +/** + * Returns a chainable object that acts like a drizzle query result. + * Any method call (.where, .orderBy, .limit) returns the same chainable, + * but the FIRST terminal call (.where or .orderBy when no further chain) + * resolves the result from the queue. + * + * To handle `.where().orderBy()` chaining, we make the result of shifting + * also have .orderBy/.limit methods, and we wrap the shifted array in a proxy. + */ +function makeChainableResult(data: unknown[]): unknown { + // Make data act both as array and as chainable + const arr = [...data]; + return new Proxy(arr, { + get(target, prop) { + if (prop === "orderBy" || prop === "limit") { + // Further chaining just returns the same data + return () => makeChainableResult(data); + } + // @ts-expect-error proxy access + return target[prop]; + }, + }); +} + +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 }; + }, + } + ); + } + + return { + getDb: () => ({ + select: () => ({ + from: () => ({ + where: () => { + const data = selectQueue.shift() ?? []; + return makeChainableResult(data); + }, + orderBy: () => { + const data = selectQueue.shift() ?? []; + return makeChainableResult(data); + }, + limit: () => { + const data = selectQueue.shift() ?? []; + return makeChainableResult(data); + }, + }), + }), + insert: (table: { _name: string }) => ({ + values: (vals: unknown) => { + const tableName = table?._name ?? "unknown"; + insertedValues.push({ table: tableName, vals }); + return { + returning: () => { + if (tableName === "sessions") { + return [makeSession(vals as Record)]; + } + return [makeAuditLog(vals as Record)]; + }, + }; + }, + }), + update: (table: { _name: string }) => ({ + set: (data: Record) => ({ + where: () => { + const tableName = table?._name ?? "unknown"; + updatedValues.push({ table: tableName, set: data }); + return { + returning: () => { + const base = makeSession(); + return [{ ...base, ...data }]; + }, + }; + }, + }), + }), + }), + staff: makeTable("staff"), + clients: makeTable("clients"), + impersonationSessions: makeTable("sessions"), + impersonationAuditLogs: makeTable("auditLogs"), + eq: vi.fn(), + and: vi.fn(), + desc: vi.fn(), + }; +}); + +// ─── App setup ─────────────────────────────────────────────────────────────── + +const { impersonationRouter } = await import("../routes/impersonation.js"); + +function createApp(sub: string) { + const app = new Hono<{ Variables: { jwtPayload: JwtPayload } }>(); + app.use("*", async (c, next) => { + c.set("jwtPayload", { sub } as JwtPayload); + await next(); + }); + app.route("/impersonation", impersonationRouter); + return app; +} + +function jsonPost(path: string, body: unknown) { + return { + method: "POST" as const, + headers: { "Content-Type": "application/json" }, + body: JSON.stringify(body), + }; +} + +// ─── Tests ─────────────────────────────────────────────────────────────────── + +beforeEach(() => resetMock()); + +// ─── POST /sessions — Create session ───────────────────────────────────────── + +describe("POST /impersonation/sessions", () => { + it("creates a session for a manager", async () => { + const app = createApp("oidc-manager-sub"); + selectQueue.push( + [MANAGER_STAFF], // resolveStaff + [CLIENT], // client lookup + [], // expireTimedOutSessions active query + [] // existing active check + ); + + const res = await app.request( + "/impersonation/sessions", + jsonPost("/impersonation/sessions", { clientId: CLIENT.id }) + ); + + expect(res.status).toBe(201); + expect(insertedValues.some((v) => v.table === "sessions")).toBe(true); + expect(insertedValues.some((v) => v.table === "auditLogs")).toBe(true); + }); + + it("rejects non-managers", async () => { + const app = createApp("oidc-groomer-sub"); + selectQueue.push([GROOMER_STAFF]); + + const res = await app.request( + "/impersonation/sessions", + jsonPost("/impersonation/sessions", { clientId: CLIENT.id }) + ); + + expect(res.status).toBe(403); + const body = await res.json(); + expect(body.error).toMatch(/only managers/i); + }); + + it("returns 403 when staff record not found", async () => { + const app = createApp("unknown-sub"); + selectQueue.push([]); + + const res = await app.request( + "/impersonation/sessions", + jsonPost("/impersonation/sessions", { clientId: CLIENT.id }) + ); + + expect(res.status).toBe(403); + }); + + it("returns 404 when client not found", async () => { + const app = createApp("oidc-manager-sub"); + selectQueue.push( + [MANAGER_STAFF], // resolveStaff + [] // client not found + ); + + const res = await app.request( + "/impersonation/sessions", + jsonPost("/impersonation/sessions", { clientId: CLIENT.id }) + ); + + expect(res.status).toBe(404); + }); + + it("returns 409 when active session already exists", async () => { + const app = createApp("oidc-manager-sub"); + const existing = makeSession(); + selectQueue.push( + [MANAGER_STAFF], // resolveStaff + [CLIENT], // client lookup + [], // expireTimedOutSessions + [existing] // existing active session + ); + + const res = await app.request( + "/impersonation/sessions", + jsonPost("/impersonation/sessions", { clientId: CLIENT.id }) + ); + + expect(res.status).toBe(409); + const body = await res.json(); + expect(body.error).toMatch(/already have an active/i); + }); +}); + +// ─── GET /sessions/:id — Authorization ─────────────────────────────────────── + +describe("GET /impersonation/sessions/:id", () => { + it("returns session for the owning staff member", async () => { + const app = createApp("oidc-manager-sub"); + const session = makeSession(); + selectQueue.push( + [MANAGER_STAFF], // resolveStaff + [session] // session lookup + ); + + const res = await app.request("/impersonation/sessions/session-uuid-1"); + expect(res.status).toBe(200); + }); + + it("returns 403 for a different staff member", async () => { + const app = createApp("oidc-groomer-sub"); + const session = makeSession(); // owned by manager + selectQueue.push( + [GROOMER_STAFF], // resolveStaff + [session] // session lookup + ); + + const res = await app.request("/impersonation/sessions/session-uuid-1"); + expect(res.status).toBe(403); + const body = await res.json(); + expect(body.error).toMatch(/not your session/i); + }); + + it("returns 404 for nonexistent session", async () => { + const app = createApp("oidc-manager-sub"); + selectQueue.push( + [MANAGER_STAFF], // resolveStaff + [] // no session + ); + + const res = await app.request("/impersonation/sessions/nonexistent"); + expect(res.status).toBe(404); + }); + + it("auto-expires a timed-out session", async () => { + const app = createApp("oidc-manager-sub"); + const session = makeSession({ expiresAt: pastDate() }); + selectQueue.push( + [MANAGER_STAFF], // resolveStaff + [session] // session lookup + ); + + const res = await app.request("/impersonation/sessions/session-uuid-1"); + expect(res.status).toBe(200); + const body = await res.json(); + expect(body.status).toBe("expired"); + // Should have called update to mark expired + expect(updatedValues).toHaveLength(1); + expect(updatedValues[0]!.set.status).toBe("expired"); + }); +}); + +// ─── POST /sessions/:id/extend ─────────────────────────────────────────────── + +describe("POST /impersonation/sessions/:id/extend", () => { + it("extends an active non-expired session", async () => { + const app = createApp("oidc-manager-sub"); + const session = makeSession(); + selectQueue.push( + [MANAGER_STAFF], // resolveStaff + [session] // session lookup + ); + + const res = await app.request( + "/impersonation/sessions/session-uuid-1/extend", + { method: "POST" } + ); + expect(res.status).toBe(200); + // Should have extended (updated expiresAt) and logged + expect(updatedValues).toHaveLength(1); + expect(insertedValues.some((v) => { + const vals = v.vals as Record; + return vals.action === "session_extended"; + })).toBe(true); + }); + + it("returns 400 when extending a time-expired session", async () => { + const app = createApp("oidc-manager-sub"); + const session = makeSession({ expiresAt: pastDate() }); + selectQueue.push( + [MANAGER_STAFF], // resolveStaff + [session] // session lookup + ); + + const res = await app.request( + "/impersonation/sessions/session-uuid-1/extend", + { method: "POST" } + ); + expect(res.status).toBe(400); + const body = await res.json(); + expect(body.error).toMatch(/expired/i); + }); + + it("returns 403 for non-owner", async () => { + const app = createApp("oidc-groomer-sub"); + const session = makeSession(); + selectQueue.push( + [GROOMER_STAFF], // resolveStaff + [session] // owned by manager + ); + + const res = await app.request( + "/impersonation/sessions/session-uuid-1/extend", + { method: "POST" } + ); + expect(res.status).toBe(403); + }); + + it("returns 400 for an ended session", async () => { + const app = createApp("oidc-manager-sub"); + const session = makeSession({ status: "ended" }); + selectQueue.push( + [MANAGER_STAFF], + [session] + ); + + const res = await app.request( + "/impersonation/sessions/session-uuid-1/extend", + { method: "POST" } + ); + expect(res.status).toBe(400); + const body = await res.json(); + expect(body.error).toMatch(/not active/i); + }); +}); + +// ─── POST /sessions/:id/end ────────────────────────────────────────────────── + +describe("POST /impersonation/sessions/:id/end", () => { + it("ends an active non-expired session", async () => { + const app = createApp("oidc-manager-sub"); + const session = makeSession(); + selectQueue.push( + [MANAGER_STAFF], + [session] + ); + + const res = await app.request( + "/impersonation/sessions/session-uuid-1/end", + { method: "POST" } + ); + expect(res.status).toBe(200); + expect(updatedValues).toHaveLength(1); + expect(updatedValues[0]!.set.status).toBe("ended"); + }); + + it("returns 400 when ending a time-expired session", async () => { + const app = createApp("oidc-manager-sub"); + const session = makeSession({ expiresAt: pastDate() }); + selectQueue.push( + [MANAGER_STAFF], + [session] + ); + + const res = await app.request( + "/impersonation/sessions/session-uuid-1/end", + { method: "POST" } + ); + expect(res.status).toBe(400); + const body = await res.json(); + expect(body.error).toMatch(/expired/i); + }); + + it("returns 403 for non-owner", async () => { + const app = createApp("oidc-groomer-sub"); + const session = makeSession(); + selectQueue.push( + [GROOMER_STAFF], + [session] + ); + + const res = await app.request( + "/impersonation/sessions/session-uuid-1/end", + { method: "POST" } + ); + expect(res.status).toBe(403); + }); +}); + +// ─── POST /sessions/:id/log — Authorization + expiry ───────────────────────── + +describe("POST /impersonation/sessions/:id/log", () => { + const logBody = { action: "page_visit", pageVisited: "/dashboard" }; + + it("logs an audit entry for the session owner", async () => { + const app = createApp("oidc-manager-sub"); + const session = makeSession(); + selectQueue.push( + [MANAGER_STAFF], + [session] + ); + + const res = await app.request( + "/impersonation/sessions/session-uuid-1/log", + jsonPost("/", logBody) + ); + expect(res.status).toBe(201); + expect(insertedValues.some((v) => v.table === "auditLogs")).toBe(true); + }); + + it("returns 403 for non-owner", async () => { + const app = createApp("oidc-groomer-sub"); + const session = makeSession(); + selectQueue.push( + [GROOMER_STAFF], + [session] + ); + + const res = await app.request( + "/impersonation/sessions/session-uuid-1/log", + jsonPost("/", logBody) + ); + expect(res.status).toBe(403); + const body = await res.json(); + expect(body.error).toMatch(/not your session/i); + }); + + it("returns 400 when session has expired by time", async () => { + const app = createApp("oidc-manager-sub"); + const session = makeSession({ expiresAt: pastDate() }); + selectQueue.push( + [MANAGER_STAFF], + [session] + ); + + const res = await app.request( + "/impersonation/sessions/session-uuid-1/log", + jsonPost("/", logBody) + ); + expect(res.status).toBe(400); + const body = await res.json(); + expect(body.error).toMatch(/expired/i); + }); + + it("returns 400 for an ended session", async () => { + const app = createApp("oidc-manager-sub"); + const session = makeSession({ status: "ended" }); + selectQueue.push( + [MANAGER_STAFF], + [session] + ); + + const res = await app.request( + "/impersonation/sessions/session-uuid-1/log", + jsonPost("/", logBody) + ); + expect(res.status).toBe(400); + const body = await res.json(); + expect(body.error).toMatch(/not active/i); + }); +}); + +// ─── GET /sessions/:id/audit-log — Authorization ──────────────────────────── + +describe("GET /impersonation/sessions/:id/audit-log", () => { + it("returns audit logs for the session owner", async () => { + const app = createApp("oidc-manager-sub"); + const session = makeSession(); + const logs = [makeAuditLog(), makeAuditLog({ id: "audit-uuid-2", action: "page_visit" })]; + selectQueue.push( + [MANAGER_STAFF], // resolveStaff + [session], // session lookup + logs // audit logs query (where + orderBy chain) + ); + + const res = await app.request( + "/impersonation/sessions/session-uuid-1/audit-log" + ); + expect(res.status).toBe(200); + const body = await res.json(); + expect(body).toHaveLength(2); + }); + + it("returns 403 for non-owner", async () => { + const app = createApp("oidc-groomer-sub"); + const session = makeSession(); + selectQueue.push( + [GROOMER_STAFF], + [session] + ); + + const res = await app.request( + "/impersonation/sessions/session-uuid-1/audit-log" + ); + expect(res.status).toBe(403); + const body = await res.json(); + expect(body.error).toMatch(/not your session/i); + }); + + it("returns 404 for nonexistent session", async () => { + const app = createApp("oidc-manager-sub"); + selectQueue.push( + [MANAGER_STAFF], + [] + ); + + const res = await app.request( + "/impersonation/sessions/nonexistent/audit-log" + ); + expect(res.status).toBe(404); + }); +}); diff --git a/apps/api/src/routes/impersonation.ts b/apps/api/src/routes/impersonation.ts index b1572f4..d7a627e 100644 --- a/apps/api/src/routes/impersonation.ts +++ b/apps/api/src/routes/impersonation.ts @@ -58,6 +58,24 @@ async function expireTimedOutSessions(staffId: string) { } } +/** + * Check if an active session has expired by time. If so, mark it expired in DB + * and return true. Returns false if the session is still valid. + */ +async function checkAndExpireSession( + session: typeof impersonationSessions.$inferSelect +): Promise { + if (session.status !== "active") return false; + if (session.expiresAt > new Date()) return false; + const db = getDb(); + const now = new Date(); + await db + .update(impersonationSessions) + .set({ status: "expired", endedAt: now }) + .where(eq(impersonationSessions.id, session.id)); + return true; +} + // ─── POST / — Start a new impersonation session ───────────────────────────── const startSessionSchema = z.object({ @@ -132,11 +150,25 @@ impersonationRouter.post( impersonationRouter.get("/sessions/:id", async (c) => { const db = getDb(); + const jwt = c.get("jwtPayload") as JwtPayload; + const staffRow = await resolveStaff(jwt.sub); + if (!staffRow) return c.json({ error: "Staff record not found" }, 403); + const [session] = await db .select() .from(impersonationSessions) .where(eq(impersonationSessions.id, c.req.param("id"))); if (!session) return c.json({ error: "Session not found" }, 404); + if (session.staffId !== staffRow.id) { + return c.json({ error: "Not your session" }, 403); + } + + // Auto-expire if timed out + if (await checkAndExpireSession(session)) { + session.status = "expired"; + session.endedAt = new Date(); + } + return c.json(session); }); @@ -160,6 +192,11 @@ impersonationRouter.post("/sessions/:id/extend", async (c) => { return c.json({ error: "Session is not active" }, 400); } + // Check time-based expiry + if (await checkAndExpireSession(session)) { + return c.json({ error: "Session has expired" }, 400); + } + const newExpiry = expiresAt(); const [updated] = await db .update(impersonationSessions) @@ -196,6 +233,11 @@ impersonationRouter.post("/sessions/:id/end", async (c) => { return c.json({ error: "Session is not active" }, 400); } + // Check time-based expiry + if (await checkAndExpireSession(session)) { + return c.json({ error: "Session has expired" }, 400); + } + const now = new Date(); const [updated] = await db .update(impersonationSessions) @@ -224,17 +266,29 @@ impersonationRouter.post( zValidator("json", logEntrySchema), async (c) => { const db = getDb(); + const jwt = c.get("jwtPayload") as JwtPayload; const body = c.req.valid("json"); + const staffRow = await resolveStaff(jwt.sub); + if (!staffRow) return c.json({ error: "Staff record not found" }, 403); + const [session] = await db .select() .from(impersonationSessions) .where(eq(impersonationSessions.id, c.req.param("id"))); if (!session) return c.json({ error: "Session not found" }, 404); + if (session.staffId !== staffRow.id) { + return c.json({ error: "Not your session" }, 403); + } if (session.status !== "active") { return c.json({ error: "Session is not active" }, 400); } + // Check time-based expiry + if (await checkAndExpireSession(session)) { + return c.json({ error: "Session has expired" }, 400); + } + const [entry] = await db .insert(impersonationAuditLogs) .values({ @@ -253,11 +307,18 @@ impersonationRouter.post( impersonationRouter.get("/sessions/:id/audit-log", async (c) => { const db = getDb(); + const jwt = c.get("jwtPayload") as JwtPayload; + const staffRow = await resolveStaff(jwt.sub); + if (!staffRow) return c.json({ error: "Staff record not found" }, 403); + const [session] = await db .select() .from(impersonationSessions) .where(eq(impersonationSessions.id, c.req.param("id"))); if (!session) return c.json({ error: "Session not found" }, 404); + if (session.staffId !== staffRow.id) { + return c.json({ error: "Not your session" }, 403); + } const logs = await db .select()