Skip to content
This repository has been archived by the owner on Apr 26, 2024. It is now read-only.

Make POST /_matrix/client/v3/rooms/{roomId}/report/{eventId} endpoint return 404 if event exists, but the user lacks access #15300

Merged
merged 4 commits into from
Mar 21, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions changelog.d/15300.bugfix
Original file line number Diff line number Diff line change
@@ -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).
12 changes: 12 additions & 0 deletions docs/upgrade.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
16 changes: 11 additions & 5 deletions synapse/rest/client/report_event.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@
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
Expand Down Expand Up @@ -62,12 +62,18 @@ async def on_POST(
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."
Comment on lines +75 to +76
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm failing to spot a change here, was this your editor just wrapping it automatically or something?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh yes. This was just me wrapping it as it went one whole character over 80, and that was bugging me.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I usually let black handle this -- note that we have it configured to wrap at 88 characters, not 80.

)

await self.store.add_event_report(
Expand Down
1 change: 0 additions & 1 deletion synapse/storage/databases/main/events_worker.py
Original file line number Diff line number Diff line change
Expand Up @@ -805,7 +805,6 @@ async def get_missing_events_from_cache_or_db() -> Dict[
# 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.
Expand Down
37 changes: 37 additions & 0 deletions tests/rest/client/test_report_event.py
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,43 @@ def test_cannot_report_nonexistent_event(self) -> None:
)
self.assertEqual(404, channel.code, 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
Expand Down