diff --git a/packages/twenty-front/src/modules/settings/roles/role/hooks/useSaveDraftRoleToDB.ts b/packages/twenty-front/src/modules/settings/roles/role/hooks/useSaveDraftRoleToDB.ts index e133b1715d..e3cca310d7 100644 --- a/packages/twenty-front/src/modules/settings/roles/role/hooks/useSaveDraftRoleToDB.ts +++ b/packages/twenty-front/src/modules/settings/roles/role/hooks/useSaveDraftRoleToDB.ts @@ -1,6 +1,7 @@ import { GET_ROLES } from '@/settings/roles/graphql/queries/getRolesQuery'; import { useUpdateWorkspaceMemberRole } from '@/settings/roles/hooks/useUpdateWorkspaceMemberRole'; import { useRemoveFieldPermissionInDraftRole } from '@/settings/roles/role-permissions/object-level-permissions/field-permissions/hooks/useRemoveFieldPermissionInDraftRole'; +import { newFieldPermissionsFilter } from '@/settings/roles/role/hooks/utils/newFieldPermissionsFilter.util'; import { settingsDraftRoleFamilyState } from '@/settings/roles/states/settingsDraftRoleFamilyState'; import { settingsPersistedRoleFamilyState } from '@/settings/roles/states/settingsPersistedRoleFamilyState'; import { SettingsPath } from '@/types/SettingsPath'; @@ -72,7 +73,7 @@ export const useSaveDraftRoleToDB = ({ ); }); - const fieldPermissionsToUpsert = + const onlyMeaningfulFieldPermissions = dirtyFields.fieldPermissions?.filter( (dirtyFieldPermissionToFilter) => !fieldPermissionsThatShouldntBeCreatedBecauseTheyAreUseless?.some( @@ -82,6 +83,14 @@ export const useSaveDraftRoleToDB = ({ ), ) ?? []; + const fieldPermissionsToUpsert = onlyMeaningfulFieldPermissions.filter( + (dirtyFieldPermission) => + newFieldPermissionsFilter( + dirtyFieldPermission, + settingsPersistedRole?.fieldPermissions, + ), + ); + const { removeFieldPermissionInDraftRole } = useRemoveFieldPermissionInDraftRole(); diff --git a/packages/twenty-front/src/modules/settings/roles/role/hooks/utils/newFieldPermissionsFilter.util.ts b/packages/twenty-front/src/modules/settings/roles/role/hooks/utils/newFieldPermissionsFilter.util.ts new file mode 100644 index 0000000000..794a8da6ca --- /dev/null +++ b/packages/twenty-front/src/modules/settings/roles/role/hooks/utils/newFieldPermissionsFilter.util.ts @@ -0,0 +1,23 @@ +import { FieldPermission } from '~/generated/graphql'; + +export const newFieldPermissionsFilter = ( + dirtyFieldPermission: FieldPermission, + existingFieldPermissions?: FieldPermission[] | null, +) => { + const existingFieldPermission = existingFieldPermissions?.find( + (persistedFieldPermission) => + persistedFieldPermission.fieldMetadataId === + dirtyFieldPermission.fieldMetadataId, + ); + + if (!existingFieldPermission) { + return true; + } + + return ( + dirtyFieldPermission.canReadFieldValue !== + existingFieldPermission.canReadFieldValue || + dirtyFieldPermission.canUpdateFieldValue !== + existingFieldPermission.canUpdateFieldValue + ); +}; diff --git a/packages/twenty-server/src/engine/metadata-modules/object-permission/field-permission/field-permission.service.ts b/packages/twenty-server/src/engine/metadata-modules/object-permission/field-permission/field-permission.service.ts index 5f6d434eae..b00cd86edb 100644 --- a/packages/twenty-server/src/engine/metadata-modules/object-permission/field-permission/field-permission.service.ts +++ b/packages/twenty-server/src/engine/metadata-modules/object-permission/field-permission/field-permission.service.ts @@ -7,7 +7,10 @@ import { In, Repository } from 'typeorm'; import { RelationType } from 'src/engine/metadata-modules/field-metadata/interfaces/relation-type.interface'; -import { InternalServerError } from 'src/engine/core-modules/graphql/utils/graphql-errors.util'; +import { + InternalServerError, + UserInputError, +} from 'src/engine/core-modules/graphql/utils/graphql-errors.util'; import { FieldMetadataEntity } from 'src/engine/metadata-modules/field-metadata/field-metadata.entity'; import { isFieldMetadataTypeRelation } from 'src/engine/metadata-modules/field-metadata/utils/is-field-metadata-type-relation.util'; import { type UpsertFieldPermissionsInput } from 'src/engine/metadata-modules/object-permission/dtos/upsert-field-permissions.input'; @@ -74,6 +77,7 @@ export class FieldPermissionService { input.fieldPermissions.forEach((fieldPermission) => { this.validateFieldPermission({ + allFieldPermissions: input.fieldPermissions, fieldPermission, objectMetadataMapsById, rolesPermissions, @@ -169,16 +173,28 @@ export class FieldPermissionService { } private validateFieldPermission({ + allFieldPermissions, fieldPermission, objectMetadataMapsById, rolesPermissions, role, }: { + allFieldPermissions: UpsertFieldPermissionsInput['fieldPermissions']; fieldPermission: UpsertFieldPermissionsInput['fieldPermissions'][0]; objectMetadataMapsById: ObjectMetadataMaps['byId']; rolesPermissions: ObjectsPermissionsByRoleIdDeprecated; role: RoleEntity; }) { + const duplicateFieldPermissions = allFieldPermissions.filter( + (permission) => + permission.fieldMetadataId === fieldPermission.fieldMetadataId, + ); + + if (duplicateFieldPermissions.length > 1) { + throw new UserInputError( + `Cannot accept more than one fieldPermission for field ${fieldPermission.fieldMetadataId} in input.`, + ); + } if ( ('canUpdateFieldValue' in fieldPermission && fieldPermission.canUpdateFieldValue !== null && @@ -402,16 +418,32 @@ export class FieldPermissionService { fieldMetadata.settings?.relationType === RelationType.ONE_TO_MANY || fieldMetadata.settings?.relationType === RelationType.MANY_TO_ONE ) { - const fieldPermissionInputHasFieldPermissionOnRelationTargetFieldMetadata = + const fieldPermissionsOnRelationTargetField = fieldPermissions.filter( (fieldPermissionInput) => fieldPermissionInput.fieldMetadataId === fieldMetadata.relationTargetFieldMetadataId, - ).length > 0; + ); + + if (fieldPermissionsOnRelationTargetField.length > 0) { + const firstFieldPermission = + fieldPermissionsOnRelationTargetField[0]; // validation rules guarantee there can only be one + + const hasConflictingPermissions = + fieldPermission.canReadFieldValue !== + firstFieldPermission.canReadFieldValue || + fieldPermission.canUpdateFieldValue !== + firstFieldPermission.canUpdateFieldValue; + + if (hasConflictingPermissions) { + throw new UserInputError( + 'Conflicting field permissions found for relation target field', + { + userFriendlyMessage: `Contradicting field permissions have been detected on a relation field (${fieldMetadata.name}).`, + }, + ); + } - if ( - fieldPermissionInputHasFieldPermissionOnRelationTargetFieldMetadata - ) { return; }