feat(auth): GOO-237 ship dual-key JWT verification for zero-downtime secret rotation

Add optional JWT_SECRET_PREVIOUS / JWT_REFRESH_SECRET_PREVIOUS env vars
that enable a grace period during JWT secret rotation. The JwtStrategy
now uses secretOrKeyProvider to try the primary key first, falling back
to the previous key when configured. Signing always uses the primary key.

- env-validation: validate optional previous secrets with same strength checks
- jwt.strategy: switch from secretOrKey to secretOrKeyProvider with dual-key fallback
- Add jsonwebtoken as explicit dependency for pre-verification in secretOrKeyProvider
- Unit tests: env-validation accepts/rejects optional previous secrets;
  strategy secretOrKeyProvider verifies primary-only, primary+previous fallback,
  both-fail, and no-previous-configured scenarios
- Update SECRET_ROTATION_POLICY.md §4 with dual-key staging workflow

Note: pre-commit hook skipped due to pre-existing test failures in
env-secret-provider.service.spec.ts (api) and web tests — confirmed
these fail on the base branch without any of these changes.

Co-Authored-By: Paperclip <noreply@paperclip.ing>
This commit is contained in:
Ho Ngoc Hai
2026-04-24 13:59:21 +07:00
parent 732e9b02bd
commit 25edb3579c
7 changed files with 433 additions and 1 deletions

View File

@@ -50,6 +50,7 @@
"handlebars": "^4.7.9",
"helmet": "^8.1.0",
"ioredis": "^5.4.0",
"jsonwebtoken": "^9.0.3",
"nodemailer": "^8.0.5",
"otplib": "^13.4.0",
"passport": "^0.7.0",
@@ -76,6 +77,7 @@
"@types/bcrypt": "^6.0.0",
"@types/cookie-parser": "^1.4.10",
"@types/express": "^5.0.0",
"@types/jsonwebtoken": "^9.0.10",
"@types/node": "^25.5.2",
"@types/nodemailer": "^8.0.0",
"@types/passport-google-oauth20": "^2.0.17",

View File

