paulmataruso/zulip
1
0
Fork 0
mirror of https://github.com/zulip/zulip.git synced 2025-10-23 04:52:12 +00:00
Code Issues Packages Projects Releases Wiki Activity

stream: Notify users with metadata access for lib/streams.py.

Browse Source 
Users in `can_administer_channel_group` and `can_add_subscribers_group`
have access to private channel metadata. They should be notified of
relevant events.
We've only made relevant changes to lib/streams.py in this commit to
make the changes small and reviewable.
This commit is contained in:
Shubham Padia
2025-02-04 14:02:06 +00:00
committed by Tim Abbott
parent ca1aba9fc3
commit f6301c24fe
3 changed files with 91 additions and 35 deletions
Download Patch File Download Diff File Expand all files Collapse all files

24
zerver/actions/streams.py
View File

@@ -37,7 +37,7 @@ from zerver.lib.stream_subscription import (
)
from zerver.lib.stream_traffic import get_streams_traffic
from zerver.lib.streams import (
can_access_stream_user_ids,
can_access_stream_metadata_user_ids,
check_basic_stream_access,
get_group_setting_value_dict_for_streams,
get_occupied_streams,
@@ -118,7 +118,7 @@ def do_deactivate_stream(stream: Stream, *, acting_user: UserProfile | None) ->
raise JsonableError(_("Channel is already deactivated"))
# Get the affected user ids *before* we deactivate everybody.
affected_user_ids = can_access_stream_user_ids(stream)
affected_user_ids = can_access_stream_metadata_user_ids(stream)
was_public = stream.is_public()
was_web_public = stream.is_web_public
@@ -1273,11 +1273,11 @@ def do_change_stream_permission(
stream.id, include_deactivated_users=False
).values_list("user_profile_id", flat=True)
old_can_access_stream_user_ids = set(stream_subscriber_user_ids) | {
old_can_access_stream_metadata_user_ids = set(stream_subscriber_user_ids) | {
user.id for user in stream.realm.get_admin_users_and_bots()
}
non_guest_user_ids = set(active_non_guest_user_ids(stream.realm_id))
notify_stream_creation_ids = non_guest_user_ids - old_can_access_stream_user_ids
notify_stream_creation_ids = non_guest_user_ids - old_can_access_stream_metadata_user_ids
recent_traffic = get_streams_traffic({stream.id}, realm)
setting_groups_dict = get_group_setting_value_dict_for_streams([stream])
@@ -1312,7 +1312,9 @@ def do_change_stream_permission(
)
# we do not need to send update events to the users who received creation event
# since they already have the updated stream info.
notify_stream_update_ids = can_access_stream_user_ids(stream) - notify_stream_creation_ids
notify_stream_update_ids = (
can_access_stream_metadata_user_ids(stream) - notify_stream_creation_ids
)
send_event_on_commit(stream.realm, event, notify_stream_update_ids)
old_policy_name = get_stream_permission_policy_name(
@@ -1431,7 +1433,7 @@ def do_rename_stream(stream: Stream, new_name: str, user_profile: UserProfile) -
stream_id=stream.id,
name=old_name,
)
send_event_on_commit(stream.realm, event, can_access_stream_user_ids(stream))
send_event_on_commit(stream.realm, event, can_access_stream_metadata_user_ids(stream))
sender = get_system_bot(settings.NOTIFICATION_BOT, stream.realm_id)
with override_language(stream.realm.default_language):
internal_send_stream_message(
@@ -1505,7 +1507,7 @@ def do_change_stream_description(
value=new_description,
rendered_description=stream.rendered_description,
)
send_event_on_commit(stream.realm, event, can_access_stream_user_ids(stream))
send_event_on_commit(stream.realm, event, can_access_stream_metadata_user_ids(stream))
send_change_stream_description_notification(
stream,
@@ -1590,7 +1592,7 @@ def do_change_stream_message_retention_days(
stream_id=stream.id,
name=stream.name,
)
send_event_on_commit(stream.realm, event, can_access_stream_user_ids(stream))
send_event_on_commit(stream.realm, event, can_access_stream_metadata_user_ids(stream))
send_change_stream_message_retention_days_notification(
user_profile=acting_user,
stream=stream,
@@ -1644,7 +1646,7 @@ def do_change_stream_group_based_setting(
stream_id=stream.id,
name=stream.name,
)
send_event_on_commit(stream.realm, event, can_access_stream_user_ids(stream))
send_event_on_commit(stream.realm, event, can_access_stream_metadata_user_ids(stream))
if setting_name == "can_send_message_group":
old_stream_post_policy = get_stream_post_policy_value_based_on_group_setting(old_user_group)
@@ -1659,7 +1661,7 @@ def do_change_stream_group_based_setting(
stream_id=stream.id,
name=stream.name,
)
send_event_on_commit(stream.realm, event, can_access_stream_user_ids(stream))
send_event_on_commit(stream.realm, event, can_access_stream_metadata_user_ids(stream))
# Backwards-compatibility code: We removed the
# is_announcement_only property in early 2020, but we send a
@@ -1673,7 +1675,7 @@ def do_change_stream_group_based_setting(
stream_id=stream.id,
name=stream.name,
)
send_event_on_commit(stream.realm, event, can_access_stream_user_ids(stream))
send_event_on_commit(stream.realm, event, can_access_stream_metadata_user_ids(stream))
assert acting_user is not None
send_stream_posting_permission_update_notification(

36
zerver/lib/streams.py
View File

@@ -24,6 +24,7 @@ from zerver.lib.string_validation import check_stream_name
from zerver.lib.timestamp import datetime_to_timestamp
from zerver.lib.types import AnonymousSettingGroupDict, APIStreamDict
from zerver.lib.user_groups import (
get_recursive_group_members,
get_recursive_membership_groups,
get_role_based_system_groups_dict,
user_has_permission_for_group_setting,
@@ -179,6 +180,18 @@ def get_default_values_for_stream_permission_group_settings(
return group_setting_values
def get_user_ids_with_metadata_access_via_permission_groups(stream: Stream) -> set[int]:
stream_admin_user_ids = set(
get_recursive_group_members(stream.can_administer_channel_group).values_list(
"id", flat=True
)
)
stream_add_subscribers_group_user_ids = set(
get_recursive_group_members(stream.can_add_subscribers_group).values_list("id", flat=True)
)
return stream_admin_user_ids | stream_add_subscribers_group_user_ids
@transaction.atomic(savepoint=False)
def create_stream_if_needed(
realm: Realm,
@@ -264,9 +277,15 @@ def create_stream_if_needed(
realm, stream, notify_user_ids, setting_groups_dict=setting_groups_dict
)
else:
realm_admin_ids = [user.id for user in stream.realm.get_admin_users_and_bots()]
realm_admin_ids = {user.id for user in stream.realm.get_admin_users_and_bots()}
send_stream_creation_event(
realm, stream, realm_admin_ids, setting_groups_dict=setting_groups_dict
realm,
stream,
list(
realm_admin_ids
| get_user_ids_with_metadata_access_via_permission_groups(stream)
),
setting_groups_dict=setting_groups_dict,
)
return stream, created
@@ -746,7 +765,7 @@ def public_stream_user_ids(stream: Stream) -> set[int]:
return set(active_non_guest_user_ids(stream.realm_id)) | guest_subscriptions_ids
def can_access_stream_user_ids(stream: Stream) -> set[int]:
def can_access_stream_metadata_user_ids(stream: Stream) -> set[int]:
# return user ids of users who can access the attributes of a
# stream, such as its name/description. Useful for sending events
# to all users with access to a stream's attributes.
@@ -755,10 +774,13 @@ def can_access_stream_user_ids(stream: Stream) -> set[int]:
# except unsubscribed guest users
return public_stream_user_ids(stream)
else:
# for a private stream, it's subscribers plus realm admins.
return private_stream_user_ids(stream.id) | {
user.id for user in stream.realm.get_admin_users_and_bots()
}
# for a private stream, it's subscribers plus channel admins
# and users belonging to `can_add_subscribers_group`.
return (
private_stream_user_ids(stream.id)
| {user.id for user in stream.realm.get_admin_users_and_bots()}
| get_user_ids_with_metadata_access_via_permission_groups(stream)
)
def can_access_stream_history(user_profile: UserProfile, stream: Stream) -> bool:

66
zerver/tests/test_subs.py
View File

@@ -75,7 +75,7 @@ from zerver.lib.streams import (
access_stream_by_id,
access_stream_by_name,
can_access_stream_history,
can_access_stream_user_ids,
can_access_stream_metadata_user_ids,
create_stream_if_needed,
create_streams_if_needed,
do_get_streams,
@@ -286,6 +286,7 @@ class TestCreateStreams(ZulipTestCase):
stream_names = ["new1", "new2", "new3"]
stream_descriptions = ["des1", "des2", "des3"]
realm = get_realm("zulip")
iago = self.example_user("iago")
# Test stream creation events.
with self.capture_send_event_calls(expected_num_events=1) as events:
@@ -298,15 +299,34 @@ class TestCreateStreams(ZulipTestCase):
self.assertEqual(events[0]["event"]["streams"][0]["name"], "Public stream")
self.assertEqual(events[0]["event"]["streams"][0]["stream_weekly_traffic"], None)
aaron_group = check_add_user_group(
realm, "aaron_group", [self.example_user("aaron")], acting_user=iago
)
prospero_group = check_add_user_group(
realm, "prospero_group", [self.example_user("prospero")], acting_user=iago
)
with self.capture_send_event_calls(expected_num_events=1) as events:
ensure_stream(realm, "Private stream", invite_only=True, acting_user=None)
create_stream_if_needed(
realm,
"Private stream",
invite_only=True,
can_administer_channel_group=aaron_group,
can_add_subscribers_group=prospero_group
)
self.assertEqual(events[0]["event"]["type"], "stream")
self.assertEqual(events[0]["event"]["op"], "create")
# Send private stream creation event to only realm admins.
self.assert_length(events[0]["users"], 2)
self.assertTrue(self.example_user("iago").id in events[0]["users"])
self.assertTrue(self.example_user("desdemona").id in events[0]["users"])
self.assert_length(events[0]["users"], 4)
self.assertCountEqual(
[
iago.id,
self.example_user("desdemona").id,
self.example_user("aaron").id,
self.example_user("prospero").id,
],
events[0]["users"],
)
self.assertEqual(events[0]["event"]["streams"][0]["name"], "Private stream")
self.assertEqual(events[0]["event"]["streams"][0]["stream_weekly_traffic"], None)
@@ -2042,6 +2062,24 @@ class StreamAdminTest(ZulipTestCase):
stream_private = self.make_stream(
"stream_private_name1", realm=user_profile.realm, invite_only=True
)
aaron_group = check_add_user_group(
realm, "aaron_group", [self.example_user("aaron")], acting_user=user_profile
)
do_change_stream_group_based_setting(
stream_private,
"can_administer_channel_group",
aaron_group,
acting_user=None,
)
prospero_group = check_add_user_group(
realm, "prospero_group", [self.example_user("prospero")], acting_user=user_profile
)
do_change_stream_group_based_setting(
stream_private,
"can_add_subscribers_group",
prospero_group,
acting_user=None,
)
self.subscribe(self.example_user("cordelia"), "stream_private_name1")
with self.capture_send_event_calls(expected_num_events=2) as events:
stream_id = get_stream("stream_private_name1", realm).id
@@ -2051,13 +2089,16 @@ class StreamAdminTest(ZulipTestCase):
)
self.assert_json_success(result)
notified_user_ids = set(events[0]["users"])
self.assertEqual(notified_user_ids, can_access_stream_user_ids(stream_private))
self.assertEqual(notified_user_ids, can_access_stream_metadata_user_ids(stream_private))
self.assertIn(self.example_user("cordelia").id, notified_user_ids)
# An important corner case is that all organization admins are notified.
self.assertIn(self.example_user("iago").id, notified_user_ids)
# The current user, Hamlet was made an admin and thus should be notified too.
self.assertIn(user_profile.id, notified_user_ids)
self.assertNotIn(self.example_user("prospero").id, notified_user_ids)
# Channel admin should be notified.
self.assertIn(self.example_user("aaron").id, notified_user_ids)
# User belonging to `can_add_subscribers_group` should be notified.
self.assertIn(self.example_user("prospero").id, notified_user_ids)
def test_rename_stream_requires_admin(self) -> None:
user_profile = self.example_user("hamlet")
@@ -2297,15 +2338,6 @@ class StreamAdminTest(ZulipTestCase):
'<p>See <a href="https://zulip.com/team/">https://zulip.com/team/</a></p>',
)
user_profile_group = check_add_user_group(
realm, "user_profile_group", [user_profile], acting_user=user_profile
)
do_change_stream_group_based_setting(
stream,
"can_administer_channel_group",
user_profile_group,
acting_user=None,
)
do_change_user_role(user_profile, UserProfile.ROLE_MEMBER, acting_user=None)
result = self.client_patch(
f"/json/streams/{stream_id}", {"description": "Test description"}
@@ -6463,7 +6495,7 @@ class SubscriptionAPITest(ZulipTestCase):
)
# Test creating private stream.
with self.assert_database_query_count(46):
with self.assert_database_query_count(48):
self.subscribe_via_post(
self.test_user,
[new_streams[1]],