user_groups: Do not include deactivated users in anonymous group settings.

This commit updates code to not include deactivated users in the
anonymous group settings data sent to clients, where the setting
value is sent as a dict containing members and subgroups of the
anonymous group.

api_docs/changelog.md

@@ -31,6 +31,13 @@ format used by the Zulip server that they are interacting with.
   [`GET /user_groups/{user_group_id}/members/{user_id}`](/api/get-is-user-group-member),
   [`GET /user_groups/{user_group_id}/members`](/api/get-user-group-members):
   Deactivated users are not considered as members of a user group anymore.
+* [`GET /events`](/api/get-events): `realm/update_dict` and `user_group/update`
+  events are now also sent when deactivating or reactivating a user, for
+  settings which are set to anonymous groups having the user being deactivated
+  or reactivated as direct member.
+* [`POST /register`](/api/register-queue): Settings, represented as
+  [group-setting value](/api/group-setting-values), do not include deactivated
+  users in the `direct_members` list for settings set to anonymous groups.
 
 **Feature level 302**
 

zerver/actions/create_user.py

@@ -18,7 +18,11 @@ from zerver.actions.message_send import (
 )
 from zerver.actions.streams import bulk_add_subscriptions, send_peer_subscriber_events
 from zerver.actions.user_groups import do_send_user_group_members_update_event
-from zerver.actions.users import change_user_is_active, get_service_dicts_for_bot
+from zerver.actions.users import (
+    change_user_is_active,
+    get_service_dicts_for_bot,
+    send_update_events_for_anonymous_group_settings,
+)
 from zerver.lib.avatar import avatar_url
 from zerver.lib.create_user import create_user
 from zerver.lib.default_streams import get_slim_realm_default_streams
@@ -60,7 +64,7 @@ from zerver.models import (
 )
 from zerver.models.groups import SystemGroups
 from zerver.models.realm_audit_logs import AuditLogEventType
-from zerver.models.users import bot_owner_user_ids, get_system_bot
+from zerver.models.users import active_user_ids, bot_owner_user_ids, get_system_bot
 from zerver.tornado.django_api import send_event_on_commit
 
 MAX_NUM_RECENT_MESSAGES = 1000
@@ -772,10 +776,22 @@ def do_reactivate_user(user_profile: UserProfile, *, acting_user: UserProfile |
         subscriber_peer_info=subscriber_peer_info,
     )
 
-    member_user_groups = user_profile.direct_groups.exclude(named_user_group=None).select_related(
+    member_user_groups = user_profile.direct_groups.select_related("named_user_group")
-        "named_user_group"
+    named_user_groups = []
-    )
+    setting_user_groups = []
-    for user_group in member_user_groups:
+    for group in member_user_groups:
+        if hasattr(group, "named_user_group"):
+            named_user_groups.append(group)
+        else:
+            setting_user_groups.append(group)
+
+    for user_group in named_user_groups:
         do_send_user_group_members_update_event(
             "add_members", user_group.named_user_group, [user_profile.id]
         )
+
+    if setting_user_groups:
+        notify_user_ids = active_user_ids(user_profile.realm_id)
+        send_update_events_for_anonymous_group_settings(
+            setting_user_groups, user_profile.realm, list(notify_user_ids)
+        )

zerver/actions/users.py

@@ -6,6 +6,7 @@ from typing import Any
 from django.conf import settings
 from django.contrib.auth.tokens import PasswordResetTokenGenerator, default_token_generator
 from django.db import transaction
+from django.db.models import Q
 from django.http import HttpRequest
 from django.urls import reverse
 from django.utils.http import urlsafe_base64_encode
@@ -29,7 +30,7 @@ from zerver.lib.stream_subscription import bulk_get_subscriber_peer_info
 from zerver.lib.stream_traffic import get_streams_traffic
 from zerver.lib.streams import get_streams_for_user, stream_to_dict
 from zerver.lib.user_counts import realm_user_count_by_role
