Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
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/19722.feature
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
MSC4311: invites and knocks should contain the create event. Contributed by @FrenchGithubUser @Famedly
Comment thread
FrenchGithubUser marked this conversation as resolved.
Outdated
3 changes: 3 additions & 0 deletions synapse/config/api.py
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,9 @@ def _get_prejoin_state_entries(
self, config: JsonDict
) -> Iterable[tuple[str, str | None]]:
"""Get the event types and state keys to include in the prejoin state."""
# MSC4311: the create event must always be included in invite/knock state.
yield EventTypes.Create, ""

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Does this PR also cover including the full m.room.create event PDU in the invite_room_state/knock_room_state on m.room.member events (in unsigned)

This is coming from looking at #19414 and seeing the Server-Server (federation) changes mentioned as well for MSC4311

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

With further understanding as I review this, my guess is no (which is fine, something for another PR)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

indeed, it doesn't cover including the full m.room.create event PDU


room_prejoin_state_config = config.get("room_prejoin_state") or {}

# backwards-compatibility support for room_invite_state_types
Expand Down
30 changes: 24 additions & 6 deletions tests/config/test_api.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
from synapse.config import ConfigError
from synapse.config._base import RootConfig
from synapse.config.api import ApiConfig
from synapse.types.state import StateFilter

Check failure on line 8 in tests/config/test_api.py

View workflow job for this annotation

GitHub Actions / lint

Ruff (F401)

tests/config/test_api.py:8:33: F401 `synapse.types.state.StateFilter` imported but unused
Comment thread
FrenchGithubUser marked this conversation as resolved.
Outdated

DEFAULT_PREJOIN_STATE_PAIRS = {
("m.room.join_rules", ""),
Expand Down Expand Up @@ -38,7 +38,11 @@
disable_default_event_types: true
"""
)
self.assertEqual(config.room_prejoin_state, StateFilter.none())
# MSC4311: m.room.create is always included even when defaults are disabled
self.assertEqual(
set(config.room_prejoin_state.concrete_types()),
{("m.room.create", "")},
)

def test_event_without_state_key(self) -> None:
config = self.read_config(
Expand All @@ -50,7 +54,11 @@
"""
)
self.assertEqual(config.room_prejoin_state.wildcard_types(), ["foo"])
self.assertEqual(config.room_prejoin_state.concrete_types(), [])
# MSC4311: m.room.create is always included
self.assertEqual(
set(config.room_prejoin_state.concrete_types()),
{("m.room.create", "")},
)

def test_event_with_specific_state_key(self) -> None:
config = self.read_config(
Expand All @@ -62,9 +70,10 @@
"""
)
self.assertFalse(config.room_prejoin_state.has_wildcards())
# MSC4311: m.room.create is always included
self.assertEqual(
set(config.room_prejoin_state.concrete_types()),
{("foo", "bar")},
{("foo", "bar"), ("m.room.create", "")},
)

def test_repeated_event_with_specific_state_key(self) -> None:
Expand All @@ -78,9 +87,10 @@
"""
)
self.assertFalse(config.room_prejoin_state.has_wildcards())
# MSC4311: m.room.create is always included
self.assertEqual(
set(config.room_prejoin_state.concrete_types()),
{("foo", "bar"), ("foo", "baz")},
{("foo", "bar"), ("foo", "baz"), ("m.room.create", "")},
)

