diff --git a/changelog.d/12808.feature b/changelog.d/12808.feature new file mode 100644 index 0000000000..561c8b9d34 --- /dev/null +++ b/changelog.d/12808.feature @@ -0,0 +1 @@ +Update to `check_event_for_spam`. Deprecate the current callback signature, replace it with a new signature that is both less ambiguous (replacing booleans with explicit allow/block) and more powerful (ability to return explicit error codes). \ No newline at end of file diff --git a/changelog.d/12846.misc b/changelog.d/12846.misc new file mode 100644 index 0000000000..f72d3d2bea --- /dev/null +++ b/changelog.d/12846.misc @@ -0,0 +1 @@ +Experimental: expand `check_event_for_spam` with ability to return additional fields. This enables spam-checker implementations to experiment with mechanisms to give users more information about why they are blocked and whether any action is needed from them to be unblocked. \ No newline at end of file diff --git a/synapse/api/errors.py b/synapse/api/errors.py index 05e96843cf..54268e0889 100644 --- a/synapse/api/errors.py +++ b/synapse/api/errors.py @@ -146,7 +146,13 @@ class SynapseError(CodeMessageException): errcode: Matrix error code e.g 'M_FORBIDDEN' """ - def __init__(self, code: int, msg: str, errcode: str = Codes.UNKNOWN): + def __init__( + self, + code: int, + msg: str, + errcode: str = Codes.UNKNOWN, + additional_fields: Optional[Dict] = None, + ): """Constructs a synapse error. Args: @@ -156,9 +162,13 @@ class SynapseError(CodeMessageException): """ super().__init__(code, msg) self.errcode = errcode + if additional_fields is None: + self._additional_fields: Dict = {} + else: + self._additional_fields = dict(additional_fields) def error_dict(self) -> "JsonDict": - return cs_error(self.msg, self.errcode) + return cs_error(self.msg, self.errcode, **self._additional_fields) class InvalidAPICallError(SynapseError): @@ -183,14 +193,7 @@ class ProxiedRequestError(SynapseError): errcode: str = Codes.UNKNOWN, additional_fields: Optional[Dict] = None, ): - super().__init__(code, msg, errcode) - if additional_fields is None: - self._additional_fields: Dict = {} - else: - self._additional_fields = dict(additional_fields) - - def error_dict(self) -> "JsonDict": - return cs_error(self.msg, self.errcode, **self._additional_fields) + super().__init__(code, msg, errcode, additional_fields) class ConsentNotGivenError(SynapseError): diff --git a/synapse/events/spamcheck.py b/synapse/events/spamcheck.py index 7984874e21..82998ca490 100644 --- a/synapse/events/spamcheck.py +++ b/synapse/events/spamcheck.py @@ -21,6 +21,7 @@ from typing import ( Awaitable, Callable, Collection, + Dict, List, Optional, Tuple, @@ -41,13 +42,17 @@ if TYPE_CHECKING: logger = logging.getLogger(__name__) - CHECK_EVENT_FOR_SPAM_CALLBACK = Callable[ ["synapse.events.EventBase"], Awaitable[ Union[ Allow, Codes, + # Highly experimental, not officially part of the spamchecker API, may + # disappear without warning depending on the results of ongoing + # experiments. + # Use this to return additional information as part of an error. + Tuple[Codes, Dict], # Deprecated bool, # Deprecated @@ -270,7 +275,7 @@ class SpamChecker: async def check_event_for_spam( self, event: "synapse.events.EventBase" - ) -> Union[Decision, str]: + ) -> Union[Decision, Tuple[Codes, Dict], str]: """Checks if a given event is considered "spammy" by this server. If the server considers an event spammy, then it will be rejected if @@ -293,9 +298,9 @@ class SpamChecker: with Measure( self.clock, "{}.{}".format(callback.__module__, callback.__qualname__) ): - res: Union[Decision, str, bool] = await delay_cancellation( - callback(event) - ) + res: Union[ + Decision, Tuple[Codes, Dict], str, bool + ] = await delay_cancellation(callback(event)) if res is False or res is Allow.ALLOW: # This spam-checker accepts the event. # Other spam-checkers may reject it, though. @@ -305,8 +310,9 @@ class SpamChecker: # return value `True` return Codes.FORBIDDEN else: - # This spam-checker rejects the event either with a `str` - # or with a `Codes`. In either case, we stop here. + # This spam-checker rejects the event either with a `str`, + # with a `Codes` or with a `Tuple[Codes, Dict]`. In either + # case, we stop here. return res # No spam-checker has rejected the event, let it pass. diff --git a/synapse/handlers/message.py b/synapse/handlers/message.py index 7ca126dbd1..38b71a2c96 100644 --- a/synapse/handlers/message.py +++ b/synapse/handlers/message.py @@ -895,6 +895,21 @@ class EventCreationHandler: spam_check = await self.spam_checker.check_event_for_spam(event) if spam_check is not synapse.spam_checker_api.Allow.ALLOW: + if isinstance(spam_check, tuple): + try: + [code, dict] = spam_check + raise SynapseError( + 403, + "This message had been rejected as probable spam", + code, + dict, + ) + except ValueError: + logger.error( + "Spam-check module returned invalid error value. Expecting [code, dict], got %s", + spam_check, + ) + spam_check = Codes.FORBIDDEN raise SynapseError( 403, "This message had been rejected as probable spam", spam_check )