From 1b01f2c4b4fdc35ecccb42fdcbb6b62e1e45ebdf Mon Sep 17 00:00:00 2001 From: Azri Kahar <42867097+azrikahar@users.noreply.github.com> Date: Thu, 9 Nov 2023 01:06:19 +0800 Subject: [PATCH] Preserve minimal app permissions/validations when merging (#20347) * preserve app minimal permissions when merging * update test * add changeset --- .changeset/twenty-maps-cough.md | 5 +++++ api/src/utils/merge-permissions.test.ts | 18 +++++++++++---- api/src/utils/merge-permissions.ts | 30 +++++++++++-------------- 3 files changed, 32 insertions(+), 21 deletions(-) create mode 100644 .changeset/twenty-maps-cough.md diff --git a/.changeset/twenty-maps-cough.md b/.changeset/twenty-maps-cough.md new file mode 100644 index 0000000000..1de5a22268 --- /dev/null +++ b/.changeset/twenty-maps-cough.md @@ -0,0 +1,5 @@ +--- +'@directus/api': patch +--- + +Preserved minimal app permissions/validations when merging diff --git a/api/src/utils/merge-permissions.test.ts b/api/src/utils/merge-permissions.test.ts index 1b697b2e99..ecad6ddb34 100644 --- a/api/src/utils/merge-permissions.test.ts +++ b/api/src/utils/merge-permissions.test.ts @@ -76,24 +76,34 @@ describe('merging permissions', () => { }); }); - test('{} supersedes conditional permissions in _or', () => { + test('{} is removed from conditional permissions in _or', () => { const mergedPermission = mergePermission( 'or', { ...permissionTemplate, permissions: fullFilter }, { ...permissionTemplate, permissions: conditionalFilter } ); - expect(mergedPermission).toStrictEqual({ ...permissionTemplate, permissions: fullFilter }); + expect(mergedPermission).toStrictEqual({ + ...permissionTemplate, + permissions: { + _or: [conditionalFilter], + }, + }); }); - test('{} supersedes conditional validations in _or', () => { + test('{} is removed from conditional validations in _or', () => { const mergedPermission = mergePermission( 'or', { ...permissionTemplate, validation: fullFilter }, { ...permissionTemplate, validation: conditionalFilter } ); - expect(mergedPermission).toStrictEqual({ ...permissionTemplate, validation: fullFilter }); + expect(mergedPermission).toStrictEqual({ + ...permissionTemplate, + validation: { + _or: [conditionalFilter], + }, + }); }); test('{} does not supersede conditional permissions in _and', () => { diff --git a/api/src/utils/merge-permissions.ts b/api/src/utils/merge-permissions.ts index 55a587590f..86b4353801 100644 --- a/api/src/utils/merge-permissions.ts +++ b/api/src/utils/merge-permissions.ts @@ -1,5 +1,5 @@ import type { LogicalFilterAND, LogicalFilterOR, Permission } from '@directus/types'; -import { flatten, intersection, isEqual, merge, omit } from 'lodash-es'; +import { flatten, intersection, isEmpty, merge, omit } from 'lodash-es'; export function mergePermissions(strategy: 'and' | 'or', ...permissions: Permission[][]): Permission[] { const allPermissions = flatten(permissions); @@ -37,14 +37,12 @@ export function mergePermission( ], } as LogicalFilterAND | LogicalFilterOR; } else if (currentPerm.permissions) { - // Empty {} supersedes other permissions in _OR merge - if (strategy === 'or' && (isEqual(currentPerm.permissions, {}) || isEqual(newPerm.permissions, {}))) { - permissions = {}; - } else { - permissions = { - [logicalKey]: [currentPerm.permissions, newPerm.permissions], - } as LogicalFilterAND | LogicalFilterOR; - } + permissions = { + [logicalKey]: + strategy === 'or' + ? [currentPerm.permissions, newPerm.permissions].filter((p) => !isEmpty(p)) + : [currentPerm.permissions, newPerm.permissions], + } as LogicalFilterAND | LogicalFilterOR; } else { permissions = { [logicalKey]: [newPerm.permissions], @@ -61,14 +59,12 @@ export function mergePermission( ], } as LogicalFilterAND | LogicalFilterOR; } else if (currentPerm.validation) { - // Empty {} supersedes other validations in _OR merge - if (strategy === 'or' && (isEqual(currentPerm.validation, {}) || isEqual(newPerm.validation, {}))) { - validation = {}; - } else { - validation = { - [logicalKey]: [currentPerm.validation, newPerm.validation], - } as LogicalFilterAND | LogicalFilterOR; - } + validation = { + [logicalKey]: + strategy === 'or' + ? [currentPerm.validation, newPerm.validation].filter((p) => !isEmpty(p)) + : [currentPerm.validation, newPerm.validation], + } as LogicalFilterAND | LogicalFilterOR; } else { validation = { [logicalKey]: [newPerm.validation],