From 7e6e9dab52b09984b67c5840a8995a70c44abd9e Mon Sep 17 00:00:00 2001
From: David Robertson <davidr@element.io>
Date: Mon, 1 Nov 2021 13:12:04 +0000
Subject: [PATCH 01/15] Fix doc typo

---
 docs/admin_api/rooms.md | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/docs/admin_api/rooms.md b/docs/admin_api/rooms.md
index 62eeff9e1a9b..3de9323c4ba5 100644
--- a/docs/admin_api/rooms.md
+++ b/docs/admin_api/rooms.md
@@ -397,7 +397,7 @@ This API will remove all trace of the old room from your database after removing
 all local users. If `purge` is `true` (the default), all traces of the old room will
 be removed from your database after removing all local users. If you do not want
 this to happen, set `purge` to `false`.
-Depending on the amount of history being purged a call to the API may take
+Depending on the amount of history being purged, a call to the API may take
 several minutes or longer.
 
 The local server will only have the power to move local user and room aliases to

From 33afbeb5f5b06695c547966b83de244c9c7da99a Mon Sep 17 00:00:00 2001
From: David Robertson <davidr@element.io>
Date: Mon, 1 Nov 2021 13:12:30 +0000
Subject: [PATCH 02/15] Note that new_room_id may be null

---
 docs/admin_api/rooms.md  | 3 ++-
 synapse/handlers/room.py | 4 +++-
 2 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/docs/admin_api/rooms.md b/docs/admin_api/rooms.md
index 3de9323c4ba5..98320d174044 100644
--- a/docs/admin_api/rooms.md
+++ b/docs/admin_api/rooms.md
@@ -478,7 +478,8 @@ The following fields are returned in the JSON response body:
 * `failed_to_kick_users` - An array of users (`user_id`) that that were not kicked.
 * `local_aliases` - An array of strings representing the local aliases that were migrated from
                     the old room to the new.
-* `new_room_id` - A string representing the room ID of the new room.
+* `new_room_id` - A string representing the room ID of the new room, or `null` if
+                  no such room was created.
 
 
 ## Undoing room deletions
