tornado: Raise the same error for nonexistent and unauthorized queues.

Signed-off-by: Anders Kaseorg <anders@zulip.com>

zerver/tests/test_event_queue.py

@@ -13,8 +13,8 @@ from zerver.lib.user_groups import create_user_group, remove_user_from_user_grou
 from zerver.models import Recipient, Stream, Subscription, UserProfile, get_stream
 from zerver.tornado.event_queue import (
     ClientDescriptor,
+    access_client_descriptor,
     allocate_client_descriptor,
-    get_client_descriptor,
     maybe_enqueue_notifications,
     missedmessage_hook,
     persistent_queue_filename,
@@ -173,7 +173,7 @@ class MissedMessageNotificationsTest(ZulipTestCase):
             )
             self.assert_json_success(result)
             queue_id = orjson.loads(result.content)["queue_id"]
-            return get_client_descriptor(queue_id)
+            return access_client_descriptor(user.id, queue_id)
 
         def destroy_event_queue(user: UserProfile, queue_id: str) -> None:
             result = self.tornado_call(cleanup_event_queue, user, {"queue_id": queue_id})

zerver/tests/test_event_system.py

@@ -43,6 +43,7 @@ from zerver.tornado.event_queue import (
     process_message_event,
     send_restart_events,
 )
+from zerver.tornado.exceptions import BadEventQueueIdError
 from zerver.tornado.views import get_events, get_events_backend
 from zerver.views.events_register import (
     _default_all_public_streams,
@@ -453,6 +454,52 @@ class GetEventsTest(ZulipTestCase):
         self.assertEqual(message["content"], "<p><strong>hello</strong></p>")
         self.assertEqual(message["avatar_url"], None)
 
+    def test_bogus_queue_id(self) -> None:
+        user = self.example_user("hamlet")
+
+        with self.assertRaises(BadEventQueueIdError):
+            self.tornado_call(
+                get_events,
+                user,
+                {
+                    "queue_id": "hamster",
+                    "user_client": "website",
+                    "last_event_id": -1,
+                    "dont_block": orjson.dumps(True).decode(),
+                },
+            )
+
+    def test_wrong_user_queue_id(self) -> None:
+        user = self.example_user("hamlet")
+        wrong_user = self.example_user("othello")
+
+        result = self.tornado_call(
+            get_events,
+            user,
+            {
+                "apply_markdown": orjson.dumps(True).decode(),
+                "client_gravatar": orjson.dumps(True).decode(),
+                "event_types": orjson.dumps(["message"]).decode(),
+                "user_client": "website",
+                "dont_block": orjson.dumps(True).decode(),
+            },
+        )
+        self.assert_json_success(result)
+        queue_id = orjson.loads(result.content)["queue_id"]
+
+        with self.assertLogs(level="WARNING") as cm, self.assertRaises(BadEventQueueIdError):
+            self.tornado_call(
+                get_events,
+                wrong_user,
+                {
+                    "queue_id": queue_id,
+                    "user_client": "website",
+                    "last_event_id": -1,
+                    "dont_block": orjson.dumps(True).decode(),
+                },
+            )
+        self.assertIn("not authorized for queue", cm.output[0])
+
 
 class FetchInitialStateDataTest(ZulipTestCase):
     # Non-admin users don't have access to all bots

zerver/tornado/event_queue.py

@@ -442,11 +442,19 @@ def add_client_gc_hook(hook: Callable[[int, ClientDescriptor, bool], None]) -> N
     gc_hooks.append(hook)
 
 
-def get_client_descriptor(queue_id: str) -> ClientDescriptor:
+def access_client_descriptor(user_id: int, queue_id: str) -> ClientDescriptor:
-    try:
+    client = clients.get(queue_id)
-        return clients[queue_id]
+    if client is not None:
-    except KeyError:
+        if user_id == client.user_profile_id:
-        raise BadEventQueueIdError(queue_id)
+            return client
+        logging.warning(
+            "User %d is not authorized for queue %s (%d via %s)",
+            user_id,
+            queue_id,
+            client.user_profile_id,
+            client.current_client_name,
+        )
+    raise BadEventQueueIdError(queue_id)
 
 
 def get_client_descriptors_for_user(user_profile_id: int) -> List[ClientDescriptor]:
@@ -644,9 +652,7 @@ def fetch_events(
         else:
             if last_event_id is None:
                 raise JsonableError(_("Missing 'last_event_id' argument"))
-            client = get_client_descriptor(queue_id)
+            client = access_client_descriptor(user_profile_id, queue_id)
-            if user_profile_id != client.user_profile_id:
-                raise JsonableError(_("You are not authorized to get events from this queue"))
             if (
                 client.event_queue.newest_pruned_id is not None
                 and last_event_id < client.event_queue.newest_pruned_id

zerver/tornado/views.py

@@ -19,8 +19,7 @@ from zerver.lib.validator import (
     to_non_negative_int,
 )
 from zerver.models import Client, UserProfile, get_client, get_user_profile_by_id
-from zerver.tornado.event_queue import fetch_events, get_client_descriptor, process_notification
+from zerver.tornado.event_queue import access_client_descriptor, fetch_events, process_notification
-from zerver.tornado.exceptions import BadEventQueueIdError
 
 P = ParamSpec("P")
 T = TypeVar("T")
@@ -46,11 +45,7 @@ def notify(
 def cleanup_event_queue(
     request: HttpRequest, user_profile: UserProfile, queue_id: str = REQ()
 ) -> HttpResponse:
-    client = get_client_descriptor(str(queue_id))
+    client = access_client_descriptor(user_profile.id, queue_id)
-    if client is None:
-        raise BadEventQueueIdError(queue_id)
-    if user_profile.id != client.user_profile_id:
-        raise JsonableError(_("You are not authorized to access this queue"))
     log_data = RequestNotes.get_notes(request).log_data
     assert log_data is not None
     log_data["extra"] = f"[{queue_id}]"