user_groups: Check can_add_members_group when adding members.

Fixes #25942.
Users with permission to manage the group (either on the group level or
realm level) should be able to add members to the group without being
present in can_add_members_group.

web/src/settings_data.ts

@@ -215,6 +215,22 @@ export function can_manage_user_group(group_id: number): boolean {
     );
 }
 
+export function can_add_members_to_user_group(group_id: number): boolean {
+    const group = user_groups.get_user_group_from_id(group_id);
+
+    if (
+        user_has_permission_for_group_setting(
+            group.can_add_members_group,
+            "can_add_members_group",
+            "group",
+        )
+    ) {
+        return true;
+    }
+
+    return can_manage_user_group(group_id);
+}
+
 export function can_join_user_group(group_id: number): boolean {
     const group = user_groups.get_user_group_from_id(group_id);
 

web/src/user_group_edit.js

@@ -87,7 +87,7 @@ function update_add_members_elements(group) {
     const $input_element = $add_members_container.find(".input").expectOne();
     const $button_element = $add_members_container.find('button[name="add_member"]').expectOne();
 
-    if (settings_data.can_manage_user_group(group.id)) {
+    if (settings_data.can_add_members_to_user_group(group.id)) {
         $input_element.prop("contenteditable", true);
         $button_element.prop("disabled", false);
         $button_element.css("pointer-events", "");
@@ -658,6 +658,10 @@ export function update_group(event) {
         if (event.data.can_mention_group !== undefined) {
             sync_group_permission_setting("can_mention_group", group);
         }
+        if (event.data.can_manage_group !== undefined) {
+            sync_group_permission_setting("can_add_members_group", group);
+            update_group_management_ui();
+        }
         if (event.data.can_manage_group !== undefined) {
             sync_group_permission_setting("can_manage_group", group);
             update_group_management_ui();

web/tests/settings_data.test.js

@@ -585,6 +585,125 @@ run_test("can_join_user_group", ({override}) => {
     assert.ok(settings_data.can_join_user_group(students.id));
 });
 
+run_test("can_add_members_user_group", () => {
+    const admins = {
+        description: "Administrators",
+        name: "role:administrators",
+        id: 1,
+        members: new Set([1]),
+        is_system_group: true,
+        direct_subgroup_ids: new Set([]),
+        can_add_members_group: 4,
+        can_manage_group: 4,
+        can_mention_group: 1,
+    };
+    const moderators = {
+        description: "Moderators",
+        name: "role:moderators",
+        id: 2,
+        members: new Set([2]),
+        is_system_group: true,
+        direct_subgroup_ids: new Set([1]),
+        can_add_members_group: 4,
+        can_manage_group: 4,
+        can_mention_group: 1,
+    };
+    const members = {
+        description: "Members",
+        name: "role:members",
+        id: 3,
+        members: new Set([3, 4]),
+        is_system_group: true,
+        direct_subgroup_ids: new Set([1, 2]),
+        can_add_members_group: 4,
+        can_manage_group: 4,
+        can_mention_group: 4,
+    };
+    const nobody = {
+        description: "Nobody",
+        name: "role:nobody",
+        id: 4,
+        members: new Set([]),
+        is_system_group: true,
+        direct_subgroup_ids: new Set([]),
+        can_add_members_group: 4,
+        can_manage_group: 4,
+        can_mention_group: 2,
+    };
+    const students = {
+        description: "Students group",
+        name: "Students",
+        id: 5,
+        members: new Set([1, 2]),
+        is_system_group: false,
+        direct_subgroup_ids: new Set([4, 5]),
+        can_add_members_group: 1,
+        can_manage_group: {
+            direct_members: [6],
+            direct_subgroups: [],
+        },
+        can_mention_group: 3,
+        creator_id: 4,
+    };
+    user_groups.initialize({
+        realm_user_groups: [admins, moderators, members, nobody, students],
+    });
+    realm.realm_can_manage_all_groups = nobody.id;
+
+    page_params.is_spectator = true;
+    assert.ok(!settings_data.can_add_members_to_user_group(students.id));
+
+    page_params.is_spectator = false;
+    // admin user
+    current_user.user_id = 1;
+    assert.ok(settings_data.can_add_members_to_user_group(students.id));
+
+    // moderator user
+    current_user.user_id = 2;
+    assert.ok(!settings_data.can_add_members_to_user_group(students.id));
+
+    let event = {
+        group_id: students.id,
+        data: {
+            can_add_members_group: moderators.id,
+        },
+    };
+    user_groups.update(event);
+    assert.ok(settings_data.can_add_members_to_user_group(students.id));
+
+    // Some other user.
+    current_user.user_id = 5;
+    assert.ok(!settings_data.can_add_members_to_user_group(students.id));
+
+    event = {
+        group_id: students.id,
+        data: {
+            can_add_members_group: {
+                direct_members: [5],
+                direct_subgroups: [admins.id],
+            },
+        },
+    };
+    user_groups.update(event);
+    assert.ok(settings_data.can_add_members_to_user_group(students.id));
+
+    // Users with permission to manage the group should be able to add
+    // members to the group without adding themselves to
+    // can_add_members_group.
+    current_user.user_id = 4;
+    assert.ok(!settings_data.can_add_members_to_user_group(students.id));
+    event = {
+        group_id: students.id,
+        data: {
+            can_manage_group: {
+                direct_members: [4],
+            },
+        },
+    };
+    user_groups.update(event);
+    assert.ok(settings_data.can_add_members_to_user_group(students.id));
+});
+
 run_test("type_id_to_string", () => {
     page_params.bot_types = [
         {

zerver/tests/test_user_groups.py

@@ -2249,8 +2249,7 @@ class UserGroupAPITestCase(UserGroupTestCase):
         check_adding_members_to_group("othello")
         check_removing_members_from_group("othello")
 
-        # Check only members are allowed to add/remove users in the group and only if belong to the
-        # user group.
+        # Check only members are allowed to add/remove users in the group.
         members_group = NamedUserGroup.objects.get(name=SystemGroups.MEMBERS, realm=realm)
         do_change_realm_permission_group_setting(
             realm,
@@ -2380,12 +2379,12 @@ class UserGroupAPITestCase(UserGroupTestCase):
         check_removing_members_from_group("iago", "Insufficient permission")
         check_removing_members_from_group("desdemona")
 
-        members_group = system_group_dict[SystemGroups.MEMBERS]
+        everyone_group = system_group_dict[SystemGroups.EVERYONE]
         do_change_user_group_permission_setting(
-            user_group, "can_manage_group", members_group, acting_user=None
+            user_group, "can_manage_group", everyone_group, acting_user=None
         )
         do_change_user_group_permission_setting(
-            user_group, "can_add_members_group", members_group, acting_user=None
+            user_group, "can_add_members_group", everyone_group, acting_user=None
         )
         check_adding_members_to_group("polonius", "Not allowed for guest users")
         check_adding_members_to_group("cordelia")
@@ -2414,6 +2413,19 @@ class UserGroupAPITestCase(UserGroupTestCase):
         check_adding_members_to_group("desdemona")
         check_removing_members_from_group("desdemona")
 
+        # If user is part of `can_manage_group`, they need not be part
+        # of `can_add_members_group` to add members.
+        othello_group = self.create_or_update_anonymous_group_for_setting([othello], [])
+        hamlet_group = self.create_or_update_anonymous_group_for_setting([hamlet], [])
+        do_change_user_group_permission_setting(
+            user_group, "can_manage_group", othello_group, acting_user=None
+        )
+        do_change_user_group_permission_setting(
+            user_group, "can_add_members_group", hamlet_group, acting_user=None
+        )
+        check_adding_members_to_group("othello")
+        check_removing_members_from_group("othello")
+
     def test_adding_yourself_to_group(self) -> None:
         realm = get_realm("zulip")
         othello = self.example_user("othello")