-
Notifications
You must be signed in to change notification settings - Fork 376
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
MSC2375: Appservice Invite States #2375
base: old_master
Are you sure you want to change the base?
Changes from all commits
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,73 @@ | ||
# MSC 2375: Proposal to add invite states to invites sent to Application Services | ||
Application Services are a great way to extend the homeservers functionality, e.g. by bridging matrix | ||
to other networks. | ||
|
||
When receiving an invite you might want to get additional information about the room you are invited | ||
to, such as the room name and icon to e.g. bridge them nicely to the remote network. Currently | ||
Application Services do not offer this, unlike clients. | ||
|
||
This could also be of help if an application service wants to somehow verify invites before joining | ||
rooms, e.g. if a token is present and valid in the invite state. | ||
Half-Shot marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
## Proposal | ||
This proposal is about adding `invite_room_state` to the `unsigned` block of invite events when sending | ||
Sorunome marked this conversation as resolved.
Show resolved
Hide resolved
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. Why the 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. appservices only receive invites by receiving those events alongside all other events in transactions, so putting the state into those events sounded like it would make most sense. The unsigned block inside the event was chosen to not block the path for potentially sending the federation format to ASs in the future. 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. Additionally, it appears that this is already the standard in the spec, as seen with the server-server invites, so creating multiple locations for the same thing seems wrong. That is also mentioned in the MSC
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 is only true for the v1 invite endpoint, the v2 invite endpoint puts the invite_room_state next to the event instead of into it 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. Must have slipped sorus eyes 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 still think that with the current shape of the API, the unsigned block is the best position for this 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. Unsigned is a general dumping ground for not-event-data, so this makes sense to me. |
||
them out to Application Services. | ||
|
||
As content of `invite_room_state` an array of `StrippedState` (defined [here](https://matrix.org/docs/spec/server_server/r0.1.3#put-matrix-federation-v2-invite-roomid-eventid)) | ||
objects would be used. This way it would | ||
be in line with the Server-Server API to send invites. | ||
|
||
### Example | ||
An invite event of `@alice:example.com` inviting `@bob:example.com` could look as follows: | ||
|
||
```json | ||
{ | ||
"content": { | ||
"avatar_url": "mxc://example.com/some_avatar", | ||
"displayname": "New Person", | ||
"membership": "invite" | ||
}, | ||
"event_id": "$some_event_id", | ||
"origin_server_ts": 1575368937985, | ||
"room_id": "!some_room:example.com", | ||
"sender": "@alice:example.com", | ||
"state_key": "@bob:example.com", | ||
"type": "m.room.member", | ||
"unsigned": { | ||
"age": 1167, | ||
"invite_room_state": [ | ||
{ | ||
"type": "m.room.name", | ||
"sender": "@alice:example.com", | ||
"state_key": "", | ||
"content": { | ||
"name": "Example Room" | ||
} | ||
}, | ||
{ | ||
"type": "m.room.join_rules", | ||
"sender": "@alice:example.com", | ||
"state_key": "", | ||
"content": { | ||
"join_rule": "invite" | ||
} | ||
} | ||
] | ||
} | ||
} | ||
``` | ||
|
||
# Potential issues | ||
Depending on server implementation this could be a heavy operation. Additionally more data is sent | ||
down the line to the Application Service. As this is already done for normal clients it shouldn't be | ||
too bad, though. | ||
|
||
# Alternatives | ||
Instead of aligning with S-S API `/invite` it could be aligned with C-S API `/sync` and putting the | ||
Half-Shot marked this conversation as resolved.
Show resolved
Hide resolved
|
||
invite states into a separate `rooms.invite[roomId].invite_state.events` array. This, however, makes | ||
it more complicated for application services to parse as the invite state isn't directly attached to | ||
the invite itself. | ||
|
||
# Security considerations | ||
As the invite states are presented to the invitee via other API endpoints (C-S API) this shouldn't | ||
give any new security considerations. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we need any of this? It sounds like a Synapse bug if it isn't sending
invite_room_state
to the appservice. The spec says that appservices receive the same event format as the CS API, which includesinvite_room_state
inm.room.member
events.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wouldn't that mean that the invite room state should also be included when someone else is invited to a room the current user is already a part of? Because that does not seem to be the case.
From what I can see, the spec says that
invite_room_state
may be included in the event. Maybe the way to solve this is to add conditions under which that may should instead be a should, such as when sending the event to the invitee.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
oh, hmm. I might be misinterpreting what this MSC is trying to do: is it trying to solve invites sent towards it or other people who happen to be in the room? The reason why the invite state isn't included for people already in the room is because those people already have the context of the room.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is totally to solve invites send towards it, I just said that first part to make clear that the spec is very ambiguous about whether or not that
invite_room_state
is present. Joined members can just query the room state directly, obviously.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The server should be sending it straight through to the target of the invite, whatever it is, given the current spec. The spec could probably clarify that it doesn't show up for other users though - that's not an issue for this MSC.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, but the spec doesn't say when it should show up either.
This is currently in the spec
This is the clarification that would probably solve this MSC, if this is a Synapse bug and spec ambiguity instead of actually changing the intended behaviour.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
links, people. consider the next person to try to read the thread. The spec in question is https://matrix.org/docs/spec/client_server/r0.6.1#m-room-member.
I think probably all we need is some clarifications to the spec, rather than an MSC.