From 39a31170eeba93ddb92633c27625a30557a09046 Mon Sep 17 00:00:00 2001 From: Sahil Batra Date: Tue, 24 Oct 2023 08:41:34 +0530 Subject: [PATCH] streams: Send event when guest loses access to a user. This commit adds code to send "realm_user/remove" event when a guest user loses access to a user due to the user being unsubscribed from one or more streams. --- api_docs/changelog.md | 3 ++ version.py | 2 +- zerver/actions/streams.py | 68 +++++++++++++++++++++++++++++++++++++ zerver/lib/event_schema.py | 10 ++++++ zerver/lib/events.py | 7 ++++ zerver/openapi/zulip.yaml | 4 ++- zerver/tests/test_events.py | 19 +++++++++++ 7 files changed, 111 insertions(+), 2 deletions(-) diff --git a/api_docs/changelog.md b/api_docs/changelog.md index 836cff7a47..6922244052 100644 --- a/api_docs/changelog.md +++ b/api_docs/changelog.md @@ -41,6 +41,9 @@ format used by the Zulip server that they are interacting with. * [`GET /events`](/api/get-events): `realm_user` events with `op: "add"` are now also sent when a guest user gains access to a user. +* [`GET /events`](/api/get-events): `realm_user` events with `op: "remove"` + are now also sent when a guest user loses access to a user. + **Feature level 227** * [`PATCH /realm/user_settings_defaults`](/api/update-realm-user-settings-defaults), diff --git a/version.py b/version.py index 2224024eec..a4e1f6c726 100644 --- a/version.py +++ b/version.py @@ -33,7 +33,7 @@ DESKTOP_WARNING_VERSION = "5.9.3" # Changes should be accompanied by documentation explaining what the # new level means in api_docs/changelog.md, as well as "**Changes**" # entries in the endpoint's documentation in `zulip.yaml`. -API_FEATURE_LEVEL = 227 +API_FEATURE_LEVEL = 228 # Bump the minor PROVISION_VERSION to indicate that folks should provision # only when going from an old version of the code to a newer version. Bump diff --git a/zerver/actions/streams.py b/zerver/actions/streams.py index 673405fcc0..f6dd2154f6 100644 --- a/zerver/actions/streams.py +++ b/zerver/actions/streams.py @@ -34,6 +34,7 @@ from zerver.lib.stream_subscription import ( get_active_subscriptions_for_stream_id, get_bulk_stream_subscriber_info, get_used_colors_for_user_ids, + get_user_ids_for_streams, get_users_for_streams, ) from zerver.lib.stream_traffic import get_streams_traffic @@ -896,6 +897,67 @@ def send_subscription_remove_events( ) +def send_user_remove_events_on_removing_subscriptions( + realm: Realm, altered_user_dict: Dict[UserProfile, Set[int]] +) -> None: + altered_stream_ids: Set[int] = set() + altered_users = list(altered_user_dict.keys()) + for stream_ids in altered_user_dict.values(): + altered_stream_ids |= stream_ids + + users_involved_in_dms = get_users_involved_in_dms_with_target_users(altered_users, realm) + subscribers_of_altered_user_subscriptions = get_subscribers_of_target_user_subscriptions( + altered_users + ) + + non_guest_user_ids = active_non_guest_user_ids(realm.id) + + subscribers_dict = get_user_ids_for_streams(altered_stream_ids) + + for user in altered_users: + users_in_unsubscribed_streams: Set[int] = set() + for stream_id in altered_user_dict[user]: + users_in_unsubscribed_streams = ( + users_in_unsubscribed_streams | subscribers_dict[stream_id] + ) + + users_who_can_access_altered_user = ( + set(non_guest_user_ids) + | subscribers_of_altered_user_subscriptions[user.id] + | users_involved_in_dms[user.id] + | {user.id} + ) + + subscribers_without_access_to_altered_user = ( + users_in_unsubscribed_streams - users_who_can_access_altered_user + ) + + if subscribers_without_access_to_altered_user: + event_remove_user = dict( + type="realm_user", + op="remove", + person=dict(user_id=user.id, full_name=str(UserProfile.INACCESSIBLE_USER_NAME)), + ) + send_event_on_commit( + realm, event_remove_user, list(subscribers_without_access_to_altered_user) + ) + + if user.is_guest: + users_inaccessible_to_altered_user = users_in_unsubscribed_streams - ( + subscribers_of_altered_user_subscriptions[user.id] + | users_involved_in_dms[user.id] + | {user.id} + ) + + for user_id in users_inaccessible_to_altered_user: + event_remove_user = dict( + type="realm_user", + op="remove", + person=dict(user_id=user_id, full_name=str(UserProfile.INACCESSIBLE_USER_NAME)), + ) + send_event_on_commit(realm, event_remove_user, [user.id]) + + def bulk_remove_subscriptions( realm: Realm, users: Iterable[UserProfile], @@ -978,6 +1040,12 @@ def bulk_remove_subscriptions( removed_sub_tuples = [(sub_info.user, sub_info.stream) for sub_info in subs_to_deactivate] send_subscription_remove_events(realm, users, streams, removed_sub_tuples) + if realm.can_access_all_users_group.name != SystemGroups.EVERYONE: + altered_user_dict: Dict[UserProfile, Set[int]] = defaultdict(set) + for user, stream in removed_sub_tuples: + altered_user_dict[user].add(stream.id) + send_user_remove_events_on_removing_subscriptions(realm, altered_user_dict) + new_vacant_streams = set(streams_to_unsubscribe) - set(occupied_streams_after) new_vacant_private_streams = [stream for stream in new_vacant_streams if stream.invite_only] diff --git a/zerver/lib/event_schema.py b/zerver/lib/event_schema.py index af05cf456d..bd8a99482b 100644 --- a/zerver/lib/event_schema.py +++ b/zerver/lib/event_schema.py @@ -1206,6 +1206,16 @@ def check_realm_user_update( ) +realm_user_remove_event = event_dict_type( + required_keys=[ + ("type", Equals("realm_user")), + ("op", Equals("remove")), + ("person", removed_user_type), + ], +) +check_realm_user_remove = make_checker(realm_user_remove_event) + + restart_event = event_dict_type( required_keys=[ ("type", Equals("restart")), diff --git a/zerver/lib/events.py b/zerver/lib/events.py index f5413cd63c..e3cf99c7bf 100644 --- a/zerver/lib/events.py +++ b/zerver/lib/events.py @@ -63,6 +63,7 @@ from zerver.lib.user_status import get_user_status_dict from zerver.lib.user_topics import get_topic_mutes, get_user_topics from zerver.lib.users import ( get_cross_realm_dicts, + get_data_for_inaccessible_user, get_users_for_api, is_administrator_role, max_message_id_for_user, @@ -989,6 +990,12 @@ def apply_event( sub["subscribers"] = [ user_id for user_id in sub["subscribers"] if user_id != person_user_id ] + elif event["op"] == "remove": + if person_user_id in state["raw_users"]: + inaccessible_user_dict = get_data_for_inaccessible_user( + user_profile.realm, person_user_id + ) + state["raw_users"][person_user_id] = inaccessible_user_dict else: raise AssertionError("Unexpected event type {type}/{op}".format(**event)) elif event["type"] == "realm_bot": diff --git a/zerver/openapi/zulip.yaml b/zerver/openapi/zulip.yaml index 875bad29ff..adb409716d 100644 --- a/zerver/openapi/zulip.yaml +++ b/zerver/openapi/zulip.yaml @@ -1085,7 +1085,9 @@ paths: } - type: object description: | - No longer sent by modern Zulip servers. + Event sent to guest users when they lose access to a user. + + **Changes**: Prior to Zulip 8.0 (feature level 228), this event was deprecated. **Changes**: Deprecated and no longer sent since Zulip 8.0 (feature level 222). diff --git a/zerver/tests/test_events.py b/zerver/tests/test_events.py index b758857eb3..eebc554903 100644 --- a/zerver/tests/test_events.py +++ b/zerver/tests/test_events.py @@ -166,6 +166,7 @@ from zerver.lib.event_schema import ( check_realm_update, check_realm_update_dict, check_realm_user_add, + check_realm_user_remove, check_realm_user_update, check_scheduled_message_add, check_scheduled_message_remove, @@ -4078,6 +4079,15 @@ class SubscribeActionTest(BaseAction): check_subscription_peer_add("events[1]", events[1]) self.assertEqual(set(events[1]["user_ids"]), {iago.id, othello.id}) + unsubscribe_action = lambda: bulk_remove_subscriptions( + realm, [othello, iago], [stream], acting_user=None + ) + events = self.verify_action(unsubscribe_action, num_events=2) + check_subscription_peer_remove("events[0]", events[0]) + self.assertEqual(set(events[0]["user_ids"]), {iago.id, othello.id}) + check_realm_user_remove("events[1]", events[1]) + self.assertEqual(events[1]["person"]["user_id"], othello.id) + def test_user_access_events_on_changing_subscriptions_for_guests(self) -> None: self.set_up_db_for_testing_user_access() polonius = self.example_user("polonius") @@ -4094,6 +4104,15 @@ class SubscribeActionTest(BaseAction): check_realm_user_add("events[2]", events[2]) self.assertEqual(events[2]["person"]["user_id"], othello.id) + unsubscribe_action = lambda: bulk_remove_subscriptions( + realm, [polonius, self.example_user("iago")], [stream], acting_user=None + ) + events = self.verify_action(unsubscribe_action, num_events=3) + check_subscription_remove("events[0]", events[0]) + check_stream_delete("events[1]", events[1]) + check_realm_user_remove("events[2]", events[2]) + self.assertEqual(events[2]["person"]["user_id"], othello.id) + class DraftActionTest(BaseAction): def do_enable_drafts_synchronization(self, user_profile: UserProfile) -> None: