diff options
author | Erik Johnston <erik@matrix.org> | 2023-03-21 09:13:43 +0000 |
---|---|---|
committer | GitHub <noreply@github.com> | 2023-03-21 09:13:43 +0000 |
commit | 827f198177c4cf547b9d2d1eed41411e945fc199 (patch) | |
tree | 385e8599e006bf8b18b737d4ea5005151ec4eff5 | |
parent | Separate HTTP preview code and URL previewer. (#15269) (diff) | |
download | synapse-827f198177c4cf547b9d2d1eed41411e945fc199.tar.xz |
Fix error when sending message into deleted room. (#15235)
When a room is deleted in Synapse we remove the event forward extremities in the room, so if (say a bot) tries to send a message into the room we error out due to not being able to calculate prev events for the new event *before* we check if the sender is in the room. Fixes #8094
-rw-r--r-- | changelog.d/15235.bugfix | 1 | ||||
-rw-r--r-- | synapse/handlers/message.py | 17 | ||||
-rw-r--r-- | tests/rest/admin/test_room.py | 15 |
3 files changed, 31 insertions, 2 deletions
diff --git a/changelog.d/15235.bugfix b/changelog.d/15235.bugfix new file mode 100644 index 0000000000..e6a6bb1b9d --- /dev/null +++ b/changelog.d/15235.bugfix @@ -0,0 +1 @@ +Fix long-standing error when sending message into deleted room. diff --git a/synapse/handlers/message.py b/synapse/handlers/message.py index da129ec16a..4c75433a63 100644 --- a/synapse/handlers/message.py +++ b/synapse/handlers/message.py @@ -987,10 +987,11 @@ class EventCreationHandler: # a situation where event persistence can't keep up, causing # extremities to pile up, which in turn leads to state resolution # taking longer. - async with self.limiter.queue(event_dict["room_id"]): + room_id = event_dict["room_id"] + async with self.limiter.queue(room_id): if txn_id: event = await self.get_event_from_transaction( - requester, txn_id, event_dict["room_id"] + requester, txn_id, room_id ) if event: # we know it was persisted, so must have a stream ordering @@ -1000,6 +1001,18 @@ class EventCreationHandler: event.internal_metadata.stream_ordering, ) + # If we don't have any prev event IDs specified then we need to + # check that the host is in the room (as otherwise populating the + # prev events will fail), at which point we may as well check the + # local user is in the room. + if not prev_event_ids: + user_id = requester.user.to_string() + is_user_in_room = await self.store.check_local_user_in_room( + user_id, room_id + ) + if not is_user_in_room: + raise AuthError(403, f"User {user_id} not in room {room_id}") + # Try several times, it could fail with PartialStateConflictError # in handle_new_client_event, cf comment in except block. max_retries = 5 diff --git a/tests/rest/admin/test_room.py b/tests/rest/admin/test_room.py index 9dbb778679..eb50086c50 100644 --- a/tests/rest/admin/test_room.py +++ b/tests/rest/admin/test_room.py @@ -402,6 +402,21 @@ class DeleteRoomTestCase(unittest.HomeserverTestCase): # Assert we can no longer peek into the room self._assert_peek(self.room_id, expect_code=403) + def test_room_delete_send(self) -> None: + """Test that sending into a deleted room returns a 403""" + channel = self.make_request( + "DELETE", + self.url, + content={}, + access_token=self.admin_user_tok, + ) + + self.assertEqual(200, channel.code, msg=channel.json_body) + + self.helper.send( + self.room_id, "test message", expect_code=403, tok=self.other_user_tok + ) + def _is_blocked(self, room_id: str, expect: bool = True) -> None: """Assert that the room is blocked or not""" d = self.store.is_room_blocked(room_id) |