Add flag to control whether remote spaces are processed

4 files changed, 127 insertions, 54 deletions

diff --git a/docs/usage/administration/admin_api/spaces.md b/docs/usage/administration/admin_api/spaces.md
index ac91749407..98fa576543 100644
--- a/docs/usage/administration/admin_api/spaces.md
+++ b/docs/usage/administration/admin_api/spaces.md
@@ -16,31 +16,42 @@ The API is:
 DELETE /_synapse/admin/v1/rooms/<room_id>/hierarchy/members/<user_id>
 ```
 
+with an optional body of:
+
+```json
+{
+  "include_remote_spaces": true,
+}
+```
+
+`include_remote_spaces` controls whether to process subspaces that the
+local homeserver is not participating in. The listings of such subspaces
+have to be retrieved over federation and their accuracy cannot be
+guaranteed.
+
 Returning:
 
 ```json
 {
-    "left": ["!room1:example.net", "!room2:example.net", ...],
-    "failed": {
-        "!room3:example.net": [
-            "Could not explore space or room fully."
-        ],
-        "!room4:example.net": [
-            "Failed to leave room."
-        ],
+    "left_rooms": ["!room1:example.net", "!room2:example.net", ...],
+    "inaccessible_rooms": ["!subspace1:example.net", ...],
+    "failed_rooms": {
+        "!room4:example.net": "Failed to leave room.",
         ...
     }
 }
 ```
 
-`left`: A list of rooms that the user has been made to leave.
+`left_rooms`: A list of rooms that the user has been made to leave.
 
-`failed`: A dictionary with entries for rooms that could not be fully
-processed. The values of the dictionary are lists of failure reasons.
-Rooms may appear here if:
-  * The user failed to leave them for any reason.
-  * The room is a space that the local homeserver is not in, and so
-    its full list of child rooms could not be determined.
+`inaccessible_rooms`: A list of rooms and spaces that the local
+homeserver is not in, and may have not been fully processed. Rooms may
+appear here if:
+  * The room is a space that the local homeserver is not in, and so its
+    full list of child rooms could not be determined.
   * The room is inaccessible to the local homeserver, and it is not
     known whether the room is a subspace containing further rooms.
-  * Some combination of the above.
+
+`failed_rooms`: A dictionary of errors encountered when leaving rooms.
+The keys of the dictionary are room IDs and the values of the dictionary
+are error messages.
diff --git a/synapse/handlers/space_hierarchy.py b/synapse/handlers/space_hierarchy.py
index 619059b55e..53fc29051e 100644
--- a/synapse/handlers/space_hierarchy.py
+++ b/synapse/handlers/space_hierarchy.py
@@ -50,12 +50,18 @@ class SpaceHierarchyHandler:
         self._server_name = hs.hostname
 
     async def get_space_descendants(
-        self, space_id: str, via: Optional[Iterable[str]] = None
+        self,
+        space_id: str,
+        via: Optional[Iterable[str]] = None,
+        enable_federation: Optional[bool] = True,
     ) -> Tuple[Sequence[Tuple[str, Iterable[str]]], Sequence[str]]:
         """Gets the children of a space, recursively.
 
         Args:
             space_id: The room ID of the space.
+            via: A list of servers which may know about the space.
+            enable_federation: A boolean controlling whether children of unknown rooms
+                should be fetched over federation. Defaults to `True`.
 
         Returns:
             A tuple containing:
@@ -65,6 +71,8 @@ class SpaceHierarchyHandler:
                Rooms in this list are either spaces not known locally, and thus require
                listing over federation, or are unknown rooms or subspaces completely
                inaccessible to the local homeserver which may contain further rooms.
+               Subspaces requiring listing over federation are always included here,
+               regardless of the value of the `enable_federation` flag.
 
                This list is a subset of the previous list, except it may include
                `space_id`.
@@ -91,13 +99,21 @@ class SpaceHierarchyHandler:
                     children,
                     federation_room_chunks,
                 ) = await self._get_space_children(
-                    space_id, via, federation_room_chunks
+                    space_id,
+                    via,
+                    federation_room_chunks,
+                    enable_federation=enable_federation,
                 )
             except SynapseError:
                 # Could not list children over federation
                 inaccessible_room_ids.append(space_id)
                 continue
 
+            # Children were retrieved over federation, which is not guaranteed to be
+            # the full list.
+            if not is_in_room:
+                inaccessible_room_ids.append(space_id)
+
             for child_room_id, child_via in reversed(children):
                 if child_room_id in seen:
                     continue
@@ -109,11 +125,6 @@ class SpaceHierarchyHandler:
                 # `_get_space_children`.
                 todo.append((child_room_id, child_via, federation_room_chunks))
 
-            # Children were retrieved over federation, which is not guaranteed to be
-            # the full list.
-            if not is_in_room:
-                inaccessible_room_ids.append(space_id)
-
         return descendants, inaccessible_room_ids
 
     async def _get_space_children(
@@ -121,6 +132,7 @@ class SpaceHierarchyHandler:
         space_id: str,
         via: Optional[Iterable[str]] = None,
         federation_room_chunks: Optional[Mapping[str, Optional[JsonDict]]] = None,
+        enable_federation: Optional[bool] = True,
     ) -> Tuple[
         bool, Sequence[Tuple[str, Iterable[str]]], Mapping[str, Optional[JsonDict]]
     ]:
@@ -152,7 +164,7 @@ class SpaceHierarchyHandler:
 
         Raises:
             SynapseError: if `space_id` is not known locally and its children could not
-                be retrieved over federation.
+                be retrieved over federation or `enable_federation` is `False`.
         """
         via = via or []
         federation_room_chunks = federation_room_chunks or {}
@@ -178,6 +190,11 @@ class SpaceHierarchyHandler:
                     # `space_id` is not a space according to federation.
                     return False, [], {}
 
+            if not enable_federation:
+                raise SynapseError(
+                    502, f"{space_id} is not accessible to the local homeserver"
+                )
+
             children, room_chunks = await self._get_space_children_remote(space_id, via)
             return False, children, room_chunks
 
diff --git a/synapse/rest/admin/space.py b/synapse/rest/admin/space.py
index c85aec2cc5..1a152851d5 100644
--- a/synapse/rest/admin/space.py
+++ b/synapse/rest/admin/space.py
@@ -12,11 +12,16 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 import logging
+from http import HTTPStatus
 from typing import TYPE_CHECKING, Dict, List, Tuple
 
 from synapse.api.constants import EventTypes, JoinRules, Membership
 from synapse.api.errors import SynapseError
-from synapse.http.servlet import ResolveRoomIdMixin, RestServlet
+from synapse.http.servlet import (
+    ResolveRoomIdMixin,
+    RestServlet,
+    parse_json_object_from_request,
+)
 from synapse.http.site import SynapseRequest
 from synapse.rest.admin._base import admin_patterns, assert_user_is_admin
 from synapse.storage.state import StateFilter
@@ -56,26 +61,38 @@ class RemoveSpaceMemberRestServlet(ResolveRoomIdMixin, RestServlet):
 
         Returns:
             A tuple containing the HTTP status code and a JSON dictionary containing:
-             * `left`: A list of rooms that the user has been made to leave.
-             * `failed`: A with entries for rooms that could not be fully processed.
-                The values of the dictionary are lists of failure reasons.
-                Rooms may appear here if:
-                 * The user failed to leave them for any reason.
+             * `left_rooms`: A list of rooms that the user has been made to leave.
+             * `inaccessible_rooms`: A list of rooms and spaces that the local
+                homeserver is not in, and may have not been fully processed. Rooms may
+                appear here if:
                  * The room is a space that the local homeserver is not in, and so its
                    full list of child rooms could not be determined.
                  * The room is inaccessible to the local homeserver, and it is not known
                    whether the room is a subspace containing further rooms.
-                 * Some combination of the above.
+             * `failed_rooms`: A dictionary of errors encountered when leaving rooms.
+                The keys of the dictionary are room IDs and the values of the dictionary
+                are error messages.
         """
         requester = await self._auth.get_user_by_req(request)
         await assert_user_is_admin(self._auth, requester.user)
 
+        content = parse_json_object_from_request(request, allow_empty_body=True)
+        include_remote_spaces = content.get("include_remote_spaces", True)
+        if not isinstance(include_remote_spaces, bool):
+            raise SynapseError(
+                HTTPStatus.BAD_REQUEST,
+                "'include_remote_spaces' parameter must be a boolean",
+            )
+
         space_id, _ = await self.resolve_room_id(space_id)
 
         target_user = UserID.from_string(user_id)
 
         if not self._hs.is_mine(target_user):
-            raise SynapseError(400, "This endpoint can only be used with local users")
+            raise SynapseError(
+                HTTPStatus.BAD_REQUEST,
+                "This endpoint can only be used with local users",
+            )
 
         # Fetch the list of rooms the target user is currently in
         user_rooms = await self._store.get_rooms_for_local_user_where_membership_is(
@@ -87,7 +104,9 @@ class RemoveSpaceMemberRestServlet(ResolveRoomIdMixin, RestServlet):
         (
             descendants,
             inaccessible_room_ids,
-        ) = await self._space_hierarchy_handler.get_space_descendants(space_id)
+        ) = await self._space_hierarchy_handler.get_space_descendants(
+            space_id, enable_federation=include_remote_spaces
+        )
 
         # Determine which rooms to leave by checking join rules
         rooms_to_leave: List[str] = []
@@ -116,10 +135,7 @@ class RemoveSpaceMemberRestServlet(ResolveRoomIdMixin, RestServlet):
                 rooms_to_leave.append(room_id)
 
         # Now start leaving rooms
-        failures: Dict[str, List[str]] = {
-            room_id: ["Could not fully explore space or room."]
-            for room_id in inaccessible_room_ids
-        }
+        failures: Dict[str, str] = {}
         left_rooms: List[str] = []
 
         fake_requester = create_requester(
@@ -144,6 +160,10 @@ class RemoveSpaceMemberRestServlet(ResolveRoomIdMixin, RestServlet):
                 )
                 left_rooms.append(room_id)
             except Exception as e:
-                failures.get(room_id, []).append(str(e))
+                failures[room_id] = str(e)
 
-        return 200, {"left": left_rooms, "failed": failures}
+        return 200, {
+            "left_rooms": left_rooms,
+            "inaccessible_rooms": inaccessible_room_ids,
+            "failed_rooms": failures,
+        }
diff --git a/tests/rest/admin/test_space.py b/tests/rest/admin/test_space.py
index 46267b0b89..70d6776258 100644
--- a/tests/rest/admin/test_space.py
+++ b/tests/rest/admin/test_space.py
@@ -164,9 +164,14 @@ class RemoveSpaceMemberTestCase(unittest.HomeserverTestCase):
         )
 
         response = self._remove_from_space(self.target_user)
-
-        self.assertCountEqual(response["left"], [self.space_id])
-        self.assertEqual(response["failed"], {})
+        self.assertEqual(
+            response,
+            {
+                "left_rooms": [self.space_id],
+                "inaccessible_rooms": [],
+                "failed_rooms": {},
+            },
+        )
 
         membership, _ = self.get_success(
             self.store.get_local_current_membership_for_user_in_room(
@@ -183,9 +188,14 @@ class RemoveSpaceMemberTestCase(unittest.HomeserverTestCase):
         self.helper.join(public_room_id, self.target_user, tok=self.target_user_tok)
 
         response = self._remove_from_space(self.target_user)
-
-        self.assertCountEqual(response["left"], [self.space_id])
-        self.assertEqual(response["failed"], {})
+        self.assertEqual(
+            response,
+            {
+                "left_rooms": [self.space_id],
+                "inaccessible_rooms": [],
+                "failed_rooms": {},
+            },
+        )
 
         membership, _ = self.get_success(
             self.store.get_local_current_membership_for_user_in_room(
@@ -207,9 +217,14 @@ class RemoveSpaceMemberTestCase(unittest.HomeserverTestCase):
         )
 
         response = self._remove_from_space(self.target_user)
-
-        self.assertCountEqual(response["left"], [self.space_id, invite_only_room_id])
-        self.assertEqual(response["failed"], {})
+        self.assertEqual(
+            response,
+            {
+                "left_rooms": [self.space_id, invite_only_room_id],
+                "inaccessible_rooms": [],
+                "failed_rooms": {},
+            },
+        )
 
         membership, _ = self.get_success(
             self.store.get_local_current_membership_for_user_in_room(
@@ -234,9 +249,14 @@ class RemoveSpaceMemberTestCase(unittest.HomeserverTestCase):
         )
 
         response = self._remove_from_space(self.target_user)
-
-        self.assertCountEqual(response["left"], [self.space_id, invite_only_room_id])
-        self.assertEqual(response["failed"], {})
+        self.assertEqual(
+            response,
+            {
+                "left_rooms": [self.space_id, invite_only_room_id],
+                "inaccessible_rooms": [],
+                "failed_rooms": {},
+            },
+        )
 
         membership, _ = self.get_success(
             self.store.get_local_current_membership_for_user_in_room(
@@ -252,9 +272,14 @@ class RemoveSpaceMemberTestCase(unittest.HomeserverTestCase):
         self.helper.join(restricted_room_id, self.target_user, tok=self.target_user_tok)
 
         response = self._remove_from_space(self.target_user)
-
-        self.assertCountEqual(response["left"], [self.space_id, restricted_room_id])
-        self.assertEqual(response["failed"], {})
+        self.assertEqual(
+            response,
+            {
+                "left_rooms": [self.space_id, restricted_room_id],
+                "inaccessible_rooms": [],
+                "failed_rooms": {},
+            },
+        )
 
         membership, _ = self.get_success(
             self.store.get_local_current_membership_for_user_in_room(