mirror of
https://github.com/MBombeck/HealthLog.git
synced 2026-11-08 12:32:43 +00:00
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.
This commit is contained in:
1 parent
bf63f6bdba
commit
63e69e98ea
2 files changed
+31
-9
No files matched your search
@@ -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);
|
||||
});
|
||||
|
||||
|
||||
@@ -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" };
|
||||
}
|
||||
|
||||
|
||||
Reference in new issue
Block a user