From bbc26682ddbcd53f31095192da32ecadd3cbc9b3 Mon Sep 17 00:00:00 2001
From: Dakota Walsh <101994734+dakota-portainer@users.noreply.github.com>
Date: Tue, 5 Sep 2023 09:17:10 +1200
Subject: [PATCH] fix(security): block non-admins from user info listing
 EE-5825 (#10242)

---
 api/http/handler/users/user_list.go           | 19 +++++++++----------
 api/http/handler/users/user_list_test.go      | 18 ++----------------
 .../porAccessManagementController.js          |  6 ++++--
 3 files changed, 15 insertions(+), 28 deletions(-)

diff --git a/api/http/handler/users/user_list.go b/api/http/handler/users/user_list.go
index 4535dc73a..1e5810133 100644
--- a/api/http/handler/users/user_list.go
+++ b/api/http/handler/users/user_list.go
@@ -26,24 +26,23 @@ import (
 // @failure 500 "Server error"
 // @router /users [get]
 func (handler *Handler) userList(w http.ResponseWriter, r *http.Request) *httperror.HandlerError {
-	users, err := handler.DataStore.User().ReadAll()
-	if err != nil {
-		return httperror.InternalServerError("Unable to retrieve users from the database", err)
-	}
-
 	securityContext, err := security.RetrieveRestrictedRequestContext(r)
 	if err != nil {
 		return httperror.InternalServerError("Unable to retrieve info from request context", err)
 	}
 
-	availableUsers := security.FilterUsers(users, securityContext)
-	for i := range availableUsers {
-		hideFields(&availableUsers[i])
+	if !securityContext.IsAdmin {
+		return httperror.Forbidden("Permission denied to access users list", err)
+	}
+
+	users, err := handler.DataStore.User().ReadAll()
+	if err != nil {
+		return httperror.InternalServerError("Unable to retrieve users from the database", err)
 	}
 
 	endpointID, _ := request.RetrieveNumericQueryParameter(r, "environmentId", true)
 	if endpointID == 0 {
-		return response.JSON(w, availableUsers)
+		return response.JSON(w, users)
 	}
 
 	// filter out users who do not have access to the specific endpoint
@@ -58,7 +57,7 @@ func (handler *Handler) userList(w http.ResponseWriter, r *http.Request) *httper
 	}
 
 	canAccessEndpoint := make([]portainer.User, 0)
-	for _, user := range availableUsers {
+	for _, user := range users {
 		// 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 094c59530..261857c38 100644
--- a/api/http/handler/users/user_list_test.go
+++ b/api/http/handler/users/user_list_test.go
@@ -111,28 +111,14 @@ func Test_userList(t *testing.T) {
 		}
 	})
 
-	t.Run("standard user cannot list amdin users", func(t *testing.T) {
+	t.Run("standard user cannot list 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.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)
-			}
-		}
+		is.Equal(http.StatusForbidden, rr.Code)
 	})
 
 	// Case 2: the user is under an environment group and the environment group has endpoint access.
diff --git a/app/portainer/components/accessManagement/porAccessManagementController.js b/app/portainer/components/accessManagement/porAccessManagementController.js
index d306a1b8c..d0ef11629 100644
--- a/app/portainer/components/accessManagement/porAccessManagementController.js
+++ b/app/portainer/components/accessManagement/porAccessManagementController.js
@@ -6,10 +6,11 @@ import { isLimitedToBE } from '@/react/portainer/feature-flags/feature-flags.ser
 
 class PorAccessManagementController {
   /* @ngInject */
-  constructor($scope, Notifications, AccessService, RoleService) {
-    Object.assign(this, { $scope, Notifications, AccessService, RoleService });
+  constructor($scope, $state, Notifications, AccessService, RoleService) {
+    Object.assign(this, { $scope, $state, Notifications, AccessService, RoleService });
 
     this.limitedToBE = false;
+    this.$state = $state;
 
     this.unauthorizeAccess = this.unauthorizeAccess.bind(this);
     this.updateAction = this.updateAction.bind(this);
@@ -105,6 +106,7 @@ class PorAccessManagementController {
       this.availableUsersAndTeams = _.orderBy(data.availableUsersAndTeams, 'Name', 'asc');
       this.authorizedUsersAndTeams = data.authorizedUsersAndTeams;
     } catch (err) {
+      this.$state.go('portainer.home');
       this.availableUsersAndTeams = [];
       this.authorizedUsersAndTeams = [];
       this.Notifications.error('Failure', err, 'Unable to retrieve accesses');