From b5732b90d6cd22884cde850249902abeeb11a510 Mon Sep 17 00:00:00 2001 From: Sahil Batra Date: Tue, 30 Jul 2024 19:59:31 +0530 Subject: [PATCH] create_user: Do not send reactivation event for inaccessible users. --- api_docs/changelog.md | 5 +++++ zerver/actions/create_user.py | 5 +++-- zerver/openapi/zulip.yaml | 17 +++++++++++------ zerver/tests/test_events.py | 16 ++++++++++++++++ 4 files changed, 35 insertions(+), 8 deletions(-) diff --git a/api_docs/changelog.md b/api_docs/changelog.md index c8fc2c6495..3e1412ff08 100644 --- a/api_docs/changelog.md +++ b/api_docs/changelog.md @@ -20,6 +20,11 @@ format used by the Zulip server that they are interacting with. ## Changes in Zulip 10.0 +**Feature level 303** + +* [`GET /events`](/api/get-events): User reactivation event is not sent + to users who cannot access the reactivated user anymore. + **Feature level 302** * [`GET /users/{email}`](/api/get-user-by-email): Changed the `email` diff --git a/zerver/actions/create_user.py b/zerver/actions/create_user.py index 2801cdfd30..9b6527690a 100644 --- a/zerver/actions/create_user.py +++ b/zerver/actions/create_user.py @@ -37,6 +37,7 @@ from zerver.lib.users import ( format_user_row, get_api_key, get_data_for_inaccessible_user, + get_user_ids_who_can_access_user, user_access_restricted_in_realm, user_profile_to_user_row, ) @@ -59,7 +60,7 @@ from zerver.models import ( ) from zerver.models.groups import SystemGroups from zerver.models.realm_audit_logs import AuditLogEventType -from zerver.models.users import active_user_ids, bot_owner_user_ids, get_system_bot +from zerver.models.users import bot_owner_user_ids, get_system_bot from zerver.tornado.django_api import send_event_on_commit MAX_NUM_RECENT_MESSAGES = 1000 @@ -724,7 +725,7 @@ def do_reactivate_user(user_profile: UserProfile, *, acting_user: UserProfile | event = dict( type="realm_user", op="update", person=dict(user_id=user_profile.id, is_active=True) ) - send_event_on_commit(user_profile.realm, event, active_user_ids(user_profile.realm_id)) + send_event_on_commit(user_profile.realm, event, get_user_ids_who_can_access_user(user_profile)) if user_profile.is_bot: event = dict( diff --git a/zerver/openapi/zulip.yaml b/zerver/openapi/zulip.yaml index 84737117fa..bdaec6e2b2 100644 --- a/zerver/openapi/zulip.yaml +++ b/zerver/openapi/zulip.yaml @@ -610,13 +610,18 @@ paths: - type: object additionalProperties: false description: | - When a user is deactivated or reactivated. + When a user is deactivated or reactivated. Only users + who can access the deactivated or reactivated user + receive this event. - **Changes**: New in Zulip 8.0 (feature level - 222). Previously the server sent a `realm_user` event with - `op` field set to `remove` when deactivating a user and a - `realm_user` event with `op` field set to `add` when - reactivating a user. + **Changes**: Prior to Zulip 10.0 (feature level 303), + event for reactivation was also sent to users who + cannot access the reactivated user. + + New in Zulip 8.0 (feature level 222). Previously the server + sent a `realm_user` event with `op` field set to `remove` + when deactivating a user and a `realm_user` event with `op` + field set to `add` when reactivating a user. properties: user_id: type: integer diff --git a/zerver/tests/test_events.py b/zerver/tests/test_events.py index 090a483fa5..a8e2fb3c8d 100644 --- a/zerver/tests/test_events.py +++ b/zerver/tests/test_events.py @@ -3118,6 +3118,22 @@ class NormalActionsTest(BaseAction): check_subscription_peer_remove("events[4]", events[4]) check_stream_delete("events[5]", events[5]) + self.set_up_db_for_testing_user_access() + # Test that guest users receive event only + # if they can access the reactivated user. + user_profile = self.example_user("cordelia") + do_deactivate_user(user_profile, acting_user=None) + + self.user_profile = self.example_user("polonius") + with self.verify_action(num_events=0, state_change_expected=False) as events: + do_reactivate_user(user_profile, acting_user=None) + + user_profile = self.example_user("shiva") + do_deactivate_user(user_profile, acting_user=None) + with self.verify_action(num_events=1) as events: + do_reactivate_user(user_profile, acting_user=None) + check_realm_user_update("events[0]", events[0], "is_active") + def test_do_deactivate_realm(self) -> None: realm = self.user_profile.realm