From a0662d1b201bc3ca85cbf7e425b32bf68fc61c1f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc-Andr=C3=A9=20Bombeck?= Date: Mon, 15 Jun 2026 00:09:56 +0200 Subject: [PATCH] fix(oura): audit credential deletes and trim integration vestiges Deleting a user's stored Oura BYO credentials drops a live connection (a token minted against the deleted app is orphaned), but the credentials DELETE route skipped the auditLog + markDisconnected the disconnect route performs, leaving the integration ledger parked at its last state with no audit trail. Mirror the disconnect route when a token was present. Drop two unused classifier type aliases and unexport the internal invalid-grant helper, and refresh the stale 'full OAuth from env' headers on the connect route and the env-credentials helper to the v1.17.1 DB-first framing. Add the missing route tests for the BYO surface this release touched: credentials GET/PUT/DELETE (422 reject, encrypt-on-write, the audit/ledger branch) and the callback CSRF / cross-user / nocreds / success paths. --- .../api/oura/callback/__tests__/route.test.ts | 115 +++++++++++++++++ src/app/api/oura/connect/route.ts | 4 +- .../oura/credentials/__tests__/route.test.ts | 122 ++++++++++++++++++ src/app/api/oura/credentials/route.ts | 18 +++ src/lib/oura/response-classifier.ts | 14 +- 5 files changed, 263 insertions(+), 10 deletions(-) create mode 100644 src/app/api/oura/callback/__tests__/route.test.ts create mode 100644 src/app/api/oura/credentials/__tests__/route.test.ts diff --git a/src/app/api/oura/callback/__tests__/route.test.ts b/src/app/api/oura/callback/__tests__/route.test.ts new file mode 100644 index 000000000..980df247c --- /dev/null +++ b/src/app/api/oura/callback/__tests__/route.test.ts @@ -0,0 +1,115 @@ +import { describe, it, expect, vi, beforeEach } from "vitest"; + +vi.mock("@/lib/api-handler", () => ({ + apiHandler: unknown>(fn: T) => fn, +})); + +vi.mock("@/lib/db", () => ({ + prisma: { user: { update: vi.fn() } }, +})); + +vi.mock("@/lib/auth/session", () => ({ getSession: vi.fn(async () => null) })); + +vi.mock("@/lib/logging/context", () => ({ + annotate: vi.fn(), + getEvent: () => ({ setError: vi.fn(), setAuth: vi.fn() }), +})); + +vi.mock("@/lib/auth/audit", () => ({ auditLog: vi.fn() })); +vi.mock("@/lib/crypto", () => ({ encrypt: (s: string) => `enc:${s}` })); +vi.mock("@/lib/integrations/status", () => ({ markReconnected: vi.fn() })); + +const { exchangeMock, getCredsMock, matchMock, verifyMock } = vi.hoisted(() => ({ + exchangeMock: vi.fn(), + getCredsMock: vi.fn(), + matchMock: vi.fn(), + verifyMock: vi.fn(), +})); + +vi.mock("@/lib/oura/client", () => ({ exchangeCode: exchangeMock })); +vi.mock("@/lib/oura/credentials", () => ({ + getOuraClientCredentials: getCredsMock, +})); +vi.mock("@/lib/oauth/signed-state", () => ({ + oauthStateCookieName: (p: string) => `${p}_state`, + stateMatchesCookie: matchMock, + verifySignedState: verifyMock, +})); + +import { GET } from "../route"; +import { prisma } from "@/lib/db"; +import { auditLog } from "@/lib/auth/audit"; +import { getSession } from "@/lib/auth/session"; +import type { NextRequest } from "next/server"; + +const userUpdate = prisma.user.update as ReturnType; + +process.env.NEXT_PUBLIC_APP_URL = "https://app.example"; + +function makeReq(opts: { code?: string; state?: string; cookie?: string }): NextRequest { + const params = new URLSearchParams(); + if (opts.code) params.set("code", opts.code); + if (opts.state) params.set("state", opts.state); + return { + url: `https://app.example/api/oura/callback?${params}`, + cookies: { + get: (name: string) => + name === "oura_state" && opts.cookie ? { value: opts.cookie } : undefined, + }, + } as unknown as NextRequest; +} + +beforeEach(() => { + vi.clearAllMocks(); + matchMock.mockReturnValue(true); + verifyMock.mockReturnValue({ userId: "u1" }); + getCredsMock.mockResolvedValue({ clientId: "c", clientSecret: "s" }); + exchangeMock.mockResolvedValue({ access_token: "at", refresh_token: "rt" }); + userUpdate.mockResolvedValue({}); + (getSession as ReturnType).mockResolvedValue(null); +}); + +describe("GET /api/oura/callback", () => { + it("rejects a cookie/state mismatch as csrf1 before any exchange", async () => { + matchMock.mockReturnValue(false); + const res = await GET(makeReq({ code: "c", state: "S", cookie: "X" })); + expect(res.headers.get("location")).toContain("reason=csrf1"); + expect(exchangeMock).not.toHaveBeenCalled(); + }); + + it("rejects an unverifiable signed state", async () => { + verifyMock.mockReturnValue(null); + const res = await GET(makeReq({ code: "c", state: "S", cookie: "S" })); + expect(res.headers.get("location")).toContain("reason=state"); + expect(exchangeMock).not.toHaveBeenCalled(); + }); + + it("rejects when the verified userId mismatches a live session (cross_user)", async () => { + (getSession as ReturnType).mockResolvedValue({ + user: { id: "attacker" }, + }); + const res = await GET(makeReq({ code: "c", state: "S", cookie: "S" })); + expect(res.headers.get("location")).toContain("reason=cross_user"); + expect(exchangeMock).not.toHaveBeenCalled(); + }); + + it("falls back to nocreds when no BYO/env credentials resolve", async () => { + getCredsMock.mockResolvedValue(null); + const res = await GET(makeReq({ code: "c", state: "S", cookie: "S" })); + expect(res.headers.get("location")).toContain("reason=nocreds"); + expect(exchangeMock).not.toHaveBeenCalled(); + }); + + it("exchanges the code, persists encrypted tokens, and audits on success", async () => { + const res = await GET(makeReq({ code: "auth-code", state: "S", cookie: "S" })); + expect(exchangeMock).toHaveBeenCalledWith("auth-code", { + clientId: "c", + clientSecret: "s", + }); + const data = userUpdate.mock.calls[0]![0].data; + expect(data.ouraAccessTokenEncrypted).toBe("enc:at"); + expect(data.ouraRefreshTokenEncrypted).toBe("enc:rt"); + expect(auditLog).toHaveBeenCalledWith("oura.connect", { userId: "u1" }); + expect(res.headers.get("location")).toContain("oura=connected"); + }); +}); diff --git a/src/app/api/oura/connect/route.ts b/src/app/api/oura/connect/route.ts index e7cd4f237..ce2314afb 100644 --- a/src/app/api/oura/connect/route.ts +++ b/src/app/api/oura/connect/route.ts @@ -15,7 +15,9 @@ import { NextResponse } from "next/server"; /** * v1.17.0 (F4) — redirect the user to the Oura consent screen. * - * Full OAuth from env (`OURA_CLIENT_ID` / `OURA_CLIENT_SECRET`). CSRF via the + * Credentials resolve DB-first then env (`getOuraClientCredentials`): a user's + * BYO Oura client id/secret wins, falling back to the shared env app + * (`OURA_CLIENT_ID` / `OURA_CLIENT_SECRET`) when none is stored. CSRF via the * stateless signed state (`src/lib/oauth/signed-state.ts`): the same token is * the `state` URL param AND an httpOnly cookie. Rate-limited per user. */ diff --git a/src/app/api/oura/credentials/__tests__/route.test.ts b/src/app/api/oura/credentials/__tests__/route.test.ts new file mode 100644 index 000000000..e956cdee8 --- /dev/null +++ b/src/app/api/oura/credentials/__tests__/route.test.ts @@ -0,0 +1,122 @@ +import { describe, it, expect, vi, beforeEach } from "vitest"; + +vi.mock("@/lib/api-handler", () => ({ + apiHandler: unknown>(fn: T) => fn, + requireAuth: vi.fn(async () => ({ user: { id: "u1" } })), +})); + +vi.mock("@/lib/db", () => ({ + prisma: { user: { findUnique: vi.fn(), update: vi.fn() } }, +})); + +vi.mock("@/lib/logging/context", () => ({ annotate: vi.fn() })); +vi.mock("@/lib/auth/audit", () => ({ auditLog: vi.fn() })); +vi.mock("@/lib/integrations/status", () => ({ markDisconnected: vi.fn() })); + +const { storeMock, clearMock } = vi.hoisted(() => ({ + storeMock: vi.fn(), + clearMock: vi.fn(), +})); +vi.mock("@/lib/oura/credentials", () => ({ + storeOuraClientCredentials: storeMock, + clearOuraClientCredentials: clearMock, +})); + +vi.mock("@/lib/api-response", () => ({ + apiSuccess: (data: unknown) => ({ data, error: null, status: 200 }), + apiError: (error: string, status: number) => ({ data: null, error, status }), + safeJson: vi.fn(), +})); + +import { GET, PUT, DELETE } from "../route"; +import { prisma } from "@/lib/db"; +import { auditLog } from "@/lib/auth/audit"; +import { markDisconnected } from "@/lib/integrations/status"; +import { safeJson } from "@/lib/api-response"; +import type { NextRequest } from "next/server"; + +const userFind = prisma.user.findUnique as ReturnType; +const userUpdate = prisma.user.update as ReturnType; +const safeJsonMock = safeJson as ReturnType; + +type RouteResult = { data: unknown; error: string | null; status: number }; + +beforeEach(() => { + vi.clearAllMocks(); + userFind.mockResolvedValue(null); + userUpdate.mockResolvedValue({}); + storeMock.mockResolvedValue(undefined); + clearMock.mockResolvedValue(undefined); +}); + +describe("GET /api/oura/credentials", () => { + it("reports hasCredentials true only when both columns are set", async () => { + userFind.mockResolvedValue({ + ouraClientIdEncrypted: "enc:id", + ouraClientSecretEncrypted: "enc:secret", + }); + const res = (await (GET as unknown as () => Promise)())!; + expect(res.data).toEqual({ hasCredentials: true }); + }); + + it("reports false when a column is missing", async () => { + userFind.mockResolvedValue({ ouraClientIdEncrypted: "enc:id" }); + const res = (await (GET as unknown as () => Promise)())!; + expect(res.data).toEqual({ hasCredentials: false }); + }); +}); + +describe("PUT /api/oura/credentials", () => { + const put = PUT as unknown as (r: NextRequest) => Promise; + const req = {} as NextRequest; + + it("422s when the body fails the zod schema", async () => { + safeJsonMock.mockResolvedValue({ data: { clientId: "" }, error: null }); + const res = await put(req); + expect(res.status).toBe(422); + expect(storeMock).not.toHaveBeenCalled(); + }); + + it("encrypts and stores a valid client id/secret", async () => { + safeJsonMock.mockResolvedValue({ + data: { clientId: "cid", clientSecret: "csecret" }, + error: null, + }); + const res = await put(req); + expect(res.status).toBe(200); + expect(storeMock).toHaveBeenCalledWith("u1", "cid", "csecret"); + }); + + it("propagates the safeJson error envelope", async () => { + const jsonError = { data: null, error: "too big", status: 413 }; + safeJsonMock.mockResolvedValue({ data: null, error: jsonError }); + const res = await put(req); + expect(res).toBe(jsonError); + expect(storeMock).not.toHaveBeenCalled(); + }); +}); + +describe("DELETE /api/oura/credentials", () => { + const del = DELETE as unknown as () => Promise; + + it("audits + marks disconnected when a live token was present", async () => { + userFind.mockResolvedValue({ ouraAccessTokenEncrypted: "enc:tok" }); + const res = await del(); + expect(res.data).toEqual({ deleted: true }); + expect(clearMock).toHaveBeenCalledWith("u1"); + expect(userUpdate).toHaveBeenCalled(); + expect(auditLog).toHaveBeenCalledWith("oura.credentials.delete", { + userId: "u1", + }); + expect(markDisconnected).toHaveBeenCalledWith("u1", "oura"); + }); + + it("skips audit + ledger update when nothing was connected", async () => { + userFind.mockResolvedValue({ ouraAccessTokenEncrypted: null }); + const res = await del(); + expect(res.data).toEqual({ deleted: true }); + expect(clearMock).toHaveBeenCalledWith("u1"); + expect(auditLog).not.toHaveBeenCalled(); + expect(markDisconnected).not.toHaveBeenCalled(); + }); +}); diff --git a/src/app/api/oura/credentials/route.ts b/src/app/api/oura/credentials/route.ts index 6a3bad80c..40064ccaf 100644 --- a/src/app/api/oura/credentials/route.ts +++ b/src/app/api/oura/credentials/route.ts @@ -2,6 +2,8 @@ import { prisma } from "@/lib/db"; import { apiHandler, requireAuth } from "@/lib/api-handler"; import { apiSuccess, apiError, safeJson } from "@/lib/api-response"; import { annotate } from "@/lib/logging/context"; +import { auditLog } from "@/lib/auth/audit"; +import { markDisconnected } from "@/lib/integrations/status"; import { storeOuraClientCredentials, clearOuraClientCredentials, @@ -63,11 +65,22 @@ export const PUT = apiHandler(async (request: NextRequest) => { /** * Delete Oura credentials and the active connection. + * + * Deleting the BYO credentials drops a live connection (a token minted against + * the deleted app is now orphaned), so when an access token was present this + * mirrors `/api/oura/disconnect`: audit the event and park the integration + * ledger at `disconnected` rather than leaving it stale at its last state. */ export const DELETE = apiHandler(async () => { const { user } = await requireAuth(); annotate({ action: { name: "oura.credentials.delete" } }); + const dbUser = await prisma.user.findUnique({ + where: { id: user.id }, + select: { ouraAccessTokenEncrypted: true }, + }); + const wasConnected = !!dbUser?.ouraAccessTokenEncrypted; + await prisma.user.update({ where: { id: user.id }, data: { @@ -77,5 +90,10 @@ export const DELETE = apiHandler(async () => { }); await clearOuraClientCredentials(user.id); + if (wasConnected) { + await auditLog("oura.credentials.delete", { userId: user.id }); + await markDisconnected(user.id, "oura"); + } + return apiSuccess({ deleted: true }); }); diff --git a/src/lib/oura/response-classifier.ts b/src/lib/oura/response-classifier.ts index 0477cee1d..fdea5f945 100644 --- a/src/lib/oura/response-classifier.ts +++ b/src/lib/oura/response-classifier.ts @@ -19,20 +19,16 @@ import { type IntegrationClassification, } from "@/lib/integrations/http-status-classifier"; -export type OuraClassification = IntegrationClassification; - -export type ClassifiedOuraResponse = ClassifiedHttpResponse; - export function classifyOuraResponse( httpStatus: number, -): ClassifiedOuraResponse { +): ClassifiedHttpResponse { return classifyHttpStatus(httpStatus); } export class OuraApiError extends IntegrationApiError { constructor(opts: { verb: string; - classification: OuraClassification; + classification: IntegrationClassification; httpStatus: number | undefined; reason: string; upstreamError?: string; @@ -46,13 +42,13 @@ export class OuraApiError extends IntegrationApiError { * True when a caught error is an OAuth `invalid_grant` on the token endpoint — * the canonical revoked-refresh-token signal. Lifts a 400 carrying * `invalid_grant` from `persistent` to `reauth_required` so the connection - * surfaces the reconnect prompt. + * surfaces the reconnect prompt. Internal to `classifyOuraError`. */ -export function isInvalidGrant(err: unknown): boolean { +function isInvalidGrant(err: unknown): boolean { return isOAuthInvalidGrant(err, "Oura"); } -export function classifyOuraError(err: unknown): OuraClassification { +export function classifyOuraError(err: unknown): IntegrationClassification { if (isInvalidGrant(err)) return "reauth_required"; return classifyIntegrationError(err, "Oura"); }