get_streams: Return metadata access streams in include_all.

This parameter is no longer restricted to realm administrators. Any
user can get the streams they have metadata access to by setting this
parameter to true.

api_docs/changelog.md

@@ -37,8 +37,9 @@ format used by the Zulip server that they are interacting with.
   `include_can_access_content`, if set to True, returns all the
   channels that the user making the request has content access to.
 * [`GET /streams`](/api/get-streams): Rename `include_all_active` to
-  `include_all` since the separate `exclude_archived` parameter is what
-  controls whether to include archived channels.
+  `include_all` since the separate `exclude_archived` parameter is
+  what controls whether to include archived channels. The
+  `include_all` parameter is now supported for non-administrators.
 
 **Feature level 355**
 

zerver/lib/streams.py

@@ -566,6 +566,45 @@ def check_for_exactly_one_stream_arg(stream_id: int | None, stream: str | None)
         raise IncompatibleParametersError(["stream_id", "stream"])
 
 
+def user_has_metadata_access(
+    user_profile: UserProfile,
+    stream: Stream,
+    user_group_membership_details: UserGroupMembershipDetails,
+    *,
+    is_subscribed: bool,
+) -> bool:
+    if stream.is_web_public:
+        return True
+
+    if is_subscribed:
+        return True
+
+    if user_profile.is_guest:
+        return False
+
+    if stream.is_public():
+        return True
+
+    if user_profile.is_realm_admin:
+        return 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 has_metadata_access_to_channel_via_groups(
+        user_profile,
+        user_group_membership_details.user_recursive_group_ids,
+        stream.can_administer_channel_group_id,
+        stream.can_add_subscribers_group_id,
+        stream.can_subscribe_group_id,
+    ):
+        return True
+
+    return False
+
+
 def user_has_content_access(
     user_profile: UserProfile,
     stream: Stream,
@@ -1110,6 +1149,36 @@ def can_administer_accessible_channel(channel: Stream, user_profile: UserProfile
     )
 
 
+def get_metadata_access_streams(
+    user_profile: UserProfile,
+    streams: Collection[Stream],
+    user_group_membership_details: UserGroupMembershipDetails,
+) -> list[Stream]:
+    if len(streams) == 0:
+        return []
+
+    recipient_ids = [stream.recipient_id for stream in streams]
+    subscribed_recipient_ids = set(
+        Subscription.objects.filter(
+            user_profile=user_profile, recipient_id__in=recipient_ids, active=True
+        ).values_list("recipient_id", flat=True)
+    )
+
+    metadata_access_streams: list[Stream] = []
+
+    for stream in streams:
+        is_subscribed = stream.recipient_id in subscribed_recipient_ids
+        if user_has_metadata_access(
+            user_profile,
+            stream,
+            user_group_membership_details,
+            is_subscribed=is_subscribed,
+        ):
+            metadata_access_streams.append(stream)
+
+    return metadata_access_streams
+
+
 @dataclass
 class StreamsCategorizedByPermissionsForAddingSubscribers:
     authorized_streams: list[Stream]
@@ -1455,9 +1524,6 @@ def get_streams_for_user(
     include_owner_subscribed: bool = False,
     include_can_access_content: bool = False,
 ) -> list[Stream]:
-    if include_all and not user_profile.is_realm_admin:
-        raise JsonableError(_("User not authorized for this query"))
-
     include_public = include_public and user_profile.can_access_public_streams()
 
     # Start out with all streams in the realm.
@@ -1469,9 +1535,18 @@ def get_streams_for_user(
         query = query.filter(deactivated=False)
 
     if include_all:
-        streams = query.only(
-            *Stream.API_FIELDS, "can_send_message_group", "can_send_message_group__named_user_group"
+        all_streams = list(
+            query.only(
+                *Stream.API_FIELDS,
+                "can_send_message_group",
+                "can_send_message_group__named_user_group",
+                # Both of these fields are need for get_content_access_streams.
+                "is_in_zephyr_realm",
+                "recipient_id",
+            )
         )
+        user_group_membership_details = UserGroupMembershipDetails(user_recursive_group_ids=None)
+        return get_metadata_access_streams(user_profile, all_streams, user_group_membership_details)
     else:
         # We construct a query as the or (|) of the various sources
         # this user requested streams from.

zerver/openapi/python_examples.py

@@ -826,12 +826,6 @@ def get_user_groups(client: Client) -> int:
     return leadership_user_group["id"]
 
 
-def test_user_not_authorized_error(nonadmin_client: Client) -> None:
-    result = nonadmin_client.get_streams(include_all=True)
-    assert_error_response(result)
-    validate_against_openapi_schema(result, "/rest-error-handling", "post", "400")
-
-
 @openapi_test_function("/streams/{stream_id}/members:get")
 def get_subscribers(client: Client) -> None:
     user_ids = [11, 25]
@@ -1881,7 +1875,6 @@ def test_streams(client: Client, nonadmin_client: Client) -> None:
     add_default_stream(client)
     remove_default_stream(client)
 
-    test_user_not_authorized_error(nonadmin_client)
     test_authorization_errors_fatal(client, nonadmin_client)
 
 

zerver/openapi/zulip.yaml

@@ -20548,23 +20548,6 @@ paths:
                             },
                           ],
                       }
