diff --git a/changelog.d/19557.removal b/changelog.d/19557.removal new file mode 100644 index 00000000000..57bb18a4dbe --- /dev/null +++ b/changelog.d/19557.removal @@ -0,0 +1 @@ +Remove support for undocumented variants of `/send` and `/redact` without a transaction ID, and `/createRoom`, `/forget` and the membership endpoints (join, invite, leave, ban, unban, kick) with a transaction ID. Contributed by @tulir @ Beeper. diff --git a/synapse/rest/client/room.py b/synapse/rest/client/room.py index 9172bfcb4ed..425710795e9 100644 --- a/synapse/rest/client/room.py +++ b/synapse/rest/client/room.py @@ -165,32 +165,15 @@ def __init__(self, hs: "HomeServer"): class RoomCreateRestServlet(TransactionRestServlet): CATEGORY = "Client API requests" + PATTERNS = client_patterns("/createRoom$", v1=True) def __init__(self, hs: "HomeServer"): super().__init__(hs) self._room_creation_handler = hs.get_room_creation_handler() self.auth = hs.get_auth() - def register(self, http_server: HttpServer) -> None: - PATTERNS = "/createRoom" - register_txn_path(self, PATTERNS, http_server) - - async def on_PUT( - self, request: SynapseRequest, txn_id: str - ) -> tuple[int, JsonDict]: - requester = await self.auth.get_user_by_req(request) - set_tag("txn_id", txn_id) - return await self.txns.fetch_or_execute_request( - request, requester, self._do, request, requester - ) - async def on_POST(self, request: SynapseRequest) -> tuple[int, JsonDict]: requester = await self.auth.get_user_by_req(request) - return await self._do(request, requester) - - async def _do( - self, request: SynapseRequest, requester: Requester - ) -> tuple[int, JsonDict]: room_id, _, _ = await self._room_creation_handler.create_room( requester, self.get_room_config(request) ) @@ -409,6 +392,10 @@ async def on_PUT( # TODO: Needs unit testing for generic events + feedback class RoomSendEventRestServlet(TransactionRestServlet): CATEGORY = "Event sending requests" + PATTERNS = client_patterns( + "/rooms/(?P[^/]*)/send/(?P[^/]*)/(?P[^/]*)$", + v1=True, + ) def __init__(self, hs: "HomeServer"): super().__init__(hs) @@ -418,11 +405,6 @@ def __init__(self, hs: "HomeServer"): self._max_event_delay_ms = hs.config.server.max_event_delay_ms self._msc4354_enabled = hs.config.experimental.msc4354_enabled - def register(self, http_server: HttpServer) -> None: - # /rooms/$roomid/send/$event_type[/$txn_id] - PATTERNS = "/rooms/(?P[^/]*)/send/(?P[^/]*)" - register_txn_path(self, PATTERNS, http_server) - async def _do( self, request: SynapseRequest, @@ -487,15 +469,6 @@ async def _do( set_tag("event_id", event_id) return 200, {"event_id": event_id} - async def on_POST( - self, - request: SynapseRequest, - room_id: str, - event_type: str, - ) -> tuple[int, JsonDict]: - requester = await self.auth.get_user_by_req(request, allow_guest=True) - return await self._do(request, requester, room_id, event_type, None) - async def on_PUT( self, request: SynapseRequest, room_id: str, event_type: str, txn_id: str ) -> tuple[int, JsonDict]: @@ -560,24 +533,19 @@ def _parse_request_delay( # TODO: Needs unit testing for room ID + alias joins class JoinRoomAliasServlet(ResolveRoomIdMixin, TransactionRestServlet): CATEGORY = "Event sending requests" + PATTERNS = client_patterns("/join/(?P[^/]*)", v1=True) def __init__(self, hs: "HomeServer"): super().__init__(hs) super(ResolveRoomIdMixin, self).__init__(hs) # ensure the Mixin is set up self.auth = hs.get_auth() - def register(self, http_server: HttpServer) -> None: - # /join/$room_identifier[/$txn_id] - PATTERNS = "/join/(?P[^/]*)" - register_txn_path(self, PATTERNS, http_server) - - async def _do( + async def on_POST( self, request: SynapseRequest, - requester: Requester, room_identifier: str, - txn_id: str | None, ) -> tuple[int, JsonDict]: + requester = await self.auth.get_user_by_req(request, allow_guest=True) content = parse_json_object_from_request(request, allow_empty_body=True) # twisted.web.server.Request.args is incorrectly defined as Any | None @@ -598,7 +566,7 @@ async def _do( target=requester.user, room_id=room_id, action="join", - txn_id=txn_id, + txn_id=None, remote_room_hosts=remote_room_hosts, content=content, third_party_signed=content.get("third_party_signed", None), @@ -606,24 +574,6 @@ async def _do( return 200, {"room_id": room_id} - async def on_POST( - self, - request: SynapseRequest, - room_identifier: str, - ) -> tuple[int, JsonDict]: - requester = await self.auth.get_user_by_req(request, allow_guest=True) - return await self._do(request, requester, room_identifier, None) - - async def on_PUT( - self, request: SynapseRequest, room_identifier: str, txn_id: str - ) -> tuple[int, JsonDict]: - requester = await self.auth.get_user_by_req(request, allow_guest=True) - set_tag("txn_id", txn_id) - - return await self.txns.fetch_or_execute_request( - request, requester, self._do, request, requester, room_identifier, txn_id - ) - # TODO: Needs unit testing class PublicRoomListRestServlet(RestServlet): @@ -1184,40 +1134,31 @@ async def on_GET( class RoomForgetRestServlet(TransactionRestServlet): + PATTERNS = client_patterns("/rooms/(?P[^/]*)/forget$", v1=True) + def __init__(self, hs: "HomeServer"): super().__init__(hs) self.room_member_handler = hs.get_room_member_handler() self.auth = hs.get_auth() - def register(self, http_server: HttpServer) -> None: - PATTERNS = "/rooms/(?P[^/]*)/forget" - register_txn_path(self, PATTERNS, http_server) - - async def _do(self, requester: Requester, room_id: str) -> tuple[int, JsonDict]: - await self.room_member_handler.forget(user=requester.user, room_id=room_id) - - return 200, {} - async def on_POST( self, request: SynapseRequest, room_id: str ) -> tuple[int, JsonDict]: requester = await self.auth.get_user_by_req(request, allow_guest=False) - return await self._do(requester, room_id) - async def on_PUT( - self, request: SynapseRequest, room_id: str, txn_id: str - ) -> tuple[int, JsonDict]: - requester = await self.auth.get_user_by_req(request, allow_guest=False) - set_tag("txn_id", txn_id) + await self.room_member_handler.forget(user=requester.user, room_id=room_id) - return await self.txns.fetch_or_execute_request( - request, requester, self._do, requester, room_id - ) + return 200, {} # TODO: Needs unit testing class RoomMembershipRestServlet(TransactionRestServlet): CATEGORY = "Event sending requests" + PATTERNS = client_patterns( + "/rooms/(?P[^/]*)/" + "(?Pjoin|invite|leave|ban|unban|kick)", + v1=True, + ) def __init__(self, hs: "HomeServer"): super().__init__(hs) @@ -1225,22 +1166,13 @@ def __init__(self, hs: "HomeServer"): self.auth = hs.get_auth() self.config = hs.config - def register(self, http_server: HttpServer) -> None: - # /rooms/$roomid/[join|invite|leave|ban|unban|kick] - PATTERNS = ( - "/rooms/(?P[^/]*)/" - "(?Pjoin|invite|leave|ban|unban|kick)" - ) - register_txn_path(self, PATTERNS, http_server) - - async def _do( + async def on_POST( self, request: SynapseRequest, - requester: Requester, room_id: str, membership_action: str, - txn_id: str | None, ) -> tuple[int, JsonDict]: + requester = await self.auth.get_user_by_req(request, allow_guest=True) if requester.is_guest and membership_action not in { Membership.JOIN, Membership.LEAVE, @@ -1267,7 +1199,7 @@ async def _do( request_body["address"], request_body["id_server"], requester, - txn_id, + None, request_body["id_access_token"], ) except ShadowBanError: @@ -1297,7 +1229,7 @@ async def _do( target=target, room_id=room_id, action=membership_action, - txn_id=txn_id, + txn_id=None, third_party_signed=request_body.get("third_party_signed", None), content=event_content, ) @@ -1312,35 +1244,13 @@ async def _do( return 200, return_value - async def on_POST( - self, - request: SynapseRequest, - room_id: str, - membership_action: str, - ) -> tuple[int, JsonDict]: - requester = await self.auth.get_user_by_req(request, allow_guest=True) - return await self._do(request, requester, room_id, membership_action, None) - - async def on_PUT( - self, request: SynapseRequest, room_id: str, membership_action: str, txn_id: str - ) -> tuple[int, JsonDict]: - requester = await self.auth.get_user_by_req(request, allow_guest=True) - set_tag("txn_id", txn_id) - - return await self.txns.fetch_or_execute_request( - request, - requester, - self._do, - request, - requester, - room_id, - membership_action, - txn_id, - ) - class RoomRedactEventRestServlet(TransactionRestServlet): CATEGORY = "Event sending requests" + PATTERNS = client_patterns( + "/rooms/(?P[^/]*)/redact/(?P[^/]*)/(?P[^/]*)$", + v1=True, + ) def __init__(self, hs: "HomeServer"): super().__init__(hs) @@ -1352,10 +1262,6 @@ def __init__(self, hs: "HomeServer"): self._relation_handler = hs.get_relations_handler() self._msc3912_enabled = hs.config.experimental.msc3912_enabled - def register(self, http_server: HttpServer) -> None: - PATTERNS = "/rooms/(?P[^/]*)/redact/(?P[^/]*)" - register_txn_path(self, PATTERNS, http_server) - async def _do( self, request: SynapseRequest, @@ -1444,15 +1350,6 @@ async def _do( set_tag("event_id", event_id) return 200, {"event_id": event_id} - async def on_POST( - self, - request: SynapseRequest, - room_id: str, - event_id: str, - ) -> tuple[int, JsonDict]: - requester = await self.auth.get_user_by_req(request) - return await self._do(request, requester, room_id, event_id, None) - async def on_PUT( self, request: SynapseRequest, room_id: str, event_id: str, txn_id: str ) -> tuple[int, JsonDict]: @@ -1586,40 +1483,6 @@ async def on_GET(self, request: SynapseRequest) -> tuple[int, JsonDict]: return 200, {"joined_rooms": list(room_ids)} -def register_txn_path( - servlet: RestServlet, - regex_string: str, - http_server: HttpServer, -) -> None: - """Registers a transaction-based path. - - This registers two paths: - PUT regex_string/$txnid - POST regex_string - - Args: - regex_string: The regex string to register. Must NOT have a - trailing $ as this string will be appended to. - http_server: The http_server to register paths with. - """ - on_POST = getattr(servlet, "on_POST", None) - on_PUT = getattr(servlet, "on_PUT", None) - if on_POST is None or on_PUT is None: - raise RuntimeError("on_POST and on_PUT must exist when using register_txn_path") - http_server.register_paths( - "POST", - client_patterns(regex_string + "$", v1=True), - on_POST, - servlet.__class__.__name__, - ) - http_server.register_paths( - "PUT", - client_patterns(regex_string + "/(?P[^/]*)$", v1=True), - on_PUT, - servlet.__class__.__name__, - ) - - class TimestampLookupRestServlet(RestServlet): """ API endpoint to fetch the `event_id` of the closest event to the given diff --git a/tests/rest/client/test_delayed_events.py b/tests/rest/client/test_delayed_events.py index efa69a393ad..08b4944f83a 100644 --- a/tests/rest/client/test_delayed_events.py +++ b/tests/rest/client/test_delayed_events.py @@ -260,10 +260,12 @@ def test_cancel_delayed_state_event(self, action_in_path: bool) -> None: ) def test_cancel_delayed_event_ratelimit(self, action_in_path: bool) -> None: delay_ids = [] - for _ in range(2): + for i in range(2): channel = self.make_request( - "POST", - _get_path_for_delayed_send(self.room_id, _EVENT_TYPE, 100000), + "PUT", + _get_path_for_delayed_send( + self.room_id, _EVENT_TYPE, 100000, f"txn{i}" + ), {}, self.user1_access_token, ) @@ -331,10 +333,12 @@ def test_send_delayed_state_event( @unittest.override_config({"rc_message": {"per_second": 2.5, "burst_count": 3}}) def test_send_delayed_event_ratelimit(self, action_in_path: bool) -> None: delay_ids = [] - for _ in range(2): + for i in range(2): channel = self.make_request( - "POST", - _get_path_for_delayed_send(self.room_id, _EVENT_TYPE, 100000), + "PUT", + _get_path_for_delayed_send( + self.room_id, _EVENT_TYPE, 100000, f"txn{i}" + ), {}, self.user1_access_token, ) @@ -412,10 +416,12 @@ def test_restart_delayed_state_event(self, action_in_path: bool) -> None: ) def test_restart_delayed_event_ratelimit(self, action_in_path: bool) -> None: delay_ids = [] - for _ in range(2): + for i in range(2): channel = self.make_request( - "POST", - _get_path_for_delayed_send(self.room_id, _EVENT_TYPE, 100000), + "PUT", + _get_path_for_delayed_send( + self.room_id, _EVENT_TYPE, 100000, f"txn{i}" + ), {}, self.user1_access_token, ) @@ -556,5 +562,7 @@ def _get_path_for_delayed_state( return f"rooms/{room_id}/state/{event_type}/{state_key}?org.matrix.msc4140.delay={delay_ms}" -def _get_path_for_delayed_send(room_id: str, event_type: str, delay_ms: int) -> str: - return f"rooms/{room_id}/send/{event_type}?org.matrix.msc4140.delay={delay_ms}" +def _get_path_for_delayed_send( + room_id: str, event_type: str, delay_ms: int, txn_id: str +) -> str: + return f"rooms/{room_id}/send/{event_type}/{txn_id}?org.matrix.msc4140.delay={delay_ms}" diff --git a/tests/rest/client/test_redactions.py b/tests/rest/client/test_redactions.py index 997ca5f9cae..c29de469043 100644 --- a/tests/rest/client/test_redactions.py +++ b/tests/rest/client/test_redactions.py @@ -91,14 +91,18 @@ def _redact_event( Returns the json body. """ - path = "/_matrix/client/r0/rooms/%s/redact/%s" % (room_id, event_id) + path = "/_matrix/client/r0/rooms/%s/redact/%s/%s" % ( + room_id, + event_id, + event_id, + ) request_content = content or {} if with_relations: request_content["org.matrix.msc3912.with_relations"] = with_relations channel = self.make_request( - "POST", path, request_content, access_token=access_token + "PUT", path, request_content, access_token=access_token ) self.assertEqual(channel.code, expect_code) return channel.json_body diff --git a/tests/rest/client/test_relations.py b/tests/rest/client/test_relations.py index 2d8ba77a77a..a2ef59cc020 100644 --- a/tests/rest/client/test_relations.py +++ b/tests/rest/client/test_relations.py @@ -18,7 +18,7 @@ # [This file includes modifications made by New Vector Limited] # # - +import random import urllib.parse from typing import Any, Callable from unittest.mock import AsyncMock, patch @@ -114,9 +114,10 @@ def _send_relation( if key is not None: content["m.relates_to"]["key"] = key + txn_id = random.randbytes(16).hex() channel = self.make_request( - "POST", - f"/_matrix/client/v3/rooms/{self.room}/send/{event_type}", + "PUT", + f"/_matrix/client/v3/rooms/{self.room}/send/{event_type}/{txn_id}", content, access_token=access_token, ) @@ -1593,8 +1594,8 @@ class RelationRedactionTestCase(BaseRelationsTestCase): def _redact(self, event_id: str) -> None: channel = self.make_request( - "POST", - f"/_matrix/client/r0/rooms/{self.room}/redact/{event_id}", + "PUT", + f"/_matrix/client/v3/rooms/{self.room}/redact/{event_id}/{event_id}", access_token=self.user_token, content={}, ) diff --git a/tests/rest/client/test_rooms.py b/tests/rest/client/test_rooms.py index f85c9939ce4..7e97ae35045 100644 --- a/tests/rest/client/test_rooms.py +++ b/tests/rest/client/test_rooms.py @@ -2511,17 +2511,16 @@ def test_add_delayed_event_ratelimit(self) -> None: """ # Test that new delayed events are correctly ratelimited. - args = ( - "POST", - ( - "rooms/%s/send/m.room.message?org.matrix.msc4140.delay=2000" - % self.room_id - ).encode("ascii"), - {"body": "test", "msgtype": "m.text"}, - ) - channel = self.make_request(*args) + def make_args(txn_id: str) -> tuple[str, str, JsonDict]: + return ( + "PUT", + f"rooms/{self.room_id}/send/m.room.message/{txn_id}?org.matrix.msc4140.delay=2000", + {"body": "test", "msgtype": "m.text"}, + ) + + channel = self.make_request(*make_args("mid1")) self.assertEqual(HTTPStatus.OK, channel.code, channel.result) - channel = self.make_request(*args) + channel = self.make_request(*make_args("mid2")) self.assertEqual(HTTPStatus.TOO_MANY_REQUESTS, channel.code, channel.result) # Add the current user to the ratelimit overrides, allowing them no ratelimiting. @@ -2530,7 +2529,7 @@ def test_add_delayed_event_ratelimit(self) -> None: ) # Test that the new delayed events aren't ratelimited anymore. - channel = self.make_request(*args) + channel = self.make_request(*make_args("mid3")) self.assertEqual(HTTPStatus.OK, channel.code, channel.result)