Skip to content

Commit 95e6679

Browse files
abdoolyaroabdoolyaro
authored
fix: remove hardcoded JWT secret defaults, enforce min length (#892) (#995)
- Remove weak fallback defaults from AuthService constructor - Throw on startup if JWT_SECRET/JWT_REFRESH_SECRET missing or under 32 chars (256 bits) - Add length validation to validate-env.ts alongside existing presence check - Update .env.example placeholders to meet the new minimum and document it - Bump mocked JWT secrets in auth.service.captcha.spec.ts, auth-refresh-token-reuse.spec.ts, password-reset-token-validation.spec.ts, rate-limit-burst.e2e.spec.ts, and fraud-auto-block.e2e.spec.ts to 32+ chars (all were breaking under the new validation) Closes #892
1 parent 9f816d9 commit 95e6679

8 files changed

Lines changed: 149 additions & 68 deletions

‎.env.example‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,8 +5,11 @@ NODE_ENV=development
55
FRONTEND_URL=http://localhost:3000
66

77
# JWT Configuration
8-
JWT_SECRET=your-super-secret-jwt-key-change-in-production
9-
JWT_REFRESH_SECRET=your-super-secret-refresh-key-change-in-production
8+
# Required. Must be at least 32 characters (256 bits). The app will refuse to
9+
# start if either secret is missing or too short. Generate strong values with:
10+
# openssl rand -hex 32
11+
JWT_SECRET=your-super-secret-jwt-key-change-in-production-min-32-chars
12+
JWT_REFRESH_SECRET=your-super-secret-refresh-key-change-in-production-min-32c
1013
JWT_ACCESS_EXPIRES_IN=15m
1114
JWT_REFRESH_EXPIRES_IN=7d
1215

‎src/auth/auth.service.captcha.spec.ts‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -50,8 +50,8 @@ describe('AuthService – CAPTCHA failure lockout', () => {
5050
const configService = {
5151
get: jest.fn((key: string) => {
5252
const config: Record<string, string> = {
53-
JWT_SECRET: 'test-secret',
54-
JWT_REFRESH_SECRET: 'test-refresh-secret',
53+
JWT_SECRET: 'test-secret-at-least-32-characters-long',
54+
JWT_REFRESH_SECRET: 'test-refresh-secret-at-least-32-characters-long',
5555
JWT_ACCESS_EXPIRES_IN: '15m',
5656
JWT_REFRESH_EXPIRES_IN: '7d',
5757
BCRYPT_ROUNDS: '10',
@@ -87,8 +87,8 @@ describe('AuthService – CAPTCHA failure lockout', () => {
8787
const captchaConfig: Record<string, string> = {
8888
RECAPTCHA_SECRET: 'some-secret',
8989
CAPTCHA_THRESHOLD: '3',
90-
JWT_SECRET: 'test-secret',
91-
JWT_REFRESH_SECRET: 'test-refresh-secret',
90+
JWT_SECRET: 'test-secret-at-least-32-characters-long',
91+
JWT_REFRESH_SECRET: 'test-refresh-secret-at-least-32-characters-long',
9292
JWT_ACCESS_EXPIRES_IN: '15m',
9393
JWT_REFRESH_EXPIRES_IN: '7d',
9494
BCRYPT_ROUNDS: '10',

‎src/auth/auth.service.ts‎

Lines changed: 17 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,8 @@ import { UserRole } from '../types/prisma.types';
5151
import { FraudService } from '../fraud/fraud.service';
5252
import { ApiKeyAnalyticsService } from './api-key-analytics.service';
5353

54+
const MIN_JWT_SECRET_LENGTH = 32;
55+
5456
type JwtPayload = {
5557
sub: string;
5658
email: string;
@@ -92,9 +94,21 @@ export class AuthService {
9294
private readonly fraudService: FraudService,
9395
@Optional() private readonly apiKeyAnalyticsService?: ApiKeyAnalyticsService,
9496
) {
95-
this.jwtSecret = this.configService.get<string>('JWT_SECRET') ?? 'propchain-access-secret';
96-
this.jwtRefreshSecret =
97-
this.configService.get<string>('JWT_REFRESH_SECRET') ?? 'propchain-refresh-secret';
97+
const jwtSecret = this.configService.get<string>('JWT_SECRET');
98+
if (!jwtSecret || jwtSecret.length < MIN_JWT_SECRET_LENGTH) {
99+
throw new Error(
100+
`JWT_SECRET must be set and at least ${MIN_JWT_SECRET_LENGTH} characters (256 bits) long`,
101+
);
102+
}
103+
this.jwtSecret = jwtSecret;
104+
105+
const jwtRefreshSecret = this.configService.get<string>('JWT_REFRESH_SECRET');
106+
if (!jwtRefreshSecret || jwtRefreshSecret.length < MIN_JWT_SECRET_LENGTH) {
107+
throw new Error(
108+
`JWT_REFRESH_SECRET must be set and at least ${MIN_JWT_SECRET_LENGTH} characters (256 bits) long`,
109+
);
110+
}
111+
this.jwtRefreshSecret = jwtRefreshSecret;
98112
this.accessTokenTtlSeconds = parseDuration(
99113
this.configService.get<string>('JWT_ACCESS_EXPIRES_IN') ?? '15m',
100114
15 * 60,

‎src/utils/validate-env.ts‎

Lines changed: 25 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,18 +1,40 @@
11
const REQUIRED_ENV_VARS = ['DATABASE_URL', 'JWT_SECRET', 'JWT_REFRESH_SECRET'] as const;
2+
const JWT_SECRET_VARS = ['JWT_SECRET', 'JWT_REFRESH_SECRET'] as const;
3+
const MIN_JWT_SECRET_LENGTH = 32;
24

35
export function validateEnvironment(): void {
46
const MISSING: string[] = [];
7+
const WEAK: string[] = [];
58

69
for (const key of REQUIRED_ENV_VARS) {
710
if (!process.env[key]) {
811
MISSING.push(key);
912
}
1013
}
1114

12-
if (MISSING.length > 0) {
15+
for (const key of JWT_SECRET_VARS) {
16+
const value = process.env[key];
17+
if (value && value.length < MIN_JWT_SECRET_LENGTH) {
18+
WEAK.push(`${key} (found ${value.length} chars, need at least ${MIN_JWT_SECRET_LENGTH})`);
19+
}
20+
}
21+
22+
if (MISSING.length > 0 || WEAK.length > 0) {
23+
const sections: string[] = [];
24+
if (MISSING.length > 0) {
25+
sections.push(
26+
`Missing required environment variables:\n` + MISSING.map((k) => ` - ${k}`).join('\n'),
27+
);
28+
}
29+
if (WEAK.length > 0) {
30+
sections.push(
31+
`Environment variables below the minimum required length (256 bits / ${MIN_JWT_SECRET_LENGTH} chars):\n` +
32+
WEAK.map((k) => ` - ${k}`).join('\n'),
33+
);
34+
}
1335
console.error(
14-
`\n Fatal: Missing required environment variables:\n` +
15-
MISSING.map((k) => ` - ${k}`).join('\n') +
36+
`\n Fatal:\n ` +
37+
sections.join('\n\n ') +
1638
`\n\n Please set them in .env or .env.local before starting the application.\n`,
1739
);
1840
process.exit(1);

‎test/e2e/fraud-auto-block.e2e.spec.ts‎

Lines changed: 54 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -14,8 +14,8 @@ import { ConfigService } from '@nestjs/config';
1414
import { createSha256, hashPassword } from '../../src/auth/security.utils';
1515
import * as jwt from 'jsonwebtoken';
1616

17-
const ACCESS_SECRET = 'test-access-secret';
18-
const REFRESH_SECRET = 'test-refresh-secret';
17+
const ACCESS_SECRET = 'test-access-secret-at-least-32-characters-long';
18+
const REFRESH_SECRET = 'test-refresh-secret-at-least-32-characters-long';
1919

2020
describe('Fraud alert auto-block e2e', () => {
2121
let app: INestApplication;
@@ -66,12 +66,14 @@ describe('Fraud alert auto-block e2e', () => {
6666
},
6767
findFirst: async ({ where }: any) => {
6868
if (!where) return null;
69-
return Array.from(users.values()).find((u) => {
70-
for (const k of Object.keys(where)) {
71-
if (u[k] !== where[k]) return false;
72-
}
73-
return true;
74-
}) ?? null;
69+
return (
70+
Array.from(users.values()).find((u) => {
71+
for (const k of Object.keys(where)) {
72+
if (u[k] !== where[k]) return false;
73+
}
74+
return true;
75+
}) ?? null
76+
);
7577
},
7678
update: async ({ where, data }: any) => {
7779
const user = users.get(where.id);
@@ -112,7 +114,10 @@ describe('Fraud alert auto-block e2e', () => {
112114
},
113115
update: async ({ where, data }: any) => {
114116
const existing = blacklistedTokens.get(where.jti);
115-
if (existing) { Object.assign(existing, data); return existing; }
117+
if (existing) {
118+
Object.assign(existing, data);
119+
return existing;
120+
}
116121
return data;
117122
},
118123
count: async () => blacklistedTokens.size,
@@ -132,13 +137,25 @@ describe('Fraud alert auto-block e2e', () => {
132137
findUnique: async ({ where }: any) => fraudAlerts.get(where.id) ?? null,
133138
create: async ({ data }: any) => {
134139
const id = nid();
135-
const record = { id, ...data, occurrenceCount: 1, lastDetectedAt: new Date(), status: 'OPEN', autoBlocked: data.autoBlocked ?? false, createdAt: new Date(), updatedAt: new Date() };
140+
const record = {
141+
id,
142+
...data,
143+
occurrenceCount: 1,
144+
lastDetectedAt: new Date(),
145+
status: 'OPEN',
146+
autoBlocked: data.autoBlocked ?? false,
147+
createdAt: new Date(),
148+
updatedAt: new Date(),
149+
};
136150
fraudAlerts.set(id, record);
137151
return record;
138152
},
139153
update: async ({ where, data }: any) => {
140154
const existing = fraudAlerts.get(where.id);
141-
if (existing) { Object.assign(existing, data); return existing; }
155+
if (existing) {
156+
Object.assign(existing, data);
157+
return existing;
158+
}
142159
return data;
143160
},
144161
findMany: async () => Array.from(fraudAlerts.values()),
@@ -212,7 +229,8 @@ describe('Fraud alert auto-block e2e', () => {
212229
if (where?.OR) {
213230
match = false;
214231
for (const cond of where.OR) {
215-
if (cond.refreshTokenJti && s.refreshTokenJti === cond.refreshTokenJti) match = true;
232+
if (cond.refreshTokenJti && s.refreshTokenJti === cond.refreshTokenJti)
233+
match = true;
216234
if (cond.accessTokenJti && s.accessTokenJti === cond.accessTokenJti) match = true;
217235
}
218236
}
@@ -221,12 +239,17 @@ describe('Fraud alert auto-block e2e', () => {
221239
return null;
222240
},
223241
findMany: async ({ where }: any) => {
224-
return Array.from(sessions.values()).filter((s) => !where?.userId || s.userId === where.userId);
242+
return Array.from(sessions.values()).filter(
243+
(s) => !where?.userId || s.userId === where.userId,
244+
);
225245
},
226246
updateMany: async ({ where, data }: any) => {
227247
let count = 0;
228248
for (const s of sessions.values()) {
229-
if (where?.userId && s.userId === where.userId) { Object.assign(s, data); count++; }
249+
if (where?.userId && s.userId === where.userId) {
250+
Object.assign(s, data);
251+
count++;
252+
}
230253
}
231254
return { count };
232255
},
@@ -312,21 +335,23 @@ describe('Fraud alert auto-block e2e', () => {
312335
{
313336
provide: FraudService,
314337
useValue: {
315-
handleTokenReuse: jest.fn().mockImplementation(async (userId: string, jti: string, ip: string) => {
316-
await prisma.user.update({ where: { id: userId }, data: { isBlocked: true } });
317-
await prisma.fraudAlert.create({
318-
data: {
319-
userId,
320-
pattern: 'TOKEN_REUSE',
321-
severity: 'CRITICAL',
322-
status: 'OPEN',
323-
description: `Token reuse detected for user ${userId}`,
324-
ipAddress: ip,
325-
evidence: { jti },
326-
autoBlocked: true,
327-
},
328-
});
329-
}),
338+
handleTokenReuse: jest
339+
.fn()
340+
.mockImplementation(async (userId: string, jti: string, ip: string) => {
341+
await prisma.user.update({ where: { id: userId }, data: { isBlocked: true } });
342+
await prisma.fraudAlert.create({
343+
data: {
344+
userId,
345+
pattern: 'TOKEN_REUSE',
346+
severity: 'CRITICAL',
347+
status: 'OPEN',
348+
description: `Token reuse detected for user ${userId}`,
349+
ipAddress: ip,
350+
evidence: { jti },
351+
autoBlocked: true,
352+
},
353+
});
354+
}),
330355
evaluateFailedLogin: jest.fn().mockResolvedValue(null),
331356
evaluateSuccessfulLogin: jest.fn().mockResolvedValue([]),
332357
},

‎test/e2e/rate-limit-burst.e2e.spec.ts‎

Lines changed: 16 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -88,12 +88,14 @@ describe('Rate-limit guard e2e – burst traffic', () => {
8888
},
8989
findFirst: async ({ where }: any) => {
9090
if (!where) return null;
91-
return Array.from(users.values()).find((u) => {
92-
for (const k of Object.keys(where)) {
93-
if (u[k] !== where[k]) return false;
94-
}
95-
return true;
96-
}) ?? null;
91+
return (
92+
Array.from(users.values()).find((u) => {
93+
for (const k of Object.keys(where)) {
94+
if (u[k] !== where[k]) return false;
95+
}
96+
return true;
97+
}) ?? null
98+
);
9799
},
98100
update: async ({ where, data }: any) => {
99101
const user = users.get(where.id);
@@ -123,7 +125,12 @@ describe('Rate-limit guard e2e – burst traffic', () => {
123125
fraudAlert: {
124126
findFirst: async () => null,
125127
findUnique: async () => null,
126-
create: async ({ data }: any) => ({ id: nid(), ...data, occurrenceCount: 1, status: 'OPEN' }),
128+
create: async ({ data }: any) => ({
129+
id: nid(),
130+
...data,
131+
occurrenceCount: 1,
132+
status: 'OPEN',
133+
}),
127134
update: async ({ data }: any) => data,
128135
findMany: async () => [],
129136
count: async () => 0,
@@ -216,8 +223,8 @@ describe('Rate-limit guard e2e – burst traffic', () => {
216223
useValue: {
217224
get: (key: string) => {
218225
const cfg: Record<string, string> = {
219-
JWT_SECRET: 'test-access-secret',
220-
JWT_REFRESH_SECRET: 'test-refresh-secret',
226+
JWT_SECRET: 'test-access-secret-at-least-32-characters-long',
227+
JWT_REFRESH_SECRET: 'test-refresh-secret-at-least-32-characters-long',
221228
JWT_ACCESS_EXPIRES_IN: '15m',
222229
JWT_REFRESH_EXPIRES_IN: '7d',
223230
BCRYPT_ROUNDS: '4',

‎test/unit/auth-refresh-token-reuse.spec.ts‎

Lines changed: 21 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -10,8 +10,8 @@ import { LoginRateLimitService } from '../../src/auth/login-rate-limit.service';
1010
import { FraudService } from '../../src/fraud/fraud.service';
1111
import { createSha256 } from '../../src/auth/security.utils';
1212

13-
const ACCESS_SECRET = 'test-access-secret';
14-
const REFRESH_SECRET = 'test-refresh-secret';
13+
const ACCESS_SECRET = 'test-access-secret-at-least-32-characters-long';
14+
const REFRESH_SECRET = 'test-refresh-secret-at-least-32-characters-long';
1515

1616
function signRefresh(payload: Record<string, any>) {
1717
const { exp, ...rest } = payload;
@@ -184,9 +184,9 @@ describe('AuthService.refreshToken – token-reuse attack', () => {
184184
isDeactivated: false,
185185
});
186186

187-
await expect(
188-
service.refreshToken({ refreshToken: token }, '1.2.3.4', 'UA'),
189-
).rejects.toThrow('blocked');
187+
await expect(service.refreshToken({ refreshToken: token }, '1.2.3.4', 'UA')).rejects.toThrow(
188+
'blocked',
189+
);
190190
});
191191

192192
it('rejects refresh if user is deactivated', async () => {
@@ -200,9 +200,9 @@ describe('AuthService.refreshToken – token-reuse attack', () => {
200200
isDeactivated: true,
201201
});
202202

203-
await expect(
204-
service.refreshToken({ refreshToken: token }, '1.2.3.4', 'UA'),
205-
).rejects.toThrow('deactivated');
203+
await expect(service.refreshToken({ refreshToken: token }, '1.2.3.4', 'UA')).rejects.toThrow(
204+
'deactivated',
205+
);
206206
});
207207

208208
it('rejects refresh if user no longer exists', async () => {
@@ -212,21 +212,27 @@ describe('AuthService.refreshToken – token-reuse attack', () => {
212212
mockPrisma.blacklistedToken.findUnique.mockResolvedValue(null);
213213
mockPrisma.user.findUnique.mockResolvedValue(null);
214214

215-
await expect(
216-
service.refreshToken({ refreshToken: token }, '1.2.3.4', 'UA'),
217-
).rejects.toThrow('no longer exists');
215+
await expect(service.refreshToken({ refreshToken: token }, '1.2.3.4', 'UA')).rejects.toThrow(
216+
'no longer exists',
217+
);
218218
});
219219

220220
it('rejects a token that is not a refresh token', async () => {
221-
const accessPayload = { sub: 'user-1', email: 'user@example.com', role: 'USER', type: 'access', jti: 'access-jti' };
221+
const accessPayload = {
222+
sub: 'user-1',
223+
email: 'user@example.com',
224+
role: 'USER',
225+
type: 'access',
226+
jti: 'access-jti',
227+
};
222228
const token = jwt.sign(accessPayload, REFRESH_SECRET, {
223229
expiresIn: '15m',
224230
issuer: 'PropChain',
225231
});
226232

227-
await expect(
228-
service.refreshToken({ refreshToken: token }, '1.2.3.4', 'UA'),
229-
).rejects.toThrow('Invalid refresh token');
233+
await expect(service.refreshToken({ refreshToken: token }, '1.2.3.4', 'UA')).rejects.toThrow(
234+
'Invalid refresh token',
235+
);
230236
});
231237

232238
it('rejects an invalid or expired token', async () => {

‎test/unit/password-reset-token-validation.spec.ts‎

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,11 @@ describe('AuthService.resetPassword – password-reset token validation', () =>
5353
return passwordHistory.filter((h) => h.userId === where.userId);
5454
}),
5555
create: jest.fn(async ({ data }: any) => {
56-
const record = { id: Math.random().toString(36).slice(2, 8), ...data, createdAt: new Date() };
56+
const record = {
57+
id: Math.random().toString(36).slice(2, 8),
58+
...data,
59+
createdAt: new Date(),
60+
};
5761
passwordHistory.push(record);
5862
return record;
5963
}),
@@ -89,8 +93,8 @@ describe('AuthService.resetPassword – password-reset token validation', () =>
8993
const mockConfigService = {
9094
get: jest.fn((key: string) => {
9195
const config: Record<string, string> = {
92-
JWT_SECRET: 'test-access-secret',
93-
JWT_REFRESH_SECRET: 'test-refresh-secret',
96+
JWT_SECRET: 'test-access-secret-at-least-32-characters-long',
97+
JWT_REFRESH_SECRET: 'test-refresh-secret-at-least-32-characters-long',
9498
JWT_ACCESS_EXPIRES_IN: '15m',
9599
JWT_REFRESH_EXPIRES_IN: '7d',
96100
BCRYPT_ROUNDS: '4',

0 commit comments

Comments
 (0)