Merge pull request #445 from christabel888/fix/payments-limits-webhooks-improvements

feat: request id propagation, feature flag guards, webhook validation and filtering

Author: Jambox11

src/common/middleware/request-logging.middleware.spec.ts

@@ -1,5 +1,6 @@
 import requestLogger from './request-logging.middleware';
 import { Logger } from '@nestjs/common';
+import { RequestContextService } from '../request-context/request-context.service';
 
 describe('requestLogger', () => {
   beforeEach(() => jest.restoreAllMocks());
@@ -128,4 +129,32 @@ describe('requestLogger', () => {
 
     expect(req.requestId).toBe(existingId);
   });
+
+  it('propagates request ID into RequestContextService async context', () => {
+    const req: any = {
+      method: 'GET',
+      originalUrl: '/test',
+      headers: {},
+      ip: '1.2.3.4',
+    };
+    const res: any = {
+      setHeader: jest.fn(),
+      on: jest.fn(),
+      statusCode: 200,
+    };
+
+    let capturedRequestId: string | undefined;
+    const next = jest.fn().mockImplementation(() => {
+      const service = new RequestContextService();
+      capturedRequestId = service.getRequestId();
+    });
+
+    jest.spyOn(Logger.prototype, 'log').mockImplementation(() => {});
+
+    requestLogger(req, res, next as any);
+
+    expect(next).toHaveBeenCalled();
+    expect(capturedRequestId).toBeDefined();
+    expect(capturedRequestId).toBe(req.requestId);
+  });
 });

src/common/middleware/request-logging.middleware.ts

@@ -1,22 +1,23 @@
 import { Request, Response, NextFunction } from 'express';
 import { Logger } from '@nestjs/common';
 import { randomUUID } from 'crypto';
+import { RequestContextService } from '../request-context/request-context.service';
 
 export function requestLogger(
   req: Request | any,
   res: Response | any,
   next: NextFunction,
 ) {
   const logger = new Logger('RequestLogger');
-  try {
-    if (!req) {
-      logger.warn('Request logging skipped: invalid request object');
-      next();
-      return;
-    }
 
+  if (!req) {
+    logger.warn('Request logging skipped: invalid request object');
+    next();
+    return;
+  }
+
+  try {
     const idHeader =
-      req &&
       req.headers &&
       (req.headers['x-request-id'] || req.headers['X-Request-Id']);
     const id =
@@ -25,10 +26,7 @@ export function requestLogger(
         : randomUUID();
     const start = Date.now();
 
-    // Attach request ID to request object for access in controllers/services
-    if (req) {
-      req.requestId = id;
-    }
+    req.requestId = id;
 
     if (res && typeof res.setHeader === 'function') {
       try {
@@ -39,10 +37,9 @@ export function requestLogger(
     }
 
     const ip =
-      (req && (req.ip || (req.socket && req.socket.remoteAddress))) ||
-      'unknown';
-    const method = (req && req.method) || 'UNKNOWN';
-    const url = (req && (req.originalUrl || req.url)) || 'unknown';
+      (req.ip || (req.socket && req.socket.remoteAddress)) || 'unknown';
+    const method = req.method || 'UNKNOWN';
+    const url = (req.originalUrl || req.url) || 'unknown';
 
     logger.log(`${method} ${url} id=${id} ip=${ip}`);
 
@@ -58,13 +55,20 @@ export function requestLogger(
         }
       });
     }
+
+    RequestContextService.run({ requestId: id }, () => {
+      try {
+        next();
+      } catch (e) {
+        logger.warn('next() threw in requestLogger');
+      }
+    });
   } catch (err: any) {
     logger.warn('Request logging failed: ' + (err && err.message));
-  } finally {
     try {
       next();
     } catch (e) {
-      logger.warn('next() threw in requestLogger');
+      logger.warn('next() threw after requestLogger error');
     }
   }
 }

src/limits/limits.controller.spec.ts

@@ -4,6 +4,7 @@ import { LimitsController } from './limits.controller';
 import { LimitsService } from './limits.service';
 import { ApiKeyGuard } from '../api-keys/api-key.guard';
 import { LimitPeriod } from './dto/create-limit.dto';
+import { FeatureFlagGuard } from '../common/feature-flags/feature-flag.guard';
 
 const mockLimit = {
   id: 'uuid-limit-1',
@@ -33,7 +34,10 @@ describe('LimitsController', () => {
     const module: TestingModule = await Test.createTestingModule({
       controllers: [LimitsController],
       providers: [{ provide: LimitsService, useValue: limitsService }],
-    }).compile();
+    })
+      .overrideGuard(FeatureFlagGuard)
+      .useValue({ canActivate: () => true })
+      .compile();
 
     controller = module.get<LimitsController>(LimitsController);
   });

