From b1aa115d2821e43628771af4ca3ee40086097c2f Mon Sep 17 00:00:00 2001 From: Weiko Date: Fri, 9 Aug 2024 14:09:26 +0200 Subject: [PATCH] Fix auth exceptions (#6590) A regression has been introduced in https://github.com/twentyhq/twenty/pull/6459/files#diff-0a06bf2b624f77f1b7ded0fcc4ce266d1a56f4329222b46d1cf4d76a18000c97L505 where 401 have been changed to 403. However the renew token logic on the FE expects a 401, see here https://github.com/twentyhq/twenty/blob/main/packages/twenty-front/src/modules/apollo/services/apollo.factory.ts#L100 I've also introduced a fix with a proxy class in GraphQLHydrateRequestFromTokenMiddleware since this middleware calls validateToken from tokenService which are never converted back to graphqlErrors so handleExceptionAndConvertToGraphQLError below will receive an AuthException and will send capture it and return it as a 500 both issues have been fixed and should resolve the renewToken logic ## Test tested locally by playing with token expiration dates in the env Screenshot 2024-08-09 at 12 47 05 --- .../core-modules/auth/auth.exception.ts | 1 + .../auth-graphql-api-exception.filter.ts | 7 +++-- .../auth/services/token.service.ts | 4 +-- ...l-hydrate-request-from-token.middleware.ts | 27 +++++++++++++++++-- 4 files changed, 33 insertions(+), 6 deletions(-) diff --git a/packages/twenty-server/src/engine/core-modules/auth/auth.exception.ts b/packages/twenty-server/src/engine/core-modules/auth/auth.exception.ts index c245db971..6195fd37d 100644 --- a/packages/twenty-server/src/engine/core-modules/auth/auth.exception.ts +++ b/packages/twenty-server/src/engine/core-modules/auth/auth.exception.ts @@ -12,6 +12,7 @@ export enum AuthExceptionCode { CLIENT_NOT_FOUND = 'CLIENT_NOT_FOUND', INVALID_INPUT = 'INVALID_INPUT', FORBIDDEN_EXCEPTION = 'FORBIDDEN_EXCEPTION', + UNAUTHENTICATED = 'UNAUTHENTICATED', INVALID_DATA = 'INVALID_DATA', INTERNAL_SERVER_ERROR = 'INTERNAL_SERVER_ERROR', } diff --git a/packages/twenty-server/src/engine/core-modules/auth/filters/auth-graphql-api-exception.filter.ts b/packages/twenty-server/src/engine/core-modules/auth/filters/auth-graphql-api-exception.filter.ts index ebabba88f..c33c724f0 100644 --- a/packages/twenty-server/src/engine/core-modules/auth/filters/auth-graphql-api-exception.filter.ts +++ b/packages/twenty-server/src/engine/core-modules/auth/filters/auth-graphql-api-exception.filter.ts @@ -1,10 +1,11 @@ -import { Catch } from '@nestjs/common'; +import { Catch, ExceptionFilter } from '@nestjs/common'; import { AuthException, AuthExceptionCode, } from 'src/engine/core-modules/auth/auth.exception'; import { + AuthenticationError, ForbiddenError, InternalServerError, NotFoundError, @@ -12,7 +13,7 @@ import { } from 'src/engine/core-modules/graphql/utils/graphql-errors.util'; @Catch(AuthException) -export class AuthGraphqlApiExceptionFilter { +export class AuthGraphqlApiExceptionFilter implements ExceptionFilter { catch(exception: AuthException) { switch (exception.code) { case AuthExceptionCode.USER_NOT_FOUND: @@ -22,6 +23,8 @@ export class AuthGraphqlApiExceptionFilter { throw new UserInputError(exception.message); case AuthExceptionCode.FORBIDDEN_EXCEPTION: throw new ForbiddenError(exception.message); + case AuthExceptionCode.UNAUTHENTICATED: + throw new AuthenticationError(exception.message); case AuthExceptionCode.INVALID_DATA: case AuthExceptionCode.INTERNAL_SERVER_ERROR: default: diff --git a/packages/twenty-server/src/engine/core-modules/auth/services/token.service.ts b/packages/twenty-server/src/engine/core-modules/auth/services/token.service.ts index 9a45a3a37..da571fbe5 100644 --- a/packages/twenty-server/src/engine/core-modules/auth/services/token.service.ts +++ b/packages/twenty-server/src/engine/core-modules/auth/services/token.service.ts @@ -564,12 +564,12 @@ export class TokenService { if (error instanceof TokenExpiredError) { throw new AuthException( 'Token has expired.', - AuthExceptionCode.FORBIDDEN_EXCEPTION, + AuthExceptionCode.UNAUTHENTICATED, ); } else if (error instanceof JsonWebTokenError) { throw new AuthException( 'Token invalid.', - AuthExceptionCode.FORBIDDEN_EXCEPTION, + AuthExceptionCode.UNAUTHENTICATED, ); } else { throw new AuthException( diff --git a/packages/twenty-server/src/engine/middlewares/graphql-hydrate-request-from-token.middleware.ts b/packages/twenty-server/src/engine/middlewares/graphql-hydrate-request-from-token.middleware.ts index 4a402cb27..a756b8cef 100644 --- a/packages/twenty-server/src/engine/middlewares/graphql-hydrate-request-from-token.middleware.ts +++ b/packages/twenty-server/src/engine/middlewares/graphql-hydrate-request-from-token.middleware.ts @@ -1,13 +1,32 @@ import { Injectable, NestMiddleware } from '@nestjs/common'; -import { Request, Response, NextFunction } from 'express'; +import { NextFunction, Request, Response } from 'express'; +import { AuthGraphqlApiExceptionFilter } from 'src/engine/core-modules/auth/filters/auth-graphql-api-exception.filter'; import { TokenService } from 'src/engine/core-modules/auth/services/token.service'; import { AuthContext } from 'src/engine/core-modules/auth/types/auth-context.type'; import { ExceptionHandlerService } from 'src/engine/integrations/exception-handler/exception-handler.service'; import { WorkspaceCacheVersionService } from 'src/engine/metadata-modules/workspace-cache-version/workspace-cache-version.service'; import { handleExceptionAndConvertToGraphQLError } from 'src/engine/utils/global-exception-handler.util'; +class GraphqlTokenValidationProxy { + private tokenService: TokenService; + + constructor(tokenService: TokenService) { + this.tokenService = tokenService; + } + + async validateToken(req: Request) { + try { + return await this.tokenService.validateToken(req); + } catch (error) { + const authGraphqlApiExceptionFilter = new AuthGraphqlApiExceptionFilter(); + + throw authGraphqlApiExceptionFilter.catch(error); + } + } +} + @Injectable() export class GraphQLHydrateRequestFromTokenMiddleware implements NestMiddleware @@ -48,7 +67,11 @@ export class GraphQLHydrateRequestFromTokenMiddleware let data: AuthContext; try { - data = await this.tokenService.validateToken(req); + const graphqlTokenValidationProxy = new GraphqlTokenValidationProxy( + this.tokenService, + ); + + data = await graphqlTokenValidationProxy.validateToken(req); const cacheVersion = await this.workspaceCacheVersionService.getVersion( data.workspace.id, );