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,23 @@ 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,
+    /// Determines whether a room version *SHOULD* rather than *MAY* reject invites/knocks
+    /// with invalid stripped state events.
+    ///
+    /// According to MSC4311:
+    /// > If any of the [stripped state] 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`.
+    ///
+    /// Regardless of room version (we should always do these things):
+    /// 1. The `m.room.create` event *MUST* be included in
+    ///    `invite_room_state`/`knock_room_state` when sending invites/knocks over the
+    ///    federation API's.
+    /// 2. Use full PDU's in the `invite_room_state`/`knock_room_state` in the federation
+    ///    API. The client API still uses stripped state.
+    msc4311_stripped_state: bool,
 }
 
 impl RoomVersion {
@@ -187,6 +204,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 +322,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/__init__.py

@@ -199,6 +199,46 @@ class StrippedStateEvent:
     sender: str
     content: dict[str, Any]
 
+    def as_json_dict(self) -> JsonDict:
+        """
+        Serialize to the JSON representation of 'Stripped State' according to the Matrix
+        spec
+        """
+
+        return {
+            "type": self.type,
+            "state_key": self.state_key,
+            "sender": self.sender,
+            "content": self.content,
+        }
+
+    @staticmethod
+    def from_json_dict(raw_stripped_event: JsonDict) -> "StrippedStateEvent | None":
+        """
+        Given a raw value from an event's `unsigned` field, attempt to parse it into a
+        `StrippedStateEvent`.
+        """
+        if isinstance(raw_stripped_event, dict):
+            # All of these fields are required
+            type = raw_stripped_event.get("type")
+            state_key = raw_stripped_event.get("state_key")
+            sender = raw_stripped_event.get("sender")
+            content = raw_stripped_event.get("content")
+            if (
+                isinstance(type, str)
+                and isinstance(state_key, str)
+                and isinstance(sender, str)
+                and isinstance(content, dict)
+            ):
+                return StrippedStateEvent(
+                    type=type,
+                    state_key=state_key,
+                    sender=sender,
+                    content=content,
+                )
+
+        return None
+
 
 @attr.s(slots=True, frozen=True, auto_attribs=True)
 class EventMetadata:

synapse/events/utils.py

@@ -37,7 +37,6 @@
     CANONICALJSON_MAX_INT,
     CANONICALJSON_MIN_INT,
     MAX_PDU_SIZE,
-    EventTypes,
 )
 from synapse.api.errors import Codes, SynapseError
 from synapse.logging.opentracing import SynapseTags, set_tag, trace
@@ -55,7 +54,7 @@
 from synapse.synapse_rust.types import Requester
 from synapse.types import JsonDict
 
-from . import EventBase, StrippedStateEvent
+from . import EventBase
 
 # These are imported only to re-export them (callers import them from this
 # module); listing them in __all__ stops the unused-import lint flagging them
@@ -549,45 +548,9 @@ 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,
         "content": dict(event.content),
         "sender": event.sender,
     }
-
-
-def parse_stripped_state_event(raw_stripped_event: Any) -> StrippedStateEvent | None:
-    """
-    Given a raw value from an event's `unsigned` field, attempt to parse it into a
-    `StrippedStateEvent`.
-    """
-    if isinstance(raw_stripped_event, dict):
-        # All of these fields are required
-        type = raw_stripped_event.get("type")
-        state_key = raw_stripped_event.get("state_key")
-        sender = raw_stripped_event.get("sender")
-        content = raw_stripped_event.get("content")
-        if (
-            isinstance(type, str)
-            and isinstance(state_key, str)
-            and isinstance(sender, str)
-            and isinstance(content, dict)
-        ):
-            return StrippedStateEvent(
-                type=type,
-                state_key=state_key,
-                sender=sender,
-                content=content,
-            )
-
-    return None

synapse/federation/federation_client.py

@@ -23,6 +23,7 @@
 
 import itertools
 import logging
+from http import HTTPStatus
 from typing import (
     TYPE_CHECKING,
     AbstractSet,
@@ -59,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,
@@ -70,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
@@ -1303,12 +1310,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"]
 
@@ -1329,11 +1336,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
 
@@ -1344,6 +1361,19 @@ 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(
+            pdu,
+            context,
+        )
+        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,
@@ -1352,7 +1382,11 @@ 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:
@@ -1367,18 +1401,74 @@ async def _do_send_invite(
                         "User's homeserver does not support this room version",
                         Codes.UNSUPPORTED_ROOM_VERSION,
                     )
+            # 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.
+            elif (
+                err.code == HTTPStatus.BAD_REQUEST
+                and err.errcode == Codes.MISSING_PARAM
+            ):
+                raise SynapseError(
+                    500,
+                    f"Received {HTTPStatus.BAD_REQUEST} {Codes.MISSING_PARAM} response from remote homeserver "
+                    "while trying to send the invite over federation. This indicates a compatibility problem "
+                    "between your homeserver and the homeserver you're trying to send the invite to "
+                    "(either one could be at fault).",
+                    Codes.UNKNOWN,
+                    additional_fields={
+                        "cause": err.msg,
+                        "destination_server": destination,
+                    },
+                )
             else:
                 raise err
 
         # 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:
+            # Use full PDU's for `invite_room_state` according to MSC4311
+            #
+            # With the v1 invite API, `invite_room_state` is carried inside the event
+            # instead of a separate field like in v2 so we must munge it in ourselves
+            event_json = pdu.get_pdu_json(time_now)
+            event_json.setdefault("unsigned", {})["invite_room_state"] = [
+                # Use full PDU's according to MSC4311
+                state_event.get_pdu_json(time_now)
+                for state_event in state_events.values()
+            ]
+
+            _, content = await self.transport_layer.send_invite_v1(
+                destination=destination,
+                room_id=pdu.room_id,
+                event_id=pdu.event_id,
+                content=event_json,
+            )
+        except HttpResponseException as e:
+            # 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.
+            err = e.to_synapse_error()
+            if (
+                err.code == HTTPStatus.BAD_REQUEST
+                and err.errcode == Codes.MISSING_PARAM
+            ):
+                raise SynapseError(
+                    500,
+                    f"Received {HTTPStatus.BAD_REQUEST} {Codes.MISSING_PARAM} response from remote homeserver "
+                    "while trying to send the invite over federation. This indicates a compatibility problem "
+                    "between your homeserver and the homeserver you're trying to send the invite to "
+                    "(either one could be at fault).",
+                    Codes.UNKNOWN,
+                    additional_fields={
+                        "cause": err.msg,
+                        "destination_server": destination,
+                    },
+                )
+            else:
+                raise err
         return content
 
     async def send_leave(self, destinations: Iterable[str], pdu: EventBase) -> None: