From b6aef593347924d39b4ff8b07e375eb656001545 Mon Sep 17 00:00:00 2001 From: Andrew Morgan <1342360+anoadragon453@users.noreply.github.com> Date: Tue, 21 Mar 2023 13:23:47 +0000 Subject: [PATCH 1/3] Make `EventHandler.get_event` return `None` when the requested event is not found (#15298) --- changelog.d/15298.bugfix | 1 + synapse/handlers/events.py | 9 +++++---- tests/rest/client/test_report_event.py | 5 +++++ 3 files changed, 11 insertions(+), 4 deletions(-) create mode 100644 changelog.d/15298.bugfix diff --git a/changelog.d/15298.bugfix b/changelog.d/15298.bugfix new file mode 100644 index 0000000000..8f29b08444 --- /dev/null +++ b/changelog.d/15298.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/synapse/handlers/events.py b/synapse/handlers/events.py index 68c07f0265..33359f6ed7 100644 --- a/synapse/handlers/events.py +++ b/synapse/handlers/events.py @@ -159,15 +159,16 @@ class EventHandler: Returns: An event, or None if there is no event matching this ID. Raises: - SynapseError if there was a problem retrieving this event, or - AuthError if the user does not have the rights to inspect this - event. + AuthError: if the user does not have the rights to inspect this event. """ redact_behaviour = ( EventRedactBehaviour.as_is if show_redacted else EventRedactBehaviour.redact ) event = await self.store.get_event( - event_id, check_room_id=room_id, redact_behaviour=redact_behaviour + event_id, + check_room_id=room_id, + redact_behaviour=redact_behaviour, + allow_none=True, ) if not event: diff --git a/tests/rest/client/test_report_event.py b/tests/rest/client/test_report_event.py index 1250685d39..1a8ab067a9 100644 --- a/tests/rest/client/test_report_event.py +++ b/tests/rest/client/test_report_event.py @@ -84,6 +84,11 @@ class ReportEventTestCase(unittest.HomeserverTestCase): access_token=self.other_user_tok, ) 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( From ec9224bf9a7bebb6c429ef45e0d1a293f0986836 Mon Sep 17 00:00:00 2001 From: Andrew Morgan <1342360+anoadragon453@users.noreply.github.com> Date: Tue, 21 Mar 2023 13:24:03 +0000 Subject: [PATCH 2/3] Make `POST /_matrix/client/v3/rooms/{roomId}/report/{eventId}` endpoint return 404 if event exists, but the user lacks access (#15300) --- changelog.d/15300.bugfix | 1 + docs/upgrade.md | 12 ++++++ synapse/rest/client/report_event.py | 16 +++++--- .../storage/databases/main/events_worker.py | 1 - tests/rest/client/test_report_event.py | 37 +++++++++++++++++++ 5 files changed, 61 insertions(+), 6 deletions(-) create mode 100644 changelog.d/15300.bugfix 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 From 96bcc5d9028e745df2f708c92b15e4e5bfc91328 Mon Sep 17 00:00:00 2001 From: Shay Date: Tue, 21 Mar 2023 10:49:25 -0700 Subject: [PATCH 3/3] Revert "check sqlite database file exists before porting/#14692" (#15301) --- changelog.d/15301.bugfix | 3 +++ synapse/_scripts/synapse_port_db.py | 2 +- 2 files changed, 4 insertions(+), 1 deletion(-) create mode 100644 changelog.d/15301.bugfix diff --git a/changelog.d/15301.bugfix b/changelog.d/15301.bugfix new file mode 100644 index 0000000000..c8f3628d34 --- /dev/null +++ b/changelog.d/15301.bugfix @@ -0,0 +1,3 @@ +Fix a bug introduced in Synapse 1.75.0rc1 where the [SQLite port_db script](https://matrix-org.github.io/synapse/latest/postgres.html#porting-from-sqlite) +would fail to open the SQLite database. + diff --git a/synapse/_scripts/synapse_port_db.py b/synapse/_scripts/synapse_port_db.py index 2c9cbf8b27..78d76d38ad 100755 --- a/synapse/_scripts/synapse_port_db.py +++ b/synapse/_scripts/synapse_port_db.py @@ -1329,7 +1329,7 @@ def main() -> None: sqlite_config = { "name": "sqlite3", "args": { - "database": "file:{}?mode=rw".format(args.sqlite_database), + "database": args.sqlite_database, "cp_min": 1, "cp_max": 1, "check_same_thread": False,