From 1e555b3c90ea4fb8d091bedf4a0f9bdd4bd3a217 Mon Sep 17 00:00:00 2001 From: dklimpel <5740567+dklimpel@users.noreply.github.com> Date: Mon, 27 Jul 2020 20:50:39 +0200 Subject: [PATCH 1/4] Add an option to disable purge in delete room admin API --- docs/admin_api/rooms.md | 11 ++++--- synapse/rest/admin/rooms.py | 11 ++++++- tests/rest/admin/test_room.py | 56 +++++++++++++++++++++++++++++++++-- 3 files changed, 71 insertions(+), 7 deletions(-) diff --git a/docs/admin_api/rooms.md b/docs/admin_api/rooms.md index 15b83e98248b..37de1c5847dd 100644 --- a/docs/admin_api/rooms.md +++ b/docs/admin_api/rooms.md @@ -369,7 +369,7 @@ to the new room will have power level `-10` by default, and thus be unable to sp 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. +all local users. If you do not like to remove the trace set `purge` to `False`. Depending on the amount of history being purged a call to the API may take several minutes or longer. @@ -388,7 +388,8 @@ with a body of: "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 + "block": true, + "purge": true } ``` @@ -430,8 +431,10 @@ 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. Defaults to `false`. +* `purge` - Optional. If set to `true`, it will remove all trace of a room from your database. + Defaults to `true`. The JSON body must not be empty. The body must be at least `{}`. diff --git a/synapse/rest/admin/rooms.py b/synapse/rest/admin/rooms.py index b8c95d045a74..a8364d9793d7 100644 --- a/synapse/rest/admin/rooms.py +++ b/synapse/rest/admin/rooms.py @@ -103,6 +103,14 @@ async def on_POST(self, request, room_id): Codes.BAD_JSON, ) + purge = content.get("purge", True) + if not isinstance(purge, bool): + raise SynapseError( + HTTPStatus.BAD_REQUEST, + "Param 'purge' must be a boolean, if given", + Codes.BAD_JSON, + ) + ret = await self.room_shutdown_handler.shutdown_room( room_id=room_id, new_room_user_id=content.get("new_room_user_id"), @@ -113,7 +121,8 @@ async def on_POST(self, request, room_id): ) # Purge room - await self.pagination_handler.purge_room(room_id) + if purge: + await self.pagination_handler.purge_room(room_id) return (200, ret) diff --git a/tests/rest/admin/test_room.py b/tests/rest/admin/test_room.py index ba8552c29f40..ad3125b5a30b 100644 --- a/tests/rest/admin/test_room.py +++ b/tests/rest/admin/test_room.py @@ -283,6 +283,23 @@ def test_block_is_not_bool(self): self.assertEqual(400, int(channel.result["code"]), msg=channel.result["body"]) self.assertEqual(Codes.BAD_JSON, channel.json_body["errcode"]) + def test_purge_is_not_bool(self): + """ + If parameter `purge` is not boolean, return an error + """ + body = json.dumps({"purge": "NotBool"}) + + request, channel = self.make_request( + "POST", + 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. @@ -297,7 +314,7 @@ def test_purge_room_and_block(self): # Assert one user in room self._is_member(room_id=self.room_id, user_id=self.other_user) - body = json.dumps({"block": True}) + body = json.dumps({"block": True, "purge": True}) request, channel = self.make_request( "POST", @@ -331,7 +348,7 @@ def test_purge_room_and_not_block(self): # Assert one user in room self._is_member(room_id=self.room_id, user_id=self.other_user) - body = json.dumps({"block": False}) + body = json.dumps({"block": False, "purge": True}) request, channel = self.make_request( "POST", @@ -351,6 +368,41 @@ def test_purge_room_and_not_block(self): self._is_blocked(self.room_id, expect=False) self._has_no_members(self.room_id) + def test_block_room_and_not_purge(self): + """Test to block a room and do not purge 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, "purge": False}) + + request, channel = self.make_request( + "POST", + 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(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) + + with self.assertRaises(AssertionError): + 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 From 133a210e10b26e7dfa6635a82d610daf46165e38 Mon Sep 17 00:00:00 2001 From: dklimpel <5740567+dklimpel@users.noreply.github.com> Date: Mon, 27 Jul 2020 22:18:14 +0200 Subject: [PATCH 2/4] Add changelog --- changelog.d/7964.feature | 1 + 1 file changed, 1 insertion(+) create mode 100644 changelog.d/7964.feature diff --git a/changelog.d/7964.feature b/changelog.d/7964.feature new file mode 100644 index 000000000000..ffe861650ce2 --- /dev/null +++ b/changelog.d/7964.feature @@ -0,0 +1 @@ +Add an option to purge room or not with delete room admin endpoint (`POST /_synapse/admin/v1/rooms//delete`). Contributed by @dklimpel. \ No newline at end of file From 0354646b8c91c22d5442f661c93b9814bde61f79 Mon Sep 17 00:00:00 2001 From: Dirk Klimpel <5740567+dklimpel@users.noreply.github.com> Date: Tue, 28 Jul 2020 17:37:59 +0200 Subject: [PATCH 3/4] Apply suggestions from code review Co-authored-by: Andrew Morgan <1342360+anoadragon453@users.noreply.github.com> --- docs/admin_api/rooms.md | 2 +- tests/rest/admin/test_room.py | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/docs/admin_api/rooms.md b/docs/admin_api/rooms.md index 37de1c5847dd..258971b0d9a1 100644 --- a/docs/admin_api/rooms.md +++ b/docs/admin_api/rooms.md @@ -433,7 +433,7 @@ The following JSON body parameters are available: 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`. -* `purge` - Optional. If set to `true`, it will remove all trace of a room from your database. +* `purge` - Optional. If set to `true`, it will remove all traces of the room from your database. Defaults to `true`. The JSON body must not be empty. The body must be at least `{}`. diff --git a/tests/rest/admin/test_room.py b/tests/rest/admin/test_room.py index ad3125b5a30b..cec1cf928f9f 100644 --- a/tests/rest/admin/test_room.py +++ b/tests/rest/admin/test_room.py @@ -369,8 +369,9 @@ def test_purge_room_and_not_block(self): self._has_no_members(self.room_id) def test_block_room_and_not_purge(self): - """Test to block a room and do not purge it. + """Test to block a room without purging it. Members will not be moved to a new room and will not receive a message. + The room will not be purged. """ # Test that room is not purged with self.assertRaises(AssertionError): From 390d035e012cff2e618df6f14caf158fef07e86e Mon Sep 17 00:00:00 2001 From: dklimpel <5740567+dklimpel@users.noreply.github.com> Date: Tue, 28 Jul 2020 17:43:48 +0200 Subject: [PATCH 4/4] Apply suggestions for documentation --- docs/admin_api/rooms.md | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/docs/admin_api/rooms.md b/docs/admin_api/rooms.md index 258971b0d9a1..0f267d2b7bbe 100644 --- a/docs/admin_api/rooms.md +++ b/docs/admin_api/rooms.md @@ -369,7 +369,9 @@ to the new room will have power level `-10` by default, and thus be unable to sp 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. If you do not like to remove the trace set `purge` to `False`. +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 several minutes or longer.