-from zerver.lib.user_groups import get_system_user_group_for_user
+from zerver.lib.user_groups import AnonymousSettingGroupDict, get_system_user_group_for_user
 from zerver.lib.users import (
     get_active_bots_owned_by_user,
     get_user_ids_who_can_access_user,
@@ -37,13 +38,16 @@ from zerver.lib.users import (
     user_access_restricted_in_realm,
 )
 from zerver.models import (
+    GroupGroupMembership,
     Message,
+    NamedUserGroup,
     Realm,
     RealmAuditLog,
     Recipient,
     Service,
     Stream,
     Subscription,
+    UserGroup,
     UserGroupMembership,
     UserProfile,
 )
@@ -262,6 +266,108 @@ def change_user_is_active(user_profile: UserProfile, value: bool) -> None:
         Subscription.objects.filter(user_profile=user_profile).update(is_user_active=value)
 
 
+def send_group_update_event_for_anonymous_group_setting(
+    setting_group: UserGroup,
+    group_members_dict: dict[int, list[int]],
+    group_subgroups_dict: dict[int, list[int]],
+    named_group: NamedUserGroup,
+    notify_user_ids: list[int],
+) -> None:
+    realm = setting_group.realm
+    for setting_name in NamedUserGroup.GROUP_PERMISSION_SETTINGS:
+        if getattr(named_group, setting_name + "_id") == setting_group.id:
+            new_setting_value = AnonymousSettingGroupDict(
+                direct_members=group_members_dict[setting_group.id],
+                direct_subgroups=group_subgroups_dict[setting_group.id],
+            )
+            event = dict(
+                type="user_group",
+                op="update",
+                group_id=named_group.id,
+                data={setting_name: new_setting_value},
+            )
+            send_event_on_commit(realm, event, notify_user_ids)
+            return
+
+
+def send_realm_update_event_for_anonymous_group_setting(
+    setting_group: UserGroup,
+    group_members_dict: dict[int, list[int]],
+    group_subgroups_dict: dict[int, list[int]],
+    notify_user_ids: list[int],
+) -> None:
+    realm = setting_group.realm
+    for setting_name in Realm.REALM_PERMISSION_GROUP_SETTINGS:
+        if getattr(realm, setting_name + "_id") == setting_group.id:
+            new_setting_value = AnonymousSettingGroupDict(
+                direct_members=group_members_dict[setting_group.id],
+                direct_subgroups=group_subgroups_dict[setting_group.id],
+            )
+            event = dict(
+                type="realm",
+                op="update_dict",
+                property="default",
+                data={setting_name: new_setting_value},
+            )
+            send_event_on_commit(realm, event, notify_user_ids)
+            return
+
+
+def send_update_events_for_anonymous_group_settings(
+    setting_groups: list[UserGroup], realm: Realm, notify_user_ids: list[int]
+) -> None:
+    setting_group_ids = [group.id for group in setting_groups]
+    membership = (
+        UserGroupMembership.objects.filter(user_group_id__in=setting_group_ids)
+        .exclude(user_profile__is_active=False)
+        .values_list("user_group_id", "user_profile_id")
+    )
+
+    group_membership = GroupGroupMembership.objects.filter(
+        supergroup_id__in=setting_group_ids
+    ).values_list("subgroup_id", "supergroup_id")
+
+    group_members = defaultdict(list)
+    for user_group_id, user_profile_id in membership:
+        group_members[user_group_id].append(user_profile_id)
+
+    group_subgroups = defaultdict(list)
+    for subgroup_id, supergroup_id in group_membership:
+        group_subgroups[supergroup_id].append(subgroup_id)
+
+    group_setting_query = Q()
+    for setting_name in NamedUserGroup.GROUP_PERMISSION_SETTINGS:
+        group_setting_query |= Q(**{f"{setting_name}__in": setting_group_ids})
+
+    named_groups_using_setting_groups_dict = {}
+    named_groups_using_setting_groups = NamedUserGroup.objects.filter(realm=realm).filter(
+        group_setting_query
+    )
+    for group in named_groups_using_setting_groups:
+        for setting_name in NamedUserGroup.GROUP_PERMISSION_SETTINGS:
+            setting_value_id = getattr(group, setting_name + "_id")
+            if setting_value_id in setting_group_ids:
+                named_groups_using_setting_groups_dict[setting_value_id] = group
+
+    for setting_group in setting_groups:
+        if setting_group.id in named_groups_using_setting_groups_dict:
+            named_group = named_groups_using_setting_groups_dict[setting_group.id]
+            send_group_update_event_for_anonymous_group_setting(
+                setting_group,
+                group_members,
+                group_subgroups,
+                named_group,
+                notify_user_ids,
+            )
+        else:
+            send_realm_update_event_for_anonymous_group_setting(
+                setting_group,
+                group_members,
+                group_subgroups,
+                notify_user_ids,
+            )
+
+
 def send_events_for_user_deactivation(user_profile: UserProfile) -> None:
     event_deactivate_user = dict(
         type="realm_user",
@@ -321,10 +427,15 @@ def send_events_for_user_deactivation(user_profile: UserProfile) -> None:
         # data, but guests who cannot access the deactivated user
         # need an explicit 'user_group/remove_members' event to
         # update the user groups data.
-        deactivated_user_groups = user_profile.direct_groups.exclude(
+        deactivated_user_groups = user_profile.direct_groups.select_related("named_user_group")
-            named_user_group=None
+        deactivated_user_named_groups = []
-        ).select_related("named_user_group")
+        deactivated_user_setting_groups = []
-        for user_group in deactivated_user_groups:
+        for group in deactivated_user_groups:
+            if not hasattr(group, "named_user_group"):
+                deactivated_user_setting_groups.append(group)
+            else:
+                deactivated_user_named_groups.append(group)
+        for user_group in deactivated_user_named_groups:
             event = dict(
                 type="user_group",
                 op="remove_members",
@@ -335,6 +446,13 @@ def send_events_for_user_deactivation(user_profile: UserProfile) -> None:
                 user_group.realm, event, list(users_without_access_to_deactivated_user)
             )
 
+        if deactivated_user_setting_groups:
+            send_update_events_for_anonymous_group_settings(
+                deactivated_user_setting_groups,
+                user_profile.realm,
+                list(users_without_access_to_deactivated_user),
+            )
+
     users_losing_access_to_deactivated_user = (
         peer_stream_subscribers - users_with_access_to_deactivated_user
     )

zerver/lib/events.py

@@ -80,6 +80,7 @@ from zerver.models import (
     Client,
     CustomProfileField,
     Draft,
+    NamedUserGroup,
     Realm,
     RealmUserDefault,
     Recipient,
@@ -1112,6 +1113,22 @@ def apply_event(
                             for user_id in user_group["members"]
                             if user_id != person_user_id
                         ]
+
+                    for setting_name in Realm.REALM_PERMISSION_GROUP_SETTINGS_WITH_NEW_API_FORMAT:
+                        if not isinstance(state["realm_" + setting_name], int):
+                            state["realm_" + setting_name].direct_members = [
+                                user_id
+                                for user_id in state["realm_" + setting_name].direct_members
+                                if user_id != person_user_id
+                            ]
+                    for group in state["realm_user_groups"]:
+                        for setting_name in NamedUserGroup.GROUP_PERMISSION_SETTINGS:
+                            if not isinstance(group[setting_name], int):
+                                group[setting_name].direct_members = [
+                                    user_id
+                                    for user_id in group[setting_name].direct_members
+                                    if user_id != person_user_id
+                                ]
         elif event["op"] == "remove":
             if person_user_id in state["raw_users"]:
                 if user_list_incomplete:

zerver/lib/user_groups.py

@@ -516,7 +516,9 @@ def get_group_setting_value_for_api(
         return setting_value_group.id
 
     return AnonymousSettingGroupDict(
-        direct_members=[member.id for member in setting_value_group.direct_members.all()],
+        direct_members=[
+            member.id for member in setting_value_group.direct_members.filter(is_active=True)
+        ],
         direct_subgroups=[subgroup.id for subgroup in setting_value_group.direct_subgroups.all()],
     )
 
@@ -565,7 +567,7 @@ def user_groups_in_realm_serialized(
 
     membership = (
         UserGroupMembership.objects.filter(user_group__realm=realm)
-        .exclude(user_profile__is_active=False, user_group__named_user_group__isnull=False)
+        .exclude(user_profile__is_active=False)
         .values_list("user_group_id", "user_profile_id")
     )
 

zerver/openapi/zulip.yaml

@@ -3182,9 +3182,16 @@ paths:
                                 if `include_deactivated_groups` client capability
                                 is set to `true`.
 
-                                **Changes**: Prior to Zulip 10.0 (feature level 294),
+                                This event is also sent when deactivating or reactivating
-                                this event was sent to all clients when a user group
+                                a user for settings set to anonymous user groups which the
-                                was deactivated.
+                                user is direct member of. When deactivating the user, event
+                                is only sent to users who cannot access the deactivated user.
+
+                                **Changes**: Starting with Zulip 10.0 (feature level 303), this
+                                event can also be sent when deactivating or reactivating a user.
+
+                                Prior to Zulip 10.0 (feature level 294), this event was sent to
+                                all clients when a user group was deactivated.
                               properties:
                                 id:
                                   $ref: "#/components/schemas/EventIdSchema"
@@ -4231,7 +4238,15 @@ paths:
                                 event type supports multiple properties being changed in a
                                 single event.
 
-                                **Changes**: In Zulip 7.0 (feature level 163), the realm setting
+                                This event is also sent when deactivating or reactivating a user
+                                for settings set to anonymous user groups which the user is direct
+                                member of. When deactivating the user, event is only sent to users
+                                who cannot access the deactivated user.
+
+                                **Changes**: Starting with Zulip 10.0 (feature level 303), this
+                                event can also be sent when deactivating or reactivating a user.
+
+                                In Zulip 7.0 (feature level 163), the realm setting
                                 `email_address_visibility` was removed. It was replaced by a [user
                                 setting](/api/update-settings#parameter-email_address_visibility) with
                                 a [realm user default][user-defaults], with the encoding of different
@@ -22053,6 +22068,9 @@ components:
             direct_members:
               description: |
                 The list of IDs of individual users in the collection of users with this permission.
+
+                **Changes**: Prior to Zulip 10.0 (feature level 303), this list also included
+                deactivated users.
               type: array
               items:
                 type: integer

zerver/tests/test_events.py

@@ -3050,6 +3050,29 @@ class NormalActionsTest(BaseAction):
 
     def test_do_deactivate_user(self) -> None:
         user_profile = self.example_user("cordelia")
+        members_group = NamedUserGroup.objects.get(
+            name=SystemGroups.MEMBERS, realm=user_profile.realm, is_system_group=True
+        )
+        setting_group = self.create_or_update_anonymous_group_for_setting(
+            [user_profile], [members_group]
+        )
+        do_change_realm_permission_group_setting(
+            self.user_profile.realm,
+            "can_create_public_channel_group",
+            setting_group,
+            acting_user=None,
+        )
+        hamletcharacters_group = NamedUserGroup.objects.get(
+            name="hamletcharacters", realm=self.user_profile.realm
+        )
+        hamlet = self.example_user("hamlet")
+        setting_group = self.create_or_update_anonymous_group_for_setting(
+            [user_profile, hamlet], [members_group]
+        )
+        do_change_user_group_permission_setting(
+            hamletcharacters_group, "can_mention_group", setting_group, acting_user=None
+        )
+
         with self.verify_action(num_events=1) as events:
             do_deactivate_user(user_profile, acting_user=None)
         check_realm_user_update("events[0]", events[0], "is_active")
@@ -3070,19 +3093,53 @@ class NormalActionsTest(BaseAction):
         # event if they cannot access the deactivated user.
         user_profile = self.example_user("cordelia")
         self.user_profile = self.example_user("polonius")
-        with self.verify_action(num_events=3) as events:
+        with self.verify_action(num_events=6) as events:
             do_deactivate_user(user_profile, acting_user=None)
         check_user_group_remove_members("events[0]", events[0])
         check_user_group_remove_members("events[1]", events[1])
         check_user_group_remove_members("events[2]", events[2])
+        check_user_group_update("events[3]", events[3], "can_manage_group")
+        check_realm_update_dict("events[4]", events[4])
+        check_user_group_update("events[5]", events[5], "can_mention_group")
+        self.assertEqual(
+            events[3]["data"]["can_manage_group"],
+            AnonymousSettingGroupDict(direct_members=[], direct_subgroups=[]),
+        )
+        self.assertEqual(
+            events[4]["data"]["can_create_public_channel_group"],
+            AnonymousSettingGroupDict(direct_members=[], direct_subgroups=[members_group.id]),
+        )
+        self.assertEqual(
+            events[5]["data"]["can_mention_group"],
+            AnonymousSettingGroupDict(
+                direct_members=[hamlet.id], direct_subgroups=[members_group.id]
+            ),
+        )
 
         user_profile = self.example_user("cordelia")
         do_reactivate_user(user_profile, acting_user=None)
-        with self.verify_action(num_events=3, user_list_incomplete=True) as events:
+        with self.verify_action(num_events=6, user_list_incomplete=True) as events:
             do_deactivate_user(user_profile, acting_user=None)
         check_user_group_remove_members("events[0]", events[0])
         check_user_group_remove_members("events[1]", events[1])
         check_user_group_remove_members("events[2]", events[2])
+        check_user_group_update("events[3]", events[3], "can_manage_group")
+        check_realm_update_dict("events[4]", events[4])
+        check_user_group_update("events[5]", events[5], "can_mention_group")
+        self.assertEqual(
+            events[3]["data"]["can_manage_group"],
+            AnonymousSettingGroupDict(direct_members=[], direct_subgroups=[]),
+        )
+        self.assertEqual(
+            events[4]["data"]["can_create_public_channel_group"],
+            AnonymousSettingGroupDict(direct_members=[], direct_subgroups=[members_group.id]),
+        )
+        self.assertEqual(
+            events[5]["data"]["can_mention_group"],
+            AnonymousSettingGroupDict(
+                direct_members=[hamlet.id], direct_subgroups=[members_group.id]
+            ),
+        )
 
         user_profile = self.example_user("shiva")
         with self.verify_action(num_events=1) as events:
@@ -3092,12 +3149,17 @@ class NormalActionsTest(BaseAction):
         # Guest loses access to deactivated user if the user
         # was not involved in DMs.
         user_profile = self.example_user("hamlet")
-        with self.verify_action(num_events=4) as events:
+        with self.verify_action(num_events=5) as events:
             do_deactivate_user(user_profile, acting_user=None)
         check_user_group_remove_members("events[0]", events[0])
         check_user_group_remove_members("events[1]", events[1])
         check_user_group_remove_members("events[2]", events[2])
-        check_realm_user_remove("events[3]]", events[3])
+        check_user_group_update("events[3]", events[3], "can_mention_group")
+        check_realm_user_remove("events[4]]", events[4])
+        self.assertEqual(
+            events[3]["data"]["can_mention_group"],
+            AnonymousSettingGroupDict(direct_members=[], direct_subgroups=[members_group.id]),
+        )
 
         user_profile = self.example_user("aaron")
         # One update event is for a deactivating a bot owned by aaron.
@@ -3143,6 +3205,30 @@ class NormalActionsTest(BaseAction):
         check_subscription_peer_remove("events[4]", events[4])
         check_stream_delete("events[5]", events[5])
 
+        user_profile = self.example_user("cordelia")
+        members_group = NamedUserGroup.objects.get(
+            name=SystemGroups.MEMBERS, realm=user_profile.realm, is_system_group=True
+        )
+        hamletcharacters_group = NamedUserGroup.objects.get(
+            name="hamletcharacters", realm=self.user_profile.realm
+        )
+
+        setting_group = self.create_or_update_anonymous_group_for_setting(
+            [user_profile], [hamletcharacters_group]
+        )
+        do_change_realm_permission_group_setting(
+            user_profile.realm,
+            "can_create_public_channel_group",
+            setting_group,
+            acting_user=None,
+        )
+        setting_group = self.create_or_update_anonymous_group_for_setting(
+            [user_profile], [members_group]
+        )
+        do_change_user_group_permission_setting(
+            hamletcharacters_group, "can_mention_group", setting_group, acting_user=None
+        )
+
         self.set_up_db_for_testing_user_access()
         # Test that guest users receive realm_user/update event
         # only if they can access the reactivated user.
@@ -3152,11 +3238,30 @@ class NormalActionsTest(BaseAction):
         self.user_profile = self.example_user("polonius")
         # Guest users receives group members update event for three groups -
         # members group, full members group and hamletcharacters group.
-        with self.verify_action(num_events=3) as events:
+        with self.verify_action(num_events=6) as events:
             do_reactivate_user(user_profile, acting_user=None)
         check_user_group_add_members("events[0]", events[0])
         check_user_group_add_members("events[1]", events[1])
         check_user_group_add_members("events[2]", events[2])
+        check_user_group_update("events[3]", events[3], "can_manage_group")
+        check_realm_update_dict("events[4]", events[4])
+        check_user_group_update("events[5]", events[5], "can_mention_group")
+        self.assertEqual(
+            events[3]["data"]["can_manage_group"],
+            AnonymousSettingGroupDict(direct_members=[user_profile.id], direct_subgroups=[]),
+        )
+        self.assertEqual(
+            events[4]["data"]["can_create_public_channel_group"],
+            AnonymousSettingGroupDict(
+                direct_members=[user_profile.id], direct_subgroups=[hamletcharacters_group.id]
+            ),
+        )
+        self.assertEqual(
+            events[5]["data"]["can_mention_group"],
+            AnonymousSettingGroupDict(
+                direct_members=[user_profile.id], direct_subgroups=[members_group.id]
+            ),
+        )
 
         user_profile = self.example_user("shiva")
         do_deactivate_user(user_profile, acting_user=None)