diff options
Diffstat (limited to 'synapse')
| -rw-r--r-- | synapse/events/spamcheck.py | 49 | ||||
| -rw-r--r-- | synapse/federation/federation_base.py | 3 | ||||
| -rw-r--r-- | synapse/handlers/message.py | 19 | ||||
| -rw-r--r-- | synapse/module_api/__init__.py | 8 | ||||
| -rw-r--r-- | synapse/spam_checker_api/__init__.py | 25 |
5 files changed, 46 insertions, 58 deletions
diff --git a/synapse/events/spamcheck.py b/synapse/events/spamcheck.py index 7984874e21..1048b4c825 100644 --- a/synapse/events/spamcheck.py +++ b/synapse/events/spamcheck.py @@ -30,7 +30,7 @@ from typing import ( from synapse.api.errors import Codes from synapse.rest.media.v1._base import FileInfo from synapse.rest.media.v1.media_storage import ReadableFileWrapper -from synapse.spam_checker_api import Allow, Decision, RegistrationBehaviour +from synapse.spam_checker_api import RegistrationBehaviour from synapse.types import RoomAlias, UserProfile from synapse.util.async_helpers import delay_cancellation, maybe_awaitable from synapse.util.metrics import Measure @@ -46,12 +46,9 @@ CHECK_EVENT_FOR_SPAM_CALLBACK = Callable[ ["synapse.events.EventBase"], Awaitable[ Union[ - Allow, - Codes, + str, # Deprecated bool, - # Deprecated - str, ] ], ] @@ -178,6 +175,8 @@ def load_legacy_spam_checkers(hs: "synapse.server.HomeServer") -> None: class SpamChecker: + NOT_SPAM = "NOT_SPAM" + def __init__(self, hs: "synapse.server.HomeServer") -> None: self.hs = hs self.clock = hs.get_clock() @@ -268,9 +267,7 @@ class SpamChecker: if check_media_file_for_spam is not None: self._check_media_file_for_spam_callbacks.append(check_media_file_for_spam) - async def check_event_for_spam( - self, event: "synapse.events.EventBase" - ) -> Union[Decision, str]: + async def check_event_for_spam(self, event: "synapse.events.EventBase") -> 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 @@ -281,22 +278,20 @@ class SpamChecker: event: the event to be checked Returns: - - on `ALLOW`, the event is considered good (non-spammy) and should - be let through. Other spamcheck filters may still reject it. - - on `Code`, the event is considered spammy and is rejected with a specific + - `NOT_SPAM` if the event is considered good (non-spammy) and should be let + through. Other spamcheck filters may still reject it. + - A `Code` if the event is considered spammy and is rejected with a specific error message/code. - - on `str`, the event is considered spammy and the string is used as error - message. This usage is generally discouraged as it doesn't support - internationalization. + - A string that isn't `NOT_SPAM` if the event is considered spammy and the + string should be used as the client-facing error message. This usage is + generally discouraged as it doesn't support internationalization. """ for callback in self._check_event_for_spam_callbacks: with Measure( self.clock, "{}.{}".format(callback.__module__, callback.__qualname__) ): - res: Union[Decision, str, bool] = await delay_cancellation( - callback(event) - ) - if res is False or res is Allow.ALLOW: + res = await delay_cancellation(callback(event)) + if res is False or res == self.NOT_SPAM: # This spam-checker accepts the event. # Other spam-checkers may reject it, though. continue @@ -304,13 +299,23 @@ class SpamChecker: # This spam-checker rejects the event with deprecated # return value `True` return Codes.FORBIDDEN + elif not isinstance(res, str): + # mypy complains that we can't reach this code because of the + # return type in CHECK_EVENT_FOR_SPAM_CALLBACK, but we don't know + # for sure that the module actually returns it. + logger.warning( # type: ignore[unreachable] + "Module returned invalid value, rejecting message as spam" + ) + res = "This message has been rejected as probable spam" else: - # This spam-checker rejects the event either with a `str` - # or with a `Codes`. In either case, we stop here. - return res + # The module rejected the event either with a `Codes` + # or some other `str`. In either case, we stop here. + pass + + return res # No spam-checker has rejected the event, let it pass. - return Allow.ALLOW + return self.NOT_SPAM async def should_drop_federated_event( self, event: "synapse.events.EventBase" diff --git a/synapse/federation/federation_base.py b/synapse/federation/federation_base.py index 1e866b19d8..7bc54b9988 100644 --- a/synapse/federation/federation_base.py +++ b/synapse/federation/federation_base.py @@ -15,7 +15,6 @@ import logging from typing import TYPE_CHECKING -import synapse from synapse.api.constants import MAX_DEPTH, EventContentFields, EventTypes, Membership from synapse.api.errors import Codes, SynapseError from synapse.api.room_versions import EventFormatVersions, RoomVersion @@ -101,7 +100,7 @@ class FederationBase: spam_check = await self.spam_checker.check_event_for_spam(pdu) - if spam_check is not synapse.spam_checker_api.Allow.ALLOW: + if spam_check != self.spam_checker.NOT_SPAM: logger.warning("Event contains spam, soft-failing %s", pdu.event_id) # we redact (to save disk space) as well as soft-failing (to stop # using the event in prev_events). diff --git a/synapse/handlers/message.py b/synapse/handlers/message.py index cb1bc4c06f..22cdad3f33 100644 --- a/synapse/handlers/message.py +++ b/synapse/handlers/message.py @@ -23,7 +23,6 @@ from canonicaljson import encode_canonical_json from twisted.internet.interfaces import IDelayedCall -import synapse from synapse import event_auth from synapse.api.constants import ( EventContentFields, @@ -886,10 +885,22 @@ class EventCreationHandler: event.sender, ) - spam_check = await self.spam_checker.check_event_for_spam(event) - if spam_check is not synapse.spam_checker_api.Allow.ALLOW: + spam_check_result = await self.spam_checker.check_event_for_spam(event) + if spam_check_result != self.spam_checker.NOT_SPAM: + if isinstance(spam_check_result, Codes): + raise SynapseError( + 403, + "This message has been rejected as probable spam", + spam_check_result, + ) + + # Backwards compatibility: if the return value is not an error code, it + # means the module returned an error message to be included in the + # SynapseError (which is now deprecated). raise SynapseError( - 403, "This message had been rejected as probable spam", spam_check + 403, + spam_check_result, + Codes.FORBIDDEN, ) ev = await self.handle_new_client_event( diff --git a/synapse/module_api/__init__.py b/synapse/module_api/__init__.py index 95f3b27927..c44e9da121 100644 --- a/synapse/module_api/__init__.py +++ b/synapse/module_api/__init__.py @@ -35,7 +35,6 @@ from typing_extensions import ParamSpec from twisted.internet import defer from twisted.web.resource import Resource -from synapse import spam_checker_api from synapse.api.errors import SynapseError from synapse.events import EventBase from synapse.events.presence_router import ( @@ -55,6 +54,7 @@ from synapse.events.spamcheck import ( USER_MAY_JOIN_ROOM_CALLBACK, USER_MAY_PUBLISH_ROOM_CALLBACK, USER_MAY_SEND_3PID_INVITE_CALLBACK, + SpamChecker, ) from synapse.events.third_party_rules import ( CHECK_CAN_DEACTIVATE_USER_CALLBACK, @@ -140,9 +140,7 @@ are loaded into Synapse. """ PRESENCE_ALL_USERS = PresenceRouter.ALL_USERS - -ALLOW = spam_checker_api.Allow.ALLOW -# Singleton value used to mark a message as permitted. +NOT_SPAM = SpamChecker.NOT_SPAM __all__ = [ "errors", @@ -151,7 +149,7 @@ __all__ = [ "respond_with_html", "run_in_background", "cached", - "Allow", + "NOT_SPAM", "UserID", "DatabasePool", "LoggingTransaction", diff --git a/synapse/spam_checker_api/__init__.py b/synapse/spam_checker_api/__init__.py index 95132c80b7..75578270ac 100644 --- a/synapse/spam_checker_api/__init__.py +++ b/synapse/spam_checker_api/__init__.py @@ -12,9 +12,6 @@ # See the License for the specific language governing permissions and # limitations under the License. from enum import Enum -from typing import Union - -from synapse.api.errors import Codes class RegistrationBehaviour(Enum): @@ -25,25 +22,3 @@ class RegistrationBehaviour(Enum): ALLOW = "allow" SHADOW_BAN = "shadow_ban" DENY = "deny" - - -# We define the following singleton enum rather than a string to be able to -# write `Union[Allow, ..., str]` in some of the callbacks for the spam-checker -# API, where the `str` is required to maintain backwards compatibility with -# previous versions of the API. -class Allow(Enum): - """ - Singleton to allow events to pass through in SpamChecker APIs. - """ - - ALLOW = "allow" - - -Decision = Union[Allow, Codes] -""" -Union to define whether a request should be allowed or rejected. - -To accept a request, return `ALLOW`. - -To reject a request without any specific information, use `Codes.FORBIDDEN`. -""" |