From e4f06a7c97698997bb3cdda96292f5df56d1fe67 Mon Sep 17 00:00:00 2001 From: Marie <51697796+ijreilly@users.noreply.github.com> Date: Mon, 24 Feb 2025 16:59:28 +0100 Subject: [PATCH] [permissions] Add permission gates on workspaceMember (#10447) - Adding permission gates on workspaceMember to only allow user with admin permissions OR users attempting to update or delete themself to perform write operations on workspaceMember object - Reverting some changes to treat workflow objects as regular metadata objects (any user can interact with them) - (fix) Block updates on soft deleted records --- .../interfaces/base-resolver-service.ts | 15 +- ...phql-query-update-many-resolver.service.ts | 13 ++ ...aphql-query-update-one-resolver.service.ts | 9 + ...space-member-create-many.pre-query.hook.ts | 32 +++ ...kspace-member-create-one.pre-query.hook.ts | 32 +++ ...space-member-delete-many.pre-query.hook.ts | 25 ++- ...kspace-member-delete-one.pre-query.hook.ts | 21 +- ...pace-member-destroy-many.pre-query.hook.ts | 32 +++ ...space-member-destroy-one.pre-query.hook.ts | 33 +++ ...workspace-member-pre-query-hook.service.ts | 79 +++++++ .../workspace-member-query-hook.module.ts | 21 ++ ...pace-member-restore-many.pre-query.hook.ts | 32 +++ ...space-member-restore-one.pre-query.hook.ts | 33 +++ ...space-member-update-many.pre-query.hook.ts | 32 +++ ...kspace-member-update-one.pre-query.hook.ts | 33 +++ .../workspace-members.integration-spec.ts | 211 ++++++++++++++++++ ...aphql-api-request-with-member-role.util.ts | 21 ++ packages/twenty-shared/src/utils/index.ts | 1 - .../src/utils/permissions/index.ts | 1 - ...jectRecordUnderObjectRecordsPermissions.ts | 16 -- 20 files changed, 655 insertions(+), 37 deletions(-) create mode 100644 packages/twenty-server/src/modules/workspace-member/query-hooks/workspace-member-create-many.pre-query.hook.ts create mode 100644 packages/twenty-server/src/modules/workspace-member/query-hooks/workspace-member-create-one.pre-query.hook.ts create mode 100644 packages/twenty-server/src/modules/workspace-member/query-hooks/workspace-member-destroy-many.pre-query.hook.ts create mode 100644 packages/twenty-server/src/modules/workspace-member/query-hooks/workspace-member-destroy-one.pre-query.hook.ts create mode 100644 packages/twenty-server/src/modules/workspace-member/query-hooks/workspace-member-pre-query-hook.service.ts create mode 100644 packages/twenty-server/src/modules/workspace-member/query-hooks/workspace-member-restore-many.pre-query.hook.ts create mode 100644 packages/twenty-server/src/modules/workspace-member/query-hooks/workspace-member-restore-one.pre-query.hook.ts create mode 100644 packages/twenty-server/src/modules/workspace-member/query-hooks/workspace-member-update-many.pre-query.hook.ts create mode 100644 packages/twenty-server/src/modules/workspace-member/query-hooks/workspace-member-update-one.pre-query.hook.ts create mode 100644 packages/twenty-server/test/integration/graphql/suites/settings-permissions/workspace-members.integration-spec.ts create mode 100644 packages/twenty-server/test/integration/graphql/utils/make-graphql-api-request-with-member-role.util.ts delete mode 100644 packages/twenty-shared/src/utils/permissions/index.ts delete mode 100644 packages/twenty-shared/src/utils/permissions/isObjectRecordUnderObjectRecordsPermissions.ts diff --git a/packages/twenty-server/src/engine/api/graphql/graphql-query-runner/interfaces/base-resolver-service.ts b/packages/twenty-server/src/engine/api/graphql/graphql-query-runner/interfaces/base-resolver-service.ts index ba4bd2a71..87dcde5af 100644 --- a/packages/twenty-server/src/engine/api/graphql/graphql-query-runner/interfaces/base-resolver-service.ts +++ b/packages/twenty-server/src/engine/api/graphql/graphql-query-runner/interfaces/base-resolver-service.ts @@ -1,11 +1,7 @@ import { Inject, Injectable } from '@nestjs/common'; import graphqlFields from 'graphql-fields'; -import { - capitalize, - isObjectRecordUnderObjectRecordsPermissions, - PermissionsOnAllObjectRecords, -} from 'twenty-shared'; +import { capitalize, PermissionsOnAllObjectRecords } from 'twenty-shared'; import { DataSource, ObjectLiteral } from 'typeorm'; import { ObjectRecord } from 'src/engine/api/graphql/workspace-query-builder/interfaces/object-record.interface'; @@ -103,12 +99,9 @@ export abstract class GraphqlQueryBaseResolverService< if ( featureFlagsMap[FeatureFlagKey.IsPermissionsEnabled] && - isObjectRecordUnderObjectRecordsPermissions({ - isCustom: objectMetadataItemWithFieldMaps.isCustom, - nameSingular: objectMetadataItemWithFieldMaps.nameSingular, - }) + !objectMetadataItemWithFieldMaps.isSystem ) { - await this.validateCustomObjectPermissionsOrThrow({ + await this.validateObjectRecordPermissionsOrThrow({ operationName, options, }); @@ -230,7 +223,7 @@ export abstract class GraphqlQueryBaseResolverService< } } - private async validateCustomObjectPermissionsOrThrow({ + private async validateObjectRecordPermissionsOrThrow({ operationName, options, }: { diff --git a/packages/twenty-server/src/engine/api/graphql/graphql-query-runner/resolvers/graphql-query-update-many-resolver.service.ts b/packages/twenty-server/src/engine/api/graphql/graphql-query-runner/resolvers/graphql-query-update-many-resolver.service.ts index 6d6c03148..74f63c6fd 100644 --- a/packages/twenty-server/src/engine/api/graphql/graphql-query-runner/resolvers/graphql-query-update-many-resolver.service.ts +++ b/packages/twenty-server/src/engine/api/graphql/graphql-query-runner/resolvers/graphql-query-update-many-resolver.service.ts @@ -1,5 +1,7 @@ import { Injectable } from '@nestjs/common'; +import isEmpty from 'lodash.isempty'; + import { GraphqlQueryBaseResolverService, GraphqlQueryResolverExecutionArgs, @@ -9,6 +11,10 @@ import { WorkspaceQueryRunnerOptions } from 'src/engine/api/graphql/workspace-qu import { UpdateManyResolverArgs } from 'src/engine/api/graphql/workspace-resolver-builder/interfaces/workspace-resolvers-builder.interface'; import { QUERY_MAX_RECORDS } from 'src/engine/api/graphql/graphql-query-runner/constants/query-max-records.constant'; +import { + GraphqlQueryRunnerException, + GraphqlQueryRunnerExceptionCode, +} from 'src/engine/api/graphql/graphql-query-runner/errors/graphql-query-runner.exception'; import { ObjectRecordsToGraphqlConnectionHelper } from 'src/engine/api/graphql/graphql-query-runner/helpers/object-records-to-graphql-connection.helper'; import { assertIsValidUuid } from 'src/engine/api/graphql/workspace-query-runner/utils/assert-is-valid-uuid.util'; import { FeatureFlagKey } from 'src/engine/core-modules/feature-flag/enums/feature-flag-key.enum'; @@ -49,6 +55,13 @@ export class GraphqlQueryUpdateManyResolverService extends GraphqlQueryBaseResol objectMetadataMaps, ); + if (isEmpty(formattedExistingRecords)) { + throw new GraphqlQueryRunnerException( + 'Records not found', + GraphqlQueryRunnerExceptionCode.RECORD_NOT_FOUND, + ); + } + const tableName = computeTableName( objectMetadataItemWithFieldMaps.nameSingular, objectMetadataItemWithFieldMaps.isCustom, diff --git a/packages/twenty-server/src/engine/api/graphql/graphql-query-runner/resolvers/graphql-query-update-one-resolver.service.ts b/packages/twenty-server/src/engine/api/graphql/graphql-query-runner/resolvers/graphql-query-update-one-resolver.service.ts index 62361d719..c0cbb26cf 100644 --- a/packages/twenty-server/src/engine/api/graphql/graphql-query-runner/resolvers/graphql-query-update-one-resolver.service.ts +++ b/packages/twenty-server/src/engine/api/graphql/graphql-query-runner/resolvers/graphql-query-update-one-resolver.service.ts @@ -1,5 +1,7 @@ import { Injectable } from '@nestjs/common'; +import isEmpty from 'lodash.isempty'; + import { GraphqlQueryBaseResolverService, GraphqlQueryResolverExecutionArgs, @@ -53,6 +55,13 @@ export class GraphqlQueryUpdateOneResolverService extends GraphqlQueryBaseResolv objectMetadataMaps, ); + if (isEmpty(formattedExistingRecords)) { + throw new GraphqlQueryRunnerException( + 'Record not found', + GraphqlQueryRunnerExceptionCode.RECORD_NOT_FOUND, + ); + } + const nonFormattedUpdatedObjectRecords = await queryBuilder .update(data) .where({ id: executionArgs.args.id }) diff --git a/packages/twenty-server/src/modules/workspace-member/query-hooks/workspace-member-create-many.pre-query.hook.ts b/packages/twenty-server/src/modules/workspace-member/query-hooks/workspace-member-create-many.pre-query.hook.ts new file mode 100644 index 000000000..5d77d2898 --- /dev/null +++ b/packages/twenty-server/src/modules/workspace-member/query-hooks/workspace-member-create-many.pre-query.hook.ts @@ -0,0 +1,32 @@ +import { WorkspaceQueryHookInstance } from 'src/engine/api/graphql/workspace-query-runner/workspace-query-hook/interfaces/workspace-query-hook.interface'; +import { CreateManyResolverArgs } from 'src/engine/api/graphql/workspace-resolver-builder/interfaces/workspace-resolvers-builder.interface'; + +import { WorkspaceQueryHook } from 'src/engine/api/graphql/workspace-query-runner/workspace-query-hook/decorators/workspace-query-hook.decorator'; +import { AuthContext } from 'src/engine/core-modules/auth/types/auth-context.type'; +import { WorkspaceMemberPreQueryHookService } from 'src/modules/workspace-member/query-hooks/workspace-member-pre-query-hook.service'; + +@WorkspaceQueryHook(`workspaceMember.createMany`) +export class WorkspaceMemberCreateManyPreQueryHook + implements WorkspaceQueryHookInstance +{ + constructor( + private readonly workspaceMemberPreQueryHookService: WorkspaceMemberPreQueryHookService, + ) {} + + async execute( + authContext: AuthContext, + objectName: string, + payload: CreateManyResolverArgs, + ): Promise { + await this.workspaceMemberPreQueryHookService.validateWorkspaceMemberUpdatePermissionOrThrow( + { + userWorkspaceId: authContext.userWorkspaceId, + workspaceId: authContext.workspace.id, + apiKey: authContext.apiKey, + workspaceMemberId: authContext.workspaceMemberId, + }, + ); + + return payload; + } +} diff --git a/packages/twenty-server/src/modules/workspace-member/query-hooks/workspace-member-create-one.pre-query.hook.ts b/packages/twenty-server/src/modules/workspace-member/query-hooks/workspace-member-create-one.pre-query.hook.ts new file mode 100644 index 000000000..95d79e3fc --- /dev/null +++ b/packages/twenty-server/src/modules/workspace-member/query-hooks/workspace-member-create-one.pre-query.hook.ts @@ -0,0 +1,32 @@ +import { WorkspaceQueryHookInstance } from 'src/engine/api/graphql/workspace-query-runner/workspace-query-hook/interfaces/workspace-query-hook.interface'; +import { CreateOneResolverArgs } from 'src/engine/api/graphql/workspace-resolver-builder/interfaces/workspace-resolvers-builder.interface'; + +import { WorkspaceQueryHook } from 'src/engine/api/graphql/workspace-query-runner/workspace-query-hook/decorators/workspace-query-hook.decorator'; +import { AuthContext } from 'src/engine/core-modules/auth/types/auth-context.type'; +import { WorkspaceMemberPreQueryHookService } from 'src/modules/workspace-member/query-hooks/workspace-member-pre-query-hook.service'; + +@WorkspaceQueryHook(`workspaceMember.createOne`) +export class WorkspaceMemberCreateOnePreQueryHook + implements WorkspaceQueryHookInstance +{ + constructor( + private readonly workspaceMemberPreQueryHookService: WorkspaceMemberPreQueryHookService, + ) {} + + async execute( + authContext: AuthContext, + objectName: string, + payload: CreateOneResolverArgs, + ): Promise { + await this.workspaceMemberPreQueryHookService.validateWorkspaceMemberUpdatePermissionOrThrow( + { + userWorkspaceId: authContext.userWorkspaceId, + workspaceId: authContext.workspace.id, + apiKey: authContext.apiKey, + workspaceMemberId: authContext.workspaceMemberId, + }, + ); + + return payload; + } +} diff --git a/packages/twenty-server/src/modules/workspace-member/query-hooks/workspace-member-delete-many.pre-query.hook.ts b/packages/twenty-server/src/modules/workspace-member/query-hooks/workspace-member-delete-many.pre-query.hook.ts index a292f50c2..9f0345bc8 100644 --- a/packages/twenty-server/src/modules/workspace-member/query-hooks/workspace-member-delete-many.pre-query.hook.ts +++ b/packages/twenty-server/src/modules/workspace-member/query-hooks/workspace-member-delete-many.pre-query.hook.ts @@ -1,17 +1,32 @@ -import { MethodNotAllowedException } from '@nestjs/common'; - import { WorkspaceQueryHookInstance } from 'src/engine/api/graphql/workspace-query-runner/workspace-query-hook/interfaces/workspace-query-hook.interface'; import { DeleteManyResolverArgs } from 'src/engine/api/graphql/workspace-resolver-builder/interfaces/workspace-resolvers-builder.interface'; import { WorkspaceQueryHook } from 'src/engine/api/graphql/workspace-query-runner/workspace-query-hook/decorators/workspace-query-hook.decorator'; +import { AuthContext } from 'src/engine/core-modules/auth/types/auth-context.type'; +import { WorkspaceMemberPreQueryHookService } from 'src/modules/workspace-member/query-hooks/workspace-member-pre-query-hook.service'; @WorkspaceQueryHook(`workspaceMember.deleteMany`) export class WorkspaceMemberDeleteManyPreQueryHook implements WorkspaceQueryHookInstance { - constructor() {} + constructor( + private readonly workspaceMemberPreQueryHookService: WorkspaceMemberPreQueryHookService, + ) {} - async execute(): Promise { - throw new MethodNotAllowedException('Method not allowed.'); + async execute( + authContext: AuthContext, + objectName: string, + payload: DeleteManyResolverArgs, + ): Promise { + await this.workspaceMemberPreQueryHookService.validateWorkspaceMemberUpdatePermissionOrThrow( + { + userWorkspaceId: authContext.userWorkspaceId, + workspaceId: authContext.workspace.id, + apiKey: authContext.apiKey, + workspaceMemberId: authContext.workspaceMemberId, + }, + ); + + return payload; } } diff --git a/packages/twenty-server/src/modules/workspace-member/query-hooks/workspace-member-delete-one.pre-query.hook.ts b/packages/twenty-server/src/modules/workspace-member/query-hooks/workspace-member-delete-one.pre-query.hook.ts index 18ad2bbaa..f374301d0 100644 --- a/packages/twenty-server/src/modules/workspace-member/query-hooks/workspace-member-delete-one.pre-query.hook.ts +++ b/packages/twenty-server/src/modules/workspace-member/query-hooks/workspace-member-delete-one.pre-query.hook.ts @@ -5,25 +5,40 @@ import { WorkspaceQueryHook } from 'src/engine/api/graphql/workspace-query-runne import { AuthContext } from 'src/engine/core-modules/auth/types/auth-context.type'; import { TwentyORMManager } from 'src/engine/twenty-orm/twenty-orm.manager'; import { AttachmentWorkspaceEntity } from 'src/modules/attachment/standard-objects/attachment.workspace-entity'; +import { WorkspaceMemberPreQueryHookService } from 'src/modules/workspace-member/query-hooks/workspace-member-pre-query-hook.service'; @WorkspaceQueryHook(`workspaceMember.deleteOne`) export class WorkspaceMemberDeleteOnePreQueryHook implements WorkspaceQueryHookInstance { - constructor(private readonly twentyORMManager: TwentyORMManager) {} + constructor( + private readonly twentyORMManager: TwentyORMManager, + private readonly workspaceMemberPreQueryHookService: WorkspaceMemberPreQueryHookService, + ) {} - // There is no need to validate the user's access to the workspace member since we don't have permission yet. async execute( authContext: AuthContext, objectName: string, payload: DeleteOneResolverArgs, ): Promise { + const targettedWorkspaceMemberId = payload.id; + + await this.workspaceMemberPreQueryHookService.validateWorkspaceMemberUpdatePermissionOrThrow( + { + userWorkspaceId: authContext.userWorkspaceId, + workspaceMemberId: authContext.workspaceMemberId, + targettedWorkspaceMemberId, + workspaceId: authContext.workspace.id, + apiKey: authContext.apiKey, + }, + ); + const attachmentRepository = await this.twentyORMManager.getRepository( 'attachment', ); - const authorId = payload.id; + const authorId = targettedWorkspaceMemberId; await attachmentRepository.delete({ authorId, diff --git a/packages/twenty-server/src/modules/workspace-member/query-hooks/workspace-member-destroy-many.pre-query.hook.ts b/packages/twenty-server/src/modules/workspace-member/query-hooks/workspace-member-destroy-many.pre-query.hook.ts new file mode 100644 index 000000000..192f99e3a --- /dev/null +++ b/packages/twenty-server/src/modules/workspace-member/query-hooks/workspace-member-destroy-many.pre-query.hook.ts @@ -0,0 +1,32 @@ +import { WorkspaceQueryHookInstance } from 'src/engine/api/graphql/workspace-query-runner/workspace-query-hook/interfaces/workspace-query-hook.interface'; +import { DeleteManyResolverArgs } from 'src/engine/api/graphql/workspace-resolver-builder/interfaces/workspace-resolvers-builder.interface'; + +import { WorkspaceQueryHook } from 'src/engine/api/graphql/workspace-query-runner/workspace-query-hook/decorators/workspace-query-hook.decorator'; +import { AuthContext } from 'src/engine/core-modules/auth/types/auth-context.type'; +import { WorkspaceMemberPreQueryHookService } from 'src/modules/workspace-member/query-hooks/workspace-member-pre-query-hook.service'; + +@WorkspaceQueryHook(`workspaceMember.destroyMany`) +export class WorkspaceMemberDestroyManyPreQueryHook + implements WorkspaceQueryHookInstance +{ + constructor( + private readonly workspaceMemberPreQueryHookService: WorkspaceMemberPreQueryHookService, + ) {} + + async execute( + authContext: AuthContext, + objectName: string, + payload: DeleteManyResolverArgs, + ): Promise { + await this.workspaceMemberPreQueryHookService.validateWorkspaceMemberUpdatePermissionOrThrow( + { + userWorkspaceId: authContext.userWorkspaceId, + workspaceId: authContext.workspace.id, + apiKey: authContext.apiKey, + workspaceMemberId: authContext.workspaceMemberId, + }, + ); + + return payload; + } +} diff --git a/packages/twenty-server/src/modules/workspace-member/query-hooks/workspace-member-destroy-one.pre-query.hook.ts b/packages/twenty-server/src/modules/workspace-member/query-hooks/workspace-member-destroy-one.pre-query.hook.ts new file mode 100644 index 000000000..11b3b9720 --- /dev/null +++ b/packages/twenty-server/src/modules/workspace-member/query-hooks/workspace-member-destroy-one.pre-query.hook.ts @@ -0,0 +1,33 @@ +import { WorkspaceQueryHookInstance } from 'src/engine/api/graphql/workspace-query-runner/workspace-query-hook/interfaces/workspace-query-hook.interface'; +import { DeleteOneResolverArgs } from 'src/engine/api/graphql/workspace-resolver-builder/interfaces/workspace-resolvers-builder.interface'; + +import { WorkspaceQueryHook } from 'src/engine/api/graphql/workspace-query-runner/workspace-query-hook/decorators/workspace-query-hook.decorator'; +import { AuthContext } from 'src/engine/core-modules/auth/types/auth-context.type'; +import { WorkspaceMemberPreQueryHookService } from 'src/modules/workspace-member/query-hooks/workspace-member-pre-query-hook.service'; + +@WorkspaceQueryHook(`workspaceMember.destroyOne`) +export class WorkspaceMemberDestroyOnePreQueryHook + implements WorkspaceQueryHookInstance +{ + constructor( + private readonly workspaceMemberPreQueryHookService: WorkspaceMemberPreQueryHookService, + ) {} + + async execute( + authContext: AuthContext, + objectName: string, + payload: DeleteOneResolverArgs, + ): Promise { + await this.workspaceMemberPreQueryHookService.validateWorkspaceMemberUpdatePermissionOrThrow( + { + userWorkspaceId: authContext.userWorkspaceId, + targettedWorkspaceMemberId: payload.id, + workspaceId: authContext.workspace.id, + apiKey: authContext.apiKey, + workspaceMemberId: authContext.workspaceMemberId, + }, + ); + + return payload; + } +} diff --git a/packages/twenty-server/src/modules/workspace-member/query-hooks/workspace-member-pre-query-hook.service.ts b/packages/twenty-server/src/modules/workspace-member/query-hooks/workspace-member-pre-query-hook.service.ts new file mode 100644 index 000000000..b94934073 --- /dev/null +++ b/packages/twenty-server/src/modules/workspace-member/query-hooks/workspace-member-pre-query-hook.service.ts @@ -0,0 +1,79 @@ +import { Injectable } from '@nestjs/common'; + +import { isDefined } from 'twenty-shared'; + +import { FeatureFlagKey } from 'src/engine/core-modules/feature-flag/enums/feature-flag-key.enum'; +import { FeatureFlagService } from 'src/engine/core-modules/feature-flag/services/feature-flag.service'; +import { SettingsPermissions } from 'src/engine/metadata-modules/permissions/constants/settings-permissions.constants'; +import { + PermissionsException, + PermissionsExceptionCode, + PermissionsExceptionMessage, +} from 'src/engine/metadata-modules/permissions/permissions.exception'; +import { PermissionsService } from 'src/engine/metadata-modules/permissions/permissions.service'; +import { ApiKeyWorkspaceEntity } from 'src/modules/api-key/standard-objects/api-key.workspace-entity'; + +@Injectable() +export class WorkspaceMemberPreQueryHookService { + constructor( + private readonly permissionsService: PermissionsService, + private readonly featureFlagService: FeatureFlagService, + ) {} + + async validateWorkspaceMemberUpdatePermissionOrThrow({ + userWorkspaceId, + workspaceMemberId, + targettedWorkspaceMemberId, + workspaceId, + apiKey, + }: { + userWorkspaceId?: string; + workspaceMemberId?: string; + targettedWorkspaceMemberId?: string; + workspaceId: string; + apiKey?: ApiKeyWorkspaceEntity | null; + }) { + const featureFlagsMap = + await this.featureFlagService.getWorkspaceFeatureFlagsMap(workspaceId); + + const isPermissionsEnabled = + featureFlagsMap[FeatureFlagKey.IsPermissionsEnabled]; + + if (!isPermissionsEnabled) { + return; + } + + if (isDefined(apiKey)) { + return; + } + + if (!userWorkspaceId) { + throw new PermissionsException( + PermissionsExceptionMessage.USER_WORKSPACE_NOT_FOUND, + PermissionsExceptionCode.USER_WORKSPACE_NOT_FOUND, + ); + } + + if ( + isDefined(targettedWorkspaceMemberId) && + workspaceMemberId === targettedWorkspaceMemberId + ) { + return; + } + + if ( + await this.permissionsService.userHasWorkspaceSettingPermission({ + userWorkspaceId, + workspaceId, + _setting: SettingsPermissions.WORKSPACE_MEMBERS, + }) + ) { + return; + } + + throw new PermissionsException( + PermissionsExceptionMessage.PERMISSION_DENIED, + PermissionsExceptionCode.PERMISSION_DENIED, + ); + } +} diff --git a/packages/twenty-server/src/modules/workspace-member/query-hooks/workspace-member-query-hook.module.ts b/packages/twenty-server/src/modules/workspace-member/query-hooks/workspace-member-query-hook.module.ts index 7359096dd..dae6180ab 100644 --- a/packages/twenty-server/src/modules/workspace-member/query-hooks/workspace-member-query-hook.module.ts +++ b/packages/twenty-server/src/modules/workspace-member/query-hooks/workspace-member-query-hook.module.ts @@ -1,12 +1,33 @@ import { Module } from '@nestjs/common'; +import { FeatureFlagModule } from 'src/engine/core-modules/feature-flag/feature-flag.module'; +import { PermissionsModule } from 'src/engine/metadata-modules/permissions/permissions.module'; +import { WorkspaceMemberCreateManyPreQueryHook } from 'src/modules/workspace-member/query-hooks/workspace-member-create-many.pre-query.hook'; +import { WorkspaceMemberCreateOnePreQueryHook } from 'src/modules/workspace-member/query-hooks/workspace-member-create-one.pre-query.hook'; import { WorkspaceMemberDeleteManyPreQueryHook } from 'src/modules/workspace-member/query-hooks/workspace-member-delete-many.pre-query.hook'; import { WorkspaceMemberDeleteOnePreQueryHook } from 'src/modules/workspace-member/query-hooks/workspace-member-delete-one.pre-query.hook'; +import { WorkspaceMemberDestroyManyPreQueryHook } from 'src/modules/workspace-member/query-hooks/workspace-member-destroy-many.pre-query.hook'; +import { WorkspaceMemberDestroyOnePreQueryHook } from 'src/modules/workspace-member/query-hooks/workspace-member-destroy-one.pre-query.hook'; +import { WorkspaceMemberPreQueryHookService } from 'src/modules/workspace-member/query-hooks/workspace-member-pre-query-hook.service'; +import { WorkspaceMemberRestoreManyPreQueryHook } from 'src/modules/workspace-member/query-hooks/workspace-member-restore-many.pre-query.hook'; +import { WorkspaceMemberRestoreOnePreQueryHook } from 'src/modules/workspace-member/query-hooks/workspace-member-restore-one.pre-query.hook'; +import { WorkspaceMemberUpdateManyPreQueryHook } from 'src/modules/workspace-member/query-hooks/workspace-member-update-many.pre-query.hook'; +import { WorkspaceMemberUpdateOnePreQueryHook } from 'src/modules/workspace-member/query-hooks/workspace-member-update-one.pre-query.hook'; @Module({ providers: [ + WorkspaceMemberPreQueryHookService, + WorkspaceMemberCreateOnePreQueryHook, + WorkspaceMemberCreateManyPreQueryHook, WorkspaceMemberDeleteOnePreQueryHook, WorkspaceMemberDeleteManyPreQueryHook, + WorkspaceMemberDestroyOnePreQueryHook, + WorkspaceMemberDestroyManyPreQueryHook, + WorkspaceMemberRestoreOnePreQueryHook, + WorkspaceMemberRestoreManyPreQueryHook, + WorkspaceMemberUpdateOnePreQueryHook, + WorkspaceMemberUpdateManyPreQueryHook, ], + imports: [FeatureFlagModule, PermissionsModule], }) export class WorkspaceMemberQueryHookModule {} diff --git a/packages/twenty-server/src/modules/workspace-member/query-hooks/workspace-member-restore-many.pre-query.hook.ts b/packages/twenty-server/src/modules/workspace-member/query-hooks/workspace-member-restore-many.pre-query.hook.ts new file mode 100644 index 000000000..fbf8b891c --- /dev/null +++ b/packages/twenty-server/src/modules/workspace-member/query-hooks/workspace-member-restore-many.pre-query.hook.ts @@ -0,0 +1,32 @@ +import { WorkspaceQueryHookInstance } from 'src/engine/api/graphql/workspace-query-runner/workspace-query-hook/interfaces/workspace-query-hook.interface'; +import { RestoreManyResolverArgs } from 'src/engine/api/graphql/workspace-resolver-builder/interfaces/workspace-resolvers-builder.interface'; + +import { WorkspaceQueryHook } from 'src/engine/api/graphql/workspace-query-runner/workspace-query-hook/decorators/workspace-query-hook.decorator'; +import { AuthContext } from 'src/engine/core-modules/auth/types/auth-context.type'; +import { WorkspaceMemberPreQueryHookService } from 'src/modules/workspace-member/query-hooks/workspace-member-pre-query-hook.service'; + +@WorkspaceQueryHook(`workspaceMember.restoreMany`) +export class WorkspaceMemberRestoreManyPreQueryHook + implements WorkspaceQueryHookInstance +{ + constructor( + private readonly workspaceMemberPreQueryHookService: WorkspaceMemberPreQueryHookService, + ) {} + + async execute( + authContext: AuthContext, + objectName: string, + payload: RestoreManyResolverArgs, + ): Promise { + await this.workspaceMemberPreQueryHookService.validateWorkspaceMemberUpdatePermissionOrThrow( + { + userWorkspaceId: authContext.userWorkspaceId, + workspaceId: authContext.workspace.id, + apiKey: authContext.apiKey, + workspaceMemberId: authContext.workspaceMemberId, + }, + ); + + return payload; + } +} diff --git a/packages/twenty-server/src/modules/workspace-member/query-hooks/workspace-member-restore-one.pre-query.hook.ts b/packages/twenty-server/src/modules/workspace-member/query-hooks/workspace-member-restore-one.pre-query.hook.ts new file mode 100644 index 000000000..f8e95f174 --- /dev/null +++ b/packages/twenty-server/src/modules/workspace-member/query-hooks/workspace-member-restore-one.pre-query.hook.ts @@ -0,0 +1,33 @@ +import { WorkspaceQueryHookInstance } from 'src/engine/api/graphql/workspace-query-runner/workspace-query-hook/interfaces/workspace-query-hook.interface'; +import { RestoreOneResolverArgs } from 'src/engine/api/graphql/workspace-resolver-builder/interfaces/workspace-resolvers-builder.interface'; + +import { WorkspaceQueryHook } from 'src/engine/api/graphql/workspace-query-runner/workspace-query-hook/decorators/workspace-query-hook.decorator'; +import { AuthContext } from 'src/engine/core-modules/auth/types/auth-context.type'; +import { WorkspaceMemberPreQueryHookService } from 'src/modules/workspace-member/query-hooks/workspace-member-pre-query-hook.service'; + +@WorkspaceQueryHook(`workspaceMember.restoreOne`) +export class WorkspaceMemberRestoreOnePreQueryHook + implements WorkspaceQueryHookInstance +{ + constructor( + private readonly workspaceMemberPreQueryHookService: WorkspaceMemberPreQueryHookService, + ) {} + + async execute( + authContext: AuthContext, + objectName: string, + payload: RestoreOneResolverArgs, + ): Promise { + await this.workspaceMemberPreQueryHookService.validateWorkspaceMemberUpdatePermissionOrThrow( + { + userWorkspaceId: authContext.userWorkspaceId, + targettedWorkspaceMemberId: payload.id, + workspaceId: authContext.workspace.id, + apiKey: authContext.apiKey, + workspaceMemberId: authContext.workspaceMemberId, + }, + ); + + return payload; + } +} diff --git a/packages/twenty-server/src/modules/workspace-member/query-hooks/workspace-member-update-many.pre-query.hook.ts b/packages/twenty-server/src/modules/workspace-member/query-hooks/workspace-member-update-many.pre-query.hook.ts new file mode 100644 index 000000000..551d12d1d --- /dev/null +++ b/packages/twenty-server/src/modules/workspace-member/query-hooks/workspace-member-update-many.pre-query.hook.ts @@ -0,0 +1,32 @@ +import { WorkspaceQueryHookInstance } from 'src/engine/api/graphql/workspace-query-runner/workspace-query-hook/interfaces/workspace-query-hook.interface'; +import { UpdateManyResolverArgs } from 'src/engine/api/graphql/workspace-resolver-builder/interfaces/workspace-resolvers-builder.interface'; + +import { WorkspaceQueryHook } from 'src/engine/api/graphql/workspace-query-runner/workspace-query-hook/decorators/workspace-query-hook.decorator'; +import { AuthContext } from 'src/engine/core-modules/auth/types/auth-context.type'; +import { WorkspaceMemberPreQueryHookService } from 'src/modules/workspace-member/query-hooks/workspace-member-pre-query-hook.service'; + +@WorkspaceQueryHook(`workspaceMember.updateMany`) +export class WorkspaceMemberUpdateManyPreQueryHook + implements WorkspaceQueryHookInstance +{ + constructor( + private readonly workspaceMemberPreQueryHookService: WorkspaceMemberPreQueryHookService, + ) {} + + async execute( + authContext: AuthContext, + objectName: string, + payload: UpdateManyResolverArgs, + ): Promise { + await this.workspaceMemberPreQueryHookService.validateWorkspaceMemberUpdatePermissionOrThrow( + { + userWorkspaceId: authContext.userWorkspaceId, + workspaceId: authContext.workspace.id, + apiKey: authContext.apiKey, + workspaceMemberId: authContext.workspaceMemberId, + }, + ); + + return payload; + } +} diff --git a/packages/twenty-server/src/modules/workspace-member/query-hooks/workspace-member-update-one.pre-query.hook.ts b/packages/twenty-server/src/modules/workspace-member/query-hooks/workspace-member-update-one.pre-query.hook.ts new file mode 100644 index 000000000..60606309c --- /dev/null +++ b/packages/twenty-server/src/modules/workspace-member/query-hooks/workspace-member-update-one.pre-query.hook.ts @@ -0,0 +1,33 @@ +import { WorkspaceQueryHookInstance } from 'src/engine/api/graphql/workspace-query-runner/workspace-query-hook/interfaces/workspace-query-hook.interface'; +import { UpdateOneResolverArgs } from 'src/engine/api/graphql/workspace-resolver-builder/interfaces/workspace-resolvers-builder.interface'; + +import { WorkspaceQueryHook } from 'src/engine/api/graphql/workspace-query-runner/workspace-query-hook/decorators/workspace-query-hook.decorator'; +import { AuthContext } from 'src/engine/core-modules/auth/types/auth-context.type'; +import { WorkspaceMemberPreQueryHookService } from 'src/modules/workspace-member/query-hooks/workspace-member-pre-query-hook.service'; + +@WorkspaceQueryHook(`workspaceMember.updateOne`) +export class WorkspaceMemberUpdateOnePreQueryHook + implements WorkspaceQueryHookInstance +{ + constructor( + private readonly workspaceMemberPreQueryHookService: WorkspaceMemberPreQueryHookService, + ) {} + + async execute( + authContext: AuthContext, + objectName: string, + payload: UpdateOneResolverArgs, + ): Promise { + await this.workspaceMemberPreQueryHookService.validateWorkspaceMemberUpdatePermissionOrThrow( + { + userWorkspaceId: authContext.userWorkspaceId, + targettedWorkspaceMemberId: payload.id, + workspaceId: authContext.workspace.id, + apiKey: authContext.apiKey, + workspaceMemberId: authContext.workspaceMemberId, + }, + ); + + return payload; + } +} diff --git a/packages/twenty-server/test/integration/graphql/suites/settings-permissions/workspace-members.integration-spec.ts b/packages/twenty-server/test/integration/graphql/suites/settings-permissions/workspace-members.integration-spec.ts new file mode 100644 index 000000000..a08f16d45 --- /dev/null +++ b/packages/twenty-server/test/integration/graphql/suites/settings-permissions/workspace-members.integration-spec.ts @@ -0,0 +1,211 @@ +import { createOneOperationFactory } from 'test/integration/graphql/utils/create-one-operation-factory.util'; +import { deleteOneOperationFactory } from 'test/integration/graphql/utils/delete-one-operation-factory.util'; +import { makeGraphqlAPIRequestWithMemberRole } from 'test/integration/graphql/utils/make-graphql-api-request-with-member-role.util'; +import { makeGraphqlAPIRequest } from 'test/integration/graphql/utils/make-graphql-api-request.util'; +import { restoreOneOperationFactory } from 'test/integration/graphql/utils/restore-one-operation-factory.util'; +import { updateFeatureFlagFactory } from 'test/integration/graphql/utils/update-feature-flag-factory.util'; +import { updateOneOperationFactory } from 'test/integration/graphql/utils/update-one-operation-factory.util'; + +import { SEED_APPLE_WORKSPACE_ID } from 'src/database/typeorm-seeds/core/workspaces'; +import { DEV_SEED_WORKSPACE_MEMBER_IDS } from 'src/database/typeorm-seeds/workspace/workspace-members'; +import { ErrorCode } from 'src/engine/core-modules/graphql/utils/graphql-errors.util'; +import { PermissionsExceptionMessage } from 'src/engine/metadata-modules/permissions/permissions.exception'; + +const WORKSPACE_MEMBER_GQL_FIELDS = ` + id + name { + firstName + } +`; + +describe('workspace members permissions', () => { + beforeAll(async () => { + const enablePermissionsQuery = updateFeatureFlagFactory( + SEED_APPLE_WORKSPACE_ID, + 'IsPermissionsEnabled', + true, + ); + + await makeGraphqlAPIRequest(enablePermissionsQuery); + }); + + afterAll(async () => { + const disablePermissionsQuery = updateFeatureFlagFactory( + SEED_APPLE_WORKSPACE_ID, + 'IsPermissionsEnabled', + false, + ); + + await makeGraphqlAPIRequest(disablePermissionsQuery); + }); + describe('updateOne', () => { + it('should allow update when user is updating themself (member role)', async () => { + const graphqlOperation = updateOneOperationFactory({ + objectMetadataSingularName: 'workspaceMember', + gqlFields: WORKSPACE_MEMBER_GQL_FIELDS, + recordId: DEV_SEED_WORKSPACE_MEMBER_IDS.JONY, + data: { + name: { + firstName: 'Jony', + }, + }, + }); + + const response = + await makeGraphqlAPIRequestWithMemberRole(graphqlOperation); + + expect(response.body.data).toStrictEqual({ + updateWorkspaceMember: { + id: DEV_SEED_WORKSPACE_MEMBER_IDS.JONY, + name: { + firstName: 'Jony', + }, + }, + }); + expect(response.body.errors).toBeUndefined(); + }); + it('should throw when user does not have permission (member role)', async () => { + const graphqlOperation = updateOneOperationFactory({ + objectMetadataSingularName: 'workspaceMember', + gqlFields: WORKSPACE_MEMBER_GQL_FIELDS, + recordId: DEV_SEED_WORKSPACE_MEMBER_IDS.TIM, + data: { + name: { + firstName: 'Not Tim', + }, + }, + }); + + const response = + await makeGraphqlAPIRequestWithMemberRole(graphqlOperation); + + expect(response.body.data).toStrictEqual({ updateWorkspaceMember: null }); + expect(response.body.errors).toBeDefined(); + expect(response.body.errors[0].message).toBe( + PermissionsExceptionMessage.PERMISSION_DENIED, + ); + expect(response.body.errors[0].extensions.code).toBe(ErrorCode.FORBIDDEN); + }); + }); + + describe('deleteOne', () => { + afterEach(async () => { + // Restore the deleted user to maintain test isolation + const restoreOperation = restoreOneOperationFactory({ + objectMetadataSingularName: 'workspaceMember', + gqlFields: WORKSPACE_MEMBER_GQL_FIELDS, + recordId: DEV_SEED_WORKSPACE_MEMBER_IDS.JONY, + }); + + await makeGraphqlAPIRequest(restoreOperation); + }); + it('should allow delete when user is deleting themself (member role)', async () => { + const deleteOperation = deleteOneOperationFactory({ + objectMetadataSingularName: 'workspaceMember', + gqlFields: WORKSPACE_MEMBER_GQL_FIELDS, + recordId: DEV_SEED_WORKSPACE_MEMBER_IDS.JONY, + }); + + const deleteResponse = + await makeGraphqlAPIRequestWithMemberRole(deleteOperation); + + expect(deleteResponse.body.data).toStrictEqual({ + deleteWorkspaceMember: { + id: DEV_SEED_WORKSPACE_MEMBER_IDS.JONY, + name: { + firstName: 'Jony', + }, + }, + }); + expect(deleteResponse.body.errors).toBeUndefined(); + }); + + it('should throw when user does not have permission (member role)', async () => { + const graphqlOperation = deleteOneOperationFactory({ + objectMetadataSingularName: 'workspaceMember', + gqlFields: WORKSPACE_MEMBER_GQL_FIELDS, + recordId: DEV_SEED_WORKSPACE_MEMBER_IDS.TIM, + }); + + const response = + await makeGraphqlAPIRequestWithMemberRole(graphqlOperation); + + expect(response.body.data).toStrictEqual({ deleteWorkspaceMember: null }); + expect(response.body.errors).toBeDefined(); + expect(response.body.errors[0].message).toBe( + PermissionsExceptionMessage.PERMISSION_DENIED, + ); + expect(response.body.errors[0].extensions.code).toBe(ErrorCode.FORBIDDEN); + }); + }); + + describe('restoreOne', () => { + it('should allow restore when user is restoring themself (member role)', async () => { + const restoreOperation = restoreOneOperationFactory({ + objectMetadataSingularName: 'workspaceMember', + gqlFields: WORKSPACE_MEMBER_GQL_FIELDS, + recordId: DEV_SEED_WORKSPACE_MEMBER_IDS.JONY, + }); + + const response = + await makeGraphqlAPIRequestWithMemberRole(restoreOperation); + + expect(response.body.data).toStrictEqual({ + restoreWorkspaceMember: { + id: DEV_SEED_WORKSPACE_MEMBER_IDS.JONY, + name: { + firstName: 'Jony', + }, + }, + }); + expect(response.body.errors).toBeUndefined(); + }); + + it('should throw when user does not have permission (member role)', async () => { + const restoreOperation = restoreOneOperationFactory({ + objectMetadataSingularName: 'workspaceMember', + gqlFields: WORKSPACE_MEMBER_GQL_FIELDS, + recordId: DEV_SEED_WORKSPACE_MEMBER_IDS.TIM, + }); + + const response = + await makeGraphqlAPIRequestWithMemberRole(restoreOperation); + + expect(response.body.data).toStrictEqual({ + restoreWorkspaceMember: null, + }); + expect(response.body.errors).toBeDefined(); + expect(response.body.errors[0].message).toBe( + PermissionsExceptionMessage.PERMISSION_DENIED, + ); + expect(response.body.errors[0].extensions.code).toBe(ErrorCode.FORBIDDEN); + }); + }); + + describe('createOne', () => { + it('should throw when user does not have permission (member role)', async () => { + const createOperation = createOneOperationFactory({ + objectMetadataSingularName: 'workspaceMember', + gqlFields: WORKSPACE_MEMBER_GQL_FIELDS, + data: { + userId: 'cc80c2e9-3002-46ac-bcc6-24e524713f21', + name: { + firstName: 'New', + }, + }, + }); + + const response = + await makeGraphqlAPIRequestWithMemberRole(createOperation); + + expect(response.body.data).toStrictEqual({ + createWorkspaceMember: null, + }); + expect(response.body.errors).toBeDefined(); + expect(response.body.errors[0].message).toBe( + PermissionsExceptionMessage.PERMISSION_DENIED, + ); + expect(response.body.errors[0].extensions.code).toBe(ErrorCode.FORBIDDEN); + }); + }); +}); diff --git a/packages/twenty-server/test/integration/graphql/utils/make-graphql-api-request-with-member-role.util.ts b/packages/twenty-server/test/integration/graphql/utils/make-graphql-api-request-with-member-role.util.ts new file mode 100644 index 000000000..24dea5939 --- /dev/null +++ b/packages/twenty-server/test/integration/graphql/utils/make-graphql-api-request-with-member-role.util.ts @@ -0,0 +1,21 @@ +import { ASTNode, print } from 'graphql'; +import request from 'supertest'; + +type GraphqlOperation = { + query: ASTNode; + variables?: Record; +}; + +export const makeGraphqlAPIRequestWithMemberRole = ( + graphqlOperation: GraphqlOperation, +) => { + const client = request(`http://localhost:${APP_PORT}`); + + return client + .post('/graphql') + .set('Authorization', `Bearer ${MEMBER_ACCESS_TOKEN}`) + .send({ + query: print(graphqlOperation.query), + variables: graphqlOperation.variables || {}, + }); +}; diff --git a/packages/twenty-shared/src/utils/index.ts b/packages/twenty-shared/src/utils/index.ts index d43684cac..1b7d29622 100644 --- a/packages/twenty-shared/src/utils/index.ts +++ b/packages/twenty-shared/src/utils/index.ts @@ -1,5 +1,4 @@ export * from './fieldMetadata'; export * from './image'; -export * from './permissions'; export * from './strings'; export * from './validation'; diff --git a/packages/twenty-shared/src/utils/permissions/index.ts b/packages/twenty-shared/src/utils/permissions/index.ts deleted file mode 100644 index ec044f18b..000000000 --- a/packages/twenty-shared/src/utils/permissions/index.ts +++ /dev/null @@ -1 +0,0 @@ -export * from './isObjectRecordUnderObjectRecordsPermissions'; diff --git a/packages/twenty-shared/src/utils/permissions/isObjectRecordUnderObjectRecordsPermissions.ts b/packages/twenty-shared/src/utils/permissions/isObjectRecordUnderObjectRecordsPermissions.ts deleted file mode 100644 index fc580a2f3..000000000 --- a/packages/twenty-shared/src/utils/permissions/isObjectRecordUnderObjectRecordsPermissions.ts +++ /dev/null @@ -1,16 +0,0 @@ -import { STANDARD_OBJECT_RECORDS_UNDER_OBJECT_RECORDS_PERMISSIONS } from 'src/constants'; - -export const isObjectRecordUnderObjectRecordsPermissions = ({ - isCustom, - nameSingular, -}: { - isCustom: boolean; - nameSingular: string; -}) => { - return ( - isCustom || - STANDARD_OBJECT_RECORDS_UNDER_OBJECT_RECORDS_PERMISSIONS.includes( - nameSingular, - ) - ); -};