From beba4b8313447a508e7f18913bb9597516bf879a Mon Sep 17 00:00:00 2001 From: Marie <51697796+ijreilly@users.noreply.github.com> Date: Wed, 11 Jun 2025 12:50:10 +0200 Subject: [PATCH] Add missing overrides on entityManager (#12471) In this PR 1. Add missing override of insert() method on WorkspaceSelectQueryBuilder to return our custom WorkspaceInsertQueryBuilder with permission checks. 2. Replace override implementation of methods on WorkspaceEntityManager that call createQueryBuilder at a nested internal layer of typeORM (i.e. not directly in the initial implementation of EntityManager - unlike findBy for instance -, but in calls done under the hood at a level which would force us to override entire other classes to pass on our permissionOptions. It is the case for methods which call typeORM's EntityPersistExecutor for instance.), to validate permissions and then allow the subsequent calls to be made without permission checks 3. adapt tests --------- Co-authored-by: Charles Bochet --- .../datasource/workspace.datasource.ts | 55 ++ .../workspace-entity-manager.spec.ts | 65 +- .../workspace-entity-manager.ts | 934 +++++++++++------- .../workspace-select-query-builder.ts | 12 + .../match-participant.service.spec.ts | 56 +- .../match-participant.service.ts | 48 +- 6 files changed, 749 insertions(+), 421 deletions(-) diff --git a/packages/twenty-server/src/engine/twenty-orm/datasource/workspace.datasource.ts b/packages/twenty-server/src/engine/twenty-orm/datasource/workspace.datasource.ts index fbcdd4948..61483622e 100644 --- a/packages/twenty-server/src/engine/twenty-orm/datasource/workspace.datasource.ts +++ b/packages/twenty-server/src/engine/twenty-orm/datasource/workspace.datasource.ts @@ -1,3 +1,4 @@ +import { Entity } from '@microsoft/microsoft-graph-types'; import { isDefined } from 'class-validator'; import { ObjectRecordsPermissionsByRoleId } from 'twenty-shared/types'; import { @@ -9,6 +10,7 @@ import { ReplicationMode, SelectQueryBuilder, } from 'typeorm'; +import { EntityManagerFactory } from 'typeorm/entity-manager/EntityManagerFactory'; import { FeatureFlagMap } from 'src/engine/core-modules/feature-flag/interfaces/feature-flag-map.interface'; import { WorkspaceInternalContext } from 'src/engine/twenty-orm/interfaces/workspace-internal-context.interface'; @@ -33,6 +35,7 @@ export class WorkspaceDataSource extends DataSource { featureFlagMap: FeatureFlagMap; rolesPermissionsVersion: string; permissionsPerRoleId: ObjectRecordsPermissionsByRoleId; + dataSourceWithOverridenCreateQueryBuilder: WorkspaceDataSource; constructor( internalContext: WorkspaceInternalContext, @@ -90,6 +93,58 @@ export class WorkspaceDataSource extends DataSource { return queryRunner as any as WorkspaceQueryRunner; } + // Do not use, only for specific permission-related purpose + createQueryRunnerForEntityPersistExecutor( + mode = 'master' as ReplicationMode, + ) { + if (this.dataSourceWithOverridenCreateQueryBuilder) { + const queryRunner = this.driver.createQueryRunner(mode); + const manager = new EntityManagerFactory().create( + this.dataSourceWithOverridenCreateQueryBuilder, + queryRunner, + ); + + Object.assign(queryRunner, { manager: manager }); + + return queryRunner; + } + + const dataSourceWithOverridenCreateQueryBuilder = Object.assign( + Object.create(Object.getPrototypeOf(this)), + this, + { + createQueryBuilder: ( + entityOrRunner: EntityTarget | QueryRunner, + alias?: string, + queryRunner?: QueryRunner, + ) => { + if (isDefined(alias) && typeof alias === 'string') { + const entity = entityOrRunner as EntityTarget; + + return this.createQueryBuilder(entity, alias, queryRunner, { + calledByWorkspaceEntityManager: true, + }); + } else { + const runner = entityOrRunner as QueryRunner; + + return this.createQueryBuilder(runner, { + calledByWorkspaceEntityManager: true, + }); + } + }, + }, + ); + const queryRunner = this.driver.createQueryRunner(mode); + const manager = new EntityManagerFactory().create( + dataSourceWithOverridenCreateQueryBuilder, + queryRunner, + ); + + Object.assign(queryRunner, { manager: manager }); + + return queryRunner; + } + override createQueryBuilder( entityClass: EntityTarget, alias: string, diff --git a/packages/twenty-server/src/engine/twenty-orm/entity-manager/workspace-entity-manager.spec.ts b/packages/twenty-server/src/engine/twenty-orm/entity-manager/workspace-entity-manager.spec.ts index 4ea1ff6ca..431ac51fd 100644 --- a/packages/twenty-server/src/engine/twenty-orm/entity-manager/workspace-entity-manager.spec.ts +++ b/packages/twenty-server/src/engine/twenty-orm/entity-manager/workspace-entity-manager.spec.ts @@ -1,5 +1,7 @@ import { ObjectRecordsPermissions } from 'twenty-shared/types'; import { EntityManager } from 'typeorm'; +import { EntityPersistExecutor } from 'typeorm/persistence/EntityPersistExecutor'; +import { PlainObjectToDatabaseEntityTransformer } from 'typeorm/query-builder/transformer/PlainObjectToDatabaseEntityTransformer'; import { WorkspaceInternalContext } from 'src/engine/twenty-orm/interfaces/workspace-internal-context.interface'; @@ -13,6 +15,19 @@ jest.mock('src/engine/twenty-orm/repository/permissions.utils', () => ({ validateOperationIsPermittedOrThrow: jest.fn(), })); +const mockedWorkspaceUpdateQueryBuilder = { + set: jest.fn().mockImplementation(() => ({ + where: jest.fn().mockReturnThis(), + whereInIds: jest.fn().mockReturnThis(), + execute: jest + .fn() + .mockResolvedValue({ affected: 1, raw: [], generatedMaps: [] }), + })), + execute: jest + .fn() + .mockResolvedValue({ affected: 1, raw: [], generatedMaps: [] }), +}; + jest.mock('../repository/workspace-select-query-builder', () => ({ WorkspaceSelectQueryBuilder: jest.fn().mockImplementation(() => ({ where: jest.fn().mockReturnThis(), @@ -23,6 +38,8 @@ jest.mock('../repository/workspace-select-query-builder', () => ({ .fn() .mockResolvedValue({ affected: 1, raw: [], generatedMaps: [] }), setFindOptions: jest.fn().mockReturnThis(), + update: jest.fn().mockReturnValue(mockedWorkspaceUpdateQueryBuilder), + insert: jest.fn().mockReturnThis(), })), })); @@ -96,6 +113,14 @@ describe('WorkspaceEntityManager', () => { release: jest.fn(), clearTable: jest.fn(), }), + createQueryRunnerForEntityPersistExecutor: jest.fn().mockReturnValue({ + connect: jest.fn(), + startTransaction: jest.fn(), + commitTransaction: jest.fn(), + rollbackTransaction: jest.fn(), + release: jest.fn(), + clearTable: jest.fn(), + }), }; entityManager = new WorkspaceEntityManager( @@ -142,6 +167,14 @@ describe('WorkspaceEntityManager', () => { .spyOn(EntityManager.prototype, 'preload') .mockImplementation(() => Promise.resolve({})); + jest + .spyOn(EntityPersistExecutor.prototype, 'execute') + .mockImplementation(() => Promise.resolve()); + + jest + .spyOn(PlainObjectToDatabaseEntityTransformer.prototype, 'transform') + .mockImplementation(() => Promise.resolve({})); + // Mock metadata methods const mockMetadata = { hasAllPrimaryKeys: jest.fn().mockReturnValue(true), @@ -202,20 +235,14 @@ describe('WorkspaceEntityManager', () => { }); describe('Update Methods', () => { - it('should call validatePermissions and validateOperationIsPermittedOrThrow for update', async () => { + it('should call createQueryBuilder with permissionOptions for update', async () => { await entityManager.update('test-entity', {}, {}, mockPermissionOptions); - expect(entityManager['validatePermissions']).toHaveBeenCalledWith( - 'test-entity', - 'update', + expect(entityManager['createQueryBuilder']).toHaveBeenCalledWith( + undefined, + undefined, + undefined, mockPermissionOptions, ); - expect(validateOperationIsPermittedOrThrow).toHaveBeenCalledWith({ - entityName: 'test-entity', - operationType: 'update', - objectMetadataMaps: mockInternalContext.objectMetadataMaps, - objectRecordsPermissions: - mockPermissionOptions.objectRecordsPermissions, - }); }); }); @@ -235,21 +262,5 @@ describe('WorkspaceEntityManager', () => { mockPermissionOptions.objectRecordsPermissions, }); }); - - it('should call validatePermissions and validateOperationIsPermittedOrThrow for preload', async () => { - await entityManager.preload('test-entity', {}, mockPermissionOptions); - expect(entityManager['validatePermissions']).toHaveBeenCalledWith( - 'test-entity', - 'select', - mockPermissionOptions, - ); - expect(validateOperationIsPermittedOrThrow).toHaveBeenCalledWith({ - entityName: 'test-entity', - operationType: 'select', - objectMetadataMaps: mockInternalContext.objectMetadataMaps, - objectRecordsPermissions: - mockPermissionOptions.objectRecordsPermissions, - }); - }); }); }); diff --git a/packages/twenty-server/src/engine/twenty-orm/entity-manager/workspace-entity-manager.ts b/packages/twenty-server/src/engine/twenty-orm/entity-manager/workspace-entity-manager.ts index 0ae340fc7..93a776143 100644 --- a/packages/twenty-server/src/engine/twenty-orm/entity-manager/workspace-entity-manager.ts +++ b/packages/twenty-server/src/engine/twenty-orm/entity-manager/workspace-entity-manager.ts @@ -23,8 +23,11 @@ import { DeepPartial } from 'typeorm/common/DeepPartial'; import { PickKeysByType } from 'typeorm/common/PickKeysByType'; import { EntityNotFoundError } from 'typeorm/error/EntityNotFoundError'; import { FindOptionsUtils } from 'typeorm/find-options/FindOptionsUtils'; +import { EntityPersistExecutor } from 'typeorm/persistence/EntityPersistExecutor'; import { QueryDeepPartialEntity } from 'typeorm/query-builder/QueryPartialEntity'; +import { PlainObjectToDatabaseEntityTransformer } from 'typeorm/query-builder/transformer/PlainObjectToDatabaseEntityTransformer'; import { UpsertOptions } from 'typeorm/repository/UpsertOptions'; +import { InstanceChecker } from 'typeorm/util/InstanceChecker'; import { FeatureFlagMap } from 'src/engine/core-modules/feature-flag/interfaces/feature-flag-map.interface'; import { WorkspaceInternalContext } from 'src/engine/twenty-orm/interfaces/workspace-internal-context.interface'; @@ -184,9 +187,16 @@ export class WorkspaceEntityManager extends EntityManager { entity: QueryDeepPartialEntity | QueryDeepPartialEntity[], permissionOptions?: PermissionOptions, ): Promise { - this.validatePermissions(target, 'insert', permissionOptions); - - return super.insert(target, entity); + return this.createQueryBuilder( + undefined, + undefined, + undefined, + permissionOptions, + ) + .insert() + .into(target) + .values(entity) + .execute(); } override upsert( @@ -200,9 +210,59 @@ export class WorkspaceEntityManager extends EntityManager { objectRecordsPermissions?: ObjectRecordsPermissions; }, ): Promise { - this.validatePermissions(target, 'update', permissionOptions); + const metadata = this.connection.getMetadata(target); + let options; - return super.upsert(target, entityOrEntities, conflictPathsOrOptions); + if (Array.isArray(conflictPathsOrOptions)) { + options = { + conflictPaths: conflictPathsOrOptions, + }; + } else { + options = conflictPathsOrOptions; + } + let entities: QueryDeepPartialEntity[]; + + if (!Array.isArray(entityOrEntities)) { + entities = [entityOrEntities]; + } else { + entities = entityOrEntities; + } + const conflictColumns = metadata.mapPropertyPathsToColumns( + Array.isArray(options.conflictPaths) + ? options.conflictPaths + : Object.keys(options.conflictPaths), + ); + const overwriteColumns = metadata.columns.filter( + (col) => + !conflictColumns.includes(col) && + entities.some( + (entity) => typeof col.getEntityValue(entity) !== 'undefined', + ), + ); + + return this.createQueryBuilder( + undefined, + undefined, + undefined, + permissionOptions, + ) + .insert() + .into(target) + .values(entities) + .orUpdate( + [...conflictColumns, ...overwriteColumns].map( + (col) => col.databaseName, + ), + conflictColumns.map((col) => col.databaseName), + { + skipUpdateIfNoValuesChanged: options.skipUpdateIfNoValuesChanged, + indexPredicate: options.indexPredicate, + upsertType: + options.upsertType || + this.connection.driver.supportedUpsertTypes[0], + }, + ) + .execute(); } override update( @@ -220,124 +280,81 @@ export class WorkspaceEntityManager extends EntityManager { partialEntity: QueryDeepPartialEntity, permissionOptions?: PermissionOptions, ): Promise { - this.validatePermissions(target, 'update', permissionOptions); - - return super.update(target, criteria, partialEntity); - } - - override save( - entities: Entity[], - options?: SaveOptions, - permissionOptions?: PermissionOptions, - ): Promise; - - override save( - entity: Entity, - options?: SaveOptions, - permissionOptions?: PermissionOptions, - ): Promise; - - override save>( - targetOrEntity: EntityTarget, - entities: T[], - options: SaveOptions & { - reload: false; - }, - permissionOptions?: PermissionOptions, - ): Promise; - - override save>( - targetOrEntity: EntityTarget, - entities: T[], - options?: SaveOptions, - permissionOptions?: PermissionOptions, - ): Promise<(T & Entity)[]>; - - override save>( - targetOrEntity: EntityTarget, - entity: T, - options: SaveOptions & { - reload: false; - }, - permissionOptions?: PermissionOptions, - ): Promise; - - override save>( - targetOrEntity: EntityTarget, - entity: T, - options?: SaveOptions, - permissionOptions?: PermissionOptions, - ): Promise; - - override save>( - targetOrEntity: EntityTarget | Entity | Entity[], - entityOrMaybeOptions: - | T - | T[] - | SaveOptions - | (SaveOptions & { reload: false }), - maybeOptionsOrMaybePermissionOptions?: - | PermissionOptions - | SaveOptions - | (SaveOptions & { reload: false }), - permissionOptions?: PermissionOptions, - ): Promise<(T & Entity) | (T & Entity)[] | Entity | Entity[]> { - const permissionOptionsFromArgs = - maybeOptionsOrMaybePermissionOptions && - ('shouldBypassPermissionChecks' in maybeOptionsOrMaybePermissionOptions || - 'objectRecordsPermissions' in maybeOptionsOrMaybePermissionOptions) - ? maybeOptionsOrMaybePermissionOptions - : permissionOptions; - - this.validatePermissions( - targetOrEntity, - 'update', - permissionOptionsFromArgs, - ); - - const target = - arguments.length > 1 && - (typeof targetOrEntity === 'function' || - typeof targetOrEntity === 'string') - ? (targetOrEntity as EntityTarget) - : undefined; - - const entityOrEntities = target - ? (entityOrMaybeOptions as T | T[]) - : (targetOrEntity as Entity | Entity[]); - - const options = target - ? (maybeOptionsOrMaybePermissionOptions as SaveOptions | undefined) - : (entityOrMaybeOptions as SaveOptions | undefined); - - if (isDefined(target)) { - let entity: T | undefined; - let entities: T[] | undefined; - - if (Array.isArray(entityOrEntities)) { - entities = entityOrEntities as T[]; - - return super.save(target, entities, options); - } else { - entity = entityOrEntities as T; - - return super.save(target, entity, options); - } + if ( + criteria === undefined || + criteria === null || + criteria === '' || + (Array.isArray(criteria) && criteria.length === 0) + ) { + return Promise.reject( + new TypeORMError( + `Empty criteria(s) are not allowed for the update method.`, + ), + ); + } + if ( + typeof criteria === 'string' || + typeof criteria === 'number' || + criteria instanceof Date || + Array.isArray(criteria) + ) { + return this.createQueryBuilder( + undefined, + undefined, + undefined, + permissionOptions, + ) + .update(target) + .set(partialEntity) + .whereInIds(criteria) + .execute(); } else { - return super.save(entityOrEntities as Entity | Entity[], options); + return this.createQueryBuilder( + undefined, + undefined, + undefined, + permissionOptions, + ) + .update(target) + .set(partialEntity) + .where(criteria) + .execute(); } } override increment( target: EntityTarget, - criteria: unknown, + criteria: object, propertyPath: string, value: number | string, permissionOptions?: PermissionOptions, ): Promise { - this.validatePermissions(target, 'update', permissionOptions); + const metadata = this.connection.getMetadata(target); + const column = metadata.findColumnWithPropertyPath(propertyPath); - return super.increment(target, criteria, propertyPath, value); + if (!column) + throw new TypeORMError( + `Column ${propertyPath} was not found in ${metadata.targetName} entity.`, + ); + if (isNaN(Number(value))) + throw new TypeORMError(`Value "${value}" is not a number.`); + // convert possible embeded path "social.likes" into object { social: { like: () => value } } + // eslint-disable-next-line @typescript-eslint/no-explicit-any + const values = propertyPath.split('.').reduceRight( + (value, key) => ({ [key]: value }), + () => this.connection.driver.escape(column.databaseName) + ' + ' + value, + ); + + return this.createQueryBuilder( + target, + 'entity', + undefined, + permissionOptions, + ) + .update(target as QueryDeepPartialEntity) + .set(values) + .where(criteria) + .execute(); } private getRepositoryKey({ @@ -570,77 +587,45 @@ export class WorkspaceEntityManager extends EntityManager { criteria: unknown, permissionOptions?: PermissionOptions, ): Promise { - this.validatePermissions(targetOrEntity, 'delete', permissionOptions); - - return super.delete(targetOrEntity, criteria); - } - - override remove( - entity: Entity, - options?: RemoveOptions, - permissionOptions?: PermissionOptions, - ): Promise; - - override remove( - targetOrEntity: EntityTarget, - entity: Entity, - options?: RemoveOptions, - permissionOptions?: PermissionOptions, - ): Promise; - - override remove( - entity: Entity[], - options?: RemoveOptions, - permissionOptions?: PermissionOptions, - ): Promise; - - override remove( - targetOrEntity: EntityTarget, - entity: Entity[], - options?: RemoveOptions, - permissionOptions?: PermissionOptions, - ): Promise; - - override remove( - targetOrEntity: EntityTarget | Entity[] | Entity, - entityOrMaybeOptions: Entity | Entity[] | RemoveOptions, - maybeOptionsOrMaybePermissionOptions?: RemoveOptions | PermissionOptions, - permissionOptions?: PermissionOptions, - ): Promise { - const permissionOptionsFromArgs = - maybeOptionsOrMaybePermissionOptions && - ('shouldBypassPermissionChecks' in maybeOptionsOrMaybePermissionOptions || - 'objectRecordsPermissions' in maybeOptionsOrMaybePermissionOptions) - ? (maybeOptionsOrMaybePermissionOptions as PermissionOptions) - : permissionOptions; - - this.validatePermissions( - targetOrEntity, - 'delete', - permissionOptionsFromArgs, - ); - - const target = - typeof targetOrEntity === 'function' || typeof targetOrEntity === 'string' - ? (targetOrEntity as EntityTarget) - : undefined; - - const entityOrEntities = target - ? (entityOrMaybeOptions as Entity | Entity[]) - : (targetOrEntity as Entity | Entity[]); - - const options = target - ? (maybeOptionsOrMaybePermissionOptions as RemoveOptions | undefined) - : (entityOrMaybeOptions as RemoveOptions); - - if (isDefined(target)) { - if (Array.isArray(entityOrEntities)) { - return super.remove(target, entityOrEntities as Entity[], options); - } else { - return super.remove(target, entityOrEntities as Entity, options); - } + if ( + criteria === undefined || + criteria === null || + criteria === '' || + (Array.isArray(criteria) && criteria.length === 0) + ) { + return Promise.reject( + new TypeORMError( + `Empty criteria(s) are not allowed for the delete method.`, + ), + ); + } + if ( + typeof criteria === 'string' || + typeof criteria === 'number' || + criteria instanceof Date || + Array.isArray(criteria) + ) { + return this.createQueryBuilder( + undefined, + undefined, + undefined, + permissionOptions, + ) + .delete() + .from(targetOrEntity) + .whereInIds(criteria) + .execute(); } else { - return super.remove(entityOrEntities as Entity | Entity[], options); + return this.createQueryBuilder( + undefined, + undefined, + undefined, + permissionOptions, + ) + .delete() + .from(targetOrEntity) + .where(criteria) + .execute(); } } @@ -692,170 +677,6 @@ export class WorkspaceEntityManager extends EntityManager { } } - override softRemove( - entities: Entity[], - options?: SaveOptions, - permissionOptions?: PermissionOptions, - ): Promise; - - override softRemove( - entities: Entity, - options?: SaveOptions, - permissionOptions?: PermissionOptions, - ): Promise; - - override softRemove< - Entity extends ObjectLiteral, - T extends DeepPartial, - >( - targetOrEntity: EntityTarget, - entities: T[], - options?: SaveOptions, - permissionOptions?: PermissionOptions, - ): Promise; - - override softRemove< - Entity extends ObjectLiteral, - T extends DeepPartial, - >( - targetOrEntity: EntityTarget, - entities: T, - options?: SaveOptions, - permissionOptions?: PermissionOptions, - ): Promise; - - override async softRemove< - Entity extends ObjectLiteral, - T extends DeepPartial, - >( - targetOrEntityOrEntities: Entity | Entity[] | EntityTarget, - entitiesOrMaybeOptions: T | T[] | SaveOptions, - maybeOptionsOrMaybePermissionOptions?: SaveOptions | PermissionOptions, - permissionOptions?: PermissionOptions, - ): Promise { - const permissionOptionsFromArgs = - maybeOptionsOrMaybePermissionOptions && - ('shouldBypassPermissionChecks' in maybeOptionsOrMaybePermissionOptions || - 'objectRecordsPermissions' in maybeOptionsOrMaybePermissionOptions) - ? (maybeOptionsOrMaybePermissionOptions as PermissionOptions) - : permissionOptions; - - this.validatePermissions( - targetOrEntityOrEntities, - 'soft-delete', - permissionOptionsFromArgs, - ); - - const target = - typeof targetOrEntityOrEntities === 'function' || - typeof targetOrEntityOrEntities === 'string' - ? (targetOrEntityOrEntities as EntityTarget) - : undefined; - - const entityOrEntities = target - ? (entitiesOrMaybeOptions as T | T[]) - : (targetOrEntityOrEntities as Entity | Entity[]); - - const options = target - ? (maybeOptionsOrMaybePermissionOptions as SaveOptions | undefined) - : (entitiesOrMaybeOptions as SaveOptions); - - if (isDefined(target)) { - if (Array.isArray(entityOrEntities)) { - return super.softRemove(target, entityOrEntities as T[], options); - } else { - return super.softRemove(target, entityOrEntities as T, options); - } - } else { - if (Array.isArray(entityOrEntities)) { - return super.softRemove(entityOrEntities as Entity | Entity[], options); - } else { - return super.softRemove(entityOrEntities as Entity, options); - } - } - } - - override recover( - entities: Entity[], - options?: SaveOptions, - permissionOptions?: PermissionOptions, - ): Promise; - - override recover( - entity: Entity, - options?: SaveOptions, - permissionOptions?: PermissionOptions, - ): Promise; - - override recover>( - targetOrEntity: EntityTarget, - entities: T[], - options?: SaveOptions, - permissionOptions?: PermissionOptions, - ): Promise; - - override recover>( - targetOrEntity: EntityTarget, - entity: T, - options?: SaveOptions, - permissionOptions?: PermissionOptions, - ): Promise; - - override recover>( - targetOrEntityOrEntities: EntityTarget | Entity | Entity[], - entityOrEntitiesOrMaybeOptions: T | T[] | SaveOptions, - maybeOptionsOrMaybePermissionOptions?: SaveOptions | PermissionOptions, - permissionOptions?: PermissionOptions, - ): Promise { - const permissionOptionsFromArgs = - maybeOptionsOrMaybePermissionOptions && - ('shouldBypassPermissionChecks' in maybeOptionsOrMaybePermissionOptions || - 'objectRecordsPermissions' in maybeOptionsOrMaybePermissionOptions) - ? (maybeOptionsOrMaybePermissionOptions as PermissionOptions) - : permissionOptions; - - this.validatePermissions( - targetOrEntityOrEntities, - 'restore', - permissionOptionsFromArgs, - ); - - const target = - typeof targetOrEntityOrEntities === 'function' || - typeof targetOrEntityOrEntities === 'string' - ? (targetOrEntityOrEntities as EntityTarget) - : undefined; - - const options = target - ? (maybeOptionsOrMaybePermissionOptions as SaveOptions | undefined) - : (entityOrEntitiesOrMaybeOptions as SaveOptions); - - if (target) { - if (Array.isArray(entityOrEntitiesOrMaybeOptions)) { - return super.recover( - target, - entityOrEntitiesOrMaybeOptions as T[], - options, - ); - } else { - return super.recover( - target, - entityOrEntitiesOrMaybeOptions as T, - options, - ); - } - } else { - if (Array.isArray(entityOrEntitiesOrMaybeOptions)) { - return super.recover( - entityOrEntitiesOrMaybeOptions as Entity | Entity[], - options, - ); - } else { - return super.recover(entityOrEntitiesOrMaybeOptions as Entity, options); - } - } - } - override restore( targetOrEntity: EntityTarget, criteria: unknown, @@ -1080,26 +901,435 @@ export class WorkspaceEntityManager extends EntityManager { return super.clear(entityClass); } - override preload( + override async preload( entityClass: EntityTarget, entityLike: DeepPartial, permissionOptions?: PermissionOptions, ): Promise { - this.validatePermissions(entityClass, 'select', permissionOptions); + const managerWithPermissionOptions = Object.assign( + Object.create(Object.getPrototypeOf(this)), + this, + { + findByIds: (entityClass: EntityTarget, ids: string[]) => { + return this.findByIds(entityClass, ids, permissionOptions); + }, + }, + ); - return super.preload(entityClass, entityLike); + const metadata = this.connection.getMetadata(entityClass); + const plainObjectToDatabaseEntityTransformer = + new PlainObjectToDatabaseEntityTransformer(managerWithPermissionOptions); + const transformedEntity = + await plainObjectToDatabaseEntityTransformer.transform( + entityLike, + metadata, + ); + + if (transformedEntity) + return this.merge(entityClass, transformedEntity, entityLike) as Entity; + + return undefined; } override decrement( target: EntityTarget, - criteria: unknown, + criteria: object, propertyPath: string, value: number | string, permissionOptions?: PermissionOptions, ): Promise { - this.validatePermissions(target, 'update', permissionOptions); + const metadata = this.connection.getMetadata(target); + const column = metadata.findColumnWithPropertyPath(propertyPath); - return super.decrement(target, criteria, propertyPath, value); + if (!column) + throw new TypeORMError( + `Column ${propertyPath} was not found in ${metadata.targetName} entity.`, + ); + if (isNaN(Number(value))) + throw new TypeORMError(`Value "${value}" is not a number.`); + // eslint-disable-next-line @typescript-eslint/no-explicit-any + const values = propertyPath.split('.').reduceRight( + (value, key) => ({ [key]: value }), + () => this.connection.driver.escape(column.databaseName) + ' - ' + value, + ); + + return this.createQueryBuilder( + target, + 'entity', + undefined, + permissionOptions, + ) + .update(target as QueryDeepPartialEntity) + .set(values) + .where(criteria) + .execute(); + } + + override findByIds( + entityClass: EntityTarget, + ids: string[], + permissionOptions?: PermissionOptions, + ): Promise { + if (!ids.length) return Promise.resolve([]); + const metadata = this.connection.getMetadata(entityClass); + + return this.createQueryBuilder( + entityClass, + metadata.name, + undefined, + permissionOptions, + ) + .andWhereInIds(ids) + .getMany(); + } + + /** + * Functions duplicated from EntityManager but with a queryRunner that will bypass permissions + * because permissions cannot be passed on to the call to createQueryBuilder() done in SubjectExecutor called by EntityPersistExecutor + * queryBuilder checks are replaced by validatePermissions() + */ + + override save( + entities: Entity[], + options?: SaveOptions, + permissionOptions?: PermissionOptions, + ): Promise; + + override save( + entity: Entity, + options?: SaveOptions, + permissionOptions?: PermissionOptions, + ): Promise; + + override save>( + targetOrEntity: EntityTarget, + entities: T[], + options: SaveOptions & { + reload: false; + }, + permissionOptions?: PermissionOptions, + ): Promise; + + override save>( + targetOrEntity: EntityTarget, + entities: T[], + options?: SaveOptions, + permissionOptions?: PermissionOptions, + ): Promise<(T & Entity)[]>; + + override save>( + targetOrEntity: EntityTarget, + entity: T, + options: SaveOptions & { + reload: false; + }, + permissionOptions?: PermissionOptions, + ): Promise; + + override save>( + targetOrEntity: EntityTarget, + entity: T, + options?: SaveOptions, + permissionOptions?: PermissionOptions, + ): Promise; + + override save>( + targetOrEntity: EntityTarget | Entity | Entity[], + entityOrMaybeOptions: + | T + | T[] + | SaveOptions + | (SaveOptions & { reload: false }), + maybeOptionsOrMaybePermissionOptions?: + | PermissionOptions + | SaveOptions + | (SaveOptions & { reload: false }), + permissionOptions?: PermissionOptions, + ): Promise<(T & Entity) | (T & Entity)[] | Entity | Entity[]> { + const permissionOptionsFromArgs = + maybeOptionsOrMaybePermissionOptions && + ('shouldBypassPermissionChecks' in maybeOptionsOrMaybePermissionOptions || + 'objectRecordsPermissions' in maybeOptionsOrMaybePermissionOptions) + ? maybeOptionsOrMaybePermissionOptions + : permissionOptions; + + this.validatePermissions( + targetOrEntity, + 'update', + permissionOptionsFromArgs, + ); + + let target = + arguments.length > 1 && + (typeof targetOrEntity === 'function' || + InstanceChecker.isEntitySchema(targetOrEntity) || + typeof targetOrEntity === 'string') + ? targetOrEntity + : undefined; + const entity = target ? entityOrMaybeOptions : targetOrEntity; + const options = target + ? maybeOptionsOrMaybePermissionOptions + : entityOrMaybeOptions; + + if (InstanceChecker.isEntitySchema(target)) target = target.options.name; + if (Array.isArray(entity) && entity.length === 0) + return Promise.resolve(entity as Entity[]); + + const queryRunnerForEntityPersistExecutor = + this.connection.createQueryRunnerForEntityPersistExecutor(); + + return new EntityPersistExecutor( + this.connection, + queryRunnerForEntityPersistExecutor, + 'save', + target, + entity as ObjectLiteral, + options as SaveOptions | (SaveOptions & { reload: false }), + ) + .execute() + .then(() => entity as Entity) + .finally(() => queryRunnerForEntityPersistExecutor.release()); + } + + override remove( + entity: Entity, + options?: RemoveOptions, + permissionOptions?: PermissionOptions, + ): Promise; + + override remove( + targetOrEntity: EntityTarget, + entity: Entity, + options?: RemoveOptions, + permissionOptions?: PermissionOptions, + ): Promise; + + override remove( + entity: Entity[], + options?: RemoveOptions, + permissionOptions?: PermissionOptions, + ): Promise; + + override remove( + targetOrEntity: EntityTarget, + entity: Entity[], + options?: RemoveOptions, + permissionOptions?: PermissionOptions, + ): Promise; + + override remove( + targetOrEntity: EntityTarget | Entity[] | Entity, + entityOrMaybeOptions: Entity | Entity[] | RemoveOptions, + maybeOptionsOrMaybePermissionOptions?: RemoveOptions | PermissionOptions, + permissionOptions?: PermissionOptions, + ): Promise { + const permissionOptionsFromArgs = + maybeOptionsOrMaybePermissionOptions && + ('shouldBypassPermissionChecks' in maybeOptionsOrMaybePermissionOptions || + 'objectRecordsPermissions' in maybeOptionsOrMaybePermissionOptions) + ? (maybeOptionsOrMaybePermissionOptions as PermissionOptions) + : permissionOptions; + + this.validatePermissions( + targetOrEntity, + 'delete', + permissionOptionsFromArgs, + ); + + const target = + arguments.length > 1 && + (typeof targetOrEntity === 'function' || + InstanceChecker.isEntitySchema(targetOrEntity) || + typeof targetOrEntity === 'string') + ? targetOrEntity + : undefined; + const entity = target ? entityOrMaybeOptions : targetOrEntity; + const options = target + ? maybeOptionsOrMaybePermissionOptions + : entityOrMaybeOptions; + + if (Array.isArray(entity) && entity.length === 0) + return Promise.resolve(entity); + + const queryRunnerForEntityPersistExecutor = + this.connection.createQueryRunnerForEntityPersistExecutor(); + + return new EntityPersistExecutor( + this.connection, + queryRunnerForEntityPersistExecutor, + 'remove', + target as string | undefined, + entity as ObjectLiteral, + options as RemoveOptions, + ) + .execute() + .then(() => entity as Entity | Entity[]) + .finally(() => queryRunnerForEntityPersistExecutor.release()); + } + + override softRemove( + entities: Entity[], + options?: SaveOptions, + permissionOptions?: PermissionOptions, + ): Promise; + + override softRemove( + entities: Entity, + options?: SaveOptions, + permissionOptions?: PermissionOptions, + ): Promise; + + override softRemove< + Entity extends ObjectLiteral, + T extends DeepPartial, + >( + targetOrEntity: EntityTarget, + entities: T[], + options?: SaveOptions, + permissionOptions?: PermissionOptions, + ): Promise; + + override softRemove< + Entity extends ObjectLiteral, + T extends DeepPartial, + >( + targetOrEntity: EntityTarget, + entities: T, + options?: SaveOptions, + permissionOptions?: PermissionOptions, + ): Promise; + + override async softRemove< + Entity extends ObjectLiteral, + T extends DeepPartial, + >( + targetOrEntityOrEntities: Entity | Entity[] | EntityTarget, + entitiesOrMaybeOptions: T | T[] | SaveOptions, + maybeOptionsOrMaybePermissionOptions?: SaveOptions | PermissionOptions, + permissionOptions?: PermissionOptions, + ): Promise { + const permissionOptionsFromArgs = + maybeOptionsOrMaybePermissionOptions && + ('shouldBypassPermissionChecks' in maybeOptionsOrMaybePermissionOptions || + 'objectRecordsPermissions' in maybeOptionsOrMaybePermissionOptions) + ? (maybeOptionsOrMaybePermissionOptions as PermissionOptions) + : permissionOptions; + + this.validatePermissions( + targetOrEntityOrEntities, + 'soft-delete', + permissionOptionsFromArgs, + ); + + let target = + arguments.length > 1 && + (typeof targetOrEntityOrEntities === 'function' || + InstanceChecker.isEntitySchema(targetOrEntityOrEntities) || + typeof targetOrEntityOrEntities === 'string') + ? targetOrEntityOrEntities + : undefined; + const entity = target ? entitiesOrMaybeOptions : targetOrEntityOrEntities; + const options = target + ? maybeOptionsOrMaybePermissionOptions + : entitiesOrMaybeOptions; + + if (InstanceChecker.isEntitySchema(target)) target = target.options.name; + if (Array.isArray(entity) && entity.length === 0) + return Promise.resolve(entity); + + const queryRunnerForEntityPersistExecutor = + this.connection.createQueryRunnerForEntityPersistExecutor(); + + return new EntityPersistExecutor( + this.connection, + queryRunnerForEntityPersistExecutor, + 'soft-remove', + target, + entity as ObjectLiteral, + options as SaveOptions, + ) + .execute() + .then(() => entity as Entity) + .finally(() => queryRunnerForEntityPersistExecutor.release()); + } + + override recover( + entities: Entity[], + options?: SaveOptions, + permissionOptions?: PermissionOptions, + ): Promise; + + override recover( + entity: Entity, + options?: SaveOptions, + permissionOptions?: PermissionOptions, + ): Promise; + + override recover>( + targetOrEntity: EntityTarget, + entities: T[], + options?: SaveOptions, + permissionOptions?: PermissionOptions, + ): Promise; + + override recover>( + targetOrEntity: EntityTarget, + entity: T, + options?: SaveOptions, + permissionOptions?: PermissionOptions, + ): Promise; + + override recover>( + targetOrEntityOrEntities: EntityTarget | Entity | Entity[], + entityOrEntitiesOrMaybeOptions: T | T[] | SaveOptions, + maybeOptionsOrMaybePermissionOptions?: SaveOptions | PermissionOptions, + permissionOptions?: PermissionOptions, + ): Promise { + const permissionOptionsFromArgs = + maybeOptionsOrMaybePermissionOptions && + ('shouldBypassPermissionChecks' in maybeOptionsOrMaybePermissionOptions || + 'objectRecordsPermissions' in maybeOptionsOrMaybePermissionOptions) + ? (maybeOptionsOrMaybePermissionOptions as PermissionOptions) + : permissionOptions; + + this.validatePermissions( + targetOrEntityOrEntities, + 'restore', + permissionOptionsFromArgs, + ); + + let target = + arguments.length > 1 && + (typeof targetOrEntityOrEntities === 'function' || + InstanceChecker.isEntitySchema(targetOrEntityOrEntities) || + typeof targetOrEntityOrEntities === 'string') + ? targetOrEntityOrEntities + : undefined; + const entity = target + ? entityOrEntitiesOrMaybeOptions + : targetOrEntityOrEntities; + const options = target + ? maybeOptionsOrMaybePermissionOptions + : entityOrEntitiesOrMaybeOptions; + + if (InstanceChecker.isEntitySchema(target)) target = target.options.name; + if (Array.isArray(entity) && entity.length === 0) + return Promise.resolve(entity); + + const queryRunnerForEntityPersistExecutor = + this.connection.createQueryRunnerForEntityPersistExecutor(); + + return new EntityPersistExecutor( + this.connection, + queryRunnerForEntityPersistExecutor, + 'recover', + target, + entity as ObjectLiteral, + options as SaveOptions, + ) + .execute() + .then(() => entity as Entity) + .finally(() => queryRunnerForEntityPersistExecutor.release()); } // Forbidden methods diff --git a/packages/twenty-server/src/engine/twenty-orm/repository/workspace-select-query-builder.ts b/packages/twenty-server/src/engine/twenty-orm/repository/workspace-select-query-builder.ts index 9ee79aa5f..c67acd7be 100644 --- a/packages/twenty-server/src/engine/twenty-orm/repository/workspace-select-query-builder.ts +++ b/packages/twenty-server/src/engine/twenty-orm/repository/workspace-select-query-builder.ts @@ -10,6 +10,7 @@ import { } from 'src/engine/metadata-modules/permissions/permissions.exception'; import { validateQueryIsPermittedOrThrow } from 'src/engine/twenty-orm/repository/permissions.utils'; import { WorkspaceDeleteQueryBuilder } from 'src/engine/twenty-orm/repository/workspace-delete-query-builder'; +import { WorkspaceInsertQueryBuilder } from 'src/engine/twenty-orm/repository/workspace-insert-query-builder'; import { WorkspaceSoftDeleteQueryBuilder } from 'src/engine/twenty-orm/repository/workspace-soft-delete-query-builder'; import { WorkspaceUpdateQueryBuilder } from 'src/engine/twenty-orm/repository/workspace-update-query-builder'; @@ -99,6 +100,17 @@ export class WorkspaceSelectQueryBuilder< return super.getManyAndCount(); } + override insert(): WorkspaceInsertQueryBuilder { + const insertQueryBuilder = super.insert(); + + return new WorkspaceInsertQueryBuilder( + insertQueryBuilder, + this.objectRecordsPermissions, + this.internalContext, + this.shouldBypassPermissionChecks, + ); + } + override update(): WorkspaceUpdateQueryBuilder; override update( diff --git a/packages/twenty-server/src/modules/match-participant/match-participant.service.spec.ts b/packages/twenty-server/src/modules/match-participant/match-participant.service.spec.ts index 47817c9a0..e6ed79abe 100644 --- a/packages/twenty-server/src/modules/match-participant/match-participant.service.spec.ts +++ b/packages/twenty-server/src/modules/match-participant/match-participant.service.spec.ts @@ -2,7 +2,7 @@ import { Test, TestingModule } from '@nestjs/testing'; import { WorkspaceEntityManager } from 'src/engine/twenty-orm/entity-manager/workspace-entity-manager'; import { ScopedWorkspaceContextFactory } from 'src/engine/twenty-orm/factories/scoped-workspace-context.factory'; -import { TwentyORMManager } from 'src/engine/twenty-orm/twenty-orm.manager'; +import { TwentyORMGlobalManager } from 'src/engine/twenty-orm/twenty-orm-global.manager'; import { WorkspaceEventEmitter } from 'src/engine/workspace-event-emitter/workspace-event-emitter'; import { CalendarEventParticipantWorkspaceEntity } from 'src/modules/calendar/common/standard-objects/calendar-event-participant.workspace-entity'; import { MatchParticipantService } from 'src/modules/match-participant/match-participant.service'; @@ -12,7 +12,7 @@ import { WorkspaceMemberWorkspaceEntity } from 'src/modules/workspace-member/sta describe('MatchParticipantService', () => { let service: MatchParticipantService; - let twentyORMManager: TwentyORMManager; + let twentyORMGlobalManager: TwentyORMGlobalManager; let workspaceEventEmitter: WorkspaceEventEmitter; let scopedWorkspaceContextFactory: ScopedWorkspaceContextFactory; @@ -95,22 +95,24 @@ describe('MatchParticipantService', () => { providers: [ MatchParticipantService, { - provide: TwentyORMManager, + provide: TwentyORMGlobalManager, useValue: { - getRepository: jest.fn().mockImplementation((entityName) => { - switch (entityName) { - case 'messageParticipant': - return mockMessageParticipantRepository; - case 'calendarEventParticipant': - return mockCalendarEventParticipantRepository; - case 'person': - return mockPersonRepository; - case 'workspaceMember': - return mockWorkspaceMemberRepository; - default: - return {}; - } - }), + getRepositoryForWorkspace: jest + .fn() + .mockImplementation((_workspaceId, entityName) => { + switch (entityName) { + case 'messageParticipant': + return mockMessageParticipantRepository; + case 'calendarEventParticipant': + return mockCalendarEventParticipantRepository; + case 'person': + return mockPersonRepository; + case 'workspaceMember': + return mockWorkspaceMemberRepository; + default: + return {}; + } + }), }, }, { @@ -133,7 +135,9 @@ describe('MatchParticipantService', () => { service = module.get< MatchParticipantService >(MatchParticipantService); - twentyORMManager = module.get(TwentyORMManager); + twentyORMGlobalManager = module.get( + TwentyORMGlobalManager, + ); workspaceEventEmitter = module.get( WorkspaceEventEmitter, ); @@ -287,7 +291,7 @@ describe('MatchParticipantService', () => { const calendarService = new MatchParticipantService( workspaceEventEmitter, - twentyORMManager, + twentyORMGlobalManager, scopedWorkspaceContextFactory, ); @@ -705,23 +709,25 @@ describe('MatchParticipantService', () => { describe('getParticipantRepository', () => { it('should return message participant repository for messageParticipant', async () => { const repository = await (service as any).getParticipantRepository( + mockWorkspaceId, 'messageParticipant', ); - expect(twentyORMManager.getRepository).toHaveBeenCalledWith( - 'messageParticipant', - ); + expect( + twentyORMGlobalManager.getRepositoryForWorkspace, + ).toHaveBeenCalledWith(mockWorkspaceId, 'messageParticipant'); expect(repository).toBe(mockMessageParticipantRepository); }); it('should return calendar event participant repository for calendarEventParticipant', async () => { const repository = await (service as any).getParticipantRepository( + mockWorkspaceId, 'calendarEventParticipant', ); - expect(twentyORMManager.getRepository).toHaveBeenCalledWith( - 'calendarEventParticipant', - ); + expect( + twentyORMGlobalManager.getRepositoryForWorkspace, + ).toHaveBeenCalledWith(mockWorkspaceId, 'calendarEventParticipant'); expect(repository).toBe(mockCalendarEventParticipantRepository); }); }); diff --git a/packages/twenty-server/src/modules/match-participant/match-participant.service.ts b/packages/twenty-server/src/modules/match-participant/match-participant.service.ts index 9e453b672..01a97625c 100644 --- a/packages/twenty-server/src/modules/match-participant/match-participant.service.ts +++ b/packages/twenty-server/src/modules/match-participant/match-participant.service.ts @@ -4,7 +4,7 @@ import { Any, Equal } from 'typeorm'; import { WorkspaceEntityManager } from 'src/engine/twenty-orm/entity-manager/workspace-entity-manager'; import { ScopedWorkspaceContextFactory } from 'src/engine/twenty-orm/factories/scoped-workspace-context.factory'; -import { TwentyORMManager } from 'src/engine/twenty-orm/twenty-orm.manager'; +import { TwentyORMGlobalManager } from 'src/engine/twenty-orm/twenty-orm-global.manager'; import { WorkspaceEventEmitter } from 'src/engine/workspace-event-emitter/workspace-event-emitter'; import { CalendarEventParticipantWorkspaceEntity } from 'src/modules/calendar/common/standard-objects/calendar-event-participant.workspace-entity'; import { addPersonEmailFiltersToQueryBuilder } from 'src/modules/match-participant/utils/add-person-email-filters-to-query-builder'; @@ -21,20 +21,23 @@ export class MatchParticipantService< > { constructor( private readonly workspaceEventEmitter: WorkspaceEventEmitter, - private readonly twentyORMManager: TwentyORMManager, + private readonly twentyORMGlobalManager: TwentyORMGlobalManager, private readonly scopedWorkspaceContextFactory: ScopedWorkspaceContextFactory, ) {} private async getParticipantRepository( + workspaceId: string, objectMetadataName: 'messageParticipant' | 'calendarEventParticipant', ) { if (objectMetadataName === 'messageParticipant') { - return await this.twentyORMManager.getRepository( + return await this.twentyORMGlobalManager.getRepositoryForWorkspace( + workspaceId, objectMetadataName, ); } - return await this.twentyORMManager.getRepository( + return await this.twentyORMGlobalManager.getRepositoryForWorkspace( + workspaceId, objectMetadataName, ); } @@ -52,14 +55,15 @@ export class MatchParticipantService< return; } - const participantRepository = - await this.getParticipantRepository(objectMetadataName); - const workspaceId = this.scopedWorkspaceContextFactory.create().workspaceId; if (!workspaceId) { throw new Error('Workspace ID is required'); } + const participantRepository = await this.getParticipantRepository( + workspaceId, + objectMetadataName, + ); const participantIds = participants.map((participant) => participant.id); const uniqueParticipantsHandles = [ @@ -67,8 +71,10 @@ export class MatchParticipantService< ]; const personRepository = - await this.twentyORMManager.getRepository( + await this.twentyORMGlobalManager.getRepositoryForWorkspace( + workspaceId, 'person', + { shouldBypassPermissionChecks: true }, ); const queryBuilder = addPersonEmailFiltersToQueryBuilder({ @@ -83,7 +89,8 @@ export class MatchParticipantService< const people = await personRepository.formatResult(rawPeople); const workspaceMemberRepository = - await this.twentyORMManager.getRepository( + await this.twentyORMGlobalManager.getRepositoryForWorkspace( + workspaceId, 'workspaceMember', ); @@ -152,14 +159,15 @@ export class MatchParticipantService< personId?: string; workspaceMemberId?: string; }) { - const participantRepository = - await this.getParticipantRepository(objectMetadataName); - const workspaceId = this.scopedWorkspaceContextFactory.create().workspaceId; if (!workspaceId) { throw new Error('Workspace ID is required'); } + const participantRepository = await this.getParticipantRepository( + workspaceId, + objectMetadataName, + ); if (personId) { await participantRepository.update( @@ -172,8 +180,10 @@ export class MatchParticipantService< ); const personRepository = - await this.twentyORMManager.getRepository( + await this.twentyORMGlobalManager.getRepositoryForWorkspace( + workspaceId, 'person', + { shouldBypassPermissionChecks: true }, ); const queryBuilder = addPersonEmailFiltersToQueryBuilder({ @@ -253,8 +263,10 @@ export class MatchParticipantService< throw new Error('Workspace ID is required'); } - const participantRepository = - await this.getParticipantRepository(objectMetadataName); + const participantRepository = await this.getParticipantRepository( + workspaceId, + objectMetadataName, + ); const participantsToUpdate = await participantRepository.find({ where: { @@ -340,8 +352,10 @@ export class MatchParticipantService< throw new Error('Workspace ID is required'); } - const participantRepository = - await this.getParticipantRepository(objectMetadataName); + const participantRepository = await this.getParticipantRepository( + workspaceId, + objectMetadataName, + ); const participantsToUpdate = await participantRepository.find({ where: {