From a4922eb69323fbe86039b85dd0888a78618a502d Mon Sep 17 00:00:00 2001 From: Prabhat Khera <91852476+prabhat-org@users.noreply.github.com> Date: Thu, 14 Sep 2023 15:51:19 +1200 Subject: [PATCH] fix(docker): revert PR #10297 and #10242 [EE-5825] (#10308) * revert PR #10297 and #10242 --- api/http/handler/users/user_list.go | 19 +++++----- api/http/handler/users/user_list_test.go | 18 ++++++++-- .../AccessControlPaneDetails.test.tsx | 7 ++-- .../AccessControlPanelDetails.tsx | 36 ++++--------------- .../EditDetails/EditDetails.tsx | 6 ++-- .../EditDetails/useLoadState.ts | 5 ++- 6 files changed, 42 insertions(+), 49 deletions(-) diff --git a/api/http/handler/users/user_list.go b/api/http/handler/users/user_list.go index 1e5810133..4535dc73a 100644 --- a/api/http/handler/users/user_list.go +++ b/api/http/handler/users/user_list.go @@ -26,23 +26,24 @@ import ( // @failure 500 "Server error" // @router /users [get] func (handler *Handler) userList(w http.ResponseWriter, r *http.Request) *httperror.HandlerError { - securityContext, err := security.RetrieveRestrictedRequestContext(r) + users, err := handler.DataStore.User().ReadAll() if err != nil { - return httperror.InternalServerError("Unable to retrieve info from request context", err) + return httperror.InternalServerError("Unable to retrieve users from the database", err) } - if !securityContext.IsAdmin { - return httperror.Forbidden("Permission denied to access users list", err) + securityContext, err := security.RetrieveRestrictedRequestContext(r) + if err != nil { + return httperror.InternalServerError("Unable to retrieve info from request context", err) } - users, err := handler.DataStore.User().ReadAll() - if err != nil { - return httperror.InternalServerError("Unable to retrieve users from the database", err) + availableUsers := security.FilterUsers(users, securityContext) + for i := range availableUsers { + hideFields(&availableUsers[i]) } endpointID, _ := request.RetrieveNumericQueryParameter(r, "environmentId", true) if endpointID == 0 { - return response.JSON(w, users) + return response.JSON(w, availableUsers) } // filter out users who do not have access to the specific endpoint @@ -57,7 +58,7 @@ func (handler *Handler) userList(w http.ResponseWriter, r *http.Request) *httper } canAccessEndpoint := make([]portainer.User, 0) - for _, user := range users { + for _, user := range availableUsers { // the users who have the endpoint authorization if _, ok := user.EndpointAuthorizations[endpoint.ID]; ok { canAccessEndpoint = append(canAccessEndpoint, user) diff --git a/api/http/handler/users/user_list_test.go b/api/http/handler/users/user_list_test.go index 261857c38..094c59530 100644 --- a/api/http/handler/users/user_list_test.go +++ b/api/http/handler/users/user_list_test.go @@ -111,14 +111,28 @@ func Test_userList(t *testing.T) { } }) - t.Run("standard user cannot list users", func(t *testing.T) { + t.Run("standard user cannot list amdin users", func(t *testing.T) { req := httptest.NewRequest(http.MethodGet, "/users", nil) req.Header.Add("Authorization", fmt.Sprintf("Bearer %s", jwt)) rr := httptest.NewRecorder() h.ServeHTTP(rr, req) - is.Equal(http.StatusForbidden, rr.Code) + is.Equal(http.StatusOK, rr.Code) + + body, err := io.ReadAll(rr.Body) + is.NoError(err, "ReadAll should not return error") + + var resp []portainer.User + err = json.Unmarshal(body, &resp) + is.NoError(err, "response should be list json") + + is.Len(resp, 2) + if len(resp) > 0 { + for _, user := range resp { + is.NotEqual(portainer.AdministratorRole, user.Role) + } + } }) // Case 2: the user is under an environment group and the environment group has endpoint access. diff --git a/app/react/portainer/access-control/AccessControlPanel/AccessControlPaneDetails.test.tsx b/app/react/portainer/access-control/AccessControlPanel/AccessControlPaneDetails.test.tsx index 9970da4d9..dd78df8af 100644 --- a/app/react/portainer/access-control/AccessControlPanel/AccessControlPaneDetails.test.tsx +++ b/app/react/portainer/access-control/AccessControlPanel/AccessControlPaneDetails.test.tsx @@ -4,7 +4,6 @@ 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, @@ -144,9 +143,11 @@ 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 584ba873d..992da5bbd 100644 --- a/app/react/portainer/access-control/AccessControlPanel/AccessControlPanelDetails.tsx +++ b/app/react/portainer/access-control/AccessControlPanel/AccessControlPanelDetails.tsx @@ -8,8 +8,6 @@ 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'; @@ -31,8 +29,6 @@ export function AccessControlPanelDetails({ resourceControl, resourceType, }: Props) { - const { isAdmin } = useCurrentUser(); - const inheritanceMessage = getInheritanceMessage( resourceType, resourceControl @@ -44,31 +40,9 @@ export function AccessControlPanelDetails({ TeamAccesses: restrictedToTeams = [], } = resourceControl || {}; - const users = useAuthorizedUsers( - restrictedToUsers.map((ra) => ra.UserId), - isAdmin - ); + const users = useAuthorizedUsers(restrictedToUsers.map((ra) => ra.UserId)); 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 ( @@ -88,13 +62,17 @@ export function AccessControlPanelDetails({ {restrictedToUsers.length > 0 && ( - + )} {restrictedToTeams.length > 0 && ( - + )} diff --git a/app/react/portainer/access-control/EditDetails/EditDetails.tsx b/app/react/portainer/access-control/EditDetails/EditDetails.tsx index 3c2e108de..bd3b959ce 100644 --- a/app/react/portainer/access-control/EditDetails/EditDetails.tsx +++ b/app/react/portainer/access-control/EditDetails/EditDetails.tsx @@ -32,7 +32,7 @@ export function EditDetails({ }: Props) { const { user, isAdmin } = useUser(); - const { users, teams, isLoading } = useLoadState(environmentId, isAdmin); + const { users, teams, isLoading } = useLoadState(environmentId); const handleChange = useCallback( (partialValues: Partial) => { onChange({ ...values, ...partialValues }); @@ -41,7 +41,7 @@ export function EditDetails({ [values, onChange] ); - if (isLoading || !teams || (isAdmin && !users) || !values.authorizedUsers) { + if (isLoading || !teams || !users) { return null; } @@ -61,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 f4426a99e..2af95d582 100644 --- a/app/react/portainer/access-control/EditDetails/useLoadState.ts +++ b/app/react/portainer/access-control/EditDetails/useLoadState.ts @@ -2,10 +2,9 @@ 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, enabled = true) { +export function useLoadState(environmentId?: EnvironmentId) { const teams = useTeams(false, environmentId); - - const users = useUsers(false, environmentId, enabled); + const users = useUsers(false, environmentId); return { teams: teams.data,
Authorized users{userMessage} + {users.data && users.data.join(', ')} +
Authorized teams{teamsMessage} + {teams.data && teams.data.join(', ')} +