diff --git a/changelog.d/17194.bugfix b/changelog.d/17194.bugfix new file mode 100644 index 0000000000..29ac56ceac --- /dev/null +++ b/changelog.d/17194.bugfix @@ -0,0 +1 @@ +Fix hierarchy returning 403 when room is accessible through federation. Contributed by Krishan (@kfiven). diff --git a/synapse/handlers/room_summary.py b/synapse/handlers/room_summary.py index 720459f1e7..64f5bea014 100644 --- a/synapse/handlers/room_summary.py +++ b/synapse/handlers/room_summary.py @@ -183,8 +183,13 @@ class RoomSummaryHandler: ) -> JsonDict: """See docstring for SpaceSummaryHandler.get_room_hierarchy.""" - # First of all, check that the room is accessible. - if not await self._is_local_room_accessible(requested_room_id, requester): + # If the room is available locally, quickly check that the user can access it. + local_room = await self._store.is_host_joined( + requested_room_id, self._server_name + ) + if local_room and not await self._is_local_room_accessible( + requested_room_id, requester + ): raise UnstableSpecAuthError( 403, "User %s not in room %s, and room previews are disabled" @@ -192,6 +197,22 @@ class RoomSummaryHandler: errcode=Codes.NOT_JOINED, ) + if not local_room: + room_hierarchy = await self._summarize_remote_room_hierarchy( + _RoomQueueEntry(requested_room_id, ()), + False, + ) + root_room_entry = room_hierarchy[0] + if not root_room_entry or not await self._is_remote_room_accessible( + requester, requested_room_id, root_room_entry.room + ): + raise UnstableSpecAuthError( + 403, + "User %s not in room %s, and room previews are disabled" + % (requester, requested_room_id), + errcode=Codes.NOT_JOINED, + ) + # If this is continuing a previous session, pull the persisted data. if from_token: try: diff --git a/tests/handlers/test_room_summary.py b/tests/handlers/test_room_summary.py index 244a4e7689..b55fa1a8fd 100644 --- a/tests/handlers/test_room_summary.py +++ b/tests/handlers/test_room_summary.py @@ -757,6 +757,54 @@ class SpaceSummaryTestCase(unittest.HomeserverTestCase): ) self._assert_hierarchy(result, expected) + def test_fed_root(self) -> None: + """ + Test if requested room is available over federation. + """ + fed_hostname = self.hs.hostname + "2" + fed_space = "#fed_space:" + fed_hostname + fed_subroom = "#fed_sub_room:" + fed_hostname + + requested_room_entry = _RoomEntry( + fed_space, + { + "room_id": fed_space, + "world_readable": True, + "room_type": RoomTypes.SPACE, + }, + [ + { + "type": EventTypes.SpaceChild, + "room_id": fed_space, + "state_key": fed_subroom, + "content": {"via": [fed_hostname]}, + } + ], + ) + child_room = { + "room_id": fed_subroom, + "world_readable": True, + } + + async def summarize_remote_room_hierarchy( + _self: Any, room: Any, suggested_only: bool + ) -> Tuple[Optional[_RoomEntry], Dict[str, JsonDict], Set[str]]: + return requested_room_entry, {fed_subroom: child_room}, set() + + expected = [ + (fed_space, [fed_subroom]), + (fed_subroom, ()), + ] + + with mock.patch( + "synapse.handlers.room_summary.RoomSummaryHandler._summarize_remote_room_hierarchy", + new=summarize_remote_room_hierarchy, + ): + result = self.get_success( + self.handler.get_room_hierarchy(create_requester(self.user), fed_space) + ) + self._assert_hierarchy(result, expected) + def test_fed_filtering(self) -> None: """ Rooms returned over federation should be properly filtered to only include