feat: Add requester as an argument to the third party callback check_event_allowed

docs/modules/third_party_rules_callbacks.md

@@ -16,6 +16,7 @@ _First introduced in Synapse v1.39.0_
 async def check_event_allowed(
     event: "synapse.events.EventBase",
     state_events: "synapse.types.StateMap",
+    requester: "Requester | None",
 ) -> tuple[bool, dict | None]
 ```
 
@@ -31,6 +32,10 @@ corresponding state event. For example retrieving the room's `m.room.create` eve
 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.
 
+**Added for Famedly Synapse 1.157.0**: The `requesting_user_id` argument is added that will
+optionally pass in the `Requester` that placed the request for the event. This may be the
+`sender` of the event, or in the case of a kick/ban the user that placed the request.
+
 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

synapse/handlers/federation.py

@@ -1299,6 +1299,9 @@ async def on_make_knock_request(
             unpersisted_context,
         ) = await self.event_creation_handler.create_new_client_event(builder=builder)
 
+        # TODO: just remove this callback, it is literally called in the function above
+        #  and the only difference between here and there is one logs as info and the
+        #  other is a warning
         event_allowed, _ = await self._third_party_event_rules.check_event_allowed(
             event, unpersisted_context
         )

synapse/handlers/message.py

@@ -1433,7 +1433,7 @@ async def create_new_client_event(
             )
 
         res, new_content = await self._third_party_event_rules.check_event_allowed(
-            event, context
+            event, context, requester
         )
         if res is False:
             logger.info(

synapse/module_api/callbacks/third_party_event_rules_callbacks.py

@@ -18,8 +18,9 @@
 # [This file includes modifications made by New Vector Limited]
 #
 #
+import inspect
 import logging
-from typing import TYPE_CHECKING, Any, Awaitable, Callable
+from typing import TYPE_CHECKING, Any, Awaitable, Callable, cast
 
 from twisted.internet.defer import CancelledError
 
@@ -37,9 +38,13 @@
 logger = logging.getLogger(__name__)
 
 
-CHECK_EVENT_ALLOWED_CALLBACK = Callable[
-    [EventBase, StateMap[EventBase]], Awaitable[tuple[bool, dict | None]]
-]
+CHECK_EVENT_ALLOWED_CALLBACK = (
+    Callable[[EventBase, StateMap[EventBase]], Awaitable[tuple[bool, dict | None]]]
+    | Callable[
+        [EventBase, StateMap[EventBase], Requester | None],
+        Awaitable[tuple[bool, dict | None]],
+    ]
+)
 ON_CREATE_ROOM_CALLBACK = Callable[[Requester, dict, bool], Awaitable]
 CHECK_THREEPID_CAN_BE_INVITED_CALLBACK = Callable[
     [str, str, StateMap[EventBase]], Awaitable[bool]
@@ -92,16 +97,26 @@ def async_wrapper(f: Callable | None) -> Callable[..., Awaitable] | None:
             # We need to wrap check_event_allowed because its old form would return either
             # a boolean or a dict, but now we want to return the dict separately from the
             # boolean.
+            # We also need to check that the signature has either two or three args, as
+            # a requesting user argument was added later. If this new arg was not
+            # included in the registered callback, call it as a function that needs only
+            # 2 arguments and drop the `requester` on the floor.
+            checker_args = inspect.signature(f)
+
             async def wrap_check_event_allowed(
                 event: EventBase,
                 state_events: StateMap[EventBase],
+                requesting_user: Requester,
             ) -> tuple[bool, dict | None]:
                 # Assertion required because mypy can't prove we won't change
                 # `f` back to `None`. See
                 # https://mypy.readthedocs.io/en/latest/common_issues.html#narrowing-and-inner-functions
                 assert f is not None
+                if len(checker_args.parameters) == 3:
+                    res = await f(event, state_events, requesting_user)
+                else:
+                    res = await f(event, state_events)
 
-                res = await f(event, state_events)
                 if isinstance(res, dict):
                     return True, res
                 else:
@@ -262,6 +277,7 @@ async def check_event_allowed(
         self,
         event: EventBase,
         context: UnpersistedEventContextBase,
+        requester: Requester | None = None,
     ) -> tuple[bool, dict | None]:
         """Check if a provided event should be allowed in the given context.
 
@@ -275,6 +291,7 @@ async def check_event_allowed(
         Args:
             event: The event to be checked.
             context: The context of the event.
+            requester: The user who requested this event or None.
 
         Returns:
             The result from the ThirdPartyRules module, as above.
@@ -291,9 +308,32 @@ async def check_event_allowed(
 
         for callback in self._check_event_allowed_callbacks:
             try:
-                res, replacement_data = await delay_cancellation(
-                    callback(event, state_events)
-                )
+                checker_args = inspect.signature(callback)
+                # Ensure backwards compatibility with third party callbacks
+                # that don't expect the requesting_user argument.
+                if len(checker_args.parameters) == 2:
+                    callback_without_requester_id = cast(
+                        Callable[
+                            [EventBase, StateMap[EventBase]],
+                            Awaitable[tuple[bool, dict | None]],
+                        ],
+                        callback,
+                    )
+                    res, replacement_data = await delay_cancellation(
+                        callback_without_requester_id(event, state_events)
+                    )
+                else:
+                    callback_with_requester_id = cast(
+                        Callable[
+                            [EventBase, StateMap[EventBase], Requester | None],
+                            Awaitable[tuple[bool, dict | None]],
+                        ],
+                        callback,
+                    )
+                    res, replacement_data = await delay_cancellation(
+                        callback_with_requester_id(event, state_events, requester)
+                    )
+
             except CancelledError:
                 raise
             except SynapseError as e:

tests/rest/client/test_third_party_rules.py

@@ -233,7 +233,7 @@ def error_dict(self, config: HomeServerConfig | None) -> JsonDict:
 
         # add a callback that will raise our hacky exception
         async def check(
-            ev: EventBase, state: StateMap[EventBase]
+            ev: EventBase, state: StateMap[EventBase], requesting_user: Requester | None
         ) -> tuple[bool, JsonDict | None]:
             raise NastyHackException(429, "message")
 
@@ -261,7 +261,7 @@ def test_cannot_modify_event(self) -> None:
 
         # first patch the event checker so that it will try to modify the event
         async def check(
-            ev: EventBase, state: StateMap[EventBase]
+            ev: EventBase, state: StateMap[EventBase], requesting_user: Requester | None
         ) -> tuple[bool, JsonDict | None]:
             # Try and modify the content, this will fail because the event is
             # immutable. (We therefore need the type ignore linter, as the
@@ -285,13 +285,56 @@ async def check(
         self.assertEqual(channel.code, 500, channel.result)
 
     def test_modify_event(self) -> None:
-        """The module can return a modified version of the event"""
+        """
+        The module can return a modified version of the event. Use this test to check
+        the backwards compatibility for the number of arguments passed to the function.
+        """
 
         # first patch the event checker so that it will modify the event
+        async def check_v2(
+            ev: EventBase, state: StateMap[EventBase], requesting_user: Requester | None
+        ) -> tuple[bool, JsonDict | None]:
+            d = ev.get_dict()
+            # To make sure that the requesting user argument is giving the correct data,
+            # check that it should be the same as the event sender, as that is the
+            # fallback option when it is not a differing user
+            assert requesting_user is not None
+            self.assertEqual(ev.sender, requesting_user.user.to_string())
+            self.assertEqual(ev.content["x"], "x")
+            d["content"] = {"x": "y"}
+            return True, d
+
+        self.hs.get_module_api_callbacks().third_party_event_rules._check_event_allowed_callbacks = [
+            check_v2
+        ]
+
+        # now send the event
+        channel = self.make_request(
+            "PUT",
+            "/_matrix/client/r0/rooms/%s/send/modifyme/1" % self.room_id,
+            {"x": "x"},
+            access_token=self.tok,
+        )
+        self.assertEqual(channel.code, 200, channel.result)
+        event_id = channel.json_body["event_id"]
+
+        # ... and check that it got modified
+        channel = self.make_request(
+            "GET",
+            "/_matrix/client/r0/rooms/%s/event/%s" % (self.room_id, event_id),
+            access_token=self.tok,
+        )
+        self.assertEqual(channel.code, 200, channel.result)
+        ev = channel.json_body
+        self.assertEqual(ev["content"]["x"], "y")
+
         async def check(
             ev: EventBase, state: StateMap[EventBase]
         ) -> tuple[bool, JsonDict | None]:
+            # No requesting_user to check here. Just make sure it does not blow up with
+            # a 500 Internal Server Error
             d = ev.get_dict()
+            self.assertEqual(ev.content["x"], "x")
             d["content"] = {"x": "y"}
             return True, d
 
@@ -302,7 +345,7 @@ async def check(
         # now send the event
         channel = self.make_request(
             "PUT",
-            "/_matrix/client/r0/rooms/%s/send/modifyme/1" % self.room_id,
+            "/_matrix/client/r0/rooms/%s/send/modifyme/2" % self.room_id,
             {"x": "x"},
             access_token=self.tok,
         )
@@ -324,7 +367,7 @@ def test_message_edit(self) -> None:
 
         # first patch the event checker so that it will modify the event
         async def check(
-            ev: EventBase, state: StateMap[EventBase]
+            ev: EventBase, state: StateMap[EventBase], requesting_user: Requester | None
         ) -> tuple[bool, JsonDict | None]:
             d = ev.get_dict()
             d["content"] = {
@@ -541,7 +584,9 @@ def test_sent_event_end_up_in_room_state(self) -> None:
 
         # Define a callback that sends a custom event on power levels update.
         async def test_fn(
-            event: EventBase, state_events: StateMap[EventBase]
+            event: EventBase,
+            state_events: StateMap[EventBase],
+            requesting_user: Requester | None,
         ) -> tuple[bool, JsonDict | None]:
             if event.is_state() and event.type == EventTypes.PowerLevels:
                 await api.create_and_send_event_into_room(