MSC4311: Use full PDU's in stripped state (like invite_room_state) over federation and always include m.room.create event

changelog.d/19723.bugfix

@@ -0,0 +1 @@
+Remove flawed [MSC4311](https://github.com/matrix-org/matrix-spec-proposals/pull/4311) partial implementation: Client-side API's like `/sync` should still use stripped events.

rust/src/room_versions.rs

@@ -161,6 +161,25 @@ pub struct RoomVersion {
     /// This is similar to how doubly-linked lists can potentially not refer to previous items correctly
     /// without verifying the list's integrity, but doing it on every insert is too expensive.
     pub msc4242_state_dags: bool,
+    /// Whether the `m.room.create` event is required in
+    /// `invite_room_state`/`knock_room_state` when receiving invites/knocks over the
+    /// federation API's.
+    ///
+    /// Also determines whether we expect full PDU's in the
+    /// `invite_room_state`/`knock_room_state` in the federation API. The client API
+    /// still uses stripped state.
+    ///
+    /// According to MSC4311:
+    /// > If any of the events are not a PDU, not for the room ID specified, or fail
+    /// > signature checks, or the `m.room.create` event is missing, the receiving
+    /// > server MAY respond to invites with a `400 M_MISSING_PARAM` standard Matrix
+    /// > error (new to the endpoint). For invites to room version 12+ rooms, servers
+    /// > SHOULD rather than MAY respond to such requests with `400 M_MISSING_PARAM`.
+    ///
+    /// This does *not* determine whether we should include the `m.room.create` event in
+    /// stripped state or use full PDU's in stripped state over federation. We should
+    /// always do this.
+    msc4311_stripped_state: bool,
 }
 
 impl RoomVersion {
@@ -187,6 +206,7 @@ impl RoomVersion {
         msc4291_room_ids_as_hashes: false,
         strict_event_byte_limits_room_versions: false,
         msc4242_state_dags: false,
+        msc4311_stripped_state: false,
     };
 
     pub const V2: RoomVersion = RoomVersion {
@@ -304,6 +324,7 @@ impl RoomVersion {
         state_res: StateResolutionVersions::V2_1,
         msc4289_creator_power_enabled: true,
         msc4291_room_ids_as_hashes: true,
+        msc4311_stripped_state: true,
         ..Self::V11
     };
 

synapse/events/utils.py

@@ -1020,15 +1020,6 @@ def strip_event(event: EventBase) -> JsonDict:
     Stripped state events can only have the `sender`, `type`, `state_key` and `content`
     properties present.
     """
-    # MSC4311: Ensure the create event is available on invites and knocks.
-    # TODO: Implement the rest of MSC4311
-    if (
-        event.room_version.msc4291_room_ids_as_hashes
-        and event.type == EventTypes.Create
-        and event.get_state_key() == ""
-    ):
-        return event.get_pdu_json()
-
     return {
         "type": event.type,
         "state_key": event.state_key,
@@ -1062,3 +1053,12 @@ def parse_stripped_state_event(raw_stripped_event: Any) -> StrippedStateEvent |
             )
 
     return None
+
+
+def serialize_stripped_state_event(stripped_event: StrippedStateEvent) -> JsonDict:
+    return {
+        "type": stripped_event.type,
+        "state_key": stripped_event.state_key,
+        "sender": stripped_event.sender,
+        "content": stripped_event.content,
+    }

synapse/federation/federation_client.py

@@ -60,6 +60,7 @@
     RoomVersions,
 )
 from synapse.events import EventBase, builder, make_event_from_dict
+from synapse.events.snapshot import EventContext
 from synapse.federation.federation_base import (
     FederationBase,
     InvalidEventSignatureError,
@@ -71,7 +72,12 @@
 from synapse.http.types import QueryParams
 from synapse.logging.opentracing import SynapseTags, log_kv, set_tag, tag_args, trace
 from synapse.metrics import SERVER_NAME_LABEL
-from synapse.types import JsonDict, StrCollection, UserID, get_domain_from_id
+from synapse.types import (
+    JsonDict,
+    StrCollection,
+    UserID,
+    get_domain_from_id,
+)
 from synapse.util.async_helpers import concurrently_execute
 from synapse.util.caches.expiringcache import ExpiringCache
 from synapse.util.duration import Duration
@@ -138,6 +144,7 @@ def __init__(self, hs: "HomeServer"):
 
         self.server_name = hs.hostname
         self.signing_key = hs.signing_key
+        self._room_prejoin_state_types = hs.config.api.room_prejoin_state
 
         # Cache mapping `event_id` to a tuple of the event itself and the `pull_origin`
         # (which server we pulled the event from)
@@ -1309,12 +1316,12 @@ async def send_invite(
         self,
         destination: str,
         room_id: str,
-        event_id: str,
         pdu: EventBase,
+        context: EventContext,
     ) -> EventBase:
         room_version = await self.store.get_room_version(room_id)
 
-        content = await self._do_send_invite(destination, pdu, room_version)
+        content = await self._do_send_invite(destination, pdu, context, room_version)
 
         pdu_dict = content["event"]
 
@@ -1335,11 +1342,21 @@ async def send_invite(
         return pdu
 
     async def _do_send_invite(
-        self, destination: str, pdu: EventBase, room_version: RoomVersion
+        self,
+        destination: str,
+        pdu: EventBase,
+        context: EventContext,
+        room_version: RoomVersion,
     ) -> JsonDict:
         """Actually sends the invite, first trying v2 API and falling back to
         v1 API if necessary.
 
+        Args:
+            destination:
+            pdu: Invite event
+            context:
+            room_version:
+
         Returns:
             The event as a dict as returned by the remote server
 
@@ -1350,6 +1367,18 @@ async def _do_send_invite(
         """
         time_now = self._clock.time_msec()
 
+        # MSC4311: For the federation API, format events in `invite_room_state` as full
+        # PDU's
+        #
+        # Find the full events based on the state at the time of the invite
+        state_ids = await self.store.get_stripped_room_state_ids_from_event_context(
+            context, self._room_prejoin_state_types
+        )
+        state_events = await self.store.get_events(state_ids)
+        assert set(state_ids) == set(state_events.keys()), (
+            "We should have all events available that were set as stripped state."
+        )
+
         try:
             return await self.transport_layer.send_invite_v2(
                 destination=destination,
@@ -1358,10 +1387,19 @@ async def _do_send_invite(
                 content={
                     "event": pdu.get_pdu_json(time_now),
                     "room_version": room_version.identifier,
-                    "invite_room_state": pdu.unsigned.get("invite_room_state", []),
+                    "invite_room_state": [
+                        # Use full PDU's according to MSC4311
+                        state_event.get_pdu_json(time_now)
+                        for state_event in state_events.values()
+                    ],
                 },
             )
         except HttpResponseException as e:
+            # TODO: MSC4311: The 400 `M_MISSING_PARAM` error SHOULD be translated to a 5xx
+            # error by the sending server over the Client-Server API. This is done
+            # because there's nothing the client can materially do differently to make
+            # the request succeed.
+
             # If an error is received that is due to an unrecognised endpoint,
             # fallback to the v1 endpoint if the room uses old-style event IDs.
             # Otherwise, consider it a legitimate error and raise.
@@ -1379,12 +1417,19 @@ async def _do_send_invite(
         # Didn't work, try v1 API.
         # Note the v1 API returns a tuple of `(200, content)`
 
-        _, content = await self.transport_layer.send_invite_v1(
-            destination=destination,
-            room_id=pdu.room_id,
-            event_id=pdu.event_id,
-            content=pdu.get_pdu_json(time_now),
-        )
+        try:
+            _, content = await self.transport_layer.send_invite_v1(
+                destination=destination,
+                room_id=pdu.room_id,
+                event_id=pdu.event_id,
+                content=pdu.get_pdu_json(time_now),
+            )
+        except HttpResponseException as e:
+            # TODO: MSC4311: The 400 `M_MISSING_PARAM` error SHOULD be translated to a 5xx
+            # error by the sending server over the Client-Server API. This is done
+            # because there's nothing the client can materially do differently to make
+            # the request succeed.
+            raise e
         return content
 
     async def send_leave(self, destinations: Iterable[str], pdu: EventBase) -> None:

synapse/federation/federation_server.py

@@ -767,8 +767,27 @@ async def on_make_join_request(
         return {"event": pdu.get_templated_pdu_json(), "room_version": room_version}
 
     async def on_invite_request(
-        self, origin: str, content: JsonDict, room_version_id: str
+        self,
+        *,
+        origin: str,
+        expected_room_id: str,
+        expected_event_id: str,
+        event_json: JsonDict,
+        room_version_id: str,
     ) -> dict[str, Any]:
+        """
+        Args:
+            origin:
+            expected_room_id: The room ID specified in the
+                `/_matrix/federation/v1/invite/{roomId}/{eventId}` request that we expect to
+                match in the actual event itself.
+            expected_event_id: The event ID specified in the
+                `/_matrix/federation/v1/invite/{roomId}/{eventId}` request that we expect to
+                match in the actual event itself.
+            event_json:
+            room_version_id:
+        """
+
         room_version = KNOWN_ROOM_VERSIONS.get(room_version_id)
         if not room_version:
             raise SynapseError(
@@ -777,9 +796,21 @@ async def on_invite_request(
                 Codes.UNSUPPORTED_ROOM_VERSION,
             )
 
-        pdu = event_from_pdu_json(content, room_version)
+        pdu = event_from_pdu_json(event_json, room_version)
         origin_host, _ = parse_server_name(origin)
         await self.check_server_matches_acl(origin_host, pdu.room_id)
+        if pdu.event_id != expected_event_id:
+            raise SynapseError(
+                400,
+                Codes.INVALID_PARAM,
+                "Invite event ID must match event ID specified in the federation `/invite` request",
+            )
+        if pdu.room_id != expected_room_id:
+            raise SynapseError(
+                400,
+                Codes.INVALID_PARAM,
+                "The room_id specified in the invite event must match room ID specified in the federation `/invite` request",
+            )
         if await self._spam_checker_module_callbacks.should_drop_federated_event(pdu):
             logger.info(
                 "Federated event contains spam, dropping %s",
@@ -792,7 +823,11 @@ async def on_invite_request(
             errmsg = f"event id {pdu.event_id}: {e}"
             logger.warning("%s", errmsg)
             raise SynapseError(403, errmsg, Codes.FORBIDDEN)
-        ret_pdu = await self.handler.on_invite_request(origin, pdu, room_version)
+        ret_pdu = await self.handler.on_invite_request(
+            origin=origin,
+            event=pdu,
+            room_version=room_version,
+        )
         time_now = self._clock.time_msec()
         return {"event": ret_pdu.get_pdu_json(time_now)}
 
@@ -952,19 +987,31 @@ async def on_send_knock_request(
         Returns:
             The stripped room state.
         """
+        time_now = self._clock.time_msec()
+
         _, context = await self._on_send_membership_event(
             origin, content, Membership.KNOCK, room_id
         )
 
-        # Retrieve stripped state events from the room and send them back to the remote
-        # server. This will allow the remote server's clients to display information
-        # related to the room while the knock request is pending.
-        stripped_room_state = (
-            await self.store.get_stripped_room_state_from_event_context(
-                context, self._room_prejoin_state_types
-            )
+        # MSC4311: For the federation API, format events in `knock_room_state` as full
+        # PDU's
+        #
+        # Find the full events based on the state at the time of the knock
+        state_ids = await self.store.get_stripped_room_state_ids_from_event_context(
+            context, self._room_prejoin_state_types
+        )
+        state_events = await self.store.get_events(state_ids)
+        assert set(state_ids) == set(state_events.keys()), (
+            "We should have all events available that were set as stripped state."
         )
-        return {"knock_room_state": stripped_room_state}
+
+        return {
+            "knock_room_state": [
+                # Use full PDU's according to MSC4311
+                state_event.get_pdu_json(time_now)
+                for state_event in state_events.values()
+            ]
+        }
 
     async def _on_send_membership_event(
         self, origin: str, content: JsonDict, membership_type: str, room_id: str

synapse/federation/transport/server/federation.py

@@ -490,7 +490,11 @@ async def on_PUT(
         # state resolution algorithm, and we don't use that for processing
         # invites
         result = await self.handler.on_invite_request(
-            origin, content, room_version_id=RoomVersions.V1.identifier
+            origin=origin,
+            expected_room_id=room_id,
+            expected_event_id=event_id,
+            event_json=content,
+            room_version_id=RoomVersions.V1.identifier,
         )
 
         # V1 federation API is defined to return a content of `[200, {...}]`
@@ -512,9 +516,6 @@ async def on_PUT(
         room_id: str,
         event_id: str,
     ) -> tuple[int, JsonDict]:
-        # TODO(paul): assert that room_id/event_id parsed from path actually
-        #   match those given in content
-
         room_version = content["room_version"]
         event = content["event"]
         invite_room_state = content.get("invite_room_state", [])
@@ -523,12 +524,15 @@ async def on_PUT(
             invite_room_state = []
 
         # Synapse expects invite_room_state to be in unsigned, as it is in v1
-        # API
-
+        # API. We will sanitize this inside `on_invite_request(...)`
         event.setdefault("unsigned", {})["invite_room_state"] = invite_room_state
 
         result = await self.handler.on_invite_request(
-            origin, event, room_version_id=room_version
+            origin=origin,
+            expected_room_id=room_id,
+            expected_event_id=event_id,
+            event_json=event,
+            room_version_id=room_version,
         )
 
         # We only store invite_room_state for internal use, so remove it before