From f0768663eb240f5d95aaad476d890adb4c55e673 Mon Sep 17 00:00:00 2001 From: lukasdenk Date: Mon, 31 Jan 2022 18:03:12 +0100 Subject: [PATCH 01/22] Add query parameter ts to allow appservices set the original_server_ts for state events. --- changelog.d/11866.feature | 1 + synapse/handlers/room_member.py | 11 +++++++++++ synapse/rest/client/room.py | 6 ++++++ 3 files changed, 18 insertions(+) create mode 100644 changelog.d/11866.feature diff --git a/changelog.d/11866.feature b/changelog.d/11866.feature new file mode 100644 index 000000000000..316958ac5363 --- /dev/null +++ b/changelog.d/11866.feature @@ -0,0 +1 @@ +Allow appservices to set the `origin_server_ts` of a state event by providing the query parameter `ts` in `PUT /_matrix/client/r0/rooms/{roomId}/state/{eventType}/{stateKey}`. \ No newline at end of file diff --git a/synapse/handlers/room_member.py b/synapse/handlers/room_member.py index 3dd5e1b6e4dc..37f0366ef3af 100644 --- a/synapse/handlers/room_member.py +++ b/synapse/handlers/room_member.py @@ -261,6 +261,7 @@ async def _local_membership_update( target: UserID, room_id: str, membership: str, + origin_server_ts: Optional[int], prev_event_ids: List[str], auth_event_ids: Optional[List[str]] = None, txn_id: Optional[str] = None, @@ -279,6 +280,7 @@ async def _local_membership_update( target: room_id: membership: + origin_server_ts: prev_event_ids: The event IDs to use as the prev events auth_event_ids: @@ -335,6 +337,7 @@ async def _local_membership_update( "state_key": user_id, # For backwards compatibility: "membership": membership, + "origin_server_ts": origin_server_ts, }, txn_id=txn_id, prev_event_ids=prev_event_ids, @@ -441,6 +444,7 @@ async def update_membership( historical: bool = False, prev_event_ids: Optional[List[str]] = None, auth_event_ids: Optional[List[str]] = None, + origin_server_ts: Optional[int] = None, ) -> Tuple[str, int]: """Update a user's membership in a room. @@ -468,6 +472,8 @@ async def update_membership( The event ids to use as the auth_events for the new event. Should normally be left as None, which will cause them to be calculated based on the room state at the prev_events. + origin_server_ts: The origin_server_ts to use if a new event is created. Uses + the current timestamp if set to None. Returns: A tuple of the new event ID and stream ID. @@ -499,6 +505,7 @@ async def update_membership( historical=historical, prev_event_ids=prev_event_ids, auth_event_ids=auth_event_ids, + origin_server_ts=origin_server_ts, ) return result @@ -520,6 +527,7 @@ async def update_membership_locked( historical: bool = False, prev_event_ids: Optional[List[str]] = None, auth_event_ids: Optional[List[str]] = None, + origin_server_ts: Optional[int] = None, ) -> Tuple[str, int]: """Helper for update_membership. @@ -549,6 +557,7 @@ async def update_membership_locked( The event ids to use as the auth_events for the new event. Should normally be left as None, which will cause them to be calculated based on the room state at the prev_events. + origin_server_ts: Returns: A tuple of the new event ID and stream ID. @@ -679,6 +688,7 @@ async def update_membership_locked( require_consent=require_consent, outlier=outlier, historical=historical, + origin_server_ts=origin_server_ts, ) latest_event_ids = await self.store.get_prev_events_for_room(room_id) @@ -902,6 +912,7 @@ async def update_membership_locked( content=content, require_consent=require_consent, outlier=outlier, + origin_server_ts=origin_server_ts, ) async def _should_perform_remote_join( diff --git a/synapse/rest/client/room.py b/synapse/rest/client/room.py index 90355e44b25e..9e60044d9769 100644 --- a/synapse/rest/client/room.py +++ b/synapse/rest/client/room.py @@ -204,6 +204,11 @@ async def on_PUT( if state_key is not None: event_dict["state_key"] = state_key + # Twisted will have processed the args by now. + assert request.args is not None + if b"ts" in request.args and requester.app_service: + event_dict["origin_server_ts"] = parse_integer(request, "ts", 0) + try: if event_type == EventTypes.Member: membership = content.get("membership", None) @@ -213,6 +218,7 @@ async def on_PUT( room_id=room_id, action=membership, content=content, + origin_server_ts=event_dict.get("origin_server_ts", None), ) else: ( From 1a0fc0c1374896931353ffdcf0ff698c6ef48d37 Mon Sep 17 00:00:00 2001 From: lukasdenk <63459921+lukasdenk@users.noreply.github.com> Date: Tue, 1 Feb 2022 11:19:34 +0100 Subject: [PATCH 02/22] Apply suggestion to synapse/rest/client/room.py Co-authored-by: Andrew Morgan <1342360+anoadragon453@users.noreply.github.com> --- synapse/rest/client/room.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/synapse/rest/client/room.py b/synapse/rest/client/room.py index 9e60044d9769..d64c4452383b 100644 --- a/synapse/rest/client/room.py +++ b/synapse/rest/client/room.py @@ -218,7 +218,7 @@ async def on_PUT( room_id=room_id, action=membership, content=content, - origin_server_ts=event_dict.get("origin_server_ts", None), + origin_server_ts=event_dict.get("origin_server_ts"), ) else: ( From f5804c397a5ae472ef29c9a041ab5e868ca00abe Mon Sep 17 00:00:00 2001 From: lukasdenk <63459921+lukasdenk@users.noreply.github.com> Date: Tue, 1 Feb 2022 11:20:11 +0100 Subject: [PATCH 03/22] Apply missing default value Co-authored-by: Andrew Morgan <1342360+anoadragon453@users.noreply.github.com> --- synapse/handlers/room_member.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/synapse/handlers/room_member.py b/synapse/handlers/room_member.py index 37f0366ef3af..0bc9088dddd6 100644 --- a/synapse/handlers/room_member.py +++ b/synapse/handlers/room_member.py @@ -261,7 +261,7 @@ async def _local_membership_update( target: UserID, room_id: str, membership: str, - origin_server_ts: Optional[int], + origin_server_ts: Optional[int] = None, prev_event_ids: List[str], auth_event_ids: Optional[List[str]] = None, txn_id: Optional[str] = None, From ae1fbcbde156c9f9f7ffc1f5dba1b6b149382766 Mon Sep 17 00:00:00 2001 From: lukasdenk Date: Tue, 1 Feb 2022 11:43:47 +0100 Subject: [PATCH 04/22] Fix method parameter ordering. --- synapse/handlers/room_member.py | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/synapse/handlers/room_member.py b/synapse/handlers/room_member.py index 0bc9088dddd6..b8ad59c9b082 100644 --- a/synapse/handlers/room_member.py +++ b/synapse/handlers/room_member.py @@ -261,7 +261,6 @@ async def _local_membership_update( target: UserID, room_id: str, membership: str, - origin_server_ts: Optional[int] = None, prev_event_ids: List[str], auth_event_ids: Optional[List[str]] = None, txn_id: Optional[str] = None, @@ -270,6 +269,7 @@ async def _local_membership_update( require_consent: bool = True, outlier: bool = False, historical: bool = False, + origin_server_ts: Optional[int] = None, ) -> Tuple[str, int]: """ Internal membership update function to get an existing event or create @@ -280,7 +280,6 @@ async def _local_membership_update( target: room_id: membership: - origin_server_ts: prev_event_ids: The event IDs to use as the prev events auth_event_ids: @@ -299,6 +298,7 @@ async def _local_membership_update( historical: Indicates whether the message is being inserted back in time around some existing events. This is used to skip a few checks and mark the event as backfilled. + origin_server_ts: Returns: Tuple of event ID and stream ordering position @@ -680,10 +680,10 @@ async def update_membership_locked( target=target, room_id=room_id, membership=effective_membership_state, - txn_id=txn_id, - ratelimit=ratelimit, prev_event_ids=prev_event_ids, auth_event_ids=auth_event_ids, + txn_id=txn_id, + ratelimit=ratelimit, content=content, require_consent=require_consent, outlier=outlier, @@ -905,10 +905,10 @@ async def update_membership_locked( target=target, room_id=room_id, membership=effective_membership_state, - txn_id=txn_id, - ratelimit=ratelimit, prev_event_ids=latest_event_ids, auth_event_ids=auth_event_ids, + txn_id=txn_id, + ratelimit=ratelimit, content=content, require_consent=require_consent, outlier=outlier, From 248ddde2131c70e536afe5aac85fe0bdf219b189 Mon Sep 17 00:00:00 2001 From: lukasdenk Date: Tue, 1 Feb 2022 12:50:21 +0100 Subject: [PATCH 05/22] set default parsed value to None --- synapse/rest/client/room.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/synapse/rest/client/room.py b/synapse/rest/client/room.py index d64c4452383b..f01ae017393c 100644 --- a/synapse/rest/client/room.py +++ b/synapse/rest/client/room.py @@ -206,8 +206,8 @@ async def on_PUT( # Twisted will have processed the args by now. assert request.args is not None - if b"ts" in request.args and requester.app_service: - event_dict["origin_server_ts"] = parse_integer(request, "ts", 0) + if requester.app_service: + event_dict["origin_server_ts"] = parse_integer(request, "ts") try: if event_type == EventTypes.Member: From c1abcf0f6c7a672ee0bbc9c6dd0f67c35c19eaf6 Mon Sep 17 00:00:00 2001 From: lukasdenk Date: Tue, 1 Feb 2022 12:50:40 +0100 Subject: [PATCH 06/22] fix linting problem --- synapse/rest/client/room.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/synapse/rest/client/room.py b/synapse/rest/client/room.py index f01ae017393c..4237569fc313 100644 --- a/synapse/rest/client/room.py +++ b/synapse/rest/client/room.py @@ -194,7 +194,7 @@ async def on_PUT( content = parse_json_object_from_request(request) - event_dict = { + event_dict: JsonDict = { "type": event_type, "content": content, "room_id": room_id, From 69320e1c5ebdc5cd4c0080555af3c979321a759f Mon Sep 17 00:00:00 2001 From: lukasdenk Date: Tue, 1 Feb 2022 12:54:51 +0100 Subject: [PATCH 07/22] add documentation for original_server_ts parameter --- synapse/handlers/room_member.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/synapse/handlers/room_member.py b/synapse/handlers/room_member.py index b8ad59c9b082..dc22ebc0bc61 100644 --- a/synapse/handlers/room_member.py +++ b/synapse/handlers/room_member.py @@ -298,7 +298,8 @@ async def _local_membership_update( historical: Indicates whether the message is being inserted back in time around some existing events. This is used to skip a few checks and mark the event as backfilled. - origin_server_ts: + origin_server_ts: The origin_server_ts to use if a new event is created. Uses + the current timestamp if set to None. Returns: Tuple of event ID and stream ordering position @@ -557,7 +558,8 @@ async def update_membership_locked( The event ids to use as the auth_events for the new event. Should normally be left as None, which will cause them to be calculated based on the room state at the prev_events. - origin_server_ts: + origin_server_ts: The origin_server_ts to use if a new event is created. Uses + the current timestamp if set to None. Returns: A tuple of the new event ID and stream ID. From c0c7a66a9f15e1aba087b88c26431d61b36950f9 Mon Sep 17 00:00:00 2001 From: lukasdenk Date: Mon, 7 Feb 2022 15:17:28 +0100 Subject: [PATCH 08/22] test ts query param for send and state events --- tests/rest/client/test_rooms.py | 74 ++++++++++++++++++++++++++++++++- tests/rest/client/utils.py | 65 +++++++++++++++++++++++++---- 2 files changed, 129 insertions(+), 10 deletions(-) diff --git a/tests/rest/client/test_rooms.py b/tests/rest/client/test_rooms.py index 10a4a4dc5ecc..52ca5448b492 100644 --- a/tests/rest/client/test_rooms.py +++ b/tests/rest/client/test_rooms.py @@ -19,10 +19,12 @@ import json from typing import Dict, Iterable, List, Optional -from unittest.mock import Mock, call +from unittest.mock import Mock, call, patch from urllib import parse as urlparse from twisted.internet import defer +from twisted.internet.task import Clock +from twisted.internet.testing import MemoryReactor import synapse.rest.admin from synapse.api.constants import ( @@ -32,9 +34,11 @@ RelationTypes, ) from synapse.api.errors import Codes, HttpResponseException +from synapse.appservice import ApplicationService from synapse.handlers.pagination import PurgeStatus from synapse.rest import admin -from synapse.rest.client import account, directory, login, profile, room, sync +from synapse.rest.client import account, directory, login, profile, register, room, sync +from synapse.server import HomeServer from synapse.types import JsonDict, Requester, RoomAlias, UserID, create_requester from synapse.util.stringutils import random_string @@ -1081,6 +1085,72 @@ async def user_may_join_room( self.helper.join(self.room3, self.user2, expect_code=403, tok=self.tok2) +class RoomAppserviceTsParamTestCase(RoomBase): + servlets = [ + room.register_servlets, + synapse.rest.admin.register_servlets, + register.register_servlets, + ] + + def prepare(self, reactor, clock, homeserver): + self.ts = 1 + + self.appservice_user = self.register_appservice_user( + "as_user_potato", self.appservice.token + ) + + self.room = self.helper.create_room_as( + room_creator=self.appservice_user, + tok=self.appservice.token, + impersonated_user=self.appservice_user, + ) + + def make_homeserver(self, reactor: MemoryReactor, clock: Clock) -> HomeServer: + config = self.default_config() + + self.appservice = ApplicationService( + token="i_am_an_app_service", + hostname="test", + id="1234", + namespaces={"users": [{"regex": r"@as_user.*", "exclusive": True}]}, + # Note: this user does not have to match the regex above + sender="@as_main:test", + ) + + mock_load_appservices = Mock(return_value=[self.appservice]) + with patch( + "synapse.storage.databases.main.appservice.load_appservices", + mock_load_appservices, + ): + hs = self.setup_test_homeserver(config=config) + return hs + + def test_state_event(self): + normal_user = self.register_user("thomas", "hackme") + event_id = self.helper.invite( + self.room, + self.appservice_user, + normal_user, + tok=self.appservice.token, + ts=self.ts, + impersonated_user=self.appservice_user, + ) + self._check_event_ts(event_id) + + def test_send_event(self): + event_id = self.helper.send( + self.room, + tok=self.appservice.token, + ts=self.ts, + impersonated_user=self.appservice_user, + )["event_id"] + self._check_event_ts(event_id) + + def _check_event_ts(self, event_id): + res = self.get_success(self.hs.get_datastore().get_event(event_id)) + self.assertEquals(self.ts, res.origin_server_ts) + + class RoomJoinRatelimitTestCase(RoomBase): user_id = "@sid1:red" diff --git a/tests/rest/client/utils.py b/tests/rest/client/utils.py index 842438358021..0d50c305837c 100644 --- a/tests/rest/client/utils.py +++ b/tests/rest/client/utils.py @@ -28,6 +28,7 @@ MutableMapping, Optional, Tuple, + Union, overload, ) from unittest.mock import patch @@ -66,6 +67,7 @@ def create_room_as( expect_code: Literal[200] = ..., extra_content: Optional[Dict] = ..., custom_headers: Optional[Iterable[Tuple[AnyStr, AnyStr]]] = ..., + impersonated_user: Optional[str] = ..., ) -> str: ... @@ -79,6 +81,7 @@ def create_room_as( expect_code: int = ..., extra_content: Optional[Dict] = ..., custom_headers: Optional[Iterable[Tuple[AnyStr, AnyStr]]] = ..., + impersonated_user: Optional[str] = ..., ) -> Optional[str]: ... @@ -91,6 +94,7 @@ def create_room_as( expect_code: int = 200, extra_content: Optional[Dict] = None, custom_headers: Optional[Iterable[Tuple[AnyStr, AnyStr]]] = None, + impersonated_user: Optional[str] = None, ) -> Optional[str]: """ Create a room. @@ -105,6 +109,7 @@ def create_room_as( default room version. tok: The access token to use in the request. expect_code: The expected HTTP response code. + impersonated_user: The user_id an apperservice should act on behalf. Returns: The ID of the newly created room. @@ -117,8 +122,9 @@ def create_room_as( content["visibility"] = "public" if is_public else "private" if room_version: content["room_version"] = room_version - if tok: - path = path + "?access_token=%s" % tok + path = self._append_query_params( + path, {"access_token": tok, "user_id": impersonated_user} + ) channel = make_request( self.hs.get_reactor(), @@ -137,14 +143,25 @@ def create_room_as( else: return None - def invite(self, room=None, src=None, targ=None, expect_code=200, tok=None): - self.change_membership( + def invite( + self, + room=None, + src=None, + targ=None, + expect_code=200, + tok=None, + impersonated_user=None, + ts=None, + ): + return self.change_membership( room=room, src=src, targ=targ, tok=tok, membership=Membership.INVITE, expect_code=expect_code, + ts=ts, + impersonated_user=impersonated_user, ) def join(self, room=None, user=None, expect_code=200, tok=None): @@ -216,6 +233,8 @@ def change_membership( tok: Optional[str] = None, expect_code: int = 200, expect_errcode: Optional[str] = None, + ts: Optional[int] = None, + impersonated_user=None, ) -> None: """ Send a membership state event into a room. @@ -229,13 +248,24 @@ def change_membership( tok: The user access token to use expect_code: The expected HTTP response code expect_errcode: The expected Matrix error code + ts: If the user is an appservice, ts will be used as the event's + "original_server_ts" + impersonated_user: The user_id an apperservice should act on behalf. + """ temp_id = self.auth_user_id self.auth_user_id = src path = "/_matrix/client/r0/rooms/%s/state/m.room.member/%s" % (room, targ) - if tok: - path = path + "?access_token=%s" % tok + + path = self._append_query_params( + path, + { + "access_token": tok, + "ts": ts, + "user_id": impersonated_user, + }, + ) data = {"membership": membership} data.update(extra_data or {}) @@ -267,6 +297,18 @@ def change_membership( self.auth_user_id = temp_id + return channel.json_body.get("event_id") + + def _append_query_params( + self, path: str, query_params_dict: Dict[str, Union[str, None, int]] + ): + query_params = urllib.parse.urlencode( + {k: v for k, v in query_params_dict.items() if v is not None} + ) + if query_params: + path += "?%s" % query_params + return path + def send( self, room_id, @@ -275,6 +317,8 @@ def send( tok=None, expect_code=200, custom_headers: Optional[Iterable[Tuple[AnyStr, AnyStr]]] = None, + ts: Optional[int] = None, + impersonated_user: Optional[str] = None, ): if body is None: body = "body_text_here" @@ -289,6 +333,8 @@ def send( tok, expect_code, custom_headers=custom_headers, + ts=ts, + impersonated_user=impersonated_user, ) def send_event( @@ -300,13 +346,16 @@ def send_event( tok=None, expect_code=200, custom_headers: Optional[Iterable[Tuple[AnyStr, AnyStr]]] = None, + ts=None, + impersonated_user: Optional[str] = None, ): if txn_id is None: txn_id = "m%s" % (str(time.time())) path = "/_matrix/client/r0/rooms/%s/send/%s/%s" % (room_id, type, txn_id) - if tok: - path = path + "?access_token=%s" % tok + path = self._append_query_params( + path, {"access_token": tok, "user_id": impersonated_user, "ts": ts} + ) channel = make_request( self.hs.get_reactor(), From 40eeae7711cf4f59280c40fd70a6bbc62d5eb0eb Mon Sep 17 00:00:00 2001 From: lukasdenk Date: Mon, 7 Feb 2022 18:22:08 +0100 Subject: [PATCH 09/22] rename impersonated_user to appservice_user_id --- tests/rest/client/test_rooms.py | 4 ++-- tests/rest/client/utils.py | 22 +++++++++++----------- 2 files changed, 13 insertions(+), 13 deletions(-) diff --git a/tests/rest/client/test_rooms.py b/tests/rest/client/test_rooms.py index 52ca5448b492..ba4582337f07 100644 --- a/tests/rest/client/test_rooms.py +++ b/tests/rest/client/test_rooms.py @@ -1133,7 +1133,7 @@ def test_state_event(self): normal_user, tok=self.appservice.token, ts=self.ts, - impersonated_user=self.appservice_user, + appservice_user_id=self.appservice_user, ) self._check_event_ts(event_id) @@ -1142,7 +1142,7 @@ def test_send_event(self): self.room, tok=self.appservice.token, ts=self.ts, - impersonated_user=self.appservice_user, + appservice_user_id=self.appservice_user, )["event_id"] self._check_event_ts(event_id) diff --git a/tests/rest/client/utils.py b/tests/rest/client/utils.py index 3d4b7cca90e3..2088bb1246d5 100644 --- a/tests/rest/client/utils.py +++ b/tests/rest/client/utils.py @@ -68,7 +68,7 @@ def create_room_as( expect_code: Literal[200] = ..., extra_content: Optional[Dict] = ..., custom_headers: Optional[Iterable[Tuple[AnyStr, AnyStr]]] = ..., - impersonated_user: Optional[str] = ..., + appservice_user_id: Optional[str] = ..., ) -> str: ... @@ -82,7 +82,7 @@ def create_room_as( expect_code: int = ..., extra_content: Optional[Dict] = ..., custom_headers: Optional[Iterable[Tuple[AnyStr, AnyStr]]] = ..., - impersonated_user: Optional[str] = ..., + appservice_user_id: Optional[str] = ..., ) -> Optional[str]: ... @@ -95,7 +95,7 @@ def create_room_as( expect_code: int = 200, extra_content: Optional[Dict] = None, custom_headers: Optional[Iterable[Tuple[AnyStr, AnyStr]]] = None, - impersonated_user: Optional[str] = None, + appservice_user_id: Optional[str] = None, ) -> Optional[str]: """ Create a room. @@ -110,7 +110,7 @@ def create_room_as( default room version. tok: The access token to use in the request. expect_code: The expected HTTP response code. - impersonated_user: The user_id an apperservice should act on behalf. + appservice_user_id: The user_id an apperservice should act on behalf. Returns: The ID of the newly created room. @@ -124,7 +124,7 @@ def create_room_as( if room_version: content["room_version"] = room_version path = self._append_query_params( - path, {"access_token": tok, "user_id": impersonated_user} + path, {"access_token": tok, "user_id": appservice_user_id} ) channel = make_request( @@ -151,7 +151,7 @@ def invite( targ=None, expect_code=200, tok=None, - impersonated_user=None, + appservice_user_id=None, ts=None, ): return self.change_membership( @@ -162,7 +162,7 @@ def invite( membership=Membership.INVITE, expect_code=expect_code, ts=ts, - impersonated_user=impersonated_user, + appservice_user_id=appservice_user_id, ) def join( @@ -329,7 +329,7 @@ def send( expect_code=200, custom_headers: Optional[Iterable[Tuple[AnyStr, AnyStr]]] = None, ts: Optional[int] = None, - impersonated_user: Optional[str] = None, + appservice_user_id: Optional[str] = None, ): if body is None: body = "body_text_here" @@ -345,7 +345,7 @@ def send( expect_code, custom_headers=custom_headers, ts=ts, - impersonated_user=impersonated_user, + appservice_user_id=appservice_user_id, ) def send_event( @@ -358,14 +358,14 @@ def send_event( expect_code=200, custom_headers: Optional[Iterable[Tuple[AnyStr, AnyStr]]] = None, ts=None, - impersonated_user: Optional[str] = None, + appservice_user_id: Optional[str] = None, ): if txn_id is None: txn_id = "m%s" % (str(time.time())) path = "/_matrix/client/r0/rooms/%s/send/%s/%s" % (room_id, type, txn_id) path = self._append_query_params( - path, {"access_token": tok, "user_id": impersonated_user, "ts": ts} + path, {"access_token": tok, "user_id": appservice_user_id, "ts": ts} ) channel = make_request( From dcfb1705188bdaaa68e93fc0d45abc708ffd5445 Mon Sep 17 00:00:00 2001 From: lukasdenk Date: Mon, 7 Feb 2022 18:22:22 +0100 Subject: [PATCH 10/22] import urllib at top of file instead of locally writing the whole path --- tests/rest/client/utils.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/rest/client/utils.py b/tests/rest/client/utils.py index 2088bb1246d5..4425ec024d8e 100644 --- a/tests/rest/client/utils.py +++ b/tests/rest/client/utils.py @@ -313,7 +313,7 @@ def change_membership( def _append_query_params( self, path: str, query_params_dict: Dict[str, Union[str, None, int]] ): - query_params = urllib.parse.urlencode( + query_params = urlencode( {k: v for k, v in query_params_dict.items() if v is not None} ) if query_params: From 8d44470c2dc56257afa1050d3c4d3b19b8e9c582 Mon Sep 17 00:00:00 2001 From: lukasdenk Date: Mon, 7 Feb 2022 18:24:21 +0100 Subject: [PATCH 11/22] use consistent explanation of appservice_user_id param --- tests/rest/client/utils.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/tests/rest/client/utils.py b/tests/rest/client/utils.py index 4425ec024d8e..c1a4e79cc2d6 100644 --- a/tests/rest/client/utils.py +++ b/tests/rest/client/utils.py @@ -110,7 +110,9 @@ def create_room_as( default room version. tok: The access token to use in the request. expect_code: The expected HTTP response code. - appservice_user_id: The user_id an apperservice should act on behalf. + appservice_user_id: The `user_id` URL parameter to pass. + This allows driving an application service user + using an application service access token in `tok`. Returns: The ID of the newly created room. From f00f240bc7feb1e85107af61499c69ba93045b90 Mon Sep 17 00:00:00 2001 From: lukasdenk Date: Mon, 7 Feb 2022 18:52:55 +0100 Subject: [PATCH 12/22] introduce msc3316_ts query param --- synapse/rest/client/room.py | 13 ++++++++++--- tests/rest/client/test_rooms.py | 19 ++++++++++++++----- tests/rest/client/utils.py | 24 ++++++++++++++++++------ 3 files changed, 42 insertions(+), 14 deletions(-) diff --git a/synapse/rest/client/room.py b/synapse/rest/client/room.py index 4237569fc313..7f2ef2c9e564 100644 --- a/synapse/rest/client/room.py +++ b/synapse/rest/client/room.py @@ -207,7 +207,9 @@ async def on_PUT( # Twisted will have processed the args by now. assert request.args is not None if requester.app_service: - event_dict["origin_server_ts"] = parse_integer(request, "ts") + event_dict["origin_server_ts"] = parse_integer( + request, "org.matrix.msc3316.ts" + ) try: if event_type == EventTypes.Member: @@ -267,8 +269,13 @@ async def on_POST( # Twisted will have processed the args by now. assert request.args is not None - if b"ts" in request.args and requester.app_service: - event_dict["origin_server_ts"] = parse_integer(request, "ts", 0) + if requester.app_service: + if b"ts" in request.args: + event_dict["origin_server_ts"] = parse_integer(request, "ts") + elif b"org.matrix.msc3316.ts" in request.args: + event_dict["origin_server_ts"] = parse_integer( + request, "org.matrix.msc3316.ts" + ) try: ( diff --git a/tests/rest/client/test_rooms.py b/tests/rest/client/test_rooms.py index ba4582337f07..e1af16035bf3 100644 --- a/tests/rest/client/test_rooms.py +++ b/tests/rest/client/test_rooms.py @@ -1095,14 +1095,14 @@ class RoomAppserviceTsParamTestCase(RoomBase): def prepare(self, reactor, clock, homeserver): self.ts = 1 - self.appservice_user = self.register_appservice_user( + self.appservice_user, _ = self.register_appservice_user( "as_user_potato", self.appservice.token ) self.room = self.helper.create_room_as( room_creator=self.appservice_user, tok=self.appservice.token, - impersonated_user=self.appservice_user, + appservice_user_id=self.appservice_user, ) def make_homeserver(self, reactor: MemoryReactor, clock: Clock) -> HomeServer: @@ -1125,19 +1125,19 @@ def make_homeserver(self, reactor: MemoryReactor, clock: Clock) -> HomeServer: hs = self.setup_test_homeserver(config=config) return hs - def test_state_event(self): + def test_state_event_msc3316_ts(self): normal_user = self.register_user("thomas", "hackme") event_id = self.helper.invite( self.room, self.appservice_user, normal_user, tok=self.appservice.token, - ts=self.ts, + msc3316_ts=self.ts, appservice_user_id=self.appservice_user, ) self._check_event_ts(event_id) - def test_send_event(self): + def test_send_event_ts(self): event_id = self.helper.send( self.room, tok=self.appservice.token, @@ -1146,6 +1146,15 @@ def test_send_event(self): )["event_id"] self._check_event_ts(event_id) + def test_send_event_msc3316_ts(self): + event_id = self.helper.send( + self.room, + tok=self.appservice.token, + msc3316_ts=self.ts, + appservice_user_id=self.appservice_user, + )["event_id"] + self._check_event_ts(event_id) + def _check_event_ts(self, event_id): res = self.get_success(self.hs.get_datastore().get_event(event_id)) self.assertEquals(self.ts, res.origin_server_ts) diff --git a/tests/rest/client/utils.py b/tests/rest/client/utils.py index c1a4e79cc2d6..c94e6ed34c0b 100644 --- a/tests/rest/client/utils.py +++ b/tests/rest/client/utils.py @@ -154,7 +154,7 @@ def invite( expect_code=200, tok=None, appservice_user_id=None, - ts=None, + msc3316_ts=None, ): return self.change_membership( room=room, @@ -163,7 +163,7 @@ def invite( tok=tok, membership=Membership.INVITE, expect_code=expect_code, - ts=ts, + msc3316_ts=msc3316_ts, appservice_user_id=appservice_user_id, ) @@ -245,7 +245,7 @@ def change_membership( appservice_user_id: Optional[str] = None, expect_code: int = 200, expect_errcode: Optional[str] = None, - ts: Optional[int] = None, + msc3316_ts: Optional[int] = None, ) -> None: """ Send a membership state event into a room. @@ -262,9 +262,12 @@ def change_membership( using an application service access token in `tok`. expect_code: The expected HTTP response code expect_errcode: The expected Matrix error code - ts: If the user is an appservice, ts will be used as the event's + msc3316_ts: If the user is an appservice, msc3316_ts will be used as the event's "original_server_ts" + Returns: + The id of the created event. + """ temp_id = self.auth_user_id self.auth_user_id = src @@ -275,7 +278,7 @@ def change_membership( path, { "access_token": tok, - "ts": ts, + "org.matrix.msc3316.ts": msc3316_ts, "user_id": appservice_user_id, }, ) @@ -331,6 +334,7 @@ def send( expect_code=200, custom_headers: Optional[Iterable[Tuple[AnyStr, AnyStr]]] = None, ts: Optional[int] = None, + msc3316_ts: Optional[int] = None, appservice_user_id: Optional[str] = None, ): if body is None: @@ -347,6 +351,7 @@ def send( expect_code, custom_headers=custom_headers, ts=ts, + msc3316_ts=msc3316_ts, appservice_user_id=appservice_user_id, ) @@ -360,6 +365,7 @@ def send_event( expect_code=200, custom_headers: Optional[Iterable[Tuple[AnyStr, AnyStr]]] = None, ts=None, + msc3316_ts=None, appservice_user_id: Optional[str] = None, ): if txn_id is None: @@ -367,7 +373,13 @@ def send_event( path = "/_matrix/client/r0/rooms/%s/send/%s/%s" % (room_id, type, txn_id) path = self._append_query_params( - path, {"access_token": tok, "user_id": appservice_user_id, "ts": ts} + path, + { + "access_token": tok, + "user_id": appservice_user_id, + "ts": ts, + "org.matrix.msc3316.ts": msc3316_ts, + }, ) channel = make_request( From e54b2c23cdaf9c18fffda1eb8cb25d63a178679f Mon Sep 17 00:00:00 2001 From: lukasdenk Date: Thu, 17 Feb 2022 07:33:41 +0100 Subject: [PATCH 13/22] Fix memory import --- tests/rest/client/test_rooms.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/rest/client/test_rooms.py b/tests/rest/client/test_rooms.py index e1af16035bf3..6d6cd215108a 100644 --- a/tests/rest/client/test_rooms.py +++ b/tests/rest/client/test_rooms.py @@ -24,7 +24,7 @@ from twisted.internet import defer from twisted.internet.task import Clock -from twisted.internet.testing import MemoryReactor +from twisted.test.proto_helpers import MemoryReactor import synapse.rest.admin from synapse.api.constants import ( From ee295e0d531b7b3f3b86d4334bdb39010a1d4041 Mon Sep 17 00:00:00 2001 From: lukasdenk Date: Fri, 18 Feb 2022 13:30:33 +0100 Subject: [PATCH 14/22] fix linting probs --- tests/rest/client/test_rooms.py | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/tests/rest/client/test_rooms.py b/tests/rest/client/test_rooms.py index 104cafdf4c21..28e773da65c9 100644 --- a/tests/rest/client/test_rooms.py +++ b/tests/rest/client/test_rooms.py @@ -37,11 +37,10 @@ from synapse.appservice import ApplicationService from synapse.handlers.pagination import PurgeStatus from synapse.rest import admin -from synapse.rest.client import account, directory, login, profile, room, sync -from synapse.types import JsonDict,Requester, RoomAlias, UserID, create_requester -from synapse.util.stringutils import random_string +from synapse.rest.client import account, directory, login, profile, register, room, sync from synapse.server import HomeServer - +from synapse.types import JsonDict, RoomAlias, UserID, create_requester +from synapse.util.stringutils import random_string from tests import unittest from tests.test_utils import make_awaitable From 13b81ed110dc6ab68824246262a06052949d455e Mon Sep 17 00:00:00 2001 From: lukasdenk <63459921+lukasdenk@users.noreply.github.com> Date: Wed, 2 Mar 2022 07:55:35 +0100 Subject: [PATCH 15/22] Update changelog.d/11866.feature Co-authored-by: Richard van der Hoff <1389908+richvdh@users.noreply.github.com> --- changelog.d/11866.feature | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/changelog.d/11866.feature b/changelog.d/11866.feature index 316958ac5363..0b52caf80538 100644 --- a/changelog.d/11866.feature +++ b/changelog.d/11866.feature @@ -1 +1 @@ -Allow appservices to set the `origin_server_ts` of a state event by providing the query parameter `ts` in `PUT /_matrix/client/r0/rooms/{roomId}/state/{eventType}/{stateKey}`. \ No newline at end of file +Allow application services to set the `origin_server_ts` of a state event by providing the query parameter `ts` in `PUT /_matrix/client/r0/rooms/{roomId}/state/{eventType}/{stateKey}`, per [MSC3316](https://github.com/matrix-org/matrix-doc/pull/3316). Contributed by @lukasdenk. \ No newline at end of file From 331cb48f095a6931fbc2f31d19f0a79b08c52eb2 Mon Sep 17 00:00:00 2001 From: lukasdenk <63459921+lukasdenk@users.noreply.github.com> Date: Tue, 8 Mar 2022 13:59:39 +0100 Subject: [PATCH 16/22] add comment to Update tests/rest/client/test_rooms.py Co-authored-by: Richard van der Hoff <1389908+richvdh@users.noreply.github.com> --- tests/rest/client/test_rooms.py | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/rest/client/test_rooms.py b/tests/rest/client/test_rooms.py index 28e773da65c9..fa7fb4a5cfc6 100644 --- a/tests/rest/client/test_rooms.py +++ b/tests/rest/client/test_rooms.py @@ -1041,6 +1041,7 @@ def test_send_event_msc3316_ts(self): self._check_event_ts(event_id) def _check_event_ts(self, event_id): + # check that the event was successfully persisted to the database with the correct timestamp. res = self.get_success(self.hs.get_datastore().get_event(event_id)) self.assertEquals(self.ts, res.origin_server_ts) From d3e957af8cc18b989f7487c6a432eef9995f8ca8 Mon Sep 17 00:00:00 2001 From: lukasdenk Date: Tue, 15 Mar 2022 10:46:32 +0100 Subject: [PATCH 17/22] test --- tests/rest/client/test_rooms.py | 55 +++++++++++++-------------------- 1 file changed, 21 insertions(+), 34 deletions(-) diff --git a/tests/rest/client/test_rooms.py b/tests/rest/client/test_rooms.py index 28e773da65c9..003a90499ff8 100644 --- a/tests/rest/client/test_rooms.py +++ b/tests/rest/client/test_rooms.py @@ -19,6 +19,8 @@ import json from typing import Iterable, List + +from authlib.common.urls import url_encode from unittest.mock import Mock, call, patch from urllib import parse as urlparse @@ -42,7 +44,7 @@ from synapse.types import JsonDict, RoomAlias, UserID, create_requester from synapse.util.stringutils import random_string -from tests import unittest +from tests import unittest, server from tests.test_utils import make_awaitable PATH_PREFIX = b"/_matrix/client/api/v1" @@ -970,7 +972,7 @@ async def user_may_join_room( self.helper.join(self.room3, self.user2, expect_code=403, tok=self.tok2) -class RoomAppserviceTsParamTestCase(RoomBase): +class RoomAppserviceTsParamTestCase(unittest.HomeserverTestCase): servlets = [ room.register_servlets, synapse.rest.admin.register_servlets, @@ -978,8 +980,6 @@ class RoomAppserviceTsParamTestCase(RoomBase): ] def prepare(self, reactor, clock, homeserver): - self.ts = 1 - self.appservice_user, _ = self.register_appservice_user( "as_user_potato", self.appservice.token ) @@ -1010,39 +1010,26 @@ def make_homeserver(self, reactor: MemoryReactor, clock: Clock) -> HomeServer: hs = self.setup_test_homeserver(config=config) return hs - def test_state_event_msc3316_ts(self): - normal_user = self.register_user("thomas", "hackme") - event_id = self.helper.invite( - self.room, - self.appservice_user, - normal_user, - tok=self.appservice.token, - msc3316_ts=self.ts, - appservice_user_id=self.appservice_user, - ) - self._check_event_ts(event_id) def test_send_event_ts(self): - event_id = self.helper.send( - self.room, - tok=self.appservice.token, - ts=self.ts, - appservice_user_id=self.appservice_user, - )["event_id"] - self._check_event_ts(event_id) - - def test_send_event_msc3316_ts(self): - event_id = self.helper.send( - self.room, - tok=self.appservice.token, - msc3316_ts=self.ts, - appservice_user_id=self.appservice_user, - )["event_id"] - self._check_event_ts(event_id) - - def _check_event_ts(self, event_id): + ts = 1 + event_id = self._send(ts) res = self.get_success(self.hs.get_datastore().get_event(event_id)) - self.assertEquals(self.ts, res.origin_server_ts) + self.assertEquals(ts, res.origin_server_ts) + + def _send(self, ts:int) ->str: + url_params = { + "user_id": self.appservice_user, + "ts": ts, + } + channel = server.make_request( + "PUT", + path=f"/_matrix/client/r0/rooms/{self.room}/send/m.room.message/{None}?" + url_encode(url_params), + content={"membership": "invite"}, + access_token=self.appservice.token, + ) + self.assertEqual(channel.code, 200) + return channel.json_body["event_id"] class RoomJoinRatelimitTestCase(RoomBase): From fd627633131acb9455b600dd5194806d76161215 Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Fri, 30 Sep 2022 14:54:57 -0400 Subject: [PATCH 18/22] Use the proper urlencode. --- tests/rest/client/test_rooms.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/tests/rest/client/test_rooms.py b/tests/rest/client/test_rooms.py index e2bbcf58e315..b9a2a0856d70 100644 --- a/tests/rest/client/test_rooms.py +++ b/tests/rest/client/test_rooms.py @@ -23,7 +23,6 @@ from unittest.mock import Mock, call, patch from urllib import parse as urlparse -from authlib.common.urls import url_encode from parameterized import param, parameterized from typing_extensions import Literal @@ -1306,7 +1305,7 @@ def _send(self, ts: int) -> str: channel = self.make_request( "PUT", path=f"/_matrix/client/r0/rooms/{self.room}/send/m.room.message/{None}?" - + url_encode(url_params), + + urlparse.urlencode(url_params), content={"membership": "invite"}, access_token=self.appservice.token, ) From 5a320d179586e2156acf86ab946b82a83a18c237 Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Fri, 30 Sep 2022 15:20:34 -0400 Subject: [PATCH 19/22] Handle review comments and fix-up code and use stable identifier. --- synapse/rest/client/room.py | 8 ++-- tests/rest/client/test_rooms.py | 62 +++++++++++++++++++++++++---- tests/rest/client/utils.py | 69 +++++++-------------------------- 3 files changed, 72 insertions(+), 67 deletions(-) diff --git a/synapse/rest/client/room.py b/synapse/rest/client/room.py index f191c3a96280..fa85c52d2c23 100644 --- a/synapse/rest/client/room.py +++ b/synapse/rest/client/room.py @@ -278,12 +278,10 @@ async def on_PUT( if state_key is not None: event_dict["state_key"] = state_key - # Twisted will have processed the args by now. - assert request.args is not None if requester.app_service: - event_dict["origin_server_ts"] = parse_integer( - request, "org.matrix.msc3316.ts" - ) + origin_server_ts = parse_integer(request, "ts") + if origin_server_ts is not None: + event_dict["origin_server_ts"] = origin_server_ts try: if event_type == EventTypes.Member: diff --git a/tests/rest/client/test_rooms.py b/tests/rest/client/test_rooms.py index b9a2a0856d70..2954140a76b9 100644 --- a/tests/rest/client/test_rooms.py +++ b/tests/rest/client/test_rooms.py @@ -1271,6 +1271,8 @@ def prepare(self, reactor: MemoryReactor, clock: Clock, hs: HomeServer) -> None: appservice_user_id=self.appservice_user, ) + self.main_store = self.hs.get_datastores().main + def make_homeserver(self, reactor: MemoryReactor, clock: Clock) -> HomeServer: config = self.default_config() @@ -1291,26 +1293,70 @@ def make_homeserver(self, reactor: MemoryReactor, clock: Clock) -> HomeServer: return hs def test_send_event_ts(self) -> None: + """Test sending a non-state event with a custom timestamp.""" ts = 1 - event_id = self._send(ts) - # check that the event was successfully persisted to the database with the correct timestamp. - res = self.get_success(self.hs.get_datastore().get_event(event_id)) + + url_params = { + "user_id": self.appservice_user, + "ts": ts, + } + channel = self.make_request( + "PUT", + path=f"/_matrix/client/r0/rooms/{self.room}/send/m.room.message/1234?" + + urlparse.urlencode(url_params), + content={"body": "test", "msgtype": "m.text"}, + access_token=self.appservice.token, + ) + self.assertEqual(channel.code, 200, channel.json_body) + event_id = channel.json_body["event_id"] + + # Ensure the event was persisted with the correct timestamp. + res = self.get_success(self.main_store.get_event(event_id)) self.assertEquals(ts, res.origin_server_ts) - def _send(self, ts: int) -> str: + def test_send_state_event_ts(self) -> None: + """Test sending a state event with a custom timestamp.""" + ts = 1 + url_params = { "user_id": self.appservice_user, "ts": ts, } channel = self.make_request( "PUT", - path=f"/_matrix/client/r0/rooms/{self.room}/send/m.room.message/{None}?" + path=f"/_matrix/client/r0/rooms/{self.room}/state/m.room.name?" + urlparse.urlencode(url_params), - content={"membership": "invite"}, + content={"name": "test"}, access_token=self.appservice.token, ) - self.assertEqual(channel.code, 200) - return channel.json_body["event_id"] + self.assertEqual(channel.code, 200, channel.json_body) + event_id = channel.json_body["event_id"] + + # Ensure the event was persisted with the correct timestamp. + res = self.get_success(self.main_store.get_event(event_id)) + self.assertEquals(ts, res.origin_server_ts) + + def test_send_membership_event_ts(self) -> None: + """Test sending a membership event with a custom timestamp.""" + ts = 1 + + url_params = { + "user_id": self.appservice_user, + "ts": ts, + } + channel = self.make_request( + "PUT", + path=f"/_matrix/client/r0/rooms/{self.room}/state/m.room.member/{self.appservice_user}?" + + urlparse.urlencode(url_params), + content={"membership": "join", "display_name": "test"}, + access_token=self.appservice.token, + ) + self.assertEqual(channel.code, 200, channel.json_body) + event_id = channel.json_body["event_id"] + + # Ensure the event was persisted with the correct timestamp. + res = self.get_success(self.main_store.get_event(event_id)) + self.assertEquals(ts, res.origin_server_ts) class RoomJoinRatelimitTestCase(RoomBase): diff --git a/tests/rest/client/utils.py b/tests/rest/client/utils.py index a7b872b9cb02..db32ad1fa074 100644 --- a/tests/rest/client/utils.py +++ b/tests/rest/client/utils.py @@ -29,7 +29,6 @@ MutableMapping, Optional, Tuple, - Union, overload, ) from unittest.mock import patch @@ -136,9 +135,13 @@ def create_room_as( content["visibility"] = "public" if is_public else "private" if room_version: content["room_version"] = room_version - path = self._append_query_params( - path, {"access_token": tok, "user_id": appservice_user_id} - ) + args = {} + if tok: + args["access_token"] = tok + if appservice_user_id: + args["user_id"] = appservice_user_id + if args: + path = path + "?" + urlencode(args) channel = make_request( self.hs.get_reactor(), @@ -164,8 +167,6 @@ def invite( targ: Optional[str] = None, expect_code: int = HTTPStatus.OK, tok: Optional[str] = None, - appservice_user_id: Optional[str] = None, - msc3316_ts: Optional[int] = None, ) -> None: self.change_membership( room=room, @@ -174,8 +175,6 @@ def invite( tok=tok, membership=Membership.INVITE, expect_code=expect_code, - msc3316_ts=msc3316_ts, - appservice_user_id=appservice_user_id, ) def join( @@ -280,8 +279,7 @@ def change_membership( expect_code: int = HTTPStatus.OK, expect_errcode: Optional[str] = None, expect_additional_fields: Optional[dict] = None, - msc3316_ts: Optional[int] = None, - ) -> Optional[str]: + ) -> None: """ Send a membership state event into a room. @@ -297,12 +295,6 @@ def change_membership( using an application service access token in `tok`. expect_code: The expected HTTP response code expect_errcode: The expected Matrix error code - msc3316_ts: If the user is an appservice, msc3316_ts will be used as the event's - "original_server_ts" - - Returns: - The id of the created event. - """ temp_id = self.auth_user_id self.auth_user_id = src @@ -313,14 +305,11 @@ def change_membership( if tok: url_params["access_token"] = tok - path = self._append_query_params( - path, - { - "access_token": tok, - "org.matrix.msc3316.ts": msc3316_ts, - "user_id": appservice_user_id, - }, - ) + if appservice_user_id: + url_params["user_id"] = appservice_user_id + + if url_params: + path += "?" + urlencode(url_params) data = {"membership": membership} data.update(extra_data or {}) @@ -365,18 +354,6 @@ def change_membership( self.auth_user_id = temp_id - return channel.json_body.get("event_id") - - def _append_query_params( - self, path: str, query_params_dict: Dict[str, Union[str, None, int]] - ) -> str: - query_params = urlencode( - {k: v for k, v in query_params_dict.items() if v is not None} - ) - if query_params: - path += "?%s" % query_params - return path - def send( self, room_id: str, @@ -385,9 +362,6 @@ def send( tok: Optional[str] = None, expect_code: int = HTTPStatus.OK, custom_headers: Optional[Iterable[Tuple[AnyStr, AnyStr]]] = None, - ts: Optional[int] = None, - msc3316_ts: Optional[int] = None, - appservice_user_id: Optional[str] = None, ) -> JsonDict: if body is None: body = "body_text_here" @@ -402,9 +376,6 @@ def send( tok, expect_code, custom_headers=custom_headers, - ts=ts, - msc3316_ts=msc3316_ts, - appservice_user_id=appservice_user_id, ) def send_event( @@ -416,23 +387,13 @@ def send_event( tok: Optional[str] = None, expect_code: int = HTTPStatus.OK, custom_headers: Optional[Iterable[Tuple[AnyStr, AnyStr]]] = None, - ts: Optional[int] = None, - msc3316_ts: Optional[int] = None, - appservice_user_id: Optional[str] = None, ) -> JsonDict: if txn_id is None: txn_id = "m%s" % (str(time.time())) path = "/_matrix/client/r0/rooms/%s/send/%s/%s" % (room_id, type, txn_id) - path = self._append_query_params( - path, - { - "access_token": tok, - "user_id": appservice_user_id, - "ts": ts, - "org.matrix.msc3316.ts": msc3316_ts, - }, - ) + if tok: + path = path + "?access_token=%s" % tok channel = make_request( self.hs.get_reactor(), From 0bc7b546aec966e142ae74e7aef4203fe341bb5f Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Fri, 30 Sep 2022 15:23:43 -0400 Subject: [PATCH 20/22] Refactor to avoid dictionary unless needed. --- synapse/rest/client/room.py | 28 +++++++++++++++------------- 1 file changed, 15 insertions(+), 13 deletions(-) diff --git a/synapse/rest/client/room.py b/synapse/rest/client/room.py index fa85c52d2c23..e9550549417d 100644 --- a/synapse/rest/client/room.py +++ b/synapse/rest/client/room.py @@ -268,20 +268,9 @@ async def on_PUT( content = parse_json_object_from_request(request) - event_dict: JsonDict = { - "type": event_type, - "content": content, - "room_id": room_id, - "sender": requester.user.to_string(), - } - - if state_key is not None: - event_dict["state_key"] = state_key - + origin_server_ts = None if requester.app_service: origin_server_ts = parse_integer(request, "ts") - if origin_server_ts is not None: - event_dict["origin_server_ts"] = origin_server_ts try: if event_type == EventTypes.Member: @@ -292,9 +281,22 @@ async def on_PUT( room_id=room_id, action=membership, content=content, - origin_server_ts=event_dict.get("origin_server_ts"), + origin_server_ts=origin_server_ts, ) else: + event_dict: JsonDict = { + "type": event_type, + "content": content, + "room_id": room_id, + "sender": requester.user.to_string(), + } + + if state_key is not None: + event_dict["state_key"] = state_key + + if origin_server_ts is not None: + event_dict["origin_server_ts"] = origin_server_ts + ( event, _, From e3a0166c81a81d12800d35338324c8fe0d8844ae Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Fri, 30 Sep 2022 15:47:56 -0400 Subject: [PATCH 21/22] Clean-up another reference to unstable identifier. --- synapse/rest/client/room.py | 11 +++-------- 1 file changed, 3 insertions(+), 8 deletions(-) diff --git a/synapse/rest/client/room.py b/synapse/rest/client/room.py index e9550549417d..b6dedbed041c 100644 --- a/synapse/rest/client/room.py +++ b/synapse/rest/client/room.py @@ -341,15 +341,10 @@ async def on_POST( "sender": requester.user.to_string(), } - # Twisted will have processed the args by now. - assert request.args is not None if requester.app_service: - if b"ts" in request.args: - event_dict["origin_server_ts"] = parse_integer(request, "ts") - elif b"org.matrix.msc3316.ts" in request.args: - event_dict["origin_server_ts"] = parse_integer( - request, "org.matrix.msc3316.ts" - ) + origin_server_ts = parse_integer(request, "ts") + if origin_server_ts is not None: + event_dict["origin_server_ts"] = origin_server_ts try: ( From d6149afd66fb9cab5015f3f655efd23458d56ab3 Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Mon, 3 Oct 2022 08:52:47 -0400 Subject: [PATCH 22/22] Manually call /createRoom. --- tests/rest/client/test_rooms.py | 16 ++++++++++++---- tests/rest/client/utils.py | 14 +------------- 2 files changed, 13 insertions(+), 17 deletions(-) diff --git a/tests/rest/client/test_rooms.py b/tests/rest/client/test_rooms.py index 2954140a76b9..5e66b5b26cbc 100644 --- a/tests/rest/client/test_rooms.py +++ b/tests/rest/client/test_rooms.py @@ -1265,12 +1265,20 @@ def prepare(self, reactor: MemoryReactor, clock: Clock, hs: HomeServer) -> None: "as_user_potato", self.appservice.token ) - self.room = self.helper.create_room_as( - room_creator=self.appservice_user, - tok=self.appservice.token, - appservice_user_id=self.appservice_user, + # Create a room as the appservice user. + args = { + "access_token": self.appservice.token, + "user_id": self.appservice_user, + } + channel = self.make_request( + "POST", + f"/_matrix/client/r0/createRoom?{urlparse.urlencode(args)}", + content={"visibility": "public"}, ) + assert channel.code == 200 + self.room = channel.json_body["room_id"] + self.main_store = self.hs.get_datastores().main def make_homeserver(self, reactor: MemoryReactor, clock: Clock) -> HomeServer: diff --git a/tests/rest/client/utils.py b/tests/rest/client/utils.py index db32ad1fa074..c249a42bb641 100644 --- a/tests/rest/client/utils.py +++ b/tests/rest/client/utils.py @@ -70,7 +70,6 @@ def create_room_as( expect_code: Literal[200] = ..., extra_content: Optional[Dict] = ..., custom_headers: Optional[Iterable[Tuple[AnyStr, AnyStr]]] = ..., - appservice_user_id: Optional[str] = ..., ) -> str: ... @@ -84,7 +83,6 @@ def create_room_as( expect_code: int = ..., extra_content: Optional[Dict] = ..., custom_headers: Optional[Iterable[Tuple[AnyStr, AnyStr]]] = ..., - appservice_user_id: Optional[str] = ..., ) -> Optional[str]: ... @@ -97,7 +95,6 @@ def create_room_as( expect_code: int = HTTPStatus.OK, extra_content: Optional[Dict] = None, custom_headers: Optional[Iterable[Tuple[AnyStr, AnyStr]]] = None, - appservice_user_id: Optional[str] = None, ) -> Optional[str]: """ Create a room. @@ -119,10 +116,6 @@ def create_room_as( Note that if is_public is set, the "visibility" key will be overridden. If room_version is set, the "room_version" key will be overridden. custom_headers: HTTP headers to include in the request. - appservice_user_id: The `user_id` URL parameter to pass. - This allows driving an application service user - using an application service access token in `tok`. - Returns: The ID of the newly created room, or None if the request failed. @@ -135,13 +128,8 @@ def create_room_as( content["visibility"] = "public" if is_public else "private" if room_version: content["room_version"] = room_version - args = {} if tok: - args["access_token"] = tok - if appservice_user_id: - args["user_id"] = appservice_user_id - if args: - path = path + "?" + urlencode(args) + path = path + "?access_token=%s" % tok channel = make_request( self.hs.get_reactor(),