From f403c551d781c51d8defd66d3b3926ae4949c1ca Mon Sep 17 00:00:00 2001 From: Weiko Date: Mon, 7 Apr 2025 17:15:33 +0200 Subject: [PATCH] Add settings permissions check on FE (#11425) ## Context Now that we can update role settings permissions, we need to reflect that on the FE as well (hiding/showing nav items + redirection logic). Feature flag check here is not really needed because since not having any setting permission will result in the same behavior as Permission V1. This PR updates the resolvers to return settings permissions of the current user --- .../interfaces/base-resolver-service.ts | 2 +- .../core-modules/billing/billing.resolver.ts | 2 +- .../workspace/services/workspace.service.ts | 4 +- .../guards/settings-permissions.guard.ts | 2 +- .../permissions/permissions.module.ts | 2 + .../permissions/permissions.service.ts | 49 +++++++++++++++---- .../user-role/user-role.service.ts | 6 ++- ...workspace-member-pre-query-hook.service.ts | 2 +- 8 files changed, 51 insertions(+), 18 deletions(-) diff --git a/packages/twenty-server/src/engine/api/graphql/graphql-query-runner/interfaces/base-resolver-service.ts b/packages/twenty-server/src/engine/api/graphql/graphql-query-runner/interfaces/base-resolver-service.ts index b21b7f2436..569d7b0a1a 100644 --- a/packages/twenty-server/src/engine/api/graphql/graphql-query-runner/interfaces/base-resolver-service.ts +++ b/packages/twenty-server/src/engine/api/graphql/graphql-query-runner/interfaces/base-resolver-service.ts @@ -190,7 +190,7 @@ export abstract class GraphqlQueryBaseResolverService< const userHasPermission = await this.permissionsService.userHasWorkspaceSettingPermission({ userWorkspaceId: authContext.userWorkspaceId, - _setting: permissionRequired, + setting: permissionRequired, workspaceId: authContext.workspace.id, isExecutedByApiKey: isDefined(authContext.apiKey), }); diff --git a/packages/twenty-server/src/engine/core-modules/billing/billing.resolver.ts b/packages/twenty-server/src/engine/core-modules/billing/billing.resolver.ts index 82a6b1b697..5ef85c3351 100644 --- a/packages/twenty-server/src/engine/core-modules/billing/billing.resolver.ts +++ b/packages/twenty-server/src/engine/core-modules/billing/billing.resolver.ts @@ -163,7 +163,7 @@ export class BillingResolver { await this.permissionsService.userHasWorkspaceSettingPermission({ userWorkspaceId, workspaceId, - _setting: SettingPermissionType.WORKSPACE, + setting: SettingPermissionType.WORKSPACE, isExecutedByApiKey, }); diff --git a/packages/twenty-server/src/engine/core-modules/workspace/services/workspace.service.ts b/packages/twenty-server/src/engine/core-modules/workspace/services/workspace.service.ts index fb9c7e3f36..8cf9961558 100644 --- a/packages/twenty-server/src/engine/core-modules/workspace/services/workspace.service.ts +++ b/packages/twenty-server/src/engine/core-modules/workspace/services/workspace.service.ts @@ -441,7 +441,7 @@ export class WorkspaceService extends TypeOrmQueryService { const userHasPermission = await this.permissionsService.userHasWorkspaceSettingPermission({ userWorkspaceId, - _setting: SettingPermissionType.SECURITY, + setting: SettingPermissionType.SECURITY, workspaceId: workspaceId, isExecutedByApiKey: isDefined(apiKey), }); @@ -480,7 +480,7 @@ export class WorkspaceService extends TypeOrmQueryService { await this.permissionsService.userHasWorkspaceSettingPermission({ userWorkspaceId, workspaceId, - _setting: SettingPermissionType.WORKSPACE, + setting: SettingPermissionType.WORKSPACE, isExecutedByApiKey: isDefined(apiKey), }); diff --git a/packages/twenty-server/src/engine/guards/settings-permissions.guard.ts b/packages/twenty-server/src/engine/guards/settings-permissions.guard.ts index 9169e5a9b6..f9ea8c7a24 100644 --- a/packages/twenty-server/src/engine/guards/settings-permissions.guard.ts +++ b/packages/twenty-server/src/engine/guards/settings-permissions.guard.ts @@ -32,7 +32,7 @@ export const SettingsPermissionsGuard = ( const hasPermission = await this.permissionsService.userHasWorkspaceSettingPermission({ userWorkspaceId, - _setting: requiredPermission, + setting: requiredPermission, workspaceId, isExecutedByApiKey: isDefined(ctx.getContext().req.apiKey), }); diff --git a/packages/twenty-server/src/engine/metadata-modules/permissions/permissions.module.ts b/packages/twenty-server/src/engine/metadata-modules/permissions/permissions.module.ts index b59549e858..577fd9c4a0 100644 --- a/packages/twenty-server/src/engine/metadata-modules/permissions/permissions.module.ts +++ b/packages/twenty-server/src/engine/metadata-modules/permissions/permissions.module.ts @@ -7,6 +7,7 @@ import { UserWorkspace } from 'src/engine/core-modules/user-workspace/user-works import { PermissionsService } from 'src/engine/metadata-modules/permissions/permissions.service'; import { RoleEntity } from 'src/engine/metadata-modules/role/role.entity'; import { UserWorkspaceRoleEntity } from 'src/engine/metadata-modules/role/user-workspace-role.entity'; +import { SettingPermissionEntity } from 'src/engine/metadata-modules/setting-permission/setting-permission.entity'; import { UserRoleModule } from 'src/engine/metadata-modules/user-role/user-role.module'; @Module({ @@ -16,6 +17,7 @@ import { UserRoleModule } from 'src/engine/metadata-modules/user-role/user-role. TypeOrmModule.forFeature([UserWorkspace], 'core'), EnvironmentModule, UserRoleModule, + TypeOrmModule.forFeature([SettingPermissionEntity], 'metadata'), ], providers: [PermissionsService], exports: [PermissionsService], diff --git a/packages/twenty-server/src/engine/metadata-modules/permissions/permissions.service.ts b/packages/twenty-server/src/engine/metadata-modules/permissions/permissions.service.ts index 797046c49c..1164a08f1f 100644 --- a/packages/twenty-server/src/engine/metadata-modules/permissions/permissions.service.ts +++ b/packages/twenty-server/src/engine/metadata-modules/permissions/permissions.service.ts @@ -1,7 +1,9 @@ import { Injectable } from '@nestjs/common'; +import { InjectRepository } from '@nestjs/typeorm'; import { PermissionsOnAllObjectRecords } from 'twenty-shared/constants'; import { isDefined } from 'twenty-shared/utils'; +import { Repository } from 'typeorm'; import { AuthException, @@ -15,6 +17,7 @@ import { PermissionsExceptionMessage, } from 'src/engine/metadata-modules/permissions/permissions.exception'; import { RoleEntity } from 'src/engine/metadata-modules/role/role.entity'; +import { SettingPermissionEntity } from 'src/engine/metadata-modules/setting-permission/setting-permission.entity'; import { UserRoleService } from 'src/engine/metadata-modules/user-role/user-role.service'; @Injectable() @@ -22,6 +25,8 @@ export class PermissionsService { constructor( private readonly environmentService: EnvironmentService, private readonly userRoleService: UserRoleService, + @InjectRepository(SettingPermissionEntity, 'metadata') + private readonly settingPermissionRepository: Repository, ) {} public async getUserWorkspacePermissions({ @@ -43,14 +48,27 @@ export class PermissionsService { let hasPermissionOnSettingFeature = false; - if (roleOfUserWorkspace?.canUpdateAllSettings === true) { + if (!isDefined(roleOfUserWorkspace)) { + throw new PermissionsException( + PermissionsExceptionMessage.NO_ROLE_FOUND_FOR_USER_WORKSPACE, + PermissionsExceptionCode.NO_ROLE_FOUND_FOR_USER_WORKSPACE, + ); + } + + if (roleOfUserWorkspace.canUpdateAllSettings === true) { hasPermissionOnSettingFeature = true; } + const settingPermissions = roleOfUserWorkspace.settingPermissions ?? []; + const settingsPermissionsMap = Object.keys(SettingPermissionType).reduce( (acc, feature) => ({ ...acc, - [feature]: hasPermissionOnSettingFeature, + [feature]: + hasPermissionOnSettingFeature || + settingPermissions.some( + (settingPermission) => settingPermission.setting === feature, + ), }), {} as Record, ); @@ -60,13 +78,13 @@ export class PermissionsService { boolean > = { [PermissionsOnAllObjectRecords.READ_ALL_OBJECT_RECORDS]: - roleOfUserWorkspace?.canReadAllObjectRecords ?? false, + roleOfUserWorkspace.canReadAllObjectRecords ?? false, [PermissionsOnAllObjectRecords.UPDATE_ALL_OBJECT_RECORDS]: - roleOfUserWorkspace?.canUpdateAllObjectRecords ?? false, + roleOfUserWorkspace.canUpdateAllObjectRecords ?? false, [PermissionsOnAllObjectRecords.SOFT_DELETE_ALL_OBJECT_RECORDS]: - roleOfUserWorkspace?.canSoftDeleteAllObjectRecords ?? false, + roleOfUserWorkspace.canSoftDeleteAllObjectRecords ?? false, [PermissionsOnAllObjectRecords.DESTROY_ALL_OBJECT_RECORDS]: - roleOfUserWorkspace?.canDestroyAllObjectRecords ?? false, + roleOfUserWorkspace.canDestroyAllObjectRecords ?? false, }; return { @@ -78,12 +96,12 @@ export class PermissionsService { public async userHasWorkspaceSettingPermission({ userWorkspaceId, workspaceId, - _setting, + setting, isExecutedByApiKey, }: { userWorkspaceId?: string; workspaceId: string; - _setting: SettingPermissionType; + setting: SettingPermissionType; isExecutedByApiKey: boolean; }): Promise { if (isExecutedByApiKey) { @@ -104,11 +122,22 @@ export class PermissionsService { }) .then((roles) => roles?.get(userWorkspaceId) ?? []); - if (roleOfUserWorkspace?.canUpdateAllSettings === true) { + if (!isDefined(roleOfUserWorkspace)) { + throw new PermissionsException( + PermissionsExceptionMessage.NO_ROLE_FOUND_FOR_USER_WORKSPACE, + PermissionsExceptionCode.NO_ROLE_FOUND_FOR_USER_WORKSPACE, + ); + } + + if (roleOfUserWorkspace.canUpdateAllSettings === true) { return true; } - return false; + const settingPermissions = roleOfUserWorkspace.settingPermissions ?? []; + + return settingPermissions.some( + (settingPermission) => settingPermission.setting === setting, + ); } public async userHasObjectRecordsPermission({ diff --git a/packages/twenty-server/src/engine/metadata-modules/user-role/user-role.service.ts b/packages/twenty-server/src/engine/metadata-modules/user-role/user-role.service.ts index 6885760f3a..c9a2a08486 100644 --- a/packages/twenty-server/src/engine/metadata-modules/user-role/user-role.service.ts +++ b/packages/twenty-server/src/engine/metadata-modules/user-role/user-role.service.ts @@ -1,7 +1,7 @@ import { InjectRepository } from '@nestjs/typeorm'; -import { In, Not, Repository } from 'typeorm'; import { isDefined } from 'twenty-shared/utils'; +import { In, Not, Repository } from 'typeorm'; import { UserWorkspace } from 'src/engine/core-modules/user-workspace/user-workspace.entity'; import { ADMIN_ROLE_LABEL } from 'src/engine/metadata-modules/permissions/constants/admin-role-label.constants'; @@ -75,7 +75,9 @@ export class UserRoleService { workspaceId, }, relations: { - role: true, + role: { + settingPermissions: true, + }, }, }); diff --git a/packages/twenty-server/src/modules/workspace-member/query-hooks/workspace-member-pre-query-hook.service.ts b/packages/twenty-server/src/modules/workspace-member/query-hooks/workspace-member-pre-query-hook.service.ts index 2ea2631e55..8649005b03 100644 --- a/packages/twenty-server/src/modules/workspace-member/query-hooks/workspace-member-pre-query-hook.service.ts +++ b/packages/twenty-server/src/modules/workspace-member/query-hooks/workspace-member-pre-query-hook.service.ts @@ -54,7 +54,7 @@ export class WorkspaceMemberPreQueryHookService { await this.permissionsService.userHasWorkspaceSettingPermission({ userWorkspaceId, workspaceId, - _setting: SettingPermissionType.WORKSPACE_MEMBERS, + setting: SettingPermissionType.WORKSPACE_MEMBERS, isExecutedByApiKey: isDefined(apiKey), }) ) {