Split admin API for reported events into a detail and a list view (#8539)

Split admin API for reported events in detail und list view. API was introduced with #8217 in synapse v.1.21.0. It makes the list (`GET /_synapse/admin/v1/event_reports`) less complex and provides a better overview. The details can be queried with: `GET /_synapse/admin/v1/event_reports/<report_id>`. It is similar to room and users API. It is a kind of regression in `GET /_synapse/admin/v1/event_reports`. `event_json` was removed. But the api was introduced one version before and it is an admin API (not under spec). Signed-off-by: Dirk Klimpel dirk@klimpel.org
2020-10-26 19:16:37 +01:00 · 2020-10-26 19:16:37 +01:00 · 66e6801c3e
commit 66e6801c3e
parent 6c9ab61df5
6 changed files with 410 additions and 85 deletions
--- a/changelog.d/8539.feature
+++ b/changelog.d/8539.feature
@ -0,0 +1 @@
+Split admin API for reported events (`GET /_synapse/admin/v1/event_reports`) into detail and list endpoints. This is a breaking change to #8217 which was introduced in Synapse v1.21.0. Those who already use this API should check their scripts. Contributed by @dklimpel.
--- a/docs/admin_api/event_reports.rst
+++ b/docs/admin_api/event_reports.rst
@ -17,67 +17,26 @@ It returns a JSON body like the following:
    {
        "event_reports": [
            {
-                "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": "<strong>matrix.org</strong>:<br><a href=\"https://matrix.org/blog/\"><strong>This Week in Matrix</strong></a>",
-                        "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",
+                "score": -100,
                "received_ts": 1570897107409,
-                "room_alias": "#alias1:matrix.org",
+                "canonical_alias": "#alias1:matrix.org",
                "room_id": "!ERAgBpSOcCCuTJqQPk:matrix.org",
+                "name": "Matrix HQ",
                "sender": "@foobar:matrix.org",
                "user_id": "@foo:matrix.org"
            },
            {
-                "content": {
-                    "reason": "bar",
-                    "score": -100
-                },
                "event_id": "$3IcdZsDaN_En-S1DF4EMCy3v4gNRKeOJs8W5qTOKj4I",
-                "event_json": {
-                    // hidden items
-                    // see above
-                },
                "id": 3,
                "reason": "bar",
+                "score": -100,
                "received_ts": 1598889612059,
-                "room_alias": "#alias2:matrix.org",
+                "canonical_alias": "#alias2:matrix.org",
                "room_id": "!eGvUQuTCkHGVwNMOjv:matrix.org",
+                "name": "Your room name here",
                "sender": "@foobar:matrix.org",
                "user_id": "@bar:matrix.org"
            }
@ -113,17 +72,94 @@ The following fields are returned in the JSON response body:
 - ``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.
+- ``name``: string - The name of the room.
 - ``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".
-
+- ``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.
+- ``canonical_alias``: string - The canonical alias of the room. ``null`` if the room does not have a canonical alias set.
 - ``next_token``: integer - Indication for pagination. See above.
 - ``total``: integer - Total number of event reports related to the query (``user_id`` and ``room_id``).

+Show details of a specific event report
+=======================================
+
+This API returns information about a specific event report.
+
+The api is::
+
+    GET /_synapse/admin/v1/event_reports/<report_id>
+
+To use it, you will need to authenticate by providing an ``access_token`` for a
+server admin: see `README.rst <README.rst>`_.
+
+It returns a JSON body like the following:
+
+.. code:: jsonc
+
+    {
+        "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": "<strong>matrix.org</strong>:<br><a href=\"https://matrix.org/blog/\"><strong>This Week in Matrix</strong></a>",
+                "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": <report_id>,
+        "reason": "foo",
+        "score": -100,
+        "received_ts": 1570897107409,
+        "canonical_alias": "#alias1:matrix.org",
+        "room_id": "!ERAgBpSOcCCuTJqQPk:matrix.org",
+        "name": "Matrix HQ",
+        "sender": "@foobar:matrix.org",
+        "user_id": "@foo:matrix.org"
+    }
+
+**URL parameters:**
+
+- ``report_id``: string - The ID of the event report.
+
+**Response**
+
+The following fields are returned in the JSON response body:
+
+- ``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.
+- ``name``: string - The name of the room.
+- ``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.
+- ``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.
+- ``canonical_alias``: string - The canonical 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.
--- a/synapse/rest/admin/init.py
+++ b/synapse/rest/admin/init.py
@ -31,7 +31,10 @@ from synapse.rest.admin.devices import (
    DeviceRestServlet,
    DevicesRestServlet,
 )
-from synapse.rest.admin.event_reports import EventReportsRestServlet
+from synapse.rest.admin.event_reports import (
+    EventReportDetailRestServlet,
+    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
@ -222,6 +225,7 @@ def register_servlets(hs, http_server):
    DevicesRestServlet(hs).register(http_server)
    DeleteDevicesRestServlet(hs).register(http_server)
    EventReportsRestServlet(hs).register(http_server)
+    EventReportDetailRestServlet(hs).register(http_server)


 def register_servlets_for_client_rest_resource(hs, http_server):
--- a/synapse/rest/admin/event_reports.py
+++ b/synapse/rest/admin/event_reports.py
@ -15,7 +15,7 @@

 import logging

-from synapse.api.errors import Codes, SynapseError
+from synapse.api.errors import Codes, NotFoundError, SynapseError
 from synapse.http.servlet import RestServlet, parse_integer, parse_string
 from synapse.rest.admin._base import admin_patterns, assert_requester_is_admin

@ -86,3 +86,47 @@ class EventReportsRestServlet(RestServlet):
            ret["next_token"] = start + len(event_reports)

        return 200, ret
+
+
+class EventReportDetailRestServlet(RestServlet):
+    """
+    Get a specific reported event that is known to the homeserver. Results are returned
+    in a dictionary containing report information.
+    The requester must have administrator access in Synapse.
+
+    GET /_synapse/admin/v1/event_reports/<report_id>
+    returns:
+        200 OK with details report if success otherwise an error.
+
+    Args:
+        The parameter `report_id` is the ID of the event report in the database.
+    Returns:
+        JSON blob of information about the event report
+    """
+
+    PATTERNS = admin_patterns("/event_reports/(?P<report_id>[^/]*)$")
+
+    def __init__(self, hs):
+        self.hs = hs
+        self.auth = hs.get_auth()
+        self.store = hs.get_datastore()
+
+    async def on_GET(self, request, report_id):
+        await assert_requester_is_admin(self.auth, request)
+
+        message = (
+            "The report_id parameter must be a string representing a positive integer."
+        )
+        try:
+            report_id = int(report_id)
+        except ValueError:
+            raise SynapseError(400, message, errcode=Codes.INVALID_PARAM)
+
+        if report_id < 0:
+            raise SynapseError(400, message, errcode=Codes.INVALID_PARAM)
+
+        ret = await self.store.get_event_report(report_id)
+        if not ret:
+            raise NotFoundError("Event report not found")
+
+        return 200, ret
--- a/synapse/storage/databases/main/room.py
+++ b/synapse/storage/databases/main/room.py
@ -1411,6 +1411,65 @@ class RoomStore(RoomBackgroundUpdateStore, RoomWorkerStore, SearchStore):
            desc="add_event_report",
        )

+    async def get_event_report(self, report_id: int) -> Optional[Dict[str, Any]]:
+        """Retrieve an event report
+
+        Args:
+            report_id: ID of reported event in database
+        Returns:
+            event_report: json list of information from event report
+        """
+
+        def _get_event_report_txn(txn, report_id):
+
+            sql = """
+                SELECT
+                    er.id,
+                    er.received_ts,
+                    er.room_id,
+                    er.event_id,
+                    er.user_id,
+                    er.content,
+                    events.sender,
+                    room_stats_state.canonical_alias,
+                    room_stats_state.name,
+                    event_json.json AS event_json
+                FROM event_reports AS er
+                LEFT JOIN events
+                    ON events.event_id = er.event_id
+                JOIN event_json
+                    ON event_json.event_id = er.event_id
+                JOIN room_stats_state
+                    ON room_stats_state.room_id = er.room_id
+                WHERE er.id = ?
+            """
+
+            txn.execute(sql, [report_id])
+            row = txn.fetchone()
+
+            if not row:
+                return None
+
+            event_report = {
+                "id": row[0],
+                "received_ts": row[1],
+                "room_id": row[2],
+                "event_id": row[3],
+                "user_id": row[4],
+                "score": db_to_json(row[5]).get("score"),
+                "reason": db_to_json(row[5]).get("reason"),
+                "sender": row[6],
+                "canonical_alias": row[7],
+                "name": row[8],
+                "event_json": db_to_json(row[9]),
+            }
+
+            return event_report
+
+        return await self.db_pool.runInteraction(
+            "get_event_report", _get_event_report_txn, report_id
+        )
+
    async def get_event_reports_paginate(
        self,
        start: int,
@ -1468,18 +1527,15 @@ class RoomStore(RoomBackgroundUpdateStore, RoomWorkerStore, SearchStore):
                    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
+                    room_stats_state.canonical_alias,
+                    room_stats_state.name
                FROM event_reports AS er
-                LEFT JOIN room_aliases
-                    ON room_aliases.room_id = er.room_id
-                JOIN events
+                LEFT JOIN events
                    ON events.event_id = er.event_id
-                JOIN event_json
-                    ON event_json.event_id = er.event_id
+                JOIN room_stats_state
+                    ON room_stats_state.room_id = er.room_id
                {where_clause}
                ORDER BY er.received_ts {order}
                LIMIT ?
@ -1490,15 +1546,29 @@ class RoomStore(RoomBackgroundUpdateStore, RoomWorkerStore, SearchStore):

            args += [limit, start]
            txn.execute(sql, args)
-            event_reports = self.db_pool.cursor_to_dict(txn)

-            if count > 0:
-                for row in event_reports:
+            event_reports = []
+            for row in txn:
                try:
-                        row["content"] = db_to_json(row["content"])
-                        row["event_json"] = db_to_json(row["event_json"])
+                    s = db_to_json(row[5]).get("score")
+                    r = db_to_json(row[5]).get("reason")
                except Exception:
+                    logger.error("Unable to parse json from event_reports: %s", row[0])
                    continue
+                event_reports.append(
+                    {
+                        "id": row[0],
+                        "received_ts": row[1],
+                        "room_id": row[2],
+                        "event_id": row[3],
+                        "user_id": row[4],
+                        "score": s,
+                        "reason": r,
+                        "sender": row[6],
+                        "canonical_alias": row[7],
+                        "name": row[8],
+                    }
+                )

            return event_reports, count

--- a/tests/rest/admin/test_event_reports.py
+++ b/tests/rest/admin/test_event_reports.py
@ -70,6 +70,16 @@ class EventReportsTestCase(unittest.HomeserverTestCase):

        self.url = "/_synapse/admin/v1/event_reports"

+    def test_no_auth(self):
+        """
+        Try to get an event report without authentication.
+        """
+        request, channel = self.make_request("GET", self.url, b"{}")
+        self.render(request)
+
+        self.assertEqual(401, int(channel.result["code"]), msg=channel.result["body"])
+        self.assertEqual(Codes.MISSING_TOKEN, channel.json_body["errcode"])
+
    def test_requester_is_no_admin(self):
        """
        If the user is not a server admin, an error 403 is returned.
@ -266,7 +276,7 @@ class EventReportsTestCase(unittest.HomeserverTestCase):

    def test_limit_is_negative(self):
        """
-        Testing that a negative list parameter returns a 400
+        Testing that a negative limit parameter returns a 400
        """

        request, channel = self.make_request(
@ -360,7 +370,7 @@ class EventReportsTestCase(unittest.HomeserverTestCase):
        self.assertEqual(200, int(channel.result["code"]), msg=channel.result["body"])

    def _check_fields(self, content):
-        """Checks that all attributes are present in a event report
+        """Checks that all attributes are present in an event report
        """
        for c in content:
            self.assertIn("id", c)
@ -368,15 +378,175 @@ class EventReportsTestCase(unittest.HomeserverTestCase):
            self.assertIn("room_id", c)
            self.assertIn("event_id", c)
            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"])
+            self.assertIn("canonical_alias", c)
+            self.assertIn("name", c)
+            self.assertIn("score", c)
+            self.assertIn("reason", c)
+
+
+class EventReportDetailTestCase(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._create_event_and_report(
+            room_id=self.room_id1, user_tok=self.other_user_tok,
+        )
+
+        # first created event report gets `id`=2
+        self.url = "/_synapse/admin/v1/event_reports/2"
+
+    def test_no_auth(self):
+        """
+        Try to get event report without authentication.
+        """
+        request, channel = self.make_request("GET", self.url, b"{}")
+        self.render(request)
+
+        self.assertEqual(401, int(channel.result["code"]), msg=channel.result["body"])
+        self.assertEqual(Codes.MISSING_TOKEN, channel.json_body["errcode"])
+
+    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 get a reported event
+        """
+
+        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._check_fields(channel.json_body)
+
+    def test_invalid_report_id(self):
+        """
+        Testing that an invalid `report_id` returns a 400.
+        """
+
+        # `report_id` is negative
+        request, channel = self.make_request(
+            "GET",
+            "/_synapse/admin/v1/event_reports/-123",
+            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(
+            "The report_id parameter must be a string representing a positive integer.",
+            channel.json_body["error"],
+        )
+
+        # `report_id` is a non-numerical string
+        request, channel = self.make_request(
+            "GET",
+            "/_synapse/admin/v1/event_reports/abcdef",
+            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(
+            "The report_id parameter must be a string representing a positive integer.",
+            channel.json_body["error"],
+        )
+
+        # `report_id` is undefined
+        request, channel = self.make_request(
+            "GET",
+            "/_synapse/admin/v1/event_reports/",
+            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(
+            "The report_id parameter must be a string representing a positive integer.",
+            channel.json_body["error"],
+        )
+
+    def test_report_id_not_found(self):
+        """
+        Testing that a not existing `report_id` returns a 404.
+        """
+
+        request, channel = self.make_request(
+            "GET",
+            "/_synapse/admin/v1/event_reports/123",
+            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"])
+        self.assertEqual("Event report not found", channel.json_body["error"])
+
+    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 a event report
+        """
+        self.assertIn("id", content)
+        self.assertIn("received_ts", content)
+        self.assertIn("room_id", content)
+        self.assertIn("event_id", content)
+        self.assertIn("user_id", content)
+        self.assertIn("sender", content)
+        self.assertIn("canonical_alias", content)
+        self.assertIn("name", content)
+        self.assertIn("event_json", content)
+        self.assertIn("score", content)
+        self.assertIn("reason", content)
+        self.assertIn("auth_events", content["event_json"])
+        self.assertIn("type", content["event_json"])
+        self.assertIn("room_id", content["event_json"])
+        self.assertIn("sender", content["event_json"])
+        self.assertIn("content", content["event_json"])
				`@ -0,0 +1 @@`
				Split admin API for reported events (`GET /_synapse/admin/v1/event_reports`) into detail and list endpoints. This is a breaking change to #8217 which was introduced in Synapse v1.21.0. Those who already use this API should check their scripts. Contributed by @dklimpel.