From b478dc2898f1e2328aad0213b6a06b1b937492be Mon Sep 17 00:00:00 2001 From: Blaqkenny Date: Sun, 30 Aug 2026 17:09:18 +0000 Subject: [PATCH] fix(security): remove fallback JWT secrets MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Replace the hard-coded `|| 'supersecret'` / `|| 'your-secret-key'` fallbacks in the admin module, admin JWT strategy and user JWT strategy with required configuration resolution (`ConfigService.getOrThrow`), so startup fails when JWT_SECRET is absent instead of silently using a predictable secret. Adds tests proving the strategies cannot be constructed without a configured secret and never fall back to a predictable default. Closes #250 🤖 Generated with Codebuff Co-Authored-By: Codebuff --- backend/src/admin/admin.module.ts | 13 +++- .../src/admin/strategies/jwt.strategy.spec.ts | 62 +++++++++++++++++++ backend/src/admin/strategies/jwt.strategy.ts | 9 ++- .../src/auth/strategies/jwt.strategy.spec.ts | 62 +++++++++++++++++++ backend/src/auth/strategies/jwt.strategy.ts | 3 +- 5 files changed, 143 insertions(+), 6 deletions(-) create mode 100644 backend/src/admin/strategies/jwt.strategy.spec.ts create mode 100644 backend/src/auth/strategies/jwt.strategy.spec.ts diff --git a/backend/src/admin/admin.module.ts b/backend/src/admin/admin.module.ts index 27c4d49a..d66465f7 100644 --- a/backend/src/admin/admin.module.ts +++ b/backend/src/admin/admin.module.ts @@ -1,6 +1,7 @@ import { Module } from '@nestjs/common'; import { TypeOrmModule } from '@nestjs/typeorm'; import { JwtModule } from '@nestjs/jwt'; +import { ConfigModule, ConfigService } from '@nestjs/config'; import { Admin } from './admin.entity'; import { AdminService } from './admin.service'; import { AdminController } from './admin.controller'; @@ -14,9 +15,15 @@ import { AdminRole } from './admin-role.enum'; @Module({ imports: [ TypeOrmModule.forFeature([Admin]), - JwtModule.register({ - secret: process.env.JWT_SECRET || 'supersecret', - signOptions: { expiresIn: '1d' }, + // No fallback secret: JWT_SECRET is required by the global config + // validation (see AppModule), so startup fails if it is absent. + JwtModule.registerAsync({ + imports: [ConfigModule], + inject: [ConfigService], + useFactory: (configService: ConfigService) => ({ + secret: configService.getOrThrow('JWT_SECRET'), + signOptions: { expiresIn: '1d' }, + }), }), ], controllers: [AdminController], diff --git a/backend/src/admin/strategies/jwt.strategy.spec.ts b/backend/src/admin/strategies/jwt.strategy.spec.ts new file mode 100644 index 00000000..1b91e548 --- /dev/null +++ b/backend/src/admin/strategies/jwt.strategy.spec.ts @@ -0,0 +1,62 @@ +import { ConfigService } from '@nestjs/config'; +import { JwtStrategy } from './jwt.strategy'; +import { AdminService } from '../admin.service'; + +/** + * Production startup must never be able to use a predictable JWT secret: + * the strategy reads JWT_SECRET through ConfigService.getOrThrow, so + * constructing it without a configured secret must fail immediately. + */ +describe('Admin JwtStrategy (no fallback secret)', () => { + const adminServiceMock = { + findByEmail: jest.fn(), + } as unknown as AdminService; + + function makeConfig(secret?: string): ConfigService { + return { + getOrThrow: jest.fn((key: string) => { + if (key === 'JWT_SECRET' && secret !== undefined) { + return secret; + } + throw new Error('Config variable "JWT_SECRET" is required'); + }), + } as unknown as ConfigService; + } + + it('fails to construct when JWT_SECRET is absent', () => { + expect(() => new JwtStrategy(adminServiceMock, makeConfig())).toThrow( + /JWT_SECRET/, + ); + }); + + it('constructs when JWT_SECRET is configured', () => { + expect( + () => new JwtStrategy(adminServiceMock, makeConfig('a-real-secret')), + ).not.toThrow(); + }); + + it('uses the configured secret, never a hard-coded default', async () => { + const strategy = new JwtStrategy( + adminServiceMock, + makeConfig('configured-secret-value'), + ); + // passport-jwt resolves the secret through its internal provider; + // assert it is exactly what configuration provided — never + // 'supersecret' or another predictable default. + const resolved = await new Promise((resolve) => { + (strategy as unknown as { + _secretOrKeyProvider: ( + _req: unknown, + _token: string, + done: (err: unknown, secret: string) => void, + ) => void; + })._secretOrKeyProvider({}, 'token', (_err, secret) => resolve(secret)); + }); + expect(resolved).toBe('configured-secret-value'); + expect(resolved).not.toBe('supersecret'); + }); + + it('rejects an empty JWT_SECRET as if it were missing', () => { + expect(() => new JwtStrategy(adminServiceMock, makeConfig(''))).toThrow(); + }); +}); diff --git a/backend/src/admin/strategies/jwt.strategy.ts b/backend/src/admin/strategies/jwt.strategy.ts index df245186..4c2cf0e6 100644 --- a/backend/src/admin/strategies/jwt.strategy.ts +++ b/backend/src/admin/strategies/jwt.strategy.ts @@ -1,15 +1,20 @@ import { Injectable } from '@nestjs/common'; import { PassportStrategy } from '@nestjs/passport'; import { ExtractJwt, Strategy } from 'passport-jwt'; +import { ConfigService } from '@nestjs/config'; import { AdminService } from '../admin.service'; @Injectable() export class JwtStrategy extends PassportStrategy(Strategy, 'admin-jwt') { - constructor(private adminService: AdminService) { + constructor( + private adminService: AdminService, + configService: ConfigService, + ) { super({ jwtFromRequest: ExtractJwt.fromAuthHeaderAsBearerToken(), ignoreExpiration: false, - secretOrKey: process.env.JWT_SECRET || 'supersecret', + // No fallback secret: fail fast when JWT_SECRET is not configured. + secretOrKey: configService.getOrThrow('JWT_SECRET'), }); } diff --git a/backend/src/auth/strategies/jwt.strategy.spec.ts b/backend/src/auth/strategies/jwt.strategy.spec.ts new file mode 100644 index 00000000..8493fb2f --- /dev/null +++ b/backend/src/auth/strategies/jwt.strategy.spec.ts @@ -0,0 +1,62 @@ +import { ConfigService } from '@nestjs/config'; +import { JwtStrategy } from './jwt.strategy'; +import { AuthService } from '../services/auth.service'; + +/** + * The user JWT strategy must never silently fall back to a predictable + * secret: it resolves JWT_SECRET via ConfigService.getOrThrow, so building + * it without a configured secret fails at startup instead. + */ +describe('User JwtStrategy (no fallback secret)', () => { + const authServiceMock = { + validateUser: jest.fn(), + } as unknown as AuthService; + + function makeConfig(secret?: string): ConfigService { + return { + getOrThrow: jest.fn((key: string) => { + if (key === 'JWT_SECRET' && secret !== undefined) { + return secret; + } + throw new Error('Config variable "JWT_SECRET" is required'); + }), + } as unknown as ConfigService; + } + + it('fails to construct when JWT_SECRET is absent', () => { + expect(() => new JwtStrategy(authServiceMock, makeConfig())).toThrow( + /JWT_SECRET/, + ); + }); + + it('constructs when JWT_SECRET is configured', () => { + expect( + () => new JwtStrategy(authServiceMock, makeConfig('a-real-secret')), + ).not.toThrow(); + }); + + it('uses the configured secret, never a hard-coded default', async () => { + const strategy = new JwtStrategy( + authServiceMock, + makeConfig('configured-secret-value'), + ); + // passport-jwt resolves the secret through its internal provider; + // assert it is exactly what configuration provided — never + // 'your-secret-key' or another predictable default. + const resolved = await new Promise((resolve) => { + (strategy as unknown as { + _secretOrKeyProvider: ( + _req: unknown, + _token: string, + done: (err: unknown, secret: string) => void, + ) => void; + })._secretOrKeyProvider({}, 'token', (_err, secret) => resolve(secret)); + }); + expect(resolved).toBe('configured-secret-value'); + expect(resolved).not.toBe('your-secret-key'); + }); + + it('rejects an empty JWT_SECRET as if it were missing', () => { + expect(() => new JwtStrategy(authServiceMock, makeConfig(''))).toThrow(); + }); +}); diff --git a/backend/src/auth/strategies/jwt.strategy.ts b/backend/src/auth/strategies/jwt.strategy.ts index 65554ed4..99c62a9f 100644 --- a/backend/src/auth/strategies/jwt.strategy.ts +++ b/backend/src/auth/strategies/jwt.strategy.ts @@ -14,7 +14,8 @@ export class JwtStrategy extends PassportStrategy(Strategy) { super({ jwtFromRequest: ExtractJwt.fromAuthHeaderAsBearerToken(), ignoreExpiration: false, - secretOrKey: configService.get('JWT_SECRET') || 'your-secret-key', + // No fallback secret: fail fast when JWT_SECRET is not configured. + secretOrKey: configService.getOrThrow('JWT_SECRET'), }); }