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(); + }); +});