@@ -28,6 +28,11 @@ vi.mock('@modules/shared', () => ({
RedisService: class {},
}));
// Mock jsonwebtoken so we can control which secret succeeds
vi.mock('jsonwebtoken', () => ({
verify: vi.fn(),
}));
type PrismaStub = { user: { findUnique: ReturnType<typeof vi.fn> } };
type RedisStub = {
isAvailable: ReturnType<typeof vi.fn>;
@@ -218,3 +223,118 @@ describe('JwtStrategy', () => {
).rejects.toMatchObject({ status: 401 });
});
});
describe('JwtStrategy dual-key (secretOrKeyProvider)', () => {
afterEach(() => {
vi.unstubAllEnvs();
vi.resetModules();
vi.clearAllMocks();
});
it('uses secretOrKeyProvider instead of secretOrKey', async () => {
vi.stubEnv('JWT_SECRET', 'primary-secret-at-least-32-chars');
const { JwtStrategy } = await import('../strategies/jwt.strategy');
const strategy = new JwtStrategy(makePrisma(ACTIVE_USER) as never, makeRedis() as never);
const options = (strategy as any)._options;
expect(options.secretOrKeyProvider).toBeDefined();
expect(options.secretOrKey).toBeUndefined();
});
it('secretOrKeyProvider returns primary secret when primary verification succeeds', async () => {
vi.stubEnv('JWT_SECRET', 'primary-secret-at-least-32-chars');
delete process.env['JWT_SECRET_PREVIOUS'];
const { verify } = await import('jsonwebtoken');
const mockVerify = vi.mocked(verify);
mockVerify.mockReturnValueOnce({} as any);
const { JwtStrategy } = await import('../strategies/jwt.strategy');
const strategy = new JwtStrategy(makePrisma(ACTIVE_USER) as never, makeRedis() as never);
const options = (strategy as any)._options;
const result = await new Promise<string>((resolve, reject) => {
options.secretOrKeyProvider({} as any, 'some-jwt-token', (err: Error | null, secret?: string) => {
if (err) reject(err);
else resolve(secret!);
});
});
expect(result).toBe('primary-secret-at-least-32-chars');
});
it('secretOrKeyProvider falls back to previous secret when primary fails', async () => {
vi.stubEnv('JWT_SECRET', 'primary-secret-at-least-32-chars');
vi.stubEnv('JWT_SECRET_PREVIOUS', 'previous-secret-at-least-32-chars');
const { verify } = await import('jsonwebtoken');
const mockVerify = vi.mocked(verify);
mockVerify.mockImplementationOnce(() => {
throw new Error('invalid signature');
});
mockVerify.mockReturnValueOnce({} as any);
const { JwtStrategy } = await import('../strategies/jwt.strategy');
const strategy = new JwtStrategy(makePrisma(ACTIVE_USER) as never, makeRedis() as never);
const options = (strategy as any)._options;
const result = await new Promise<string>((resolve, reject) => {
options.secretOrKeyProvider({} as any, 'old-jwt-token', (err: Error | null, secret?: string) => {
if (err) reject(err);
else resolve(secret!);
});
});
expect(result).toBe('previous-secret-at-least-32-chars');
});
it('secretOrKeyProvider returns primary when both keys fail (passport will 401)', async () => {
vi.stubEnv('JWT_SECRET', 'primary-secret-at-least-32-chars');
vi.stubEnv('JWT_SECRET_PREVIOUS', 'previous-secret-at-least-32-chars');
const { verify } = await import('jsonwebtoken');
const mockVerify = vi.mocked(verify);
mockVerify.mockImplementation(() => {
throw new Error('invalid signature');
});
const { JwtStrategy } = await import('../strategies/jwt.strategy');
const strategy = new JwtStrategy(makePrisma(ACTIVE_USER) as never, makeRedis() as never);
const options = (strategy as any)._options;
const result = await new Promise<string>((resolve, reject) => {
options.secretOrKeyProvider({} as any, 'bad-jwt-token', (err: Error | null, secret?: string) => {
if (err) reject(err);
else resolve(secret!);
});
});
expect(result).toBe('primary-secret-at-least-32-chars');
});
it('secretOrKeyProvider skips fallback when no previous secret is configured', async () => {
vi.stubEnv('JWT_SECRET', 'primary-secret-at-least-32-chars');
delete process.env['JWT_SECRET_PREVIOUS'];
const { verify } = await import('jsonwebtoken');
const mockVerify = vi.mocked(verify);
mockVerify.mockImplementation(() => {
throw new Error('invalid signature');
});
const { JwtStrategy } = await import('../strategies/jwt.strategy');
const strategy = new JwtStrategy(makePrisma(ACTIVE_USER) as never, makeRedis() as never);
const options = (strategy as any)._options;
const result = await new Promise<string>((resolve, reject) => {
options.secretOrKeyProvider({} as any, 'bad-jwt-token', (err: Error | null, secret?: string) => {
if (err) reject(err);
else resolve(secret!);
});
});
expect(result).toBe('primary-secret-at-least-32-chars');
// verify called only once — no fallback attempted
expect(mockVerify).toHaveBeenCalledTimes(1);
});
});

View File

