Skip to content

Commit 90ecbc9

Browse files
committed
fix: resolve security issues #800 #803 #804 #806
#800 - EncryptionService: replace SHA-256 with scrypt KDF - Use crypto.scryptSync(secret, salt, 32) for AES-256 key derivation - Require ENCRYPTION_SALT env var at startup (fails fast if missing) - Add ENCRYPTION_SALT to .env.example with generation instructions - Add unit tests verifying scrypt is used (not SHA-256) #803 - AuthService logout: blacklist the current access token - Add jti claim to access token payload in generateTokens() - Update logout(userId, accessToken?) to decode and blacklist the JTI with TTL equal to remaining token lifetime - Update AuthController.logout to extract Bearer token and pass it - Add tests covering JTI blacklisting and graceful fallback #804 - GDPR erasure: cascade-delete financial records and active sessions - Wrap erasure in a TypeORM DataSource transaction - Anonymize payments, enrollments, audit_logs, notifications inside tx - Revoke active sessions before transaction (fast path) - Update GdprModule to import TypeOrmModule (provides DataSource) - Add tests: cascade anonymization, NotFoundException, idempotency #806 - FraudDetectionService: configurable thresholds + behavioral signals - Introduce FraudSignalProvider interface for pluggable signal sources - IpRateSignalProvider, NewDeviceSignalProvider, LargeTransactionSignalProvider - VelocitySignalProvider: flag users exceeding N purchases/hour - GeoAnomalySignalProvider: flag purchase country != registration country - All thresholds configurable via ConfigService env vars (FRAUD_*) - Unit tests for each provider independently + aggregate service Closes #800, Closes #803, Closes #804, Closes #806
1 parent 07bf990 commit 90ecbc9

11 files changed

Lines changed: 518 additions & 59 deletions

.env.example

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -106,9 +106,13 @@ JWT_REFRESH_SECRET=your-super-secret-refresh-key-change-this-in-production
106106
# Refresh token expiration time
107107
JWT_REFRESH_EXPIRES_IN=7d
108108

109-
# Encryption secret [REQUIRED, exactly 32 characters]
109+
# Encryption secret [REQUIRED, min 32 characters]
110110
ENCRYPTION_SECRET=your-super-secret-32-char-encryption-key-change-this
111111

112+
# Encryption salt for scrypt KDF [REQUIRED, randomly generated, store separately from secret]
113+
# Generate with: node -e "console.log(require('crypto').randomBytes(32).toString('hex'))"
114+
ENCRYPTION_SALT=your-random-hex-salt-change-this-in-production
115+
112116
# Bcrypt password hashing rounds (4-15, default: 10, production: 12)
113117
BCRYPT_ROUNDS=10
114118

src/auth/auth.controller.ts

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -67,7 +67,9 @@ export class AuthController {
6767
@HttpCode(HttpStatus.OK)
6868
@ApiOperation({ summary: 'Log out and invalidate refresh token' })
6969
async logout(@Req() req: any) {
70-
await this.authService.logout(req.user.id);
70+
const authHeader: string | undefined = req.headers?.authorization;
71+
const accessToken = authHeader?.startsWith('Bearer ') ? authHeader.slice(7) : undefined;
72+
await this.authService.logout(req.user.id, accessToken);
7173
return { message: 'Logged out successfully' };
7274
}
7375
}

src/auth/auth.service.spec.ts

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@ const mockUserRepo = {
3434
const mockJwtService = {
3535
signAsync: jest.fn(),
3636
verify: jest.fn(),
37+
decode: jest.fn(),
3738
};
3839

3940
const mockBlacklistService = {
@@ -92,6 +93,32 @@ describe('AuthService', () => {
9293

9394
expect(mockUserRepo.update).toHaveBeenCalledWith('user-1', { refreshToken: null });
9495
});
96+
97+
it('blacklists the access token JTI when a valid access token is provided', async () => {
98+
const jti = 'access-jti-xyz';
99+
const exp = Math.floor(Date.now() / 1000) + 900; // 15 min from now
100+
mockJwtService.decode = jest.fn().mockReturnValue({ jti, exp });
101+
mockBlacklistService.addToBlacklist.mockResolvedValue(undefined);
102+
mockUserRepo.update.mockResolvedValue(undefined);
103+
104+
await service.logout('user-1', 'fake.access.token');
105+
106+
expect(mockBlacklistService.addToBlacklist).toHaveBeenCalledWith(
107+
jti,
108+
expect.any(Number),
109+
);
110+
expect(mockUserRepo.update).toHaveBeenCalledWith('user-1', { refreshToken: null });
111+
});
112+
113+
it('still revokes refresh token when access token has no jti', async () => {
114+
mockJwtService.decode = jest.fn().mockReturnValue({ sub: 'user-1' });
115+
mockUserRepo.update.mockResolvedValue(undefined);
116+
117+
await service.logout('user-1', 'token.without.jti');
118+
119+
expect(mockBlacklistService.addToBlacklist).not.toHaveBeenCalled();
120+
expect(mockUserRepo.update).toHaveBeenCalledWith('user-1', { refreshToken: null });
121+
});
95122
});
96123

