Fix join being denied after being invited over federation

changelog.d/18075.bugfix

@@ -0,0 +1 @@
+Fix join being denied after being invited over federation. Also fixes other out-of-band membership transitions.

scripts-dev/complement.sh

@@ -220,15 +220,15 @@ extra_test_args=()
 
 test_packages=(
     ./tests/csapi
-    ./tests
-    ./tests/msc3874
-    ./tests/msc3890
-    ./tests/msc3391
-    ./tests/msc3757
-    ./tests/msc3930
-    ./tests/msc3902
-    ./tests/msc3967
-    ./tests/msc4140
+    # ./tests
+    # ./tests/msc3874
+    # ./tests/msc3890
+    # ./tests/msc3391
+    # ./tests/msc3757
+    # ./tests/msc3930
+    # ./tests/msc3902
+    # ./tests/msc3967
+    # ./tests/msc4140
 )
 
 # Enable dirty runs, so tests will reuse the same container where possible.

synapse/event_auth.py

@@ -563,9 +563,10 @@ def _is_membership_change_allowed(
     invite_level = get_named_level(auth_events, "invite", 0)
     ban_level = get_named_level(auth_events, "ban", 50)
 
-    logger.debug(
+    logger.info(
         "_is_membership_change_allowed: %s",
         {
+            "caller_membership": caller.membership if caller else None,
             "caller_in_room": caller_in_room,
             "caller_invited": caller_invited,
             "caller_knocked": caller_knocked,
@@ -677,7 +678,8 @@ def _is_membership_change_allowed(
                 and join_rule == JoinRules.KNOCK_RESTRICTED
             )
         ):
-            if not caller_in_room and not caller_invited:
+            # You can only join the room if you are invited or are already in the room.
+            if not (caller_in_room or caller_invited):
                 raise AuthError(403, "You are not invited to this room.")
         else:
             # TODO (erikj): may_join list

synapse/events/__init__.py

@@ -42,7 +42,7 @@
 from typing_extensions import Literal
 from unpaddedbase64 import encode_base64
 
-from synapse.api.constants import RelationTypes
+from synapse.api.constants import EventTypes, RelationTypes
 from synapse.api.room_versions import EventFormatVersions, RoomVersion, RoomVersions
 from synapse.synapse_rust.events import EventInternalMetadata
 from synapse.types import JsonDict, StrCollection
@@ -325,12 +325,17 @@ def __str__(self) -> str:
     def __repr__(self) -> str:
         rejection = f"REJECTED={self.rejected_reason}, " if self.rejected_reason else ""
 
+        conditional_membership_string = ""
+        if self.get("type") == EventTypes.Member:
+            conditional_membership_string = f"membership={self.membership}, "
+
         return (
             f"<{self.__class__.__name__} "
             f"{rejection}"
             f"event_id={self.event_id}, "
             f"type={self.get('type')}, "
             f"state_key={self.get('state_key')}, "
+            f"{conditional_membership_string}"
             f"outlier={self.internal_metadata.is_outlier()}"
             ">"
         )

synapse/events/builder.py

@@ -24,7 +24,7 @@
 import attr
 from signedjson.types import SigningKey
 
-from synapse.api.constants import MAX_DEPTH
+from synapse.api.constants import MAX_DEPTH, EventTypes
 from synapse.api.room_versions import (
     KNOWN_EVENT_FORMAT_VERSIONS,
     EventFormatVersions,
@@ -109,6 +109,19 @@ def state_key(self) -> str:
     def is_state(self) -> bool:
         return self._state_key is not None
 
+    def is_mine_id(self, string: str) -> bool:
+        """Determines whether a user ID or room alias originates from this homeserver.
+
+        Returns:
+            `True` if the hostname part of the user ID or room alias matches this
+            homeserver.
+            `False` otherwise, or if the user ID or room alias is malformed.
+        """
+        localpart_hostname = string.split(":", 1)
+        if len(localpart_hostname) < 2:
+            return False
+        return localpart_hostname[1] == self._hostname
+
     async def build(
         self,
         prev_event_ids: List[str],
@@ -142,6 +155,37 @@ async def build(
                 self, state_ids
             )
 
+            # Check for out-of-band membership that may that may have been exposed on
+            # `/sync` but the events have not been de-outliered yet so they won't be
+            # part of the room state yet.
+            #
+            # This helps in situations where a remote homeserver invites a local user to
+            # a room that we're already participating in; and we've persisted the invite
+            # as an out-of-band membership (outlier), but it hasn't been pushed to us as
+            # part of a `/send` transaction yet and de-outliered. This also helps for
+            # any of the other out-of-band membership transitions.
+            #
+            # As an optimization, we could check if the room state already includes a
+            # non-`leave` membership event, then we can assume the membership event has
+            # been de-outliered and we don't need to check for an out-of-band
+            # membership. But we don't have the necessary information from a
+            # `StateMap[str]` and we'll just have to take the hit of this extra lookup
+            # for any membership event for now.
+            if self.type == EventTypes.Member and self.is_mine_id(self.state_key):
+                (
+                    _membership,
+                    member_event_id,
+                ) = await self._store.get_local_current_membership_for_user_in_room(
+                    user_id=self.state_key,
+                    room_id=self.room_id,
+                )
+                if member_event_id is not None:
+                    # There is no need to check if the membership is actually an
+                    # out-of-band membership (`outlier`) as we would end up with the
+                    # same result either way (adding the member event to the
+                    # `auth_event_ids`).
+                    auth_event_ids.append(member_event_id)
+
         format_version = self.room_version.event_format
         # The types of auth/prev events changes between event versions.
         prev_events: Union[StrCollection, List[Tuple[str, Dict[str, str]]]]

synapse/federation/federation_server.py

@@ -1256,7 +1256,7 @@ async def _process_incoming_pdus_in_room_inner(
         # has started processing).
         while True:
             async with lock:
-                logger.info("handling received PDU in room %s: %s", room_id, event)
+                logger.info("📬 handling received PDU in room %s: %s", room_id, event)
                 try:
                     with nested_logging_context(event.event_id):
                         # We're taking out a lock within a lock, which could
@@ -1271,6 +1271,9 @@ async def _process_incoming_pdus_in_room_inner(
                             await self._federation_event_handler.on_receive_pdu(
                                 origin, event
                             )
+                    logger.info(
+                        "✅ handled received PDU in room %s: %s", room_id, event
+                    )
                 except FederationError as e:
                     # XXX: Ideally we'd inform the remote we failed to process
                     # the event, but we can't return an error in the transaction

synapse/handlers/federation.py

@@ -606,6 +606,8 @@ async def do_invite_join(
 
             content: The event content to use for the join event.
         """
+        logger.info("🧲 do_invite_join for %s in %s", joinee, room_id)
+
         # TODO: We should be able to call this on workers, but the upgrading of
         # room stuff after join currently doesn't work on workers.
         # TODO: Before we relax this condition, we need to allow re-syncing of
@@ -1051,6 +1053,8 @@ async def on_invite_request(
 
         Respond with the now signed event.
         """
+        logger.info("🗳️ on_invite_request: handling event %s", event)
+
         if event.state_key is None:
             raise SynapseError(400, "The invite event did not have a state key")
 
@@ -1127,6 +1131,7 @@ async def on_invite_request(
             await self.store.remove_push_actions_from_staging(event.event_id)
             raise
 
+        logger.info("✅ on_invite_request: handled event %s", event)
         return event
 
     async def do_remotely_reject_invite(

synapse/handlers/federation_event.py

@@ -382,6 +382,10 @@ async def on_send_membership_event(
             event.event_id,
             event.signatures,
         )
+        # logger.info(
+        #     "📮 on_send_membership_event: received event: %s",
+        #     event,
+        # )
 
         if get_domain_from_id(event.sender) != origin:
             logger.info(
@@ -436,6 +440,10 @@ async def on_send_membership_event(
 
         await self._check_for_soft_fail(event, context=context, origin=origin)
         await self._run_push_actions_and_persist_event(event, context)
+        # logger.info(
+        #     "✅ on_send_membership_event: handled event: %s",
+        #     event,
+        # )
         return event, context
 
     async def check_join_restrictions(
@@ -2272,10 +2280,6 @@ async def persist_events_and_notify(
                 event_and_contexts, backfilled=backfilled
             )
 
-            # After persistence we always need to notify replication there may
-            # be new data.
-            self._notifier.notify_replication()
-
             if self._ephemeral_messages_enabled:
                 for event in events:
                     # If there's an expiry timestamp on the event, schedule its expiry.

synapse/handlers/message.py

@@ -1415,6 +1415,9 @@ async def handle_new_client_event(
             PartialStateConflictError if attempting to persist a partial state event in
                 a room that has been un-partial stated.
         """
+        for event, _ in events_and_context:
+            logger.info("📮 handle_new_client_event: handling %s", event)
+
         extra_users = extra_users or []
 
         for event, context in events_and_context:
@@ -1460,7 +1463,7 @@ async def handle_new_client_event(
                         event, batched_auth_events
                     )
                 except AuthError as err:
-                    logger.warning("Denying new event %r because %s", event, err)
+                    logger.warning("❌ Denying new event %r because %s", event, err)
                     raise err
 
             # Ensure that we can round trip before trying to persist in db
@@ -1492,6 +1495,7 @@ async def handle_new_client_event(
             gather_results(deferreds, consumeErrors=True)
         ).addErrback(unwrapFirstError)
 
+        logger.info("✅ handle_new_client_event: handled %s", event)
         return result
 
     async def _persist_events(

synapse/notifier.py

@@ -308,6 +308,7 @@ async def on_new_room_events(
         notify_new_room_events with the results."""
         event_entries = []
         for event, pos in events_and_pos:
+            logger.info("📨 Notifying about new event %s", event, exc_info=True)
             entry = self.create_pending_room_event_entry(
                 pos,
                 extra_users,

synapse/storage/databases/main/events.py

@@ -2836,6 +2836,8 @@ def _store_room_members_txn(
                 for backfilled events because backfilled events in the past do
                 not affect the current local state.
         """
+        for event in events:
+            logger.info("🔦 _store_room_members_txn update room_memberships: %s", event)
 
         self.db_pool.simple_insert_many_txn(
             txn,
@@ -2892,6 +2894,10 @@ def _store_room_members_txn(
                     Membership.LEAVE,
                 )
 
+                logger.info(
+                    "🔦 _store_room_members_txn update local_current_membership: %s",
+                    event,
+                )
                 self.db_pool.simple_upsert_txn(
                     txn,
                     table="local_current_membership",