Delete room endpoint (#9889)

Support the delete of a room through DELETE request and mark
previous request as deprecated through documentation.

Signed-off-by: Thibault Ferrante <[email protected]>

Author: ThibF

changelog.d/9889.feature

@@ -0,0 +1 @@
+Add support for `DELETE /_synapse/admin/v1/rooms/<room_id>`.

changelog.d/9889.removal

@@ -0,0 +1 @@
+Mark as deprecated `POST /_synapse/admin/v1/rooms/<room_id>/delete`.

docs/admin_api/rooms.md

@@ -427,7 +427,7 @@ the new room. Users on other servers will be unaffected.
 The API is:
 
 ```
-POST /_synapse/admin/v1/rooms/<room_id>/delete
+DELETE /_synapse/admin/v1/rooms/<room_id>
 ```
 
 with a body of:
@@ -528,6 +528,15 @@ You will have to manually handle, if you so choose, the following:
 * Users that would have been booted from the room (and will have been force-joined to the Content Violation room).
 * Removal of the Content Violation room if desired.
 
+## Deprecated endpoint
+
+The previous deprecated API will be removed in a future release, it was:
+
+```
+POST /_synapse/admin/v1/rooms/<room_id>/delete
+```
+
+It behaves the same way than the current endpoint except the path and the method.
 
 # Make Room Admin API
 

synapse/rest/admin/rooms.py

@@ -37,9 +37,11 @@
 from synapse.util import json_decoder
 
 if TYPE_CHECKING:
+    from synapse.api.auth import Auth
+    from synapse.handlers.pagination import PaginationHandler
+    from synapse.handlers.room import RoomShutdownHandler
     from synapse.server import HomeServer
 
-
 logger = logging.getLogger(__name__)
 
 
@@ -146,50 +148,14 @@ def __init__(self, hs: "HomeServer"):
     async def on_POST(
         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.user)
-
-        content = parse_json_object_from_request(request)
-
-        block = content.get("block", False)
-        if not isinstance(block, bool):
-            raise SynapseError(
-                HTTPStatus.BAD_REQUEST,
-                "Param 'block' must be a boolean, if given",
-                Codes.BAD_JSON,
-            )
-
-        purge = content.get("purge", True)
-        if not isinstance(purge, bool):
-            raise SynapseError(
-                HTTPStatus.BAD_REQUEST,
-                "Param 'purge' must be a boolean, if given",
-                Codes.BAD_JSON,
-            )
-
-        force_purge = content.get("force_purge", False)
-        if not isinstance(force_purge, bool):
-            raise SynapseError(
-                HTTPStatus.BAD_REQUEST,
-                "Param 'force_purge' must be a boolean, if given",
-                Codes.BAD_JSON,
-            )
-
-        ret = await self.room_shutdown_handler.shutdown_room(
-            room_id=room_id,
-            new_room_user_id=content.get("new_room_user_id"),
-            new_room_name=content.get("room_name"),
-            message=content.get("message"),
-            requester_user_id=requester.user.to_string(),
-            block=block,
+        return await _delete_room(
+            request,
+            room_id,
+            self.auth,
+            self.room_shutdown_handler,
+            self.pagination_handler,
         )
 
-        # Purge room
-        if purge:
-            await self.pagination_handler.purge_room(room_id, force=force_purge)
-
-        return (200, ret)
-
 
 class ListRoomRestServlet(RestServlet):
     """
@@ -282,7 +248,22 @@ async def on_GET(self, request: SynapseRequest) -> Tuple[int, JsonDict]:
 
 
 class RoomRestServlet(RestServlet):
-    """Get room details.
+    """Manage a room.
+
+    On GET : Get details of a room.
+
+    On DELETE : Delete a room from server.
+
+    It is a combination and improvement of shutdown and purge room.
+
+    Shuts down a room by removing all local users from the room.
+    Blocking all future invites and joins to the room is optional.
+
+    If desired any local aliases will be repointed to a new room
+    created by `new_room_user_id` and kicked users will be auto-
+    joined to the new room.
+
+    If 'purge' is true, it will remove all traces of a room from the database.
 
     TODO: Add on_POST to allow room creation without joining the room
     """
@@ -293,6 +274,8 @@ def __init__(self, hs: "HomeServer"):
         self.hs = hs
         self.auth = hs.get_auth()
         self.store = hs.get_datastore()
+        self.room_shutdown_handler = hs.get_room_shutdown_handler()
+        self.pagination_handler = hs.get_pagination_handler()
 
     async def on_GET(
         self, request: SynapseRequest, room_id: str
@@ -308,6 +291,17 @@ async def on_GET(
 
         return (200, ret)
 
+    async def on_DELETE(
+        self, request: SynapseRequest, room_id: str
+    ) -> Tuple[int, JsonDict]:
+        return await _delete_room(
+            request,
+            room_id,
+            self.auth,
+            self.room_shutdown_handler,
+            self.pagination_handler,
+        )
+
 
 class RoomMembersRestServlet(RestServlet):
     """
@@ -694,3 +688,55 @@ async def on_GET(
         )
 
         return 200, results
+
+
+async def _delete_room(
+    request: SynapseRequest,
+    room_id: str,
+    auth: "Auth",
+    room_shutdown_handler: "RoomShutdownHandler",
+    pagination_handler: "PaginationHandler",
+) -> Tuple[int, JsonDict]:
+    requester = await auth.get_user_by_req(request)
+    await assert_user_is_admin(auth, requester.user)
+
+    content = parse_json_object_from_request(request)
+
+    block = content.get("block", False)
+    if not isinstance(block, bool):
+        raise SynapseError(
+            HTTPStatus.BAD_REQUEST,
+            "Param 'block' must be a boolean, if given",
+            Codes.BAD_JSON,
+        )
+
+    purge = content.get("purge", True)
+    if not isinstance(purge, bool):
+        raise SynapseError(
+            HTTPStatus.BAD_REQUEST,
+            "Param 'purge' must be a boolean, if given",
+            Codes.BAD_JSON,
+        )
+
+    force_purge = content.get("force_purge", False)
+    if not isinstance(force_purge, bool):
+        raise SynapseError(
+            HTTPStatus.BAD_REQUEST,
+            "Param 'force_purge' must be a boolean, if given",
+            Codes.BAD_JSON,
+        )
+
+    ret = await room_shutdown_handler.shutdown_room(
+        room_id=room_id,
+        new_room_user_id=content.get("new_room_user_id"),
+        new_room_name=content.get("room_name"),
+        message=content.get("message"),
+        requester_user_id=requester.user.to_string(),
+        block=block,
+    )
+
+    # Purge room
+    if purge:
+        await pagination_handler.purge_room(room_id, force=force_purge)
+
+    return (200, ret)

tests/rest/admin/test_room.py

@@ -17,6 +17,8 @@
 from typing import List, Optional
 from unittest.mock import Mock
 
+from parameterized import parameterized_class
+
 import synapse.rest.admin
 from synapse.api.constants import EventTypes, Membership
 from synapse.api.errors import Codes
@@ -144,6 +146,13 @@ def _assert_peek(self, room_id, expect_code):
         )
 
 
+@parameterized_class(
+    ("method", "url_template"),
+    [
+        ("POST", "/_synapse/admin/v1/rooms/%s/delete"),
+        ("DELETE", "/_synapse/admin/v1/rooms/%s"),
+    ],
+)
 class DeleteRoomTestCase(unittest.HomeserverTestCase):
     servlets = [
         synapse.rest.admin.register_servlets,
@@ -175,15 +184,15 @@ def prepare(self, reactor, clock, hs):
         self.room_id = self.helper.create_room_as(
             self.other_user, tok=self.other_user_tok
         )
-        self.url = "/_synapse/admin/v1/rooms/%s/delete" % self.room_id
+        self.url = self.url_template % self.room_id
 
     def test_requester_is_no_admin(self):
         """
         If the user is not a server admin, an error 403 is returned.
         """
 
         channel = self.make_request(
-            "POST",
+            self.method,
             self.url,
             json.dumps({}),
             access_token=self.other_user_tok,
@@ -196,10 +205,10 @@ def test_room_does_not_exist(self):
         """
         Check that unknown rooms/server return error 404.
         """
-        url = "/_synapse/admin/v1/rooms/!unknown:test/delete"
+        url = self.url_template % "!unknown:test"
 
         channel = self.make_request(
-            "POST",
+            self.method,
             url,
             json.dumps({}),
             access_token=self.admin_user_tok,
@@ -212,10 +221,10 @@ def test_room_is_not_valid(self):
         """
         Check that invalid room names, return an error 400.
         """
-        url = "/_synapse/admin/v1/rooms/invalidroom/delete"
+        url = self.url_template % "invalidroom"
 
         channel = self.make_request(
-            "POST",
+            self.method,
             url,
             json.dumps({}),
             access_token=self.admin_user_tok,
@@ -234,7 +243,7 @@ def test_new_room_user_does_not_exist(self):
         body = json.dumps({"new_room_user_id": "@unknown:test"})
 
         channel = self.make_request(
-            "POST",
+            self.method,
             self.url,
             content=body.encode(encoding="utf_8"),
             access_token=self.admin_user_tok,
@@ -253,7 +262,7 @@ def test_new_room_user_is_not_local(self):
         body = json.dumps({"new_room_user_id": "@not:exist.bla"})
 
         channel = self.make_request(
-            "POST",
+            self.method,
             self.url,
             content=body.encode(encoding="utf_8"),
             access_token=self.admin_user_tok,
@@ -272,7 +281,7 @@ def test_block_is_not_bool(self):
         body = json.dumps({"block": "NotBool"})
 
         channel = self.make_request(
-            "POST",
+            self.method,
             self.url,
             content=body.encode(encoding="utf_8"),
             access_token=self.admin_user_tok,
@@ -288,7 +297,7 @@ def test_purge_is_not_bool(self):
         body = json.dumps({"purge": "NotBool"})
 
         channel = self.make_request(
-            "POST",
+            self.method,
             self.url,
             content=body.encode(encoding="utf_8"),
             access_token=self.admin_user_tok,
@@ -314,7 +323,7 @@ def test_purge_room_and_block(self):
         body = json.dumps({"block": True, "purge": True})
 
         channel = self.make_request(
-            "POST",
+            self.method,
             self.url.encode("ascii"),
             content=body.encode(encoding="utf_8"),
             access_token=self.admin_user_tok,
@@ -347,7 +356,7 @@ def test_purge_room_and_not_block(self):
         body = json.dumps({"block": False, "purge": True})
 
         channel = self.make_request(
-            "POST",
+            self.method,
             self.url.encode("ascii"),
             content=body.encode(encoding="utf_8"),
             access_token=self.admin_user_tok,
@@ -381,7 +390,7 @@ def test_block_room_and_not_purge(self):
         body = json.dumps({"block": False, "purge": False})
 
         channel = self.make_request(
-            "POST",
+            self.method,
             self.url.encode("ascii"),
             content=body.encode(encoding="utf_8"),
             access_token=self.admin_user_tok,
@@ -426,10 +435,9 @@ def test_shutdown_room_consent(self):
         self._is_member(room_id=self.room_id, user_id=self.other_user)
 
         # Test that the admin can still send shutdown
-        url = "/_synapse/admin/v1/rooms/%s/delete" % self.room_id
         channel = self.make_request(
-            "POST",
-            url.encode("ascii"),
+            self.method,
+            self.url,
             json.dumps({"new_room_user_id": self.admin_user}),
             access_token=self.admin_user_tok,
         )
@@ -473,10 +481,9 @@ def test_shutdown_room_block_peek(self):
         self._is_member(room_id=self.room_id, user_id=self.other_user)
 
         # Test that the admin can still send shutdown
-        url = "/_synapse/admin/v1/rooms/%s/delete" % self.room_id
         channel = self.make_request(
-            "POST",
-            url.encode("ascii"),
+            self.method,
+            self.url,
             json.dumps({"new_room_user_id": self.admin_user}),
             access_token=self.admin_user_tok,
         )