From 63e69e98ea7cdd5bdbd089b4a2a24aa63193ad4b Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Marc-Andr=C3=A9=20Bombeck?= <mbombeck@gmail.com>
Date: Sat, 26 Sep 2026 20:23:55 +0200
Subject: [PATCH] fix(auth): refuse a device-bound refresh token presented
 without its device

A refresh token issued to a device is refused when presented under another
device id, but leaving the X-Device-Id header out skipped the comparison,
so a stolen token rotated simply by naming no device. A token whose row
carries a device id now rotates only for a caller naming that same id.
Tokens issued without one still rotate as before. The iOS app sends its
device id on every request, sign-in and refresh included.
---
 src/lib/auth/__tests__/refresh-token.test.ts | 27 +++++++++++++++++---
 src/lib/auth/refresh-token.ts                | 13 ++++++----
 2 files changed, 31 insertions(+), 9 deletions(-)

diff --git a/src/lib/auth/__tests__/refresh-token.test.ts b/src/lib/auth/__tests__/refresh-token.test.ts
index 943539135..87461fdd9 100644
--- a/src/lib/auth/__tests__/refresh-token.test.ts
+++ b/src/lib/auth/__tests__/refresh-token.test.ts
@@ -509,10 +509,9 @@ describe("rotateRefreshToken", () => {
     expect(legitimate.ok).toBe(true);
   });
 
-  it("still rotates when the caller sends no device id at all", async () => {
-    // An older client that never sends `X-Device-Id` must not be locked out
-    // by the device binding — a presented null is unattributable, not a
-    // mismatch.
+  it("refuses a device-bound token presented with no device id", async () => {
+    // Leaving the header out must not be a way round the device binding: a
+    // token issued to dev-1 is rotated only by a caller that says dev-1.
     const dev1 = await issueAccessAndRefresh({
       userId: "u1",
       policy: NATIVE_POLICY,
@@ -525,6 +524,26 @@ describe("rotateRefreshToken", () => {
       policy: NATIVE_POLICY,
     });
 
+    expect(result.ok).toBe(false);
+    if (result.ok) return;
+    expect(result.reason).toBe("device_mismatch");
+    expect(dbState.refreshTokens[0].usedAt).toBeNull();
+  });
+
+  it("still rotates a token issued without a device id, header or not", async () => {
+    // An older client that never sent `X-Device-Id` holds rows with no stored
+    // id, and nothing to compare means nothing to refuse.
+    const legacy = await issueAccessAndRefresh({
+      userId: "u1",
+      policy: NATIVE_POLICY,
+      source: "login.password",
+    });
+
+    const result = await rotateRefreshToken({
+      refreshToken: legacy.refreshToken,
+      policy: NATIVE_POLICY,
+    });
+
     expect(result.ok).toBe(true);
   });
 
diff --git a/src/lib/auth/refresh-token.ts b/src/lib/auth/refresh-token.ts
index 007c70ed7..1edb8bfc8 100644
--- a/src/lib/auth/refresh-token.ts
+++ b/src/lib/auth/refresh-token.ts
@@ -193,13 +193,16 @@ export async function rotateRefreshToken(input: {
   // `row.deviceId ?? input.deviceId` — would sit outside the victim device's
   // containment blast radius.
   //
-  // Both ids must be present to compare. A stored null means the row predates
-  // per-device issuance and cannot be attributed; a presented null means the
-  // caller sent no `X-Device-Id` header. Neither is treated as a mismatch, so
-  // this cannot lock out an older client that never sends the header.
+  // A stored null means the row predates per-device issuance, or the client
+  // sent no `X-Device-Id` when it signed in: nothing to compare, so it
+  // rotates. A stored id must be matched exactly, and an absent header does
+  // not match it. It used to: leaving the header out skipped the check, so a
+  // thief holding a stolen token rotated it simply by not naming a device. A
+  // client that sent its id at sign-in sends it on every refresh, so this
+  // locks out nobody who signed in honestly.
   const presentedId = input.deviceId ?? null;
   const storedId = row.deviceId ?? null;
-  if (presentedId !== null && storedId !== null && presentedId !== storedId) {
+  if (storedId !== null && presentedId !== storedId) {
     return { ok: false, reason: "device_mismatch" };
   }