From d3493872da0cc3691df6bc0402bda629bcfdab3a Mon Sep 17 00:00:00 2001 From: dklimpel <5740567+dklimpel@users.noreply.github.com> Date: Mon, 1 Jun 2020 16:56:14 +0200 Subject: [PATCH 01/12] Add delete room admin endpoint --- docs/admin_api/purge_room.md | 2 + docs/admin_api/rooms.md | 85 +++++ docs/admin_api/shutdown_room.md | 2 + synapse/rest/admin/__init__.py | 2 + synapse/rest/admin/rooms.py | 190 +++++++++++ synapse/storage/data_stores/main/room.py | 11 +- tests/rest/admin/test_room.py | 393 +++++++++++++++++++++++ tests/storage/test_room.py | 4 + 8 files changed, 685 insertions(+), 4 deletions(-) diff --git a/docs/admin_api/purge_room.md b/docs/admin_api/purge_room.md index 64ea7b6a648e..ae01a543c604 100644 --- a/docs/admin_api/purge_room.md +++ b/docs/admin_api/purge_room.md @@ -5,6 +5,8 @@ This API will remove all trace of a room from your database. All local users must have left the room before it can be removed. +See also: [Delete Room API](rooms.md#delete-room-api) + The API is: ``` diff --git a/docs/admin_api/rooms.md b/docs/admin_api/rooms.md index 624e7745baa7..426636976c61 100644 --- a/docs/admin_api/rooms.md +++ b/docs/admin_api/rooms.md @@ -318,3 +318,88 @@ Response: "state_events": 93534 } ``` + +# Delete Room API + +The Delete Room admin API allows server admins to remove rooms from server +and block these rooms. +It is a combination and improvement of "[Shutdown room](shutdown_room.md)" +and "[Purge room](purge_room.md)" API. + +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 +leaves the room without any information. + +The new room will be created with the user specified by the `new_room_user_id` parameter +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 preventing new joins to the old room. + +This API will remove all trace of the old room from your database after removing +all local users. +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 +the new room. Users on other servers will be unaffected. + +## Parameters + +The following query parameters are available: + +* `room_id` - The ID of the room. + +The following JSON body parameters are available: + +* `new_room_user_id` - Optional. A string representing the user ID of the user that will admin + the new room that all users in the old room will be moved to. If not + set the users will not be moved to a new room and only leave the old room + without any information. Defaults to `None`. +* `room_name` - Optional. A string representing the name of the room that new users will be + invited to. Defaults to `Content Violation Notification` +* `message` - Optional. A string containing the first message that will be sent as + `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. A boolean if `room_id` will be set on blocking list. The room will be + blocked for this server and preventing new joins. Defaults to `True`. + +The following fields are possible in the JSON response body: + +* `kicked_users` - An array of users (`user_id`) that were kicked. +* `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. + +## Usage + +A standard request: + +```json +DELETE /_synapse/admin/v1/rooms/ + +{ + "new_room_user_id": "@someuser:example.com", + "room_name": "Content Violation Notification", + "message": "Bad Room has been shutdown due to content violations on this server. Please review our Terms of Service.", + "block": true +} +``` + +Response: + +```json +{ + "kicked_users": [ + "@foobar:example.com" + ], + "failed_to_kick_users": [], + "local_aliases": [ + "#badroom:example.com", + "#evilsaloon:example.com" + ], + "new_room_id": "!newroomid:example.com" +} +``` diff --git a/docs/admin_api/shutdown_room.md b/docs/admin_api/shutdown_room.md index 54ce1cd2349b..808caeec7903 100644 --- a/docs/admin_api/shutdown_room.md +++ b/docs/admin_api/shutdown_room.md @@ -10,6 +10,8 @@ disallow any further invites or joins. The local server will only have the power to move local user and room aliases to the new room. Users on other servers will be unaffected. +See also: [Delete Room API](rooms.md#delete-room-api) + ## API You will need to authenticate with an access token for an admin user. diff --git a/synapse/rest/admin/__init__.py b/synapse/rest/admin/__init__.py index 6b85148a32fb..8d11b51eaac8 100644 --- a/synapse/rest/admin/__init__.py +++ b/synapse/rest/admin/__init__.py @@ -30,6 +30,7 @@ from synapse.rest.admin.media import ListMediaInRoom, register_servlets_for_media_repo from synapse.rest.admin.purge_room_servlet import PurgeRoomServlet from synapse.rest.admin.rooms import ( + DeleteRoomRestServlet, JoinRoomAliasServlet, ListRoomRestServlet, RoomRestServlet, @@ -195,6 +196,7 @@ def register_servlets(hs, http_server): register_servlets_for_client_rest_resource(hs, http_server) ListRoomRestServlet(hs).register(http_server) RoomRestServlet(hs).register(http_server) + DeleteRoomRestServlet(hs).register(http_server) JoinRoomAliasServlet(hs).register(http_server) PurgeRoomServlet(hs).register(http_server) SendServerNoticeServlet(hs).register(http_server) diff --git a/synapse/rest/admin/rooms.py b/synapse/rest/admin/rooms.py index 8173baef8f2c..84190e811f93 100644 --- a/synapse/rest/admin/rooms.py +++ b/synapse/rest/admin/rooms.py @@ -15,6 +15,8 @@ import logging from typing import List, Optional +from six.moves import http_client + from synapse.api.constants import EventTypes, JoinRules, Membership from synapse.api.errors import Codes, NotFoundError, SynapseError from synapse.http.servlet import ( @@ -179,6 +181,194 @@ async def on_POST(self, request, room_id): ) +class DeleteRoomRestServlet(RestServlet): + """Delete a room from server. It is a combination and improvement of + shut down 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. + It will remove all trace of a room from the database. + """ + + PATTERNS = admin_patterns("/rooms/(?P[^/]+)$") + + DEFAULT_MESSAGE = ( + "Sharing illegal content on this server is not permitted and rooms in" + " violation will be blocked." + ) + DEFAULT_ROOM_NAME = "Content Violation Notification" + + def __init__(self, hs): + self.hs = hs + self.store = hs.get_datastore() + self.state = hs.get_state_handler() + self._room_creation_handler = hs.get_room_creation_handler() + self.event_creation_handler = hs.get_event_creation_handler() + self.room_member_handler = hs.get_room_member_handler() + self.auth = hs.get_auth() + self._replication = hs.get_replication_data_handler() + self.pagination_handler = hs.get_pagination_handler() + self.admin_handler = hs.get_handlers().admin_handler + + async def on_DELETE(self, request, room_id): + 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) + requester_user_id = requester.user.to_string() + + # Check RoomID + if not RoomID.is_valid(room_id): + raise SynapseError(400, "%s was not legal room ID" % (room_id)) + + if not await self.store.get_room_with_stats(room_id): + raise NotFoundError("Room not found") + + # Check parameter `block` + block = content.get("block", True) + if not isinstance(block, bool): + raise SynapseError( + http_client.BAD_REQUEST, + "Param 'block' must be a boolean, if given", + Codes.BAD_JSON, + ) + + # 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. + if block: + await self.store.block_room(room_id, requester_user_id) + + new_room_user_id = content.get("new_room_user_id") + if new_room_user_id: + if not self.hs.is_mine_id(new_room_user_id): + raise SynapseError( + 400, "This endpoint can only be used with local users" + ) + + if not await self.admin_handler.get_user( + UserID.from_string(new_room_user_id) + ): + raise NotFoundError("User not found") + + room_creator_requester = create_requester(new_room_user_id) + + message = content.get("message", self.DEFAULT_MESSAGE) + room_name = content.get("room_name", self.DEFAULT_ROOM_NAME) + + info, stream_id = await self._room_creation_handler.create_room( + room_creator_requester, + config={ + "preset": "public_chat", + "name": room_name, + "power_level_content_override": {"users_default": -10}, + }, + ratelimit=False, + ) + new_room_id = info["room_id"] + + logger.info( + "Shutting down room %r, joining to new room: %r", room_id, new_room_id + ) + + # We now wait for the create room to come back in via replication so + # that we can assume that all the joins/invites have propogated before + # we try and auto join below. + # + # TODO: Currently the events stream is written to from master + await self._replication.wait_for_stream_position( + self.hs.config.worker.writers.events, "events", stream_id + ) + + users = await self.state.get_current_users_in_room(room_id) + kicked_users = [] + failed_to_kick_users = [] + for user_id in users: + if not self.hs.is_mine_id(user_id): + continue + + logger.info("Kicking %r from %r...", user_id, room_id) + + # Kick users from room + try: + target_requester = create_requester(user_id) + _, stream_id = await self.room_member_handler.update_membership( + requester=target_requester, + target=target_requester.user, + room_id=room_id, + action=Membership.LEAVE, + content={}, + ratelimit=False, + require_consent=False, + ) + + # Wait for leave to come in over replication before trying to forget. + await self._replication.wait_for_stream_position( + self.hs.config.worker.writers.events, "events", stream_id + ) + + await self.room_member_handler.forget(target_requester.user, room_id) + kicked_users.append(user_id) + except Exception: + logger.exception("Failed to leave old room for %r", user_id) + failed_to_kick_users.append(user_id) + + # Join users to new room + try: + if new_room_user_id: + await self.room_member_handler.update_membership( + requester=target_requester, + target=target_requester.user, + room_id=new_room_id, + action=Membership.JOIN, + content={}, + ratelimit=False, + require_consent=False, + ) + except Exception: + logger.exception("Failed to join new room for %r", user_id) + + # Send message in new room and move aliases + if new_room_user_id: + await self.event_creation_handler.create_and_send_nonmember_event( + room_creator_requester, + { + "type": "m.room.message", + "content": {"body": message, "msgtype": "m.text"}, + "room_id": new_room_id, + "sender": new_room_user_id, + }, + ratelimit=False, + ) + + aliases_for_room = await maybe_awaitable( + self.store.get_aliases_for_room(room_id) + ) + + await self.store.update_aliases_for_room( + room_id, new_room_id, requester_user_id + ) + + # Purge room + await self.pagination_handler.purge_room(room_id) + + # Set empty values if no new room was created + if not new_room_user_id: + aliases_for_room = [] + new_room_id = "" + + return ( + 200, + { + "kicked_users": kicked_users, + "failed_to_kick_users": failed_to_kick_users, + "local_aliases": aliases_for_room, + "new_room_id": new_room_id, + }, + ) + + class ListRoomRestServlet(RestServlet): """ List all rooms that are known to the homeserver. Results are returned diff --git a/synapse/storage/data_stores/main/room.py b/synapse/storage/data_stores/main/room.py index 46f643c6b904..02abe571dcc6 100644 --- a/synapse/storage/data_stores/main/room.py +++ b/synapse/storage/data_stores/main/room.py @@ -118,10 +118,13 @@ def get_room_with_stats_txn(txn, room_id): WHERE room_id = ? """ txn.execute(sql, [room_id]) - res = self.db.cursor_to_dict(txn)[0] - res["federatable"] = bool(res["federatable"]) - res["public"] = bool(res["public"]) - return res + try: + res = self.db.cursor_to_dict(txn)[0] + res["federatable"] = bool(res["federatable"]) + res["public"] = bool(res["public"]) + return res + except IndexError: + return return self.db.runInteraction( "get_room_with_stats", get_room_with_stats_txn, room_id diff --git a/tests/rest/admin/test_room.py b/tests/rest/admin/test_room.py index 54cd24bf645d..11c60a84a1d8 100644 --- a/tests/rest/admin/test_room.py +++ b/tests/rest/admin/test_room.py @@ -151,6 +151,399 @@ def _assert_peek(self, room_id, expect_code): ) +class DeleteRoomTestCase(unittest.HomeserverTestCase): + servlets = [ + synapse.rest.admin.register_servlets, + login.register_servlets, + events.register_servlets, + room.register_servlets, + room.register_deprecated_servlets, + ] + + def prepare(self, reactor, clock, hs): + self.event_creation_handler = hs.get_event_creation_handler() + hs.config.user_consent_version = "1" + + consent_uri_builder = Mock() + consent_uri_builder.build_user_consent_uri.return_value = "http://example.com" + self.event_creation_handler._consent_uri_builder = consent_uri_builder + + self.store = hs.get_datastore() + + 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") + + # Mark the admin user as having consented + self.get_success(self.store.user_set_consent_version(self.admin_user, "1")) + + self.room_id = self.helper.create_room_as( + self.other_user, tok=self.other_user_tok + ) + self.url = "/_synapse/admin/v1/rooms/%s" % self.room_id + + def test_requester_is_no_admin(self): + """ + If the user is not a server admin, an error 403 is returned. + """ + + request, channel = self.make_request( + "DELETE", self.url, access_token=self.other_user_tok, + ) + self.render(request) + + self.assertEqual(403, int(channel.result["code"]), msg=channel.result["body"]) + self.assertEqual(Codes.FORBIDDEN, channel.json_body["errcode"]) + + def test_room_does_not_exist(self): + """ + Check that unknown rooms/server return error 404. + """ + url = "/_synapse/admin/v1/rooms/!unknown:test" + + request, channel = self.make_request( + "DELETE", url, access_token=self.admin_user_tok, + ) + self.render(request) + + self.assertEqual(404, int(channel.result["code"]), msg=channel.result["body"]) + self.assertEqual(Codes.NOT_FOUND, channel.json_body["errcode"]) + + def test_room_is_not_valid(self): + """ + Check that invalid room names, return an error 400. + """ + url = "/_synapse/admin/v1/rooms/invalidroom" + + request, channel = self.make_request( + "DELETE", url, access_token=self.admin_user_tok, + ) + self.render(request) + + self.assertEqual(400, int(channel.result["code"]), msg=channel.result["body"]) + self.assertEqual( + "invalidroom was not legal room ID", channel.json_body["error"], + ) + + def test_new_room_user_does_not_exist(self): + """ + Tests that a lookup for a user that does not exist returns a 404 + """ + body = json.dumps({"new_room_user_id": "@unknown:test"}) + + request, channel = self.make_request( + "DELETE", + self.url, + content=body.encode(encoding="utf_8"), + access_token=self.admin_user_tok, + ) + self.render(request) + + self.assertEqual(404, int(channel.result["code"]), msg=channel.result["body"]) + self.assertEqual(Codes.NOT_FOUND, channel.json_body["errcode"]) + + def test_new_room_user_is_not_local(self): + """ + Check that only local users can create new room to move members. + """ + body = json.dumps({"new_room_user_id": "@not:exist.bla"}) + + request, channel = self.make_request( + "DELETE", + self.url, + content=body.encode(encoding="utf_8"), + access_token=self.admin_user_tok, + ) + self.render(request) + + self.assertEqual(400, int(channel.result["code"]), msg=channel.result["body"]) + self.assertEqual( + "This endpoint can only be used with local users", + channel.json_body["error"], + ) + + def test_block_is_not_bool(self): + """ + If parameter `block` is not boolean, return an error + """ + body = json.dumps({"block": "NotBool"}) + + request, channel = self.make_request( + "DELETE", + self.url, + content=body.encode(encoding="utf_8"), + access_token=self.admin_user_tok, + ) + self.render(request) + + self.assertEqual(400, int(channel.result["code"]), msg=channel.result["body"]) + self.assertEqual(Codes.BAD_JSON, channel.json_body["errcode"]) + + def test_purge_room_and_block(self): + """Test to purge a room and block it. + Members will not be moved to a new room and will not receive a message. + """ + # Test that room is not purged + with self.assertRaises(AssertionError): + self._is_purged(self.room_id) + + # Test that room is not blocked + self._is_blocked(self.room_id, expect=False) + + # Assert one user in room + self._is_member(room_id=self.room_id, user_id=self.other_user) + + body = json.dumps({"block": True}) + + request, channel = self.make_request( + "DELETE", + self.url.encode("ascii"), + content=body.encode(encoding="utf_8"), + access_token=self.admin_user_tok, + ) + self.render(request) + + self.assertEqual(200, int(channel.result["code"]), msg=channel.result["body"]) + self.assertEqual("", channel.json_body["new_room_id"]) + self.assertEqual(self.other_user, channel.json_body["kicked_users"][0]) + self.assertIn("failed_to_kick_users", channel.json_body) + self.assertIn("local_aliases", channel.json_body) + + self._is_purged(self.room_id) + self._is_blocked(self.room_id, expect=True) + self._has_no_members(self.room_id) + + def test_purge_room_and_not_block(self): + """Test to purge a room and do not block it. + Members will not be moved to a new room and will not receive a message. + """ + # Test that room is not purged + with self.assertRaises(AssertionError): + self._is_purged(self.room_id) + + # Test that room is not blocked + self._is_blocked(self.room_id, expect=False) + + # Assert one user in room + self._is_member(room_id=self.room_id, user_id=self.other_user) + + body = json.dumps({"block": False}) + + request, channel = self.make_request( + "DELETE", + self.url.encode("ascii"), + content=body.encode(encoding="utf_8"), + access_token=self.admin_user_tok, + ) + self.render(request) + + self.assertEqual(200, int(channel.result["code"]), msg=channel.result["body"]) + self.assertEqual("", channel.json_body["new_room_id"]) + self.assertEqual(self.other_user, channel.json_body["kicked_users"][0]) + self.assertIn("failed_to_kick_users", channel.json_body) + self.assertIn("local_aliases", channel.json_body) + + self._is_purged(self.room_id) + self._is_blocked(self.room_id, expect=False) + self._has_no_members(self.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 + force part the user from the old room. + Members will be moved to a new room and will receive a message. + """ + self.event_creation_handler._block_events_without_consent_error = None + + # Assert one user in room + users_in_room = self.get_success(self.store.get_users_in_room(self.room_id)) + self.assertEqual([self.other_user], users_in_room) + + # Enable require consent to send events + self.event_creation_handler._block_events_without_consent_error = "Error" + + # Assert that the user is getting consent error + self.helper.send( + self.room_id, body="foo", tok=self.other_user_tok, expect_code=403 + ) + + # Test that room is not purged + with self.assertRaises(AssertionError): + self._is_purged(self.room_id) + + # Assert one user in room + 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" % self.room_id + request, channel = self.make_request( + "DELETE", + url.encode("ascii"), + json.dumps({"new_room_user_id": self.admin_user}), + access_token=self.admin_user_tok, + ) + self.render(request) + + self.assertEqual(200, int(channel.result["code"]), msg=channel.result["body"]) + self.assertEqual(self.other_user, channel.json_body["kicked_users"][0]) + self.assertIn("new_room_id", channel.json_body) + self.assertIn("failed_to_kick_users", channel.json_body) + self.assertIn("local_aliases", channel.json_body) + + # Test that member has moved to new room + self._is_member( + room_id=channel.json_body["new_room_id"], user_id=self.other_user + ) + + self._is_purged(self.room_id) + self._has_no_members(self.room_id) + + def test_shutdown_room_block_peek(self): + """Test that a world_readable room can no longer be peeked into after + it has been shut down. + Members will be moved to a new room and will receive a message. + """ + self.event_creation_handler._block_events_without_consent_error = None + + # Enable world readable + url = "rooms/%s/state/m.room.history_visibility" % (self.room_id,) + request, channel = self.make_request( + "PUT", + url.encode("ascii"), + json.dumps({"history_visibility": "world_readable"}), + access_token=self.other_user_tok, + ) + self.render(request) + self.assertEqual(200, int(channel.result["code"]), msg=channel.result["body"]) + + # Test that room is not purged + with self.assertRaises(AssertionError): + self._is_purged(self.room_id) + + # Assert one user in room + 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" % self.room_id + request, channel = self.make_request( + "DELETE", + url.encode("ascii"), + json.dumps({"new_room_user_id": self.admin_user}), + access_token=self.admin_user_tok, + ) + self.render(request) + + self.assertEqual(200, int(channel.result["code"]), msg=channel.result["body"]) + self.assertEqual(self.other_user, channel.json_body["kicked_users"][0]) + self.assertIn("new_room_id", channel.json_body) + self.assertIn("failed_to_kick_users", channel.json_body) + self.assertIn("local_aliases", channel.json_body) + + # Test that member has moved to new room + self._is_member( + room_id=channel.json_body["new_room_id"], user_id=self.other_user + ) + + self._is_purged(self.room_id) + self._has_no_members(self.room_id) + + # Assert we can no longer peek into the room + self._assert_peek(self.room_id, expect_code=403) + + def _is_blocked(self, room_id, expect=True): + """Assert that the room is blocked or not + """ + d = self.store.is_room_blocked(room_id) + if expect: + self.assertTrue(self.get_success(d)) + else: + self.assertIsNone(self.get_success(d)) + + def _has_no_members(self, room_id): + """Assert there is now no longer anyone in the room + """ + users_in_room = self.get_success(self.store.get_users_in_room(room_id)) + self.assertEqual([], users_in_room) + + def _is_member(self, room_id, user_id): + """Test that user is member of the room + """ + users_in_room = self.get_success(self.store.get_users_in_room(room_id)) + self.assertIn(user_id, users_in_room) + + def _is_purged(self, room_id): + """Test that the following tables have been purged of all rows related to the room. + """ + for table in ( + "current_state_events", + "event_backward_extremities", + "event_forward_extremities", + "event_json", + "event_push_actions", + "event_search", + "events", + "group_rooms", + "public_room_list_stream", + "receipts_graph", + "receipts_linearized", + "room_aliases", + "room_depth", + "room_memberships", + "room_stats_state", + "room_stats_current", + "room_stats_historical", + "room_stats_earliest_token", + "rooms", + "stream_ordering_to_exterm", + "users_in_public_rooms", + "users_who_share_private_rooms", + "appservice_room_list", + "e2e_room_keys", + "event_push_summary", + "pusher_throttle", + "group_summary_rooms", + "local_invites", + "room_account_data", + "room_tags", + # "state_groups", # Current impl leaves orphaned state groups around. + "state_groups_state", + ): + count = self.get_success( + self.store.db.simple_select_one_onecol( + table=table, + keyvalues={"room_id": room_id}, + retcol="COUNT(*)", + desc="test_purge_room", + ) + ) + + self.assertEqual(count, 0, msg="Rows not purged in {}".format(table)) + + def _assert_peek(self, room_id, expect_code): + """Assert that the admin user can (or cannot) peek into the room. + """ + + url = "rooms/%s/initialSync" % (room_id,) + request, channel = self.make_request( + "GET", url.encode("ascii"), access_token=self.admin_user_tok + ) + self.render(request) + self.assertEqual( + expect_code, int(channel.result["code"]), msg=channel.result["body"] + ) + + url = "events?timeout=0&room_id=" + room_id + request, channel = self.make_request( + "GET", url.encode("ascii"), access_token=self.admin_user_tok + ) + self.render(request) + self.assertEqual( + expect_code, int(channel.result["code"]), msg=channel.result["body"] + ) + + class PurgeRoomTestCase(unittest.HomeserverTestCase): """Test /purge_room admin API. """ diff --git a/tests/storage/test_room.py b/tests/storage/test_room.py index 3b78d488965b..a37aec5bbe21 100644 --- a/tests/storage/test_room.py +++ b/tests/storage/test_room.py @@ -66,6 +66,10 @@ def test_get_room_with_stats(self): (yield self.store.get_room_with_stats(self.room.to_string())), ) + @defer.inlineCallbacks + def test_get_room_with_stats_unknown_room(self): + self.assertIsNone((yield self.store.get_room_with_stats("!uknown:test")),) + class RoomEventsStoreTestCase(unittest.TestCase): @defer.inlineCallbacks From 47ebed9c149f8ff187b27cdc3cfde84f6b6ea480 Mon Sep 17 00:00:00 2001 From: dklimpel <5740567+dklimpel@users.noreply.github.com> Date: Mon, 1 Jun 2020 17:21:31 +0200 Subject: [PATCH 02/12] Add newsfile --- changelog.d/7613.feature | 1 + 1 file changed, 1 insertion(+) create mode 100644 changelog.d/7613.feature diff --git a/changelog.d/7613.feature b/changelog.d/7613.feature new file mode 100644 index 000000000000..897cfc172694 --- /dev/null +++ b/changelog.d/7613.feature @@ -0,0 +1 @@ +Add delete room admin endpoint (`DELETE /_synapse/admin/v1/rooms/`). \ No newline at end of file From 3276b1a0c8f722fbac60c895f28aacc8a05fe0db Mon Sep 17 00:00:00 2001 From: dklimpel <5740567+dklimpel@users.noreply.github.com> Date: Fri, 5 Jun 2020 23:20:46 +0200 Subject: [PATCH 03/12] Move functions to own RoomShutdownHandler --- docs/admin_api/rooms.md | 15 +- synapse/handlers/room.py | 172 +++++++++++++++++++- synapse/rest/admin/rooms.py | 289 +++------------------------------- synapse/server.py | 10 +- synapse/server.pyi | 4 + tests/rest/admin/test_room.py | 3 +- 6 files changed, 211 insertions(+), 282 deletions(-) diff --git a/docs/admin_api/rooms.md b/docs/admin_api/rooms.md index 426636976c61..34a9b21d0b1b 100644 --- a/docs/admin_api/rooms.md +++ b/docs/admin_api/rooms.md @@ -328,13 +328,13 @@ and "[Purge room](purge_room.md)" API. 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 -leaves the room without any information. +leave the room without any information. The new room will be created with the user specified by the `new_room_user_id` parameter 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 preventing new joins to the old room. +If `block` is `True` it prevents new joins to the old room. This API will remove all trace of the old room from your database after removing all local users. @@ -346,7 +346,7 @@ the new room. Users on other servers will be unaffected. ## Parameters -The following query parameters are available: +The following parameters should be set in the URL: * `room_id` - The ID of the room. @@ -373,9 +373,7 @@ The following fields are possible in the JSON response body: the old room to the new. * `new_room_id` - A string representing the room ID of the new room. -## Usage - -A standard request: +The API is: ```json DELETE /_synapse/admin/v1/rooms/ @@ -388,7 +386,10 @@ DELETE /_synapse/admin/v1/rooms/ } ``` -Response: +To use it, you will need to authenticate by providing an ``access_token`` for a +server admin: see `README.rst `_. + +A response body like the following is returned: ```json { diff --git a/synapse/handlers/room.py b/synapse/handlers/room.py index 61db3ccc43ba..000bb1977f90 100644 --- a/synapse/handlers/room.py +++ b/synapse/handlers/room.py @@ -22,11 +22,11 @@ import math import string from collections import OrderedDict -from typing import Tuple +from typing import List, Optional, Tuple from six import iteritems, string_types -from synapse.api.constants import EventTypes, JoinRules, RoomCreationPreset +from synapse.api.constants import EventTypes, JoinRules, Membership, RoomCreationPreset from synapse.api.errors import AuthError, Codes, NotFoundError, StoreError, SynapseError from synapse.api.room_versions import KNOWN_ROOM_VERSIONS, RoomVersion from synapse.events.utils import copy_power_levels_contents @@ -40,9 +40,10 @@ StateMap, StreamToken, UserID, + create_requester, ) from synapse.util import stringutils -from synapse.util.async_helpers import Linearizer +from synapse.util.async_helpers import Linearizer, maybe_awaitable from synapse.util.caches.response_cache import ResponseCache from synapse.visibility import filter_events_for_client @@ -1071,3 +1072,168 @@ def get_current_key(self): def get_current_key_for_room(self, room_id): return self.store.get_room_events_max_id(room_id) + + +class RoomShutdownHandler(object): + + DEFAULT_MESSAGE = ( + "Sharing illegal content on this server is not permitted and rooms in" + " violation will be blocked." + ) + DEFAULT_ROOM_NAME = "Content Violation Notification" + + def __init__(self, hs): + self.hs = hs + self.room_member_handler = hs.get_room_member_handler() + self._room_creation_handler = hs.get_room_creation_handler() + self._replication = hs.get_replication_data_handler() + self.event_creation_handler = hs.get_event_creation_handler() + self.state = hs.get_state_handler() + self.store = hs.get_datastore() + self.admin_handler = hs.get_handlers().admin_handler + + async def shutdown_room( + self, + room_id: str, + requester_user_id: UserID, + new_room_user_id: Optional[UserID] = None, + new_room_name: Optional[str] = None, + message: Optional[str] = None, + block: bool = True, + ) -> List[str]: + + if not new_room_name: + new_room_name = self.DEFAULT_ROOM_NAME + if not message: + message = self.DEFAULT_MESSAGE + + # Check RoomID + if not RoomID.is_valid(room_id): + raise SynapseError(400, "%s was not legal room ID" % (room_id,)) + + if not await self.store.get_room_with_stats(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. + if block: + await self.store.block_room(room_id, requester_user_id) + + if new_room_user_id: + if not self.hs.is_mine_id(new_room_user_id): + raise SynapseError( + 400, "User must be our own: %s" % (new_room_user_id,) + ) + + if not await self.admin_handler.get_user( + UserID.from_string(new_room_user_id) + ): + raise NotFoundError("Unknown user %s" % (new_room_user_id,)) + + room_creator_requester = create_requester(new_room_user_id) + + info, stream_id = await self._room_creation_handler.create_room( + room_creator_requester, + config={ + "preset": "public_chat", + "name": new_room_name, + "power_level_content_override": {"users_default": -10}, + }, + ratelimit=False, + ) + new_room_id = info["room_id"] + + logger.info( + "Shutting down room %r, joining to new room: %r", room_id, new_room_id + ) + + # We now wait for the create room to come back in via replication so + # that we can assume that all the joins/invites have propogated before + # we try and auto join below. + # + # TODO: Currently the events stream is written to from master + await self._replication.wait_for_stream_position( + self.hs.config.worker.writers.events, "events", stream_id + ) + else: + new_room_id = "" + logger.info("Shutting down room %r", room_id) + + users = await self.state.get_current_users_in_room(room_id) + kicked_users = [] + failed_to_kick_users = [] + for user_id in users: + if not self.hs.is_mine_id(user_id): + continue + + logger.info("Kicking %r from %r...", user_id, room_id) + + # Kick users from room + try: + target_requester = create_requester(user_id) + _, stream_id = await self.room_member_handler.update_membership( + requester=target_requester, + target=target_requester.user, + room_id=room_id, + action=Membership.LEAVE, + content={}, + ratelimit=False, + require_consent=False, + ) + + # Wait for leave to come in over replication before trying to forget. + await self._replication.wait_for_stream_position( + self.hs.config.worker.writers.events, "events", stream_id + ) + + await self.room_member_handler.forget(target_requester.user, room_id) + + kicked_users.append(user_id) + except Exception: + logger.exception("Failed to leave old room for %r", user_id) + failed_to_kick_users.append(user_id) + + # Join users to new room + try: + if new_room_user_id: + await self.room_member_handler.update_membership( + requester=target_requester, + target=target_requester.user, + room_id=new_room_id, + action=Membership.JOIN, + content={}, + ratelimit=False, + require_consent=False, + ) + except Exception: + logger.exception("Failed to join new room for %r", user_id) + + # Send message in new room and move aliases + if new_room_user_id: + await self.event_creation_handler.create_and_send_nonmember_event( + room_creator_requester, + { + "type": "m.room.message", + "content": {"body": message, "msgtype": "m.text"}, + "room_id": new_room_id, + "sender": new_room_user_id, + }, + ratelimit=False, + ) + + aliases_for_room = await maybe_awaitable( + self.store.get_aliases_for_room(room_id) + ) + + await self.store.update_aliases_for_room( + room_id, new_room_id, requester_user_id + ) + else: + aliases_for_room = [] + + return { + "kicked_users": kicked_users, + "failed_to_kick_users": failed_to_kick_users, + "local_aliases": aliases_for_room, + "new_room_id": new_room_id, + } diff --git a/synapse/rest/admin/rooms.py b/synapse/rest/admin/rooms.py index 84190e811f93..54b7d3389a20 100644 --- a/synapse/rest/admin/rooms.py +++ b/synapse/rest/admin/rooms.py @@ -17,7 +17,7 @@ from six.moves import http_client -from synapse.api.constants import EventTypes, JoinRules, Membership +from synapse.api.constants import EventTypes, JoinRules from synapse.api.errors import Codes, NotFoundError, SynapseError from synapse.http.servlet import ( RestServlet, @@ -34,7 +34,6 @@ ) from synapse.storage.data_stores.main.room import RoomSortOrder from synapse.types import RoomAlias, RoomID, UserID, create_requester -from synapse.util.async_helpers import maybe_awaitable logger = logging.getLogger(__name__) @@ -48,20 +47,10 @@ class ShutdownRoomRestServlet(RestServlet): PATTERNS = historical_admin_path_patterns("/shutdown_room/(?P[^/]+)") - DEFAULT_MESSAGE = ( - "Sharing illegal content on this server is not permitted and rooms in" - " violation will be blocked." - ) - def __init__(self, hs): self.hs = hs - self.store = hs.get_datastore() - self.state = hs.get_state_handler() - self._room_creation_handler = hs.get_room_creation_handler() - self.event_creation_handler = hs.get_event_creation_handler() - self.room_member_handler = hs.get_room_member_handler() self.auth = hs.get_auth() - self._replication = hs.get_replication_data_handler() + self.room_shutdown_handler = hs.get_room_shutdown_handler() async def on_POST(self, request, room_id): requester = await self.auth.get_user_by_req(request) @@ -69,116 +58,17 @@ async def on_POST(self, request, room_id): content = parse_json_object_from_request(request) assert_params_in_dict(content, ["new_room_user_id"]) - new_room_user_id = content["new_room_user_id"] - - room_creator_requester = create_requester(new_room_user_id) - - message = content.get("message", self.DEFAULT_MESSAGE) - room_name = content.get("room_name", "Content Violation Notification") - - info, stream_id = await self._room_creation_handler.create_room( - room_creator_requester, - config={ - "preset": "public_chat", - "name": room_name, - "power_level_content_override": {"users_default": -10}, - }, - ratelimit=False, - ) - new_room_id = info["room_id"] - - requester_user_id = requester.user.to_string() - - logger.info( - "Shutting down room %r, joining to new room: %r", room_id, new_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. - await self.store.block_room(room_id, requester_user_id) - - # We now wait for the create room to come back in via replication so - # that we can assume that all the joins/invites have propogated before - # we try and auto join below. - # - # TODO: Currently the events stream is written to from master - await self._replication.wait_for_stream_position( - self.hs.config.worker.writers.events, "events", stream_id - ) - - users = await self.state.get_current_users_in_room(room_id) - kicked_users = [] - failed_to_kick_users = [] - for user_id in users: - if not self.hs.is_mine_id(user_id): - continue - - logger.info("Kicking %r from %r...", user_id, room_id) - - try: - target_requester = create_requester(user_id) - _, stream_id = await self.room_member_handler.update_membership( - requester=target_requester, - target=target_requester.user, - room_id=room_id, - action=Membership.LEAVE, - content={}, - ratelimit=False, - require_consent=False, - ) - - # Wait for leave to come in over replication before trying to forget. - await self._replication.wait_for_stream_position( - self.hs.config.worker.writers.events, "events", stream_id - ) - - await self.room_member_handler.forget(target_requester.user, room_id) - - await self.room_member_handler.update_membership( - requester=target_requester, - target=target_requester.user, - room_id=new_room_id, - action=Membership.JOIN, - content={}, - ratelimit=False, - require_consent=False, - ) - kicked_users.append(user_id) - except Exception: - logger.exception( - "Failed to leave old room and join new room for %r", user_id - ) - failed_to_kick_users.append(user_id) - - await self.event_creation_handler.create_and_send_nonmember_event( - room_creator_requester, - { - "type": "m.room.message", - "content": {"body": message, "msgtype": "m.text"}, - "room_id": new_room_id, - "sender": new_room_user_id, - }, - ratelimit=False, - ) - - aliases_for_room = await maybe_awaitable( - self.store.get_aliases_for_room(room_id) - ) - - await self.store.update_aliases_for_room( - room_id, new_room_id, requester_user_id + 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=True, ) - return ( - 200, - { - "kicked_users": kicked_users, - "failed_to_kick_users": failed_to_kick_users, - "local_aliases": aliases_for_room, - "new_room_id": new_room_id, - }, - ) + return (200, ret) class DeleteRoomRestServlet(RestServlet): @@ -194,37 +84,17 @@ class DeleteRoomRestServlet(RestServlet): PATTERNS = admin_patterns("/rooms/(?P[^/]+)$") - DEFAULT_MESSAGE = ( - "Sharing illegal content on this server is not permitted and rooms in" - " violation will be blocked." - ) - DEFAULT_ROOM_NAME = "Content Violation Notification" - def __init__(self, hs): self.hs = hs - self.store = hs.get_datastore() - self.state = hs.get_state_handler() - self._room_creation_handler = hs.get_room_creation_handler() - self.event_creation_handler = hs.get_event_creation_handler() - self.room_member_handler = hs.get_room_member_handler() self.auth = hs.get_auth() - self._replication = hs.get_replication_data_handler() + self.room_shutdown_handler = hs.get_room_shutdown_handler() self.pagination_handler = hs.get_pagination_handler() - self.admin_handler = hs.get_handlers().admin_handler async def on_DELETE(self, request, room_id): 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) - requester_user_id = requester.user.to_string() - - # Check RoomID - if not RoomID.is_valid(room_id): - raise SynapseError(400, "%s was not legal room ID" % (room_id)) - - if not await self.store.get_room_with_stats(room_id): - raise NotFoundError("Room not found") # Check parameter `block` block = content.get("block", True) @@ -235,138 +105,19 @@ async def on_DELETE(self, request, room_id): Codes.BAD_JSON, ) - # 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. - if block: - await self.store.block_room(room_id, requester_user_id) - - new_room_user_id = content.get("new_room_user_id") - if new_room_user_id: - if not self.hs.is_mine_id(new_room_user_id): - raise SynapseError( - 400, "This endpoint can only be used with local users" - ) - - if not await self.admin_handler.get_user( - UserID.from_string(new_room_user_id) - ): - raise NotFoundError("User not found") - - room_creator_requester = create_requester(new_room_user_id) - - message = content.get("message", self.DEFAULT_MESSAGE) - room_name = content.get("room_name", self.DEFAULT_ROOM_NAME) - - info, stream_id = await self._room_creation_handler.create_room( - room_creator_requester, - config={ - "preset": "public_chat", - "name": room_name, - "power_level_content_override": {"users_default": -10}, - }, - ratelimit=False, - ) - new_room_id = info["room_id"] - - logger.info( - "Shutting down room %r, joining to new room: %r", room_id, new_room_id - ) - - # We now wait for the create room to come back in via replication so - # that we can assume that all the joins/invites have propogated before - # we try and auto join below. - # - # TODO: Currently the events stream is written to from master - await self._replication.wait_for_stream_position( - self.hs.config.worker.writers.events, "events", stream_id - ) - - users = await self.state.get_current_users_in_room(room_id) - kicked_users = [] - failed_to_kick_users = [] - for user_id in users: - if not self.hs.is_mine_id(user_id): - continue - - logger.info("Kicking %r from %r...", user_id, room_id) - - # Kick users from room - try: - target_requester = create_requester(user_id) - _, stream_id = await self.room_member_handler.update_membership( - requester=target_requester, - target=target_requester.user, - room_id=room_id, - action=Membership.LEAVE, - content={}, - ratelimit=False, - require_consent=False, - ) - - # Wait for leave to come in over replication before trying to forget. - await self._replication.wait_for_stream_position( - self.hs.config.worker.writers.events, "events", stream_id - ) - - await self.room_member_handler.forget(target_requester.user, room_id) - kicked_users.append(user_id) - except Exception: - logger.exception("Failed to leave old room for %r", user_id) - failed_to_kick_users.append(user_id) - - # Join users to new room - try: - if new_room_user_id: - await self.room_member_handler.update_membership( - requester=target_requester, - target=target_requester.user, - room_id=new_room_id, - action=Membership.JOIN, - content={}, - ratelimit=False, - require_consent=False, - ) - except Exception: - logger.exception("Failed to join new room for %r", user_id) - - # Send message in new room and move aliases - if new_room_user_id: - await self.event_creation_handler.create_and_send_nonmember_event( - room_creator_requester, - { - "type": "m.room.message", - "content": {"body": message, "msgtype": "m.text"}, - "room_id": new_room_id, - "sender": new_room_user_id, - }, - ratelimit=False, - ) - - aliases_for_room = await maybe_awaitable( - self.store.get_aliases_for_room(room_id) - ) - - await self.store.update_aliases_for_room( - room_id, new_room_id, requester_user_id - ) + 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, + ) # Purge room await self.pagination_handler.purge_room(room_id) - # Set empty values if no new room was created - if not new_room_user_id: - aliases_for_room = [] - new_room_id = "" - - return ( - 200, - { - "kicked_users": kicked_users, - "failed_to_kick_users": failed_to_kick_users, - "local_aliases": aliases_for_room, - "new_room_id": new_room_id, - }, - ) + return (200, ret) class ListRoomRestServlet(RestServlet): diff --git a/synapse/server.py b/synapse/server.py index ca2deb49bbe4..756e92b43fb8 100644 --- a/synapse/server.py +++ b/synapse/server.py @@ -73,7 +73,11 @@ from synapse.handlers.read_marker import ReadMarkerHandler from synapse.handlers.receipts import ReceiptsHandler from synapse.handlers.register import RegistrationHandler -from synapse.handlers.room import RoomContextHandler, RoomCreationHandler +from synapse.handlers.room import ( + RoomContextHandler, + RoomCreationHandler, + RoomShutdownHandler, +) from synapse.handlers.room_list import RoomListHandler from synapse.handlers.room_member import RoomMemberMasterHandler from synapse.handlers.room_member_worker import RoomMemberWorkerHandler @@ -144,6 +148,7 @@ def build_DEPENDENCY(self) "handlers", "auth", "room_creation_handler", + "room_shutdown_handler", "state_handler", "state_resolution_handler", "presence_handler", @@ -358,6 +363,9 @@ def build_proxied_http_client(self): def build_room_creation_handler(self): return RoomCreationHandler(self) + def build_room_shutdown_handler(self): + return RoomShutdownHandler(self) + def build_sendmail(self): return sendmail diff --git a/synapse/server.pyi b/synapse/server.pyi index fe8024d2d4e6..d3c27d79983d 100644 --- a/synapse/server.pyi +++ b/synapse/server.pyi @@ -71,6 +71,10 @@ class HomeServer(object): pass def get_room_member_handler(self) -> synapse.handlers.room_member.RoomMemberHandler: pass + def get_room_shutdown_handler( + self, + ) -> synapse.handlers.room_member.RoomShutdownHandler: + pass def get_event_creation_handler( self, ) -> synapse.handlers.message.EventCreationHandler: diff --git a/tests/rest/admin/test_room.py b/tests/rest/admin/test_room.py index 11c60a84a1d8..6e574cdabce1 100644 --- a/tests/rest/admin/test_room.py +++ b/tests/rest/admin/test_room.py @@ -260,8 +260,7 @@ def test_new_room_user_is_not_local(self): self.assertEqual(400, int(channel.result["code"]), msg=channel.result["body"]) self.assertEqual( - "This endpoint can only be used with local users", - channel.json_body["error"], + "User must be our own: @not:exist.bla", channel.json_body["error"], ) def test_block_is_not_bool(self): From 16b4fbbeddea83cc854647a4c8f2361d7dd3e6f8 Mon Sep 17 00:00:00 2001 From: dklimpel <5740567+dklimpel@users.noreply.github.com> Date: Fri, 5 Jun 2020 23:27:32 +0200 Subject: [PATCH 04/12] Fix server.pyi --- synapse/server.pyi | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/synapse/server.pyi b/synapse/server.pyi index d3c27d79983d..58cd099e6d22 100644 --- a/synapse/server.pyi +++ b/synapse/server.pyi @@ -71,9 +71,7 @@ class HomeServer(object): pass def get_room_member_handler(self) -> synapse.handlers.room_member.RoomMemberHandler: pass - def get_room_shutdown_handler( - self, - ) -> synapse.handlers.room_member.RoomShutdownHandler: + def get_room_shutdown_handler(self) -> synapse.handlers.room.RoomShutdownHandler: pass def get_event_creation_handler( self, From 10150506b3bea09e44ac8b465102e74fe295f3a1 Mon Sep 17 00:00:00 2001 From: Dirk Klimpel <5740567+dklimpel@users.noreply.github.com> Date: Thu, 11 Jun 2020 18:50:59 +0200 Subject: [PATCH 05/12] Apply suggestions from code review Co-authored-by: Richard van der Hoff <1389908+richvdh@users.noreply.github.com> --- docs/admin_api/rooms.md | 14 +++++++------- synapse/handlers/room.py | 5 ++--- synapse/rest/admin/rooms.py | 1 - synapse/storage/data_stores/main/room.py | 2 +- 4 files changed, 10 insertions(+), 12 deletions(-) diff --git a/docs/admin_api/rooms.md b/docs/admin_api/rooms.md index 34a9b21d0b1b..2799f370fbda 100644 --- a/docs/admin_api/rooms.md +++ b/docs/admin_api/rooms.md @@ -352,20 +352,20 @@ The following parameters should be set in the URL: The following JSON body parameters are available: -* `new_room_user_id` - Optional. A string representing the user ID of the user that will admin - the new room that all users in the old room will be moved to. If not - set the users will not be moved to a new room and only leave the old room - without any information. Defaults to `None`. +* `new_room_user_id` - Optional. If set, a new room will be created with this user ID + as the creator and admin, and all users in the old room will be moved into that + room. If not set, no new room will be created and the users will just be removed + from the old room. * `room_name` - Optional. A string representing the name of the room that new users will be invited to. Defaults to `Content Violation Notification` * `message` - Optional. A string containing the first message that will be sent as `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. A boolean if `room_id` will be set on blocking list. The room will be - blocked for this server and preventing new joins. Defaults to `True`. +* `block` - Optional. If set to `true`, this room will be added to a blocking list, preventing future attempts to + join the room. Defaults to `true`. -The following fields are possible in the JSON response body: +The following fields are returned in the JSON response body: * `kicked_users` - An array of users (`user_id`) that were kicked. * `failed_to_kick_users` - An array of users (`user_id`) that that were not kicked. diff --git a/synapse/handlers/room.py b/synapse/handlers/room.py index f41ca04c512e..85b3d82d0ca5 100644 --- a/synapse/handlers/room.py +++ b/synapse/handlers/room.py @@ -1128,9 +1128,8 @@ async def shutdown_room( if not message: message = self.DEFAULT_MESSAGE - # Check RoomID if not RoomID.is_valid(room_id): - raise SynapseError(400, "%s was not legal room ID" % (room_id,)) + raise SynapseError(400, "%s is not a legal room ID" % (room_id,)) if not await self.store.get_room_with_stats(room_id): raise NotFoundError("Unknown room id %s" % (room_id,)) @@ -1177,7 +1176,7 @@ async def shutdown_room( self.hs.config.worker.writers.events, "events", stream_id ) else: - new_room_id = "" + new_room_id = None logger.info("Shutting down room %r", room_id) users = await self.state.get_current_users_in_room(room_id) diff --git a/synapse/rest/admin/rooms.py b/synapse/rest/admin/rooms.py index 54b7d3389a20..c9b410261642 100644 --- a/synapse/rest/admin/rooms.py +++ b/synapse/rest/admin/rooms.py @@ -96,7 +96,6 @@ async def on_DELETE(self, request, room_id): content = parse_json_object_from_request(request, allow_empty_body=True) - # Check parameter `block` block = content.get("block", True) if not isinstance(block, bool): raise SynapseError( diff --git a/synapse/storage/data_stores/main/room.py b/synapse/storage/data_stores/main/room.py index 02abe571dcc6..ad0a1fad9cea 100644 --- a/synapse/storage/data_stores/main/room.py +++ b/synapse/storage/data_stores/main/room.py @@ -124,7 +124,7 @@ def get_room_with_stats_txn(txn, room_id): res["public"] = bool(res["public"]) return res except IndexError: - return + return None return self.db.runInteraction( "get_room_with_stats", get_room_with_stats_txn, room_id From 79010b5769239588b10014d3629bb6282f02ea11 Mon Sep 17 00:00:00 2001 From: dklimpel <5740567+dklimpel@users.noreply.github.com> Date: Tue, 16 Jun 2020 15:30:17 +0200 Subject: [PATCH 06/12] Apply a lot of changes requested from code review --- docs/admin_api/rooms.md | 65 ++++++++++---------- synapse/handlers/room.py | 75 ++++++++++++++++++------ synapse/rest/admin/rooms.py | 4 +- synapse/storage/data_stores/main/room.py | 8 ++- tests/rest/admin/test_room.py | 36 ++++++------ 5 files changed, 117 insertions(+), 71 deletions(-) diff --git a/docs/admin_api/rooms.md b/docs/admin_api/rooms.md index 2799f370fbda..a9549248f04c 100644 --- a/docs/admin_api/rooms.md +++ b/docs/admin_api/rooms.md @@ -344,40 +344,14 @@ several minutes or longer. The local server will only have the power to move local user and room aliases to the new room. Users on other servers will be unaffected. -## Parameters - -The following parameters should be set in the URL: - -* `room_id` - The ID of the room. - -The following JSON body parameters are available: - -* `new_room_user_id` - Optional. If set, a new room will be created with this user ID - as the creator and admin, and all users in the old room will be moved into that - room. If not set, no new room will be created and the users will just be removed - from the old room. -* `room_name` - Optional. A string representing the name of the room that new users will be - invited to. Defaults to `Content Violation Notification` -* `message` - Optional. A string containing the first message that will be sent as - `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 `true`. - -The following fields are returned in the JSON response body: - -* `kicked_users` - An array of users (`user_id`) that were kicked. -* `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. - The API is: ```json -DELETE /_synapse/admin/v1/rooms/ +POST /_synapse/admin/v1/rooms//delete +``` +with a body of: +```json { "new_room_user_id": "@someuser:example.com", "room_name": "Content Violation Notification", @@ -387,7 +361,7 @@ DELETE /_synapse/admin/v1/rooms/ ``` To use it, you will need to authenticate by providing an ``access_token`` for a -server admin: see `README.rst `_. +server admin: see [README.rst](README.rst). A response body like the following is returned: @@ -404,3 +378,32 @@ A response body like the following is returned: "new_room_id": "!newroomid:example.com" } ``` + +## Parameters + +The following parameters should be set in the URL: + +* `room_id` - The ID of the room. + +The following JSON body parameters are available: + +* `new_room_user_id` - Optional. If set, a new room will be created with this user ID + as the creator and admin, and all users in the old room will be moved into that + room. If not set, no new room will be created and the users will just be removed + from the old room. +* `room_name` - Optional. A string representing the name of the room that new users will be + invited to. Defaults to `Content Violation Notification` +* `message` - Optional. A string containing the first message that will be sent as + `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 `true`. + +The following fields are returned in the JSON response body: + +* `kicked_users` - An array of users (`user_id`) that were kicked. +* `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. diff --git a/synapse/handlers/room.py b/synapse/handlers/room.py index 85b3d82d0ca5..283e2942a23a 100644 --- a/synapse/handlers/room.py +++ b/synapse/handlers/room.py @@ -22,7 +22,7 @@ import math import string from collections import OrderedDict -from typing import List, Optional, Tuple +from typing import Optional, Tuple from six import iteritems, string_types @@ -1111,17 +1111,61 @@ def __init__(self, hs): self.event_creation_handler = hs.get_event_creation_handler() self.state = hs.get_state_handler() self.store = hs.get_datastore() - self.admin_handler = hs.get_handlers().admin_handler async def shutdown_room( self, room_id: str, - requester_user_id: UserID, - new_room_user_id: Optional[UserID] = None, + requester_user_id: str, + new_room_user_id: Optional[str] = None, new_room_name: Optional[str] = None, message: Optional[str] = None, block: bool = True, - ) -> List[str]: + ) -> dict: + """ + 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 + leave the room without any information. + + The new room will be created with the user specified by the + `new_room_user_id` parameter 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. + + The local server will only have the power to move local user and room + aliases to the new room. Users on other servers will be unaffected. + + Args: + room_id: The ID of the room to shut down. + requester_user_id: + User who requested the action and put the room on the + blocking list. + new_room_user_id: + If set, a new room will be created with this user ID + as the creator and admin, and all users in the old room will be + moved into that room. If not set, no new room will be created + and the users will just be removed from the old room. + new_room_name: + A string representing the name of the room that new users will + be invited to. Defaults to `Content Violation Notification` + message: + A string containing the first message that will be sent as + `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: + If set to `true`, this room will be added to a blocking list, + preventing future attempts to join the room. Defaults to `true`. + + Returns: + kicked_users: An array of users (`user_id`) that were kicked. + 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. + """ if not new_room_name: new_room_name = self.DEFAULT_ROOM_NAME @@ -1145,9 +1189,7 @@ async def shutdown_room( 400, "User must be our own: %s" % (new_room_user_id,) ) - if not await self.admin_handler.get_user( - UserID.from_string(new_room_user_id) - ): + if not await self.store.get_user_by_id(new_room_user_id): raise NotFoundError("Unknown user %s" % (new_room_user_id,)) room_creator_requester = create_requester(new_room_user_id) @@ -1188,8 +1230,8 @@ async def shutdown_room( logger.info("Kicking %r from %r...", user_id, room_id) - # Kick users from room try: + # Kick users from room target_requester = create_requester(user_id) _, stream_id = await self.room_member_handler.update_membership( requester=target_requester, @@ -1208,13 +1250,7 @@ async def shutdown_room( await self.room_member_handler.forget(target_requester.user, room_id) - kicked_users.append(user_id) - except Exception: - logger.exception("Failed to leave old room for %r", user_id) - failed_to_kick_users.append(user_id) - - # Join users to new room - try: + # Join users to new room if new_room_user_id: await self.room_member_handler.update_membership( requester=target_requester, @@ -1225,8 +1261,13 @@ async def shutdown_room( ratelimit=False, require_consent=False, ) + + kicked_users.append(user_id) except Exception: - logger.exception("Failed to join new room for %r", user_id) + logger.exception( + "Failed to leave old room and join new room for %r", user_id + ) + failed_to_kick_users.append(user_id) # Send message in new room and move aliases if new_room_user_id: diff --git a/synapse/rest/admin/rooms.py b/synapse/rest/admin/rooms.py index c9b410261642..89d22492cc0e 100644 --- a/synapse/rest/admin/rooms.py +++ b/synapse/rest/admin/rooms.py @@ -82,7 +82,7 @@ class DeleteRoomRestServlet(RestServlet): It will remove all trace of a room from the database. """ - PATTERNS = admin_patterns("/rooms/(?P[^/]+)$") + PATTERNS = admin_patterns("/rooms/(?P[^/]+)/delete$") def __init__(self, hs): self.hs = hs @@ -90,7 +90,7 @@ def __init__(self, hs): self.room_shutdown_handler = hs.get_room_shutdown_handler() self.pagination_handler = hs.get_pagination_handler() - async def on_DELETE(self, request, room_id): + async def on_POST(self, request, room_id): requester = await self.auth.get_user_by_req(request) await assert_user_is_admin(self.auth, requester.user) diff --git a/synapse/storage/data_stores/main/room.py b/synapse/storage/data_stores/main/room.py index ad0a1fad9cea..60ed0b711620 100644 --- a/synapse/storage/data_stores/main/room.py +++ b/synapse/storage/data_stores/main/room.py @@ -118,14 +118,16 @@ def get_room_with_stats_txn(txn, room_id): WHERE room_id = ? """ txn.execute(sql, [room_id]) + # Catch error if sql returns empty result to return "None" instead of an error try: res = self.db.cursor_to_dict(txn)[0] - res["federatable"] = bool(res["federatable"]) - res["public"] = bool(res["public"]) - return res except IndexError: return None + res["federatable"] = bool(res["federatable"]) + res["public"] = bool(res["public"]) + return res + return self.db.runInteraction( "get_room_with_stats", get_room_with_stats_txn, room_id ) diff --git a/tests/rest/admin/test_room.py b/tests/rest/admin/test_room.py index 6e574cdabce1..911c3a4889c8 100644 --- a/tests/rest/admin/test_room.py +++ b/tests/rest/admin/test_room.py @@ -182,7 +182,7 @@ 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" % self.room_id + self.url = "/_synapse/admin/v1/rooms/%s/delete" % self.room_id def test_requester_is_no_admin(self): """ @@ -190,7 +190,7 @@ def test_requester_is_no_admin(self): """ request, channel = self.make_request( - "DELETE", self.url, access_token=self.other_user_tok, + "POST", self.url, access_token=self.other_user_tok, ) self.render(request) @@ -201,10 +201,10 @@ def test_room_does_not_exist(self): """ Check that unknown rooms/server return error 404. """ - url = "/_synapse/admin/v1/rooms/!unknown:test" + url = "/_synapse/admin/v1/rooms/!unknown:test/delete" request, channel = self.make_request( - "DELETE", url, access_token=self.admin_user_tok, + "POST", url, access_token=self.admin_user_tok, ) self.render(request) @@ -215,16 +215,16 @@ def test_room_is_not_valid(self): """ Check that invalid room names, return an error 400. """ - url = "/_synapse/admin/v1/rooms/invalidroom" + url = "/_synapse/admin/v1/rooms/invalidroom/delete" request, channel = self.make_request( - "DELETE", url, access_token=self.admin_user_tok, + "POST", url, access_token=self.admin_user_tok, ) self.render(request) self.assertEqual(400, int(channel.result["code"]), msg=channel.result["body"]) self.assertEqual( - "invalidroom was not legal room ID", channel.json_body["error"], + "invalidroom is not a legal room ID", channel.json_body["error"], ) def test_new_room_user_does_not_exist(self): @@ -234,7 +234,7 @@ def test_new_room_user_does_not_exist(self): body = json.dumps({"new_room_user_id": "@unknown:test"}) request, channel = self.make_request( - "DELETE", + "POST", self.url, content=body.encode(encoding="utf_8"), access_token=self.admin_user_tok, @@ -251,7 +251,7 @@ def test_new_room_user_is_not_local(self): body = json.dumps({"new_room_user_id": "@not:exist.bla"}) request, channel = self.make_request( - "DELETE", + "POST", self.url, content=body.encode(encoding="utf_8"), access_token=self.admin_user_tok, @@ -270,7 +270,7 @@ def test_block_is_not_bool(self): body = json.dumps({"block": "NotBool"}) request, channel = self.make_request( - "DELETE", + "POST", self.url, content=body.encode(encoding="utf_8"), access_token=self.admin_user_tok, @@ -297,7 +297,7 @@ def test_purge_room_and_block(self): body = json.dumps({"block": True}) request, channel = self.make_request( - "DELETE", + "POST", self.url.encode("ascii"), content=body.encode(encoding="utf_8"), access_token=self.admin_user_tok, @@ -305,7 +305,7 @@ def test_purge_room_and_block(self): self.render(request) self.assertEqual(200, int(channel.result["code"]), msg=channel.result["body"]) - self.assertEqual("", channel.json_body["new_room_id"]) + self.assertEqual(None, channel.json_body["new_room_id"]) self.assertEqual(self.other_user, channel.json_body["kicked_users"][0]) self.assertIn("failed_to_kick_users", channel.json_body) self.assertIn("local_aliases", channel.json_body) @@ -331,7 +331,7 @@ def test_purge_room_and_not_block(self): body = json.dumps({"block": False}) request, channel = self.make_request( - "DELETE", + "POST", self.url.encode("ascii"), content=body.encode(encoding="utf_8"), access_token=self.admin_user_tok, @@ -339,7 +339,7 @@ def test_purge_room_and_not_block(self): self.render(request) self.assertEqual(200, int(channel.result["code"]), msg=channel.result["body"]) - self.assertEqual("", channel.json_body["new_room_id"]) + self.assertEqual(None, channel.json_body["new_room_id"]) self.assertEqual(self.other_user, channel.json_body["kicked_users"][0]) self.assertIn("failed_to_kick_users", channel.json_body) self.assertIn("local_aliases", channel.json_body) @@ -376,9 +376,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" % self.room_id + url = "/_synapse/admin/v1/rooms/%s/delete" % self.room_id request, channel = self.make_request( - "DELETE", + "POST", url.encode("ascii"), json.dumps({"new_room_user_id": self.admin_user}), access_token=self.admin_user_tok, @@ -425,9 +425,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" % self.room_id + url = "/_synapse/admin/v1/rooms/%s/delete" % self.room_id request, channel = self.make_request( - "DELETE", + "POST", url.encode("ascii"), json.dumps({"new_room_user_id": self.admin_user}), access_token=self.admin_user_tok, From 8c8eb7bfffb58e268793ccd8a9a56cddac9bc73c Mon Sep 17 00:00:00 2001 From: dklimpel <5740567+dklimpel@users.noreply.github.com> Date: Tue, 16 Jun 2020 15:44:17 +0200 Subject: [PATCH 07/12] Change defaults of `block` to `false` --- docs/admin_api/rooms.md | 2 +- synapse/handlers/room.py | 4 ++-- synapse/rest/admin/rooms.py | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/docs/admin_api/rooms.md b/docs/admin_api/rooms.md index a9549248f04c..69d0708a5965 100644 --- a/docs/admin_api/rooms.md +++ b/docs/admin_api/rooms.md @@ -398,7 +398,7 @@ The following JSON body parameters are available: 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 `true`. + join the room. Defaults to `false`. The following fields are returned in the JSON response body: diff --git a/synapse/handlers/room.py b/synapse/handlers/room.py index 283e2942a23a..922f4ee69f0b 100644 --- a/synapse/handlers/room.py +++ b/synapse/handlers/room.py @@ -1119,7 +1119,7 @@ async def shutdown_room( new_room_user_id: Optional[str] = None, new_room_name: Optional[str] = None, message: Optional[str] = None, - block: bool = True, + block: bool = False, ) -> dict: """ Shuts down a room. Moves all local users and room aliases automatically @@ -1155,7 +1155,7 @@ async def shutdown_room( 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 `true`. + preventing future attempts to join the room. Defaults to `false`. Returns: kicked_users: An array of users (`user_id`) that were kicked. diff --git a/synapse/rest/admin/rooms.py b/synapse/rest/admin/rooms.py index 89d22492cc0e..6053449dbe02 100644 --- a/synapse/rest/admin/rooms.py +++ b/synapse/rest/admin/rooms.py @@ -96,7 +96,7 @@ async def on_POST(self, request, room_id): content = parse_json_object_from_request(request, allow_empty_body=True) - block = content.get("block", True) + block = content.get("block", False) if not isinstance(block, bool): raise SynapseError( http_client.BAD_REQUEST, From 5e3da7ef38b12e40863334b2a0c29abd7a61cbc2 Mon Sep 17 00:00:00 2001 From: dklimpel <5740567+dklimpel@users.noreply.github.com> Date: Tue, 16 Jun 2020 15:50:45 +0200 Subject: [PATCH 08/12] Add headline to documentation --- docs/admin_api/rooms.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/docs/admin_api/rooms.md b/docs/admin_api/rooms.md index 69d0708a5965..fc66c30bbf84 100644 --- a/docs/admin_api/rooms.md +++ b/docs/admin_api/rooms.md @@ -400,6 +400,8 @@ The following JSON body parameters are available: * `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`. +## Response + The following fields are returned in the JSON response body: * `kicked_users` - An array of users (`user_id`) that were kicked. From f82a0ec3624f37109f7100ca09504e38f4d78bb7 Mon Sep 17 00:00:00 2001 From: dklimpel <5740567+dklimpel@users.noreply.github.com> Date: Tue, 16 Jun 2020 22:24:26 +0200 Subject: [PATCH 09/12] Replace six usage with native Python --- synapse/rest/admin/rooms.py | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/synapse/rest/admin/rooms.py b/synapse/rest/admin/rooms.py index 6053449dbe02..1d2b07dbfc0a 100644 --- a/synapse/rest/admin/rooms.py +++ b/synapse/rest/admin/rooms.py @@ -13,10 +13,9 @@ # See the License for the specific language governing permissions and # limitations under the License. import logging +from http import HTTPStatus from typing import List, Optional -from six.moves import http_client - from synapse.api.constants import EventTypes, JoinRules from synapse.api.errors import Codes, NotFoundError, SynapseError from synapse.http.servlet import ( @@ -99,7 +98,7 @@ async def on_POST(self, request, room_id): block = content.get("block", False) if not isinstance(block, bool): raise SynapseError( - http_client.BAD_REQUEST, + HTTPStatus.BAD_REQUEST, "Param 'block' must be a boolean, if given", Codes.BAD_JSON, ) From 4bde87c0207dda318fbe170cb24daac9ed333ee0 Mon Sep 17 00:00:00 2001 From: Dirk Klimpel <5740567+dklimpel@users.noreply.github.com> Date: Tue, 16 Jun 2020 22:32:56 +0200 Subject: [PATCH 10/12] Update newsfile to changed endpoint / URL --- changelog.d/7613.feature | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/changelog.d/7613.feature b/changelog.d/7613.feature index 897cfc172694..f552c5879818 100644 --- a/changelog.d/7613.feature +++ b/changelog.d/7613.feature @@ -1 +1 @@ -Add delete room admin endpoint (`DELETE /_synapse/admin/v1/rooms/`). \ No newline at end of file +Add delete room admin endpoint (`POST /_synapse/admin/v1/rooms//delete`). From e8f8f0e052229b3ec5850d3783c0564f849acecc Mon Sep 17 00:00:00 2001 From: dklimpel <5740567+dklimpel@users.noreply.github.com> Date: Sun, 12 Jul 2020 12:00:40 +0200 Subject: [PATCH 11/12] Apply a lot suggestions from code review --- changelog.d/7613.feature | 2 +- docs/admin_api/rooms.md | 4 +++- synapse/handlers/room.py | 11 ++++------- synapse/rest/admin/rooms.py | 4 ++-- tests/rest/admin/test_room.py | 15 +++++++++------ tests/storage/test_room.py | 4 ++++ 6 files changed, 23 insertions(+), 17 deletions(-) diff --git a/changelog.d/7613.feature b/changelog.d/7613.feature index f552c5879818..b671dc2fcc33 100644 --- a/changelog.d/7613.feature +++ b/changelog.d/7613.feature @@ -1 +1 @@ -Add delete room admin endpoint (`POST /_synapse/admin/v1/rooms//delete`). +Add delete room admin endpoint (`POST /_synapse/admin/v1/rooms//delete`). Contributed by @dklimpel. diff --git a/docs/admin_api/rooms.md b/docs/admin_api/rooms.md index fc66c30bbf84..c02aa309a651 100644 --- a/docs/admin_api/rooms.md +++ b/docs/admin_api/rooms.md @@ -390,7 +390,7 @@ The following JSON body parameters are available: * `new_room_user_id` - Optional. If set, a new room will be created with this user ID as the creator and admin, and all users in the old room will be moved into that room. If not set, no new room will be created and the users will just be removed - from the old room. + from the old room. The user ID must be from local server but it does not have to exist. * `room_name` - Optional. A string representing the name of the room that new users will be invited to. Defaults to `Content Violation Notification` * `message` - Optional. A string containing the first message that will be sent as @@ -400,6 +400,8 @@ The following JSON body parameters are available: * `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`. +The JSON body must not be empty. The body must be at least `{}`. + ## Response The following fields are returned in the JSON response body: diff --git a/synapse/handlers/room.py b/synapse/handlers/room.py index 6a50f140dcc3..fb37d371ad45 100644 --- a/synapse/handlers/room.py +++ b/synapse/handlers/room.py @@ -1155,7 +1155,7 @@ async def shutdown_room( If set to `true`, this room will be added to a blocking list, preventing future attempts to join the room. Defaults to `false`. - Returns: + Returns: a dict containing the following keys: kicked_users: An array of users (`user_id`) that were kicked. failed_to_kick_users: An array of users (`user_id`) that that were not kicked. @@ -1173,7 +1173,7 @@ 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_with_stats(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 @@ -1181,21 +1181,18 @@ async def shutdown_room( if block: await self.store.block_room(room_id, requester_user_id) - if new_room_user_id: + if new_room_user_id is not None: if not self.hs.is_mine_id(new_room_user_id): raise SynapseError( 400, "User must be our own: %s" % (new_room_user_id,) ) - if not await self.store.get_user_by_id(new_room_user_id): - raise NotFoundError("Unknown user %s" % (new_room_user_id,)) - room_creator_requester = create_requester(new_room_user_id) info, stream_id = await self._room_creation_handler.create_room( room_creator_requester, config={ - "preset": "public_chat", + "preset": RoomCreationPreset.PUBLIC_CHAT, "name": new_room_name, "power_level_content_override": {"users_default": -10}, }, diff --git a/synapse/rest/admin/rooms.py b/synapse/rest/admin/rooms.py index 1d2b07dbfc0a..544be4706034 100644 --- a/synapse/rest/admin/rooms.py +++ b/synapse/rest/admin/rooms.py @@ -60,7 +60,7 @@ async def on_POST(self, request, room_id): ret = await self.room_shutdown_handler.shutdown_room( room_id=room_id, - new_room_user_id=content.get("new_room_user_id"), + new_room_user_id=content["new_room_user_id"], new_room_name=content.get("room_name"), message=content.get("message"), requester_user_id=requester.user.to_string(), @@ -93,7 +93,7 @@ async def on_POST(self, request, room_id): 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) + content = parse_json_object_from_request(request) block = content.get("block", False) if not isinstance(block, bool): diff --git a/tests/rest/admin/test_room.py b/tests/rest/admin/test_room.py index c0dd05d018a6..a80537c4fcd5 100644 --- a/tests/rest/admin/test_room.py +++ b/tests/rest/admin/test_room.py @@ -190,7 +190,7 @@ def test_requester_is_no_admin(self): """ request, channel = self.make_request( - "POST", self.url, access_token=self.other_user_tok, + "POST", self.url, json.dumps({}), access_token=self.other_user_tok, ) self.render(request) @@ -204,7 +204,7 @@ def test_room_does_not_exist(self): url = "/_synapse/admin/v1/rooms/!unknown:test/delete" request, channel = self.make_request( - "POST", url, access_token=self.admin_user_tok, + "POST", url, json.dumps({}), access_token=self.admin_user_tok, ) self.render(request) @@ -218,7 +218,7 @@ def test_room_is_not_valid(self): url = "/_synapse/admin/v1/rooms/invalidroom/delete" request, channel = self.make_request( - "POST", url, access_token=self.admin_user_tok, + "POST", url, json.dumps({}), access_token=self.admin_user_tok, ) self.render(request) @@ -229,7 +229,7 @@ def test_room_is_not_valid(self): def test_new_room_user_does_not_exist(self): """ - Tests that a lookup for a user that does not exist returns a 404 + Tests that the user ID must be from local server but it does not have to exist. """ body = json.dumps({"new_room_user_id": "@unknown:test"}) @@ -241,8 +241,11 @@ def test_new_room_user_does_not_exist(self): ) self.render(request) - self.assertEqual(404, int(channel.result["code"]), msg=channel.result["body"]) - self.assertEqual(Codes.NOT_FOUND, channel.json_body["errcode"]) + self.assertEqual(200, int(channel.result["code"]), msg=channel.result["body"]) + self.assertIn("new_room_id", channel.json_body) + self.assertIn("kicked_users", channel.json_body) + self.assertIn("failed_to_kick_users", channel.json_body) + self.assertIn("local_aliases", channel.json_body) def test_new_room_user_is_not_local(self): """ diff --git a/tests/storage/test_room.py b/tests/storage/test_room.py index a37aec5bbe21..b1dceb29187c 100644 --- a/tests/storage/test_room.py +++ b/tests/storage/test_room.py @@ -55,6 +55,10 @@ def test_get_room(self): (yield self.store.get_room(self.room.to_string())), ) + @defer.inlineCallbacks + def test_get_room_unknown_room(self): + self.assertIsNone((yield self.store.get_room("!uknown:test")),) + @defer.inlineCallbacks def test_get_room_with_stats(self): self.assertDictContainsSubset( From b90ada785fe15322e3d0182990ac38c94bbab324 Mon Sep 17 00:00:00 2001 From: Richard van der Hoff <1389908+richvdh@users.noreply.github.com> Date: Tue, 14 Jul 2020 11:35:31 +0100 Subject: [PATCH 12/12] Update docs/admin_api/rooms.md --- docs/admin_api/rooms.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/docs/admin_api/rooms.md b/docs/admin_api/rooms.md index c02aa309a651..3f26adc16caa 100644 --- a/docs/admin_api/rooms.md +++ b/docs/admin_api/rooms.md @@ -390,7 +390,8 @@ The following JSON body parameters are available: * `new_room_user_id` - Optional. If set, a new room will be created with this user ID as the creator and admin, and all users in the old room will be moved into that room. If not set, no new room will be created and the users will just be removed - from the old room. The user ID must be from local server but it does not have to exist. + from the old room. The user ID must be on the local server, but does not necessarily + have to belong to a registered user. * `room_name` - Optional. A string representing the name of the room that new users will be invited to. Defaults to `Content Violation Notification` * `message` - Optional. A string containing the first message that will be sent as