Fix an edge-case for being invited to a room for the spaces summary.

synapse/handlers/space_summary.py

@@ -160,7 +160,7 @@ async def get_space_summary(
 
                     # The user can see the room, include it!
                     if await self._is_remote_room_accessible(
-                        queue_entry.room_id, requester, fed_room_id, room
+                        requester, fed_room_id, room
                     ):
                         # Before returning to the client, remove the allowed_room_ids
                         # and allowed_spaces keys.
@@ -513,7 +513,7 @@ async def _is_local_room_accessible(
         return False
 
     async def _is_remote_room_accessible(
-        self, requested_room_id: str, requester: str, room_id: str, room: JsonDict
+        self, requester: str, room_id: str, room: JsonDict
     ) -> bool:
         """
         Calculate whether the room received over federation should be shown in the spaces summary.
@@ -523,14 +523,13 @@ async def _is_remote_room_accessible(
         * The requester is joined or can join the room (per MSC3173).
         * The history visibility is set to world readable.
 
-        Note that we know the user is not in the requested room (which is why the
-        remote call was made in the first place), but the user could be in one
-        of the children rooms and we just didn't know about the link.
+        Note that the local server is not in the requested room (which is why the
+        remote call was made in the first place), but the user could have access
+        due to an invite, etc.
 
         Args:
-            requested_room_id: The room ID which was requested.
             requester: The user requesting the summary.
-            room_id: The child room ID returned over federation.
+            room_id: The room ID returned over federation.
             room: The summary of the child room returned over federation.
 
         Returns:
@@ -553,13 +552,10 @@ async def _is_remote_room_accessible(
             ):
                 return True
 
-        # Finally, if this isn't the requested room, check ourselves
-        # if we can access the room.
-        if room_id != requested_room_id:
-            if await self._is_local_room_accessible(room_id, requester, None):
-                return True
-
-        return False
+        # Finally, check locally if we can access the room. The user might
+        # already be in the room (if it was a child room), or there might be a
+        # pending invite, etc.
+        return await self._is_local_room_accessible(room_id, requester, None)
 
     async def _build_room_entry(self, room_id: str, for_federation: bool) -> JsonDict:
         """

tests/handlers/test_space_summary.py

@@ -30,7 +30,7 @@
 from synapse.rest import admin
 from synapse.rest.client.v1 import login, room
 from synapse.server import HomeServer
-from synapse.types import JsonDict
+from synapse.types import JsonDict, UserID
 
 from tests import unittest
 
@@ -149,6 +149,36 @@ def _assert_events(
             events,
         )
 
+    def _poke_fed_invite(self, room_id: str, from_user: str) -> None:
+        """
+        Creates a invite (as if received over federation) for the room from the
+        given hostname.
+
+        Args:
+            room_id: The room ID to issue an invite for.
+            fed_hostname: The user to invite from.
+        """
+        # Poke an invite over federation into the database.
+        fed_handler = self.hs.get_federation_handler()
+        fed_hostname = UserID.from_string(from_user).domain
+        event = make_event_from_dict(
+            {
+                "room_id": room_id,
+                "event_id": "!abcd:" + fed_hostname,
+                "type": EventTypes.Member,
+                "sender": from_user,
+                "state_key": self.user,
+                "content": {"membership": Membership.INVITE},
+                "prev_events": [],
+                "auth_events": [],
+                "depth": 1,
+                "origin_server_ts": 1234,
+            }
+        )
+        self.get_success(
+            fed_handler.on_invite_request(fed_hostname, event, RoomVersions.V6)
+        )
+
     def test_simple_space(self):
         """Test a simple space with a single room."""
         result = self.get_success(self.handler.get_space_summary(self.user, self.space))
@@ -416,24 +446,7 @@ def test_fed_filtering(self):
         joined_room = self.helper.create_room_as(self.user, tok=self.token)
 
         # Poke an invite over federation into the database.
-        fed_handler = self.hs.get_federation_handler()
-        event = make_event_from_dict(
-            {
-                "room_id": invited_room,
-                "event_id": "!abcd:" + fed_hostname,
-                "type": EventTypes.Member,
-                "sender": "@remote:" + fed_hostname,
-                "state_key": self.user,
-                "content": {"membership": Membership.INVITE},
-                "prev_events": [],
-                "auth_events": [],
-                "depth": 1,
-                "origin_server_ts": 1234,
-            }
-        )
-        self.get_success(
-            fed_handler.on_invite_request(fed_hostname, event, RoomVersions.V6)
-        )
+        self._poke_fed_invite(invited_room, "@remote:" + fed_hostname)
 
         async def summarize_remote_room(
             _self, room, suggested_only, max_children, exclude_rooms
@@ -570,3 +583,55 @@ async def summarize_remote_room(
                 (subspace, joined_room),
             ],
         )
+
+    def test_fed_invited(self):
+        """
+        Rooms returned over federation should be properly filtered to only include
+        rooms the user has access to.
+        """
+        fed_hostname = self.hs.hostname + "2"
+        fed_room = "#subroom:" + fed_hostname
+
+        # Poke an invite over federation into the database.
+        self._poke_fed_invite(fed_room, "@remote:" + fed_hostname)
+
+        async def summarize_remote_room(
+            _self, room, suggested_only, max_children, exclude_rooms
+        ):
+            return [
+                _RoomEntry(
+                    fed_room,
+                    {
+                        "room_id": fed_room,
+                        "world_readable": False,
+                        "join_rules": JoinRules.INVITE,
+                    },
+                ),
+            ]
+
+        # Add a room to the space which is on another server.
+        self._add_child(self.space, fed_room, self.token)
+
+        with mock.patch(
+            "synapse.handlers.space_summary.SpaceSummaryHandler._summarize_remote_room",
+            new=summarize_remote_room,
+        ):
+            result = self.get_success(
+                self.handler.get_space_summary(self.user, self.space)
+            )
+
+        self._assert_rooms(
+            result,
+            [
+                self.space,
+                self.room,
+                fed_room,
+            ],
+        )
+        self._assert_events(
+            result,
+            [
+                (self.space, self.room),
+                (self.space, fed_room),
+            ],
+        )