Skip to content

Commit 8f201e8

Browse files
committed
fix: comprehensive code quality improvements across all packages
Security: - Fix RolesGuard to use singular user.role instead of user.roles - Read emailVerified/isActive from JWT payload instead of hardcoding true - Extract userId from JWT via @currentuser decorator in OTP controller - Add path traversal prevention in template service - Remove dead oldRecord code in audit middleware - Document auth guard requirements on unguarded controllers Data integrity & BullMQ: - Add job cleanup options (removeOnComplete/removeOnFail) to prevent Redis leak - Fix duplicate notification record for in-app channel - Add retry/backoff (3 attempts, exponential 5s) to all queues - Close queue connections on module shutdown - Only mark status='failed' on final retry attempt - Add optional model scoping to soft-delete middleware Feature flag parity: - Add runtime feature guards for forRootAsync controllers (auth, storage, audit-log, notifications) that return 404 when features are disabled Constants & type safety: - Export PRISMA_SERVICE constant, replace 30+ string literals across packages - Add ROLES, VERIFICATION_TOKEN_TYPES, AUTH_PROVIDERS constants (auth) - Add NOTIFICATION_STATUSES, NOTIFICATION_CHANNELS constants (notifications) - Add @isin() DTO validation for role, platform, and channel fields - Change StorageService.validateFile to throw BadRequestException - Replace Promise<any> with concrete types in OrganizationService Code reuse: - Extract BaseCodeOtpProvider with shared verify/hash/generate logic - Extract BaseNotificationProcessor with processWithStatusTracking() - Extract buildRedisConnection() helper in notification module - Extract validateAndConsumeToken() and toAuthenticatedUser() in auth service - Standardize OTP event emission to async with error handling - Deduplicate rate limiting (guard delegates to OtpService.checkRateLimit) Efficiency: - Parallelize getEnabledMethods queries with Promise.all - Fix double getEnabledMethods call in auth login - Make template service use async file reads (fs.promises) - Add bounded template cache (max 100 entries, FIFO eviction) - Add pagination limits to unbounded queries (findAll, findByEntity, sessions) - Add select clauses to register/getProfile/org-member queries - Fix double useFactory in auth module forRootAsync Cleanup: - Remove dead ChangePasswordDto and empty SetupTotpDto - Rename HttpExceptionFilter to AllExceptionsFilter (with backward-compat alias) - Import event interfaces from shared file instead of re-declaring
1 parent 01f9975 commit 8f201e8

80 files changed

Lines changed: 1883 additions & 709 deletions

File tree

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

apps/demo/test/app.e2e-spec.ts

Lines changed: 678 additions & 1 deletion
Large diffs are not rendered by default.

packages/nestjs-audit-log/src/audit-log.controller.spec.ts

Lines changed: 3 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,10 @@
11
import { AuditLogController } from './audit-log.controller';
22
import { AuditLogService } from './audit-log.service';
3-
import { AuditLogModuleOptions } from './interfaces';
43
import { AuditLogQueryDto } from './dto/audit-log-query.dto';
54

65
describe('AuditLogController', () => {
76
let controller: AuditLogController;
87
let mockAuditLogService: jest.Mocked<Pick<AuditLogService, 'findAll' | 'findById' | 'findByEntity'>>;
9-
let mockOptions: AuditLogModuleOptions;
108

119
beforeEach(() => {
1210
mockAuditLogService = {
@@ -21,14 +19,8 @@ describe('AuditLogController', () => {
2119
findByEntity: jest.fn().mockResolvedValue([]),
2220
};
2321

24-
mockOptions = {
25-
features: { registerController: true },
26-
adminRoles: ['ADMIN'],
27-
};
28-
2922
controller = new AuditLogController(
3023
mockAuditLogService as any,
31-
mockOptions,
3224
);
3325
});
3426

@@ -53,7 +45,7 @@ describe('AuditLogController', () => {
5345
action: undefined,
5446
startDate: undefined,
5547
endDate: undefined,
56-
page: 0,
48+
page: 1,
5749
limit: 10,
5850
});
5951
});
@@ -80,7 +72,7 @@ describe('AuditLogController', () => {
8072
action: 'UPDATE',
8173
startDate: new Date(startDate),
8274
endDate: new Date(endDate),
83-
page: 2,
75+
page: 3,
8476
limit: 10,
8577
});
8678
});
@@ -99,7 +91,7 @@ describe('AuditLogController', () => {
9991
action: 'CREATE',
10092
startDate: undefined,
10193
endDate: undefined,
102-
page: 1,
94+
page: 2,
10395
limit: 10,
10496
});
10597
});

