Do not allow the Site Administrator to set the Manager role for a group

src/plone/restapi/services/groups/update.py

@@ -40,11 +40,15 @@ def __init__(self, context, request):
     def is_zope_manager(self):
         return getSecurityManager().checkPermission(ManagePortal, self.context)
 
-    def can_update(self, group, users):
+    def can_update(self, group, users, roles):
         if self.is_zope_manager:
             return True
-        if "Manager" in group.getRoles() and users:
+        current_group_roles = group.getRoles()
+        if "Manager" in current_group_roles and users:
             return False
+        if "Manager" in roles:
+            return "Manager" in current_group_roles
+        return "Manager" not in current_group_roles
 
     def publishTraverse(self, request, name):
         # Consume any path segments after /@groups as parameters
@@ -70,11 +74,11 @@ def reply(self):
             raise BadRequest("Trying to update a non-existing group.")
 
         users = data.get("users", {})
+        roles = data.get("roles", None)
 
-        if not self.can_update(group, users):
+        if not self.can_update(group, users, roles):
             return self.reply_no_content(status=403)
 
-        roles = data.get("roles", None)
         groups = data.get("groups", None)
 
         # Disable CSRF protection

src/plone/restapi/tests/test_services_groups.py

@@ -198,3 +198,14 @@ def test_siteadm_not_add_user_to_group_with_manager_role(self):
 
         administrators = self.gtool.getGroupById("Administrators")
         self.assertNotIn(TEST_USER_ID, administrators.getGroupMemberIds())
+
+    def test_siteadm_not_set_manager_to_group(self):
+        self.set_siteadm()
+        payload = {
+            "roles": ["Manager"],
+        }
+        self.api_session.patch("/@groups/ploneteam", json=payload)
+        transaction.commit()
+
+        ploneteam = self.gtool.getGroupById("ploneteam")
+        self.assertNotIn("Manager", ploneteam.getRoles())