Automated cherry pick of #36197 (#36382)

* [MM-68393] Tighten protected role patch authorization (#36197)

* [MM-68393] Tighten protected role patch authorization

Harden role patch authorization for protected system roles and cover the restricted paths with focused API tests.

Made-with: Cursor

* [MM-68393] Fix role patch test shadowing

Rename shadowing response variables in the protected role patch tests so govet passes in core and enterprise check-style jobs.

Made-with: Cursor

* [MM-68393] Block privileged role permissions

Made-with: Cursor
(cherry picked from commit 99b73d4c4acf5ff3546c2548a5aaa804c2aa1b04)

* Fix role patch tests for release-10.11 context and LoginSystemManager APIs

Co-authored-by: Cursor <cursoragent@cursor.com>

---------

Co-authored-by: Nick Misasi <nick.misasi@mattermost.com>
Co-authored-by: Cursor <cursoragent@cursor.com>

server/channels/api4/role.go

@@ -151,9 +151,9 @@ func patchRole(c *Context, w http.ResponseWriter, r *http.Request) {
 	auditRec.AddEventPriorState(oldRole)
 	auditRec.AddEventObjectType("role")
 
-	// manage_system permission is required to patch system_admin
+	// manage_system permission is required to patch system_admin and other protected system roles.
 	requiredPermission := model.PermissionSysconsoleWriteUserManagementPermissions
-	specialProtectedSystemRoles := append(model.NewSystemRoleIDs, model.SystemAdminRoleId)
+	specialProtectedSystemRoles := append(append([]string{}, model.NewSystemRoleIDs...), model.SystemAdminRoleId, model.SystemUserRoleId, model.SystemGuestRoleId)
 	for _, roleID := range specialProtectedSystemRoles {
 		if oldRole.Name == roleID {
 			requiredPermission = model.PermissionManageSystem

server/channels/api4/role_test.go

@@ -330,6 +330,63 @@ func TestPatchRole(t *testing.T) {
 		Permissions: &[]string{"create_direct_channel", "manage_incoming_webhooks", "manage_outgoing_webhooks"},
 	}
 
+	t.Run("system manager cannot patch system_user", func(t *testing.T) {
+		systemUserRole, appErr := th.App.GetRoleByName(context.Background(), model.SystemUserRoleId)
+		require.Nil(t, appErr)
+
+		originalPermissions := append([]string{}, systemUserRole.Permissions...)
+		require.NotContains(t, originalPermissions, model.PermissionEditOtherUsers.Id)
+
+		patchedPermissions := append([]string{}, originalPermissions...)
+		patchedPermissions = append(patchedPermissions, model.PermissionEditOtherUsers.Id)
+
+		th.LoginSystemManager()
+
+		_, systemUserResp, err := th.SystemManagerClient.PatchRole(context.Background(), systemUserRole.Id, &model.RolePatch{
+			Permissions: &patchedPermissions,
+		})
+		if assert.Error(t, err, "system_manager must not be able to patch system_user") {
+			CheckForbiddenStatus(t, systemUserResp)
+		}
+
+		systemUserRole, appErr = th.App.GetRoleByName(context.Background(), model.SystemUserRoleId)
+		require.Nil(t, appErr)
+		assert.ElementsMatch(t, originalPermissions, systemUserRole.Permissions)
+		assert.NotContains(t, systemUserRole.Permissions, model.PermissionEditOtherUsers.Id, "system_manager must not be able to inject privileged permissions into system_user")
+	})
+
+	t.Run("system manager cannot patch system_guest", func(t *testing.T) {
+		license := model.NewTestLicense()
+		license.Features.GuestAccountsPermissions = model.NewPointer(true)
+		th.App.Srv().SetLicense(license)
+		t.Cleanup(func() {
+			th.App.Srv().SetLicense(nil)
+		})
+
+		systemGuestRole, appErr := th.App.GetRoleByName(context.Background(), model.SystemGuestRoleId)
+		require.Nil(t, appErr)
+
+		originalPermissions := append([]string{}, systemGuestRole.Permissions...)
+		require.NotContains(t, originalPermissions, model.PermissionEditOtherUsers.Id)
+
+		patchedPermissions := append([]string{}, originalPermissions...)
+		patchedPermissions = append(patchedPermissions, model.PermissionEditOtherUsers.Id)
+
+		th.LoginSystemManager()
+
+		_, systemGuestResp, err := th.SystemManagerClient.PatchRole(context.Background(), systemGuestRole.Id, &model.RolePatch{
+			Permissions: &patchedPermissions,
+		})
+		if assert.Error(t, err, "system_manager must not be able to patch system_guest") {
+			CheckForbiddenStatus(t, systemGuestResp)
+		}
+
+		systemGuestRole, appErr = th.App.GetRoleByName(context.Background(), model.SystemGuestRoleId)
+		require.Nil(t, appErr)
+		assert.ElementsMatch(t, originalPermissions, systemGuestRole.Permissions)
+		assert.NotContains(t, systemGuestRole.Permissions, model.PermissionEditOtherUsers.Id, "system_manager must not be able to inject privileged permissions into system_guest")
+	})
+
 	th.TestForSystemAdminAndLocal(t, func(t *testing.T, client *model.Client4) {
 		received, _, err := client.PatchRole(context.Background(), role.Id, patch)
 		require.NoError(t, err)