MSC4319: Room member events in stripped state #4319
Conversation
Signed-off-by: Kévin Commaille <[email protected]>
Signed-off-by: Kévin Commaille <[email protected]>
zecakeh
changed the title
Signed-off-by: Kévin Commaille <[email protected]>
tulir
added
proposal
Signed-off-by: Kévin Commaille <[email protected]>
zecakeh
changed the title
| display information about the sender of an invite, like their display name or avatar. | ||
|
|
||
|
|
||
| ## Potential issues |
There was a problem hiding this comment.
Clients have depended on synapse's behaviour of including the full client formatted event for the m.room.member invite event.
the-draupnir-project/Draupnir#945
In draupnir this dependency is more than just a parsing issue. The matrix-protection-suite provides a handle called handleExternalMembership which invitations are fed into from the matrix-protection-suite's conception of the timeline, which is closer to appservice push than /sync. As appservice push also pushed invitations this way.
Deduplication and buffering is then coordinated using the event id as it is identical to any other client formatted event and not special cased. So this is a deep dependency.
https://github.com/Gnuxie/matrix-protection-suite/blob/a85102c0aaa6ba73beae8fdb35600eec0999eada/src/Protection/Protection.ts#L176
https://github.com/the-draupnir-project/Draupnir/blob/main/src/protections/invitation/JoinRoomsOnInviteProtection.tsx#L86-L96
https://github.com/the-draupnir-project/Draupnir/blob/f1dad52288ff32a096cbd3ff5be110648574c51f/src/Draupnir.ts#L355
https://github.com/Gnuxie/matrix-protection-suite/blob/453e55b182282aad59db430f53549d89541a8e21/src/ClientManagement/StandardClientRooms.ts#L95-L126
https://github.com/the-draupnir-project/Draupnir/blob/f1dad52288ff32a096cbd3ff5be110648574c51f/src/DraupnirBotMode.ts#L97-L99
https://github.com/the-draupnir-project/Draupnir/blob/f1dad52288ff32a096cbd3ff5be110648574c51f/src/appservice/AppService.ts#L355
| display information about the sender of an invite, like their display name or avatar. | ||
|
|
||
|
|
||
| ## Potential issues |
There was a problem hiding this comment.
The change complicates clients that allow either /sync or appservice push to be their event source. Since now invitations sourced from /sync will be a different format to those provided in appservice push. See #4319 (comment).
There was a problem hiding this comment.
Additionally, the appservice event can't be stripped, because then you'd have two different data formats in the events array of /transactions
In this specific context, the AS API is closer to S-S than C-S /sync, as both AS and S-S send full invite events with stripped state in unsigned -> invite_room_state, while /sync flattens invite event + stripped state into an invite_state array
There was a problem hiding this comment.
I have changed the proposal in 1b81df4 to send the full event in a different place to attempt to take care of two concerns at once: the event formats should not be mixed in invite_state, and implementations rely on all fields of the full event format.
There was a problem hiding this comment.
It feels like including the event in stripped format in invite_room_state doesn't satisfy anyone, if clients are relying on having full events there? It neither provides backwards-compatibility, nor gives us a clean API. We're just going to end up continuing to be in a mess.
I think I'd suggest a fairly aggressive timeline like
- have server implementations continue to include the full event in
{invite,knock}_room_stateas a temporary, strictly time-limited measure - clients that are currently relying on the unspecified behaviour having the invite or knock event in
{invite,knock}_room_stateswitch to usingstate(with fallback to{invite,knock}_room_state, if they want compat with older servers) - after a few months, servers remove the event from
{invite,knock}_room_state. Clients still relying on the unspecced behaviour are out of luck.
Interested to hear thoughts from others on this, though
Signed-off-by: Kévin Commaille <[email protected]>
| - `m.room.encryption` | ||
|
|
||
| Although these events are useful to be able to present information about the room, they don't | ||
| contain information about the event itself. |
There was a problem hiding this comment.
| contain information about the event itself. | |
| contain information about the invite/knock event itself. |
| display information about the sender of an invite, like their display name or avatar. | ||
|
|
||
|
|
||
| ## Potential issues |
There was a problem hiding this comment.
It feels like including the event in stripped format in invite_room_state doesn't satisfy anyone, if clients are relying on having full events there? It neither provides backwards-compatibility, nor gives us a clean API. We're just going to end up continuing to be in a mess.
I think I'd suggest a fairly aggressive timeline like
- have server implementations continue to include the full event in
{invite,knock}_room_stateas a temporary, strictly time-limited measure - clients that are currently relying on the unspecified behaviour having the invite or knock event in
{invite,knock}_room_stateswitch to usingstate(with fallback to{invite,knock}_room_state, if they want compat with older servers) - after a few months, servers remove the event from
{invite,knock}_room_state. Clients still relying on the unspecced behaviour are out of luck.
Interested to hear thoughts from others on this, though
| For compatibility with the current client implementations, homeservers SHOULD also include this | ||
| event in the `events` array of the `invite_state` or `knock_state` in stripped format. | ||
|
|
||
| Clients SHOULD expect the `state` key to be missing and SHOULD look for the `m.room.member` event in | ||
| `invite_state` or `knock_state` as a fallback. |
There was a problem hiding this comment.
per my comment on the other thread, this feels a bit weak, and is just propagating the status quo.
|
|
||
| Finally, the list of events that should be included in the stripped state is extended with the | ||
| stripped `m.room.member` event of the `sender` of the invite. This allows clients to be able to | ||
| display information about the sender of an invite, like their display name or avatar. |
There was a problem hiding this comment.
In federation contexts, I don't think the server has the sender of the invite? We shouldn't include it unless it's going to work reliably.
There was a problem hiding this comment.
The list of events that should be included in the stripped state in the spec applies to the CS and SS APIs alike. This is asking for the m.room.member event of the sender to be sent in the invite_room_state and knock_room_state over federation and forwarded in stripped state to the client.
There was a problem hiding this comment.
oh, I see, right. It would probably be good to clarify that in the MSC text.
| @@ -0,0 +1,181 @@ | |||
| # MSC4139: Room member events in stripped state | |||
There was a problem hiding this comment.
It strikes me that the current proposal is not really "Room member events in stripped state"; rather, it is "add new fields [which are independent of stripped state] to the /sync response".
6 participants
This was spawned by the discussion in matrix-org/matrix-spec#2181.
Server Implementations:
m.room.memberininvite_state: https://github.com/element-hq/synapse/blob/458e6410e8388fbc6b60b5575850405939bc53eb/synapse/rest/client/sync.py#L450-L462m.room.memberininvite_state: https://github.com/element-hq/synapse/blob/a6e326582f735e03e420c3b86475388fb5f2656c/synapse/handlers/message.py#L1835-L1839m.room.memberinknock_state: https://github.com/element-hq/synapse/blob/458e6410e8388fbc6b60b5575850405939bc53eb/synapse/rest/client/sync.py#L486-L514m.room.memberininvite_state: https://github.com/element-hq/dendrite/blob/8d2da78744387e55c28bb8925ae0cc70dd3e02e3/syncapi/types/types.go#L530-L574invite_state.knock_state.m.room.memberininvite_state(common to all forks): https://gitlab.com/famedly/conduit/-/blob/ed5b0514f52f848e53785dee5fe8de63780cf092/src/api/server_server.rs#L2160-2172m.room.memberininvite_state(Conduit): https://gitlab.com/famedly/conduit/-/merge_requests/766m.room.memberinknock_state(Conduit): https://gitlab.com/famedly/conduit/-/blob/ed5b0514f52f848e53785dee5fe8de63780cf092/src/api/server_server.rs#L2160-2172Most clients already rely on being able to access the
m.room.memberevents of the knock or invite process, and of the inviter.The following client implementations also rely on being able to access the
origin_server_ts:StrippedStateand add optional fields toStrippedStateEventinstead ruma/ruma#2187Rendered