From 05fe89575fde8cfd8871a56a38844003a5004d8d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc-Andr=C3=A9=20Bombeck?= Date: Mon, 11 May 2026 01:08:27 +0200 Subject: [PATCH] fix(auth): refresh-token reuse-detection scopes to originating device MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Pre-1.4.23, a replay attempt on a single device's stale refresh token revoked every refresh token the user owned — signing them out of every device. The legitimate two-device case (iPad + iPhone, one drops into airplane mode and rotates) hit the same code path. v1.4.23 scopes the blast radius to rows matching the offending refresh row's deviceId so only the suspect device is signed out. Legacy tokens issued before v1.4.23 carry deviceId=null and fall back to the wider revoke — a safety hatch we keep until the last 60-day refresh token from the old policy expires. The native-client routes already forward X-Device-Id on every refresh, so the legitimate path always carries a deviceId. A missing deviceId on a replay is itself a suspicious signal, hence the conservative fall-through. Co-Authored-By: Claude Opus 4.7 (1M context) --- src/lib/auth/__tests__/refresh-token.test.ts | 89 +++++++++++++++++++- src/lib/auth/refresh-token.ts | 21 +++-- 2 files changed, 100 insertions(+), 10 deletions(-) diff --git a/src/lib/auth/__tests__/refresh-token.test.ts b/src/lib/auth/__tests__/refresh-token.test.ts index a8d56a1e9..cadd12f47 100644 --- a/src/lib/auth/__tests__/refresh-token.test.ts +++ b/src/lib/auth/__tests__/refresh-token.test.ts @@ -205,7 +205,7 @@ describe("rotateRefreshToken", () => { expect(dbState.refreshTokens).toHaveLength(2); }); - it("rejects + revokes family on reuse of consumed refresh token", async () => { + it("rejects + revokes per-device family on reuse of consumed refresh token", async () => { const initial = await issueAccessAndRefresh({ userId: "u1", policy: NATIVE_POLICY, @@ -220,7 +220,8 @@ describe("rotateRefreshToken", () => { expect(first.ok).toBe(true); // Replay the original token — must be rejected as `already_used` AND - // the replacement family must be revoked too (defence in depth). + // the device's replacement family must be revoked too (defence in + // depth). Per-device scope means dev-1 rows get revoked. const replay = await rotateRefreshToken({ refreshToken: initial.refreshToken, policy: NATIVE_POLICY, @@ -229,7 +230,89 @@ describe("rotateRefreshToken", () => { expect(replay.ok).toBe(false); if (replay.ok) return; expect(replay.reason).toBe("already_used"); - // All non-revoked refresh rows should now be revoked + // Every dev-1 refresh row should now be revoked + const liveDev1 = dbState.refreshTokens.filter( + (r) => r.revokedAt === null && r.deviceId === "dev-1", + ); + expect(liveDev1).toHaveLength(0); + }); + + it("v1.4.23 — per-device replay does NOT revoke another device's tokens", async () => { + // Two-device legitimate scenario: an iPad and an iPhone both + // refresh independently. A replay attempt on dev-1 must NOT sign + // dev-2 out — that was the v1.4.22 behaviour the W4 brief + // explicitly called out as broken. + const dev1 = await issueAccessAndRefresh({ + userId: "u1", + policy: NATIVE_POLICY, + source: "login.password", + deviceId: "dev-1", + }); + const dev2 = await issueAccessAndRefresh({ + userId: "u1", + policy: NATIVE_POLICY, + source: "login.password", + deviceId: "dev-2", + }); + + // dev-1 rotates legitimately, then someone replays its stale token. + await rotateRefreshToken({ + refreshToken: dev1.refreshToken, + policy: NATIVE_POLICY, + deviceId: "dev-1", + }); + const replay = await rotateRefreshToken({ + refreshToken: dev1.refreshToken, + policy: NATIVE_POLICY, + deviceId: "dev-1", + }); + expect(replay.ok).toBe(false); + + // dev-2's refresh token must still be live. + const dev2Row = dbState.refreshTokens.find( + (r) => r.tokenHash === `hash:${dev2.refreshToken}`, + ); + expect(dev2Row?.revokedAt).toBeNull(); + + // dev-1's family is dead. + const liveDev1 = dbState.refreshTokens.filter( + (r) => r.revokedAt === null && r.deviceId === "dev-1", + ); + expect(liveDev1).toHaveLength(0); + }); + + it("v1.4.23 — legacy null-deviceId replay still revokes ALL user tokens", async () => { + // Tokens issued before v1.4.23 carry deviceId === null. We can't + // safely scope the blast radius — fall back to the conservative + // user-wide revoke so a stolen pre-1.4.23 token can't bleed across + // every device the user owns. + const legacy = await issueAccessAndRefresh({ + userId: "u1", + policy: NATIVE_POLICY, + source: "login.password", + // deviceId omitted → null in DB row + }); + const dev2 = await issueAccessAndRefresh({ + userId: "u1", + policy: NATIVE_POLICY, + source: "login.password", + deviceId: "dev-2", + }); + + // legacy rotates once, then someone replays it. + await rotateRefreshToken({ + refreshToken: legacy.refreshToken, + policy: NATIVE_POLICY, + }); + const replay = await rotateRefreshToken({ + refreshToken: legacy.refreshToken, + policy: NATIVE_POLICY, + }); + expect(replay.ok).toBe(false); + + // dev-2 should ALSO be revoked here — null-deviceId triggers the + // wide blast radius (safety hatch). + void dev2; const live = dbState.refreshTokens.filter((r) => r.revokedAt === null); expect(live).toHaveLength(0); }); diff --git a/src/lib/auth/refresh-token.ts b/src/lib/auth/refresh-token.ts index bbcfb6dc1..228a54cbc 100644 --- a/src/lib/auth/refresh-token.ts +++ b/src/lib/auth/refresh-token.ts @@ -116,13 +116,20 @@ export async function rotateRefreshToken(input: { if (row.usedAt) { // Reuse-detection: a previously-consumed refresh token shouldn't be - // presented again. Treat as compromise. Defence-in-depth justifies the - // user-wide blast radius: an attacker who stole the token could rotate - // without an X-Device-Id header (or with a different one); device-scoped - // revocation would leave the attacker's family alive on a "no-device" or - // different-device branch. The legitimate user logging back in is the - // small price; an undetected stolen-token replay is the bigger problem. - const where = { userId: row.userId, revokedAt: null }; + // presented again. v1.4.23 scopes the blast radius to the originating + // device so the legitimate two-device case keeps working — an iPhone + // dropping into airplane mode shouldn't sign the user out of their + // iPad. Tokens issued before v1.4.23 (deviceId === null) still + // revoke-user-wide because we can't isolate them safely. + // + // Defence-in-depth note: the iOS client always sends X-Device-Id on + // refresh, so the legitimate path always carries a deviceId. A + // missing deviceId on a replay is itself a suspicious signal, hence + // the fall-through to the wider revoke. + const where: { userId: string; revokedAt: null; deviceId?: string } = + row.deviceId !== null && row.deviceId !== undefined + ? { userId: row.userId, revokedAt: null, deviceId: row.deviceId } + : { userId: row.userId, revokedAt: null }; const compromised = await prisma.refreshToken.findMany({ where }); await prisma.refreshToken.updateMany({ where,