From 66296a4787954538a0d9fef5f6b29f3a70e7cd8c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?P=20A=20C=20=C2=B7=20=E5=85=88=E7=94=9F?= Date: Fri, 31 Jan 2025 17:12:20 +0200 Subject: [PATCH] [1/n]: Migrate `deleteOne` Rest API to use TwentyORM directly (#9784) # This PR - Addressing #3644 - Migrates the `DELETE /rest/*` endpoint to use TwentyORM - Factorizes common middleware logic into a common module --------- Co-authored-by: martmull --- .../tests/authentication/fixture.ts | 6 +- packages/twenty-server/@types/jest.d.ts | 2 + .../twenty-server/jest-integration.config.ts | 2 + packages/twenty-server/src/app.module.ts | 14 ++ .../controllers/rest-api-core.controller.ts | 15 +- .../api/rest/core/rest-api-core-v2.service.ts | 73 ++++++++ .../api/rest/rest-api-exception.filter.ts | 24 +++ .../src/engine/api/rest/rest-api.module.ts | 4 + .../http-exception-handler.service.ts | 11 +- .../default-error-message.constant.ts | 1 + ...excluded-middleware-operations.constant.ts | 19 ++ ...l-hydrate-request-from-token.middleware.ts | 96 +--------- .../engine/middlewares/middleware.module.ts | 19 ++ .../engine/middlewares/middleware.service.ts | 167 ++++++++++++++++++ .../middlewares/rest-core.middleware.ts | 22 +++ .../utils/graphql-token-validation-utils.ts | 22 +++ .../utils/global-exception-handler.util.ts | 2 +- .../constants/mock-person-ids.constants.ts | 4 + .../constants/person-gql-fields.constants.ts | 14 ++ .../all-people-resolvers.integration-spec.ts | 25 +-- .../rest-api-core-delete.integration-spec.ts | 107 +++++++++++ .../rest/utils/make-rest-api-request.util.ts | 18 ++ 22 files changed, 548 insertions(+), 119 deletions(-) create mode 100644 packages/twenty-server/src/engine/api/rest/core/rest-api-core-v2.service.ts create mode 100644 packages/twenty-server/src/engine/api/rest/rest-api-exception.filter.ts create mode 100644 packages/twenty-server/src/engine/middlewares/constants/default-error-message.constant.ts create mode 100644 packages/twenty-server/src/engine/middlewares/constants/excluded-middleware-operations.constant.ts create mode 100644 packages/twenty-server/src/engine/middlewares/middleware.module.ts create mode 100644 packages/twenty-server/src/engine/middlewares/middleware.service.ts create mode 100644 packages/twenty-server/src/engine/middlewares/rest-core.middleware.ts create mode 100644 packages/twenty-server/src/engine/middlewares/utils/graphql-token-validation-utils.ts create mode 100644 packages/twenty-server/test/integration/constants/mock-person-ids.constants.ts create mode 100644 packages/twenty-server/test/integration/constants/person-gql-fields.constants.ts create mode 100644 packages/twenty-server/test/integration/rest/suites/rest-api-core-delete.integration-spec.ts create mode 100644 packages/twenty-server/test/integration/rest/utils/make-rest-api-request.util.ts diff --git a/packages/twenty-e2e-testing/tests/authentication/fixture.ts b/packages/twenty-e2e-testing/tests/authentication/fixture.ts index 16cf35bec..c8e10221a 100644 --- a/packages/twenty-e2e-testing/tests/authentication/fixture.ts +++ b/packages/twenty-e2e-testing/tests/authentication/fixture.ts @@ -1,10 +1,10 @@ import { test as base } from '../../lib/fixtures/screenshot'; -import { LoginPage } from '../../lib/pom/loginPage'; +import { ConfirmationModal } from '../../lib/pom/helper/confirmationModal'; import { LeftMenu } from '../../lib/pom/leftMenu'; -import { SettingsPage } from '../../lib/pom/settingsPage'; +import { LoginPage } from '../../lib/pom/loginPage'; import { MembersSection } from '../../lib/pom/settings/membersSection'; import { ProfileSection } from '../../lib/pom/settings/profileSection'; -import { ConfirmationModal } from '../../lib/pom/helper/confirmationModal'; +import { SettingsPage } from '../../lib/pom/settingsPage'; type Fixtures = { confirmationModal: ConfirmationModal; diff --git a/packages/twenty-server/@types/jest.d.ts b/packages/twenty-server/@types/jest.d.ts index 225e29ff2..aefc412e2 100644 --- a/packages/twenty-server/@types/jest.d.ts +++ b/packages/twenty-server/@types/jest.d.ts @@ -5,6 +5,7 @@ declare module '@jest/types' { interface ConfigGlobals { APP_PORT: number; ACCESS_TOKEN: string; + EXPIRED_ACCESS_TOKEN: string; } } } @@ -12,6 +13,7 @@ declare module '@jest/types' { declare global { const APP_PORT: number; const ACCESS_TOKEN: string; + const EXPIRED_ACCESS_TOKEN: string; } export {}; diff --git a/packages/twenty-server/jest-integration.config.ts b/packages/twenty-server/jest-integration.config.ts index d8d0d975e..99b500c9c 100644 --- a/packages/twenty-server/jest-integration.config.ts +++ b/packages/twenty-server/jest-integration.config.ts @@ -36,6 +36,8 @@ const jestConfig: JestConfigWithTsJest = { APP_PORT: 4000, ACCESS_TOKEN: 'eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJzdWIiOiIyMDIwMjAyMC05ZTNiLTQ2ZDQtYTU1Ni04OGI5ZGRjMmIwMzQiLCJ3b3Jrc3BhY2VJZCI6IjIwMjAyMDIwLTFjMjUtNGQwMi1iZjI1LTZhZWNjZjdlYTQxOSIsIndvcmtzcGFjZU1lbWJlcklkIjoiMjAyMDIwMjAtMDY4Ny00YzQxLWI3MDctZWQxYmZjYTk3MmE3IiwiaWF0IjoxNzI2NDkyNTAyLCJleHAiOjEzMjQ1MDE2NTAyfQ._ISjY_dlVWskeQ6wkE0-kOn641G_mee5GiqoZTQFIfE', + EXPIRED_ACCESS_TOKEN: + 'eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJzdWIiOiIyMDIwMjAyMC05ZTNiLTQ2ZDQtYTU1Ni04OGI5ZGRjMmIwMzQiLCJ3b3Jrc3BhY2VJZCI6IjIwMjAyMDIwLTFjMjUtNGQwMi1iZjI1LTZhZWNjZjdlYTQxOSIsIndvcmtzcGFjZU1lbWJlcklkIjoiMjAyMDIwMjAtMDY4Ny00YzQxLWI3MDctZWQxYmZjYTk3MmE3IiwiaWF0IjoxNzM4MzIzODc5LCJleHAiOjE3MzgzMjU2Nzl9.m73hHVpnw5uGNGrSuKxn6XtKEUK3Wqkp4HsQdYfZiHo', }, }; diff --git a/packages/twenty-server/src/app.module.ts b/packages/twenty-server/src/app.module.ts index fa45e996d..45be22021 100644 --- a/packages/twenty-server/src/app.module.ts +++ b/packages/twenty-server/src/app.module.ts @@ -21,7 +21,11 @@ import { MetadataGraphQLApiModule } from 'src/engine/api/graphql/metadata-graphq import { RestApiModule } from 'src/engine/api/rest/rest-api.module'; import { MessageQueueDriverType } from 'src/engine/core-modules/message-queue/interfaces'; import { MessageQueueModule } from 'src/engine/core-modules/message-queue/message-queue.module'; +import { DataSourceModule } from 'src/engine/metadata-modules/data-source/data-source.module'; +import { WorkspaceMetadataCacheModule } from 'src/engine/metadata-modules/workspace-metadata-cache/workspace-metadata-cache.module'; import { GraphQLHydrateRequestFromTokenMiddleware } from 'src/engine/middlewares/graphql-hydrate-request-from-token.middleware'; +import { MiddlewareModule } from 'src/engine/middlewares/middleware.module'; +import { RestCoreMiddleware } from 'src/engine/middlewares/rest-core.middleware'; import { TwentyORMModule } from 'src/engine/twenty-orm/twenty-orm.module'; import { WorkspaceCacheStorageModule } from 'src/engine/workspace-cache-storage/workspace-cache-storage.module'; import { ModulesModule } from 'src/modules/modules.module'; @@ -29,6 +33,9 @@ import { ModulesModule } from 'src/modules/modules.module'; import { CoreEngineModule } from './engine/core-modules/core-engine.module'; import { I18nModule } from './engine/core-modules/i18n/i18n.module'; +// TODO: Remove this middleware when all the rest endpoints are migrated to TwentyORM +const MIGRATED_REST_METHODS = [RequestMethod.DELETE]; + @Module({ imports: [ SentryModule.forRoot(), @@ -52,6 +59,9 @@ import { I18nModule } from './engine/core-modules/i18n/i18n.module'; CoreGraphQLApiModule, MetadataGraphQLApiModule, RestApiModule, + DataSourceModule, + MiddlewareModule, + WorkspaceMetadataCacheModule, // I18n module for translations I18nModule, // Conditional modules @@ -89,5 +99,9 @@ export class AppModule { consumer .apply(GraphQLHydrateRequestFromTokenMiddleware) .forRoutes({ path: 'metadata', method: RequestMethod.ALL }); + + for (const method of MIGRATED_REST_METHODS) { + consumer.apply(RestCoreMiddleware).forRoutes({ path: 'rest/*', method }); + } } } diff --git a/packages/twenty-server/src/engine/api/rest/core/controllers/rest-api-core.controller.ts b/packages/twenty-server/src/engine/api/rest/core/controllers/rest-api-core.controller.ts index 488b03a26..d24477bfc 100644 --- a/packages/twenty-server/src/engine/api/rest/core/controllers/rest-api-core.controller.ts +++ b/packages/twenty-server/src/engine/api/rest/core/controllers/rest-api-core.controller.ts @@ -7,20 +7,26 @@ import { Put, Req, Res, + UseFilters, UseGuards, } from '@nestjs/common'; import { Request, Response } from 'express'; +import { RestApiCoreServiceV2 } from 'src/engine/api/rest/core/rest-api-core-v2.service'; import { RestApiCoreService } from 'src/engine/api/rest/core/rest-api-core.service'; import { cleanGraphQLResponse } from 'src/engine/api/rest/utils/clean-graphql-response.utils'; import { JwtAuthGuard } from 'src/engine/guards/jwt-auth.guard'; import { WorkspaceAuthGuard } from 'src/engine/guards/workspace-auth.guard'; +import { RestApiExceptionFilter } from 'src/engine/api/rest/rest-api-exception.filter'; @Controller('rest/*') @UseGuards(JwtAuthGuard, WorkspaceAuthGuard) export class RestApiCoreController { - constructor(private readonly restApiCoreService: RestApiCoreService) {} + constructor( + private readonly restApiCoreService: RestApiCoreService, + private readonly restApiCoreServiceV2: RestApiCoreServiceV2, + ) {} @Post('/duplicates') async handleApiFindDuplicates(@Req() request: Request, @Res() res: Response) { @@ -37,10 +43,13 @@ export class RestApiCoreController { } @Delete() + // We should move this exception filter to RestApiCoreController class level + // when all endpoints are migrated to v2 + @UseFilters(RestApiExceptionFilter) async handleApiDelete(@Req() request: Request, @Res() res: Response) { - const result = await this.restApiCoreService.delete(request); + const result = await this.restApiCoreServiceV2.delete(request); - res.status(200).send(cleanGraphQLResponse(result.data.data)); + res.status(200).send(result); } @Post() diff --git a/packages/twenty-server/src/engine/api/rest/core/rest-api-core-v2.service.ts b/packages/twenty-server/src/engine/api/rest/core/rest-api-core-v2.service.ts new file mode 100644 index 000000000..760d36c9b --- /dev/null +++ b/packages/twenty-server/src/engine/api/rest/core/rest-api-core-v2.service.ts @@ -0,0 +1,73 @@ +import { BadRequestException, Injectable } from '@nestjs/common'; + +import { Request } from 'express'; +import { capitalize } from 'twenty-shared'; + +import { CoreQueryBuilderFactory } from 'src/engine/api/rest/core/query-builder/core-query-builder.factory'; +import { parseCorePath } from 'src/engine/api/rest/core/query-builder/utils/path-parsers/parse-core-path.utils'; +import { TwentyORMGlobalManager } from 'src/engine/twenty-orm/twenty-orm-global.manager'; + +@Injectable() +export class RestApiCoreServiceV2 { + constructor( + private readonly coreQueryBuilderFactory: CoreQueryBuilderFactory, + private readonly twentyORMGlobalManager: TwentyORMGlobalManager, + ) {} + + async delete(request: Request) { + const { workspace } = request; + const { object: parsedObject, id: recordId } = parseCorePath(request); + + const objectMetadata = await this.coreQueryBuilderFactory.getObjectMetadata( + request, + parsedObject, + ); + + if (!objectMetadata) { + throw new BadRequestException('Object metadata not found'); + } + + if (!recordId) { + throw new BadRequestException('Record ID not found'); + } + + const objectMetadataNameSingular = + objectMetadata.objectMetadataItem.nameSingular; + + if (!workspace?.id) { + throw new BadRequestException('Workspace not found'); + } + + const repository = + await this.twentyORMGlobalManager.getRepositoryForWorkspace( + workspace.id, + objectMetadataNameSingular, + ); + + const recordToDelete = await repository.findOneOrFail({ + where: { + id: recordId, + }, + }); + + await repository.delete(recordId); + + return this.formatResult('delete', objectMetadataNameSingular, { + id: recordToDelete.id, + }); + } + + private formatResult( + operation: 'delete' | 'create' | 'update' | 'find', + objectNameSingular: string, + data: T, + ) { + const result = { + data: { + [operation + capitalize(objectNameSingular)]: data, + }, + }; + + return result; + } +} diff --git a/packages/twenty-server/src/engine/api/rest/rest-api-exception.filter.ts b/packages/twenty-server/src/engine/api/rest/rest-api-exception.filter.ts new file mode 100644 index 000000000..a94dfc577 --- /dev/null +++ b/packages/twenty-server/src/engine/api/rest/rest-api-exception.filter.ts @@ -0,0 +1,24 @@ +import { ArgumentsHost, Catch, ExceptionFilter } from '@nestjs/common'; + +import { Response } from 'express'; + +import { HttpExceptionHandlerService } from 'src/engine/core-modules/exception-handler/http-exception-handler.service'; +import { CustomException } from 'src/utils/custom-exception'; + +@Catch() +export class RestApiExceptionFilter implements ExceptionFilter { + constructor( + private readonly httpExceptionHandlerService: HttpExceptionHandlerService, + ) {} + + catch(exception: unknown, host: ArgumentsHost) { + const ctx = host.switchToHttp(); + const response = ctx.getResponse(); + + return this.httpExceptionHandlerService.handleError( + exception as CustomException, + response, + 400, + ); + } +} diff --git a/packages/twenty-server/src/engine/api/rest/rest-api.module.ts b/packages/twenty-server/src/engine/api/rest/rest-api.module.ts index f54d89537..a0f38ecb4 100644 --- a/packages/twenty-server/src/engine/api/rest/rest-api.module.ts +++ b/packages/twenty-server/src/engine/api/rest/rest-api.module.ts @@ -4,6 +4,7 @@ import { Module } from '@nestjs/common'; import { RestApiCoreBatchController } from 'src/engine/api/rest/core/controllers/rest-api-core-batch.controller'; import { RestApiCoreController } from 'src/engine/api/rest/core/controllers/rest-api-core.controller'; import { CoreQueryBuilderModule } from 'src/engine/api/rest/core/query-builder/core-query-builder.module'; +import { RestApiCoreServiceV2 } from 'src/engine/api/rest/core/rest-api-core-v2.service'; import { RestApiCoreService } from 'src/engine/api/rest/core/rest-api-core.service'; import { EndingBeforeInputFactory } from 'src/engine/api/rest/input-factories/ending-before-input.factory'; import { LimitInputFactory } from 'src/engine/api/rest/input-factories/limit-input.factory'; @@ -13,6 +14,7 @@ import { RestApiMetadataController } from 'src/engine/api/rest/metadata/rest-api import { RestApiMetadataService } from 'src/engine/api/rest/metadata/rest-api-metadata.service'; import { RestApiService } from 'src/engine/api/rest/rest-api.service'; import { AuthModule } from 'src/engine/core-modules/auth/auth.module'; +import { TwentyORMModule } from 'src/engine/twenty-orm/twenty-orm.module'; import { WorkspaceCacheStorageModule } from 'src/engine/workspace-cache-storage/workspace-cache-storage.module'; @Module({ @@ -22,6 +24,7 @@ import { WorkspaceCacheStorageModule } from 'src/engine/workspace-cache-storage/ WorkspaceCacheStorageModule, AuthModule, HttpModule, + TwentyORMModule, ], controllers: [ RestApiMetadataController, @@ -31,6 +34,7 @@ import { WorkspaceCacheStorageModule } from 'src/engine/workspace-cache-storage/ providers: [ RestApiMetadataService, RestApiCoreService, + RestApiCoreServiceV2, RestApiService, StartingAfterInputFactory, EndingBeforeInputFactory, diff --git a/packages/twenty-server/src/engine/core-modules/exception-handler/http-exception-handler.service.ts b/packages/twenty-server/src/engine/core-modules/exception-handler/http-exception-handler.service.ts index 45e32f44d..3aeb598a2 100644 --- a/packages/twenty-server/src/engine/core-modules/exception-handler/http-exception-handler.service.ts +++ b/packages/twenty-server/src/engine/core-modules/exception-handler/http-exception-handler.service.ts @@ -14,8 +14,10 @@ export const handleException = ( exceptionHandlerService: ExceptionHandlerService, user?: ExceptionHandlerUser, workspace?: ExceptionHandlerWorkspace, -): void => { +): CustomException => { exceptionHandlerService.captureExceptions([exception], { user, workspace }); + + return exception; }; interface RequestAndParams { @@ -45,7 +47,12 @@ export class HttpExceptionHandlerService { if (params?.userId) user = { ...user, id: params.userId }; handleException(exception, this.exceptionHandlerService, user, workspace); + const statusCode = errorCode || 500; - return response.status(errorCode || 500).send(exception.message); + return response.status(statusCode).send({ + statusCode, + error: exception.code || 'Bad Request', + messages: [exception?.message], + }); }; } diff --git a/packages/twenty-server/src/engine/middlewares/constants/default-error-message.constant.ts b/packages/twenty-server/src/engine/middlewares/constants/default-error-message.constant.ts new file mode 100644 index 000000000..1f1c07b70 --- /dev/null +++ b/packages/twenty-server/src/engine/middlewares/constants/default-error-message.constant.ts @@ -0,0 +1 @@ +export const INTERNAL_SERVER_ERROR = 'Internal server error'; diff --git a/packages/twenty-server/src/engine/middlewares/constants/excluded-middleware-operations.constant.ts b/packages/twenty-server/src/engine/middlewares/constants/excluded-middleware-operations.constant.ts new file mode 100644 index 000000000..fc5f340af --- /dev/null +++ b/packages/twenty-server/src/engine/middlewares/constants/excluded-middleware-operations.constant.ts @@ -0,0 +1,19 @@ +export const EXCLUDED_MIDDLEWARE_OPERATIONS = [ + 'GetClientConfig', + 'GetWorkspaceFromInviteHash', + 'Track', + 'CheckUserExists', + 'GetLoginTokenFromCredentials', + 'GetAuthTokensFromLoginToken', + 'GetLoginTokenFromEmailVerificationToken', + 'ResendEmailVerificationToken', + 'SignUp', + 'RenewToken', + 'EmailPasswordResetLink', + 'ValidatePasswordResetToken', + 'UpdatePasswordViaResetToken', + 'IntrospectionQuery', + 'ExchangeAuthorizationCode', + 'GetAuthorizationUrl', + 'GetPublicWorkspaceDataBySubdomain', +] as const; 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 4a986e832..5a4892610 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,114 +1,28 @@ import { Injectable, NestMiddleware } from '@nestjs/common'; import { NextFunction, Request, Response } from 'express'; -import { ExtractJwt } from 'passport-jwt'; -import { AuthGraphqlApiExceptionFilter } from 'src/engine/core-modules/auth/filters/auth-graphql-api-exception.filter'; -import { AccessTokenService } from 'src/engine/core-modules/auth/token/services/access-token.service'; -import { AuthContext } from 'src/engine/core-modules/auth/types/auth-context.type'; -import { ExceptionHandlerService } from 'src/engine/core-modules/exception-handler/exception-handler.service'; -import { handleExceptionAndConvertToGraphQLError } from 'src/engine/utils/global-exception-handler.util'; -import { WorkspaceCacheStorageService } from 'src/engine/workspace-cache-storage/workspace-cache-storage.service'; -class GraphqlTokenValidationProxy { - private accessTokenService: AccessTokenService; - - constructor(accessTokenService: AccessTokenService) { - this.accessTokenService = accessTokenService; - } - - async validateToken(req: Request) { - try { - return await this.accessTokenService.validateTokenByRequest(req); - } catch (error) { - const authGraphqlApiExceptionFilter = new AuthGraphqlApiExceptionFilter(); - - throw authGraphqlApiExceptionFilter.catch(error); - } - } -} +import { MiddlewareService } from 'src/engine/middlewares/middleware.service'; @Injectable() export class GraphQLHydrateRequestFromTokenMiddleware implements NestMiddleware { - constructor( - private readonly accessTokenService: AccessTokenService, - private readonly workspaceStorageCacheService: WorkspaceCacheStorageService, - private readonly exceptionHandlerService: ExceptionHandlerService, - ) {} + constructor(private readonly middlewareService: MiddlewareService) {} async use(req: Request, res: Response, next: NextFunction) { - const body = req.body; - - const excludedOperations = [ - 'GetClientConfig', - 'GetWorkspaceFromInviteHash', - 'Track', - 'CheckUserExists', - 'GetLoginTokenFromCredentials', - 'GetAuthTokensFromLoginToken', - 'GetLoginTokenFromEmailVerificationToken', - 'ResendEmailVerificationToken', - 'SignUp', - 'RenewToken', - 'EmailPasswordResetLink', - 'ValidatePasswordResetToken', - 'UpdatePasswordViaResetToken', - 'IntrospectionQuery', - 'ExchangeAuthorizationCode', - 'GetAuthorizationUrl', - 'GetPublicWorkspaceDataBySubdomain', - ]; - - if ( - !this.isTokenPresent(req) && - (!body?.operationName || excludedOperations.includes(body.operationName)) - ) { + if (this.middlewareService.checkUnauthenticatedAccess(req)) { return next(); } - let data: AuthContext; - try { - const graphqlTokenValidationProxy = new GraphqlTokenValidationProxy( - this.accessTokenService, - ); - - data = await graphqlTokenValidationProxy.validateToken(req); - const metadataVersion = - await this.workspaceStorageCacheService.getMetadataVersion( - data.workspace.id, - ); - - req.user = data.user; - req.apiKey = data.apiKey; - req.workspace = data.workspace; - req.workspaceId = data.workspace.id; - req.workspaceMetadataVersion = metadataVersion; - req.workspaceMemberId = data.workspaceMemberId; + await this.middlewareService.authenticateGraphqlRequest(req); } catch (error) { - res.writeHead(200, { 'Content-Type': 'application/json' }); - res.write( - JSON.stringify({ - errors: [ - handleExceptionAndConvertToGraphQLError( - error, - this.exceptionHandlerService, - ), - ], - }), - ); - res.end(); + this.middlewareService.writeGraphqlResponseOnExceptionCaught(res, error); return; } next(); } - - isTokenPresent(request: Request): boolean { - const token = ExtractJwt.fromAuthHeaderAsBearerToken()(request); - - return !!token; - } } diff --git a/packages/twenty-server/src/engine/middlewares/middleware.module.ts b/packages/twenty-server/src/engine/middlewares/middleware.module.ts new file mode 100644 index 000000000..2fd0b922c --- /dev/null +++ b/packages/twenty-server/src/engine/middlewares/middleware.module.ts @@ -0,0 +1,19 @@ +import { Module } from '@nestjs/common'; + +import { TokenModule } from 'src/engine/core-modules/auth/token/token.module'; +import { DataSourceModule } from 'src/engine/metadata-modules/data-source/data-source.module'; +import { WorkspaceMetadataCacheModule } from 'src/engine/metadata-modules/workspace-metadata-cache/workspace-metadata-cache.module'; +import { MiddlewareService } from 'src/engine/middlewares/middleware.service'; +import { WorkspaceCacheStorageModule } from 'src/engine/workspace-cache-storage/workspace-cache-storage.module'; + +@Module({ + imports: [ + DataSourceModule, + WorkspaceCacheStorageModule, + WorkspaceMetadataCacheModule, + TokenModule, + ], + providers: [MiddlewareService], + exports: [MiddlewareService], +}) +export class MiddlewareModule {} diff --git a/packages/twenty-server/src/engine/middlewares/middleware.service.ts b/packages/twenty-server/src/engine/middlewares/middleware.service.ts new file mode 100644 index 000000000..1fa56e476 --- /dev/null +++ b/packages/twenty-server/src/engine/middlewares/middleware.service.ts @@ -0,0 +1,167 @@ +import { Injectable } from '@nestjs/common'; + +import { Request, Response } from 'express'; +import { ExtractJwt } from 'passport-jwt'; + +import { AuthExceptionCode } from 'src/engine/core-modules/auth/auth.exception'; +import { AccessTokenService } from 'src/engine/core-modules/auth/token/services/access-token.service'; +import { AuthContext } from 'src/engine/core-modules/auth/types/auth-context.type'; +import { ExceptionHandlerService } from 'src/engine/core-modules/exception-handler/exception-handler.service'; +import { ErrorCode } from 'src/engine/core-modules/graphql/utils/graphql-errors.util'; +import { DataSourceService } from 'src/engine/metadata-modules/data-source/data-source.service'; +import { WorkspaceMetadataCacheService } from 'src/engine/metadata-modules/workspace-metadata-cache/services/workspace-metadata-cache.service'; +import { INTERNAL_SERVER_ERROR } from 'src/engine/middlewares/constants/default-error-message.constant'; +import { EXCLUDED_MIDDLEWARE_OPERATIONS } from 'src/engine/middlewares/constants/excluded-middleware-operations.constant'; +import { GraphqlTokenValidationProxy } from 'src/engine/middlewares/utils/graphql-token-validation-utils'; +import { + handleException, + handleExceptionAndConvertToGraphQLError, +} from 'src/engine/utils/global-exception-handler.util'; +import { WorkspaceCacheStorageService } from 'src/engine/workspace-cache-storage/workspace-cache-storage.service'; +import { CustomException } from 'src/utils/custom-exception'; +import { isDefined } from 'src/utils/is-defined'; + +@Injectable() +export class MiddlewareService { + constructor( + private readonly accessTokenService: AccessTokenService, + private readonly workspaceStorageCacheService: WorkspaceCacheStorageService, + private readonly workspaceMetadataCacheService: WorkspaceMetadataCacheService, + private readonly dataSourceService: DataSourceService, + private readonly exceptionHandlerService: ExceptionHandlerService, + ) {} + + private excludedOperations = EXCLUDED_MIDDLEWARE_OPERATIONS; + + public isTokenPresent(request: Request): boolean { + const token = ExtractJwt.fromAuthHeaderAsBearerToken()(request); + + return !!token; + } + + public checkUnauthenticatedAccess(request: Request): boolean { + const { body } = request; + + const isUserUnauthenticated = !this.isTokenPresent(request); + const isExcludedOperation = + !body?.operationName || + this.excludedOperations.includes(body.operationName); + + return isUserUnauthenticated && isExcludedOperation; + } + + public writeRestResponseOnExceptionCaught(res: Response, error: any) { + // capture and handle custom exceptions + handleException(error as CustomException, this.exceptionHandlerService); + + const statusCode = this.getStatus(error); + + res.writeHead(statusCode, { 'Content-Type': 'application/json' }); + res.write( + JSON.stringify({ + statusCode, + messages: [error?.message || INTERNAL_SERVER_ERROR], + error: error?.code || ErrorCode.INTERNAL_SERVER_ERROR, + }), + ); + + res.end(); + } + + public writeGraphqlResponseOnExceptionCaught(res: Response, error: any) { + const errors = [ + handleExceptionAndConvertToGraphQLError( + error as Error, + this.exceptionHandlerService, + ), + ]; + + const statusCode = 200; + + res.writeHead(statusCode, { + 'Content-Type': 'application/json', + }); + + res.write( + JSON.stringify({ + errors, + }), + ); + + res.end(); + } + + public async authenticateRestRequest(request: Request) { + const data = await this.accessTokenService.validateTokenByRequest(request); + const metadataVersion = + await this.workspaceStorageCacheService.getMetadataVersion( + data.workspace.id, + ); + + if (metadataVersion === undefined) { + await this.workspaceMetadataCacheService.recomputeMetadataCache({ + workspaceId: data.workspace.id, + }); + throw new Error('Metadata cache version not found'); + } + + const dataSourcesMetadata = + await this.dataSourceService.getDataSourcesMetadataFromWorkspaceId( + data.workspace.id, + ); + + if (!dataSourcesMetadata || dataSourcesMetadata.length === 0) { + throw new Error('No data sources found'); + } + + this.bindDataToRequestObject(data, request, metadataVersion); + } + + public async authenticateGraphqlRequest(request: Request) { + const graphqlTokenValidationProxy = new GraphqlTokenValidationProxy( + this.accessTokenService, + ); + + const data = await graphqlTokenValidationProxy.validateToken(request); + const metadataVersion = + await this.workspaceStorageCacheService.getMetadataVersion( + data.workspace.id, + ); + + this.bindDataToRequestObject(data, request, metadataVersion); + } + + private hasErrorStatus(error: unknown): error is { status: number } { + return isDefined((error as { status: number })?.status); + } + + private bindDataToRequestObject( + data: AuthContext, + request: Request, + metadataVersion: number | undefined, + ) { + request.user = data.user; + request.apiKey = data.apiKey; + request.workspace = data.workspace; + request.workspaceId = data.workspace.id; + request.workspaceMetadataVersion = metadataVersion; + request.workspaceMemberId = data.workspaceMemberId; + } + + private getStatus(error: any): number { + if (this.hasErrorStatus(error)) { + return error.status; + } + + if (error instanceof CustomException) { + switch (error.code) { + case AuthExceptionCode.UNAUTHENTICATED: + return 401; + default: + return 400; + } + } + + return 500; + } +} diff --git a/packages/twenty-server/src/engine/middlewares/rest-core.middleware.ts b/packages/twenty-server/src/engine/middlewares/rest-core.middleware.ts new file mode 100644 index 000000000..420bb6ee5 --- /dev/null +++ b/packages/twenty-server/src/engine/middlewares/rest-core.middleware.ts @@ -0,0 +1,22 @@ +import { Injectable, NestMiddleware } from '@nestjs/common'; + +import { NextFunction, Request, Response } from 'express'; + +import { MiddlewareService } from 'src/engine/middlewares/middleware.service'; + +@Injectable() +export class RestCoreMiddleware implements NestMiddleware { + constructor(private readonly middlewareService: MiddlewareService) {} + + async use(req: Request, res: Response, next: NextFunction) { + try { + await this.middlewareService.authenticateRestRequest(req); + } catch (error) { + this.middlewareService.writeRestResponseOnExceptionCaught(res, error); + + return; + } + + next(); + } +} diff --git a/packages/twenty-server/src/engine/middlewares/utils/graphql-token-validation-utils.ts b/packages/twenty-server/src/engine/middlewares/utils/graphql-token-validation-utils.ts new file mode 100644 index 000000000..3f2d41c2c --- /dev/null +++ b/packages/twenty-server/src/engine/middlewares/utils/graphql-token-validation-utils.ts @@ -0,0 +1,22 @@ +import { Request } from 'express'; + +import { AuthGraphqlApiExceptionFilter } from 'src/engine/core-modules/auth/filters/auth-graphql-api-exception.filter'; +import { AccessTokenService } from 'src/engine/core-modules/auth/token/services/access-token.service'; + +export class GraphqlTokenValidationProxy { + private accessTokenService: AccessTokenService; + + constructor(accessTokenService: AccessTokenService) { + this.accessTokenService = accessTokenService; + } + + async validateToken(req: Request) { + try { + return await this.accessTokenService.validateTokenByRequest(req); + } catch (error) { + const authGraphqlApiExceptionFilter = new AuthGraphqlApiExceptionFilter(); + + throw authGraphqlApiExceptionFilter.catch(error); + } + } +} diff --git a/packages/twenty-server/src/engine/utils/global-exception-handler.util.ts b/packages/twenty-server/src/engine/utils/global-exception-handler.util.ts index 24e8dbfa7..0edd33762 100644 --- a/packages/twenty-server/src/engine/utils/global-exception-handler.util.ts +++ b/packages/twenty-server/src/engine/utils/global-exception-handler.util.ts @@ -72,7 +72,7 @@ export const shouldFilterException = (exception: Error): boolean => { return false; }; -const handleException = ( +export const handleException = ( exception: Error, exceptionHandlerService: ExceptionHandlerService, user?: ExceptionHandlerUser, diff --git a/packages/twenty-server/test/integration/constants/mock-person-ids.constants.ts b/packages/twenty-server/test/integration/constants/mock-person-ids.constants.ts new file mode 100644 index 000000000..c81e4c826 --- /dev/null +++ b/packages/twenty-server/test/integration/constants/mock-person-ids.constants.ts @@ -0,0 +1,4 @@ +export const PERSON_1_ID = '777a8457-eb2d-40ac-a707-551b615b6987'; +export const PERSON_2_ID = '777a8457-eb2d-40ac-a707-551b615b6988'; +export const PERSON_3_ID = '777a8457-eb2d-40ac-a707-551b615b6989'; +export const FAKE_PERSON_ID = '777a8457-eb2d-40ac-a707-551b615b6990'; diff --git a/packages/twenty-server/test/integration/constants/person-gql-fields.constants.ts b/packages/twenty-server/test/integration/constants/person-gql-fields.constants.ts new file mode 100644 index 000000000..5c6e0deee --- /dev/null +++ b/packages/twenty-server/test/integration/constants/person-gql-fields.constants.ts @@ -0,0 +1,14 @@ +export const PERSON_GQL_FIELDS = ` + id + city + jobTitle + avatarUrl + intro + searchVector + name { + firstName + lastName + } + createdAt + deletedAt +`; diff --git a/packages/twenty-server/test/integration/graphql/suites/all-people-resolvers.integration-spec.ts b/packages/twenty-server/test/integration/graphql/suites/all-people-resolvers.integration-spec.ts index e5a5ce5c7..b1ce7b7bb 100644 --- a/packages/twenty-server/test/integration/graphql/suites/all-people-resolvers.integration-spec.ts +++ b/packages/twenty-server/test/integration/graphql/suites/all-people-resolvers.integration-spec.ts @@ -1,3 +1,9 @@ +import { + PERSON_1_ID, + PERSON_2_ID, + PERSON_3_ID, +} from 'test/integration/constants/mock-person-ids.constants'; +import { PERSON_GQL_FIELDS } from 'test/integration/constants/person-gql-fields.constants'; import { createManyOperationFactory } from 'test/integration/graphql/utils/create-many-operation-factory.util'; import { createOneOperationFactory } from 'test/integration/graphql/utils/create-one-operation-factory.util'; import { deleteManyOperationFactory } from 'test/integration/graphql/utils/delete-many-operation-factory.util'; @@ -11,25 +17,6 @@ import { updateManyOperationFactory } from 'test/integration/graphql/utils/updat import { updateOneOperationFactory } from 'test/integration/graphql/utils/update-one-operation-factory.util'; import { generateRecordName } from 'test/integration/utils/generate-record-name'; -const PERSON_1_ID = '777a8457-eb2d-40ac-a707-551b615b6987'; -const PERSON_2_ID = '777a8457-eb2d-40ac-a707-551b615b6988'; -const PERSON_3_ID = '777a8457-eb2d-40ac-a707-551b615b6989'; - -const PERSON_GQL_FIELDS = ` - id - city - jobTitle - avatarUrl - intro - searchVector - name { - firstName - lastName - } - createdAt - deletedAt -`; - describe('people resolvers (integration)', () => { it('1. should create and return people', async () => { const personCity1 = generateRecordName(PERSON_1_ID); diff --git a/packages/twenty-server/test/integration/rest/suites/rest-api-core-delete.integration-spec.ts b/packages/twenty-server/test/integration/rest/suites/rest-api-core-delete.integration-spec.ts new file mode 100644 index 000000000..54e4add3d --- /dev/null +++ b/packages/twenty-server/test/integration/rest/suites/rest-api-core-delete.integration-spec.ts @@ -0,0 +1,107 @@ +import { + FAKE_PERSON_ID, + PERSON_1_ID, +} from 'test/integration/constants/mock-person-ids.constants'; +import { PERSON_GQL_FIELDS } from 'test/integration/constants/person-gql-fields.constants'; +import { createManyOperationFactory } from 'test/integration/graphql/utils/create-many-operation-factory.util'; +import { findOneOperationFactory } from 'test/integration/graphql/utils/find-one-operation-factory.util'; +import { makeGraphqlAPIRequest } from 'test/integration/graphql/utils/make-graphql-api-request.util'; +import { makeRestAPIRequest } from 'test/integration/rest/utils/make-rest-api-request.util'; +import { generateRecordName } from 'test/integration/utils/generate-record-name'; + +describe('Core REST API Delete One endpoint', () => { + let people: any; + + beforeAll(async () => { + const personCity1 = generateRecordName(PERSON_1_ID); + + // TODO: move this creation to REST API when the POST method is migrated + const graphqlOperation = createManyOperationFactory({ + objectMetadataSingularName: 'person', + objectMetadataPluralName: 'people', + gqlFields: PERSON_GQL_FIELDS, + data: [ + { + id: PERSON_1_ID, + city: personCity1, + }, + ], + }); + + const response = await makeGraphqlAPIRequest(graphqlOperation); + + people = response.body.data.createPeople; + expect(people.length).toBe(1); + expect(people[0].id).toBe(PERSON_1_ID); + }); + + afterAll(async () => { + // TODO: move this creation to REST API when the GET method is migrated + const graphqlOperation = findOneOperationFactory({ + objectMetadataSingularName: 'person', + gqlFields: PERSON_GQL_FIELDS, + filter: { + id: { + eq: PERSON_1_ID, + }, + }, + }); + + await makeGraphqlAPIRequest(graphqlOperation) + .expect(400) + .expect((res) => { + expect(res.body.error.message).toContain(`Record not found`); + }); + }); + + it('1a. should delete one person', async () => { + const response = await makeRestAPIRequest( + 'delete', + `/people/${PERSON_1_ID}`, + ); + + expect(response.body.data.deletePerson.id).toBe(PERSON_1_ID); + }); + + it('1.b. should return a BadRequestException when trying to delete a non-existing person', async () => { + await makeRestAPIRequest('delete', `/people/${FAKE_PERSON_ID}`) + .expect(400) + .expect((res) => { + expect(res.body.messages[0]).toContain( + `Could not find any entity of type "person"`, + ); + expect(res.body.error).toBe('Bad Request'); + }); + }); + + it('1.c. should return an UnauthorizedException when no token is provided', async () => { + await makeRestAPIRequest('delete', `/people/${PERSON_1_ID}`, { + authorization: '', + }) + .expect(401) + .expect((res) => { + expect(res.body.error).toBe('UNAUTHENTICATED'); + }); + }); + + it('1.d. should return an UnauthorizedException when an invalid token is provided', async () => { + await makeRestAPIRequest('delete', `/people/${PERSON_1_ID}`, { + authorization: 'Bearer invalid-token', + }) + .expect(401) + .expect((res) => { + expect(res.body.error).toBe('UNAUTHENTICATED'); + }); + }); + + it('1.e. should return an UnauthorizedException when an expired token is provided', async () => { + await makeRestAPIRequest('delete', `/people/${PERSON_1_ID}`, { + authorization: `Bearer ${EXPIRED_ACCESS_TOKEN}`, + }) + .expect(401) + .expect((res) => { + expect(res.body.error).toBe('UNAUTHENTICATED'); + expect(res.body.messages[0]).toBe('Token has expired.'); // Adjust this based on your API's error response + }); + }); +}); diff --git a/packages/twenty-server/test/integration/rest/utils/make-rest-api-request.util.ts b/packages/twenty-server/test/integration/rest/utils/make-rest-api-request.util.ts new file mode 100644 index 000000000..6fd5f5912 --- /dev/null +++ b/packages/twenty-server/test/integration/rest/utils/make-rest-api-request.util.ts @@ -0,0 +1,18 @@ +import { IncomingHttpHeaders } from 'http'; + +import request from 'supertest'; + +export type RestAPIRequestMethod = 'get' | 'post' | 'put' | 'patch' | 'delete'; + +export const makeRestAPIRequest = ( + method: RestAPIRequestMethod, + path: string, + headers: IncomingHttpHeaders = {}, +) => { + const client = request(`http://localhost:${APP_PORT}`); + + return client[method]('/rest' + path) + .set('Authorization', `Bearer ${ACCESS_TOKEN}`) + .set({ ...headers }) + .send(); +};