From 2e015ee68dbeb2cc6cd8713b977ccbcf8333fd46 Mon Sep 17 00:00:00 2001 From: Thomas Trompette Date: Thu, 26 Mar 2026 15:20:05 +0100 Subject: [PATCH] Add missing row lvl permission check on Kanban view (#19002) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The Kanban view builds a query in two layers: - Inner query — selects actual records from the table (has all the permission context) - Outer query — wraps the inner query's raw SQL string to do grouping/pagination The problem: the inner query's SQL is copied out as a plain string before RLS predicates are added to it. RLS predicates are normally added lazily when you execute the query, but here the execution happens on the outer query — which doesn't know about the entity or its RLS rules. So RLS predicates are never applied anywhere. The fix: explicitly apply RLS predicates to the inner query before its SQL is extracted. Additonnaly, fixed a temporal issue in Datetime pickers. --- .../ObjectFilterDropdownDateTimeInput.tsx | 10 +- .../components/FormDateTimeFieldInput.tsx | 20 +- .../services/group-by-with-records.service.ts | 9 + ...up-by-with-records-rls.integration-spec.ts | 239 ++++++++++++++++++ 4 files changed, 269 insertions(+), 9 deletions(-) create mode 100644 packages/twenty-server/test/integration/graphql/suites/group-by-with-records-rls.integration-spec.ts diff --git a/packages/twenty-front/src/modules/object-record/object-filter-dropdown/components/ObjectFilterDropdownDateTimeInput.tsx b/packages/twenty-front/src/modules/object-record/object-filter-dropdown/components/ObjectFilterDropdownDateTimeInput.tsx index 78966e523a6..be621d98742 100644 --- a/packages/twenty-front/src/modules/object-record/object-filter-dropdown/components/ObjectFilterDropdownDateTimeInput.tsx +++ b/packages/twenty-front/src/modules/object-record/object-filter-dropdown/components/ObjectFilterDropdownDateTimeInput.tsx @@ -103,9 +103,13 @@ export const ObjectFilterDropdownDateTimeInput = () => { const internalZonedDateTime = !isRelativeDateFilter && isNonEmptyString(stringFilterValue) - ? Temporal.Instant.from(stringFilterValue).toZonedDateTimeISO( - timeZone ?? userTimezone, - ) + ? stringFilterValue.includes('T') + ? Temporal.Instant.from(stringFilterValue).toZonedDateTimeISO( + timeZone ?? userTimezone, + ) + : Temporal.PlainDate.from(stringFilterValue).toZonedDateTime( + timeZone ?? userTimezone, + ) : null; return ( diff --git a/packages/twenty-front/src/modules/object-record/record-field/ui/form-types/components/FormDateTimeFieldInput.tsx b/packages/twenty-front/src/modules/object-record/record-field/ui/form-types/components/FormDateTimeFieldInput.tsx index 152ac8dae9a..71c6589646a 100644 --- a/packages/twenty-front/src/modules/object-record/record-field/ui/form-types/components/FormDateTimeFieldInput.tsx +++ b/packages/twenty-front/src/modules/object-record/record-field/ui/form-types/components/FormDateTimeFieldInput.tsx @@ -228,13 +228,21 @@ export const FormDateTimeFieldInput = ({ const { userTimezone } = useUserTimezone(); - const dateValue = isStandaloneVariableString(defaultValue) - ? null - : defaultValue === 'null' || defaultValue === '' || !isDefined(defaultValue) + const isVariable = Boolean(isStandaloneVariableString(defaultValue)); + + const dateValue = + isVariable || + !isDefined(defaultValue) || + defaultValue === 'null' || + defaultValue === '' ? null - : Temporal.Instant.from(defaultValue).toZonedDateTimeISO( - timeZone ?? userTimezone, - ); + : defaultValue.includes('T') + ? Temporal.Instant.from(defaultValue).toZonedDateTimeISO( + timeZone ?? userTimezone, + ) + : Temporal.PlainDate.from(defaultValue).toZonedDateTime( + timeZone ?? userTimezone, + ); return ( diff --git a/packages/twenty-server/src/engine/api/graphql/graphql-query-runner/group-by/services/group-by-with-records.service.ts b/packages/twenty-server/src/engine/api/graphql/graphql-query-runner/group-by/services/group-by-with-records.service.ts index 6ab9e963c4e..bc0baf09dc3 100644 --- a/packages/twenty-server/src/engine/api/graphql/graphql-query-runner/group-by/services/group-by-with-records.service.ts +++ b/packages/twenty-server/src/engine/api/graphql/graphql-query-runner/group-by/services/group-by-with-records.service.ts @@ -23,6 +23,7 @@ import { type FlatEntityMaps } from 'src/engine/metadata-modules/flat-entity/typ import { type FlatFieldMetadata } from 'src/engine/metadata-modules/flat-field-metadata/types/flat-field-metadata.type'; import { type FlatObjectMetadata } from 'src/engine/metadata-modules/flat-object-metadata/types/flat-object-metadata.type'; import { type WorkspaceSelectQueryBuilder } from 'src/engine/twenty-orm/repository/workspace-select-query-builder'; +import { applyRowLevelPermissionPredicates } from 'src/engine/twenty-orm/utils/apply-row-level-permission-predicates.util'; import { type WorkspaceRepository } from 'src/engine/twenty-orm/repository/workspace.repository'; const RECORDS_PER_GROUP_LIMIT = 10; @@ -84,6 +85,14 @@ export class GroupByWithRecordsService { flatFieldMetadataMaps, }); + applyRowLevelPermissionPredicates({ + queryBuilder: queryBuilderWithFiltersAndWithoutGroupBy, + objectMetadata: flatObjectMetadata, + internalContext: queryBuilderWithFiltersAndWithoutGroupBy.internalContext, + authContext: queryBuilderWithFiltersAndWithoutGroupBy.authContext, + featureFlagMap: queryBuilderWithFiltersAndWithoutGroupBy.featureFlagMap, + }); + const queryBuilderWithPartitionBy = this.addPartitionByToQueryBuilder({ queryBuilderForSubQuery: queryBuilderWithFiltersAndWithoutGroupBy, columnsToSelect, diff --git a/packages/twenty-server/test/integration/graphql/suites/group-by-with-records-rls.integration-spec.ts b/packages/twenty-server/test/integration/graphql/suites/group-by-with-records-rls.integration-spec.ts new file mode 100644 index 00000000000..b83165a31f0 --- /dev/null +++ b/packages/twenty-server/test/integration/graphql/suites/group-by-with-records-rls.integration-spec.ts @@ -0,0 +1,239 @@ +import { randomUUID } from 'crypto'; + +import gql from 'graphql-tag'; +import { COMPANY_GQL_FIELDS } from 'test/integration/constants/company-gql-fields.constants'; +import { createOneOperationFactory } from 'test/integration/graphql/utils/create-one-operation-factory.util'; +import { destroyOneOperationFactory } from 'test/integration/graphql/utils/destroy-one-operation-factory.util'; +import { makeGraphqlAPIRequest } from 'test/integration/graphql/utils/make-graphql-api-request.util'; +import { findManyObjectMetadata } from 'test/integration/metadata/suites/object-metadata/utils/find-many-object-metadata.util'; +import { createOneRole } from 'test/integration/metadata/suites/role/utils/create-one-role.util'; +import { deleteOneRole } from 'test/integration/metadata/suites/role/utils/delete-one-role.util'; +import { findOneRoleByLabel } from 'test/integration/metadata/suites/role/utils/find-one-role-by-label.util'; +import { updateWorkspaceMemberRole } from 'test/integration/metadata/suites/role/utils/update-workspace-member-role.util'; +import { upsertRowLevelPermissionPredicates } from 'test/integration/metadata/suites/row-level-permission-predicate/utils/upsert-row-level-permission-predicates.util'; +import { updateFeatureFlag } from 'test/integration/metadata/suites/utils/update-feature-flag.util'; +import { jestExpectToBeDefined } from 'test/utils/jest-expect-to-be-defined.util.test'; +import { + FeatureFlagKey, + RowLevelPermissionPredicateOperand, +} from 'twenty-shared/types'; + +import { WORKSPACE_MEMBER_DATA_SEED_IDS } from 'src/engine/workspace-manager/dev-seeder/data/constants/workspace-member-data-seeds.constant'; + +const FILTER_2020 = { + and: [ + { createdAt: { gte: '2020-01-01T00:00:00.000Z' } }, + { createdAt: { lte: '2020-03-03T23:59:59.999Z' } }, + ], +}; + +describe('group-by with records respects row-level permission predicates', () => { + const testCompanyId1 = randomUUID(); + const testCompanyId2 = randomUUID(); + let customRoleId: string; + let originalMemberRoleId: string; + let companyObjectMetadataId: string; + let companyNameFieldMetadataId: string; + + beforeAll(async () => { + await updateFeatureFlag({ + featureFlag: FeatureFlagKey.IS_ROW_LEVEL_PERMISSION_PREDICATES_ENABLED, + value: true, + expectToFail: false, + }); + + const { objects } = await findManyObjectMetadata({ + expectToFail: false, + input: { + filter: {}, + paging: { first: 1000 }, + }, + gqlFields: ` + id + nameSingular + fieldsList { + id + name + } + `, + }); + + jestExpectToBeDefined(objects); + + const companyObjectMetadata = objects.find( + (object: { nameSingular: string }) => object.nameSingular === 'company', + ); + + jestExpectToBeDefined(companyObjectMetadata); + companyObjectMetadataId = companyObjectMetadata.id; + + const nameField = companyObjectMetadata.fieldsList?.find( + (field: { name: string }) => field.name === 'name', + ); + + jestExpectToBeDefined(nameField); + companyNameFieldMetadataId = nameField.id; + + const memberRole = await findOneRoleByLabel({ label: 'Member' }); + + originalMemberRoleId = memberRole.id; + + const { data: roleData } = await createOneRole({ + expectToFail: false, + input: { + label: 'RLS GroupBy Test Role', + description: 'Role for testing RLS in group-by with records', + icon: 'IconSettings', + canUpdateAllSettings: false, + canAccessAllTools: true, + canReadAllObjectRecords: true, + canUpdateAllObjectRecords: true, + canSoftDeleteAllObjectRecords: false, + canDestroyAllObjectRecords: false, + canBeAssignedToUsers: true, + canBeAssignedToAgents: false, + canBeAssignedToApiKeys: false, + }, + }); + + customRoleId = roleData?.createOneRole?.id; + jestExpectToBeDefined(customRoleId); + + await upsertRowLevelPermissionPredicates({ + expectToFail: false, + input: { + roleId: customRoleId, + objectMetadataId: companyObjectMetadataId, + predicates: [ + { + fieldMetadataId: companyNameFieldMetadataId, + operand: RowLevelPermissionPredicateOperand.CONTAINS, + value: 'Visible', + }, + ], + predicateGroups: [], + }, + }); + + await updateWorkspaceMemberRole({ + input: { + roleId: customRoleId, + workspaceMemberId: WORKSPACE_MEMBER_DATA_SEED_IDS.JONY, + }, + expectToFail: false, + }); + + await makeGraphqlAPIRequest( + createOneOperationFactory({ + objectMetadataSingularName: 'company', + gqlFields: COMPANY_GQL_FIELDS, + data: { + id: testCompanyId1, + name: 'RLS Visible Company', + employees: 99, + createdAt: '2020-02-05T08:00:00.000Z', + }, + }), + ); + + await makeGraphqlAPIRequest( + createOneOperationFactory({ + objectMetadataSingularName: 'company', + gqlFields: COMPANY_GQL_FIELDS, + data: { + id: testCompanyId2, + name: 'RLS Hidden Company', + employees: 99, + createdAt: '2020-02-05T08:00:00.000Z', + }, + }), + ); + }); + + afterAll(async () => { + await updateWorkspaceMemberRole({ + input: { + workspaceMemberId: WORKSPACE_MEMBER_DATA_SEED_IDS.JONY, + roleId: originalMemberRoleId, + }, + expectToFail: false, + }); + + for (const id of [testCompanyId1, testCompanyId2]) { + await makeGraphqlAPIRequest( + destroyOneOperationFactory({ + objectMetadataSingularName: 'company', + gqlFields: 'id', + recordId: id, + }), + ); + } + + if (customRoleId) { + await deleteOneRole({ + expectToFail: false, + input: { idToDelete: customRoleId }, + }); + } + + await updateFeatureFlag({ + featureFlag: FeatureFlagKey.IS_ROW_LEVEL_PERMISSION_PREDICATES_ENABLED, + value: false, + expectToFail: false, + }); + }); + + it('filters records in group-by results based on RLS predicates', async () => { + const response = await makeGraphqlAPIRequest( + { + query: gql` + query CompaniesGroupBy( + $groupBy: [CompanyGroupByInput!]! + $filter: CompanyFilterInput + $limit: Int + ) { + companiesGroupBy( + groupBy: $groupBy + filter: $filter + limit: $limit + ) { + groupByDimensionValues + edges { + node { + name + employees + } + } + } + } + `, + variables: { + groupBy: [{ employees: true }], + filter: FILTER_2020, + limit: 10, + }, + }, + APPLE_JONY_MEMBER_ACCESS_TOKEN, + ); + + expect(response.body.errors).toBeUndefined(); + expect(response.body.data).toBeDefined(); + + const groups = response.body.data.companiesGroupBy; + + const allRecords = groups.flatMap( + (group: { edges: { node: { name: string } }[] }) => + group.edges.map((edge: { node: { name: string } }) => edge.node), + ); + + const visibleRecords = allRecords.filter( + (record: { name: string }) => record.name === 'RLS Visible Company', + ); + const hiddenRecords = allRecords.filter( + (record: { name: string }) => record.name === 'RLS Hidden Company', + ); + + expect(visibleRecords).toHaveLength(1); + expect(hiddenRecords).toHaveLength(0); + }); +});