From 644381b35e89dede77f29b2f24bed027d6b8ae0b Mon Sep 17 00:00:00 2001
From: Scott Bishel <scott.bishel@mattermost.com>
Date: Thu, 22 Jun 2023 10:40:21 -0600
Subject: [PATCH] MM-53092 Fix for Updating SysAdmin user. (#23750)

* check retrieved user for role and permissions

* add unit tests

---------

Co-authored-by: Mattermost Build <build@mattermost.com>
---
 server/channels/api4/user.go      | 12 +++++-----
 server/channels/api4/user_test.go | 40 +++++++++++++++++++++++++++++++
 2 files changed, 46 insertions(+), 6 deletions(-)

diff --git a/server/channels/api4/user.go b/server/channels/api4/user.go
index 28b46862e0..b010489e66 100644
--- a/server/channels/api4/user.go
+++ b/server/channels/api4/user.go
@@ -1249,12 +1249,6 @@ func updateUser(c *Context, w http.ResponseWriter, r *http.Request) {
 		return
 	}
 
-	// Cannot update a system admin unless user making request is a systemadmin also.
-	if user.IsSystemAdmin() && !c.App.SessionHasPermissionTo(*c.AppContext.Session(), model.PermissionManageSystem) {
-		c.SetPermissionError(model.PermissionManageSystem)
-		return
-	}
-
 	if !c.App.SessionHasPermissionToUser(*c.AppContext.Session(), user.Id) {
 		c.SetPermissionError(model.PermissionEditOtherUsers)
 		return
@@ -1265,6 +1259,12 @@ func updateUser(c *Context, w http.ResponseWriter, r *http.Request) {
 		c.Err = err
 		return
 	}
+	// Cannot update a system admin unless user making request is a systemadmin also.
+	if ouser.IsSystemAdmin() && !c.App.SessionHasPermissionTo(*c.AppContext.Session(), model.PermissionManageSystem) {
+		c.SetPermissionError(model.PermissionManageSystem)
+		return
+	}
+
 	auditRec.AddEventPriorState(ouser)
 	auditRec.AddEventObjectType("user")
 
diff --git a/server/channels/api4/user_test.go b/server/channels/api4/user_test.go
index acee1b1a83..f17f90e303 100644
--- a/server/channels/api4/user_test.go
+++ b/server/channels/api4/user_test.go
@@ -1910,6 +1910,26 @@ func TestUpdateUser(t *testing.T) {
 	})
 }
 
+func TestUpdateAdminUser(t *testing.T) {
+	th := Setup(t).InitBasic()
+	defer th.TearDown()
+
+	user := th.CreateUser()
+	th.App.UpdateUserRoles(th.Context, user.Id, model.SystemUserRoleId+" "+model.SystemAdminRoleId, false)
+	user.Email = th.GenerateTestEmail()
+
+	th.AddPermissionToRole(model.PermissionEditOtherUsers.Id, model.SystemUserManagerRoleId)
+	th.App.UpdateUserRoles(th.Context, th.BasicUser.Id, model.SystemUserManagerRoleId+" "+model.SystemUserAccessTokenRoleId, false)
+
+	_, resp, err := th.Client.UpdateUser(context.Background(), user)
+	require.Error(t, err)
+	CheckForbiddenStatus(t, resp)
+
+	u2, _, err := th.SystemAdminClient.UpdateUser(context.Background(), user)
+	require.NoError(t, err)
+	require.Equal(t, user.Email, u2.Email)
+}
+
 func TestPatchUser(t *testing.T) {
 	th := Setup(t).InitBasic()
 	defer th.TearDown()
@@ -2021,6 +2041,26 @@ func TestPatchUser(t *testing.T) {
 	require.NoError(t, err)
 }
 
+func TestPatchAdminUser(t *testing.T) {
+	th := Setup(t).InitBasic()
+	defer th.TearDown()
+
+	user := th.CreateUser()
+	th.App.UpdateUserRoles(th.Context, user.Id, model.SystemUserRoleId+" "+model.SystemAdminRoleId, false)
+
+	patch := &model.UserPatch{}
+	patch.Email = model.NewString(th.GenerateTestEmail())
+
+	th.AddPermissionToRole(model.PermissionEditOtherUsers.Id, model.SystemUserManagerRoleId)
+	th.App.UpdateUserRoles(th.Context, th.BasicUser.Id, model.SystemUserManagerRoleId+" "+model.SystemUserAccessTokenRoleId, false)
+
+	_, resp, err := th.Client.PatchUser(context.Background(), user.Id, patch)
+	require.Error(t, err)
+	CheckForbiddenStatus(t, resp)
+
+	_, _, err = th.SystemAdminClient.PatchUser(context.Background(), user.Id, patch)
+	require.NoError(t, err)
+}
 func TestUserUnicodeNames(t *testing.T) {
 	th := Setup(t)
 	defer th.TearDown()