From f963b8283369207027635ecb4329b5ea2e81a8a0 Mon Sep 17 00:00:00 2001 From: "H. Shay" Date: Mon, 6 Oct 2025 11:01:53 -0700 Subject: [PATCH 01/13] add admin api to search for parent space of room and other rooms in space --- synapse/rest/admin/__init__.py | 2 + synapse/rest/admin/rooms.py | 95 ++++++++++++++++++++++++++ synapse/storage/databases/main/room.py | 37 ++++++++++ 3 files changed, 134 insertions(+) diff --git a/synapse/rest/admin/__init__.py b/synapse/rest/admin/__init__.py index d9a6e99c5d3..ba8b00a4fdb 100644 --- a/synapse/rest/admin/__init__.py +++ b/synapse/rest/admin/__init__.py @@ -85,6 +85,7 @@ RoomRestV2Servlet, RoomStateRestServlet, RoomTimestampToEventRestServlet, + SpacesSearchServlet, ) from synapse.rest.admin.scheduled_tasks import ScheduledTasksRestServlet from synapse.rest.admin.server_notice_servlet import SendServerNoticeServlet @@ -339,6 +340,7 @@ def register_servlets(hs: "HomeServer", http_server: HttpServer) -> None: ExperimentalFeaturesRestServlet(hs).register(http_server) SuspendAccountRestServlet(hs).register(http_server) ScheduledTasksRestServlet(hs).register(http_server) + SpacesSearchServlet(hs).register(http_server) def register_servlets_for_client_rest_resource( diff --git a/synapse/rest/admin/rooms.py b/synapse/rest/admin/rooms.py index 5bed89c2c46..01260197409 100644 --- a/synapse/rest/admin/rooms.py +++ b/synapse/rest/admin/rooms.py @@ -63,6 +63,101 @@ logger = logging.getLogger(__name__) +class SpacesSearchServlet(RestServlet): + """ + For each room provided, get the details of all rooms which are parents to each room, + as well as any other rooms which are children of the found parent spaces + """ + + PATTERNS = admin_patterns("/rooms/space/spaces_search") + + def __init__(self, hs: "HomeServer"): + self._auth = hs.get_auth() + self._store = hs.get_datastores().main + self._storage_controllers = hs.get_storage_controllers() + + async def on_POST(self, request: SynapseRequest) -> Tuple[int, JsonDict]: + requester = await self._auth.get_user_by_req(request) + await assert_user_is_admin(self._auth, requester) + + content = parse_json_object_from_request(request) + rooms = content.get("rooms") + + if not rooms: + raise SynapseError( + HTTPStatus.BAD_REQUEST, "rooms must be provided", Codes.BAD_JSON + ) + if not isinstance(rooms, list): + raise SynapseError( + HTTPStatus.BAD_REQUEST, "rooms must be a list", Codes.BAD_JSON + ) + + async def _fetch_room_details(room_id: str, is_space: bool) -> JsonDict: + details = await self._store.get_room_with_stats(room_id) + try: + creation_event = await self._store.get_create_event_for_room(room_id) + except NotFoundError: + # the room may have been deleted + creation_event = None + aliases = await self._store.get_aliases_for_room(room_id) + + pl_event = await self._storage_controllers.state.get_current_state_event( + room_id, EventTypes.PowerLevels, "" + ) + pl_users = [] + if pl_event is not None: + users = pl_event.content.get("users", {}) + for user, level in users.items(): + if level >= 25: + pl_users.append(user) + if ( + creation_event + and creation_event.room_version.msc4289_creator_power_enabled + ): + additional_creators = creation_event.content.get( + "additional_creators", [] + ) + pl_users.extend(additional_creators) + pl_users.append(creation_event.sender) + + return { + "room_id": room_id, + "name": details.name if details else None, + "join_rules": details.join_rules if details else None, + "is_space": is_space, + "topic": details.topic if details else None, + "origin_server_ts": creation_event.origin_server_ts + if creation_event + else None, + "sender": creation_event.sender if creation_event else None, + "event_id": creation_event.event_id if creation_event else None, + "power_users": pl_users, + "deleted": True if not details or not creation_event else False, + "aliases": aliases, + } + + parents_and_children = await self._store.get_parents_and_siblings_of_rooms( + rooms + ) + room_details_list = [] + seen_parents = set() + for parent_id, child_id in parents_and_children: + if parent_id in seen_parents: + child_details = await _fetch_room_details(child_id, False) + room_details_list.append(child_details) + else: + child_details = await _fetch_room_details(child_id, False) + room_details_list.append(child_details) + parent_details = await _fetch_room_details(parent_id, True) + room_details_list.append(parent_details) + seen_parents.add(parent_id) + + if not room_details_list: + return HTTPStatus.NOT_FOUND, {} + else: + return HTTPStatus.OK, {"found_rooms": room_details_list} + + class RoomRestV2Servlet(RestServlet): """Delete a room from server asynchronously with a background task. diff --git a/synapse/storage/databases/main/room.py b/synapse/storage/databases/main/room.py index 9f03c084a59..8fe0caf1b3d 100644 --- a/synapse/storage/databases/main/room.py +++ b/synapse/storage/databases/main/room.py @@ -2783,3 +2783,40 @@ def _clear_partial_state_room_txn( WHERE stream_id <= ? """ txn.execute(sql, (device_lists_stream_id,)) + + async def get_parents_and_siblings_of_rooms( + self, room_ids: list[tuple] + ) -> list[tuple]: + """ + For each room in the list, get any parents of the room, and any children of the + parent room + + Args: + room_ids: a list of rooms to check for parent spaces + """ + + def get_parents_and_children_of_rooms_txn( + txn: LoggingTransaction, room_ids: list[str] + ) -> list[tuple]: + clause, param = make_in_list_sql_clause( + self.database_engine, "state_key", room_ids + ) + sql = f""" + SELECT room_id AS parent_space_id, state_key AS room_id + FROM current_state_events + WHERE type = 'm.space.child' + AND room_id IN ( + SELECT room_id + FROM current_state_events + WHERE {clause} + AND type = 'm.space.child' + ) + """ + txn.execute(sql, param) + return txn.fetchall() + + return await self.db_pool.runInteraction( + "get_parents_and_children_of_rooms_txn", + get_parents_and_children_of_rooms_txn, + room_ids, + ) From e66408836265afcb470b3fce329cde88864ee6a9 Mon Sep 17 00:00:00 2001 From: "H. Shay" Date: Mon, 6 Oct 2025 11:13:08 -0700 Subject: [PATCH 02/13] docs + tests --- docs/admin_api/rooms.md | 53 +++++ tests/rest/admin/test_room.py | 357 +++++++++++++++++++++++++++++++++- 2 files changed, 409 insertions(+), 1 deletion(-) diff --git a/docs/admin_api/rooms.md b/docs/admin_api/rooms.md index 12af87148df..0ce93acbee8 100644 --- a/docs/admin_api/rooms.md +++ b/docs/admin_api/rooms.md @@ -1115,3 +1115,56 @@ Example response: ] } ``` + +# Parent Space Search API + +This API allows an admin to search for room details about the parent space(s) (if any) of each room in a provided set of rooms, as well as any other children of the found parent spaces. If no parent spaces are found, will return 404. + +Request: + +``` + POST /_synapse/admin/v1/rooms/space/spaces_search +``` + +with a request body of: +```json +{ + "rooms": ["!roomID1", "!roomID2"] +} +``` + +Response: + +```json +{ + "found_rooms": + [ + { + "room_id": "!parentroomID", + "name": "parent space", + "join_rules": "public", + "is_space": true, + "topic": null, + "origin_server_ts": 234235, + "sender": "@user:somewhere.org", + "event_id": "$creationeventID", + "power_users": ["@user:somewhere.org"], + "deleted": false, + "aliases": [] + }, + { + "room_id": "!otherchildroomID", + "name": "other room in space", + "join_rules": "invite", + "is_space": false, + "topic": "dogs", + "origin_server_ts": 23523, + "sender": "@dogowner42:somewhere.org", + "event_id": "$wghhrhrehett", + "power_users": ["@dogowner42:somewhere.org", "@user:somewhere.org"], + "deleted": false, + "aliases": ["#hurrayfordogs:matrix.org"] + } + ] +} +``` diff --git a/tests/rest/admin/test_room.py b/tests/rest/admin/test_room.py index 30b2de26e4a..aec5c31cc29 100644 --- a/tests/rest/admin/test_room.py +++ b/tests/rest/admin/test_room.py @@ -31,7 +31,7 @@ from twisted.internet.testing import MemoryReactor import synapse.rest.admin -from synapse.api.constants import EventTypes, Membership, RoomTypes +from synapse.api.constants import EventContentFields, EventTypes, Membership, RoomTypes from synapse.api.errors import Codes from synapse.api.room_versions import RoomVersions from synapse.handlers.pagination import ( @@ -56,6 +56,361 @@ ONE_HOUR_IN_S = 3600 +class SpacesSearchTestCase(unittest.HomeserverTestCase): + servlets = [ + synapse.rest.admin.register_servlets, + login.register_servlets, + room.register_servlets, + ] + + def prepare(self, reactor: MemoryReactor, clock: Clock, hs: HomeServer) -> None: + # create some users + self.admin_user = self.register_user("admin", "pass", admin=True) + self.admin_user_tok = self.login("admin", "pass") + + self.other_user = self.register_user("user", "pass") + self.other_user_tok = self.login("user", "pass") + + self.third_user = self.register_user("third_user", "pass") + self.third_user_tok = self.login("third_user", "pass") + + # create some rooms with different options + self.rm1 = self.helper.create_room_as( + self.other_user, + is_public=False, + tok=self.other_user_tok, + extra_content={"name": "nefarious", "topic": "being bad"}, + ) + self.rm2 = self.helper.create_room_as( + self.third_user, + tok=self.third_user_tok, + extra_content={"name": "also nefarious"}, + ) + self.rm3 = self.helper.create_room_as( + self.admin_user, + is_public=False, + tok=self.admin_user_tok, + extra_content={ + "name": "not nefarious", + "topic": "happy things", + "creation_content": { + "additional_creators": [self.other_user, self.third_user] + }, + }, + room_version="12", + ) + self.rm4 = self.helper.create_room_as( + self.other_user, + tok=self.other_user_tok, + extra_content={"name": "not related to other rooms"}, + ) + + # create a space room + self.space_rm = self.helper.create_room_as( + self.other_user, + is_public=True, + extra_content={ + "visibility": "public", + "creation_content": {EventContentFields.ROOM_TYPE: RoomTypes.SPACE}, + }, + tok=self.other_user_tok, + ) + + # add three of the rooms to space + for state_key in [self.rm1, self.rm2, self.rm3]: + self.helper.send_state( + self.space_rm, + EventTypes.SpaceChild, + body={}, + tok=self.other_user_tok, + state_key=state_key, + ) + + def test_no_auth(self) -> None: + """ + If the requester does not provide authentication, a 401 is returned + """ + + channel = self.make_request( + "POST", + "/_synapse/admin/v1/rooms/space/spaces_search", + {"rooms": [self.rm1]}, + ) + + self.assertEqual(401, channel.code, msg=channel.json_body) + self.assertEqual(Codes.MISSING_TOKEN, channel.json_body["errcode"]) + + def test_requester_is_no_admin(self) -> None: + """ + If the requester is not a server admin, an error 403 is returned. + """ + + channel = self.make_request( + "POST", + "/_synapse/admin/v1/rooms/space/spaces_search", + {"rooms": [self.rm1]}, + access_token=self.other_user_tok, + ) + + self.assertEqual(403, channel.code, msg=channel.json_body) + self.assertEqual(Codes.FORBIDDEN, channel.json_body["errcode"]) + + def test_bad_request(self) -> None: + """ + Test that if rooms is not provided in the json body or if it is not a list, + a bad request error is returned. + """ + channel = self.make_request( + "POST", + "/_synapse/admin/v1/rooms/space/spaces_search", + {"rooms": "rooms"}, + access_token=self.admin_user_tok, + ) + self.assertEqual(400, channel.code, msg=channel.json_body) + self.assertEqual(Codes.BAD_JSON, channel.json_body["errcode"]) + + channel = self.make_request( + "POST", + "/_synapse/admin/v1/rooms/space/spaces_search", + access_token=self.admin_user_tok, + content={}, + ) + self.assertEqual(400, channel.code, msg=channel.json_body) + self.assertEqual(Codes.BAD_JSON, channel.json_body["errcode"]) + + def test_spaces_search(self) -> None: + """ + Test that when provided with a child room the details of the parent space and + child spaces are returned + """ + + channel = self.make_request( + "POST", + "/_synapse/admin/v1/rooms/space/spaces_search", + {"rooms": [self.rm1]}, + access_token=self.admin_user_tok, + ) + self.assertEqual(channel.code, 200, msg=channel.json_body) + rooms = channel.json_body["found_rooms"] + self.assertEqual(len(rooms), 4) + + rm1_found = False + rm2_found = False + rm3_found = False + spacerm_found = False + for room_result in rooms: + room_id = room_result["room_id"] + if room_id == self.rm1: + self.assertEqual(room_result["name"], "nefarious") + self.assertEqual(room_result["join_rules"], "invite") + self.assertEqual(room_result["is_space"], False) + self.assertEqual(room_result["topic"], "being bad") + self.assertEqual(room_result["sender"], self.other_user) + self.assertEqual(room_result["power_users"], [self.other_user]) + self.assertEqual(room_result["deleted"], False) + self.assertEqual(room_result["aliases"], []) + rm1_found = True + elif room_id == self.rm2: + self.assertEqual(room_result["name"], "also nefarious") + self.assertEqual(room_result["join_rules"], "public") + self.assertEqual(room_result["is_space"], False) + self.assertEqual(room_result["topic"], None) + self.assertEqual(room_result["sender"], self.third_user) + self.assertEqual(room_result["power_users"], [self.third_user]) + self.assertEqual(room_result["deleted"], False) + self.assertEqual(room_result["aliases"], []) + rm2_found = True + elif room_id == self.rm3: + self.assertEqual(room_result["name"], "not nefarious") + self.assertEqual(room_result["join_rules"], "invite") + self.assertEqual(room_result["is_space"], False) + self.assertEqual(room_result["topic"], "happy things") + self.assertEqual(room_result["sender"], self.admin_user) + self.assertCountEqual( + room_result["power_users"], + [self.admin_user, self.third_user, self.other_user], + ) + self.assertEqual(room_result["deleted"], False) + self.assertEqual(room_result["aliases"], []) + rm3_found = True + elif room_id == self.rm4: + self.fail("this room should not have been returned") + elif room_id == self.space_rm: + self.assertEqual(room_result["name"], None) + self.assertEqual(room_result["join_rules"], "public") + self.assertEqual(room_result["is_space"], True) + self.assertEqual(room_result["topic"], None) + self.assertEqual(room_result["sender"], self.other_user) + self.assertEqual(room_result["power_users"], [self.other_user]) + self.assertEqual(room_result["deleted"], False) + self.assertEqual(room_result["aliases"], []) + spacerm_found = True + else: + self.fail("unknown room returned") + self.assertEqual(rm1_found, True) + self.assertEqual(rm2_found, True) + self.assertEqual(rm3_found, True) + self.assertEqual(spacerm_found, True) + + # should be the same if rm1 and rm2 are provided, as they share the same parent space + channel = self.make_request( + "POST", + "/_synapse/admin/v1/rooms/space/spaces_search", + {"rooms": [self.rm1, self.rm2]}, + access_token=self.admin_user_tok, + ) + self.assertEqual(channel.code, 200, msg=channel.json_body) + rooms = channel.json_body["found_rooms"] + self.assertEqual(len(rooms), 4) + rm1_found = False + rm2_found = False + rm3_found = False + spacerm_found = False + for room_result in rooms: + room_id = room_result["room_id"] + if room_id == self.rm1: + self.assertEqual(room_result["name"], "nefarious") + self.assertEqual(room_result["join_rules"], "invite") + self.assertEqual(room_result["is_space"], False) + self.assertEqual(room_result["topic"], "being bad") + self.assertEqual(room_result["sender"], self.other_user) + self.assertEqual(room_result["power_users"], [self.other_user]) + self.assertEqual(room_result["deleted"], False) + self.assertEqual(room_result["aliases"], []) + rm1_found = True + elif room_id == self.rm2: + self.assertEqual(room_result["name"], "also nefarious") + self.assertEqual(room_result["join_rules"], "public") + self.assertEqual(room_result["is_space"], False) + self.assertEqual(room_result["topic"], None) + self.assertEqual(room_result["sender"], self.third_user) + self.assertEqual(room_result["power_users"], [self.third_user]) + self.assertEqual(room_result["deleted"], False) + self.assertEqual(room_result["aliases"], []) + rm2_found = True + elif room_id == self.rm3: + self.assertEqual(room_result["name"], "not nefarious") + self.assertEqual(room_result["join_rules"], "invite") + self.assertEqual(room_result["is_space"], False) + self.assertEqual(room_result["topic"], "happy things") + self.assertEqual(room_result["sender"], self.admin_user) + self.assertCountEqual( + room_result["power_users"], + [self.admin_user, self.third_user, self.other_user], + ) + self.assertEqual(room_result["deleted"], False) + self.assertEqual(room_result["aliases"], []) + rm3_found = True + elif room_id == self.rm4: + self.fail("this room should not have been returned") + elif room_id == self.space_rm: + self.assertEqual(room_result["name"], None) + self.assertEqual(room_result["join_rules"], "public") + self.assertEqual(room_result["is_space"], True) + self.assertEqual(room_result["topic"], None) + self.assertEqual(room_result["sender"], self.other_user) + self.assertEqual(room_result["power_users"], [self.other_user]) + self.assertEqual(room_result["deleted"], False) + self.assertEqual(room_result["aliases"], []) + spacerm_found = True + else: + self.fail("unknown room returned") + self.assertEqual(rm1_found, True) + self.assertEqual(rm2_found, True) + self.assertEqual(rm3_found, True) + self.assertEqual(spacerm_found, True) + + # nothing should be found for rm4 as it is not a child of any space + channel = self.make_request( + "POST", + "/_synapse/admin/v1/rooms/space/spaces_search", + {"rooms": [self.rm4]}, + access_token=self.admin_user_tok, + ) + self.assertEqual(channel.code, 404, msg=channel.json_body) + + def test_spaces_search_on_deleted_room(self) -> None: + """ + Test that when provided with a deleted child room the details of the parent space and + other child spaces are still returned + """ + channel = self.make_request( + "DELETE", + f"/_synapse/admin/v1/rooms/{self.rm1}", + {"block": True, "purge": True}, + access_token=self.admin_user_tok, + ) + self.assertEqual(channel.code, 200, msg=channel.json_body) + + channel = self.make_request( + "POST", + "/_synapse/admin/v1/rooms/space/spaces_search", + {"rooms": [self.rm1]}, + access_token=self.admin_user_tok, + ) + self.assertEqual(channel.code, 200, msg=channel.json_body) + rooms = channel.json_body["found_rooms"] + self.assertEqual(len(rooms), 4) + + rm1_found = False + rm2_found = False + rm3_found = False + spacerm_found = False + for room_result in rooms: + room_id = room_result["room_id"] + if room_id == self.rm1: + self.assertEqual(room_result["name"], None) + self.assertEqual(room_result["join_rules"], None) + self.assertEqual(room_result["is_space"], False) + self.assertEqual(room_result["topic"], None) + self.assertEqual(room_result["sender"], None) + self.assertEqual(room_result["power_users"], []) + self.assertEqual(room_result["deleted"], True) + self.assertEqual(room_result["aliases"], []) + rm1_found = True + elif room_id == self.rm2: + self.assertEqual(room_result["name"], "also nefarious") + self.assertEqual(room_result["join_rules"], "public") + self.assertEqual(room_result["is_space"], False) + self.assertEqual(room_result["topic"], None) + self.assertEqual(room_result["sender"], self.third_user) + self.assertEqual(room_result["power_users"], [self.third_user]) + self.assertEqual(room_result["deleted"], False) + self.assertEqual(room_result["aliases"], []) + rm2_found = True + elif room_id == self.rm3: + self.assertEqual(room_result["name"], "not nefarious") + self.assertEqual(room_result["join_rules"], "invite") + self.assertEqual(room_result["is_space"], False) + self.assertEqual(room_result["topic"], "happy things") + self.assertEqual(room_result["sender"], self.admin_user) + self.assertCountEqual( + room_result["power_users"], + [self.admin_user, self.third_user, self.other_user], + ) + self.assertEqual(room_result["deleted"], False) + self.assertEqual(room_result["aliases"], []) + rm3_found = True + elif room_id == self.rm4: + self.fail("this room should not have been returned") + elif room_id == self.space_rm: + self.assertEqual(room_result["name"], None) + self.assertEqual(room_result["join_rules"], "public") + self.assertEqual(room_result["is_space"], True) + self.assertEqual(room_result["topic"], None) + self.assertEqual(room_result["sender"], self.other_user) + self.assertEqual(room_result["power_users"], [self.other_user]) + self.assertEqual(room_result["deleted"], False) + self.assertEqual(room_result["aliases"], []) + spacerm_found = True + else: + self.fail("unknown room returned") + self.assertEqual(rm1_found, True) + self.assertEqual(rm2_found, True) + self.assertEqual(rm3_found, True) + self.assertEqual(spacerm_found, True) + + class DeleteRoomTestCase(unittest.HomeserverTestCase): servlets = [ synapse.rest.admin.register_servlets, From 58c6a79aa847b31e61ad47046b531ce1d099ea58 Mon Sep 17 00:00:00 2001 From: "H. Shay" Date: Mon, 6 Oct 2025 11:21:53 -0700 Subject: [PATCH 03/13] newsfragment --- changelog.d/19021.feature | 3 +++ 1 file changed, 3 insertions(+) create mode 100644 changelog.d/19021.feature diff --git a/changelog.d/19021.feature b/changelog.d/19021.feature new file mode 100644 index 00000000000..047f035c2b3 --- /dev/null +++ b/changelog.d/19021.feature @@ -0,0 +1,3 @@ +Add an [Admin API](https://element-hq.github.io/synapse/latest/usage/administration/admin_api/index.html) to allow an admin +to search for room details about the parent space(s) (if any) of each room in a provided set of rooms, as well as any +other children of the found parent spaces. \ No newline at end of file From fcb54a33799537309bf7b909587c5e5207c435cb Mon Sep 17 00:00:00 2001 From: "H. Shay" Date: Wed, 15 Oct 2025 15:26:20 -0700 Subject: [PATCH 04/13] rewrite to model after cs hierarchy endpoint --- synapse/handlers/room_summary.py | 105 +++++++++++++++---------- synapse/rest/admin/__init__.py | 4 +- synapse/rest/admin/rooms.py | 99 +++++++++++------------ synapse/storage/databases/main/room.py | 37 --------- 4 files changed, 113 insertions(+), 132 deletions(-) diff --git a/synapse/handlers/room_summary.py b/synapse/handlers/room_summary.py index 838fee6a303..a356cb3ef98 100644 --- a/synapse/handlers/room_summary.py +++ b/synapse/handlers/room_summary.py @@ -116,6 +116,8 @@ def __init__(self, hs: "HomeServer"): str, str, bool, + bool, + bool, Optional[int], Optional[int], Optional[str], @@ -133,6 +135,8 @@ async def get_room_hierarchy( requester: Requester, requested_room_id: str, suggested_only: bool = False, + omit_remote_rooms: bool = False, + admin_skip_room_check: bool = False, max_depth: Optional[int] = None, limit: Optional[int] = None, from_token: Optional[str] = None, @@ -146,6 +150,9 @@ async def get_room_hierarchy( requested_room_id: The room ID to start the hierarchy at (the "root" room). suggested_only: Whether we should only return children with the "suggested" flag set. + omit_remote_rooms: Whether to omit rooms which the server is not currently participating in + admin_skip_room_check: Whether to skip checking if the room can be accessed by the requester, + used when the requester is a server admin max_depth: The maximum depth in the tree to explore, must be a non-negative integer. @@ -173,6 +180,8 @@ async def get_room_hierarchy( requester.user.to_string(), requested_room_id, suggested_only, + omit_remote_rooms, + admin_skip_room_check, max_depth, limit, from_token, @@ -182,6 +191,8 @@ async def get_room_hierarchy( requester.user.to_string(), requested_room_id, suggested_only, + omit_remote_rooms, + admin_skip_room_check, max_depth, limit, from_token, @@ -193,6 +204,8 @@ async def _get_room_hierarchy( requester: str, requested_room_id: str, suggested_only: bool = False, + omit_remote_rooms: bool = False, + admin_skip_room_check: bool = False, max_depth: Optional[int] = None, limit: Optional[int] = None, from_token: Optional[str] = None, @@ -204,17 +217,18 @@ async def _get_room_hierarchy( 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" - % (requester, requested_room_id), - errcode=Codes.NOT_JOINED, - ) + if not admin_skip_room_check: + 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" + % (requester, requested_room_id), + errcode=Codes.NOT_JOINED, + ) - if not local_room: + if not local_room and not omit_remote_rooms: room_hierarchy = await self._summarize_remote_room_hierarchy( _RoomQueueEntry(requested_room_id, remote_room_hosts or ()), False, @@ -247,6 +261,8 @@ async def _get_room_hierarchy( or requested_room_id != pagination_session["room_id"] or suggested_only != pagination_session["suggested_only"] or max_depth != pagination_session["max_depth"] + or omit_remote_rooms != pagination_session["omit_remote_rooms"] + or admin_skip_room_check != pagination_session["admin_skip_room_check"] ): raise SynapseError(400, "Unknown pagination token", Codes.INVALID_PARAM) @@ -301,42 +317,44 @@ async def _get_room_hierarchy( None, room_id, suggested_only, + admin_skip_room_check=admin_skip_room_check, ) # Otherwise, attempt to use information for federation. else: - # A previous call might have included information for this room. - # It can be used if either: - # - # 1. The room is not a space. - # 2. The maximum depth has been achieved (since no children - # information is needed). - if queue_entry.remote_room and ( - queue_entry.remote_room.get("room_type") != RoomTypes.SPACE - or (max_depth is not None and current_depth >= max_depth) - ): - room_entry = _RoomEntry( - queue_entry.room_id, queue_entry.remote_room - ) + if not omit_remote_rooms: + # A previous call might have included information for this room. + # It can be used if either: + # + # 1. The room is not a space. + # 2. The maximum depth has been achieved (since no children + # information is needed). + if queue_entry.remote_room and ( + queue_entry.remote_room.get("room_type") != RoomTypes.SPACE + or (max_depth is not None and current_depth >= max_depth) + ): + room_entry = _RoomEntry( + queue_entry.room_id, queue_entry.remote_room + ) - # If the above isn't true, attempt to fetch the room - # information over federation. - else: - ( - room_entry, - children_room_entries, - inaccessible_children, - ) = await self._summarize_remote_room_hierarchy( - queue_entry, - suggested_only, - ) + # If the above isn't true, attempt to fetch the room + # information over federation. + else: + ( + room_entry, + children_room_entries, + inaccessible_children, + ) = await self._summarize_remote_room_hierarchy( + queue_entry, + suggested_only, + ) - # Ensure this room is accessible to the requester (and not just - # the homeserver). - if room_entry and not await self._is_remote_room_accessible( - requester, queue_entry.room_id, room_entry.room - ): - room_entry = None + # Ensure this room is accessible to the requester (and not just + # the homeserver). + if room_entry and not await self._is_remote_room_accessible( + requester, queue_entry.room_id, room_entry.room + ): + room_entry = None # This room has been processed and should be ignored if it appears # elsewhere in the hierarchy. @@ -378,6 +396,8 @@ async def _get_room_hierarchy( "room_id": requested_room_id, "suggested_only": suggested_only, "max_depth": max_depth, + "omit_remote_rooms": omit_remote_rooms, + "admin_skip_room_check": admin_skip_room_check, # The stored state. "room_queue": [ attr.astuple(room_entry) for room_entry in room_queue @@ -460,6 +480,7 @@ async def _summarize_local_room( room_id: str, suggested_only: bool, include_children: bool = True, + admin_skip_room_check: bool = False, ) -> Optional["_RoomEntry"]: """ Generate a room entry and a list of event entries for a given room. @@ -480,7 +501,9 @@ async def _summarize_local_room( Returns: A room entry if the room should be returned. None, otherwise. """ - if not await self._is_local_room_accessible(room_id, requester, origin): + if not admin_skip_room_check and not await self._is_local_room_accessible( + room_id, requester, origin + ): return None room_entry = await self._build_room_entry(room_id, for_federation=bool(origin)) diff --git a/synapse/rest/admin/__init__.py b/synapse/rest/admin/__init__.py index ba8b00a4fdb..9ce498b452d 100644 --- a/synapse/rest/admin/__init__.py +++ b/synapse/rest/admin/__init__.py @@ -71,6 +71,7 @@ RegistrationTokenRestServlet, ) from synapse.rest.admin.rooms import ( + AdminRoomHierarchy, BlockRoomRestServlet, DeleteRoomStatusByDeleteIdRestServlet, DeleteRoomStatusByRoomIdRestServlet, @@ -85,7 +86,6 @@ RoomRestV2Servlet, RoomStateRestServlet, RoomTimestampToEventRestServlet, - SpacesSearchServlet, ) from synapse.rest.admin.scheduled_tasks import ScheduledTasksRestServlet from synapse.rest.admin.server_notice_servlet import SendServerNoticeServlet @@ -340,7 +340,7 @@ def register_servlets(hs: "HomeServer", http_server: HttpServer) -> None: ExperimentalFeaturesRestServlet(hs).register(http_server) SuspendAccountRestServlet(hs).register(http_server) ScheduledTasksRestServlet(hs).register(http_server) - SpacesSearchServlet(hs).register(http_server) + AdminRoomHierarchy(hs).register(http_server) def register_servlets_for_client_rest_resource( diff --git a/synapse/rest/admin/rooms.py b/synapse/rest/admin/rooms.py index 01260197409..3891e196263 100644 --- a/synapse/rest/admin/rooms.py +++ b/synapse/rest/admin/rooms.py @@ -63,41 +63,44 @@ logger = logging.getLogger(__name__) -class SpacesSearchServlet(RestServlet): +class AdminRoomHierarchy(RestServlet): """ - For each room provided, get the details of all rooms which are parents to each room, - as well as any other rooms which are children of the found parent spaces + Given a room returns room details on that room and any children of the provided room. + Does not return information about remote rooms which the server is not currently + participating in """ - PATTERNS = admin_patterns("/rooms/space/spaces_search") + PATTERNS = admin_patterns("/rooms/(?P[^/]*)/hierarchy$") def __init__(self, hs: "HomeServer"): self._auth = hs.get_auth() + self._room_summary_handler = hs.get_room_summary_handler() self._store = hs.get_datastores().main self._storage_controllers = hs.get_storage_controllers() - async def on_POST(self, request: SynapseRequest) -> Tuple[int, JsonDict]: + async def on_GET( + self, request: SynapseRequest, room_id: str + ) -> Tuple[int, JsonDict]: requester = await self._auth.get_user_by_req(request) await assert_user_is_admin(self._auth, requester) - content = parse_json_object_from_request(request) - rooms = content.get("rooms") + max_depth = parse_integer(request, "max_depth") + limit = parse_integer(request, "limit") - if not rooms: - raise SynapseError( - HTTPStatus.BAD_REQUEST, "rooms must be provided", Codes.BAD_JSON - ) - if not isinstance(rooms, list): - raise SynapseError( - HTTPStatus.BAD_REQUEST, "rooms must be a list", Codes.BAD_JSON - ) + room_entry_summary = await self._room_summary_handler.get_room_hierarchy( + requester, + room_id, + omit_remote_rooms=True, + admin_skip_room_check=True, + max_depth=max_depth, + limit=limit, + from_token=parse_string(request, "from"), + ) - async def _fetch_room_details(room_id: str, is_space: bool) -> JsonDict: - details = await self._store.get_room_with_stats(room_id) + async def _fetch_additional_details(room_id: str) -> JsonDict: try: creation_event = await self._store.get_create_event_for_room(room_id) except NotFoundError: - # the room may have been deleted creation_event = None aliases = await self._store.get_aliases_for_room(room_id) @@ -119,43 +122,35 @@ async def _fetch_room_details(room_id: str, is_space: bool) -> JsonDict: ) pl_users.extend(additional_creators) pl_users.append(creation_event.sender) - - return { - "room_id": room_id, - "name": details.name if details else None, - "join_rules": details.join_rules if details else None, - "is_space": is_space, - "topic": details.topic if details else None, - "origin_server_ts": creation_event.origin_server_ts + additional_details = { + "aliases": aliases, + "power_users": pl_users, + "room_creation_ts": creation_event.origin_server_ts + if creation_event + else None, + "creator": creation_event.sender if creation_event else None, + "creation_event_id": creation_event.event_id if creation_event else None, - "sender": creation_event.sender if creation_event else None, - "event_id": creation_event.event_id if creation_event else None, - "power_users": pl_users, - "deleted": True if not details or not creation_event else False, - "aliases": aliases, } - - parents_and_children = await self._store.get_parents_and_siblings_of_rooms( - rooms - ) - room_details_list = [] - seen_parents = set() - for parent_id, child_id in parents_and_children: - if parent_id in seen_parents: - child_details = await _fetch_room_details(child_id, False) - room_details_list.append(child_details) - else: - child_details = await _fetch_room_details(child_id, False) - room_details_list.append(child_details) - parent_details = await _fetch_room_details(parent_id, True) - room_details_list.append(parent_details) - seen_parents.add(parent_id) - - if not room_details_list: - return HTTPStatus.NOT_FOUND, {} - else: - return HTTPStatus.OK, {"found_rooms": room_details_list} + return additional_details + + rooms = room_entry_summary.get("rooms") + if rooms is not None: + for room in rooms: + room_id = room.get("room_id") + assert room_id is not None + # fetch additional details of interest not provided by room summary + additional_details = await _fetch_additional_details(room_id) + room.update(additional_details) + # add any missing fields + if not room.get("name"): + room.update({"name": None}) + if not room.get("topic"): + room.update({"topic": None}) + is_space = len(room.get("children_state")) > 0 + room.update({"is_space": is_space}) + return HTTPStatus.OK, room_entry_summary class RoomRestV2Servlet(RestServlet): diff --git a/synapse/storage/databases/main/room.py b/synapse/storage/databases/main/room.py index 8fe0caf1b3d..9f03c084a59 100644 --- a/synapse/storage/databases/main/room.py +++ b/synapse/storage/databases/main/room.py @@ -2783,40 +2783,3 @@ def _clear_partial_state_room_txn( WHERE stream_id <= ? """ txn.execute(sql, (device_lists_stream_id,)) - - async def get_parents_and_siblings_of_rooms( - self, room_ids: list[tuple] - ) -> list[tuple]: - """ - For each room in the list, get any parents of the room, and any children of the - parent room - - Args: - room_ids: a list of rooms to check for parent spaces - """ - - def get_parents_and_children_of_rooms_txn( - txn: LoggingTransaction, room_ids: list[str] - ) -> list[tuple]: - clause, param = make_in_list_sql_clause( - self.database_engine, "state_key", room_ids - ) - sql = f""" - SELECT room_id AS parent_space_id, state_key AS room_id - FROM current_state_events - WHERE type = 'm.space.child' - AND room_id IN ( - SELECT room_id - FROM current_state_events - WHERE {clause} - AND type = 'm.space.child' - ) - """ - txn.execute(sql, param) - return txn.fetchall() - - return await self.db_pool.runInteraction( - "get_parents_and_children_of_rooms_txn", - get_parents_and_children_of_rooms_txn, - room_ids, - ) From 5f50fd5c8a04ebddf1629a0ffaf01425bce98f67 Mon Sep 17 00:00:00 2001 From: "H. Shay" Date: Wed, 15 Oct 2025 15:28:11 -0700 Subject: [PATCH 05/13] update docs and tests --- docs/admin_api/rooms.md | 99 ++++++++++------- tests/rest/admin/test_room.py | 196 ++++++++++------------------------ 2 files changed, 114 insertions(+), 181 deletions(-) diff --git a/docs/admin_api/rooms.md b/docs/admin_api/rooms.md index 0ce93acbee8..6d6f7be4096 100644 --- a/docs/admin_api/rooms.md +++ b/docs/admin_api/rooms.md @@ -1116,55 +1116,76 @@ Example response: } ``` -# Parent Space Search API +# Admin Room Hierarchy Endpoint -This API allows an admin to search for room details about the parent space(s) (if any) of each room in a provided set of rooms, as well as any other children of the found parent spaces. If no parent spaces are found, will return 404. +This API allows an admin to fetch the room hierarchy for a given room, returning details about that room and any children +the room may have. This is functionally similar to the [CS Hierarchy](https://spec.matrix.org/v1.16/client-server-api/#get_matrixclientv1roomsroomidhierarchy) endpoint but does not return information about any remote +rooms that the server is not currently participating in and does not check for room membership when returning room summaries. -Request: +**Parameters** -``` - POST /_synapse/admin/v1/rooms/space/spaces_search -``` +The following query parameters are available: -with a request body of: -```json -{ - "rooms": ["!roomID1", "!roomID2"] -} +* `from` - An optional pagination token, provided when there are more rooms to return than the limit. +* `limit` - Maximum amount of rooms to return. Must be a non-negative integer, defaults to `50`. +* `max_depth` - The maximum depth in the tree to explore, must be a non-negative integer. If not provided will recurse + into the space tree without limit. + +Request: + +```http +GET /_synapse/admin/v1/rooms//hierarchy ``` Response: ```json { - "found_rooms": + "rooms": [ - { - "room_id": "!parentroomID", - "name": "parent space", - "join_rules": "public", - "is_space": true, - "topic": null, - "origin_server_ts": 234235, - "sender": "@user:somewhere.org", - "event_id": "$creationeventID", - "power_users": ["@user:somewhere.org"], - "deleted": false, - "aliases": [] - }, - { - "room_id": "!otherchildroomID", - "name": "other room in space", - "join_rules": "invite", - "is_space": false, - "topic": "dogs", - "origin_server_ts": 23523, - "sender": "@dogowner42:somewhere.org", - "event_id": "$wghhrhrehett", - "power_users": ["@dogowner42:somewhere.org", "@user:somewhere.org"], - "deleted": false, - "aliases": ["#hurrayfordogs:matrix.org"] - } - ] + {"aliases": [], + "children_state": [ + { + "content": { + "via": ["local_test_server"] + }, + "origin_server_ts": 1500, + "sender": "@user:test", + "state_key": "!QrMkkqBSwYRIFNFCso:test", + "type": "m.space.child" + } + ], + "creation_event_id": "$bVkNVtm4aDw4c0LRf_U5Ad7mZSo4WKzzQKImrk_rQcg", + "creator": "@user:test", + "guest_can_join": false, + "is_space": true, + "join_rule": "public", + "name": null, + "num_joined_members": 1, + "power_users": ["@user:test"], + "room_creation_ts": 1400, + "room_id": "!sPOpNyMHbZAoAOsOFL:test", + "room_type": "m.space", + "topic": null, + "world_readable": false + }, + + { + "aliases": [], + "children_state": [], + "creation_event_id": "$kymNeN-gA5kzLwZ6FEQUu0_2MfeenYKINSO3dUuLYf8", + "creator": "@user:test", + "guest_can_join": true, + "is_space": false, + "join_rule": "invite", + "name": "nefarious", + "num_joined_members": 1, + "power_users": ["@user:test"], + "room_creation_ts": 999, + "room_id": "!QrMkkqBSwYRIFNFCso:test", + "topic": "being bad", + "world_readable": false} + ], + "next_batch": "KUYmRbeSpAoaAIgOKGgyaCEn" } ``` diff --git a/tests/rest/admin/test_room.py b/tests/rest/admin/test_room.py index aec5c31cc29..bdd6ef71c74 100644 --- a/tests/rest/admin/test_room.py +++ b/tests/rest/admin/test_room.py @@ -56,7 +56,7 @@ ONE_HOUR_IN_S = 3600 -class SpacesSearchTestCase(unittest.HomeserverTestCase): +class AdminHierarchyTestCase(unittest.HomeserverTestCase): servlets = [ synapse.rest.admin.register_servlets, login.register_servlets, @@ -121,7 +121,7 @@ def prepare(self, reactor: MemoryReactor, clock: Clock, hs: HomeServer) -> None: self.helper.send_state( self.space_rm, EventTypes.SpaceChild, - body={}, + body={"via": ["local_test_server"]}, tok=self.other_user_tok, state_key=state_key, ) @@ -132,9 +132,8 @@ def test_no_auth(self) -> None: """ channel = self.make_request( - "POST", - "/_synapse/admin/v1/rooms/space/spaces_search", - {"rooms": [self.rm1]}, + "GET", + f"/_synapse/admin/v1/rooms/{self.space_rm}/hierarchy", ) self.assertEqual(401, channel.code, msg=channel.json_body) @@ -146,9 +145,8 @@ def test_requester_is_no_admin(self) -> None: """ channel = self.make_request( - "POST", - "/_synapse/admin/v1/rooms/space/spaces_search", - {"rooms": [self.rm1]}, + "GET", + f"/_synapse/admin/v1/rooms/{self.space_rm}/hierarchy", access_token=self.other_user_tok, ) @@ -157,41 +155,36 @@ def test_requester_is_no_admin(self) -> None: def test_bad_request(self) -> None: """ - Test that if rooms is not provided in the json body or if it is not a list, - a bad request error is returned. + Test that invalid param values raise an erro """ channel = self.make_request( - "POST", - "/_synapse/admin/v1/rooms/space/spaces_search", - {"rooms": "rooms"}, + "GET", + f"/_synapse/admin/v1/rooms/{self.space_rm}/hierarchy?limit=ten", access_token=self.admin_user_tok, ) self.assertEqual(400, channel.code, msg=channel.json_body) - self.assertEqual(Codes.BAD_JSON, channel.json_body["errcode"]) + self.assertEqual(Codes.INVALID_PARAM, channel.json_body["errcode"]) channel = self.make_request( - "POST", - "/_synapse/admin/v1/rooms/space/spaces_search", + "GET", + f"/_synapse/admin/v1/rooms/{self.space_rm}/hierarchy?max_depth=four", access_token=self.admin_user_tok, - content={}, ) self.assertEqual(400, channel.code, msg=channel.json_body) - self.assertEqual(Codes.BAD_JSON, channel.json_body["errcode"]) + self.assertEqual(Codes.INVALID_PARAM, channel.json_body["errcode"]) - def test_spaces_search(self) -> None: + def test_room_summary(self) -> None: """ - Test that when provided with a child room the details of the parent space and - child spaces are returned + Test that details of room and details of children of room are provided correctly """ channel = self.make_request( - "POST", - "/_synapse/admin/v1/rooms/space/spaces_search", - {"rooms": [self.rm1]}, + "GET", + f"/_synapse/admin/v1/rooms/{self.space_rm}/hierarchy", access_token=self.admin_user_tok, ) self.assertEqual(channel.code, 200, msg=channel.json_body) - rooms = channel.json_body["found_rooms"] + rooms = channel.json_body["rooms"] self.assertEqual(len(rooms), 4) rm1_found = False @@ -202,115 +195,43 @@ def test_spaces_search(self) -> None: room_id = room_result["room_id"] if room_id == self.rm1: self.assertEqual(room_result["name"], "nefarious") - self.assertEqual(room_result["join_rules"], "invite") - self.assertEqual(room_result["is_space"], False) - self.assertEqual(room_result["topic"], "being bad") - self.assertEqual(room_result["sender"], self.other_user) - self.assertEqual(room_result["power_users"], [self.other_user]) - self.assertEqual(room_result["deleted"], False) - self.assertEqual(room_result["aliases"], []) - rm1_found = True - elif room_id == self.rm2: - self.assertEqual(room_result["name"], "also nefarious") - self.assertEqual(room_result["join_rules"], "public") - self.assertEqual(room_result["is_space"], False) - self.assertEqual(room_result["topic"], None) - self.assertEqual(room_result["sender"], self.third_user) - self.assertEqual(room_result["power_users"], [self.third_user]) - self.assertEqual(room_result["deleted"], False) - self.assertEqual(room_result["aliases"], []) - rm2_found = True - elif room_id == self.rm3: - self.assertEqual(room_result["name"], "not nefarious") - self.assertEqual(room_result["join_rules"], "invite") - self.assertEqual(room_result["is_space"], False) - self.assertEqual(room_result["topic"], "happy things") - self.assertEqual(room_result["sender"], self.admin_user) - self.assertCountEqual( - room_result["power_users"], - [self.admin_user, self.third_user, self.other_user], - ) - self.assertEqual(room_result["deleted"], False) - self.assertEqual(room_result["aliases"], []) - rm3_found = True - elif room_id == self.rm4: - self.fail("this room should not have been returned") - elif room_id == self.space_rm: - self.assertEqual(room_result["name"], None) - self.assertEqual(room_result["join_rules"], "public") - self.assertEqual(room_result["is_space"], True) - self.assertEqual(room_result["topic"], None) - self.assertEqual(room_result["sender"], self.other_user) - self.assertEqual(room_result["power_users"], [self.other_user]) - self.assertEqual(room_result["deleted"], False) - self.assertEqual(room_result["aliases"], []) - spacerm_found = True - else: - self.fail("unknown room returned") - self.assertEqual(rm1_found, True) - self.assertEqual(rm2_found, True) - self.assertEqual(rm3_found, True) - self.assertEqual(spacerm_found, True) - - # should be the same if rm1 and rm2 are provided, as they share the same parent space - channel = self.make_request( - "POST", - "/_synapse/admin/v1/rooms/space/spaces_search", - {"rooms": [self.rm1, self.rm2]}, - access_token=self.admin_user_tok, - ) - self.assertEqual(channel.code, 200, msg=channel.json_body) - rooms = channel.json_body["found_rooms"] - self.assertEqual(len(rooms), 4) - rm1_found = False - rm2_found = False - rm3_found = False - spacerm_found = False - for room_result in rooms: - room_id = room_result["room_id"] - if room_id == self.rm1: - self.assertEqual(room_result["name"], "nefarious") - self.assertEqual(room_result["join_rules"], "invite") + self.assertEqual(room_result["join_rule"], "invite") self.assertEqual(room_result["is_space"], False) self.assertEqual(room_result["topic"], "being bad") - self.assertEqual(room_result["sender"], self.other_user) + self.assertEqual(room_result["creator"], self.other_user) self.assertEqual(room_result["power_users"], [self.other_user]) - self.assertEqual(room_result["deleted"], False) self.assertEqual(room_result["aliases"], []) rm1_found = True elif room_id == self.rm2: self.assertEqual(room_result["name"], "also nefarious") - self.assertEqual(room_result["join_rules"], "public") + self.assertEqual(room_result["join_rule"], "public") self.assertEqual(room_result["is_space"], False) self.assertEqual(room_result["topic"], None) - self.assertEqual(room_result["sender"], self.third_user) + self.assertEqual(room_result["creator"], self.third_user) self.assertEqual(room_result["power_users"], [self.third_user]) - self.assertEqual(room_result["deleted"], False) self.assertEqual(room_result["aliases"], []) rm2_found = True elif room_id == self.rm3: self.assertEqual(room_result["name"], "not nefarious") - self.assertEqual(room_result["join_rules"], "invite") + self.assertEqual(room_result["join_rule"], "invite") self.assertEqual(room_result["is_space"], False) self.assertEqual(room_result["topic"], "happy things") - self.assertEqual(room_result["sender"], self.admin_user) + self.assertEqual(room_result["creator"], self.admin_user) self.assertCountEqual( room_result["power_users"], [self.admin_user, self.third_user, self.other_user], ) - self.assertEqual(room_result["deleted"], False) self.assertEqual(room_result["aliases"], []) rm3_found = True elif room_id == self.rm4: self.fail("this room should not have been returned") elif room_id == self.space_rm: self.assertEqual(room_result["name"], None) - self.assertEqual(room_result["join_rules"], "public") + self.assertEqual(room_result["join_rule"], "public") self.assertEqual(room_result["is_space"], True) self.assertEqual(room_result["topic"], None) - self.assertEqual(room_result["sender"], self.other_user) + self.assertEqual(room_result["creator"], self.other_user) self.assertEqual(room_result["power_users"], [self.other_user]) - self.assertEqual(room_result["deleted"], False) self.assertEqual(room_result["aliases"], []) spacerm_found = True else: @@ -320,37 +241,32 @@ def test_spaces_search(self) -> None: self.assertEqual(rm3_found, True) self.assertEqual(spacerm_found, True) - # nothing should be found for rm4 as it is not a child of any space - channel = self.make_request( - "POST", - "/_synapse/admin/v1/rooms/space/spaces_search", - {"rooms": [self.rm4]}, - access_token=self.admin_user_tok, - ) - self.assertEqual(channel.code, 404, msg=channel.json_body) - - def test_spaces_search_on_deleted_room(self) -> None: + def test_room_summary_pagination(self) -> None: """ - Test that when provided with a deleted child room the details of the parent space and - other child spaces are still returned + Test that details of room and details of children of room are provided correctly + when paginating """ + channel = self.make_request( - "DELETE", - f"/_synapse/admin/v1/rooms/{self.rm1}", - {"block": True, "purge": True}, + "GET", + f"/_synapse/admin/v1/rooms/{self.space_rm}/hierarchy?limit=2", access_token=self.admin_user_tok, ) self.assertEqual(channel.code, 200, msg=channel.json_body) + rooms = channel.json_body["rooms"] + self.assertEqual(len(rooms), 2) + next_batch = channel.json_body["next_batch"] - channel = self.make_request( - "POST", - "/_synapse/admin/v1/rooms/space/spaces_search", - {"rooms": [self.rm1]}, + channel2 = self.make_request( + "GET", + f"/_synapse/admin/v1/rooms/{self.space_rm}/hierarchy?from={next_batch}", access_token=self.admin_user_tok, ) - self.assertEqual(channel.code, 200, msg=channel.json_body) - rooms = channel.json_body["found_rooms"] - self.assertEqual(len(rooms), 4) + self.assertEqual(channel2.code, 200, msg=channel2.json_body) + new_rooms = channel2.json_body["rooms"] + self.assertEqual(len(new_rooms), 2) + + rooms = rooms + new_rooms rm1_found = False rm2_found = False @@ -359,48 +275,44 @@ def test_spaces_search_on_deleted_room(self) -> None: for room_result in rooms: room_id = room_result["room_id"] if room_id == self.rm1: - self.assertEqual(room_result["name"], None) - self.assertEqual(room_result["join_rules"], None) + self.assertEqual(room_result["name"], "nefarious") + self.assertEqual(room_result["join_rule"], "invite") self.assertEqual(room_result["is_space"], False) - self.assertEqual(room_result["topic"], None) - self.assertEqual(room_result["sender"], None) - self.assertEqual(room_result["power_users"], []) - self.assertEqual(room_result["deleted"], True) + self.assertEqual(room_result["topic"], "being bad") + self.assertEqual(room_result["creator"], self.other_user) + self.assertEqual(room_result["power_users"], [self.other_user]) self.assertEqual(room_result["aliases"], []) rm1_found = True elif room_id == self.rm2: self.assertEqual(room_result["name"], "also nefarious") - self.assertEqual(room_result["join_rules"], "public") + self.assertEqual(room_result["join_rule"], "public") self.assertEqual(room_result["is_space"], False) self.assertEqual(room_result["topic"], None) - self.assertEqual(room_result["sender"], self.third_user) + self.assertEqual(room_result["creator"], self.third_user) self.assertEqual(room_result["power_users"], [self.third_user]) - self.assertEqual(room_result["deleted"], False) self.assertEqual(room_result["aliases"], []) rm2_found = True elif room_id == self.rm3: self.assertEqual(room_result["name"], "not nefarious") - self.assertEqual(room_result["join_rules"], "invite") + self.assertEqual(room_result["join_rule"], "invite") self.assertEqual(room_result["is_space"], False) self.assertEqual(room_result["topic"], "happy things") - self.assertEqual(room_result["sender"], self.admin_user) + self.assertEqual(room_result["creator"], self.admin_user) self.assertCountEqual( room_result["power_users"], [self.admin_user, self.third_user, self.other_user], ) - self.assertEqual(room_result["deleted"], False) self.assertEqual(room_result["aliases"], []) rm3_found = True elif room_id == self.rm4: self.fail("this room should not have been returned") elif room_id == self.space_rm: self.assertEqual(room_result["name"], None) - self.assertEqual(room_result["join_rules"], "public") + self.assertEqual(room_result["join_rule"], "public") self.assertEqual(room_result["is_space"], True) self.assertEqual(room_result["topic"], None) - self.assertEqual(room_result["sender"], self.other_user) + self.assertEqual(room_result["creator"], self.other_user) self.assertEqual(room_result["power_users"], [self.other_user]) - self.assertEqual(room_result["deleted"], False) self.assertEqual(room_result["aliases"], []) spacerm_found = True else: From 76b86cffa44b668cf307bc110342aeea6b85c18e Mon Sep 17 00:00:00 2001 From: "H. Shay" Date: Wed, 15 Oct 2025 15:30:54 -0700 Subject: [PATCH 06/13] update newsfragment --- changelog.d/19021.feature | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/changelog.d/19021.feature b/changelog.d/19021.feature index 047f035c2b3..723e35774cf 100644 --- a/changelog.d/19021.feature +++ b/changelog.d/19021.feature @@ -1,3 +1,2 @@ Add an [Admin API](https://element-hq.github.io/synapse/latest/usage/administration/admin_api/index.html) to allow an admin -to search for room details about the parent space(s) (if any) of each room in a provided set of rooms, as well as any -other children of the found parent spaces. \ No newline at end of file +to search for room details and any children in a provided room. \ No newline at end of file From 501257e4d3dcf897cf720d05a311f828d1cfeaaa Mon Sep 17 00:00:00 2001 From: "H. Shay" Date: Fri, 17 Oct 2025 11:18:30 -0700 Subject: [PATCH 07/13] requested changes --- docs/admin_api/rooms.md | 9 ++- synapse/handlers/room_summary.py | 28 ++++---- synapse/rest/admin/rooms.py | 55 +-------------- tests/rest/admin/test_room.py | 117 ++++++++++++------------------- 4 files changed, 66 insertions(+), 143 deletions(-) diff --git a/docs/admin_api/rooms.md b/docs/admin_api/rooms.md index 6d6f7be4096..1a652743309 100644 --- a/docs/admin_api/rooms.md +++ b/docs/admin_api/rooms.md @@ -1116,10 +1116,10 @@ Example response: } ``` -# Admin Room Hierarchy Endpoint +# Admin Space Hierarchy Endpoint -This API allows an admin to fetch the room hierarchy for a given room, returning details about that room and any children -the room may have. This is functionally similar to the [CS Hierarchy](https://spec.matrix.org/v1.16/client-server-api/#get_matrixclientv1roomsroomidhierarchy) endpoint but does not return information about any remote +This API allows an admin to fetch the room hierarchy for a given space, returning details about that room and any children +the room may have, paginating over the space tree in a depth-first manner to locate child rooms. This is functionally similar to the [CS Hierarchy](https://spec.matrix.org/v1.16/client-server-api/#get_matrixclientv1roomsroomidhierarchy) endpoint but does not return information about any remote rooms that the server is not currently participating in and does not check for room membership when returning room summaries. **Parameters** @@ -1128,8 +1128,7 @@ The following query parameters are available: * `from` - An optional pagination token, provided when there are more rooms to return than the limit. * `limit` - Maximum amount of rooms to return. Must be a non-negative integer, defaults to `50`. -* `max_depth` - The maximum depth in the tree to explore, must be a non-negative integer. If not provided will recurse - into the space tree without limit. +* `max_depth` - The maximum depth in the tree to explore, must be a non-negative integer. 0 would correspond to just the root room, 1 would include just the root room's children, etc. If not provided will recurse into the space tree without limit. Request: diff --git a/synapse/handlers/room_summary.py b/synapse/handlers/room_summary.py index a356cb3ef98..ccf493cd177 100644 --- a/synapse/handlers/room_summary.py +++ b/synapse/handlers/room_summary.py @@ -136,7 +136,7 @@ async def get_room_hierarchy( requested_room_id: str, suggested_only: bool = False, omit_remote_rooms: bool = False, - admin_skip_room_check: bool = False, + admin_skip_room_visibility_check: bool = False, max_depth: Optional[int] = None, limit: Optional[int] = None, from_token: Optional[str] = None, @@ -151,7 +151,7 @@ async def get_room_hierarchy( suggested_only: Whether we should only return children with the "suggested" flag set. omit_remote_rooms: Whether to omit rooms which the server is not currently participating in - admin_skip_room_check: Whether to skip checking if the room can be accessed by the requester, + admin_skip_room_visibility_check: Whether to skip checking if the room can be accessed by the requester, used when the requester is a server admin max_depth: The maximum depth in the tree to explore, must be a non-negative integer. @@ -181,7 +181,7 @@ async def get_room_hierarchy( requested_room_id, suggested_only, omit_remote_rooms, - admin_skip_room_check, + admin_skip_room_visibility_check, max_depth, limit, from_token, @@ -192,7 +192,7 @@ async def get_room_hierarchy( requested_room_id, suggested_only, omit_remote_rooms, - admin_skip_room_check, + admin_skip_room_visibility_check, max_depth, limit, from_token, @@ -205,7 +205,7 @@ async def _get_room_hierarchy( requested_room_id: str, suggested_only: bool = False, omit_remote_rooms: bool = False, - admin_skip_room_check: bool = False, + admin_skip_room_visibility_check: bool = False, max_depth: Optional[int] = None, limit: Optional[int] = None, from_token: Optional[str] = None, @@ -217,7 +217,7 @@ async def _get_room_hierarchy( local_room = await self._store.is_host_joined( requested_room_id, self._server_name ) - if not admin_skip_room_check: + if not admin_skip_room_visibility_check: if local_room and not await self._is_local_room_accessible( requested_room_id, requester ): @@ -262,7 +262,8 @@ async def _get_room_hierarchy( or suggested_only != pagination_session["suggested_only"] or max_depth != pagination_session["max_depth"] or omit_remote_rooms != pagination_session["omit_remote_rooms"] - or admin_skip_room_check != pagination_session["admin_skip_room_check"] + or admin_skip_room_visibility_check + != pagination_session["admin_skip_room_visibility_check"] ): raise SynapseError(400, "Unknown pagination token", Codes.INVALID_PARAM) @@ -317,7 +318,7 @@ async def _get_room_hierarchy( None, room_id, suggested_only, - admin_skip_room_check=admin_skip_room_check, + admin_skip_room_visibility_check=admin_skip_room_visibility_check, ) # Otherwise, attempt to use information for federation. @@ -397,7 +398,7 @@ async def _get_room_hierarchy( "suggested_only": suggested_only, "max_depth": max_depth, "omit_remote_rooms": omit_remote_rooms, - "admin_skip_room_check": admin_skip_room_check, + "admin_skip_room_visibility_check": admin_skip_room_visibility_check, # The stored state. "room_queue": [ attr.astuple(room_entry) for room_entry in room_queue @@ -480,7 +481,7 @@ async def _summarize_local_room( room_id: str, suggested_only: bool, include_children: bool = True, - admin_skip_room_check: bool = False, + admin_skip_room_visibility_check: bool = False, ) -> Optional["_RoomEntry"]: """ Generate a room entry and a list of event entries for a given room. @@ -497,12 +498,15 @@ async def _summarize_local_room( Otherwise, all children are returned. include_children: Whether to include the events of any children. + admin_skip_room_visibility_check: Whether to skip checking if the room can be accessed by the requester, + used when the requester is a server admin Returns: A room entry if the room should be returned. None, otherwise. """ - if not admin_skip_room_check and not await self._is_local_room_accessible( - room_id, requester, origin + if ( + not admin_skip_room_visibility_check + and not await self._is_local_room_accessible(room_id, requester, origin) ): return None diff --git a/synapse/rest/admin/rooms.py b/synapse/rest/admin/rooms.py index 3891e196263..6c6e9fc5d66 100644 --- a/synapse/rest/admin/rooms.py +++ b/synapse/rest/admin/rooms.py @@ -91,65 +91,12 @@ async def on_GET( requester, room_id, omit_remote_rooms=True, - admin_skip_room_check=True, + admin_skip_room_visibility_check=True, max_depth=max_depth, limit=limit, from_token=parse_string(request, "from"), ) - async def _fetch_additional_details(room_id: str) -> JsonDict: - try: - creation_event = await self._store.get_create_event_for_room(room_id) - except NotFoundError: - creation_event = None - aliases = await self._store.get_aliases_for_room(room_id) - - pl_event = await self._storage_controllers.state.get_current_state_event( - room_id, EventTypes.PowerLevels, "" - ) - pl_users = [] - if pl_event is not None: - users = pl_event.content.get("users", {}) - for user, level in users.items(): - if level >= 25: - pl_users.append(user) - if ( - creation_event - and creation_event.room_version.msc4289_creator_power_enabled - ): - additional_creators = creation_event.content.get( - "additional_creators", [] - ) - pl_users.extend(additional_creators) - pl_users.append(creation_event.sender) - additional_details = { - "aliases": aliases, - "power_users": pl_users, - "room_creation_ts": creation_event.origin_server_ts - if creation_event - else None, - "creator": creation_event.sender if creation_event else None, - "creation_event_id": creation_event.event_id - if creation_event - else None, - } - return additional_details - - rooms = room_entry_summary.get("rooms") - if rooms is not None: - for room in rooms: - room_id = room.get("room_id") - assert room_id is not None - # fetch additional details of interest not provided by room summary - additional_details = await _fetch_additional_details(room_id) - room.update(additional_details) - # add any missing fields - if not room.get("name"): - room.update({"name": None}) - if not room.get("topic"): - room.update({"topic": None}) - is_space = len(room.get("children_state")) > 0 - room.update({"is_space": is_space}) return HTTPStatus.OK, room_entry_summary diff --git a/tests/rest/admin/test_room.py b/tests/rest/admin/test_room.py index bdd6ef71c74..153c2d673df 100644 --- a/tests/rest/admin/test_room.py +++ b/tests/rest/admin/test_room.py @@ -155,7 +155,7 @@ def test_requester_is_no_admin(self) -> None: def test_bad_request(self) -> None: """ - Test that invalid param values raise an erro + Test that invalid param values raise an error """ channel = self.make_request( "GET", @@ -186,60 +186,47 @@ def test_room_summary(self) -> None: self.assertEqual(channel.code, 200, msg=channel.json_body) rooms = channel.json_body["rooms"] self.assertEqual(len(rooms), 4) + self.assertCountEqual( + [self.rm1, self.rm2, self.rm3, self.space_rm], + [rm["room_id"] for rm in rooms], + ) - rm1_found = False - rm2_found = False - rm3_found = False - spacerm_found = False for room_result in rooms: room_id = room_result["room_id"] if room_id == self.rm1: self.assertEqual(room_result["name"], "nefarious") - self.assertEqual(room_result["join_rule"], "invite") - self.assertEqual(room_result["is_space"], False) self.assertEqual(room_result["topic"], "being bad") - self.assertEqual(room_result["creator"], self.other_user) - self.assertEqual(room_result["power_users"], [self.other_user]) - self.assertEqual(room_result["aliases"], []) - rm1_found = True + self.assertEqual(room_result["join_rule"], "invite") + self.assertEqual(len(room_result["children_state"]), 0) + (self.assertEqual(room_result["world_readable"], False),) + self.assertEqual(room_result["guest_can_join"], True) + self.assertEqual(room_result["num_joined_members"], 1) elif room_id == self.rm2: self.assertEqual(room_result["name"], "also nefarious") self.assertEqual(room_result["join_rule"], "public") - self.assertEqual(room_result["is_space"], False) - self.assertEqual(room_result["topic"], None) - self.assertEqual(room_result["creator"], self.third_user) - self.assertEqual(room_result["power_users"], [self.third_user]) - self.assertEqual(room_result["aliases"], []) - rm2_found = True + self.assertEqual(len(room_result["children_state"]), 0) + (self.assertEqual(room_result["world_readable"], False),) + self.assertEqual(room_result["guest_can_join"], False) + self.assertEqual(room_result["num_joined_members"], 1) elif room_id == self.rm3: self.assertEqual(room_result["name"], "not nefarious") self.assertEqual(room_result["join_rule"], "invite") - self.assertEqual(room_result["is_space"], False) self.assertEqual(room_result["topic"], "happy things") - self.assertEqual(room_result["creator"], self.admin_user) - self.assertCountEqual( - room_result["power_users"], - [self.admin_user, self.third_user, self.other_user], - ) - self.assertEqual(room_result["aliases"], []) - rm3_found = True + self.assertEqual(len(room_result["children_state"]), 0) + (self.assertEqual(room_result["world_readable"], False),) + self.assertEqual(room_result["guest_can_join"], True) + self.assertEqual(room_result["num_joined_members"], 1) elif room_id == self.rm4: self.fail("this room should not have been returned") elif room_id == self.space_rm: - self.assertEqual(room_result["name"], None) self.assertEqual(room_result["join_rule"], "public") - self.assertEqual(room_result["is_space"], True) - self.assertEqual(room_result["topic"], None) - self.assertEqual(room_result["creator"], self.other_user) - self.assertEqual(room_result["power_users"], [self.other_user]) - self.assertEqual(room_result["aliases"], []) - spacerm_found = True + self.assertEqual(len(room_result["children_state"]), 3) + self.assertEqual(room_result["room_type"], "m.space") + (self.assertEqual(room_result["world_readable"], False),) + self.assertEqual(room_result["guest_can_join"], False) + self.assertEqual(room_result["num_joined_members"], 1) else: self.fail("unknown room returned") - self.assertEqual(rm1_found, True) - self.assertEqual(rm2_found, True) - self.assertEqual(rm3_found, True) - self.assertEqual(spacerm_found, True) def test_room_summary_pagination(self) -> None: """ @@ -265,62 +252,48 @@ def test_room_summary_pagination(self) -> None: self.assertEqual(channel2.code, 200, msg=channel2.json_body) new_rooms = channel2.json_body["rooms"] self.assertEqual(len(new_rooms), 2) - rooms = rooms + new_rooms + self.assertCountEqual( + [self.rm1, self.rm2, self.rm3, self.space_rm], + [rm["room_id"] for rm in rooms], + ) - rm1_found = False - rm2_found = False - rm3_found = False - spacerm_found = False for room_result in rooms: room_id = room_result["room_id"] if room_id == self.rm1: self.assertEqual(room_result["name"], "nefarious") - self.assertEqual(room_result["join_rule"], "invite") - self.assertEqual(room_result["is_space"], False) self.assertEqual(room_result["topic"], "being bad") - self.assertEqual(room_result["creator"], self.other_user) - self.assertEqual(room_result["power_users"], [self.other_user]) - self.assertEqual(room_result["aliases"], []) - rm1_found = True + self.assertEqual(room_result["join_rule"], "invite") + self.assertEqual(len(room_result["children_state"]), 0) + (self.assertEqual(room_result["world_readable"], False),) + self.assertEqual(room_result["guest_can_join"], True) + self.assertEqual(room_result["num_joined_members"], 1) elif room_id == self.rm2: self.assertEqual(room_result["name"], "also nefarious") self.assertEqual(room_result["join_rule"], "public") - self.assertEqual(room_result["is_space"], False) - self.assertEqual(room_result["topic"], None) - self.assertEqual(room_result["creator"], self.third_user) - self.assertEqual(room_result["power_users"], [self.third_user]) - self.assertEqual(room_result["aliases"], []) - rm2_found = True + self.assertEqual(len(room_result["children_state"]), 0) + (self.assertEqual(room_result["world_readable"], False),) + self.assertEqual(room_result["guest_can_join"], False) + self.assertEqual(room_result["num_joined_members"], 1) elif room_id == self.rm3: self.assertEqual(room_result["name"], "not nefarious") self.assertEqual(room_result["join_rule"], "invite") - self.assertEqual(room_result["is_space"], False) self.assertEqual(room_result["topic"], "happy things") - self.assertEqual(room_result["creator"], self.admin_user) - self.assertCountEqual( - room_result["power_users"], - [self.admin_user, self.third_user, self.other_user], - ) - self.assertEqual(room_result["aliases"], []) - rm3_found = True + self.assertEqual(len(room_result["children_state"]), 0) + (self.assertEqual(room_result["world_readable"], False),) + self.assertEqual(room_result["guest_can_join"], True) + self.assertEqual(room_result["num_joined_members"], 1) elif room_id == self.rm4: self.fail("this room should not have been returned") elif room_id == self.space_rm: - self.assertEqual(room_result["name"], None) self.assertEqual(room_result["join_rule"], "public") - self.assertEqual(room_result["is_space"], True) - self.assertEqual(room_result["topic"], None) - self.assertEqual(room_result["creator"], self.other_user) - self.assertEqual(room_result["power_users"], [self.other_user]) - self.assertEqual(room_result["aliases"], []) - spacerm_found = True + self.assertEqual(len(room_result["children_state"]), 3) + self.assertEqual(room_result["room_type"], "m.space") + (self.assertEqual(room_result["world_readable"], False),) + self.assertEqual(room_result["guest_can_join"], False) + self.assertEqual(room_result["num_joined_members"], 1) else: self.fail("unknown room returned") - self.assertEqual(rm1_found, True) - self.assertEqual(rm2_found, True) - self.assertEqual(rm3_found, True) - self.assertEqual(spacerm_found, True) class DeleteRoomTestCase(unittest.HomeserverTestCase): From 6d8c5c0901a13cb1b4517116a3bfcfe7d95e79cd Mon Sep 17 00:00:00 2001 From: "H. Shay" Date: Mon, 20 Oct 2025 13:18:51 -0700 Subject: [PATCH 08/13] requested changes --- docs/admin_api/rooms.md | 2 +- synapse/handlers/room_summary.py | 67 ++++++++++++++++---------------- synapse/rest/admin/rooms.py | 5 ++- tests/rest/admin/test_room.py | 24 +++++++++--- 4 files changed, 56 insertions(+), 42 deletions(-) diff --git a/docs/admin_api/rooms.md b/docs/admin_api/rooms.md index 1a652743309..f2b56c663a1 100644 --- a/docs/admin_api/rooms.md +++ b/docs/admin_api/rooms.md @@ -1118,7 +1118,7 @@ Example response: # Admin Space Hierarchy Endpoint -This API allows an admin to fetch the room hierarchy for a given space, returning details about that room and any children +This API allows an admin to fetch the space/room hierarchy for a given space, returning details about that room and any children the room may have, paginating over the space tree in a depth-first manner to locate child rooms. This is functionally similar to the [CS Hierarchy](https://spec.matrix.org/v1.16/client-server-api/#get_matrixclientv1roomsroomidhierarchy) endpoint but does not return information about any remote rooms that the server is not currently participating in and does not check for room membership when returning room summaries. diff --git a/synapse/handlers/room_summary.py b/synapse/handlers/room_summary.py index ccf493cd177..6c722004fc0 100644 --- a/synapse/handlers/room_summary.py +++ b/synapse/handlers/room_summary.py @@ -152,7 +152,7 @@ async def get_room_hierarchy( flag set. omit_remote_rooms: Whether to omit rooms which the server is not currently participating in admin_skip_room_visibility_check: Whether to skip checking if the room can be accessed by the requester, - used when the requester is a server admin + used for the admin endpoints. max_depth: The maximum depth in the tree to explore, must be a non-negative integer. @@ -322,40 +322,39 @@ async def _get_room_hierarchy( ) # Otherwise, attempt to use information for federation. - else: - if not omit_remote_rooms: - # A previous call might have included information for this room. - # It can be used if either: - # - # 1. The room is not a space. - # 2. The maximum depth has been achieved (since no children - # information is needed). - if queue_entry.remote_room and ( - queue_entry.remote_room.get("room_type") != RoomTypes.SPACE - or (max_depth is not None and current_depth >= max_depth) - ): - room_entry = _RoomEntry( - queue_entry.room_id, queue_entry.remote_room - ) + elif not omit_remote_rooms: + # A previous call might have included information for this room. + # It can be used if either: + # + # 1. The room is not a space. + # 2. The maximum depth has been achieved (since no children + # information is needed). + if queue_entry.remote_room and ( + queue_entry.remote_room.get("room_type") != RoomTypes.SPACE + or (max_depth is not None and current_depth >= max_depth) + ): + room_entry = _RoomEntry( + queue_entry.room_id, queue_entry.remote_room + ) - # If the above isn't true, attempt to fetch the room - # information over federation. - else: - ( - room_entry, - children_room_entries, - inaccessible_children, - ) = await self._summarize_remote_room_hierarchy( - queue_entry, - suggested_only, - ) + # If the above isn't true, attempt to fetch the room + # information over federation. + else: + ( + room_entry, + children_room_entries, + inaccessible_children, + ) = await self._summarize_remote_room_hierarchy( + queue_entry, + suggested_only, + ) - # Ensure this room is accessible to the requester (and not just - # the homeserver). - if room_entry and not await self._is_remote_room_accessible( - requester, queue_entry.room_id, room_entry.room - ): - room_entry = None + # Ensure this room is accessible to the requester (and not just + # the homeserver). + if room_entry and not await self._is_remote_room_accessible( + requester, queue_entry.room_id, room_entry.room + ): + room_entry = None # This room has been processed and should be ignored if it appears # elsewhere in the hierarchy. @@ -499,7 +498,7 @@ async def _summarize_local_room( include_children: Whether to include the events of any children. admin_skip_room_visibility_check: Whether to skip checking if the room can be accessed by the requester, - used when the requester is a server admin + used for the admin endpoints. Returns: A room entry if the room should be returned. None, otherwise. diff --git a/synapse/rest/admin/rooms.py b/synapse/rest/admin/rooms.py index 6c6e9fc5d66..81ec043015f 100644 --- a/synapse/rest/admin/rooms.py +++ b/synapse/rest/admin/rooms.py @@ -65,7 +65,7 @@ class AdminRoomHierarchy(RestServlet): """ - Given a room returns room details on that room and any children of the provided room. + Given a room, returns room details on that room and any space children of the provided room. Does not return information about remote rooms which the server is not currently participating in """ @@ -87,6 +87,9 @@ async def on_GET( max_depth = parse_integer(request, "max_depth") limit = parse_integer(request, "limit") + # we omit returning remote rooms that the server is not currently participating in, + # as that information shouldn't be available to the server admin (as they are not + # participating in those rooms) room_entry_summary = await self._room_summary_handler.get_room_hierarchy( requester, room_id, diff --git a/tests/rest/admin/test_room.py b/tests/rest/admin/test_room.py index 153c2d673df..0676658ef4a 100644 --- a/tests/rest/admin/test_room.py +++ b/tests/rest/admin/test_room.py @@ -81,11 +81,13 @@ def prepare(self, reactor: MemoryReactor, clock: Clock, hs: HomeServer) -> None: tok=self.other_user_tok, extra_content={"name": "nefarious", "topic": "being bad"}, ) + self.rm2 = self.helper.create_room_as( self.third_user, tok=self.third_user_tok, extra_content={"name": "also nefarious"}, ) + self.rm3 = self.helper.create_room_as( self.admin_user, is_public=False, @@ -99,6 +101,7 @@ def prepare(self, reactor: MemoryReactor, clock: Clock, hs: HomeServer) -> None: }, room_version="12", ) + self.rm4 = self.helper.create_room_as( self.other_user, tok=self.other_user_tok, @@ -112,10 +115,19 @@ def prepare(self, reactor: MemoryReactor, clock: Clock, hs: HomeServer) -> None: extra_content={ "visibility": "public", "creation_content": {EventContentFields.ROOM_TYPE: RoomTypes.SPACE}, + "name": "space_room", }, tok=self.other_user_tok, ) + self.room_id_to_human_name_map = { + self.rm1: "room1", + self.rm2: "room2", + self.rm3: "room3", + self.rm4: "room4", + self.space_rm: "space_room", + } + # add three of the rooms to space for state_key in [self.rm1, self.rm2, self.rm3]: self.helper.send_state( @@ -185,10 +197,9 @@ def test_room_summary(self) -> None: ) self.assertEqual(channel.code, 200, msg=channel.json_body) rooms = channel.json_body["rooms"] - self.assertEqual(len(rooms), 4) self.assertCountEqual( - [self.rm1, self.rm2, self.rm3, self.space_rm], - [rm["room_id"] for rm in rooms], + {self.room_id_to_human_name_map[room["room_id"]] for room in rooms}, + {"space_room", "room1", "room2", "room3"}, ) for room_result in rooms: @@ -225,6 +236,7 @@ def test_room_summary(self) -> None: (self.assertEqual(room_result["world_readable"], False),) self.assertEqual(room_result["guest_can_join"], False) self.assertEqual(room_result["num_joined_members"], 1) + self.assertEqual(room_result["name"], "space_room") else: self.fail("unknown room returned") @@ -251,11 +263,10 @@ def test_room_summary_pagination(self) -> None: ) self.assertEqual(channel2.code, 200, msg=channel2.json_body) new_rooms = channel2.json_body["rooms"] - self.assertEqual(len(new_rooms), 2) rooms = rooms + new_rooms self.assertCountEqual( - [self.rm1, self.rm2, self.rm3, self.space_rm], - [rm["room_id"] for rm in rooms], + {self.room_id_to_human_name_map[room["room_id"]] for room in rooms}, + {"space_room", "room1", "room2", "room3"}, ) for room_result in rooms: @@ -292,6 +303,7 @@ def test_room_summary_pagination(self) -> None: (self.assertEqual(room_result["world_readable"], False),) self.assertEqual(room_result["guest_can_join"], False) self.assertEqual(room_result["num_joined_members"], 1) + self.assertEqual(room_result["name"], "space_room") else: self.fail("unknown room returned") From 6148cb76e7ca0b5f46a1c8b26f11f67d11dd6502 Mon Sep 17 00:00:00 2001 From: "H. Shay" Date: Tue, 21 Oct 2025 12:40:46 -0700 Subject: [PATCH 09/13] requested changes --- docs/admin_api/rooms.md | 2 +- synapse/handlers/room_summary.py | 29 +++++----- synapse/rest/admin/rooms.py | 9 ++-- tests/rest/admin/test_room.py | 91 +++++++++++++++++++++----------- 4 files changed, 81 insertions(+), 50 deletions(-) diff --git a/docs/admin_api/rooms.md b/docs/admin_api/rooms.md index f2b56c663a1..d6c73c42d8f 100644 --- a/docs/admin_api/rooms.md +++ b/docs/admin_api/rooms.md @@ -1120,7 +1120,7 @@ Example response: This API allows an admin to fetch the space/room hierarchy for a given space, returning details about that room and any children the room may have, paginating over the space tree in a depth-first manner to locate child rooms. This is functionally similar to the [CS Hierarchy](https://spec.matrix.org/v1.16/client-server-api/#get_matrixclientv1roomsroomidhierarchy) endpoint but does not return information about any remote -rooms that the server is not currently participating in and does not check for room membership when returning room summaries. +rooms that the server is not currently joined to and does not check for room membership when returning room summaries. **Parameters** diff --git a/synapse/handlers/room_summary.py b/synapse/handlers/room_summary.py index 6c722004fc0..d2140a575c5 100644 --- a/synapse/handlers/room_summary.py +++ b/synapse/handlers/room_summary.py @@ -135,7 +135,7 @@ async def get_room_hierarchy( requester: Requester, requested_room_id: str, suggested_only: bool = False, - omit_remote_rooms: bool = False, + omit_remote_room_hierarchy: bool = False, admin_skip_room_visibility_check: bool = False, max_depth: Optional[int] = None, limit: Optional[int] = None, @@ -150,7 +150,8 @@ async def get_room_hierarchy( requested_room_id: The room ID to start the hierarchy at (the "root" room). suggested_only: Whether we should only return children with the "suggested" flag set. - omit_remote_rooms: Whether to omit rooms which the server is not currently participating in + omit_remote_room_hierarchy: Whether to skip reaching out over federation to get information on rooms which the server is not + currently joined to admin_skip_room_visibility_check: Whether to skip checking if the room can be accessed by the requester, used for the admin endpoints. max_depth: The maximum depth in the tree to explore, must be a @@ -180,7 +181,7 @@ async def get_room_hierarchy( requester.user.to_string(), requested_room_id, suggested_only, - omit_remote_rooms, + omit_remote_room_hierarchy, admin_skip_room_visibility_check, max_depth, limit, @@ -191,7 +192,7 @@ async def get_room_hierarchy( requester.user.to_string(), requested_room_id, suggested_only, - omit_remote_rooms, + omit_remote_room_hierarchy, admin_skip_room_visibility_check, max_depth, limit, @@ -204,7 +205,7 @@ async def _get_room_hierarchy( requester: str, requested_room_id: str, suggested_only: bool = False, - omit_remote_rooms: bool = False, + omit_remote_room_hierarchy: bool = False, admin_skip_room_visibility_check: bool = False, max_depth: Optional[int] = None, limit: Optional[int] = None, @@ -228,7 +229,7 @@ async def _get_room_hierarchy( errcode=Codes.NOT_JOINED, ) - if not local_room and not omit_remote_rooms: + if not local_room and not omit_remote_room_hierarchy: room_hierarchy = await self._summarize_remote_room_hierarchy( _RoomQueueEntry(requested_room_id, remote_room_hosts or ()), False, @@ -254,14 +255,15 @@ async def _get_room_hierarchy( except StoreError: raise SynapseError(400, "Unknown pagination token", Codes.INVALID_PARAM) - # If the requester, room ID, suggested-only, or max depth were modified - # the session is invalid. + # If the requester, room ID, suggested-only, max depth, omit_remote_room_hierarchy, or admin_skip_room_visibility_check + # were modified the session is invalid. if ( requester != pagination_session["requester"] or requested_room_id != pagination_session["room_id"] or suggested_only != pagination_session["suggested_only"] or max_depth != pagination_session["max_depth"] - or omit_remote_rooms != pagination_session["omit_remote_rooms"] + or omit_remote_room_hierarchy + != pagination_session["omit_remote_room_hierarchy"] or admin_skip_room_visibility_check != pagination_session["admin_skip_room_visibility_check"] ): @@ -320,9 +322,12 @@ async def _get_room_hierarchy( suggested_only, admin_skip_room_visibility_check=admin_skip_room_visibility_check, ) - + # if we are not fetching remote room details over federation, return what is + # known about the room + elif omit_remote_room_hierarchy: + room_entry = _RoomEntry(room_id, {"room_id": room_id}, ()) # Otherwise, attempt to use information for federation. - elif not omit_remote_rooms: + else: # A previous call might have included information for this room. # It can be used if either: # @@ -396,7 +401,7 @@ async def _get_room_hierarchy( "room_id": requested_room_id, "suggested_only": suggested_only, "max_depth": max_depth, - "omit_remote_rooms": omit_remote_rooms, + "omit_remote_room_hierarchy": omit_remote_room_hierarchy, "admin_skip_room_visibility_check": admin_skip_room_visibility_check, # The stored state. "room_queue": [ diff --git a/synapse/rest/admin/rooms.py b/synapse/rest/admin/rooms.py index 81ec043015f..ae260ee9fbc 100644 --- a/synapse/rest/admin/rooms.py +++ b/synapse/rest/admin/rooms.py @@ -66,8 +66,8 @@ class AdminRoomHierarchy(RestServlet): """ Given a room, returns room details on that room and any space children of the provided room. - Does not return information about remote rooms which the server is not currently - participating in + Does not reach out over federation to fetch details information about any remote rooms which + the server is not currently participating in, returning only the room id of those rooms """ PATTERNS = admin_patterns("/rooms/(?P[^/]*)/hierarchy$") @@ -87,13 +87,10 @@ async def on_GET( max_depth = parse_integer(request, "max_depth") limit = parse_integer(request, "limit") - # we omit returning remote rooms that the server is not currently participating in, - # as that information shouldn't be available to the server admin (as they are not - # participating in those rooms) room_entry_summary = await self._room_summary_handler.get_room_hierarchy( requester, room_id, - omit_remote_rooms=True, + omit_remote_room_hierarchy=True, # We omit details about remote rooms because we only care about managing rooms local to the homeserver. This also immensely helps with the response time of the endpoint since we don't need to reach out over federation. There is a trade-off as this will leave holes where information about public/peekable remote rooms the server is not participating in will be omitted. admin_skip_room_visibility_check=True, max_depth=max_depth, limit=limit, diff --git a/tests/rest/admin/test_room.py b/tests/rest/admin/test_room.py index 0676658ef4a..fecbed36a18 100644 --- a/tests/rest/admin/test_room.py +++ b/tests/rest/admin/test_room.py @@ -75,20 +75,20 @@ def prepare(self, reactor: MemoryReactor, clock: Clock, hs: HomeServer) -> None: self.third_user_tok = self.login("third_user", "pass") # create some rooms with different options - self.rm1 = self.helper.create_room_as( + self.room_id1 = self.helper.create_room_as( self.other_user, is_public=False, tok=self.other_user_tok, extra_content={"name": "nefarious", "topic": "being bad"}, ) - self.rm2 = self.helper.create_room_as( + self.room_id2 = self.helper.create_room_as( self.third_user, tok=self.third_user_tok, extra_content={"name": "also nefarious"}, ) - self.rm3 = self.helper.create_room_as( + self.room_id3 = self.helper.create_room_as( self.admin_user, is_public=False, tok=self.admin_user_tok, @@ -102,7 +102,7 @@ def prepare(self, reactor: MemoryReactor, clock: Clock, hs: HomeServer) -> None: room_version="12", ) - self.rm4 = self.helper.create_room_as( + self.room_id4 = self.helper.create_room_as( self.other_user, tok=self.other_user_tok, extra_content={"name": "not related to other rooms"}, @@ -120,16 +120,20 @@ def prepare(self, reactor: MemoryReactor, clock: Clock, hs: HomeServer) -> None: tok=self.other_user_tok, ) + # and an unjoined remote room + self.remote_room_id = "!remote_room" + self.room_id_to_human_name_map = { - self.rm1: "room1", - self.rm2: "room2", - self.rm3: "room3", - self.rm4: "room4", + self.room_id1: "room1", + self.room_id2: "room2", + self.room_id3: "room3", + self.room_id4: "room4", self.space_rm: "space_room", + self.remote_room_id: "remote_room", } # add three of the rooms to space - for state_key in [self.rm1, self.rm2, self.rm3]: + for state_key in [self.room_id1, self.room_id2, self.room_id3]: self.helper.send_state( self.space_rm, EventTypes.SpaceChild, @@ -138,6 +142,15 @@ def prepare(self, reactor: MemoryReactor, clock: Clock, hs: HomeServer) -> None: state_key=state_key, ) + # and add remote room to space + self.helper.send_state( + self.space_rm, + EventTypes.SpaceChild, + body={"via": ["remote_test_server"]}, + tok=self.other_user_tok, + state_key=self.remote_room_id, + ) + def test_no_auth(self) -> None: """ If the requester does not provide authentication, a 401 is returned @@ -198,45 +211,53 @@ def test_room_summary(self) -> None: self.assertEqual(channel.code, 200, msg=channel.json_body) rooms = channel.json_body["rooms"] self.assertCountEqual( - {self.room_id_to_human_name_map[room["room_id"]] for room in rooms}, - {"space_room", "room1", "room2", "room3"}, + { + self.room_id_to_human_name_map.get( + room["room_id"], f"Unknown room: {room['room_id']}" + ) + for room in rooms + }, + {"space_room", "room1", "room2", "room3", "remote_room"}, ) for room_result in rooms: room_id = room_result["room_id"] - if room_id == self.rm1: + if room_id == self.room_id1: self.assertEqual(room_result["name"], "nefarious") self.assertEqual(room_result["topic"], "being bad") self.assertEqual(room_result["join_rule"], "invite") self.assertEqual(len(room_result["children_state"]), 0) - (self.assertEqual(room_result["world_readable"], False),) + self.assertEqual(room_result["world_readable"], False) self.assertEqual(room_result["guest_can_join"], True) self.assertEqual(room_result["num_joined_members"], 1) - elif room_id == self.rm2: + elif room_id == self.room_id2: self.assertEqual(room_result["name"], "also nefarious") self.assertEqual(room_result["join_rule"], "public") self.assertEqual(len(room_result["children_state"]), 0) - (self.assertEqual(room_result["world_readable"], False),) + self.assertEqual(room_result["world_readable"], False) self.assertEqual(room_result["guest_can_join"], False) self.assertEqual(room_result["num_joined_members"], 1) - elif room_id == self.rm3: + elif room_id == self.room_id3: self.assertEqual(room_result["name"], "not nefarious") self.assertEqual(room_result["join_rule"], "invite") self.assertEqual(room_result["topic"], "happy things") self.assertEqual(len(room_result["children_state"]), 0) - (self.assertEqual(room_result["world_readable"], False),) + self.assertEqual(room_result["world_readable"], False) self.assertEqual(room_result["guest_can_join"], True) self.assertEqual(room_result["num_joined_members"], 1) - elif room_id == self.rm4: + elif room_id == self.room_id4: self.fail("this room should not have been returned") elif room_id == self.space_rm: self.assertEqual(room_result["join_rule"], "public") - self.assertEqual(len(room_result["children_state"]), 3) + self.assertEqual(len(room_result["children_state"]), 4) self.assertEqual(room_result["room_type"], "m.space") - (self.assertEqual(room_result["world_readable"], False),) + self.assertEqual(room_result["world_readable"], False) self.assertEqual(room_result["guest_can_join"], False) self.assertEqual(room_result["num_joined_members"], 1) self.assertEqual(room_result["name"], "space_room") + elif room_id == self.remote_room_id: + # only room_id and children_state fields are returned for remote rooms + self.assertEqual(len(room_result["children_state"]), 0) else: self.fail("unknown room returned") @@ -265,45 +286,53 @@ def test_room_summary_pagination(self) -> None: new_rooms = channel2.json_body["rooms"] rooms = rooms + new_rooms self.assertCountEqual( - {self.room_id_to_human_name_map[room["room_id"]] for room in rooms}, - {"space_room", "room1", "room2", "room3"}, + { + self.room_id_to_human_name_map.get( + room["room_id"], f"Unknown room: {room['room_id']}" + ) + for room in rooms + }, + {"space_room", "room1", "room2", "room3", "remote_room"}, ) for room_result in rooms: room_id = room_result["room_id"] - if room_id == self.rm1: + if room_id == self.room_id1: self.assertEqual(room_result["name"], "nefarious") self.assertEqual(room_result["topic"], "being bad") self.assertEqual(room_result["join_rule"], "invite") self.assertEqual(len(room_result["children_state"]), 0) - (self.assertEqual(room_result["world_readable"], False),) + self.assertEqual(room_result["world_readable"], False) self.assertEqual(room_result["guest_can_join"], True) self.assertEqual(room_result["num_joined_members"], 1) - elif room_id == self.rm2: + elif room_id == self.room_id2: self.assertEqual(room_result["name"], "also nefarious") self.assertEqual(room_result["join_rule"], "public") self.assertEqual(len(room_result["children_state"]), 0) - (self.assertEqual(room_result["world_readable"], False),) + self.assertEqual(room_result["world_readable"], False) self.assertEqual(room_result["guest_can_join"], False) self.assertEqual(room_result["num_joined_members"], 1) - elif room_id == self.rm3: + elif room_id == self.room_id3: self.assertEqual(room_result["name"], "not nefarious") self.assertEqual(room_result["join_rule"], "invite") self.assertEqual(room_result["topic"], "happy things") self.assertEqual(len(room_result["children_state"]), 0) - (self.assertEqual(room_result["world_readable"], False),) + self.assertEqual(room_result["world_readable"], False) self.assertEqual(room_result["guest_can_join"], True) self.assertEqual(room_result["num_joined_members"], 1) - elif room_id == self.rm4: + elif room_id == self.room_id4: self.fail("this room should not have been returned") elif room_id == self.space_rm: self.assertEqual(room_result["join_rule"], "public") - self.assertEqual(len(room_result["children_state"]), 3) + self.assertEqual(len(room_result["children_state"]), 4) self.assertEqual(room_result["room_type"], "m.space") - (self.assertEqual(room_result["world_readable"], False),) + self.assertEqual(room_result["world_readable"], False) self.assertEqual(room_result["guest_can_join"], False) self.assertEqual(room_result["num_joined_members"], 1) self.assertEqual(room_result["name"], "space_room") + elif room_id == self.remote_room_id: + # only room_id and children_state fields are returned for remote rooms + self.assertEqual(len(room_result["children_state"]), 0) else: self.fail("unknown room returned") From e119448ab1f9a1e8bd3654ee829496ac2fd6f4ba Mon Sep 17 00:00:00 2001 From: "H. Shay" Date: Tue, 21 Oct 2025 12:50:50 -0700 Subject: [PATCH 10/13] lint --- scripts-dev/release.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/scripts-dev/release.py b/scripts-dev/release.py index fafa55c770a..c5c72156ccd 100755 --- a/scripts-dev/release.py +++ b/scripts-dev/release.py @@ -821,7 +821,9 @@ def get_repo_and_check_clean_checkout( f"{path} is not a git repository (expecting a {name} repository)." ) while repo.is_dirty(): - if not click.confirm(f"Uncommitted changes exist in {path}. Commit or stash them. Ready to continue?"): + if not click.confirm( + f"Uncommitted changes exist in {path}. Commit or stash them. Ready to continue?" + ): raise click.ClickException("Aborted.") return repo From 99eb9ebe6cb09f548f7fa26350ba8fda69e56cdf Mon Sep 17 00:00:00 2001 From: "H. Shay" Date: Tue, 21 Oct 2025 13:20:49 -0700 Subject: [PATCH 11/13] fix up docs --- changelog.d/19021.feature | 2 +- docs/admin_api/rooms.md | 2 -- 2 files changed, 1 insertion(+), 3 deletions(-) diff --git a/changelog.d/19021.feature b/changelog.d/19021.feature index 723e35774cf..c8c9940cef3 100644 --- a/changelog.d/19021.feature +++ b/changelog.d/19021.feature @@ -1,2 +1,2 @@ Add an [Admin API](https://element-hq.github.io/synapse/latest/usage/administration/admin_api/index.html) to allow an admin -to search for room details and any children in a provided room. \ No newline at end of file +to fetch the space/room hierarchy for a given space. \ No newline at end of file diff --git a/docs/admin_api/rooms.md b/docs/admin_api/rooms.md index d6c73c42d8f..93d2aa2d7cb 100644 --- a/docs/admin_api/rooms.md +++ b/docs/admin_api/rooms.md @@ -1159,13 +1159,11 @@ Response: "guest_can_join": false, "is_space": true, "join_rule": "public", - "name": null, "num_joined_members": 1, "power_users": ["@user:test"], "room_creation_ts": 1400, "room_id": "!sPOpNyMHbZAoAOsOFL:test", "room_type": "m.space", - "topic": null, "world_readable": false }, From b187d6f7d5ac9dfb1771b5c04b7a63984e063343 Mon Sep 17 00:00:00 2001 From: "H. Shay" Date: Thu, 23 Oct 2025 10:18:55 -0700 Subject: [PATCH 12/13] requested changes --- docs/admin_api/rooms.md | 21 +++-------- synapse/handlers/room_summary.py | 13 ++++--- synapse/rest/admin/rooms.py | 7 +++- tests/rest/admin/test_room.py | 63 +++++++++++++++++++++----------- 4 files changed, 60 insertions(+), 44 deletions(-) diff --git a/docs/admin_api/rooms.md b/docs/admin_api/rooms.md index 93d2aa2d7cb..458f0f7368e 100644 --- a/docs/admin_api/rooms.md +++ b/docs/admin_api/rooms.md @@ -1119,8 +1119,10 @@ Example response: # Admin Space Hierarchy Endpoint This API allows an admin to fetch the space/room hierarchy for a given space, returning details about that room and any children -the room may have, paginating over the space tree in a depth-first manner to locate child rooms. This is functionally similar to the [CS Hierarchy](https://spec.matrix.org/v1.16/client-server-api/#get_matrixclientv1roomsroomidhierarchy) endpoint but does not return information about any remote -rooms that the server is not currently joined to and does not check for room membership when returning room summaries. +the room may have, paginating over the space tree in a depth-first manner to locate child rooms. This is functionally similar to the [CS Hierarchy](https://spec.matrix.org/v1.16/client-server-api/#get_matrixclientv1roomsroomidhierarchy) endpoint but does not check for room membership when returning room summaries. The endpoint also +does not return information about any remote rooms that the server is not currently joined to as it does not reach out over +federation to fill in details about those rooms, as we only care about managing rooms local to the homeserver. This is a trade-off as this will leave holes where information about public/peekable remote rooms the server is not participating in are omitted, but does greatly improve the response time of the endpoint. These rooms will be indicated in the response +by having only a room id and an empty `children_state`. **Parameters** @@ -1142,8 +1144,7 @@ Response: { "rooms": [ - {"aliases": [], - "children_state": [ + { "children_state": [ { "content": { "via": ["local_test_server"] @@ -1154,31 +1155,21 @@ Response: "type": "m.space.child" } ], - "creation_event_id": "$bVkNVtm4aDw4c0LRf_U5Ad7mZSo4WKzzQKImrk_rQcg", - "creator": "@user:test", + "name": "space room", "guest_can_join": false, - "is_space": true, "join_rule": "public", "num_joined_members": 1, - "power_users": ["@user:test"], - "room_creation_ts": 1400, "room_id": "!sPOpNyMHbZAoAOsOFL:test", "room_type": "m.space", "world_readable": false }, { - "aliases": [], "children_state": [], - "creation_event_id": "$kymNeN-gA5kzLwZ6FEQUu0_2MfeenYKINSO3dUuLYf8", - "creator": "@user:test", "guest_can_join": true, - "is_space": false, "join_rule": "invite", "name": "nefarious", "num_joined_members": 1, - "power_users": ["@user:test"], - "room_creation_ts": 999, "room_id": "!QrMkkqBSwYRIFNFCso:test", "topic": "being bad", "world_readable": false} diff --git a/synapse/handlers/room_summary.py b/synapse/handlers/room_summary.py index d2140a575c5..78c36194a04 100644 --- a/synapse/handlers/room_summary.py +++ b/synapse/handlers/room_summary.py @@ -238,12 +238,13 @@ async def _get_room_hierarchy( 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 not admin_skip_room_visibility_check: + 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: diff --git a/synapse/rest/admin/rooms.py b/synapse/rest/admin/rooms.py index ae260ee9fbc..92bdf5bec0a 100644 --- a/synapse/rest/admin/rooms.py +++ b/synapse/rest/admin/rooms.py @@ -90,7 +90,12 @@ async def on_GET( room_entry_summary = await self._room_summary_handler.get_room_hierarchy( requester, room_id, - omit_remote_room_hierarchy=True, # We omit details about remote rooms because we only care about managing rooms local to the homeserver. This also immensely helps with the response time of the endpoint since we don't need to reach out over federation. There is a trade-off as this will leave holes where information about public/peekable remote rooms the server is not participating in will be omitted. + # We omit details about remote rooms because we only care about managing rooms + # local to the homeserver. This also immensely helps with the response time + # of the endpoint since we don't need to reach out over federation. There is + # a trade-off as this will leave holes where information about public/peekable + # remote rooms the server is not participating in will be omitted. + omit_remote_room_hierarchy=True, admin_skip_room_visibility_check=True, max_depth=max_depth, limit=limit, diff --git a/tests/rest/admin/test_room.py b/tests/rest/admin/test_room.py index fecbed36a18..aef9b8230a1 100644 --- a/tests/rest/admin/test_room.py +++ b/tests/rest/admin/test_room.py @@ -74,6 +74,10 @@ def prepare(self, reactor: MemoryReactor, clock: Clock, hs: HomeServer) -> None: self.third_user = self.register_user("third_user", "pass") self.third_user_tok = self.login("third_user", "pass") + # mock out the function which pulls room information in over federation. + self._room_summary_handler = hs.get_room_summary_handler() + self._room_summary_handler._summarize_remote_room_hierarchy = Mock() # type: ignore[method-assign] + # create some rooms with different options self.room_id1 = self.helper.create_room_as( self.other_user, @@ -102,14 +106,14 @@ def prepare(self, reactor: MemoryReactor, clock: Clock, hs: HomeServer) -> None: room_version="12", ) - self.room_id4 = self.helper.create_room_as( + self.not_in_space_room_id = self.helper.create_room_as( self.other_user, tok=self.other_user_tok, extra_content={"name": "not related to other rooms"}, ) # create a space room - self.space_rm = self.helper.create_room_as( + self.space_room_id = self.helper.create_room_as( self.other_user, is_public=True, extra_content={ @@ -127,24 +131,28 @@ def prepare(self, reactor: MemoryReactor, clock: Clock, hs: HomeServer) -> None: self.room_id1: "room1", self.room_id2: "room2", self.room_id3: "room3", - self.room_id4: "room4", - self.space_rm: "space_room", + self.not_in_space_room_id: "room4", + self.space_room_id: "space_room", self.remote_room_id: "remote_room", } # add three of the rooms to space for state_key in [self.room_id1, self.room_id2, self.room_id3]: self.helper.send_state( - self.space_rm, + self.space_room_id, EventTypes.SpaceChild, body={"via": ["local_test_server"]}, tok=self.other_user_tok, state_key=state_key, ) - # and add remote room to space + # and add remote room to space - ideally we'd add an actual remote space with rooms + # in it but the test framework doesn't currently support that. Instead we add a + # room which the server would have to reach out over federation to get details about + # and assert that the federation call was not made and that nothing other than it's room + # id and empty child state are returned self.helper.send_state( - self.space_rm, + self.space_room_id, EventTypes.SpaceChild, body={"via": ["remote_test_server"]}, tok=self.other_user_tok, @@ -158,7 +166,7 @@ def test_no_auth(self) -> None: channel = self.make_request( "GET", - f"/_synapse/admin/v1/rooms/{self.space_rm}/hierarchy", + f"/_synapse/admin/v1/rooms/{self.space_room_id}/hierarchy", ) self.assertEqual(401, channel.code, msg=channel.json_body) @@ -171,7 +179,7 @@ def test_requester_is_no_admin(self) -> None: channel = self.make_request( "GET", - f"/_synapse/admin/v1/rooms/{self.space_rm}/hierarchy", + f"/_synapse/admin/v1/rooms/{self.space_room_id}/hierarchy", access_token=self.other_user_tok, ) @@ -184,7 +192,7 @@ def test_bad_request(self) -> None: """ channel = self.make_request( "GET", - f"/_synapse/admin/v1/rooms/{self.space_rm}/hierarchy?limit=ten", + f"/_synapse/admin/v1/rooms/{self.space_room_id}/hierarchy?limit=ten", access_token=self.admin_user_tok, ) self.assertEqual(400, channel.code, msg=channel.json_body) @@ -192,7 +200,7 @@ def test_bad_request(self) -> None: channel = self.make_request( "GET", - f"/_synapse/admin/v1/rooms/{self.space_rm}/hierarchy?max_depth=four", + f"/_synapse/admin/v1/rooms/{self.space_room_id}/hierarchy?max_depth=four", access_token=self.admin_user_tok, ) self.assertEqual(400, channel.code, msg=channel.json_body) @@ -205,7 +213,7 @@ def test_room_summary(self) -> None: channel = self.make_request( "GET", - f"/_synapse/admin/v1/rooms/{self.space_rm}/hierarchy", + f"/_synapse/admin/v1/rooms/{self.space_room_id}/hierarchy", access_token=self.admin_user_tok, ) self.assertEqual(channel.code, 200, msg=channel.json_body) @@ -245,9 +253,9 @@ def test_room_summary(self) -> None: self.assertEqual(room_result["world_readable"], False) self.assertEqual(room_result["guest_can_join"], True) self.assertEqual(room_result["num_joined_members"], 1) - elif room_id == self.room_id4: + elif room_id == self.not_in_space_room_id: self.fail("this room should not have been returned") - elif room_id == self.space_rm: + elif room_id == self.space_room_id: self.assertEqual(room_result["join_rule"], "public") self.assertEqual(len(room_result["children_state"]), 4) self.assertEqual(room_result["room_type"], "m.space") @@ -258,6 +266,8 @@ def test_room_summary(self) -> None: elif room_id == self.remote_room_id: # only room_id and children_state fields are returned for remote rooms self.assertEqual(len(room_result["children_state"]), 0) + # assert that a federation call to look up details about this room is not called + self._room_summary_handler._summarize_remote_room_hierarchy.assert_not_called() # type: ignore[attr-defined] else: self.fail("unknown room returned") @@ -269,30 +279,37 @@ def test_room_summary_pagination(self) -> None: channel = self.make_request( "GET", - f"/_synapse/admin/v1/rooms/{self.space_rm}/hierarchy?limit=2", + f"/_synapse/admin/v1/rooms/{self.space_room_id}/hierarchy?limit=2", access_token=self.admin_user_tok, ) self.assertEqual(channel.code, 200, msg=channel.json_body) rooms = channel.json_body["rooms"] - self.assertEqual(len(rooms), 2) + self.assertCountEqual( + { + self.room_id_to_human_name_map.get( + room["room_id"], f"Unknown room: {room['room_id']}" + ) + for room in rooms + }, + {"space_room", "room1"}, + ) next_batch = channel.json_body["next_batch"] channel2 = self.make_request( "GET", - f"/_synapse/admin/v1/rooms/{self.space_rm}/hierarchy?from={next_batch}", + f"/_synapse/admin/v1/rooms/{self.space_room_id}/hierarchy?from={next_batch}", access_token=self.admin_user_tok, ) self.assertEqual(channel2.code, 200, msg=channel2.json_body) new_rooms = channel2.json_body["rooms"] - rooms = rooms + new_rooms self.assertCountEqual( { self.room_id_to_human_name_map.get( room["room_id"], f"Unknown room: {room['room_id']}" ) - for room in rooms + for room in new_rooms }, - {"space_room", "room1", "room2", "room3", "remote_room"}, + {"room2", "room3", "remote_room"}, ) for room_result in rooms: @@ -320,9 +337,9 @@ def test_room_summary_pagination(self) -> None: self.assertEqual(room_result["world_readable"], False) self.assertEqual(room_result["guest_can_join"], True) self.assertEqual(room_result["num_joined_members"], 1) - elif room_id == self.room_id4: + elif room_id == self.not_in_space_room_id: self.fail("this room should not have been returned") - elif room_id == self.space_rm: + elif room_id == self.space_room_id: self.assertEqual(room_result["join_rule"], "public") self.assertEqual(len(room_result["children_state"]), 4) self.assertEqual(room_result["room_type"], "m.space") @@ -333,6 +350,8 @@ def test_room_summary_pagination(self) -> None: elif room_id == self.remote_room_id: # only room_id and children_state fields are returned for remote rooms self.assertEqual(len(room_result["children_state"]), 0) + # assert that a federation call to look up details about this room is not called + self._room_summary_handler._summarize_remote_room_hierarchy.assert_not_called() # type: ignore[attr-defined] else: self.fail("unknown room returned") From f360c983c6db6d12e37ff76ee3228cc9b42c2238 Mon Sep 17 00:00:00 2001 From: "H. Shay" Date: Thu, 23 Oct 2025 10:26:26 -0700 Subject: [PATCH 13/13] lint --- synapse/rest/admin/rooms.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/synapse/rest/admin/rooms.py b/synapse/rest/admin/rooms.py index 4189ed0b1a4..25148f80e51 100644 --- a/synapse/rest/admin/rooms.py +++ b/synapse/rest/admin/rooms.py @@ -80,7 +80,7 @@ def __init__(self, hs: "HomeServer"): async def on_GET( self, request: SynapseRequest, room_id: str - ) -> Tuple[int, JsonDict]: + ) -> tuple[int, JsonDict]: requester = await self._auth.get_user_by_req(request) await assert_user_is_admin(self._auth, requester)