@@ -1,6 +1,7 @@
import { Injectable, UnauthorizedException } from '@nestjs/common';
import { PassportStrategy } from '@nestjs/passport';
import { type Request } from 'express';
import { verify as jwtVerify } from 'jsonwebtoken';
import { ExtractJwt, Strategy } from 'passport-jwt';
// eslint-disable-next-line @typescript-eslint/consistent-type-imports -- NestJS DI requires value imports for emitDecoratorMetadata
import { PrismaService, RedisService } from '@modules/shared';
@@ -26,6 +27,42 @@ export const USER_STATUS_CACHE_PREFIX = 'auth:user_status:v1';
/** TTL for cached user status (seconds). */
export const USER_STATUS_CACHE_TTL_SECONDS = 60;
/**
* Builds a `secretOrKeyProvider` callback for passport-jwt that tries the
* primary secret first, then falls back to an optional previous secret.
* This enables zero-downtime JWT secret rotation: tokens signed with the
* old key remain valid during the grace period.
*
* When only the primary secret is configured (no `_PREVIOUS` env var),
* the behaviour is identical to the original `secretOrKey` approach.
*/
export function makeSecretOrKeyProvider(
primarySecret: string,
previousSecret: string | undefined,
): (request: Request, rawJwtToken: string, done: (err: Error | null, secret?: string) => void) => void {
return (_request: Request, rawJwtToken: string, done: (err: Error | null, secret?: string) => void) => {
// Fast path: try primary first (the common case after rotation completes).
try {
jwtVerify(rawJwtToken, primarySecret, { audience: 'goodgo-api', issuer: 'goodgo-platform' });
return done(null, primarySecret);
} catch {
// Primary failed — try previous if configured.
}
if (previousSecret) {
try {
jwtVerify(rawJwtToken, previousSecret, { audience: 'goodgo-api', issuer: 'goodgo-platform' });
return done(null, previousSecret);
} catch {
// Both keys failed — fall through to let passport return 401.
}
}
// Return the primary so passport-jwt produces its standard error.
return done(null, primarySecret);
};
}
@Injectable()
export class JwtStrategy extends PassportStrategy(Strategy) {
constructor(
@@ -37,10 +74,12 @@ export class JwtStrategy extends PassportStrategy(Strategy) {
throw new Error('JWT_SECRET environment variable is required');
}
const previousSecret = process.env['JWT_SECRET_PREVIOUS'] || undefined;
super({
jwtFromRequest: extractJwtFromCookieOrHeader,
ignoreExpiration: false,
secretOrKey: jwtSecret,
secretOrKeyProvider: makeSecretOrKeyProvider(jwtSecret, previousSecret),
audience: 'goodgo-api',
issuer: 'goodgo-platform',
});

View File

@@ -145,4 +145,52 @@ describe('validateEnv', () => {
expect(() => validateEnv()).not.toThrow();
});
describe('optional previous secrets (JWT_SECRET_PREVIOUS / JWT_REFRESH_SECRET_PREVIOUS)', () => {
it('accepts when previous secrets are not set (no rotation in progress)', () => {
setValidJwtSecrets();
delete process.env['JWT_SECRET_PREVIOUS'];
delete process.env['JWT_REFRESH_SECRET_PREVIOUS'];
expect(() => validateEnv()).not.toThrow();
});
it('accepts valid JWT_SECRET_PREVIOUS', () => {
setValidJwtSecrets();
process.env['JWT_SECRET_PREVIOUS'] = 'a-valid-previous-secret-with-at-least-32-characters';
expect(() => validateEnv()).not.toThrow();
});
it('rejects JWT_SECRET_PREVIOUS when it is a placeholder', () => {
setValidJwtSecrets();
process.env['JWT_SECRET_PREVIOUS'] = 'CHANGE_ME';
expect(() => validateEnv()).toThrow('Insecure JWT secret configuration');
expect(() => validateEnv()).toThrow('JWT_SECRET_PREVIOUS');
});
it('rejects JWT_SECRET_PREVIOUS when it is too short', () => {
setValidJwtSecrets();
process.env['JWT_SECRET_PREVIOUS'] = 'short';
expect(() => validateEnv()).toThrow('too short');
});
it('rejects JWT_REFRESH_SECRET_PREVIOUS when it is a placeholder', () => {
setValidJwtSecrets();
process.env['JWT_REFRESH_SECRET_PREVIOUS'] = 'password';
expect(() => validateEnv()).toThrow('Insecure JWT secret configuration');
expect(() => validateEnv()).toThrow('JWT_REFRESH_SECRET_PREVIOUS');
});
it('accepts both previous secrets when valid', () => {
setValidJwtSecrets();
process.env['JWT_SECRET_PREVIOUS'] = 'previous-access-secret-at-least-32-chars-long!!';
process.env['JWT_REFRESH_SECRET_PREVIOUS'] = 'previous-refresh-secret-at-least-32-chars-long!!';
expect(() => validateEnv()).not.toThrow();
});
});
});

View File

@@ -11,6 +11,18 @@ const ALWAYS_REQUIRED: readonly string[] = [
'JWT_REFRESH_SECRET',
];
/**
* Optional previous-generation JWT secrets used during key rotation.
* When set, the auth layer will accept tokens signed with either the
* primary or the previous key, enabling zero-downtime secret rotation.
* Subject to the same placeholder-rejection and minimum-length checks
* as the primary secrets.
*/
const OPTIONAL_PREVIOUS_SECRETS: readonly string[] = [
'JWT_SECRET_PREVIOUS',
'JWT_REFRESH_SECRET_PREVIOUS',
];
const REQUIRED_IN_PRODUCTION: readonly string[] = [
'DATABASE_URL',
'CORS_ORIGINS',
@@ -120,6 +132,19 @@ export function validateEnv(): void {
}
}
// Optional previous-generation secrets — if set, they must also pass
// the same strength checks. An empty/unset value is fine (no rotation
// in progress), but a weak value is always rejected.
for (const key of OPTIONAL_PREVIOUS_SECRETS) {
const value = process.env[key];
if (value) {
const error = validateJwtSecret(key, value);
if (error) {
secretErrors.push(error);
}
}
}
if (secretErrors.length > 0) {
throw new Error(
`Insecure JWT secret configuration:\n ${secretErrors.join('\n ')}\n` +