Fix 'delete room' admin api to work on incomplete rooms (#11523)

If, for some reason, we don't have the create event, we should still be able to
purge a room.
This commit is contained in:
Richard van der Hoff 2021-12-07 11:37:54 +00:00 committed by GitHub
parent 9c55dedc8c
commit b1ecd19c5d
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
5 changed files with 33 additions and 37 deletions

View File

@ -0,0 +1 @@
Extend the "delete room" admin api to work correctly on rooms which have previously been partially deleted.

View File

@ -406,9 +406,6 @@ class PaginationHandler:
force: set true to skip checking for joined users. force: set true to skip checking for joined users.
""" """
with await self.pagination_lock.write(room_id): with await self.pagination_lock.write(room_id):
# check we know about the room
await self.store.get_room_version_id(room_id)
# first check that we have no users in this room # first check that we have no users in this room
if not force: if not force:
joined = await self.store.is_host_joined(room_id, self._server_name) joined = await self.store.is_host_joined(room_id, self._server_name)

View File

@ -1535,20 +1535,13 @@ class RoomShutdownHandler:
await self.store.block_room(room_id, requester_user_id) await self.store.block_room(room_id, requester_user_id)
if not await self.store.get_room(room_id): if not await self.store.get_room(room_id):
if block: # if we don't know about the room, there is nothing left to do.
# We allow you to block an unknown room. return {
return { "kicked_users": [],
"kicked_users": [], "failed_to_kick_users": [],
"failed_to_kick_users": [], "local_aliases": [],
"local_aliases": [], "new_room_id": None,
"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(
"Cannot shut down room: unknown room id %s" % (room_id,)
)
if new_room_user_id is not None: if new_room_user_id is not None:
if not self.hs.is_mine_id(new_room_user_id): if not self.hs.is_mine_id(new_room_user_id):

View File

@ -106,9 +106,6 @@ class RoomRestV2Servlet(RestServlet):
HTTPStatus.BAD_REQUEST, "%s is not a legal room ID" % (room_id,) HTTPStatus.BAD_REQUEST, "%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,))
delete_id = self._pagination_handler.start_shutdown_and_purge_room( delete_id = self._pagination_handler.start_shutdown_and_purge_room(
room_id=room_id, room_id=room_id,
new_room_user_id=content.get("new_room_user_id"), new_room_user_id=content.get("new_room_user_id"),

View File

@ -83,7 +83,7 @@ class DeleteRoomTestCase(unittest.HomeserverTestCase):
def test_room_does_not_exist(self): def test_room_does_not_exist(self):
""" """
Check that unknown rooms/server return error HTTPStatus.NOT_FOUND. Check that unknown rooms/server return 200
""" """
url = "/_synapse/admin/v1/rooms/%s" % "!unknown:test" url = "/_synapse/admin/v1/rooms/%s" % "!unknown:test"
@ -94,8 +94,7 @@ class DeleteRoomTestCase(unittest.HomeserverTestCase):
access_token=self.admin_user_tok, access_token=self.admin_user_tok,
) )
self.assertEqual(HTTPStatus.NOT_FOUND, channel.code, msg=channel.json_body) self.assertEqual(HTTPStatus.OK, channel.code, msg=channel.json_body)
self.assertEqual(Codes.NOT_FOUND, channel.json_body["errcode"])
def test_room_is_not_valid(self): def test_room_is_not_valid(self):
""" """
@ -508,27 +507,36 @@ class DeleteRoomV2TestCase(unittest.HomeserverTestCase):
self.assertEqual(HTTPStatus.FORBIDDEN, channel.code, msg=channel.json_body) self.assertEqual(HTTPStatus.FORBIDDEN, channel.code, msg=channel.json_body)
self.assertEqual(Codes.FORBIDDEN, channel.json_body["errcode"]) self.assertEqual(Codes.FORBIDDEN, channel.json_body["errcode"])
@parameterized.expand( def test_room_does_not_exist(self):
[
("DELETE", "/_synapse/admin/v2/rooms/%s"),
("GET", "/_synapse/admin/v2/rooms/%s/delete_status"),
("GET", "/_synapse/admin/v2/rooms/delete_status/%s"),
]
)
def test_room_does_not_exist(self, method: str, url: str):
"""
Check that unknown rooms/server return error HTTPStatus.NOT_FOUND.
""" """
Check that unknown rooms/server return 200
This is important, as it allows incomplete vestiges of rooms to be cleared up
even if the create event/etc is missing.
"""
room_id = "!unknown:test"
channel = self.make_request( channel = self.make_request(
method, "DELETE",
url % "!unknown:test", f"/_synapse/admin/v2/rooms/{room_id}",
content={}, content={},
access_token=self.admin_user_tok, access_token=self.admin_user_tok,
) )
self.assertEqual(HTTPStatus.NOT_FOUND, channel.code, msg=channel.json_body) self.assertEqual(HTTPStatus.OK, channel.code, msg=channel.json_body)
self.assertEqual(Codes.NOT_FOUND, channel.json_body["errcode"]) self.assertIn("delete_id", channel.json_body)
delete_id = channel.json_body["delete_id"]
# get status
channel = self.make_request(
"GET",
f"/_synapse/admin/v2/rooms/{room_id}/delete_status",
access_token=self.admin_user_tok,
)
self.assertEqual(HTTPStatus.OK, channel.code, msg=channel.json_body)
self.assertEqual(1, len(channel.json_body["results"]))
self.assertEqual("complete", channel.json_body["results"][0]["status"])
self.assertEqual(delete_id, channel.json_body["results"][0]["delete_id"])
@parameterized.expand( @parameterized.expand(
[ [