From 4c8af378af50c23b96e2b877ab9e296c18f62a7e Mon Sep 17 00:00:00 2001 From: Chaim Lev-Ari Date: Thu, 22 Jun 2023 21:12:49 +0700 Subject: [PATCH] fix(access-control): set user id when private (#8839) --- .../CreateView/useLoadFormState.ts | 6 ++-- .../AccessControlForm.stories.tsx | 2 +- .../AccessControlPanelForm.tsx | 10 ++++-- .../portainer/access-control/utils.test.ts | 27 +++++++++++--- app/react/portainer/access-control/utils.ts | 36 ++++++++++++++----- 5 files changed, 61 insertions(+), 20 deletions(-) diff --git a/app/react/azure/container-instances/CreateView/useLoadFormState.ts b/app/react/azure/container-instances/CreateView/useLoadFormState.ts index 7e3f41e10..83c68822d 100644 --- a/app/react/azure/container-instances/CreateView/useLoadFormState.ts +++ b/app/react/azure/container-instances/CreateView/useLoadFormState.ts @@ -6,7 +6,7 @@ import { Subscription, } from '@/react/azure/types'; import { parseAccessControlFormData } from '@/react/portainer/access-control/utils'; -import { useUser } from '@/react/hooks/useUser'; +import { useCurrentUser } from '@/react/hooks/useUser'; import { useProvider } from '@/react/azure/queries/useProvider'; import { useResourceGroups } from '@/react/azure/queries/useResourceGroups'; import { useSubscriptions } from '@/react/azure/queries/useSubscriptions'; @@ -37,7 +37,7 @@ export function useFormState( resourceGroups: Record = {}, providers: Record = {} ) { - const { isAdmin } = useUser(); + const { isAdmin, user } = useCurrentUser(); const subscriptionOptions = subscriptions.map((s) => ({ value: s.subscriptionId, @@ -67,7 +67,7 @@ export function useFormState( cpu: 1, ports: [{ container: 80, host: 80, protocol: 'TCP' }], allocatePublicIP: true, - accessControl: parseAccessControlFormData(isAdmin), + accessControl: parseAccessControlFormData(isAdmin, user.Id), }; return { diff --git a/app/react/portainer/access-control/AccessControlForm/AccessControlForm.stories.tsx b/app/react/portainer/access-control/AccessControlForm/AccessControlForm.stories.tsx index 4606e795b..8e1d389c8 100644 --- a/app/react/portainer/access-control/AccessControlForm/AccessControlForm.stories.tsx +++ b/app/react/portainer/access-control/AccessControlForm/AccessControlForm.stories.tsx @@ -31,7 +31,7 @@ interface Args { function Template({ userRole }: Args) { const isAdmin = userRole === Role.Admin; - const defaults = parseAccessControlFormData(isAdmin); + const defaults = parseAccessControlFormData(isAdmin, 0); const [value, setValue] = useState(defaults); diff --git a/app/react/portainer/access-control/AccessControlPanel/AccessControlPanelForm.tsx b/app/react/portainer/access-control/AccessControlPanel/AccessControlPanelForm.tsx index c78f8eb5b..10c7c3d52 100644 --- a/app/react/portainer/access-control/AccessControlPanel/AccessControlPanelForm.tsx +++ b/app/react/portainer/access-control/AccessControlPanel/AccessControlPanelForm.tsx @@ -3,7 +3,7 @@ import clsx from 'clsx'; import { useMutation } from 'react-query'; import { object } from 'yup'; -import { useUser } from '@/react/hooks/useUser'; +import { useCurrentUser } from '@/react/hooks/useUser'; import { notifySuccess } from '@/portainer/services/notifications'; import { EnvironmentId } from '@/react/portainer/environments/types'; @@ -43,7 +43,7 @@ export function AccessControlPanelForm({ onCancelClick, onUpdateSuccess, }: Props) { - const { isAdmin } = useUser(); + const { isAdmin, user } = useCurrentUser(); const updateAccess = useMutation( (variables: AccessControlFormData) => @@ -64,7 +64,11 @@ export function AccessControlPanelForm({ ); const initialValues = { - accessControl: parseAccessControlFormData(isAdmin, resourceControl), + accessControl: parseAccessControlFormData( + isAdmin, + user.Id, + resourceControl + ), }; return ( diff --git a/app/react/portainer/access-control/utils.test.ts b/app/react/portainer/access-control/utils.test.ts index 0622a9fdc..1194bb84e 100644 --- a/app/react/portainer/access-control/utils.test.ts +++ b/app/react/portainer/access-control/utils.test.ts @@ -3,6 +3,25 @@ import { ResourceControlOwnership, ResourceControlType } from './types'; import { parseAccessControlFormData } from './utils'; describe('parseAccessControlFormData', () => { + test('when ownership is private and resource is supplied, authorizedUsers should be UserAccess', () => { + const resourceControl = buildResourceControl( + ResourceControlOwnership.PRIVATE + ); + resourceControl.UserAccesses = [{ UserId: 1, AccessLevel: 1 }]; + + const actual = parseAccessControlFormData(false, 1, resourceControl); + expect(actual.authorizedUsers).toContain(1); + expect(actual.authorizedUsers).toHaveLength(1); + }); + + test('when not admin and no resource control, ownership should be set to private and authorizedUsers is only currentUserId', () => { + const actual = parseAccessControlFormData(false, 1); + + expect(actual.ownership).toBe(ResourceControlOwnership.PRIVATE); + expect(actual.authorizedUsers).toContain(1); + expect(actual.authorizedUsers).toHaveLength(1); + }); + [ ResourceControlOwnership.ADMINISTRATORS, ResourceControlOwnership.RESTRICTED, @@ -12,7 +31,7 @@ describe('parseAccessControlFormData', () => { test(`when resource control supplied, if user is not admin, will change ownership to rc ownership (${ownership})`, () => { const resourceControl = buildResourceControl(ownership); - const actual = parseAccessControlFormData(false, resourceControl); + const actual = parseAccessControlFormData(false, 0, resourceControl); expect(actual.ownership).toBe(resourceControl.Ownership); }); }); @@ -25,13 +44,13 @@ describe('parseAccessControlFormData', () => { test(`when resource control supplied and user is admin, if resource ownership is ${ownership} , will change ownership to rc ownership`, () => { const resourceControl = buildResourceControl(ownership); - const actual = parseAccessControlFormData(true, resourceControl); + const actual = parseAccessControlFormData(true, 0, resourceControl); expect(actual.ownership).toBe(resourceControl.Ownership); }); }); test('when isAdmin and resource control not supplied, ownership should be set to Administrator', () => { - const actual = parseAccessControlFormData(true); + const actual = parseAccessControlFormData(true, 0); expect(actual.ownership).toBe(ResourceControlOwnership.ADMINISTRATORS); }); @@ -41,7 +60,7 @@ describe('parseAccessControlFormData', () => { ResourceControlOwnership.PRIVATE ); - const actual = parseAccessControlFormData(true, resourceControl); + const actual = parseAccessControlFormData(true, 0, resourceControl); expect(actual.ownership).toBe(ResourceControlOwnership.RESTRICTED); }); diff --git a/app/react/portainer/access-control/utils.ts b/app/react/portainer/access-control/utils.ts index 922dcf9aa..b8ad9a8c2 100644 --- a/app/react/portainer/access-control/utils.ts +++ b/app/react/portainer/access-control/utils.ts @@ -45,24 +45,42 @@ export function parseOwnershipParameters( }; } +export function defaultValues( + isAdmin: boolean, + currentUserId: UserId +): AccessControlFormData { + if (!isAdmin) { + return { + ownership: ResourceControlOwnership.PRIVATE, + authorizedTeams: [], + authorizedUsers: [currentUserId], + }; + } + + return { + ownership: ResourceControlOwnership.ADMINISTRATORS, + authorizedTeams: [], + authorizedUsers: [], + }; +} + export function parseAccessControlFormData( isAdmin: boolean, + currentUserId: UserId, resourceControl?: ResourceControlViewModel ): AccessControlFormData { - let ownership = - resourceControl?.Ownership || ResourceControlOwnership.PRIVATE; - if (isAdmin) { - if (!resourceControl) { - ownership = ResourceControlOwnership.ADMINISTRATORS; - } else if (ownership === ResourceControlOwnership.PRIVATE) { - ownership = ResourceControlOwnership.RESTRICTED; - } + if (!resourceControl) { + return defaultValues(isAdmin, currentUserId); + } + + let ownership = resourceControl.Ownership; + if (isAdmin && ownership === ResourceControlOwnership.PRIVATE) { + ownership = ResourceControlOwnership.RESTRICTED; } let authorizedTeams: TeamId[] = []; let authorizedUsers: UserId[] = []; if ( - resourceControl && [ ResourceControlOwnership.PRIVATE, ResourceControlOwnership.RESTRICTED,