mirror of
				https://github.com/zulip/zulip.git
				synced 2025-11-04 05:53:43 +00:00 
			
		
		
		
	subscriptions: Change in API used for adding new subscriptions.
Earlier when a user who is not allowed to add subscribers to a stream because of realm level setting "Who can add users to streams" is subscribing other users while creating a new stream than new stream was created but no one is subscribed to stream. To fix this issue this commit makes changes in the API used for adding subscriptions. Now stream will be created only when user has permissions to add other users. With a rewrite of the test by Tim Abbott.
This commit is contained in:
		@@ -4422,7 +4422,7 @@ class SubscriptionAPITest(ZulipTestCase):
 | 
			
		||||
 | 
			
		||||
        # Now add ourselves
 | 
			
		||||
        with self.capture_send_event_calls(expected_num_events=2) as events:
 | 
			
		||||
            with self.assert_database_query_count(13):
 | 
			
		||||
            with self.assert_database_query_count(12):
 | 
			
		||||
                self.common_subscribe_to_streams(
 | 
			
		||||
                    self.test_user,
 | 
			
		||||
                    streams_to_sub,
 | 
			
		||||
@@ -4731,28 +4731,36 @@ class SubscriptionAPITest(ZulipTestCase):
 | 
			
		||||
 | 
			
		||||
    def test_bulk_subscribe_MIT(self) -> None:
 | 
			
		||||
        mit_user = self.mit_user("starnine")
 | 
			
		||||
        num_streams = 15
 | 
			
		||||
 | 
			
		||||
        realm = get_realm("zephyr")
 | 
			
		||||
        stream_names = [f"stream_{i}" for i in range(40)]
 | 
			
		||||
        stream_names = [f"stream_{i}" for i in range(num_streams)]
 | 
			
		||||
        streams = [self.make_stream(stream_name, realm=realm) for stream_name in stream_names]
 | 
			
		||||
 | 
			
		||||
        for stream in streams:
 | 
			
		||||
            stream.is_in_zephyr_realm = True
 | 
			
		||||
            stream.save()
 | 
			
		||||
 | 
			
		||||
        # Make sure Zephyr mirroring realms such as MIT do not get
 | 
			
		||||
        # any tornado subscription events
 | 
			
		||||
        with self.capture_send_event_calls(expected_num_events=0):
 | 
			
		||||
            with self.assert_database_query_count(5):
 | 
			
		||||
        # Verify that peer_event events are never sent in Zephyr
 | 
			
		||||
        # realm. This does generate stream creation events from
 | 
			
		||||
        # send_stream_creation_events_for_private_streams.
 | 
			
		||||
        with self.capture_send_event_calls(expected_num_events=num_streams + 1) as events:
 | 
			
		||||
            with self.assert_database_query_count(num_streams + 12):
 | 
			
		||||
                self.common_subscribe_to_streams(
 | 
			
		||||
                    mit_user,
 | 
			
		||||
                    stream_names,
 | 
			
		||||
                    dict(principals=orjson.dumps([mit_user.id]).decode()),
 | 
			
		||||
                    subdomain="zephyr",
 | 
			
		||||
                    allow_fail=True,
 | 
			
		||||
                )
 | 
			
		||||
            # num_streams stream creation events:
 | 
			
		||||
            self.assertEqual(
 | 
			
		||||
                {(event["event"]["type"], event["event"]["op"]) for event in events[0:num_streams]},
 | 
			
		||||
                {("stream", "create")},
 | 
			
		||||
            )
 | 
			
		||||
            # Followed by one subscription event:
 | 
			
		||||
            self.assertEqual(events[num_streams]["event"]["type"], "subscription")
 | 
			
		||||
 | 
			
		||||
        with self.capture_send_event_calls(expected_num_events=0):
 | 
			
		||||
        with self.capture_send_event_calls(expected_num_events=1):
 | 
			
		||||
            bulk_remove_subscriptions(
 | 
			
		||||
                realm,
 | 
			
		||||
                users=[mit_user],
 | 
			
		||||
 
 | 
			
		||||
@@ -619,6 +619,21 @@ def add_subscriptions_backend(
 | 
			
		||||
 | 
			
		||||
        stream_dicts.append(stream_dict_copy)
 | 
			
		||||
 | 
			
		||||
    is_subscribing_other_users = False
 | 
			
		||||
    if len(principals) > 0 and not all(user_id == user_profile.id for user_id in principals):
 | 
			
		||||
        is_subscribing_other_users = True
 | 
			
		||||
 | 
			
		||||
    if is_subscribing_other_users:
 | 
			
		||||
        if not user_profile.can_subscribe_other_users():
 | 
			
		||||
            # Guest users case will not be handled here as it will
 | 
			
		||||
            # be handled by the decorator above.
 | 
			
		||||
            raise JsonableError(_("Insufficient permission"))
 | 
			
		||||
        subscribers = {
 | 
			
		||||
            principal_to_user_profile(user_profile, principal) for principal in principals
 | 
			
		||||
        }
 | 
			
		||||
    else:
 | 
			
		||||
        subscribers = {user_profile}
 | 
			
		||||
 | 
			
		||||
    # Validation of the streams arguments, including enforcement of
 | 
			
		||||
    # can_create_streams policy and check_stream_name policy is inside
 | 
			
		||||
    # list_to_streams.
 | 
			
		||||
@@ -635,20 +650,14 @@ def add_subscriptions_backend(
 | 
			
		||||
    # Newly created streams are also authorized for the creator
 | 
			
		||||
    streams = authorized_streams + created_streams
 | 
			
		||||
 | 
			
		||||
    if len(principals) > 0:
 | 
			
		||||
        if realm.is_zephyr_mirror_realm and not all(stream.invite_only for stream in streams):
 | 
			
		||||
    if (
 | 
			
		||||
        is_subscribing_other_users
 | 
			
		||||
        and realm.is_zephyr_mirror_realm
 | 
			
		||||
        and not all(stream.invite_only for stream in streams)
 | 
			
		||||
    ):
 | 
			
		||||
        raise JsonableError(
 | 
			
		||||
            _("You can only invite other Zephyr mirroring users to private streams.")
 | 
			
		||||
        )
 | 
			
		||||
        if not user_profile.can_subscribe_other_users():
 | 
			
		||||
            # Guest users case will not be handled here as it will
 | 
			
		||||
            # be handled by the decorator above.
 | 
			
		||||
            raise JsonableError(_("Insufficient permission"))
 | 
			
		||||
        subscribers = {
 | 
			
		||||
            principal_to_user_profile(user_profile, principal) for principal in principals
 | 
			
		||||
        }
 | 
			
		||||
    else:
 | 
			
		||||
        subscribers = {user_profile}
 | 
			
		||||
 | 
			
		||||
    (subscribed, already_subscribed) = bulk_add_subscriptions(
 | 
			
		||||
        realm, streams, subscribers, acting_user=user_profile, color_map=color_map
 | 
			
		||||
 
 | 
			
		||||
		Reference in New Issue
	
	Block a user