From 9f49403d0a6cdefe6c45a41a08ef0d58cdbaec6b Mon Sep 17 00:00:00 2001
From: Devin Binnie <52460000+devinbinnie@users.noreply.github.com>
Date: Wed, 26 Feb 2025 15:25:02 -0500
Subject: [PATCH] [MM-62687] Patch permission check to avoid modifying the
 system admin (#30292)

* [MM-62687] Patch permission check to avoid modifying the system admin

* Check for manage system first

* PR feedback

* Add another test

* Lint

* Fix test
---
 server/channels/api4/user_test.go         |  8 ++++++++
 server/channels/app/authorization.go      | 17 +++++++++++++----
 server/channels/app/authorization_test.go |  1 +
 3 files changed, 22 insertions(+), 4 deletions(-)

diff --git a/server/channels/api4/user_test.go b/server/channels/api4/user_test.go
index ece4cf00399..4a4d4589316 100644
--- a/server/channels/api4/user_test.go
+++ b/server/channels/api4/user_test.go
@@ -4237,6 +4237,14 @@ func TestSetDefaultProfileImage(t *testing.T) {
 	_, err = th.SystemAdminClient.SetDefaultProfileImage(context.Background(), user.Id)
 	require.NoError(t, err)
 
+	// Check that a system admin can set the default profile image for another system admin
+	anotherAdmin := th.CreateUser()
+	_, appErr := th.App.UpdateUserRoles(th.Context, anotherAdmin.Id, model.SystemAdminRoleId+" "+model.SystemUserRoleId, false)
+	require.Nil(t, appErr)
+
+	_, err = th.SystemAdminClient.SetDefaultProfileImage(context.Background(), anotherAdmin.Id)
+	require.NoError(t, err)
+
 	ruser, appErr := th.App.GetUser(user.Id)
 	require.Nil(t, appErr)
 	assert.Less(t, ruser.LastPictureUpdate, iuser.LastPictureUpdate, "LastPictureUpdate should be updated to a lower negative number")
diff --git a/server/channels/app/authorization.go b/server/channels/app/authorization.go
index 7933a2ad9ef..da16acca353 100644
--- a/server/channels/app/authorization.go
+++ b/server/channels/app/authorization.go
@@ -202,7 +202,7 @@ func (a *App) SessionHasPermissionToUser(session model.Session, userID string) b
 	if userID == "" {
 		return false
 	}
-	if session.IsUnrestricted() {
+	if session.IsUnrestricted() || a.SessionHasPermissionTo(session, model.PermissionManageSystem) {
 		return true
 	}
 
@@ -210,11 +210,20 @@ func (a *App) SessionHasPermissionToUser(session model.Session, userID string) b
 		return true
 	}
 
-	if a.SessionHasPermissionTo(session, model.PermissionEditOtherUsers) {
-		return true
+	if !a.SessionHasPermissionTo(session, model.PermissionEditOtherUsers) {
+		return false
 	}
 
-	return false
+	user, err := a.GetUser(userID)
+	if err != nil {
+		return false
+	}
+
+	if user.IsSystemAdmin() {
+		return false
+	}
+
+	return true
 }
 
 func (a *App) SessionHasPermissionToUserOrBot(rctx request.CTX, session model.Session, userID string) bool {
diff --git a/server/channels/app/authorization_test.go b/server/channels/app/authorization_test.go
index 6ec559aab8a..909b7a1ac3b 100644
--- a/server/channels/app/authorization_test.go
+++ b/server/channels/app/authorization_test.go
@@ -382,6 +382,7 @@ func TestSessionHasPermissionToUser(t *testing.T) {
 
 		th.AddPermissionToRole(model.PermissionEditOtherUsers.Id, model.SystemUserManagerRoleId)
 		assert.True(t, th.App.SessionHasPermissionToUser(session, th.BasicUser2.Id))
+		assert.False(t, th.App.SessionHasPermissionToUser(session, th.SystemAdminUser.Id))
 		th.RemovePermissionFromRole(model.PermissionEditOtherUsers.Id, model.SystemUserManagerRoleId)
 
 		bot, err := th.App.CreateBot(th.Context, &model.Bot{