Allow admins to see policy server-flagged events (#18585)

changelog.d/18585.feature

@@ -0,0 +1 @@
+When admins enable themselves to see soft-failed events, they will also see if the cause is due to the policy server flagging them as spam via `unsigned`.

docs/admin_api/client_server_api_extensions.md

@@ -22,4 +22,46 @@ To receive soft failed events in APIs like `/sync` and `/messages`, set `return_
 to `true` in the admin client config. When `false`, the normal behaviour of these endpoints is to
 exclude soft failed events.
 
+**Note**: If the policy server flagged the event as spam and that caused soft failure, that will be indicated
+in the event's `unsigned` content like so:
+
+```json
+{
+  "type": "m.room.message",
+  "other": "event_fields_go_here",
+  "unsigned": {
+    "io.element.synapse.soft_failed": true,
+    "io.element.synapse.policy_server_spammy": true
+  }
+}
+```
+
 Default: `false`
+
+## See events marked spammy by policy servers
+
+Learn more about policy servers from [MSC4284](https://github.com/matrix-org/matrix-spec-proposals/pull/4284).
+
+Similar to `return_soft_failed_events`, clients logged in with admin accounts can see events which were
+flagged by the policy server as spammy (and thus soft failed) by setting `return_policy_server_spammy_events`
+to `true`.
+
+`return_policy_server_spammy_events` may be `true` while `return_soft_failed_events` is `false` to only see
+policy server-flagged events. When `return_soft_failed_events` is `true` however, `return_policy_server_spammy_events`
+is always `true`.
+
+Events which were flagged by the policy will be flagged as `io.element.synapse.policy_server_spammy` in the
+event's `unsigned` content, like so:
+
+```json
+{
+  "type": "m.room.message",
+  "other": "event_fields_go_here",
+  "unsigned": {
+    "io.element.synapse.soft_failed": true,
+    "io.element.synapse.policy_server_spammy": true
+  }
+}
+```
+
+Default: `true` if `return_soft_failed_events` is `true`, otherwise `false`

rust/src/events/internal_metadata.rs

@@ -54,6 +54,7 @@ enum EventInternalMetadataData {
     RecheckRedaction(bool),
     SoftFailed(bool),
     ProactivelySend(bool),
+    PolicyServerSpammy(bool),
     Redacted(bool),
     TxnId(Box<str>),
     TokenId(i64),
@@ -96,6 +97,13 @@ impl EventInternalMetadataData {
                     .to_owned()
                     .into_any(),
             ),
+            EventInternalMetadataData::PolicyServerSpammy(o) => (
+                pyo3::intern!(py, "policy_server_spammy"),
+                o.into_pyobject(py)
+                    .unwrap_infallible()
+                    .to_owned()
+                    .into_any(),
+            ),
             EventInternalMetadataData::Redacted(o) => (
                 pyo3::intern!(py, "redacted"),
                 o.into_pyobject(py)
@@ -155,6 +163,11 @@ impl EventInternalMetadataData {
                     .extract()
                     .with_context(|| format!("'{key_str}' has invalid type"))?,
             ),
+            "policy_server_spammy" => EventInternalMetadataData::PolicyServerSpammy(
+                value
+                    .extract()
+                    .with_context(|| format!("'{key_str}' has invalid type"))?,
+            ),
             "redacted" => EventInternalMetadataData::Redacted(
                 value
                     .extract()
@@ -427,6 +440,17 @@ impl EventInternalMetadata {
         set_property!(self, ProactivelySend, obj);
     }
 
+    #[getter]
+    fn get_policy_server_spammy(&self) -> PyResult<bool> {
+        Ok(get_property_opt!(self, PolicyServerSpammy)
+            .copied()
+            .unwrap_or(false))
+    }
+    #[setter]
+    fn set_policy_server_spammy(&mut self, obj: bool) {
+        set_property!(self, PolicyServerSpammy, obj);
+    }
+
     #[getter]
     fn get_redacted(&self) -> PyResult<bool> {
         let bool = get_property!(self, Redacted)?;

synapse/events/utils.py

@@ -538,8 +538,11 @@ def serialize_event(
             d["content"] = dict(d["content"])
             d["content"]["redacts"] = e.redacts
 
-    if config.include_admin_metadata and e.internal_metadata.is_soft_failed():
+    if config.include_admin_metadata:
-        d["unsigned"]["io.element.synapse.soft_failed"] = True
+        if e.internal_metadata.is_soft_failed():
+            d["unsigned"]["io.element.synapse.soft_failed"] = True
+        if e.internal_metadata.policy_server_spammy:
+            d["unsigned"]["io.element.synapse.policy_server_spammy"] = True
 
     only_event_fields = config.only_event_fields
     if only_event_fields:

synapse/federation/federation_base.py

@@ -174,6 +174,7 @@ class FederationBase:
                 "Event not allowed by policy server, soft-failing %s", pdu.event_id
             )
             pdu.internal_metadata.soft_failed = True
+            pdu.internal_metadata.policy_server_spammy = True
             # Note: we don't redact the event so admins can inspect the event after the
             # fact. Other processes may redact the event, but that won't be applied to
             # the database copy of the event until the server's config requires it.

synapse/handlers/message.py

@@ -1101,6 +1101,9 @@ class EventCreationHandler:
 
                 policy_allowed = await self._policy_handler.is_event_allowed(event)
                 if not policy_allowed:
+                    # We shouldn't need to set the metadata because the raise should
+                    # cause the request to be denied, but just in case:
+                    event.internal_metadata.policy_server_spammy = True
                     logger.warning(
                         "Event not allowed by policy server, rejecting %s",
                         event.event_id,

synapse/storage/admin_client_config.py

@@ -15,8 +15,12 @@ class AdminClientConfig:
         # `unsigned` portion of the event to inform clients that the event
         # is soft-failed.
         self.return_soft_failed_events: bool = False
+        self.return_policy_server_spammy_events: bool = False
 
         if account_data:
             self.return_soft_failed_events = account_data.get(
                 "return_soft_failed_events", False
             )
+            self.return_policy_server_spammy_events = account_data.get(
+                "return_policy_server_spammy_events", self.return_soft_failed_events
+            )

synapse/synapse_rust/events.pyi

@@ -33,6 +33,9 @@ class EventInternalMetadata:
     proactively_send: bool
     redacted: bool
 
+    policy_server_spammy: bool
+    """whether the policy server indicated that this event is spammy"""
+
     txn_id: str
     """The transaction ID, if it was set when the event was created."""
     token_id: int

synapse/visibility.py

@@ -117,13 +117,29 @@ async def filter_events_for_client(
     # We copy the events list to guarantee any modifications we make will only
     # happen within the function.
     events_before_filtering = events.copy()
+    # Default case is to *exclude* soft-failed events
+    events = [e for e in events if not e.internal_metadata.is_soft_failed()]
     client_config = await storage.main.get_admin_client_config_for_user(user_id)
-    if not (
+    if filter_send_to_client and await storage.main.is_server_admin(
-        filter_send_to_client
+        UserID.from_string(user_id)
-        and client_config.return_soft_failed_events
-        and await storage.main.is_server_admin(UserID.from_string(user_id))
     ):
-        events = [e for e in events if not e.internal_metadata.is_soft_failed()]
+        if client_config.return_soft_failed_events:
+            # The user has requested that all events be included, so do that.
+            # We copy the list for mutation safety.
+            events = events_before_filtering.copy()
+        elif client_config.return_policy_server_spammy_events:
+            # Include events that were soft failed by a policy server (marked spammy),
+            # but exclude all other soft failed events. We also want to include all
+            # not-soft-failed events, per usual operation.
+            events = [
+                e
+                for e in events_before_filtering
+                if not e.internal_metadata.is_soft_failed()
+                or e.internal_metadata.policy_server_spammy
+            ]
+        # else - no change in behaviour; use default case
+    # else - no change in behaviour; use default case
+
     if len(events_before_filtering) != len(events):
         if filtered_event_logger.isEnabledFor(logging.DEBUG):
             filtered_event_logger.debug(

tests/events/test_utils.py

@@ -822,6 +822,32 @@ class SerializeEventTestCase(stdlib_unittest.TestCase):
                 "unsigned": {"io.element.synapse.soft_failed": True},
             },
         )
+        self.assertEqual(
+            self.serialize(
+                MockEvent(
+                    type="foo",
+                    event_id="test",
+                    room_id="!foo:bar",
+                    content={"foo": "bar"},
+                    internal_metadata={
+                        "soft_failed": True,
+                        "policy_server_spammy": True,
+                    },
+                ),
+                [],
+                True,
+            ),
+            {
+                "type": "foo",
+                "event_id": "test",
+                "room_id": "!foo:bar",
+                "content": {"foo": "bar"},
+                "unsigned": {
+                    "io.element.synapse.soft_failed": True,
+                    "io.element.synapse.policy_server_spammy": True,
+                },
+            },
+        )
 
     def test_make_serialize_config_for_admin_retains_other_fields(self) -> None:
         non_default_config = SerializeEventConfig(

tests/rest/client/test_relations.py

@@ -1181,7 +1181,7 @@ class BundledAggregationsTestCase(BaseRelationsTestCase):
                 bundled_aggregations,
             )
 
-        self._test_bundled_aggregations(RelationTypes.REFERENCE, assert_annotations, 8)
+        self._test_bundled_aggregations(RelationTypes.REFERENCE, assert_annotations, 9)
 
     def test_thread(self) -> None:
         """
@@ -1226,21 +1226,21 @@ class BundledAggregationsTestCase(BaseRelationsTestCase):
 
         # The "user" sent the root event and is making queries for the bundled
         # aggregations: they have participated.
-        self._test_bundled_aggregations(RelationTypes.THREAD, _gen_assert(True), 9)
+        self._test_bundled_aggregations(RelationTypes.THREAD, _gen_assert(True), 10)
         # The "user2" sent replies in the thread and is making queries for the
         # bundled aggregations: they have participated.
         #
         # Note that this re-uses some cached values, so the total number of
         # queries is much smaller.
         self._test_bundled_aggregations(
-            RelationTypes.THREAD, _gen_assert(True), 6, access_token=self.user2_token
+            RelationTypes.THREAD, _gen_assert(True), 7, access_token=self.user2_token
         )
 
         # A user with no interactions with the thread: they have not participated.
         user3_id, user3_token = self._create_user("charlie")
         self.helper.join(self.room, user=user3_id, tok=user3_token)
         self._test_bundled_aggregations(
-            RelationTypes.THREAD, _gen_assert(False), 6, access_token=user3_token
+            RelationTypes.THREAD, _gen_assert(False), 7, access_token=user3_token
         )
 
     def test_thread_with_bundled_aggregations_for_latest(self) -> None:
@@ -1287,7 +1287,7 @@ class BundledAggregationsTestCase(BaseRelationsTestCase):
                 bundled_aggregations["latest_event"].get("unsigned"),
             )
 
-        self._test_bundled_aggregations(RelationTypes.THREAD, assert_thread, 9)
+        self._test_bundled_aggregations(RelationTypes.THREAD, assert_thread, 10)
 
     def test_nested_thread(self) -> None:
         """

tests/test_utils/event_injection.py

@@ -105,6 +105,13 @@ async def create_event(
         builder, prev_event_ids=prev_event_ids
     )
 
+    # Copy over writable internal_metadata, if set
+    # Dev note: This isn't everything that's writable. `for k,v` doesn't work here :(
+    if kwargs.get("internal_metadata", {}).get("soft_failed", False):
+        event.internal_metadata.soft_failed = True
+    if kwargs.get("internal_metadata", {}).get("policy_server_spammy", False):
+        event.internal_metadata.policy_server_spammy = True
+
     context = await unpersisted_context.persist(event)
 
     return event, context

tests/test_visibility.py

@@ -21,7 +21,9 @@ import logging
 from typing import Optional
 from unittest.mock import patch
 
-from synapse.api.constants import EventUnsignedContentFields
+from twisted.test.proto_helpers import MemoryReactor
+
+from synapse.api.constants import AccountDataTypes, EventUnsignedContentFields
 from synapse.api.room_versions import RoomVersions
 from synapse.events import EventBase, make_event_from_dict
 from synapse.events.snapshot import EventContext
@@ -29,6 +31,7 @@ from synapse.rest import admin
 from synapse.rest.client import login, room
 from synapse.server import HomeServer
 from synapse.types import create_requester
+from synapse.util import Clock
 from synapse.visibility import filter_events_for_client, filter_events_for_server
 
 from tests import unittest
@@ -272,6 +275,210 @@ class FilterEventsForServerTestCase(unittest.HomeserverTestCase):
         return event
 
 
+class FilterEventsForServerAdminsTestCase(HomeserverTestCase):
+    servlets = [
+        admin.register_servlets,
+        login.register_servlets,
+        room.register_servlets,
+    ]
+
+    def prepare(
+        self, reactor: MemoryReactor, clock: Clock, homeserver: HomeServer
+    ) -> None:
+        self.register_user("admin", "password", admin=True)
+        self.tok = self.login("admin", "password")
+        self.room_id = self.helper.create_room_as("admin", tok=self.tok)
+        self.get_success(
+            inject_visibility_event(self.hs, self.room_id, "@admin:test", "joined")
+        )
+        self.regular_event = self.get_success(
+            inject_message_event(self.hs, self.room_id, "@admin:test", body="regular")
+        )
+        self.soft_failed_event = self.get_success(
+            inject_message_event(
+                self.hs,
+                self.room_id,
+                "@admin:test",
+                body="soft failed",
+                soft_failed=True,
+            )
+        )
+        self.spammy_event = self.get_success(
+            inject_message_event(
+                self.hs,
+                self.room_id,
+                "@admin:test",
+                body="spammy",
+                soft_failed=True,
+                policy_server_spammy=True,
+            )
+        )
+
+    def test_normal_operation_as_admin(self) -> None:
+        # `filter_events_for_client` shouldn't include soft failed events by default
+        # for admins.
+
+        # Reload events from DB
+        events_to_filter = [
+            self.get_success(
+                self.hs.get_storage_controllers().main.get_event(
+                    e.event_id,
+                    get_prev_content=True,
+                )
+            )
+            for e in [self.regular_event, self.soft_failed_event]
+        ]
+
+        # Do filter & assert
+        filtered_events = self.get_success(
+            filter_events_for_client(
+                self.hs.get_storage_controllers(),
+                "@admin:test",
+                events_to_filter,
+            )
+        )
+        self.assertEqual(
+            [e.event_id for e in [self.regular_event]],
+            [e.event_id for e in filtered_events],
+        )
+
+    def test_see_soft_failed_events(self) -> None:
+        # `filter_events_for_client` should include soft failed events when configured
+
+        # Reload events from DB
+        events_to_filter = [
+            self.get_success(
+                self.hs.get_storage_controllers().main.get_event(
+                    e.event_id,
+                    get_prev_content=True,
+                )
+            )
+            for e in [self.regular_event, self.soft_failed_event]
+        ]
+
+        # Inject client config
+        self.get_success(
+            self.hs.get_account_data_handler().add_account_data_for_user(
+                "@admin:test",
+                AccountDataTypes.SYNAPSE_ADMIN_CLIENT_CONFIG,
+                {"return_soft_failed_events": True},
+            )
+        )
+
+        # Sanity check
+        self.assertEqual(True, events_to_filter[1].internal_metadata.soft_failed)
+
+        # Do filter & assert
+        filtered_events = self.get_success(
+            filter_events_for_client(
+                self.hs.get_storage_controllers(),
+                "@admin:test",
+                events_to_filter,
+            )
+        )
+        self.assertEqual(
+            [e.event_id for e in [self.regular_event, self.soft_failed_event]],
+            [e.event_id for e in filtered_events],
+        )
+
+    def test_see_policy_server_spammy_events(self) -> None:
+        # `filter_events_for_client` should include policy server-flagged events, but
+        # not other soft-failed events, when asked.
+
+        # Reload events from DB
+        events_to_filter = [
+            self.get_success(
+                self.hs.get_storage_controllers().main.get_event(
+                    e.event_id,
+                    get_prev_content=True,
+                )
+            )
+            for e in [self.regular_event, self.soft_failed_event, self.spammy_event]
+        ]
+
+        # Inject client config
+        self.get_success(
+            self.hs.get_account_data_handler().add_account_data_for_user(
+                "@admin:test",
+                AccountDataTypes.SYNAPSE_ADMIN_CLIENT_CONFIG,
+                {
+                    "return_soft_failed_events": False,
+                    "return_policy_server_spammy_events": True,
+                },
+            )
+        )
+
+        # Sanity checks
+        self.assertEqual(True, events_to_filter[1].internal_metadata.soft_failed)
+        self.assertEqual(True, events_to_filter[2].internal_metadata.soft_failed)
+        self.assertEqual(
+            True, events_to_filter[2].internal_metadata.policy_server_spammy
+        )
+
+        # Do filter & assert
+        filtered_events = self.get_success(
+            filter_events_for_client(
+                self.hs.get_storage_controllers(),
+                "@admin:test",
+                events_to_filter,
+            )
+        )
+        self.assertEqual(
+            [e.event_id for e in [self.regular_event, self.spammy_event]],
+            [e.event_id for e in filtered_events],
+        )
+
+    def test_see_soft_failed_and_policy_server_spammy_events(self) -> None:
+        # `filter_events_for_client` should include both types of soft failed events
+        # when configured.
+
+        # Reload events from DB
+        events_to_filter = [
+            self.get_success(
+                self.hs.get_storage_controllers().main.get_event(
+                    e.event_id,
+                    get_prev_content=True,
+                )
+            )
+            for e in [self.regular_event, self.soft_failed_event, self.spammy_event]
+        ]
+
+        # Inject client config
+        self.get_success(
+            self.hs.get_account_data_handler().add_account_data_for_user(
+                "@admin:test",
+                AccountDataTypes.SYNAPSE_ADMIN_CLIENT_CONFIG,
+                {
+                    "return_soft_failed_events": True,
+                    "return_policy_server_spammy_events": True,
+                },
+            )
+        )
+
+        # Sanity checks
+        self.assertEqual(True, events_to_filter[1].internal_metadata.soft_failed)
+        self.assertEqual(True, events_to_filter[2].internal_metadata.soft_failed)
+        self.assertEqual(
+            True, events_to_filter[2].internal_metadata.policy_server_spammy
+        )
+
+        # Do filter & assert
+        filtered_events = self.get_success(
+            filter_events_for_client(
+                self.hs.get_storage_controllers(),
+                "@admin:test",
+                events_to_filter,
+            )
+        )
+        self.assertEqual(
+            [
+                e.event_id
+                for e in [self.regular_event, self.soft_failed_event, self.spammy_event]
+            ],
+            [e.event_id for e in filtered_events],
+        )
+
+
 class FilterEventsForClientTestCase(HomeserverTestCase):
     servlets = [
         admin.register_servlets,
@@ -487,6 +694,8 @@ async def inject_message_event(
     room_id: str,
     sender: str,
     body: Optional[str] = "testytest",
+    soft_failed: Optional[bool] = False,
+    policy_server_spammy: Optional[bool] = False,
 ) -> EventBase:
     return await inject_event(
         hs,
@@ -494,4 +703,8 @@ async def inject_message_event(
         sender=sender,
         room_id=room_id,
         content={"body": body, "msgtype": "m.text"},
+        internal_metadata={
+            "soft_failed": soft_failed,
+            "policy_server_spammy": policy_server_spammy,
+        },
     )