Check for space membership during a remote join of a restricted room. #9763
Conversation
|
See matrix-org/complement#99 for integration tests on this. |
| # Check if the user is already in the room or invited to the room. | ||
| user_id = event.state_key | ||
| prev_member_event_id = prev_state_ids.get((EventTypes.Member, user_id), None) | ||
| newly_joined = True | ||
| is_invite = False | ||
| if prev_member_event_id: | ||
| prev_member_event = await self.store.get_event(prev_member_event_id) | ||
| newly_joined = prev_member_event.membership != Membership.JOIN | ||
| is_invite = prev_member_event.membership == Membership.INVITE |
There was a problem hiding this comment.
Much of this logic (and the if-statement below) is now duplicated between this and local joins in RoomMemberHandler._local_membership_update. I wonder if we should instead update can_join_without_invite to raise a SynapseError and try to push some of this logic in there?
The local join logic has a couple of interleaved bits though (it needs prev_member_event_id and newly_joined for something else and it seems gross to return those from this).
There was a problem hiding this comment.
I don't think it would be the end of the world to have this logic in both _local_membership_update and can_join_without_invite, to save us from duplicating the if newly_joined and not is_invite condition. I don't think it matters too much though.
clokep
force-pushed
the
clokep/restricted-remote-joins
branch
from
b37f1a8
to
94bdb01
Compare
clokep
changed the title
| # Check if the user is already in the room or invited to the room. | ||
| user_id = event.state_key | ||
| prev_member_event_id = prev_state_ids.get((EventTypes.Member, user_id), None) | ||
| newly_joined = True | ||
| is_invite = False | ||
| if prev_member_event_id: | ||
| prev_member_event = await self.store.get_event(prev_member_event_id) | ||
| newly_joined = prev_member_event.membership != Membership.JOIN | ||
| is_invite = prev_member_event.membership == Membership.INVITE |
There was a problem hiding this comment.
I don't think it would be the end of the world to have this logic in both _local_membership_update and can_join_without_invite, to save us from duplicating the if newly_joined and not is_invite condition. I don't think it matters too much though.
Co-authored-by: Richard van der Hoff <1389908+richvdh@users.noreply.github.com>
…o clokep/restricted-remote-joins
This depends on #9735 and #9800, but applies the same logic for remote joins. It only applies this logic during the
/send_joinaspect of the handshake (not the/make_join) since this seems to be where we apply other error checking logic. I think we need to do it regardless during/send_joinso I'm not sure of the benefit of doing it in both places.Fixes #9715