From b4aeddca58d3447f33024c3f1737490991eb8b44 Mon Sep 17 00:00:00 2001
From: crazyoptimist <hey@crazyoptimist.net>
Date: Tue, 18 Jun 2024 13:10:06 -0500
Subject: [PATCH] refactor(security): global error handler

- Prevent exposing internal error details
- Remove duplicate code by applying guards and api doc decorators at
  controller level
---
 src/modules/auth/auth.controller.ts           |  1 +
 .../exception-filter/all-exceptions.filter.ts | 22 +++++++++++++++----
 src/modules/user/user.controller.ts           | 12 ++--------
 3 files changed, 21 insertions(+), 14 deletions(-)

diff --git a/src/modules/auth/auth.controller.ts b/src/modules/auth/auth.controller.ts
index 25513cc..030a921 100644
--- a/src/modules/auth/auth.controller.ts
+++ b/src/modules/auth/auth.controller.ts
@@ -47,6 +47,7 @@ export class AuthController {
   }
 
   @Post('logout')
+  @ApiBearerAuth()
   @ApiResponse({ status: 201, description: 'Successful Logout' })
   @ApiResponse({ status: 401, description: 'Unauthorized' })
   async logout(@Request() request: IRequest): Promise<any> {
diff --git a/src/modules/common/exception-filter/all-exceptions.filter.ts b/src/modules/common/exception-filter/all-exceptions.filter.ts
index 6f4de4b..3b592b5 100644
--- a/src/modules/common/exception-filter/all-exceptions.filter.ts
+++ b/src/modules/common/exception-filter/all-exceptions.filter.ts
@@ -36,11 +36,24 @@ export class AllExceptionsFilter implements ExceptionFilter {
       statusCode = exception.getStatus();
       responseBody = exception.getResponse() as ResponseBody;
     } else if (exception instanceof QueryFailedError) {
-      statusCode = HttpStatus.BAD_REQUEST;
+      this.logger.error((exception as Error).stack);
+
+      const isDuplicateKeyError = exception.message?.includes(
+        'duplicate key value violates unique constraint',
+      );
+
+      statusCode = isDuplicateKeyError
+        ? HttpStatus.BAD_REQUEST
+        : HttpStatus.INTERNAL_SERVER_ERROR;
+
+      const message = isDuplicateKeyError
+        ? exception.message
+        : 'Service Unavailable';
+
       responseBody = {
         statusCode,
-        message: exception.message,
-        error: 'Database Query Failed Error',
+        message,
+        error: 'Query Failed Error',
       };
     } else if (exception instanceof EntityNotFoundError) {
       statusCode = HttpStatus.NOT_FOUND;
@@ -51,10 +64,11 @@ export class AllExceptionsFilter implements ExceptionFilter {
       };
     } else {
       this.logger.error((exception as Error).stack);
+
       statusCode = HttpStatus.INTERNAL_SERVER_ERROR;
       responseBody = {
         statusCode,
-        message: (exception as Error).message,
+        message: 'Service Unavailable',
         error: 'Internal Server Error',
       };
     }
diff --git a/src/modules/user/user.controller.ts b/src/modules/user/user.controller.ts
index fe69a4f..f16dbff 100644
--- a/src/modules/user/user.controller.ts
+++ b/src/modules/user/user.controller.ts
@@ -28,14 +28,14 @@ import { Action } from '../infrastructure/casl/action.enum';
 import { User } from './user.entity';
 
 @Controller('api/users')
+@UseGuards(PoliciesGuard)
 @ApiTags('users')
+@ApiBearerAuth()
 export class UserController {
   constructor(private readonly userService: UserService) {}
 
   @Post()
-  @UseGuards(PoliciesGuard)
   @CheckPolicies((ability) => ability.can(Action.Create, User))
-  @ApiBearerAuth()
   @ApiResponse({ status: 201, description: 'New User Created' })
   @ApiResponse({ status: 400, description: 'Bad Request' })
   @ApiResponse({ status: 401, description: 'Unauthorized' })
@@ -44,9 +44,7 @@ export class UserController {
   }
 
   @Get()
-  @UseGuards(PoliciesGuard)
   @CheckPolicies((ability) => ability.can(Action.Read, User))
-  @ApiBearerAuth()
   @ApiResponse({ status: 200, description: 'All Users' })
   @ApiResponse({ status: 401, description: 'Unauthorized' })
   async findAll(
@@ -69,9 +67,7 @@ export class UserController {
   }
 
   @Get(':id')
-  @UseGuards(PoliciesGuard)
   @CheckPolicies((ability) => ability.can(Action.Read, User))
-  @ApiBearerAuth()
   @ApiResponse({ status: 200, description: 'User For Given ID' })
   @ApiResponse({ status: 401, description: 'Unauthorized' })
   @ApiResponse({ status: 404, description: 'Not Found' })
@@ -80,9 +76,7 @@ export class UserController {
   }
 
   @Patch(':id')
-  @UseGuards(PoliciesGuard)
   @CheckPolicies((ability) => ability.can(Action.Update, User))
-  @ApiBearerAuth()
   @ApiResponse({ status: 200, description: 'Successful Update' })
   @ApiResponse({ status: 400, description: 'Bad Request' })
   @ApiResponse({ status: 401, description: 'Unauthorized' })
@@ -95,9 +89,7 @@ export class UserController {
   }
 
   @Delete(':id')
-  @UseGuards(PoliciesGuard)
   @CheckPolicies((ability) => ability.can(Action.Delete, User))
-  @ApiBearerAuth()
   @ApiResponse({ status: 200, description: 'Successful Delete' })
   @ApiResponse({ status: 400, description: 'Bad Request' })
   @ApiResponse({ status: 401, description: 'Unauthorized' })