-
Notifications
You must be signed in to change notification settings - Fork 450
MSC4319: Room member events for invite and knock rooms in the /sync response
#4319
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 14 commits
bbf271f
3a07bc6
a4530d1
01c8ee4
1b81df4
f497f19
98a7a3e
3bb2ab3
e279119
3fb1fc9
ad5aa1f
0c0d731
8d67bda
8c44d6a
63aebd5
f673805
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,220 @@ | ||
| # MSC4139: Room member events for invite and knock rooms in the `/sync` response | ||
|
|
||
| In the Client-Server API, the response of the [`GET /sync`](https://spec.matrix.org/v1.15/client-server-api/#get_matrixclientv3sync) | ||
| endpoint, the `events` array in the `invite_state` for rooms under `invite` and in the `knock_state` | ||
| for rooms under `knock` are defined as containing the stripped state of the room. | ||
|
|
||
| This stripped state comes over federation, via the | ||
| [`PUT /invite`](https://spec.matrix.org/v1.15/server-server-api/#put_matrixfederationv2inviteroomideventid) | ||
| and [`PUT /send_knock`](https://spec.matrix.org/v1.15/server-server-api/#put_matrixfederationv1send_knockroomideventid) | ||
| endpoints in the Server-Server API. | ||
|
|
||
| In the definition of the [stripped state][stripped-state] | ||
| it is recommended to contain the following state events: | ||
|
|
||
| - `m.room.create` | ||
| - `m.room.name` | ||
| - `m.room.avatar` | ||
| - `m.room.topic` | ||
| - `m.room.join_rules` | ||
| - `m.room.canonical_alias` | ||
| - `m.room.encryption` | ||
|
|
||
| Although these events are useful to be able to present information about the room, they don't | ||
| contain information about the invite/knock event itself. | ||
|
|
||
| The following information can be useful for clients: | ||
|
|
||
| - The sender of the invite, including their display name and avatar, to present to the user. | ||
| - The time of the invite or knock, to present to the user or to sort the invite/knock list by | ||
| recency. | ||
| - Whether an invite event was preceded by a knock, if the client wants to auto-accept invites that | ||
| come from knocking. | ||
|
|
||
| Furthermore, appservices may want to deduplicate this event received via `/sync` from events | ||
| received via `/_matrix/app/v1/transactions`, so they need to be able to identify it uniquely. The | ||
| easiest way to do this is to use its `event_id`. | ||
|
|
||
| > [!NOTE] | ||
| > The initial purpose of this proposed change was to specify behavior that has been implemented for | ||
| > a long time in some homeserver implementations and that clients already rely on, but that is | ||
| > currently unspecced. | ||
| > | ||
| > Because current homeserver implementations differ there is no clear consensus on what this | ||
| > solution should actually look like, so the purpose of this MSC has shifted to propose a change | ||
| > that still includes all the same data necessary for clients but offers a more coherent API. | ||
|
|
||
|
|
||
| ## Proposal | ||
|
|
||
| ### `m.room.member` event with a `membership` of `invite` or `knock` | ||
|
|
||
| For clients to be able to get all the details about an invite or knock, a `state` key is added to | ||
| the [`InvitedRoom`](https://spec.matrix.org/v1.15/client-server-api/#get_matrixclientv3sync_response-200_invited-room) | ||
| and [`KnockedRoom`](https://spec.matrix.org/v1.15/client-server-api/#get_matrixclientv3sync_response-200_knocked-room) | ||
| objects. It uses the same format as the [`State`](https://spec.matrix.org/v1.15/client-server-api/#get_matrixclientv3sync_response-200_state) | ||
| object in [`JoinedRoom`](https://spec.matrix.org/v1.15/client-server-api/#get_matrixclientv3sync_response-200_joined-room) | ||
| and MUST include the `m.room.member` event that was created during the invite or knock process, in | ||
| the [`ClientEventWithoutRoomID`](https://spec.matrix.org/v1.15/client-server-api/#get_matrixclientv3sync_response-200_clienteventwithoutroomid) | ||
| format. This SHOULD be the only event present in the `events` array. | ||
|
|
||
| > [!NOTE] | ||
| > Making it mandatory makes sense because this event is the reason why the room appears in `invite` | ||
| > or `knock` in the first place. Providing the full event format allows clients to access details | ||
| > like the `origin_server_ts`, the `event_id` or the `unsigned` object. | ||
|
|
||
| > [!IMPORTANT] | ||
| > This MSC doesn't change the format of this event over the Server-Server API. This only proposes to | ||
| > forward more information to clients that has always been available to homeservers. | ||
|
|
||
| For compatibility with the current client implementations, homeservers SHOULD also continue to | ||
| include this event in the `events` array of the `invite_state` or `knock_state` in the same format | ||
| as in `State` for a time limited to 1 spec release after this proposal is released in a new spec | ||
| version. | ||
|
Comment on lines
+70
to
+73
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What other clients have relied on the full state of the The only example given so far has been Draupnir wanting
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. In the description of the PR there are a couple of clients accessing the
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This seems like much too short of a timeline. I think quite a few clients rely on this event being in the invite_state, even if it is stripped. Removing that after 3 months, when until then clients couldn't rely on it being elsewhere and breaking every client someone might have gotten from a distro repo like Ubuntu, is a bit problematic. But I guess servers will have to support it in both places for longer, if they support an old spec version?
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The version of the spec matters less here. The timeline is probably a bit short, but as a |
||
|
|
||
| For compatibility with the current server implementations, clients MAY look for the invite/knock | ||
| `m.room.member` event in `invite_state` or `knock_state` as a fallback if it is not found under the | ||
| `state` key. | ||
|
|
||
| > [!NOTE] | ||
| > The example for the response of `GET /sync` includes the stripped `m.room.member` event although | ||
| > it is not specified. | ||
|
|
||
| ### `m.room.member` event of the `sender` of the invite | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I feel like this MSC should solely propose this change on its own: Include the All of the extra complexity around clients wanting access to
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The purpose of this MSC is to specify behavior that has already been implemented for a long time in some homeservers and clients while not being part of the spec. It turns out that the current implementations rely on an undesirable API (like mixing event formats) so the extra complexity is needed to have a more consistent API at the cost of requiring changes in implementations. I don't believe that wanting to access the extra fields is a separate topic because it is all about clients having enough data to present/handle invites and knocks. Without the extra fields, the
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The stripped state version does provide value: The invite is useful to see the invite I would consider the current Synapse behavior flawed and including the full state event probably was a simple oversight (element-hq/synapse@ab9cf73). The only examples of clients that use this extra information are progressive enhancement examples. We can equally instead fix the spec and the Synapse implementation to do the right thing without the clients losing much at all for spec-compliant homeservers. The desire for more info is valid and can be separately advanced in its own MSC that adds the necessary metadata (whatever solution that may be).
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The SCT has previously expressed interest in consistency: either return full state events, or don't. There should not be a mix of formats. The SCT has also expressed that continuing to call it "stripped state" when it contains fully formatted events is wrong. There's also mild opinions around naming it specifically "[invite|knock]_state" when it's the same thing regardless of membership type - it doesn't need to be overly qualified. These considerations are mentioned somewhere in the depths of #4311, alongside other arguments not relevant to this proposal. Whether the information is valuable or not is somewhat irrelevant - the format should be consistent, and getting the information needed from an
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It is valuable to confirm that this half-baked state that this MSC ships is not great. I still think this MSC should have simply defined the extra It sounds like the way forward now that this MSC is merged is to just define that all stripped state events should be presented as full state events in
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. That would be my preferred direction, as an individual. Introducing
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
(it does, see "The stripped state version does provide value:" above) There are other use cases it doesn't cover but those are the things to be covered by the further MSC. |
||
|
|
||
| The [list of events that should be included in the stripped state][stripped-state] over the | ||
| Client-Server and Server-Server APIs is extended with the `m.room.member` event of the `sender` of | ||
| the invite for rooms that the user has been invited to. This event has the same format as other | ||
| events in the stripped state, i.e. the full event format according to the room version in the | ||
| Server-Server API endpoints, and the `Stripped state event` format in the Client-Server API's | ||
| `/sync` endpoint. This event allows clients to be able to display information about the sender of | ||
| an invite, like their display name or avatar. | ||
|
|
||
| > [!NOTE] | ||
| > For a knock the target user and the sender MUST be the same user according to the | ||
| > [authorisation rules](https://spec.matrix.org/v1.15/rooms/v11/#authorisation-rules), so the | ||
| > `m.room.member` event of the sender is the same as in the previous section and it doesn't need to | ||
| > be provided separately. | ||
|
|
||
| ### Example | ||
|
|
||
| Example of an `InvitedState` object: | ||
|
|
||
| ```json | ||
| { | ||
| "state": { | ||
| "events": [ | ||
| { | ||
| "content": { | ||
| "membership": "invite", | ||
| "displayname": "Alice" | ||
| }, | ||
| "type": "m.room.member", | ||
| "state_key": "@alice:example.org", | ||
| "sender": "@bob:example.org", | ||
| "event_id": "$Rqnc-F-dvnEYJTyHq_iKxU2bZ1CI92-kuZq3a5lr5Zg", | ||
| "origin_server_ts": 1432735824653, | ||
| "unsigned": { | ||
| "prev_content": { | ||
| "membership": "knock", | ||
| "displayname": "Alice" | ||
| } | ||
| } | ||
| } | ||
| ] | ||
| }, | ||
| "invite_state": { | ||
| "events": [ | ||
| { | ||
| "content": { | ||
| "membership": "invite", | ||
| "displayname": "Alice" | ||
| }, | ||
| "type": "m.room.member", | ||
| "state_key": "@alice:example.org", | ||
| "sender": "@bob:example.org", | ||
| }, | ||
| { | ||
| "content": { | ||
| "m.federate": true, | ||
| "predecessor": { | ||
| "event_id": "$something:example.org", | ||
| "room_id": "!oldroom:example.org" | ||
| }, | ||
| "room_version": "11" | ||
| }, | ||
| "sender": "@example:example.org", | ||
| "state_key": "", | ||
| "type": "m.room.create", | ||
| }, | ||
| { | ||
| "content": { | ||
| "membership": "join", | ||
| "displayname": "Bob" | ||
| }, | ||
| "type": "m.room.member", | ||
| "state_key": "@bob:example.org", | ||
| "sender": "@bob:example.org", | ||
| }, | ||
| ] | ||
| } | ||
| } | ||
| ``` | ||
|
|
||
| ### Dependencies | ||
|
|
||
| Although it is not considered a dependency of this MSC, if this is accepted the new `/sync` | ||
| endpoint introduced in [MSC4286: Simplified Sliding Sync](https://github.com/matrix-org/matrix-spec-proposals/pull/4186) | ||
|
zecakeh marked this conversation as resolved.
|
||
| should also be updated for invited and knocked rooms to include the full invite/knock | ||
| `m.room.member` event in `required_state` and the inviter's `m.room.member` event in | ||
| `stripped_state`. | ||
|
|
||
|
|
||
| ## Potential issues | ||
|
zecakeh marked this conversation as resolved.
zecakeh marked this conversation as resolved.
|
||
|
|
||
| This changes the current expectations of clients by moving the `m.room.member` event outside of the | ||
| `invite_state` and `knock_state` so they will need to adapt to the change. This is mitigated by | ||
| encouraging servers to keep sending the event in these objects. | ||
|
|
||
| By showing more information about the sender of an invite, users might be subject to undesirable | ||
| content like abusive language or images. Mitigating this is out of scope of this MSC, and other MSCs | ||
| exist for this, like [MSC4278](https://github.com/matrix-org/matrix-spec-proposals/pull/4278). | ||
|
|
||
|
|
||
| ## Alternatives | ||
|
|
||
| We could put the full `m.room.member` event in the `events` array of the `invite_state` or | ||
| `knock_state`, but mixing event formats in a list is undesirable. | ||
|
|
||
| We could put the full `m.room.member` event under another key in the `invite_state` or | ||
| `knock_state`, the `state` key was chosen for its similarity with other room objects. It will also | ||
| allow to add more events using their full format in the future if needed. | ||
|
|
||
| We could add more fields to the stripped state format, but given all the fields that are needed for | ||
| the different use cases, it would mean that the stripped state has the same format as normal state. | ||
| Using the full event format might give the wrong idea that this state has been validated by the | ||
| homeserver, which is currently not possible | ||
| (see [this discussion in MSC4311](https://github.com/matrix-org/matrix-spec-proposals/pull/4311#discussion_r2274781824)). | ||
|
Comment on lines
+195
to
+197
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This equally applies to the new Related reading on
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This doesn't say that the event that we place in Maybe this should be clarified in the spec PR for this MSC?
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Using full state event in I'm not seeing the benefit of the complexity around having both
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah, this introduces additional complexity, that doesn't seem that beneficial. It would be much easier and more backwards compatible to simply have those events in the invite_state as before.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The original concern of this thread regarding validation of events has a long and storied history. The conclusion is validation isn't needed right now because the data remains untrusted by nature. If/when a future MSC includes the full recursive auth chain, then servers can verify the invite. For the concern of having two fields doing different things: another MSC to deprecate |
||
| Besides, those fields are really only necessary for the `invite` or `knock` `m.room.member` event. | ||
|
|
||
| This doesn't solve the case where a room doesn't have an `m.room.name` or `m.room.canonical_alias` | ||
| state event, so [its display name should be computed using the room summary](https://spec.matrix.org/v1.15/client-server-api/#calculating-the-display-name-for-a-room). | ||
| This is left to another MSC. | ||
|
zecakeh marked this conversation as resolved.
|
||
|
|
||
|
|
||
| ## Security considerations | ||
|
|
||
| No potential security issues are known to the author. | ||
|
|
||
|
|
||
| ## Unstable prefix | ||
|
|
||
| While this proposal is not considered stable, implementations should use `org.matrix.msc4319.state` | ||
| for the `state` key in `InvitedRoom` and `KnockedRoom`. | ||
|
|
||
|
|
||
| ## Dependencies | ||
|
|
||
| None. | ||
|
|
||
| [stripped-state]: https://spec.matrix.org/v1.15/client-server-api/#stripped-state | ||
Uh oh!
There was an error while loading. Please reload this page.