diff --git a/packages/twenty-server/src/engine/core-modules/workspace/constants/workspace-fields-updatable-before-activation.constant.ts b/packages/twenty-server/src/engine/core-modules/workspace/constants/workspace-fields-updatable-before-activation.constant.ts new file mode 100644 index 0000000000..541a355854 --- /dev/null +++ b/packages/twenty-server/src/engine/core-modules/workspace/constants/workspace-fields-updatable-before-activation.constant.ts @@ -0,0 +1,7 @@ +import { type WorkspaceEntity } from 'src/engine/core-modules/workspace/workspace.entity'; + +export const WORKSPACE_FIELDS_UPDATABLE_BEFORE_ACTIVATION = { + displayName: true, + subdomain: true, + logo: true, +} as const satisfies Partial>; diff --git a/packages/twenty-server/src/engine/core-modules/workspace/services/workspace.service.ts b/packages/twenty-server/src/engine/core-modules/workspace/services/workspace.service.ts index fa0ab9bdbc..b4f3388b05 100644 --- a/packages/twenty-server/src/engine/core-modules/workspace/services/workspace.service.ts +++ b/packages/twenty-server/src/engine/core-modules/workspace/services/workspace.service.ts @@ -49,6 +49,7 @@ import { UpgradeSequenceReaderService } from 'src/engine/core-modules/upgrade/se import { UserWorkspaceEntity } from 'src/engine/core-modules/user-workspace/user-workspace.entity'; import { UserWorkspaceService } from 'src/engine/core-modules/user-workspace/user-workspace.service'; import { UserEntity } from 'src/engine/core-modules/user/user.entity'; +import { WORKSPACE_FIELDS_UPDATABLE_BEFORE_ACTIVATION } from 'src/engine/core-modules/workspace/constants/workspace-fields-updatable-before-activation.constant'; import { WorkspaceEntity } from 'src/engine/core-modules/workspace/workspace.entity'; import { WorkspaceException, @@ -818,12 +819,6 @@ export class WorkspaceService { apiKey: ApiKeyEntity | undefined; workspaceActivationStatus: WorkspaceActivationStatus; }) { - if ( - workspaceActivationStatus === WorkspaceActivationStatus.PENDING_CREATION - ) { - return; - } - const systemFields = new Set(['id', 'createdAt', 'updatedAt', 'deletedAt']); const fieldsBeingUpdated = Object.keys(payload).filter( @@ -834,6 +829,28 @@ export class WorkspaceService { return; } + if ( + workspaceActivationStatus === WorkspaceActivationStatus.PENDING_CREATION + ) { + const fieldsRequiringActivation = fieldsBeingUpdated.filter( + (field) => !(field in WORKSPACE_FIELDS_UPDATABLE_BEFORE_ACTIVATION), + ); + + if (fieldsRequiringActivation.length > 0) { + const fieldsList = fieldsRequiringActivation.join(', '); + + throw new PermissionsException( + PermissionsExceptionMessage.PERMISSION_DENIED, + PermissionsExceptionCode.PERMISSION_DENIED, + { + userFriendlyMessage: msg`These fields cannot be updated before the workspace is activated: ${fieldsList}.`, + }, + ); + } + + return; + } + if (!userWorkspaceId) { throw new Error('Missing userWorkspaceId in authContext'); } diff --git a/packages/twenty-server/test/integration/graphql/suites/settings-permissions/__snapshots__/workspace-update-before-activation.integration-spec.ts.snap b/packages/twenty-server/test/integration/graphql/suites/settings-permissions/__snapshots__/workspace-update-before-activation.integration-spec.ts.snap new file mode 100644 index 0000000000..4db86ada6c --- /dev/null +++ b/packages/twenty-server/test/integration/graphql/suites/settings-permissions/__snapshots__/workspace-update-before-activation.integration-spec.ts.snap @@ -0,0 +1,25 @@ +// Jest Snapshot v1, https://goo.gl/fbAQLP + +exports[`updateWorkspace while the workspace is pending creation rejects security sensitive fields 1`] = ` +{ + "extensions": { + "code": "FORBIDDEN", + "subCode": "PERMISSION_DENIED", + "userFriendlyMessage": "User does not have permission.", + }, + "message": "Entity performing the request does not have permission", + "name": "ForbiddenError", +} +`; + +exports[`updateWorkspace while the workspace is pending creation rejects the whole update when a setup field is mixed with a security sensitive one 1`] = ` +{ + "extensions": { + "code": "FORBIDDEN", + "subCode": "PERMISSION_DENIED", + "userFriendlyMessage": "User does not have permission.", + }, + "message": "Entity performing the request does not have permission", + "name": "ForbiddenError", +} +`; diff --git a/packages/twenty-server/test/integration/graphql/suites/settings-permissions/workspace-update-before-activation.integration-spec.ts b/packages/twenty-server/test/integration/graphql/suites/settings-permissions/workspace-update-before-activation.integration-spec.ts new file mode 100644 index 0000000000..6f5f2a8a32 --- /dev/null +++ b/packages/twenty-server/test/integration/graphql/suites/settings-permissions/workspace-update-before-activation.integration-spec.ts @@ -0,0 +1,116 @@ +import { expectOneNotInternalServerErrorSnapshot } from 'test/integration/graphql/utils/expect-one-not-internal-server-error-snapshot.util'; +import { updateWorkspace } from 'test/integration/graphql/utils/update-workspace.util'; +import { WorkspaceActivationStatus } from 'twenty-shared/workspace'; + +import { WORKSPACE_FIELDS_UPDATABLE_BEFORE_ACTIVATION } from 'src/engine/core-modules/workspace/constants/workspace-fields-updatable-before-activation.constant'; +import { type UpdateWorkspaceInput } from 'src/engine/core-modules/workspace/dtos/update-workspace-input'; +import { SEED_APPLE_WORKSPACE_ID } from 'src/engine/workspace-manager/dev-seeder/core/constants/seeder-workspaces.constant'; + +// Every allowlisted field needs a value here, so adding one to the constant +// without covering it fails to compile. +const SETUP_FIELD_VALUES = { + displayName: `Pending setup ${Date.now()}`, + subdomain: `pending-setup-${Date.now()}`, + logo: 'pending-setup-logo.png', +} satisfies Record< + keyof typeof WORKSPACE_FIELDS_UPDATABLE_BEFORE_ACTIVATION, + string +>; + +const setActivationStatus = (activationStatus: WorkspaceActivationStatus) => + testDataSource.query( + 'UPDATE core.workspace SET "activationStatus" = $1 WHERE id = $2', + [activationStatus, SEED_APPLE_WORKSPACE_ID], + ); + +describe('updateWorkspace while the workspace is pending creation', () => { + let originalDisplayName: string; + let originalSubdomain: string; + let originalLogo: string | null; + let originalAllowImpersonation: boolean; + + beforeAll(async () => { + const [workspace] = await testDataSource.query( + 'SELECT "displayName", "subdomain", "logo", "allowImpersonation" FROM core.workspace WHERE id = $1', + [SEED_APPLE_WORKSPACE_ID], + ); + + originalDisplayName = workspace.displayName; + originalSubdomain = workspace.subdomain; + originalLogo = workspace.logo; + originalAllowImpersonation = workspace.allowImpersonation; + + await setActivationStatus(WorkspaceActivationStatus.PENDING_CREATION); + }); + + afterAll(async () => { + await setActivationStatus(WorkspaceActivationStatus.ACTIVE); + + await testDataSource.query( + 'UPDATE core.workspace SET "displayName" = $1, "subdomain" = $2, "logo" = $3 WHERE id = $4', + [ + originalDisplayName, + originalSubdomain, + originalLogo, + SEED_APPLE_WORKSPACE_ID, + ], + ); + }); + + it('rejects security sensitive fields', async () => { + const { data, errors } = await updateWorkspace({ + data: { allowImpersonation: !originalAllowImpersonation }, + expectToFail: true, + }); + + expect(data?.updateWorkspace).toBeFalsy(); + expectOneNotInternalServerErrorSnapshot({ errors }); + + const [workspace] = await testDataSource.query( + 'SELECT "allowImpersonation" FROM core.workspace WHERE id = $1', + [SEED_APPLE_WORKSPACE_ID], + ); + + expect(workspace.allowImpersonation).toBe(originalAllowImpersonation); + }); + + it('rejects the whole update when a setup field is mixed with a security sensitive one', async () => { + const { data, errors } = await updateWorkspace({ + data: { + displayName: 'Should not be applied', + isPublicInviteLinkEnabled: true, + }, + expectToFail: true, + }); + + expect(data?.updateWorkspace).toBeFalsy(); + expectOneNotInternalServerErrorSnapshot({ errors }); + + const [workspace] = await testDataSource.query( + 'SELECT "displayName" FROM core.workspace WHERE id = $1', + [SEED_APPLE_WORKSPACE_ID], + ); + + expect(workspace.displayName).toBe(originalDisplayName); + }); + + it.each(Object.entries(SETUP_FIELD_VALUES))( + 'still allows %s, which is needed to set the workspace up', + async (field, value) => { + const { data, errors } = await updateWorkspace({ + data: { [field]: value } as UpdateWorkspaceInput, + expectToFail: false, + }); + + expect(errors).toBeUndefined(); + expect(data.updateWorkspace.id).toBe(SEED_APPLE_WORKSPACE_ID); + + const [workspace] = await testDataSource.query( + `SELECT "${field}" FROM core.workspace WHERE id = $1`, + [SEED_APPLE_WORKSPACE_ID], + ); + + expect(workspace[field]).toBe(value); + }, + ); +}); diff --git a/packages/twenty-server/test/integration/graphql/utils/update-workspace-operation-factory.util.ts b/packages/twenty-server/test/integration/graphql/utils/update-workspace-operation-factory.util.ts new file mode 100644 index 0000000000..27b71ab65f --- /dev/null +++ b/packages/twenty-server/test/integration/graphql/utils/update-workspace-operation-factory.util.ts @@ -0,0 +1,18 @@ +import gql from 'graphql-tag'; + +import { type UpdateWorkspaceInput } from 'src/engine/core-modules/workspace/dtos/update-workspace-input'; + +export const updateWorkspaceOperationFactory = ({ + data, +}: { + data: UpdateWorkspaceInput; +}) => ({ + query: gql` + mutation UpdateWorkspace($data: UpdateWorkspaceInput!) { + updateWorkspace(data: $data) { + id + } + } + `, + variables: { data }, +}); diff --git a/packages/twenty-server/test/integration/graphql/utils/update-workspace.util.ts b/packages/twenty-server/test/integration/graphql/utils/update-workspace.util.ts new file mode 100644 index 0000000000..7406e7a190 --- /dev/null +++ b/packages/twenty-server/test/integration/graphql/utils/update-workspace.util.ts @@ -0,0 +1,42 @@ +import { updateWorkspaceOperationFactory } from 'test/integration/graphql/utils/update-workspace-operation-factory.util'; +import { makeMetadataAPIRequest } from 'test/integration/metadata/suites/utils/make-metadata-api-request.util'; +import { type CommonResponseBody } from 'test/integration/metadata/types/common-response-body.type'; +import { warnIfErrorButNotExpectedToFail } from 'test/integration/metadata/utils/warn-if-error-but-not-expected-to-fail.util'; +import { warnIfNoErrorButExpectedToFail } from 'test/integration/metadata/utils/warn-if-no-error-but-expected-to-fail.util'; + +import { type UpdateWorkspaceInput } from 'src/engine/core-modules/workspace/dtos/update-workspace-input'; + +type UpdateWorkspaceUtilArgs = { + data: UpdateWorkspaceInput; + expectToFail?: boolean; + token?: string; +}; + +export const updateWorkspace = async ({ + data, + expectToFail = false, + token, +}: UpdateWorkspaceUtilArgs): CommonResponseBody<{ + updateWorkspace: { id: string }; +}> => { + const response = await makeMetadataAPIRequest( + updateWorkspaceOperationFactory({ data }), + token, + ); + + if (expectToFail === true) { + warnIfNoErrorButExpectedToFail({ + response, + errorMessage: 'updateWorkspace should have failed but did not', + }); + } + + if (expectToFail === false) { + warnIfErrorButNotExpectedToFail({ + response, + errorMessage: 'updateWorkspace has failed but should not', + }); + } + + return { data: response.body.data, errors: response.body.errors }; +};