[permissions] Remove raw queries and restrict its usage (#12360)
Closes https://github.com/twentyhq/core-team-issues/issues/748 In the frame of the work on permissions we - remove all raw queries possible to use repositories instead - forbid usage workspaceDataSource.executeRawQueries() - restrict usage of workspaceDataSource.query() to force developers to pass on shouldBypassPermissionChecks to use it. --------- Co-authored-by: greptile-apps[bot] <165735046+greptile-apps[bot]@users.noreply.github.com>
This commit is contained in:
Marie
@ -2,16 +2,10 @@ import { Module } from '@nestjs/common';
|
||||
|
||||
import { AuditModule } from 'src/engine/core-modules/audit/audit.module';
|
||||
import { CreateAuditLogFromInternalEvent } from 'src/engine/core-modules/audit/jobs/create-audit-log-from-internal-event';
|
||||
import { ObjectMetadataRepositoryModule } from 'src/engine/object-metadata-repository/object-metadata-repository.module';
|
||||
import { TimelineActivityModule } from 'src/modules/timeline/timeline-activity.module';
|
||||
import { WorkspaceMemberWorkspaceEntity } from 'src/modules/workspace-member/standard-objects/workspace-member.workspace-entity';
|
||||
|
||||
@Module({
|
||||
imports: [
|
||||
ObjectMetadataRepositoryModule.forFeature([WorkspaceMemberWorkspaceEntity]),
|
||||
TimelineActivityModule,
|
||||
AuditModule,
|
||||
],
|
||||
imports: [TimelineActivityModule, AuditModule],
|
||||
providers: [CreateAuditLogFromInternalEvent],
|
||||
})
|
||||
export class AuditJobModule {}
|
||||
|
||||
@ -1,11 +1,11 @@
|
||||
import { Module } from '@nestjs/common';
|
||||
|
||||
import { WorkspaceDataSourceModule } from 'src/engine/workspace-datasource/workspace-datasource.module';
|
||||
import { TwentyORMModule } from 'src/engine/twenty-orm/twenty-orm.module';
|
||||
|
||||
import { RecordPositionService } from './services/record-position.service';
|
||||
|
||||
@Module({
|
||||
imports: [WorkspaceDataSourceModule],
|
||||
imports: [TwentyORMModule],
|
||||
providers: [RecordPositionService],
|
||||
exports: [RecordPositionService],
|
||||
})
|
||||
|
||||
@ -1,24 +1,31 @@
|
||||
import { Test, TestingModule } from '@nestjs/testing';
|
||||
|
||||
import { RecordPositionService } from 'src/engine/core-modules/record-position/services/record-position.service';
|
||||
import { WorkspaceDataSourceService } from 'src/engine/workspace-datasource/workspace-datasource.service';
|
||||
import { TwentyORMGlobalManager } from 'src/engine/twenty-orm/twenty-orm-global.manager';
|
||||
|
||||
describe('RecordPositionService', () => {
|
||||
let workspaceDataSourceService;
|
||||
|
||||
let twentyORMGlobalManager: jest.Mocked<TwentyORMGlobalManager>;
|
||||
let mockRepository: any;
|
||||
let service: RecordPositionService;
|
||||
|
||||
beforeEach(async () => {
|
||||
workspaceDataSourceService = {
|
||||
getSchemaName: jest.fn().mockReturnValue('schemaName'),
|
||||
executeRawQuery: jest.fn().mockResolvedValue([{ position: 1 }]),
|
||||
mockRepository = {
|
||||
findOneBy: jest.fn(),
|
||||
update: jest.fn(),
|
||||
minimum: jest.fn().mockResolvedValue(1),
|
||||
maximum: jest.fn().mockResolvedValue(1),
|
||||
};
|
||||
|
||||
twentyORMGlobalManager = {
|
||||
getRepositoryForWorkspace: jest.fn().mockResolvedValue(mockRepository),
|
||||
} as unknown as jest.Mocked<TwentyORMGlobalManager>;
|
||||
|
||||
const module: TestingModule = await Test.createTestingModule({
|
||||
providers: [
|
||||
RecordPositionService,
|
||||
{
|
||||
provide: WorkspaceDataSourceService,
|
||||
useValue: workspaceDataSourceService,
|
||||
provide: TwentyORMGlobalManager,
|
||||
useValue: twentyORMGlobalManager,
|
||||
},
|
||||
],
|
||||
}).compile();
|
||||
@ -30,7 +37,7 @@ describe('RecordPositionService', () => {
|
||||
expect(service).toBeDefined();
|
||||
});
|
||||
|
||||
describe('create', () => {
|
||||
describe('buildRecordPosition', () => {
|
||||
const objectMetadata = { isCustom: false, nameSingular: 'company' };
|
||||
const workspaceId = 'workspaceId';
|
||||
|
||||
|
||||
@ -1,13 +1,6 @@
|
||||
import { Injectable } from '@nestjs/common';
|
||||
|
||||
import { isDefined } from 'class-validator';
|
||||
|
||||
import {
|
||||
RecordPositionQueryArgs,
|
||||
RecordPositionQueryType,
|
||||
} from 'src/engine/core-modules/record-position/types/record-position-query.type';
|
||||
import { buildRecordPositionQuery } from 'src/engine/core-modules/record-position/utils/build-record-position-query.util';
|
||||
import { WorkspaceDataSourceService } from 'src/engine/workspace-datasource/workspace-datasource.service';
|
||||
import { TwentyORMGlobalManager } from 'src/engine/twenty-orm/twenty-orm-global.manager';
|
||||
|
||||
export type RecordPositionServiceCreateArgs = {
|
||||
value: number | 'first' | 'last';
|
||||
@ -19,7 +12,7 @@ export type RecordPositionServiceCreateArgs = {
|
||||
@Injectable()
|
||||
export class RecordPositionService {
|
||||
constructor(
|
||||
private readonly workspaceDataSourceService: WorkspaceDataSourceService,
|
||||
private readonly twentyORMGlobalManager: TwentyORMGlobalManager,
|
||||
) {}
|
||||
|
||||
async buildRecordPosition({
|
||||
@ -28,62 +21,101 @@ export class RecordPositionService {
|
||||
workspaceId,
|
||||
index = 0,
|
||||
}: RecordPositionServiceCreateArgs): Promise<number> {
|
||||
const dataSourceSchema =
|
||||
this.workspaceDataSourceService.getSchemaName(workspaceId);
|
||||
|
||||
if (typeof value === 'number') {
|
||||
return value;
|
||||
}
|
||||
|
||||
if (value === 'first') {
|
||||
const recordWithMinPosition =
|
||||
await this.createAndExecuteRecordPositionQuery(
|
||||
{
|
||||
recordPositionQueryType: RecordPositionQueryType.FIND_MIN_POSITION,
|
||||
},
|
||||
objectMetadata,
|
||||
dataSourceSchema,
|
||||
workspaceId,
|
||||
);
|
||||
|
||||
return isDefined(recordWithMinPosition?.position)
|
||||
? recordWithMinPosition.position - index - 1
|
||||
: 1;
|
||||
}
|
||||
|
||||
const recordWithMaxPosition =
|
||||
await this.createAndExecuteRecordPositionQuery(
|
||||
{
|
||||
recordPositionQueryType: RecordPositionQueryType.FIND_MAX_POSITION,
|
||||
},
|
||||
const recordWithMinPosition = await this.findMinPosition(
|
||||
objectMetadata,
|
||||
dataSourceSchema,
|
||||
workspaceId,
|
||||
);
|
||||
|
||||
return isDefined(recordWithMaxPosition?.position)
|
||||
? recordWithMaxPosition.position + index + 1
|
||||
: 1;
|
||||
}
|
||||
return recordWithMinPosition !== null
|
||||
? recordWithMinPosition - index - 1
|
||||
: 1;
|
||||
}
|
||||
|
||||
private async createAndExecuteRecordPositionQuery(
|
||||
recordPositionQueryArgs: RecordPositionQueryArgs,
|
||||
objectMetadata: { isCustom: boolean; nameSingular: string },
|
||||
dataSourceSchema: string,
|
||||
workspaceId: string,
|
||||
) {
|
||||
const [query, params] = buildRecordPositionQuery(
|
||||
recordPositionQueryArgs,
|
||||
const recordWithMaxPosition = await this.findMaxPosition(
|
||||
objectMetadata,
|
||||
dataSourceSchema,
|
||||
);
|
||||
|
||||
const records = await this.workspaceDataSourceService.executeRawQuery(
|
||||
query,
|
||||
params,
|
||||
workspaceId,
|
||||
);
|
||||
|
||||
return records?.[0];
|
||||
return recordWithMaxPosition !== null
|
||||
? recordWithMaxPosition + index + 1
|
||||
: 1;
|
||||
}
|
||||
|
||||
async findByPosition(
|
||||
positionValue: number | null,
|
||||
objectMetadata: { isCustom: boolean; nameSingular: string },
|
||||
workspaceId: string,
|
||||
): Promise<{ id: string; position: number } | null> {
|
||||
const repository =
|
||||
await this.twentyORMGlobalManager.getRepositoryForWorkspace(
|
||||
workspaceId,
|
||||
objectMetadata.nameSingular,
|
||||
{
|
||||
shouldBypassPermissionChecks: true,
|
||||
},
|
||||
);
|
||||
|
||||
const record = await repository.findOneBy({
|
||||
position: positionValue,
|
||||
});
|
||||
|
||||
return record ? { id: record.id, position: record.position } : null;
|
||||
}
|
||||
|
||||
async updatePosition(
|
||||
recordId: string,
|
||||
positionValue: number,
|
||||
objectMetadata: { isCustom: boolean; nameSingular: string },
|
||||
workspaceId: string,
|
||||
): Promise<void> {
|
||||
const repository =
|
||||
await this.twentyORMGlobalManager.getRepositoryForWorkspace(
|
||||
workspaceId,
|
||||
objectMetadata.nameSingular,
|
||||
{
|
||||
shouldBypassPermissionChecks: true,
|
||||
},
|
||||
);
|
||||
|
||||
await repository.update(recordId, {
|
||||
position: positionValue,
|
||||
});
|
||||
}
|
||||
|
||||
private async findMinPosition(
|
||||
objectMetadata: { isCustom: boolean; nameSingular: string },
|
||||
workspaceId: string,
|
||||
): Promise<number | null> {
|
||||
const repository =
|
||||
await this.twentyORMGlobalManager.getRepositoryForWorkspace(
|
||||
workspaceId,
|
||||
objectMetadata.nameSingular,
|
||||
{
|
||||
shouldBypassPermissionChecks: true,
|
||||
},
|
||||
);
|
||||
|
||||
return repository.minimum('position');
|
||||
}
|
||||
|
||||
private async findMaxPosition(
|
||||
objectMetadata: { isCustom: boolean; nameSingular: string },
|
||||
workspaceId: string,
|
||||
): Promise<number | null> {
|
||||
const repository =
|
||||
await this.twentyORMGlobalManager.getRepositoryForWorkspace(
|
||||
workspaceId,
|
||||
objectMetadata.nameSingular,
|
||||
{
|
||||
shouldBypassPermissionChecks: true,
|
||||
},
|
||||
);
|
||||
|
||||
return repository.maximum('position');
|
||||
}
|
||||
}
|
||||
|
||||
@ -1,72 +0,0 @@
|
||||
import { RecordPositionQueryType } from 'src/engine/core-modules/record-position/types/record-position-query.type';
|
||||
import { buildRecordPositionQuery } from 'src/engine/core-modules/record-position/utils/build-record-position-query.util';
|
||||
|
||||
describe('buildRecordPositionQuery', () => {
|
||||
const objectMetadataItem = {
|
||||
isCustom: false,
|
||||
nameSingular: 'company',
|
||||
};
|
||||
const dataSourceSchema = 'workspace_test';
|
||||
|
||||
it('should return query and params for FIND_BY_POSITION', async () => {
|
||||
const positionValue = 1;
|
||||
const queryType = RecordPositionQueryType.FIND_BY_POSITION;
|
||||
const [query, params] = buildRecordPositionQuery(
|
||||
{ positionValue, recordPositionQueryType: queryType },
|
||||
objectMetadataItem,
|
||||
dataSourceSchema,
|
||||
);
|
||||
|
||||
expect(query).toEqual(
|
||||
`SELECT id, position FROM ${dataSourceSchema}."${objectMetadataItem.nameSingular}"
|
||||
WHERE "position" = $1`,
|
||||
);
|
||||
expect(params).toEqual([positionValue]);
|
||||
});
|
||||
|
||||
it('should return query and params for FIND_MIN_POSITION', async () => {
|
||||
const queryType = RecordPositionQueryType.FIND_MIN_POSITION;
|
||||
const [query, params] = buildRecordPositionQuery(
|
||||
{ recordPositionQueryType: queryType },
|
||||
objectMetadataItem,
|
||||
dataSourceSchema,
|
||||
);
|
||||
|
||||
expect(query).toEqual(
|
||||
`SELECT MIN(position) as position FROM ${dataSourceSchema}."${objectMetadataItem.nameSingular}"`,
|
||||
);
|
||||
expect(params).toEqual([]);
|
||||
});
|
||||
|
||||
it('should return query and params for FIND_MAX_POSITION', async () => {
|
||||
const queryType = RecordPositionQueryType.FIND_MAX_POSITION;
|
||||
const [query, params] = buildRecordPositionQuery(
|
||||
{ recordPositionQueryType: queryType },
|
||||
objectMetadataItem,
|
||||
dataSourceSchema,
|
||||
);
|
||||
|
||||
expect(query).toEqual(
|
||||
`SELECT MAX(position) as position FROM ${dataSourceSchema}."${objectMetadataItem.nameSingular}"`,
|
||||
);
|
||||
expect(params).toEqual([]);
|
||||
});
|
||||
|
||||
it('should return query and params for UPDATE_POSITION', async () => {
|
||||
const positionValue = 1;
|
||||
const recordId = '1';
|
||||
const queryType = RecordPositionQueryType.UPDATE_POSITION;
|
||||
const [query, params] = buildRecordPositionQuery(
|
||||
{ positionValue, recordId, recordPositionQueryType: queryType },
|
||||
objectMetadataItem,
|
||||
dataSourceSchema,
|
||||
);
|
||||
|
||||
expect(query).toEqual(
|
||||
`UPDATE ${dataSourceSchema}."${objectMetadataItem.nameSingular}"
|
||||
SET "position" = $1
|
||||
WHERE "id" = $2`,
|
||||
);
|
||||
expect(params).toEqual([positionValue, recordId]);
|
||||
});
|
||||
});
|
||||
@ -1,91 +0,0 @@
|
||||
import {
|
||||
FindByPositionQueryArgs,
|
||||
RecordPositionQueryArgs,
|
||||
RecordPositionQueryType,
|
||||
UpdatePositionQueryArgs,
|
||||
} from 'src/engine/core-modules/record-position/types/record-position-query.type';
|
||||
import { computeTableName } from 'src/engine/utils/compute-table-name.util';
|
||||
|
||||
type RecordPositionQuery = string;
|
||||
|
||||
// eslint-disable-next-line @typescript-eslint/no-explicit-any
|
||||
type RecordPositionQueryParams = any[];
|
||||
|
||||
export const buildRecordPositionQuery = (
|
||||
recordPositionQueryArgs: RecordPositionQueryArgs,
|
||||
objectMetadata: { isCustom: boolean; nameSingular: string },
|
||||
dataSourceSchema: string,
|
||||
): [RecordPositionQuery, RecordPositionQueryParams] => {
|
||||
const tableName = computeTableName(
|
||||
objectMetadata.nameSingular,
|
||||
objectMetadata.isCustom,
|
||||
);
|
||||
|
||||
switch (recordPositionQueryArgs.recordPositionQueryType) {
|
||||
case RecordPositionQueryType.FIND_BY_POSITION:
|
||||
return buildFindByPositionQuery(
|
||||
recordPositionQueryArgs satisfies FindByPositionQueryArgs,
|
||||
tableName,
|
||||
dataSourceSchema,
|
||||
);
|
||||
case RecordPositionQueryType.FIND_MIN_POSITION:
|
||||
return buildFindMinPositionQuery(tableName, dataSourceSchema);
|
||||
case RecordPositionQueryType.FIND_MAX_POSITION:
|
||||
return buildFindMaxPositionQuery(tableName, dataSourceSchema);
|
||||
case RecordPositionQueryType.UPDATE_POSITION:
|
||||
return buildUpdatePositionQuery(
|
||||
recordPositionQueryArgs satisfies UpdatePositionQueryArgs,
|
||||
tableName,
|
||||
dataSourceSchema,
|
||||
);
|
||||
default:
|
||||
throw new Error('Invalid RecordPositionQueryType');
|
||||
}
|
||||
};
|
||||
|
||||
const buildFindByPositionQuery = (
|
||||
{ positionValue }: FindByPositionQueryArgs,
|
||||
name: string,
|
||||
dataSourceSchema: string,
|
||||
): [RecordPositionQuery, RecordPositionQueryParams] => {
|
||||
const positionStringParam = positionValue ? '= $1' : 'IS NULL';
|
||||
|
||||
return [
|
||||
`SELECT id, position FROM ${dataSourceSchema}."${name}"
|
||||
WHERE "position" ${positionStringParam}`,
|
||||
positionValue ? [positionValue] : [],
|
||||
];
|
||||
};
|
||||
|
||||
const buildFindMaxPositionQuery = (
|
||||
name: string,
|
||||
dataSourceSchema: string,
|
||||
): [RecordPositionQuery, RecordPositionQueryParams] => {
|
||||
return [
|
||||
`SELECT MAX(position) as position FROM ${dataSourceSchema}."${name}"`,
|
||||
[],
|
||||
];
|
||||
};
|
||||
|
||||
const buildFindMinPositionQuery = (
|
||||
name: string,
|
||||
dataSourceSchema: string,
|
||||
): [RecordPositionQuery, RecordPositionQueryParams] => {
|
||||
return [
|
||||
`SELECT MIN(position) as position FROM ${dataSourceSchema}."${name}"`,
|
||||
[],
|
||||
];
|
||||
};
|
||||
|
||||
const buildUpdatePositionQuery = (
|
||||
{ recordId, positionValue }: UpdatePositionQueryArgs,
|
||||
name: string,
|
||||
dataSourceSchema: string,
|
||||
): [RecordPositionQuery, RecordPositionQueryParams] => {
|
||||
return [
|
||||
`UPDATE ${dataSourceSchema}."${name}"
|
||||
SET "position" = $1
|
||||
WHERE "id" = $2`,
|
||||
[positionValue, recordId],
|
||||
];
|
||||
};
|
||||
Reference in New Issue
Block a user