From 754de411fe22a240b5c3742dbfbf6266ac98834d Mon Sep 17 00:00:00 2001 From: martmull Date: Thu, 19 Feb 2026 14:00:52 +0100 Subject: [PATCH] Factorize and add public-assets/*path endpoint (#18080) as title --- .../file/controllers/file-by-id.controller.ts | 78 ----- .../file/controllers/file.controller.spec.ts | 289 +++++++++++++++--- .../file/controllers/file.controller.ts | 102 ++++++- .../engine/core-modules/file/file.module.ts | 3 +- .../file/services/file.service.ts | 68 +++-- 5 files changed, 402 insertions(+), 138 deletions(-) delete mode 100644 packages/twenty-server/src/engine/core-modules/file/controllers/file-by-id.controller.ts diff --git a/packages/twenty-server/src/engine/core-modules/file/controllers/file-by-id.controller.ts b/packages/twenty-server/src/engine/core-modules/file/controllers/file-by-id.controller.ts deleted file mode 100644 index cf0cd23aa2..0000000000 --- a/packages/twenty-server/src/engine/core-modules/file/controllers/file-by-id.controller.ts +++ /dev/null @@ -1,78 +0,0 @@ -import { - Controller, - Get, - Param, - Req, - Res, - UseFilters, - UseGuards, -} from '@nestjs/common'; - -import { Request, Response } from 'express'; - -import { - FileStorageException, - FileStorageExceptionCode, -} from 'src/engine/core-modules/file-storage/interfaces/file-storage-exception'; - -import { - FileException, - FileExceptionCode, -} from 'src/engine/core-modules/file/file.exception'; -import { FileApiExceptionFilter } from 'src/engine/core-modules/file/filters/file-api-exception.filter'; -import { - FileByIdGuard, - SupportedFileFolder, -} from 'src/engine/core-modules/file/guards/file-by-id.guard'; -import { FileService } from 'src/engine/core-modules/file/services/file.service'; -import { NoPermissionGuard } from 'src/engine/guards/no-permission.guard'; - -@Controller('file') -@UseFilters(FileApiExceptionFilter) -export class FileByIdController { - constructor(private readonly fileService: FileService) {} - - @Get(':fileFolder/:id') - @UseGuards(FileByIdGuard, NoPermissionGuard) - async getFileById( - @Res() res: Response, - @Req() req: Request, - @Param('fileFolder') fileFolder: SupportedFileFolder, - @Param('id') fileId: string, - ) { - // eslint-disable-next-line @typescript-eslint/no-explicit-any - const workspaceId = (req as any)?.workspaceId; - - try { - const fileStream = await this.fileService.getFileStreamById({ - fileId, - workspaceId, - fileFolder, - }); - - fileStream.on('error', () => { - throw new FileException( - 'Error streaming file from storage', - FileExceptionCode.INTERNAL_SERVER_ERROR, - ); - }); - - fileStream.pipe(res); - } catch (error) { - if ( - error instanceof FileStorageException && - error.code === FileStorageExceptionCode.FILE_NOT_FOUND - ) { - throw new FileException( - 'File not found', - FileExceptionCode.FILE_NOT_FOUND, - ); - } - - throw new FileException( - `Error retrieving file: ${error.message}`, - FileExceptionCode.INTERNAL_SERVER_ERROR, - ); - } - } -} diff --git a/packages/twenty-server/src/engine/core-modules/file/controllers/file.controller.spec.ts b/packages/twenty-server/src/engine/core-modules/file/controllers/file.controller.spec.ts index c77236b261..f0772dd05d 100644 --- a/packages/twenty-server/src/engine/core-modules/file/controllers/file.controller.spec.ts +++ b/packages/twenty-server/src/engine/core-modules/file/controllers/file.controller.spec.ts @@ -3,16 +3,47 @@ import { Test, type TestingModule } from '@nestjs/testing'; import { Readable } from 'stream'; +import { FileFolder } from 'twenty-shared/types'; + +import { + FileStorageException, + FileStorageExceptionCode, +} from 'src/engine/core-modules/file-storage/interfaces/file-storage-exception'; + +import { + FileException, + FileExceptionCode, +} from 'src/engine/core-modules/file/file.exception'; import { FileApiExceptionFilter } from 'src/engine/core-modules/file/filters/file-api-exception.filter'; import { FilePathGuard } from 'src/engine/core-modules/file/guards/file-path-guard'; +import { FileByIdGuard } from 'src/engine/core-modules/file/guards/file-by-id.guard'; import { FileService } from 'src/engine/core-modules/file/services/file.service'; +import { PublicEndpointGuard } from 'src/engine/guards/public-endpoint.guard'; +import { NoPermissionGuard } from 'src/engine/guards/no-permission.guard'; import { FileController } from './file.controller'; +const createMockStream = (): Readable => { + const stream = new Readable(); + + stream.push('file content'); + stream.push(null); + stream.pipe = jest.fn(); + + return stream; +}; + describe('FileController', () => { let controller: FileController; let fileService: FileService; const mock_FilePathGuard: CanActivate = { canActivate: jest.fn(() => true) }; + const mock_FileByIdGuard: CanActivate = { canActivate: jest.fn(() => true) }; + const mock_PublicEndpointGuard: CanActivate = { + canActivate: jest.fn(() => true), + }; + const mock_NoPermissionGuard: CanActivate = { + canActivate: jest.fn(() => true), + }; beforeEach(async () => { const module: TestingModule = await Test.createTestingModule({ @@ -22,12 +53,20 @@ describe('FileController', () => { provide: FileService, useValue: { getFileStream: jest.fn(), + getFileStreamById: jest.fn(), + getFileStreamByPath: jest.fn(), }, }, ], }) .overrideGuard(FilePathGuard) .useValue(mock_FilePathGuard) + .overrideGuard(FileByIdGuard) + .useValue(mock_FileByIdGuard) + .overrideGuard(PublicEndpointGuard) + .useValue(mock_PublicEndpointGuard) + .overrideGuard(NoPermissionGuard) + .useValue(mock_NoPermissionGuard) .overrideFilter(FileApiExceptionFilter) .useValue({}) .compile(); @@ -40,53 +79,235 @@ describe('FileController', () => { expect(controller).toBeDefined(); }); - it('should extract folder, token and filename from 3-segment path', async () => { - const mockStream = new Readable(); + describe('getFile', () => { + it('should extract folder, token and filename from 3-segment path', async () => { + const mockStream = createMockStream(); - mockStream.push('file content'); - mockStream.push(null); - mockStream.pipe = jest.fn(); + jest.spyOn(fileService, 'getFileStream').mockResolvedValue(mockStream); - jest.spyOn(fileService, 'getFileStream').mockResolvedValue(mockStream); + const mockRequest = { + path: '/files/attachment/test-token/test-file.csv', + workspaceId: 'workspace-id', + } as any; - const mockRequest = { - path: '/files/attachment/test-token/test-file.csv', - workspaceId: 'workspace-id', - } as any; + const mockResponse = {} as any; - const mockResponse = {} as any; + await controller.getFile(mockResponse, mockRequest); - await controller.getFile(mockResponse, mockRequest); + expect(fileService.getFileStream).toHaveBeenCalledWith( + 'attachment', + 'test-file.csv', + 'workspace-id', + ); + }); - expect(fileService.getFileStream).toHaveBeenCalledWith( - 'attachment', - 'test-file.csv', - 'workspace-id', - ); + it('should extract folder with size, token and filename from 4-segment path', async () => { + const mockStream = createMockStream(); + + jest.spyOn(fileService, 'getFileStream').mockResolvedValue(mockStream); + + const mockRequest = { + path: '/files/profile-picture/original/test-token/avatar.jpg', + workspaceId: 'workspace-id', + } as any; + + const mockResponse = {} as any; + + await controller.getFile(mockResponse, mockRequest); + + expect(fileService.getFileStream).toHaveBeenCalledWith( + 'profile-picture/original', + 'avatar.jpg', + 'workspace-id', + ); + }); }); - it('should extract folder with size, token and filename from 4-segment path', async () => { - const mockStream = new Readable(); + describe('getFileById', () => { + it('should call fileService.getFileStreamById and pipe the result', async () => { + const mockStream = createMockStream(); - mockStream.push('file content'); - mockStream.push(null); - mockStream.pipe = jest.fn(); + jest + .spyOn(fileService, 'getFileStreamById') + .mockResolvedValue(mockStream); - jest.spyOn(fileService, 'getFileStream').mockResolvedValue(mockStream); + const mockRequest = { workspaceId: 'workspace-id' } as any; + const mockResponse = {} as any; - const mockRequest = { - path: '/files/profile-picture/original/test-token/avatar.jpg', - workspaceId: 'workspace-id', - } as any; + await controller.getFileById( + mockResponse, + mockRequest, + FileFolder.CorePicture, + 'file-123', + ); - const mockResponse = {} as any; + expect(fileService.getFileStreamById).toHaveBeenCalledWith({ + fileId: 'file-123', + workspaceId: 'workspace-id', + fileFolder: FileFolder.CorePicture, + }); + expect(mockStream.pipe).toHaveBeenCalledWith(mockResponse); + }); - await controller.getFile(mockResponse, mockRequest); + it('should throw FileException with FILE_NOT_FOUND when file is not found', async () => { + jest + .spyOn(fileService, 'getFileStreamById') + .mockRejectedValue( + new FileStorageException( + 'File not found', + FileStorageExceptionCode.FILE_NOT_FOUND, + ), + ); - expect(fileService.getFileStream).toHaveBeenCalledWith( - 'profile-picture/original', - 'avatar.jpg', - 'workspace-id', - ); + const mockRequest = { workspaceId: 'workspace-id' } as any; + const mockResponse = {} as any; + + await expect( + controller.getFileById( + mockResponse, + mockRequest, + FileFolder.FilesField, + 'missing-file', + ), + ).rejects.toThrow( + new FileException('File not found', FileExceptionCode.FILE_NOT_FOUND), + ); + }); + + it('should throw FileException with INTERNAL_SERVER_ERROR for unexpected errors', async () => { + jest + .spyOn(fileService, 'getFileStreamById') + .mockRejectedValue(new Error('Storage unavailable')); + + const mockRequest = { workspaceId: 'workspace-id' } as any; + const mockResponse = {} as any; + + await expect( + controller.getFileById( + mockResponse, + mockRequest, + FileFolder.Workflow, + 'file-456', + ), + ).rejects.toThrow( + new FileException( + 'Error retrieving file: Storage unavailable', + FileExceptionCode.INTERNAL_SERVER_ERROR, + ), + ); + }); + }); + + describe('getPublicAssets', () => { + it('should call fileService.getFileStreamByPath and pipe the result', async () => { + const mockStream = createMockStream(); + + jest + .spyOn(fileService, 'getFileStreamByPath') + .mockResolvedValue(mockStream); + + const mockRequest = { + params: { path: ['images', 'logo.png'] }, + } as any; + + const mockResponse = {} as any; + + await controller.getPublicAssets( + mockResponse, + mockRequest, + 'workspace-id', + 'app-id', + ); + + expect(fileService.getFileStreamByPath).toHaveBeenCalledWith({ + workspaceId: 'workspace-id', + applicationId: 'app-id', + fileFolder: FileFolder.PublicAsset, + filepath: 'images/logo.png', + }); + expect(mockStream.pipe).toHaveBeenCalledWith(mockResponse); + }); + + it('should handle single-segment path', async () => { + const mockStream = createMockStream(); + + jest + .spyOn(fileService, 'getFileStreamByPath') + .mockResolvedValue(mockStream); + + const mockRequest = { + params: { path: ['favicon.ico'] }, + } as any; + + const mockResponse = {} as any; + + await controller.getPublicAssets( + mockResponse, + mockRequest, + 'workspace-id', + 'app-id', + ); + + expect(fileService.getFileStreamByPath).toHaveBeenCalledWith({ + workspaceId: 'workspace-id', + applicationId: 'app-id', + fileFolder: FileFolder.PublicAsset, + filepath: 'favicon.ico', + }); + }); + + it('should throw FileException with FILE_NOT_FOUND when asset is not found', async () => { + jest + .spyOn(fileService, 'getFileStreamByPath') + .mockRejectedValue( + new FileStorageException( + 'File not found', + FileStorageExceptionCode.FILE_NOT_FOUND, + ), + ); + + const mockRequest = { + params: { path: ['missing-asset.png'] }, + } as any; + + const mockResponse = {} as any; + + await expect( + controller.getPublicAssets( + mockResponse, + mockRequest, + 'workspace-id', + 'app-id', + ), + ).rejects.toThrow( + new FileException('File not found', FileExceptionCode.FILE_NOT_FOUND), + ); + }); + + it('should throw FileException with INTERNAL_SERVER_ERROR for unexpected errors', async () => { + jest + .spyOn(fileService, 'getFileStreamByPath') + .mockRejectedValue(new Error('Connection refused')); + + const mockRequest = { + params: { path: ['broken-asset.png'] }, + } as any; + + const mockResponse = {} as any; + + await expect( + controller.getPublicAssets( + mockResponse, + mockRequest, + 'workspace-id', + 'app-id', + ), + ).rejects.toThrow( + new FileException( + 'Error retrieving file: Connection refused', + FileExceptionCode.INTERNAL_SERVER_ERROR, + ), + ); + }); }); }); diff --git a/packages/twenty-server/src/engine/core-modules/file/controllers/file.controller.ts b/packages/twenty-server/src/engine/core-modules/file/controllers/file.controller.ts index e4a1c14f8a..65f7439168 100644 --- a/packages/twenty-server/src/engine/core-modules/file/controllers/file.controller.ts +++ b/packages/twenty-server/src/engine/core-modules/file/controllers/file.controller.ts @@ -1,13 +1,17 @@ import { Controller, Get, + Param, Req, Res, UseFilters, UseGuards, } from '@nestjs/common'; +import { join } from 'path'; + import { Request, Response } from 'express'; +import { FileFolder } from 'twenty-shared/types'; import { FileStorageException, @@ -23,13 +27,63 @@ import { FilePathGuard } from 'src/engine/core-modules/file/guards/file-path-gua import { FileService } from 'src/engine/core-modules/file/services/file.service'; import { extractFileInfoFromRequest } from 'src/engine/core-modules/file/utils/extract-file-info-from-request.utils'; import { NoPermissionGuard } from 'src/engine/guards/no-permission.guard'; +import { PublicEndpointGuard } from 'src/engine/guards/public-endpoint.guard'; +import { + FileByIdGuard, + SupportedFileFolder, +} from 'src/engine/core-modules/file/guards/file-by-id.guard'; -@Controller('files') +@Controller() @UseFilters(FileApiExceptionFilter) export class FileController { constructor(private readonly fileService: FileService) {} - @Get('*path') + @Get('public-assets/:workspaceId/:applicationId/*path') + @UseGuards(PublicEndpointGuard, NoPermissionGuard) + async getPublicAssets( + @Res() res: Response, + @Req() req: Request, + @Param('workspaceId') workspaceId: string, + @Param('applicationId') + applicationId: string, + ) { + const filepath = join(...req.params.path); + + try { + const fileStream = await this.fileService.getFileStreamByPath({ + workspaceId, + applicationId, + fileFolder: FileFolder.PublicAsset, + filepath, + }); + + fileStream.on('error', () => { + throw new FileException( + 'Error streaming file from storage', + FileExceptionCode.INTERNAL_SERVER_ERROR, + ); + }); + + fileStream.pipe(res); + } catch (error) { + if ( + error instanceof FileStorageException && + error.code === FileStorageExceptionCode.FILE_NOT_FOUND + ) { + throw new FileException( + 'File not found', + FileExceptionCode.FILE_NOT_FOUND, + ); + } + + throw new FileException( + `Error retrieving file: ${error.message}`, + FileExceptionCode.INTERNAL_SERVER_ERROR, + ); + } + } + + @Get('files/*path') @UseGuards(FilePathGuard, NoPermissionGuard) async getFile(@Res() res: Response, @Req() req: Request) { // eslint-disable-next-line @typescript-eslint/no-explicit-any @@ -69,4 +123,48 @@ export class FileController { ); } } + + @Get('file/:fileFolder/:id') + @UseGuards(FileByIdGuard, NoPermissionGuard) + async getFileById( + @Res() res: Response, + @Req() req: Request, + @Param('fileFolder') fileFolder: SupportedFileFolder, + @Param('id') fileId: string, + ) { + // eslint-disable-next-line @typescript-eslint/no-explicit-any + const workspaceId = (req as any)?.workspaceId; + + try { + const fileStream = await this.fileService.getFileStreamById({ + fileId, + workspaceId, + fileFolder, + }); + + fileStream.on('error', () => { + throw new FileException( + 'Error streaming file from storage', + FileExceptionCode.INTERNAL_SERVER_ERROR, + ); + }); + + fileStream.pipe(res); + } catch (error) { + if ( + error instanceof FileStorageException && + error.code === FileStorageExceptionCode.FILE_NOT_FOUND + ) { + throw new FileException( + 'File not found', + FileExceptionCode.FILE_NOT_FOUND, + ); + } + + throw new FileException( + `Error retrieving file: ${error.message}`, + FileExceptionCode.INTERNAL_SERVER_ERROR, + ); + } + } } diff --git a/packages/twenty-server/src/engine/core-modules/file/file.module.ts b/packages/twenty-server/src/engine/core-modules/file/file.module.ts index 454dc96e89..394f195db5 100644 --- a/packages/twenty-server/src/engine/core-modules/file/file.module.ts +++ b/packages/twenty-server/src/engine/core-modules/file/file.module.ts @@ -13,7 +13,6 @@ import { SecureHttpClientModule } from 'src/engine/core-modules/secure-http-clie import { WorkspaceEntity } from 'src/engine/core-modules/workspace/workspace.entity'; import { PermissionsModule } from 'src/engine/metadata-modules/permissions/permissions.module'; -import { FileByIdController } from './controllers/file-by-id.controller'; import { FileController } from './controllers/file.controller'; import { FileEntity } from './entities/file.entity'; import { FileCorePictureModule } from './file-core-picture/file-core-picture.module'; @@ -59,6 +58,6 @@ import { FileService } from './services/file.service'; FileWorkflowModule, FileUploadService, ], - controllers: [FileController, FileByIdController], + controllers: [FileController], }) export class FileModule {} diff --git a/packages/twenty-server/src/engine/core-modules/file/services/file.service.ts b/packages/twenty-server/src/engine/core-modules/file/services/file.service.ts index 854692abe1..3f9e93e73e 100644 --- a/packages/twenty-server/src/engine/core-modules/file/services/file.service.ts +++ b/packages/twenty-server/src/engine/core-modules/file/services/file.service.ts @@ -48,6 +48,32 @@ export class FileService { }); } + async getFileStreamByPath({ + workspaceId, + applicationId, + filepath, + fileFolder, + }: { + workspaceId: string; + applicationId: string; + filepath: string; + fileFolder: FileFolder; + }) { + const application = await this.applicationRepository.findOneOrFail({ + where: { + id: applicationId, + workspaceId, + }, + }); + + return this.fileStorageService.readFile({ + resourcePath: filepath, + fileFolder, + applicationUniversalIdentifier: application.universalIdentifier, + workspaceId, + }); + } + async getFileStreamById({ fileId, workspaceId, @@ -57,29 +83,27 @@ export class FileService { workspaceId: string; fileFolder: FileFolder; }): Promise { - { - const file = await this.fileRepository.findOneOrFail({ - where: { - id: fileId, - workspaceId, - path: Like(`${fileFolder}/%`), - }, - }); - - const application = await this.applicationRepository.findOneOrFail({ - where: { - id: file.applicationId, - workspaceId, - }, - }); - - return this.fileStorageService.readFile({ - resourcePath: removeFileFolderFromFileEntityPath(file.path), - fileFolder, - applicationUniversalIdentifier: application.universalIdentifier, + const file = await this.fileRepository.findOneOrFail({ + where: { + id: fileId, workspaceId, - }); - } + path: Like(`${fileFolder}/%`), + }, + }); + + const application = await this.applicationRepository.findOneOrFail({ + where: { + id: file.applicationId, + workspaceId, + }, + }); + + return this.fileStorageService.readFile({ + resourcePath: removeFileFolderFromFileEntityPath(file.path), + fileFolder, + applicationUniversalIdentifier: application.universalIdentifier, + workspaceId, + }); } signFileUrl({ url, workspaceId }: { url: string; workspaceId: string }) {