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

Add query parameter ts to allow appservices set the origin_server_ts for state events #11866

Merged
merged 30 commits into from
Oct 3, 2022
Merged
Show file tree
Hide file tree
Changes from 27 commits
Commits
Show all changes
30 commits
Select commit Hold shift + click to select a range
f076866
Add query parameter ts to allow appservices set the original_server_t…
lukasdenk Jan 31, 2022
1a0fc0c
Apply suggestion to synapse/rest/client/room.py
lukasdenk Feb 1, 2022
f5804c3
Apply missing default value
lukasdenk Feb 1, 2022
ae1fbcb
Fix method parameter ordering.
lukasdenk Feb 1, 2022
248ddde
set default parsed value to None
lukasdenk Feb 1, 2022
c1abcf0
fix linting problem
lukasdenk Feb 1, 2022
69320e1
add documentation for original_server_ts parameter
lukasdenk Feb 1, 2022
cb2d34c
Merge remote-tracking branch 'origin/develop' into ts_for_states
lukasdenk Feb 1, 2022
c0c7a66
test ts query param for send and state events
lukasdenk Feb 7, 2022
72d606f
Merge branch 'develop' into ts_for_states
lukasdenk Feb 7, 2022
40eeae7
rename impersonated_user to appservice_user_id
lukasdenk Feb 7, 2022
dcfb170
import urllib at top of file instead of locally writing the whole path
lukasdenk Feb 7, 2022
8d44470
use consistent explanation of appservice_user_id param
lukasdenk Feb 7, 2022
f00f240
introduce msc3316_ts query param
lukasdenk Feb 7, 2022
f6e28bc
Merge remote-tracking branch 'origin/develop' into ts_for_states
lukasdenk Feb 7, 2022
4329180
Merge remote-tracking branch 'origin/develop' into ts_for_states
lukasdenk Feb 8, 2022
e54b2c2
Fix memory import
lukasdenk Feb 17, 2022
6b87d69
Merge remote-tracking branch 'origin/develop' into ts_for_states
lukasdenk Feb 17, 2022
ee295e0
fix linting probs
lukasdenk Feb 18, 2022
13b81ed
Update changelog.d/11866.feature
lukasdenk Mar 2, 2022
331cb48
add comment to Update tests/rest/client/test_rooms.py
lukasdenk Mar 8, 2022
d3e957a
test
lukasdenk Mar 15, 2022
5fab986
Merge remote-tracking branch 'fork2/ts_for_states' into ts_for_states
lukasdenk Mar 15, 2022
6b56dfc
Merge remote-tracking branch 'origin/develop' into ts_for_states
clokep Sep 30, 2022
fd62763
Use the proper urlencode.
clokep Sep 30, 2022
5a320d1
Handle review comments and fix-up code and use stable identifier.
clokep Sep 30, 2022
0bc7b54
Refactor to avoid dictionary unless needed.
clokep Sep 30, 2022
e3a0166
Clean-up another reference to unstable identifier.
clokep Sep 30, 2022
d6149af
Manually call /createRoom.
clokep Oct 3, 2022
65a79c0
Merge remote-tracking branch 'origin/develop' into ts_for_states
clokep Oct 3, 2022
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/11866.feature
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
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.
13 changes: 13 additions & 0 deletions synapse/handlers/room_member.py
Original file line number Diff line number Diff line change
Expand Up @@ -322,6 +322,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
Expand Down Expand Up @@ -361,6 +362,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: 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
Expand Down Expand Up @@ -399,6 +402,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,
allow_no_prev_events=allow_no_prev_events,
Expand Down Expand Up @@ -504,6 +508,7 @@ async def update_membership(
prev_event_ids: Optional[List[str]] = None,
state_event_ids: Optional[List[str]] = None,
depth: Optional[int] = None,
origin_server_ts: Optional[int] = None,
) -> Tuple[str, int]:
"""Update a user's membership in a room.

Expand Down Expand Up @@ -542,6 +547,8 @@ async def update_membership(
depth: Override the depth used to order the event in the DAG.
Should normally be set to None, which will cause the depth to be calculated
based on 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.
Expand Down Expand Up @@ -583,6 +590,7 @@ async def update_membership(
prev_event_ids=prev_event_ids,
state_event_ids=state_event_ids,
depth=depth,
origin_server_ts=origin_server_ts,
)

return result
Expand All @@ -606,6 +614,7 @@ async def update_membership_locked(
prev_event_ids: Optional[List[str]] = None,
state_event_ids: Optional[List[str]] = None,
depth: Optional[int] = None,
origin_server_ts: Optional[int] = None,
) -> Tuple[str, int]:
"""Helper for update_membership.

Expand Down Expand Up @@ -646,6 +655,8 @@ async def update_membership_locked(
depth: Override the depth used to order the event in the DAG.
Should normally be set to None, which will cause the depth to be calculated
based on 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.
Expand Down Expand Up @@ -785,6 +796,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)
Expand Down Expand Up @@ -1030,6 +1042,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(
Expand Down
35 changes: 24 additions & 11 deletions synapse/rest/client/room.py
Original file line number Diff line number Diff line change
Expand Up @@ -268,15 +268,9 @@ async def on_PUT(

content = parse_json_object_from_request(request)

event_dict = {
"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")

lukasdenk marked this conversation as resolved.
Show resolved Hide resolved
try:
if event_type == EventTypes.Member:
Expand All @@ -287,8 +281,22 @@ async def on_PUT(
room_id=room_id,
action=membership,
content=content,
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,
_,
Expand Down Expand Up @@ -335,8 +343,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"
)
clokep marked this conversation as resolved.
Show resolved Hide resolved

try:
(
Expand Down
111 changes: 109 additions & 2 deletions tests/rest/client/test_rooms.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@
import json
from http import HTTPStatus
from typing import Any, Dict, Iterable, List, Optional, Tuple, Union
from unittest.mock import Mock, call
from unittest.mock import Mock, call, patch
from urllib import parse as urlparse

from parameterized import param, parameterized
Expand All @@ -39,9 +39,10 @@
RoomTypes,
)
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, RoomAlias, UserID, create_requester
from synapse.util import Clock
Expand Down Expand Up @@ -1252,6 +1253,112 @@ async def user_may_join_room(
)


class RoomAppserviceTsParamTestCase(unittest.HomeserverTestCase):
servlets = [
room.register_servlets,
synapse.rest.admin.register_servlets,
register.register_servlets,
]

def prepare(self, reactor: MemoryReactor, clock: Clock, hs: HomeServer) -> None:
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,
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()

self.appservice = ApplicationService(
token="i_am_an_app_service",
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_send_event_ts(self) -> None:
"""Test sending a non-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/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 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}/state/m.room.name?"
+ urlparse.urlencode(url_params),
content={"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)

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):
user_id = "@sid1:red"

Expand Down
14 changes: 13 additions & 1 deletion tests/rest/client/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,7 @@ 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:
...

Expand All @@ -83,6 +84,7 @@ 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]:
...

Expand All @@ -95,6 +97,7 @@ 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.
Expand All @@ -116,6 +119,10 @@ 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`.

clokep marked this conversation as resolved.
Show resolved Hide resolved

Returns:
The ID of the newly created room, or None if the request failed.
Expand All @@ -128,8 +135,13 @@ def create_room_as(
content["visibility"] = "public" if is_public else "private"
if room_version:
content["room_version"] = room_version
args = {}
if tok:
path = path + "?access_token=%s" % 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(),
Expand Down