src/limits/limits.controller.ts

@@ -7,6 +7,7 @@ import {
   Delete,
   HttpCode,
   HttpStatus,
+  UseGuards,
 } from '@nestjs/common';
 import {
   ApiTags,
@@ -17,9 +18,15 @@ import {
 } from '@nestjs/swagger';
 import { LimitsService } from './limits.service';
 import { SetLimitsDto } from './dto/set-limits.dto';
+import {
+  FeatureFlagGuard,
+  FeatureFlag,
+} from '../common/feature-flags/feature-flag.guard';
 
 @ApiTags('limits')
 @Controller('wallets/:walletId/limits')
+@UseGuards(FeatureFlagGuard)
+@FeatureFlag('limits_api')
 export class LimitsController {
   constructor(private readonly limitsService: LimitsService) {}
 

src/limits/limits.module.ts

@@ -1,12 +1,21 @@
 import { Module } from '@nestjs/common';
+import { ConfigModule } from '@nestjs/config';
 import { LimitsService } from './limits.service';
 import { LimitsController } from './limits.controller';
 import { PrismaModule } from '../prisma/prisma.module';
+import { RequestContextService } from '../common/request-context/request-context.service';
+import { FeatureFlagService } from '../common/feature-flags/feature-flag.service';
+import { FeatureFlagGuard } from '../common/feature-flags/feature-flag.guard';
 
 @Module({
-  imports: [PrismaModule],
+  imports: [ConfigModule, PrismaModule],
   controllers: [LimitsController],
-  providers: [LimitsService],
+  providers: [
+    LimitsService,
+    RequestContextService,
+    FeatureFlagService,
+    FeatureFlagGuard,
+  ],
   exports: [LimitsService],
 })
 export class LimitsModule {}

src/limits/limits.service.ts

@@ -120,12 +120,17 @@ export class LimitsService {
   }
 
   async checkLimits(walletId: string, amount: number): Promise<void> {
+    const requestId = this.requestContext.getRequestId();
     const limits = await this.getLimits(walletId);
     if (!limits) {
       this.metrics.incrementLimitChecks('allowed');
       return;
     }
 
+    this.logger.log(
+      `Checking limits walletId=${walletId} amount=${amount} requestId=${requestId}`,
+    );
+
     // Enforce per-transaction cap: a cap of 0 blocks all transactions
     if (
       limits.perTransactionLimit >= 0 &&

src/payments/payments-limits.integration.spec.ts

@@ -6,6 +6,7 @@ import { WalletsService } from '../wallets/wallets.service';
 import { PrismaService } from '../prisma/prisma.service';
 import { PaymentStatus } from './entities/payment.entity';
 import { WalletStatus } from '../wallets/domain/wallet.model';
+import { RequestContextService } from '../common/request-context/request-context.service';
 
 describe('Payments and Limits Integration', () => {
   let paymentsService: PaymentsService;
@@ -30,6 +31,10 @@ describe('Payments and Limits Integration', () => {
     findWalletById: jest.fn(),
   };
 
+  const mockRequestContext = {
+    getRequestId: jest.fn().mockReturnValue('integration-req-id'),
+  };
+
   beforeEach(async () => {
     jest.clearAllMocks();
 
@@ -39,6 +44,7 @@ describe('Payments and Limits Integration', () => {
         LimitsService,
         { provide: PrismaService, useValue: mockPrisma },
         { provide: WalletsService, useValue: mockWalletsService },
+        { provide: RequestContextService, useValue: mockRequestContext },
       ],
     }).compile();
 

src/payments/payments.controller.spec.ts

@@ -4,6 +4,7 @@ import { PaymentsController } from './payments.controller';
 import { PaymentsService } from './payments.service';
 import { ApiKeyGuard } from '../api-keys/api-key.guard';
 import { RateLimitGuard } from '../rate-limit/rate-limit.guard';
+import { FeatureFlagGuard } from '../common/feature-flags/feature-flag.guard';
 import { PaymentStatus } from './entities/payment.entity';
 
 describe('PaymentsController', () => {
@@ -27,6 +28,8 @@ describe('PaymentsController', () => {
       .useValue({ canActivate: () => true })
       .overrideGuard(RateLimitGuard)
       .useValue({ canActivate: () => true })
+      .overrideGuard(FeatureFlagGuard)
+      .useValue({ canActivate: () => true })
       .compile();
 
     controller = module.get<PaymentsController>(PaymentsController);
@@ -77,6 +80,30 @@ describe('PaymentsController', () => {
     });
   });
 
+  describe('feature flag guard', () => {
+    it('should deny access when feature flag is disabled', async () => {
+      const restrictedModule = await Test.createTestingModule({
+        controllers: [PaymentsController],
+        providers: [{ provide: PaymentsService, useValue: paymentsService }],
+      })
+        .overrideGuard(ApiKeyGuard)
+        .useValue({ canActivate: () => true })
+        .overrideGuard(RateLimitGuard)
+        .useValue({ canActivate: () => true })
+        .overrideGuard(FeatureFlagGuard)
+        .useValue({
+          canActivate: () => {
+            throw new Error('Feature not available');
+          },
+        })
+        .compile();
+
+      const restrictedController =
+        restrictedModule.get<PaymentsController>(PaymentsController);
+      expect(restrictedController).toBeDefined();
+    });
+  });
+
   describe('swagger decorators', () => {
     it('should have @ApiResponse decorators on all routes', () => {
       const routes = ['create', 'findAll', 'findOne', 'update', 'remove'];

src/payments/payments.controller.ts

@@ -27,10 +27,15 @@ import {
   SensitiveEndpoint,
 } from '../rate-limit/rate-limit.guard';
 import { PaginationDto } from '../common/dto/pagination.dto';
+import {
+  FeatureFlagGuard,
+  FeatureFlag,
+} from '../common/feature-flags/feature-flag.guard';
 
 @ApiTags('payments')
 @Controller('payments')
-@UseGuards(ApiKeyGuard, RateLimitGuard)
+@UseGuards(ApiKeyGuard, RateLimitGuard, FeatureFlagGuard)
+@FeatureFlag('payments_api')
 export class PaymentsController {
   constructor(private readonly paymentsService: PaymentsService) {}
 

src/payments/payments.module.ts

@@ -1,12 +1,21 @@
 import { Module } from '@nestjs/common';
+import { ConfigModule } from '@nestjs/config';
 import { PaymentsService } from './payments.service';
 import { PaymentsController } from './payments.controller';
 import { LimitsModule } from '../limits/limits.module';
 import { WalletsModule } from '../wallets/wallets.module';
+import { RequestContextService } from '../common/request-context/request-context.service';
+import { FeatureFlagService } from '../common/feature-flags/feature-flag.service';
+import { FeatureFlagGuard } from '../common/feature-flags/feature-flag.guard';
 
 @Module({
-  imports: [LimitsModule, WalletsModule],
+  imports: [ConfigModule, LimitsModule, WalletsModule],
   controllers: [PaymentsController],
-  providers: [PaymentsService],
+  providers: [
+    PaymentsService,
+    RequestContextService,
+    FeatureFlagService,
+    FeatureFlagGuard,
+  ],
 })
 export class PaymentsModule {}