From 11105b75f8e7f70adca47931db41f54eed32cea9 Mon Sep 17 00:00:00 2001 From: dklimpel <5740567+dklimpel@users.noreply.github.com> Date: Mon, 31 Aug 2020 22:22:34 +0200 Subject: [PATCH 1/9] Admin API for reported events Add an admin API to read entries of table `event_reports`. API: `GET /_synapse/admin/v1/event_reports` --- docs/admin_api/event_reports.rst | 57 ++++++ synapse/rest/admin/__init__.py | 2 + synapse/rest/admin/event_reports.py | 73 ++++++++ synapse/storage/databases/main/room.py | 52 ++++++ tests/rest/admin/test_event_reports.py | 238 +++++++++++++++++++++++++ 5 files changed, 422 insertions(+) create mode 100644 docs/admin_api/event_reports.rst create mode 100644 synapse/rest/admin/event_reports.py create mode 100644 tests/rest/admin/test_event_reports.py diff --git a/docs/admin_api/event_reports.rst b/docs/admin_api/event_reports.rst new file mode 100644 index 000000000000..39a8558264dc --- /dev/null +++ b/docs/admin_api/event_reports.rst @@ -0,0 +1,57 @@ +Shows reported events +==================== + +This API returns information about reported events. + +The api is:: + + GET /_synapse/admin/v1/event_reports?from=0&limit=10 + +To use it, you will need to authenticate by providing an ``access_token`` for a +server admin: see `README.rst `_. + +It returns a JSON body like the following: + +.. code:: json + + { + "event_reports": [ + { + "content": "{\"reason\": \"foo\", \"score\": -100}", + "event_id": "$bNUFCwGzWca1meCGkjp-zwslF-GfVcXukvRLI1_FaVY", + "id": 2, + "reason": "foo", + "received_ts": 1570897107409, + "room_id": "!ERAgBpSOcCCuTJqQPk:matrix.org", + "user_id": "@foo:matrix.org" + }, + { + "content": "{\"score\":-100,\"reason\":\"bar\"}", + "event_id": "$3IcdZsDaN_En-S1DF4EMCy3v4gNRKeOJs8W5qTOKj4I", + "id": 3, + "reason": "bar", + "received_ts": 1598889612059, + "room_id": "!eGvUQuTCkHGVwNMOjv:matrix.org", + "user_id": "@bar:matrix.org" + } + ], + "next_token": "2", + "total": 4 +} + +To paginate, check for ``next_token`` and if present, call the endpoint again +with from set to the value of ``next_token``. This will return a new page. + +If the endpoint does not return a ``next_token`` then there are no more +users to paginate through. + +**URL parameters:** + +- ``limit``: Is optional but is used for pagination, + denoting the maximum number of items to return in this call. Defaults to ``100``. +- ``from``: Is optional but used for pagination, + denoting the offset in the returned results. This should be treated as an opaque value and + not explicitly set to anything other than the return value of next_token from a previous call. + Defaults to ``0``. +- ``user_id``: Is optional and filters to only return users with user IDs that contain this value. +- ``room_id``: Is optional and filters to only return rooms with room IDs that contain this value. diff --git a/synapse/rest/admin/__init__.py b/synapse/rest/admin/__init__.py index 1c88c93f3836..1a0faee09daa 100644 --- a/synapse/rest/admin/__init__.py +++ b/synapse/rest/admin/__init__.py @@ -55,6 +55,7 @@ UsersRestServletV2, WhoisRestServlet, ) +from synapse.rest.admin.event_reports import EventReportsRestServlet from synapse.util.versionstring import get_version_string logger = logging.getLogger(__name__) @@ -214,6 +215,7 @@ def register_servlets(hs, http_server): DeviceRestServlet(hs).register(http_server) DevicesRestServlet(hs).register(http_server) DeleteDevicesRestServlet(hs).register(http_server) + EventReportsRestServlet(hs).register(http_server) def register_servlets_for_client_rest_resource(hs, http_server): diff --git a/synapse/rest/admin/event_reports.py b/synapse/rest/admin/event_reports.py new file mode 100644 index 000000000000..40194a98a2e8 --- /dev/null +++ b/synapse/rest/admin/event_reports.py @@ -0,0 +1,73 @@ +# -*- coding: utf-8 -*- +# Copyright 2020 Dirk Klimpel +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +import logging + +from synapse.http.servlet import ( + RestServlet, + parse_integer, + parse_string, +) +from synapse.rest.admin._base import ( + assert_requester_is_admin, + admin_patterns, +) + +logger = logging.getLogger(__name__) + + +class EventReportsRestServlet(RestServlet): + """ + List all reported events that are known to the homeserver. Results are returned + in a dictionary containing report information. Supports pagination. + This needs user to have administrator access in Synapse. + + GET /_synapse/admin/v1/event_reports?from=0&limit=10 + returns: + 200 OK with list of reports if success otherwise an error. + + Args + The parameters `from` and `limit` are required only for pagination. + By default, a `limit` of 100 is used. + The parameter `user_id` can be used to filter by user id. + The parameter `room_id` can be used to filter by room id. + Returns: + A list of reported events and an integer representing the total number of + reported events that exist given this query + """ + + PATTERNS = admin_patterns("/event_reports$") + + def __init__(self, hs): + self.hs = hs + self.auth = hs.get_auth() + self.store = hs.get_datastore() + + async def on_GET(self, request): + await assert_requester_is_admin(self.auth, request) + + start = parse_integer(request, "from", default=0) + limit = parse_integer(request, "limit", default=100) + user_id = parse_string(request, "user_id", default=None) + room_id = parse_string(request, "room_id", default=None) + + event_reports, total = await self.store.get_event_reports_paginate( + start, limit, user_id, room_id + ) + ret = {"event_reports": event_reports, "total": total} + if len(event_reports) >= limit: + ret["next_token"] = str(start + len(event_reports)) + + return 200, ret diff --git a/synapse/storage/databases/main/room.py b/synapse/storage/databases/main/room.py index a92641c33927..9b6744b04118 100644 --- a/synapse/storage/databases/main/room.py +++ b/synapse/storage/databases/main/room.py @@ -1320,6 +1320,58 @@ async def add_event_report( desc="add_event_report", ) + async def get_event_reports_paginate( + self, start: int, limit: int, user_id: str = None, room_id: str = None + ) -> Tuple[List[Dict[str, Any]], int]: + """Function to retrieve a paginated list of event reports + This will return a json list of event reports and the + total number of event reports matching the filter criteria. + + Args: + start: start number to begin the query from + limit: number of rows to retrieve + user_id: search for user_id. ignored if name is not None + room_id: search for room_id. ignored if name is not None + """ + + def _get_event_reports_paginate_txn(txn): + filters = [] + args = [] + + if user_id: + filters.append("user_id LIKE ?") + args.extend(["%" + user_id + "%"]) + if room_id: + filters.append("room_id LIKE ?") + args.extend(["%" + room_id + "%"]) + + where_clause = "WHERE " + " AND ".join(filters) if len(filters) > 0 else "" + + sql_base = """ + FROM event_reports + {} + """.format( + where_clause + ) + logger.warning("sql_base: %s ", sql_base) + sql = "SELECT COUNT(*) as total_event_reports " + sql_base + txn.execute(sql, args) + count = txn.fetchone()[0] + + sql = ( + "SELECT id, received_ts, room_id, event_id, user_id, reason, content " + + sql_base + + " ORDER BY received_ts LIMIT ? OFFSET ?" + ) + args += [limit, start] + txn.execute(sql, args) + event_reports = self.db_pool.cursor_to_dict(txn) + return event_reports, count + + return await self.db_pool.runInteraction( + "get_event_reports_paginate", _get_event_reports_paginate_txn + ) + def get_current_public_room_stream_id(self): return self._public_room_id_gen.get_current_token() diff --git a/tests/rest/admin/test_event_reports.py b/tests/rest/admin/test_event_reports.py new file mode 100644 index 000000000000..baedfc8cbbda --- /dev/null +++ b/tests/rest/admin/test_event_reports.py @@ -0,0 +1,238 @@ +# -*- coding: utf-8 -*- +# Copyright 2020 Dirk Klimpel +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +import json + +import synapse.rest.admin +from synapse.api.errors import Codes +from synapse.rest.client.v1 import login, room +from synapse.rest.client.v2_alpha import report_event + +from tests import unittest + + +class EventReportsTestCase(unittest.HomeserverTestCase): + servlets = [ + synapse.rest.admin.register_servlets, + login.register_servlets, + room.register_servlets, + report_event.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_id1 = self.helper.create_room_as( + self.other_user, tok=self.other_user_tok, is_public=True + ) + self.helper.join(self.room_id1, user=self.admin_user, tok=self.admin_user_tok) + + self.room_id2 = self.helper.create_room_as( + self.other_user, tok=self.other_user_tok, is_public=True + ) + self.helper.join(self.room_id2, user=self.admin_user, tok=self.admin_user_tok) + + # Two rooms and two users. Every user sends and reports in every room events + for i in range(5): + self._create_event_and_report( + room_id=self.room_id1, user_tok=self.other_user_tok, + ) + for i in range(5): + self._create_event_and_report( + room_id=self.room_id2, user_tok=self.other_user_tok, + ) + for i in range(5): + self._create_event_and_report( + room_id=self.room_id1, user_tok=self.admin_user_tok, + ) + for i in range(5): + self._create_event_and_report( + room_id=self.room_id2, user_tok=self.admin_user_tok, + ) + + self.url = "/_synapse/admin/v1/event_reports" + + 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( + "GET", 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_default_success(self): + """ + Testing list of reported events + """ + + request, channel = self.make_request( + "GET", self.url, 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["total"], 20) + self.assertNotIn("next_token", channel.json_body) + self._check_fields(channel.json_body["event_reports"]) + + def test_limit(self): + """ + Testing list of reported events with limit + """ + + request, channel = self.make_request( + "GET", self.url + "?limit=5", 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["total"], 20) + self.assertEqual(channel.json_body["next_token"], "5") + self.assertEqual(len(channel.json_body["event_reports"]), 5) + self._check_fields(channel.json_body["event_reports"]) + + def test_from(self): + """ + Testing list of reported events with definied starting point (from) + """ + + request, channel = self.make_request( + "GET", self.url + "?from=5", 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["total"], 20) + self.assertEqual(len(channel.json_body["event_reports"]), 15) + self.assertNotIn("next_token", channel.json_body) + self._check_fields(channel.json_body["event_reports"]) + + def test_limit_and_from(self): + """ + Testing list of reported events with definied starting point and limit + """ + + request, channel = self.make_request( + "GET", self.url + "?from=5&limit=10", 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["total"], 20) + self.assertEqual(channel.json_body["next_token"], "15") + self.assertEqual(len(channel.json_body["event_reports"]), 10) + self._check_fields(channel.json_body["event_reports"]) + + def test_filter_room(self): + """ + Testing list of reported events with a filter of room + """ + + request, channel = self.make_request( + "GET", + self.url + "?room_id=%s" % self.room_id1, + 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["total"], 10) + self.assertEqual(len(channel.json_body["event_reports"]), 10) + self.assertNotIn("next_token", channel.json_body) + self._check_fields(channel.json_body["event_reports"]) + + for f in channel.json_body["event_reports"]: + self.assertEqual(f["room_id"], self.room_id1) + + def test_filter_user(self): + """ + Testing list of reported events with a filter of user + """ + + request, channel = self.make_request( + "GET", + self.url + "?user_id=%s" % self.other_user, + 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["total"], 10) + self.assertEqual(len(channel.json_body["event_reports"]), 10) + self.assertNotIn("next_token", channel.json_body) + self._check_fields(channel.json_body["event_reports"]) + + for f in channel.json_body["event_reports"]: + self.assertEqual(f["user_id"], self.other_user) + + def test_filter_user_and_room(self): + """ + Testing list of reported events with a filter of user and room + """ + + request, channel = self.make_request( + "GET", + self.url + "?user_id=%s&room_id=%s" % (self.other_user, self.room_id1), + 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["total"], 5) + self.assertEqual(len(channel.json_body["event_reports"]), 5) + self.assertNotIn("next_token", channel.json_body) + self._check_fields(channel.json_body["event_reports"]) + + for f in channel.json_body["event_reports"]: + self.assertEqual(f["user_id"], self.other_user) + self.assertEqual(f["room_id"], self.room_id1) + + def _create_event_and_report(self, room_id, user_tok): + """ Create and report events + """ + resp = self.helper.send(room_id, tok=user_tok) + event_id = resp["event_id"] + + request, channel = self.make_request( + "POST", + "rooms/%s/report/%s" % (room_id, event_id), + json.dumps({"score": -100, "reason": "this makes me sad"}), + access_token=user_tok, + ) + self.render(request) + self.assertEqual(200, int(channel.result["code"]), msg=channel.result["body"]) + + def _check_fields(self, content): + """ Checks that all attributes are present in result + """ + for c in content: + self.assertIn("id", c) + self.assertIn("received_ts", c) + self.assertIn("room_id", c) + self.assertIn("event_id", c) + self.assertIn("user_id", c) + self.assertIn("reason", c) + self.assertIn("content", c) From f3a10fc9536bf258c3a212d5dbd0d276415f050e Mon Sep 17 00:00:00 2001 From: dklimpel <5740567+dklimpel@users.noreply.github.com> Date: Mon, 31 Aug 2020 22:31:20 +0200 Subject: [PATCH 2/9] Add changelog --- changelog.d/8217.feature | 1 + docs/admin_api/event_reports.rst | 114 ++++++++++++++-------------- synapse/rest/admin/__init__.py | 3 +- synapse/rest/admin/event_reports.py | 12 +-- 4 files changed, 61 insertions(+), 69 deletions(-) create mode 100644 changelog.d/8217.feature diff --git a/changelog.d/8217.feature b/changelog.d/8217.feature new file mode 100644 index 000000000000..899cbf14ef56 --- /dev/null +++ b/changelog.d/8217.feature @@ -0,0 +1 @@ +Add an admin API `GET /_synapse/admin/v1/event_reports` to read entries of table `event_reports`. Contributed by @dklimpel. \ No newline at end of file diff --git a/docs/admin_api/event_reports.rst b/docs/admin_api/event_reports.rst index 39a8558264dc..8c727fc5f61b 100644 --- a/docs/admin_api/event_reports.rst +++ b/docs/admin_api/event_reports.rst @@ -1,57 +1,57 @@ -Shows reported events -==================== - -This API returns information about reported events. - -The api is:: - - GET /_synapse/admin/v1/event_reports?from=0&limit=10 - -To use it, you will need to authenticate by providing an ``access_token`` for a -server admin: see `README.rst `_. - -It returns a JSON body like the following: - -.. code:: json - - { - "event_reports": [ - { - "content": "{\"reason\": \"foo\", \"score\": -100}", - "event_id": "$bNUFCwGzWca1meCGkjp-zwslF-GfVcXukvRLI1_FaVY", - "id": 2, - "reason": "foo", - "received_ts": 1570897107409, - "room_id": "!ERAgBpSOcCCuTJqQPk:matrix.org", - "user_id": "@foo:matrix.org" - }, - { - "content": "{\"score\":-100,\"reason\":\"bar\"}", - "event_id": "$3IcdZsDaN_En-S1DF4EMCy3v4gNRKeOJs8W5qTOKj4I", - "id": 3, - "reason": "bar", - "received_ts": 1598889612059, - "room_id": "!eGvUQuTCkHGVwNMOjv:matrix.org", - "user_id": "@bar:matrix.org" - } - ], - "next_token": "2", - "total": 4 -} - -To paginate, check for ``next_token`` and if present, call the endpoint again -with from set to the value of ``next_token``. This will return a new page. - -If the endpoint does not return a ``next_token`` then there are no more -users to paginate through. - -**URL parameters:** - -- ``limit``: Is optional but is used for pagination, - denoting the maximum number of items to return in this call. Defaults to ``100``. -- ``from``: Is optional but used for pagination, - denoting the offset in the returned results. This should be treated as an opaque value and - not explicitly set to anything other than the return value of next_token from a previous call. - Defaults to ``0``. -- ``user_id``: Is optional and filters to only return users with user IDs that contain this value. -- ``room_id``: Is optional and filters to only return rooms with room IDs that contain this value. +Shows reported events +==================== + +This API returns information about reported events. + +The api is:: + + GET /_synapse/admin/v1/event_reports?from=0&limit=10 + +To use it, you will need to authenticate by providing an ``access_token`` for a +server admin: see `README.rst `_. + +It returns a JSON body like the following: + +.. code:: json + + { + "event_reports": [ + { + "content": "{\"reason\": \"foo\", \"score\": -100}", + "event_id": "$bNUFCwGzWca1meCGkjp-zwslF-GfVcXukvRLI1_FaVY", + "id": 2, + "reason": "foo", + "received_ts": 1570897107409, + "room_id": "!ERAgBpSOcCCuTJqQPk:matrix.org", + "user_id": "@foo:matrix.org" + }, + { + "content": "{\"score\":-100,\"reason\":\"bar\"}", + "event_id": "$3IcdZsDaN_En-S1DF4EMCy3v4gNRKeOJs8W5qTOKj4I", + "id": 3, + "reason": "bar", + "received_ts": 1598889612059, + "room_id": "!eGvUQuTCkHGVwNMOjv:matrix.org", + "user_id": "@bar:matrix.org" + } + ], + "next_token": "2", + "total": 4 +} + +To paginate, check for ``next_token`` and if present, call the endpoint again +with from set to the value of ``next_token``. This will return a new page. + +If the endpoint does not return a ``next_token`` then there are no more +users to paginate through. + +**URL parameters:** + +- ``limit``: Is optional but is used for pagination, + denoting the maximum number of items to return in this call. Defaults to ``100``. +- ``from``: Is optional but used for pagination, + denoting the offset in the returned results. This should be treated as an opaque value and + not explicitly set to anything other than the return value of next_token from a previous call. + Defaults to ``0``. +- ``user_id``: Is optional and filters to only return users with user IDs that contain this value. +- ``room_id``: Is optional and filters to only return rooms with room IDs that contain this value. diff --git a/synapse/rest/admin/__init__.py b/synapse/rest/admin/__init__.py index 1a0faee09daa..862eb6169887 100644 --- a/synapse/rest/admin/__init__.py +++ b/synapse/rest/admin/__init__.py @@ -13,7 +13,6 @@ # WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. # See the License for the specific language governing permissions and # limitations under the License. - import logging import platform import re @@ -31,6 +30,7 @@ DeviceRestServlet, DevicesRestServlet, ) +from synapse.rest.admin.event_reports import EventReportsRestServlet from synapse.rest.admin.groups import DeleteGroupAdminRestServlet from synapse.rest.admin.media import ListMediaInRoom, register_servlets_for_media_repo from synapse.rest.admin.purge_room_servlet import PurgeRoomServlet @@ -55,7 +55,6 @@ UsersRestServletV2, WhoisRestServlet, ) -from synapse.rest.admin.event_reports import EventReportsRestServlet from synapse.util.versionstring import get_version_string logger = logging.getLogger(__name__) diff --git a/synapse/rest/admin/event_reports.py b/synapse/rest/admin/event_reports.py index 40194a98a2e8..d3361ee7d5d9 100644 --- a/synapse/rest/admin/event_reports.py +++ b/synapse/rest/admin/event_reports.py @@ -12,18 +12,10 @@ # WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. # See the License for the specific language governing permissions and # limitations under the License. - import logging -from synapse.http.servlet import ( - RestServlet, - parse_integer, - parse_string, -) -from synapse.rest.admin._base import ( - assert_requester_is_admin, - admin_patterns, -) +from synapse.http.servlet import RestServlet, parse_integer, parse_string +from synapse.rest.admin._base import admin_patterns, assert_requester_is_admin logger = logging.getLogger(__name__) From 66563b1a44eb0a55bf60dc26956da87605e6153b Mon Sep 17 00:00:00 2001 From: Dirk Klimpel <5740567+dklimpel@users.noreply.github.com> Date: Fri, 4 Sep 2020 15:06:07 +0200 Subject: [PATCH 3/9] Apply suggestions from code review Co-authored-by: Andrew Morgan <1342360+anoadragon453@users.noreply.github.com> --- docs/admin_api/event_reports.rst | 10 +++++----- synapse/rest/admin/event_reports.py | 2 +- synapse/storage/databases/main/room.py | 4 ++-- tests/rest/admin/test_event_reports.py | 8 ++++---- 4 files changed, 12 insertions(+), 12 deletions(-) diff --git a/docs/admin_api/event_reports.rst b/docs/admin_api/event_reports.rst index 8c727fc5f61b..3a69043327d9 100644 --- a/docs/admin_api/event_reports.rst +++ b/docs/admin_api/event_reports.rst @@ -1,4 +1,4 @@ -Shows reported events +Show reported events ==================== This API returns information about reported events. @@ -37,13 +37,13 @@ It returns a JSON body like the following: ], "next_token": "2", "total": 4 -} + } To paginate, check for ``next_token`` and if present, call the endpoint again -with from set to the value of ``next_token``. This will return a new page. +with ``from`` set to the value of ``next_token``. This will return a new page. If the endpoint does not return a ``next_token`` then there are no more -users to paginate through. +reports to paginate through. **URL parameters:** @@ -51,7 +51,7 @@ users to paginate through. denoting the maximum number of items to return in this call. Defaults to ``100``. - ``from``: Is optional but used for pagination, denoting the offset in the returned results. This should be treated as an opaque value and - not explicitly set to anything other than the return value of next_token from a previous call. + not explicitly set to anything other than the return value of ``next_token`` from a previous call. Defaults to ``0``. - ``user_id``: Is optional and filters to only return users with user IDs that contain this value. - ``room_id``: Is optional and filters to only return rooms with room IDs that contain this value. diff --git a/synapse/rest/admin/event_reports.py b/synapse/rest/admin/event_reports.py index d3361ee7d5d9..774f1b7c90d0 100644 --- a/synapse/rest/admin/event_reports.py +++ b/synapse/rest/admin/event_reports.py @@ -30,7 +30,7 @@ class EventReportsRestServlet(RestServlet): returns: 200 OK with list of reports if success otherwise an error. - Args + Args: The parameters `from` and `limit` are required only for pagination. By default, a `limit` of 100 is used. The parameter `user_id` can be used to filter by user id. diff --git a/synapse/storage/databases/main/room.py b/synapse/storage/databases/main/room.py index 9b6744b04118..234bcc0f5a0e 100644 --- a/synapse/storage/databases/main/room.py +++ b/synapse/storage/databases/main/room.py @@ -1323,12 +1323,12 @@ async def add_event_report( async def get_event_reports_paginate( self, start: int, limit: int, user_id: str = None, room_id: str = None ) -> Tuple[List[Dict[str, Any]], int]: - """Function to retrieve a paginated list of event reports + """Retrieve a paginated list of event reports This will return a json list of event reports and the total number of event reports matching the filter criteria. Args: - start: start number to begin the query from + start: event offset to begin the query from limit: number of rows to retrieve user_id: search for user_id. ignored if name is not None room_id: search for room_id. ignored if name is not None diff --git a/tests/rest/admin/test_event_reports.py b/tests/rest/admin/test_event_reports.py index baedfc8cbbda..7c7673c0c6ea 100644 --- a/tests/rest/admin/test_event_reports.py +++ b/tests/rest/admin/test_event_reports.py @@ -50,7 +50,7 @@ def prepare(self, reactor, clock, hs): ) self.helper.join(self.room_id2, user=self.admin_user, tok=self.admin_user_tok) - # Two rooms and two users. Every user sends and reports in every room events + # Two rooms and two users. Every user sends and reports every room event for i in range(5): self._create_event_and_report( room_id=self.room_id1, user_tok=self.other_user_tok, @@ -116,7 +116,7 @@ def test_limit(self): def test_from(self): """ - Testing list of reported events with definied starting point (from) + Testing list of reported events with a defined starting point (from) """ request, channel = self.make_request( @@ -132,7 +132,7 @@ def test_from(self): def test_limit_and_from(self): """ - Testing list of reported events with definied starting point and limit + Testing list of reported events with a defined starting point and limit """ request, channel = self.make_request( @@ -226,7 +226,7 @@ def _create_event_and_report(self, room_id, user_tok): self.assertEqual(200, int(channel.result["code"]), msg=channel.result["body"]) def _check_fields(self, content): - """ Checks that all attributes are present in result + """Checks that all attributes are present in a event report """ for c in content: self.assertIn("id", c) From 860bc4bfa497a2efeefed83a6d3bfe96b34cb36c Mon Sep 17 00:00:00 2001 From: dklimpel <5740567+dklimpel@users.noreply.github.com> Date: Mon, 7 Sep 2020 23:26:01 +0200 Subject: [PATCH 4/9] A lot of updates after code review: Tests, validations, sort parameter --- docs/admin_api/event_reports.rst | 3 + synapse/rest/__init__.py | 1 + synapse/rest/admin/event_reports.py | 31 ++++++-- synapse/storage/databases/main/room.py | 37 +++++++--- tests/rest/admin/test_event_reports.py | 98 +++++++++++++++++++++++--- 5 files changed, 147 insertions(+), 23 deletions(-) diff --git a/docs/admin_api/event_reports.rst b/docs/admin_api/event_reports.rst index 3a69043327d9..070225f5e64d 100644 --- a/docs/admin_api/event_reports.rst +++ b/docs/admin_api/event_reports.rst @@ -53,5 +53,8 @@ reports to paginate through. denoting the offset in the returned results. This should be treated as an opaque value and not explicitly set to anything other than the return value of ``next_token`` from a previous call. Defaults to ``0``. +- `dir` - Direction of event report order. Whether to fetch the most recent first (``b``) or the + oldest first (``f``). Defaults to ``b``. - ``user_id``: Is optional and filters to only return users with user IDs that contain this value. + This is the user who reported the event and wrote the reason. - ``room_id``: Is optional and filters to only return rooms with room IDs that contain this value. diff --git a/synapse/rest/__init__.py b/synapse/rest/__init__.py index 46e458e95ba0..d66b8e74b688 100644 --- a/synapse/rest/__init__.py +++ b/synapse/rest/__init__.py @@ -13,6 +13,7 @@ # WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. # See the License for the specific language governing permissions and # limitations under the License. + import synapse.rest.admin from synapse.http.server import JsonResource from synapse.rest.client import versions diff --git a/synapse/rest/admin/event_reports.py b/synapse/rest/admin/event_reports.py index 774f1b7c90d0..0d3c14f3c579 100644 --- a/synapse/rest/admin/event_reports.py +++ b/synapse/rest/admin/event_reports.py @@ -12,8 +12,10 @@ # WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. # See the License for the specific language governing permissions and # limitations under the License. + import logging +from synapse.api.errors import Codes, SynapseError from synapse.http.servlet import RestServlet, parse_integer, parse_string from synapse.rest.admin._base import admin_patterns, assert_requester_is_admin @@ -26,13 +28,14 @@ class EventReportsRestServlet(RestServlet): in a dictionary containing report information. Supports pagination. This needs user to have administrator access in Synapse. - GET /_synapse/admin/v1/event_reports?from=0&limit=10 + GET /_synapse/admin/v1/event_reports returns: 200 OK with list of reports if success otherwise an error. Args: The parameters `from` and `limit` are required only for pagination. By default, a `limit` of 100 is used. + The parameter `dir` can be used to define the order of results. The parameter `user_id` can be used to filter by user id. The parameter `room_id` can be used to filter by room id. Returns: @@ -52,11 +55,31 @@ async def on_GET(self, request): start = parse_integer(request, "from", default=0) limit = parse_integer(request, "limit", default=100) - user_id = parse_string(request, "user_id", default=None) - room_id = parse_string(request, "room_id", default=None) + direction = parse_string(request, "dir", default="b") + user_id = parse_string(request, "user_id") + room_id = parse_string(request, "room_id") + + if start < 0: + raise SynapseError( + 400, + "The start parameter must be a positive integer.", + errcode=Codes.INVALID_PARAM + ) + + if limit < 0: + raise SynapseError( + 400, + "The limit parameter must be a positive integer.", + errcode=Codes.INVALID_PARAM + ) + + if direction not in ("f", "b"): + raise SynapseError( + 400, "Unknown direction: %s" % (direction,), errcode=Codes.INVALID_PARAM + ) event_reports, total = await self.store.get_event_reports_paginate( - start, limit, user_id, room_id + start, limit, direction, user_id, room_id ) ret = {"event_reports": event_reports, "total": total} if len(event_reports) >= limit: diff --git a/synapse/storage/databases/main/room.py b/synapse/storage/databases/main/room.py index 234bcc0f5a0e..3e2571d4c002 100644 --- a/synapse/storage/databases/main/room.py +++ b/synapse/storage/databases/main/room.py @@ -1321,17 +1321,25 @@ async def add_event_report( ) async def get_event_reports_paginate( - self, start: int, limit: int, user_id: str = None, room_id: str = None + self, + start: int, + limit: int, + direction: str = "b", + user_id: str = None, + room_id: str = None, ) -> Tuple[List[Dict[str, Any]], int]: """Retrieve a paginated list of event reports - This will return a json list of event reports and the - total number of event reports matching the filter criteria. Args: start: event offset to begin the query from limit: number of rows to retrieve - user_id: search for user_id. ignored if name is not None - room_id: search for room_id. ignored if name is not None + direction: Whether to fetch the most recent first (`"b"`) or the + oldest first (`"f"`). + user_id: search for user_id. Ignored if user_id is None + room_id: search for room_id. Ignored if room_id is None + Returns: + event_reports: json list of event reports + count: total number of event reports matching the filter criteria """ def _get_event_reports_paginate_txn(txn): @@ -1345,6 +1353,11 @@ def _get_event_reports_paginate_txn(txn): filters.append("room_id LIKE ?") args.extend(["%" + room_id + "%"]) + if direction == "b": + order = "DESC" + else: + order = "ASC" + where_clause = "WHERE " + " AND ".join(filters) if len(filters) > 0 else "" sql_base = """ @@ -1353,16 +1366,20 @@ def _get_event_reports_paginate_txn(txn): """.format( where_clause ) - logger.warning("sql_base: %s ", sql_base) + sql = "SELECT COUNT(*) as total_event_reports " + sql_base txn.execute(sql, args) count = txn.fetchone()[0] - sql = ( - "SELECT id, received_ts, room_id, event_id, user_id, reason, content " - + sql_base - + " ORDER BY received_ts LIMIT ? OFFSET ?" + sql = """ + SELECT id, received_ts, room_id, event_id, user_id, reason, content + {sql_base} + ORDER BY received_ts {order} + LIMIT ? OFFSET ? + """.format( + sql_base=sql_base, order=order, ) + args += [limit, start] txn.execute(sql, args) event_reports = self.db_pool.cursor_to_dict(txn) diff --git a/tests/rest/admin/test_event_reports.py b/tests/rest/admin/test_event_reports.py index 7c7673c0c6ea..bb12fb3abd56 100644 --- a/tests/rest/admin/test_event_reports.py +++ b/tests/rest/admin/test_event_reports.py @@ -95,6 +95,7 @@ def test_default_success(self): self.assertEqual(200, int(channel.result["code"]), msg=channel.result["body"]) self.assertEqual(channel.json_body["total"], 20) + self.assertEqual(len(channel.json_body["event_reports"]), 20) self.assertNotIn("next_token", channel.json_body) self._check_fields(channel.json_body["event_reports"]) @@ -110,8 +111,8 @@ def test_limit(self): self.assertEqual(200, int(channel.result["code"]), msg=channel.result["body"]) self.assertEqual(channel.json_body["total"], 20) - self.assertEqual(channel.json_body["next_token"], "5") self.assertEqual(len(channel.json_body["event_reports"]), 5) + self.assertEqual(channel.json_body["next_token"], "5") self._check_fields(channel.json_body["event_reports"]) def test_from(self): @@ -164,8 +165,8 @@ def test_filter_room(self): self.assertNotIn("next_token", channel.json_body) self._check_fields(channel.json_body["event_reports"]) - for f in channel.json_body["event_reports"]: - self.assertEqual(f["room_id"], self.room_id1) + for report in channel.json_body["event_reports"]: + self.assertEqual(report["room_id"], self.room_id1) def test_filter_user(self): """ @@ -185,8 +186,8 @@ def test_filter_user(self): self.assertNotIn("next_token", channel.json_body) self._check_fields(channel.json_body["event_reports"]) - for f in channel.json_body["event_reports"]: - self.assertEqual(f["user_id"], self.other_user) + for report in channel.json_body["event_reports"]: + self.assertEqual(report["user_id"], self.other_user) def test_filter_user_and_room(self): """ @@ -206,12 +207,91 @@ def test_filter_user_and_room(self): self.assertNotIn("next_token", channel.json_body) self._check_fields(channel.json_body["event_reports"]) - for f in channel.json_body["event_reports"]: - self.assertEqual(f["user_id"], self.other_user) - self.assertEqual(f["room_id"], self.room_id1) + for report in channel.json_body["event_reports"]: + self.assertEqual(report["user_id"], self.other_user) + self.assertEqual(report["room_id"], self.room_id1) + + def test_valid_search_order(self): + """ + Testing search order. Order by timestamps. + """ + + # fetch the most recent first, largest timestamp + request, channel = self.make_request( + "GET", self.url + "?dir=b", 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["total"], 20) + self.assertEqual(len(channel.json_body["event_reports"]), 20) + report = 1 + while report < len(channel.json_body["event_reports"]): + self.assertGreaterEqual( + channel.json_body["event_reports"][report - 1]["received_ts"], + channel.json_body["event_reports"][report]["received_ts"], + ) + report += 1 + + # fetch the oldest first, smallest timestamp + request, channel = self.make_request( + "GET", self.url + "?dir=f", 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["total"], 20) + self.assertEqual(len(channel.json_body["event_reports"]), 20) + report = 1 + while report < len(channel.json_body["event_reports"]): + self.assertLessEqual( + channel.json_body["event_reports"][report - 1]["received_ts"], + channel.json_body["event_reports"][report]["received_ts"], + ) + report += 1 + + def test_invalid_search_order(self): + """ + Testing that a invalid search order returns a 400 + """ + + request, channel = self.make_request( + "GET", self.url + "?dir=bar", access_token=self.admin_user_tok, + ) + self.render(request) + + self.assertEqual(400, int(channel.result["code"]), msg=channel.result["body"]) + self.assertEqual(Codes.INVALID_PARAM, channel.json_body["errcode"]) + self.assertEqual("Unknown direction: bar", channel.json_body["error"]) + + def test_limit_is_negative(self): + """ + Testing that a negative list parameter returns a 400 + """ + + request, channel = self.make_request( + "GET", self.url + "?limit=-5", access_token=self.admin_user_tok, + ) + self.render(request) + + self.assertEqual(400, int(channel.result["code"]), msg=channel.result["body"]) + self.assertEqual(Codes.INVALID_PARAM, channel.json_body["errcode"]) + + def test_from_is_negative(self): + """ + Testing that a negative from parameter returns a 400 + """ + + request, channel = self.make_request( + "GET", self.url + "?from=-5", access_token=self.admin_user_tok, + ) + self.render(request) + + self.assertEqual(400, int(channel.result["code"]), msg=channel.result["body"]) + self.assertEqual(Codes.INVALID_PARAM, channel.json_body["errcode"]) def _create_event_and_report(self, room_id, user_tok): - """ Create and report events + """Create and report events """ resp = self.helper.send(room_id, tok=user_tok) event_id = resp["event_id"] From 610bed34c4dcf80418cff8c2b55833b2714bfeeb Mon Sep 17 00:00:00 2001 From: dklimpel <5740567+dklimpel@users.noreply.github.com> Date: Mon, 7 Sep 2020 23:30:57 +0200 Subject: [PATCH 5/9] small typos --- docs/admin_api/event_reports.rst | 2 +- synapse/rest/__init__.py | 1 - synapse/rest/admin/__init__.py | 1 + synapse/rest/admin/event_reports.py | 4 ++-- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/docs/admin_api/event_reports.rst b/docs/admin_api/event_reports.rst index 070225f5e64d..d88646336247 100644 --- a/docs/admin_api/event_reports.rst +++ b/docs/admin_api/event_reports.rst @@ -53,7 +53,7 @@ reports to paginate through. denoting the offset in the returned results. This should be treated as an opaque value and not explicitly set to anything other than the return value of ``next_token`` from a previous call. Defaults to ``0``. -- `dir` - Direction of event report order. Whether to fetch the most recent first (``b``) or the +- ``dir`` - Direction of event report order. Whether to fetch the most recent first (``b``) or the oldest first (``f``). Defaults to ``b``. - ``user_id``: Is optional and filters to only return users with user IDs that contain this value. This is the user who reported the event and wrote the reason. diff --git a/synapse/rest/__init__.py b/synapse/rest/__init__.py index d66b8e74b688..46e458e95ba0 100644 --- a/synapse/rest/__init__.py +++ b/synapse/rest/__init__.py @@ -13,7 +13,6 @@ # WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. # See the License for the specific language governing permissions and # limitations under the License. - import synapse.rest.admin from synapse.http.server import JsonResource from synapse.rest.client import versions diff --git a/synapse/rest/admin/__init__.py b/synapse/rest/admin/__init__.py index 862eb6169887..2f39386ee5fd 100644 --- a/synapse/rest/admin/__init__.py +++ b/synapse/rest/admin/__init__.py @@ -13,6 +13,7 @@ # WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. # See the License for the specific language governing permissions and # limitations under the License. + import logging import platform import re diff --git a/synapse/rest/admin/event_reports.py b/synapse/rest/admin/event_reports.py index 0d3c14f3c579..c3651ec3dd26 100644 --- a/synapse/rest/admin/event_reports.py +++ b/synapse/rest/admin/event_reports.py @@ -63,14 +63,14 @@ async def on_GET(self, request): raise SynapseError( 400, "The start parameter must be a positive integer.", - errcode=Codes.INVALID_PARAM + errcode=Codes.INVALID_PARAM, ) if limit < 0: raise SynapseError( 400, "The limit parameter must be a positive integer.", - errcode=Codes.INVALID_PARAM + errcode=Codes.INVALID_PARAM, ) if direction not in ("f", "b"): From 503539e413b2083d1ef5b05404d4ac7362c8a303 Mon Sep 17 00:00:00 2001 From: dklimpel <5740567+dklimpel@users.noreply.github.com> Date: Sat, 12 Sep 2020 22:10:39 +0200 Subject: [PATCH 6/9] Add the original event to the JSON response body --- docs/admin_api/event_reports.rst | 66 +++++++++++++++++++++++++- synapse/storage/databases/main/room.py | 48 ++++++++++++++----- tests/rest/admin/test_event_reports.py | 10 ++++ 3 files changed, 111 insertions(+), 13 deletions(-) diff --git a/docs/admin_api/event_reports.rst b/docs/admin_api/event_reports.rst index d88646336247..33e99fb8e763 100644 --- a/docs/admin_api/event_reports.rst +++ b/docs/admin_api/event_reports.rst @@ -17,21 +17,67 @@ It returns a JSON body like the following: { "event_reports": [ { - "content": "{\"reason\": \"foo\", \"score\": -100}", + "content": { + "reason": "foo", + "score": -100 + }, "event_id": "$bNUFCwGzWca1meCGkjp-zwslF-GfVcXukvRLI1_FaVY", + "event_json": { + "auth_events": [ + "$YK4arsKKcc0LRoe700pS8DSjOvUT4NDv0HfInlMFw2M", + "$oggsNXxzPFRE3y53SUNd7nsj69-QzKv03a1RucHu-ws" + ], + "content": { + "body": "matrix.org: This Week in Matrix", + "format": "org.matrix.custom.html", + "formatted_body": "matrix.org:
This Week in Matrix", + "msgtype": "m.notice" + }, + "depth": 546, + "hashes": { + "sha256": "xK1//xnmvHJIOvbgXlkI8eEqdvoMmihVDJ9J4SNlsAw" + }, + "origin": "matrix.org", + "origin_server_ts": 1592291711430, + "prev_events": [ + "$YK4arsKKcc0LRoe700pS8DSjOvUT4NDv0HfInlMFw2M" + ], + "prev_state": [], + "room_id": "!ERAgBpSOcCCuTJqQPk:matrix.org", + "sender": "@foobar:matrix.org", + "signatures": { + "matrix.org": { + "ed25519:a_JaEG": "cs+OUKW/iHx5pEidbWxh0UiNNHwe46Ai9LwNz+Ah16aWDNszVIe2gaAcVZfvNsBhakQTew51tlKmL2kspXk/Dg" + } + }, + "type": "m.room.message", + "unsigned": { + "age_ts": 1592291711430, + } + }, "id": 2, "reason": "foo", "received_ts": 1570897107409, + "room_alias": "#alias1:matrix.org", "room_id": "!ERAgBpSOcCCuTJqQPk:matrix.org", + "sender": "@foobar:matrix.org", "user_id": "@foo:matrix.org" }, { - "content": "{\"score\":-100,\"reason\":\"bar\"}", + "content": { + "reason": "bar", + "score": -100 + }, "event_id": "$3IcdZsDaN_En-S1DF4EMCy3v4gNRKeOJs8W5qTOKj4I", + "event_json": { + "_comment": "... (hidden items) ..." + }, "id": 3, "reason": "bar", "received_ts": 1598889612059, + "room_alias": "#alias2:matrix.org", "room_id": "!eGvUQuTCkHGVwNMOjv:matrix.org", + "sender": "@foobar:matrix.org", "user_id": "@bar:matrix.org" } ], @@ -58,3 +104,19 @@ reports to paginate through. - ``user_id``: Is optional and filters to only return users with user IDs that contain this value. This is the user who reported the event and wrote the reason. - ``room_id``: Is optional and filters to only return rooms with room IDs that contain this value. + +**Response** + +The following fields are returned in the JSON response body: + +- ``id``: Id of event report. +- ``received_ts``: The timestamp (in milliseconds since the unix epoch) when this report was sent. +- ``room_id``: The ID of the room. +- ``event_id``: The ID of the reported event. +- ``user_id``: This is the user who reported the event and wrote the reason. +- ``reason``: Comment made by the ``user_id`` in this report. +- ``content``: Content of reported event. +- ``sender``: This is the ID of the user who sent the original message/event that was reported. +- ``room_alias``: The alias of the room. +- ``event_json``: Details of the original event that was reported. + diff --git a/synapse/storage/databases/main/room.py b/synapse/storage/databases/main/room.py index 3e2571d4c002..5115032af386 100644 --- a/synapse/storage/databases/main/room.py +++ b/synapse/storage/databases/main/room.py @@ -1347,10 +1347,10 @@ def _get_event_reports_paginate_txn(txn): args = [] if user_id: - filters.append("user_id LIKE ?") + filters.append("er.user_id LIKE ?") args.extend(["%" + user_id + "%"]) if room_id: - filters.append("room_id LIKE ?") + filters.append("er.room_id LIKE ?") args.extend(["%" + room_id + "%"]) if direction == "b": @@ -1360,29 +1360,55 @@ def _get_event_reports_paginate_txn(txn): where_clause = "WHERE " + " AND ".join(filters) if len(filters) > 0 else "" - sql_base = """ - FROM event_reports + sql = """ + SELECT COUNT(*) as total_event_reports + FROM event_reports AS er {} """.format( where_clause ) - - sql = "SELECT COUNT(*) as total_event_reports " + sql_base txn.execute(sql, args) count = txn.fetchone()[0] sql = """ - SELECT id, received_ts, room_id, event_id, user_id, reason, content - {sql_base} - ORDER BY received_ts {order} - LIMIT ? OFFSET ? + SELECT + er.id, + er.received_ts, + er.room_id, + er.event_id, + er.user_id, + er.reason, + er.content, + events.sender, + room_aliases.room_alias, + event_json.json AS event_json + FROM event_reports AS er + LEFT JOIN room_aliases + ON room_aliases.room_id = er.room_id + JOIN events + ON events.event_id = er.event_id + JOIN event_json + ON event_json.event_id = er.event_id + {where_clause} + ORDER BY er.received_ts {order} + LIMIT ? + OFFSET ? """.format( - sql_base=sql_base, order=order, + where_clause=where_clause, order=order, ) args += [limit, start] txn.execute(sql, args) event_reports = self.db_pool.cursor_to_dict(txn) + + if count > 0: + for row in event_reports: + try: + row["content"] = db_to_json(row["content"]) + row["event_json"] = db_to_json(row["event_json"]) + except Exception: + continue + return event_reports, count return await self.db_pool.runInteraction( diff --git a/tests/rest/admin/test_event_reports.py b/tests/rest/admin/test_event_reports.py index bb12fb3abd56..3250cd113d36 100644 --- a/tests/rest/admin/test_event_reports.py +++ b/tests/rest/admin/test_event_reports.py @@ -316,3 +316,13 @@ def _check_fields(self, content): self.assertIn("user_id", c) self.assertIn("reason", c) self.assertIn("content", c) + self.assertIn("sender", c) + self.assertIn("room_alias", c) + self.assertIn("event_json", c) + self.assertIn("score", c["content"]) + self.assertIn("reason", c["content"]) + self.assertIn("auth_events", c["event_json"]) + self.assertIn("type", c["event_json"]) + self.assertIn("room_id", c["event_json"]) + self.assertIn("sender", c["event_json"]) + self.assertIn("content", c["event_json"]) From 286c84f96d4982d228b39c6f318df72f6bb641d7 Mon Sep 17 00:00:00 2001 From: dklimpel <5740567+dklimpel@users.noreply.github.com> Date: Sat, 19 Sep 2020 16:23:35 +0200 Subject: [PATCH 7/9] Update docs, fix `next_token`, add test --- docs/admin_api/event_reports.rst | 43 +++++++++++-------- synapse/rest/admin/event_reports.py | 6 +-- synapse/storage/databases/main/room.py | 6 +-- tests/rest/admin/test_event_reports.py | 58 +++++++++++++++++++++++++- 4 files changed, 87 insertions(+), 26 deletions(-) diff --git a/docs/admin_api/event_reports.rst b/docs/admin_api/event_reports.rst index 33e99fb8e763..461be012300e 100644 --- a/docs/admin_api/event_reports.rst +++ b/docs/admin_api/event_reports.rst @@ -12,7 +12,7 @@ server admin: see `README.rst `_. It returns a JSON body like the following: -.. code:: json +.. code:: jsonc { "event_reports": [ @@ -70,7 +70,8 @@ It returns a JSON body like the following: }, "event_id": "$3IcdZsDaN_En-S1DF4EMCy3v4gNRKeOJs8W5qTOKj4I", "event_json": { - "_comment": "... (hidden items) ..." + // hidden items + // see above }, "id": 3, "reason": "bar", @@ -81,7 +82,7 @@ It returns a JSON body like the following: "user_id": "@bar:matrix.org" } ], - "next_token": "2", + "next_token": 2, "total": 4 } @@ -93,30 +94,36 @@ reports to paginate through. **URL parameters:** -- ``limit``: Is optional but is used for pagination, +- ``limit``: integer - Is optional but is used for pagination, denoting the maximum number of items to return in this call. Defaults to ``100``. -- ``from``: Is optional but used for pagination, +- ``from``: integer - Is optional but used for pagination, denoting the offset in the returned results. This should be treated as an opaque value and not explicitly set to anything other than the return value of ``next_token`` from a previous call. Defaults to ``0``. -- ``dir`` - Direction of event report order. Whether to fetch the most recent first (``b``) or the +- ``dir``: string - Direction of event report order. Whether to fetch the most recent first (``b``) or the oldest first (``f``). Defaults to ``b``. -- ``user_id``: Is optional and filters to only return users with user IDs that contain this value. +- ``user_id``: string - Is optional and filters to only return users with user IDs that contain this value. This is the user who reported the event and wrote the reason. -- ``room_id``: Is optional and filters to only return rooms with room IDs that contain this value. +- ``room_id``: string - Is optional and filters to only return rooms with room IDs that contain this value. **Response** The following fields are returned in the JSON response body: -- ``id``: Id of event report. -- ``received_ts``: The timestamp (in milliseconds since the unix epoch) when this report was sent. -- ``room_id``: The ID of the room. -- ``event_id``: The ID of the reported event. -- ``user_id``: This is the user who reported the event and wrote the reason. -- ``reason``: Comment made by the ``user_id`` in this report. -- ``content``: Content of reported event. -- ``sender``: This is the ID of the user who sent the original message/event that was reported. -- ``room_alias``: The alias of the room. -- ``event_json``: Details of the original event that was reported. +- ``id``: integer - ID of event report. +- ``received_ts``: integer - The timestamp (in milliseconds since the unix epoch) when this report was sent. +- ``room_id``: string - The ID of the room in which the event being reported is located. +- ``event_id``: string - The ID of the reported event. +- ``user_id``: string - This is the user who reported the event and wrote the reason. +- ``reason``: string - Comment made by the ``user_id`` in this report. May be blank. +- ``content``: object - Content of reported event. + + - ``reason``: string - Comment made by the ``user_id`` in this report. May be blank. + - ``score``: integer - Content is reported based upon a negative score, where -100 is "most offensive" and 0 is "inoffensive". + +- ``sender``: string - This is the ID of the user who sent the original message/event that was reported. +- ``room_alias``: string - The alias of the room. ``null`` if the room does not have a canonical alias set. +- ``event_json``: object - Details of the original event that was reported. +- ``next_token``: integer - Indication for pagination. See above. +- ``total``: integer - Total number of event reports related to the query (``user_id`` and ``room_id``). diff --git a/synapse/rest/admin/event_reports.py b/synapse/rest/admin/event_reports.py index c3651ec3dd26..5b8d0594cddc 100644 --- a/synapse/rest/admin/event_reports.py +++ b/synapse/rest/admin/event_reports.py @@ -26,7 +26,7 @@ class EventReportsRestServlet(RestServlet): """ List all reported events that are known to the homeserver. Results are returned in a dictionary containing report information. Supports pagination. - This needs user to have administrator access in Synapse. + The requester must have administrator access in Synapse. GET /_synapse/admin/v1/event_reports returns: @@ -82,7 +82,7 @@ async def on_GET(self, request): start, limit, direction, user_id, room_id ) ret = {"event_reports": event_reports, "total": total} - if len(event_reports) >= limit: - ret["next_token"] = str(start + len(event_reports)) + if (start + limit) < total: + ret["next_token"] = start + len(event_reports) return 200, ret diff --git a/synapse/storage/databases/main/room.py b/synapse/storage/databases/main/room.py index 5115032af386..c57d1043b164 100644 --- a/synapse/storage/databases/main/room.py +++ b/synapse/storage/databases/main/room.py @@ -1325,8 +1325,8 @@ async def get_event_reports_paginate( start: int, limit: int, direction: str = "b", - user_id: str = None, - room_id: str = None, + user_id: Optional[str] = None, + room_id: Optional[str] = None, ) -> Tuple[List[Dict[str, Any]], int]: """Retrieve a paginated list of event reports @@ -1334,7 +1334,7 @@ async def get_event_reports_paginate( start: event offset to begin the query from limit: number of rows to retrieve direction: Whether to fetch the most recent first (`"b"`) or the - oldest first (`"f"`). + oldest first (`"f"`) user_id: search for user_id. Ignored if user_id is None room_id: search for room_id. Ignored if room_id is None Returns: diff --git a/tests/rest/admin/test_event_reports.py b/tests/rest/admin/test_event_reports.py index 3250cd113d36..17c8dfd0307b 100644 --- a/tests/rest/admin/test_event_reports.py +++ b/tests/rest/admin/test_event_reports.py @@ -112,7 +112,7 @@ def test_limit(self): self.assertEqual(200, int(channel.result["code"]), msg=channel.result["body"]) self.assertEqual(channel.json_body["total"], 20) self.assertEqual(len(channel.json_body["event_reports"]), 5) - self.assertEqual(channel.json_body["next_token"], "5") + self.assertEqual(channel.json_body["next_token"], 5) self._check_fields(channel.json_body["event_reports"]) def test_from(self): @@ -143,7 +143,7 @@ def test_limit_and_from(self): self.assertEqual(200, int(channel.result["code"]), msg=channel.result["body"]) self.assertEqual(channel.json_body["total"], 20) - self.assertEqual(channel.json_body["next_token"], "15") + self.assertEqual(channel.json_body["next_token"], 15) self.assertEqual(len(channel.json_body["event_reports"]), 10) self._check_fields(channel.json_body["event_reports"]) @@ -290,6 +290,60 @@ def test_from_is_negative(self): self.assertEqual(400, int(channel.result["code"]), msg=channel.result["body"]) self.assertEqual(Codes.INVALID_PARAM, channel.json_body["errcode"]) + def test_next_token(self): + """ + Testing that ``next_token`` appears at the right place + """ + + # ``next_token`` does not appears + # Number of results is number of entries + request, channel = self.make_request( + "GET", self.url + "?limit=20", 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["total"], 20) + self.assertEqual(len(channel.json_body["event_reports"]), 20) + self.assertNotIn("next_token", channel.json_body) + + # ``next_token`` does not appears + # Number of max results is larger than number of entries + request, channel = self.make_request( + "GET", self.url + "?limit=21", 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["total"], 20) + self.assertEqual(len(channel.json_body["event_reports"]), 20) + self.assertNotIn("next_token", channel.json_body) + + # ``next_token`` does appears + # Number of max results is smaller than number of entries + request, channel = self.make_request( + "GET", self.url + "?limit=19", 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["total"], 20) + self.assertEqual(len(channel.json_body["event_reports"]), 19) + self.assertEqual(channel.json_body["next_token"], 19) + + # Check + # Set ``from`` to value of ``next_token`` for request remaining entries + # ``next_token`` does not appears + request, channel = self.make_request( + "GET", self.url + "?from=19", 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["total"], 20) + self.assertEqual(len(channel.json_body["event_reports"]), 1) + self.assertNotIn("next_token", channel.json_body) + def _create_event_and_report(self, room_id, user_tok): """Create and report events """ From 72cdbc1c70205599124c3c10bf486f53b39a7b86 Mon Sep 17 00:00:00 2001 From: Dirk Klimpel <5740567+dklimpel@users.noreply.github.com> Date: Tue, 22 Sep 2020 08:29:07 +0200 Subject: [PATCH 8/9] Changed `` to ` --- tests/rest/admin/test_event_reports.py | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/tests/rest/admin/test_event_reports.py b/tests/rest/admin/test_event_reports.py index 17c8dfd0307b..f6d9a65d44fa 100644 --- a/tests/rest/admin/test_event_reports.py +++ b/tests/rest/admin/test_event_reports.py @@ -292,10 +292,10 @@ def test_from_is_negative(self): def test_next_token(self): """ - Testing that ``next_token`` appears at the right place + Testing that `next_token` appears at the right place """ - # ``next_token`` does not appears + # `next_token` does not appears # Number of results is number of entries request, channel = self.make_request( "GET", self.url + "?limit=20", access_token=self.admin_user_tok, @@ -307,7 +307,7 @@ def test_next_token(self): self.assertEqual(len(channel.json_body["event_reports"]), 20) self.assertNotIn("next_token", channel.json_body) - # ``next_token`` does not appears + # `next_token` does not appears # Number of max results is larger than number of entries request, channel = self.make_request( "GET", self.url + "?limit=21", access_token=self.admin_user_tok, @@ -319,7 +319,7 @@ def test_next_token(self): self.assertEqual(len(channel.json_body["event_reports"]), 20) self.assertNotIn("next_token", channel.json_body) - # ``next_token`` does appears + # `next_token` does appears # Number of max results is smaller than number of entries request, channel = self.make_request( "GET", self.url + "?limit=19", access_token=self.admin_user_tok, @@ -332,8 +332,8 @@ def test_next_token(self): self.assertEqual(channel.json_body["next_token"], 19) # Check - # Set ``from`` to value of ``next_token`` for request remaining entries - # ``next_token`` does not appears + # Set `from` to value of `next_token` for request remaining entries + # `next_token` does not appears request, channel = self.make_request( "GET", self.url + "?from=19", access_token=self.admin_user_tok, ) From d60c707a7b58dbf4eeadd1dc563e0b630dbdc702 Mon Sep 17 00:00:00 2001 From: Dirk Klimpel <5740567+dklimpel@users.noreply.github.com> Date: Tue, 22 Sep 2020 08:38:05 +0200 Subject: [PATCH 9/9] Small typo in spelling --- tests/rest/admin/test_event_reports.py | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/tests/rest/admin/test_event_reports.py b/tests/rest/admin/test_event_reports.py index f6d9a65d44fa..bf79086f7813 100644 --- a/tests/rest/admin/test_event_reports.py +++ b/tests/rest/admin/test_event_reports.py @@ -295,8 +295,8 @@ def test_next_token(self): Testing that `next_token` appears at the right place """ - # `next_token` does not appears - # Number of results is number of entries + # `next_token` does not appear + # Number of results is the number of entries request, channel = self.make_request( "GET", self.url + "?limit=20", access_token=self.admin_user_tok, ) @@ -307,8 +307,8 @@ def test_next_token(self): self.assertEqual(len(channel.json_body["event_reports"]), 20) self.assertNotIn("next_token", channel.json_body) - # `next_token` does not appears - # Number of max results is larger than number of entries + # `next_token` does not appear + # Number of max results is larger than the number of entries request, channel = self.make_request( "GET", self.url + "?limit=21", access_token=self.admin_user_tok, ) @@ -319,8 +319,8 @@ def test_next_token(self): self.assertEqual(len(channel.json_body["event_reports"]), 20) self.assertNotIn("next_token", channel.json_body) - # `next_token` does appears - # Number of max results is smaller than number of entries + # `next_token` does appear + # Number of max results is smaller than the number of entries request, channel = self.make_request( "GET", self.url + "?limit=19", access_token=self.admin_user_tok, ) @@ -333,7 +333,7 @@ def test_next_token(self): # Check # Set `from` to value of `next_token` for request remaining entries - # `next_token` does not appears + # `next_token` does not appear request, channel = self.make_request( "GET", self.url + "?from=19", access_token=self.admin_user_tok, )