Merge branch 'fix/v1439-oura' into release/v1.43.9

prisma/schema.prisma

@@ -4560,8 +4560,8 @@ model GoogleHealthOAuthState {
 /// sends it to the same place instead of to an error page. Holds no user id,
 /// token or code: `location` is the app's own result URL (`?oura=connected`,
 /// `?whoop=error&reason=…`). Rows outlive the state they guard by a few
-/// minutes and are swept by expiry on the next settle
-/// (`src/lib/oauth/callback-claim.ts`).
+/// minutes and are swept by expiry when a callback completes and by the daily
+/// OAuth-state cleanup (`src/lib/jobs/oauth-callback-claim-cleanup.ts`).
 model OAuthCallbackClaim {
   /// sha256(provider + NUL + state), hex.
   key       String    @id

src/app/api/oura/callback/__tests__/route.test.ts

@@ -3,13 +3,17 @@ import { describe, it, expect, vi, beforeEach } from "vitest";
 // Single-use claiming is covered against Postgres in
 // tests/integration/oauth-callback-single-use.test.ts; here every callback
 // is the first and only one.
+const { claimSpy } = vi.hoisted(() => ({ claimSpy: vi.fn() }));
 vi.mock("@/lib/oauth/callback-claim", () => ({
   runOAuthCallbackOnce: (
     _p: string,
     _s: string,
     _o: unknown,
     run: () => Promise<unknown>,
-  ) => run(),
+  ) => {
+    claimSpy();
+    return run();
+  },
   settleOAuthCallback: (_p: string, _s: string, run: () => Promise<unknown>) =>
     run(),
   consumeLedgerState: async (
@@ -227,3 +231,36 @@ describe("GET /api/oura/callback — connect-time first sync", () => {
     expect(res.headers.get("location")).toContain("oura=connected");
   });
 });
+
+// A refused callback must not spend the state: Oura sends a denied consent
+// back without a code, and the person may return to the same authorize URL
+// and allow it.
+describe("GET /api/oura/callback — claim only what can be exchanged", () => {
+  it("does not claim the state for a callback without a code", async () => {
+    const res = await GET(makeReq({ state: "S", cookie: "S" }));
+    expect(res.headers.get("location")).toContain("reason=nocode");
+    expect(claimSpy).not.toHaveBeenCalled();
+  });
+
+  it("does not claim the state for a foreign issuer or a cross-user session", async () => {
+    await GET(
+      makeReq({
+        code: "c",
+        state: "S",
+        cookie: "S",
+        iss: "https://evil.example/oauth",
+      }),
+    );
+    (getSession as ReturnType<typeof vi.fn>).mockResolvedValue({
+      user: { id: "attacker" },
+    });
+    await GET(makeReq({ code: "c", state: "S", cookie: "S" }));
+    expect(claimSpy).not.toHaveBeenCalled();
+  });
+
+  it("claims the state once before exchanging a valid code", async () => {
+    await GET(makeReq({ code: "c", state: "S", cookie: "S" }));
+    expect(claimSpy).toHaveBeenCalledTimes(1);
+    expect(exchangeMock).toHaveBeenCalledTimes(1);
+  });
+});

src/app/api/oura/callback/route.ts

@@ -70,23 +70,12 @@ export const GET = apiHandler(async (request: NextRequest) => {
   }
   const userId = verified.userId;
 
-  return runOAuthCallbackOnce(
-    "oura",
-    state,
-    {
-      fallbackUrl: SETTINGS_URL(),
-      stateCookie: oauthStateCookieName("oura"),
-    },
-    () => completeCallback(request, userId),
-  );
-});
-
-async function completeCallback(
-  request: NextRequest,
-  userId: string,
-): Promise<NextResponse> {
-  const { searchParams } = new URL(request.url);
-  const code = searchParams.get("code");
+  // Everything that can refuse the callback without the code runs before
+  // the claim, so a refused callback (a denied consent arrives without a
+  // code) never spends the state: the same authorize URL still works. The
+  // claim stays strictly before the exchange.
+  const params = new URL(request.url).searchParams;
+  const code = params.get("code");
 
   const evt = getEvent();
   if (evt) evt.setAuth({ user_id: userId, auth_method: "session" });
@@ -102,12 +91,28 @@ async function completeCallback(
   // RFC 9207: an authorization server that names itself must be Oura's. The
   // issuer also tells which token endpoint the code belongs to. Older apps
   // send none, which is fine.
-  const iss = searchParams.get("iss");
+  const iss = params.get("iss");
   if (!isAcceptedOuraIssuer(iss)) {
     annotate({ meta: { reason: "issuer" } });
     return ERR("issuer");
   }
 
+  return runOAuthCallbackOnce(
+    "oura",
+    state,
+    {
+      fallbackUrl: SETTINGS_URL(),
+      stateCookie: oauthStateCookieName("oura"),
+    },
+    () => completeCallback(userId, code, iss),
+  );
+});
+
+async function completeCallback(
+  userId: string,
+  code: string,
+  iss: string | null,
+): Promise<NextResponse> {
   try {
     const creds = await getOuraClientCredentials(userId);
     if (!creds) return ERR("nocreds");

src/app/api/polar/callback/__tests__/route.test.ts

@@ -25,13 +25,17 @@ const {
 // Single-use claiming is covered against Postgres in
 // tests/integration/oauth-callback-single-use.test.ts; here every callback
 // is the first and only one.
+const { claimSpy } = vi.hoisted(() => ({ claimSpy: vi.fn() }));
 vi.mock("@/lib/oauth/callback-claim", () => ({
   runOAuthCallbackOnce: (
     _p: string,
     _s: string,
     _o: unknown,
     run: () => Promise<unknown>,
-  ) => run(),
+  ) => {
+    claimSpy();
+    return run();
+  },
   settleOAuthCallback: (_p: string, _s: string, run: () => Promise<unknown>) =>
     run(),
   consumeLedgerState: async (
@@ -78,9 +82,9 @@ import { GET } from "../route";
 
 process.env.NEXT_PUBLIC_APP_URL = "https://app.example";
 
-function makeReq(): Parameters<typeof GET>[0] {
+function makeReq(query = "code=abc&state=s"): Parameters<typeof GET>[0] {
   return {
-    url: "https://app.example/api/polar/callback?code=abc&state=s",
+    url: `https://app.example/api/polar/callback?${query}`,
     cookies: { get: () => ({ value: "s" }) },
   } as unknown as Parameters<typeof GET>[0];
 }
@@ -163,3 +167,14 @@ describe("GET /api/polar/callback — connect-time first sync", () => {
     expect(res.headers.get("location")).toContain("polar=connected");
   });
 });
+
+describe("GET /api/polar/callback — claim only what can be exchanged", () => {
+  it("does not claim the state for a callback without a code or for another user", async () => {
+    const denied = await run(makeReq("state=s&error=access_denied"));
+    expect(denied.headers.get("location")).toContain("reason=nocode");
+    getSessionMock.mockResolvedValue({ user: { id: "other" } });
+    await run(makeReq());
+    expect(claimSpy).not.toHaveBeenCalled();
+    expect(exchangeCodeMock).not.toHaveBeenCalled();
+  });
+});

src/app/api/polar/callback/route.ts

@@ -60,26 +60,10 @@ export const GET = apiHandler(async (request: NextRequest) => {
   }
   const userId = verified.userId;
 
-  // Single-use: a callback delivered twice exchanges its code once, and the
-  // duplicate follows the first request's result (src/lib/oauth/callback-claim.ts).
-  return runOAuthCallbackOnce(
-    "polar",
-    state,
-    {
-      fallbackUrl: new URL(
-        "/settings/integrations",
-        process.env.NEXT_PUBLIC_APP_URL!,
-      ).toString(),
-      stateCookie: oauthStateCookieName("polar"),
-    },
-    () => completeCallback(request, userId),
-  );
-});
-
-async function completeCallback(
-  request: NextRequest,
-  userId: string,
-): Promise<NextResponse> {
+  // Everything that can refuse the callback without the code runs before
+  // the claim, so a refused callback (a denied consent arrives without a
+  // code) never spends the state: the same authorize URL still works. The
+  // claim stays strictly before the exchange.
   const code = new URL(request.url).searchParams.get("code");
 
   const evt = getEvent();
@@ -94,6 +78,26 @@ async function completeCallback(
 
   if (!code) return ERR("nocode");
 
+  // Single-use: a callback delivered twice exchanges its code once, and the
+  // duplicate follows the first request's result (src/lib/oauth/callback-claim.ts).
+  return runOAuthCallbackOnce(
+    "polar",
+    state,
+    {
+      fallbackUrl: new URL(
+        "/settings/integrations",
+        process.env.NEXT_PUBLIC_APP_URL!,
+      ).toString(),
+      stateCookie: oauthStateCookieName("polar"),
+    },
+    () => completeCallback(userId, code),
+  );
+});
+
+async function completeCallback(
+  userId: string,
+  code: string,
+): Promise<NextResponse> {
   try {
     const creds = await getPolarClientCredentials(userId);
     if (!creds) return ERR("nocreds");

src/app/api/strava/callback/__tests__/route.test.ts

@@ -28,13 +28,17 @@ const {
 // Single-use claiming is covered against Postgres in
 // tests/integration/oauth-callback-single-use.test.ts; here every callback
 // is the first and only one.
+const { claimSpy } = vi.hoisted(() => ({ claimSpy: vi.fn() }));
 vi.mock("@/lib/oauth/callback-claim", () => ({
   runOAuthCallbackOnce: (
     _p: string,
     _s: string,
     _o: unknown,
     run: () => Promise<unknown>,
-  ) => run(),
+  ) => {
+    claimSpy();
+    return run();
+  },
   settleOAuthCallback: (_p: string, _s: string, run: () => Promise<unknown>) =>
     run(),
   consumeLedgerState: async (
@@ -85,11 +89,12 @@ import { GET } from "../route";
 
 process.env.NEXT_PUBLIC_APP_URL = "https://app.example";
 
-function callback(): Promise<{ headers: { get(k: string): string | null } }> {
-  const req = new NextRequest(
-    "http://localhost/api/strava/callback?code=abc&state=signed-state",
-    { headers: { cookie: "strava_oauth_state=signed-state" } },
-  );
+function callback(
+  query = "code=abc&state=signed-state",
+): Promise<{ headers: { get(k: string): string | null } }> {
+  const req = new NextRequest(`http://localhost/api/strava/callback?${query}`, {
+    headers: { cookie: "strava_oauth_state=signed-state" },
+  });
   return (GET as unknown as (r: NextRequest) => Promise<never>)(req);
 }
 
@@ -127,3 +132,12 @@ describe("GET /api/strava/callback — S-1 backfill enqueue", () => {
     expect(bossSend).not.toHaveBeenCalled();
   });
 });
+
+describe("GET /api/strava/callback — claim only what can be exchanged", () => {
+  it("does not claim the state for a callback without a code", async () => {
+    const res = await callback("state=signed-state&error=access_denied");
+    expect(res.headers.get("location")).toContain("reason=nocode");
+    expect(claimSpy).not.toHaveBeenCalled();
+    expect(exchangeCodeMock).not.toHaveBeenCalled();
+  });
+});

src/app/api/strava/callback/route.ts

@@ -60,26 +60,10 @@ export const GET = apiHandler(async (request: NextRequest) => {
   }
   const userId = verified.userId;
 
-  // Single-use: a callback delivered twice exchanges its code once, and the
-  // duplicate follows the first request's result (src/lib/oauth/callback-claim.ts).
-  return runOAuthCallbackOnce(
-    "strava",
-    state,
-    {
-      fallbackUrl: new URL(
-        "/settings/integrations",
-        process.env.NEXT_PUBLIC_APP_URL!,
-      ).toString(),
-      stateCookie: oauthStateCookieName("strava"),
-    },
-    () => completeCallback(request, userId),
-  );
-});
-
-async function completeCallback(
-  request: NextRequest,
-  userId: string,
-): Promise<NextResponse> {
+  // Everything that can refuse the callback without the code runs before
+  // the claim, so a refused callback (a denied consent arrives without a
+  // code) never spends the state: the same authorize URL still works. The
+  // claim stays strictly before the exchange.
   const code = new URL(request.url).searchParams.get("code");
 
   const evt = getEvent();
@@ -93,6 +77,26 @@ async function completeCallback(
 
   if (!code) return ERR("nocode");
 
+  // Single-use: a callback delivered twice exchanges its code once, and the
+  // duplicate follows the first request's result (src/lib/oauth/callback-claim.ts).
+  return runOAuthCallbackOnce(
+    "strava",
+    state,
+    {
+      fallbackUrl: new URL(
+        "/settings/integrations",
+        process.env.NEXT_PUBLIC_APP_URL!,
+      ).toString(),
+      stateCookie: oauthStateCookieName("strava"),
+    },
+    () => completeCallback(userId, code),
+  );
+});
+
+async function completeCallback(
+  userId: string,
+  code: string,
+): Promise<NextResponse> {
   try {
     const creds = await getStravaClientCredentials(userId);
     if (!creds) return ERR("nocreds");

src/lib/jobs/__tests__/oauth-callback-claim-cleanup.test.ts

@@ -0,0 +1,77 @@
+/**
+ * Expired OAuth callback claims are swept by the daily OAuth-state cleanup,
+ * not only when a callback completes: a quiet instance may not see another
+ * connect for weeks.
+ */
+import { beforeEach, describe, expect, it, vi } from "vitest";
+
+const h = vi.hoisted(() => ({
+  withingsCleanup: vi.fn(),
+  claimDeleteMany: vi.fn(),
+  meta: {} as Record<string, unknown>,
+}));
+
+vi.mock("@/lib/jobs/withings-oauth-state-cleanup", () => ({
+  cleanupExpiredWithingsOAuthStates: h.withingsCleanup,
+}));
+vi.mock("../reminder/shared", async (importOriginal) => ({
+  ...(await importOriginal<Record<string, unknown>>()),
+  getWorkerPrisma: () => ({
+    oAuthCallbackClaim: { deleteMany: h.claimDeleteMany },
+  }),
+}));
+vi.mock("@/lib/logging/background", () => ({
+  withBackgroundEvent: async (
+    _name: string,
+    fn: (evt: {
+      addMeta: (k: string, v: unknown) => void;
+      addWarning: (w: string) => void;
+    }) => Promise<unknown>,
+  ) =>
+    fn({
+      addMeta: (k, v) => {
+        h.meta[k] = v;
+      },
+      addWarning: () => {},
+    }),
+}));
+
+import { cleanupExpiredOAuthCallbackClaims } from "../oauth-callback-claim-cleanup";
+import { handleWithingsOAuthStateCleanup } from "../reminder/cleanup-handlers";
+
+beforeEach(() => {
+  vi.clearAllMocks();
+  for (const key of Object.keys(h.meta)) delete h.meta[key];
+  h.withingsCleanup.mockResolvedValue(0);
+  h.claimDeleteMany.mockResolvedValue({ count: 3 });
+});
+
+describe("cleanupExpiredOAuthCallbackClaims", () => {
+  it("deletes the claims whose expiry has passed and returns the count", async () => {
+    const now = new Date("2026-10-21T03:20:00Z");
+    const deleted = await cleanupExpiredOAuthCallbackClaims(
+      { oAuthCallbackClaim: { deleteMany: h.claimDeleteMany } } as never,
+      now,
+    );
+    expect(deleted).toBe(3);
+    expect(h.claimDeleteMany).toHaveBeenCalledWith({
+      where: { expiresAt: { lt: now } },
+    });
+  });
+});
+
+describe("daily OAuth-state cleanup", () => {
+  it("sweeps expired callback claims alongside the Withings states", async () => {
+    await handleWithingsOAuthStateCleanup([]);
+    expect(h.withingsCleanup).toHaveBeenCalledTimes(1);
+    expect(h.claimDeleteMany).toHaveBeenCalledTimes(1);
+    expect(h.meta.oauth_callback_claim_cleanup_deleted).toBe(3);
+  });
+
+  it("fails the job when the claim sweep fails", async () => {
+    h.claimDeleteMany.mockRejectedValue(new Error("statement timeout"));
+    await expect(handleWithingsOAuthStateCleanup([])).resolves.toMatchObject({
+      ok: false,
+    });
+  });
+});

src/lib/jobs/job-outcome.ts

@@ -63,6 +63,8 @@ export const JOB_FACT_ALLOWLIST: ReadonlySet<string> = new Set([
   "buckets_recomputed",
   "budget_blocked",
   "cached",
+  // The daily Withings OAuth-state cleanup also sweeps the callback claims.
+  "callback_claims_deleted",
   "candidates_scanned",
   // The nightly sign-in ledger cleanup (`auth-ledger-cleanup`).
   "challenges_deleted",

src/lib/jobs/oauth-callback-claim-cleanup.ts

@@ -0,0 +1,21 @@
+/**
+ * Daily sweep for `oauth_callback_claims`.
+ *
+ * A claim is taken when a connect callback is about to exchange its code
+ * (`src/lib/oauth/callback-claim.ts`) and is useless once its state has
+ * expired. The callback itself sweeps expired rows whenever one completes, so
+ * on a quiet instance the last few claims would stay until the next connect.
+ * This pass rides the daily Withings OAuth-state cleanup (03:20 Europe/Berlin)
+ * so the table empties on its own regardless. Idempotent.
+ */
+import type { PrismaClient } from "@/generated/prisma/client";
+
+export async function cleanupExpiredOAuthCallbackClaims(
+  prisma: Pick<PrismaClient, "oAuthCallbackClaim">,
+  now: Date = new Date(),
+): Promise<number> {
+  const { count } = await prisma.oAuthCallbackClaim.deleteMany({
+    where: { expiresAt: { lt: now } },
+  });
+  return count;
+}

src/lib/jobs/reminder/cleanup-handlers.ts

@@ -1,5 +1,5 @@
 /**
- * Retention and maintenance cleanups: rate-limit rows, idempotency keys, audit logs, OAuth states (Withings / WHOOP), mood-reminder events, push attempts, and measurement tombstones.
+ * Retention and maintenance cleanups: rate-limit rows, idempotency keys, audit logs, OAuth states (Withings / WHOOP) and OAuth callback claims, mood-reminder events, push attempts, and measurement tombstones.
  *
  * Extracted from reminder-worker.ts, which owns the queue names, cron
  * schedules, and the queue bindings.
@@ -33,6 +33,7 @@ import { cleanupOldAuditLogs } from "@/lib/jobs/audit-log-cleanup";
 import { cleanupOldCoachMessages } from "@/lib/jobs/coach-message-cleanup";
 import { cleanupOldWorkoutInsightClaims } from "@/lib/jobs/workout-insight-claim-cleanup";
 import { cleanupExpiredWithingsOAuthStates } from "@/lib/jobs/withings-oauth-state-cleanup";
+import { cleanupExpiredOAuthCallbackClaims } from "@/lib/jobs/oauth-callback-claim-cleanup";
 import { cleanupExpiredOidcNativeHandoffs } from "@/lib/jobs/oidc-handoff-cleanup";
 import {
   cleanupExpiredMeasurementTombstones,
@@ -462,7 +463,12 @@ export async function handleWithingsOAuthStateCleanup(
       try {
         const deleted = await cleanupExpiredWithingsOAuthStates(p);
         evt.addMeta("withings_oauth_state_cleanup_deleted", deleted);
-        return jobDone({ deleted });
+        // The provider-neutral callback claims ride this daily pass: the
+        // callbacks sweep them only when one completes, which a quiet
+        // instance may not see for weeks.
+        const claimsDeleted = await cleanupExpiredOAuthCallbackClaims(p);
+        evt.addMeta("oauth_callback_claim_cleanup_deleted", claimsDeleted);
+        return jobDone({ deleted, callback_claims_deleted: claimsDeleted });
       } catch (err) {
         // The OAuth flow tolerates a stale row for an extra day, which used
         // to be the argument for swallowing this so the queue would not

src/lib/oauth/callback-claim.ts

@@ -32,6 +32,7 @@ import { NextResponse } from "next/server";
 import { prisma } from "@/lib/db";
 import type { Prisma } from "@/generated/prisma/client";
 import { annotate, getEvent } from "@/lib/logging/context";
+import { cleanupExpiredOAuthCallbackClaims } from "@/lib/jobs/oauth-callback-claim-cleanup";
 
 /** Providers whose callbacks claim their state here. */
 export type OAuthCallbackProvider =
@@ -130,9 +131,7 @@ export async function settleOAuthCallback(
               : null,
         },
       });
-      await prisma.oAuthCallbackClaim.deleteMany({
-        where: { expiresAt: { lt: now } },
-      });
+      await cleanupExpiredOAuthCallbackClaims(prisma, now);
     } catch (err) {
       getEvent()?.addWarning(`oauth callback claim settle failed: ${err}`);
     }

tests/integration/oauth-callback-single-use.test.ts

@@ -87,9 +87,13 @@ afterEach(() => {
   vi.restoreAllMocks();
 });
 
-async function ouraCallback(state: string, code = "oura-code") {
+async function ouraCallback(state: string, code: string | null = "oura-code") {
   const { GET } = await import("@/app/api/oura/callback/route");
-  const params = new URLSearchParams({ code, state, iss: OURA_ISS });
+  const params = new URLSearchParams(
+    code === null
+      ? { error: "access_denied", state, iss: OURA_ISS }
+      : { code, state, iss: OURA_ISS },
+  );
   return GET(
     new NextRequest(`http://localhost/api/oura/callback?${params}`, {
       method: "GET",
@@ -221,6 +225,37 @@ describe("Oura callback delivered twice", () => {
   });
 });
 
+describe("Oura consent denied, then allowed", () => {
+  it("still exchanges the code when the person returns to the same authorize URL", async () => {
+    let tokenCalls = 0;
+    vi.spyOn(globalThis, "fetch").mockImplementation(async (input) => {
+      if (urlOf(input) !== OURA_TOKEN_URL) throw new Error("unexpected");
+      tokenCalls++;
+      return new Response(
+        JSON.stringify({
+          access_token: "oura-access",
+          refresh_token: "oura-refresh",
+          expires_in: 86400,
+        }),
+        { status: 200, headers: { "content-type": "application/json" } },
+      );
+    });
+
+    const state = await mintOuraState();
+    // Deny: Oura returns without a code. Nothing is exchanged or claimed.
+    const denied = await ouraCallback(state, null);
+    expect(denied.headers.get("location")).toContain(
+      "oura=error&reason=nocode",
+    );
+    expect(tokenCalls).toBe(0);
+
+    // Allow on the same authorize URL: same state, now with a code.
+    const allowed = await ouraCallback(state, "allowed-code");
+    expect(tokenCalls).toBe(1);
+    expect(allowed.headers.get("location")).toContain("oura=connected");
+  });
+});
+
 describe("Withings callback delivered twice", () => {
   it("exchanges the code once and sends the duplicate where the first went", async () => {
     const prisma = getPrismaClient();