events: Clean up logic for spectator events_register parameters.

Unfortunately, doing so requires forking common API documentation
text, since we're not making any changes to other endpoints that don't
allow unauthenticated requests at all.

Follow-up on #21995.

templates/zerver/api/changelog.md

@@ -20,6 +20,13 @@ format used by the Zulip server that they are interacting with.
 
 ## Changes in Zulip 6.0
 
+**Feature level 149**
+
+* [`POST /register`](/api/register-queue): The `client_gravatar` and
+  `include_subscribers` parameters now return an error for
+  [unauthenticated requests](/help/public-access-option) if an
+  unsupported value is requested by the client.
+
 **Feature level 148**
 
 * [`POST /users/me/status`](/api/update-status):

version.py

@@ -33,7 +33,7 @@ DESKTOP_WARNING_VERSION = "5.4.3"
 # Changes should be accompanied by documentation explaining what the
 # new level means in templates/zerver/api/changelog.md, as well as
 # "**Changes**" entries in the endpoint's documentation in `zulip.yaml`.
-API_FEATURE_LEVEL = 148
+API_FEATURE_LEVEL = 149
 
 # Bump the minor PROVISION_VERSION to indicate that folks should provision
 # only when going from an old version of the code to a newer version. Bump

zerver/lib/events.py