-        "400":
-          description: Bad request.
-          content:
-            application/json:
-              schema:
-                allOf:
-                  - $ref: "#/components/schemas/CodedError"
-                  - example:
-                      {
-                        "code": "BAD_REQUEST",
-                        "msg": "User not authorized for this query",
-                        "result": "error",
-                      }
-                    description: |
-                      An example JSON response for when the user is not authorized to use the
-                      `include_all` parameter (i.e. because they are not an organization
-                      administrator):
   /streams/{stream_id}:
     get:
       operationId: get-stream-by-id

zerver/tests/test_subs.py

@@ -7062,21 +7062,100 @@ class GetStreamsTest(ZulipTestCase):
 
         self.assertEqual(actual, expected)
 
-    def test_all_active_streams_api(self) -> None:
+    def test_all_streams_api(self) -> None:
         url = "/api/v1/streams"
         data = {"include_all": "true"}
         backward_compatible_data = {"include_all_active": "true"}
 
-        # Check non-superuser can't use include_all
+        # Normal user should be able to make this request and get all
+        # the streams they have metadata access to.
         normal_user = self.example_user("cordelia")
-        result = self.api_get(normal_user, url, data)
-        self.assertEqual(result.status_code, 400)
+        realm = normal_user.realm
+        normal_user_group = self.create_or_update_anonymous_group_for_setting([normal_user], [])
 
