From 6f3a86c4a97ebceb3155aefa7e510cf874f45569 Mon Sep 17 00:00:00 2001 From: Weiko Date: Thu, 2 Apr 2026 16:01:51 +0200 Subject: [PATCH] Fix insert conflict between field permission and RLS (#19244) Insert operations blocked by field-level update permissions on non-editable fields The insert code path in permissions.utils.ts fell through to the update case (no break), causing validateUpdateFieldPermissionOrThrow to reject inserts when any field had "Edit disabled" which could conflict with RLS predicates (used for insertion of new records) Fixes https://github.com/twentyhq/twenty/issues/19201 We will keep checking update permissions for insertion (until we decide to have a separate permission flag for insertion) but to fix the issue we will skip this part if it conflicts with an RLS predicate --- .../repository/permissions.utils.ts | 34 +++++ .../update-permissions.integration-spec.ts | 66 ++++++++- .../field-permissions.integration-spec.ts | 132 ++++++++++++++++-- 3 files changed, 218 insertions(+), 14 deletions(-) diff --git a/packages/twenty-server/src/engine/twenty-orm/repository/permissions.utils.ts b/packages/twenty-server/src/engine/twenty-orm/repository/permissions.utils.ts index 8099eb4920..d100067ec0 100644 --- a/packages/twenty-server/src/engine/twenty-orm/repository/permissions.utils.ts +++ b/packages/twenty-server/src/engine/twenty-orm/repository/permissions.utils.ts @@ -139,6 +139,40 @@ export const validateOperationIsPermittedOrThrow = ({ }); break; case 'insert': + if (!permissionsForEntity?.canUpdateObjectRecords) { + throw new PermissionsException( + PermissionsExceptionMessage.PERMISSION_DENIED, + PermissionsExceptionCode.PERMISSION_DENIED, + ); + } + + validateReadFieldPermissionOrThrow({ + restrictedFields: permissionsForEntity.restrictedFields, + selectedColumns, + columnNameToFieldMetadataIdMap, + }); + + if (updatedColumns.length > 0) { + const rlsFieldMetadataIds = new Set( + permissionsForEntity.rowLevelPermissionPredicates.map( + (predicate) => predicate.fieldMetadataId, + ), + ); + + const updatedColumnsWithoutRlsFields = updatedColumns.filter( + (column) => + !rlsFieldMetadataIds.has(columnNameToFieldMetadataIdMap[column]), + ); + + if (updatedColumnsWithoutRlsFields.length > 0) { + validateUpdateFieldPermissionOrThrow({ + restrictedFields: permissionsForEntity.restrictedFields, + updatedColumns: updatedColumnsWithoutRlsFields, + columnNameToFieldMetadataIdMap, + }); + } + } + break; case 'update': if (!permissionsForEntity?.canUpdateObjectRecords) { throw new PermissionsException( diff --git a/packages/twenty-server/test/integration/graphql/suites/object-records-permissions/fields-permissions/update-permissions.integration-spec.ts b/packages/twenty-server/test/integration/graphql/suites/object-records-permissions/fields-permissions/update-permissions.integration-spec.ts index 3a28e53db2..0e196de1eb 100644 --- a/packages/twenty-server/test/integration/graphql/suites/object-records-permissions/fields-permissions/update-permissions.integration-spec.ts +++ b/packages/twenty-server/test/integration/graphql/suites/object-records-permissions/fields-permissions/update-permissions.integration-spec.ts @@ -12,7 +12,9 @@ import { updateManyOperationFactory } from 'test/integration/graphql/utils/updat import { updateOneOperationFactory } from 'test/integration/graphql/utils/update-one-operation-factory.util'; import { updateWorkspaceMemberRole } from 'test/integration/graphql/utils/update-workspace-member-role.util'; import { upsertFieldPermissions } from 'test/integration/graphql/utils/upsert-field-permissions.util'; +import { upsertRowLevelPermissionPredicates } from 'test/integration/metadata/suites/row-level-permission-predicate/utils/upsert-row-level-permission-predicates.util'; import { makeMetadataAPIRequest } from 'test/integration/metadata/suites/utils/make-metadata-api-request.util'; +import { RowLevelPermissionPredicateOperand } from 'twenty-shared/types'; import { ErrorCode } from 'src/engine/core-modules/graphql/utils/graphql-errors.util'; import { PermissionsExceptionMessage } from 'src/engine/metadata-modules/permissions/permissions.exception'; @@ -39,6 +41,27 @@ const expectPermissionDeniedError = (response: any) => { expect(response.body.errors[0].extensions.code).toBe(ErrorCode.FORBIDDEN); }; +const expectEmployeesIsAccessible = ({ + response, + operationName, + expectedEmployees, +}: { + response: any; + operationName: 'createCompanies' | 'createCompany'; + expectedEmployees: number; +}) => { + expect(response.body.errors).toBeUndefined(); + expect(response.body.data).toBeDefined(); + + const result = + operationName === 'createCompany' + ? response.body.data[operationName] + : response.body.data[operationName]?.[0]; + + expect(result).toBeDefined(); + expect(result.employees).toBe(expectedEmployees); +}; + describe('Field update permissions restrictions', () => { let companyId: string; let personId: string; @@ -296,16 +319,41 @@ describe('Field update permissions restrictions', () => { // }); // }); - describe('should block creating with update-restricted field in data', () => { + describe('should allow employees field when creating if field is in RLS predicate', () => { beforeEach(async () => { await restrictUpdateAccessToCompanyEmployee( customRoleId, companyObjectId, restrictedCompanyFieldId, ); + + await upsertRowLevelPermissionPredicates({ + input: { + roleId: customRoleId, + objectMetadataId: companyObjectId, + predicates: [ + { + fieldMetadataId: restrictedCompanyFieldId, + operand: RowLevelPermissionPredicateOperand.IS_NOT_EMPTY, + }, + ], + predicateGroups: [], + }, + }); }); - it('1. createMany with restricted field', async () => { + afterEach(async () => { + await upsertRowLevelPermissionPredicates({ + input: { + roleId: customRoleId, + objectMetadataId: companyObjectId, + predicates: [], + predicateGroups: [], + }, + }); + }); + + it('1. createMany with restricted field in RLS predicate', async () => { const graphqlOperation = createManyOperationFactory({ objectMetadataSingularName: 'company', objectMetadataPluralName: 'companies', @@ -319,10 +367,14 @@ describe('Field update permissions restrictions', () => { const response = await makeGraphqlAPIRequestWithMemberRole(graphqlOperation); - expectPermissionDeniedError(response); + expectEmployeesIsAccessible({ + response, + operationName: 'createCompanies', + expectedEmployees: 15, + }); }); - it('2. createOne with restricted field', async () => { + it('2. createOne with restricted field in RLS predicate', async () => { const graphqlOperation = createOneOperationFactory({ objectMetadataSingularName: 'company', gqlFields: COMPANY_GQL_FIELDS_WITH_EMPLOYEES, @@ -332,7 +384,11 @@ describe('Field update permissions restrictions', () => { const response = await makeGraphqlAPIRequestWithMemberRole(graphqlOperation); - expectPermissionDeniedError(response); + expectEmployeesIsAccessible({ + response, + operationName: 'createCompany', + expectedEmployees: 25, + }); }); }); describe('should block read-restricted field in update operation responses', () => { diff --git a/packages/twenty-server/test/integration/rest/suites/field-permissions.integration-spec.ts b/packages/twenty-server/test/integration/rest/suites/field-permissions.integration-spec.ts index c7ffd619c2..dc165d15ae 100644 --- a/packages/twenty-server/test/integration/rest/suites/field-permissions.integration-spec.ts +++ b/packages/twenty-server/test/integration/rest/suites/field-permissions.integration-spec.ts @@ -3,9 +3,11 @@ import { TEST_COMPANY_1_ID } from 'test/integration/constants/test-company-ids.c import { TEST_PERSON_1_ID } from 'test/integration/constants/test-person-ids.constants'; import { TEST_PRIMARY_LINK_URL } from 'test/integration/constants/test-primary-link-url.constant'; import { upsertFieldPermissions } from 'test/integration/graphql/utils/upsert-field-permissions.util'; +import { upsertRowLevelPermissionPredicates } from 'test/integration/metadata/suites/row-level-permission-predicate/utils/upsert-row-level-permission-predicates.util'; import { makeMetadataAPIRequest } from 'test/integration/metadata/suites/utils/make-metadata-api-request.util'; import { makeRestAPIRequest } from 'test/integration/rest/utils/make-rest-api-request.util'; import { generateRecordName } from 'test/integration/utils/generate-record-name'; +import { RowLevelPermissionPredicateOperand } from 'twenty-shared/types'; describe('Restricted fields', () => { let personCity: string; @@ -253,8 +255,7 @@ describe('Restricted fields', () => { }); describe('createOne', () => { - it('should block create when user has restricted update permissions on phones field', async () => { - // Create field permission restricting update access to phones field + it('should block create when restricted field is not in any RLS predicate', async () => { await upsertFieldPermissions({ roleId: memberRoleId, fieldPermissions: [ @@ -287,8 +288,64 @@ describe('Restricted fields', () => { }); }); + it('should allow create when restricted field is referenced in an RLS predicate', async () => { + await upsertFieldPermissions({ + roleId: memberRoleId, + fieldPermissions: [ + { + objectMetadataId: personObjectId, + fieldMetadataId: phonesFieldId, + canReadFieldValue: null, + canUpdateFieldValue: false, + }, + ], + }); + + await upsertRowLevelPermissionPredicates({ + input: { + roleId: memberRoleId, + objectMetadataId: personObjectId, + predicates: [ + { + fieldMetadataId: phonesFieldId, + operand: RowLevelPermissionPredicateOperand.IS_NOT_EMPTY, + }, + ], + predicateGroups: [], + }, + }); + + await makeRestAPIRequest({ + method: 'post', + path: `/people`, + bearer: APPLE_JONY_MEMBER_ACCESS_TOKEN, + body: { + phones: { + primaryPhoneNumber: '555123456', + primaryPhoneCountryCode: 'US', + primaryPhoneCallingCode: '+1', + }, + }, + }) + .expect(201) + .expect((res) => { + const createdPerson = res.body.data.createPerson; + + expect(createdPerson).toBeDefined(); + expect(createdPerson.phones.primaryPhoneNumber).toBe('555123456'); + }); + + await upsertRowLevelPermissionPredicates({ + input: { + roleId: memberRoleId, + objectMetadataId: personObjectId, + predicates: [], + predicateGroups: [], + }, + }); + }); + it('should allow create when user has no restricted update permissions', async () => { - // Remove field permission restrictions on phones; restrict read on emails so response excludes it await upsertFieldPermissions({ roleId: memberRoleId, fieldPermissions: [ @@ -320,14 +377,13 @@ describe('Restricted fields', () => { const createdPerson = res.body.data.createPerson; expect(createdPerson.city).toBe('New City'); - expect(createdPerson.emails).toBeUndefined(); // No reading rights on emails + expect(createdPerson.emails).toBeUndefined(); }); }); }); describe('createMany', () => { - it('should block createMany when user has restricted update permissions on phones field', async () => { - // Create field permission restricting update access to phones field + it('should block createMany when restricted field is not in any RLS predicate', async () => { await upsertFieldPermissions({ roleId: memberRoleId, fieldPermissions: [ @@ -362,8 +418,66 @@ describe('Restricted fields', () => { }); }); + it('should allow createMany when restricted field is referenced in an RLS predicate', async () => { + await upsertFieldPermissions({ + roleId: memberRoleId, + fieldPermissions: [ + { + objectMetadataId: personObjectId, + fieldMetadataId: phonesFieldId, + canReadFieldValue: null, + canUpdateFieldValue: false, + }, + ], + }); + + await upsertRowLevelPermissionPredicates({ + input: { + roleId: memberRoleId, + objectMetadataId: personObjectId, + predicates: [ + { + fieldMetadataId: phonesFieldId, + operand: RowLevelPermissionPredicateOperand.IS_NOT_EMPTY, + }, + ], + predicateGroups: [], + }, + }); + + await makeRestAPIRequest({ + method: 'post', + path: `/batch/people`, + bearer: APPLE_JONY_MEMBER_ACCESS_TOKEN, + body: [ + { + phones: { + primaryPhoneNumber: '555123456', + primaryPhoneCountryCode: 'US', + primaryPhoneCallingCode: '+1', + }, + }, + ], + }) + .expect(201) + .expect((res) => { + const createdPeople = res.body.data.createPeople; + + expect(createdPeople).toHaveLength(1); + expect(createdPeople[0].phones.primaryPhoneNumber).toBe('555123456'); + }); + + await upsertRowLevelPermissionPredicates({ + input: { + roleId: memberRoleId, + objectMetadataId: personObjectId, + predicates: [], + predicateGroups: [], + }, + }); + }); + it('should allow createMany when user has no restricted update permissions', async () => { - // Remove field permission restrictions on phones; restrict read on emails so response excludes it await upsertFieldPermissions({ roleId: memberRoleId, fieldPermissions: [ @@ -401,9 +515,9 @@ describe('Restricted fields', () => { expect(createdPeople).toHaveLength(2); expect(createdPeople[0].city).toBe('Batch City 1'); - expect(createdPeople[0].emails).toBeUndefined(); // No reading rights on emails + expect(createdPeople[0].emails).toBeUndefined(); expect(createdPeople[1].city).toBe('Batch City 2'); - expect(createdPeople[1].emails).toBeUndefined(); // No reading rights on emails + expect(createdPeople[1].emails).toBeUndefined(); }); }); });