Merge pull request #929 from MBombeck/fix/rotation-sunset

fix(auth): give the rotated access token a fifteen-second sunset

CHANGELOG.md

@@ -36,6 +36,22 @@
   layout, restore exactly as before and the restore script needs no flag to
   tell them apart.
 
+- **A busy phone could be signed out at the very moment it renewed its
+  credentials.** The app renews its access token about once a day, and a
+  renewal that landed while several requests were already on the wire killed
+  the token those requests were carrying. They came back "this credential was
+  withdrawn", the app read that as "sign in again", and asked for another
+  renewal using the refresh token it had just spent — which looks exactly
+  like a stolen token being replayed, so every credential for that device was
+  withdrawn and the person had to log in again. Renewal now retires the
+  outgoing token by pulling its expiry in to fifteen seconds rather than
+  withdrawing it: long enough for requests already in flight to finish, short
+  enough that the old token gains nothing worth having over the seconds it
+  was valid for anyway. A token whose own expiry falls inside that window
+  keeps its own. The replay defence is unchanged — a refresh token presented
+  twice still withdraws the device's credentials on the spot, and so does
+  signing out.
+
 ## [1.38.9] — 2026-09-05
 
 The date order you choose now reaches every date on screen, a plain-HTTP

src/lib/auth/__tests__/refresh-token.test.ts

@@ -17,13 +17,42 @@ const dbState: {
     ipAddress: string | null;
     createdAt: Date;
   }>;
