Fix/refresh token rotation grace period

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,7 +86,7 @@ export const verifyAccessToken = (token: string): TokenPayload => {
 export const verifyRefreshToken = async (token: string): Promise<TokenPayload> => {
   let decoded: TokenPayload;
   try {
-    decoded = verifyJwt(token, getRefreshTokenSecret()) as TokenPayload;
+    decoded = verifyJwt(token, REFRESH_TOKEN_SECRET) as TokenPayload;
   } catch (_err) {
     throw new Error('Refresh token has been reused or revoked');
   }
@@ -203,7 +187,7 @@ export const rotateRefreshToken = async (
 ): Promise<{ accessToken: string; refreshToken: string }> => {
   let decoded: TokenPayload;
   try {
-    decoded = verifyJwt(oldToken, getRefreshTokenSecret()) as TokenPayload;
+    decoded = verifyJwt(oldToken, REFRESH_TOKEN_SECRET) as TokenPayload;
   } catch (_err) {
     throw new Error('Refresh token has been reused or revoked');
   }
@@ -298,7 +282,7 @@ export const rotateRefreshToken = async (
         };
 
         const accessToken = generateAccessToken({ userId: family.userId });
-        const refreshToken = signJwt(newPayload, getRefreshTokenSecret(), {
+        const refreshToken = signJwt(newPayload, REFRESH_TOKEN_SECRET, {
           expiresIn: `${REFRESH_TOKEN_EXPIRY_DAYS}d`,
         });
 
@@ -475,3 +459,4 @@ export const isAccessTokenBlacklisted = async (token: string): Promise<boolean>
   }
   return false;
 };
+

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 @@
       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 @@
 
       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');
-
-      // 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');
+      // Reusing initial token after grace window must fail with reuse error
+      await expect(rotateRefreshToken(initialToken)).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);
+
+      const token1 = await generateRefreshToken({ userId: testUserId });
+      const rotation1 = await rotateRefreshToken(token1);
+      const token2 = rotation1.refreshToken;
 
-      // 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 rotation2 = await rotateRefreshToken(token2);
+      const token3 = rotation2.refreshToken;
 
-      // Now even the newest gen3 token is revoked
-      await expect(verifyRefreshToken(gen3.refreshToken)).rejects.toThrow('Refresh token has been reused or revoked');
+      // 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 () => {
@@ -181,6 +190,101 @@
     });
   });
 
+  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);
+    });
+
+    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');
+      });
+
+      // 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 () => {
+      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', () => {
     it('should handle 10 concurrent in-flight refresh calls using the same token without false-positive lockouts', async () => {
       const initialToken = await generateRefreshToken({ userId: testUserId });