From 666360fa3a6a9080a895658a4e29a453d0577b7f Mon Sep 17 00:00:00 2001 From: Igor Propisnov Date: Sun, 8 Sep 2024 17:57:50 +0200 Subject: [PATCH 1/2] added basic error handling for backend --- backend/src/main.ts | 12 +++ .../auth-module/services/auth.service.ts | 75 ++++++++--------- .../services/email-verification.service.ts | 84 ++++++++++++------- .../src/shared/exceptions/base.exception.ts | 11 +++ .../shared/exceptions/conflict.exception.ts | 9 ++ .../shared/exceptions/forbidden.exception.ts | 9 ++ backend/src/shared/exceptions/index.ts | 4 + .../internal-server-error.exception.ts | 14 ++++ .../shared/exceptions/not-found.exception.ts | 9 ++ .../shared/filters/http-exception.filter.ts | 70 ++++++++++++++++ backend/src/shared/filters/index.ts | 1 + .../error-handling.interceptor.ts | 37 ++++++++ backend/src/shared/interceptors/index.ts | 1 + 13 files changed, 265 insertions(+), 71 deletions(-) create mode 100644 backend/src/shared/exceptions/base.exception.ts create mode 100644 backend/src/shared/exceptions/conflict.exception.ts create mode 100644 backend/src/shared/exceptions/forbidden.exception.ts create mode 100644 backend/src/shared/exceptions/index.ts create mode 100644 backend/src/shared/exceptions/internal-server-error.exception.ts create mode 100644 backend/src/shared/exceptions/not-found.exception.ts create mode 100644 backend/src/shared/filters/http-exception.filter.ts create mode 100644 backend/src/shared/filters/index.ts create mode 100644 backend/src/shared/interceptors/error-handling.interceptor.ts create mode 100644 backend/src/shared/interceptors/index.ts diff --git a/backend/src/main.ts b/backend/src/main.ts index 2e64907..12fd784 100644 --- a/backend/src/main.ts +++ b/backend/src/main.ts @@ -8,6 +8,8 @@ import * as cookieParser from 'cookie-parser'; import { AppModule } from './app.module'; import { SessionInitService } from './modules/session/services'; +import { HttpExceptionFilter } from './shared/filters'; +import { ErrorHandlingInterceptor } from './shared/interceptors'; async function setupSwagger(app: INestApplication): Promise { const config = new DocumentBuilder() @@ -50,12 +52,22 @@ async function setupClassValidator(app: INestApplication): Promise { app.useGlobalPipes(new ValidationPipe()); } +async function setupGlobalFilters(app: INestApplication): Promise { + app.useGlobalFilters(new HttpExceptionFilter()); +} + +async function setupGlobalInterceptors(app: INestApplication): Promise { + app.useGlobalInterceptors(new ErrorHandlingInterceptor()); +} + async function bootstrap(): Promise { const app = await NestFactory.create(AppModule); await setupCookieParser(app); await setupSwagger(app); await setupPrefix(app); + await setupGlobalFilters(app); + await setupGlobalInterceptors(app); await setupClassValidator(app); await setupSessions(app); await app.listen(3000); diff --git a/backend/src/modules/auth-module/services/auth.service.ts b/backend/src/modules/auth-module/services/auth.service.ts index 442c740..53e70f4 100644 --- a/backend/src/modules/auth-module/services/auth.service.ts +++ b/backend/src/modules/auth-module/services/auth.service.ts @@ -1,13 +1,12 @@ -import { - ConflictException, - ForbiddenException, - HttpException, - HttpStatus, - Injectable, -} from '@nestjs/common'; +import { Injectable } from '@nestjs/common'; import { UserCredentials } from 'src/entities'; import { SessionService } from 'src/modules/session/services/session.service'; import { EncryptionService, SuccessDto } from 'src/shared'; +import { + ConflictException, + ForbiddenException, + InternalServerErrorException, +} from 'src/shared/exceptions'; import { PasswordConfirmationMailService } from '../../sendgrid-module/services/password-confirmation.mail.service'; import { UserDataRepository } from '../../user-module/repositories/user-data.repository'; @@ -34,7 +33,7 @@ export class AuthService { ); if (existingUser) { - throw new ConflictException('User already exists'); + throw new ConflictException('USER_ALREADY_EXISTS'); } const passwordHashed = await EncryptionService.hashData( @@ -63,14 +62,11 @@ export class AuthService { }; } catch (error) { if (error instanceof ConflictException) { - throw new ConflictException( - 'User already exists. Please try to login instead.' - ); + throw error; } else { - throw new HttpException( - 'Error while signing up', - HttpStatus.INTERNAL_SERVER_ERROR - ); + throw new InternalServerErrorException('SIGNUP_ERROR', { + cause: error, + }); } } } @@ -83,7 +79,7 @@ export class AuthService { const user = await this.userCredentialsRepository.findUserByEmail(email); if (!user) { - throw new ForbiddenException('Access Denied'); + throw new ForbiddenException('INVALID_CREDENTIALS'); } const passwordMatch = await EncryptionService.compareHash( @@ -92,60 +88,61 @@ export class AuthService { ); if (!passwordMatch) { - throw new ForbiddenException('Access Denied'); + throw new ForbiddenException('INVALID_CREDENTIALS'); } return user; } catch (error) { if (error instanceof ForbiddenException) { - throw new ForbiddenException( - 'E-Mail address or password is incorrect. Please try again.' - ); + throw error; } else { - throw new HttpException( - 'Error while validating user credentials', - HttpStatus.INTERNAL_SERVER_ERROR - ); + throw new InternalServerErrorException('VALIDATION_ERROR', { + cause: error, + }); } } } - public async signout(sessionId: string): Promise<{ success: boolean }> { + public async signout(sessionId: string): Promise { try { - this.sessionService.deleteSessionBySessionId(sessionId); + await this.sessionService.deleteSessionBySessionId(sessionId); return { success: true }; } catch (error) { - throw new HttpException( - 'Fehler beim Logout', - HttpStatus.INTERNAL_SERVER_ERROR - ); + throw new InternalServerErrorException('SIGNOUT_ERROR', { + message: + 'An error occurred during the sign out process. Please try again later.', + }); } } public async checkAuthStatus( sessionId: string, - userAgend: string + userAgent: string ): Promise { try { const session = await this.sessionService.findSessionBySessionId(sessionId); if (!session) { - throw new ForbiddenException('Session not found'); + throw new ForbiddenException('SESSION_NOT_FOUND'); } - const userAgendFromSession = JSON.parse(session.json).passport.user + const userAgentFromSession = JSON.parse(session.json).passport.user .userAgent; - if (userAgendFromSession !== userAgend) { - throw new ForbiddenException('User-Agent does not match'); + if (userAgentFromSession !== userAgent) { + throw new ForbiddenException('USER_AGENT_MISMATCH'); } + return { success: true }; } catch (error) { - throw new HttpException( - 'Error while checking auth status', - HttpStatus.INTERNAL_SERVER_ERROR - ); + if (error instanceof ForbiddenException) { + throw error; + } else { + throw new InternalServerErrorException('AUTH_STATUS_CHECK_ERROR', { + cause: error, + }); + } } } diff --git a/backend/src/modules/verify-module/services/email-verification.service.ts b/backend/src/modules/verify-module/services/email-verification.service.ts index 3f07e61..f963838 100644 --- a/backend/src/modules/verify-module/services/email-verification.service.ts +++ b/backend/src/modules/verify-module/services/email-verification.service.ts @@ -5,6 +5,7 @@ import { ConfigService } from '@nestjs/config'; import { EmailVerification } from 'src/entities'; import { SessionService } from 'src/modules/session/services/session.service'; import { UriEncoderService } from 'src/shared'; +import { InternalServerErrorException } from 'src/shared/exceptions'; import { UserDataRepository } from '../../user-module/repositories/user-data.repository'; import { EmailVerifyRepository } from '../repositories'; @@ -19,58 +20,77 @@ export class EmailVerificationService { ) {} public async generateEmailVerificationToken(userId: string): Promise { - const verificationToken = await this.createVerificationToken(); + try { + const verificationToken = await this.createVerificationToken(); + const expiration = new Date(Date.now() + 24 * 60 * 60 * 1000); - // TODO Check users local time zone and set expiration time accordingly - const expiration = new Date(Date.now() + 24 * 60 * 60 * 1000); + await this.emailVerifyRepository.createEmailVerification( + verificationToken, + expiration, + userId + ); - this.emailVerifyRepository.createEmailVerification( - verificationToken, - expiration, - userId - ); - - return verificationToken; + return verificationToken; + } catch (error) { + throw new InternalServerErrorException( + 'EMAIL_VERIFICATION_TOKEN_GENERATION_ERROR', + { + message: + 'An error occurred while generating the email verification token.', + } + ); + } } public async verifyEmail(tokenToVerify: string): Promise { - const isTokenVerified = - await this.emailVerifyRepository.findEmailVerificationByToken( - tokenToVerify - ); - - if (isTokenVerified) { + try { const emailVerification = + await this.emailVerifyRepository.findEmailVerificationByToken( + tokenToVerify + ); + + if (!emailVerification) { + return false; + } + + const deletedVerification = await this.deleteEmailVerificationToken(tokenToVerify); - if (emailVerification && emailVerification.user) { + if (deletedVerification && deletedVerification.user) { const isStatusUpdated = await this.userDataRepository.updateEmailVerificationStatus( - emailVerification.user.id + deletedVerification.user.id ); return isStatusUpdated; } - } - return false; + return false; + } catch (error) { + throw new InternalServerErrorException('EMAIL_VERIFICATION_ERROR', { + message: 'An error occurred while verifying the email.', + }); + } } public async isEmailVerified(sessionID: string): Promise { - const userId = await this.sessionService.getUserIdBySessionId(sessionID); + try { + const userId = await this.sessionService.getUserIdBySessionId(sessionID); - if (!userId) { - return false; + if (!userId) { + return false; + } + + const isVerified = + await this.userDataRepository.isEmailConfirmedByUserId(userId); + + return isVerified; + } catch (error) { + throw new InternalServerErrorException('EMAIL_VERIFICATION_CHECK_ERROR', { + message: + 'An error occurred while checking the email verification status.', + }); } - - const isVerfiied = - await this.userDataRepository.isEmailConfirmedByUserId(userId); - - if (isVerfiied) { - return true; - } - - return false; } private async createVerificationToken(): Promise { diff --git a/backend/src/shared/exceptions/base.exception.ts b/backend/src/shared/exceptions/base.exception.ts new file mode 100644 index 0000000..842df21 --- /dev/null +++ b/backend/src/shared/exceptions/base.exception.ts @@ -0,0 +1,11 @@ +export class BaseException extends Error { + public constructor( + public readonly message: string, + public readonly status: number, + public readonly error: string, + public readonly details?: unknown + ) { + super(message); + this.name = this.constructor.name; + } +} diff --git a/backend/src/shared/exceptions/conflict.exception.ts b/backend/src/shared/exceptions/conflict.exception.ts new file mode 100644 index 0000000..e9b3962 --- /dev/null +++ b/backend/src/shared/exceptions/conflict.exception.ts @@ -0,0 +1,9 @@ +import { HttpStatus } from '@nestjs/common'; + +import { BaseException } from './base.exception'; + +export class ConflictException extends BaseException { + public constructor(errorCode: string, details?: unknown) { + super('Conflict Error', HttpStatus.CONFLICT, errorCode, details); + } +} diff --git a/backend/src/shared/exceptions/forbidden.exception.ts b/backend/src/shared/exceptions/forbidden.exception.ts new file mode 100644 index 0000000..b78a08e --- /dev/null +++ b/backend/src/shared/exceptions/forbidden.exception.ts @@ -0,0 +1,9 @@ +import { HttpStatus } from '@nestjs/common'; + +import { BaseException } from './base.exception'; + +export class ForbiddenException extends BaseException { + public constructor(errorCode: string, details?: unknown) { + super('Forbidden Error', HttpStatus.FORBIDDEN, errorCode, details); + } +} diff --git a/backend/src/shared/exceptions/index.ts b/backend/src/shared/exceptions/index.ts new file mode 100644 index 0000000..c43a40a --- /dev/null +++ b/backend/src/shared/exceptions/index.ts @@ -0,0 +1,4 @@ +export * from './conflict.exception'; +export * from './forbidden.exception'; +export * from './internal-server-error.exception'; +export * from './not-found.exception'; diff --git a/backend/src/shared/exceptions/internal-server-error.exception.ts b/backend/src/shared/exceptions/internal-server-error.exception.ts new file mode 100644 index 0000000..ba31ade --- /dev/null +++ b/backend/src/shared/exceptions/internal-server-error.exception.ts @@ -0,0 +1,14 @@ +import { HttpStatus } from '@nestjs/common'; + +import { BaseException } from './base.exception'; + +export class InternalServerErrorException extends BaseException { + public constructor(errorCode: string, details?: unknown) { + super( + 'Internal Server Error', + HttpStatus.INTERNAL_SERVER_ERROR, + errorCode, + details + ); + } +} diff --git a/backend/src/shared/exceptions/not-found.exception.ts b/backend/src/shared/exceptions/not-found.exception.ts new file mode 100644 index 0000000..cbdbc33 --- /dev/null +++ b/backend/src/shared/exceptions/not-found.exception.ts @@ -0,0 +1,9 @@ +import { HttpStatus } from '@nestjs/common'; + +import { BaseException } from './base.exception'; + +export class NotFoundException extends BaseException { + public constructor(errorCode: string, details?: unknown) { + super('Not Found Error', HttpStatus.NOT_FOUND, errorCode, details); + } +} diff --git a/backend/src/shared/filters/http-exception.filter.ts b/backend/src/shared/filters/http-exception.filter.ts new file mode 100644 index 0000000..d507bb8 --- /dev/null +++ b/backend/src/shared/filters/http-exception.filter.ts @@ -0,0 +1,70 @@ +/* eslint-disable @typescript-eslint/no-explicit-any */ +import { + ExceptionFilter, + Catch, + ArgumentsHost, + HttpException, + HttpStatus, + Logger, +} from '@nestjs/common'; +import { Response } from 'express'; + +import { BaseException } from '../exceptions/base.exception'; + +@Catch() +export class HttpExceptionFilter implements ExceptionFilter { + private readonly logger: Logger = new Logger(HttpExceptionFilter.name); + + public catch(exception: unknown, host: ArgumentsHost): void { + const ctx = host.switchToHttp(); + const response = ctx.getResponse(); + + let status: HttpStatus = HttpStatus.INTERNAL_SERVER_ERROR; + let message: string = 'Internal server error'; + let error: string = 'INTERNAL_SERVER_ERROR'; + let details: unknown = undefined; + + if (exception instanceof BaseException) { + status = exception.status; + message = exception.message; + error = exception.error; + details = exception.details; + } else if (exception instanceof HttpException) { + status = exception.getStatus(); + message = exception.message; + } + + // Logging + this.logger.error(`${error}: ${message}`, exception); + + // TODO: Error reporting (mock implementation) + this.reportError(error, message, details); + + response.status(status).json({ + status: status, + message, + error, + details: this.sanitizeErrorDetails(details), + timestamp: new Date().toISOString(), + }); + } + + private sanitizeErrorDetails(details: any): any { + if (details && typeof details === 'object') { + const sanitized = { ...details }; + + // TODO: Remove sensitive data + // delete sanitized.password; + // delete sanitized.creditCard; + + return sanitized; + } + return details; + } + + private reportError(errorCode: string, message: string, details: any): void { + console.log(`Error reported: ${errorCode} - ${message}`); + // TODO: Implement error reporting (i. e. Sentry) + // Example Sentry.captureException(new Error(`${errorCode}: ${message}`), { extra: details }); + } +} diff --git a/backend/src/shared/filters/index.ts b/backend/src/shared/filters/index.ts new file mode 100644 index 0000000..18204b0 --- /dev/null +++ b/backend/src/shared/filters/index.ts @@ -0,0 +1 @@ +export * from './http-exception.filter'; diff --git a/backend/src/shared/interceptors/error-handling.interceptor.ts b/backend/src/shared/interceptors/error-handling.interceptor.ts new file mode 100644 index 0000000..e11782d --- /dev/null +++ b/backend/src/shared/interceptors/error-handling.interceptor.ts @@ -0,0 +1,37 @@ +/* eslint-disable @typescript-eslint/no-explicit-any */ +import { + Injectable, + NestInterceptor, + ExecutionContext, + CallHandler, + HttpException, + InternalServerErrorException, +} from '@nestjs/common'; +import { Observable } from 'rxjs'; +import { catchError } from 'rxjs/operators'; + +import { BaseException } from '../exceptions/base.exception'; + +@Injectable() +export class ErrorHandlingInterceptor implements NestInterceptor { + public intercept( + context: ExecutionContext, + next: CallHandler + ): Observable { + return next.handle().pipe( + catchError((error) => { + if (error instanceof BaseException) { + throw error; + } else if (error instanceof HttpException) { + throw error; + } else { + throw new InternalServerErrorException({ + message: 'An unexpected error occurred', + errorCode: 'UNEXPECTED_ERROR', + details: { originalError: error.message }, + }); + } + }) + ); + } +} diff --git a/backend/src/shared/interceptors/index.ts b/backend/src/shared/interceptors/index.ts new file mode 100644 index 0000000..433a76e --- /dev/null +++ b/backend/src/shared/interceptors/index.ts @@ -0,0 +1 @@ +export * from './error-handling.interceptor'; -- 2.40.1 From b61b4743e40613fc0612210970c9e55715c5b5e3 Mon Sep 17 00:00:00 2001 From: Igor Propisnov Date: Sun, 8 Sep 2024 18:00:06 +0200 Subject: [PATCH 2/2] remove unsed code --- .../verify-module/services/email-verification.service.ts | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/backend/src/modules/verify-module/services/email-verification.service.ts b/backend/src/modules/verify-module/services/email-verification.service.ts index f963838..c0e9214 100644 --- a/backend/src/modules/verify-module/services/email-verification.service.ts +++ b/backend/src/modules/verify-module/services/email-verification.service.ts @@ -1,7 +1,6 @@ import { randomBytes } from 'crypto'; import { Injectable } from '@nestjs/common'; -import { ConfigService } from '@nestjs/config'; import { EmailVerification } from 'src/entities'; import { SessionService } from 'src/modules/session/services/session.service'; import { UriEncoderService } from 'src/shared'; @@ -15,8 +14,7 @@ export class EmailVerificationService { public constructor( private readonly emailVerifyRepository: EmailVerifyRepository, private readonly userDataRepository: UserDataRepository, - private readonly sessionService: SessionService, - private readonly configService: ConfigService + private readonly sessionService: SessionService ) {} public async generateEmailVerificationToken(userId: string): Promise { -- 2.40.1