diff --git a/packages/twenty-server/src/engine/core-modules/user-workspace/user-workspace.service.ts b/packages/twenty-server/src/engine/core-modules/user-workspace/user-workspace.service.ts index 1163d47b2..1e3670079 100644 --- a/packages/twenty-server/src/engine/core-modules/user-workspace/user-workspace.service.ts +++ b/packages/twenty-server/src/engine/core-modules/user-workspace/user-workspace.service.ts @@ -2,9 +2,9 @@ import { InjectRepository } from '@nestjs/typeorm'; import { TypeOrmQueryService } from '@ptc-org/nestjs-query-typeorm'; -import { Repository } from 'typeorm'; -import { isDefined } from 'twenty-shared/utils'; import { SOURCE_LOCALE } from 'twenty-shared/translations'; +import { isDefined } from 'twenty-shared/utils'; +import { Repository } from 'typeorm'; import { TypeORMService } from 'src/database/typeorm/typeorm.service'; import { DatabaseEventAction } from 'src/engine/api/graphql/graphql-query-runner/enums/database-event-action'; @@ -37,8 +37,6 @@ export class UserWorkspaceService extends TypeOrmQueryService { constructor( @InjectRepository(UserWorkspace, 'core') private readonly userWorkspaceRepository: Repository, - @InjectRepository(Workspace, 'core') - private readonly workspaceRepository: Repository, @InjectRepository(User, 'core') private readonly userRepository: Repository, @InjectRepository(ObjectMetadataEntity, 'metadata') 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 4c3bd1ba1..af844d778 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 @@ -31,6 +31,11 @@ export class ObjectPermissionService { input: UpsertObjectPermissionInput; }): Promise { try { + await this.validateRoleIsEditableOrThrow({ + roleId: input.roleId, + workspaceId, + }); + const result = await this.objectPermissionRepository.upsert( { workspaceId, @@ -76,7 +81,12 @@ export class ObjectPermissionService { objectMetadataId: string; }) { if (error.message.includes('violates foreign key constraint')) { - const role = await this.getRole(roleId, workspaceId); + const role = await this.roleRepository.findOne({ + where: { + id: roleId, + workspaceId, + }, + }); if (!isDefined(role)) { throw new PermissionsException( @@ -101,15 +111,25 @@ export class ObjectPermissionService { } } - private async getRole( - roleId: string, - workspaceId: string, - ): Promise { - return this.roleRepository.findOne({ + private async validateRoleIsEditableOrThrow({ + roleId, + workspaceId, + }: { + roleId: string; + workspaceId: string; + }) { + const role = await this.roleRepository.findOne({ where: { id: roleId, workspaceId, }, }); + + 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 ddd2f4fbf..3b84e765a 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 @@ -28,6 +28,7 @@ export enum PermissionsExceptionCode { OBJECT_METADATA_NOT_FOUND = 'OBJECT_METADATA_NOT_FOUND', INVALID_SETTING = 'INVALID_SETTING', ROLE_NOT_EDITABLE = 'ROLE_NOT_EDITABLE', + DEFAULT_ROLE_CANNOT_BE_DELETED = 'DEFAULT_ROLE_CANNOT_BE_DELETED', } export enum PermissionsExceptionMessage { @@ -51,4 +52,5 @@ export enum PermissionsExceptionMessage { OBJECT_METADATA_NOT_FOUND = 'Object metadata not found', INVALID_SETTING = 'Invalid permission setting (unknown value)', ROLE_NOT_EDITABLE = 'Role is not editable', + DEFAULT_ROLE_CANNOT_BE_DELETED = 'Default role cannot be deleted', } diff --git a/packages/twenty-server/src/engine/metadata-modules/role/role.module.ts b/packages/twenty-server/src/engine/metadata-modules/role/role.module.ts index 4c0d9275f..52636242a 100644 --- a/packages/twenty-server/src/engine/metadata-modules/role/role.module.ts +++ b/packages/twenty-server/src/engine/metadata-modules/role/role.module.ts @@ -4,6 +4,7 @@ import { TypeOrmModule } from '@nestjs/typeorm'; import { FeatureFlagModule } from 'src/engine/core-modules/feature-flag/feature-flag.module'; import { UserWorkspace } from 'src/engine/core-modules/user-workspace/user-workspace.entity'; import { UserWorkspaceModule } from 'src/engine/core-modules/user-workspace/user-workspace.module'; +import { Workspace } from 'src/engine/core-modules/workspace/workspace.entity'; import { ObjectPermissionModule } from 'src/engine/metadata-modules/object-permission/object-permission.module'; import { PermissionsModule } from 'src/engine/metadata-modules/permissions/permissions.module'; import { RoleEntity } from 'src/engine/metadata-modules/role/role.entity'; @@ -16,7 +17,7 @@ import { UserRoleModule } from 'src/engine/metadata-modules/user-role/user-role. @Module({ imports: [ TypeOrmModule.forFeature([RoleEntity, UserWorkspaceRoleEntity], 'metadata'), - TypeOrmModule.forFeature([UserWorkspace], 'core'), + TypeOrmModule.forFeature([UserWorkspace, Workspace], 'core'), UserRoleModule, PermissionsModule, UserWorkspaceModule, diff --git a/packages/twenty-server/src/engine/metadata-modules/role/role.resolver.ts b/packages/twenty-server/src/engine/metadata-modules/role/role.resolver.ts index 6d7a5b409..db2be37dc 100644 --- a/packages/twenty-server/src/engine/metadata-modules/role/role.resolver.ts +++ b/packages/twenty-server/src/engine/metadata-modules/role/role.resolver.ts @@ -123,10 +123,6 @@ export class RoleResolver { @Args('updateRoleInput') updateRoleInput: UpdateRoleInput, ): Promise { await this.validatePermissionsV2EnabledOrThrow(workspace); - await this.validateRoleIsEditableOrThrow({ - roleId: updateRoleInput.id, - workspaceId: workspace.id, - }); return this.roleService.updateRole({ input: updateRoleInput, @@ -134,6 +130,16 @@ export class RoleResolver { }); } + @Mutation(() => String) + async deleteOneRole( + @AuthWorkspace() workspace: Workspace, + @Args('roleId') roleId: string, + ): Promise { + await this.validatePermissionsV2EnabledOrThrow(workspace); + + return this.roleService.deleteRole(roleId, workspace.id); + } + @Mutation(() => ObjectPermissionDTO) async upsertOneObjectPermission( @AuthWorkspace() workspace: Workspace, @@ -141,10 +147,6 @@ export class RoleResolver { upsertObjectPermissionInput: UpsertObjectPermissionInput, ) { await this.validatePermissionsV2EnabledOrThrow(workspace); - await this.validateRoleIsEditableOrThrow({ - roleId: upsertObjectPermissionInput.roleId, - workspaceId: workspace.id, - }); return this.objectPermissionService.upsertObjectPermission({ workspaceId: workspace.id, @@ -159,10 +161,6 @@ export class RoleResolver { upsertSettingPermissionInput: UpsertSettingPermissionInput, ) { await this.validatePermissionsV2EnabledOrThrow(workspace); - await this.validateRoleIsEditableOrThrow({ - roleId: upsertSettingPermissionInput.roleId, - workspaceId: workspace.id, - }); return this.settingPermissionService.upsertSettingPermission({ workspaceId: workspace.id, @@ -195,21 +193,4 @@ export class RoleResolver { ); } } - - private async validateRoleIsEditableOrThrow({ - roleId, - workspaceId, - }: { - roleId: string; - workspaceId: string; - }) { - const role = await this.roleService.getRoleById(roleId, workspaceId); - - if (!role?.isEditable) { - throw new PermissionsException( - PermissionsExceptionMessage.ROLE_NOT_EDITABLE, - PermissionsExceptionCode.ROLE_NOT_EDITABLE, - ); - } - } } 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 e4ac7e2b2..6f4ccee83 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 @@ -3,6 +3,7 @@ import { InjectRepository } from '@nestjs/typeorm'; import { isDefined } from 'twenty-shared/utils'; import { Repository } from 'typeorm'; +import { Workspace } from 'src/engine/core-modules/workspace/workspace.entity'; import { ADMIN_ROLE_LABEL } from 'src/engine/metadata-modules/permissions/constants/admin-role-label.constants'; import { MEMBER_ROLE_LABEL } from 'src/engine/metadata-modules/permissions/constants/member-role-label.constants'; import { @@ -16,12 +17,19 @@ import { UpdateRolePayload, } from 'src/engine/metadata-modules/role/dtos/update-role-input.dto'; import { RoleEntity } from 'src/engine/metadata-modules/role/role.entity'; +import { UserWorkspaceRoleEntity } from 'src/engine/metadata-modules/role/user-workspace-role.entity'; +import { UserRoleService } from 'src/engine/metadata-modules/user-role/user-role.service'; import { isArgDefinedIfProvidedOrThrow } from 'src/engine/metadata-modules/utils/is-arg-defined-if-provided-or-throw.util'; export class RoleService { constructor( + @InjectRepository(Workspace, 'core') + private readonly workspaceRepository: Repository, @InjectRepository(RoleEntity, 'metadata') private readonly roleRepository: Repository, + @InjectRepository(UserWorkspaceRoleEntity, 'metadata') + private readonly userWorkspaceRoleRepository: Repository, + private readonly userRoleService: UserRoleService, ) {} public async getWorkspaceRoles(workspaceId: string): Promise { @@ -76,6 +84,11 @@ export class RoleService { input: UpdateRoleInput; workspaceId: string; }): Promise { + await this.validateRoleIsEditableOrThrow({ + roleId: input.id, + workspaceId, + }); + const existingRole = await this.roleRepository.findOne({ where: { id: input.id, @@ -123,6 +136,49 @@ export class RoleService { }); } + public async deleteRole( + roleId: string, + workspaceId: string, + ): Promise { + await this.validateRoleIsEditableOrThrow({ + roleId, + workspaceId, + }); + + const defaultRole = await this.workspaceRepository.findOne({ + where: { + id: workspaceId, + }, + }); + + const defaultRoleId = defaultRole?.defaultRoleId; + + if (!isDefined(defaultRoleId)) { + throw new PermissionsException( + PermissionsExceptionMessage.DEFAULT_ROLE_NOT_FOUND, + PermissionsExceptionCode.DEFAULT_ROLE_NOT_FOUND, + ); + } + + await this.validateRoleIsNotDefaultRoleOrThrow({ + roleId, + defaultRoleId, + }); + + await this.assignDefaultRoleToMembersWithRoleToDelete({ + roleId, + workspaceId, + defaultRoleId, + }); + + await this.roleRepository.delete({ + id: roleId, + workspaceId, + }); + + return roleId; + } + public async createMemberRole({ workspaceId, }: { @@ -210,4 +266,91 @@ 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, + ); + } + } + + private async assignDefaultRoleToMembersWithRoleToDelete({ + roleId, + workspaceId, + defaultRoleId, + }: { + roleId: string; + workspaceId: string; + defaultRoleId: string; + }): Promise { + const userWorkspaceIds = await this.getUserWorkspaceIdsForRole( + roleId, + workspaceId, + ); + + await Promise.all( + userWorkspaceIds.map((userWorkspaceId) => + this.userRoleService.assignRoleToUserWorkspace({ + userWorkspaceId, + roleId: defaultRoleId, + workspaceId, + }), + ), + ); + } + + private async getUserWorkspaceIdsForRole( + roleId: string, + workspaceId: string, + ): Promise { + return this.userWorkspaceRoleRepository + .find({ + where: { + roleId: roleId, + workspaceId, + }, + }) + .then((userWorkspaceRoles) => + userWorkspaceRoles.map( + (userWorkspaceRole) => userWorkspaceRole.userWorkspaceId, + ), + ); + } + + private async getRole( + roleId: string, + workspaceId: string, + ): Promise { + return this.roleRepository.findOne({ + where: { + id: roleId, + workspaceId, + }, + }); + } + + private async validateRoleIsEditableOrThrow({ + roleId, + workspaceId, + }: { + roleId: string; + workspaceId: string; + }) { + const role = await this.getRole(roleId, workspaceId); + + if (!role?.isEditable) { + throw new PermissionsException( + PermissionsExceptionMessage.ROLE_NOT_EDITABLE, + PermissionsExceptionCode.ROLE_NOT_EDITABLE, + ); + } + } } diff --git a/packages/twenty-server/src/engine/metadata-modules/setting-permission/setting-permission.service.ts b/packages/twenty-server/src/engine/metadata-modules/setting-permission/setting-permission.service.ts index 8d426be5b..2ee3444d0 100644 --- a/packages/twenty-server/src/engine/metadata-modules/setting-permission/setting-permission.service.ts +++ b/packages/twenty-server/src/engine/metadata-modules/setting-permission/setting-permission.service.ts @@ -28,6 +28,11 @@ export class SettingPermissionService { workspaceId: string; input: UpsertSettingPermissionInput; }): Promise { + await this.validateRoleIsEditableOrThrow({ + roleId: input.roleId, + workspaceId, + }); + if (!Object.values(SettingPermissionType).includes(input.setting)) { throw new PermissionsException( PermissionsExceptionMessage.INVALID_SETTING, @@ -76,4 +81,26 @@ export class SettingPermissionService { throw error; } } + + private async validateRoleIsEditableOrThrow({ + roleId, + workspaceId, + }: { + roleId: string; + workspaceId: string; + }) { + const role = await this.roleRepository.findOne({ + where: { + id: roleId, + workspaceId, + }, + }); + + if (!role?.isEditable) { + throw new PermissionsException( + PermissionsExceptionMessage.ROLE_NOT_EDITABLE, + PermissionsExceptionCode.ROLE_NOT_EDITABLE, + ); + } + } } 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 08b645db0..e6f0a301f 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 @@ -1,4 +1,5 @@ import request from 'supertest'; +import { deleteOneRoleOperationFactory } from 'test/integration/graphql/utils/delete-one-role-operation-factory.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 { createListingCustomObject } from 'test/integration/metadata/suites/object-metadata/utils/create-test-object-metadata.util'; @@ -316,51 +317,72 @@ describe('roles permissions', () => { await assertPermissionDeniedForMemberWithMemberRole({ query }); }); - // TODO - to uncomment after deleteOneRole has been implemented - // it('should create a role when user has permission to create a role (admin role)', async () => { - // const query = { - // query: ` - // mutation CreateOneRole { - // createOneRole(createRoleInput: {label: "Test role"}) { - // id - // } - // } - // `, - // }; + it('should create a role when user has permission to create a role (admin role)', async () => { + // Act and assert + const query = { + query: ` + mutation CreateOneRole { + createOneRole(createRoleInput: {label: "Test role"}) { + id + } + } + `, + }; - // await client - // .post('/graphql') - // .set('Authorization', `Bearer ${ADMIN_ACCESS_TOKEN}`) - // .send(query) - // .expect(200) - // .expect((res) => { - // expect(res.body.data).toBeDefined(); - // expect(res.body.errors).toBeUndefined(); - // }); - // }); + const result = await client + .post('/graphql') + .set('Authorization', `Bearer ${ADMIN_ACCESS_TOKEN}`) + .send(query) + .expect(200) + .expect((res) => { + expect(res.body.data).toBeDefined(); + expect(res.body.errors).toBeUndefined(); + }); + + const createdRoleId = result.body.data.createOneRole.id; + + // Clean + const deleteOneRoleQuery = deleteOneRoleOperationFactory(createdRoleId); + + await client + .post('/graphql') + .set('Authorization', `Bearer ${ADMIN_ACCESS_TOKEN}`) + .send(deleteOneRoleQuery); + }); }); describe('updateRole', () => { - // let createdEditableRoleId: string; + let createdEditableRoleId: string; beforeAll(async () => { - // TODO - to uncomment after deleteOneRole has been implemented - // const query = { - // query: ` - // mutation CreateOneRole { - // createOneRole(createRoleInput: {label: "Test role 2"}) { - // id - // } - // } - // `, - // }; - // await client - // .post('/graphql') - // .set('Authorization', `Bearer ${ADMIN_ACCESS_TOKEN}`) - // .send(query) - // .then((res) => { - // createdEditableRoleId = res.body.data.createOneRole.id; - // }); + const query = { + query: ` + mutation CreateOneRole { + createOneRole(createRoleInput: {label: "Test role 2"}) { + id + } + } + `, + }; + + await client + .post('/graphql') + .set('Authorization', `Bearer ${ADMIN_ACCESS_TOKEN}`) + .send(query) + .then((res) => { + createdEditableRoleId = res.body.data.createOneRole.id; + }); + }); + + afterAll(async () => { + const deleteOneRoleQuery = deleteOneRoleOperationFactory( + createdEditableRoleId, + ); + + await client + .post('/graphql') + .set('Authorization', `Bearer ${ADMIN_ACCESS_TOKEN}`) + .send(deleteOneRoleQuery); }); describe('updateRole', () => { @@ -368,7 +390,7 @@ describe('roles permissions', () => { const query = { query: ` mutation UpdateOneRole { - updateOneRole(updateRoleInput: {id: "test-role-id", update: {label: "new-role-label"}}) { + updateOneRole(updateRoleInput: {id: "${createdEditableRoleId}", update: {label: "new role label (1)"}}) { id } } @@ -382,7 +404,7 @@ describe('roles permissions', () => { const query = { query: ` mutation UpdateOneRole { - updateOneRole(updateRoleInput: {id: "${adminRoleId}", update: {label: "new-role-label"}}) { + updateOneRole(updateRoleInput: {id: "${adminRoleId}", update: {label: "new role label (2)"}}) { id } } @@ -405,6 +427,33 @@ describe('roles permissions', () => { ); }); }); + + it('should update a role when user has permission to update a role (admin role)', async () => { + const query = { + query: ` + mutation UpdateOneRole { + updateOneRole(updateRoleInput: {id: "${createdEditableRoleId}", update: {label: "new role label (3)"}}) { + id + label + } + } + `, + }; + + await client + .post('/graphql') + .set('Authorization', `Bearer ${ADMIN_ACCESS_TOKEN}`) + .send(query) + .expect(200) + .expect((res) => { + expect(res.body.data).toBeDefined(); + expect(res.body.errors).toBeUndefined(); + expect(res.body.data.updateOneRole.id).toBe(createdEditableRoleId); + expect(res.body.data.updateOneRole.label).toBe( + 'new role label (3)', + ); + }); + }); }); describe('upsertObjectPermission', () => { @@ -431,6 +480,8 @@ describe('roles permissions', () => { mutation UpsertObjectPermissions { upsertOneObjectPermission(upsertObjectPermissionInput: {objectMetadataId: "${objectMetadataId}", roleId: "${roleId}", canUpdateObjectRecords: true}) { id + roleId + canUpdateObjectRecords } } `; @@ -471,25 +522,30 @@ describe('roles permissions', () => { }); }); - // TODO - to uncomment after deleteOneRole has been implemented - // it('should upsert a setting permission when user has permission to create a setting permission', async () => { - // const query = { - // query: upsertObjectPermissionMutation({ - // objectMetadataId: listingObjectId, - // roleId: createdEditableRoleId, - // }), - // }; + it('should upsert an object permission when user has permission', async () => { + const query = { + query: upsertObjectPermissionMutation({ + objectMetadataId: listingObjectId, + roleId: createdEditableRoleId, + }), + }; - // await client - // .post('/graphql') - // .set('Authorization', `Bearer ${ADMIN_ACCESS_TOKEN}`) - // .send(query) - // .expect(200) - // .expect((res) => { - // expect(res.body.data).toBeDefined(); - // expect(res.body.errors).toBeUndefined(); - // }); - // }); + await client + .post('/graphql') + .set('Authorization', `Bearer ${ADMIN_ACCESS_TOKEN}`) + .send(query) + .expect(200) + .expect((res) => { + expect(res.body.data).toBeDefined(); + expect(res.body.errors).toBeUndefined(); + expect(res.body.data.upsertOneObjectPermission.roleId).toBe( + createdEditableRoleId, + ); + expect( + res.body.data.upsertOneObjectPermission.canUpdateObjectRecords, + ).toBe(true); + }); + }); }); describe('upsertSettingPermission', () => { @@ -499,8 +555,11 @@ describe('roles permissions', () => { roleId: string; }) => ` mutation UpsertSettingPermissions { - upsertOneSettingPermission(upsertSettingPermissionInput: {roleId: "${roleId}", setting: ${SettingPermissionType.DATA_MODEL}}) { + upsertOneSettingPermission(upsertSettingPermissionInput: {roleId: "${roleId}", setting: ${SettingPermissionType.DATA_MODEL}, canUpdateSetting: true}) { id + roleId + setting + canUpdateSetting } } `; @@ -538,6 +597,33 @@ describe('roles permissions', () => { ); }); }); + + it('should upsert a setting permission when user has permission', async () => { + const query = { + query: upsertSettingPermissionMutation({ + roleId: createdEditableRoleId, + }), + }; + + await client + .post('/graphql') + .set('Authorization', `Bearer ${ADMIN_ACCESS_TOKEN}`) + .send(query) + .expect(200) + .expect((res) => { + expect(res.body.data).toBeDefined(); + expect(res.body.errors).toBeUndefined(); + expect(res.body.data.upsertOneSettingPermission.roleId).toBe( + createdEditableRoleId, + ); + expect( + res.body.data.upsertOneSettingPermission.canUpdateSetting, + ).toBe(true); + expect(res.body.data.upsertOneSettingPermission.setting).toBe( + SettingPermissionType.DATA_MODEL, + ); + }); + }); }); }); }); diff --git a/packages/twenty-server/test/integration/graphql/utils/delete-one-role-operation-factory.util.ts b/packages/twenty-server/test/integration/graphql/utils/delete-one-role-operation-factory.util.ts new file mode 100644 index 000000000..3e99679e1 --- /dev/null +++ b/packages/twenty-server/test/integration/graphql/utils/delete-one-role-operation-factory.util.ts @@ -0,0 +1,7 @@ +export const deleteOneRoleOperationFactory = (roleId: string) => ({ + query: ` + mutation DeleteOneRole { + deleteOneRole(roleId: "${roleId}") + } + `, +});