def test_no_specific_state_key_overrides_specific_state_key(self) -> None:
Expand All @@ -94,7 +104,11 @@
"""
)
self.assertEqual(config.room_prejoin_state.wildcard_types(), ["foo"])
self.assertEqual(config.room_prejoin_state.concrete_types(), [])
# MSC4311: m.room.create is always included
self.assertEqual(
set(config.room_prejoin_state.concrete_types()),
{("m.room.create", "")},
)

config = self.read_config(
"""
Expand All @@ -106,7 +120,11 @@
"""
)
self.assertEqual(config.room_prejoin_state.wildcard_types(), ["foo"])
self.assertEqual(config.room_prejoin_state.concrete_types(), [])
# MSC4311: m.room.create is always included
self.assertEqual(
set(config.room_prejoin_state.concrete_types()),
{("m.room.create", "")},
)

def test_bad_event_type_entry_raises(self) -> None:
with self.assertRaises(ConfigError):
Expand Down
78 changes: 78 additions & 0 deletions tests/rest/client/test_sync.py
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@
from synapse.api.constants import (
EventContentFields,
EventTypes,
JoinRules,
ReceiptTypes,
RelationTypes,
)
Expand Down Expand Up @@ -394,6 +395,83 @@
)


class SyncCreateEventInPrejoinStateTestCase(unittest.HomeserverTestCase):
"""MSC4311: Tests that m.room.create is present in invite_state and knock_state"""

servlets = [
synapse.rest.admin.register_servlets,
login.register_servlets,
room.register_servlets,
sync.register_servlets,
knock.register_servlets,
]

def default_config(self) -> JsonDict:
config = super().default_config()
return config

def test_create_event_present_in_invite_state(self) -> None:

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

For reference, it looks like we also have TestMSC4311FullCreateEventOnStrippedState in Complement to cover invite_state. Although that test looks flawed

It looks like that test already passes on develop 🤔 How/why would that be the case?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

It looks like this is a combination of MSC1772 but it was only a recommendation. And MSC4311 changes that to a requirement

# Per MSC1772.
(EventTypes.Create, ""),

And then there was a flawed partial MSC4311 implementation introduced in 0eb7252. I'm reverting that change in #19723

"""m.room.create must appear in invite_state."""
inviter = self.register_user("inviter", "pass")
inviter_tok = self.login("inviter", "pass")
invitee = self.register_user("invitee", "pass")

Check failure on line 417 in tests/rest/client/test_sync.py

View workflow job for this annotation

GitHub Actions / lint

Ruff (F841)

tests/rest/client/test_sync.py:417:9: F841 Local variable `invitee` is assigned to but never used
invitee_tok = self.login("invitee", "pass")

room_id = self.helper.create_room_as(inviter, tok=inviter_tok)
self.helper.invite(
room=room_id, src=inviter, targ="@invitee:test", tok=inviter_tok
Comment thread
FrenchGithubUser marked this conversation as resolved.
Outdated
)

channel = self.make_request("GET", "/sync", access_token=invitee_tok)
self.assertEqual(channel.code, 200, channel.json_body)

invite_state_events = channel.json_body["rooms"]["invite"][room_id][
"invite_state"
]["events"]
event_types = {e["type"] for e in invite_state_events}
Comment thread
FrenchGithubUser marked this conversation as resolved.
Outdated
self.assertIn(EventTypes.Create, event_types)

def test_create_event_present_in_knock_state(self) -> None:
"""m.room.create must appear in knock_state."""
host = self.register_user("host", "pass")
host_tok = self.login("host", "pass")
knocker = self.register_user("knocker", "pass")

Check failure on line 438 in tests/rest/client/test_sync.py

View workflow job for this annotation

GitHub Actions / lint

Ruff (F841)

tests/rest/client/test_sync.py:438:9: F841 Local variable `knocker` is assigned to but never used
knocker_tok = self.login("knocker", "pass")

room_id = self.helper.create_room_as(
host, is_public=False, room_version="7", tok=host_tok
)
self.helper.send_state(
room_id,
EventTypes.JoinRules,
{"join_rule": JoinRules.KNOCK},
tok=host_tok,
)

channel = self.make_request("GET", "/sync", access_token=knocker_tok)
self.assertEqual(channel.code, 200, channel.json_body)
next_batch = channel.json_body["next_batch"]
Comment thread
FrenchGithubUser marked this conversation as resolved.
Outdated

channel = self.make_request(
"POST",
f"/_matrix/client/r0/knock/{room_id}",
b"{}",
knocker_tok,
)
Comment thread
FrenchGithubUser marked this conversation as resolved.
Outdated
self.assertEqual(200, channel.code, channel.result)

channel = self.make_request(
"GET", f"/sync?since={next_batch}", access_token=knocker_tok
)
self.assertEqual(channel.code, 200, channel.json_body)

knock_state_events = channel.json_body["rooms"]["knock"][room_id][
"knock_state"
]["events"]
event_types = {e["type"] for e in knock_state_events}
self.assertIn(EventTypes.Create, event_types)


class UnreadMessagesTestCase(unittest.HomeserverTestCase):
servlets = [
synapse.rest.admin.register_servlets,
Expand Down
Loading