summary refs log tree commit diff
diff options
context:
space:
mode:
authorAndrew Morgan <1342360+anoadragon453@users.noreply.github.com>2023-03-21 13:24:03 +0000
committerGitHub <noreply@github.com>2023-03-21 13:24:03 +0000
commitec9224bf9a7bebb6c429ef45e0d1a293f0986836 (patch)
treed140e37a1f09dabca17f85377639cacfeae326e2
parentMake `EventHandler.get_event` return `None` when the requested event is not f... (diff)
downloadsynapse-ec9224bf9a7bebb6c429ef45e0d1a293f0986836.tar.xz
Make `POST /_matrix/client/v3/rooms/{roomId}/report/{eventId}` endpoint return 404 if event exists, but the user lacks access (#15300)
-rw-r--r--changelog.d/15300.bugfix1
-rw-r--r--docs/upgrade.md12
-rw-r--r--synapse/rest/client/report_event.py16
-rw-r--r--synapse/storage/databases/main/events_worker.py1
-rw-r--r--tests/rest/client/test_report_event.py37
5 files changed, 61 insertions, 6 deletions
diff --git a/changelog.d/15300.bugfix b/changelog.d/15300.bugfix
new file mode 100644
index 0000000000..8f29b08444
--- /dev/null
+++ b/changelog.d/15300.bugfix
@@ -0,0 +1 @@
+Fix a bug in which the [`POST /_matrix/client/v3/rooms/{roomId}/report/{eventId}`](https://spec.matrix.org/v1.6/client-server-api/#post_matrixclientv3roomsroomidreporteventid) endpoint would return the wrong error if the user did not have permission to view the event. This aligns Synapse's implementation with [MSC2249](https://github.com/matrix-org/matrix-spec-proposals/pull/2249).
\ No newline at end of file
diff --git a/docs/upgrade.md b/docs/upgrade.md
index f06e874054..f14444a400 100644
--- a/docs/upgrade.md
+++ b/docs/upgrade.md
@@ -88,6 +88,18 @@ process, for example:
     dpkg -i matrix-synapse-py3_1.3.0+stretch1_amd64.deb
     ```
 
+# Upgrading to v1.80.0
+
+## Reporting events error code change
+
+Before this update, the
+[`POST /_matrix/client/v3/rooms/{roomId}/report/{eventId}`](https://spec.matrix.org/v1.6/client-server-api/#post_matrixclientv3roomsroomidreporteventid)
+endpoint would return a `403` if a user attempted to report an event that they did not have access to.
+This endpoint will now return a `404` in this case instead.
+
+Clients that implement event reporting should check that their error handling code will handle this
+change.
+
 # Upgrading to v1.79.0
 
 ## The `on_threepid_bind` module callback method has been deprecated
diff --git a/synapse/rest/client/report_event.py b/synapse/rest/client/report_event.py
index 9be5860221..ac1a63ca27 100644
--- a/synapse/rest/client/report_event.py
+++ b/synapse/rest/client/report_event.py
@@ -16,7 +16,7 @@ import logging
 from http import HTTPStatus
 from typing import TYPE_CHECKING, Tuple
 
-from synapse.api.errors import Codes, NotFoundError, SynapseError
+from synapse.api.errors import AuthError, Codes, NotFoundError, SynapseError
 from synapse.http.server import HttpServer
 from synapse.http.servlet import RestServlet, parse_json_object_from_request
 from synapse.http.site import SynapseRequest
@@ -62,12 +62,18 @@ class ReportEventRestServlet(RestServlet):
                 Codes.BAD_JSON,
             )
 
-        event = await self._event_handler.get_event(
-            requester.user, room_id, event_id, show_redacted=False
-        )
+        try:
+            event = await self._event_handler.get_event(
+                requester.user, room_id, event_id, show_redacted=False
+            )
+        except AuthError:
+            # The event exists, but this user is not allowed to access this event.
+            event = None
+
         if event is None:
             raise NotFoundError(
-                "Unable to report event: it does not exist or you aren't able to see it."
+                "Unable to report event: "
+                "it does not exist or you aren't able to see it."
             )
 
         await self.store.add_event_report(
diff --git a/synapse/storage/databases/main/events_worker.py b/synapse/storage/databases/main/events_worker.py
index 20b7a68362..0cf46626d2 100644
--- a/synapse/storage/databases/main/events_worker.py
+++ b/synapse/storage/databases/main/events_worker.py
@@ -805,7 +805,6 @@ class EventsWorkerStore(SQLBaseStore):
                 # the events have been redacted, and if so pulling the redaction event
                 # out of the database to check it.
                 #
-                missing_events = {}
                 try:
                     # Try to fetch from any external cache. We already checked the
                     # in-memory cache above.
diff --git a/tests/rest/client/test_report_event.py b/tests/rest/client/test_report_event.py
index 1a8ab067a9..b88f1d61a0 100644
--- a/tests/rest/client/test_report_event.py
+++ b/tests/rest/client/test_report_event.py
@@ -90,6 +90,43 @@ class ReportEventTestCase(unittest.HomeserverTestCase):
             msg=channel.result["body"],
         )
 
+    def test_cannot_report_event_if_not_in_room(self) -> None:
+        """
+        Tests that we don't accept event reports for events that exist, but for which
+        the reporter should not be able to view (because they are not in the room).
+        """
+        # Have the admin user create a room (the "other" user will not join this room).
+        new_room_id = self.helper.create_room_as(tok=self.admin_user_tok)
+
+        # Have the admin user send an event in this room.
+        response = self.helper.send_event(
+            new_room_id,
+            "m.room.message",
+            content={
+                "msgtype": "m.text",
+                "body": "This event has some bad words in it! Flip!",
+            },
+            tok=self.admin_user_tok,
+        )
+        event_id = response["event_id"]
+
+        # Have the "other" user attempt to report it. Perhaps they found the event ID
+        # in a screenshot or something...
+        channel = self.make_request(
+            "POST",
+            f"rooms/{new_room_id}/report/{event_id}",
+            {"reason": "I'm not in this room but I have opinions anyways!"},
+            access_token=self.other_user_tok,
+        )
+
+        # The "other" user is not in the room, so their report should be rejected.
+        self.assertEqual(404, channel.code, msg=channel.result["body"])
+        self.assertEqual(
+            "Unable to report event: it does not exist or you aren't able to see it.",
+            channel.json_body["error"],
+            msg=channel.result["body"],
+        )
+
     def _assert_status(self, response_status: int, data: JsonDict) -> None:
         channel = self.make_request(
             "POST", self.report_path, data, access_token=self.other_user_tok