user_groups: Do not allow editing system user groups from API.

We do not allow any user to edit the system user groups (including
renaming, deleting, adding or removing members, etc.) from the
API. These user groups will change only by the code when a new
user is added or role of a user is changed.

This is implemented by rejecting access_user_group_by_id always
except the case when it is use to get the user group for sending
email and push notifications, as we would need to send notifications
to the mentioned user group.

zerver/lib/email_notifications.py

@@ -371,7 +371,7 @@ def get_mentioned_user_group_name(
     smallest_user_group_size = math.inf
     smallest_user_group_name = None
     for user_group_id in mentioned_user_group_ids:
-        current_user_group = access_user_group_by_id(user_group_id, user_profile)
+        current_user_group = access_user_group_by_id(user_group_id, user_profile, for_mention=True)
         current_user_group_size = len(get_user_group_members(current_user_group))
 
         if current_user_group_size < smallest_user_group_size:

zerver/lib/push_notifications.py

@@ -937,7 +937,9 @@ def handle_push_notification(user_profile_id: int, missed_message: Dict[str, Any
     mentioned_user_group_id = missed_message.get("mentioned_user_group_id")
 
     if mentioned_user_group_id is not None:
-        user_group = access_user_group_by_id(mentioned_user_group_id, user_profile)
+        user_group = access_user_group_by_id(
+            mentioned_user_group_id, user_profile, for_mention=True
+        )
         mentioned_user_group_name = user_group.name
 
     apns_payload = get_message_payload_apns(

zerver/lib/user_groups.py

@@ -7,9 +7,13 @@ from zerver.lib.exceptions import JsonableError
 from zerver.models import Realm, UserGroup, UserGroupMembership, UserProfile
 
 
-def access_user_group_by_id(user_group_id: int, user_profile: UserProfile) -> UserGroup:
+def access_user_group_by_id(
+    user_group_id: int, user_profile: UserProfile, for_mention: bool = False
+) -> UserGroup:
     try:
         user_group = UserGroup.objects.get(id=user_group_id, realm=user_profile.realm)
+        if not for_mention and user_group.is_system_group:
+            raise JsonableError(_("Insufficient permission"))
         group_member_ids = get_user_group_members(user_group)
         if (
             not user_profile.is_realm_admin
@@ -61,10 +65,17 @@ def remove_user_from_user_group(user_profile: UserProfile, user_group: UserGroup
 
 
 def create_user_group(
-    name: str, members: List[UserProfile], realm: Realm, *, description: str = ""
+    name: str,
+    members: List[UserProfile],
+    realm: Realm,
+    *,
+    description: str = "",
+    is_system_group: bool = False,
 ) -> UserGroup:
     with transaction.atomic():
-        user_group = UserGroup.objects.create(name=name, realm=realm, description=description)
+        user_group = UserGroup.objects.create(
+            name=name, realm=realm, description=description, is_system_group=is_system_group
+        )
         UserGroupMembership.objects.bulk_create(
             UserGroupMembership(user_profile=member, user_group=user_group) for member in members
         )

zerver/tests/test_email_notifications.py

@@ -824,6 +824,33 @@ class TestMissedMessages(ZulipTestCase):
         for text in expected_email_include:
             self.assertIn(text, self.normalize_string(mail.outbox[0].body))
 
+    def test_system_user_group_mention_sends_email(self) -> None:
+        hamlet = self.example_user("hamlet")
+        cordelia = self.example_user("cordelia")
+        othello = self.example_user("othello")
+
+        hamlet_and_cordelia = create_user_group(
+            "hamlet_and_cordelia", [hamlet, cordelia], get_realm("zulip"), is_system_group=True
+        )
+        user_group_mentioned_message_id = self.send_stream_message(
+            othello, "Denmark", "@*hamlet_and_cordelia*"
+        )
+
+        handle_missedmessage_emails(
+            hamlet.id,
+            [
+                {
+                    "message_id": user_group_mentioned_message_id,
+                    "trigger": "mentioned",
+                    "mentioned_user_group_id": hamlet_and_cordelia.id,
+                },
+            ],
+        )
+        self.assertIn(
+            "Othello, the Moor of Venice: @*hamlet_and_cordelia* -- ",
+            self.normalize_string(mail.outbox[0].body),
+        )
+
     def test_realm_name_in_notifications(self) -> None:
         # Test with realm_name_in_notifications for hamlet disabled.
         self._realm_name_in_missed_message_email_subject(False)

zerver/tests/test_user_groups.py

@@ -577,3 +577,39 @@ class UserGroupAPITestCase(UserGroupTestCase):
         othello.date_joined = timezone_now() - timedelta(days=11)
         othello.save()
         check_removing_members_from_group("othello")
+
+    def test_editing_system_user_groups(self) -> None:
+        desdemona = self.example_user("desdemona")
+        iago = self.example_user("iago")
+        othello = self.example_user("othello")
+        aaron = self.example_user("aaron")
+        members = [iago, othello]
+
+        user_group = create_user_group(
+            "System group",
+            members,
+            iago.realm,
+            description="This is a system group",
+            is_system_group=True,
+        )
+
+        def check_support_group_permission(acting_user: UserProfile) -> None:
+            self.login_user(acting_user)
+            params = {
+                "name": "Test system group",
+                "description": "This is a test system group.",
+            }
+            result = self.client_patch(f"/json/user_groups/{user_group.id}", info=params)
+            self.assert_json_error(result, "Insufficient permission")
+
+            params = {"add": orjson.dumps([aaron.id]).decode()}
+            result = self.client_post(f"/json/user_groups/{user_group.id}/members", info=params)
+            self.assert_json_error(result, "Insufficient permission")
+
+            params = {"delete": orjson.dumps([othello.id]).decode()}
+            result = self.client_post(f"/json/user_groups/{user_group.id}/members", info=params)
+            self.assert_json_error(result, "Insufficient permission")
+
+        check_support_group_permission(desdemona)
+        check_support_group_permission(iago)
+        check_support_group_permission(othello)