packages/nestjs-audit-log/src/audit-log.controller.ts

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ import {
33
Get,
44
Param,
55
Query,
6-
Inject,
6+
UseGuards,
77
} from '@nestjs/common';
88
import {
99
ApiTags,
@@ -13,17 +13,17 @@ import {
1313
ApiParam,
1414
} from '@nestjs/swagger';
1515
import { AuditLogService } from './audit-log.service';
16-
import { AUDIT_LOG_MODULE_OPTIONS, AuditLogModuleOptions } from './interfaces';
16+
import { AuditLogFeatureGuard } from './guards/feature-enabled.guard';
1717
import { AuditLogQueryDto } from './dto/audit-log-query.dto';
1818

19+
/** Requires a global authentication guard (e.g., JwtAuthGuard). Guard enforcement is the consumer's responsibility. */
1920
@ApiTags('Audit Logs')
2021
@ApiBearerAuth()
22+
@UseGuards(AuditLogFeatureGuard)
2123
@Controller('audit-logs')
2224
export class AuditLogController {
2325
constructor(
2426
private readonly auditLogService: AuditLogService,
25-
@Inject(AUDIT_LOG_MODULE_OPTIONS)
26-
private readonly options: AuditLogModuleOptions,
2727
) {}
2828

2929
@Get()

packages/nestjs-audit-log/src/audit-log.service.spec.ts

Lines changed: 9 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import { Test, TestingModule } from '@nestjs/testing';
2+
import { PRISMA_SERVICE, createMockPrismaService } from '@bbv/nestjs-prisma';
23
import { AuditLogService } from './audit-log.service';
34
import { AUDIT_LOG_MODULE_OPTIONS, AuditLogModuleOptions } from './interfaces';
45

@@ -8,15 +9,12 @@ describe('AuditLogService', () => {
89
let mockOptions: AuditLogModuleOptions;
910

1011
beforeEach(async () => {
11-
mockPrisma = {
12-
auditLog: {
13-
create: jest.fn().mockResolvedValue({ id: 'log-1' }),
14-
findMany: jest.fn().mockResolvedValue([]),
15-
findUnique: jest.fn().mockResolvedValue(null),
16-
count: jest.fn().mockResolvedValue(0),
17-
deleteMany: jest.fn().mockResolvedValue({ count: 0 }),
18-
},
19-
};
12+
mockPrisma = createMockPrismaService();
13+
mockPrisma.auditLog.create.mockResolvedValue({ id: 'log-1' });
14+
mockPrisma.auditLog.findMany.mockResolvedValue([]);
15+
mockPrisma.auditLog.findUnique.mockResolvedValue(null);
16+
mockPrisma.auditLog.count.mockResolvedValue(0);
17+
mockPrisma.auditLog.deleteMany.mockResolvedValue({ count: 0 });
2018

2119
mockOptions = {
2220
features: {
@@ -31,7 +29,7 @@ describe('AuditLogService', () => {
3129
const module: TestingModule = await Test.createTestingModule({
3230
providers: [
3331
AuditLogService,
34-
{ provide: 'PRISMA_SERVICE', useValue: mockPrisma },
32+
{ provide: PRISMA_SERVICE, useValue: mockPrisma },
3533
{ provide: AUDIT_LOG_MODULE_OPTIONS, useValue: mockOptions },
3634
],
3735
}).compile();
@@ -225,6 +223,7 @@ describe('AuditLogService', () => {
225223
expect(mockPrisma.auditLog.findMany).toHaveBeenCalledWith({
226224
where: { entity: 'Claim', entityId: 'c-1' },
227225
orderBy: { createdAt: 'desc' },
226+
take: 50,
228227
});
229228
});
230229

packages/nestjs-audit-log/src/audit-log.service.ts

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import { Inject, Injectable, Logger } from '@nestjs/common';
2+
import { PRISMA_SERVICE } from '@bbv/nestjs-prisma';
23
import {
34
AuditLogEntry,
45
AuditLogModuleOptions,
@@ -11,7 +12,7 @@ export class AuditLogService {
1112
private readonly logger = new Logger(AuditLogService.name);
1213

1314
constructor(
14-
@Inject('PRISMA_SERVICE') private readonly prisma: any,
15+
@Inject(PRISMA_SERVICE) private readonly prisma: any,
1516
@Inject(AUDIT_LOG_MODULE_OPTIONS)
1617
private readonly options: AuditLogModuleOptions,
1718
) {}
@@ -108,6 +109,7 @@ export class AuditLogService {
108109
return this.prisma.auditLog.findMany({
109110
where: { entity, entityId },
110111
orderBy: { createdAt: 'desc' },
112+
take: 50,
111113
});
112114
}
113115

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,34 @@
1+
import {
2+
CanActivate,
3+
ExecutionContext,
4+
Inject,
5+
Injectable,
6+
NotFoundException,
7+
} from '@nestjs/common';
8+
import { AUDIT_LOG_MODULE_OPTIONS, AuditLogModuleOptions } from '../interfaces';
9+
10+
/**
11+
* Guard that checks if the `registerController` feature is enabled in the
12+
* resolved audit-log module options. When used with `forRootAsync`, the
13+
* controller is always registered (NestJS limitation), so this guard provides
14+
* runtime parity with the `forRoot` path which conditionally registers the
15+
* controller at declaration time.
16+
*
17+
* If the feature is disabled the guard throws a `NotFoundException` so the
18+
* route behaves as if it does not exist.
19+
*/
20+
@Injectable()
21+
export class AuditLogFeatureGuard implements CanActivate {
22+
constructor(
23+
@Inject(AUDIT_LOG_MODULE_OPTIONS)
24+
private readonly options: AuditLogModuleOptions,
25+
) {}
26+
27+
canActivate(_context: ExecutionContext): boolean {
28+
if (!this.options.features?.registerController) {
29+
throw new NotFoundException();
30+
}
31+
32+
return true;
33+
}
34+
}

packages/nestjs-audit-log/src/index.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
export { AuditLogModule } from './audit-log.module';
22
export { AuditLogService } from './audit-log.service';
33
export { AuditLogController } from './audit-log.controller';
4+
export { AuditLogFeatureGuard } from './guards/feature-enabled.guard';
45
export { AuditLogQueryDto } from './dto/audit-log-query.dto';
56
export {
67
Audited,

packages/nestjs-audit-log/src/middleware/prisma-audit.middleware.ts

Lines changed: 11 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -100,6 +100,16 @@ export function createAuditMiddleware(
100100
};
101101
}
102102

103+
/**
104+
* Handles UPDATE audit logging.
105+
*
106+
* TODO: Change tracking (old vs new values) requires a reference to the Prisma client
107+
* instance to fetch the record before the update. The Prisma middleware `params` object
108+
* does not expose the client, so `old` values are not available. Currently, only the
109+
* new values from the update data are logged. To enable full change tracking, the
110+
* consuming application would need to pass the Prisma client into the audit middleware
111+
* factory, or use Prisma's `$extends` client extension API instead of middleware.
112+
*/
103113
async function handleUpdate(
104114
params: any,
105115
next: (params: any) => Promise<any>,
@@ -108,20 +118,6 @@ async function handleUpdate(
108118
context: AuditContext | undefined,
109119
excludeFields: Set<string>,
110120
): Promise<any> {
111-
let oldRecord: Record<string, unknown> | undefined;
112-
113-
try {
114-
const prisma = params.__internalParams?.prisma ?? params.dataPath?.[0];
115-
116-
if (params.args?.where) {
117-
oldRecord = await (params as any).__internalParams?.transaction
118-
? undefined
119-
: undefined;
120-
}
121-
} catch {
122-
// If we cannot fetch old record, continue without it
123-
}
124-
125121
const result = await next(params);
126122

127123
const changes: Record<string, { old: unknown; new: unknown }> = {};
@@ -134,7 +130,7 @@ async function handleUpdate(
134130

135131
if (newValue !== undefined) {
136132
changes[key] = {
137-
old: oldRecord?.[key] ?? null,
133+
old: null,
138134
new: newValue,
139135
};
140136
}

packages/nestjs-auth/src/auth.module.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -65,7 +65,6 @@ export class AuthModule {
6565
...(asyncOptions.imports ?? []),
6666
PassportModule.register({ defaultStrategy: 'jwt' }),
6767
JwtModule.registerAsync({
68-
imports: asyncOptions.imports,
6968
useFactory: async (...args: any[]) => {
7069
const options = await asyncOptions.useFactory(...args);
7170
return {
@@ -76,6 +75,7 @@ export class AuthModule {
7675
};
7776
},
7877
inject: asyncOptions.inject ?? [],
78+
imports: asyncOptions.imports ?? [],
7979
}),
8080
],
8181
controllers: [AuthController, OrganizationController],

packages/nestjs-auth/src/auth.service.events.spec.ts

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import { EventEmitter2 } from '@nestjs/event-emitter';
44
import { Test, TestingModule } from '@nestjs/testing';
55
import * as bcrypt from 'bcryptjs';
66

7+
import { PRISMA_SERVICE } from '@bbv/nestjs-prisma';
78
import { AuthService } from './auth.service';
89
import { AUTH_MODULE_OPTIONS, AuthModuleOptions } from './interfaces';
910
import { AUTH_EVENTS } from './events';
@@ -54,7 +55,7 @@ describe('AuthService - Event Emission', () => {
5455
providers: [
5556
AuthService,
5657
{ provide: AUTH_MODULE_OPTIONS, useValue: defaultOptions },
57-
{ provide: 'PRISMA_SERVICE', useValue: mockPrismaService },
58+
{ provide: PRISMA_SERVICE, useValue: mockPrismaService },
5859
{ provide: JwtService, useValue: mockJwtService },
5960
{ provide: EventEmitter2, useValue: mockEventEmitter },
6061
],
@@ -111,7 +112,7 @@ describe('AuthService - Event Emission', () => {
111112
features: { ...defaultOptions.features, emailVerification: false },
112113
},
113114
},
114-
{ provide: 'PRISMA_SERVICE', useValue: mockPrismaService },
115+
{ provide: PRISMA_SERVICE, useValue: mockPrismaService },
115116
{ provide: JwtService, useValue: mockJwtService },
116117
{ provide: EventEmitter2, useValue: mockEventEmitter },
117118
],
@@ -223,7 +224,7 @@ describe('AuthService - Event Emission', () => {
223224
providers: [
224225
AuthService,
225226
{ provide: AUTH_MODULE_OPTIONS, useValue: defaultOptions },
226-
{ provide: 'PRISMA_SERVICE', useValue: mockPrismaService },
227+
{ provide: PRISMA_SERVICE, useValue: mockPrismaService },
227228
{ provide: JwtService, useValue: mockJwtService },
228229
],
229230
}).compile();

0 commit comments

Comments
 (0)