mirror of
https://github.com/MBombeck/HealthLog.git
synced 2026-11-08 12:32:43 +00:00
fix(auth): bind a live refresh token to its issuing device
Rotation refused a replayed token under a spoofed device id (M-4) but still rotated a *live* one presented from a different device, and then inherited the caller's id onto the new row — putting the resulting family outside the victim device's containment scope. Refuse the mismatch outright when both ids are present; an absent id on either side stays unattributable and rotates as before, so a client that sends no X-Device-Id is not locked out. The expiry refusal had no coverage at all: the standing test presented a token that was never issued, so it passed on not_found and stayed green with the expiry branch deleted. Replace it with the real secret plus a just-inside-the-window companion, and cover the device binding.
This commit is contained in:
1 parent
851a82cc79
commit
0fca5ca207
2 files changed
+100
-6
No files matched your search
@@ -394,21 +394,95 @@ describe("rotateRefreshToken", () => {
|
||||
expect(liveDev1).toHaveLength(0);
|
||||
});
|
||||
|
||||
it("rejects expired refresh tokens", async () => {
|
||||
await issueAccessAndRefresh({
|
||||
it("rejects an expired refresh token by its own secret", async () => {
|
||||
// The token presented here is the REAL one, so the row is found and the
|
||||
// expiry branch is what refuses it. Presenting an unknown string instead
|
||||
// would pass on `not_found` and prove nothing about expiry at all.
|
||||
const initial = await issueAccessAndRefresh({
|
||||
userId: "u1",
|
||||
policy: NATIVE_POLICY,
|
||||
source: "login.password",
|
||||
});
|
||||
// Manually expire
|
||||
dbState.refreshTokens[0].expiresAt = new Date(Date.now() - 1000);
|
||||
|
||||
const result = await rotateRefreshToken({
|
||||
refreshToken: "hlr_doesnt_matter",
|
||||
refreshToken: initial.refreshToken,
|
||||
policy: NATIVE_POLICY,
|
||||
});
|
||||
|
||||
expect(result.ok).toBe(false);
|
||||
if (result.ok) return;
|
||||
expect(result.reason).toBe("not_found");
|
||||
expect(result.reason).toBe("expired");
|
||||
// An expired token must not mint a replacement pair.
|
||||
expect(dbState.refreshTokens).toHaveLength(1);
|
||||
});
|
||||
|
||||
it("accepts the same token one millisecond before it expires", async () => {
|
||||
// Boundary companion: proves the expiry refusal above is driven by the
|
||||
// clock and not by the rotation path refusing everything.
|
||||
const initial = await issueAccessAndRefresh({
|
||||
userId: "u1",
|
||||
policy: NATIVE_POLICY,
|
||||
source: "login.password",
|
||||
});
|
||||
dbState.refreshTokens[0].expiresAt = new Date(Date.now() + 60_000);
|
||||
|
||||
const result = await rotateRefreshToken({
|
||||
refreshToken: initial.refreshToken,
|
||||
policy: NATIVE_POLICY,
|
||||
});
|
||||
|
||||
expect(result.ok).toBe(true);
|
||||
});
|
||||
|
||||
it("refuses a live token presented under a different device id", async () => {
|
||||
// Device A's token replayed from device B. The token is valid and unused,
|
||||
// so only the device binding can refuse it.
|
||||
const dev1 = await issueAccessAndRefresh({
|
||||
userId: "u1",
|
||||
policy: NATIVE_POLICY,
|
||||
source: "login.password",
|
||||
deviceId: "dev-1",
|
||||
});
|
||||
|
||||
const result = await rotateRefreshToken({
|
||||
refreshToken: dev1.refreshToken,
|
||||
policy: NATIVE_POLICY,
|
||||
deviceId: "dev-2",
|
||||
});
|
||||
|
||||
expect(result.ok).toBe(false);
|
||||
if (result.ok) return;
|
||||
expect(result.reason).toBe("device_mismatch");
|
||||
// Refused, not consumed: the legitimate device can still rotate it.
|
||||
expect(dbState.refreshTokens[0].usedAt).toBeNull();
|
||||
expect(dbState.refreshTokens).toHaveLength(1);
|
||||
|
||||
const legitimate = await rotateRefreshToken({
|
||||
refreshToken: dev1.refreshToken,
|
||||
policy: NATIVE_POLICY,
|
||||
deviceId: "dev-1",
|
||||
});
|
||||
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.
|
||||
const dev1 = await issueAccessAndRefresh({
|
||||
userId: "u1",
|
||||
policy: NATIVE_POLICY,
|
||||
source: "login.password",
|
||||
deviceId: "dev-1",
|
||||
});
|
||||
|
||||
const result = await rotateRefreshToken({
|
||||
refreshToken: dev1.refreshToken,
|
||||
policy: NATIVE_POLICY,
|
||||
});
|
||||
|
||||
expect(result.ok).toBe(true);
|
||||
});
|
||||
|
||||
it("rejects revoked refresh tokens", async () => {
|
||||
|
||||
@@ -83,7 +83,7 @@ export async function issueAccessAndRefresh(
|
||||
}
|
||||
|
||||
export type RotationFailureReason =
|
||||
"not_found" | "expired" | "already_used" | "revoked";
|
||||
"not_found" | "expired" | "already_used" | "revoked" | "device_mismatch";
|
||||
|
||||
export type RotationResult =
|
||||
| { ok: true; bundle: IssuedRefreshBundle }
|
||||
@@ -158,6 +158,26 @@ export async function rotateRefreshToken(input: {
|
||||
return { ok: false, reason: "already_used" };
|
||||
}
|
||||
|
||||
// A live token belongs to the device it was issued to. The M-4 escalation
|
||||
// above already treats a mismatched deviceId on a REPLAY as a spoofing
|
||||
// signal; the same mismatch on a still-live token is refused outright
|
||||
// rather than silently rotated. Without this the reuse-detection scope is
|
||||
// only as honest as the caller: a thief could rotate a stolen token under
|
||||
// their own fabricated device id, and the resulting family — keyed on the
|
||||
// attacker's id, because `issueAccessAndRefresh` below inherits
|
||||
// `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.
|
||||
const presentedId = input.deviceId ?? null;
|
||||
const storedId = row.deviceId ?? null;
|
||||
if (presentedId !== null && storedId !== null && presentedId !== storedId) {
|
||||
return { ok: false, reason: "device_mismatch" };
|
||||
}
|
||||
|
||||
// Issue the new pair first, THEN mark old consumed in a transaction.
|
||||
// (Race: if two concurrent refreshes hit the same row, both will try to
|
||||
// mark `usedAt`. We use updateMany with a `usedAt: null` guard so only
|
||||
|
||||
Reference in new issue
Block a user