97124
describe('refreshTokens', () => {

src/auth/auth.service.ts

Lines changed: 22 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -91,7 +91,20 @@ export class AuthService {
9191
}
9292
}
9393

94-
async logout(userId: string) {
94+
async logout(userId: string, accessToken?: string) {
95+
if (accessToken) {
96+
try {
97+
const decoded = this.jwtService.decode(accessToken) as any;
98+
if (decoded?.jti) {
99+
const remainingMs = decoded.exp * 1000 - Date.now();
100+
if (remainingMs > 0) {
101+
await this.tokenBlacklistService.addToBlacklist(decoded.jti, remainingMs);
102+
}
103+
}
104+
} catch {
105+
// malformed token — still revoke refresh token below
106+
}
107+
}
95108
await this.revokeUserTokens(userId);
96109
}
97110

@@ -107,13 +120,17 @@ export class AuthService {
107120

108121
private async generateTokens(user: User) {
109122
const payload = { sub: user.id, email: user.email, role: user.role };
123+
const accessJti = uuidv4();
110124
const refreshJti = uuidv4();
111125

112126
const [accessToken, refreshToken] = await Promise.all([
113-
this.jwtService.signAsync(payload, {
114-
secret: process.env.JWT_SECRET || 'default-jwt-secret',
115-
expiresIn: (process.env.JWT_EXPIRES_IN || '15m') as any,
116-
}),
127+
this.jwtService.signAsync(
128+
{ ...payload, jti: accessJti },
129+
{
130+
secret: process.env.JWT_SECRET || 'default-jwt-secret',
131+
expiresIn: (process.env.JWT_EXPIRES_IN || '15m') as any,
132+
},
133+
),
117134
this.jwtService.signAsync(
118135
{ ...payload, jti: refreshJti },
119136
{

src/modules/gdpr/gdpr.module.ts

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,12 @@
11
import { Module } from '@nestjs/common';
2+
import { TypeOrmModule } from '@nestjs/typeorm';
23
import { SessionModule } from '../../session/session.module';
4+
import { UserConsent } from './entities/user-consent.entity';
5+
import { GdprService } from './gdpr.service';
6+
import { GdprController } from './gdpr.controller';
37

48
@Module({
5-
imports: [SessionModule],
9+
imports: [SessionModule, TypeOrmModule.forFeature([UserConsent])],
610
controllers: [GdprController],
711
providers: [GdprService],
812
})

src/modules/gdpr/gdpr.service.ts

Lines changed: 55 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
import { Inject, Injectable, NotFoundException } from '@nestjs/common';
22
import { InjectRepository } from '@nestjs/typeorm';
3-
import { Repository } from 'typeorm';
3+
import { InjectDataSource } from '@nestjs/typeorm';
4+
import { Repository, DataSource } from 'typeorm';
45
import { plainToInstance, instanceToPlain } from 'class-transformer';
56
import { UserConsent } from './entities/user-consent.entity';
67
import { ConsentDto } from './dto/consent.dto';
@@ -20,6 +21,9 @@ export class GdprService {
2021
private readonly consentRepository: Repository<UserConsent>,
2122

2223
private readonly sessionService: SessionService,
24+
25+
@InjectDataSource()
26+
private readonly dataSource: DataSource,
2327
) {}
2428

2529
async exportUserData(userId: string) {
@@ -53,23 +57,62 @@ export class GdprService {
5357
throw new NotFoundException('User not found');
5458
}
5559

60+
// Revoke all active sessions immediately (outside transaction — fast path)
5661
await this.sessionService.deleteAllSessionsForUser(userId);
5762

58-
await this.usersService.update(userId, {
59-
email: null,
60-
firstName: '[DELETED]',
61-
lastName: '[DELETED]',
62-
phone: null,
63-
address: null,
64-
deletedAt: new Date(),
65-
refreshToken: null,
63+
await this.dataSource.transaction(async (manager) => {
64+
// Anonymize payments
65+
await manager
66+
.createQueryBuilder()
67+
.update('payments')
68+
.set({ userId: null, metadata: null } as any)
69+
.where('user_id = :userId', { userId })
70+
.execute();
71+
72+
// Anonymize enrollments — soft-delete so course analytics remain intact
73+
await manager
74+
.createQueryBuilder()
75+
.update('enrollment')
76+
.set({ deletedAt: new Date() } as any)
77+
.where('user_id = :userId AND deleted_at IS NULL', { userId })
78+
.execute();
79+
80+
// Anonymize audit logs (null out PII fields, keep the log entry for compliance)
81+
await manager
82+
.createQueryBuilder()
83+
.update('audit_logs')
84+
.set({ userId: null, userEmail: null, ipAddress: null } as any)
85+
.where('user_id = :userId', { userId })
86+
.execute();
87+
88+
// Soft-delete notifications
89+
await manager
90+
.createQueryBuilder()
91+
.update('notifications')
92+
.set({ deletedAt: new Date() } as any)
93+
.where('userId = :userId AND deleted_at IS NULL', { userId })
94+
.execute();
95+
96+
// Null out user profile PII
97+
await manager
98+
.createQueryBuilder()
99+
.update('users')
100+
.set({
101+
email: null,
102+
firstName: '[DELETED]',
103+
lastName: '[DELETED]',
104+
phone: null,
105+
address: null,
106+
refreshToken: null,
107+
deletedAt: new Date(),
108+
} as any)
109+
.where('id = :userId', { userId })
110+
.execute();
66111
});
67112

68113
await this.auditService.log('GDPR_ERASURE', userId);
69114

70-
return {
71-
success: true,
72-
};
115+
return { success: true };
73116
}
74117

75118
async updateConsent(userId: string, dto: ConsentDto) {

src/modules/gdpr/tests/gdpr.service.spec.ts

Lines changed: 40 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
import { Test, TestingModule } from '@nestjs/testing';
2-
import { getRepositoryToken } from '@nestjs/typeorm';
2+
import { getRepositoryToken, getDataSourceToken } from '@nestjs/typeorm';
3+
import { NotFoundException } from '@nestjs/common';
34
import { GdprService } from '../gdpr.service';
45
import { UserConsent } from '../entities/user-consent.entity';
56
import { SessionService } from '../../../session/session.service';
@@ -33,17 +34,37 @@ const mockConsentRepository = {
3334
save: jest.fn((consent) => Promise.resolve(consent)),
3435
};
3536

37+
// QueryBuilder mock reused across table updates
38+
function makeQb() {
39+
const qb: any = {
40+
update: jest.fn().mockReturnThis(),
41+
set: jest.fn().mockReturnThis(),
42+
where: jest.fn().mockReturnThis(),
43+
execute: jest.fn().mockResolvedValue(undefined),
44+
};
45+
return qb;
46+
}
47+
48+
const mockDataSource = {
49+
transaction: jest.fn((cb: (manager: any) => Promise<any>) => {
50+
const manager = { createQueryBuilder: jest.fn(() => makeQb()) };
51+
return cb(manager);
52+
}),
53+
};
54+
3655
describe('GdprService', () => {
3756
let service: GdprService;
3857

3958
beforeEach(async () => {
59+
jest.clearAllMocks();
4060
const module: TestingModule = await Test.createTestingModule({
4161
providers: [
4262
GdprService,
4363
{ provide: 'UsersService', useValue: mockUsersService },
4464
{ provide: 'AuditService', useValue: mockAuditService },
4565
{ provide: SessionService, useValue: mockSessionService },
4666
{ provide: getRepositoryToken(UserConsent), useValue: mockConsentRepository },
67+
{ provide: getDataSourceToken(), useValue: mockDataSource },
4768
],
4869
}).compile();
4970

@@ -54,37 +75,40 @@ describe('GdprService', () => {
5475
const result = await service.exportUserData('user-1');
5576
expect(result.profile).toBeDefined();
5677

57-
// Check that sensitive fields are explicitly excluded
5878
expect(result.profile.password).toBeUndefined();
5979
expect(result.profile.refreshToken).toBeUndefined();
6080
expect(result.profile.passwordHistory).toBeUndefined();
6181
expect(result.profile.totpSecret).toBeUndefined();
6282
expect(result.profile.token).toBeUndefined();
6383

64-
// Check that PII fields are preserved
6584
expect(result.profile.id).toBe('user-1');
6685
expect(result.profile.email).toBe('test@test.com');
6786
expect(result.profile.firstName).toBe('John');
6887
expect(result.profile.lastName).toBe('Doe');
6988
});
7089

71-
it('erases user data and invalidates sessions', async () => {
90+
it('erases user data: revokes sessions and runs transactional cascade anonymization', async () => {
7291
const result = await service.eraseUserData('user-1');
7392

7493
expect(result.success).toBe(true);
94+
// Sessions revoked before transaction
7595
expect(mockSessionService.deleteAllSessionsForUser).toHaveBeenCalledWith('user-1');
76-
expect(mockUsersService.update).toHaveBeenCalledWith(
77-
'user-1',
78-
expect.objectContaining({
79-
email: null,
80-
firstName: '[DELETED]',
81-
lastName: '[DELETED]',
82-
phone: null,
83-
address: null,
84-
deletedAt: expect.any(Date),
85-
refreshToken: null,
86-
}),
87-
);
96+
// Transaction executed
97+
expect(mockDataSource.transaction).toHaveBeenCalled();
98+
// Audit log written
99+
expect(mockAuditService.log).toHaveBeenCalledWith('GDPR_ERASURE', 'user-1');
100+
});
101+
102+
it('throws NotFoundException when user does not exist', async () => {
103+
mockUsersService.findById.mockResolvedValueOnce(null);
104+
await expect(service.eraseUserData('missing-user')).rejects.toThrow(NotFoundException);
105+
});
106+
107+
it('is idempotent: second erasure call succeeds even when user is already deleted', async () => {
108+
// First call succeeds normally
109+
await service.eraseUserData('user-1');
110+
// Second call: findById still returns something (soft-deleted row)
111+
await expect(service.eraseUserData('user-1')).resolves.toEqual({ success: true });
88112
});
89113

90114
it('stores consent changes', async () => {
Lines changed: 65 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,65 @@
1+
import * as crypto from 'crypto';
2+
import { EncryptionService } from './encryption.service';
3+
4+
const ENCRYPTION_SECRET = 'test-secret-for-scrypt-kdf';
5+
const ENCRYPTION_SALT = 'test-salt-hex';
6+
7+
function makeService(): EncryptionService {
8+
process.env.ENCRYPTION_SECRET = ENCRYPTION_SECRET;
9+
process.env.ENCRYPTION_SALT = ENCRYPTION_SALT;
10+
return new EncryptionService();
11+
}
12+
13+
describe('EncryptionService', () => {
14+
afterEach(() => {
15+
delete process.env.ENCRYPTION_SECRET;
16+
delete process.env.ENCRYPTION_SALT;
17+
});
18+
19+
it('should be defined', () => {
20+
expect(makeService()).toBeDefined();
21+
});
22+
23+
it('throws when ENCRYPTION_SECRET is missing', () => {
24+
delete process.env.ENCRYPTION_SECRET;
25+
process.env.ENCRYPTION_SALT = ENCRYPTION_SALT;
26+
expect(() => new EncryptionService()).toThrow('ENCRYPTION_SECRET');
27+
});
28+
29+
it('throws when ENCRYPTION_SALT is missing', () => {
30+
process.env.ENCRYPTION_SECRET = ENCRYPTION_SECRET;
31+
delete process.env.ENCRYPTION_SALT;
32+
expect(() => new EncryptionService()).toThrow('ENCRYPTION_SALT');
33+
});
34+
35+
it('derives key using scrypt, not a plain SHA-256 hash', () => {
36+
const service = makeService();
37+
// Access private key via casting
38+
const derivedKey = (service as any).key as Buffer;
39+
40+
const sha256Key = crypto.createHash('sha256').update(ENCRYPTION_SECRET).digest();
41+
const scryptKey = crypto.scryptSync(ENCRYPTION_SECRET, ENCRYPTION_SALT, 32, {
42+
N: 16384,
43+
r: 8,
44+
p: 1,
45+
});
46+
47+
expect(derivedKey).toEqual(scryptKey);
48+
expect(derivedKey).not.toEqual(sha256Key);
49+
});
50+
51+
it('encrypts and decrypts round-trip correctly', () => {
52+
const service = makeService();
53+
const plaintext = 'sensitive data';
54+
const payload = service.encrypt(plaintext);
55+
expect(service.decrypt(payload)).toBe(plaintext);
56+
});
57+
58+
it('produces different ciphertext for the same plaintext (random IV)', () => {
59+
const service = makeService();
60+
const a = service.encrypt('same text');
61+
const b = service.encrypt('same text');
62+
expect(a.iv).not.toBe(b.iv);
63+
expect(a.content).not.toBe(b.content);
64+
});
65+
});

0 commit comments

Comments
 (0)