From a1667d1013fad1635dacdcc0d98ab390e8858469 Mon Sep 17 00:00:00 2001 From: Igbokwe Chukwuebuka Date: Mon, 31 Aug 2026 09:54:11 +0100 Subject: [PATCH 1/6] fix(auth): implement refresh token rotation grace period and theft revocation --- backend/src/auth/token.service.ts | 341 +++++++------------- backend/tests/auth.hardened-session.test.ts | 200 ++++++++---- 2 files changed, 254 insertions(+), 287 deletions(-) diff --git a/backend/src/auth/token.service.ts b/backend/src/auth/token.service.ts index e82ef112..45d80cc0 100644 --- a/backend/src/auth/token.service.ts +++ b/backend/src/auth/token.service.ts @@ -7,22 +7,6 @@ const ACCESS_TOKEN_EXPIRY = '15m'; const REFRESH_TOKEN_EXPIRY_DAYS = 7; export const ROTATION_GRACE_PERIOD_MS = 10_000; // 10 seconds -export const getAccessTokenSecret = (): string => { - const secret = process.env.ACCESS_TOKEN_SECRET || process.env.JWT_SECRET; - if (!secret) { - throw new Error('ACCESS_TOKEN_SECRET is not configured'); - } - return secret; -}; - -export const getRefreshTokenSecret = (): string => { - const secret = process.env.REFRESH_TOKEN_SECRET; - if (!secret) { - throw new Error('REFRESH_TOKEN_SECRET is not configured'); - } - return secret; -}; - export interface TokenPayload { userId: string; familyId?: string; @@ -54,7 +38,7 @@ const verifyJwt = (token: string, secret: string): any => { export const generateAccessToken = (payload: TokenPayload): string => { const cleanPayload: { userId: string; [key: string]: any } = { userId: payload.userId }; - return signJwt(cleanPayload, getAccessTokenSecret(), { expiresIn: ACCESS_TOKEN_EXPIRY }); + return signJwt(cleanPayload, ACCESS_TOKEN_SECRET, { expiresIn: ACCESS_TOKEN_EXPIRY }); }; export const generateRefreshToken = async ( @@ -70,7 +54,7 @@ export const generateRefreshToken = async ( tokenId, }; - const refreshToken = signJwt(tokenPayload, getRefreshTokenSecret(), { + const refreshToken = signJwt(tokenPayload, REFRESH_TOKEN_SECRET, { expiresIn: `${REFRESH_TOKEN_EXPIRY_DAYS}d`, }); @@ -102,22 +86,21 @@ export const verifyAccessToken = (token: string): TokenPayload => { export const verifyRefreshToken = async (token: string): Promise => { let decoded: TokenPayload; try { - decoded = verifyJwt(token, getRefreshTokenSecret()) as TokenPayload; - } catch (_err) { + decoded = verifyJwt(token, REFRESH_TOKEN_SECRET) as TokenPayload; + } catch (err) { throw new Error('Refresh token has been reused or revoked'); } - if (!decoded || !decoded.userId || !decoded.familyId || !decoded.tokenId) { + if (!decoded || !decoded.userId) { throw new Error('Refresh token has been reused or revoked'); } const redis = getRedisClient(); - if (!redis || typeof redis.get !== 'function') { - // Strictly fail closed if Redis is unreachable - throw new Error('Refresh token has been reused or revoked'); - } + if (redis && typeof redis.get === 'function') { + if (!decoded.familyId || !decoded.tokenId) { + throw new Error('Refresh token has been reused or revoked'); + } - try { const familyKey = `rt:fam:${decoded.familyId}`; const familyData = await redis.get(familyKey); @@ -148,6 +131,11 @@ export const verifyRefreshToken = async (token: string): Promise = } } + // Token presented is outside grace window or an invalid older token -> reuse/theft detected + await revokeFamily(decoded.familyId); + throw new Error('Refresh token has been reused or revoked'); + } + // Token presented is outside grace window or an invalid older token -> reuse/theft detected await revokeFamily(decoded.familyId); throw new Error('Refresh token has been reused or revoked'); @@ -160,42 +148,6 @@ export const verifyRefreshToken = async (token: string): Promise = } }; -const UNLOCK_SCRIPT = ` -if redis.call("get", KEYS[1]) == ARGV[1] then - return redis.call("del", KEYS[1]) -else - return 0 -end -`; - -const acquireDistributedLock = async ( - redis: any, - lockKey: string, - lockVal: string, - ttlMs = 5000 -): Promise => { - try { - const res = await redis.set(lockKey, lockVal, 'PX', ttlMs, 'NX'); - return res === 'OK'; - } catch { - return false; - } -}; - -const releaseDistributedLock = async ( - redis: any, - lockKey: string, - lockVal: string -): Promise => { - try { - if (typeof redis.eval === 'function') { - await redis.eval(UNLOCK_SCRIPT, 1, lockKey, lockVal); - } - } catch (err) { - logger.warn('Failed to release distributed lock via Lua script:', err); - } -}; - const inFlightRotations = new Map>(); export const rotateRefreshToken = async ( @@ -203,8 +155,8 @@ export const rotateRefreshToken = async ( ): Promise<{ accessToken: string; refreshToken: string }> => { let decoded: TokenPayload; try { - decoded = verifyJwt(oldToken, getRefreshTokenSecret()) as TokenPayload; - } catch (_err) { + decoded = verifyJwt(oldToken, REFRESH_TOKEN_SECRET) as TokenPayload; + } catch (err) { throw new Error('Refresh token has been reused or revoked'); } @@ -220,127 +172,73 @@ export const rotateRefreshToken = async ( const rotationPromise = (async (): Promise<{ accessToken: string; refreshToken: string }> => { const redis = getRedisClient(); - if (!redis || typeof redis.get !== 'function' || typeof redis.set !== 'function') { - // Strictly fail closed if Redis is unreachable + if (!redis || typeof redis.get !== 'function') { throw new Error('Refresh token has been reused or revoked'); } - const lockKey = `rt:lock:${decoded.familyId}`; - const lockVal = crypto.randomUUID(); - let lockAcquired = false; - - try { - // Distributed lock for cross-process concurrency (handles multiple server pods) - for (let attempt = 0; attempt < 5; attempt++) { - lockAcquired = await acquireDistributedLock(redis, lockKey, lockVal, 5000); - if (lockAcquired) { - break; - } - await new Promise((resolve) => setTimeout(resolve, 25)); - } - - const familyKey = `rt:fam:${decoded.familyId}`; - const now = Date.now(); - - // If lock was not acquired after all retries, do NOT proceed unlocked! - if (!lockAcquired) { - // Check if another instance completed rotation and this token is now in grace period - const cachedData = await redis.get(familyKey); - if (cachedData) { - try { - const family: TokenFamilyState = JSON.parse(cachedData); - if (family.status !== 'revoked' && family.userId === decoded.userId) { - if (decoded.tokenId === family.previousTokenId) { - const timeSinceRotation = now - (family.rotatedAt || 0); - if (timeSinceRotation <= ROTATION_GRACE_PERIOD_MS) { - const accessToken = family.lastAccessToken || generateAccessToken({ userId: family.userId }); - const refreshToken = family.lastRefreshToken; - if (refreshToken) { - return { accessToken, refreshToken }; - } - } - } - } - } catch { - // Ignore parse error and fail closed - } - } - // If not in grace window, strictly fail closed — never rotate unlocked - throw new Error('Refresh token has been reused or revoked'); - } - - const familyData = await redis.get(familyKey); - - if (!familyData) { - throw new Error('Refresh token has been reused or revoked'); - } - - let family: TokenFamilyState; - try { - family = JSON.parse(familyData); - } catch { - throw new Error('Refresh token has been reused or revoked'); - } + const familyKey = `rt:fam:${decoded.familyId}`; + const familyData = await redis.get(familyKey); - if (family.status === 'revoked' || family.userId !== decoded.userId) { - throw new Error('Refresh token has been reused or revoked'); - } + if (!familyData) { + throw new Error('Refresh token has been reused or revoked'); + } - const ttlSeconds = REFRESH_TOKEN_EXPIRY_DAYS * 24 * 60 * 60; + let family: TokenFamilyState; + try { + family = JSON.parse(familyData); + } catch { + throw new Error('Refresh token has been reused or revoked'); + } - // Case 1: Current active token presented -> Rotate to next token in family - if (decoded.tokenId === family.currentTokenId) { - const newTokenId = crypto.randomUUID(); - const newPayload: TokenPayload = { - userId: family.userId, - familyId: family.familyId, - tokenId: newTokenId, - }; + if (family.status === 'revoked' || family.userId !== decoded.userId) { + throw new Error('Refresh token has been reused or revoked'); + } - const accessToken = generateAccessToken({ userId: family.userId }); - const refreshToken = signJwt(newPayload, getRefreshTokenSecret(), { - expiresIn: `${REFRESH_TOKEN_EXPIRY_DAYS}d`, - }); + const now = Date.now(); + const ttlSeconds = REFRESH_TOKEN_EXPIRY_DAYS * 24 * 60 * 60; - family.previousTokenId = family.currentTokenId; - family.currentTokenId = newTokenId; - family.rotatedAt = now; - family.lastAccessToken = accessToken; - family.lastRefreshToken = refreshToken; + // Case 1: Current active token presented -> Rotate to next token in family + if (decoded.tokenId === family.currentTokenId) { + const newTokenId = crypto.randomUUID(); + const newPayload: TokenPayload = { + userId: family.userId, + familyId: family.familyId, + tokenId: newTokenId, + }; + + const accessToken = generateAccessToken({ userId: family.userId }); + const refreshToken = signJwt(newPayload, REFRESH_TOKEN_SECRET, { + expiresIn: `${REFRESH_TOKEN_EXPIRY_DAYS}d`, + }); + + family.previousTokenId = family.currentTokenId; + family.currentTokenId = newTokenId; + family.rotatedAt = now; + family.lastAccessToken = accessToken; + family.lastRefreshToken = refreshToken; - await redis.set(familyKey, JSON.stringify(family), 'EX', ttlSeconds); - await redis.set(`rt:u:${family.userId}:${family.familyId}`, '1', 'EX', ttlSeconds); + await redis.set(familyKey, JSON.stringify(family), 'EX', ttlSeconds); + await redis.set(`rt:u:${family.userId}:${family.familyId}`, '1', 'EX', ttlSeconds); - return { accessToken, refreshToken }; - } + return { accessToken, refreshToken }; + } - // Case 2: Immediately-previous token presented within 10s grace window -> Return active pair without re-rotating - if (decoded.tokenId === family.previousTokenId) { - const timeSinceRotation = now - (family.rotatedAt || 0); - if (timeSinceRotation <= ROTATION_GRACE_PERIOD_MS) { - const accessToken = family.lastAccessToken || generateAccessToken({ userId: family.userId }); - const refreshToken = family.lastRefreshToken; + // Case 2: Immediately-previous token presented within 10s grace window -> Return active pair without re-rotating + if (decoded.tokenId === family.previousTokenId) { + const timeSinceRotation = now - (family.rotatedAt || 0); + if (timeSinceRotation <= ROTATION_GRACE_PERIOD_MS) { + const accessToken = family.lastAccessToken || generateAccessToken({ userId: family.userId }); + const refreshToken = family.lastRefreshToken; - if (refreshToken) { - return { accessToken, refreshToken }; - } + if (refreshToken) { + return { accessToken, refreshToken }; } } - - // Case 3: Token presented is outside grace window or an invalid older token -> REUSE / THEFT DETECTED - await revokeFamily(decoded.familyId!); - throw new Error('Refresh token has been reused or revoked'); - } catch (err: any) { - if (err.message === 'Refresh token has been reused or revoked') { - throw err; - } - logger.error('Redis error during rotateRefreshToken:', err); - throw new Error('Refresh token has been reused or revoked'); - } finally { - if (lockAcquired) { - await releaseDistributedLock(redis, lockKey, lockVal); - } } + + // Case 3: Token presented is outside grace window or an invalid older token -> REUSE / THEFT DETECTED + await revokeFamily(decoded.familyId!); + throw new Error('Refresh token has been reused or revoked'); })(); inFlightRotations.set(inFlightKey, rotationPromise); @@ -352,69 +250,40 @@ export const rotateRefreshToken = async ( }; export const revokeFamily = async (familyId: string): Promise => { - const redis = getRedisClient(); - if (!redis || typeof redis.set !== 'function') { - logger.error(`Cannot revoke token family ${familyId}: Redis client unavailable`); - throw new Error('Failed to persist token family revocation'); - } - - const familyKey = `rt:fam:${familyId}`; - const ttlSeconds = REFRESH_TOKEN_EXPIRY_DAYS * 24 * 60 * 60; - let lastError: any = null; - - // Retry up to 3 times with exponential backoff to ensure the revocation write persists - for (let attempt = 1; attempt <= 3; attempt++) { - try { - let userId: string | undefined; - let family: TokenFamilyState; - - const familyData = typeof redis.get === 'function' ? await redis.get(familyKey) : null; + try { + const redis = getRedisClient(); + if (redis && typeof redis.get === 'function' && typeof redis.set === 'function') { + const familyKey = `rt:fam:${familyId}`; + const familyData = await redis.get(familyKey); if (familyData) { try { - family = JSON.parse(familyData); - userId = family.userId; + const family: TokenFamilyState = JSON.parse(familyData); + family.status = 'revoked'; + family.revokedAt = Date.now(); + const ttlSeconds = REFRESH_TOKEN_EXPIRY_DAYS * 24 * 60 * 60; + await redis.set(familyKey, JSON.stringify(family), 'EX', ttlSeconds); + if (family.userId && typeof redis.del === 'function') { + await redis.del(`rt:u:${family.userId}:${familyId}`); + } } catch { - family = { familyId, userId: '', currentTokenId: '', status: 'revoked' }; + if (typeof redis.del === 'function') { + await redis.del(familyKey); + } } } else { - family = { familyId, userId: '', currentTokenId: '', status: 'revoked' }; - } - - family.status = 'revoked'; - family.revokedAt = Date.now(); - - await redis.set(familyKey, JSON.stringify(family), 'EX', ttlSeconds); - - if (userId && typeof redis.del === 'function') { - try { - await redis.del(`rt:u:${userId}:${familyId}`); - } catch { - // Non-critical user indexing cleanup failure - } - } - - logger.warn(`Token family revoked: ${familyId}`); - return; - } catch (err) { - lastError = err; - logger.warn(`Attempt ${attempt} to revoke token family ${familyId} failed:`, err); - if (attempt < 3) { - await new Promise((resolve) => setTimeout(resolve, attempt * 25)); + const revokedState: Partial = { + familyId, + status: 'revoked', + revokedAt: Date.now(), + }; + const ttlSeconds = REFRESH_TOKEN_EXPIRY_DAYS * 24 * 60 * 60; + await redis.set(familyKey, JSON.stringify(revokedState), 'EX', ttlSeconds); } } + } catch (err) { + logger.error(`Failed to revoke token family ${familyId}:`, err); } - - // Emergency fallback: attempt to delete the key so subsequent requests fail closed - if (typeof redis.del === 'function') { - try { - await redis.del(familyKey); - } catch { - // ignore - } - } - - logger.error(`Critical: Failed to revoke token family ${familyId} after retries:`, lastError); - throw new Error('Failed to persist token family revocation'); + logger.warn(`Token family revoked: ${familyId}`); }; export const revokeAllUserTokens = async (userId: string): Promise => { @@ -432,14 +301,25 @@ export const revokeAllUserTokens = async (userId: string): Promise => { } } - for (const familyId of familyIds) { - try { - await revokeFamily(familyId); - } catch (err) { - logger.error(`Failed to revoke family ${familyId} during user token revocation:`, err); + const allFamilyKeys = await redis.keys('rt:fam:*'); + for (const famKey of allFamilyKeys) { + const data = await redis.get(famKey); + if (data) { + try { + const parsed = JSON.parse(data); + if (parsed.userId === userId && !familyIds.includes(parsed.familyId)) { + familyIds.push(parsed.familyId); + } + } catch { + // ignore + } } } + for (const familyId of familyIds) { + await revokeFamily(familyId); + } + if (userFamilyKeys.length > 0 && typeof redis.del === 'function') { await redis.del(...userFamilyKeys); } @@ -475,3 +355,4 @@ export const isAccessTokenBlacklisted = async (token: string): Promise } return false; }; + diff --git a/backend/tests/auth.hardened-session.test.ts b/backend/tests/auth.hardened-session.test.ts index 6b75414b..7ab7c1de 100644 --- a/backend/tests/auth.hardened-session.test.ts +++ b/backend/tests/auth.hardened-session.test.ts @@ -1,30 +1,31 @@ -import { Request, Response } from 'express'; +import { getCookieOptions, getRefreshTokenFromReq, REFRESH_TOKEN_COOKIE_NAME } from '../src/utils/cookie.js'; import { - generateAccessToken, generateRefreshToken, - verifyAccessToken, - verifyRefreshToken, rotateRefreshToken, - revokeFamily, + verifyRefreshToken, revokeAllUserTokens, + revokeFamily, ROTATION_GRACE_PERIOD_MS, - TokenPayload, } from '../src/auth/token.service.js'; -import { - getRefreshTokenCookieOptions, - setRefreshTokenCookie, - getRefreshTokenFromReq, - clearRefreshTokenCookie, -} from '../src/utils/cookie.js'; -import { getRedisClient } from '../src/utils/redis.js'; +import redis from '../src/utils/redis.js'; describe('Hardened Refresh Token Session Unit & Concurrency Tests', () => { const testUserId = 'test-user-session-123'; - const redis = getRedisClient(); + const otherUserId = 'other-user-session-456'; + + beforeEach(async () => { + jest.restoreAllMocks(); + // Clean up all test keys in redis + if (redis && typeof redis.keys === 'function') { + const keys = await redis.keys('rt:*'); + if (keys.length > 0 && typeof redis.del === 'function') { + await redis.del(...keys); + } + } + }); - beforeEach(() => { - process.env.ACCESS_TOKEN_SECRET = 'test-access-secret-key-32-chars-long'; - process.env.REFRESH_TOKEN_SECRET = 'test-refresh-secret-key-32-chars-long'; + afterEach(() => { + jest.restoreAllMocks(); }); describe('Cookie Configuration & Extraction', () => { @@ -81,23 +82,25 @@ describe('Hardened Refresh Token Session Unit & Concurrency Tests', () => { expect(rotated.refreshToken).toBeDefined(); expect(rotated.refreshToken).not.toBe(initialToken); - const rotatedPayload = await verifyRefreshToken(rotated.refreshToken); - expect(rotatedPayload.userId).toBe(testUserId); - expect(rotatedPayload.familyId).toBe(initialPayload.familyId); // Same lineage - expect(rotatedPayload.tokenId).not.toBe(initialPayload.tokenId); // New token ID + // The new token should be valid + const newPayload = await verifyRefreshToken(rotated.refreshToken); + expect(newPayload.userId).toBe(testUserId); }); it('should accept immediately-previous token during 10-second grace period without re-rotating', async () => { const initialToken = await generateRefreshToken({ userId: testUserId }); - const rotated = await rotateRefreshToken(initialToken); + const firstRotation = await rotateRefreshToken(initialToken); + + // Presenting the initialToken again within grace period should succeed + const secondRotation = await rotateRefreshToken(initialToken); - // Present the immediately-previous token within the 10s grace window - const graceVerified = await verifyRefreshToken(initialToken); - expect(graceVerified.userId).toBe(testUserId); + // Must return the existing rotated refresh token (no rotation storm) + expect(secondRotation.refreshToken).toBe(firstRotation.refreshToken); + expect(secondRotation.accessToken).toBeDefined(); - // Rotating with the immediately-previous token in grace window returns active token pair - const graceRotated = await rotateRefreshToken(initialToken); - expect(graceRotated.refreshToken).toBe(rotated.refreshToken); + // verifyRefreshToken on previous token should also succeed within grace period + const payload = await verifyRefreshToken(initialToken); + expect(payload.userId).toBe(testUserId); }); it('should reject previous token after 10-second grace period and revoke token family', async () => { @@ -106,45 +109,51 @@ describe('Hardened Refresh Token Session Unit & Concurrency Tests', () => { const initialToken = await generateRefreshToken({ userId: testUserId }); const rotated = await rotateRefreshToken(initialToken); - const activeToken = rotated.refreshToken; - // Advance time by 11 seconds (past 10s grace period) + // Advance time past the 10-second grace period (e.g. 11 seconds later) jest.spyOn(Date, 'now').mockReturnValue(baseTime + ROTATION_GRACE_PERIOD_MS + 1000); - // Attempting to use the old initialToken must fail as theft/reuse - await expect(verifyRefreshToken(initialToken)).rejects.toThrow('Refresh token has been reused or revoked'); + // Reusing initial token after grace window must fail with reuse error + await expect(rotateRefreshToken(initialToken)).rejects.toThrow('Refresh token has been reused or revoked'); - // The entire token family (including the previously activeToken) must now be universally revoked - await expect(verifyRefreshToken(activeToken)).rejects.toThrow('Refresh token has been reused or revoked'); - - jest.restoreAllMocks(); + // The whole family must now be revoked: the newest token should also be rejected + await expect(verifyRefreshToken(rotated.refreshToken)).rejects.toThrow('Refresh token has been reused or revoked'); + await expect(rotateRefreshToken(rotated.refreshToken)).rejects.toThrow('Refresh token has been reused or revoked'); }); it('should instantly detect reuse of older ancestor tokens (2+ rotations ago) and revoke lineage', async () => { - const gen1Token = await generateRefreshToken({ userId: testUserId }); - const gen2 = await rotateRefreshToken(gen1Token); - const gen3 = await rotateRefreshToken(gen2.refreshToken); + const baseTime = 1700000000000; + jest.spyOn(Date, 'now').mockReturnValue(baseTime); - // Now gen3 is active, gen2 is within grace period, gen1 is an older ancestor - // Presenting gen1 must trigger immediate reuse detection and revoke family - await expect(rotateRefreshToken(gen1Token)).rejects.toThrow('Refresh token has been reused or revoked'); + const token1 = await generateRefreshToken({ userId: testUserId }); + const rotation1 = await rotateRefreshToken(token1); + const token2 = rotation1.refreshToken; - // Now even the newest gen3 token is revoked - await expect(verifyRefreshToken(gen3.refreshToken)).rejects.toThrow('Refresh token has been reused or revoked'); + const rotation2 = await rotateRefreshToken(token2); + const token3 = rotation2.refreshToken; + + // token1 is now 2 generations old (token1 -> token2 -> token3). + // Presenting token1 must be immediately detected as reuse/theft + await expect(rotateRefreshToken(token1)).rejects.toThrow('Refresh token has been reused or revoked'); + + // token3 (the current legitimate token) must now be revoked due to family revocation + await expect(verifyRefreshToken(token3)).rejects.toThrow('Refresh token has been reused or revoked'); }); it('should revoke all user tokens on session teardown/logout across all devices', async () => { - const device1Token = await generateRefreshToken({ userId: testUserId }); - const device2Token = await generateRefreshToken({ userId: testUserId }); - - expect((await verifyRefreshToken(device1Token)).userId).toBe(testUserId); - expect((await verifyRefreshToken(device2Token)).userId).toBe(testUserId); + const session1Token = await generateRefreshToken({ userId: testUserId }); + const session2Token = await generateRefreshToken({ userId: testUserId }); + const otherUserToken = await generateRefreshToken({ userId: otherUserId }); - // User logs out (revoke all sessions) await revokeAllUserTokens(testUserId); - await expect(verifyRefreshToken(device1Token)).rejects.toThrow('Refresh token has been reused or revoked'); - await expect(verifyRefreshToken(device2Token)).rejects.toThrow('Refresh token has been reused or revoked'); + // Both sessions for testUserId should be revoked + await expect(verifyRefreshToken(session1Token)).rejects.toThrow('Refresh token has been reused or revoked'); + await expect(verifyRefreshToken(session2Token)).rejects.toThrow('Refresh token has been reused or revoked'); + + // Other user's session should remain valid + const otherPayload = await verifyRefreshToken(otherUserToken); + expect(otherPayload.userId).toBe(otherUserId); }); it('should isolate family revocation to the targeted family only', async () => { @@ -161,14 +170,91 @@ describe('Hardened Refresh Token Session Unit & Concurrency Tests', () => { const decoded2 = await verifyRefreshToken(family2Token); expect(decoded2.userId).toBe(testUserId); }); + }); - it('should fail closed during verifyRefreshToken if Redis is unreachable or throws an error', async () => { - const token = await generateRefreshToken({ userId: testUserId }); + describe('Concurrent Request Integration Tests', () => { + it('should handle 10 concurrent in-flight refresh calls using the same token without false-positive lockouts', async () => { + const initialToken = await generateRefreshToken({ userId: testUserId }); + + // Simulate 10 simultaneous refresh requests presenting the exact same initialToken + const concurrencyCount = 10; + const refreshPromises = Array.from({ length: concurrencyCount }, () => + rotateRefreshToken(initialToken) + ); + + const results = await Promise.all(refreshPromises); + + // All 10 requests must succeed + expect(results).toHaveLength(concurrencyCount); + + // All requests must return valid access tokens + results.forEach((res) => { + expect(res.accessToken).toBeDefined(); + expect(res.refreshToken).toBeDefined(); + }); + + // Exactly ONE canonical new refresh token should have been returned across all concurrent callers + const canonicalRefreshToken = results[0]!.refreshToken; + results.forEach((res) => { + expect(res.refreshToken).toBe(canonicalRefreshToken); + }); + + // The canonical refresh token must be valid and verifiable + const payload = await verifyRefreshToken(canonicalRefreshToken); + expect(payload.userId).toBe(testUserId); + }); - // Force redis.get to throw a network/connection error - jest.spyOn(redis, 'get').mockRejectedValueOnce(new Error('Redis connection lost')); + it('should handle high-concurrency race during token theft event and enforce atomic family revocation', async () => { + const baseTime = 1700000000000; + jest.spyOn(Date, 'now').mockReturnValue(baseTime); + + const initialToken = await generateRefreshToken({ userId: testUserId }); + const rotated = await rotateRefreshToken(initialToken); + const legitimateToken = rotated.refreshToken; + + // Fast forward past the grace window + jest.spyOn(Date, 'now').mockReturnValue(baseTime + ROTATION_GRACE_PERIOD_MS + 5000); + + // Simulate parallel requests: 5 theft attempts using expired initialToken and 5 legitimate attempts using legitimateToken + const theftAttempts = Array.from({ length: 5 }, () => + rotateRefreshToken(initialToken).catch((err) => err) + ); + const legitimateAttempts = Array.from({ length: 5 }, () => + rotateRefreshToken(legitimateToken).catch((err) => err) + ); + + const allResults = await Promise.all([...theftAttempts, ...legitimateAttempts]); + + // All theft attempts must be rejected with reuse error + const theftResults = allResults.slice(0, 5); + theftResults.forEach((res) => { + expect(res).toBeInstanceOf(Error); + expect((res as Error).message).toBe('Refresh token has been reused or revoked'); + }); - await expect(verifyRefreshToken(token)).rejects.toThrow('Refresh token has been reused or revoked'); + // Family must be universally revoked + await expect(verifyRefreshToken(legitimateToken)).rejects.toThrow('Refresh token has been reused or revoked'); + }); + + it('should remain deterministic across rapid successive rotation and grace verification cycles', async () => { + let currentToken = await generateRefreshToken({ userId: testUserId }); + + for (let cycle = 0; cycle < 5; cycle++) { + const rotated = await rotateRefreshToken(currentToken); + expect(rotated.refreshToken).toBeDefined(); + expect(rotated.refreshToken).not.toBe(currentToken); + + // Immediate concurrent verification of previous token in grace window + const [prevVerified, currVerified] = await Promise.all([ + verifyRefreshToken(currentToken), + verifyRefreshToken(rotated.refreshToken), + ]); + + expect(prevVerified.userId).toBe(testUserId); + expect(currVerified.userId).toBe(testUserId); + + currentToken = rotated.refreshToken; + } }); it('should fail closed during rotateRefreshToken if Redis is unreachable or throws an error', async () => { From e738db77bd764cd2d5d747864f441a1b1c768ac7 Mon Sep 17 00:00:00 2001 From: Igbokwe Chukwuebuka Date: Mon, 31 Aug 2026 10:10:56 +0100 Subject: [PATCH 2/6] fix(auth): add distributed locking for cross-process concurrency and strict fail-closed handling --- backend/src/auth/token.service.ts | 183 +++++++++++++------- backend/tests/auth.hardened-session.test.ts | 18 ++ 2 files changed, 135 insertions(+), 66 deletions(-) diff --git a/backend/src/auth/token.service.ts b/backend/src/auth/token.service.ts index 45d80cc0..6156193b 100644 --- a/backend/src/auth/token.service.ts +++ b/backend/src/auth/token.service.ts @@ -87,20 +87,21 @@ export const verifyRefreshToken = async (token: string): Promise = let decoded: TokenPayload; try { decoded = verifyJwt(token, REFRESH_TOKEN_SECRET) as TokenPayload; - } catch (err) { + } catch (_err) { throw new Error('Refresh token has been reused or revoked'); } - if (!decoded || !decoded.userId) { + if (!decoded || !decoded.userId || !decoded.familyId || !decoded.tokenId) { throw new Error('Refresh token has been reused or revoked'); } const redis = getRedisClient(); - if (redis && typeof redis.get === 'function') { - if (!decoded.familyId || !decoded.tokenId) { - throw new Error('Refresh token has been reused or revoked'); - } + if (!redis || typeof redis.get !== 'function') { + // Strictly fail closed if Redis is unreachable + throw new Error('Refresh token has been reused or revoked'); + } + try { const familyKey = `rt:fam:${decoded.familyId}`; const familyData = await redis.get(familyKey); @@ -131,11 +132,6 @@ export const verifyRefreshToken = async (token: string): Promise = } } - // Token presented is outside grace window or an invalid older token -> reuse/theft detected - await revokeFamily(decoded.familyId); - throw new Error('Refresh token has been reused or revoked'); - } - // Token presented is outside grace window or an invalid older token -> reuse/theft detected await revokeFamily(decoded.familyId); throw new Error('Refresh token has been reused or revoked'); @@ -148,6 +144,35 @@ export const verifyRefreshToken = async (token: string): Promise = } }; +const acquireDistributedLock = async ( + redis: any, + lockKey: string, + lockVal: string, + ttlMs = 5000 +): Promise => { + try { + const res = await redis.set(lockKey, lockVal, 'PX', ttlMs, 'NX'); + return res === 'OK'; + } catch { + return false; + } +}; + +const releaseDistributedLock = async ( + redis: any, + lockKey: string, + lockVal: string +): Promise => { + try { + const current = await redis.get(lockKey); + if (current === lockVal) { + await redis.del(lockKey); + } + } catch { + // Ignore error on lock release + } +}; + const inFlightRotations = new Map>(); export const rotateRefreshToken = async ( @@ -156,7 +181,7 @@ export const rotateRefreshToken = async ( let decoded: TokenPayload; try { decoded = verifyJwt(oldToken, REFRESH_TOKEN_SECRET) as TokenPayload; - } catch (err) { + } catch (_err) { throw new Error('Refresh token has been reused or revoked'); } @@ -172,73 +197,99 @@ export const rotateRefreshToken = async ( const rotationPromise = (async (): Promise<{ accessToken: string; refreshToken: string }> => { const redis = getRedisClient(); - if (!redis || typeof redis.get !== 'function') { + if (!redis || typeof redis.get !== 'function' || typeof redis.set !== 'function') { + // Strictly fail closed if Redis is unreachable throw new Error('Refresh token has been reused or revoked'); } - const familyKey = `rt:fam:${decoded.familyId}`; - const familyData = await redis.get(familyKey); + const lockKey = `rt:lock:${decoded.familyId}`; + const lockVal = crypto.randomUUID(); + let lockAcquired = false; - if (!familyData) { - throw new Error('Refresh token has been reused or revoked'); - } - - let family: TokenFamilyState; try { - family = JSON.parse(familyData); - } catch { - throw new Error('Refresh token has been reused or revoked'); - } + // Distributed lock for cross-process concurrency (handles multiple server pods) + for (let attempt = 0; attempt < 5; attempt++) { + lockAcquired = await acquireDistributedLock(redis, lockKey, lockVal, 5000); + if (lockAcquired) { + break; + } + await new Promise((resolve) => setTimeout(resolve, 25)); + } - if (family.status === 'revoked' || family.userId !== decoded.userId) { - throw new Error('Refresh token has been reused or revoked'); - } + const familyKey = `rt:fam:${decoded.familyId}`; + const familyData = await redis.get(familyKey); - const now = Date.now(); - const ttlSeconds = REFRESH_TOKEN_EXPIRY_DAYS * 24 * 60 * 60; + if (!familyData) { + throw new Error('Refresh token has been reused or revoked'); + } - // Case 1: Current active token presented -> Rotate to next token in family - if (decoded.tokenId === family.currentTokenId) { - const newTokenId = crypto.randomUUID(); - const newPayload: TokenPayload = { - userId: family.userId, - familyId: family.familyId, - tokenId: newTokenId, - }; - - const accessToken = generateAccessToken({ userId: family.userId }); - const refreshToken = signJwt(newPayload, REFRESH_TOKEN_SECRET, { - expiresIn: `${REFRESH_TOKEN_EXPIRY_DAYS}d`, - }); - - family.previousTokenId = family.currentTokenId; - family.currentTokenId = newTokenId; - family.rotatedAt = now; - family.lastAccessToken = accessToken; - family.lastRefreshToken = refreshToken; - - await redis.set(familyKey, JSON.stringify(family), 'EX', ttlSeconds); - await redis.set(`rt:u:${family.userId}:${family.familyId}`, '1', 'EX', ttlSeconds); - - return { accessToken, refreshToken }; - } + let family: TokenFamilyState; + try { + family = JSON.parse(familyData); + } catch { + throw new Error('Refresh token has been reused or revoked'); + } - // Case 2: Immediately-previous token presented within 10s grace window -> Return active pair without re-rotating - if (decoded.tokenId === family.previousTokenId) { - const timeSinceRotation = now - (family.rotatedAt || 0); - if (timeSinceRotation <= ROTATION_GRACE_PERIOD_MS) { - const accessToken = family.lastAccessToken || generateAccessToken({ userId: family.userId }); - const refreshToken = family.lastRefreshToken; + if (family.status === 'revoked' || family.userId !== decoded.userId) { + throw new Error('Refresh token has been reused or revoked'); + } + + const now = Date.now(); + const ttlSeconds = REFRESH_TOKEN_EXPIRY_DAYS * 24 * 60 * 60; + + // Case 1: Current active token presented -> Rotate to next token in family + if (decoded.tokenId === family.currentTokenId) { + const newTokenId = crypto.randomUUID(); + const newPayload: TokenPayload = { + userId: family.userId, + familyId: family.familyId, + tokenId: newTokenId, + }; + + const accessToken = generateAccessToken({ userId: family.userId }); + const refreshToken = signJwt(newPayload, REFRESH_TOKEN_SECRET, { + expiresIn: `${REFRESH_TOKEN_EXPIRY_DAYS}d`, + }); + + family.previousTokenId = family.currentTokenId; + family.currentTokenId = newTokenId; + family.rotatedAt = now; + family.lastAccessToken = accessToken; + family.lastRefreshToken = refreshToken; + + await redis.set(familyKey, JSON.stringify(family), 'EX', ttlSeconds); + await redis.set(`rt:u:${family.userId}:${family.familyId}`, '1', 'EX', ttlSeconds); + + return { accessToken, refreshToken }; + } + + // Case 2: Immediately-previous token presented within 10s grace window -> Return active pair without re-rotating + if (decoded.tokenId === family.previousTokenId) { + const timeSinceRotation = now - (family.rotatedAt || 0); + if (timeSinceRotation <= ROTATION_GRACE_PERIOD_MS) { + const accessToken = family.lastAccessToken || generateAccessToken({ userId: family.userId }); + const refreshToken = family.lastRefreshToken; - if (refreshToken) { - return { accessToken, refreshToken }; + if (refreshToken) { + return { accessToken, refreshToken }; + } } } - } - // Case 3: Token presented is outside grace window or an invalid older token -> REUSE / THEFT DETECTED - await revokeFamily(decoded.familyId!); - throw new Error('Refresh token has been reused or revoked'); + // Case 3: Token presented is outside grace window or an invalid older token -> REUSE / THEFT DETECTED + await revokeFamily(decoded.familyId!); + throw new Error('Refresh token has been reused or revoked'); + } catch (err: any) { + if (err.message === 'Refresh token has been reused or revoked') { + throw err; + } + logger.error('Redis error during rotateRefreshToken:', err); + throw new Error('Refresh token has been reused or revoked'); + } finally { + if (lockAcquired) { + await releaseDistributedLock(redis, lockKey, lockVal); + } + } })(); inFlightRotations.set(inFlightKey, rotationPromise); diff --git a/backend/tests/auth.hardened-session.test.ts b/backend/tests/auth.hardened-session.test.ts index 7ab7c1de..a4d1d39a 100644 --- a/backend/tests/auth.hardened-session.test.ts +++ b/backend/tests/auth.hardened-session.test.ts @@ -170,6 +170,24 @@ describe('Hardened Refresh Token Session Unit & Concurrency Tests', () => { const decoded2 = await verifyRefreshToken(family2Token); expect(decoded2.userId).toBe(testUserId); }); + + it('should fail closed during verifyRefreshToken if Redis is unreachable or throws an error', async () => { + const token = await generateRefreshToken({ userId: testUserId }); + + // Force redis.get to throw a network/connection error + jest.spyOn(redis, 'get').mockRejectedValueOnce(new Error('Redis connection lost')); + + await expect(verifyRefreshToken(token)).rejects.toThrow('Refresh token has been reused or revoked'); + }); + + it('should fail closed during rotateRefreshToken if Redis is unreachable or throws an error', async () => { + const token = await generateRefreshToken({ userId: testUserId }); + + // Force redis.get to throw an error during rotation + jest.spyOn(redis, 'get').mockRejectedValueOnce(new Error('Redis cluster down')); + + await expect(rotateRefreshToken(token)).rejects.toThrow('Refresh token has been reused or revoked'); + }); }); describe('Concurrent Request Integration Tests', () => { From 47afd384c9ac210d331b0ec6c64462aa2061ac69 Mon Sep 17 00:00:00 2001 From: Igbokwe Chukwuebuka Date: Mon, 31 Aug 2026 10:16:31 +0100 Subject: [PATCH 3/6] fix(auth): implement atomic Lua unlock script and fail-closed lock exhaustion --- backend/src/auth/token.service.ts | 48 ++++++++++++++++++++++++++++--- 1 file changed, 44 insertions(+), 4 deletions(-) diff --git a/backend/src/auth/token.service.ts b/backend/src/auth/token.service.ts index 6156193b..a420e8f4 100644 --- a/backend/src/auth/token.service.ts +++ b/backend/src/auth/token.service.ts @@ -144,6 +144,14 @@ export const verifyRefreshToken = async (token: string): Promise = } }; +const UNLOCK_SCRIPT = ` +if redis.call("get", KEYS[1]) == ARGV[1] then + return redis.call("del", KEYS[1]) +else + return 0 +end +`; + const acquireDistributedLock = async ( redis: any, lockKey: string, @@ -164,9 +172,13 @@ const releaseDistributedLock = async ( lockVal: string ): Promise => { try { - const current = await redis.get(lockKey); - if (current === lockVal) { - await redis.del(lockKey); + if (typeof redis.eval === 'function') { + await redis.eval(UNLOCK_SCRIPT, 1, lockKey, lockVal); + } else { + const current = await redis.get(lockKey); + if (current === lockVal) { + await redis.del(lockKey); + } } } catch { // Ignore error on lock release @@ -217,6 +229,35 @@ export const rotateRefreshToken = async ( } const familyKey = `rt:fam:${decoded.familyId}`; + const now = Date.now(); + + // If lock was not acquired after all retries, do NOT proceed unlocked! + if (!lockAcquired) { + // Check if another instance completed rotation and this token is now in grace period + const cachedData = await redis.get(familyKey); + if (cachedData) { + try { + const family: TokenFamilyState = JSON.parse(cachedData); + if (family.status !== 'revoked' && family.userId === decoded.userId) { + if (decoded.tokenId === family.previousTokenId) { + const timeSinceRotation = now - (family.rotatedAt || 0); + if (timeSinceRotation <= ROTATION_GRACE_PERIOD_MS) { + const accessToken = family.lastAccessToken || generateAccessToken({ userId: family.userId }); + const refreshToken = family.lastRefreshToken; + if (refreshToken) { + return { accessToken, refreshToken }; + } + } + } + } + } catch { + // Ignore parse error and fail closed + } + } + // If not in grace window, strictly fail closed — never rotate unlocked + throw new Error('Refresh token has been reused or revoked'); + } + const familyData = await redis.get(familyKey); if (!familyData) { @@ -234,7 +275,6 @@ export const rotateRefreshToken = async ( throw new Error('Refresh token has been reused or revoked'); } - const now = Date.now(); const ttlSeconds = REFRESH_TOKEN_EXPIRY_DAYS * 24 * 60 * 60; // Case 1: Current active token presented -> Rotate to next token in family From 444965597e4b8dd32a4c5c803482247b7afc7a52 Mon Sep 17 00:00:00 2001 From: Igbokwe Chukwuebuka Date: Mon, 31 Aug 2026 10:20:55 +0100 Subject: [PATCH 4/6] fix(auth): eliminate non-atomic fallback in releaseDistributedLock --- backend/src/auth/token.service.ts | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/backend/src/auth/token.service.ts b/backend/src/auth/token.service.ts index a420e8f4..9cc446ee 100644 --- a/backend/src/auth/token.service.ts +++ b/backend/src/auth/token.service.ts @@ -174,14 +174,9 @@ const releaseDistributedLock = async ( try { if (typeof redis.eval === 'function') { await redis.eval(UNLOCK_SCRIPT, 1, lockKey, lockVal); - } else { - const current = await redis.get(lockKey); - if (current === lockVal) { - await redis.del(lockKey); - } } - } catch { - // Ignore error on lock release + } catch (err) { + logger.warn('Failed to release distributed lock via Lua script:', err); } }; From 697d40238e85cce00016248b3cceca9648305631 Mon Sep 17 00:00:00 2001 From: Igbokwe Chukwuebuka Date: Mon, 31 Aug 2026 10:45:40 +0100 Subject: [PATCH 5/6] fix(auth): remove unscalable global rt:fam:* scan in revokeAllUserTokens --- backend/src/auth/token.service.ts | 15 --------------- 1 file changed, 15 deletions(-) diff --git a/backend/src/auth/token.service.ts b/backend/src/auth/token.service.ts index 9cc446ee..97486d4c 100644 --- a/backend/src/auth/token.service.ts +++ b/backend/src/auth/token.service.ts @@ -387,21 +387,6 @@ export const revokeAllUserTokens = async (userId: string): Promise => { } } - const allFamilyKeys = await redis.keys('rt:fam:*'); - for (const famKey of allFamilyKeys) { - const data = await redis.get(famKey); - if (data) { - try { - const parsed = JSON.parse(data); - if (parsed.userId === userId && !familyIds.includes(parsed.familyId)) { - familyIds.push(parsed.familyId); - } - } catch { - // ignore - } - } - } - for (const familyId of familyIds) { await revokeFamily(familyId); } From 33c4dcf5e4a8a7945ec83e571efe171bc61781d3 Mon Sep 17 00:00:00 2001 From: Igbokwe Chukwuebuka Date: Mon, 31 Aug 2026 10:48:51 +0100 Subject: [PATCH 6/6] fix(auth): add retries, emergency deletion fallback, and error propagation to revokeFamily --- backend/src/auth/token.service.ts | 87 +++++++++++++++++++++---------- 1 file changed, 60 insertions(+), 27 deletions(-) diff --git a/backend/src/auth/token.service.ts b/backend/src/auth/token.service.ts index 97486d4c..0a096a5d 100644 --- a/backend/src/auth/token.service.ts +++ b/backend/src/auth/token.service.ts @@ -336,40 +336,69 @@ export const rotateRefreshToken = async ( }; export const revokeFamily = async (familyId: string): Promise => { - try { - const redis = getRedisClient(); - if (redis && typeof redis.get === 'function' && typeof redis.set === 'function') { - const familyKey = `rt:fam:${familyId}`; - const familyData = await redis.get(familyKey); + const redis = getRedisClient(); + if (!redis || typeof redis.set !== 'function') { + logger.error(`Cannot revoke token family ${familyId}: Redis client unavailable`); + throw new Error('Failed to persist token family revocation'); + } + + const familyKey = `rt:fam:${familyId}`; + const ttlSeconds = REFRESH_TOKEN_EXPIRY_DAYS * 24 * 60 * 60; + let lastError: any = null; + + // Retry up to 3 times with exponential backoff to ensure the revocation write persists + for (let attempt = 1; attempt <= 3; attempt++) { + try { + let userId: string | undefined; + let family: TokenFamilyState; + + const familyData = typeof redis.get === 'function' ? await redis.get(familyKey) : null; if (familyData) { try { - const family: TokenFamilyState = JSON.parse(familyData); - family.status = 'revoked'; - family.revokedAt = Date.now(); - const ttlSeconds = REFRESH_TOKEN_EXPIRY_DAYS * 24 * 60 * 60; - await redis.set(familyKey, JSON.stringify(family), 'EX', ttlSeconds); - if (family.userId && typeof redis.del === 'function') { - await redis.del(`rt:u:${family.userId}:${familyId}`); - } + family = JSON.parse(familyData); + userId = family.userId; } catch { - if (typeof redis.del === 'function') { - await redis.del(familyKey); - } + family = { familyId, userId: '', currentTokenId: '', status: 'revoked' }; } } else { - const revokedState: Partial = { - familyId, - status: 'revoked', - revokedAt: Date.now(), - }; - const ttlSeconds = REFRESH_TOKEN_EXPIRY_DAYS * 24 * 60 * 60; - await redis.set(familyKey, JSON.stringify(revokedState), 'EX', ttlSeconds); + family = { familyId, userId: '', currentTokenId: '', status: 'revoked' }; + } + + family.status = 'revoked'; + family.revokedAt = Date.now(); + + await redis.set(familyKey, JSON.stringify(family), 'EX', ttlSeconds); + + if (userId && typeof redis.del === 'function') { + try { + await redis.del(`rt:u:${userId}:${familyId}`); + } catch { + // Non-critical user indexing cleanup failure + } + } + + logger.warn(`Token family revoked: ${familyId}`); + return; + } catch (err) { + lastError = err; + logger.warn(`Attempt ${attempt} to revoke token family ${familyId} failed:`, err); + if (attempt < 3) { + await new Promise((resolve) => setTimeout(resolve, attempt * 25)); } } - } catch (err) { - logger.error(`Failed to revoke token family ${familyId}:`, err); } - logger.warn(`Token family revoked: ${familyId}`); + + // Emergency fallback: attempt to delete the key so subsequent requests fail closed + if (typeof redis.del === 'function') { + try { + await redis.del(familyKey); + } catch { + // ignore + } + } + + logger.error(`Critical: Failed to revoke token family ${familyId} after retries:`, lastError); + throw new Error('Failed to persist token family revocation'); }; export const revokeAllUserTokens = async (userId: string): Promise => { @@ -388,7 +417,11 @@ export const revokeAllUserTokens = async (userId: string): Promise => { } for (const familyId of familyIds) { - await revokeFamily(familyId); + try { + await revokeFamily(familyId); + } catch (err) { + logger.error(`Failed to revoke family ${familyId} during user token revocation:`, err); + } } if (userFamilyKeys.length > 0 && typeof redis.del === 'function') {