diff --git a/docs/modules/third_party_rules_callbacks.md b/docs/modules/third_party_rules_callbacks.md index 43bd21b814..8d5ad99c1d 100644 --- a/docs/modules/third_party_rules_callbacks.md +++ b/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 diff --git a/synapse/handlers/federation.py b/synapse/handlers/federation.py index 1761f169cc..9b96a193b4 100644 --- a/synapse/handlers/federation.py +++ b/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 ) diff --git a/synapse/handlers/message.py b/synapse/handlers/message.py index 6bdef7e202..b8f10e0192 100644 --- a/synapse/handlers/message.py +++ b/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( diff --git a/synapse/module_api/callbacks/third_party_event_rules_callbacks.py b/synapse/module_api/callbacks/third_party_event_rules_callbacks.py index 6f951c4b72..7449a3b36c 100644 --- a/synapse/module_api/callbacks/third_party_event_rules_callbacks.py +++ b/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: diff --git a/tests/rest/client/test_third_party_rules.py b/tests/rest/client/test_third_party_rules.py index 625b8e4207..4c927ef793 100644 --- a/tests/rest/client/test_third_party_rules.py +++ b/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(