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/__tests__/object-permission.service.spec.ts similarity index 95% rename from packages/twenty-server/src/engine/metadata-modules/object-permission/object-permission.service.spec.ts rename to packages/twenty-server/src/engine/metadata-modules/object-permission/__tests__/object-permission.service.spec.ts index fff2ce851..db67879b4 100644 --- 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/__tests__/object-permission.service.spec.ts @@ -4,6 +4,9 @@ import { getRepositoryToken } from '@nestjs/typeorm'; import { Repository } from 'typeorm'; import { ObjectMetadataEntity } from 'src/engine/metadata-modules/object-metadata/object-metadata.entity'; +import { UpsertObjectPermissionsInput } from 'src/engine/metadata-modules/object-permission/dtos/upsert-object-permissions.input'; +import { ObjectPermissionEntity } from 'src/engine/metadata-modules/object-permission/object-permission.entity'; +import { ObjectPermissionService } from 'src/engine/metadata-modules/object-permission/object-permission.service'; import { PermissionsException, PermissionsExceptionCode, @@ -14,11 +17,6 @@ import { ObjectMetadataItemWithFieldMaps } from 'src/engine/metadata-modules/typ 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< @@ -89,7 +87,8 @@ describe('ObjectPermissionService', () => { id: roleId, workspaceId, isEditable: true, - } as RoleEntity); + objectPermissions: [], + } as unknown as RoleEntity); }); it('should throw PermissionsException when trying to add object permission on system object', async () => { 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 313c90398..ff8a15804 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 @@ -4,7 +4,10 @@ import { isDefined } from 'twenty-shared/utils'; import { In, Repository } from 'typeorm'; import { ObjectMetadataEntity } from 'src/engine/metadata-modules/object-metadata/object-metadata.entity'; -import { UpsertObjectPermissionsInput } from 'src/engine/metadata-modules/object-permission/dtos/upsert-object-permissions.input'; +import { + ObjectPermissionInput, + UpsertObjectPermissionsInput, +} from 'src/engine/metadata-modules/object-permission/dtos/upsert-object-permissions.input'; import { ObjectPermissionEntity } from 'src/engine/metadata-modules/object-permission/object-permission.entity'; import { PermissionsException, @@ -35,11 +38,20 @@ export class ObjectPermissionService { input: UpsertObjectPermissionsInput; }): Promise { try { - await this.validateRoleIsEditableOrThrow({ + const role = await this.getRoleOrThrow({ roleId: input.roleId, workspaceId, }); + await this.validateRoleIsEditableOrThrow({ + role, + }); + + await this.validateObjectPermissionsReadAndWriteConsistencyOrThrow({ + objectPermissions: input.objectPermissions, + roleWithObjectPermissions: role, + }); + const { byId: objectMetadataMapsById } = await this.workspaceCacheStorageService.getObjectMetadataMapsOrThrow( workspaceId, @@ -117,6 +129,58 @@ export class ObjectPermissionService { } } + private async validateObjectPermissionsReadAndWriteConsistencyOrThrow({ + objectPermissions: newObjectPermissions, + roleWithObjectPermissions, + }: { + objectPermissions: ObjectPermissionInput[]; + roleWithObjectPermissions: RoleEntity; + }) { + const existingObjectPermissions = + roleWithObjectPermissions.objectPermissions; + + for (const newObjectPermission of newObjectPermissions) { + const existingObjectRecordPermission = existingObjectPermissions.find( + (objectPermission) => + objectPermission.objectMetadataId === + newObjectPermission.objectMetadataId, + ); + + const hasReadPermissionAfterUpdate = + newObjectPermission.canReadObjectRecords ?? + existingObjectRecordPermission?.canReadObjectRecords ?? + roleWithObjectPermissions.canReadAllObjectRecords; + + if (hasReadPermissionAfterUpdate === false) { + const hasUpdatePermissionAfterUpdate = + newObjectPermission.canUpdateObjectRecords ?? + existingObjectRecordPermission?.canUpdateObjectRecords ?? + roleWithObjectPermissions.canUpdateAllObjectRecords; + + const hasSoftDeletePermissionAfterUpdate = + newObjectPermission.canSoftDeleteObjectRecords ?? + existingObjectRecordPermission?.canSoftDeleteObjectRecords ?? + roleWithObjectPermissions.canSoftDeleteAllObjectRecords; + + const hasDestroyPermissionAfterUpdate = + newObjectPermission.canDestroyObjectRecords ?? + existingObjectRecordPermission?.canDestroyObjectRecords ?? + roleWithObjectPermissions.canDestroyAllObjectRecords; + + if ( + hasUpdatePermissionAfterUpdate || + hasSoftDeletePermissionAfterUpdate || + hasDestroyPermissionAfterUpdate + ) { + throw new PermissionsException( + PermissionsExceptionMessage.CANNOT_GIVE_WRITING_PERMISSION_ON_NON_READABLE_OBJECT, + PermissionsExceptionCode.CANNOT_GIVE_WRITING_PERMISSION_ON_NON_READABLE_OBJECT, + ); + } + } + } + } + private async handleForeignKeyError({ error, roleId, @@ -159,7 +223,7 @@ export class ObjectPermissionService { } } - private async validateRoleIsEditableOrThrow({ + private async getRoleOrThrow({ roleId, workspaceId, }: { @@ -171,9 +235,21 @@ export class ObjectPermissionService { id: roleId, workspaceId, }, + relations: ['objectPermissions'], }); - if (!role?.isEditable) { + if (!isDefined(role)) { + throw new PermissionsException( + PermissionsExceptionMessage.ROLE_NOT_FOUND, + PermissionsExceptionCode.ROLE_NOT_FOUND, + ); + } + + return role; + } + + private async validateRoleIsEditableOrThrow({ role }: { role: RoleEntity }) { + if (!role.isEditable) { throw new PermissionsException( PermissionsExceptionMessage.ROLE_NOT_EDITABLE, PermissionsExceptionCode.ROLE_NOT_EDITABLE, 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 b9444281d..c9289e77c 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 @@ -34,6 +34,8 @@ export enum PermissionsExceptionCode { 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', + CANNOT_GIVE_WRITING_PERMISSION_ON_NON_READABLE_OBJECT = 'CANNOT_GIVE_WRITING_PERMISSION_ON_NON_READABLE_OBJECT', + CANNOT_GIVE_WRITING_PERMISSION_WITHOUT_READING_PERMISSION = 'CANNOT_GIVE_WRITING_PERMISSION_WITHOUT_READING_PERMISSION', } export enum PermissionsExceptionMessage { @@ -60,4 +62,6 @@ export enum PermissionsExceptionMessage { 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', + CANNOT_GIVE_WRITING_PERMISSION_ON_NON_READABLE_OBJECT = 'Cannot give update permission to non-readable object', + CANNOT_GIVE_WRITING_PERMISSION_WITHOUT_READING_PERMISSION = 'Cannot give writing permission without reading permission', } 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 c3bf608c9..109403943 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 @@ -23,6 +23,8 @@ export const permissionGraphqlApiExceptionHandler = ( throw new ForbiddenError(error.message); case PermissionsExceptionCode.INVALID_ARG: case PermissionsExceptionCode.INVALID_SETTING: + case PermissionsExceptionCode.CANNOT_GIVE_WRITING_PERMISSION_ON_NON_READABLE_OBJECT: + case PermissionsExceptionCode.CANNOT_GIVE_WRITING_PERMISSION_WITHOUT_READING_PERMISSION: throw new UserInputError(error.message); case PermissionsExceptionCode.ROLE_NOT_FOUND: case PermissionsExceptionCode.USER_WORKSPACE_NOT_FOUND: diff --git a/packages/twenty-server/src/engine/metadata-modules/role/role.service.ts b/packages/twenty-server/src/engine/metadata-modules/role/role.service.ts index 049da8943..80b269602 100644 --- a/packages/twenty-server/src/engine/metadata-modules/role/role.service.ts +++ b/packages/twenty-server/src/engine/metadata-modules/role/role.service.ts @@ -67,7 +67,7 @@ export class RoleService { input: CreateRoleInput; workspaceId: string; }): Promise { - await this.validateRoleInput({ input, workspaceId }); + await this.validateRoleInputOrThrow({ input, workspaceId }); const role = await this.roleRepository.save({ label: input.label, @@ -117,7 +117,7 @@ export class RoleService { ); } - await this.validateRoleInput({ + await this.validateRoleInputOrThrow({ input: input.update, workspaceId, roleId: input.id, @@ -243,7 +243,7 @@ export class RoleService { }); } - private async validateRoleInput({ + private async validateRoleInputOrThrow({ input, workspaceId, roleId, @@ -277,33 +277,64 @@ export class RoleService { } } + const workspaceRoles = await this.getWorkspaceRoles(workspaceId); + if (isDefined(input.label)) { - let workspaceRoles = await this.getWorkspaceRoles(workspaceId); + let rolesForLabelComparison = workspaceRoles; if (isDefined(roleId)) { - workspaceRoles = workspaceRoles.filter((role) => role.id !== roleId); + rolesForLabelComparison = workspaceRoles.filter( + (role) => role.id !== roleId, + ); } - if (workspaceRoles.some((role) => role.label === input.label)) { + if (rolesForLabelComparison.some((role) => role.label === input.label)) { throw new PermissionsException( PermissionsExceptionMessage.ROLE_LABEL_ALREADY_EXISTS, PermissionsExceptionCode.ROLE_LABEL_ALREADY_EXISTS, ); } } + + const existingRole = workspaceRoles.find((role) => role.id === roleId); + + await this.validateRoleReadAndWirtePermissionsConsistencyOrThrow({ + input, + existingRole, + }); } - private async validateRoleIsNotDefaultRoleOrThrow({ - roleId, - defaultRoleId, + private async validateRoleReadAndWirtePermissionsConsistencyOrThrow({ + input, + existingRole, }: { - roleId: string; - defaultRoleId: string; - }): Promise { - if (defaultRoleId === roleId) { + input: CreateRoleInput | UpdateRolePayload; + existingRole?: RoleEntity; + }) { + const hasReadingPermissionsAfterUpdate = + input.canReadAllObjectRecords ?? existingRole?.canReadAllObjectRecords; + + const hasUpdatePermissionsAfterUpdate = + input.canUpdateAllObjectRecords ?? + existingRole?.canUpdateAllObjectRecords; + + const hasSoftDeletePermissionsAfterUpdate = + input.canSoftDeleteAllObjectRecords ?? + existingRole?.canSoftDeleteAllObjectRecords; + + const hasDestroyPermissionsAfterUpdate = + input.canDestroyAllObjectRecords ?? + existingRole?.canDestroyAllObjectRecords; + + if ( + hasReadingPermissionsAfterUpdate === false && + (hasUpdatePermissionsAfterUpdate || + hasSoftDeletePermissionsAfterUpdate || + hasDestroyPermissionsAfterUpdate) + ) { throw new PermissionsException( - PermissionsExceptionMessage.DEFAULT_ROLE_CANNOT_BE_DELETED, - PermissionsExceptionCode.DEFAULT_ROLE_CANNOT_BE_DELETED, + PermissionsExceptionMessage.CANNOT_GIVE_WRITING_PERMISSION_WITHOUT_READING_PERMISSION, + PermissionsExceptionCode.CANNOT_GIVE_WRITING_PERMISSION_WITHOUT_READING_PERMISSION, ); } } @@ -362,4 +393,19 @@ export class RoleService { ); } } + + private async validateRoleIsNotDefaultRoleOrThrow({ + roleId, + defaultRoleId, + }: { + roleId: string; + defaultRoleId: string; + }): Promise { + if (defaultRoleId === roleId) { + throw new PermissionsException( + PermissionsExceptionMessage.DEFAULT_ROLE_CANNOT_BE_DELETED, + PermissionsExceptionCode.DEFAULT_ROLE_CANNOT_BE_DELETED, + ); + } + } } 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 93a776143..eb07916b8 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 @@ -965,7 +965,7 @@ export class WorkspaceEntityManager extends EntityManager { .execute(); } - override findByIds( + override async findByIds( entityClass: EntityTarget, ids: string[], permissionOptions?: PermissionOptions, @@ -1033,7 +1033,10 @@ export class WorkspaceEntityManager extends EntityManager { permissionOptions?: PermissionOptions, ): Promise; - override save>( + override async save< + Entity extends ObjectLiteral, + T extends DeepPartial, + >( targetOrEntity: EntityTarget | Entity | Entity[], entityOrMaybeOptions: | T @@ -1117,7 +1120,7 @@ export class WorkspaceEntityManager extends EntityManager { permissionOptions?: PermissionOptions, ): Promise; - override remove( + override async remove( targetOrEntity: EntityTarget | Entity[] | Entity, entityOrMaybeOptions: Entity | Entity[] | RemoveOptions, maybeOptionsOrMaybePermissionOptions?: RemoveOptions | PermissionOptions, @@ -1279,7 +1282,10 @@ export class WorkspaceEntityManager extends EntityManager { permissionOptions?: PermissionOptions, ): Promise; - override recover>( + override async recover< + Entity extends ObjectLiteral, + T extends DeepPartial, + >( targetOrEntityOrEntities: EntityTarget | Entity | Entity[], entityOrEntitiesOrMaybeOptions: T | T[] | SaveOptions, maybeOptionsOrMaybePermissionOptions?: SaveOptions | PermissionOptions, diff --git a/packages/twenty-server/test/integration/graphql/suites/role/object-permissions.integration-spec.ts b/packages/twenty-server/test/integration/graphql/suites/role/object-permissions.integration-spec.ts new file mode 100644 index 000000000..9754d1a4d --- /dev/null +++ b/packages/twenty-server/test/integration/graphql/suites/role/object-permissions.integration-spec.ts @@ -0,0 +1,421 @@ +import gql from 'graphql-tag'; +import { default as request } from 'supertest'; +import { createRoleOperation } from 'test/integration/graphql/utils/create-custom-role-operation-factory.util'; +import { deleteRole } from 'test/integration/graphql/utils/delete-one-role.util'; +import { makeGraphqlAPIRequest } from 'test/integration/graphql/utils/make-graphql-api-request.util'; +import { updateFeatureFlagFactory } from 'test/integration/graphql/utils/update-feature-flag-factory.util'; +import { createUpsertObjectPermissionsOperation } from 'test/integration/graphql/utils/upsert-object-permission-operation-factory.util'; +import { makeMetadataAPIRequest } from 'test/integration/metadata/suites/utils/make-metadata-api-request.util'; + +import { ErrorCode } from 'src/engine/core-modules/graphql/utils/graphql-errors.util'; +import { PermissionsExceptionMessage } from 'src/engine/metadata-modules/permissions/permissions.exception'; +import { SEED_APPLE_WORKSPACE_ID } from 'src/engine/workspace-manager/dev-seeder/core/utils/seed-workspaces.util'; + +const client = request(`http://localhost:${APP_PORT}`); + +describe('Object Permissions Validation', () => { + let customRoleId: string; + let personObjectId: string; + let companyObjectId: string; + + beforeAll(async () => { + const enablePermissionsQuery = updateFeatureFlagFactory( + SEED_APPLE_WORKSPACE_ID, + 'IS_PERMISSIONS_V2_ENABLED', + true, + ); + + await makeGraphqlAPIRequest(enablePermissionsQuery); + // Get object metadata IDs for Person and Company + const getObjectMetadataOperation = { + query: gql` + query { + objects(paging: { first: 1000 }) { + edges { + node { + id + nameSingular + } + } + } + } + `, + }; + + const objectMetadataResponse = await makeMetadataAPIRequest( + getObjectMetadataOperation, + ); + const objects = objectMetadataResponse.body.data.objects.edges; + + personObjectId = objects.find( + (obj: any) => obj.node.nameSingular === 'person', + )?.node.id; + companyObjectId = objects.find( + (obj: any) => obj.node.nameSingular === 'company', + )?.node.id; + + expect(personObjectId).toBeDefined(); + expect(companyObjectId).toBeDefined(); + }); + + afterAll(async () => { + const disablePermissionsQuery = updateFeatureFlagFactory( + SEED_APPLE_WORKSPACE_ID, + 'IS_PERMISSIONS_V2_ENABLED', + false, + ); + + await makeGraphqlAPIRequest(disablePermissionsQuery); + }); + + describe('cases with role with all rights by default', () => { + beforeEach(async () => { + // Create a custom role for each test + const roleOperation = createRoleOperation({ + label: 'TestRole', + description: 'Test role for object permission validation', + canUpdateAllSettings: true, + canReadAllObjectRecords: true, + canUpdateAllObjectRecords: true, + canSoftDeleteAllObjectRecords: true, + canDestroyAllObjectRecords: true, + }); + + const response = await makeGraphqlAPIRequest(roleOperation); + + customRoleId = response.body.data.createOneRole.id; + }); + + afterEach(async () => { + // Clean up the role after each test + if (customRoleId) { + await deleteRole(client, customRoleId); + } + }); + + describe('validateObjectPermissionsOrThrow - basic valid cases', () => { + it('should allow read=true with any write permissions', async () => { + const operation = createUpsertObjectPermissionsOperation(customRoleId, [ + { + objectMetadataId: personObjectId, + canReadObjectRecords: true, + canUpdateObjectRecords: true, + canSoftDeleteObjectRecords: true, + canDestroyObjectRecords: true, + }, + ]); + + const response = await makeGraphqlAPIRequest(operation); + + expect(response.body.errors).toBeUndefined(); + expect(response.body.data.upsertObjectPermissions).toHaveLength(1); + expect(response.body.data.upsertObjectPermissions[0]).toMatchObject({ + objectMetadataId: personObjectId, + canReadObjectRecords: true, + canUpdateObjectRecords: true, + canSoftDeleteObjectRecords: true, + canDestroyObjectRecords: true, + }); + }); + + it('should allow read=false with all write permissions=false', async () => { + const operation = createUpsertObjectPermissionsOperation(customRoleId, [ + { + objectMetadataId: personObjectId, + canReadObjectRecords: false, + canUpdateObjectRecords: false, + canSoftDeleteObjectRecords: false, + canDestroyObjectRecords: false, + }, + ]); + + const response = await makeGraphqlAPIRequest(operation); + + expect(response.body.errors).toBeUndefined(); + expect(response.body.data.upsertObjectPermissions).toHaveLength(1); + expect(response.body.data.upsertObjectPermissions[0]).toMatchObject({ + objectMetadataId: personObjectId, + canReadObjectRecords: false, + canUpdateObjectRecords: false, + canSoftDeleteObjectRecords: false, + canDestroyObjectRecords: false, + }); + }); + }); + + describe('validateObjectPermissionsOrThrow - Invalid Cases', () => { + it('should throw error when read=false but canUpdateObjectRecords=true', async () => { + const operation = createUpsertObjectPermissionsOperation( + customRoleId, + [ + { + objectMetadataId: personObjectId, + canReadObjectRecords: false, + canUpdateObjectRecords: true, + canSoftDeleteObjectRecords: false, + canDestroyObjectRecords: false, + }, + ], + [ + 'objectMetadataId', + 'canReadObjectRecords', + 'canUpdateObjectRecords', + ], + ); + + const response = await makeGraphqlAPIRequest(operation); + + expect(response.body.data).toBeNull(); + expect(response.body.errors).toBeDefined(); + expect(response.body.errors[0].message).toBe( + PermissionsExceptionMessage.CANNOT_GIVE_WRITING_PERMISSION_ON_NON_READABLE_OBJECT, + ); + expect(response.body.errors[0].extensions.code).toBe( + ErrorCode.BAD_USER_INPUT, + ); + }); + + it('should throw error when read=false but canSoftDeleteObjectRecords=true', async () => { + const operation = createUpsertObjectPermissionsOperation( + customRoleId, + [ + { + objectMetadataId: personObjectId, + canReadObjectRecords: false, + canUpdateObjectRecords: false, + canSoftDeleteObjectRecords: true, + canDestroyObjectRecords: false, + }, + ], + [ + 'objectMetadataId', + 'canReadObjectRecords', + 'canSoftDeleteObjectRecords', + ], + ); + + const response = await makeGraphqlAPIRequest(operation); + + expect(response.body.data).toBeNull(); + expect(response.body.errors).toBeDefined(); + expect(response.body.errors[0].message).toBe( + PermissionsExceptionMessage.CANNOT_GIVE_WRITING_PERMISSION_ON_NON_READABLE_OBJECT, + ); + expect(response.body.errors[0].extensions.code).toBe( + ErrorCode.BAD_USER_INPUT, + ); + }); + + it('should throw error when read=false but canDestroyObjectRecords=true', async () => { + const operation = createUpsertObjectPermissionsOperation( + customRoleId, + [ + { + objectMetadataId: personObjectId, + canReadObjectRecords: false, + canUpdateObjectRecords: false, + canSoftDeleteObjectRecords: false, + canDestroyObjectRecords: true, + }, + ], + [ + 'objectMetadataId', + 'canReadObjectRecords', + 'canDestroyObjectRecords', + ], + ); + + const response = await makeGraphqlAPIRequest(operation); + + expect(response.body.data).toBeNull(); + expect(response.body.errors).toBeDefined(); + expect(response.body.errors[0].message).toBe( + PermissionsExceptionMessage.CANNOT_GIVE_WRITING_PERMISSION_ON_NON_READABLE_OBJECT, + ); + expect(response.body.errors[0].extensions.code).toBe( + ErrorCode.BAD_USER_INPUT, + ); + }); + + it('should throw error when read=false but multiple write permissions=true', async () => { + const operation = createUpsertObjectPermissionsOperation( + customRoleId, + [ + { + objectMetadataId: personObjectId, + canReadObjectRecords: false, + canUpdateObjectRecords: true, + canSoftDeleteObjectRecords: true, + canDestroyObjectRecords: false, + }, + ], + [ + 'objectMetadataId', + 'canReadObjectRecords', + 'canUpdateObjectRecords', + 'canSoftDeleteObjectRecords', + ], + ); + + const response = await makeGraphqlAPIRequest(operation); + + expect(response.body.data).toBeNull(); + expect(response.body.errors).toBeDefined(); + expect(response.body.errors[0].message).toBe( + PermissionsExceptionMessage.CANNOT_GIVE_WRITING_PERMISSION_ON_NON_READABLE_OBJECT, + ); + expect(response.body.errors[0].extensions.code).toBe( + ErrorCode.BAD_USER_INPUT, + ); + }); + }); + + describe('validateObjectPermissionsOrThrow - Multiple Objects', () => { + it('should validate permissions across multiple objects correctly', async () => { + const operation = createUpsertObjectPermissionsOperation( + customRoleId, + [ + { + objectMetadataId: personObjectId, + canReadObjectRecords: true, + canUpdateObjectRecords: true, + canSoftDeleteObjectRecords: false, + canDestroyObjectRecords: false, + }, + { + objectMetadataId: companyObjectId, + canReadObjectRecords: false, + canUpdateObjectRecords: false, + canSoftDeleteObjectRecords: false, + canDestroyObjectRecords: false, + }, + ], + [ + 'objectMetadataId', + 'canReadObjectRecords', + 'canUpdateObjectRecords', + ], + ); + + const response = await makeGraphqlAPIRequest(operation); + + expect(response.body.errors).toBeUndefined(); + expect(response.body.data.upsertObjectPermissions).toHaveLength(2); + }); + + it('should throw error when one object has invalid permissions', async () => { + const operation = createUpsertObjectPermissionsOperation( + customRoleId, + [ + { + objectMetadataId: personObjectId, + canReadObjectRecords: true, + canUpdateObjectRecords: true, + canSoftDeleteObjectRecords: false, + canDestroyObjectRecords: false, + }, + { + objectMetadataId: companyObjectId, + canReadObjectRecords: false, + canUpdateObjectRecords: true, // This should fail + canSoftDeleteObjectRecords: false, + canDestroyObjectRecords: false, + }, + ], + [ + 'objectMetadataId', + 'canReadObjectRecords', + 'canUpdateObjectRecords', + ], + ); + + const response = await makeGraphqlAPIRequest(operation); + + expect(response.body.data).toBeNull(); + expect(response.body.errors).toBeDefined(); + expect(response.body.errors[0].message).toBe( + PermissionsExceptionMessage.CANNOT_GIVE_WRITING_PERMISSION_ON_NON_READABLE_OBJECT, + ); + expect(response.body.errors[0].extensions.code).toBe( + ErrorCode.BAD_USER_INPUT, + ); + }); + }); + }); + + describe('cases with role with no rights by default', () => { + let roleWithoutPermissions: string; + + beforeEach(async () => { + // Create a role with write permissions as defaults + const roleWithoutPermissionsQuery = createRoleOperation({ + label: 'TestRoleWithNoRights', + description: 'Test role with no rights', + canUpdateAllSettings: false, + canReadAllObjectRecords: false, + canUpdateAllObjectRecords: false, + canSoftDeleteAllObjectRecords: false, + canDestroyAllObjectRecords: false, + }); + + const response = await makeGraphqlAPIRequest(roleWithoutPermissionsQuery); + + roleWithoutPermissions = response.body.data.createOneRole.id; + }); + + afterEach(async () => { + if (roleWithoutPermissions) { + await deleteRole(client, roleWithoutPermissions); + } + }); + + it('should throw error when read=true and write permissions inherit false from role defaults', async () => { + const operation = createUpsertObjectPermissionsOperation( + roleWithoutPermissions, + [ + { + objectMetadataId: personObjectId, + canUpdateObjectRecords: true, + }, + ], + ['objectMetadataId', 'canReadObjectRecords'], + ); + + const response = await makeGraphqlAPIRequest(operation); + + expect(response.body.data).toBeNull(); + expect(response.body.errors).toBeDefined(); + expect(response.body.errors[0].message).toBe( + PermissionsExceptionMessage.CANNOT_GIVE_WRITING_PERMISSION_ON_NON_READABLE_OBJECT, + ); + expect(response.body.errors[0].extensions.code).toBe( + ErrorCode.BAD_USER_INPUT, + ); + }); + + it('should work when read=true and update=true', async () => { + const operation = createUpsertObjectPermissionsOperation( + roleWithoutPermissions, + [ + { + objectMetadataId: personObjectId, + canReadObjectRecords: true, + canUpdateObjectRecords: true, + }, + ], + ); + + const response = await makeGraphqlAPIRequest(operation); + + expect(response.body.errors).toBeUndefined(); + expect(response.body.data.upsertObjectPermissions).toHaveLength(1); + expect(response.body.data.upsertObjectPermissions[0]).toMatchObject({ + objectMetadataId: personObjectId, + canReadObjectRecords: true, + canUpdateObjectRecords: true, + canSoftDeleteObjectRecords: null, + canDestroyObjectRecords: null, + }); + }); + }); +}); diff --git a/packages/twenty-server/test/integration/graphql/suites/role/role.integration-spec.ts b/packages/twenty-server/test/integration/graphql/suites/role/role.integration-spec.ts new file mode 100644 index 000000000..fa3929cb5 --- /dev/null +++ b/packages/twenty-server/test/integration/graphql/suites/role/role.integration-spec.ts @@ -0,0 +1,347 @@ +import gql from 'graphql-tag'; +import { default as request } from 'supertest'; +import { createRoleOperation } from 'test/integration/graphql/utils/create-custom-role-operation-factory.util'; +import { deleteRole } from 'test/integration/graphql/utils/delete-one-role.util'; +import { makeGraphqlAPIRequest } from 'test/integration/graphql/utils/make-graphql-api-request.util'; +import { updateFeatureFlagFactory } from 'test/integration/graphql/utils/update-feature-flag-factory.util'; + +import { ErrorCode } from 'src/engine/core-modules/graphql/utils/graphql-errors.util'; +import { PermissionsExceptionMessage } from 'src/engine/metadata-modules/permissions/permissions.exception'; +import { SEED_APPLE_WORKSPACE_ID } from 'src/engine/workspace-manager/dev-seeder/core/utils/seed-workspaces.util'; + +const client = request(`http://localhost:${APP_PORT}`); + +describe('Role Permissions Validation', () => { + beforeAll(async () => { + const enablePermissionsQuery = updateFeatureFlagFactory( + SEED_APPLE_WORKSPACE_ID, + 'IS_PERMISSIONS_V2_ENABLED', + true, + ); + + await makeGraphqlAPIRequest(enablePermissionsQuery); + }); + + afterAll(async () => { + const disablePermissionsQuery = updateFeatureFlagFactory( + SEED_APPLE_WORKSPACE_ID, + 'IS_PERMISSIONS_V2_ENABLED', + false, + ); + + await makeGraphqlAPIRequest(disablePermissionsQuery); + }); + + describe('validateRoleDoesNotHaveWritingPermissionsWithoutReadingPermissionsOrThrow', () => { + describe('createRole - Valid Cases', () => { + it('should allow creating role with read=true and any write permissions', async () => { + const operation = createRoleOperation({ + label: 'ValidRole', + description: 'Valid role with read and write permissions', + canUpdateAllSettings: true, + canReadAllObjectRecords: true, + canUpdateAllObjectRecords: true, + canSoftDeleteAllObjectRecords: true, + canDestroyAllObjectRecords: true, + }); + + const response = await makeGraphqlAPIRequest(operation); + + expect(response.body.errors).toBeUndefined(); + expect(response.body.data.createOneRole).toBeDefined(); + expect(response.body.data.createOneRole.label).toBe('ValidRole'); + }); + + it('should allow creating role with read=false and all write permissions=false', async () => { + const operation = createRoleOperation({ + label: 'ValidNoWriteRole', + description: 'Valid role with no write permissions', + canUpdateAllSettings: false, + canReadAllObjectRecords: false, + canUpdateAllObjectRecords: false, + canSoftDeleteAllObjectRecords: false, + canDestroyAllObjectRecords: false, + }); + + const response = await makeGraphqlAPIRequest(operation); + + expect(response.body.errors).toBeUndefined(); + expect(response.body.data.createOneRole).toBeDefined(); + expect(response.body.data.createOneRole.label).toBe('ValidNoWriteRole'); + }); + }); + + describe('createRole - Invalid Cases', () => { + it('should throw error when creating role with read=false but canDestroyAllObjectRecords=true', async () => { + const operation = createRoleOperation({ + label: 'InvalidDestroyRole', + description: 'Invalid role with destroy permission but no read', + canUpdateAllSettings: false, + canReadAllObjectRecords: false, + canUpdateAllObjectRecords: false, + canSoftDeleteAllObjectRecords: false, + canDestroyAllObjectRecords: true, + }); + + const response = await makeGraphqlAPIRequest(operation); + + expect(response.body.data).toBeNull(); + expect(response.body.errors).toBeDefined(); + expect(response.body.errors[0].message).toBe( + PermissionsExceptionMessage.CANNOT_GIVE_WRITING_PERMISSION_WITHOUT_READING_PERMISSION, + ); + expect(response.body.errors[0].extensions.code).toBe( + ErrorCode.BAD_USER_INPUT, + ); + }); + }); + + describe('updateRole - Valid Cases', () => { + let baseRoleId: string; + + beforeEach(async () => { + const operation = createRoleOperation({ + label: 'BaseRole', + description: 'Base role for update tests', + canUpdateAllSettings: false, + canReadAllObjectRecords: true, + canUpdateAllObjectRecords: false, + canSoftDeleteAllObjectRecords: false, + canDestroyAllObjectRecords: false, + }); + + const response = await makeGraphqlAPIRequest(operation); + + baseRoleId = response.body.data.createOneRole.id; + }); + + afterEach(async () => { + if (baseRoleId) { + await deleteRole(client, baseRoleId); + } + }); + + it('should allow updating role to have read=true and any write permissions', async () => { + const updateRoleOperation = { + query: gql` + mutation UpdateOneRole($updateRoleInput: UpdateRoleInput!) { + updateOneRole(updateRoleInput: $updateRoleInput) { + id + canReadAllObjectRecords + canUpdateAllObjectRecords + canSoftDeleteAllObjectRecords + canDestroyAllObjectRecords + } + } + `, + variables: { + updateRoleInput: { + id: baseRoleId, + update: { + canUpdateAllObjectRecords: true, + canSoftDeleteAllObjectRecords: true, + canDestroyAllObjectRecords: true, + }, + }, + }, + }; + + const response = await makeGraphqlAPIRequest(updateRoleOperation); + + expect(response.body.errors).toBeUndefined(); + expect(response.body.data.updateOneRole).toBeDefined(); + expect(response.body.data.updateOneRole.canReadAllObjectRecords).toBe( + true, + ); + expect(response.body.data.updateOneRole.canUpdateAllObjectRecords).toBe( + true, + ); + expect( + response.body.data.updateOneRole.canSoftDeleteAllObjectRecords, + ).toBe(true); + expect( + response.body.data.updateOneRole.canDestroyAllObjectRecords, + ).toBe(true); + }); + + it('should allow updating role to have read=false and all write permissions=false', async () => { + const updateRoleOperation = { + query: gql` + mutation UpdateOneRole($updateRoleInput: UpdateRoleInput!) { + updateOneRole(updateRoleInput: $updateRoleInput) { + id + canReadAllObjectRecords + canUpdateAllObjectRecords + canSoftDeleteAllObjectRecords + canDestroyAllObjectRecords + } + } + `, + variables: { + updateRoleInput: { + id: baseRoleId, + update: { + canReadAllObjectRecords: false, + canUpdateAllObjectRecords: false, + canSoftDeleteAllObjectRecords: false, + canDestroyAllObjectRecords: false, + }, + }, + }, + }; + + const response = await makeGraphqlAPIRequest(updateRoleOperation); + + expect(response.body.errors).toBeUndefined(); + expect(response.body.data.updateOneRole).toBeDefined(); + expect(response.body.data.updateOneRole.canReadAllObjectRecords).toBe( + false, + ); + expect(response.body.data.updateOneRole.canUpdateAllObjectRecords).toBe( + false, + ); + expect( + response.body.data.updateOneRole.canSoftDeleteAllObjectRecords, + ).toBe(false); + expect( + response.body.data.updateOneRole.canDestroyAllObjectRecords, + ).toBe(false); + }); + }); + + describe('updateRole - Invalid Cases', () => { + let roleWithWritePermissionsId: string; + + beforeEach(async () => { + const operation = createRoleOperation({ + label: 'RoleWithWritePermissions', + description: 'Role with write permissions for update tests', + canUpdateAllSettings: false, + canReadAllObjectRecords: true, + canUpdateAllObjectRecords: true, + canSoftDeleteAllObjectRecords: true, + canDestroyAllObjectRecords: true, + }); + + const response = await makeGraphqlAPIRequest(operation); + + roleWithWritePermissionsId = response.body.data.createOneRole.id; + }); + + afterEach(async () => { + if (roleWithWritePermissionsId) { + await deleteRole(client, roleWithWritePermissionsId); + } + }); + + it('should throw error when updating role to read=false but keeping write permissions', async () => { + const updateRoleOperation = { + query: gql` + mutation UpdateOneRole($updateRoleInput: UpdateRoleInput!) { + updateOneRole(updateRoleInput: $updateRoleInput) { + id + } + } + `, + variables: { + updateRoleInput: { + id: roleWithWritePermissionsId, + update: { + canReadAllObjectRecords: false, + // Not explicitly setting write permissions, so they keep existing values (true) + }, + }, + }, + }; + + const response = await makeGraphqlAPIRequest(updateRoleOperation); + + expect(response.body.data).toBeNull(); + expect(response.body.errors).toBeDefined(); + expect(response.body.errors[0].message).toBe( + PermissionsExceptionMessage.CANNOT_GIVE_WRITING_PERMISSION_WITHOUT_READING_PERMISSION, + ); + expect(response.body.errors[0].extensions.code).toBe( + ErrorCode.BAD_USER_INPUT, + ); + }); + + it('should allow updating role to read=false when explicitly setting all write permissions to false', async () => { + const updateRoleOperation = { + query: gql` + mutation UpdateOneRole($updateRoleInput: UpdateRoleInput!) { + updateOneRole(updateRoleInput: $updateRoleInput) { + id + canReadAllObjectRecords + canUpdateAllObjectRecords + canSoftDeleteAllObjectRecords + canDestroyAllObjectRecords + } + } + `, + variables: { + updateRoleInput: { + id: roleWithWritePermissionsId, + update: { + canReadAllObjectRecords: false, + canUpdateAllObjectRecords: false, + canSoftDeleteAllObjectRecords: false, + canDestroyAllObjectRecords: false, + }, + }, + }, + }; + + const response = await makeGraphqlAPIRequest(updateRoleOperation); + + expect(response.body.errors).toBeUndefined(); + expect(response.body.data.updateOneRole).toBeDefined(); + expect(response.body.data.updateOneRole.canReadAllObjectRecords).toBe( + false, + ); + expect(response.body.data.updateOneRole.canUpdateAllObjectRecords).toBe( + false, + ); + expect( + response.body.data.updateOneRole.canSoftDeleteAllObjectRecords, + ).toBe(false); + expect( + response.body.data.updateOneRole.canDestroyAllObjectRecords, + ).toBe(false); + }); + + it('should allow updating role to read=false when explicitly setting some write permissions to false', async () => { + const updateRoleOperation = { + query: gql` + mutation UpdateOneRole($updateRoleInput: UpdateRoleInput!) { + updateOneRole(updateRoleInput: $updateRoleInput) { + id + canReadAllObjectRecords + canUpdateAllObjectRecords + canSoftDeleteAllObjectRecords + canDestroyAllObjectRecords + } + } + `, + variables: { + updateRoleInput: { + id: roleWithWritePermissionsId, + update: { + canSoftDeleteAllObjectRecords: false, + // Keep other permissions as they are + }, + }, + }, + }; + + const response = await makeGraphqlAPIRequest(updateRoleOperation); + + expect(response.body.errors).toBeUndefined(); + expect(response.body.data.updateOneRole).toBeDefined(); + expect( + response.body.data.updateOneRole.canSoftDeleteAllObjectRecords, + ).toBe(false); + }); + }); + }); +}); diff --git a/packages/twenty-server/test/integration/graphql/suites/settings-permissions/roles.integration-spec.ts b/packages/twenty-server/test/integration/graphql/suites/settings-permissions/roles.integration-spec.ts index d560e3c54..ca0c3d83a 100644 --- a/packages/twenty-server/test/integration/graphql/suites/settings-permissions/roles.integration-spec.ts +++ b/packages/twenty-server/test/integration/graphql/suites/settings-permissions/roles.integration-spec.ts @@ -477,7 +477,7 @@ describe('roles permissions', () => { roleId: string; }) => ` mutation UpsertObjectPermissions { - upsertObjectPermissions(upsertObjectPermissionsInput: { roleId: "${roleId}", objectPermissions: [{objectMetadataId: "${objectMetadataId}", canUpdateObjectRecords: true}]}) { + upsertObjectPermissions(upsertObjectPermissionsInput: { roleId: "${roleId}", objectPermissions: [{objectMetadataId: "${objectMetadataId}", canUpdateObjectRecords: true, canReadObjectRecords: true}]}) { objectMetadataId canUpdateObjectRecords } diff --git a/packages/twenty-server/test/integration/graphql/utils/create-custom-role-operation-factory.util.ts b/packages/twenty-server/test/integration/graphql/utils/create-custom-role-operation-factory.util.ts new file mode 100644 index 000000000..eabaf30bf --- /dev/null +++ b/packages/twenty-server/test/integration/graphql/utils/create-custom-role-operation-factory.util.ts @@ -0,0 +1,39 @@ +import gql from 'graphql-tag'; + +export const createRoleOperation = ({ + label, + description, + canUpdateAllSettings, + canReadAllObjectRecords, + canDestroyAllObjectRecords, + canUpdateAllObjectRecords, + canSoftDeleteAllObjectRecords, +}: { + label: string; + description: string; + canUpdateAllSettings: boolean; + canReadAllObjectRecords: boolean; + canDestroyAllObjectRecords: boolean; + canUpdateAllObjectRecords: boolean; + canSoftDeleteAllObjectRecords: boolean; +}) => ({ + query: gql` + mutation CreateOneRole($createRoleInput: CreateRoleInput!) { + createOneRole(createRoleInput: $createRoleInput) { + id + label + } + } + `, + variables: { + createRoleInput: { + label, + description, + canUpdateAllSettings, + canReadAllObjectRecords, + canUpdateAllObjectRecords, + canSoftDeleteAllObjectRecords, + canDestroyAllObjectRecords, + }, + }, +}); diff --git a/packages/twenty-server/test/integration/graphql/utils/upsert-object-permission-operation-factory.util.ts b/packages/twenty-server/test/integration/graphql/utils/upsert-object-permission-operation-factory.util.ts new file mode 100644 index 000000000..fccd5f706 --- /dev/null +++ b/packages/twenty-server/test/integration/graphql/utils/upsert-object-permission-operation-factory.util.ts @@ -0,0 +1,39 @@ +import gql from 'graphql-tag'; + +export const createUpsertObjectPermissionsOperation = ( + roleId: string, + objectPermissions: Array<{ + objectMetadataId: string; + canReadObjectRecords?: boolean; + canUpdateObjectRecords?: boolean; + canSoftDeleteObjectRecords?: boolean; + canDestroyObjectRecords?: boolean; + }>, + selectedFields: string[] = [ + 'objectMetadataId', + 'canReadObjectRecords', + 'canUpdateObjectRecords', + 'canSoftDeleteObjectRecords', + 'canDestroyObjectRecords', + ], +) => ({ + query: gql` + mutation UpsertObjectPermissions( + $roleId: String! + $objectPermissions: [ObjectPermissionInput!]! + ) { + upsertObjectPermissions( + upsertObjectPermissionsInput: { + roleId: $roleId + objectPermissions: $objectPermissions + } + ) { + ${selectedFields.join('\n')} + } + } + `, + variables: { + roleId, + objectPermissions, + }, +});