-  apiTokens: Array<{ tokenHash: string; revoked: boolean }>;
+  apiTokens: Array<{
+    tokenHash: string;
+    revoked: boolean;
+    expiresAt: Date | null;
+  }>;
 } = { refreshTokens: [], apiTokens: [] };
 
 let issuedCounter = 0;
 
 vi.mock("@/lib/db", () => ({
   prisma: {
+    /**
+     * Stands in for the one raw statement in this module: the access-token
+     * sunset, `SET expires_at = LEAST(expires_at, $1) WHERE token_hash = $2`.
+     * The parameters arrive in that order. This mock reproduces the minimum,
+     * nothing more — the statement itself is exercised against a real
+     * PostgreSQL in `tests/integration/refresh-rotation-access-sunset.test.ts`.
+     */
+    $executeRaw: vi.fn(
+      async (
+        _sql: TemplateStringsArray,
+        sunsetIso: string,
+        tokenHash: string,
+      ) => {
+        const sunsetAt = new Date(sunsetIso);
+        let count = 0;
+        for (const row of dbState.apiTokens) {
+          if (row.tokenHash !== tokenHash || row.revoked) continue;
+          if (!row.expiresAt || row.expiresAt.getTime() > sunsetAt.getTime()) {
+            row.expiresAt = sunsetAt;
+          }
+          count++;
+        }
+        return count;
+      },
+    ),
     refreshToken: {
       create: vi.fn(async ({ data }: { data: Record<string, unknown> }) => {
         const row = {
@@ -111,13 +140,17 @@ vi.mock("@/lib/auth/issue-token", () => ({
     async (opts: { userId: string; expiresInDays?: number }) => {
       issuedCounter++;
       const token = `hlk_token_${issuedCounter}`;
+      const expiresAt = new Date(
+        Date.now() + (opts.expiresInDays ?? 1) * 86400000,
+      );
       dbState.apiTokens.push({
         tokenHash: `hash:${token}`,
         revoked: false,
+        expiresAt,
       });
       return {
         token,
-        expiresAt: new Date(Date.now() + (opts.expiresInDays ?? 1) * 86400000),
+        expiresAt,
         tokenId: `t_${issuedCounter}`,
         name: "test",
       };
@@ -130,6 +163,7 @@ vi.mock("@/lib/auth/hmac", () => ({
 }));
 
 import {
+  ACCESS_TOKEN_SUNSET_MS,
   issueAccessAndRefresh,
   rotateRefreshToken,
   revokeRefreshToken,
@@ -178,7 +212,13 @@ describe("issueAccessAndRefresh", () => {
 });
 
 describe("rotateRefreshToken", () => {
-  it("rotates: marks old used, issues new pair, revokes old access token", async () => {
+  // The predecessor of this test asserted the opposite — that rotation flips
+  // the old access token to `revoked` on the spot. That instant revoke is the
+  // defect: requests already on the wire came back 401 `revoked`, which sends
+  // a client into a second rotation with the refresh token it just consumed
+  // and costs it the whole family. The token still dies; it dies of expiry, a
+  // few seconds later.
+  it("rotates: marks old used, issues new pair, sunsets old access token", async () => {
     const initial = await issueAccessAndRefresh({
       userId: "u1",
       policy: NATIVE_POLICY,
@@ -199,9 +239,12 @@ describe("rotateRefreshToken", () => {
     const oldRow = dbState.refreshTokens[0];
     expect(oldRow.usedAt).not.toBeNull();
     expect(oldRow.replacedById).not.toBeNull();
-    expect(
-      dbState.apiTokens.find((a) => a.tokenHash === oldHash)?.revoked,
-    ).toBe(true);
+    const oldAccess = dbState.apiTokens.find((a) => a.tokenHash === oldHash);
+    expect(oldAccess?.revoked).toBe(false);
+    expect(oldAccess?.expiresAt?.getTime()).toBeLessThanOrEqual(
+      Date.now() + ACCESS_TOKEN_SUNSET_MS,
+    );
+    expect(oldAccess?.expiresAt?.getTime()).toBeGreaterThan(Date.now());
     expect(dbState.refreshTokens).toHaveLength(2);
   });
 

src/lib/auth/refresh-token.ts

@@ -2,10 +2,12 @@
  * Refresh-token issuance + rotation (v1.4 G4).
  *
  * One-time-use semantics: every successful refresh marks the consumed
- * row's `usedAt`, sets `replacedById` to the new row, and revokes the
- * paired access token. Reuse of an already-consumed token is treated as
- * a stolen-token signal and revokes the entire token family (caller must
- * log in again).
+ * row's `usedAt`, sets `replacedById` to the new row, and sunsets the
+ * paired access token — its expiry is pulled in to a few seconds so
+ * requests already on the wire finish, without the token outliving its
+ * sibling. Reuse of an already-consumed token is treated as a
+ * stolen-token signal and revokes the entire token family instantly
+ * (caller must log in again).
  */
 import { randomBytes } from "node:crypto";
 import { prisma } from "@/lib/db";
@@ -30,6 +32,28 @@ export interface IssueRefreshOpts {
   source: string;
 }
 
+/**
+ * How long an access token stays usable after the refresh token it was paired
+ * with has been rotated away.
+ *
+ * A native client fires several requests at once and rotates in the middle of
+ * them: a request that left the device a few hundred milliseconds before the
+ * rotation committed still carries the outgoing access token and lands after
+ * it. Killing that token at the instant of the commit answered those requests
+ * with 401 `revoked`, which a client reads as "re-authenticate" — it then
+ * rotates again with the refresh token it just consumed, trips reuse
+ * detection, and loses the whole family. Being busy was enough to be signed
+ * out.
+ *
+ * 15 seconds is picked from both ends. Long enough that a request already on
+ * the wire over a slow mobile link finishes on the old credential, including a
+ * retry; short enough that a leaked access token gains nothing worth having —
+ * it was already valid for the seconds before the rotation, and this only
+ * carries that same validity a few seconds further, against an access token
+ * that lives a day.
+ */
+export const ACCESS_TOKEN_SUNSET_MS = 15_000;
+
 function generateRefreshTokenSecret(): string {
   return `hlr_${randomBytes(32).toString("hex")}`;
 }
@@ -91,8 +115,9 @@ export type RotationResult =
 
 /**
  * Atomically rotate a refresh token: validate, mark consumed, issue a new
- * pair, revoke the previously-paired access token. Reuse of a consumed
- * token revokes the whole family (defence against stolen refresh tokens).
+ * pair, sunset the previously-paired access token (see
+ * `ACCESS_TOKEN_SUNSET_MS`). Reuse of a consumed token revokes the whole
+ * family immediately (defence against stolen refresh tokens).
  */
 export async function rotateRefreshToken(input: {
   refreshToken: string;
@@ -221,13 +246,45 @@ export async function rotateRefreshToken(input: {
     return { ok: false, reason: "already_used" };
   }
 
-  // Best-effort: revoke the access token paired with the consumed refresh,
-  // so any leaked access token can't outlive its refresh-token sibling.
+  // Best-effort: sunset the access token paired with the consumed refresh, so
+  // a leaked access token can't meaningfully outlive its refresh-token
+  // sibling. It is a SHORTENED EXPIRY, not a revoke: a request that left the
+  // device before this rotation committed must still be served, and
+  // `expiresAt <= now` is the verdict a client already knows how to handle
+  // (`bearer.ts` answers it with reason `expired`), whereas `revoked` reads as
+  // "re-authenticate" and drives the client into a second rotation with the
+  // refresh token it just consumed.
+  //
+  // This grace applies to the ORDINARY rotation only. The reuse-detection path
+  // above — a replayed refresh token, a spoofed device id — is the
+  // stolen-token defence and keeps revoking the family and its access tokens
+  // instantly, with no window. So does `revokeBearerAccessToken` at logout.
+  //
+  // The write can only ever SHORTEN. `LEAST(expires_at, $1)` takes the minimum
+  // inside the single UPDATE, so a token already inside its last seconds is
+  // not handed extra life by the very call that retires it, and there is no
+  // read-then-write window for a concurrent update to fall into. Postgres
+  // `LEAST` skips NULLs, which is the semantics we want for a token carrying
+  // no fixed expiry: it gets the window and nothing longer.
+  //
+  // Raw because Prisma's `updateMany` cannot express a column-referencing
+  // expression in `data`. Both values ride as tagged-template parameters.
   if (row.accessTokenHash) {
-    await prisma.apiToken.updateMany({
-      where: { tokenHash: row.accessTokenHash, revoked: false },
-      data: { revoked: true },
-    });
+    const sunsetAt = new Date(Date.now() + ACCESS_TOKEN_SUNSET_MS);
+    // The bound value is an ISO-8601 UTC string cast in SQL, not a JS `Date`:
+    // a Date parameter is serialised in the process's local zone, and
+    // `expires_at` is a zone-less UTC column, so the comparison would shift by
+    // the host's offset. `::timestamptz AT TIME ZONE 'UTC'` pins the instant
+    // whatever TZ the app process runs under.
+    await prisma.$executeRaw`
+      UPDATE api_tokens
+         SET expires_at = LEAST(
+               expires_at,
+               ${sunsetAt.toISOString()}::timestamptz AT TIME ZONE 'UTC'
+             )
+       WHERE token_hash = ${row.accessTokenHash}
+         AND revoked = false
+    `;
   }
 
   return { ok: true, bundle };
@@ -243,6 +300,11 @@ export async function rotateRefreshToken(input: {
  * revokes its paired `RefreshToken` sibling (matched by `accessTokenHash`)
  * so the whole credential pair dies with the logout.
  *
+ * A logout is instant and stays instant: no sunset window applies here. The
+ * rotation-time grace exists for requests the client did not choose to
+ * abandon; a person signing out chose, and must not keep a live token for a
+ * further few seconds.
+ *
  * Returns true when a matching live ApiToken row was revoked.
  */
 export async function revokeBearerAccessToken(
@@ -256,7 +318,7 @@ export async function revokeBearerAccessToken(
   });
 
   // Revoke the paired refresh sibling so a native logout kills both halves
-  // of the pair, mirroring the rotation-time access-token revoke.
+  // of the pair in one call.
   await prisma.refreshToken.updateMany({
     where: { accessTokenHash: accessHash, revokedAt: null },
     data: { revokedAt: new Date() },

tests/integration/refresh-rotation-access-sunset.test.ts

@@ -0,0 +1,183 @@
+/**
+ * Refresh rotation must not kill the outgoing access token mid-flight.
+ *
+ * A native client keeps several requests in flight across the access-token
+ * boundary. Some of them leave the device with the old access token a few
+ * hundred milliseconds before the rotation commits and land after it. While
+ * rotation revoked the paired access token instantly, those requests came back
+ * 401 `revoked`, the client read that as "re-authenticate", rotated again with
+ * the refresh token it had just consumed, tripped reuse detection, and the
+ * whole family died: signed out for having been busy.
+ *
+ * The contract this file pins:
+ *   a) the outgoing access token still authenticates inside the sunset window,
+ *      and outside it fails as `expired` — never `revoked`;
+ *   b) reuse detection is untouched: a replayed refresh token revokes the
+ *      family and its access tokens instantly, with no window;
+ *   c) the sunset only ever SHORTENS — a token expiring sooner than the window
+ *      keeps its own expiry;
+ *   d) logout still revokes instantly.
+ */
+import { beforeEach, describe, expect, it, vi } from "vitest";
+
+import { getPrismaClient, truncateAllTables } from "./setup";
+
+const {
+  ACCESS_TOKEN_SUNSET_MS,
+  issueAccessAndRefresh,
+  rotateRefreshToken,
+  revokeBearerAccessToken,
+} = await import("@/lib/auth/refresh-token");
+const { resolveBearerToken, BearerAuthError } =
+  await import("@/lib/auth/bearer");
+const { hashToken } = await import("@/lib/auth/hmac");
+
+const NATIVE_POLICY = {
+  policy: "native" as const,
+  accessTokenDays: 1,
+  refreshTokenDays: 30,
+  tokenLabel: "native",
+};
+
+const DEVICE_ID = "sunset-device-1";
+
+async function seedUser(username: string) {
+  return getPrismaClient().user.create({
+    data: { username, email: `${username}@example.test` },
+  });
+}
+
+/** The bearer verdict as a plain string, so a failure reads as a diff. */
+async function bearerVerdict(rawToken: string): Promise<string> {
+  try {
+    await resolveBearerToken(rawToken, { kind: "wildcard-only" });
+  } catch (error) {
+    if (error instanceof BearerAuthError) return error.reason;
+    throw error;
+  }
+  return "accepted";
+}
+
+async function rotate(refreshToken: string) {
+  const result = await rotateRefreshToken({
+    refreshToken,
+    policy: NATIVE_POLICY,
+    deviceId: DEVICE_ID,
+  });
+  return result;
+}
+
+beforeEach(async () => {
+  await truncateAllTables(getPrismaClient());
+  vi.useRealTimers();
+});
+
+describe("refresh rotation — paired access-token sunset", () => {
+  it("keeps the outgoing access token alive inside the window and expires it after", async () => {
+    const user = await seedUser("sunset-inflight");
+    const first = await issueAccessAndRefresh({
+      userId: user.id,
+      policy: NATIVE_POLICY,
+      deviceId: DEVICE_ID,
+      source: "test",
+    });
+
+    const rotated = await rotate(first.refreshToken);
+    if (!rotated.ok) throw new Error(`rotation failed: ${rotated.reason}`);
+
+    // The in-flight request that left the device before the rotation
+    // committed: it must still be served.
+    expect(await bearerVerdict(first.accessToken)).toBe("accepted");
+
+    // Past the window the token is dead — but as an expiry, not a revoke, so
+    // the client treats it as "refresh", not "re-authenticate".
+    const afterWindow = Date.now() + ACCESS_TOKEN_SUNSET_MS + 1_000;
+    vi.useFakeTimers({ toFake: ["Date"] });
+    vi.setSystemTime(new Date(afterWindow));
+    try {
+      expect(await bearerVerdict(first.accessToken)).toBe("expired");
+    } finally {
+      vi.useRealTimers();
+    }
+
+    // The replacement pair is untouched by the sunset.
+    expect(await bearerVerdict(rotated.bundle.accessToken)).toBe("accepted");
+  });
+
+  it("revokes the family and its access tokens instantly on refresh-token reuse", async () => {
+    const user = await seedUser("sunset-reuse");
+    const first = await issueAccessAndRefresh({
+      userId: user.id,
+      policy: NATIVE_POLICY,
+      deviceId: DEVICE_ID,
+      source: "test",
+    });
+
+    const rotated = await rotate(first.refreshToken);
+    if (!rotated.ok) throw new Error(`rotation failed: ${rotated.reason}`);
+
+    // Replay of the consumed refresh token: the stolen-token defence.
+    const replay = await rotate(first.refreshToken);
+    expect(replay).toEqual({ ok: false, reason: "already_used" });
+
+    // No window here: the live pair's access token is revoked, not sunset.
+    expect(await bearerVerdict(rotated.bundle.accessToken)).toBe("revoked");
+    const revokedRow = await getPrismaClient().apiToken.findUnique({
+      where: { tokenHash: hashToken(rotated.bundle.accessToken) },
+      select: { revoked: true },
+    });
+    expect(revokedRow?.revoked).toBe(true);
+
+    const liveRefresh = await getPrismaClient().refreshToken.findMany({
+      where: { userId: user.id, revokedAt: null },
+    });
+    expect(liveRefresh).toHaveLength(0);
+  });
+
+  it("never extends an access token whose own expiry is sooner than the window", async () => {
+    const user = await seedUser("sunset-min-guard");
+    const first = await issueAccessAndRefresh({
+      userId: user.id,
+      policy: NATIVE_POLICY,
+      deviceId: DEVICE_ID,
+      source: "test",
+    });
+
+    // This token has 5 seconds left of its own — less than the sunset window.
+    const ownExpiry = new Date(Date.now() + 5_000);
+    const accessHash = hashToken(first.accessToken);
+    await getPrismaClient().apiToken.updateMany({
+      where: { tokenHash: accessHash },
+      data: { expiresAt: ownExpiry },
+    });
+
+    const rotated = await rotate(first.refreshToken);
+    if (!rotated.ok) throw new Error(`rotation failed: ${rotated.reason}`);
+
+    const row = await getPrismaClient().apiToken.findUnique({
+      where: { tokenHash: accessHash },
+      select: { expiresAt: true, revoked: true },
+    });
+    expect(row?.revoked).toBe(false);
+    expect(row?.expiresAt?.getTime()).toBe(ownExpiry.getTime());
+  });
+
+  it("revokes instantly at logout", async () => {
+    const user = await seedUser("sunset-logout");
+    const pair = await issueAccessAndRefresh({
+      userId: user.id,
+      policy: NATIVE_POLICY,
+      deviceId: DEVICE_ID,
+      source: "test",
+    });
+
+    expect(await revokeBearerAccessToken(pair.accessToken)).toBe(true);
+
+    expect(await bearerVerdict(pair.accessToken)).toBe("revoked");
+    const refreshRow = await getPrismaClient().refreshToken.findUnique({
+      where: { tokenHash: hashToken(pair.refreshToken) },
+      select: { revokedAt: true },
+    });
+    expect(refreshRow?.revokedAt).not.toBeNull();
+  });
+});