From da5ae341096f518fbb68cd1a16309b6bce0ff0be Mon Sep 17 00:00:00 2001 From: Marie <51697796+ijreilly@users.noreply.github.com> Date: Wed, 18 Jun 2025 17:12:58 +0200 Subject: [PATCH] [permissions] Filter tabs + registered actions according to permissions (#12657) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Note and task tabs in side panel should only show if user has reading permission on them. "Go to companies", "Go to workflows", etc. in command menu should only show is user has reading permission on related objects. Capture d’écran 2025-06-17 à 11 09 50 Capture d’écran 2025-06-17 à 11 09 56 **How to test** Assign a user with a custom role that has **no** read permissions on notes/tasks/workflows/companies/opportunities/people (no need to test them all but at least one between note and tasks; workflows; one between companies/opportunities/people). Check that you don't see the related tab / action. --------- Co-authored-by: Charles Bochet --- .../constants/DefaultRecordActionsConfig.tsx | 65 ++++++++++++++----- .../types/ShouldBeRegisteredFunctionParams.ts | 3 + .../actions/utils/getActionConfig.ts | 17 +++-- .../action-menu/hooks/useRegisteredActions.ts | 4 +- .../useShouldActionBeRegisteredParams.ts | 18 ++++- .../states/objectPermissionsFamilySelector.ts | 37 +++++++++++ .../__stories__/CommandMenu.stories.tsx | 31 +++++++++ .../record-show/constants/BaseRecordLayout.ts | 4 ++ .../hooks/useRecordShowContainerTabs.ts | 26 ++++++-- .../layout/tab-list/types/RecordLayoutTab.ts | 1 + .../tab-list/types/TabVisibilityConfig.ts | 1 + .../src/testing/mock-data/users.ts | 34 ++++++++-- 12 files changed, 206 insertions(+), 35 deletions(-) create mode 100644 packages/twenty-front/src/modules/auth/states/objectPermissionsFamilySelector.ts diff --git a/packages/twenty-front/src/modules/action-menu/actions/record-actions/constants/DefaultRecordActionsConfig.tsx b/packages/twenty-front/src/modules/action-menu/actions/record-actions/constants/DefaultRecordActionsConfig.tsx index c0ed019ca..f3f5b31b9 100644 --- a/packages/twenty-front/src/modules/action-menu/actions/record-actions/constants/DefaultRecordActionsConfig.tsx +++ b/packages/twenty-front/src/modules/action-menu/actions/record-actions/constants/DefaultRecordActionsConfig.tsx @@ -410,7 +410,13 @@ export const DEFAULT_RECORD_ACTIONS_CONFIG: Record< Icon: IconSettingsAutomation, accent: 'default', isPinned: false, - shouldBeRegistered: ({ objectMetadataItem, viewType, isWorkflowEnabled }) => + shouldBeRegistered: ({ + objectMetadataItem, + viewType, + isWorkflowEnabled, + getTargetObjectReadPermission, + }) => + getTargetObjectReadPermission(CoreObjectNameSingular.Workflow) === true && (objectMetadataItem?.nameSingular !== CoreObjectNameSingular.Workflow || viewType === ActionViewType.SHOW_PAGE) && isWorkflowEnabled, @@ -443,9 +449,14 @@ export const DEFAULT_RECORD_ACTIONS_CONFIG: Record< ActionViewType.INDEX_PAGE_BULK_SELECTION, ActionViewType.SHOW_PAGE, ], - shouldBeRegistered: ({ objectMetadataItem, viewType }) => - objectMetadataItem?.nameSingular !== CoreObjectNameSingular.Person || - viewType === ActionViewType.SHOW_PAGE, + shouldBeRegistered: ({ + objectMetadataItem, + viewType, + getTargetObjectReadPermission, + }) => + getTargetObjectReadPermission(CoreObjectNameSingular.Person) === true && + (objectMetadataItem?.nameSingular !== CoreObjectNameSingular.Person || + viewType === ActionViewType.SHOW_PAGE), component: ( - objectMetadataItem?.nameSingular !== CoreObjectNameSingular.Company || - viewType === ActionViewType.SHOW_PAGE, + shouldBeRegistered: ({ + objectMetadataItem, + viewType, + getTargetObjectReadPermission, + }) => + getTargetObjectReadPermission(CoreObjectNameSingular.Company) === true && + (objectMetadataItem?.nameSingular !== CoreObjectNameSingular.Company || + viewType === ActionViewType.SHOW_PAGE), component: ( - objectMetadataItem?.nameSingular !== CoreObjectNameSingular.Opportunity || - viewType === ActionViewType.SHOW_PAGE, + shouldBeRegistered: ({ + objectMetadataItem, + viewType, + getTargetObjectReadPermission, + }) => + getTargetObjectReadPermission(CoreObjectNameSingular.Opportunity) === + true && + (objectMetadataItem?.nameSingular !== + CoreObjectNameSingular.Opportunity || + viewType === ActionViewType.SHOW_PAGE), component: ( - objectMetadataItem?.nameSingular !== CoreObjectNameSingular.Task || - viewType === ActionViewType.SHOW_PAGE, + shouldBeRegistered: ({ + objectMetadataItem, + viewType, + getTargetObjectReadPermission, + }) => + getTargetObjectReadPermission(CoreObjectNameSingular.Task) === true && + (objectMetadataItem?.nameSingular !== CoreObjectNameSingular.Task || + viewType === ActionViewType.SHOW_PAGE), component: ( - objectMetadataItem?.nameSingular !== CoreObjectNameSingular.Note || - viewType === ActionViewType.SHOW_PAGE, + shouldBeRegistered: ({ + objectMetadataItem, + viewType, + getTargetObjectReadPermission, + }) => + getTargetObjectReadPermission(CoreObjectNameSingular.Note) === true && + (objectMetadataItem?.nameSingular !== CoreObjectNameSingular.Note || + viewType === ActionViewType.SHOW_PAGE), component: ( boolean; }; diff --git a/packages/twenty-front/src/modules/action-menu/actions/utils/getActionConfig.ts b/packages/twenty-front/src/modules/action-menu/actions/utils/getActionConfig.ts index 145ef834e..30dc8644c 100644 --- a/packages/twenty-front/src/modules/action-menu/actions/utils/getActionConfig.ts +++ b/packages/twenty-front/src/modules/action-menu/actions/utils/getActionConfig.ts @@ -7,20 +7,25 @@ import { CoreObjectNameSingular } from '@/object-metadata/types/CoreObjectNameSi import { ObjectMetadataItem } from '@/object-metadata/types/ObjectMetadataItem'; import { isDefined } from 'twenty-shared/utils'; -export const getActionConfig = ( - objectMetadataItem?: ObjectMetadataItem, -): Record => { +export const getActionConfig = ({ + objectMetadataItem, +}: { + objectMetadataItem?: ObjectMetadataItem; +}): Record => { if (!isDefined(objectMetadataItem)) { return {}; } switch (objectMetadataItem.nameSingular) { - case CoreObjectNameSingular.Workflow: + case CoreObjectNameSingular.Workflow: { return WORKFLOW_ACTIONS_CONFIG; - case CoreObjectNameSingular.WorkflowVersion: + } + case CoreObjectNameSingular.WorkflowVersion: { return WORKFLOW_VERSIONS_ACTIONS_CONFIG; - case CoreObjectNameSingular.WorkflowRun: + } + case CoreObjectNameSingular.WorkflowRun: { return WORKFLOW_RUNS_ACTIONS_CONFIG; + } default: return DEFAULT_RECORD_ACTIONS_CONFIG; } diff --git a/packages/twenty-front/src/modules/action-menu/hooks/useRegisteredActions.ts b/packages/twenty-front/src/modules/action-menu/hooks/useRegisteredActions.ts index add0872a5..658b04d95 100644 --- a/packages/twenty-front/src/modules/action-menu/hooks/useRegisteredActions.ts +++ b/packages/twenty-front/src/modules/action-menu/hooks/useRegisteredActions.ts @@ -26,7 +26,9 @@ export const useRegisteredActions = ( contextStoreTargetedRecordsRule, ); - const recordActionConfig = getActionConfig(objectMetadataItem); + const recordActionConfig = getActionConfig({ + objectMetadataItem, + }); const recordAgnosticActionConfig = RECORD_AGNOSTIC_ACTIONS_CONFIG; diff --git a/packages/twenty-front/src/modules/action-menu/hooks/useShouldActionBeRegisteredParams.ts b/packages/twenty-front/src/modules/action-menu/hooks/useShouldActionBeRegisteredParams.ts index 9e9647907..484af1567 100644 --- a/packages/twenty-front/src/modules/action-menu/hooks/useShouldActionBeRegisteredParams.ts +++ b/packages/twenty-front/src/modules/action-menu/hooks/useShouldActionBeRegisteredParams.ts @@ -1,6 +1,7 @@ import { ShouldBeRegisteredFunctionParams } from '@/action-menu/actions/types/ShouldBeRegisteredFunctionParams'; import { getActionViewType } from '@/action-menu/actions/utils/getActionViewType'; import { ActionMenuContext } from '@/action-menu/contexts/ActionMenuContext'; +import { objectPermissionsFamilySelector } from '@/auth/states/objectPermissionsFamilySelector'; import { contextStoreCurrentViewTypeComponentState } from '@/context-store/states/contextStoreCurrentViewTypeComponentState'; import { contextStoreNumberOfSelectedRecordsComponentState } from '@/context-store/states/contextStoreNumberOfSelectedRecordsComponentState'; import { contextStoreTargetedRecordsRuleComponentState } from '@/context-store/states/contextStoreTargetedRecordsRuleComponentState'; @@ -14,7 +15,7 @@ import { isSoftDeleteFilterActiveComponentState } from '@/object-record/record-t import { useRecoilComponentValueV2 } from '@/ui/utilities/state/component-state/hooks/useRecoilComponentValueV2'; import { useIsFeatureEnabled } from '@/workspace/hooks/useIsFeatureEnabled'; import { useContext } from 'react'; -import { useRecoilValue } from 'recoil'; +import { useRecoilCallback, useRecoilValue } from 'recoil'; import { FeatureFlagKey } from '~/generated-metadata/graphql'; export const useShouldActionBeRegisteredParams = ({ @@ -77,6 +78,20 @@ export const useShouldActionBeRegisteredParams = ({ contextStoreTargetedRecordsRule, ); + const getObjectReadPermission = useRecoilCallback( + ({ snapshot }) => + (objectMetadataNameSingular: string) => { + return snapshot + .getLoadable( + objectPermissionsFamilySelector({ + objectNameSingular: objectMetadataNameSingular, + }), + ) + .getValue().canRead; + }, + [], + ); + return { objectMetadataItem, isFavorite, @@ -89,5 +104,6 @@ export const useShouldActionBeRegisteredParams = ({ isWorkflowEnabled, numberOfSelectedRecords, viewType: viewType ?? undefined, + getTargetObjectReadPermission: getObjectReadPermission, }; }; diff --git a/packages/twenty-front/src/modules/auth/states/objectPermissionsFamilySelector.ts b/packages/twenty-front/src/modules/auth/states/objectPermissionsFamilySelector.ts new file mode 100644 index 000000000..8a292a7d8 --- /dev/null +++ b/packages/twenty-front/src/modules/auth/states/objectPermissionsFamilySelector.ts @@ -0,0 +1,37 @@ +import { currentUserWorkspaceState } from '@/auth/states/currentUserWorkspaceState'; +import { objectMetadataItemsState } from '@/object-metadata/states/objectMetadataItemsState'; +import { selectorFamily } from 'recoil'; + +export const objectPermissionsFamilySelector = selectorFamily< + { + canRead: boolean; + }, + { objectNameSingular: string } +>({ + key: 'objectPermissionsFamilySelector', + get: + ({ objectNameSingular }) => + ({ get }) => { + const currentUserWorkspace = get(currentUserWorkspaceState); + const objectMetadataItems = get(objectMetadataItemsState); + + const objectMetadataItem = objectMetadataItems.find( + (item) => item.nameSingular === objectNameSingular, + ); + + if (!objectMetadataItem) { + return { + canRead: false, + canUpdate: false, + }; + } + + const objectPermissions = currentUserWorkspace?.objectPermissions?.find( + (permission) => permission.objectMetadataId === objectMetadataItem.id, + ); + + return { + canRead: objectPermissions?.canReadObjectRecords ?? false, + }; + }, +}); diff --git a/packages/twenty-front/src/modules/command-menu/components/__stories__/CommandMenu.stories.tsx b/packages/twenty-front/src/modules/command-menu/components/__stories__/CommandMenu.stories.tsx index 5abf9fc14..6e284e892 100644 --- a/packages/twenty-front/src/modules/command-menu/components/__stories__/CommandMenu.stories.tsx +++ b/packages/twenty-front/src/modules/command-menu/components/__stories__/CommandMenu.stories.tsx @@ -10,11 +10,14 @@ import { SnackBarDecorator } from '~/testing/decorators/SnackBarDecorator'; import { graphqlMocks } from '~/testing/graphqlMocks'; import { mockCurrentWorkspace, + mockedLimitedPermissionsUserData, + mockedUserData, mockedWorkspaceMemberData, } from '~/testing/mock-data/users'; import { sleep } from '~/utils/sleep'; import { ActionMenuComponentInstanceContext } from '@/action-menu/states/contexts/ActionMenuComponentInstanceContext'; +import { currentUserWorkspaceState } from '@/auth/states/currentUserWorkspaceState'; import { CommandMenuRouter } from '@/command-menu/components/CommandMenuRouter'; import { COMMAND_MENU_COMPONENT_INSTANCE_ID } from '@/command-menu/constants/CommandMenuComponentInstanceId'; import { commandMenuNavigationStackState } from '@/command-menu/states/commandMenuNavigationStackState'; @@ -72,6 +75,9 @@ const meta: Meta = { I18nFrontDecorator, (Story) => { const setCurrentWorkspace = useSetRecoilState(currentWorkspaceState); + const setCurrentUserWorkspace = useSetRecoilState( + currentUserWorkspaceState, + ); const setCurrentWorkspaceMember = useSetRecoilState( currentWorkspaceMemberState, ); @@ -84,6 +90,8 @@ const meta: Meta = { setCurrentWorkspace(mockCurrentWorkspace); setCurrentWorkspaceMember(mockedWorkspaceMemberData); + setCurrentUserWorkspace(mockedUserData.currentUserWorkspace); + setIsCommandMenuOpened(true); setCommandMenuNavigationStack([ { @@ -122,6 +130,29 @@ export const DefaultWithoutSearch: Story = { }, }; +export const LimitedPermissions: Story = { + play: async () => { + const canvas = within(document.body); + await expect(canvas.findByText('Go to Opportunities')).rejects.toThrow(); + await expect(canvas.findByText('Go to Tasks')).rejects.toThrow(); + expect(await canvas.findByText('Go to People')).toBeVisible(); + expect(await canvas.findByText('Go to Settings')).toBeVisible(); + expect(await canvas.findByText('Go to Notes')).toBeVisible(); + }, + decorators: [ + (Story) => { + const setCurrentUserWorkspace = useSetRecoilState( + currentUserWorkspaceState, + ); + setCurrentUserWorkspace( + mockedLimitedPermissionsUserData.currentUserWorkspace, + ); + + return ; + }, + ], +}; + export const MatchingNavigate: Story = { play: async () => { const canvas = within(document.body); diff --git a/packages/twenty-front/src/modules/object-record/record-show/constants/BaseRecordLayout.ts b/packages/twenty-front/src/modules/object-record/record-show/constants/BaseRecordLayout.ts index 3892da683..7091608a8 100644 --- a/packages/twenty-front/src/modules/object-record/record-show/constants/BaseRecordLayout.ts +++ b/packages/twenty-front/src/modules/object-record/record-show/constants/BaseRecordLayout.ts @@ -44,6 +44,7 @@ export const BASE_RECORD_LAYOUT: RecordLayout = { Icon: IconCheckbox, position: 300, cards: [{ type: CardType.TaskCard }], + targetObjectNameSingular: CoreObjectNameSingular.Task, hide: { ifMobile: false, ifDesktop: false, @@ -51,6 +52,7 @@ export const BASE_RECORD_LAYOUT: RecordLayout = { ifFeaturesDisabled: [], ifRequiredObjectsInactive: [CoreObjectNameSingular.Task], ifRelationsMissing: ['taskTargets'], + ifNoReadPermission: true, }, }, notes: { @@ -58,6 +60,7 @@ export const BASE_RECORD_LAYOUT: RecordLayout = { Icon: IconNotes, position: 400, cards: [{ type: CardType.NoteCard }], + targetObjectNameSingular: CoreObjectNameSingular.Note, hide: { ifMobile: false, ifDesktop: false, @@ -65,6 +68,7 @@ export const BASE_RECORD_LAYOUT: RecordLayout = { ifFeaturesDisabled: [], ifRequiredObjectsInactive: [CoreObjectNameSingular.Note], ifRelationsMissing: ['noteTargets'], + ifNoReadPermission: true, }, }, files: { diff --git a/packages/twenty-front/src/modules/object-record/record-show/hooks/useRecordShowContainerTabs.ts b/packages/twenty-front/src/modules/object-record/record-show/hooks/useRecordShowContainerTabs.ts index 8cbf1e8ca..e78755e5a 100644 --- a/packages/twenty-front/src/modules/object-record/record-show/hooks/useRecordShowContainerTabs.ts +++ b/packages/twenty-front/src/modules/object-record/record-show/hooks/useRecordShowContainerTabs.ts @@ -2,6 +2,7 @@ import { currentWorkspaceState } from '@/auth/states/currentWorkspaceState'; import { objectMetadataItemsState } from '@/object-metadata/states/objectMetadataItemsState'; import { CoreObjectNameSingular } from '@/object-metadata/types/CoreObjectNameSingular'; import { ObjectMetadataItem } from '@/object-metadata/types/ObjectMetadataItem'; +import { useObjectPermissions } from '@/object-record/hooks/useObjectPermissions'; import { BASE_RECORD_LAYOUT } from '@/object-record/record-show/constants/BaseRecordLayout'; import { CardType } from '@/object-record/record-show/types/CardType'; import { RecordLayout } from '@/object-record/record-show/types/RecordLayout'; @@ -10,6 +11,7 @@ import { SingleTabProps } from '@/ui/layout/tab-list/types/SingleTabProps'; import { useIsMobile } from '@/ui/utilities/responsive/hooks/useIsMobile'; import { useMemo } from 'react'; import { useRecoilValue } from 'recoil'; +import { isDefined } from 'twenty-shared/utils'; import { IconCalendarEvent, IconHome, @@ -30,6 +32,7 @@ export const useRecordShowContainerTabs = ( const objectMetadataItems = useRecoilValue(objectMetadataItemsState); const currentWorkspace = useRecoilValue(currentWorkspaceState); + const { objectPermissionsByObjectMetadataId } = useObjectPermissions(); // Object-specific layouts that override or extend the base layout const OBJECT_SPECIFIC_LAYOUTS: Partial< @@ -212,17 +215,19 @@ export const useRecordShowContainerTabs = ( [], ); + const baseRecordLayout = BASE_RECORD_LAYOUT; + // Merge base layout with object-specific layout const recordLayout: RecordLayout = useMemo(() => { return { - ...BASE_RECORD_LAYOUT, + ...baseRecordLayout, ...(OBJECT_SPECIFIC_LAYOUTS[targetObjectNameSingular] || {}), tabs: { - ...BASE_RECORD_LAYOUT.tabs, + ...baseRecordLayout.tabs, ...(OBJECT_SPECIFIC_LAYOUTS[targetObjectNameSingular]?.tabs || {}), }, }; - }, [OBJECT_SPECIFIC_LAYOUTS, targetObjectNameSingular]); + }, [OBJECT_SPECIFIC_LAYOUTS, baseRecordLayout, targetObjectNameSingular]); return { layout: recordLayout, @@ -232,7 +237,7 @@ export const useRecordShowContainerTabs = ( entry[1] !== null && entry[1] !== undefined, ) .sort(([, a], [, b]) => a.position - b.position) - .map(([key, { title, Icon, hide, cards }]) => { + .map(([key, { title, Icon, hide, cards, targetObjectNameSingular }]) => { // Special handling for fields tab if (key === 'fields') { return { @@ -257,6 +262,16 @@ export const useRecordShowContainerTabs = ( ); }); + const targetObjectMetadataId = objectMetadataItems.find( + (item) => item.nameSingular === targetObjectNameSingular, + )?.id; + + const permissionHide = + hide.ifNoReadPermission && + isDefined(targetObjectNameSingular) && + !objectPermissionsByObjectMetadataId[targetObjectMetadataId] + ?.canReadObjectRecords; + const requiredObjectsInactive = hide.ifRequiredObjectsInactive.length > 0 && !hide.ifRequiredObjectsInactive.every((obj) => @@ -286,7 +301,8 @@ export const useRecordShowContainerTabs = ( baseHide || featureNotEnabled || requiredObjectsInactive || - relationsDontExist, + relationsDontExist || + permissionHide, }; }) // When isInRightDrawer === true, we merge first and second tab into first tab diff --git a/packages/twenty-front/src/modules/ui/layout/tab-list/types/RecordLayoutTab.ts b/packages/twenty-front/src/modules/ui/layout/tab-list/types/RecordLayoutTab.ts index c4cdc2831..d23e71e3b 100644 --- a/packages/twenty-front/src/modules/ui/layout/tab-list/types/RecordLayoutTab.ts +++ b/packages/twenty-front/src/modules/ui/layout/tab-list/types/RecordLayoutTab.ts @@ -8,4 +8,5 @@ export type RecordLayoutTab = { Icon: IconComponent; hide: TabVisibilityConfig; cards: LayoutCard[]; + targetObjectNameSingular?: string; }; diff --git a/packages/twenty-front/src/modules/ui/layout/tab-list/types/TabVisibilityConfig.ts b/packages/twenty-front/src/modules/ui/layout/tab-list/types/TabVisibilityConfig.ts index cc7fd55d3..713d4b852 100644 --- a/packages/twenty-front/src/modules/ui/layout/tab-list/types/TabVisibilityConfig.ts +++ b/packages/twenty-front/src/modules/ui/layout/tab-list/types/TabVisibilityConfig.ts @@ -8,4 +8,5 @@ export type TabVisibilityConfig = { ifFeaturesDisabled: FeatureFlagKey[]; ifRequiredObjectsInactive: CoreObjectNameSingular[]; ifRelationsMissing: string[]; + ifNoReadPermission?: boolean; }; diff --git a/packages/twenty-front/src/testing/mock-data/users.ts b/packages/twenty-front/src/testing/mock-data/users.ts index 6f174bcee..e6da563ae 100644 --- a/packages/twenty-front/src/testing/mock-data/users.ts +++ b/packages/twenty-front/src/testing/mock-data/users.ts @@ -12,6 +12,7 @@ import { WorkspaceMemberDateFormatEnum, WorkspaceMemberTimeFormatEnum, } from '~/generated/graphql'; +import { generatedMockObjectMetadataItems } from '~/testing/mock-data/generatedMockObjectMetadataItems'; type MockedUser = Pick< User, @@ -128,12 +129,13 @@ export const mockedUserData: MockedUser = { currentWorkspace: mockCurrentWorkspace, currentUserWorkspace: { settingsPermissions: [SettingPermissionType.WORKSPACE_MEMBERS], - objectPermissions: [ - { - objectMetadataId: '4a45f524-b8cb-40e8-8450-28e402b442cf', - canReadObjectRecords: true, - }, - ], + objectPermissions: generatedMockObjectMetadataItems.map((item) => ({ + objectMetadataId: item.id, + canReadObjectRecords: true, + canUpdateObjectRecords: true, + canSoftDeleteObjectRecords: true, + canDestroyObjectRecords: true, + })), }, locale: 'en', workspaces: [{ workspace: mockCurrentWorkspace }], @@ -146,6 +148,26 @@ export const mockedUserData: MockedUser = { userVars: {}, }; +export const mockedLimitedPermissionsUserData: MockedUser = { + ...mockedUserData, + currentUserWorkspace: { + ...mockedUserData.currentUserWorkspace, + objectPermissions: generatedMockObjectMetadataItems + .filter( + (objectMetadata) => + objectMetadata.nameSingular !== 'task' && + objectMetadata.nameSingular !== 'opportunity', + ) + .map((item) => ({ + objectMetadataId: item.id, + canReadObjectRecords: true, + canUpdateObjectRecords: true, + canSoftDeleteObjectRecords: true, + canDestroyObjectRecords: true, + })), + }, +}; + export const mockedOnboardingUserData = ( onboardingStatus?: OnboardingStatus, ) => {