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 <noreply@anthropic.com>

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<string, unknown> = {}) {
+  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<string, unknown> = {}) {
+  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<string, unknown> }> = [];
+
+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<string, unknown>)];
+              }
+              return [makeAuditLog(vals as Record<string, unknown>)];
+            },
+          };
+        },
+      }),
+      update: (table: { _name: string }) => ({
+        set: (data: Record<string, unknown>) => ({
+          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<string, unknown>;
+      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);
+  });
+});

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<boolean> {
+  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()