From a26b50736165908b2fca9229330843f60bf1a6df Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rapha=C3=ABl=20Bosi?= <71827178+bosiraphael@users.noreply.github.com> Date: Tue, 4 Aug 2026 11:43:45 +0200 Subject: [PATCH] Enforce row-level permissions on joined relations (#23369) Row-level permission predicates were only ever applied to a query's main alias, so any SQL join leaked rows the caller is not allowed to see. The visible symptom: dashboard charts grouped by a relation field (e.g. Opportunities by Company) read the group dimension off an unfiltered joined table, surfacing hidden companies as chart labels. `WorkspaceSelectQueryBuilder` now applies the joined object's predicate to every relation join's `ON` condition. Using `ON` rather than `WHERE` keeps left-join semantics correct: a visible record linked to a hidden related row is still counted, it just falls into the null group instead of being attributed to the hidden row. This closes the same class of leak in relation filters and order-by-on-relation, plus the three paths that serialize a builder via `.getQuery()` and never reach the execution overrides (group-by with records, per-parent relation limiting, mutation id subqueries). The per-parent fix also stops hidden rows from consuming `LIMIT` slots before being filtered out. ```mermaid flowchart TD A["WorkspaceSelectQueryBuilder
SELECT FROM person LEFT JOIN company"] A --> B["getMany / getOne / getCount / execute
(execution overrides)"] A --> C["getQuery() serialization:
group-by with records,
per-parent relation limit,
mutation id subquery"] B --> D["validatePermissions()"] D --> E["applyRowLevelPermissionPredicates
ToMainAliasAndJoinedRelations()"] C --> E E --> F["main alias:
WHERE person predicate"] E --> G["every relation join:
ON person.companyId = company.id
AND company predicate"] F --> H["hidden companies never surface as group dimensions,
relation-filter matches or sort keys;
a hidden link sorts as NULL and the row is still counted"] G --> H ``` The last two commits remove the duplication this fix would otherwise have introduced: one shared `and`/`or`/`not` filter walker (the GraphQL filter parser and the RLS util were verbatim forks), one RLS record-filter resolver used by all three call sites, and one shared set of RLS integration-test fixtures. Behaviour-preserving, with new characterization tests pinning the emitted condition tree. Reviewer notes: - Results change where a join is involved: relation filters no longer match hidden related records, and order-by-on-relation sorts hidden-linked rows as null, which can shift pagination. - Joins on subqueries/custom tables are skipped, and objects with no predicates for the role are a no-op, so admins and system contexts are unaffected. - Timeline messaging inner joins are filtered too, so thread counts can change for restricted roles. - Predicates that need the current workspace member (Me) are still skipped for API key and application contexts, on joins as on the main alias. - The join renderer skips the field-level read-permission check the main-alias parser performs: predicates on read-restricted fields still filter joins, and the field values are never selected. - One user-facing change beyond the leak fix: the empty-array filter error no longer echoes the submitted value back (`Invalid filter value: ""` -> `Invalid filter value`), on every filter path rather than just RLS. Catalogs are not regenerated here, so it falls back to English until the next i18n sync. Review in cubic --- .../process-nested-relations-v2.helper.ts | 12 +- .../build-mutation-query-builder.util.ts | 2 + ...phql-query-filter-condition.parser.spec.ts | 268 ++++++++++++++ .../graphql-query-filter-condition.parser.ts | 135 +------ .../graphql-query-filter-field.parser.ts | 50 +-- ...filter-entries-to-where-expression.util.ts | 131 +++++++ .../resolve-filter-key-field-metadata.util.ts | 34 ++ .../services/group-by-with-records.service.ts | 11 +- ...-operator-value-is-non-empty-array.util.ts | 29 ++ .../workspace-select-query-builder.ts | 121 ++++++- ...w-level-permission-predicates.util.spec.ts | 147 ++++++++ ...evel-permission-filter-to-sql.util.spec.ts | 334 ++++++++++++++++++ ...ly-row-level-permission-predicates.util.ts | 190 +--------- ...row-level-permission-filter-to-sql.util.ts | 276 +++++++++++++++ ...row-level-permission-record-filter.util.ts | 46 +++ ...alidate-rls-predicates-for-records.util.ts | 29 +- ...oup-by-by-relation-rls.integration-spec.ts | 135 +++++++ ...up-by-with-records-rls.integration-spec.ts | 169 ++------- ...on-relation-filter-rls.integration-spec.ts | 73 ++++ ...n-per-parent-limit-rls.integration-spec.ts | 137 +++++++ .../relation-join-rls.integration-spec.ts | 107 ++++++ .../utils/group-by-operation-factory.util.ts | 7 +- .../utils/setup-company-name-rls-role.util.ts | 86 +++++ ...setup-rls-company-relation-records.util.ts | 110 ++++++ .../upsert-contains-rls-predicate.util.ts | 59 ++++ .../create-where-expression-recorder.util.ts | 64 ++++ 26 files changed, 2253 insertions(+), 509 deletions(-) create mode 100644 packages/twenty-server/src/engine/api/graphql/graphql-query-runner/graphql-query-parsers/graphql-query-filter/__tests__/graphql-query-filter-condition.parser.spec.ts create mode 100644 packages/twenty-server/src/engine/api/graphql/graphql-query-runner/graphql-query-parsers/utils/apply-filter-entries-to-where-expression.util.ts create mode 100644 packages/twenty-server/src/engine/api/graphql/graphql-query-runner/graphql-query-parsers/utils/resolve-filter-key-field-metadata.util.ts create mode 100644 packages/twenty-server/src/engine/api/graphql/graphql-query-runner/utils/assert-array-operator-value-is-non-empty-array.util.ts create mode 100644 packages/twenty-server/src/engine/twenty-orm/utils/__tests__/apply-row-level-permission-predicates.util.spec.ts create mode 100644 packages/twenty-server/src/engine/twenty-orm/utils/__tests__/render-row-level-permission-filter-to-sql.util.spec.ts create mode 100644 packages/twenty-server/src/engine/twenty-orm/utils/render-row-level-permission-filter-to-sql.util.ts create mode 100644 packages/twenty-server/src/engine/twenty-orm/utils/resolve-row-level-permission-record-filter.util.ts create mode 100644 packages/twenty-server/test/integration/graphql/suites/group-by-by-relation-rls.integration-spec.ts create mode 100644 packages/twenty-server/test/integration/graphql/suites/mutation-relation-filter-rls.integration-spec.ts create mode 100644 packages/twenty-server/test/integration/graphql/suites/nested-relation-per-parent-limit-rls.integration-spec.ts create mode 100644 packages/twenty-server/test/integration/graphql/suites/relation-join-rls.integration-spec.ts create mode 100644 packages/twenty-server/test/integration/graphql/utils/setup-company-name-rls-role.util.ts create mode 100644 packages/twenty-server/test/integration/graphql/utils/setup-rls-company-relation-records.util.ts create mode 100644 packages/twenty-server/test/integration/graphql/utils/upsert-contains-rls-predicate.util.ts create mode 100644 packages/twenty-server/test/utils/create-where-expression-recorder.util.ts diff --git a/packages/twenty-server/src/engine/api/common/common-nested-relations-processor/process-nested-relations-v2.helper.ts b/packages/twenty-server/src/engine/api/common/common-nested-relations-processor/process-nested-relations-v2.helper.ts index 5e4f23e915..adb2cb3aee 100644 --- a/packages/twenty-server/src/engine/api/common/common-nested-relations-processor/process-nested-relations-v2.helper.ts +++ b/packages/twenty-server/src/engine/api/common/common-nested-relations-processor/process-nested-relations-v2.helper.ts @@ -501,12 +501,15 @@ export class ProcessNestedRelationsV2Helper { return []; } - const perParentRecordIdsSql = targetObjectRepository + const perParentRecordIdsQueryBuilder = targetObjectRepository .createQueryBuilder(targetObjectNameSingular) .select('id', 'id') .where(`${column} = "lateralParents"."parentId"`) - .limit(perParentLimit) - .getQuery(); + .limit(perParentLimit); + + perParentRecordIdsQueryBuilder.applyRowLevelPermissionPredicatesToMainAliasAndJoinedRelations(); + + const perParentRecordIdsSql = perParentRecordIdsQueryBuilder.getQuery(); const parentValues = sanitizedIds.map((id) => `('${id}'::uuid)`).join(', '); @@ -518,7 +521,8 @@ export class ProcessNestedRelationsV2Helper { const limitedRecordsQueryBuilder = targetObjectRepository .createQueryBuilder() .from(lateralFromSubquery, 'limited_relation_records') - .select('limited_relation_records.id', 'id'); + .select('limited_relation_records.id', 'id') + .setParameters(perParentRecordIdsQueryBuilder.getParameters()); limitedRecordsQueryBuilder.expressionMap.aliases = limitedRecordsQueryBuilder.expressionMap.aliases.filter((alias) => diff --git a/packages/twenty-server/src/engine/api/common/common-query-runners/utils/build-mutation-query-builder.util.ts b/packages/twenty-server/src/engine/api/common/common-query-runners/utils/build-mutation-query-builder.util.ts index 384b8328ff..7f52fd5932 100644 --- a/packages/twenty-server/src/engine/api/common/common-query-runners/utils/build-mutation-query-builder.util.ts +++ b/packages/twenty-server/src/engine/api/common/common-query-runners/utils/build-mutation-query-builder.util.ts @@ -43,6 +43,8 @@ export const buildMutationQueryBuilder = ({ .select(`${alias}.id`) .withDeleted(); + idSubQueryBuilder.applyRowLevelPermissionPredicatesToMainAliasAndJoinedRelations(); + return repository .createQueryBuilder(alias) .where(`"${alias}"."id" IN (${idSubQueryBuilder.getQuery()})`) diff --git a/packages/twenty-server/src/engine/api/graphql/graphql-query-runner/graphql-query-parsers/graphql-query-filter/__tests__/graphql-query-filter-condition.parser.spec.ts b/packages/twenty-server/src/engine/api/graphql/graphql-query-runner/graphql-query-parsers/graphql-query-filter/__tests__/graphql-query-filter-condition.parser.spec.ts new file mode 100644 index 0000000000..74836b32f5 --- /dev/null +++ b/packages/twenty-server/src/engine/api/graphql/graphql-query-runner/graphql-query-parsers/graphql-query-filter/__tests__/graphql-query-filter-condition.parser.spec.ts @@ -0,0 +1,268 @@ +import { createWhereExpressionRecorder } from 'test/utils/create-where-expression-recorder.util'; +import { FieldMetadataType } from 'twenty-shared/types'; +import { type ObjectLiteral } from 'typeorm'; + +import { GraphqlQueryFilterConditionParser } from 'src/engine/api/graphql/graphql-query-runner/graphql-query-parsers/graphql-query-filter/graphql-query-filter-condition.parser'; +import { type FlatEntityMaps } from 'src/engine/metadata-modules/flat-entity/types/flat-entity-maps.type'; +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'; + +const createFlatFieldMetadata = ( + overrides: Partial, +): FlatFieldMetadata => + ({ + id: 'field-id', + name: 'field', + type: FieldMetadataType.TEXT, + universalIdentifier: 'field-universal-id', + ...overrides, + }) as FlatFieldMetadata; + +const nameField = createFlatFieldMetadata({ + id: 'name-field-id', + name: 'name', + type: FieldMetadataType.TEXT, + universalIdentifier: 'name-field-universal-id', +}); + +const employeesField = createFlatFieldMetadata({ + id: 'employees-field-id', + name: 'employees', + type: FieldMetadataType.NUMBER, + universalIdentifier: 'employees-field-universal-id', +}); + +const companyFields = [nameField, employeesField]; + +const companyObjectMetadata = { + id: 'company-object-id', + nameSingular: 'company', + namePlural: 'companies', + fieldIds: companyFields.map((field) => field.id), + universalIdentifier: 'company-object-universal-id', +} as FlatObjectMetadata; + +const flatFieldMetadataMaps: FlatEntityMaps = { + byUniversalIdentifier: Object.fromEntries( + companyFields.map((field) => [field.universalIdentifier, field]), + ), + universalIdentifierById: Object.fromEntries( + companyFields.map((field) => [field.id, field.universalIdentifier]), + ), + universalIdentifiersByApplicationId: {}, +}; + +const outerQueryBuilder = { + objectRecordsPermissions: {}, +} as unknown as WorkspaceSelectQueryBuilder; + +const recordFilterEntries = (filter: Record) => { + const recorder = createWhereExpressionRecorder(); + const parser = new GraphqlQueryFilterConditionParser( + companyObjectMetadata, + flatFieldMetadataMaps, + ); + + parser.applyFilterEntriesToWhereBrackets( + recorder.whereExpression, + outerQueryBuilder, + 'company', + filter, + ); + + return recorder.calls; +}; + +describe('GraphqlQueryFilterConditionParser', () => { + describe('applyFilterEntriesToWhereBrackets', () => { + it('emits the first entry with where and later entries with andWhere', () => { + const calls = recordFilterEntries({ + name: { ilike: '%acme%' }, + employees: { gte: 5 }, + }); + + expect(calls).toHaveLength(2); + expect(calls[0].method).toBe('where'); + expect(calls[1].method).toBe('andWhere'); + expect(calls[0].node.kind).toBe('sql'); + expect(calls[1].node.kind).toBe('sql'); + }); + + it('passes the leaf condition and its parameters through to the query builder', () => { + const calls = recordFilterEntries({ name: { ilike: '%acme%' } }); + + expect(calls).toHaveLength(1); + + const node = calls[0].node; + + if (node.kind !== 'sql') { + throw new Error('Expected a sql node'); + } + + expect(node.sql).toContain('"company"."name"'); + expect(Object.values(node.parameters ?? {})).toEqual(['%acme%']); + }); + + it('wraps an and group in brackets and joins its elements with andWhere', () => { + const calls = recordFilterEntries({ + and: [{ name: { ilike: '%a%' } }, { employees: { gte: 1 } }], + }); + + expect(calls).toEqual([ + { + method: 'where', + node: { + kind: 'brackets', + children: [ + { + method: 'where', + node: { + kind: 'brackets', + children: [ + { + method: 'where', + node: expect.objectContaining({ kind: 'sql' }), + }, + ], + }, + }, + { + method: 'andWhere', + node: { + kind: 'brackets', + children: [ + { + method: 'where', + node: expect.objectContaining({ kind: 'sql' }), + }, + ], + }, + }, + ], + }, + }, + ]); + }); + + it('wraps an or group in brackets and joins its elements with orWhere', () => { + const calls = recordFilterEntries({ + or: [{ name: { ilike: '%a%' } }, { employees: { gte: 1 } }], + }); + + expect(calls).toEqual([ + { + method: 'where', + node: { + kind: 'brackets', + children: [ + { + method: 'where', + node: expect.objectContaining({ kind: 'brackets' }), + }, + { + method: 'orWhere', + node: expect.objectContaining({ kind: 'brackets' }), + }, + ], + }, + }, + ]); + }); + + it('emits a not group as notBrackets', () => { + const calls = recordFilterEntries({ not: { name: { ilike: '%a%' } } }); + + expect(calls).toEqual([ + { + method: 'where', + node: { + kind: 'notBrackets', + children: [ + { + method: 'where', + node: expect.objectContaining({ kind: 'sql' }), + }, + ], + }, + }, + ]); + }); + + it('attaches a logical group with andWhere when it is not the first entry', () => { + const calls = recordFilterEntries({ + name: { ilike: '%a%' }, + and: [{ employees: { gte: 1 } }], + }); + + expect(calls).toHaveLength(2); + expect(calls[0]).toEqual({ + method: 'where', + node: expect.objectContaining({ kind: 'sql' }), + }); + expect(calls[1]).toEqual({ + method: 'andWhere', + node: expect.objectContaining({ kind: 'brackets' }), + }); + }); + + it('recurses through nested logical groups', () => { + const calls = recordFilterEntries({ + and: [{ or: [{ not: { name: { ilike: '%a%' } } }] }], + }); + + expect(calls).toEqual([ + { + method: 'where', + node: { + kind: 'brackets', + children: [ + { + method: 'where', + node: { + kind: 'brackets', + children: [ + { + method: 'where', + node: { + kind: 'brackets', + children: [ + { + method: 'where', + node: { + kind: 'brackets', + children: [ + { + method: 'where', + node: { + kind: 'notBrackets', + children: [ + { + method: 'where', + node: expect.objectContaining({ + kind: 'sql', + }), + }, + ], + }, + }, + ], + }, + }, + ], + }, + }, + ], + }, + }, + ], + }, + }, + ]); + }); + + it('emits nothing for an empty filter', () => { + expect(recordFilterEntries({})).toEqual([]); + }); + }); +}); diff --git a/packages/twenty-server/src/engine/api/graphql/graphql-query-runner/graphql-query-parsers/graphql-query-filter/graphql-query-filter-condition.parser.ts b/packages/twenty-server/src/engine/api/graphql/graphql-query-runner/graphql-query-parsers/graphql-query-filter/graphql-query-filter-condition.parser.ts index 5aad5221b4..e7eb380d0a 100644 --- a/packages/twenty-server/src/engine/api/graphql/graphql-query-runner/graphql-query-parsers/graphql-query-filter/graphql-query-filter-condition.parser.ts +++ b/packages/twenty-server/src/engine/api/graphql/graphql-query-runner/graphql-query-parsers/graphql-query-filter/graphql-query-filter-condition.parser.ts @@ -1,12 +1,12 @@ import { Brackets, - NotBrackets, type ObjectLiteral, type WhereExpressionBuilder, } from 'typeorm'; import { type ObjectRecordFilter } from 'src/engine/api/graphql/workspace-query-builder/interfaces/object-record.interface'; +import { applyFilterEntriesToWhereExpression } from 'src/engine/api/graphql/graphql-query-runner/graphql-query-parsers/utils/apply-filter-entries-to-where-expression.util'; import { type FlatEntityMaps } from 'src/engine/metadata-modules/flat-entity/types/flat-entity-maps.type'; 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'; @@ -15,7 +15,6 @@ import { type WorkspaceSelectQueryBuilder } from 'src/engine/twenty-orm/reposito import { GraphqlQueryFilterFieldParser } from './graphql-query-filter-field.parser'; export class GraphqlQueryFilterConditionParser { - private flatObjectMetadata: FlatObjectMetadata; private queryFilterFieldParser: GraphqlQueryFilterFieldParser; constructor( @@ -24,9 +23,8 @@ export class GraphqlQueryFilterConditionParser { flatObjectMetadataMaps?: FlatEntityMaps, depth = 0, ) { - this.flatObjectMetadata = flatObjectMetadata; this.queryFilterFieldParser = new GraphqlQueryFilterFieldParser( - this.flatObjectMetadata, + flatObjectMetadata, flatFieldMetadataMaps, flatObjectMetadataMaps, depth, @@ -60,129 +58,12 @@ export class GraphqlQueryFilterConditionParser { objectNameSingular: string, filter: Partial, ): void { - Object.entries(filter).forEach(([key, value], index) => { - this.parseKeyFilter( - innerQueryBuilder, - outerQueryBuilder, - objectNameSingular, - key, - value, - index === 0, - ); + applyFilterEntriesToWhereExpression({ + whereExpression: innerQueryBuilder, + outerQueryBuilder, + objectNameSingular, + filter, + fieldParser: this.queryFilterFieldParser, }); } - - private parseKeyFilter( - queryBuilder: WhereExpressionBuilder, - outerQueryBuilder: WorkspaceSelectQueryBuilder, - objectNameSingular: string, - key: string, - // oxlint-disable-next-line typescript/no-explicit-any - value: any, - isFirst = false, - ): void { - switch (key) { - case 'and': { - const andWhereCondition = new Brackets((qb) => { - value.forEach((filter: ObjectRecordFilter, index: number) => { - const whereCondition = new Brackets((qb2) => { - Object.entries(filter).forEach( - ([subFilterkey, subFilterValue], index) => { - this.parseKeyFilter( - qb2, - outerQueryBuilder, - objectNameSingular, - subFilterkey, - subFilterValue, - index === 0, - ); - }, - ); - }); - - if (index === 0) { - qb.where(whereCondition); - } else { - qb.andWhere(whereCondition); - } - }); - }); - - if (isFirst) { - queryBuilder.where(andWhereCondition); - } else { - queryBuilder.andWhere(andWhereCondition); - } - break; - } - case 'or': { - const orWhereCondition = new Brackets((qb) => { - value.forEach((filter: ObjectRecordFilter, index: number) => { - const whereCondition = new Brackets((qb2) => { - Object.entries(filter).forEach( - ([subFilterkey, subFilterValue], index) => { - this.parseKeyFilter( - qb2, - outerQueryBuilder, - objectNameSingular, - subFilterkey, - subFilterValue, - index === 0, - ); - }, - ); - }); - - if (index === 0) { - qb.where(whereCondition); - } else { - qb.orWhere(whereCondition); - } - }); - }); - - if (isFirst) { - queryBuilder.where(orWhereCondition); - } else { - queryBuilder.andWhere(orWhereCondition); - } - - break; - } - case 'not': { - const notWhereCondition = new NotBrackets((qb) => { - Object.entries(value).forEach( - ([subFilterkey, subFilterValue], index) => { - this.parseKeyFilter( - qb, - outerQueryBuilder, - objectNameSingular, - subFilterkey, - subFilterValue, - index === 0, - ); - }, - ); - }); - - if (isFirst) { - queryBuilder.where(notWhereCondition); - } else { - queryBuilder.andWhere(notWhereCondition); - } - - break; - } - default: - this.queryFilterFieldParser.parse( - queryBuilder, - outerQueryBuilder, - objectNameSingular, - key, - value, - isFirst, - ); - break; - } - } } diff --git a/packages/twenty-server/src/engine/api/graphql/graphql-query-runner/graphql-query-parsers/graphql-query-filter/graphql-query-filter-field.parser.ts b/packages/twenty-server/src/engine/api/graphql/graphql-query-runner/graphql-query-parsers/graphql-query-filter/graphql-query-filter-field.parser.ts index 1169b0526f..97e4cd4698 100644 --- a/packages/twenty-server/src/engine/api/graphql/graphql-query-runner/graphql-query-parsers/graphql-query-filter/graphql-query-filter-field.parser.ts +++ b/packages/twenty-server/src/engine/api/graphql/graphql-query-runner/graphql-query-parsers/graphql-query-filter/graphql-query-filter-field.parser.ts @@ -14,6 +14,8 @@ import { GraphqlQueryRunnerExceptionCode, } from 'src/engine/api/graphql/graphql-query-runner/errors/graphql-query-runner.exception'; import { addRelationJoinAliasToQueryBuilder } from 'src/engine/api/graphql/graphql-query-runner/graphql-query-parsers/utils/add-relation-join-alias.util'; +import { resolveFilterKeyFieldMetadata } from 'src/engine/api/graphql/graphql-query-runner/graphql-query-parsers/utils/resolve-filter-key-field-metadata.util'; +import { assertArrayOperatorValueIsNonEmptyArray } from 'src/engine/api/graphql/graphql-query-runner/utils/assert-array-operator-value-is-non-empty-array.util'; import { computeWhereConditionParts } from 'src/engine/api/graphql/graphql-query-runner/utils/compute-where-condition-parts'; import { type CompositeFieldMetadataType } from 'src/engine/metadata-modules/field-metadata/types/composite-field-metadata-type.type'; import { isCompositeFieldMetadataType } from 'src/engine/metadata-modules/field-metadata/utils/is-composite-field-metadata-type.util'; @@ -32,8 +34,6 @@ import { type WorkspaceSelectQueryBuilder } from 'src/engine/twenty-orm/reposito import { GraphqlQueryFilterConditionParser } from './graphql-query-filter-condition.parser'; -const ARRAY_OPERATORS = ['in', 'contains', 'notContains']; - export class GraphqlQueryFilterFieldParser { private flatObjectMetadata: FlatObjectMetadata; private flatFieldMetadataMaps: FlatEntityMaps; @@ -72,14 +72,13 @@ export class GraphqlQueryFilterFieldParser { isFirst = false, useDirectTableReference = false, ): void { - const isFilterKeyARelation = isDefined(this.fieldIdByName[key]); - const fieldMetadataId = - this.fieldIdByName[`${key}`] || this.fieldIdByJoinColumnName[`${key}`]; - - const fieldMetadata = findFlatEntityByIdInFlatEntityMaps({ - flatEntityId: fieldMetadataId, - flatEntityMaps: this.flatFieldMetadataMaps, - }); + const { fieldMetadata, isReferencedByFieldName } = + resolveFilterKeyFieldMetadata({ + filterKey: key, + fieldIdByName: this.fieldIdByName, + fieldIdByJoinColumnName: this.fieldIdByJoinColumnName, + flatFieldMetadataMaps: this.flatFieldMetadataMaps, + }); if (!isDefined(fieldMetadata)) { throw new Error(`Field metadata not found for field: ${key}`); @@ -99,7 +98,7 @@ export class GraphqlQueryFilterFieldParser { } if ( - isFilterKeyARelation && + isReferencedByFieldName && isMorphOrRelationFlatFieldMetadata(fieldMetadata) && fieldMetadata.settings?.relationType === RelationType.MANY_TO_ONE ) { @@ -125,16 +124,8 @@ export class GraphqlQueryFilterFieldParser { } const [[operator, value]] = Object.entries(filterValue); - if ( - ARRAY_OPERATORS.includes(operator) && - (!Array.isArray(value) || value.length === 0) - ) { - throw new GraphqlQueryRunnerException( - `Invalid filter value for field ${key}. Expected non-empty array`, - GraphqlQueryRunnerExceptionCode.INVALID_QUERY_INPUT, - { userFriendlyMessage: msg`Invalid filter value: "${String(value)}"` }, - ); - } + assertArrayOperatorValueIsNonEmptyArray({ operator, value, key }); + const { sql, params } = computeWhereConditionParts({ operator, objectNameSingular, @@ -267,18 +258,11 @@ export class GraphqlQueryFilterFieldParser { subFieldFilter as Record, ); - if ( - ARRAY_OPERATORS.includes(operator) && - (!Array.isArray(value) || value.length === 0) - ) { - throw new GraphqlQueryRunnerException( - `Invalid filter value for field ${subFieldKey}. Expected non-empty array`, - GraphqlQueryRunnerExceptionCode.INVALID_QUERY_INPUT, - { - userFriendlyMessage: msg`Invalid filter value: "${String(value)}"`, - }, - ); - } + assertArrayOperatorValueIsNonEmptyArray({ + operator, + value, + key: subFieldKey, + }); const { sql, params } = computeWhereConditionParts({ operator, diff --git a/packages/twenty-server/src/engine/api/graphql/graphql-query-runner/graphql-query-parsers/utils/apply-filter-entries-to-where-expression.util.ts b/packages/twenty-server/src/engine/api/graphql/graphql-query-runner/graphql-query-parsers/utils/apply-filter-entries-to-where-expression.util.ts new file mode 100644 index 0000000000..15864b5817 --- /dev/null +++ b/packages/twenty-server/src/engine/api/graphql/graphql-query-runner/graphql-query-parsers/utils/apply-filter-entries-to-where-expression.util.ts @@ -0,0 +1,131 @@ +import { + Brackets, + NotBrackets, + type ObjectLiteral, + type WhereExpressionBuilder, +} from 'typeorm'; + +import { type GraphqlQueryFilterFieldParser } from 'src/engine/api/graphql/graphql-query-runner/graphql-query-parsers/graphql-query-filter/graphql-query-filter-field.parser'; +import { type WorkspaceSelectQueryBuilder } from 'src/engine/twenty-orm/repository/workspace-select-query-builder'; + +type FilterWalkContext = { + outerQueryBuilder: WorkspaceSelectQueryBuilder; + objectNameSingular: string; + fieldParser: GraphqlQueryFilterFieldParser; + useDirectTableReference: boolean; +}; + +export const applyFilterEntriesToWhereExpression = ({ + whereExpression, + outerQueryBuilder, + objectNameSingular, + filter, + fieldParser, + useDirectTableReference = false, +}: { + whereExpression: WhereExpressionBuilder; + outerQueryBuilder: WorkspaceSelectQueryBuilder; + objectNameSingular: string; + filter: Record; + fieldParser: GraphqlQueryFilterFieldParser; + useDirectTableReference?: boolean; +}): void => { + applyFilterEntries(whereExpression, filter, { + outerQueryBuilder, + objectNameSingular, + fieldParser, + useDirectTableReference, + }); +}; + +const applyFilterEntries = ( + whereExpression: WhereExpressionBuilder, + filter: Record, + context: FilterWalkContext, +): void => { + Object.entries(filter).forEach(([filterKey, filterValue], index) => { + applyFilterEntry( + whereExpression, + filterKey, + filterValue, + index === 0, + context, + ); + }); +}; + +const applyFilterEntry = ( + whereExpression: WhereExpressionBuilder, + filterKey: string, + // oxlint-disable-next-line typescript/no-explicit-any + filterValue: any, + isFirst: boolean, + context: FilterWalkContext, +): void => { + switch (filterKey) { + case 'and': + applyLogicalGroup(whereExpression, filterValue, 'and', isFirst, context); + break; + case 'or': + applyLogicalGroup(whereExpression, filterValue, 'or', isFirst, context); + break; + case 'not': + applyCondition( + whereExpression, + new NotBrackets((negatedWhereExpression) => { + applyFilterEntries(negatedWhereExpression, filterValue, context); + }), + isFirst, + ); + break; + default: + context.fieldParser.parse( + whereExpression, + context.outerQueryBuilder, + context.objectNameSingular, + filterKey, + filterValue, + isFirst, + context.useDirectTableReference, + ); + break; + } +}; + +const applyLogicalGroup = ( + whereExpression: WhereExpressionBuilder, + filters: Record[], + logicalOperator: 'and' | 'or', + isFirst: boolean, + context: FilterWalkContext, +): void => { + const groupCondition = new Brackets((groupWhereExpression) => { + filters.forEach((filter, index) => { + const elementCondition = new Brackets((elementWhereExpression) => { + applyFilterEntries(elementWhereExpression, filter, context); + }); + + if (index === 0) { + groupWhereExpression.where(elementCondition); + } else if (logicalOperator === 'or') { + groupWhereExpression.orWhere(elementCondition); + } else { + groupWhereExpression.andWhere(elementCondition); + } + }); + }); + + applyCondition(whereExpression, groupCondition, isFirst); +}; + +const applyCondition = ( + whereExpression: WhereExpressionBuilder, + condition: Brackets, + isFirst: boolean, +): void => { + if (isFirst) { + whereExpression.where(condition); + } else { + whereExpression.andWhere(condition); + } +}; diff --git a/packages/twenty-server/src/engine/api/graphql/graphql-query-runner/graphql-query-parsers/utils/resolve-filter-key-field-metadata.util.ts b/packages/twenty-server/src/engine/api/graphql/graphql-query-runner/graphql-query-parsers/utils/resolve-filter-key-field-metadata.util.ts new file mode 100644 index 0000000000..bfe98c5288 --- /dev/null +++ b/packages/twenty-server/src/engine/api/graphql/graphql-query-runner/graphql-query-parsers/utils/resolve-filter-key-field-metadata.util.ts @@ -0,0 +1,34 @@ +import { isDefined } from 'twenty-shared/utils'; + +import { type FlatEntityMaps } from 'src/engine/metadata-modules/flat-entity/types/flat-entity-maps.type'; +import { findFlatEntityByIdInFlatEntityMaps } from 'src/engine/metadata-modules/flat-entity/utils/find-flat-entity-by-id-in-flat-entity-maps.util'; +import { type FlatFieldMetadata } from 'src/engine/metadata-modules/flat-field-metadata/types/flat-field-metadata.type'; + +export const resolveFilterKeyFieldMetadata = ({ + filterKey, + fieldIdByName, + fieldIdByJoinColumnName, + flatFieldMetadataMaps, +}: { + filterKey: string; + fieldIdByName: Record; + fieldIdByJoinColumnName: Record; + flatFieldMetadataMaps: FlatEntityMaps; +}): { + fieldMetadata: FlatFieldMetadata | undefined; + isReferencedByFieldName: boolean; +} => { + const isReferencedByFieldName = isDefined(fieldIdByName[filterKey]); + + const fieldMetadataId = + fieldIdByName[filterKey] ?? fieldIdByJoinColumnName[filterKey]; + + const fieldMetadata = isDefined(fieldMetadataId) + ? findFlatEntityByIdInFlatEntityMaps({ + flatEntityId: fieldMetadataId, + flatEntityMaps: flatFieldMetadataMaps, + }) + : undefined; + + return { fieldMetadata, isReferencedByFieldName }; +}; 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 afb22c8266..93da472ec3 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 @@ -24,7 +24,6 @@ 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; @@ -86,14 +85,6 @@ 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, @@ -209,6 +200,8 @@ export class GroupByWithRecordsService { queryBuilder: subQuery, }); + subQuery.applyRowLevelPermissionPredicatesToMainAliasAndJoinedRelations(); + let mainQueryQueryBuilder = repository.createQueryBuilder(); const pageStart = offsetForRecords; diff --git a/packages/twenty-server/src/engine/api/graphql/graphql-query-runner/utils/assert-array-operator-value-is-non-empty-array.util.ts b/packages/twenty-server/src/engine/api/graphql/graphql-query-runner/utils/assert-array-operator-value-is-non-empty-array.util.ts new file mode 100644 index 0000000000..882788c93c --- /dev/null +++ b/packages/twenty-server/src/engine/api/graphql/graphql-query-runner/utils/assert-array-operator-value-is-non-empty-array.util.ts @@ -0,0 +1,29 @@ +import { msg } from '@lingui/core/macro'; + +import { + GraphqlQueryRunnerException, + GraphqlQueryRunnerExceptionCode, +} from 'src/engine/api/graphql/graphql-query-runner/errors/graphql-query-runner.exception'; + +const ARRAY_OPERATORS = ['in', 'contains', 'notContains']; + +export const assertArrayOperatorValueIsNonEmptyArray = ({ + operator, + value, + key, +}: { + operator: string; + value: unknown; + key: string; +}): void => { + if ( + ARRAY_OPERATORS.includes(operator) && + (!Array.isArray(value) || value.length === 0) + ) { + throw new GraphqlQueryRunnerException( + `Invalid filter value for field ${key}. Expected non-empty array`, + GraphqlQueryRunnerExceptionCode.INVALID_QUERY_INPUT, + { userFriendlyMessage: msg`Invalid filter value` }, + ); + } +}; diff --git a/packages/twenty-server/src/engine/twenty-orm/repository/workspace-select-query-builder.ts b/packages/twenty-server/src/engine/twenty-orm/repository/workspace-select-query-builder.ts index 10ea0c366d..96c76667ab 100644 --- a/packages/twenty-server/src/engine/twenty-orm/repository/workspace-select-query-builder.ts +++ b/packages/twenty-server/src/engine/twenty-orm/repository/workspace-select-query-builder.ts @@ -1,15 +1,20 @@ +import { isNonEmptyString } from '@sniptt/guards'; import { type ObjectsPermissions } from 'twenty-shared/types'; +import { isDefined } from 'twenty-shared/utils'; import { type EntityTarget, type ObjectLiteral, SelectQueryBuilder, } from 'typeorm'; +import { type JoinAttribute } from 'typeorm/query-builder/JoinAttribute'; import { type QueryDeepPartialEntity } from 'typeorm/query-builder/QueryPartialEntity'; import { type FeatureFlagMap } from 'src/engine/core-modules/feature-flag/interfaces/feature-flag-map.interface'; import { type WorkspaceInternalContext } from 'src/engine/twenty-orm/interfaces/workspace-internal-context.interface'; import { type WorkspaceAuthContext } from 'src/engine/core-modules/auth/types/workspace-auth-context.type'; +import { findFlatEntityByIdInFlatEntityMaps } from 'src/engine/metadata-modules/flat-entity/utils/find-flat-entity-by-id-in-flat-entity-maps.util'; +import { type FlatObjectMetadata } from 'src/engine/metadata-modules/flat-object-metadata/types/flat-object-metadata.type'; import { PermissionsException, PermissionsExceptionCode, @@ -27,6 +32,34 @@ import { WorkspaceUpdateQueryBuilder } from 'src/engine/twenty-orm/repository/wo import { applyRowLevelPermissionPredicates } from 'src/engine/twenty-orm/utils/apply-row-level-permission-predicates.util'; import { formatResult } from 'src/engine/twenty-orm/utils/format-result.util'; import { getObjectMetadataFromEntityTarget } from 'src/engine/twenty-orm/utils/get-object-metadata-from-entity-target.util'; +import { renderRowLevelPermissionFilterToSql } from 'src/engine/twenty-orm/utils/render-row-level-permission-filter-to-sql.util'; +import { resolveRowLevelPermissionRecordFilter } from 'src/engine/twenty-orm/utils/resolve-row-level-permission-record-filter.util'; + +type JoinAttributeWithRowLevelPermissionMarker = JoinAttribute & { + hasRowLevelPermissionPredicateApplied?: true; +}; + +const hasRowLevelPermissionPredicateApplied = ( + joinAttribute: JoinAttribute, +): boolean => + (joinAttribute as JoinAttributeWithRowLevelPermissionMarker) + .hasRowLevelPermissionPredicateApplied === true; + +const markRowLevelPermissionPredicateApplied = ( + joinAttribute: JoinAttribute, +): void => { + ( + joinAttribute as JoinAttributeWithRowLevelPermissionMarker + ).hasRowLevelPermissionPredicateApplied = true; +}; + +const andWithExistingJoinCondition = ( + existingJoinCondition: string | undefined, + rowLevelPermissionCondition: string, +): string => + isNonEmptyString(existingJoinCondition) + ? `(${existingJoinCondition}) AND (${rowLevelPermissionCondition})` + : rowLevelPermissionCondition; export class WorkspaceSelectQueryBuilder< T extends ObjectLiteral, @@ -340,7 +373,7 @@ export class WorkspaceSelectQueryBuilder< } private validatePermissions(): void { - this.applyRowLevelPermissionPredicates(); + this.applyRowLevelPermissionPredicatesToMainAliasAndJoinedRelations(); validateQueryIsPermittedOrThrow({ expressionMap: this.expressionMap, objectsPermissions: this.objectRecordsPermissions, @@ -351,6 +384,11 @@ export class WorkspaceSelectQueryBuilder< }); } + applyRowLevelPermissionPredicatesToMainAliasAndJoinedRelations(): void { + this.applyRowLevelPermissionPredicates(); + this.applyRowLevelPermissionPredicatesToJoinedRelations(); + } + private getMainAliasTarget(): EntityTarget { const mainAlias = this.expressionMap.mainAlias; @@ -392,4 +430,85 @@ export class WorkspaceSelectQueryBuilder< featureFlagMap: this.featureFlagMap, }); } + + private applyRowLevelPermissionPredicatesToJoinedRelations(): void { + if (this.shouldBypassPermissionChecks) { + return; + } + + for (const joinAttribute of this.expressionMap.joinAttributes) { + if (hasRowLevelPermissionPredicateApplied(joinAttribute)) { + continue; + } + + const joinedObjectMetadata = + this.getJoinedObjectMetadataOrUndefined(joinAttribute); + + if (!isDefined(joinedObjectMetadata)) { + continue; + } + + const recordFilter = resolveRowLevelPermissionRecordFilter({ + internalContext: this.internalContext, + authContext: this.authContext, + objectMetadata: joinedObjectMetadata, + }); + + if (!isDefined(recordFilter)) { + markRowLevelPermissionPredicateApplied(joinAttribute); + continue; + } + + const renderedCondition = renderRowLevelPermissionFilterToSql({ + recordFilter, + tableAlias: joinAttribute.alias.name, + objectMetadata: joinedObjectMetadata, + flatFieldMetadataMaps: this.internalContext.flatFieldMetadataMaps, + }); + + if (!isDefined(renderedCondition)) { + markRowLevelPermissionPredicateApplied(joinAttribute); + continue; + } + + joinAttribute.condition = andWithExistingJoinCondition( + joinAttribute.condition, + renderedCondition.sql, + ); + + this.setParameters(renderedCondition.parameters); + markRowLevelPermissionPredicateApplied(joinAttribute); + } + } + + private getJoinedObjectMetadataOrUndefined( + joinAttribute: JoinAttribute, + ): FlatObjectMetadata | undefined { + const joinedEntityMetadata = joinAttribute.metadata; + const isJoinOnSubQueryOrCustomTable = + isDefined(joinAttribute.alias?.subQuery) || + !isDefined(joinedEntityMetadata); + + if (isJoinOnSubQueryOrCustomTable) { + return undefined; + } + + const joinedEntityTarget = joinedEntityMetadata.target; + + if (typeof joinedEntityTarget !== 'string') { + return undefined; + } + + const joinedObjectMetadataId = + this.internalContext.objectIdByNameSingular[joinedEntityTarget]; + + if (!isDefined(joinedObjectMetadataId)) { + return undefined; + } + + return findFlatEntityByIdInFlatEntityMaps({ + flatEntityId: joinedObjectMetadataId, + flatEntityMaps: this.internalContext.flatObjectMetadataMaps, + }); + } } diff --git a/packages/twenty-server/src/engine/twenty-orm/utils/__tests__/apply-row-level-permission-predicates.util.spec.ts b/packages/twenty-server/src/engine/twenty-orm/utils/__tests__/apply-row-level-permission-predicates.util.spec.ts new file mode 100644 index 0000000000..775e052f0a --- /dev/null +++ b/packages/twenty-server/src/engine/twenty-orm/utils/__tests__/apply-row-level-permission-predicates.util.spec.ts @@ -0,0 +1,147 @@ +import { createWhereExpressionRecorder } from 'test/utils/create-where-expression-recorder.util'; +import { FieldMetadataType } from 'twenty-shared/types'; +import { type ObjectLiteral } from 'typeorm'; + +import { type FeatureFlagMap } from 'src/engine/core-modules/feature-flag/interfaces/feature-flag-map.interface'; +import { type WorkspaceAuthContext } from 'src/engine/core-modules/auth/types/workspace-auth-context.type'; +import { type FlatEntityMaps } from 'src/engine/metadata-modules/flat-entity/types/flat-entity-maps.type'; +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 WorkspaceInternalContext } from 'src/engine/twenty-orm/interfaces/workspace-internal-context.interface'; +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 { buildRowLevelPermissionRecordFilter } from 'src/engine/twenty-orm/utils/build-row-level-permission-record-filter.util'; + +jest.mock( + 'src/engine/twenty-orm/utils/build-row-level-permission-record-filter.util', + () => ({ + buildRowLevelPermissionRecordFilter: jest.fn(), + }), +); + +jest.mock( + 'src/engine/twenty-orm/utils/resolve-role-ids-from-auth-context.util', + () => ({ + resolveRoleIdsFromAuthContext: jest.fn(() => ['role-id']), + }), +); + +const nameField = { + id: 'name-field-id', + name: 'name', + type: FieldMetadataType.TEXT, + universalIdentifier: 'name-field-universal-id', +} as FlatFieldMetadata; + +const companyObjectMetadata = { + id: 'company-object-id', + nameSingular: 'company', + namePlural: 'companies', + fieldIds: [nameField.id], + universalIdentifier: 'company-object-universal-id', +} as FlatObjectMetadata; + +const flatFieldMetadataMaps: FlatEntityMaps = { + byUniversalIdentifier: { [nameField.universalIdentifier]: nameField }, + universalIdentifierById: { [nameField.id]: nameField.universalIdentifier }, + universalIdentifiersByApplicationId: {}, +}; + +const internalContext = { + flatFieldMetadataMaps, + flatRowLevelPermissionPredicateMaps: {}, + flatRowLevelPermissionPredicateGroupMaps: {}, + userWorkspaceRoleMap: {}, + apiKeyRoleMap: {}, +} as unknown as WorkspaceInternalContext; + +const applyToFakeQueryBuilder = ({ + queryType = 'select', + wheres = [], +}: { + queryType?: string; + wheres?: unknown[]; +} = {}) => { + const recorder = createWhereExpressionRecorder(); + const queryBuilder = Object.assign(recorder.whereExpression, { + expressionMap: { queryType, wheres }, + objectRecordsPermissions: {}, + }) as unknown as WorkspaceSelectQueryBuilder; + + applyRowLevelPermissionPredicates({ + queryBuilder, + objectMetadata: companyObjectMetadata, + internalContext, + authContext: {} as WorkspaceAuthContext, + featureFlagMap: {} as FeatureFlagMap, + }); + + return recorder.calls; +}; + +const getFirstLeafSql = (calls: ReturnType) => { + const groupNode = calls[0].node; + + if (groupNode.kind !== 'brackets') { + throw new Error('Expected the record filter to be wrapped in brackets'); + } + + const leafNode = groupNode.children[0].node; + + if (leafNode.kind !== 'sql') { + throw new Error('Expected a sql leaf'); + } + + return leafNode.sql; +}; + +describe('applyRowLevelPermissionPredicates', () => { + beforeEach(() => { + (buildRowLevelPermissionRecordFilter as jest.Mock).mockReturnValue({ + name: { ilike: '%Visible%' }, + }); + }); + + it('uses where when the query builder has no existing where clause', () => { + const calls = applyToFakeQueryBuilder({ wheres: [] }); + + expect(calls).toHaveLength(1); + expect(calls[0].method).toBe('where'); + expect(calls[0].node.kind).toBe('brackets'); + }); + + it('appends with andWhere so it never resets an existing where clause', () => { + const calls = applyToFakeQueryBuilder({ wheres: [{ type: 'simple' }] }); + + expect(calls).toHaveLength(1); + expect(calls[0].method).toBe('andWhere'); + }); + + it('references the column through the table alias for a select query', () => { + expect( + getFirstLeafSql(applyToFakeQueryBuilder({ queryType: 'select' })), + ).toContain('"company"."name"'); + }); + + it.each(['update', 'delete', 'soft-delete'])( + 'references the column directly for a %s query', + (queryType) => { + const sql = getFirstLeafSql(applyToFakeQueryBuilder({ queryType })); + + expect(sql).toContain('"name"'); + expect(sql).not.toContain('"company"."name"'); + }, + ); + + it('emits nothing when there is no record filter', () => { + (buildRowLevelPermissionRecordFilter as jest.Mock).mockReturnValue(null); + + expect(applyToFakeQueryBuilder()).toEqual([]); + }); + + it('emits nothing when the record filter is empty', () => { + (buildRowLevelPermissionRecordFilter as jest.Mock).mockReturnValue({}); + + expect(applyToFakeQueryBuilder()).toEqual([]); + }); +}); diff --git a/packages/twenty-server/src/engine/twenty-orm/utils/__tests__/render-row-level-permission-filter-to-sql.util.spec.ts b/packages/twenty-server/src/engine/twenty-orm/utils/__tests__/render-row-level-permission-filter-to-sql.util.spec.ts new file mode 100644 index 0000000000..3d37dd9cf4 --- /dev/null +++ b/packages/twenty-server/src/engine/twenty-orm/utils/__tests__/render-row-level-permission-filter-to-sql.util.spec.ts @@ -0,0 +1,334 @@ +import { + FieldMetadataType, + type RecordGqlOperationFilter, + RelationType, +} from 'twenty-shared/types'; +import { type ObjectLiteral } from 'typeorm'; + +import { type FlatEntityMaps } from 'src/engine/metadata-modules/flat-entity/types/flat-entity-maps.type'; +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 { renderRowLevelPermissionFilterToSql } from 'src/engine/twenty-orm/utils/render-row-level-permission-filter-to-sql.util'; + +const createFlatFieldMetadata = ( + overrides: Partial, +): FlatFieldMetadata => + ({ + id: 'field-id', + name: 'field', + type: FieldMetadataType.TEXT, + universalIdentifier: 'field-universal-id', + ...overrides, + }) as FlatFieldMetadata; + +const nameField = createFlatFieldMetadata({ + id: 'name-field-id', + name: 'name', + type: FieldMetadataType.TEXT, + universalIdentifier: 'name-field-universal-id', +}); + +const employeesField = createFlatFieldMetadata({ + id: 'employees-field-id', + name: 'employees', + type: FieldMetadataType.NUMBER, + universalIdentifier: 'employees-field-universal-id', +}); + +const annualRecurringRevenueField = createFlatFieldMetadata({ + id: 'annual-recurring-revenue-field-id', + name: 'annualRecurringRevenue', + type: FieldMetadataType.CURRENCY, + universalIdentifier: 'annual-recurring-revenue-field-universal-id', +}); + +const accountOwnerField = createFlatFieldMetadata({ + id: 'account-owner-field-id', + name: 'accountOwner', + type: FieldMetadataType.RELATION, + universalIdentifier: 'account-owner-field-universal-id', + settings: { + relationType: RelationType.MANY_TO_ONE, + joinColumnName: 'accountOwnerId', + }, +} as Partial); + +const companyFields = [ + nameField, + employeesField, + annualRecurringRevenueField, + accountOwnerField, +]; + +const companyObjectMetadata = { + id: 'company-object-id', + nameSingular: 'company', + namePlural: 'companies', + fieldIds: companyFields.map((field) => field.id), + universalIdentifier: 'company-object-universal-id', +} as FlatObjectMetadata; + +const flatFieldMetadataMaps: FlatEntityMaps = { + byUniversalIdentifier: Object.fromEntries( + companyFields.map((field) => [field.universalIdentifier, field]), + ), + universalIdentifierById: Object.fromEntries( + companyFields.map((field) => [field.id, field.universalIdentifier]), + ), + universalIdentifiersByApplicationId: {}, +}; + +const findParameterKeyByValue = ( + parameters: ObjectLiteral, + value: unknown, +): string | undefined => + Object.entries(parameters).find( + ([, parameterValue]) => parameterValue === value, + )?.[0]; + +describe('renderRowLevelPermissionFilterToSql', () => { + const baseArgs = { + tableAlias: 'company', + objectMetadata: companyObjectMetadata, + flatFieldMetadataMaps, + }; + + it('returns null for an empty filter', () => { + expect( + renderRowLevelPermissionFilterToSql({ + ...baseArgs, + recordFilter: {}, + }), + ).toBeNull(); + }); + + it('renders a single field condition referencing the join alias', () => { + const result = renderRowLevelPermissionFilterToSql({ + ...baseArgs, + recordFilter: { name: { ilike: '%Visible%' } }, + }); + + const nameParameterKey = findParameterKeyByValue( + result?.parameters ?? {}, + '%Visible%', + ); + + expect(nameParameterKey).toMatch(/^name[0-9a-f]{10}$/); + expect(result?.sql).toBe( + `("company"."name"::text ILIKE :${nameParameterKey})`, + ); + }); + + it('combines multiple top-level keys with AND', () => { + const result = renderRowLevelPermissionFilterToSql({ + ...baseArgs, + recordFilter: { name: { ilike: '%x%' }, employees: { gte: 5 } }, + }); + + const parameters = result?.parameters ?? {}; + const nameParameterKey = findParameterKeyByValue(parameters, '%x%'); + const employeesParameterKey = findParameterKeyByValue(parameters, 5); + + expect(result?.sql).toBe( + `(("company"."name"::text ILIKE :${nameParameterKey}) AND ("company"."employees" >= :${employeesParameterKey}))`, + ); + }); + + it('combines multiple operators on a single field with AND', () => { + const result = renderRowLevelPermissionFilterToSql({ + ...baseArgs, + recordFilter: { employees: { gte: 1, lte: 5 } }, + }); + + const parameters = result?.parameters ?? {}; + const lowerBoundParameterKey = findParameterKeyByValue(parameters, 1); + const upperBoundParameterKey = findParameterKeyByValue(parameters, 5); + + expect(Object.keys(parameters)).toHaveLength(2); + expect(result?.sql).toBe( + `(("company"."employees" >= :${lowerBoundParameterKey}) AND ("company"."employees" <= :${upperBoundParameterKey}))`, + ); + }); + + it('renders an or group', () => { + const result = renderRowLevelPermissionFilterToSql({ + ...baseArgs, + recordFilter: { + or: [{ employees: { gte: 1 } }, { employees: { lte: 5 } }], + }, + }); + + const parameters = result?.parameters ?? {}; + const lowerBoundParameterKey = findParameterKeyByValue(parameters, 1); + const upperBoundParameterKey = findParameterKeyByValue(parameters, 5); + + expect(result?.sql).toBe( + `(("company"."employees" >= :${lowerBoundParameterKey}) OR ("company"."employees" <= :${upperBoundParameterKey}))`, + ); + }); + + it('renders a singleton or filter that is not wrapped in an array', () => { + const result = renderRowLevelPermissionFilterToSql({ + ...baseArgs, + recordFilter: { or: { name: { ilike: '%x%' } } }, + }); + + const nameParameterKey = findParameterKeyByValue( + result?.parameters ?? {}, + '%x%', + ); + + expect(result?.sql).toBe( + `(("company"."name"::text ILIKE :${nameParameterKey}))`, + ); + }); + + it('renders a not group over a join column condition', () => { + const result = renderRowLevelPermissionFilterToSql({ + ...baseArgs, + recordFilter: { not: { accountOwnerId: { in: ['id-1'] } } }, + }); + + const parameters = result?.parameters ?? {}; + const [accountOwnerIdParameterKey] = Object.keys(parameters); + + expect(accountOwnerIdParameterKey).toMatch(/^accountOwnerId[0-9a-f]{10}$/); + expect(parameters[accountOwnerIdParameterKey]).toEqual(['id-1']); + expect(result?.sql).toBe( + `NOT (("company"."accountOwnerId" IN (:...${accountOwnerIdParameterKey})))`, + ); + }); + + it('renders the IS_NOT relation shape (or of not-in and is-null)', () => { + const result = renderRowLevelPermissionFilterToSql({ + ...baseArgs, + recordFilter: { + or: [ + { not: { accountOwnerId: { in: ['id-1'] } } }, + { accountOwnerId: { is: 'NULL' } }, + ], + }, + }); + + const parameters = result?.parameters ?? {}; + const [accountOwnerIdParameterKey] = Object.keys(parameters); + + expect(result?.sql).toBe( + `(NOT (("company"."accountOwnerId" IN (:...${accountOwnerIdParameterKey}))) OR ("company"."accountOwnerId" IS NULL))`, + ); + }); + + it('renders a composite sub field condition with the flattened column name', () => { + const result = renderRowLevelPermissionFilterToSql({ + ...baseArgs, + recordFilter: { annualRecurringRevenue: { amountMicros: { gte: 1000 } } }, + }); + + const parameters = result?.parameters ?? {}; + const amountMicrosParameterKey = findParameterKeyByValue(parameters, 1000); + + expect(amountMicrosParameterKey).toMatch( + /^annualRecurringRevenueAmountMicros[0-9a-f]{10}$/, + ); + expect(result?.sql).toBe( + `("company"."annualRecurringRevenueAmountMicros" >= :${amountMicrosParameterKey})`, + ); + }); + + it('throws for an unknown composite sub field', () => { + expect(() => + renderRowLevelPermissionFilterToSql({ + ...baseArgs, + recordFilter: { + annualRecurringRevenue: { bogus: { eq: 1 } }, + } as unknown as RecordGqlOperationFilter, + }), + ).toThrow('is not a sub field of composite type'); + }); + + it('throws for a field that does not exist on the object', () => { + expect(() => + renderRowLevelPermissionFilterToSql({ + ...baseArgs, + recordFilter: { nonexistent: { eq: 1 } }, + }), + ).toThrow('field "nonexistent" does not exist'); + }); + + it('throws for a relation referenced by field name', () => { + expect(() => + renderRowLevelPermissionFilterToSql({ + ...baseArgs, + recordFilter: { accountOwner: { name: { eq: 'x' } } }, + }), + ).toThrow('traversing a relation requires an additional join'); + }); + + it('throws for an array operator with an empty array', () => { + expect(() => + renderRowLevelPermissionFilterToSql({ + ...baseArgs, + recordFilter: { name: { in: [] } }, + }), + ).toThrow('Expected non-empty array'); + }); + + it('renders a nested empty logical group as always-true, like TypeORM empty Brackets', () => { + const result = renderRowLevelPermissionFilterToSql({ + ...baseArgs, + recordFilter: { or: [{ or: [] }, { name: { ilike: '%x%' } }] }, + }); + + const nameParameterKey = findParameterKeyByValue( + result?.parameters ?? {}, + '%x%', + ); + + expect(result?.sql).toBe( + `(1=1 OR ("company"."name"::text ILIKE :${nameParameterKey}))`, + ); + }); + + it('renders a top-level empty logical group as always-true', () => { + const result = renderRowLevelPermissionFilterToSql({ + ...baseArgs, + recordFilter: { or: [] }, + }); + + expect(result?.sql).toBe('1=1'); + expect(result?.parameters).toEqual({}); + }); + + it('renders not of an empty filter as the negation of always-true', () => { + const result = renderRowLevelPermissionFilterToSql({ + ...baseArgs, + recordFilter: { not: {} }, + }); + + expect(result?.sql).toBe('NOT (1=1)'); + }); + + it('merges parameters from every leaf and conjoins the groups', () => { + const result = renderRowLevelPermissionFilterToSql({ + ...baseArgs, + recordFilter: { + and: [{ name: { ilike: 'a' } }, { employees: { eq: 2 } }], + }, + }); + + const parameters = result?.parameters ?? {}; + const nameParameterKey = findParameterKeyByValue(parameters, 'a'); + const employeesParameterKey = findParameterKeyByValue(parameters, 2); + + expect(result?.sql).toBe( + `(("company"."name"::text ILIKE :${nameParameterKey}) AND ("company"."employees" = :${employeesParameterKey}))`, + ); + expect(Object.keys(parameters)).toHaveLength(2); + expect(findParameterKeyByValue(parameters, 'a')).toMatch( + /^name[0-9a-f]{10}$/, + ); + expect(findParameterKeyByValue(parameters, 2)).toMatch( + /^employees[0-9a-f]{10}$/, + ); + }); +}); diff --git a/packages/twenty-server/src/engine/twenty-orm/utils/apply-row-level-permission-predicates.util.ts b/packages/twenty-server/src/engine/twenty-orm/utils/apply-row-level-permission-predicates.util.ts index 7e00a20f21..8309485b15 100644 --- a/packages/twenty-server/src/engine/twenty-orm/utils/apply-row-level-permission-predicates.util.ts +++ b/packages/twenty-server/src/engine/twenty-orm/utils/apply-row-level-permission-predicates.util.ts @@ -1,22 +1,17 @@ /* @license Enterprise */ -import { - Brackets, - NotBrackets, - type ObjectLiteral, - type WhereExpressionBuilder, -} from 'typeorm'; +import { isDefined } from 'twenty-shared/utils'; +import { Brackets, type ObjectLiteral } from 'typeorm'; import { type FeatureFlagMap } from 'src/engine/core-modules/feature-flag/interfaces/feature-flag-map.interface'; import { type WorkspaceInternalContext } from 'src/engine/twenty-orm/interfaces/workspace-internal-context.interface'; import { GraphqlQueryFilterFieldParser } from 'src/engine/api/graphql/graphql-query-runner/graphql-query-parsers/graphql-query-filter/graphql-query-filter-field.parser'; -import { isUserAuthContext } from 'src/engine/core-modules/auth/guards/is-user-auth-context.guard'; +import { applyFilterEntriesToWhereExpression } from 'src/engine/api/graphql/graphql-query-runner/graphql-query-parsers/utils/apply-filter-entries-to-where-expression.util'; import { type WorkspaceAuthContext } from 'src/engine/core-modules/auth/types/workspace-auth-context.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 { buildRowLevelPermissionRecordFilter } from 'src/engine/twenty-orm/utils/build-row-level-permission-record-filter.util'; -import { resolveRoleIdsFromAuthContext } from 'src/engine/twenty-orm/utils/resolve-role-ids-from-auth-context.util'; +import { resolveRowLevelPermissionRecordFilter } from 'src/engine/twenty-orm/utils/resolve-row-level-permission-record-filter.util'; type ApplyRowLevelPermissionPredicatesArgs = { queryBuilder: WorkspaceSelectQueryBuilder; @@ -33,26 +28,13 @@ export const applyRowLevelPermissionPredicates = ({ authContext, featureFlagMap: _featureFlagMap, }: ApplyRowLevelPermissionPredicatesArgs): void => { - const roleIds = resolveRoleIdsFromAuthContext({ + const recordFilter = resolveRowLevelPermissionRecordFilter({ + internalContext, authContext, - userWorkspaceRoleMap: internalContext.userWorkspaceRoleMap, - apiKeyRoleMap: internalContext.apiKeyRoleMap, - }); - - const recordFilter = buildRowLevelPermissionRecordFilter({ - flatRowLevelPermissionPredicateMaps: - internalContext.flatRowLevelPermissionPredicateMaps, - flatRowLevelPermissionPredicateGroupMaps: - internalContext.flatRowLevelPermissionPredicateGroupMaps, - flatFieldMetadataMaps: internalContext.flatFieldMetadataMaps, objectMetadata, - roleIds, - workspaceMember: isUserAuthContext(authContext) - ? authContext.workspaceMember - : undefined, }); - if (!recordFilter || Object.keys(recordFilter).length === 0) { + if (!isDefined(recordFilter)) { return; } @@ -86,27 +68,19 @@ const applyObjectRecordFilterToQueryBuilder = ({ fieldParser: GraphqlQueryFilterFieldParser; useDirectTableReference?: boolean; }): void => { - if (!recordFilter || Object.keys(recordFilter).length === 0) { - return; - } - - // parseKeyFilter only uses the join surface, so widen back to ObjectLiteral - // here rather than threading the concrete T through every recursive call. + // The walker only uses the join surface, so widen back to ObjectLiteral here + // rather than threading the concrete T through every recursive call. const outerQueryBuilderAsObjectLiteral = queryBuilder as WorkspaceSelectQueryBuilder; const whereCondition = new Brackets((qb) => { - Object.entries(recordFilter).forEach(([key, value], index) => { - parseKeyFilter({ - queryBuilder: qb, - outerQueryBuilder: outerQueryBuilderAsObjectLiteral, - objectNameSingular, - key, - value, - isFirst: index === 0, - fieldParser, - useDirectTableReference, - }); + applyFilterEntriesToWhereExpression({ + whereExpression: qb, + outerQueryBuilder: outerQueryBuilderAsObjectLiteral, + objectNameSingular, + filter: recordFilter, + fieldParser, + useDirectTableReference, }); }); @@ -116,135 +90,3 @@ const applyObjectRecordFilterToQueryBuilder = ({ queryBuilder.andWhere(whereCondition); } }; - -const parseKeyFilter = ({ - queryBuilder, - outerQueryBuilder, - objectNameSingular, - key, - value, - isFirst, - fieldParser, - useDirectTableReference = false, -}: { - queryBuilder: WhereExpressionBuilder; - outerQueryBuilder: WorkspaceSelectQueryBuilder; - objectNameSingular: string; - key: string; - // oxlint-disable-next-line typescript/no-explicit-any - value: any; - isFirst: boolean; - fieldParser: GraphqlQueryFilterFieldParser; - useDirectTableReference?: boolean; -}): void => { - switch (key) { - case 'and': { - const andWhereCondition = new Brackets((qb) => { - value.forEach((filter: Record, index: number) => { - const whereCondition = new Brackets((qb2) => { - Object.entries(filter).forEach( - ([subFilterKey, subFilterValue], subIndex) => { - parseKeyFilter({ - queryBuilder: qb2, - outerQueryBuilder, - objectNameSingular, - key: subFilterKey, - value: subFilterValue, - isFirst: subIndex === 0, - fieldParser, - useDirectTableReference, - }); - }, - ); - }); - - if (index === 0) { - qb.where(whereCondition); - } else { - qb.andWhere(whereCondition); - } - }); - }); - - if (isFirst) { - queryBuilder.where(andWhereCondition); - } else { - queryBuilder.andWhere(andWhereCondition); - } - break; - } - case 'or': { - const orWhereCondition = new Brackets((qb) => { - value.forEach((filter: Record, index: number) => { - const whereCondition = new Brackets((qb2) => { - Object.entries(filter).forEach( - ([subFilterKey, subFilterValue], subIndex) => { - parseKeyFilter({ - queryBuilder: qb2, - outerQueryBuilder, - objectNameSingular, - key: subFilterKey, - value: subFilterValue, - isFirst: subIndex === 0, - fieldParser, - useDirectTableReference, - }); - }, - ); - }); - - if (index === 0) { - qb.where(whereCondition); - } else { - qb.orWhere(whereCondition); - } - }); - }); - - if (isFirst) { - queryBuilder.where(orWhereCondition); - } else { - queryBuilder.andWhere(orWhereCondition); - } - - break; - } - case 'not': { - const notWhereCondition = new NotBrackets((qb) => { - Object.entries(value).forEach( - ([subFilterKey, subFilterValue], subIndex) => { - parseKeyFilter({ - queryBuilder: qb, - outerQueryBuilder, - objectNameSingular, - key: subFilterKey, - value: subFilterValue, - isFirst: subIndex === 0, - fieldParser, - useDirectTableReference, - }); - }, - ); - }); - - if (isFirst) { - queryBuilder.where(notWhereCondition); - } else { - queryBuilder.andWhere(notWhereCondition); - } - - break; - } - default: - fieldParser.parse( - queryBuilder, - outerQueryBuilder, - objectNameSingular, - key, - value, - isFirst, - useDirectTableReference, - ); - break; - } -}; diff --git a/packages/twenty-server/src/engine/twenty-orm/utils/render-row-level-permission-filter-to-sql.util.ts b/packages/twenty-server/src/engine/twenty-orm/utils/render-row-level-permission-filter-to-sql.util.ts new file mode 100644 index 0000000000..56ddcd3e4e --- /dev/null +++ b/packages/twenty-server/src/engine/twenty-orm/utils/render-row-level-permission-filter-to-sql.util.ts @@ -0,0 +1,276 @@ +/* @license Enterprise */ + +import { isNonEmptyString } from '@sniptt/guards'; +import { + compositeTypeDefinitions, + type RecordGqlOperationFilter, +} from 'twenty-shared/types'; +import { capitalize, isDefined } from 'twenty-shared/utils'; +import { type ObjectLiteral } from 'typeorm'; + +import { resolveFilterKeyFieldMetadata } from 'src/engine/api/graphql/graphql-query-runner/graphql-query-parsers/utils/resolve-filter-key-field-metadata.util'; +import { assertArrayOperatorValueIsNonEmptyArray } from 'src/engine/api/graphql/graphql-query-runner/utils/assert-array-operator-value-is-non-empty-array.util'; +import { computeWhereConditionParts } from 'src/engine/api/graphql/graphql-query-runner/utils/compute-where-condition-parts'; +import { type CompositeFieldMetadataType } from 'src/engine/metadata-modules/field-metadata/types/composite-field-metadata-type.type'; +import { isCompositeFieldMetadataType } from 'src/engine/metadata-modules/field-metadata/utils/is-composite-field-metadata-type.util'; +import { type FlatEntityMaps } from 'src/engine/metadata-modules/flat-entity/types/flat-entity-maps.type'; +import { type FlatFieldMetadata } from 'src/engine/metadata-modules/flat-field-metadata/types/flat-field-metadata.type'; +import { buildFieldMapsFromFlatObjectMetadata } from 'src/engine/metadata-modules/flat-field-metadata/utils/build-field-maps-from-flat-object-metadata.util'; +import { isMorphOrRelationFlatFieldMetadata } from 'src/engine/metadata-modules/flat-field-metadata/utils/is-morph-or-relation-flat-field-metadata.util'; +import { type FlatObjectMetadata } from 'src/engine/metadata-modules/flat-object-metadata/types/flat-object-metadata.type'; +import { + TwentyORMException, + TwentyORMExceptionCode, +} from 'src/engine/twenty-orm/exceptions/twenty-orm.exception'; + +const ALWAYS_TRUE_CONDITION = '1=1'; + +type SqlRenderingContext = { + tableAlias: string; + fieldIdByName: Record; + fieldIdByJoinColumnName: Record; + flatFieldMetadataMaps: FlatEntityMaps; + collectedParameters: ObjectLiteral; +}; + +type RenderedSqlCondition = { + sql: string; + parameters: ObjectLiteral; +}; + +export const renderRowLevelPermissionFilterToSql = ({ + recordFilter, + tableAlias, + objectMetadata, + flatFieldMetadataMaps, +}: { + recordFilter: RecordGqlOperationFilter; + tableAlias: string; + objectMetadata: FlatObjectMetadata; + flatFieldMetadataMaps: FlatEntityMaps; +}): RenderedSqlCondition | null => { + const { fieldIdByName, fieldIdByJoinColumnName } = + buildFieldMapsFromFlatObjectMetadata(flatFieldMetadataMaps, objectMetadata); + + const collectedParameters: ObjectLiteral = {}; + + const sql = renderFilterAsConjunction(recordFilter, { + tableAlias, + fieldIdByName, + fieldIdByJoinColumnName, + flatFieldMetadataMaps, + collectedParameters, + }); + + if (!isNonEmptyString(sql)) { + return null; + } + + return { sql, parameters: collectedParameters }; +}; + +const renderFilterAsConjunction = ( + filter: RecordGqlOperationFilter, + context: SqlRenderingContext, +): string => { + const conditions = Object.entries(filter) + .map(([filterKey, filterValue]) => + renderFilterEntry(filterKey, filterValue, context), + ) + .filter(isNonEmptyString); + + return joinConditions(conditions, 'AND'); +}; + +const renderFilterEntry = ( + filterKey: string, + filterValue: unknown, + context: SqlRenderingContext, +): string => { + switch (filterKey) { + case 'and': + return renderLogicalGroup( + filterValue as RecordGqlOperationFilter[] | RecordGqlOperationFilter, + 'AND', + context, + ); + case 'or': + return renderLogicalGroup( + filterValue as RecordGqlOperationFilter[] | RecordGqlOperationFilter, + 'OR', + context, + ); + case 'not': { + const negatedCondition = renderFilterAsConjunction( + filterValue as RecordGqlOperationFilter, + context, + ); + + const conditionToNegate = isNonEmptyString(negatedCondition) + ? negatedCondition + : ALWAYS_TRUE_CONDITION; + + return `NOT (${conditionToNegate})`; + } + default: + return renderFieldCondition(filterKey, filterValue, context); + } +}; + +const renderLogicalGroup = ( + filters: RecordGqlOperationFilter[] | RecordGqlOperationFilter, + logicalOperator: 'AND' | 'OR', + context: SqlRenderingContext, +): string => { + const filterList = Array.isArray(filters) ? filters : [filters]; + + const conditions = filterList.map((filter) => { + const renderedCondition = renderFilterAsConjunction(filter, context); + + return isNonEmptyString(renderedCondition) + ? renderedCondition + : ALWAYS_TRUE_CONDITION; + }); + + if (conditions.length === 0) { + return ALWAYS_TRUE_CONDITION; + } + + return `(${conditions.join(` ${logicalOperator} `)})`; +}; + +const joinConditions = ( + conditions: string[], + logicalOperator: 'AND' | 'OR', +): string => { + if (conditions.length === 0) { + return ''; + } + + if (conditions.length === 1) { + return conditions[0]; + } + + return `(${conditions.join(` ${logicalOperator} `)})`; +}; + +const renderFieldCondition = ( + fieldNameOrJoinColumnName: string, + filterValue: unknown, + context: SqlRenderingContext, +): string => { + const { + tableAlias, + fieldIdByName, + fieldIdByJoinColumnName, + flatFieldMetadataMaps, + } = context; + + const { fieldMetadata, isReferencedByFieldName } = + resolveFilterKeyFieldMetadata({ + filterKey: fieldNameOrJoinColumnName, + fieldIdByName, + fieldIdByJoinColumnName, + flatFieldMetadataMaps, + }); + + if (!isDefined(fieldMetadata)) { + throw new TwentyORMException( + `Cannot render row level permission predicate: field "${fieldNameOrJoinColumnName}" does not exist on object "${tableAlias}"`, + TwentyORMExceptionCode.MALFORMED_METADATA, + ); + } + + if ( + isReferencedByFieldName && + isMorphOrRelationFlatFieldMetadata(fieldMetadata) + ) { + throw new TwentyORMException( + `Cannot render row level permission predicate on relation "${fieldNameOrJoinColumnName}": traversing a relation requires an additional join, which a join condition cannot express`, + TwentyORMExceptionCode.MALFORMED_METADATA, + ); + } + + if (isCompositeFieldMetadataType(fieldMetadata.type)) { + return renderCompositeFieldCondition(fieldMetadata, filterValue, context); + } + + const operatorConditions = Object.entries( + filterValue as Record, + ).map(([operator, operatorValue]) => { + assertArrayOperatorValueIsNonEmptyArray({ + operator, + value: operatorValue, + key: fieldNameOrJoinColumnName, + }); + + const { sql, params } = computeWhereConditionParts({ + operator, + objectNameSingular: tableAlias, + key: fieldNameOrJoinColumnName, + value: operatorValue, + fieldMetadataType: fieldMetadata.type, + }); + + Object.assign(context.collectedParameters, params); + + return `(${sql})`; + }); + + return joinConditions(operatorConditions, 'AND'); +}; + +const renderCompositeFieldCondition = ( + fieldMetadata: FlatFieldMetadata, + filterValue: unknown, + context: SqlRenderingContext, +): string => { + const compositeType = compositeTypeDefinitions.get( + fieldMetadata.type as CompositeFieldMetadataType, + ); + + if (!isDefined(compositeType)) { + throw new TwentyORMException( + `Cannot render row level permission predicate: composite type definition not found for type "${fieldMetadata.type}"`, + TwentyORMExceptionCode.MALFORMED_METADATA, + ); + } + + const conditions = Object.entries( + filterValue as Record>, + ).flatMap(([subFieldName, subFieldFilter]) => { + const isKnownSubField = compositeType.properties.some( + (property) => property.name === subFieldName, + ); + + if (!isKnownSubField) { + throw new TwentyORMException( + `Cannot render row level permission predicate: "${subFieldName}" is not a sub field of composite type "${fieldMetadata.type}"`, + TwentyORMExceptionCode.MALFORMED_METADATA, + ); + } + + return Object.entries(subFieldFilter).map(([operator, operatorValue]) => { + assertArrayOperatorValueIsNonEmptyArray({ + operator, + value: operatorValue, + key: subFieldName, + }); + + const { sql, params } = computeWhereConditionParts({ + operator, + objectNameSingular: context.tableAlias, + key: `${fieldMetadata.name}${capitalize(subFieldName)}`, + subFieldKey: subFieldName, + value: operatorValue, + fieldMetadataType: fieldMetadata.type, + }); + + Object.assign(context.collectedParameters, params); + + return `(${sql})`; + }); + }); + + return joinConditions(conditions, 'AND'); +}; diff --git a/packages/twenty-server/src/engine/twenty-orm/utils/resolve-row-level-permission-record-filter.util.ts b/packages/twenty-server/src/engine/twenty-orm/utils/resolve-row-level-permission-record-filter.util.ts new file mode 100644 index 0000000000..d4a7b1a042 --- /dev/null +++ b/packages/twenty-server/src/engine/twenty-orm/utils/resolve-row-level-permission-record-filter.util.ts @@ -0,0 +1,46 @@ +/* @license Enterprise */ + +import { type RecordGqlOperationFilter } from 'twenty-shared/types'; +import { isDefined } from 'twenty-shared/utils'; + +import { isUserAuthContext } from 'src/engine/core-modules/auth/guards/is-user-auth-context.guard'; +import { type WorkspaceAuthContext } from 'src/engine/core-modules/auth/types/workspace-auth-context.type'; +import { type FlatObjectMetadata } from 'src/engine/metadata-modules/flat-object-metadata/types/flat-object-metadata.type'; +import { type WorkspaceInternalContext } from 'src/engine/twenty-orm/interfaces/workspace-internal-context.interface'; +import { buildRowLevelPermissionRecordFilter } from 'src/engine/twenty-orm/utils/build-row-level-permission-record-filter.util'; +import { resolveRoleIdsFromAuthContext } from 'src/engine/twenty-orm/utils/resolve-role-ids-from-auth-context.util'; + +export const resolveRowLevelPermissionRecordFilter = ({ + internalContext, + authContext, + objectMetadata, +}: { + internalContext: WorkspaceInternalContext; + authContext: WorkspaceAuthContext; + objectMetadata: FlatObjectMetadata; +}): RecordGqlOperationFilter | null => { + const roleIds = resolveRoleIdsFromAuthContext({ + authContext, + userWorkspaceRoleMap: internalContext.userWorkspaceRoleMap, + apiKeyRoleMap: internalContext.apiKeyRoleMap, + }); + + const recordFilter = buildRowLevelPermissionRecordFilter({ + flatRowLevelPermissionPredicateMaps: + internalContext.flatRowLevelPermissionPredicateMaps, + flatRowLevelPermissionPredicateGroupMaps: + internalContext.flatRowLevelPermissionPredicateGroupMaps, + flatFieldMetadataMaps: internalContext.flatFieldMetadataMaps, + objectMetadata, + roleIds, + workspaceMember: isUserAuthContext(authContext) + ? authContext.workspaceMember + : undefined, + }); + + if (!isDefined(recordFilter) || Object.keys(recordFilter).length === 0) { + return null; + } + + return recordFilter; +}; diff --git a/packages/twenty-server/src/engine/twenty-orm/utils/validate-rls-predicates-for-records.util.ts b/packages/twenty-server/src/engine/twenty-orm/utils/validate-rls-predicates-for-records.util.ts index a3a4ae5482..cce4a1d4bc 100644 --- a/packages/twenty-server/src/engine/twenty-orm/utils/validate-rls-predicates-for-records.util.ts +++ b/packages/twenty-server/src/engine/twenty-orm/utils/validate-rls-predicates-for-records.util.ts @@ -1,21 +1,19 @@ /* @license Enterprise */ import { type ObjectRecord } from 'twenty-shared/types'; -import { isNonEmptyArray } from 'twenty-shared/utils'; +import { isDefined } from 'twenty-shared/utils'; import { type ObjectLiteral } from 'typeorm'; import { type WorkspaceInternalContext } from 'src/engine/twenty-orm/interfaces/workspace-internal-context.interface'; import { type WorkspaceAuthContext } from 'src/engine/core-modules/auth/types/workspace-auth-context.type'; -import { isUserAuthContext } from 'src/engine/core-modules/auth/guards/is-user-auth-context.guard'; import { type FlatObjectMetadata } from 'src/engine/metadata-modules/flat-object-metadata/types/flat-object-metadata.type'; import { TwentyORMException, TwentyORMExceptionCode, } from 'src/engine/twenty-orm/exceptions/twenty-orm.exception'; -import { buildRowLevelPermissionRecordFilter } from 'src/engine/twenty-orm/utils/build-row-level-permission-record-filter.util'; import { isRecordMatchingRLSRowLevelPermissionPredicate } from 'src/engine/twenty-orm/utils/is-record-matching-rls-row-level-permission-predicate.util'; -import { resolveRoleIdsFromAuthContext } from 'src/engine/twenty-orm/utils/resolve-role-ids-from-auth-context.util'; +import { resolveRowLevelPermissionRecordFilter } from 'src/engine/twenty-orm/utils/resolve-row-level-permission-record-filter.util'; type ValidateRLSPredicatesForRecordsArgs = { records: T[]; @@ -38,30 +36,13 @@ export const validateRLSPredicatesForRecords = ({ return; } - const roleIds = resolveRoleIdsFromAuthContext({ + const recordFilter = resolveRowLevelPermissionRecordFilter({ + internalContext, authContext, - userWorkspaceRoleMap: internalContext.userWorkspaceRoleMap, - apiKeyRoleMap: internalContext.apiKeyRoleMap, - }); - - if (!isNonEmptyArray(roleIds)) { - return; - } - - const recordFilter = buildRowLevelPermissionRecordFilter({ - flatRowLevelPermissionPredicateMaps: - internalContext.flatRowLevelPermissionPredicateMaps, - flatRowLevelPermissionPredicateGroupMaps: - internalContext.flatRowLevelPermissionPredicateGroupMaps, - flatFieldMetadataMaps: internalContext.flatFieldMetadataMaps, objectMetadata, - roleIds, - workspaceMember: isUserAuthContext(authContext) - ? authContext.workspaceMember - : undefined, }); - if (!recordFilter || Object.keys(recordFilter).length === 0) { + if (!isDefined(recordFilter)) { return; } diff --git a/packages/twenty-server/test/integration/graphql/suites/group-by-by-relation-rls.integration-spec.ts b/packages/twenty-server/test/integration/graphql/suites/group-by-by-relation-rls.integration-spec.ts new file mode 100644 index 0000000000..fe6fedbb3e --- /dev/null +++ b/packages/twenty-server/test/integration/graphql/suites/group-by-by-relation-rls.integration-spec.ts @@ -0,0 +1,135 @@ +import { groupByOperationFactory } from 'test/integration/graphql/utils/group-by-operation-factory.util'; +import { makeGraphqlAPIRequest } from 'test/integration/graphql/utils/make-graphql-api-request.util'; +import { + type CompanyNameRlsRoleSetup, + cleanupCompanyNameRlsRole, + setupCompanyNameRlsRole, +} from 'test/integration/graphql/utils/setup-company-name-rls-role.util'; +import { + type RlsCompanyRelationRecords, + cleanupRlsCompanyRelationRecords, + setupRlsCompanyRelationRecords, +} from 'test/integration/graphql/utils/setup-rls-company-relation-records.util'; +import { jestExpectToBeDefined } from 'test/utils/jest-expect-to-be-defined.util.test'; + +const SEEDED_PEOPLE_COUNT = 3; +const PEOPLE_LINKED_TO_VISIBLE_COMPANY_COUNT = 1; +const PEOPLE_LINKED_TO_HIDDEN_COMPANY_OR_NO_COMPANY_COUNT = 2; + +const RECORDS_CREATED_AT = '2019-06-15T10:00:00.000Z'; +const RECORDS_WINDOW_FILTER = { + and: [ + { createdAt: { gte: '2019-06-15T00:00:00.000Z' } }, + { createdAt: { lte: '2019-06-15T23:59:59.999Z' } }, + ], +}; + +describe('group-by on a relation respects row-level permission predicates', () => { + let rlsRole: CompanyNameRlsRoleSetup; + let records: RlsCompanyRelationRecords; + + beforeAll(async () => { + rlsRole = await setupCompanyNameRlsRole({ + label: 'RLS GroupBy Relation Test Role', + description: 'Role for testing RLS on relation group-by', + }); + + records = await setupRlsCompanyRelationRecords({ + companyNamePrefix: 'RLS GroupBy Relation', + createdAt: RECORDS_CREATED_AT, + }); + }); + + afterAll(async () => { + await cleanupRlsCompanyRelationRecords(records); + await cleanupCompanyNameRlsRole(rlsRole); + }); + + it('does not expose hidden related dimension values and folds hidden-linked records into the null group', async () => { + const response = await makeGraphqlAPIRequest( + groupByOperationFactory({ + objectMetadataSingularName: 'person', + objectMetadataPluralName: 'people', + groupBy: [{ company: { name: true } }], + filter: RECORDS_WINDOW_FILTER, + }), + APPLE_JONY_MEMBER_ACCESS_TOKEN, + ); + + expect(response.body.errors).toBeUndefined(); + + const groups = response.body.data.peopleGroupBy; + + jestExpectToBeDefined(groups); + + const dimensionValues = groups.flatMap( + (group: { groupByDimensionValues: unknown[] }) => + group.groupByDimensionValues, + ); + + expect(dimensionValues).toContain(records.visibleCompanyName); + expect(dimensionValues).not.toContain(records.hiddenCompanyName); + + const peopleCountAcrossAllGroups = groups.reduce( + (sum: number, group: { totalCount: number }) => sum + group.totalCount, + 0, + ); + + expect(peopleCountAcrossAllGroups).toBe(SEEDED_PEOPLE_COUNT); + + const visibleCompanyGroup = groups.find( + (group: { groupByDimensionValues: unknown[] }) => + group.groupByDimensionValues.includes(records.visibleCompanyName), + ); + + expect(visibleCompanyGroup?.totalCount).toBe( + PEOPLE_LINKED_TO_VISIBLE_COMPANY_COUNT, + ); + + const groupWithoutVisibleCompanyName = groups.find( + (group: { groupByDimensionValues: unknown[] }) => + !group.groupByDimensionValues.includes(records.visibleCompanyName), + ); + + expect(groupWithoutVisibleCompanyName?.totalCount).toBe( + PEOPLE_LINKED_TO_HIDDEN_COMPANY_OR_NO_COMPANY_COUNT, + ); + }); + + it('sorts records linked to a hidden related record as null when ordering records within groups by that relation', async () => { + const response = await makeGraphqlAPIRequest( + groupByOperationFactory({ + objectMetadataSingularName: 'person', + objectMetadataPluralName: 'people', + groupBy: [{ jobTitle: true }], + filter: RECORDS_WINDOW_FILTER, + orderByForRecords: [{ company: { name: 'AscNullsLast' } }], + offsetForRecords: 1, + gqlFields: 'edges { node { id } }', + }), + APPLE_JONY_MEMBER_ACCESS_TOKEN, + ); + + expect(response.body.errors).toBeUndefined(); + + const groups = response.body.data.peopleGroupBy; + + jestExpectToBeDefined(groups); + expect(groups).toHaveLength(1); + + const recordIdsAfterSkippingFirstRankedRecord = groups[0].edges.map( + (edge: { node: { id: string } }) => edge.node.id, + ); + + expect(recordIdsAfterSkippingFirstRankedRecord).not.toContain( + records.personWithVisibleCompanyId, + ); + expect(recordIdsAfterSkippingFirstRankedRecord).toEqual( + expect.arrayContaining([ + records.personWithHiddenCompanyId, + records.personWithoutCompanyId, + ]), + ); + expect(recordIdsAfterSkippingFirstRankedRecord).toHaveLength(2); + }); +}); 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 index bf777d522a..f9543758d3 100644 --- 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 @@ -5,16 +5,11 @@ import { COMPANY_GQL_FIELDS } from 'test/integration/constants/company-gql-field 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 { jestExpectToBeDefined } from 'test/utils/jest-expect-to-be-defined.util.test'; -import { 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'; +import { + type CompanyNameRlsRoleSetup, + cleanupCompanyNameRlsRole, + setupCompanyNameRlsRole, +} from 'test/integration/graphql/utils/setup-company-name-rls-role.util'; const FILTER_2020 = { and: [ @@ -22,149 +17,53 @@ const FILTER_2020 = { { createdAt: { lte: '2020-03-03T23:59:59.999Z' } }, ], }; +const COMPANIES_CREATED_AT = '2020-02-05T08:00:00.000Z'; +const VISIBLE_COMPANY_NAME = 'RLS Visible Company'; +const HIDDEN_COMPANY_NAME = 'RLS Hidden Company'; 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; + + let rlsRole: CompanyNameRlsRoleSetup; beforeAll(async () => { - const { objects } = await findManyObjectMetadata({ - expectToFail: false, - input: { - filter: {}, - paging: { first: 1000 }, - }, - gqlFields: ` - id - nameSingular - fieldsList { - id - name - } - `, + rlsRole = await setupCompanyNameRlsRole({ + label: 'RLS GroupBy Test Role', + description: 'Role for testing RLS in group-by with records', }); - 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', + for (const { id, name } of [ + { id: testCompanyId1, name: VISIBLE_COMPANY_NAME }, + { id: testCompanyId2, name: HIDDEN_COMPANY_NAME }, + ]) { + await makeGraphqlAPIRequest( + createOneOperationFactory({ + objectMetadataSingularName: 'company', + gqlFields: COMPANY_GQL_FIELDS, + data: { + id, + name, + employees: 99, + createdAt: COMPANIES_CREATED_AT, }, - ], - 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]) { + for (const recordId of [testCompanyId1, testCompanyId2]) { await makeGraphqlAPIRequest( destroyOneOperationFactory({ objectMetadataSingularName: 'company', gqlFields: 'id', - recordId: id, + recordId, }), ); } - if (customRoleId) { - await deleteOneRole({ - expectToFail: false, - input: { idToDelete: customRoleId }, - }); - } + await cleanupCompanyNameRlsRole(rlsRole); }); it('filters records in group-by results based on RLS predicates', async () => { @@ -211,10 +110,10 @@ describe('group-by with records respects row-level permission predicates', () => ); const visibleRecords = allRecords.filter( - (record: { name: string }) => record.name === 'RLS Visible Company', + (record: { name: string }) => record.name === VISIBLE_COMPANY_NAME, ); const hiddenRecords = allRecords.filter( - (record: { name: string }) => record.name === 'RLS Hidden Company', + (record: { name: string }) => record.name === HIDDEN_COMPANY_NAME, ); expect(visibleRecords).toHaveLength(1); diff --git a/packages/twenty-server/test/integration/graphql/suites/mutation-relation-filter-rls.integration-spec.ts b/packages/twenty-server/test/integration/graphql/suites/mutation-relation-filter-rls.integration-spec.ts new file mode 100644 index 0000000000..aaa2ad77c5 --- /dev/null +++ b/packages/twenty-server/test/integration/graphql/suites/mutation-relation-filter-rls.integration-spec.ts @@ -0,0 +1,73 @@ +import { makeGraphqlAPIRequest } from 'test/integration/graphql/utils/make-graphql-api-request.util'; +import { + type CompanyNameRlsRoleSetup, + cleanupCompanyNameRlsRole, + setupCompanyNameRlsRole, +} from 'test/integration/graphql/utils/setup-company-name-rls-role.util'; +import { + type RlsCompanyRelationRecords, + cleanupRlsCompanyRelationRecords, + setupRlsCompanyRelationRecords, +} from 'test/integration/graphql/utils/setup-rls-company-relation-records.util'; +import { updateManyOperationFactory } from 'test/integration/graphql/utils/update-many-operation-factory.util'; + +const RECORDS_CREATED_AT = '2019-08-15T10:00:00.000Z'; + +describe('mutations filtered on a relation respect row-level permission predicates', () => { + let rlsRole: CompanyNameRlsRoleSetup; + let records: RlsCompanyRelationRecords; + + beforeAll(async () => { + rlsRole = await setupCompanyNameRlsRole({ + label: 'RLS Mutation Relation Filter Test Role', + description: 'Role for testing RLS on mutations filtered on a relation', + }); + + records = await setupRlsCompanyRelationRecords({ + companyNamePrefix: 'RLS Mutation Relation Filter', + createdAt: RECORDS_CREATED_AT, + }); + }); + + afterAll(async () => { + await cleanupRlsCompanyRelationRecords(records); + await cleanupCompanyNameRlsRole(rlsRole); + }); + + it('does not update records linked to a hidden related record', async () => { + const response = await makeGraphqlAPIRequest( + updateManyOperationFactory({ + objectMetadataSingularName: 'person', + objectMetadataPluralName: 'people', + gqlFields: 'id', + data: { jobTitle: 'Updated through a hidden company' }, + filter: { company: { name: { eq: records.hiddenCompanyName } } }, + }), + APPLE_JONY_MEMBER_ACCESS_TOKEN, + ); + + expect(response.body.errors).toBeUndefined(); + expect(response.body.data.updatePeople).toEqual([]); + }); + + it('updates records linked to a visible related record', async () => { + const response = await makeGraphqlAPIRequest( + updateManyOperationFactory({ + objectMetadataSingularName: 'person', + objectMetadataPluralName: 'people', + gqlFields: 'id', + data: { jobTitle: 'Updated through a visible company' }, + filter: { company: { name: { eq: records.visibleCompanyName } } }, + }), + APPLE_JONY_MEMBER_ACCESS_TOKEN, + ); + + expect(response.body.errors).toBeUndefined(); + + const updatedRecordIds = response.body.data.updatePeople.map( + (record: { id: string }) => record.id, + ); + + expect(updatedRecordIds).toEqual([records.personWithVisibleCompanyId]); + }); +}); diff --git a/packages/twenty-server/test/integration/graphql/suites/nested-relation-per-parent-limit-rls.integration-spec.ts b/packages/twenty-server/test/integration/graphql/suites/nested-relation-per-parent-limit-rls.integration-spec.ts new file mode 100644 index 0000000000..ae426f3761 --- /dev/null +++ b/packages/twenty-server/test/integration/graphql/suites/nested-relation-per-parent-limit-rls.integration-spec.ts @@ -0,0 +1,137 @@ +import { createManyOperationFactory } from 'test/integration/graphql/utils/create-many-operation-factory.util'; +import { destroyManyOperationFactory } from 'test/integration/graphql/utils/destroy-many-operation-factory.util'; +import { findManyOperationFactory } from 'test/integration/graphql/utils/find-many-operation-factory.util'; +import { makeGraphqlAPIRequest } from 'test/integration/graphql/utils/make-graphql-api-request.util'; +import { + type CompanyNameRlsRoleSetup, + VISIBLE_COMPANY_NAME_TOKEN, + cleanupCompanyNameRlsRole, + setupCompanyNameRlsRole, +} from 'test/integration/graphql/utils/setup-company-name-rls-role.util'; +import { upsertContainsRlsPredicate } from 'test/integration/graphql/utils/upsert-contains-rls-predicate.util'; +import { QUERY_MAX_RECORDS_FROM_RELATION } from 'twenty-shared/constants'; + +const COMPANY_ID = '20202020-dddd-4000-8000-000000000001'; +const COMPANY_NAME = `RLS Per Parent Limit ${VISIBLE_COMPANY_NAME_TOKEN} Co`; + +const VISIBLE_JOB_TITLE_TOKEN = 'Visible'; +const HIDDEN_PEOPLE_COUNT = QUERY_MAX_RECORDS_FROM_RELATION + 1; +const VISIBLE_PEOPLE_COUNT = 2; + +const buildPersonId = (index: number) => + `20202020-dddd-4001-8000-${index.toString().padStart(12, '0')}`; + +const HIDDEN_PERSON_IDS_INSERTED_FIRST = Array.from( + { length: HIDDEN_PEOPLE_COUNT }, + (_, i) => buildPersonId(i), +); +const VISIBLE_PERSON_IDS = Array.from( + { length: VISIBLE_PEOPLE_COUNT }, + (_, i) => buildPersonId(HIDDEN_PEOPLE_COUNT + i), +); + +describe('nested relation per-parent limit respects row-level permission predicates', () => { + let rlsRole: CompanyNameRlsRoleSetup; + + beforeAll(async () => { + rlsRole = await setupCompanyNameRlsRole({ + label: 'RLS Per Parent Limit Test Role', + description: 'Role for testing RLS on the per-parent relation limit', + }); + + await upsertContainsRlsPredicate({ + roleId: rlsRole.customRoleId, + objectNameSingular: 'person', + fieldName: 'jobTitle', + value: VISIBLE_JOB_TITLE_TOKEN, + }); + + await makeGraphqlAPIRequest( + createManyOperationFactory({ + objectMetadataSingularName: 'company', + objectMetadataPluralName: 'companies', + gqlFields: 'id', + data: [{ id: COMPANY_ID, name: COMPANY_NAME }], + }), + ); + + await makeGraphqlAPIRequest( + createManyOperationFactory({ + objectMetadataSingularName: 'person', + objectMetadataPluralName: 'people', + gqlFields: 'id', + data: [ + ...HIDDEN_PERSON_IDS_INSERTED_FIRST.map((id) => ({ + id, + companyId: COMPANY_ID, + jobTitle: 'Hidden', + })), + ...VISIBLE_PERSON_IDS.map((id) => ({ + id, + companyId: COMPANY_ID, + jobTitle: `${VISIBLE_JOB_TITLE_TOKEN} contributor`, + })), + ], + }), + ); + }); + + afterAll(async () => { + await makeGraphqlAPIRequest( + destroyManyOperationFactory({ + objectMetadataSingularName: 'person', + objectMetadataPluralName: 'people', + gqlFields: 'id', + filter: { + id: { + in: [...HIDDEN_PERSON_IDS_INSERTED_FIRST, ...VISIBLE_PERSON_IDS], + }, + }, + }), + ); + + await makeGraphqlAPIRequest( + destroyManyOperationFactory({ + objectMetadataSingularName: 'company', + objectMetadataPluralName: 'companies', + gqlFields: 'id', + filter: { id: { in: [COMPANY_ID] } }, + }), + ); + + await cleanupCompanyNameRlsRole(rlsRole); + }); + + it('does not let hidden related records consume the per-parent limit', async () => { + const response = await makeGraphqlAPIRequest( + findManyOperationFactory({ + objectMetadataSingularName: 'company', + objectMetadataPluralName: 'companies', + gqlFields: ` + id + people { + edges { + node { + id + } + } + } + `, + filter: { id: { in: [COMPANY_ID] } }, + }), + APPLE_JONY_MEMBER_ACCESS_TOKEN, + ); + + expect(response.body.errors).toBeUndefined(); + + const companyEdges = response.body.data.companies.edges; + + expect(companyEdges).toHaveLength(1); + + const relatedPeopleIds = companyEdges[0].node.people.edges.map( + (edge: { node: { id: string } }) => edge.node.id, + ); + + expect(relatedPeopleIds.sort()).toEqual([...VISIBLE_PERSON_IDS].sort()); + }); +}); diff --git a/packages/twenty-server/test/integration/graphql/suites/relation-join-rls.integration-spec.ts b/packages/twenty-server/test/integration/graphql/suites/relation-join-rls.integration-spec.ts new file mode 100644 index 0000000000..f855b7c8cc --- /dev/null +++ b/packages/twenty-server/test/integration/graphql/suites/relation-join-rls.integration-spec.ts @@ -0,0 +1,107 @@ +import { findManyOperationFactory } from 'test/integration/graphql/utils/find-many-operation-factory.util'; +import { makeGraphqlAPIRequest } from 'test/integration/graphql/utils/make-graphql-api-request.util'; +import { + type CompanyNameRlsRoleSetup, + cleanupCompanyNameRlsRole, + setupCompanyNameRlsRole, +} from 'test/integration/graphql/utils/setup-company-name-rls-role.util'; +import { + type RlsCompanyRelationRecords, + cleanupRlsCompanyRelationRecords, + setupRlsCompanyRelationRecords, +} from 'test/integration/graphql/utils/setup-rls-company-relation-records.util'; + +const RECORDS_CREATED_AT = '2019-07-15T10:00:00.000Z'; +const RECORDS_WINDOW_FILTER = { + and: [ + { createdAt: { gte: '2019-07-15T00:00:00.000Z' } }, + { createdAt: { lte: '2019-07-15T23:59:59.999Z' } }, + ], +}; + +describe('relation-filter and order-by respect row-level permission predicates', () => { + let rlsRole: CompanyNameRlsRoleSetup; + let records: RlsCompanyRelationRecords; + + beforeAll(async () => { + rlsRole = await setupCompanyNameRlsRole({ + label: 'RLS Relation Join Test Role', + description: 'Role for testing RLS on relation-filter and order-by', + }); + + records = await setupRlsCompanyRelationRecords({ + companyNamePrefix: 'RLS Relation Join', + createdAt: RECORDS_CREATED_AT, + }); + }); + + afterAll(async () => { + await cleanupRlsCompanyRelationRecords(records); + await cleanupCompanyNameRlsRole(rlsRole); + }); + + it('does not match a relation filter targeting a hidden related record', async () => { + const response = await makeGraphqlAPIRequest( + findManyOperationFactory({ + objectMetadataSingularName: 'person', + objectMetadataPluralName: 'people', + gqlFields: 'id', + filter: { company: { name: { eq: records.hiddenCompanyName } } }, + }), + APPLE_JONY_MEMBER_ACCESS_TOKEN, + ); + + expect(response.body.errors).toBeUndefined(); + expect(response.body.data.people.edges).toHaveLength(0); + }); + + it('matches a relation filter targeting a visible related record', async () => { + const response = await makeGraphqlAPIRequest( + findManyOperationFactory({ + objectMetadataSingularName: 'person', + objectMetadataPluralName: 'people', + gqlFields: 'id', + filter: { company: { name: { eq: records.visibleCompanyName } } }, + }), + APPLE_JONY_MEMBER_ACCESS_TOKEN, + ); + + expect(response.body.errors).toBeUndefined(); + + const ids = response.body.data.people.edges.map( + (edge: { node: { id: string } }) => edge.node.id, + ); + + expect(ids).toEqual([records.personWithVisibleCompanyId]); + }); + + it('sorts records linked to a hidden related record as null when ordering by that relation', async () => { + const response = await makeGraphqlAPIRequest( + findManyOperationFactory({ + objectMetadataSingularName: 'person', + objectMetadataPluralName: 'people', + gqlFields: 'id', + filter: RECORDS_WINDOW_FILTER, + orderBy: [{ company: { name: 'AscNullsLast' } }], + first: 10, + }), + APPLE_JONY_MEMBER_ACCESS_TOKEN, + ); + + expect(response.body.errors).toBeUndefined(); + + const [personIdWithNonNullSortKey, ...personIdsSortedAsNull] = + response.body.data.people.edges.map( + (edge: { node: { id: string } }) => edge.node.id, + ); + + expect(personIdWithNonNullSortKey).toBe(records.personWithVisibleCompanyId); + expect(personIdsSortedAsNull).toEqual( + expect.arrayContaining([ + records.personWithHiddenCompanyId, + records.personWithoutCompanyId, + ]), + ); + expect(personIdsSortedAsNull).toHaveLength(2); + }); +}); diff --git a/packages/twenty-server/test/integration/graphql/utils/group-by-operation-factory.util.ts b/packages/twenty-server/test/integration/graphql/utils/group-by-operation-factory.util.ts index 6a6060e7b6..11808483f0 100644 --- a/packages/twenty-server/test/integration/graphql/utils/group-by-operation-factory.util.ts +++ b/packages/twenty-server/test/integration/graphql/utils/group-by-operation-factory.util.ts @@ -11,6 +11,7 @@ type GroupByOperationFactoryParams = { viewId?: string; gqlFields?: string; limit?: number; + offsetForRecords?: number; }; export const groupByOperationFactory = ({ @@ -23,10 +24,11 @@ export const groupByOperationFactory = ({ viewId, gqlFields, limit, + offsetForRecords, }: GroupByOperationFactoryParams) => ({ query: gql` - query ${capitalize(objectMetadataPluralName)}GroupBy($groupBy: [${capitalize(objectMetadataSingularName)}GroupByInput!]!, $filter: ${capitalize(objectMetadataSingularName)}FilterInput, $orderBy: [${capitalize(objectMetadataSingularName)}OrderByWithGroupByInput!], $viewId: UUID, $limit: Int) { - ${objectMetadataPluralName}GroupBy(groupBy: $groupBy, filter: $filter, orderBy: $orderBy, viewId: $viewId, limit: $limit) { + query ${capitalize(objectMetadataPluralName)}GroupBy($groupBy: [${capitalize(objectMetadataSingularName)}GroupByInput!]!, $filter: ${capitalize(objectMetadataSingularName)}FilterInput, $orderBy: [${capitalize(objectMetadataSingularName)}OrderByWithGroupByInput!], $orderByForRecords: [${capitalize(objectMetadataSingularName)}OrderByInput!], $viewId: UUID, $limit: Int, $offsetForRecords: Int) { + ${objectMetadataPluralName}GroupBy(groupBy: $groupBy, filter: $filter, orderBy: $orderBy, orderByForRecords: $orderByForRecords, viewId: $viewId, limit: $limit, offsetForRecords: $offsetForRecords) { ${gqlFields ? gqlFields : ''} groupByDimensionValues totalCount @@ -39,6 +41,7 @@ export const groupByOperationFactory = ({ orderBy, orderByForRecords, limit, + offsetForRecords, ...(viewId && { viewId }), }, }); diff --git a/packages/twenty-server/test/integration/graphql/utils/setup-company-name-rls-role.util.ts b/packages/twenty-server/test/integration/graphql/utils/setup-company-name-rls-role.util.ts new file mode 100644 index 0000000000..ab44290025 --- /dev/null +++ b/packages/twenty-server/test/integration/graphql/utils/setup-company-name-rls-role.util.ts @@ -0,0 +1,86 @@ +import { upsertContainsRlsPredicate } from 'test/integration/graphql/utils/upsert-contains-rls-predicate.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 { jestExpectToBeDefined } from 'test/utils/jest-expect-to-be-defined.util.test'; +import { isDefined } from 'twenty-shared/utils'; + +import { WORKSPACE_MEMBER_DATA_SEED_IDS } from 'src/engine/workspace-manager/dev-seeder/data/constants/workspace-member-data-seeds.constant'; + +export const VISIBLE_COMPANY_NAME_TOKEN = 'Visible'; + +export type CompanyNameRlsRoleSetup = { + customRoleId: string; + originalMemberRoleId: string; +}; + +export const setupCompanyNameRlsRole = async ({ + label, + description, +}: { + label: string; + description: string; +}): Promise => { + const memberRole = await findOneRoleByLabel({ label: 'Member' }); + + const { data: roleData } = await createOneRole({ + expectToFail: false, + input: { + label, + description, + icon: 'IconSettings', + canUpdateAllSettings: false, + canAccessAllTools: true, + canReadAllObjectRecords: true, + canUpdateAllObjectRecords: true, + canSoftDeleteAllObjectRecords: false, + canDestroyAllObjectRecords: false, + canBeAssignedToUsers: true, + canBeAssignedToAgents: false, + canBeAssignedToApiKeys: false, + }, + }); + + const customRoleId = roleData?.createOneRole?.id; + + jestExpectToBeDefined(customRoleId); + + await upsertContainsRlsPredicate({ + roleId: customRoleId, + objectNameSingular: 'company', + fieldName: 'name', + value: VISIBLE_COMPANY_NAME_TOKEN, + }); + + await updateWorkspaceMemberRole({ + input: { + roleId: customRoleId, + workspaceMemberId: WORKSPACE_MEMBER_DATA_SEED_IDS.JONY, + }, + expectToFail: false, + }); + + return { customRoleId, originalMemberRoleId: memberRole.id }; +}; + +export const cleanupCompanyNameRlsRole = async ( + setup: Partial = {}, +): Promise => { + if (isDefined(setup.originalMemberRoleId)) { + await updateWorkspaceMemberRole({ + input: { + workspaceMemberId: WORKSPACE_MEMBER_DATA_SEED_IDS.JONY, + roleId: setup.originalMemberRoleId, + }, + expectToFail: false, + }); + } + + if (isDefined(setup.customRoleId)) { + await deleteOneRole({ + expectToFail: false, + input: { idToDelete: setup.customRoleId }, + }); + } +}; diff --git a/packages/twenty-server/test/integration/graphql/utils/setup-rls-company-relation-records.util.ts b/packages/twenty-server/test/integration/graphql/utils/setup-rls-company-relation-records.util.ts new file mode 100644 index 0000000000..1f1700399d --- /dev/null +++ b/packages/twenty-server/test/integration/graphql/utils/setup-rls-company-relation-records.util.ts @@ -0,0 +1,110 @@ +import { randomUUID } from 'crypto'; + +import { COMPANY_GQL_FIELDS } from 'test/integration/constants/company-gql-fields.constants'; +import { PERSON_GQL_FIELDS } from 'test/integration/constants/person-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 { VISIBLE_COMPANY_NAME_TOKEN } from 'test/integration/graphql/utils/setup-company-name-rls-role.util'; +import { isDefined } from 'twenty-shared/utils'; + +export type RlsCompanyRelationRecords = { + visibleCompanyId: string; + hiddenCompanyId: string; + visibleCompanyName: string; + hiddenCompanyName: string; + personWithVisibleCompanyId: string; + personWithHiddenCompanyId: string; + personWithoutCompanyId: string; +}; + +export const setupRlsCompanyRelationRecords = async ({ + companyNamePrefix, + createdAt, +}: { + companyNamePrefix: string; + createdAt: string; +}): Promise => { + const records: RlsCompanyRelationRecords = { + visibleCompanyId: randomUUID(), + hiddenCompanyId: randomUUID(), + visibleCompanyName: `${companyNamePrefix} ${VISIBLE_COMPANY_NAME_TOKEN} Co`, + hiddenCompanyName: `${companyNamePrefix} Hidden Co`, + personWithVisibleCompanyId: randomUUID(), + personWithHiddenCompanyId: randomUUID(), + personWithoutCompanyId: randomUUID(), + }; + + for (const { id, name } of [ + { id: records.visibleCompanyId, name: records.visibleCompanyName }, + { id: records.hiddenCompanyId, name: records.hiddenCompanyName }, + ]) { + await makeGraphqlAPIRequest( + createOneOperationFactory({ + objectMetadataSingularName: 'company', + gqlFields: COMPANY_GQL_FIELDS, + data: { id, name, createdAt }, + }), + ); + } + + for (const { id, companyId } of [ + { + id: records.personWithVisibleCompanyId, + companyId: records.visibleCompanyId, + }, + { + id: records.personWithHiddenCompanyId, + companyId: records.hiddenCompanyId, + }, + { id: records.personWithoutCompanyId, companyId: undefined }, + ]) { + await makeGraphqlAPIRequest( + createOneOperationFactory({ + objectMetadataSingularName: 'person', + gqlFields: PERSON_GQL_FIELDS, + data: { + id, + createdAt, + ...(isDefined(companyId) ? { companyId } : {}), + }, + }), + ); + } + + return records; +}; + +export const cleanupRlsCompanyRelationRecords = async ( + records: RlsCompanyRelationRecords | undefined, +): Promise => { + if (!isDefined(records)) { + return; + } + + const peopleIds = [ + records.personWithVisibleCompanyId, + records.personWithHiddenCompanyId, + records.personWithoutCompanyId, + ]; + + for (const recordId of peopleIds) { + await makeGraphqlAPIRequest( + destroyOneOperationFactory({ + objectMetadataSingularName: 'person', + gqlFields: 'id', + recordId, + }), + ); + } + + for (const recordId of [records.visibleCompanyId, records.hiddenCompanyId]) { + await makeGraphqlAPIRequest( + destroyOneOperationFactory({ + objectMetadataSingularName: 'company', + gqlFields: 'id', + recordId, + }), + ); + } +}; diff --git a/packages/twenty-server/test/integration/graphql/utils/upsert-contains-rls-predicate.util.ts b/packages/twenty-server/test/integration/graphql/utils/upsert-contains-rls-predicate.util.ts new file mode 100644 index 0000000000..43e8306ee7 --- /dev/null +++ b/packages/twenty-server/test/integration/graphql/utils/upsert-contains-rls-predicate.util.ts @@ -0,0 +1,59 @@ +import { findManyObjectMetadata } from 'test/integration/metadata/suites/object-metadata/utils/find-many-object-metadata.util'; +import { upsertRowLevelPermissionPredicates } from 'test/integration/metadata/suites/row-level-permission-predicate/utils/upsert-row-level-permission-predicates.util'; +import { jestExpectToBeDefined } from 'test/utils/jest-expect-to-be-defined.util.test'; +import { RowLevelPermissionPredicateOperand } from 'twenty-shared/types'; + +export const upsertContainsRlsPredicate = async ({ + roleId, + objectNameSingular, + fieldName, + value, +}: { + roleId: string; + objectNameSingular: string; + fieldName: string; + value: string; +}): Promise => { + const { objects } = await findManyObjectMetadata({ + expectToFail: false, + input: { filter: {}, paging: { first: 1000 } }, + gqlFields: ` + id + nameSingular + fieldsList { + id + name + } + `, + }); + + jestExpectToBeDefined(objects); + + const objectMetadata = objects.find( + (object) => object.nameSingular === objectNameSingular, + ); + + jestExpectToBeDefined(objectMetadata); + + const fieldMetadata = objectMetadata.fieldsList?.find( + (field) => field.name === fieldName, + ); + + jestExpectToBeDefined(fieldMetadata); + + await upsertRowLevelPermissionPredicates({ + expectToFail: false, + input: { + roleId, + objectMetadataId: objectMetadata.id, + predicates: [ + { + fieldMetadataId: fieldMetadata.id, + operand: RowLevelPermissionPredicateOperand.CONTAINS, + value, + }, + ], + predicateGroups: [], + }, + }); +}; diff --git a/packages/twenty-server/test/utils/create-where-expression-recorder.util.ts b/packages/twenty-server/test/utils/create-where-expression-recorder.util.ts new file mode 100644 index 0000000000..56689381f7 --- /dev/null +++ b/packages/twenty-server/test/utils/create-where-expression-recorder.util.ts @@ -0,0 +1,64 @@ +import { + Brackets, + NotBrackets, + type ObjectLiteral, + type WhereExpressionBuilder, +} from 'typeorm'; + +export type RecordedWhereNode = + | { kind: 'sql'; sql: string; parameters: ObjectLiteral | undefined } + | { kind: 'brackets'; children: RecordedWhereCall[] } + | { kind: 'notBrackets'; children: RecordedWhereCall[] }; + +export type RecordedWhereCall = { + method: 'where' | 'andWhere' | 'orWhere'; + node: RecordedWhereNode; +}; + +export type WhereExpressionRecorder = { + whereExpression: WhereExpressionBuilder; + calls: RecordedWhereCall[]; +}; + +export const createWhereExpressionRecorder = (): WhereExpressionRecorder => { + const calls: RecordedWhereCall[] = []; + + const recordNode = ( + condition: unknown, + parameters: ObjectLiteral | undefined, + ): RecordedWhereNode => { + if (condition instanceof NotBrackets) { + const childRecorder = createWhereExpressionRecorder(); + + condition.whereFactory(childRecorder.whereExpression); + + return { kind: 'notBrackets', children: childRecorder.calls }; + } + + if (condition instanceof Brackets) { + const childRecorder = createWhereExpressionRecorder(); + + condition.whereFactory(childRecorder.whereExpression); + + return { kind: 'brackets', children: childRecorder.calls }; + } + + return { kind: 'sql', sql: String(condition), parameters }; + }; + + const recordCall = + (method: RecordedWhereCall['method']) => + (condition: unknown, parameters?: ObjectLiteral) => { + calls.push({ method, node: recordNode(condition, parameters) }); + + return whereExpression; + }; + + const whereExpression = { + where: recordCall('where'), + andWhere: recordCall('andWhere'), + orWhere: recordCall('orWhere'), + } as unknown as WhereExpressionBuilder; + + return { whereExpression, calls }; +};