diff --git a/zerver/lib/mention.py b/zerver/lib/mention.py index a46f764ea2..226dedfe39 100644 --- a/zerver/lib/mention.py +++ b/zerver/lib/mention.py @@ -8,7 +8,7 @@ from django.conf import settings from django.db.models import Q from django_stubs_ext import StrPromise -from zerver.lib.streams import filter_stream_authorization +from zerver.lib.streams import UserGroupMembershipDetails, get_content_access_streams from zerver.lib.topic import get_first_message_for_user_in_topic from zerver.lib.user_groups import get_root_id_annotated_recursive_subgroups_for_groups from zerver.lib.users import get_inaccessible_user_ids @@ -200,7 +200,7 @@ class MentionBackend: ) result[row["name"]] = row["id"] else: - authorization = filter_stream_authorization( + content_access_streams = get_content_access_streams( acting_user, list( Stream.objects.filter( @@ -209,9 +209,11 @@ class MentionBackend: functools.reduce(lambda a, b: a | b, q_list), ) ), - is_subscribing_other_users=False, + user_group_membership_details=UserGroupMembershipDetails( + user_recursive_group_ids=None + ), ) - for stream in authorization.authorized_streams: + for stream in content_access_streams: assert stream.recipient_id is not None self.stream_cache[stream.name] = ChannelInfo( stream.id, stream.recipient_id, stream.history_public_to_subscribers diff --git a/zerver/lib/streams.py b/zerver/lib/streams.py index 8b11102724..493a6cce24 100644 --- a/zerver/lib/streams.py +++ b/zerver/lib/streams.py @@ -560,6 +560,7 @@ def user_has_content_access( user_group_membership_details: UserGroupMembershipDetails, *, is_subscribed: bool, + exclude_zephyr_realms_from_public_check: bool = False, ) -> bool: if stream.is_web_public: return True @@ -570,6 +571,9 @@ def user_has_content_access( if user_profile.is_guest: return False + if exclude_zephyr_realms_from_public_check and not stream.invite_only: + return True + if stream.is_public(): return True @@ -1040,6 +1044,7 @@ def get_streams_to_which_user_cannot_add_subscribers( user_profile: UserProfile, *, allow_default_streams: bool = False, + user_group_membership_details: UserGroupMembershipDetails, ) -> list[Stream]: # IMPORTANT: This function expects its callers to have already # checked that the user can access the provided channels, and thus @@ -1056,9 +1061,10 @@ def get_streams_to_which_user_cannot_add_subscribers( if user_profile.is_realm_admin: return [] - user_recursive_group_ids = set( - get_recursive_membership_groups(user_profile).values_list("id", flat=True) - ) + if user_group_membership_details.user_recursive_group_ids is None: + user_group_membership_details.user_recursive_group_ids = set( + get_recursive_membership_groups(user_profile).values_list("id", flat=True) + ) if allow_default_streams: default_stream_ids = get_default_stream_ids_for_realm(user_profile.realm_id) @@ -1075,10 +1081,14 @@ def get_streams_to_which_user_cannot_add_subscribers( if allow_default_streams and stream.id in default_stream_ids: continue - if is_user_in_can_administer_channel_group(stream, user_recursive_group_ids): + if is_user_in_can_administer_channel_group( + stream, user_group_membership_details.user_recursive_group_ids + ): continue - if not is_user_in_can_add_subscribers_group(stream, user_recursive_group_ids): + if not is_user_in_can_add_subscribers_group( + stream, user_group_membership_details.user_recursive_group_ids + ): result.append(stream) return result @@ -1097,21 +1107,21 @@ def can_administer_accessible_channel(channel: Stream, user_profile: UserProfile @dataclass -class StreamsCategorizedByPermissions: +class StreamsCategorizedByPermissionsForAddingSubscribers: authorized_streams: list[Stream] unauthorized_streams: list[Stream] streams_to_which_user_cannot_add_subscribers: list[Stream] -def filter_stream_authorization( - user_profile: UserProfile, streams: Collection[Stream], is_subscribing_other_users: bool = False -) -> StreamsCategorizedByPermissions: +def get_content_access_streams( + user_profile: UserProfile, + streams: Collection[Stream], + user_group_membership_details: UserGroupMembershipDetails, + *, + exclude_zephyr_realms_from_public_check: bool = False, +) -> list[Stream]: if len(streams) == 0: - return StreamsCategorizedByPermissions( - authorized_streams=[], - unauthorized_streams=[], - streams_to_which_user_cannot_add_subscribers=[], - ) + return [] recipient_ids = [stream.recipient_id for stream in streams] subscribed_recipient_ids = set( @@ -1120,52 +1130,70 @@ def filter_stream_authorization( ).values_list("recipient_id", flat=True) ) - unauthorized_streams: list[Stream] = [] + content_access_streams: list[Stream] = [] + + for stream in streams: + is_subscribed = stream.recipient_id in subscribed_recipient_ids + if user_has_content_access( + user_profile, + stream, + user_group_membership_details, + is_subscribed=is_subscribed, + exclude_zephyr_realms_from_public_check=exclude_zephyr_realms_from_public_check, + ): + content_access_streams.append(stream) + + return content_access_streams + + +def filter_stream_authorization_for_adding_subscribers( + user_profile: UserProfile, streams: Collection[Stream], is_subscribing_other_users: bool = False +) -> StreamsCategorizedByPermissionsForAddingSubscribers: + if len(streams) == 0: + return StreamsCategorizedByPermissionsForAddingSubscribers( + authorized_streams=[], + unauthorized_streams=[], + streams_to_which_user_cannot_add_subscribers=[], + ) + + # For adding subscribers, we consider streams in zephyr realm + # public. + user_group_membership_details = UserGroupMembershipDetails(user_recursive_group_ids=None) + content_access_streams = get_content_access_streams( + user_profile, + streams, + user_group_membership_details, + exclude_zephyr_realms_from_public_check=True, + ) + content_access_stream_ids = {stream.id for stream in content_access_streams} + streams_to_which_user_cannot_add_subscribers: list[Stream] = [] if is_subscribing_other_users: streams_to_which_user_cannot_add_subscribers = ( - get_streams_to_which_user_cannot_add_subscribers(list(streams), user_profile) + get_streams_to_which_user_cannot_add_subscribers( + content_access_streams, + user_profile, + user_group_membership_details=user_group_membership_details, + ) ) - for stream in streams: - # Deactivated streams are not accessible - if stream.deactivated: - unauthorized_streams.append(stream) - continue - - # The user is authorized for their own streams - if stream.recipient_id in subscribed_recipient_ids: - continue - - # Web-public streams are accessible even to guests - if stream.is_web_public: - continue - - if user_profile.is_guest: - unauthorized_streams.append(stream) - continue - - # Members and administrators are authorized for public streams - if not stream.invite_only: - continue - - if stream.invite_only: - user_recursive_group_ids = set( - get_recursive_membership_groups(user_profile).values_list("id", flat=True) - ) - # The above has checked that the user is not a guest for the below settings. - if is_user_in_groups_granting_content_access(stream, user_recursive_group_ids): - continue - - unauthorized_streams.append(stream) - - authorized_streams = [ + unauthorized_streams = [ stream for stream in streams - if stream.id not in {stream.id for stream in unauthorized_streams} - and stream.id not in {stream.id for stream in streams_to_which_user_cannot_add_subscribers} + if stream.deactivated or stream.id not in content_access_stream_ids ] - return StreamsCategorizedByPermissions( + unauthorized_stream_ids = {stream.id for stream in unauthorized_streams} + + stream_ids_to_which_user_cannot_add_subscribers = { + stream.id for stream in streams_to_which_user_cannot_add_subscribers + } + authorized_streams = [ + stream + for stream in content_access_streams + if stream.id not in stream_ids_to_which_user_cannot_add_subscribers + and stream.id not in unauthorized_stream_ids + ] + return StreamsCategorizedByPermissionsForAddingSubscribers( authorized_streams=authorized_streams, unauthorized_streams=unauthorized_streams, streams_to_which_user_cannot_add_subscribers=streams_to_which_user_cannot_add_subscribers, diff --git a/zerver/tests/test_markdown.py b/zerver/tests/test_markdown.py index f72edfa8d8..752604d0f5 100644 --- a/zerver/tests/test_markdown.py +++ b/zerver/tests/test_markdown.py @@ -19,6 +19,7 @@ from zerver.actions.alert_words import do_add_alert_words from zerver.actions.create_realm import do_create_realm from zerver.actions.realm_emoji import do_remove_realm_emoji from zerver.actions.realm_settings import do_set_realm_property +from zerver.actions.streams import do_change_stream_group_based_setting from zerver.actions.user_groups import ( add_subgroups_to_user_group, check_add_user_group, @@ -3292,6 +3293,51 @@ class MarkdownStreamTopicMentionTests(ZulipTestCase): f'
', ) + private_stream = self.make_stream("private", realm, invite_only=True) + content = "#**private>testing**" + self.assertEqual( + markdown_convert_wrapper(content), + f'#{private_stream.name} > testing
', + ) + + # Permalink would not be generated if a user has metadata + # access to a stream but not content access. + cordelia_group = self.create_or_update_anonymous_group_for_setting([cordelia], []) + do_change_stream_group_based_setting( + private_stream, + "can_administer_channel_group", + cordelia_group, + acting_user=None, + ) + msg = Message( + sender=cordelia, + sending_client=get_client("test"), + realm=realm, + ) + content = "#**private>testing**" + self.assertEqual( + render_message_markdown(msg, content, acting_user=cordelia).rendered_content, + "#private>testing
", + ) + + # User has content access now, so permalink would be generated. + do_change_stream_group_based_setting( + private_stream, + "can_add_subscribers_group", + cordelia_group, + acting_user=None, + ) + msg = Message( + sender=cordelia, + sending_client=get_client("test"), + realm=realm, + ) + content = "#**private>testing**" + self.assertEqual( + render_message_markdown(msg, content, acting_user=cordelia).rendered_content, + f'', + ) + def test_message_id_multiple(self) -> None: denmark = get_stream("Denmark", get_realm("zulip")) sender_user_profile = self.example_user("othello") diff --git a/zerver/tests/test_subs.py b/zerver/tests/test_subs.py index df71d0b132..ebd0df5f8c 100644 --- a/zerver/tests/test_subs.py +++ b/zerver/tests/test_subs.py @@ -71,7 +71,7 @@ from zerver.lib.stream_traffic import ( ) from zerver.lib.streams import ( StreamDict, - StreamsCategorizedByPermissions, + StreamsCategorizedByPermissionsForAddingSubscribers, UserGroupMembershipDetails, access_stream_by_id, access_stream_by_name, @@ -81,7 +81,7 @@ from zerver.lib.streams import ( create_streams_if_needed, do_get_streams, ensure_stream, - filter_stream_authorization, + filter_stream_authorization_for_adding_subscribers, list_to_streams, public_stream_user_ids, user_has_content_access, @@ -6006,10 +6006,12 @@ class SubscriptionAPITest(ZulipTestCase): # Verify the internal checks also block guest users. stream = get_stream("Denmark", guest_user.realm) - streams_categorized_by_permissions = filter_stream_authorization(guest_user, [stream]) + streams_categorized_by_permissions = filter_stream_authorization_for_adding_subscribers( + guest_user, [stream] + ) self.assertEqual( streams_categorized_by_permissions, - StreamsCategorizedByPermissions( + StreamsCategorizedByPermissionsForAddingSubscribers( authorized_streams=[], unauthorized_streams=[stream], streams_to_which_user_cannot_add_subscribers=[], @@ -6019,10 +6021,12 @@ class SubscriptionAPITest(ZulipTestCase): stream = self.make_stream("private_stream", invite_only=True) result = self.subscribe_via_post(guest_user, ["private_stream"], allow_fail=True) self.assert_json_error(result, "Not allowed for guest users") - streams_categorized_by_permissions = filter_stream_authorization(guest_user, [stream]) + streams_categorized_by_permissions = filter_stream_authorization_for_adding_subscribers( + guest_user, [stream] + ) self.assertEqual( streams_categorized_by_permissions, - StreamsCategorizedByPermissions( + StreamsCategorizedByPermissionsForAddingSubscribers( authorized_streams=[], unauthorized_streams=[stream], streams_to_which_user_cannot_add_subscribers=[], @@ -6041,10 +6045,12 @@ class SubscriptionAPITest(ZulipTestCase): # is_web_public=True, allow_fail=True) # self.assert_json_success(result) streams_to_sub = [web_public_stream, public_stream, private_stream] - streams_categorized_by_permissions = filter_stream_authorization(guest_user, streams_to_sub) + streams_categorized_by_permissions = filter_stream_authorization_for_adding_subscribers( + guest_user, streams_to_sub + ) self.assertEqual( streams_categorized_by_permissions, - StreamsCategorizedByPermissions( + StreamsCategorizedByPermissionsForAddingSubscribers( authorized_streams=[web_public_stream], unauthorized_streams=[public_stream, private_stream], streams_to_which_user_cannot_add_subscribers=[], diff --git a/zerver/views/invite.py b/zerver/views/invite.py index 00a1dc2349..e3ad4dd9b4 100644 --- a/zerver/views/invite.py +++ b/zerver/views/invite.py @@ -20,7 +20,11 @@ from zerver.actions.invites import ( from zerver.decorator import require_member_or_admin from zerver.lib.exceptions import InvitationError, JsonableError, OrganizationOwnerRequiredError from zerver.lib.response import json_success -from zerver.lib.streams import access_stream_by_id, get_streams_to_which_user_cannot_add_subscribers +from zerver.lib.streams import ( + UserGroupMembershipDetails, + access_stream_by_id, + get_streams_to_which_user_cannot_add_subscribers, +) from zerver.lib.typed_endpoint import ApiParamConfig, PathOnly, typed_endpoint from zerver.lib.typed_endpoint_validators import check_int_in_validator from zerver.lib.user_groups import access_user_group_for_update @@ -93,8 +97,12 @@ def access_streams_for_invite(stream_ids: list[int], user_profile: UserProfile) ) streams.append(stream) + user_group_membership_details = UserGroupMembershipDetails(user_recursive_group_ids=None) streams_to_which_user_cannot_add_subscribers = get_streams_to_which_user_cannot_add_subscribers( - streams, user_profile, allow_default_streams=True + streams, + user_profile, + allow_default_streams=True, + user_group_membership_details=user_group_membership_details, ) if len(streams_to_which_user_cannot_add_subscribers) > 0: raise JsonableError(_("You do not have permission to subscribe other users to channels.")) diff --git a/zerver/views/streams.py b/zerver/views/streams.py index 46b75a27f7..7d16f55c55 100644 --- a/zerver/views/streams.py +++ b/zerver/views/streams.py @@ -70,7 +70,7 @@ from zerver.lib.streams import ( access_web_public_stream, check_stream_name_available, do_get_streams, - filter_stream_authorization, + filter_stream_authorization_for_adding_subscribers, get_group_setting_value_dict_for_streams, get_stream_permission_default_group, get_stream_permission_policy_name, @@ -699,7 +699,7 @@ def add_subscriptions_backend( setting_groups_dict=setting_groups_dict, ) - streams_categorized_by_permissions = filter_stream_authorization( + streams_categorized_by_permissions = filter_stream_authorization_for_adding_subscribers( user_profile, existing_streams, is_subscribing_other_users ) authorized_streams = streams_categorized_by_permissions.authorized_streams