From e454dbce00e750fdcac46101c6f8766fcbd9a1c1 Mon Sep 17 00:00:00 2001 From: Ktbch Date: Fri, 29 Sep 2023 11:36:45 +0100 Subject: [PATCH 1/2] feat(permissions): move permision hierachy to the FE --- src/frontend/views/entity/Crud/index.tsx | 5 ++- .../roles/Permissions/MutatePermission.tsx | 41 ++++++++++++++++--- 2 files changed, 39 insertions(+), 7 deletions(-) diff --git a/src/frontend/views/entity/Crud/index.tsx b/src/frontend/views/entity/Crud/index.tsx index 12750bd38..34d0843fc 100644 --- a/src/frontend/views/entity/Crud/index.tsx +++ b/src/frontend/views/entity/Crud/index.tsx @@ -118,10 +118,13 @@ function useEntityCrudView() { if (field === "details" && !newState.details) { newState.update = false; + newState.delete = false; } else if (field === "update" && newState.update) { newState.details = true; + } else if (field === "delete" && newState.delete) { + newState.details = true; } - + setEntityCrudSettingsState(newState); upsertCrudSettingsMutation.mutateAsync( newState as unknown as Record diff --git a/src/frontend/views/roles/Permissions/MutatePermission.tsx b/src/frontend/views/roles/Permissions/MutatePermission.tsx index 234bfb2f7..631fdb990 100644 --- a/src/frontend/views/roles/Permissions/MutatePermission.tsx +++ b/src/frontend/views/roles/Permissions/MutatePermission.tsx @@ -10,6 +10,8 @@ import { useRolePermissionDeletionMutation, useRolePermissions, } from "../permissions.store"; +import { USER_PERMISSIONS } from "shared/constants/user"; +import { PORTAL_PERMISSION_HEIRACHIES } from "shared/logic/permissions/portal"; interface IProps { permissionList: ILabelValue[]; @@ -17,6 +19,15 @@ interface IProps { singular: string; } +const PERMISSION_HEIRACHIES = { + [USER_PERMISSIONS.CAN_MANAGE_USERS]: USER_PERMISSIONS.CAN_RESET_PASSWORD, + [USER_PERMISSIONS.CAN_CONFIGURE_APP]: + USER_PERMISSIONS.CAN_MANAGE_INTEGRATIONS, + [USER_PERMISSIONS.CAN_MANAGE_ALL_ENTITIES]: + USER_PERMISSIONS.CAN_CONFIGURE_APP, + ...PORTAL_PERMISSION_HEIRACHIES, +}; + export function MutatePermission({ permissionList, singular, @@ -79,13 +90,31 @@ export function MutatePermission({ selected: isPermissionSelected, onChange: () => { if (isPermissionSelected) { - rolePermissionDeletionMutation.mutate([ - menuItem.value, - ]); + const permissionsToSend = [menuItem.value]; + const permissionHeirachy = Object.entries( + PERMISSION_HEIRACHIES + ).find(([index, _]) => index === menuItem.value); + + if (permissionHeirachy) { + permissionsToSend.push(permissionHeirachy[1]); + } + rolePermissionDeletionMutation.mutate( + permissionsToSend + ); } else { - rolePermissionCreationMutation.mutate([ - menuItem.value, - ]); + const permissionsToSend = [menuItem.value]; + + const permissionHeirachy = Object.entries( + PERMISSION_HEIRACHIES + ).find(([_, value]) => value === menuItem.value); + + if (permissionHeirachy) { + permissionsToSend.push(permissionHeirachy[0]); + } + + rolePermissionCreationMutation.mutate( + permissionsToSend + ); } }, } From 21112863bbfb4b2372d1bc5d96c3ee56f84ab763 Mon Sep 17 00:00:00 2001 From: Ktbch Date: Fri, 20 Oct 2023 10:18:45 +0100 Subject: [PATCH 2/2] Commented out discord todo --- src/__tests__/roles/[roleId]/index.spec.tsx | 2 + src/frontend/docs/roles.tsx | 33 ------------- .../roles/Permissions/MutatePermission.tsx | 48 +++++++++++-------- .../views/roles/Permissions/index.tsx | 1 + .../logic/permissions/__tests__/index.spec.ts | 22 +-------- src/shared/logic/permissions/index.ts | 35 +------------- 6 files changed, 34 insertions(+), 107 deletions(-) diff --git a/src/__tests__/roles/[roleId]/index.spec.tsx b/src/__tests__/roles/[roleId]/index.spec.tsx index d39ba784a..1e91ffe6f 100644 --- a/src/__tests__/roles/[roleId]/index.spec.tsx +++ b/src/__tests__/roles/[roleId]/index.spec.tsx @@ -296,4 +296,6 @@ describe("pages/roles/[roleId]/index", () => { }) ).toBeChecked(); }); + + // todo test heirachy }); diff --git a/src/frontend/docs/roles.tsx b/src/frontend/docs/roles.tsx index 2af9ce81c..ff925255c 100644 --- a/src/frontend/docs/roles.tsx +++ b/src/frontend/docs/roles.tsx @@ -94,39 +94,6 @@ export function RolesDocumentation(props: IDocumentationRootProps) { : enables users to reset passwords.

- -

Permissions Dependencies

-

- Enabling some permissions will automatically grant some other - permission. For example, giving someone permission to reset a user' - password without giving them permission to manage users doesn't - make sense as they need to be able to access users first to get to where - they will be able to reset the password. This is what we call - permissions dependencies. -

-

- These are the permission dependencies. -

    -
  • - {userFriendlyCase(USER_PERMISSIONS.CAN_RESET_PASSWORD)}{" "} - - {userFriendlyCase(USER_PERMISSIONS.CAN_MANAGE_USERS)} -
    • -
  • - - {userFriendlyCase(USER_PERMISSIONS.CAN_MANAGE_INTEGRATIONS)} - {" "} - -{" "} - {userFriendlyCase(USER_PERMISSIONS.CAN_CONFIGURE_APP)} -
    • -
  • - {userFriendlyCase(USER_PERMISSIONS.CAN_CONFIGURE_APP)}{" "} - -{" "} - - {userFriendlyCase(USER_PERMISSIONS.CAN_MANAGE_ALL_ENTITIES)} - -
    • -
-

); } diff --git a/src/frontend/views/roles/Permissions/MutatePermission.tsx b/src/frontend/views/roles/Permissions/MutatePermission.tsx index 631fdb990..475ab9c6d 100644 --- a/src/frontend/views/roles/Permissions/MutatePermission.tsx +++ b/src/frontend/views/roles/Permissions/MutatePermission.tsx @@ -19,6 +19,10 @@ interface IProps { singular: string; } +/* + IMPORTANT NOTE: + LESSER PERMISSION FIRST +*/ const PERMISSION_HEIRACHIES = { [USER_PERMISSIONS.CAN_MANAGE_USERS]: USER_PERMISSIONS.CAN_RESET_PASSWORD, [USER_PERMISSIONS.CAN_CONFIGURE_APP]: @@ -28,6 +32,28 @@ const PERMISSION_HEIRACHIES = { ...PORTAL_PERMISSION_HEIRACHIES, }; +export const getPermissionChildren = ( + permission: string, + mainKey: 1 | 0, + permissions: string[] = [] +): string[] => { + permissions.push(permission); + + const permissionHeirachy = Object.entries(PERMISSION_HEIRACHIES).find( + (value) => value[mainKey === 1 ? 0 : 1] === permission + ); + + if (!permissionHeirachy) { + return permissions; + } + + return getPermissionChildren( + permissionHeirachy[mainKey], + mainKey, + permissions + ); +}; + export function MutatePermission({ permissionList, singular, @@ -90,30 +116,12 @@ export function MutatePermission({ selected: isPermissionSelected, onChange: () => { if (isPermissionSelected) { - const permissionsToSend = [menuItem.value]; - const permissionHeirachy = Object.entries( - PERMISSION_HEIRACHIES - ).find(([index, _]) => index === menuItem.value); - - if (permissionHeirachy) { - permissionsToSend.push(permissionHeirachy[1]); - } rolePermissionDeletionMutation.mutate( - permissionsToSend + getPermissionChildren(menuItem.value, 1) ); } else { - const permissionsToSend = [menuItem.value]; - - const permissionHeirachy = Object.entries( - PERMISSION_HEIRACHIES - ).find(([_, value]) => value === menuItem.value); - - if (permissionHeirachy) { - permissionsToSend.push(permissionHeirachy[0]); - } - rolePermissionCreationMutation.mutate( - permissionsToSend + getPermissionChildren(menuItem.value, 0) ); } }, diff --git a/src/frontend/views/roles/Permissions/index.tsx b/src/frontend/views/roles/Permissions/index.tsx index 7d21d6d30..4310f9c90 100644 --- a/src/frontend/views/roles/Permissions/index.tsx +++ b/src/frontend/views/roles/Permissions/index.tsx @@ -29,6 +29,7 @@ const mapPermissionStringToLabelValue = (permissionStringList: string[]) => { })); }; +// TODO sort by heirachy const adminPermissionList: ILabelValue[] = mapPermissionStringToLabelValue( Object.values(BASE_USER_PERMISSIONS) ); diff --git a/src/shared/logic/permissions/__tests__/index.spec.ts b/src/shared/logic/permissions/__tests__/index.spec.ts index 359db8aae..61bd92d0f 100644 --- a/src/shared/logic/permissions/__tests__/index.spec.ts +++ b/src/shared/logic/permissions/__tests__/index.spec.ts @@ -122,27 +122,7 @@ describe("user role checks", () => { ).toBe(false); }); }); - describe("Heirachy", () => { - it("should return true for PERMISSION_HEIRACHIES down", () => { - expect( - doesPermissionAllowPermission( - ["CAN_RESET_PASSWORD", "PERMISSION_2"], - "CAN_MANAGE_USERS", - false - ) - ).toBe(true); - }); - it("should return false for PERMISSION_HEIRACHIES down", () => { - expect( - doesPermissionAllowPermission( - ["CAN_MANAGE_USERS", "PERMISSION_2"], - "CAN_RESET_PASSWORD", - false - ) - ).toBe(false); - }); - }); - describe.only("Check Granular", () => { + describe("Check Granular", () => { it("should pass check granular when granular is true", () => { expect( doesPermissionAllowPermission( diff --git a/src/shared/logic/permissions/index.ts b/src/shared/logic/permissions/index.ts index ca2b4bc73..b5271bc96 100644 --- a/src/shared/logic/permissions/index.ts +++ b/src/shared/logic/permissions/index.ts @@ -5,24 +5,7 @@ import { } from "shared/constants/user"; import { GranularEntityPermissions, SystemRoles } from "shared/types/user"; import { replaceGranular } from "shared/constants/user/shared"; -import { - portalMetaPermissionCheck, - PORTAL_PERMISSION_HEIRACHIES, -} from "./portal"; - -/* - IMPORTANT NOTE: - LESSER PERMISSION FIRST -*/ -// TODO move this logic to the FE for more visibility (For contributors) -const PERMISSION_HEIRACHIES = { - [USER_PERMISSIONS.CAN_MANAGE_USERS]: USER_PERMISSIONS.CAN_RESET_PASSWORD, - [USER_PERMISSIONS.CAN_CONFIGURE_APP]: - USER_PERMISSIONS.CAN_MANAGE_INTEGRATIONS, - [USER_PERMISSIONS.CAN_MANAGE_ALL_ENTITIES]: - USER_PERMISSIONS.CAN_CONFIGURE_APP, - ...PORTAL_PERMISSION_HEIRACHIES, -}; +import { portalMetaPermissionCheck } from "./portal"; const doMetaPermissionCheck = (permissions: string[], requiredPermission: string) => @@ -77,21 +60,7 @@ export const doesPermissionAllowPermission = ( ); } - const can = permissions.includes(requiredPermission); - - if (can) { - return true; - } - - if (!PERMISSION_HEIRACHIES[requiredPermission]) { - return false; - } - - return doesPermissionAllowPermission( - permissions, - PERMISSION_HEIRACHIES[requiredPermission], - checkGranular - ); + return permissions.includes(requiredPermission); }; const doSystemRoleCheck = (