@@ -1396,27 +1396,25 @@ def do_events_register(
         event_types_set = None
 
     if user_profile is None:
-        # TODO: Unify this with the below code path once if/when we
-        # support requesting an event queue for spectators.
-        #
-        # Doing so likely has a prerequisite of making this function's
-        # caller enforce client_gravatar=False,
-        # include_subscribers=False and include_streams=False.
+        # TODO: Unify the two fetch_initial_state_data code paths.
+        assert client_gravatar is False
+        assert include_subscribers is False
+        assert include_streams is False
         ret = fetch_initial_state_data(
             user_profile,
             realm=realm,
             event_types=event_types_set,
             queue_id=None,
             # Force client_gravatar=False for security reasons.
-            client_gravatar=False,
+            client_gravatar=client_gravatar,
             user_avatar_url_field_optional=user_avatar_url_field_optional,
             user_settings_object=user_settings_object,
             # slim_presence is a noop, because presence is not included.
             slim_presence=True,
             # Force include_subscribers=False for security reasons.
-            include_subscribers=False,
+            include_subscribers=include_subscribers,
             # Force include_streams=False for security reasons.
-            include_streams=False,
+            include_streams=include_streams,
             spectator_requested_language=spectator_requested_language,
         )
 

zerver/openapi/zulip.yaml

@@ -9551,7 +9551,50 @@ paths:
             type: boolean
             default: false
           example: true
-        - $ref: "#/components/parameters/ClientGravatar"
+        - name: client_gravatar
+          in: query
+          description: |
+            Whether the client supports computing gravatars URLs. If
+            enabled, `avatar_url` will be included in the response only
+            if there is a Zulip avatar, and will be `null` for users who
+            are using gravatar as their avatar. This option
+            significantly reduces the compressed size of user data,
+            since gravatar URLs are long, random strings and thus do not
+            compress well. The `client_gravatar` field is set to `true` if
+            clients can compute their own gravatars.
+
+            The default value is `true` for authenticated requests and
+            `false` for [unauthenticated
+            requests](/help/public-access-option). Passing `true` in
+            an unauthenticated request is an error.
+
+            **Changes**: Before Zulip 6.0 (feature level 149), this
+            parameter was silently ignored and processed as though it
+            were `false` in unauthenticated requests.
+          schema:
+            type: boolean
+          example: false
+        - name: include_subscribers
+          in: query
+          description: |
+            Whether each returned stream object should include a `subscribers`
+            field containing a list of the user IDs of its subscribers.
+
+            (This may be significantly slower in organizations with
+            thousands of users subscribed to many streams.)
+
+            Passing `true` in an [unauthenticated
+            request](/help/public-access-option) is an error.
+
+            **Changes**: Before Zulip 6.0 (feature level 149), this
+            parameter was silently ignored and processed as though it
+            were `false` in unauthenticated requests.
+
+            New in Zulip 2.1.0.
+          schema:
+            type: boolean
+            default: false
+          example: true
         - name: slim_presence
           in: query
           description: |
@@ -9565,7 +9608,6 @@ paths:
           example: true
         - $ref: "#/components/parameters/Event_types"
         - $ref: "#/components/parameters/AllPublicStreams"
-        - $ref: "#/components/parameters/IncludeSubscribers"
         - name: client_capabilities
           in: query
           description: |

zerver/tests/test_event_system.py

@@ -151,18 +151,38 @@ class EventsEndpointTest(ZulipTestCase):
         # Verify that POST /register works for spectators, but not for
         # normal users.
         with self.settings(WEB_PUBLIC_STREAMS_ENABLED=False):
-            result = self.client_post("/json/register", dict())
+            result = self.client_post("/json/register")
             self.assert_json_error(
                 result,
                 "Not logged in: API authentication or user session required",
                 status_code=401,
             )
 
-        result = self.client_post("/json/register", dict())
+        result = self.client_post("/json/register")
         result_dict = self.assert_json_success(result)
         self.assertEqual(result_dict["queue_id"], None)
         self.assertEqual(result_dict["realm_uri"], "http://zulip.testserver")
 
+        result = self.client_post("/json/register")
+        self.assertEqual(result.status_code, 200)
+
+        result = self.client_post("/json/register", dict(client_gravatar="false"))
+        self.assertEqual(result.status_code, 200)
+
+        result = self.client_post("/json/register", dict(client_gravatar="true"))
+        self.assert_json_error(
+            result,
+            "Invalid 'client_gravatar' parameter for anonymous request",
+            status_code=400,
+        )
+
+        result = self.client_post("/json/register", dict(include_subscribers="true"))
+        self.assert_json_error(
+            result,
+            "Invalid 'include_subscribers' parameter for anonymous request",
+            status_code=400,
+        )
+
     def test_events_register_endpoint_all_public_streams_access(self) -> None:
         guest_user = self.example_user("polonius")
         normal_user = self.example_user("hamlet")

zerver/views/events_register.py

@@ -40,7 +40,9 @@ def events_register_backend(
     request: HttpRequest,
     maybe_user_profile: Union[UserProfile, AnonymousUser],
     apply_markdown: bool = REQ(default=False, json_validator=check_bool),
-    client_gravatar: bool = REQ(default=True, json_validator=check_bool),
+    client_gravatar_raw: Optional[bool] = REQ(
+        "client_gravatar", default=None, json_validator=check_bool
+    ),
     slim_presence: bool = REQ(default=False, json_validator=check_bool),
     all_public_streams: Optional[bool] = REQ(default=None, json_validator=check_bool),
     include_subscribers: bool = REQ(default=False, json_validator=check_bool),
@@ -74,11 +76,17 @@ def events_register_backend(
     ),
     queue_lifespan_secs: int = REQ(json_validator=check_int, default=0, documentation_pending=True),
 ) -> HttpResponse:
+    if client_gravatar_raw is None:
+        client_gravatar = maybe_user_profile.is_authenticated
+    else:
+        client_gravatar = client_gravatar_raw
+
     if maybe_user_profile.is_authenticated:
         user_profile = maybe_user_profile
         spectator_requested_language = None
         assert isinstance(user_profile, UserProfile)
         realm = user_profile.realm
+        include_streams = True
 
         if all_public_streams and not user_profile.can_access_public_streams():
             raise JsonableError(_("User not authorized for this query"))
@@ -88,15 +96,26 @@ def events_register_backend(
     else:
         user_profile = None
         realm = get_valid_realm_from_request(request)
+        if not realm.allow_web_public_streams_access():
+            raise MissingAuthenticationError()
+
+        # These parameters must be false for anonymous requests.
+        if client_gravatar:
+            raise JsonableError(
+                _("Invalid '{}' parameter for anonymous request").format("client_gravatar")
+            )
+        if include_subscribers:
+            raise JsonableError(
+                _("Invalid '{}' parameter for anonymous request").format("include_subscribers")
+            )
+
         # Language set by spectator to be passed down to clients as user_settings.
         spectator_requested_language = request.COOKIES.get(
             settings.LANGUAGE_COOKIE_NAME, realm.default_language
         )
 
-        if not realm.allow_web_public_streams_access():
-            raise MissingAuthenticationError()
-
         all_public_streams = False
+        include_streams = False
 
     if client_capabilities is None:
         client_capabilities = {}
@@ -116,6 +135,7 @@ def events_register_backend(
         all_public_streams,
         narrow=narrow,
         include_subscribers=include_subscribers,
+        include_streams=include_streams,
         client_capabilities=client_capabilities,
         fetch_event_types=fetch_event_types,
         spectator_requested_language=spectator_requested_language,