Refuse an unbound or replayed state with a page back to the app, and keep use records through the skew

Browsers (Accept: text/html) refused for state_not_bound, state_already_launched
or state_already_used get the handoff page linking to return_to, re-checked
against the app's allowed origins; other callers keep the JSON code. A callback
with no binding denies the attempt its signed state names (pending only).
The memory store prunes a use record at exp + 30 s, as the state itself is
accepted that long; the schema comment says the same.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>

src/app.ts

@@ -1,7 +1,7 @@
 import { createHmac, randomUUID } from "node:crypto";
 import cookie from "@fastify/cookie";
 import cors from "@fastify/cors";
-import Fastify, { type FastifyError, type FastifyInstance, type FastifyReply } from "fastify";
+import Fastify, { type FastifyError, type FastifyInstance, type FastifyReply, type FastifyRequest } from "fastify";
 import { type AppCaller, isAppCaller, resolveAppCaller } from "./app-caller.js";
 import { deliverBackendHandoff, type BackendHandoffPayload } from "./backend-handoff.js";
 import {
@@ -1094,6 +1094,38 @@ export async function buildApp(options: BuildAppOptions = {}): Promise<FastifyIn
     }
   );
 
+  /**
+   * A refusal of a verified state. A browser (Accept: text/html) gets the
+   * handoff page linking back to the state's return_to, so the person is not
+   * left on a bare JSON body; the origin is re-checked against the app's
+   * allowlist here rather than trusting the signed value. Anything else gets
+   * the fixed JSON code.
+   */
+  const refuseVerifiedState = (
+    request: FastifyRequest,
+    reply: FastifyReply,
+    state: OAuthStatePayload,
+    error: "state_not_bound" | "state_already_launched" | "state_already_used"
+  ) => {
+    reply.code(400);
+    const profile = getAppProfile(state.app_slug);
+    if (prefersHtml(request.headers.accept) && profile && isAllowedReturnOrigin(profile, state.return_to)) {
+      reply.type("text/html; charset=utf-8");
+      return reply.send(
+        renderBrowserHandoffPage({
+          handoffKind: (state.handoff ?? { kind: "browser_completion" }).kind,
+          status: "error",
+          provider: state.provider,
+          appSlug: state.app_slug,
+          mode: state.mode,
+          returnTo: state.return_to,
+          error,
+        })
+      );
+    }
+    return { error };
+  };
+
   app.get<{
     Params: { provider: Provider };
     Querystring: { state: string };
@@ -1140,8 +1172,7 @@ export async function buildApp(options: BuildAppOptions = {}): Promise<FastifyIn
         expiresAt: new Date(state.exp * 1000).toISOString(),
       });
       if (!first) {
-        reply.code(400);
-        return { error: "state_already_launched" };
+        return refuseVerifiedState(request, reply, state, "state_already_launched");
       }
 
       const oidc = providerCatalog[state.provider].oidc ? deriveOidcSecrets(state.jti, keys.privateKey) : undefined;
@@ -1201,11 +1232,23 @@ export async function buildApp(options: BuildAppOptions = {}): Promise<FastifyIn
         return { error: verified.error };
       }
       const statePayload = verified.payload;
+      const handoff = statePayload.handoff ?? { kind: "browser_completion" };
+      const denyBrowserAttempt = async (denialCode: string): Promise<void> => {
+        if (handoff.kind !== "browser_completion" || !handoff.attemptId) return;
+        await store.updateAuthAttempt(statePayload.app_slug, handoff.attemptId, {
+          status: "denied",
+          at: new Date().toISOString(),
+          denialCode,
+        });
+      };
       // The cookie is checked before the claim: a stranger without it must not be
       // able to burn the state that the browser that launched it still needs.