diff --git a/synapse/handlers/room.py b/synapse/handlers/room.py
index 99e9b3734457..9d76da251c64 100644
--- a/synapse/handlers/room.py
+++ b/synapse/handlers/room.py
@@ -1344,7 +1344,9 @@ async def shutdown_room(
             local_aliases:
                 An array of strings representing the local aliases that were
                 migrated from the old room to the new.
-            new_room_id: A string representing the room ID of the new room.
+            new_room_id:
+                A string representing the room ID of the new room, or None if
+                no such room was created.
         """
 
         if not new_room_name:

From 47fd059fdf8537ac86db2c84f7449ef1b566e651 Mon Sep 17 00:00:00 2001
From: David Robertson <davidr@element.io>
Date: Mon, 1 Nov 2021 13:14:09 +0000
Subject: [PATCH 03/15] Avoid confusion with other meanings of "event".

---
 synapse/handlers/room.py | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/synapse/handlers/room.py b/synapse/handlers/room.py
index 9d76da251c64..3ab58c46b0f0 100644
--- a/synapse/handlers/room.py
+++ b/synapse/handlers/room.py
@@ -12,8 +12,7 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
-"""Contains functions for performing events on rooms."""
-
+"""Contains functions for performing actions on rooms."""
 import itertools
 import logging
 import math

From 033adb7e537dff3b7340635106a70bb28ea0d5f4 Mon Sep 17 00:00:00 2001
From: David Robertson <davidr@element.io>
Date: Mon, 1 Nov 2021 13:13:47 +0000
Subject: [PATCH 04/15] Enforce the type of the shutdown_room response

---
 synapse/handlers/room.py    | 11 ++++++++++-
 synapse/rest/admin/rooms.py |  6 ++++--
 2 files changed, 14 insertions(+), 3 deletions(-)

diff --git a/synapse/handlers/room.py b/synapse/handlers/room.py
index 3ab58c46b0f0..8d315ac5af86 100644
--- a/synapse/handlers/room.py
+++ b/synapse/handlers/room.py
@@ -30,6 +30,8 @@
     Tuple,
 )
 
+from typing_extensions import TypedDict
+
 from synapse.api.constants import (
     EventContentFields,
     EventTypes,
@@ -1274,6 +1276,13 @@ def get_current_key_for_room(self, room_id: str) -> Awaitable[str]:
         return self.store.get_room_events_max_id(room_id)
 
 
+class ShutdownRoomResponse(TypedDict):
+    kicked_users: List[str]
+    failed_to_kick_users: List[str]
+    local_aliases: List[str]
+    new_room_id: Optional[str]
+
+
 class RoomShutdownHandler:
 
     DEFAULT_MESSAGE = (
@@ -1299,7 +1308,7 @@ async def shutdown_room(
         new_room_name: Optional[str] = None,
         message: Optional[str] = None,
         block: bool = False,
-    ) -> dict:
+    ) -> ShutdownRoomResponse:
         """
         Shuts down a room. Moves all local users and room aliases automatically
         to a new room if `new_room_user_id` is set. Otherwise local users only
diff --git a/synapse/rest/admin/rooms.py b/synapse/rest/admin/rooms.py
index 05c5b4bf0c84..5f1d27ac1435 100644
--- a/synapse/rest/admin/rooms.py
+++ b/synapse/rest/admin/rooms.py
@@ -13,7 +13,7 @@
 # limitations under the License.
 import logging
 from http import HTTPStatus
-from typing import TYPE_CHECKING, List, Optional, Tuple
+from typing import TYPE_CHECKING, List, Optional, Tuple, cast
 from urllib import parse as urlparse
 
 from synapse.api.constants import EventTypes, JoinRules, Membership
@@ -241,7 +241,9 @@ async def _delete_room(
         if purge:
             await pagination_handler.purge_room(room_id, force=force_purge)
 
-        return 200, ret
+        # Cast safety: I'm casting away the knowledge that this is a TypedDict.
+        # `ret` is an opaque dictionary blob as far as the rest of the app cares.
+        return 200, cast(JsonDict, ret)
 
 
 class RoomMembersRestServlet(RestServlet):

From 0ed7a3157ff4c991fc1ac437c7fec158ff77c126 Mon Sep 17 00:00:00 2001
From: David Robertson <davidr@element.io>
Date: Mon, 1 Nov 2021 16:01:46 +0000
Subject: [PATCH 05/15] Test that you can block an unknown room

---
 tests/rest/admin/test_room.py | 28 ++++++++++++++++++++++++++++
 1 file changed, 28 insertions(+)

diff --git a/tests/rest/admin/test_room.py b/tests/rest/admin/test_room.py
index b62a7248e820..01241455dc24 100644
--- a/tests/rest/admin/test_room.py
+++ b/tests/rest/admin/test_room.py
@@ -14,9 +14,12 @@
 
 import json
 import urllib.parse
+from http import HTTPStatus
 from typing import List, Optional
 from unittest.mock import Mock
 
+from parameterized import parameterized
+
 import synapse.rest.admin
 from synapse.api.constants import EventTypes, Membership
 from synapse.api.errors import Codes
@@ -281,6 +284,31 @@ def test_block_room_and_not_purge(self):
         self._is_blocked(self.room_id, expect=True)
         self._has_no_members(self.room_id)
 
+    @parameterized.expand([(True,), (False,)])
+    def test_block_unknown_room(self, purge: bool) -> None:
+        """
+        We can block an unknown room. In this case, the `purge` argument
+        should be ignored.
+        """
+        room_id = "!unknown:test"
+
+        # The room isn't already in the blocked rooms table
+        self._is_blocked(room_id, expect=False)
+
+        # Request the room be blocked.
+        channel = self.make_request(
+            self.method,
+            f"/_synapse/admin/v1/rooms/{room_id}",
+            {"block": True, "purge": purge},
+            access_token=self.admin_user_tok,
+        )
+
+        # The room is now blocked.
+        self.assertEqual(
+            HTTPStatus.OK, int(channel.result["code"]), msg=channel.result["body"]
+        )
+        self._is_blocked(room_id)
+
     def test_shutdown_room_consent(self):
         """Test that we can shutdown rooms with local users who have not
         yet accepted the privacy policy. This used to fail when we tried to

From 933197b7c40a7ea04cf06afbb9d2e3f4fe95c176 Mon Sep 17 00:00:00 2001
From: David Robertson <davidr@element.io>
Date: Mon, 1 Nov 2021 16:03:24 +0000
Subject: [PATCH 06/15] Allow blocking an unknown room

---
 synapse/handlers/room.py    | 22 +++++++++++++++++-----
 synapse/rest/admin/rooms.py | 11 ++++++++++-
 2 files changed, 27 insertions(+), 6 deletions(-)

diff --git a/synapse/handlers/room.py b/synapse/handlers/room.py
index 8d315ac5af86..03b2cd8449a1 100644
--- a/synapse/handlers/room.py
+++ b/synapse/handlers/room.py
@@ -1365,14 +1365,26 @@ async def shutdown_room(
         if not RoomID.is_valid(room_id):
             raise SynapseError(400, "%s is not a legal room ID" % (room_id,))
 
-        if not await self.store.get_room(room_id):
-            raise NotFoundError("Unknown room id %s" % (room_id,))
-
-        # This will work even if the room is already blocked, but that is
-        # desirable in case the first attempt at blocking the room failed below.
+        # Action the block first (even if the room doesn't exist yet)
         if block:
+            # This will work even if the room is already blocked, but that is
+            # desirable in case the first attempt at blocking the room failed below.
             await self.store.block_room(room_id, requester_user_id)
 
+        if not await self.store.get_room(room_id):
+            if block:
+                # We allow you to block an unknown room.
+                return {
+                    "kicked_users": [],
+                    "failed_to_kick_users": [],
+                    "local_aliases": [],
+                    "new_room_id": None,
+                }
+            else:
+                # But if you don't want to preventatively block another room,
+                # this function can't do anything useful.
+                raise NotFoundError("Unknown room id %s" % (room_id,))
+
         if new_room_user_id is not None:
             if not self.hs.is_mine_id(new_room_user_id):
                 raise SynapseError(
diff --git a/synapse/rest/admin/rooms.py b/synapse/rest/admin/rooms.py
index 5f1d27ac1435..8e01ed3c208b 100644
--- a/synapse/rest/admin/rooms.py
+++ b/synapse/rest/admin/rooms.py
@@ -239,7 +239,16 @@ async def _delete_room(
 
         # Purge room
         if purge:
-            await pagination_handler.purge_room(room_id, force=force_purge)
+            try:
+                await pagination_handler.purge_room(room_id, force=force_purge)
+            except NotFoundError:
+                if block:
+                    # We can block unknown rooms with this endpoint, in which case
+                    # a failed purge is expected.
+                    pass
+                else:
+                    # But otherwise, we expect this purge to have succeeded.
+                    raise
 
         # Cast safety: I'm casting away the knowledge that this is a TypedDict.
         # `ret` is an opaque dictionary blob as far as the rest of the app cares.

From 3bd4984b3a7b318e520da4dc925c2ce1f393e7bb Mon Sep 17 00:00:00 2001
From: David Robertson <davidr@element.io>
Date: Mon, 1 Nov 2021 16:29:23 +0000
Subject: [PATCH 07/15] Changelog

---
 changelog.d/11228.feature | 1 +
 1 file changed, 1 insertion(+)
 create mode 100644 changelog.d/11228.feature

diff --git a/changelog.d/11228.feature b/changelog.d/11228.feature
new file mode 100644
index 000000000000..a4528a6b712f
--- /dev/null
+++ b/changelog.d/11228.feature
@@ -0,0 +1 @@
+Allow the admin [Delete Room API](https://matrix-org.github.io/synapse/latest/admin_api/rooms.html#delete-room-api) to block a room before this server joins it.

From 92efc6265377304d93760fd703fdc76629683363 Mon Sep 17 00:00:00 2001
From: David Robertson <davidr@element.io>
Date: Tue, 2 Nov 2021 10:54:22 +0000
Subject: [PATCH 08/15] Update tests/rest/admin/test_room.py

Co-authored-by: Dirk Klimpel <5740567+dklimpel@users.noreply.github.com>
---
 tests/rest/admin/test_room.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/rest/admin/test_room.py b/tests/rest/admin/test_room.py
index 01241455dc24..bc9ab2c1d873 100644
--- a/tests/rest/admin/test_room.py
+++ b/tests/rest/admin/test_room.py
@@ -297,7 +297,7 @@ def test_block_unknown_room(self, purge: bool) -> None:
 
         # Request the room be blocked.
         channel = self.make_request(
-            self.method,
+            "DELETE",
             f"/_synapse/admin/v1/rooms/{room_id}",
             {"block": True, "purge": purge},
             access_token=self.admin_user_tok,

From b820b60a6d4219e7eae64d91e3a9e98b3a1deb8d Mon Sep 17 00:00:00 2001
From: David Robertson <davidr@element.io>
Date: Fri, 5 Nov 2021 14:43:47 +0000
Subject: [PATCH 09/15] Update changelog.d/11228.feature

Co-authored-by: Andrew Morgan <1342360+anoadragon453@users.noreply.github.com>
---
 changelog.d/11228.feature | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/changelog.d/11228.feature b/changelog.d/11228.feature
index a4528a6b712f..33c1756b50fb 100644
--- a/changelog.d/11228.feature
+++ b/changelog.d/11228.feature
@@ -1 +1 @@
-Allow the admin [Delete Room API](https://matrix-org.github.io/synapse/latest/admin_api/rooms.html#delete-room-api) to block a room before this server joins it.
+Allow the admin [Delete Room API](https://matrix-org.github.io/synapse/latest/admin_api/rooms.html#delete-room-api) to block a room without the need to join it.

From 8f9bf40cb5dd015a61bb4542d0effb72dfb109ff Mon Sep 17 00:00:00 2001
From: David Robertson <davidr@element.io>
Date: Fri, 5 Nov 2021 14:45:14 +0000
Subject: [PATCH 10/15] Update synapse/handlers/room.py

Co-authored-by: Andrew Morgan <1342360+anoadragon453@users.noreply.github.com>
---
 synapse/handlers/room.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/synapse/handlers/room.py b/synapse/handlers/room.py
index 03b2cd8449a1..1e5a9ac811f7 100644
--- a/synapse/handlers/room.py
+++ b/synapse/handlers/room.py
@@ -1383,7 +1383,7 @@ async def shutdown_room(
             else:
                 # But if you don't want to preventatively block another room,
                 # this function can't do anything useful.
-                raise NotFoundError("Unknown room id %s" % (room_id,))
+                raise NotFoundError("Cannot shut down room: unknown room id %s" % (room_id,))
 
         if new_room_user_id is not None:
             if not self.hs.is_mine_id(new_room_user_id):

From a2da43861a6ccfbf4c1121c59e885d1329a76e31 Mon Sep 17 00:00:00 2001
From: David Robertson <davidr@element.io>
Date: Fri, 5 Nov 2021 14:26:45 +0000
Subject: [PATCH 11/15] Cast safety comment

---
 synapse/rest/admin/rooms.py | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/synapse/rest/admin/rooms.py b/synapse/rest/admin/rooms.py
index 8e01ed3c208b..aa0e2087fa11 100644
--- a/synapse/rest/admin/rooms.py
+++ b/synapse/rest/admin/rooms.py
@@ -250,7 +250,9 @@ async def _delete_room(
                     # But otherwise, we expect this purge to have succeeded.
                     raise
 
-        # Cast safety: I'm casting away the knowledge that this is a TypedDict.
+        # Cast safety: cast away the knowledge that this is a TypedDict.
+        # See https://github.com/python/mypy/issues/4976#issuecomment-579883622
+        # for some discussion on why this is necessary. Either way,
         # `ret` is an opaque dictionary blob as far as the rest of the app cares.
         return 200, cast(JsonDict, ret)
 

From de6aa379c75bb579f63ea57c76c0d4c6a6df44d5 Mon Sep 17 00:00:00 2001
From: David Robertson <davidr@element.io>
Date: Fri, 5 Nov 2021 14:59:10 +0000
Subject: [PATCH 12/15] Update docs(trings)

---
 docs/admin_api/rooms.md                | 10 +++++++---
 synapse/handlers/room.py               |  6 ++++--
 synapse/storage/databases/main/room.py |  7 ++++++-
 3 files changed, 17 insertions(+), 6 deletions(-)

diff --git a/docs/admin_api/rooms.md b/docs/admin_api/rooms.md
index 98320d174044..42b5d172c615 100644
--- a/docs/admin_api/rooms.md
+++ b/docs/admin_api/rooms.md
@@ -391,7 +391,10 @@ The new room will be created with the user specified by the `new_room_user_id` p
 as room administrator and will contain a message explaining what happened. Users invited
 to the new room will have power level `-10` by default, and thus be unable to speak.
 
-If `block` is `True` it prevents new joins to the old room.
+If `block` is `true`, users will be prevented from joining the old room.
+This option can also be used to pre-emptively block a room, even if it's unknown
+to this homeserver. (If `block` is `false`, attempting to delete an unknown room
+is invalid and will be rejected as a bad request.)
 
 This API will remove all trace of the old room from your database after removing
 all local users. If `purge` is `true` (the default), all traces of the old room will
@@ -459,8 +462,9 @@ The following JSON body parameters are available:
               `new_room_user_id` in the new room. Ideally this will clearly convey why the
                original room was shut down. Defaults to `Sharing illegal content on this server
                is not permitted and rooms in violation will be blocked.`
-* `block` - Optional. If set to `true`, this room will be added to a blocking list, preventing
-            future attempts to join the room. Defaults to `false`.
+* `block` - Optional. If set to `true`, this room will be added to a blocking list,
+            preventing future attempts to join the room. Rooms can be blocked
+            even if they're not yet known to the homeserver. Defaults to `false`.
 * `purge` - Optional. If set to `true`, it will remove all traces of the room from your database.
             Defaults to `true`.
 * `force_purge` - Optional, and ignored unless `purge` is `true`. If set to `true`, it
diff --git a/synapse/handlers/room.py b/synapse/handlers/room.py
index 1e5a9ac811f7..79d1c9345c0a 100644
--- a/synapse/handlers/room.py
+++ b/synapse/handlers/room.py
@@ -1342,8 +1342,10 @@ async def shutdown_room(
                 Defaults to `Sharing illegal content on this server is not
                 permitted and rooms in violation will be blocked.`
             block:
-                If set to `true`, this room will be added to a blocking list,
-                preventing future attempts to join the room. Defaults to `false`.
+                If set to `True`, this room will be added to a blocking list,
+                preventing future attempts to join the room. Rooms can be blocked
+                even if they're not yet known to the homeserver. Defaults to
+                `False`.
 
         Returns: a dict containing the following keys:
             kicked_users: An array of users (`user_id`) that were kicked.
diff --git a/synapse/storage/databases/main/room.py b/synapse/storage/databases/main/room.py
index f879bbe7c720..d755345eafce 100644
--- a/synapse/storage/databases/main/room.py
+++ b/synapse/storage/databases/main/room.py
@@ -1744,7 +1744,12 @@ def _get_event_reports_paginate_txn(txn):
         )
 
     async def block_room(self, room_id: str, user_id: str) -> None:
-        """Marks the room as blocked. Can be called multiple times.
+        """Marks the room as blocked.
+
+        Can be called multiple times (though we'll only track the last user to
+        block this room).
+
+        Can be called on a room unknown to this homeserver.
 
         Args:
             room_id: Room to block

From c5fccfad50e2ccca2bf818e96f085d0f88ddfa5b Mon Sep 17 00:00:00 2001
From: David Robertson <davidr@element.io>
Date: Fri, 5 Nov 2021 15:01:04 +0000
Subject: [PATCH 13/15] Lint

---
 synapse/handlers/room.py | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/synapse/handlers/room.py b/synapse/handlers/room.py
index 79d1c9345c0a..6841e5543342 100644
--- a/synapse/handlers/room.py
+++ b/synapse/handlers/room.py
@@ -1385,7 +1385,9 @@ async def shutdown_room(
             else:
                 # But if you don't want to preventatively block another room,
                 # this function can't do anything useful.
-                raise NotFoundError("Cannot shut down room: unknown room id %s" % (room_id,))
+                raise NotFoundError(
+                    "Cannot shut down room: unknown room id %s" % (room_id,)
+                )
 
         if new_room_user_id is not None:
             if not self.hs.is_mine_id(new_room_user_id):

From e3970687fad8a270cec6f45d685edccfe7be27d6 Mon Sep 17 00:00:00 2001
From: David Robertson <davidr@element.io>
Date: Tue, 9 Nov 2021 12:32:38 +0000
Subject: [PATCH 14/15] Update docs/admin_api/rooms.md

Co-authored-by: Andrew Morgan <1342360+anoadragon453@users.noreply.github.com>
---
 docs/admin_api/rooms.md | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/docs/admin_api/rooms.md b/docs/admin_api/rooms.md
index 42b5d172c615..1df087e42ff4 100644
--- a/docs/admin_api/rooms.md
+++ b/docs/admin_api/rooms.md
@@ -393,8 +393,9 @@ to the new room will have power level `-10` by default, and thus be unable to sp
 
 If `block` is `true`, users will be prevented from joining the old room.
 This option can also be used to pre-emptively block a room, even if it's unknown
-to this homeserver. (If `block` is `false`, attempting to delete an unknown room
-is invalid and will be rejected as a bad request.)
+to this homeserver. In this case, the room will be blocked, and no further action
+will be taken. If `block` is `false`, attempting to delete an unknown room is
+invalid and will be rejected as a bad request.
 
 This API will remove all trace of the old room from your database after removing
 all local users. If `purge` is `true` (the default), all traces of the old room will

From 5402a693aeaf0619a5ebf48b28b75fa2b39a449d Mon Sep 17 00:00:00 2001
From: David Robertson <davidr@element.io>
Date: Tue, 9 Nov 2021 12:43:50 +0000
Subject: [PATCH 15/15] Update docstring to match

---
 synapse/handlers/room.py | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/synapse/handlers/room.py b/synapse/handlers/room.py
index 6841e5543342..8b8e941ffaba 100644
--- a/synapse/handlers/room.py
+++ b/synapse/handlers/room.py
@@ -1342,10 +1342,13 @@ async def shutdown_room(
                 Defaults to `Sharing illegal content on this server is not
                 permitted and rooms in violation will be blocked.`
             block:
-                If set to `True`, this room will be added to a blocking list,
-                preventing future attempts to join the room. Rooms can be blocked
-                even if they're not yet known to the homeserver. Defaults to
-                `False`.
+                If set to `True`, users will be prevented from joining the old
+                room. This option can also be used to pre-emptively block a room,
+                even if it's unknown to this homeserver. In this case, the room
+                will be blocked, and no further action will be taken. If `False`,
+                attempting to delete an unknown room is invalid.
+
+                Defaults to `False`.
 
         Returns: a dict containing the following keys:
             kicked_users: An array of users (`user_id`) that were kicked.