Add a new third party callback check_event_allowed_v2 that is compatible with new batch persisting mechanisms.

changelog.d/15131.misc

@@ -0,0 +1 @@
+Add a new third party callback `check_event_allowed_v2` that is compatible with new batch persisting mechanisms.

docs/modules/third_party_rules_callbacks.md

@@ -10,6 +10,75 @@ The available third party rules callbacks are:
 
 ### `check_event_allowed`
 
+_First introduced in Synapse v1.7x.x
+
+```python
+async def check_event_allowed_v2(
+    event: "synapse.events.EventBase",
+    state_events: "synapse.types.StateMap",
+) -> Tuple[bool, Optional[dict], Optional[dict]]
+```
+
+**<span style="color:red">
+This callback is very experimental and can and will break without notice. Module developers
+are encouraged to implement `check_event_for_spam` from the spam checker category instead.
+</span>**
+
+Returns:
+
+- A tuple consisting of:
+
+  - a boolean representing whether or not the event is allowed
+  - an optional dict to form the basis of a replacement event for the event
+  - an optional dict to form the basis of an additional event to be sent into the
+          room
+
+Called when processing any incoming event, with the event and a `StateMap`
+representing the current state of the room the event is being sent into. A `StateMap` is
+a dictionary that maps tuples containing an event type and a state key to the
+corresponding state event. For example retrieving the room's `m.room.create` event from
+the `state_events` argument would look like this: `state_events.get(("m.room.create", ""))`.
+The module must return a boolean indicating whether the event can be allowed.
+
+Note that this callback function processes incoming events coming via federation
+traffic (on top of client traffic). This means denying an event might cause the local
+copy of the room's history to diverge from that of remote servers. This may cause
+federation issues in the room. It is strongly recommended to only deny events using this
+callback function if the sender is a local user, or in a private federation in which all
+servers are using the same module, with the same configuration.
+
+If the boolean returned by the module is `True`, it may tell Synapse to replace the
+event with new data by returning the new event's data as a dictionary. In order to do
+that, it is recommended the module calls `event.get_dict()` to get the current event as a
+dictionary, and modify the returned dictionary accordingly.
+
+Module writers may also wish to use this check to send an event into the room concurrent
+with the event being checked, if this is the case the module writer must provide a dict that 
+will form the basis of the event that is to be added to the room and it must be returned by `check_event_allowed_v2`.
+This dict will then be turned into an event at the appropriate time and it will be persisted after the event
+that triggered it, and if the event that triggered it is in a batch of events for persisting, it will be added to the 
+end of that batch. 
+
+If `check_event_allowed_v2` raises an exception, the module is assumed to have failed.
+The event will not be accepted but is not treated as explicitly rejected, either.
+An HTTP request causing the module check will likely result in a 500 Internal
+Server Error.
+
+When the boolean returned by the module is `False`, the event is rejected.
+(Module developers should not use exceptions for rejection.)
+
+Note that replacing the event or adding an event only works for events sent by local users, not for events
+received over federation.
+
+If multiple modules implement this callback, they will be considered in order. If a
+callback returns `True`, Synapse falls through to the next one. The value of the first
+callback that does not return `True` will be used. If this happens, Synapse will not call
+any of the subsequent implementations of this callback. This callback cannot be used in conjunction with `check_event_allowed`,
+only one of these callbacks may be operational at a time - if both `check_event_allowed` and `check_event_allowed_v2`
+active only `check_event_allowed` will be executed.
+
+### `check_event_allowed`
+
 _First introduced in Synapse v1.39.0_
 
 ```python

synapse/events/third_party_rules.py

@@ -32,6 +32,10 @@
 CHECK_EVENT_ALLOWED_CALLBACK = Callable[
     [EventBase, StateMap[EventBase]], Awaitable[Tuple[bool, Optional[dict]]]
 ]
+CHECK_EVENT_ALLOWED_V2_CALLBACK = Callable[
+    [EventBase, StateMap[EventBase]],
+    Awaitable[Tuple[bool, Optional[dict], Optional[dict]]],
+]
 ON_CREATE_ROOM_CALLBACK = Callable[[Requester, dict, bool], Awaitable]
 CHECK_THREEPID_CAN_BE_INVITED_CALLBACK = Callable[
     [str, str, StateMap[EventBase]], Awaitable[bool]
@@ -155,6 +159,9 @@ def __init__(self, hs: "HomeServer"):
         self._storage_controllers = hs.get_storage_controllers()
 
         self._check_event_allowed_callbacks: List[CHECK_EVENT_ALLOWED_CALLBACK] = []
+        self._check_event_allowed_v2_callbacks: List[
+            CHECK_EVENT_ALLOWED_V2_CALLBACK
+        ] = []
         self._on_create_room_callbacks: List[ON_CREATE_ROOM_CALLBACK] = []
         self._check_threepid_can_be_invited_callbacks: List[
             CHECK_THREEPID_CAN_BE_INVITED_CALLBACK
@@ -251,15 +258,16 @@ async def check_event_allowed(
         self,
         event: EventBase,
         context: UnpersistedEventContextBase,
-    ) -> Tuple[bool, Optional[dict]]:
+    ) -> Tuple[bool, Optional[dict], Optional[dict]]:
         """Check if a provided event should be allowed in the given context.
 
         The module can return:
             * True: the event is allowed.
             * False: the event is not allowed, and should be rejected with M_FORBIDDEN.
 
         If the event is allowed, the module can also return a dictionary to use as a
-        replacement for the event.
+        replacement for the event, and/or return a dictionary to use as the basis for
+        another event to be sent into the room.
 
         Args:
             event: The event to be checked.
@@ -269,8 +277,11 @@ async def check_event_allowed(
             The result from the ThirdPartyRules module, as above.
         """
         # Bail out early without hitting the store if we don't have any callbacks to run.
-        if len(self._check_event_allowed_callbacks) == 0:
-            return True, None
+        if (
+            len(self._check_event_allowed_callbacks) == 0
+            and len(self._check_event_allowed_v2_callbacks) == 0
+        ):
+            return True, None, None
 
         prev_state_ids = await context.get_prev_state_ids()
 
@@ -283,35 +294,63 @@ async def check_event_allowed(
         # the hashes and signatures.
         event.freeze()
 
-        for callback in self._check_event_allowed_callbacks:
-            try:
-                res, replacement_data = await delay_cancellation(
-                    callback(event, state_events)
-                )
-            except CancelledError:
-                raise
-            except SynapseError as e:
-                # FIXME: Being able to throw SynapseErrors is relied upon by
-                # some modules. PR #10386 accidentally broke this ability.
-                # That said, we aren't keen on exposing this implementation detail
-                # to modules and we should one day have a proper way to do what
-                # is wanted.
-                # This module callback needs a rework so that hacks such as
-                # this one are not necessary.
-                raise e
-            except Exception:
-                raise ModuleFailedException(
-                    "Failed to run `check_event_allowed` module API callback"
-                )
+        if len(self._check_event_allowed_callbacks) != 0:
+            for callback in self._check_event_allowed_callbacks:
+                try:
+                    res, replacement_data = await delay_cancellation(
+                        callback(event, state_events)
+                    )
+                except CancelledError:
+                    raise
+                except SynapseError as e:
+                    # FIXME: Being able to throw SynapseErrors is relied upon by
+                    # some modules. PR #10386 accidentally broke this ability.
+                    # That said, we aren't keen on exposing this implementation detail
+                    # to modules and we should one day have a proper way to do what
+                    # is wanted.
+                    # This module callback needs a rework so that hacks such as
+                    # this one are not necessary.
+                    raise e
+                except Exception:
+                    raise ModuleFailedException(
+                        "Failed to run `check_event_allowed` module API callback"
+                    )
 
-            # Return if the event shouldn't be allowed or if the module came up with a
-            # replacement dict for the event.
-            if res is False:
-                return res, None
-            elif isinstance(replacement_data, dict):
-                return True, replacement_data
+                # Return if the event shouldn't be allowed or if the module came up with a
+                # replacement dict for the event.
+                if res is False:
+                    return res, None, None
+                elif isinstance(replacement_data, dict):
+                    return True, replacement_data, None
+        else:
+            for v2_callback in self._check_event_allowed_v2_callbacks:
+                try:
+                    res, replacement_data, new_event = await delay_cancellation(
+                        v2_callback(event, state_events)
+                    )
+                except CancelledError:
+                    raise
+                except SynapseError as e:
+                    # FIXME: Being able to throw SynapseErrors is relied upon by
+                    # some modules. PR #10386 accidentally broke this ability.
+                    # That said, we aren't keen on exposing this implementation detail
+                    # to modules and we should one day have a proper way to do what
+                    # is wanted.
+                    # This module callback needs a rework so that hacks such as
+                    # this one are not necessary.
+                    raise e
+                except Exception:
+                    raise ModuleFailedException(
+                        "Failed to run `check_event_allowed_v2` module API callback"
+                    )
 
-        return True, None
+                # Return if the event shouldn't be allowed, if the module came up with a
+                # replacement dict for the event, or if the module wants to send a new event
+                if res is False:
+                    return res, None, None
+                else:
+                    return True, replacement_data, new_event
+        return True, None, None
 
     async def on_create_room(
         self, requester: Requester, config: dict, is_requester_admin: bool

synapse/handlers/federation.py

@@ -1006,6 +1006,7 @@ async def on_make_join_request(
             (
                 event,
                 unpersisted_context,
+                _,
             ) = await self.event_creation_handler.create_new_client_event(
                 builder=builder,
                 prev_event_ids=prev_event_ids,
@@ -1197,7 +1198,7 @@ async def on_make_leave_request(
             },
         )
 
-        event, _ = await self.event_creation_handler.create_new_client_event(
+        event, _, _ = await self.event_creation_handler.create_new_client_event(
             builder=builder
         )
 
@@ -1250,9 +1251,10 @@ async def on_make_knock_request(
         (
             event,
             unpersisted_context,
+            _,
         ) = await self.event_creation_handler.create_new_client_event(builder=builder)
 
-        event_allowed, _ = await self.third_party_event_rules.check_event_allowed(
+        event_allowed, _, _ = await self.third_party_event_rules.check_event_allowed(
             event, unpersisted_context
         )
         if not event_allowed:
@@ -1427,6 +1429,7 @@ async def exchange_third_party_invite(
                     (
                         event,
                         unpersisted_context,
+                        _,
                     ) = await self.event_creation_handler.create_new_client_event(
                         builder=builder
                     )
@@ -1509,6 +1512,7 @@ async def on_exchange_third_party_invite_request(
                 (
                     event,
                     unpersisted_context,
+                    _,
                 ) = await self.event_creation_handler.create_new_client_event(
                     builder=builder
                 )
@@ -1591,6 +1595,7 @@ async def add_display_name_to_third_party_invite(
         (
             event,
             unpersisted_context,
+            _,
         ) = await self.event_creation_handler.create_new_client_event(builder=builder)
 
         EventValidator().validate_new(event, self.config)

synapse/handlers/federation_event.py

@@ -404,9 +404,11 @@ async def on_send_membership_event(
         # for knock events, we run the third-party event rules. It's not entirely clear
         # why we don't do this for other sorts of membership events.
         if event.membership == Membership.KNOCK:
-            event_allowed, _ = await self._third_party_event_rules.check_event_allowed(
-                event, context
-            )
+            (
+                event_allowed,
+                _,
+                _,
+            ) = await self._third_party_event_rules.check_event_allowed(event, context)
             if not event_allowed:
                 logger.info("Sending of knock %s forbidden by third-party rules", event)
                 raise SynapseError(