mirror of
https://github.com/MBombeck/HealthLog.git
synced 2026-11-08 20:42:43 +00:00
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.
This commit is contained in:
1 parent
c5e91c4cb6
commit
a0662d1b20
5 files changed
+263
-10
No files matched your search
@@ -0,0 +1,115 @@
|
||||
import { describe, it, expect, vi, beforeEach } from "vitest";
|
||||
|
||||
vi.mock("@/lib/api-handler", () => ({
|
||||
apiHandler: <T extends (...args: unknown[]) => 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<typeof vi.fn>;
|
||||
|
||||
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<typeof vi.fn>).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<typeof vi.fn>).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");
|
||||
});
|
||||
});
|
||||
@@ -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.
|
||||
*/
|
||||
|
||||
@@ -0,0 +1,122 @@
|
||||
import { describe, it, expect, vi, beforeEach } from "vitest";
|
||||
|
||||
vi.mock("@/lib/api-handler", () => ({
|
||||
apiHandler: <T extends (...args: unknown[]) => 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<typeof vi.fn>;
|
||||
const userUpdate = prisma.user.update as ReturnType<typeof vi.fn>;
|
||||
const safeJsonMock = safeJson as ReturnType<typeof vi.fn>;
|
||||
|
||||
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<RouteResult>)())!;
|
||||
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<RouteResult>)())!;
|
||||
expect(res.data).toEqual({ hasCredentials: false });
|
||||
});
|
||||
});
|
||||
|
||||
describe("PUT /api/oura/credentials", () => {
|
||||
const put = PUT as unknown as (r: NextRequest) => Promise<RouteResult>;
|
||||
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<RouteResult>;
|
||||
|
||||
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();
|
||||
});
|
||||
});
|
||||
@@ -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 });
|
||||
});
|
||||
@@ -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");
|
||||
}
|
||||
Reference in new issue
Block a user