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 <charles@twenty.com>
This commit is contained in:
@ -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<Entity> | QueryRunner,
|
||||
alias?: string,
|
||||
queryRunner?: QueryRunner,
|
||||
) => {
|
||||
if (isDefined(alias) && typeof alias === 'string') {
|
||||
const entity = entityOrRunner as EntityTarget<Entity>;
|
||||
|
||||
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<Entity extends ObjectLiteral>(
|
||||
entityClass: EntityTarget<Entity>,
|
||||
alias: string,
|
||||
|
||||
@ -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,
|
||||
});
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
File diff suppressed because it is too large
Load Diff
@ -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<T> {
|
||||
const insertQueryBuilder = super.insert();
|
||||
|
||||
return new WorkspaceInsertQueryBuilder<T>(
|
||||
insertQueryBuilder,
|
||||
this.objectRecordsPermissions,
|
||||
this.internalContext,
|
||||
this.shouldBypassPermissionChecks,
|
||||
);
|
||||
}
|
||||
|
||||
override update(): WorkspaceUpdateQueryBuilder<T>;
|
||||
|
||||
override update(
|
||||
|
||||
@ -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<MessageParticipantWorkspaceEntity>;
|
||||
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<MessageParticipantWorkspaceEntity>
|
||||
>(MatchParticipantService);
|
||||
twentyORMManager = module.get<TwentyORMManager>(TwentyORMManager);
|
||||
twentyORMGlobalManager = module.get<TwentyORMGlobalManager>(
|
||||
TwentyORMGlobalManager,
|
||||
);
|
||||
workspaceEventEmitter = module.get<WorkspaceEventEmitter>(
|
||||
WorkspaceEventEmitter,
|
||||
);
|
||||
@ -287,7 +291,7 @@ describe('MatchParticipantService', () => {
|
||||
const calendarService =
|
||||
new MatchParticipantService<CalendarEventParticipantWorkspaceEntity>(
|
||||
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);
|
||||
});
|
||||
});
|
||||
|
||||
@ -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<MessageParticipantWorkspaceEntity>(
|
||||
return await this.twentyORMGlobalManager.getRepositoryForWorkspace<MessageParticipantWorkspaceEntity>(
|
||||
workspaceId,
|
||||
objectMetadataName,
|
||||
);
|
||||
}
|
||||
|
||||
return await this.twentyORMManager.getRepository<CalendarEventParticipantWorkspaceEntity>(
|
||||
return await this.twentyORMGlobalManager.getRepositoryForWorkspace<CalendarEventParticipantWorkspaceEntity>(
|
||||
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<PersonWorkspaceEntity>(
|
||||
await this.twentyORMGlobalManager.getRepositoryForWorkspace<PersonWorkspaceEntity>(
|
||||
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<WorkspaceMemberWorkspaceEntity>(
|
||||
await this.twentyORMGlobalManager.getRepositoryForWorkspace<WorkspaceMemberWorkspaceEntity>(
|
||||
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<PersonWorkspaceEntity>(
|
||||
await this.twentyORMGlobalManager.getRepositoryForWorkspace<PersonWorkspaceEntity>(
|
||||
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: {
|
||||
|
||||
Reference in New Issue
Block a user