From dc205370df1246ef4e1888d16da919d3f2420f78 Mon Sep 17 00:00:00 2001 From: Marie <51697796+ijreilly@users.noreply.github.com> Date: Mon, 2 Jun 2025 17:03:37 +0200 Subject: [PATCH] Forbid upsert of objectPermissions on system objects (#12382) Closes https://github.com/twentyhq/core-team-issues/issues/865 --- .../search/__tests__/search.service.spec.ts | 8 +- .../core-modules/search/search.module.ts | 2 +- .../search/services/search.service.ts | 54 +--- .../field-metadata-relation.service.ts | 30 +- .../object-metadata/object-metadata.module.ts | 2 + .../object-permission.module.ts | 2 + .../object-permission.service.spec.ts | 263 ++++++++++++++++++ .../object-permission.service.ts | 26 ++ .../permissions/permissions.exception.ts | 10 +- ...sion-graphql-api-exception-handler.util.ts | 1 + .../relation-metadata.service.ts | 28 +- .../workspace-cache-storage.service.ts | 33 +++ .../query-hooks/workflow-query-hook.module.ts | 2 + .../workflow-common.workspace-service.ts | 22 +- 14 files changed, 358 insertions(+), 125 deletions(-) create mode 100644 packages/twenty-server/src/engine/metadata-modules/object-permission/object-permission.service.spec.ts diff --git a/packages/twenty-server/src/engine/core-modules/search/__tests__/search.service.spec.ts b/packages/twenty-server/src/engine/core-modules/search/__tests__/search.service.spec.ts index 20c8f8eec..3ebff4baa 100644 --- a/packages/twenty-server/src/engine/core-modules/search/__tests__/search.service.spec.ts +++ b/packages/twenty-server/src/engine/core-modules/search/__tests__/search.service.spec.ts @@ -1,11 +1,11 @@ import { Test, TestingModule } from '@nestjs/testing'; -import { FileService } from 'src/engine/core-modules/file/services/file.service'; -import { mockObjectMetadataItemsWithFieldMaps } from 'src/engine/core-modules/__mocks__/mockObjectMetadataItemsWithFieldMaps'; -import { SearchService } from 'src/engine/core-modules/search/services/search.service'; import { encodeCursorData } from 'src/engine/api/graphql/graphql-query-runner/utils/cursors.util'; -import { WorkspaceCacheStorageService } from 'src/engine/workspace-cache-storage/workspace-cache-storage.service'; +import { mockObjectMetadataItemsWithFieldMaps } from 'src/engine/core-modules/__mocks__/mockObjectMetadataItemsWithFieldMaps'; +import { FileService } from 'src/engine/core-modules/file/services/file.service'; +import { SearchService } from 'src/engine/core-modules/search/services/search.service'; import { TwentyORMManager } from 'src/engine/twenty-orm/twenty-orm.manager'; +import { WorkspaceCacheStorageService } from 'src/engine/workspace-cache-storage/workspace-cache-storage.service'; describe('SearchService', () => { let service: SearchService; diff --git a/packages/twenty-server/src/engine/core-modules/search/search.module.ts b/packages/twenty-server/src/engine/core-modules/search/search.module.ts index e0d33cc94..8f14cdd26 100644 --- a/packages/twenty-server/src/engine/core-modules/search/search.module.ts +++ b/packages/twenty-server/src/engine/core-modules/search/search.module.ts @@ -6,7 +6,7 @@ import { SearchService } from 'src/engine/core-modules/search/services/search.se import { WorkspaceCacheStorageModule } from 'src/engine/workspace-cache-storage/workspace-cache-storage.module'; @Module({ - imports: [WorkspaceCacheStorageModule, FileModule], + imports: [FileModule, WorkspaceCacheStorageModule], providers: [SearchResolver, SearchService], }) export class SearchModule {} diff --git a/packages/twenty-server/src/engine/core-modules/search/services/search.service.ts b/packages/twenty-server/src/engine/core-modules/search/services/search.service.ts index e8e6ec11d..58d1869f9 100644 --- a/packages/twenty-server/src/engine/core-modules/search/services/search.service.ts +++ b/packages/twenty-server/src/engine/core-modules/search/services/search.service.ts @@ -1,10 +1,10 @@ import { Injectable } from '@nestjs/common'; import { isNonEmptyString } from '@sniptt/guards'; +import chunk from 'lodash.chunk'; import { FieldMetadataType } from 'twenty-shared/types'; import { getLogoUrlFromDomainName } from 'twenty-shared/utils'; import { Brackets, ObjectLiteral } from 'typeorm'; -import chunk from 'lodash.chunk'; import { ObjectRecord, @@ -12,38 +12,30 @@ import { } from 'src/engine/api/graphql/workspace-query-builder/interfaces/object-record.interface'; import { GraphqlQueryParser } from 'src/engine/api/graphql/graphql-query-runner/graphql-query-parsers/graphql-query.parser'; +import { + decodeCursor, + encodeCursorData, +} from 'src/engine/api/graphql/graphql-query-runner/utils/cursors.util'; import { FileService } from 'src/engine/core-modules/file/services/file.service'; import { STANDARD_OBJECTS_BY_PRIORITY_RANK } from 'src/engine/core-modules/search/constants/standard-objects-by-priority-rank'; import { ObjectRecordFilterInput } from 'src/engine/core-modules/search/dtos/object-record-filter-input'; +import { SearchArgs } from 'src/engine/core-modules/search/dtos/search-args'; +import { SearchRecordDTO } from 'src/engine/core-modules/search/dtos/search-record.dto'; +import { SearchResultConnectionDTO } from 'src/engine/core-modules/search/dtos/search-result-connection.dto'; +import { SearchResultEdgeDTO } from 'src/engine/core-modules/search/dtos/search-result-edge.dto'; import { SearchException, SearchExceptionCode, } from 'src/engine/core-modules/search/exceptions/search.exception'; import { RecordsWithObjectMetadataItem } from 'src/engine/core-modules/search/types/records-with-object-metadata-item'; +import { formatSearchTerms } from 'src/engine/core-modules/search/utils/format-search-terms'; +import { Workspace } from 'src/engine/core-modules/workspace/workspace.entity'; import { SEARCH_VECTOR_FIELD } from 'src/engine/metadata-modules/constants/search-vector-field.constants'; import { ObjectMetadataItemWithFieldMaps } from 'src/engine/metadata-modules/types/object-metadata-item-with-field-maps'; import { generateObjectMetadataMaps } from 'src/engine/metadata-modules/utils/generate-object-metadata-maps.util'; import { WorkspaceRepository } from 'src/engine/twenty-orm/repository/workspace.repository'; -import { - decodeCursor, - encodeCursorData, -} from 'src/engine/api/graphql/graphql-query-runner/utils/cursors.util'; -import { - WorkspaceMetadataVersionException, - WorkspaceMetadataVersionExceptionCode, -} from 'src/engine/metadata-modules/workspace-metadata-version/exceptions/workspace-metadata-version.exception'; -import { - WorkspaceMetadataCacheException, - WorkspaceMetadataCacheExceptionCode, -} from 'src/engine/metadata-modules/workspace-metadata-cache/exceptions/workspace-metadata-cache.exception'; -import { WorkspaceCacheStorageService } from 'src/engine/workspace-cache-storage/workspace-cache-storage.service'; -import { Workspace } from 'src/engine/core-modules/workspace/workspace.entity'; import { TwentyORMManager } from 'src/engine/twenty-orm/twenty-orm.manager'; -import { formatSearchTerms } from 'src/engine/core-modules/search/utils/format-search-terms'; -import { SearchArgs } from 'src/engine/core-modules/search/dtos/search-args'; -import { SearchResultConnectionDTO } from 'src/engine/core-modules/search/dtos/search-result-connection.dto'; -import { SearchResultEdgeDTO } from 'src/engine/core-modules/search/dtos/search-result-edge.dto'; -import { SearchRecordDTO } from 'src/engine/core-modules/search/dtos/search-record.dto'; +import { WorkspaceCacheStorageService } from 'src/engine/workspace-cache-storage/workspace-cache-storage.service'; type LastRanks = { tsRankCD: number; tsRank: number }; @@ -58,34 +50,16 @@ const OBJECT_METADATA_ITEMS_CHUNK_SIZE = 5; export class SearchService { constructor( private readonly twentyORMManager: TwentyORMManager, - private readonly workspaceCacheStorageService: WorkspaceCacheStorageService, private readonly fileService: FileService, + private readonly workspaceCacheStorageService: WorkspaceCacheStorageService, ) {} async getObjectMetadataItemWithFieldMaps(workspace: Workspace) { - const currentCacheVersion = - await this.workspaceCacheStorageService.getMetadataVersion(workspace.id); - - if (currentCacheVersion === undefined) { - throw new WorkspaceMetadataVersionException( - `Metadata version not found for workspace ${workspace.id}`, - WorkspaceMetadataVersionExceptionCode.METADATA_VERSION_NOT_FOUND, - ); - } - const objectMetadataMaps = - await this.workspaceCacheStorageService.getObjectMetadataMaps( + await this.workspaceCacheStorageService.getObjectMetadataMapsOrThrow( workspace.id, - currentCacheVersion, ); - if (!objectMetadataMaps) { - throw new WorkspaceMetadataCacheException( - `Object metadata map not found for workspace ${workspace.id} and metadata version ${currentCacheVersion}`, - WorkspaceMetadataCacheExceptionCode.OBJECT_METADATA_MAP_NOT_FOUND, - ); - } - return Object.values(objectMetadataMaps.byId); } diff --git a/packages/twenty-server/src/engine/metadata-modules/field-metadata/relation/field-metadata-relation.service.ts b/packages/twenty-server/src/engine/metadata-modules/field-metadata/relation/field-metadata-relation.service.ts index 927439c54..9b4baaece 100644 --- a/packages/twenty-server/src/engine/metadata-modules/field-metadata/relation/field-metadata-relation.service.ts +++ b/packages/twenty-server/src/engine/metadata-modules/field-metadata/relation/field-metadata-relation.service.ts @@ -1,7 +1,5 @@ import { Injectable } from '@nestjs/common'; -import { isDefined } from 'twenty-shared/utils'; - import { FieldMetadataInterface } from 'src/engine/metadata-modules/field-metadata/interfaces/field-metadata.interface'; import { FieldMetadataEntity } from 'src/engine/metadata-modules/field-metadata/field-metadata.entity'; @@ -11,14 +9,6 @@ import { } from 'src/engine/metadata-modules/field-metadata/field-metadata.exception'; import { ObjectMetadataEntity } from 'src/engine/metadata-modules/object-metadata/object-metadata.entity'; import { removeFieldMapsFromObjectMetadata } from 'src/engine/metadata-modules/utils/remove-field-maps-from-object-metadata.util'; -import { - WorkspaceMetadataCacheException, - WorkspaceMetadataCacheExceptionCode, -} from 'src/engine/metadata-modules/workspace-metadata-cache/exceptions/workspace-metadata-cache.exception'; -import { - WorkspaceMetadataVersionException, - WorkspaceMetadataVersionExceptionCode, -} from 'src/engine/metadata-modules/workspace-metadata-version/exceptions/workspace-metadata-version.exception'; import { WorkspaceCacheStorageService } from 'src/engine/workspace-cache-storage/workspace-cache-storage.service'; @Injectable() @@ -47,29 +37,11 @@ export class FieldMetadataRelationService { targetFieldMetadata: FieldMetadataEntity; }> > { - const metadataVersion = - await this.workspaceCacheStorageService.getMetadataVersion(workspaceId); - - if (!isDefined(metadataVersion)) { - throw new WorkspaceMetadataVersionException( - `Metadata version not found for workspace ${workspaceId}`, - WorkspaceMetadataVersionExceptionCode.METADATA_VERSION_NOT_FOUND, - ); - } - const objectMetadataMaps = - await this.workspaceCacheStorageService.getObjectMetadataMaps( + await this.workspaceCacheStorageService.getObjectMetadataMapsOrThrow( workspaceId, - metadataVersion, ); - if (!objectMetadataMaps) { - throw new WorkspaceMetadataCacheException( - `Object metadata map not found for workspace ${workspaceId} and metadata version ${metadataVersion}`, - WorkspaceMetadataCacheExceptionCode.OBJECT_METADATA_MAP_NOT_FOUND, - ); - } - return fieldMetadataItems.map((fieldMetadataItem) => { const { id, diff --git a/packages/twenty-server/src/engine/metadata-modules/object-metadata/object-metadata.module.ts b/packages/twenty-server/src/engine/metadata-modules/object-metadata/object-metadata.module.ts index 4c0969204..5554c3179 100644 --- a/packages/twenty-server/src/engine/metadata-modules/object-metadata/object-metadata.module.ts +++ b/packages/twenty-server/src/engine/metadata-modules/object-metadata/object-metadata.module.ts @@ -30,6 +30,7 @@ import { SearchVectorModule } from 'src/engine/metadata-modules/search-vector/se import { WorkspaceMetadataVersionModule } from 'src/engine/metadata-modules/workspace-metadata-version/workspace-metadata-version.module'; import { WorkspaceMigrationModule } from 'src/engine/metadata-modules/workspace-migration/workspace-migration.module'; import { WorkspacePermissionsCacheModule } from 'src/engine/metadata-modules/workspace-permissions-cache/workspace-permissions-cache.module'; +import { WorkspaceCacheStorageModule } from 'src/engine/workspace-cache-storage/workspace-cache-storage.module'; import { WorkspaceMigrationRunnerModule } from 'src/engine/workspace-manager/workspace-migration-runner/workspace-migration-runner.module'; import { ObjectMetadataEntity } from './object-metadata.entity'; @@ -58,6 +59,7 @@ import { UpdateObjectPayload } from './dtos/update-object.input'; IndexMetadataModule, PermissionsModule, WorkspacePermissionsCacheModule, + WorkspaceCacheStorageModule, ], services: [ ObjectMetadataService, diff --git a/packages/twenty-server/src/engine/metadata-modules/object-permission/object-permission.module.ts b/packages/twenty-server/src/engine/metadata-modules/object-permission/object-permission.module.ts index c6fd88839..d97bfe369 100644 --- a/packages/twenty-server/src/engine/metadata-modules/object-permission/object-permission.module.ts +++ b/packages/twenty-server/src/engine/metadata-modules/object-permission/object-permission.module.ts @@ -6,6 +6,7 @@ import { ObjectPermissionEntity } from 'src/engine/metadata-modules/object-permi import { ObjectPermissionService } from 'src/engine/metadata-modules/object-permission/object-permission.service'; import { RoleEntity } from 'src/engine/metadata-modules/role/role.entity'; import { WorkspacePermissionsCacheModule } from 'src/engine/metadata-modules/workspace-permissions-cache/workspace-permissions-cache.module'; +import { WorkspaceCacheStorageModule } from 'src/engine/workspace-cache-storage/workspace-cache-storage.module'; @Module({ imports: [ @@ -13,6 +14,7 @@ import { WorkspacePermissionsCacheModule } from 'src/engine/metadata-modules/wor [ObjectPermissionEntity, RoleEntity, ObjectMetadataEntity], 'metadata', ), + WorkspaceCacheStorageModule, WorkspacePermissionsCacheModule, ], providers: [ObjectPermissionService], diff --git a/packages/twenty-server/src/engine/metadata-modules/object-permission/object-permission.service.spec.ts b/packages/twenty-server/src/engine/metadata-modules/object-permission/object-permission.service.spec.ts new file mode 100644 index 000000000..58c824355 --- /dev/null +++ b/packages/twenty-server/src/engine/metadata-modules/object-permission/object-permission.service.spec.ts @@ -0,0 +1,263 @@ +import { Test, TestingModule } from '@nestjs/testing'; +import { getRepositoryToken } from '@nestjs/typeorm'; + +import { Repository } from 'typeorm'; + +import { ObjectMetadataEntity } from 'src/engine/metadata-modules/object-metadata/object-metadata.entity'; +import { + PermissionsException, + PermissionsExceptionCode, + PermissionsExceptionMessage, +} from 'src/engine/metadata-modules/permissions/permissions.exception'; +import { RoleEntity } from 'src/engine/metadata-modules/role/role.entity'; +import { ObjectMetadataItemWithFieldMaps } from 'src/engine/metadata-modules/types/object-metadata-item-with-field-maps'; +import { WorkspacePermissionsCacheService } from 'src/engine/metadata-modules/workspace-permissions-cache/workspace-permissions-cache.service'; +import { WorkspaceCacheStorageService } from 'src/engine/workspace-cache-storage/workspace-cache-storage.service'; + +import { ObjectPermissionEntity } from './object-permission.entity'; +import { ObjectPermissionService } from './object-permission.service'; + +import { UpsertObjectPermissionsInput } from './dtos/upsert-object-permissions.input'; + +describe('ObjectPermissionService', () => { + let service: ObjectPermissionService; + let objectPermissionRepository: jest.Mocked< + Repository + >; + let roleRepository: jest.Mocked>; + let workspacePermissionsCacheService: jest.Mocked; + let workspaceCacheStorageService: jest.Mocked; + + beforeEach(async () => { + const module: TestingModule = await Test.createTestingModule({ + providers: [ + ObjectPermissionService, + { + provide: getRepositoryToken(ObjectPermissionEntity, 'metadata'), + useValue: { + upsert: jest.fn(), + find: jest.fn(), + }, + }, + { + provide: getRepositoryToken(RoleEntity, 'metadata'), + useValue: { + findOne: jest.fn(), + }, + }, + { + provide: getRepositoryToken(ObjectMetadataEntity, 'metadata'), + useValue: { + find: jest.fn(), + }, + }, + { + provide: WorkspacePermissionsCacheService, + useValue: { + recomputeRolesPermissionsCache: jest.fn(), + }, + }, + { + provide: WorkspaceCacheStorageService, + useValue: { + getObjectMetadataMapsOrThrow: jest.fn(), + }, + }, + ], + }).compile(); + + service = module.get(ObjectPermissionService); + objectPermissionRepository = module.get( + getRepositoryToken(ObjectPermissionEntity, 'metadata'), + ); + roleRepository = module.get(getRepositoryToken(RoleEntity, 'metadata')); + workspacePermissionsCacheService = module.get( + WorkspacePermissionsCacheService, + ); + workspaceCacheStorageService = module.get(WorkspaceCacheStorageService); + }); + + describe('upsertObjectPermissions', () => { + const workspaceId = 'workspace-id'; + const roleId = 'role-id'; + const systemObjectMetadataId = 'system-object-id'; + const customObjectMetadataId = 'custom-object-id'; + + beforeEach(() => { + // Mock role validation + roleRepository.findOne.mockResolvedValue({ + id: roleId, + workspaceId, + isEditable: true, + } as RoleEntity); + }); + + it('should throw PermissionsException when trying to add object permission on system object', async () => { + // Arrange + const input: UpsertObjectPermissionsInput = { + roleId, + objectPermissions: [ + { + objectMetadataId: systemObjectMetadataId, + canReadObjectRecords: true, + canUpdateObjectRecords: false, + canSoftDeleteObjectRecords: false, + canDestroyObjectRecords: false, + }, + ], + }; + + // Mock object metadata maps with a system object + workspaceCacheStorageService.getObjectMetadataMapsOrThrow.mockResolvedValue( + { + byId: { + [systemObjectMetadataId]: { + id: systemObjectMetadataId, + isSystem: true, + workspaceId, + } as ObjectMetadataItemWithFieldMaps, + }, + idByNameSingular: {}, + }, + ); + + // Act & Assert + await expect( + service.upsertObjectPermissions({ + workspaceId, + input, + }), + ).rejects.toThrow( + new PermissionsException( + PermissionsExceptionMessage.CANNOT_ADD_OBJECT_PERMISSION_ON_SYSTEM_OBJECT, + PermissionsExceptionCode.CANNOT_ADD_OBJECT_PERMISSION_ON_SYSTEM_OBJECT, + ), + ); + + // Verify that upsert was never called + expect(objectPermissionRepository.upsert).not.toHaveBeenCalled(); + expect( + workspacePermissionsCacheService.recomputeRolesPermissionsCache, + ).not.toHaveBeenCalled(); + }); + + it('should successfully create object permission for custom (non-system) object', async () => { + // Arrange + const input: UpsertObjectPermissionsInput = { + roleId, + objectPermissions: [ + { + objectMetadataId: customObjectMetadataId, + canReadObjectRecords: true, + canUpdateObjectRecords: true, + canSoftDeleteObjectRecords: false, + canDestroyObjectRecords: false, + }, + ], + }; + + // Mock object metadata maps with a custom object + workspaceCacheStorageService.getObjectMetadataMapsOrThrow.mockResolvedValue( + { + byId: { + [customObjectMetadataId]: { + id: customObjectMetadataId, + isSystem: false, + workspaceId, + } as ObjectMetadataItemWithFieldMaps, + }, + idByNameSingular: {}, + }, + ); + + // Mock successful upsert + const mockObjectPermission = { + id: 'permission-id', + roleId, + objectMetadataId: customObjectMetadataId, + workspaceId, + canReadObjectRecords: true, + canUpdateObjectRecords: true, + canSoftDeleteObjectRecords: false, + canDestroyObjectRecords: false, + } as ObjectPermissionEntity; + + objectPermissionRepository.upsert.mockResolvedValue({ + generatedMaps: [{ id: 'permission-id' }], + identifiers: [], + raw: [], + }); + + objectPermissionRepository.find.mockResolvedValue([mockObjectPermission]); + + // Act + const result = await service.upsertObjectPermissions({ + workspaceId, + input, + }); + + // Assert + expect(result).toEqual([mockObjectPermission]); + expect(objectPermissionRepository.upsert).toHaveBeenCalledWith( + [ + { + objectMetadataId: customObjectMetadataId, + canReadObjectRecords: true, + canUpdateObjectRecords: true, + canSoftDeleteObjectRecords: false, + canDestroyObjectRecords: false, + roleId, + workspaceId, + }, + ], + { + conflictPaths: ['objectMetadataId', 'roleId'], + }, + ); + expect( + workspacePermissionsCacheService.recomputeRolesPermissionsCache, + ).toHaveBeenCalledWith({ + workspaceId, + roleIds: [roleId], + ignoreLock: true, + }); + }); + + it('should throw PermissionsException when object metadata is not found', async () => { + // Arrange + const input: UpsertObjectPermissionsInput = { + roleId, + objectPermissions: [ + { + objectMetadataId: 'non-existent-object-id', + canReadObjectRecords: true, + canUpdateObjectRecords: false, + canSoftDeleteObjectRecords: false, + canDestroyObjectRecords: false, + }, + ], + }; + + // Mock empty object metadata maps + workspaceCacheStorageService.getObjectMetadataMapsOrThrow.mockResolvedValue( + { + byId: {}, + idByNameSingular: {}, + }, + ); + + // Act & Assert + await expect( + service.upsertObjectPermissions({ + workspaceId, + input, + }), + ).rejects.toThrow( + new PermissionsException( + 'Object metadata id not found', + PermissionsExceptionCode.OBJECT_METADATA_NOT_FOUND, + ), + ); + }); + }); +}); diff --git a/packages/twenty-server/src/engine/metadata-modules/object-permission/object-permission.service.ts b/packages/twenty-server/src/engine/metadata-modules/object-permission/object-permission.service.ts index e29cbe310..a370b94c5 100644 --- a/packages/twenty-server/src/engine/metadata-modules/object-permission/object-permission.service.ts +++ b/packages/twenty-server/src/engine/metadata-modules/object-permission/object-permission.service.ts @@ -13,6 +13,7 @@ import { } from 'src/engine/metadata-modules/permissions/permissions.exception'; import { RoleEntity } from 'src/engine/metadata-modules/role/role.entity'; import { WorkspacePermissionsCacheService } from 'src/engine/metadata-modules/workspace-permissions-cache/workspace-permissions-cache.service'; +import { WorkspaceCacheStorageService } from 'src/engine/workspace-cache-storage/workspace-cache-storage.service'; export class ObjectPermissionService { constructor( @@ -23,6 +24,7 @@ export class ObjectPermissionService { @InjectRepository(ObjectMetadataEntity, 'metadata') private readonly objectMetadataRepository: Repository, private readonly workspacePermissionsCacheService: WorkspacePermissionsCacheService, + private readonly workspaceCacheStorageService: WorkspaceCacheStorageService, ) {} public async upsertObjectPermissions({ @@ -38,6 +40,30 @@ export class ObjectPermissionService { workspaceId, }); + const { byId: objectMetadataMapsById } = + await this.workspaceCacheStorageService.getObjectMetadataMapsOrThrow( + workspaceId, + ); + + input.objectPermissions.forEach((objectPermission) => { + const objectMetadataForObjectPermission = + objectMetadataMapsById[objectPermission.objectMetadataId]; + + if (!isDefined(objectMetadataForObjectPermission)) { + throw new PermissionsException( + 'Object metadata id not found', + PermissionsExceptionCode.OBJECT_METADATA_NOT_FOUND, + ); + } + + if (objectMetadataForObjectPermission.isSystem === true) { + throw new PermissionsException( + PermissionsExceptionMessage.CANNOT_ADD_OBJECT_PERMISSION_ON_SYSTEM_OBJECT, + PermissionsExceptionCode.CANNOT_ADD_OBJECT_PERMISSION_ON_SYSTEM_OBJECT, + ); + } + }); + const objectPermissions = input.objectPermissions.map( (objectPermission) => ({ ...objectPermission, diff --git a/packages/twenty-server/src/engine/metadata-modules/permissions/permissions.exception.ts b/packages/twenty-server/src/engine/metadata-modules/permissions/permissions.exception.ts index c3d6224ef..b9444281d 100644 --- a/packages/twenty-server/src/engine/metadata-modules/permissions/permissions.exception.ts +++ b/packages/twenty-server/src/engine/metadata-modules/permissions/permissions.exception.ts @@ -18,19 +18,20 @@ export enum PermissionsExceptionCode { ROLE_NOT_FOUND = 'ROLE_NOT_FOUND', CANNOT_UNASSIGN_LAST_ADMIN = 'CANNOT_UNASSIGN_LAST_ADMIN', CANNOT_DELETE_LAST_ADMIN_USER = 'CANNOT_DELETE_LAST_ADMIN_USER', - UNKNOWN_OPERATION_NAME = 'UNKNOWN_OPERATION_NAME', + UNKNOWN_OPERATION_NAME = 'UNKNOWN_OPERATION_NAME_PERMISSIONS', UNKNOWN_REQUIRED_PERMISSION = 'UNKNOWN_REQUIRED_PERMISSION', CANNOT_UPDATE_SELF_ROLE = 'CANNOT_UPDATE_SELF_ROLE', NO_ROLE_FOUND_FOR_USER_WORKSPACE = 'NO_ROLE_FOUND_FOR_USER_WORKSPACE', - INVALID_ARG = 'INVALID_ARG', + INVALID_ARG = 'INVALID_ARG_PERMISSIONS', PERMISSIONS_V2_NOT_ENABLED = 'PERMISSIONS_V2_NOT_ENABLED', ROLE_LABEL_ALREADY_EXISTS = 'ROLE_LABEL_ALREADY_EXISTS', DEFAULT_ROLE_NOT_FOUND = 'DEFAULT_ROLE_NOT_FOUND', - OBJECT_METADATA_NOT_FOUND = 'OBJECT_METADATA_NOT_FOUND', - INVALID_SETTING = 'INVALID_SETTING', + OBJECT_METADATA_NOT_FOUND = 'OBJECT_METADATA_NOT_FOUND_PERMISSIONS', + INVALID_SETTING = 'INVALID_SETTING_PERMISSIONS', ROLE_NOT_EDITABLE = 'ROLE_NOT_EDITABLE', DEFAULT_ROLE_CANNOT_BE_DELETED = 'DEFAULT_ROLE_CANNOT_BE_DELETED', NO_PERMISSIONS_FOUND_IN_DATASOURCE = 'NO_PERMISSIONS_FOUND_IN_DATASOURCE', + CANNOT_ADD_OBJECT_PERMISSION_ON_SYSTEM_OBJECT = 'CANNOT_ADD_OBJECT_PERMISSION_ON_SYSTEM_OBJECT', METHOD_NOT_ALLOWED = 'METHOD_NOT_ALLOWED', RAW_SQL_NOT_ALLOWED = 'RAW_SQL_NOT_ALLOWED', } @@ -58,4 +59,5 @@ export enum PermissionsExceptionMessage { ROLE_NOT_EDITABLE = 'Role is not editable', DEFAULT_ROLE_CANNOT_BE_DELETED = 'Default role cannot be deleted', NO_PERMISSIONS_FOUND_IN_DATASOURCE = 'No permissions found in datasource', + CANNOT_ADD_OBJECT_PERMISSION_ON_SYSTEM_OBJECT = 'Cannot add object permission on system object', } diff --git a/packages/twenty-server/src/engine/metadata-modules/permissions/utils/permission-graphql-api-exception-handler.util.ts b/packages/twenty-server/src/engine/metadata-modules/permissions/utils/permission-graphql-api-exception-handler.util.ts index 0c0ad41fa..c3bf608c9 100644 --- a/packages/twenty-server/src/engine/metadata-modules/permissions/utils/permission-graphql-api-exception-handler.util.ts +++ b/packages/twenty-server/src/engine/metadata-modules/permissions/utils/permission-graphql-api-exception-handler.util.ts @@ -19,6 +19,7 @@ export const permissionGraphqlApiExceptionHandler = ( case PermissionsExceptionCode.PERMISSIONS_V2_NOT_ENABLED: case PermissionsExceptionCode.ROLE_LABEL_ALREADY_EXISTS: case PermissionsExceptionCode.ROLE_NOT_EDITABLE: + case PermissionsExceptionCode.CANNOT_ADD_OBJECT_PERMISSION_ON_SYSTEM_OBJECT: throw new ForbiddenError(error.message); case PermissionsExceptionCode.INVALID_ARG: case PermissionsExceptionCode.INVALID_SETTING: diff --git a/packages/twenty-server/src/engine/metadata-modules/relation-metadata/relation-metadata.service.ts b/packages/twenty-server/src/engine/metadata-modules/relation-metadata/relation-metadata.service.ts index 49e05d44e..53efee924 100644 --- a/packages/twenty-server/src/engine/metadata-modules/relation-metadata/relation-metadata.service.ts +++ b/packages/twenty-server/src/engine/metadata-modules/relation-metadata/relation-metadata.service.ts @@ -25,14 +25,6 @@ import { import { InvalidMetadataException } from 'src/engine/metadata-modules/utils/exceptions/invalid-metadata.exception'; import { validateFieldNameAvailabilityOrThrow } from 'src/engine/metadata-modules/utils/validate-field-name-availability.utils'; import { validateMetadataNameOrThrow } from 'src/engine/metadata-modules/utils/validate-metadata-name.utils'; -import { - WorkspaceMetadataCacheException, - WorkspaceMetadataCacheExceptionCode, -} from 'src/engine/metadata-modules/workspace-metadata-cache/exceptions/workspace-metadata-cache.exception'; -import { - WorkspaceMetadataVersionException, - WorkspaceMetadataVersionExceptionCode, -} from 'src/engine/metadata-modules/workspace-metadata-version/exceptions/workspace-metadata-version.exception'; import { WorkspaceMetadataVersionService } from 'src/engine/metadata-modules/workspace-metadata-version/services/workspace-metadata-version.service'; import { generateMigrationName } from 'src/engine/metadata-modules/workspace-migration/utils/generate-migration-name.util'; import { @@ -501,29 +493,11 @@ export class RelationMetadataService extends TypeOrmQueryService, workspaceId: string, ): Promise<(RelationMetadataEntity | NotFoundException)[]> { - const metadataVersion = - await this.workspaceCacheStorageService.getMetadataVersion(workspaceId); - - if (!isDefined(metadataVersion)) { - throw new WorkspaceMetadataVersionException( - `Metadata version not found for workspace ${workspaceId}`, - WorkspaceMetadataVersionExceptionCode.METADATA_VERSION_NOT_FOUND, - ); - } - const objectMetadataMaps = - await this.workspaceCacheStorageService.getObjectMetadataMaps( + await this.workspaceCacheStorageService.getObjectMetadataMapsOrThrow( workspaceId, - metadataVersion, ); - if (!objectMetadataMaps) { - throw new WorkspaceMetadataCacheException( - `Object metadata map not found for workspace ${workspaceId} and metadata version ${metadataVersion}`, - WorkspaceMetadataCacheExceptionCode.OBJECT_METADATA_MAP_NOT_FOUND, - ); - } - const mappedResult = fieldMetadataItems.map((fieldMetadataItem) => { const objectMetadata = objectMetadataMaps.byId[fieldMetadataItem.objectMetadataId]; diff --git a/packages/twenty-server/src/engine/workspace-cache-storage/workspace-cache-storage.service.ts b/packages/twenty-server/src/engine/workspace-cache-storage/workspace-cache-storage.service.ts index 1db7ad0c1..845ab3408 100644 --- a/packages/twenty-server/src/engine/workspace-cache-storage/workspace-cache-storage.service.ts +++ b/packages/twenty-server/src/engine/workspace-cache-storage/workspace-cache-storage.service.ts @@ -11,6 +11,14 @@ import { InjectCacheStorage } from 'src/engine/core-modules/cache-storage/decora import { CacheStorageService } from 'src/engine/core-modules/cache-storage/services/cache-storage.service'; import { CacheStorageNamespace } from 'src/engine/core-modules/cache-storage/types/cache-storage-namespace.enum'; import { ObjectMetadataMaps } from 'src/engine/metadata-modules/types/object-metadata-maps'; +import { + WorkspaceMetadataCacheException, + WorkspaceMetadataCacheExceptionCode, +} from 'src/engine/metadata-modules/workspace-metadata-cache/exceptions/workspace-metadata-cache.exception'; +import { + WorkspaceMetadataVersionException, + WorkspaceMetadataVersionExceptionCode, +} from 'src/engine/metadata-modules/workspace-metadata-version/exceptions/workspace-metadata-version.exception'; export enum WorkspaceCacheKeys { GraphQLTypeDefs = 'graphql:type-defs', @@ -133,6 +141,31 @@ export class WorkspaceCacheStorageService { ); } + async getObjectMetadataMapsOrThrow(workspaceId: string) { + const currentCacheVersion = await this.getMetadataVersion(workspaceId); + + if (currentCacheVersion === undefined) { + throw new WorkspaceMetadataVersionException( + `Metadata version not found for workspace ${workspaceId}`, + WorkspaceMetadataVersionExceptionCode.METADATA_VERSION_NOT_FOUND, + ); + } + + const objectMetadataMaps = await this.getObjectMetadataMaps( + workspaceId, + currentCacheVersion, + ); + + if (!objectMetadataMaps) { + throw new WorkspaceMetadataCacheException( + `Object metadata map not found for workspace ${workspaceId} and metadata version ${currentCacheVersion}`, + WorkspaceMetadataCacheExceptionCode.OBJECT_METADATA_MAP_NOT_FOUND, + ); + } + + return objectMetadataMaps; + } + setGraphQLTypeDefs( workspaceId: string, metadataVersion: number, diff --git a/packages/twenty-server/src/modules/workflow/common/query-hooks/workflow-query-hook.module.ts b/packages/twenty-server/src/modules/workflow/common/query-hooks/workflow-query-hook.module.ts index 338acabff..91a1f4690 100644 --- a/packages/twenty-server/src/modules/workflow/common/query-hooks/workflow-query-hook.module.ts +++ b/packages/twenty-server/src/modules/workflow/common/query-hooks/workflow-query-hook.module.ts @@ -4,6 +4,7 @@ import { NestjsQueryTypeOrmModule } from '@ptc-org/nestjs-query-typeorm'; import { RecordPositionModule } from 'src/engine/core-modules/record-position/record-position.module'; import { ObjectMetadataEntity } from 'src/engine/metadata-modules/object-metadata/object-metadata.entity'; +import { ObjectMetadataModule } from 'src/engine/metadata-modules/object-metadata/object-metadata.module'; import { ServerlessFunctionModule } from 'src/engine/metadata-modules/serverless-function/serverless-function.module'; import { WorkspaceCacheStorageModule } from 'src/engine/workspace-cache-storage/workspace-cache-storage.module'; import { WorkflowCreateManyPostQueryHook } from 'src/modules/workflow/common/query-hooks/workflow-create-many.post-query.hook'; @@ -35,6 +36,7 @@ import { WorkflowVersionValidationWorkspaceService } from 'src/modules/workflow/ ServerlessFunctionModule, RecordPositionModule, WorkspaceCacheStorageModule, + ObjectMetadataModule, ], providers: [ WorkflowCreateOnePreQueryHook, diff --git a/packages/twenty-server/src/modules/workflow/common/workspace-services/workflow-common.workspace-service.ts b/packages/twenty-server/src/modules/workflow/common/workspace-services/workflow-common.workspace-service.ts index 3ef0db64b..67ddab366 100644 --- a/packages/twenty-server/src/modules/workflow/common/workspace-services/workflow-common.workspace-service.ts +++ b/packages/twenty-server/src/modules/workflow/common/workspace-services/workflow-common.workspace-service.ts @@ -11,6 +11,7 @@ import { WorkflowCommonException, WorkflowCommonExceptionCode, } from 'src/modules/workflow/common/exceptions/workflow-common.exception'; +import { WorkflowAutomatedTriggerWorkspaceEntity } from 'src/modules/workflow/common/standard-objects/workflow-automated-trigger.workspace-entity'; import { WorkflowRunWorkspaceEntity } from 'src/modules/workflow/common/standard-objects/workflow-run.workspace-entity'; import { WorkflowVersionWorkspaceEntity } from 'src/modules/workflow/common/standard-objects/workflow-version.workspace-entity'; import { WorkflowActionType } from 'src/modules/workflow/workflow-executor/workflow-actions/types/workflow-action.type'; @@ -18,7 +19,6 @@ import { WorkflowTriggerException, WorkflowTriggerExceptionCode, } from 'src/modules/workflow/workflow-trigger/exceptions/workflow-trigger.exception'; -import { WorkflowAutomatedTriggerWorkspaceEntity } from 'src/modules/workflow/common/standard-objects/workflow-automated-trigger.workspace-entity'; export type ObjectMetadataInfo = { objectMetadataItemWithFieldsMaps: ObjectMetadataItemWithFieldMaps; @@ -81,29 +81,11 @@ export class WorkflowCommonWorkspaceService { async getObjectMetadataMaps( workspaceId: string, ): Promise { - const currentCacheVersion = - await this.workspaceCacheStorageService.getMetadataVersion(workspaceId); - - if (currentCacheVersion === undefined) { - throw new WorkflowCommonException( - 'Failed to read: Metadata cache version not found', - WorkflowCommonExceptionCode.INVALID_CACHE_VERSION, - ); - } - const objectMetadataMaps = - await this.workspaceCacheStorageService.getObjectMetadataMaps( + await this.workspaceCacheStorageService.getObjectMetadataMapsOrThrow( workspaceId, - currentCacheVersion, ); - if (!objectMetadataMaps) { - throw new WorkflowCommonException( - 'Failed to read: Object metadata collection not found', - WorkflowCommonExceptionCode.OBJECT_METADATA_NOT_FOUND, - ); - } - return objectMetadataMaps; }