From aa58259019544e734391426cfc8abac5d4b0424e Mon Sep 17 00:00:00 2001 From: martmull Date: Mon, 26 May 2025 22:05:21 +0200 Subject: [PATCH] 11744 emails broken image in emails (#12265) - refactor file tokens - update file token management - generate one token per file per workspaceId - move token from query params to url path --- .../src/generated-metadata/graphql.ts | 14 ++- .../twenty-front/src/generated/graphql.tsx | 42 ++++++--- .../useUploadAttachmentFile.test.tsx | 14 --- .../files/hooks/useUploadAttachmentFile.tsx | 15 ++- .../attachments/graphql/queries/uploadFile.ts | 5 +- .../graphql/queries/uploadImage.ts | 5 +- .../hooks/useRecordShowContainerActions.ts | 6 +- .../components/ProfilePictureUploader.tsx | 10 +- .../components/WorkspaceLogoUploader.tsx | 3 +- .../graphql/mutations/uploadProfilePicture.ts | 5 +- .../graphql/mutations/uploadWorkspaceLogo.ts | 5 +- .../src/database/typeorm/typeorm.service.ts | 7 +- .../activity-query-result-getter.handler.ts | 10 +- .../attachment-query-result-getter.handler.ts | 14 ++- .../person-query-result-getter.handler.ts | 12 ++- ...pace-member-query-result-getter.handler.ts | 14 ++- .../file/controllers/file.controller.ts | 20 +--- .../file/file-upload/dtos/signed-file.dto.ts | 10 ++ .../resolvers/file-upload.resolver.ts | 25 +++-- .../services/file-upload.service.ts | 46 ++++++---- .../core-modules/file/file.utils.spec.ts | 92 ------------------- .../engine/core-modules/file/file.utils.ts | 66 ------------- .../file/guards/file-path-guard.ts | 33 ++++--- .../file/interfaces/file-folder.interface.ts | 4 + .../file/services/file.service.ts | 8 +- .../__tests__/build-file-info.utils.spec.ts | 42 +++++++++ .../__tests__/check-file-folder.utils.spec.ts | 33 +++++++ .../__tests__/check-file-name.utils.spec.ts | 28 ++++++ .../__tests__/check-file-path.utils.spec.ts | 35 +++++++ .../extract-file-id-from-path.utils.spec.ts | 37 ++++++++ ...tract-file-info-from-request.utils.spec.ts | 49 ++++++++++ .../file/utils/build-file-info.utils.ts | 11 +++ .../file/utils/check-file-folder.utils.ts | 23 +++++ .../file/utils/check-file-name.utils.ts | 18 ++++ .../file/utils/check-file-path.utils.ts | 33 +++++++ .../utils/extract-file-id-from-path.utils.ts | 17 ++++ .../extract-file-info-from-request.utils.ts | 29 ++++++ .../search/services/search.service.ts | 14 ++- .../twenty-config/twenty-config.service.ts | 14 +++ .../user-workspace.service.spec.ts | 9 +- .../user-workspace/user-workspace.service.ts | 8 +- ...ted-workspace-member-transpiler.service.ts | 14 ++- .../engine/core-modules/user/user.resolver.ts | 31 ++++--- .../workspace-invitation.resolver.ts | 11 ++- .../workspace/workspace.resolver.ts | 38 +++++--- .../factories/workspace-datasource.factory.ts | 7 +- .../graphql/suites/auth.integration-spec.ts | 4 +- .../search-resolver.integration-spec.ts | 31 +++++-- .../integration/utils/clean-test-database.ts | 44 --------- packages/twenty-shared/src/utils/index.ts | 1 + .../url/__tests__/buildSignedPath.test.ts | 69 ++++++++++++++ .../src/utils/url/buildSignedPath.ts | 25 +++++ packages/twenty-shared/src/utils/url/index.ts | 1 + 53 files changed, 775 insertions(+), 386 deletions(-) delete mode 100644 packages/twenty-front/src/modules/activities/files/hooks/__tests__/useUploadAttachmentFile.test.tsx create mode 100644 packages/twenty-server/src/engine/core-modules/file/file-upload/dtos/signed-file.dto.ts delete mode 100644 packages/twenty-server/src/engine/core-modules/file/file.utils.spec.ts delete mode 100644 packages/twenty-server/src/engine/core-modules/file/file.utils.ts create mode 100644 packages/twenty-server/src/engine/core-modules/file/utils/__tests__/build-file-info.utils.spec.ts create mode 100644 packages/twenty-server/src/engine/core-modules/file/utils/__tests__/check-file-folder.utils.spec.ts create mode 100644 packages/twenty-server/src/engine/core-modules/file/utils/__tests__/check-file-name.utils.spec.ts create mode 100644 packages/twenty-server/src/engine/core-modules/file/utils/__tests__/check-file-path.utils.spec.ts create mode 100644 packages/twenty-server/src/engine/core-modules/file/utils/__tests__/extract-file-id-from-path.utils.spec.ts create mode 100644 packages/twenty-server/src/engine/core-modules/file/utils/__tests__/extract-file-info-from-request.utils.spec.ts create mode 100644 packages/twenty-server/src/engine/core-modules/file/utils/build-file-info.utils.ts create mode 100644 packages/twenty-server/src/engine/core-modules/file/utils/check-file-folder.utils.ts create mode 100644 packages/twenty-server/src/engine/core-modules/file/utils/check-file-name.utils.ts create mode 100644 packages/twenty-server/src/engine/core-modules/file/utils/check-file-path.utils.ts create mode 100644 packages/twenty-server/src/engine/core-modules/file/utils/extract-file-id-from-path.utils.ts create mode 100644 packages/twenty-server/src/engine/core-modules/file/utils/extract-file-info-from-request.utils.ts delete mode 100644 packages/twenty-server/test/integration/utils/clean-test-database.ts create mode 100644 packages/twenty-shared/src/utils/url/__tests__/buildSignedPath.test.ts create mode 100644 packages/twenty-shared/src/utils/url/buildSignedPath.ts diff --git a/packages/twenty-front/src/generated-metadata/graphql.ts b/packages/twenty-front/src/generated-metadata/graphql.ts index 06aec14f8..bc8e36aa0 100644 --- a/packages/twenty-front/src/generated-metadata/graphql.ts +++ b/packages/twenty-front/src/generated-metadata/graphql.ts @@ -1025,10 +1025,10 @@ export type Mutation = { updateWorkspace: Workspace; updateWorkspaceFeatureFlag: Scalars['Boolean']['output']; updateWorkspaceMemberRole: WorkspaceMember; - uploadFile: Scalars['String']['output']; - uploadImage: Scalars['String']['output']; - uploadProfilePicture: Scalars['String']['output']; - uploadWorkspaceLogo: Scalars['String']['output']; + uploadFile: SignedFileDto; + uploadImage: SignedFileDto; + uploadProfilePicture: SignedFileDto; + uploadWorkspaceLogo: SignedFileDto; upsertObjectPermissions: Array; upsertSettingPermissions: Array; userLookupAdminPanel: UserLookup; @@ -2175,6 +2175,12 @@ export type SignUpOutput = { workspace: WorkspaceUrlsAndId; }; +export type SignedFileDto = { + __typename?: 'SignedFileDTO'; + path: Scalars['String']['output']; + token: Scalars['String']['output']; +}; + export type StandardOverrides = { __typename?: 'StandardOverrides'; description?: Maybe; diff --git a/packages/twenty-front/src/generated/graphql.tsx b/packages/twenty-front/src/generated/graphql.tsx index 5ca2f0c76..96bfd725c 100644 --- a/packages/twenty-front/src/generated/graphql.tsx +++ b/packages/twenty-front/src/generated/graphql.tsx @@ -941,10 +941,10 @@ export type Mutation = { updateWorkspace: Workspace; updateWorkspaceFeatureFlag: Scalars['Boolean']; updateWorkspaceMemberRole: WorkspaceMember; - uploadFile: Scalars['String']; - uploadImage: Scalars['String']; - uploadProfilePicture: Scalars['String']; - uploadWorkspaceLogo: Scalars['String']; + uploadFile: SignedFileDto; + uploadImage: SignedFileDto; + uploadProfilePicture: SignedFileDto; + uploadWorkspaceLogo: SignedFileDto; upsertObjectPermissions: Array; upsertSettingPermissions: Array; userLookupAdminPanel: UserLookup; @@ -1970,6 +1970,12 @@ export type SignUpOutput = { workspace: WorkspaceUrlsAndId; }; +export type SignedFileDto = { + __typename?: 'SignedFileDTO'; + path: Scalars['String']; + token: Scalars['String']; +}; + export type StandardOverrides = { __typename?: 'StandardOverrides'; description?: Maybe; @@ -2554,7 +2560,7 @@ export type UploadFileMutationVariables = Exact<{ }>; -export type UploadFileMutation = { __typename?: 'Mutation', uploadFile: string }; +export type UploadFileMutation = { __typename?: 'Mutation', uploadFile: { __typename?: 'SignedFileDTO', path: string, token: string } }; export type UploadImageMutationVariables = Exact<{ file: Scalars['Upload']; @@ -2562,7 +2568,7 @@ export type UploadImageMutationVariables = Exact<{ }>; -export type UploadImageMutation = { __typename?: 'Mutation', uploadImage: string }; +export type UploadImageMutation = { __typename?: 'Mutation', uploadImage: { __typename?: 'SignedFileDTO', path: string, token: string } }; export type AuthTokenFragmentFragment = { __typename?: 'AuthToken', token: string, expiresAt: string }; @@ -2973,7 +2979,7 @@ export type UploadProfilePictureMutationVariables = Exact<{ }>; -export type UploadProfilePictureMutation = { __typename?: 'Mutation', uploadProfilePicture: string }; +export type UploadProfilePictureMutation = { __typename?: 'Mutation', uploadProfilePicture: { __typename?: 'SignedFileDTO', path: string, token: string } }; export type GetCurrentUserQueryVariables = Exact<{ [key: string]: never; }>; @@ -3104,7 +3110,7 @@ export type UploadWorkspaceLogoMutationVariables = Exact<{ }>; -export type UploadWorkspaceLogoMutation = { __typename?: 'Mutation', uploadWorkspaceLogo: string }; +export type UploadWorkspaceLogoMutation = { __typename?: 'Mutation', uploadWorkspaceLogo: { __typename?: 'SignedFileDTO', path: string, token: string } }; export type CheckCustomDomainValidRecordsMutationVariables = Exact<{ [key: string]: never; }>; @@ -3570,7 +3576,10 @@ export type TrackAnalyticsMutationResult = Apollo.MutationResult; export const UploadFileDocument = gql` mutation uploadFile($file: Upload!, $fileFolder: FileFolder) { - uploadFile(file: $file, fileFolder: $fileFolder) + uploadFile(file: $file, fileFolder: $fileFolder) { + path + token + } } `; export type UploadFileMutationFn = Apollo.MutationFunction; @@ -3602,7 +3611,10 @@ export type UploadFileMutationResult = Apollo.MutationResult export type UploadFileMutationOptions = Apollo.BaseMutationOptions; export const UploadImageDocument = gql` mutation uploadImage($file: Upload!, $fileFolder: FileFolder) { - uploadImage(file: $file, fileFolder: $fileFolder) + uploadImage(file: $file, fileFolder: $fileFolder) { + path + token + } } `; export type UploadImageMutationFn = Apollo.MutationFunction; @@ -5833,7 +5845,10 @@ export type DeleteUserAccountMutationResult = Apollo.MutationResult; export const UploadProfilePictureDocument = gql` mutation UploadProfilePicture($file: Upload!) { - uploadProfilePicture(file: $file) + uploadProfilePicture(file: $file) { + path + token + } } `; export type UploadProfilePictureMutationFn = Apollo.MutationFunction; @@ -6499,7 +6514,10 @@ export type UpdateWorkspaceMutationResult = Apollo.MutationResult; export const UploadWorkspaceLogoDocument = gql` mutation UploadWorkspaceLogo($file: Upload!) { - uploadWorkspaceLogo(file: $file) + uploadWorkspaceLogo(file: $file) { + path + token + } } `; export type UploadWorkspaceLogoMutationFn = Apollo.MutationFunction; diff --git a/packages/twenty-front/src/modules/activities/files/hooks/__tests__/useUploadAttachmentFile.test.tsx b/packages/twenty-front/src/modules/activities/files/hooks/__tests__/useUploadAttachmentFile.test.tsx deleted file mode 100644 index 00bcb29cc..000000000 --- a/packages/twenty-front/src/modules/activities/files/hooks/__tests__/useUploadAttachmentFile.test.tsx +++ /dev/null @@ -1,14 +0,0 @@ -import { computePathWithoutToken } from '../useUploadAttachmentFile'; - -describe('computePathWithoutToken', () => { - it('should remove token from path', () => { - const input = 'https://example.com/image.jpg?token=abc123'; - const expected = 'https://example.com/image.jpg'; - expect(computePathWithoutToken(input)).toBe(expected); - }); - - it('should handle path without token', () => { - const input = 'https://example.com/image.jpg?size=large'; - expect(computePathWithoutToken(input)).toBe(input); - }); -}); diff --git a/packages/twenty-front/src/modules/activities/files/hooks/useUploadAttachmentFile.tsx b/packages/twenty-front/src/modules/activities/files/hooks/useUploadAttachmentFile.tsx index e9c156246..5a408b25c 100644 --- a/packages/twenty-front/src/modules/activities/files/hooks/useUploadAttachmentFile.tsx +++ b/packages/twenty-front/src/modules/activities/files/hooks/useUploadAttachmentFile.tsx @@ -7,13 +7,8 @@ import { getActivityTargetObjectFieldIdName } from '@/activities/utils/getActivi import { currentWorkspaceMemberState } from '@/auth/states/currentWorkspaceMemberState'; import { CoreObjectNameSingular } from '@/object-metadata/types/CoreObjectNameSingular'; import { useCreateOneRecord } from '@/object-record/hooks/useCreateOneRecord'; -import { isNonEmptyString } from '@sniptt/guards'; import { FileFolder, useUploadFileMutation } from '~/generated/graphql'; - -// Note: This is probably not the right way to do this. -export const computePathWithoutToken = (attachmentPath: string): string => { - return attachmentPath.replace(/\?token=[^&]*$/, ''); -}; +import { isDefined } from 'twenty-shared/utils'; export const useUploadAttachmentFile = () => { const currentWorkspaceMember = useRecoilValue(currentWorkspaceMemberState); @@ -36,12 +31,14 @@ export const useUploadAttachmentFile = () => { }, }); - const attachmentPath = result?.data?.uploadFile; + const signedFile = result?.data?.uploadFile; - if (!isNonEmptyString(attachmentPath)) { + if (!isDefined(signedFile)) { throw new Error("Couldn't upload the attachment."); } + const { path: attachmentPath } = signedFile; + const targetableObjectFieldIdName = getActivityTargetObjectFieldIdName({ nameSingular: targetableObject.targetObjectNameSingular, }); @@ -49,7 +46,7 @@ export const useUploadAttachmentFile = () => { const attachmentToCreate = { authorId: currentWorkspaceMember?.id, name: file.name, - fullPath: computePathWithoutToken(attachmentPath), + fullPath: attachmentPath, type: getFileType(file.name), [targetableObjectFieldIdName]: targetableObject.id, createdAt: new Date().toISOString(), diff --git a/packages/twenty-front/src/modules/attachments/graphql/queries/uploadFile.ts b/packages/twenty-front/src/modules/attachments/graphql/queries/uploadFile.ts index d07fc1c5e..632d32a34 100644 --- a/packages/twenty-front/src/modules/attachments/graphql/queries/uploadFile.ts +++ b/packages/twenty-front/src/modules/attachments/graphql/queries/uploadFile.ts @@ -2,6 +2,9 @@ import { gql } from '@apollo/client'; export const UPLOAD_FILE = gql` mutation uploadFile($file: Upload!, $fileFolder: FileFolder) { - uploadFile(file: $file, fileFolder: $fileFolder) + uploadFile(file: $file, fileFolder: $fileFolder) { + path + token + } } `; diff --git a/packages/twenty-front/src/modules/attachments/graphql/queries/uploadImage.ts b/packages/twenty-front/src/modules/attachments/graphql/queries/uploadImage.ts index 80cc0920e..7c86e292c 100644 --- a/packages/twenty-front/src/modules/attachments/graphql/queries/uploadImage.ts +++ b/packages/twenty-front/src/modules/attachments/graphql/queries/uploadImage.ts @@ -2,6 +2,9 @@ import { gql } from '@apollo/client'; export const UPLOAD_IMAGE = gql` mutation uploadImage($file: Upload!, $fileFolder: FileFolder) { - uploadImage(file: $file, fileFolder: $fileFolder) + uploadImage(file: $file, fileFolder: $fileFolder) { + path + token + } } `; diff --git a/packages/twenty-front/src/modules/object-record/record-show/hooks/useRecordShowContainerActions.ts b/packages/twenty-front/src/modules/object-record/record-show/hooks/useRecordShowContainerActions.ts index 928594bd0..996a9830b 100644 --- a/packages/twenty-front/src/modules/object-record/record-show/hooks/useRecordShowContainerActions.ts +++ b/packages/twenty-front/src/modules/object-record/record-show/hooks/useRecordShowContainerActions.ts @@ -42,16 +42,16 @@ export const useRecordShowContainerActions = ({ }, }); - const avatarUrl = result?.data?.uploadImage; + const avatarSignedFile = result?.data?.uploadImage; - if (!avatarUrl || isUndefinedOrNull(updateOneRecord)) { + if (!avatarSignedFile || isUndefinedOrNull(updateOneRecord)) { return; } await updateOneRecord({ idToUpdate: objectRecordId, updateOneRecordInput: { - avatarUrl, + avatarUrl: avatarSignedFile.path, }, }); }; diff --git a/packages/twenty-front/src/modules/settings/profile/components/ProfilePictureUploader.tsx b/packages/twenty-front/src/modules/settings/profile/components/ProfilePictureUploader.tsx index 60cb35e26..08e4bf173 100644 --- a/packages/twenty-front/src/modules/settings/profile/components/ProfilePictureUploader.tsx +++ b/packages/twenty-front/src/modules/settings/profile/components/ProfilePictureUploader.tsx @@ -5,7 +5,7 @@ import { currentWorkspaceMemberState } from '@/auth/states/currentWorkspaceMembe import { CoreObjectNameSingular } from '@/object-metadata/types/CoreObjectNameSingular'; import { useUpdateOneRecord } from '@/object-record/hooks/useUpdateOneRecord'; import { ImageInput } from '@/ui/input/components/ImageInput'; -import { isDefined } from 'twenty-shared/utils'; +import { buildSignedPath, isDefined } from 'twenty-shared/utils'; import { useUploadProfilePictureMutation } from '~/generated/graphql'; import { isUndefinedOrNull } from '~/utils/isUndefinedOrNull'; @@ -51,22 +51,22 @@ export const ProfilePictureUploader = () => { setUploadController(null); setErrorMessage(null); - const avatarUrl = result?.data?.uploadProfilePicture.split('?')[0]; + const signedFile = result?.data?.uploadProfilePicture; - if (!avatarUrl) { + if (!isDefined(signedFile)) { throw new Error('Avatar URL not found'); } await updateOneRecord({ idToUpdate: currentWorkspaceMember?.id, updateOneRecordInput: { - avatarUrl, + avatarUrl: signedFile.path, }, }); setCurrentWorkspaceMember({ ...currentWorkspaceMember, - avatarUrl: result?.data?.uploadProfilePicture, + avatarUrl: buildSignedPath(signedFile), }); return result; diff --git a/packages/twenty-front/src/modules/settings/workspace/components/WorkspaceLogoUploader.tsx b/packages/twenty-front/src/modules/settings/workspace/components/WorkspaceLogoUploader.tsx index 430205c52..dec1b233a 100644 --- a/packages/twenty-front/src/modules/settings/workspace/components/WorkspaceLogoUploader.tsx +++ b/packages/twenty-front/src/modules/settings/workspace/components/WorkspaceLogoUploader.tsx @@ -7,6 +7,7 @@ import { useUploadWorkspaceLogoMutation, } from '~/generated/graphql'; import { isUndefinedOrNull } from '~/utils/isUndefinedOrNull'; +import { buildSignedPath } from 'twenty-shared/utils'; export const WorkspaceLogoUploader = () => { const [uploadLogo] = useUploadWorkspaceLogoMutation(); @@ -29,7 +30,7 @@ export const WorkspaceLogoUploader = () => { onCompleted: (data) => { setCurrentWorkspace({ ...currentWorkspace, - logo: data.uploadWorkspaceLogo, + logo: buildSignedPath(data.uploadWorkspaceLogo), }); }, }); diff --git a/packages/twenty-front/src/modules/users/graphql/mutations/uploadProfilePicture.ts b/packages/twenty-front/src/modules/users/graphql/mutations/uploadProfilePicture.ts index 711a1efc2..6fd592a82 100644 --- a/packages/twenty-front/src/modules/users/graphql/mutations/uploadProfilePicture.ts +++ b/packages/twenty-front/src/modules/users/graphql/mutations/uploadProfilePicture.ts @@ -2,6 +2,9 @@ import { gql } from '@apollo/client'; export const UPLOAD_PROFILE_PICTURE = gql` mutation UploadProfilePicture($file: Upload!) { - uploadProfilePicture(file: $file) + uploadProfilePicture(file: $file) { + path + token + } } `; diff --git a/packages/twenty-front/src/modules/workspace/graphql/mutations/uploadWorkspaceLogo.ts b/packages/twenty-front/src/modules/workspace/graphql/mutations/uploadWorkspaceLogo.ts index 68435c50a..9d3041947 100644 --- a/packages/twenty-front/src/modules/workspace/graphql/mutations/uploadWorkspaceLogo.ts +++ b/packages/twenty-front/src/modules/workspace/graphql/mutations/uploadWorkspaceLogo.ts @@ -2,6 +2,9 @@ import { gql } from '@apollo/client'; export const UPLOAD_WORKSPACE_LOGO = gql` mutation UploadWorkspaceLogo($file: Upload!) { - uploadWorkspaceLogo(file: $file) + uploadWorkspaceLogo(file: $file) { + path + token + } } `; diff --git a/packages/twenty-server/src/database/typeorm/typeorm.service.ts b/packages/twenty-server/src/database/typeorm/typeorm.service.ts index 4ddf7c474..99cb7b322 100644 --- a/packages/twenty-server/src/database/typeorm/typeorm.service.ts +++ b/packages/twenty-server/src/database/typeorm/typeorm.service.ts @@ -2,8 +2,6 @@ import { Injectable, OnModuleDestroy, OnModuleInit } from '@nestjs/common'; import { DataSource } from 'typeorm'; -import { NodeEnvironment } from 'src/engine/core-modules/twenty-config/interfaces/node-environment.interface'; - import { AppToken } from 'src/engine/core-modules/app-token/app-token.entity'; import { ApprovedAccessDomain } from 'src/engine/core-modules/approved-access-domain/approved-access-domain.entity'; import { BillingCustomer } from 'src/engine/core-modules/billing/entities/billing-customer.entity'; @@ -30,10 +28,7 @@ export class TypeORMService implements OnModuleInit, OnModuleDestroy { this.mainDataSource = new DataSource({ url: twentyConfigService.get('PG_DATABASE_URL'), type: 'postgres', - logging: - twentyConfigService.get('NODE_ENV') === NodeEnvironment.development - ? ['query', 'error'] - : ['error'], + logging: twentyConfigService.getLoggingConfig(), schema: 'core', entities: [ User, diff --git a/packages/twenty-server/src/engine/api/graphql/workspace-query-runner/factories/query-result-getters/handlers/activity-query-result-getter.handler.ts b/packages/twenty-server/src/engine/api/graphql/workspace-query-runner/factories/query-result-getters/handlers/activity-query-result-getter.handler.ts index cb0fc924d..c15dc61c1 100644 --- a/packages/twenty-server/src/engine/api/graphql/workspace-query-runner/factories/query-result-getters/handlers/activity-query-result-getter.handler.ts +++ b/packages/twenty-server/src/engine/api/graphql/workspace-query-runner/factories/query-result-getters/handlers/activity-query-result-getter.handler.ts @@ -1,9 +1,12 @@ +import { buildSignedPath } from 'twenty-shared/utils'; + import { QueryResultGetterHandlerInterface } from 'src/engine/api/graphql/workspace-query-runner/factories/query-result-getters/interfaces/query-result-getter-handler.interface'; import { FeatureFlagService } from 'src/engine/core-modules/feature-flag/services/feature-flag.service'; import { FileService } from 'src/engine/core-modules/file/services/file.service'; import { NoteWorkspaceEntity } from 'src/modules/note/standard-objects/note.workspace-entity'; import { TaskWorkspaceEntity } from 'src/modules/task/standard-objects/task.workspace-entity'; +import { extractFilenameFromPath } from 'src/engine/core-modules/file/utils/extract-file-id-from-path.utils'; // eslint-disable-next-line @typescript-eslint/no-explicit-any type RichTextBlock = Record; @@ -55,7 +58,7 @@ export class ActivityQueryResultGetterHandler imageUrl.searchParams.delete('token'); const signedPayload = this.fileService.encodeFileToken({ - noteBlockId: block.id, + filename: extractFilenameFromPath(imageProps.url), workspaceId: workspaceId, }); @@ -63,7 +66,10 @@ export class ActivityQueryResultGetterHandler ...block, props: { ...imageProps, - url: `${imageUrl.toString()}?token=${signedPayload}`, + url: buildSignedPath({ + path: imageUrl.toString(), + token: signedPayload, + }), }, }; }), diff --git a/packages/twenty-server/src/engine/api/graphql/workspace-query-runner/factories/query-result-getters/handlers/attachment-query-result-getter.handler.ts b/packages/twenty-server/src/engine/api/graphql/workspace-query-runner/factories/query-result-getters/handlers/attachment-query-result-getter.handler.ts index d8b19a033..3632e4217 100644 --- a/packages/twenty-server/src/engine/api/graphql/workspace-query-runner/factories/query-result-getters/handlers/attachment-query-result-getter.handler.ts +++ b/packages/twenty-server/src/engine/api/graphql/workspace-query-runner/factories/query-result-getters/handlers/attachment-query-result-getter.handler.ts @@ -1,7 +1,10 @@ +import { buildSignedPath } from 'twenty-shared/utils'; + import { QueryResultGetterHandlerInterface } from 'src/engine/api/graphql/workspace-query-runner/factories/query-result-getters/interfaces/query-result-getter-handler.interface'; import { FileService } from 'src/engine/core-modules/file/services/file.service'; import { AttachmentWorkspaceEntity } from 'src/modules/attachment/standard-objects/attachment.workspace-entity'; +import { extractFilenameFromPath } from 'src/engine/core-modules/file/utils/extract-file-id-from-path.utils'; export class AttachmentQueryResultGetterHandler implements QueryResultGetterHandlerInterface @@ -17,13 +20,20 @@ export class AttachmentQueryResultGetterHandler } const signedPayload = this.fileService.encodeFileToken({ - attachmentId: attachment.id, + filename: extractFilenameFromPath(attachment.fullPath), workspaceId: workspaceId, }); + const signedPath = buildSignedPath({ + path: attachment.fullPath, + token: signedPayload, + }); + + const fullPath = `${process.env.SERVER_URL}/files/${signedPath}`; + return { ...attachment, - fullPath: `${process.env.SERVER_URL}/files/${attachment.fullPath}?token=${signedPayload}`, + fullPath, }; } } diff --git a/packages/twenty-server/src/engine/api/graphql/workspace-query-runner/factories/query-result-getters/handlers/person-query-result-getter.handler.ts b/packages/twenty-server/src/engine/api/graphql/workspace-query-runner/factories/query-result-getters/handlers/person-query-result-getter.handler.ts index b70fbde6f..4abafd086 100644 --- a/packages/twenty-server/src/engine/api/graphql/workspace-query-runner/factories/query-result-getters/handlers/person-query-result-getter.handler.ts +++ b/packages/twenty-server/src/engine/api/graphql/workspace-query-runner/factories/query-result-getters/handlers/person-query-result-getter.handler.ts @@ -1,7 +1,10 @@ +import { buildSignedPath } from 'twenty-shared/utils'; + import { QueryResultGetterHandlerInterface } from 'src/engine/api/graphql/workspace-query-runner/factories/query-result-getters/interfaces/query-result-getter-handler.interface'; import { FileService } from 'src/engine/core-modules/file/services/file.service'; import { PersonWorkspaceEntity } from 'src/modules/person/standard-objects/person.workspace-entity'; +import { extractFilenameFromPath } from 'src/engine/core-modules/file/utils/extract-file-id-from-path.utils'; export class PersonQueryResultGetterHandler implements QueryResultGetterHandlerInterface @@ -17,13 +20,16 @@ export class PersonQueryResultGetterHandler } const signedPayload = this.fileService.encodeFileToken({ - personId: person.id, - workspaceId: workspaceId, + filename: extractFilenameFromPath(person.avatarUrl), + workspaceId, }); return { ...person, - avatarUrl: `${person.avatarUrl}?token=${signedPayload}`, + avatarUrl: buildSignedPath({ + path: person.avatarUrl, + token: signedPayload, + }), }; } } diff --git a/packages/twenty-server/src/engine/api/graphql/workspace-query-runner/factories/query-result-getters/handlers/workspace-member-query-result-getter.handler.ts b/packages/twenty-server/src/engine/api/graphql/workspace-query-runner/factories/query-result-getters/handlers/workspace-member-query-result-getter.handler.ts index 46808713f..782888add 100644 --- a/packages/twenty-server/src/engine/api/graphql/workspace-query-runner/factories/query-result-getters/handlers/workspace-member-query-result-getter.handler.ts +++ b/packages/twenty-server/src/engine/api/graphql/workspace-query-runner/factories/query-result-getters/handlers/workspace-member-query-result-getter.handler.ts @@ -1,7 +1,10 @@ +import { buildSignedPath } from 'twenty-shared/utils'; + import { QueryResultGetterHandlerInterface } from 'src/engine/api/graphql/workspace-query-runner/factories/query-result-getters/interfaces/query-result-getter-handler.interface'; import { FileService } from 'src/engine/core-modules/file/services/file.service'; import { WorkspaceMemberWorkspaceEntity } from 'src/modules/workspace-member/standard-objects/workspace-member.workspace-entity'; +import { extractFilenameFromPath } from 'src/engine/core-modules/file/utils/extract-file-id-from-path.utils'; export class WorkspaceMemberQueryResultGetterHandler implements QueryResultGetterHandlerInterface @@ -16,14 +19,17 @@ export class WorkspaceMemberQueryResultGetterHandler return workspaceMember; } - const signedPayload = await this.fileService.encodeFileToken({ - workspaceMemberId: workspaceMember.id, - workspaceId: workspaceId, + const signedPayload = this.fileService.encodeFileToken({ + filename: extractFilenameFromPath(workspaceMember.avatarUrl), + workspaceId, }); return { ...workspaceMember, - avatarUrl: `${workspaceMember.avatarUrl}?token=${signedPayload}`, + avatarUrl: buildSignedPath({ + path: workspaceMember.avatarUrl, + token: signedPayload, + }), }; } } 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 108afc3d0..e77a759dc 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 @@ -8,7 +8,7 @@ import { UseGuards, } from '@nestjs/common'; -import { Response } from 'express'; +import { Request, Response } from 'express'; import { FileStorageException, @@ -19,13 +19,10 @@ import { FileException, FileExceptionCode, } from 'src/engine/core-modules/file/file.exception'; -import { - checkFilePath, - checkFilename, -} from 'src/engine/core-modules/file/file.utils'; 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 { 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'; @Controller('files') @UseFilters(FileApiExceptionFilter) @@ -39,23 +36,14 @@ export class FileController { @Res() res: Response, @Req() req: Request, ) { - const folderPath = checkFilePath(params[0]); - // @ts-expect-error legacy noImplicitAny - const filename = checkFilename(params['filename']); - // eslint-disable-next-line @typescript-eslint/no-explicit-any const workspaceId = (req as any)?.workspaceId; - if (!workspaceId) { - throw new FileException( - 'Unauthorized: missing workspaceId', - FileExceptionCode.UNAUTHENTICATED, - ); - } + const { filename, rawFolder } = extractFileInfoFromRequest(req); try { const fileStream = await this.fileService.getFileStream( - folderPath, + rawFolder, filename, workspaceId, ); diff --git a/packages/twenty-server/src/engine/core-modules/file/file-upload/dtos/signed-file.dto.ts b/packages/twenty-server/src/engine/core-modules/file/file-upload/dtos/signed-file.dto.ts new file mode 100644 index 000000000..5ca6c98dd --- /dev/null +++ b/packages/twenty-server/src/engine/core-modules/file/file-upload/dtos/signed-file.dto.ts @@ -0,0 +1,10 @@ +import { Field, ObjectType } from '@nestjs/graphql'; + +@ObjectType() +export class SignedFileDTO { + @Field(() => String) + path: string; + + @Field(() => String) + token: string; +} diff --git a/packages/twenty-server/src/engine/core-modules/file/file-upload/resolvers/file-upload.resolver.ts b/packages/twenty-server/src/engine/core-modules/file/file-upload/resolvers/file-upload.resolver.ts index a16f099d7..0c35e78d4 100644 --- a/packages/twenty-server/src/engine/core-modules/file/file-upload/resolvers/file-upload.resolver.ts +++ b/packages/twenty-server/src/engine/core-modules/file/file-upload/resolvers/file-upload.resolver.ts @@ -10,24 +10,25 @@ import { Workspace } from 'src/engine/core-modules/workspace/workspace.entity'; import { AuthWorkspace } from 'src/engine/decorators/auth/auth-workspace.decorator'; import { WorkspaceAuthGuard } from 'src/engine/guards/workspace-auth.guard'; import { streamToBuffer } from 'src/utils/stream-to-buffer'; +import { SignedFileDTO } from 'src/engine/core-modules/file/file-upload/dtos/signed-file.dto'; @UseGuards(WorkspaceAuthGuard) @Resolver() export class FileUploadResolver { constructor(private readonly fileUploadService: FileUploadService) {} - @Mutation(() => String) + @Mutation(() => SignedFileDTO) async uploadFile( @AuthWorkspace() { id: workspaceId }: Workspace, @Args({ name: 'file', type: () => GraphQLUpload }) { createReadStream, filename, mimetype }: FileUpload, @Args('fileFolder', { type: () => FileFolder, nullable: true }) fileFolder: FileFolder, - ): Promise { + ): Promise { const stream = createReadStream(); const buffer = await streamToBuffer(stream); - const { path } = await this.fileUploadService.uploadFile({ + const { files } = await this.fileUploadService.uploadFile({ file: buffer, filename, mimeType: mimetype, @@ -35,21 +36,25 @@ export class FileUploadResolver { workspaceId, }); - return path; + if (!files.length) { + throw new Error('Failed to upload file'); + } + + return files[0]; } - @Mutation(() => String) + @Mutation(() => SignedFileDTO) async uploadImage( @AuthWorkspace() { id: workspaceId }: Workspace, @Args({ name: 'file', type: () => GraphQLUpload }) { createReadStream, filename, mimetype }: FileUpload, @Args('fileFolder', { type: () => FileFolder, nullable: true }) fileFolder: FileFolder, - ): Promise { + ): Promise { const stream = createReadStream(); const buffer = await streamToBuffer(stream); - const { paths } = await this.fileUploadService.uploadImage({ + const { files } = await this.fileUploadService.uploadImage({ file: buffer, filename, mimeType: mimetype, @@ -57,6 +62,10 @@ export class FileUploadResolver { workspaceId, }); - return paths[0]; + if (!files.length) { + throw new Error('Failed to upload image'); + } + + return files[0]; } } diff --git a/packages/twenty-server/src/engine/core-modules/file/file-upload/services/file-upload.service.ts b/packages/twenty-server/src/engine/core-modules/file/file-upload/services/file-upload.service.ts index aa114c076..6e8de9c7a 100644 --- a/packages/twenty-server/src/engine/core-modules/file/file-upload/services/file-upload.service.ts +++ b/packages/twenty-server/src/engine/core-modules/file/file-upload/services/file-upload.service.ts @@ -5,7 +5,7 @@ import DOMPurify from 'dompurify'; import FileType from 'file-type'; import { JSDOM } from 'jsdom'; import sharp from 'sharp'; -import { v4 as uuidV4, v4 } from 'uuid'; +import { v4 } from 'uuid'; import { FileFolder } from 'src/engine/core-modules/file/interfaces/file-folder.interface'; @@ -13,6 +13,15 @@ import { settings } from 'src/engine/constants/settings'; import { FileStorageService } from 'src/engine/core-modules/file-storage/file-storage.service'; import { FileService } from 'src/engine/core-modules/file/services/file.service'; import { getCropSize, getImageBufferFromUrl } from 'src/utils/image'; +import { buildFileInfo } from 'src/engine/core-modules/file/utils/build-file-info.utils'; + +export type SignedFile = { path: string; token: string }; + +export type SignedFilesResult = { + name: string; + mimeType: string | undefined; + files: SignedFile[]; +}; @Injectable() export class FileUploadService { @@ -72,10 +81,8 @@ export class FileUploadService { mimeType: string | undefined; fileFolder: FileFolder; workspaceId: string; - }) { - const ext = filename.split('.')?.[1]; - const id = uuidV4(); - const name = `${id}${ext ? `.${ext}` : ''}`; + }): Promise { + const { ext, name } = buildFileInfo(filename); const folder = this.getWorkspaceFolderName(workspaceId, fileFolder); await this._uploadFile({ @@ -85,14 +92,15 @@ export class FileUploadService { folder, }); - const signedPayload = await this.fileService.encodeFileToken({ + const signedPayload = this.fileService.encodeFileToken({ + filename: name, workspaceId: workspaceId, }); return { - id, + name, mimeType, - path: `${fileFolder}/${name}?token=${signedPayload}`, + files: [{ path: `${fileFolder}/${name}`, token: signedPayload }], }; } @@ -133,10 +141,8 @@ export class FileUploadService { mimeType: string | undefined; fileFolder: FileFolder; workspaceId: string; - }) { - const ext = filename.split('.')?.[1]; - const id = uuidV4(); - const name = `${id}${ext ? `.${ext}` : ''}`; + }): Promise { + const { name } = buildFileInfo(filename); const cropSizes = settings.storage.imageCropSizes[fileFolder]; @@ -153,14 +159,22 @@ export class FileUploadService { ), ); - const paths: Array = []; + const files: Array = []; await Promise.all( images.map(async (image, index) => { const buffer = await image.toBuffer(); const folder = this.getWorkspaceFolderName(workspaceId, fileFolder); - paths.push(`${fileFolder}/${cropSizes[index]}/${name}`); + const token = this.fileService.encodeFileToken({ + filename: name, + workspaceId: workspaceId, + }); + + files.push({ + path: `${fileFolder}/${cropSizes[index]}/${name}`, + token, + }); return this._uploadFile({ file: buffer, @@ -172,9 +186,9 @@ export class FileUploadService { ); return { - id, + name, mimeType, - paths, + files, }; } diff --git a/packages/twenty-server/src/engine/core-modules/file/file.utils.spec.ts b/packages/twenty-server/src/engine/core-modules/file/file.utils.spec.ts deleted file mode 100644 index b5919ac37..000000000 --- a/packages/twenty-server/src/engine/core-modules/file/file.utils.spec.ts +++ /dev/null @@ -1,92 +0,0 @@ -import { BadRequestException } from '@nestjs/common'; - -import { FileFolder } from 'src/engine/core-modules/file/interfaces/file-folder.interface'; - -import { - checkFilename, - checkFilePath, - checkFileFolder, -} from 'src/engine/core-modules/file/file.utils'; - -describe('FileUtils', () => { - describe('checkFilePath', () => { - it('should return sanitized file path', () => { - const filePath = `${FileFolder.Attachment}\0`; - const sanitizedFilePath = checkFilePath(filePath); - - expect(sanitizedFilePath).toBe(`${FileFolder.Attachment}`); - }); - - it('should return sanitized file path with size', () => { - const filePath = `${FileFolder.ProfilePicture}\0/original`; - const sanitizedFilePath = checkFilePath(filePath); - - expect(sanitizedFilePath).toBe(`${FileFolder.ProfilePicture}/original`); - }); - - it('should throw an error for invalid image size', () => { - const filePath = `${FileFolder.ProfilePicture}\0/invalid-size`; - - expect(() => checkFilePath(filePath)).toThrow( - `Size invalid-size is not allowed`, - ); - }); - - it('should throw an error for invalid folder', () => { - const filePath = `invalid-folder`; - - expect(() => checkFilePath(filePath)).toThrow( - `Folder invalid-folder is not allowed`, - ); - }); - }); - - describe('checkFilename', () => { - it('should return sanitized filename', () => { - const filename = `${FileFolder.Attachment}\0.png`; - const sanitizedFilename = checkFilename(filename); - - expect(sanitizedFilename).toBe(`${FileFolder.Attachment}.png`); - }); - - it('should throw an error for invalid filename', () => { - const filename = `invalid-filename`; - - expect(() => checkFilename(filename)).toThrow(`Filename is not allowed`); - }); - - it('should throw an error for invalid filename', () => { - const filename = `\0`; - - expect(() => checkFilename(filename)).toThrow(`Filename is not allowed`); - }); - }); - - describe('checkFileFolder', () => { - it('should return the root folder when it is allowed', () => { - expect(checkFileFolder(`${FileFolder.Attachment}/file.txt`)).toBe( - FileFolder.Attachment, - ); - }); - - it('should throw BadRequestException for disallowed folders', () => { - expect(() => checkFileFolder('invalid-folder/file.txt')).toThrow( - BadRequestException, - ); - }); - - it('should sanitize null characters in file path', () => { - expect(() => checkFileFolder('\0invalid-folder/file.txt')).toThrow( - BadRequestException, - ); - }); - - it('should handle edge cases like empty file path', () => { - expect(() => checkFileFolder('')).toThrow(BadRequestException); - }); - - it('should handle cases where filePath has no folder', () => { - expect(() => checkFileFolder('file.txt')).toThrow(BadRequestException); - }); - }); -}); diff --git a/packages/twenty-server/src/engine/core-modules/file/file.utils.ts b/packages/twenty-server/src/engine/core-modules/file/file.utils.ts deleted file mode 100644 index 2183c14dc..000000000 --- a/packages/twenty-server/src/engine/core-modules/file/file.utils.ts +++ /dev/null @@ -1,66 +0,0 @@ -import { BadRequestException } from '@nestjs/common'; - -import { basename } from 'path'; - -import { KebabCase } from 'type-fest'; - -import { settings } from 'src/engine/constants/settings'; -import { kebabCase } from 'src/utils/kebab-case'; - -import { FileFolder } from './interfaces/file-folder.interface'; - -type AllowedFolders = KebabCase; - -export const checkFilePath = (filePath: string): string => { - const allowedFolders = Object.values(FileFolder).map((value) => - kebabCase(value), - ); - - const sanitizedFilePath = filePath.replace(/\0/g, ''); - const [folder, size] = sanitizedFilePath.split('/'); - - if (!allowedFolders.includes(folder as AllowedFolders)) { - throw new BadRequestException(`Folder ${folder} is not allowed`); - } - - if ( - folder !== kebabCase(FileFolder.ServerlessFunction) && - size && - // @ts-expect-error legacy noImplicitAny - !settings.storage.imageCropSizes[folder]?.includes(size) - ) { - throw new BadRequestException(`Size ${size} is not allowed`); - } - - return sanitizedFilePath; -}; - -export const checkFilename = (filename: string) => { - const sanitizedFilename = basename(filename.replace(/\0/g, '')); - - if ( - !sanitizedFilename || - sanitizedFilename.includes('/') || - sanitizedFilename.includes('\\') || - !sanitizedFilename.includes('.') - ) { - throw new BadRequestException(`Filename is not allowed`); - } - - return basename(sanitizedFilename); -}; - -export const checkFileFolder = (filePath: string): FileFolder => { - const allowedFolders = Object.values(FileFolder).map((value) => - kebabCase(value), - ); - - const sanitizedFilePath = filePath.replace(/\0/g, ''); - const [rootFolder] = sanitizedFilePath.split('/'); - - if (!allowedFolders.includes(rootFolder as AllowedFolders)) { - throw new BadRequestException(`Folder ${rootFolder} is not allowed`); - } - - return rootFolder as FileFolder; -}; diff --git a/packages/twenty-server/src/engine/core-modules/file/guards/file-path-guard.ts b/packages/twenty-server/src/engine/core-modules/file/guards/file-path-guard.ts index e74374e42..a22a9e873 100644 --- a/packages/twenty-server/src/engine/core-modules/file/guards/file-path-guard.ts +++ b/packages/twenty-server/src/engine/core-modules/file/guards/file-path-guard.ts @@ -1,9 +1,8 @@ import { CanActivate, ExecutionContext, Injectable } from '@nestjs/common'; -import { fileFolderConfigs } from 'src/engine/core-modules/file/interfaces/file-folder.interface'; - -import { checkFileFolder } from 'src/engine/core-modules/file/file.utils'; import { JwtWrapperService } from 'src/engine/core-modules/jwt/services/jwt-wrapper.service'; +import { FilePayloadToEncode } from 'src/engine/core-modules/file/services/file.service'; +import { extractFileInfoFromRequest } from 'src/engine/core-modules/file/utils/extract-file-info-from-request.utils'; @Injectable() export class FilePathGuard implements CanActivate { @@ -11,37 +10,37 @@ export class FilePathGuard implements CanActivate { async canActivate(context: ExecutionContext): Promise { const request = context.switchToHttp().getRequest(); - const fileFolder = checkFileFolder(request.params[0]); - const ignoreExpirationToken = - fileFolderConfigs[fileFolder].ignoreExpirationToken; - const query = request.query; + const { filename, fileSignature, ignoreExpirationToken } = + extractFileInfoFromRequest(request); - if (!query || !query['token']) { + if (!fileSignature) { return false; } try { - const payload = await this.jwtWrapperService.verifyWorkspaceToken( - query['token'], + const payload = (await this.jwtWrapperService.verifyWorkspaceToken( + fileSignature, 'FILE', ignoreExpirationToken ? { ignoreExpiration: true } : {}, - ); + )) as FilePayloadToEncode; - if (!payload.workspaceId) { + if ( + !payload.workspaceId || + !payload.filename || + filename !== payload.filename + ) { return false; } } catch (error) { return false; } - const decodedPayload = await this.jwtWrapperService.decode(query['token'], { + const decodedPayload = (await this.jwtWrapperService.decode(fileSignature, { json: true, - }); + })) as FilePayloadToEncode; - const workspaceId = decodedPayload?.['workspaceId']; - - request.workspaceId = workspaceId; + request.workspaceId = decodedPayload.workspaceId; return true; } diff --git a/packages/twenty-server/src/engine/core-modules/file/interfaces/file-folder.interface.ts b/packages/twenty-server/src/engine/core-modules/file/interfaces/file-folder.interface.ts index 845783aa5..4676ff047 100644 --- a/packages/twenty-server/src/engine/core-modules/file/interfaces/file-folder.interface.ts +++ b/packages/twenty-server/src/engine/core-modules/file/interfaces/file-folder.interface.ts @@ -1,5 +1,7 @@ import { registerEnumType } from '@nestjs/graphql'; +import { KebabCase } from 'type-fest'; + export enum FileFolder { ProfilePicture = 'profile-picture', WorkspaceLogo = 'workspace-logo', @@ -33,3 +35,5 @@ export const fileFolderConfigs: Record = { ignoreExpirationToken: false, }, }; + +export type AllowedFolders = KebabCase; 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 4b91dedaf..954909d6d 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 @@ -9,6 +9,11 @@ import { FileStorageService } from 'src/engine/core-modules/file-storage/file-st import { JwtWrapperService } from 'src/engine/core-modules/jwt/services/jwt-wrapper.service'; import { TwentyConfigService } from 'src/engine/core-modules/twenty-config/twenty-config.service'; +export type FilePayloadToEncode = { + workspaceId: string; + filename: string; +}; + @Injectable() export class FileService { constructor( @@ -30,8 +35,7 @@ export class FileService { }); } - // eslint-disable-next-line @typescript-eslint/no-explicit-any - encodeFileToken(payloadToEncode: Record) { + encodeFileToken(payloadToEncode: FilePayloadToEncode) { const fileTokenExpiresIn = this.twentyConfigService.get( 'FILE_TOKEN_EXPIRES_IN', ); diff --git a/packages/twenty-server/src/engine/core-modules/file/utils/__tests__/build-file-info.utils.spec.ts b/packages/twenty-server/src/engine/core-modules/file/utils/__tests__/build-file-info.utils.spec.ts new file mode 100644 index 000000000..03e18ddf4 --- /dev/null +++ b/packages/twenty-server/src/engine/core-modules/file/utils/__tests__/build-file-info.utils.spec.ts @@ -0,0 +1,42 @@ +import { v4 as uuidV4 } from 'uuid'; + +import { buildFileInfo } from 'src/engine/core-modules/file/utils/build-file-info.utils'; + +jest.mock('uuid', () => ({ + v4: jest.fn(), +})); + +describe('buildFileInfo', () => { + const mockId = '1234-uuid'; + + beforeEach(() => { + (uuidV4 as jest.Mock).mockReturnValue(mockId); + }); + + it('should extract extension and generate correct name with extension', () => { + const result = buildFileInfo('file.txt'); + + expect(result).toEqual({ + ext: 'txt', + name: `${mockId}.txt`, + }); + }); + + it('should handle filenames without extension', () => { + const result = buildFileInfo('file'); + + expect(result).toEqual({ + ext: '', + name: mockId, + }); + }); + + it('should handle filenames with multiple dots', () => { + const result = buildFileInfo('archive.tar.gz'); + + expect(result).toEqual({ + ext: 'gz', + name: `${mockId}.gz`, + }); + }); +}); diff --git a/packages/twenty-server/src/engine/core-modules/file/utils/__tests__/check-file-folder.utils.spec.ts b/packages/twenty-server/src/engine/core-modules/file/utils/__tests__/check-file-folder.utils.spec.ts new file mode 100644 index 000000000..b57943bbc --- /dev/null +++ b/packages/twenty-server/src/engine/core-modules/file/utils/__tests__/check-file-folder.utils.spec.ts @@ -0,0 +1,33 @@ +import { BadRequestException } from '@nestjs/common'; + +import { FileFolder } from 'src/engine/core-modules/file/interfaces/file-folder.interface'; + +import { checkFileFolder } from 'src/engine/core-modules/file/utils/check-file-folder.utils'; + +describe('checkFileFolder', () => { + it('should return the root folder when it is allowed', () => { + expect(checkFileFolder(`${FileFolder.Attachment}/file.txt`)).toBe( + FileFolder.Attachment, + ); + }); + + it('should throw BadRequestException for disallowed folders', () => { + expect(() => checkFileFolder('invalid-folder/file.txt')).toThrow( + BadRequestException, + ); + }); + + it('should sanitize null characters in file path', () => { + expect(() => checkFileFolder('\0invalid-folder/file.txt')).toThrow( + BadRequestException, + ); + }); + + it('should handle edge cases like empty file path', () => { + expect(() => checkFileFolder('')).toThrow(BadRequestException); + }); + + it('should handle cases where filePath has no folder', () => { + expect(() => checkFileFolder('file.txt')).toThrow(BadRequestException); + }); +}); diff --git a/packages/twenty-server/src/engine/core-modules/file/utils/__tests__/check-file-name.utils.spec.ts b/packages/twenty-server/src/engine/core-modules/file/utils/__tests__/check-file-name.utils.spec.ts new file mode 100644 index 000000000..fe228dfc5 --- /dev/null +++ b/packages/twenty-server/src/engine/core-modules/file/utils/__tests__/check-file-name.utils.spec.ts @@ -0,0 +1,28 @@ +import { FileFolder } from 'src/engine/core-modules/file/interfaces/file-folder.interface'; + +import { checkFilename } from 'src/engine/core-modules/file/utils/check-file-name.utils'; + +describe('checkFilename', () => { + it('should return sanitized filename', () => { + const filename = `${FileFolder.Attachment}\0.png`; + const sanitizedFilename = checkFilename(filename); + + expect(sanitizedFilename).toBe(`${FileFolder.Attachment}.png`); + }); + + it('should throw an error for invalid filename', () => { + const filename = `invalid-filename`; + + expect(() => checkFilename(filename)).toThrow( + `Filename 'invalid-filename' is not allowed`, + ); + }); + + it('should throw an error for invalid filename', () => { + const filename = `\0`; + + expect(() => checkFilename(filename)).toThrow( + `Filename '\0' is not allowed`, + ); + }); +}); diff --git a/packages/twenty-server/src/engine/core-modules/file/utils/__tests__/check-file-path.utils.spec.ts b/packages/twenty-server/src/engine/core-modules/file/utils/__tests__/check-file-path.utils.spec.ts new file mode 100644 index 000000000..4d4aac28b --- /dev/null +++ b/packages/twenty-server/src/engine/core-modules/file/utils/__tests__/check-file-path.utils.spec.ts @@ -0,0 +1,35 @@ +import { FileFolder } from 'src/engine/core-modules/file/interfaces/file-folder.interface'; + +import { checkFilePath } from 'src/engine/core-modules/file/utils/check-file-path.utils'; + +describe('checkFilePath', () => { + it('should return sanitized file path', () => { + const filePath = `${FileFolder.Attachment}\0`; + const sanitizedFilePath = checkFilePath(filePath); + + expect(sanitizedFilePath).toBe(`${FileFolder.Attachment}`); + }); + + it('should return sanitized file path with size', () => { + const filePath = `${FileFolder.ProfilePicture}\0/original`; + const sanitizedFilePath = checkFilePath(filePath); + + expect(sanitizedFilePath).toBe(`${FileFolder.ProfilePicture}/original`); + }); + + it('should throw an error for invalid image size', () => { + const filePath = `${FileFolder.ProfilePicture}\0/invalid-size`; + + expect(() => checkFilePath(filePath)).toThrow( + `Size invalid-size is not allowed`, + ); + }); + + it('should throw an error for invalid folder', () => { + const filePath = `invalid-folder`; + + expect(() => checkFilePath(filePath)).toThrow( + `Folder invalid-folder is not allowed`, + ); + }); +}); diff --git a/packages/twenty-server/src/engine/core-modules/file/utils/__tests__/extract-file-id-from-path.utils.spec.ts b/packages/twenty-server/src/engine/core-modules/file/utils/__tests__/extract-file-id-from-path.utils.spec.ts new file mode 100644 index 000000000..186ff4e80 --- /dev/null +++ b/packages/twenty-server/src/engine/core-modules/file/utils/__tests__/extract-file-id-from-path.utils.spec.ts @@ -0,0 +1,37 @@ +import { extractFilenameFromPath } from 'src/engine/core-modules/file/utils/extract-file-id-from-path.utils'; + +describe('extractFileIdFromPath', () => { + it('should return the last segment of a normal path', () => { + const result = extractFilenameFromPath('uploads/files/1234.txt'); + + expect(result).toBe('1234.txt'); + }); + + it('should return the last segment when there is no slash', () => { + const result = extractFilenameFromPath('file.txt'); + + expect(result).toBe('file.txt'); + }); + + it('should throw when empty path', () => { + expect(() => extractFilenameFromPath('')).toThrow( + new Error('Cannot extract id from empty path'), + ); + }); + + it('should throw when empty filename', () => { + const folderPath = 'uploads/files/'; + + expect(() => extractFilenameFromPath(folderPath)).toThrow( + new Error(`Cannot extract id from folder path '${folderPath}'`), + ); + }); + + it('should throw when empty filename absolute path', () => { + const folderPath = '/a/b/c/'; + + expect(() => extractFilenameFromPath(folderPath)).toThrow( + new Error(`Cannot extract id from folder path '${folderPath}'`), + ); + }); +}); diff --git a/packages/twenty-server/src/engine/core-modules/file/utils/__tests__/extract-file-info-from-request.utils.spec.ts b/packages/twenty-server/src/engine/core-modules/file/utils/__tests__/extract-file-info-from-request.utils.spec.ts new file mode 100644 index 000000000..c7485cae8 --- /dev/null +++ b/packages/twenty-server/src/engine/core-modules/file/utils/__tests__/extract-file-info-from-request.utils.spec.ts @@ -0,0 +1,49 @@ +import { Request } from 'express'; + +import { checkFilename } from 'src/engine/core-modules/file/utils/check-file-name.utils'; +import { checkFileFolder } from 'src/engine/core-modules/file/utils/check-file-folder.utils'; +import { extractFileInfoFromRequest } from 'src/engine/core-modules/file/utils/extract-file-info-from-request.utils'; + +jest.mock('src/engine/core-modules/file/utils/check-file-name.utils', () => ({ + checkFilename: jest.fn(), +})); + +jest.mock('src/engine/core-modules/file/utils/check-file-folder.utils', () => ({ + checkFileFolder: jest.fn(), +})); + +jest.mock( + 'src/engine/core-modules/file/interfaces/file-folder.interface', + () => ({ + fileFolderConfigs: { + 'some-folder': { ignoreExpirationToken: true }, + }, + }), +); + +describe('extractFileInfoFromRequest', () => { + it('should extract all file info correctly from request', () => { + const mockRequest = { + params: { + filename: 'myfile.txt', + '0': 'some-folder/some-subfolder/filesig123', + }, + } as unknown as Request; + + (checkFilename as jest.Mock).mockReturnValue('validated-file.txt'); + (checkFileFolder as jest.Mock).mockReturnValue('some-folder'); + + const result = extractFileInfoFromRequest(mockRequest); + + expect(checkFilename).toHaveBeenCalledWith('myfile.txt'); + expect(checkFileFolder).toHaveBeenCalledWith('some-folder/some-subfolder'); + + expect(result).toEqual({ + filename: 'validated-file.txt', + fileSignature: 'filesig123', + rawFolder: 'some-folder/some-subfolder', + fileFolder: 'some-folder', + ignoreExpirationToken: true, + }); + }); +}); diff --git a/packages/twenty-server/src/engine/core-modules/file/utils/build-file-info.utils.ts b/packages/twenty-server/src/engine/core-modules/file/utils/build-file-info.utils.ts new file mode 100644 index 000000000..9d817a5a9 --- /dev/null +++ b/packages/twenty-server/src/engine/core-modules/file/utils/build-file-info.utils.ts @@ -0,0 +1,11 @@ +import { v4 as uuidV4 } from 'uuid'; + +export const buildFileInfo = (filename: string) => { + const parts = filename.split('.'); + + const ext = parts.length > 1 ? parts.pop() || '' : ''; + + const name = `${uuidV4()}${ext ? `.${ext}` : ''}`; + + return { ext, name }; +}; diff --git a/packages/twenty-server/src/engine/core-modules/file/utils/check-file-folder.utils.ts b/packages/twenty-server/src/engine/core-modules/file/utils/check-file-folder.utils.ts new file mode 100644 index 000000000..e9526ce9d --- /dev/null +++ b/packages/twenty-server/src/engine/core-modules/file/utils/check-file-folder.utils.ts @@ -0,0 +1,23 @@ +import { BadRequestException } from '@nestjs/common'; + +import { + AllowedFolders, + FileFolder, +} from 'src/engine/core-modules/file/interfaces/file-folder.interface'; + +import { kebabCase } from 'src/utils/kebab-case'; + +export const checkFileFolder = (filePath: string): FileFolder => { + const allowedFolders = Object.values(FileFolder).map((value) => + kebabCase(value), + ); + + const sanitizedFilePath = filePath.replace(/\0/g, ''); + const [rootFolder] = sanitizedFilePath.split('/'); + + if (!allowedFolders.includes(rootFolder as AllowedFolders)) { + throw new BadRequestException(`Folder ${rootFolder} is not allowed`); + } + + return rootFolder as FileFolder; +}; diff --git a/packages/twenty-server/src/engine/core-modules/file/utils/check-file-name.utils.ts b/packages/twenty-server/src/engine/core-modules/file/utils/check-file-name.utils.ts new file mode 100644 index 000000000..23b6fc91b --- /dev/null +++ b/packages/twenty-server/src/engine/core-modules/file/utils/check-file-name.utils.ts @@ -0,0 +1,18 @@ +import { BadRequestException } from '@nestjs/common'; + +import { basename } from 'path'; + +export const checkFilename = (filename: string) => { + const sanitizedFilename = filename.replace(/\0/g, ''); + + if ( + !sanitizedFilename || + sanitizedFilename.includes('/') || + sanitizedFilename.includes('\\') || + !sanitizedFilename.includes('.') + ) { + throw new BadRequestException(`Filename '${filename}' is not allowed`); + } + + return basename(sanitizedFilename); +}; diff --git a/packages/twenty-server/src/engine/core-modules/file/utils/check-file-path.utils.ts b/packages/twenty-server/src/engine/core-modules/file/utils/check-file-path.utils.ts new file mode 100644 index 000000000..141789042 --- /dev/null +++ b/packages/twenty-server/src/engine/core-modules/file/utils/check-file-path.utils.ts @@ -0,0 +1,33 @@ +import { BadRequestException } from '@nestjs/common'; + +import { + AllowedFolders, + FileFolder, +} from 'src/engine/core-modules/file/interfaces/file-folder.interface'; + +import { kebabCase } from 'src/utils/kebab-case'; +import { settings } from 'src/engine/constants/settings'; + +export const checkFilePath = (filePath: string): string => { + const allowedFolders = Object.values(FileFolder).map((value) => + kebabCase(value), + ); + + const sanitizedFilePath = filePath.replace(/\0/g, ''); + const [folder, size] = sanitizedFilePath.split('/'); + + if (!allowedFolders.includes(folder as AllowedFolders)) { + throw new BadRequestException(`Folder ${folder} is not allowed`); + } + + if ( + folder !== kebabCase(FileFolder.ServerlessFunction) && + size && + // @ts-expect-error legacy noImplicitAny + !settings.storage.imageCropSizes[folder]?.includes(size) + ) { + throw new BadRequestException(`Size ${size} is not allowed`); + } + + return sanitizedFilePath; +}; diff --git a/packages/twenty-server/src/engine/core-modules/file/utils/extract-file-id-from-path.utils.ts b/packages/twenty-server/src/engine/core-modules/file/utils/extract-file-id-from-path.utils.ts new file mode 100644 index 000000000..346d6c307 --- /dev/null +++ b/packages/twenty-server/src/engine/core-modules/file/utils/extract-file-id-from-path.utils.ts @@ -0,0 +1,17 @@ +import { basename } from 'path'; + +import { isNonEmptyString } from '@sniptt/guards'; + +export const extractFilenameFromPath = (path: string) => { + if (path.endsWith('/')) { + throw new Error(`Cannot extract id from folder path '${path}'`); + } + + const fileId = basename(path); + + if (!isNonEmptyString(fileId)) { + throw new Error(`Cannot extract id from empty path`); + } + + return fileId; +}; diff --git a/packages/twenty-server/src/engine/core-modules/file/utils/extract-file-info-from-request.utils.ts b/packages/twenty-server/src/engine/core-modules/file/utils/extract-file-info-from-request.utils.ts new file mode 100644 index 000000000..60374966d --- /dev/null +++ b/packages/twenty-server/src/engine/core-modules/file/utils/extract-file-info-from-request.utils.ts @@ -0,0 +1,29 @@ +import { Request } from 'express'; + +import { fileFolderConfigs } from 'src/engine/core-modules/file/interfaces/file-folder.interface'; + +import { checkFileFolder } from 'src/engine/core-modules/file/utils/check-file-folder.utils'; +import { checkFilename } from 'src/engine/core-modules/file/utils/check-file-name.utils'; + +export const extractFileInfoFromRequest = (request: Request) => { + const filename = checkFilename(request.params.filename); + + const parts = request.params[0].split('/'); + + const fileSignature = parts.pop(); + + const rawFolder = parts.join('/'); + + const fileFolder = checkFileFolder(rawFolder); + + const ignoreExpirationToken = + fileFolderConfigs[fileFolder].ignoreExpirationToken; + + return { + filename, + fileSignature, + rawFolder, + fileFolder, + ignoreExpirationToken, + }; +}; diff --git a/packages/twenty-server/src/engine/core-modules/search/services/search.service.ts b/packages/twenty-server/src/engine/core-modules/search/services/search.service.ts index c0b9b80ca..e4e873fb7 100644 --- a/packages/twenty-server/src/engine/core-modules/search/services/search.service.ts +++ b/packages/twenty-server/src/engine/core-modules/search/services/search.service.ts @@ -2,7 +2,7 @@ import { Injectable } from '@nestjs/common'; import { isNonEmptyString } from '@sniptt/guards'; import { FieldMetadataType } from 'twenty-shared/types'; -import { getLogoUrlFromDomainName } from 'twenty-shared/utils'; +import { buildSignedPath, getLogoUrlFromDomainName } from 'twenty-shared/utils'; import { Brackets, ObjectLiteral } from 'typeorm'; import chunk from 'lodash.chunk'; @@ -43,6 +43,7 @@ import { formatSearchTerms } from 'src/engine/core-modules/search/utils/format-s import { SearchArgs } from 'src/engine/core-modules/search/dtos/search-args'; import { SearchResultConnectionDTO } from 'src/engine/core-modules/search/dtos/search-result-connection.dto'; import { SearchResultEdgeDTO } from 'src/engine/core-modules/search/dtos/search-result-edge.dto'; +import { extractFilenameFromPath } from 'src/engine/core-modules/file/utils/extract-file-id-from-path.utils'; import { SearchRecordDTO } from 'src/engine/core-modules/search/dtos/search-record.dto'; type LastRanks = { tsRankCD: number; tsRank: number }; @@ -365,11 +366,15 @@ export class SearchService { } private getImageUrlWithToken(avatarUrl: string, workspaceId: string): string { - const avatarUrlToken = this.fileService.encodeFileToken({ + const signedPayload = this.fileService.encodeFileToken({ + filename: extractFilenameFromPath(avatarUrl), workspaceId, }); - return `${avatarUrl}?token=${avatarUrlToken}`; + return buildSignedPath({ + path: avatarUrl, + token: signedPayload, + }); } getImageIdentifierValue( @@ -384,7 +389,8 @@ export class SearchService { return getLogoUrlFromDomainName(record.domainNamePrimaryLinkUrl) || ''; } - return imageIdentifierField + return imageIdentifierField && + isNonEmptyString(record[imageIdentifierField]) ? this.getImageUrlWithToken(record[imageIdentifierField], workspaceId) : ''; } diff --git a/packages/twenty-server/src/engine/core-modules/twenty-config/twenty-config.service.ts b/packages/twenty-server/src/engine/core-modules/twenty-config/twenty-config.service.ts index 6cabc1028..31cfc3bdd 100644 --- a/packages/twenty-server/src/engine/core-modules/twenty-config/twenty-config.service.ts +++ b/packages/twenty-server/src/engine/core-modules/twenty-config/twenty-config.service.ts @@ -1,6 +1,9 @@ import { Injectable, Logger, Optional } from '@nestjs/common'; import { isString } from 'class-validator'; +import { LoggerOptions } from 'typeorm/logger/LoggerOptions'; + +import { NodeEnvironment } from 'src/engine/core-modules/twenty-config/interfaces/node-environment.interface'; import { ConfigVariables } from 'src/engine/core-modules/twenty-config/config-variables'; import { CONFIG_VARIABLES_MASKING_CONFIG } from 'src/engine/core-modules/twenty-config/constants/config-variables-masking-config'; @@ -196,6 +199,17 @@ export class TwentyConfigService { } } + getLoggingConfig(): LoggerOptions { + switch (this.get('NODE_ENV')) { + case NodeEnvironment.development: + return ['query', 'error']; + case NodeEnvironment.test: + return []; + default: + return ['error']; + } + } + private validateNotEnvOnly( key: T, operation: string, diff --git a/packages/twenty-server/src/engine/core-modules/user-workspace/user-workspace.service.spec.ts b/packages/twenty-server/src/engine/core-modules/user-workspace/user-workspace.service.spec.ts index fa85e564e..48cfd88ec 100644 --- a/packages/twenty-server/src/engine/core-modules/user-workspace/user-workspace.service.spec.ts +++ b/packages/twenty-server/src/engine/core-modules/user-workspace/user-workspace.service.spec.ts @@ -11,7 +11,10 @@ import { USER_SIGNUP_EVENT_NAME } from 'src/engine/api/graphql/workspace-query-r import { AuthException } from 'src/engine/core-modules/auth/auth.exception'; import { DomainManagerService } from 'src/engine/core-modules/domain-manager/services/domain-manager.service'; import { FileStorageService } from 'src/engine/core-modules/file-storage/file-storage.service'; -import { FileUploadService } from 'src/engine/core-modules/file/file-upload/services/file-upload.service'; +import { + FileUploadService, + SignedFilesResult, +} from 'src/engine/core-modules/file/file-upload/services/file-upload.service'; import { FileService } from 'src/engine/core-modules/file/services/file.service'; import { UserWorkspace } from 'src/engine/core-modules/user-workspace/user-workspace.entity'; import { UserWorkspaceService } from 'src/engine/core-modules/user-workspace/user-workspace.service'; @@ -266,8 +269,8 @@ describe('UserWorkspaceService', () => { .spyOn(workspaceEventEmitter, 'emitCustomBatchEvent') .mockImplementation(); jest.spyOn(fileUploadService, 'uploadImageFromUrl').mockResolvedValue({ - paths: ['path/to/file'], - } as any); + files: [{ path: 'path/to/file', token: 'token' }], + } as SignedFilesResult); const result = await service.create({ userId, diff --git a/packages/twenty-server/src/engine/core-modules/user-workspace/user-workspace.service.ts b/packages/twenty-server/src/engine/core-modules/user-workspace/user-workspace.service.ts index 6bd4099b5..e55519056 100644 --- a/packages/twenty-server/src/engine/core-modules/user-workspace/user-workspace.service.ts +++ b/packages/twenty-server/src/engine/core-modules/user-workspace/user-workspace.service.ts @@ -371,12 +371,16 @@ export class UserWorkspaceService extends TypeOrmQueryService { if (!isDefined(pictureUrl)) return; - const { paths } = await this.fileUploadService.uploadImageFromUrl({ + const { files } = await this.fileUploadService.uploadImageFromUrl({ imageUrl: pictureUrl, fileFolder: FileFolder.ProfilePicture, workspaceId, }); - return paths[0]; + if (!files.length) { + throw new Error('Failed to upload avatar'); + } + + return files[0].path; } } diff --git a/packages/twenty-server/src/engine/core-modules/user/services/deleted-workspace-member-transpiler.service.ts b/packages/twenty-server/src/engine/core-modules/user/services/deleted-workspace-member-transpiler.service.ts index c2568cb0d..71f7c1c96 100644 --- a/packages/twenty-server/src/engine/core-modules/user/services/deleted-workspace-member-transpiler.service.ts +++ b/packages/twenty-server/src/engine/core-modules/user/services/deleted-workspace-member-transpiler.service.ts @@ -1,8 +1,11 @@ import { Injectable } from '@nestjs/common'; +import { buildSignedPath } from 'twenty-shared/utils'; + import { FileService } from 'src/engine/core-modules/file/services/file.service'; import { DeletedWorkspaceMember } from 'src/engine/core-modules/user/dtos/deleted-workspace-member.dto'; import { WorkspaceMemberWorkspaceEntity } from 'src/modules/workspace-member/standard-objects/workspace-member.workspace-entity'; +import { extractFilenameFromPath } from 'src/engine/core-modules/file/utils/extract-file-id-from-path.utils'; @Injectable() export class DeletedWorkspaceMemberTranspiler { @@ -15,12 +18,15 @@ export class DeletedWorkspaceMemberTranspiler { workspaceMember: Pick; workspaceId: string; }): string { - const avatarUrlToken = this.fileService.encodeFileToken({ - workspaceMemberId: workspaceMember.id, - workspaceId: workspaceId, + const signedPayload = this.fileService.encodeFileToken({ + filename: extractFilenameFromPath(workspaceMember.avatarUrl), + workspaceId, }); - return `${workspaceMember.avatarUrl}?token=${avatarUrlToken}`; + return buildSignedPath({ + path: workspaceMember.avatarUrl, + token: signedPayload, + }); } toDeletedWorkspaceMemberDto( diff --git a/packages/twenty-server/src/engine/core-modules/user/user.resolver.ts b/packages/twenty-server/src/engine/core-modules/user/user.resolver.ts index 3783a778d..9f3d65760 100644 --- a/packages/twenty-server/src/engine/core-modules/user/user.resolver.ts +++ b/packages/twenty-server/src/engine/core-modules/user/user.resolver.ts @@ -14,6 +14,7 @@ import crypto from 'crypto'; import { GraphQLJSONObject } from 'graphql-type-json'; import { FileUpload, GraphQLUpload } from 'graphql-upload'; import { PermissionsOnAllObjectRecords } from 'twenty-shared/constants'; +import { buildSignedPath } from 'twenty-shared/utils'; import { WorkspaceActivationStatus } from 'twenty-shared/workspace'; import { In, Repository } from 'typeorm'; @@ -52,6 +53,8 @@ import { RoleDTO } from 'src/engine/metadata-modules/role/dtos/role.dto'; import { UserRoleService } from 'src/engine/metadata-modules/user-role/user-role.service'; import { AccountsToReconnectKeys } from 'src/modules/connected-account/types/accounts-to-reconnect-key-value.type'; import { streamToBuffer } from 'src/utils/stream-to-buffer'; +import { SignedFileDTO } from 'src/engine/core-modules/file/file-upload/dtos/signed-file.dto'; +import { extractFilenameFromPath } from 'src/engine/core-modules/file/utils/extract-file-id-from-path.utils'; const getHMACKey = (email?: string, key?: string | null) => { if (!email || !key) return null; @@ -186,11 +189,14 @@ export class UserResolver { if (workspaceMember && workspaceMember.avatarUrl) { const avatarUrlToken = this.fileService.encodeFileToken({ - workspaceMemberId: workspaceMember.id, + filename: extractFilenameFromPath(workspaceMember.avatarUrl), workspaceId: workspace.id, }); - workspaceMember.avatarUrl = `${workspaceMember.avatarUrl}?token=${avatarUrlToken}`; + workspaceMember.avatarUrl = buildSignedPath({ + path: workspaceMember.avatarUrl, + token: avatarUrlToken, + }); } // TODO Refactor to be transpiled to WorkspaceMember instead @@ -235,11 +241,14 @@ export class UserResolver { for (const workspaceMemberEntity of workspaceMemberEntities) { if (workspaceMemberEntity.avatarUrl) { const avatarUrlToken = this.fileService.encodeFileToken({ - workspaceMemberId: workspaceMemberEntity.id, + filename: extractFilenameFromPath(workspaceMemberEntity.avatarUrl), workspaceId: workspace.id, }); - workspaceMemberEntity.avatarUrl = `${workspaceMemberEntity.avatarUrl}?token=${avatarUrlToken}`; + workspaceMemberEntity.avatarUrl = buildSignedPath({ + path: workspaceMemberEntity.avatarUrl, + token: avatarUrlToken, + }); } // TODO Refactor to be transpiled to WorkspaceMember instead @@ -314,13 +323,13 @@ export class UserResolver { return getHMACKey(parent.email, key); } - @Mutation(() => String) + @Mutation(() => SignedFileDTO) async uploadProfilePicture( @AuthUser() { id }: User, @AuthWorkspace() { id: workspaceId }: Workspace, @Args({ name: 'file', type: () => GraphQLUpload }) { createReadStream, filename, mimetype }: FileUpload, - ): Promise { + ): Promise { if (!id) { throw new Error('User not found'); } @@ -329,7 +338,7 @@ export class UserResolver { const buffer = await streamToBuffer(stream); const fileFolder = FileFolder.ProfilePicture; - const { paths } = await this.fileUploadService.uploadImage({ + const { files } = await this.fileUploadService.uploadImage({ file: buffer, filename, mimeType: mimetype, @@ -337,11 +346,11 @@ export class UserResolver { workspaceId, }); - const fileToken = this.fileService.encodeFileToken({ - workspaceId: workspaceId, - }); + if (!files.length) { + throw new Error('Failed to upload profile picture'); + } - return `${paths[0]}?token=${fileToken}`; + return files[0]; } @Mutation(() => User) diff --git a/packages/twenty-server/src/engine/core-modules/workspace-invitation/workspace-invitation.resolver.ts b/packages/twenty-server/src/engine/core-modules/workspace-invitation/workspace-invitation.resolver.ts index 3c564995b..8cfdbfd31 100644 --- a/packages/twenty-server/src/engine/core-modules/workspace-invitation/workspace-invitation.resolver.ts +++ b/packages/twenty-server/src/engine/core-modules/workspace-invitation/workspace-invitation.resolver.ts @@ -1,6 +1,8 @@ import { UseFilters, UseGuards } from '@nestjs/common'; import { Args, Mutation, Query, Resolver } from '@nestjs/graphql'; +import { buildSignedPath } from 'twenty-shared/utils'; + import { FileService } from 'src/engine/core-modules/file/services/file.service'; import { User } from 'src/engine/core-modules/user/user.entity'; import { SendInvitationsOutput } from 'src/engine/core-modules/workspace-invitation/dtos/send-invitations.output'; @@ -14,6 +16,7 @@ import { UserAuthGuard } from 'src/engine/guards/user-auth.guard'; import { WorkspaceAuthGuard } from 'src/engine/guards/workspace-auth.guard'; import { SettingPermissionType } from 'src/engine/metadata-modules/permissions/constants/setting-permission-type.constants'; import { PermissionsGraphqlApiExceptionFilter } from 'src/engine/metadata-modules/permissions/utils/permissions-graphql-api-exception.filter'; +import { extractFilenameFromPath } from 'src/engine/core-modules/file/utils/extract-file-id-from-path.utils'; import { SendInvitationsInput } from './dtos/send-invitations.input'; @@ -69,11 +72,15 @@ export class WorkspaceInvitationResolver { let workspaceLogoWithToken = ''; if (workspace.logo) { - const workspaceLogoToken = this.fileService.encodeFileToken({ + const signedPayload = this.fileService.encodeFileToken({ + filename: extractFilenameFromPath(workspace.logo), workspaceId: workspace.id, }); - workspaceLogoWithToken = `${workspace.logo}?token=${workspaceLogoToken}`; + workspaceLogoWithToken = buildSignedPath({ + path: workspace.logo, + token: signedPayload, + }); } return await this.workspaceInvitationService.sendInvitations( diff --git a/packages/twenty-server/src/engine/core-modules/workspace/workspace.resolver.ts b/packages/twenty-server/src/engine/core-modules/workspace/workspace.resolver.ts index 32681242e..8cc081215 100644 --- a/packages/twenty-server/src/engine/core-modules/workspace/workspace.resolver.ts +++ b/packages/twenty-server/src/engine/core-modules/workspace/workspace.resolver.ts @@ -12,7 +12,7 @@ import { InjectRepository } from '@nestjs/typeorm'; import assert from 'assert'; import { FileUpload, GraphQLUpload } from 'graphql-upload'; -import { isDefined } from 'twenty-shared/utils'; +import { buildSignedPath, isDefined } from 'twenty-shared/utils'; import { Repository } from 'typeorm'; import { FileFolder } from 'src/engine/core-modules/file/interfaces/file-folder.interface'; @@ -52,6 +52,8 @@ import { RoleDTO } from 'src/engine/metadata-modules/role/dtos/role.dto'; import { RoleService } from 'src/engine/metadata-modules/role/role.service'; import { GraphqlValidationExceptionFilter } from 'src/filters/graphql-validation-exception.filter'; import { streamToBuffer } from 'src/utils/stream-to-buffer'; +import { extractFilenameFromPath } from 'src/engine/core-modules/file/utils/extract-file-id-from-path.utils'; +import { SignedFileDTO } from 'src/engine/core-modules/file/file-upload/dtos/signed-file.dto'; import { Workspace } from './workspace.entity'; @@ -119,7 +121,7 @@ export class WorkspaceResolver { } } - @Mutation(() => String) + @Mutation(() => SignedFileDTO) @UseGuards( WorkspaceAuthGuard, SettingsPermissionsGuard(SettingPermissionType.WORKSPACE), @@ -128,12 +130,12 @@ export class WorkspaceResolver { @AuthWorkspace() { id }: Workspace, @Args({ name: 'file', type: () => GraphQLUpload }) { createReadStream, filename, mimetype }: FileUpload, - ): Promise { + ): Promise { const stream = createReadStream(); const buffer = await streamToBuffer(stream); const fileFolder = FileFolder.WorkspaceLogo; - const { paths } = await this.fileUploadService.uploadImage({ + const { files } = await this.fileUploadService.uploadImage({ file: buffer, filename, mimeType: mimetype, @@ -141,15 +143,15 @@ export class WorkspaceResolver { workspaceId: id, }); + if (!files.length) { + throw new Error('Failed to upload workspace logo'); + } + await this.workspaceService.updateOne(id, { - logo: paths[0], + logo: files[0].path, }); - const workspaceLogoToken = this.fileService.encodeFileToken({ - workspaceId: id, - }); - - return `${paths[0]}?token=${workspaceLogoToken}`; + return files[0]; } @ResolveField(() => [FeatureFlagDTO], { nullable: true }) @@ -227,11 +229,15 @@ export class WorkspaceResolver { async logo(@Parent() workspace: Workspace): Promise { if (workspace.logo) { try { - const workspaceLogoToken = this.fileService.encodeFileToken({ + const signedPayload = this.fileService.encodeFileToken({ + filename: extractFilenameFromPath(workspace.logo), workspaceId: workspace.id, }); - return `${workspace.logo}?token=${workspaceLogoToken}`; + return buildSignedPath({ + path: workspace.logo, + token: signedPayload, + }); } catch (e) { return workspace.logo; } @@ -298,11 +304,15 @@ export class WorkspaceResolver { if (workspace.logo) { try { - const workspaceLogoToken = this.fileService.encodeFileToken({ + const signedPayload = this.fileService.encodeFileToken({ + filename: extractFilenameFromPath(workspace.logo), workspaceId: workspace.id, }); - workspaceLogoWithToken = `${workspace.logo}?token=${workspaceLogoToken}`; + workspaceLogoWithToken = buildSignedPath({ + path: workspace.logo, + token: signedPayload, + }); } catch (e) { workspaceLogoWithToken = workspace.logo; } diff --git a/packages/twenty-server/src/engine/twenty-orm/factories/workspace-datasource.factory.ts b/packages/twenty-server/src/engine/twenty-orm/factories/workspace-datasource.factory.ts index 1ed21b871..cf568813f 100644 --- a/packages/twenty-server/src/engine/twenty-orm/factories/workspace-datasource.factory.ts +++ b/packages/twenty-server/src/engine/twenty-orm/factories/workspace-datasource.factory.ts @@ -5,7 +5,6 @@ import { isDefined } from 'twenty-shared/utils'; import { EntitySchema } from 'typeorm'; import { FeatureFlagMap } from 'src/engine/core-modules/feature-flag/interfaces/feature-flag-map.interface'; -import { NodeEnvironment } from 'src/engine/core-modules/twenty-config/interfaces/node-environment.interface'; import { TwentyConfigService } from 'src/engine/core-modules/twenty-config/twenty-config.service'; import { DataSourceService } from 'src/engine/metadata-modules/data-source/data-source.service'; @@ -213,11 +212,7 @@ export class WorkspaceDatasourceFactory { dataSourceMetadata.url ?? this.twentyConfigService.get('PG_DATABASE_URL'), type: 'postgres', - logging: - this.twentyConfigService.get('NODE_ENV') === - NodeEnvironment.development - ? ['query', 'error'] - : ['error'], + logging: this.twentyConfigService.getLoggingConfig(), schema: dataSourceMetadata.schema, entities: cachedEntitySchemas, ssl: this.twentyConfigService.get('PG_SSL_ALLOW_SELF_SIGNED') diff --git a/packages/twenty-server/test/integration/graphql/suites/auth.integration-spec.ts b/packages/twenty-server/test/integration/graphql/suites/auth.integration-spec.ts index fca06b007..027f38ceb 100644 --- a/packages/twenty-server/test/integration/graphql/suites/auth.integration-spec.ts +++ b/packages/twenty-server/test/integration/graphql/suites/auth.integration-spec.ts @@ -23,7 +23,7 @@ describe('AuthResolve (integration)', () => { const queryData = { query: ` mutation GetLoginTokenFromCredentials { - getLoginTokenFromCredentials(email: "${auth.email}", password: "${auth.password}", origin: "http://localhost") { + getLoginTokenFromCredentials(email: "${auth.email}", password: "${auth.password}", origin: "${ORIGIN.toString()}") { loginToken { token expiresAt @@ -56,7 +56,7 @@ describe('AuthResolve (integration)', () => { const queryData = { query: ` mutation GetAuthTokensFromLoginToken { - getAuthTokensFromLoginToken(loginToken: "${loginToken}", origin: "http://localhost") { + getAuthTokensFromLoginToken(loginToken: "${loginToken}", origin: "${ORIGIN.toString()}") { tokens { accessToken { token diff --git a/packages/twenty-server/test/integration/graphql/suites/search/search-resolver.integration-spec.ts b/packages/twenty-server/test/integration/graphql/suites/search/search-resolver.integration-spec.ts index 1949a6355..5a3dea62c 100644 --- a/packages/twenty-server/test/integration/graphql/suites/search/search-resolver.integration-spec.ts +++ b/packages/twenty-server/test/integration/graphql/suites/search/search-resolver.integration-spec.ts @@ -10,11 +10,11 @@ import { TEST_PERSON_3_ID, } from 'test/integration/constants/test-person-ids.constants'; import { TEST_API_KEY_1_ID } from 'test/integration/constants/test-api-key-ids.constant'; -import { cleanTestDatabase } from 'test/integration/utils/clean-test-database'; import { TEST_PET_ID_1, TEST_PET_ID_2, } from 'test/integration/constants/test-pet-ids.constants'; +import { deleteAllRecords } from 'test/integration/utils/delete-all-records'; import { SearchResultEdgeDTO } from 'src/engine/core-modules/search/dtos/search-result-edge.dto'; import { @@ -45,7 +45,11 @@ describe('SearchResolver', () => { ]; beforeAll(async () => { - await cleanTestDatabase({ seed: false }); + await deleteAllRecords('person'); + await deleteAllRecords('company'); + await deleteAllRecords('opportunity'); + await deleteAllRecords('_pet'); + await deleteAllRecords('_surveyResult'); try { await performCreateManyOperation( @@ -74,10 +78,6 @@ describe('SearchResolver', () => { } }); - afterAll(async () => { - await cleanTestDatabase({ seed: true }); - }); - const testsUseCases: EachTestingContext<{ input: SearchArgs; eval: { @@ -94,6 +94,7 @@ describe('SearchResolver', () => { context: { input: { searchInput: '', + excludedObjectNameSingulars: ['workspaceMember'], limit: 50, }, eval: { @@ -122,6 +123,7 @@ describe('SearchResolver', () => { context: { input: { searchInput: 'searchInput1', + excludedObjectNameSingulars: ['workspaceMember'], limit: 50, }, eval: { @@ -144,6 +146,7 @@ describe('SearchResolver', () => { context: { input: { searchInput: '', + excludedObjectNameSingulars: ['workspaceMember'], includedObjectNameSingulars: ['pet'], limit: 50, }, @@ -166,7 +169,7 @@ describe('SearchResolver', () => { context: { input: { searchInput: '', - excludedObjectNameSingulars: ['person'], + excludedObjectNameSingulars: ['workspaceMember', 'person'], limit: 50, }, eval: { @@ -188,6 +191,7 @@ describe('SearchResolver', () => { context: { input: { searchInput: '', + excludedObjectNameSingulars: ['workspaceMember'], filter: { id: { eq: firstPet.id } }, limit: 50, }, @@ -210,6 +214,7 @@ describe('SearchResolver', () => { context: { input: { searchInput: '', + excludedObjectNameSingulars: ['workspaceMember'], limit: 4, }, eval: { @@ -237,6 +242,7 @@ describe('SearchResolver', () => { context: { input: { searchInput: '', + excludedObjectNameSingulars: ['workspaceMember'], limit: 2, }, eval: { @@ -258,6 +264,7 @@ describe('SearchResolver', () => { context: { input: { searchInput: '', + excludedObjectNameSingulars: ['workspaceMember'], after: encodeCursorData({ lastRanks: { tsRank: 0, tsRankCD: 0 }, lastRecordIdsPerObject: { @@ -286,6 +293,7 @@ describe('SearchResolver', () => { context: { input: { searchInput: 'searchInput', + excludedObjectNameSingulars: ['workspaceMember'], limit: 4, }, eval: { @@ -313,6 +321,7 @@ describe('SearchResolver', () => { context: { input: { searchInput: 'searchInput', + excludedObjectNameSingulars: ['workspaceMember'], limit: 2, }, eval: { @@ -335,6 +344,7 @@ describe('SearchResolver', () => { context: { input: { searchInput: 'searchInput', + excludedObjectNameSingulars: ['workspaceMember'], after: encodeCursorData({ lastRanks: { tsRank: 0.06079271, tsRankCD: 0.1 }, lastRecordIdsPerObject: { @@ -364,6 +374,7 @@ describe('SearchResolver', () => { context: { input: { searchInput: 'searchInput', + excludedObjectNameSingulars: ['workspaceMember'], after: encodeCursorData({ lastRanks: { tsRank: 0.06079271, tsRankCD: 0.1 }, lastRecordIdsPerObject: { @@ -393,7 +404,7 @@ describe('SearchResolver', () => { context: { input: { searchInput: '', - excludedObjectNameSingulars: ['person'], + excludedObjectNameSingulars: ['workspaceMember', 'person'], limit: 1, }, eval: { @@ -415,6 +426,7 @@ describe('SearchResolver', () => { context: { input: { searchInput: '', + excludedObjectNameSingulars: ['workspaceMember'], includedObjectNameSingulars: ['pet'], limit: 1, }, @@ -437,6 +449,7 @@ describe('SearchResolver', () => { context: { input: { searchInput: '', + excludedObjectNameSingulars: ['workspaceMember'], limit: 0, }, eval: { @@ -481,6 +494,7 @@ describe('SearchResolver', () => { it('should return cursor for each search edge', async () => { const graphqlOperation = searchFactory({ searchInput: 'searchInput', + excludedObjectNameSingulars: ['workspaceMember'], limit: 2, }); @@ -529,6 +543,7 @@ describe('SearchResolver', () => { it('should return cursor for each search edge with after cursor input', async () => { const graphqlOperation = searchFactory({ searchInput: 'searchInput', + excludedObjectNameSingulars: ['workspaceMember'], limit: 2, after: encodeCursorData({ lastRanks: { tsRankCD: 0.1, tsRank: 0.06079271 }, diff --git a/packages/twenty-server/test/integration/utils/clean-test-database.ts b/packages/twenty-server/test/integration/utils/clean-test-database.ts deleted file mode 100644 index 0dccf87fe..000000000 --- a/packages/twenty-server/test/integration/utils/clean-test-database.ts +++ /dev/null @@ -1,44 +0,0 @@ -import { deleteAllRecords } from 'test/integration/utils/delete-all-records'; - -import { SEED_APPLE_WORKSPACE_ID } from 'src/database/typeorm-seeds/core/workspaces'; - -export const cleanTestDatabase = async ({ seed }: { seed: boolean }) => { - if (process.env.NODE_ENV !== 'test') { - throw new Error( - "Don't run this 'setupTest' function in a non test environment", - ); - } - - await Promise.all([ - ...[ - 'person', - 'company', - 'opportunity', - 'workspaceMember', - '_pet', - '_surveyResult', - ].map( - async (objectMetadataNameSingular) => - await deleteAllRecords(objectMetadataNameSingular), - ), - ]); - - if (!seed) { - return; - } - - // @ts-expect-error legacy noImplicitAny - const mainDataSource = global.typeOrmService.getMainDataSource(); - - const dataSourceMetadata = - // @ts-expect-error legacy noImplicitAny - await global.dataSourceService.getLastDataSourceMetadataFromWorkspaceIdOrFail( - SEED_APPLE_WORKSPACE_ID, - ); - - // @ts-expect-error legacy noImplicitAny - await global.dataSeedWorkspaceCommand.seedRecords({ - mainDataSource, - dataSourceMetadata, - }); -}; diff --git a/packages/twenty-shared/src/utils/index.ts b/packages/twenty-shared/src/utils/index.ts index 9961e1bdc..f12ae0cd4 100644 --- a/packages/twenty-shared/src/utils/index.ts +++ b/packages/twenty-shared/src/utils/index.ts @@ -16,6 +16,7 @@ export { } from './image/getLogoUrlFromDomainName'; export { capitalize } from './strings/capitalize'; export { absoluteUrlSchema } from './url/absoluteUrlSchema'; +export { buildSignedPath } from './url/buildSignedPath'; export { getAbsoluteUrlOrThrow } from './url/getAbsoluteUrlOrThrow'; export { getUrlHostnameOrThrow } from './url/getUrlHostnameOrThrow'; export { isValidHostname } from './url/isValidHostname'; diff --git a/packages/twenty-shared/src/utils/url/__tests__/buildSignedPath.test.ts b/packages/twenty-shared/src/utils/url/__tests__/buildSignedPath.test.ts new file mode 100644 index 000000000..010582e34 --- /dev/null +++ b/packages/twenty-shared/src/utils/url/__tests__/buildSignedPath.test.ts @@ -0,0 +1,69 @@ +import { buildSignedPath } from '@/utils'; + +describe('buildSignedPath', () => { + it('should build a signed path', () => { + expect( + buildSignedPath({ path: 'folder/test.txt', token: 'tokenValue' }), + ).toBe('folder/tokenValue/test.txt'); + }); + + it('should build a signed path with original subFolder', () => { + expect( + buildSignedPath({ + path: 'folder/original/test.txt', + token: 'tokenValue', + }), + ).toBe('folder/original/tokenValue/test.txt'); + }); + + it('should build a signed path with multiple dots filename', () => { + expect( + buildSignedPath({ + path: 'folder/test.data.12032026.txt', + token: 'tokenValue', + }), + ).toBe('folder/tokenValue/test.data.12032026.txt'); + }); + + it('should throw when building signed path with missing filename', () => { + expect(() => + buildSignedPath({ + path: 'folder/', + token: 'tokenValue', + }), + ).toThrow( + "Filename empty: cannot build signed path from folderPath 'folder/'", + ); + }); + + it('should throw when building signed path with empty path', () => { + expect(() => + buildSignedPath({ + path: '', + token: 'tokenValue', + }), + ).toThrow("Filename empty: cannot build signed path from folderPath ''"); + }); + + it('should ignore absolute https urls', () => { + expect( + buildSignedPath({ + path: 'https://twentyhq.github.io/placeholder-images/workspaces/twenty-logo.png', + token: 'tokenValue', + }), + ).toBe( + 'https://twentyhq.github.io/placeholder-images/workspaces/twenty-logo.png', + ); + }); + + it('should ignore absolute http urls', () => { + expect( + buildSignedPath({ + path: 'http://twentyhq.github.io/placeholder-images/workspaces/twenty-logo.png', + token: 'tokenValue', + }), + ).toBe( + 'http://twentyhq.github.io/placeholder-images/workspaces/twenty-logo.png', + ); + }); +}); diff --git a/packages/twenty-shared/src/utils/url/buildSignedPath.ts b/packages/twenty-shared/src/utils/url/buildSignedPath.ts new file mode 100644 index 000000000..0592b18d8 --- /dev/null +++ b/packages/twenty-shared/src/utils/url/buildSignedPath.ts @@ -0,0 +1,25 @@ +import { isNonEmptyString } from '@sniptt/guards'; + +export const buildSignedPath = ({ + path, + token, +}: { + path: string; + token: string; +}) => { + if (path.startsWith('https:') || path.startsWith('http:')) { + return path; + } + + const directories = path.split('/'); + + const filename = directories.pop(); + + if (!isNonEmptyString(filename)) { + throw new Error( + `Filename empty: cannot build signed path from folderPath '${path}'`, + ); + } + + return `${directories.join('/')}/${token}/${filename}`; +}; diff --git a/packages/twenty-shared/src/utils/url/index.ts b/packages/twenty-shared/src/utils/url/index.ts index 9e4c6ac74..5b9cb1eb2 100644 --- a/packages/twenty-shared/src/utils/url/index.ts +++ b/packages/twenty-shared/src/utils/url/index.ts @@ -3,3 +3,4 @@ export * from './getAbsoluteUrlOrThrow'; export * from './getUrlHostnameOrThrow'; export * from './isValidHostname'; export * from './isValidUrl'; +export * from './buildSignedPath';