From 44c10fbcae02d75bc4161e6ee4f3449b329a55a6 Mon Sep 17 00:00:00 2001 From: dklimpel <5740567+dklimpel@users.noreply.github.com> Date: Fri, 12 Nov 2021 16:10:41 +0100 Subject: [PATCH 1/6] Add dedicated admin API for blocking a room --- docs/admin_api/rooms.md | 77 +++++++++ synapse/rest/admin/__init__.py | 2 + synapse/rest/admin/rooms.py | 61 +++++++ synapse/storage/databases/main/room.py | 28 +++ tests/rest/admin/test_room.py | 228 +++++++++++++++++++++++++ 5 files changed, 396 insertions(+) diff --git a/docs/admin_api/rooms.md b/docs/admin_api/rooms.md index 6a6ae92d66ba..9148c821f61c 100644 --- a/docs/admin_api/rooms.md +++ b/docs/admin_api/rooms.md @@ -3,6 +3,7 @@ - [Room Details API](#room-details-api) - [Room Members API](#room-members-api) - [Room State API](#room-state-api) +- [Block Room API](#block-room-api) - [Delete Room API](#delete-room-api) * [Version 1 (old version)](#version-1-old-version) * [Version 2 (new version)](#version-2-new-version) @@ -386,6 +387,82 @@ A response body like the following is returned: } ``` +# Block Room API +The Block Room admin API allows server admins to block rooms and get the status. +This API can be used to pre-emptively block a room, even if it's unknown to this +homeserver. Users will be prevented from joining a blocked room. + +## Block or unblock a room + +The API is: + +``` +PUT /_synapse/admin/v1/rooms//block +``` + +with a body of: + +```json +{ + "block": true +} +``` + +A response body like the following is returned: + +```json +{ + "block": true +} +``` + +**Parameters** + +The following parameters should be set in the URL: + +- `room_id` - The ID of the room. + +The following JSON body parameters are available: + +- `block` - If `true` the room will be blocked and if `false` the room will be unblocked. + +**Response** + +The following fields are possible in the JSON response body: + +- `block` - A boolean if the room is blocked or not. + +## Get block status + +The API is: + +``` +GET /_synapse/admin/v1/rooms//block +``` + +A response body like the following is returned: + +```json +{ + "block": true, + "user_id": "" +} +``` + +**Parameters** + +The following parameters should be set in the URL: + +- `room_id` - The ID of the room. + +**Response** + +The following fields are possible in the JSON response body: + +- `block` - A boolean if the room is blocked or not. +- `user_id` - Optional. If the room is blocked (`block` is `true`) shows the user who has + add the room to blocking list. Otherwise it is not displayed. + # Delete Room API The Delete Room admin API allows server admins to remove rooms from the server diff --git a/synapse/rest/admin/__init__.py b/synapse/rest/admin/__init__.py index d78fe406c40a..65b76fa10c57 100644 --- a/synapse/rest/admin/__init__.py +++ b/synapse/rest/admin/__init__.py @@ -46,6 +46,7 @@ RegistrationTokenRestServlet, ) from synapse.rest.admin.rooms import ( + BlockRoomRestServlet, DeleteRoomStatusByDeleteIdRestServlet, DeleteRoomStatusByRoomIdRestServlet, ForwardExtremitiesRestServlet, @@ -223,6 +224,7 @@ def register_servlets(hs: "HomeServer", http_server: HttpServer) -> None: Register all the admin servlets. """ register_servlets_for_client_rest_resource(hs, http_server) + BlockRoomRestServlet(hs).register(http_server) ListRoomRestServlet(hs).register(http_server) RoomStateRestServlet(hs).register(http_server) RoomRestServlet(hs).register(http_server) diff --git a/synapse/rest/admin/rooms.py b/synapse/rest/admin/rooms.py index 37cb4d079618..79ee01d28bd1 100644 --- a/synapse/rest/admin/rooms.py +++ b/synapse/rest/admin/rooms.py @@ -782,3 +782,64 @@ async def on_GET( ) return 200, results + + +class BlockRoomRestServlet(RestServlet): + """ + Manage blocking of rooms. + On PUT: Add or remove a room from blocking list. + On GET: Get blocking status of room and user who has blocked this room. + """ + + PATTERNS = admin_patterns("/rooms/(?P[^/]+)/block$") + + def __init__(self, hs: "HomeServer"): + self._auth = hs.get_auth() + self._store = hs.get_datastore() + + async def on_GET( + self, request: SynapseRequest, room_id: str + ) -> Tuple[int, JsonDict]: + await assert_requester_is_admin(self._auth, request) + + if not RoomID.is_valid(room_id): + raise SynapseError( + HTTPStatus.BAD_REQUEST, "%s is not a legal room ID" % (room_id,) + ) + + blocked_by = await self._store.room_is_blocked_by(room_id) + if blocked_by: + response = {"block": True, "user_id": blocked_by} + else: + response = {"block": False} + + return 200, response + + async def on_PUT( + self, request: SynapseRequest, room_id: str + ) -> Tuple[int, JsonDict]: + requester = await self._auth.get_user_by_req(request) + await assert_user_is_admin(self._auth, requester.user) + + content = parse_json_object_from_request(request) + + if not RoomID.is_valid(room_id): + raise SynapseError( + HTTPStatus.BAD_REQUEST, "%s is not a legal room ID" % (room_id,) + ) + + assert_params_in_dict(content, ["block"]) + block = content.get("block") + if not isinstance(block, bool): + raise SynapseError( + HTTPStatus.BAD_REQUEST, + "Param 'block' must be a boolean.", + Codes.BAD_JSON, + ) + + if block: + await self._store.block_room(room_id, requester.user.to_string()) + else: + await self._store.unblock_room(room_id) + + return 200, {"block": block} diff --git a/synapse/storage/databases/main/room.py b/synapse/storage/databases/main/room.py index 17b398bb6907..a21f5e7d999b 100644 --- a/synapse/storage/databases/main/room.py +++ b/synapse/storage/databases/main/room.py @@ -397,6 +397,16 @@ async def is_room_blocked(self, room_id: str) -> Optional[bool]: desc="is_room_blocked", ) + async def room_is_blocked_by(self, room_id: str) -> Optional[str]: + """Function to retrieve user who has blocked the room""" + return await self.db_pool.simple_select_one_onecol( + table="blocked_rooms", + keyvalues={"room_id": room_id}, + retcol="user_id", + allow_none=True, + desc="room_is_blocked_by", + ) + async def get_rooms_paginate( self, start: int, @@ -1775,3 +1785,21 @@ async def block_room(self, room_id: str, user_id: str) -> None: self.is_room_blocked, (room_id,), ) + + async def unblock_room(self, room_id: str) -> None: + """Remove the room from blocking list. + + Args: + room_id: Room to unblock + """ + await self.db_pool.simple_delete( + table="blocked_rooms", + keyvalues={"room_id": room_id}, + desc="unblock_room", + ) + await self.db_pool.runInteraction( + "block_room_invalidation", + self._invalidate_cache_and_stream, + self.is_room_blocked, + (room_id,), + ) diff --git a/tests/rest/admin/test_room.py b/tests/rest/admin/test_room.py index b48fc12e5f91..07077aff78d6 100644 --- a/tests/rest/admin/test_room.py +++ b/tests/rest/admin/test_room.py @@ -2226,6 +2226,234 @@ def test_not_enough_power(self): ) +class BlockRoomTestCase(unittest.HomeserverTestCase): + servlets = [ + synapse.rest.admin.register_servlets, + room.register_servlets, + login.register_servlets, + ] + + def prepare(self, reactor, clock, hs): + 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") + + self.room_id = self.helper.create_room_as( + self.other_user, tok=self.other_user_tok + ) + self.url = "/_synapse/admin/v1/rooms/%s/block" + + @parameterized.expand([("PUT",), ("GET",)]) + def test_requester_is_no_admin(self, method: str): + """If the user is not a server admin, an error 403 is returned.""" + + channel = self.make_request( + method, + self.url % self.room_id, + content={}, + access_token=self.other_user_tok, + ) + + self.assertEqual(HTTPStatus.FORBIDDEN, channel.code, msg=channel.json_body) + self.assertEqual(Codes.FORBIDDEN, channel.json_body["errcode"]) + + @parameterized.expand([("PUT",), ("GET",)]) + def test_room_is_not_valid(self, method: str): + """Check that invalid room names, return an error 400.""" + + channel = self.make_request( + method, + self.url % "invalidroom", + content={}, + access_token=self.admin_user_tok, + ) + + self.assertEqual(HTTPStatus.BAD_REQUEST, channel.code, msg=channel.json_body) + self.assertEqual( + "invalidroom is not a legal room ID", + channel.json_body["error"], + ) + + def test_block_is_not_valid(self): + """If parameter `block` is not valid, return an error.""" + + # `block` is not valid + channel = self.make_request( + "PUT", + self.url % self.room_id, + content={"block": "NotBool"}, + access_token=self.admin_user_tok, + ) + + self.assertEqual(HTTPStatus.BAD_REQUEST, channel.code, msg=channel.json_body) + self.assertEqual(Codes.BAD_JSON, channel.json_body["errcode"]) + + # `block` is not set + channel = self.make_request( + "PUT", + self.url % self.room_id, + content={}, + access_token=self.admin_user_tok, + ) + + self.assertEqual(HTTPStatus.BAD_REQUEST, channel.code, msg=channel.json_body) + self.assertEqual(Codes.MISSING_PARAM, channel.json_body["errcode"]) + + # no content is send + channel = self.make_request( + "PUT", + self.url % self.room_id, + access_token=self.admin_user_tok, + ) + + self.assertEqual(HTTPStatus.BAD_REQUEST, channel.code, msg=channel.json_body) + self.assertEqual(Codes.NOT_JSON, channel.json_body["errcode"]) + + def test_block_room(self): + """Test that block a room is successful.""" + + def _request_and_test_block_room(room_id: str) -> None: + self._is_blocked(room_id, expect=False) + channel = self.make_request( + "PUT", + self.url % room_id, + content={"block": True}, + access_token=self.admin_user_tok, + ) + self.assertEqual(HTTPStatus.OK, channel.code, msg=channel.json_body) + self.assertTrue(channel.json_body["block"]) + self._is_blocked(room_id, expect=True) + + # known internal room + _request_and_test_block_room(self.room_id) + + # unknown internal room + _request_and_test_block_room("!unknown:test") + + # unknown remote room + _request_and_test_block_room("!unknown:remote") + + def test_block_room_twice(self): + """Test that block a room that is already blocked is successful.""" + + self._is_blocked(self.room_id, expect=False) + for _ in range(2): + channel = self.make_request( + "PUT", + self.url % self.room_id, + content={"block": True}, + access_token=self.admin_user_tok, + ) + self.assertEqual(HTTPStatus.OK, channel.code, msg=channel.json_body) + self.assertTrue(channel.json_body["block"]) + self._is_blocked(self.room_id, expect=True) + + def test_unblock_room(self): + """Test that unblock a room is successful.""" + + def _request_and_test_unblock_room(room_id: str) -> None: + self._block_room(room_id) + + channel = self.make_request( + "PUT", + self.url % room_id, + content={"block": False}, + access_token=self.admin_user_tok, + ) + self.assertEqual(HTTPStatus.OK, channel.code, msg=channel.json_body) + self.assertFalse(channel.json_body["block"]) + self._is_blocked(room_id, expect=False) + + # known internal room + _request_and_test_unblock_room(self.room_id) + + # unknown internal room + _request_and_test_unblock_room("!unknown:test") + + # unknown remote room + _request_and_test_unblock_room("!unknown:remote") + + def test_unblock_room_twice(self): + """Test that unblock a room that is not blocked is successful.""" + + self._block_room(self.room_id) + for _ in range(2): + channel = self.make_request( + "PUT", + self.url % self.room_id, + content={"block": False}, + access_token=self.admin_user_tok, + ) + self.assertEqual(HTTPStatus.OK, channel.code, msg=channel.json_body) + self.assertFalse(channel.json_body["block"]) + self._is_blocked(self.room_id, expect=False) + + def test_get_blocked_room(self): + """Test get status of a blocked room""" + + def _request_blocked_room(room_id: str) -> None: + self._block_room(room_id) + + channel = self.make_request( + "GET", + self.url % room_id, + access_token=self.admin_user_tok, + ) + self.assertEqual(HTTPStatus.OK, channel.code, msg=channel.json_body) + self.assertTrue(channel.json_body["block"]) + self.assertEqual(self.other_user, channel.json_body["user_id"]) + + # known internal room + _request_blocked_room(self.room_id) + + # unknown internal room + _request_blocked_room("!unknown:test") + + # unknown remote room + _request_blocked_room("!unknown:remote") + + def test_get_unblocked_room(self): + """Test get status of a unblocked room""" + + def _request_unblocked_room(room_id: str) -> None: + self._is_blocked(room_id, expect=False) + + channel = self.make_request( + "GET", + self.url % room_id, + access_token=self.admin_user_tok, + ) + self.assertEqual(HTTPStatus.OK, channel.code, msg=channel.json_body) + self.assertFalse(channel.json_body["block"]) + self.assertNotIn("user_id", channel.json_body) + + # known internal room + _request_unblocked_room(self.room_id) + + # unknown internal room + _request_unblocked_room("!unknown:test") + + # unknown remote room + _request_unblocked_room("!unknown:remote") + + def _is_blocked(self, room_id: str, expect: bool = True) -> None: + """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 _block_room(self, room_id: str) -> None: + """Block a room in database""" + self.get_success(self._store.block_room(room_id, self.other_user)) + self._is_blocked(room_id, expect=True) + + PURGE_TABLES = [ "current_state_events", "event_backward_extremities", From 3011cf4ecb0b903b25511c40921651ce23435860 Mon Sep 17 00:00:00 2001 From: dklimpel <5740567+dklimpel@users.noreply.github.com> Date: Fri, 12 Nov 2021 16:14:24 +0100 Subject: [PATCH 2/6] newsfile --- changelog.d/11324.feature | 1 + 1 file changed, 1 insertion(+) create mode 100644 changelog.d/11324.feature diff --git a/changelog.d/11324.feature b/changelog.d/11324.feature new file mode 100644 index 000000000000..55494358bb8e --- /dev/null +++ b/changelog.d/11324.feature @@ -0,0 +1 @@ +Add dedicated admin API for blocking a room. \ No newline at end of file From 5008f5854fd08fb3bfac8224299c0a0043e8e382 Mon Sep 17 00:00:00 2001 From: Dirk Klimpel <5740567+dklimpel@users.noreply.github.com> Date: Thu, 18 Nov 2021 15:23:26 +0100 Subject: [PATCH 3/6] Apply suggestions from code review Co-authored-by: David Robertson --- docs/admin_api/rooms.md | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/docs/admin_api/rooms.md b/docs/admin_api/rooms.md index 9148c821f61c..2eb6360849ce 100644 --- a/docs/admin_api/rooms.md +++ b/docs/admin_api/rooms.md @@ -388,7 +388,7 @@ A response body like the following is returned: ``` # Block Room API -The Block Room admin API allows server admins to block rooms and get the status. +The Block Room admin API allows server admins to block and unblock rooms, and query to see if a given room is blocked. This API can be used to pre-emptively block a room, even if it's unknown to this homeserver. Users will be prevented from joining a blocked room. @@ -430,7 +430,7 @@ The following JSON body parameters are available: The following fields are possible in the JSON response body: -- `block` - A boolean if the room is blocked or not. +- `block` - A boolean. `true` if the room is blocked, otherwise `false` ## Get block status @@ -459,7 +459,7 @@ The following parameters should be set in the URL: The following fields are possible in the JSON response body: -- `block` - A boolean if the room is blocked or not. +- `block` - A boolean. `true` if the room is blocked, otherwise `false` - `user_id` - Optional. If the room is blocked (`block` is `true`) shows the user who has add the room to blocking list. Otherwise it is not displayed. From 8bad9edf776f5098bac69fca73b3be4bf48d37cc Mon Sep 17 00:00:00 2001 From: dklimpel <5740567+dklimpel@users.noreply.github.com> Date: Thu, 18 Nov 2021 15:31:08 +0100 Subject: [PATCH 4/6] chnage response to `HTTPStatus.OK` --- synapse/rest/admin/rooms.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/synapse/rest/admin/rooms.py b/synapse/rest/admin/rooms.py index 79ee01d28bd1..23afb62be95b 100644 --- a/synapse/rest/admin/rooms.py +++ b/synapse/rest/admin/rooms.py @@ -813,7 +813,7 @@ async def on_GET( else: response = {"block": False} - return 200, response + return HTTPStatus.OK, response async def on_PUT( self, request: SynapseRequest, room_id: str @@ -842,4 +842,4 @@ async def on_PUT( else: await self._store.unblock_room(room_id) - return 200, {"block": block} + return HTTPStatus.OK, {"block": block} From 03cdf598faed2783c8aa8fba32e7a0cc74fb9cee Mon Sep 17 00:00:00 2001 From: dklimpel <5740567+dklimpel@users.noreply.github.com> Date: Thu, 18 Nov 2021 15:42:14 +0100 Subject: [PATCH 5/6] some docs --- docs/admin_api/rooms.md | 7 ++++--- synapse/rest/admin/rooms.py | 4 +++- synapse/storage/databases/main/room.py | 6 +++++- 3 files changed, 12 insertions(+), 5 deletions(-) diff --git a/docs/admin_api/rooms.md b/docs/admin_api/rooms.md index 2eb6360849ce..729304d43bd4 100644 --- a/docs/admin_api/rooms.md +++ b/docs/admin_api/rooms.md @@ -388,7 +388,8 @@ A response body like the following is returned: ``` # Block Room API -The Block Room admin API allows server admins to block and unblock rooms, and query to see if a given room is blocked. +The Block Room admin API allows server admins to block and unblock rooms, +and query to see if a given room is blocked. This API can be used to pre-emptively block a room, even if it's unknown to this homeserver. Users will be prevented from joining a blocked room. @@ -460,8 +461,8 @@ The following parameters should be set in the URL: The following fields are possible in the JSON response body: - `block` - A boolean. `true` if the room is blocked, otherwise `false` -- `user_id` - Optional. If the room is blocked (`block` is `true`) shows the user who has - add the room to blocking list. Otherwise it is not displayed. +- `user_id` - A optional string. If the room is blocked (`block` is `true`) shows + the user who has add the room to blocking list. Otherwise it is not displayed. # Delete Room API diff --git a/synapse/rest/admin/rooms.py b/synapse/rest/admin/rooms.py index 23afb62be95b..5b8ec1e5ca89 100644 --- a/synapse/rest/admin/rooms.py +++ b/synapse/rest/admin/rooms.py @@ -808,7 +808,9 @@ async def on_GET( ) blocked_by = await self._store.room_is_blocked_by(room_id) - if blocked_by: + # Test `not None` if `user_id` is an empty string + # if someone add manually an entry in database + if blocked_by is not None: response = {"block": True, "user_id": blocked_by} else: response = {"block": False} diff --git a/synapse/storage/databases/main/room.py b/synapse/storage/databases/main/room.py index a21f5e7d999b..7d694d852d53 100644 --- a/synapse/storage/databases/main/room.py +++ b/synapse/storage/databases/main/room.py @@ -398,7 +398,11 @@ async def is_room_blocked(self, room_id: str) -> Optional[bool]: ) async def room_is_blocked_by(self, room_id: str) -> Optional[str]: - """Function to retrieve user who has blocked the room""" + """ + Function to retrieve user who has blocked the room. + user_id is non-nullable + It returns None if the room is not blocked. + """ return await self.db_pool.simple_select_one_onecol( table="blocked_rooms", keyvalues={"room_id": room_id}, From 82af5cffbb0babf250ee8b23c5b561e50bd11731 Mon Sep 17 00:00:00 2001 From: Dirk Klimpel <5740567+dklimpel@users.noreply.github.com> Date: Thu, 18 Nov 2021 17:39:10 +0100 Subject: [PATCH 6/6] Apply suggestions from code review Co-authored-by: David Robertson --- docs/admin_api/rooms.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/admin_api/rooms.md b/docs/admin_api/rooms.md index 729304d43bd4..0f1a74134ff9 100644 --- a/docs/admin_api/rooms.md +++ b/docs/admin_api/rooms.md @@ -461,7 +461,7 @@ The following parameters should be set in the URL: The following fields are possible in the JSON response body: - `block` - A boolean. `true` if the room is blocked, otherwise `false` -- `user_id` - A optional string. If the room is blocked (`block` is `true`) shows +- `user_id` - An optional string. If the room is blocked (`block` is `true`) shows the user who has add the room to blocking list. Otherwise it is not displayed. # Delete Room API