Faster joins: don't stale when an user joins during a fast join

Signed-off-by: Mathieu Velten <[email protected]>

changelog.d/14606.misc

@@ -0,0 +1 @@
+Faster joins: don't stale when another user joins during a fast join resync.

synapse/handlers/event_auth.py

@@ -202,7 +202,7 @@ async def check_restricted_join_rules(
         state_ids: StateMap[str],
         room_version: RoomVersion,
         user_id: str,
-        prev_member_event: Optional[EventBase],
+        prev_membership: Optional[str],
     ) -> None:
         """
         Check whether a user can join a room without an invite due to restricted join rules.
@@ -214,15 +214,13 @@ async def check_restricted_join_rules(
             state_ids: The state of the room as it currently is.
             room_version: The room version of the room being joined.
             user_id: The user joining the room.
-            prev_member_event: The current membership event for this user.
+            prev_membership: The current membership state for this user.
 
         Raises:
             AuthError if the user cannot join the room.
         """
         # If the member is invited or currently joined, then nothing to do.
-        if prev_member_event and (
-            prev_member_event.membership in (Membership.JOIN, Membership.INVITE)
-        ):
+        if prev_membership in (Membership.JOIN, Membership.INVITE):
             return
 
         # This is not a room with a restricted join rule, so we don't need to do the

synapse/handlers/federation.py

@@ -599,6 +599,8 @@ async def do_invite_join(
             except ValueError:
                 pass
 
+            already_partially_joined = await self.store.is_partial_state_room(room_id)
+
             ret = await self.federation_client.send_join(
                 host_list, event, room_version_obj
             )
@@ -629,7 +631,7 @@ async def do_invite_join(
                 state_events=state,
             )
 
-            if ret.partial_state:
+            if ret.partial_state and not already_partially_joined:
                 # Mark the room as having partial state.
                 # The background process is responsible for unmarking this flag,
                 # even if the join fails.
@@ -676,7 +678,7 @@ async def do_invite_join(
                 # state for the room.
                 # If the join failed, the background process is responsible for
                 # cleaning up — including unmarking the room as a partial state room.
-                if ret.partial_state:
+                if ret.partial_state and not already_partially_joined:
                     # Kick off the process of asynchronously fetching the state for this
                     # room.
                     run_as_background_process(

synapse/handlers/federation_event.py

@@ -438,16 +438,17 @@ async def check_join_restrictions(
         # 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)
-        prev_member_event = None
+        prev_membership = None
         if prev_member_event_id:
             prev_member_event = await self._store.get_event(prev_member_event_id)
+            prev_membership = prev_member_event.membership
 
         # Check if the member should be allowed access via membership in a space.
         await self._event_auth_handler.check_restricted_join_rules(
             prev_state_ids,
             event.room_version,
             user_id,
-            prev_member_event,
+            prev_membership,
         )
 
     @trace

synapse/handlers/room_member.py

@@ -823,8 +823,14 @@ async def update_membership_locked(
 
         latest_event_ids = await self.store.get_prev_events_for_room(room_id)
 
+        # There are 2 reasons that mandate to have the full state:
+        # - to calculate `is_host_in_room`, however we now approximate that we consider the server
+        # in the room if we are still in the middle of a fast join.
+        # - to calculate if another user trying to join is allowed to. In this case we now
+        # make another `make/send_join` to a resident server to validate that. We could just accept
+        # it but it would mean that we would potentially leaked the history to a banned user.
         state_before_join = await self.state_handler.compute_state_after_events(
-            room_id, latest_event_ids
+            room_id, latest_event_ids, await_full_state=False
         )
 
         # TODO: Refactor into dictionary of explicitly allowed transitions
@@ -881,7 +887,10 @@ async def update_membership_locked(
             if action == "kick":
                 raise AuthError(403, "The target user is not in the room")
 
-        is_host_in_room = await self._is_host_in_room(state_before_join)
+        is_partial_state_room = await self.store.is_partial_state_room(room_id)
+        is_host_in_room = await self._is_host_in_room(
+            is_partial_state_room, state_before_join
+        )
 
         if effective_membership_state == Membership.JOIN:
             if requester.is_guest:
@@ -927,6 +936,7 @@ async def update_membership_locked(
                 room_id,
                 remote_room_hosts,
                 content,
+                is_partial_state_room,
                 is_host_in_room,
                 state_before_join,
             )
@@ -1073,6 +1083,7 @@ async def _should_perform_remote_join(
         room_id: str,
         remote_room_hosts: List[str],
         content: JsonDict,
+        is_partial_state_room: bool,
         is_host_in_room: bool,
         state_before_join: StateMap[str],
     ) -> Tuple[bool, List[str]]:
@@ -1093,6 +1104,8 @@ async def _should_perform_remote_join(
             remote_room_hosts: A list of remote room hosts.
             content: The content to use as the event body of the join. This may
                 be modified.
+            is_partial_state_room: True if the server currently doesn't hold the fully
+                state of the room.
             is_host_in_room: True if the host is in the room.
             state_before_join: The state before the join event (i.e. the resolution of
                 the states after its parent events).
@@ -1109,6 +1122,20 @@ async def _should_perform_remote_join(
         if not is_host_in_room:
             return True, remote_room_hosts
 
+        prev_member_event_id = state_before_join.get((EventTypes.Member, user_id), None)
+        previous_membership = None
+        if prev_member_event_id:
+            prev_member_event = await self.store.get_event(prev_member_event_id)
+            previous_membership = prev_member_event.membership
+
+        # If we are not fully join yet, and the target is not already in the room,
+        # let's do a remote join so another server with the full state can validate
+        # that the user has not been ban for example.
+        # We could just accept the join and wait for state-res to resolve that later on
+        # but we would then leak room history to this person until then, which is pretty bad.
+        if is_partial_state_room and previous_membership != Membership.JOIN:
+            return True, remote_room_hosts
+
         # If the host is in the room, but not one of the authorised hosts
         # for restricted join rules, a remote join must be used.
         room_version = await self.store.get_room_version(room_id)
@@ -1122,15 +1149,8 @@ async def _should_perform_remote_join(
 
         # If the user is invited to the room or already joined, the join
         # event can always be issued locally.
-        prev_member_event_id = state_before_join.get((EventTypes.Member, user_id), None)
-        prev_member_event = None
-        if prev_member_event_id:
-            prev_member_event = await self.store.get_event(prev_member_event_id)
-            if prev_member_event.membership in (
-                Membership.JOIN,
-                Membership.INVITE,
-            ):
-                return False, []
+        if previous_membership in (Membership.JOIN, Membership.INVITE):
+            return False, []
 
         # If the local host has a user who can issue invites, then a local
         # join can be done.
@@ -1154,7 +1174,7 @@ async def _should_perform_remote_join(
 
         # Ensure the member should be allowed access via membership in a room.
         await self.event_auth_handler.check_restricted_join_rules(
-            state_before_join, room_version, user_id, prev_member_event
+            state_before_join, room_version, user_id, previous_membership
         )
 
         # If this is going to be a local join, additional information must
@@ -1634,7 +1654,13 @@ async def _make_and_store_3pid_invite(
         )
         return event, stream_id
 
-    async def _is_host_in_room(self, current_state_ids: StateMap[str]) -> bool:
+    async def _is_host_in_room(
+        self, is_partial_state_room: bool, current_state_ids: StateMap[str]
+    ) -> bool:
+        # When we only have a partial state, let's assume we are still in the room
+        if is_partial_state_room:
+            return True
+
         # Have we just created the room, and is this about to be the very
         # first member event?
         create_event_id = current_state_ids.get(("m.room.create", ""))