-        # Realm admin users can see all active streams.
+        private_stream_1 = self.make_stream("private_stream_1", realm=realm, invite_only=True)
+        private_stream_2 = self.make_stream("private_stream_2", realm=realm, invite_only=True)
+        private_stream_3 = self.make_stream("private_stream_3", realm=realm, invite_only=True)
+        self.make_stream("private_stream_4", realm=realm, invite_only=True)
+        deactivated_public_stream = self.make_stream(
+            "deactivated_public_stream", realm=realm, invite_only=False
+        )
+        do_deactivate_stream(deactivated_public_stream, acting_user=normal_user)
+
+        self.subscribe(normal_user, private_stream_1.name)
+        do_change_stream_group_based_setting(
+            private_stream_2,
+            "can_add_subscribers_group",
+            normal_user_group,
+            acting_user=normal_user,
+        )
+        do_change_stream_group_based_setting(
+            private_stream_3,
+            "can_administer_channel_group",
+            normal_user_group,
+            acting_user=normal_user,
+        )
+
+        result_stream_names: list[str] = [
+            stream.name
+            for stream in Stream.objects.filter(realm=realm, invite_only=False, deactivated=False)
+        ]
+        result_stream_names.extend(
+            [private_stream_1.name, private_stream_2.name, private_stream_3.name]
+        )
+        with self.assert_database_query_count(8):
+            result = self.api_get(normal_user, url, data)
+        json = self.assert_json_success(result)
+        self.assertEqual(sorted(s["name"] for s in json["streams"]), sorted(result_stream_names))
+
+        # Normal user should be able to make this request and get all
+        # the streams they have metadata access to.
+        guest_user = self.example_user("polonius")
+        guest_user_group = self.create_or_update_anonymous_group_for_setting([guest_user], [])
+
+        self.subscribe(guest_user, private_stream_1.name)
+        self.subscribe(guest_user, "design")
+        do_change_stream_group_based_setting(
+            private_stream_2,
+            "can_add_subscribers_group",
+            guest_user_group,
+            acting_user=normal_user,
+        )
+        do_change_stream_group_based_setting(
+            get_stream("Rome", realm),
+            "can_add_subscribers_group",
+            guest_user_group,
+            acting_user=normal_user,
+        )
+        do_change_stream_group_based_setting(
+            private_stream_3,
+            "can_administer_channel_group",
+            guest_user_group,
+            acting_user=normal_user,
+        )
+        do_change_stream_group_based_setting(
+            get_stream("Denmark", realm),
+            "can_administer_channel_group",
+            guest_user_group,
+            acting_user=normal_user,
+        )
+
+        # Guest user should not gain metadata access to a channel via
+        # `can_add_subscribers_group` or `can_administer_channel_group`
+        # since `allow_everyone_group` if false for both of those groups.
+        result_stream_names = ["Verona", "private_stream_1", "design", "Rome"]
+        with self.assert_database_query_count(7):
+            result = self.api_get(guest_user, url, data)
+        json = self.assert_json_success(result)
+        self.assertEqual(sorted(s["name"] for s in json["streams"]), sorted(result_stream_names))
+
+        # Realm admin users can see all active streams if
+        # `exclude_archived` is not set.
         admin_user = self.example_user("iago")
         self.assertTrue(admin_user.is_realm_admin)
 
-        result = self.api_get(admin_user, url, data)
+        with self.assert_database_query_count(7):
+            result = self.api_get(admin_user, url, data)
         json = self.assert_json_success(result)
 
         backward_compatible_result = self.api_get(admin_user, url, backward_compatible_data)
@@ -7088,21 +7167,42 @@ class GetStreamsTest(ZulipTestCase):
         self.assertIsInstance(json["streams"], list)
 
         stream_names = {s["name"] for s in json["streams"]}
-
+        result_stream_names = [
+            stream.name for stream in Stream.objects.filter(realm=realm, deactivated=False)
+        ]
         self.assertEqual(
-            stream_names,
-            {
-                "Venice",
-                "Denmark",
-                "Scotland",
-                "Verona",
-                "Rome",
-                "core team",
-                "Zulip",
-                "sandbox",
-            },
+            sorted(stream_names),
+            sorted(result_stream_names),
         )
 
+        # Realm admin users can see all streams if `exclude_archived`
+        # is set to false.
+        data = {"include_all": "true", "exclude_archived": "false"}
+        with self.assert_database_query_count(7):
+            result = self.api_get(admin_user, url, data)
+        json = self.assert_json_success(result)
+        stream_names = {s["name"] for s in json["streams"]}
+        result_stream_names = [stream.name for stream in Stream.objects.filter(realm=realm)]
+        self.assertEqual(
+            sorted(stream_names),
+            sorted(result_stream_names),
+        )
+
+        # This case will not happen in practice, we are adding this
+        # test block to add coverage for the case where
+        # `get_metadata_access_streams` returns an empty list without
+        # query if an empty list of streams is passed to it.
+        all_active_streams = Stream.objects.filter(realm=realm, deactivated=False)
+        for stream in all_active_streams:
+            do_deactivate_stream(stream, acting_user=None)
+
+        data = {"include_all": "true"}
+        with self.assert_database_query_count(3):
+            result = self.api_get(admin_user, url, data)
+        json = self.assert_json_success(result)
+        stream_names = {s["name"] for s in json["streams"]}
+        self.assertEqual(stream_names, set())
+
     def test_public_streams_api(self) -> None:
         """
         Ensure that the query we use to get public streams successfully returns