From dfb5d7483f9485e99feb5a4ec442e55e43d0dbf6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc-Andr=C3=A9=20Bombeck?= Date: Sun, 6 Sep 2026 15:35:08 +0200 Subject: [PATCH] fix(auth): sunset the rotated-away access token instead of revoking it Rotation revoked the access token paired with the consumed refresh token the instant the rotation committed. A native client with requests already on the wire got 401 revoked on those, read that as "re-authenticate", rotated again with the refresh token it had just spent, and tripped reuse detection: the device's whole token family died because the client was busy at the boundary. Pull the paired token's expiry in to fifteen seconds instead. It is long enough for an in-flight request on a slow mobile link to finish, including a retry, and short enough that the token gains nothing meaningful over the validity it already had. bearer.ts answers a passed expiry with reason expired, which a client treats as "refresh", not as "sign in again". LEAST(expires_at, $1) takes the minimum inside the single UPDATE, so the write can only ever shorten: a token already inside its last seconds keeps its own expiry, and there is no read-then-write window. The bound value is an ISO-8601 UTC string cast in SQL so the comparison does not shift with the host's timezone. Reuse detection is untouched. A replayed refresh token, or one presented under a spoofed device id, still revokes the family and its access tokens on the spot, and so does the logout path. --- CHANGELOG.md | 16 ++ src/lib/auth/__tests__/refresh-token.test.ts | 55 +++++- src/lib/auth/refresh-token.ts | 88 +++++++-- .../refresh-rotation-access-sunset.test.ts | 183 ++++++++++++++++++ 4 files changed, 323 insertions(+), 19 deletions(-) create mode 100644 tests/integration/refresh-rotation-access-sunset.test.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index 80d5daf31..960c8f91e 100644 --- a/CHANGELOG.md +++ b/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 diff --git a/src/lib/auth/__tests__/refresh-token.test.ts b/src/lib/auth/__tests__/refresh-token.test.ts index b19d98a4b..943539135 100644 --- a/src/lib/auth/__tests__/refresh-token.test.ts +++ b/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 }) => { 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); }); diff --git a/src/lib/auth/refresh-token.ts b/src/lib/auth/refresh-token.ts index 37997964f..bf597ee01 100644 --- a/src/lib/auth/refresh-token.ts +++ b/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() }, diff --git a/tests/integration/refresh-rotation-access-sunset.test.ts b/tests/integration/refresh-rotation-access-sunset.test.ts new file mode 100644 index 000000000..a7ff45d6d --- /dev/null +++ b/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 { + 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(); + }); +});