+      // The attempt is denied, though: a launcher whose cookie lapsed can never
+      // complete this state, and its page polls an attempt that would otherwise
+      // sit pending until it expires.
       if (!oauthBindingHolds(config, keys, request.cookies[oauthBindingCookieName(statePayload.jti)], statePayload)) {
-        reply.code(400);
-        return { error: "state_not_bound" };
+        await denyBrowserAttempt("state_not_bound");
+        return refuseVerifiedState(request, reply, statePayload, "state_not_bound");
       }
       setOAuthBindingCookie(reply, statePayload, "", 0);
       const firstCallback = await store.claimOAuthStateUse({
@@ -1216,18 +1259,8 @@ export async function buildApp(options: BuildAppOptions = {}): Promise<FastifyIn
         expiresAt: new Date(statePayload.exp * 1000).toISOString(),
       });
       if (!firstCallback) {
-        reply.code(400);
-        return { error: "state_already_used" };
+        return refuseVerifiedState(request, reply, statePayload, "state_already_used");
       }
-      const handoff = statePayload.handoff ?? { kind: "browser_completion" };
-      const denyBrowserAttempt = async (denialCode: string): Promise<void> => {
-        if (handoff.kind !== "browser_completion" || !handoff.attemptId) return;
-        await store.updateAuthAttempt(statePayload.app_slug, handoff.attemptId, {
-          status: "denied",
-          at: new Date().toISOString(),
-          denialCode,
-        });
-      };
 
       if (request.query.error) {
         const providerError = relayedProviderText(request.query.error, 64);

src/store/in-memory.ts

@@ -24,6 +24,9 @@ import {
   type UpsertLinkedIdentityInput,
 } from "./types.js";
 
+/** The skew a signed OAuth state is still accepted for past its exp (verifyOAuthState). */
+const OAUTH_STATE_SKEW_MS = 30_000;
+
 function clone<T>(value: T): T {
   return JSON.parse(JSON.stringify(value)) as T;
 }
@@ -452,8 +455,10 @@ export class InMemoryStore implements HeimdallStore {
   }
 
   async claimOAuthStateUse(input: ClaimOAuthStateUseInput): Promise<boolean> {
+    // A state is still accepted until its exp + 30 s skew, so a use record must outlive that.
+    const usedAtMs = Date.parse(input.usedAt);
     for (const [key, expiresAt] of this.oauthStateUses) {
-      if (expiresAt <= input.usedAt) {
+      if (Date.parse(expiresAt) + OAUTH_STATE_SKEW_MS <= usedAtMs) {
         this.oauthStateUses.delete(key);
       }
     }

src/store/schema.ts

@@ -138,9 +138,9 @@ CREATE UNIQUE INDEX IF NOT EXISTS auth_completions_attempt_unconsumed_unique_idx
 
 -- One use per (state jti, phase): a signed OAuth state launches once and
 -- completes once. The first INSERT for a pair wins; ON CONFLICT DO NOTHING
--- reports every later claim as no row. The store deletes nothing, so rows stay
--- like consumed auth_completions; after expires_at a state is refused before
--- any claim, so a row is never consulted again.
+-- reports every later claim as no row. The Postgres store deletes nothing, so rows
+-- stay like consumed auth_completions; a state is accepted until expires_at + 30 s
+-- of skew, so a row may be pruned only after that and is never consulted again.
 CREATE TABLE IF NOT EXISTS oauth_state_uses (
   jti TEXT NOT NULL,
   phase TEXT NOT NULL,

tests/oauth-state-binding.test.ts

@@ -176,13 +176,129 @@ describe("OAuth state binding", () => {
       expect(stranger.statusCode, query).toBe(400);
       expect(stranger.json(), query).toEqual({ error: "state_not_bound" });
     }
-    expect((await ctx.store.findAuthAttempt("repixelizer", attempt.handle))?.status).toBe("pending");
 
-    // The victim's own callback is unharmed.
+    // The state is still unclaimed, so the launcher's cookie still completes it...
     const response = await callback(app, victim.state, victim.cookie, "code=victim-code");
     expect(response.statusCode).toBe(201);
   });
 
+  it("a callback with no binding denies the attempt its state names, once, and never a completed one", async () => {
+    const app = await newApp();
+    const ctx = getHeimdallRuntimeContext(app);
+    const make = async (handle: string) => {
+      await ctx.store.createAuthAttempt({
+        handle,
+        appSlug: "repixelizer",
+        provider: "discord",
+        mode: "sign_in",
+        returnTo: "https://repixelizer.gamecult.org/app/",
+        createdAt: new Date().toISOString(),
+        expiresAt: new Date(Date.now() + 600_000).toISOString(),
+      });
+      const caller = resolveAppCaller(ctx.config, "repixelizer", { "x-heimdall-app-secret": "repixelizer-secret" });
+      const started = await startOAuthFlow(
+        ctx,
+        "discord",
+        caller,
+        { appSlug: "repixelizer", mode: "sign_in", returnTo: "https://repixelizer.gamecult.org/app/", handoff: { kind: "browser_completion" } },
+        { trustedBrowserAttemptId: handle }
+      );
+      return started.body as unknown as { authorizationUrl: string };
+    };
+
+    // Lapsed cookie: the launcher launched, then its cookie is gone at the callback.
+    const lapsed = await make("lapsed");
+    await launchFlow(app, lapsed);
+    const refused = await callback(app, stateOf(lapsed), undefined, "code=c");
+    expect(refused.json()).toEqual({ error: "state_not_bound" });
+    const denied = await ctx.store.findAuthAttempt("repixelizer", "lapsed");
+    expect(denied?.status).toBe("denied");
+    expect(denied?.denialCode).toBe("state_not_bound");
+
+    // A completed attempt stays completed under a later cookieless callback.
+    const done = await make("done");
+    const launched = await launchFlow(app, done);
+    expect((await callback(app, launched.state, launched.cookie, "code=c")).statusCode).toBe(201);
+    await callback(app, launched.state, undefined, "code=c");
+    expect(["completed", "consumed"]).toContain((await ctx.store.findAuthAttempt("repixelizer", "done"))?.status);
+  });
+
+  it("a state with no jti is refused as invalid and names no attempt to deny", async () => {
+    const app = await newApp();
+    const ctx = getHeimdallRuntimeContext(app);
+    const original = decodeJwt(stateOf(await bifrostStart(app))).payload;
+    const { jti: _jti, ...rest } = original;
+    const token = signJwt(rest, ctx.keys);
+    const response = await callback(app, token, undefined, "code=c");
+    expect(response.statusCode).toBe(400);
+    expect(response.json()).toEqual({ error: "invalid_state" });
+  });
+
+  describe("a browser refused for its binding gets a page back to the app, not bare JSON", () => {
+    const html = { accept: "text/html" };
+    const get = (app: FastifyInstance, url: string, cookie?: string) =>
+      app.inject({ method: "GET", url, headers: { ...html, ...(cookie ? { cookie } : {}) } });
+
+    it("on a cookieless callback, a lapsed cookie and a reloaded launch", async () => {
+      const app = await newApp();
+      const start = await bifrostStart(app);
+      const state = stateOf(start);
+
+      const reloadBeforeLaunch = await get(app, `/v1/oauth/discord/callback?code=c&state=${encodeURIComponent(state)}`);
+      expect(reloadBeforeLaunch.statusCode).toBe(400);
+      expect(reloadBeforeLaunch.headers["content-type"]).toContain("text/html");
+      expect(reloadBeforeLaunch.body).toContain("state_not_bound");
+      expect(reloadBeforeLaunch.body).toContain("https://bifrost.gamecult.org/auth/heimdall/complete");
+
+      const victim = await launchFlow(app, start);
+      const reloaded = await get(app, launchUrl(start));
+      expect(reloaded.statusCode).toBe(400);
+      expect(reloaded.headers["content-type"]).toContain("text/html");
+      expect(reloaded.body).toContain("state_already_launched");
+      expect(setCookieOf(reloaded)).toEqual([]);
+
+      // Handoff reload: the page after a completed callback, loaded again.
+      expect((await get(app, `/v1/oauth/discord/callback?code=c&state=${encodeURIComponent(victim.state)}`, victim.cookie)).statusCode).toBeLessThan(300);
+      const reload = await get(app, `/v1/oauth/discord/callback?code=c&state=${encodeURIComponent(victim.state)}`, victim.cookie);
+      expect(reload.statusCode).toBe(400);
+      expect(reload.headers["content-type"]).toContain("text/html");
+      expect(reload.body).toContain("state_already_used");
+    });
+
+    it("never links to an origin the app does not own, and JSON stays JSON without Accept", async () => {
+      const app = await newApp();
+      const ctx = getHeimdallRuntimeContext(app);
+      const original = decodeJwt(stateOf(await bifrostStart(app))).payload;
+      const token = signJwt({ ...original, return_to: "https://evil.example/steal" }, ctx.keys);
+      const evil = await get(app, `/v1/oauth/discord/callback?code=c&state=${encodeURIComponent(token)}`);
+      expect(evil.statusCode).toBe(400);
+      expect(evil.json()).toEqual({ error: "state_not_bound" });
+      expect(evil.body).not.toContain("evil.example");
+
+      const plain = await callback(app, stateOf(await bifrostStart(app)));
+      expect(plain.json()).toEqual({ error: "state_not_bound" });
+    });
+  });
+
+  it("the memory store keeps a use record until the state's exp + 30 s skew", async () => {
+    const app = await newApp();
+    const ctx = getHeimdallRuntimeContext(app);
+    const claim = (jti: string, usedAt: number) =>
+      ctx.store.claimOAuthStateUse({
+        jti,
+        phase: "launch",
+        appSlug: "bifrost",
+        usedAt: new Date(usedAt).toISOString(),
+        expiresAt: new Date(1_000_000).toISOString(),
+      });
+    expect(await claim("a", 900_000)).toBe(true);
+    // Past exp but inside the skew a state still verifies, so its record must still refuse a replay.
+    expect(await claim("b", 1_000_000 + 29_999)).toBe(true);
+    expect(await claim("a", 1_000_000 + 29_999)).toBe(false);
+    expect(await claim("c", 1_000_000 + 30_000)).toBe(true);
+    expect(await claim("a", 1_000_000 + 30_000)).toBe(true);
+  });
+
   it("a provider error from the launching browser still denies the attempt, once", async () => {
     const app = await newApp();
     const start = await bifrostStart(app);

tests/store.test.ts

@@ -216,14 +216,17 @@ describe("InMemoryStore.claimOAuthStateUse", () => {
     expect(await store.claimOAuthStateUse({ ...use, jti: "state-2" })).toBe(true);
   });
 
-  it("drops a record once its state has expired, and keeps it until then", async () => {
+  it("drops a record once its state's exp + 30 s skew has passed, and keeps it until then", async () => {
     const store = new InMemoryStore();
     await store.claimOAuthStateUse(use);
     // Before expiry a later claim of another state leaves the record in force.
     await store.claimOAuthStateUse({ ...use, jti: "state-2", usedAt: "2026-04-26T12:09:59.000Z" });
     expect(await store.claimOAuthStateUse(use)).toBe(false);
-    // At and after expiry the record is dead weight: the state is refused before any claim.
-    await store.claimOAuthStateUse({ ...use, jti: "state-3", usedAt: "2026-04-26T12:10:00.000Z" });
+    // Inside the 30 s skew past expiry the state still verifies, so the record still refuses a replay.
+    await store.claimOAuthStateUse({ ...use, jti: "state-3", usedAt: "2026-04-26T12:10:29.999Z" });
+    expect(await store.claimOAuthStateUse(use)).toBe(false);
+    // At exp + 30 s the state is refused before any claim and the record is dead weight.
+    await store.claimOAuthStateUse({ ...use, jti: "state-4", usedAt: "2026-04-26T12:10:30.000Z" });
     expect(await store.claimOAuthStateUse(use)).toBe(true);
   });
 });