From f7ff07833f8e3da7b6add219f1b3d316107ae5dd Mon Sep 17 00:00:00 2001 From: Prabhat Khera <91852476+prabhat-org@users.noreply.github.com> Date: Tue, 12 Sep 2023 13:15:29 +1200 Subject: [PATCH] fix(security): added restrictions to see user names [EE-5825] (#10297) * fix(security): added restrictions to see user names [EE-5825] * use pluralize method --- .../AccessControlPaneDetails.test.tsx | 7 +- .../AccessControlPanelDetails.tsx | 65 +++++++++++++------ .../EditDetails/EditDetails.tsx | 7 +- .../EditDetails/useLoadState.ts | 4 +- 4 files changed, 54 insertions(+), 29 deletions(-) diff --git a/app/react/portainer/access-control/AccessControlPanel/AccessControlPaneDetails.test.tsx b/app/react/portainer/access-control/AccessControlPanel/AccessControlPaneDetails.test.tsx index dd78df8af..9970da4d9 100644 --- a/app/react/portainer/access-control/AccessControlPanel/AccessControlPaneDetails.test.tsx +++ b/app/react/portainer/access-control/AccessControlPanel/AccessControlPaneDetails.test.tsx @@ -4,6 +4,7 @@ import { createMockTeams, createMockUsers } from '@/react-tools/test-mocks'; import { renderWithQueryClient } from '@/react-tools/test-utils'; import { rest, server } from '@/setup-tests/server'; import { Role } from '@/portainer/users/types'; +import { withUserProvider } from '@/react/test-utils/withUserProvider'; import { ResourceControlOwnership, @@ -143,11 +144,9 @@ async function renderComponent( resourceType: ResourceControlType = ResourceControlType.Container, resourceControl?: ResourceControlViewModel ) { + const WithUser = withUserProvider(AccessControlPanelDetails); const queries = renderWithQueryClient( - + ); await expect(queries.findByText('Ownership')).resolves.toBeVisible(); diff --git a/app/react/portainer/access-control/AccessControlPanel/AccessControlPanelDetails.tsx b/app/react/portainer/access-control/AccessControlPanel/AccessControlPanelDetails.tsx index f89f02a8c..584ba873d 100644 --- a/app/react/portainer/access-control/AccessControlPanel/AccessControlPanelDetails.tsx +++ b/app/react/portainer/access-control/AccessControlPanel/AccessControlPanelDetails.tsx @@ -8,6 +8,8 @@ import { UserId } from '@/portainer/users/types'; import { TeamId } from '@/react/portainer/users/teams/types'; import { useTeams } from '@/react/portainer/users/teams/queries'; import { useUsers } from '@/portainer/users/queries'; +import { useCurrentUser } from '@/react/hooks/useUser'; +import { pluralize } from '@/portainer/helpers/strings'; import { Link } from '@@/Link'; import { Tooltip } from '@@/Tip/Tooltip'; @@ -29,6 +31,8 @@ export function AccessControlPanelDetails({ resourceControl, resourceType, }: Props) { + const { isAdmin } = useCurrentUser(); + const inheritanceMessage = getInheritanceMessage( resourceType, resourceControl @@ -40,9 +44,31 @@ export function AccessControlPanelDetails({ TeamAccesses: restrictedToTeams = [], } = resourceControl || {}; - const users = useAuthorizedUsers(restrictedToUsers.map((ra) => ra.UserId)); + const users = useAuthorizedUsers( + restrictedToUsers.map((ra) => ra.UserId), + isAdmin + ); const teams = useAuthorizedTeams(restrictedToTeams.map((ra) => ra.TeamId)); + const teamsLength = teams.data ? teams.data.length : 0; + const unauthoisedTeams = restrictedToTeams.length - teamsLength; + + let teamsMessage = teams.data && teams.data.join(', '); + if (unauthoisedTeams > 0 && teams.isFetched) { + teamsMessage += teamsLength > 0 ? ' and' : ''; + teamsMessage += ` ${unauthoisedTeams} ${pluralize( + unauthoisedTeams, + 'team' + )} you are not part of`; + } + + const userMessage = isAdmin + ? (users.data && users.data.join(', ')) || '' + : `${restrictedToUsers.length} ${pluralize( + restrictedToUsers.length, + 'user' + )}`; + return ( @@ -62,17 +88,13 @@ export function AccessControlPanelDetails({ {restrictedToUsers.length > 0 && ( - + )} {restrictedToTeams.length > 0 && ( - + )} @@ -197,17 +219,22 @@ function useAuthorizedTeams(authorizedTeamIds: TeamId[]) { }); } -function useAuthorizedUsers(authorizedUserIds: UserId[]) { - return useUsers(false, 0, authorizedUserIds.length > 0, (users) => { - if (authorizedUserIds.length === 0) { - return []; - } +function useAuthorizedUsers(authorizedUserIds: UserId[], enabled = true) { + return useUsers( + false, + 0, + authorizedUserIds.length > 0 && enabled, + (users) => { + if (authorizedUserIds.length === 0) { + return []; + } - return _.compact( - authorizedUserIds.map((id) => { - const user = users.find((u) => u.Id === id); - return user?.Username; - }) - ); - }); + return _.compact( + authorizedUserIds.map((id) => { + const user = users.find((u) => u.Id === id); + return user?.Username; + }) + ); + } + ); } diff --git a/app/react/portainer/access-control/EditDetails/EditDetails.tsx b/app/react/portainer/access-control/EditDetails/EditDetails.tsx index 45c6b1890..3c2e108de 100644 --- a/app/react/portainer/access-control/EditDetails/EditDetails.tsx +++ b/app/react/portainer/access-control/EditDetails/EditDetails.tsx @@ -32,8 +32,7 @@ export function EditDetails({ }: Props) { const { user, isAdmin } = useUser(); - const { users, teams, isLoading } = useLoadState(environmentId); - + const { users, teams, isLoading } = useLoadState(environmentId, isAdmin); const handleChange = useCallback( (partialValues: Partial) => { onChange({ ...values, ...partialValues }); @@ -42,7 +41,7 @@ export function EditDetails({ [values, onChange] ); - if (isLoading || !teams || !users) { + if (isLoading || !teams || (isAdmin && !users) || !values.authorizedUsers) { return null; } @@ -62,7 +61,7 @@ export function EditDetails({ {isAdmin && ( handleChange({ authorizedUsers })} value={values.authorizedUsers} errors={errors?.authorizedUsers} diff --git a/app/react/portainer/access-control/EditDetails/useLoadState.ts b/app/react/portainer/access-control/EditDetails/useLoadState.ts index 18904d7d9..f4426a99e 100644 --- a/app/react/portainer/access-control/EditDetails/useLoadState.ts +++ b/app/react/portainer/access-control/EditDetails/useLoadState.ts @@ -2,10 +2,10 @@ import { useTeams } from '@/react/portainer/users/teams/queries'; import { useUsers } from '@/portainer/users/queries'; import { EnvironmentId } from '@/react/portainer/environments/types'; -export function useLoadState(environmentId?: EnvironmentId) { +export function useLoadState(environmentId?: EnvironmentId, enabled = true) { const teams = useTeams(false, environmentId); - const users = useUsers(false, environmentId); + const users = useUsers(false, environmentId, enabled); return { teams: teams.data,
Authorized users - {users.data && users.data.join(', ')} - {userMessage}
Authorized teams - {teams.data && teams.data.join(', ')} - {teamsMessage}