Handle rescinding invites over federation #18823
Conversation
We should send events that rescind invites over federation. Similiarly, we should handle receiving such events. Unfortunately, the protocol doesn't make it possible to fully auth such events, and so we can only handle the case where the original inviter rescinded the invite (rather than a room admin).
erikjohnston
force-pushed
the
erikj/revoke_invite_fed
branch
from
ead3a78 to
885f7cc
Compare
| and membership_event_id | ||
| in pdu.auth_event_ids() # The invite should be in the auth events of the rescission. |
There was a problem hiding this comment.
Why do we care about checking whether the invite event is in the auth events here?
There was a problem hiding this comment.
We probably don't need it I guess, I was mirroring how we test if an event is an invite rescission on the send path.
There probably isn't a way for the server to receive a leave event for an invited user that isn't a rescission?
There was a problem hiding this comment.
I was mirroring how we test if an event is an invite rescission on the send path.
Where is that?
This shotgun validation seems sketchy and flawed. Feels like some shenanigans can be had.
Since we're not in the room, we can't do auth checks since we don't have the state of the room (only the stripped state from the invite).
Perhaps we should be checking that the sender is at-least on the same server as the person who made the invite? We assume the server knows what it's doing.
Otherwise, feels like I can just craft a leave event from my evil server (not even in the room) that could trigger this. With the current check, this assumes you know the membership_event_id but that could be leaked like when I share one of the OpenTracing trace JSON files.
There was a problem hiding this comment.
I was mirroring how we test if an event is an invite rescission on the send path.
Where is that?
in the federation sender code here: https://github.com/element-hq/synapse/pull/18823/files#diff-4db75239ae91da3be91b6ccacb4dc969be778e832da2976b9b8fd628f16e6b52R655-R671
Perhaps we should be checking that the
senderis at-least on the same server as the person who made the invite? We assume the server knows what it's doing.
We only accept recessions from the original invite sender, and the check is below. Before this point we check that the event is signed by the sender's domain, so other servers shouldn't be able to spoof it.
We should add a complement test for it though.
| # If we've rescinded an invite then we want to tell the | ||
| # other server. |
There was a problem hiding this comment.
Why not use PUT /_matrix/federation/v2/send_leave/{roomId}/{eventId} for this kind of thing?
I understand how PUT /_matrix/federation/v1/send/{txnId} should also probably work but any reason beyond convenient? Perhaps this might be a problem if homeservers ignore PDU's received here from rooms they're not participating in?
There was a problem hiding this comment.
The /send_leave API is used in the other direction: where a HS not in the room wants to add an event that is in the room (e.g. when the invitee rejects an invite).
We could reuse it or add a /rescind_invite API that mirrors the /invite one, but I'm not sure there is much benefit to doing so. The reason we need a dedicated /invite is because the invitee server needs to sign the event before it gets added to the DAG
| if not is_in_room: | ||
| # Check if this is a leave event rescinding an invite |
There was a problem hiding this comment.
It makes sense to also be able to catch this scenario here but should we pass off to our normal handler of send_leave (on_send_membership_event)?
Currently, it says that it doesn't handle kicks but we could update it with the same rounded corner we're using here "We cannot fully auth the rescission event, but we can check if the sender of the leave event is the same as the invite."
There was a problem hiding this comment.
It makes sense to also be able to catch this scenario here but should we pass off to our normal handler of
send_leave(on_send_membership_event)?
I don't think so, as on_send_membership_event assumes the server is in thhe room, which is not the case here.
| membership == Membership.INVITE | ||
| and membership_event_id | ||
| and membership_event_id | ||
| in pdu.auth_event_ids() # The invite should be in the auth events of the rescission. |
There was a problem hiding this comment.
What happens if I get multiple invites from different servers? Who wins? Are all invites stored?
Can my server craft invites to a room I'm not even in? They wouldn't work to join but seems like they might appear
We should send events that rescind invites over federation.
Similarly, we should handle receiving such events. Unfortunately, the protocol doesn't make it possible to fully auth such events, and so we can only handle the case where the original inviter rescinded the invite (rather than a room admin).
Complement test: matrix-org/complement#797