From e3883979e8ba6130958ba35235c315e20ffadbca Mon Sep 17 00:00:00 2001 From: Brendan Abolivier Date: Tue, 25 May 2021 16:40:37 +0100 Subject: [PATCH 01/34] First cut at a standardised module interface --- docs/modules.md | 28 +++++++++++++++ docs/sample_config.yaml | 17 +++++++++ synapse/app/homeserver.py | 4 +++ synapse/config/_base.pyi | 2 ++ synapse/config/homeserver.py | 5 +-- synapse/config/modules.py | 51 ++++++++++++++++++++++++++ synapse/handlers/modules.py | 70 ++++++++++++++++++++++++++++++++++++ synapse/server.py | 9 +++-- 8 files changed, 181 insertions(+), 5 deletions(-) create mode 100644 docs/modules.md create mode 100644 synapse/config/modules.py create mode 100644 synapse/handlers/modules.py diff --git a/docs/modules.md b/docs/modules.md new file mode 100644 index 000000000000..cef4c1fb886b --- /dev/null +++ b/docs/modules.md @@ -0,0 +1,28 @@ +# Modules + +Synapse supports extending its functionalities by configuring external modules. + +## Using modules + +To use a module on Synapse, add it to the `modules` section of the configuration file: + +```yaml +modules: + - module: my_super_module.MySuperClass + config: + do_thing: true + - module: my_other_super_module.SomeClass + config: {} +``` + +Each module is defined by a path to a Python class as well as a configuration. This +information for a given module should be available in the module's own documentation. + +**Note**: When using third-party modules, you effectively allow someone else to run +custom code on your Synapse homeserver. Server admins are encouraged to verify the +provenance of the modules they use on their homeserver and make sure the modules aren't +running malicious code on their instance. + +## Writing a module + +TODO \ No newline at end of file diff --git a/docs/sample_config.yaml b/docs/sample_config.yaml index 2952f2ba3201..0b8a05cec887 100644 --- a/docs/sample_config.yaml +++ b/docs/sample_config.yaml @@ -31,6 +31,23 @@ # # [1] https://docs.ansible.com/ansible/latest/reference_appendices/YAMLSyntax.html + +## Modules ## + +# Server admins can expand Synapse's functionalities by using external modules +# to complement certain operations. +# +# See https://github.com/matrix-org/synapse/tree/master/docs/modules.md for +# more documentation on how to configure or create custom modules for Synapse. +# +modules: + # - module: my_super_module.MySuperClass + # config: + # do_thing: true + # - module: my_other_super_module.SomeClass + # config: {} + + ## Server ## # The public-facing domain of the server diff --git a/synapse/app/homeserver.py b/synapse/app/homeserver.py index b2501ee4d7f0..f5a85c486363 100644 --- a/synapse/app/homeserver.py +++ b/synapse/app/homeserver.py @@ -124,6 +124,10 @@ def _listener_http(self, config: HomeServerConfig, listener_config: ListenerConf ) resources[path] = resource + # Attach additional resources registered by modules. + module_handler = self.get_modules_handler() + resources.update(module_handler.get_registered_web_resources()) + # try to find something useful to redirect '/' to if WEB_CLIENT_PREFIX in resources: root_resource = RootOptionsRedirectResource(WEB_CLIENT_PREFIX) diff --git a/synapse/config/_base.pyi b/synapse/config/_base.pyi index ff9abbc23212..dd9e3844e512 100644 --- a/synapse/config/_base.pyi +++ b/synapse/config/_base.pyi @@ -16,6 +16,7 @@ from synapse.config import ( key, logger, metrics, + modules, oidc, password_auth_providers, push, @@ -85,6 +86,7 @@ class RootConfig: thirdpartyrules: third_party_event_rules.ThirdPartyRulesConfig tracer: tracer.TracerConfig redis: redis.RedisConfig + modules: modules.ModulesConfig config_classes: List = ... def __init__(self) -> None: ... diff --git a/synapse/config/homeserver.py b/synapse/config/homeserver.py index c23b66c88c76..f78c532ec50f 100644 --- a/synapse/config/homeserver.py +++ b/synapse/config/homeserver.py @@ -1,5 +1,4 @@ -# Copyright 2014-2016 OpenMarket Ltd -# Copyright 2018 New Vector Ltd +# Copyright 2021 The Matrix.org Foundation C.I.C. # # Licensed under the Apache License, Version 2.0 (the "License"); # you may not use this file except in compliance with the License. @@ -30,6 +29,7 @@ from .key import KeyConfig from .logger import LoggingConfig from .metrics import MetricsConfig +from .modules import ModulesConfig from .oidc import OIDCConfig from .password_auth_providers import PasswordAuthProviderConfig from .push import PushConfig @@ -56,6 +56,7 @@ class HomeServerConfig(RootConfig): config_classes = [ + ModulesConfig, ServerConfig, ExperimentalConfig, TlsConfig, diff --git a/synapse/config/modules.py b/synapse/config/modules.py new file mode 100644 index 000000000000..ed936b73cc19 --- /dev/null +++ b/synapse/config/modules.py @@ -0,0 +1,51 @@ +# Copyright 2021 The Matrix.org Foundation C.I.C. +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +from synapse.config._base import Config, ConfigError +from synapse.util.module_loader import load_module + + +class ModulesConfig(Config): + section = "modules" + + def read_config(self, config: dict, **kwargs): + self.loaded_modules = [] + + configured_modules = config.get("modules", []) + for i, module in enumerate(configured_modules): + config_path = ("modules", "" % i) + if not isinstance(module, dict): + raise ConfigError("expected a mapping", config_path) + + self.loaded_modules.append(load_module(module, config_path)) + + def generate_config_section(self, **kwargs): + return ( + """ + ## Modules ## + + # Server admins can expand Synapse's functionalities by using external modules + # to complement certain operations. + # + # See https://github.com/matrix-org/synapse/tree/master/docs/modules.md for + # more documentation on how to configure or create custom modules for Synapse. + # + modules: + # - module: my_super_module.MySuperClass + # config: + # do_thing: true + # - module: my_other_super_module.SomeClass + # config: {} + """ + ) diff --git a/synapse/handlers/modules.py b/synapse/handlers/modules.py new file mode 100644 index 000000000000..fc1f3126d7f5 --- /dev/null +++ b/synapse/handlers/modules.py @@ -0,0 +1,70 @@ +# Copyright 2021 The Matrix.org Foundation C.I.C. +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +import logging +from typing import TYPE_CHECKING, Dict + +from twisted.web.resource import IResource + +if TYPE_CHECKING: + from synapse.server import HomeServer + +logger = logging.getLogger(__name__) + + +class ModulesHandler: + def __init__(self, hs: "HomeServer"): + self.modules = [] + + module_api = hs.get_module_api() + for module, config in hs.config.modules.loaded_modules: + self.modules.append(module(config=config, api=module_api)) + + self.hooks_cache: Dict[str, list] = {} + + def get_registered_web_resources(self) -> Dict[str, IResource]: + """Retrieve the custom resources registered by each module. + + If several modules attempt to register a resource for the same path, the module + defined the highest in the configuration file takes priority. + + Returns: + A dictionary associating paths to the resources to attach to them. + """ + resources = {} + + # We reverse the list of modules so that if two modules try to register the same + # path the highest one in the configuration file takes priority. + reversed_module_list = self.modules.copy() + reversed_module_list.reverse() + for module in reversed_module_list: + if hasattr(module, "register_web_resources"): + resources.update(module.register_web_resources()) + + return resources + + def _get_modules_for_hook(self, fn_name: str) -> list: + """Get the modules implementing a hook (function) with the given name + + Returns: + A list of modules implementing the given function. + """ + if fn_name not in self.hooks_cache.keys(): + self.hooks_cache[fn_name] = [] + + for module in self.modules: + if hasattr(module, fn_name): + self.hooks_cache[fn_name].append(module) + + return self.hooks_cache[fn_name] diff --git a/synapse/server.py b/synapse/server.py index fec0024c89a5..8b68e4778e66 100644 --- a/synapse/server.py +++ b/synapse/server.py @@ -1,6 +1,4 @@ -# Copyright 2014-2016 OpenMarket Ltd -# Copyright 2017-2018 New Vector Ltd -# Copyright 2019 The Matrix.org Foundation C.I.C. +# Copyright 2021 The Matrix.org Foundation C.I.C. # # Licensed under the Apache License, Version 2.0 (the "License"); # you may not use this file except in compliance with the License. @@ -84,6 +82,7 @@ from synapse.handlers.identity import IdentityHandler from synapse.handlers.initial_sync import InitialSyncHandler from synapse.handlers.message import EventCreationHandler, MessageHandler +from synapse.handlers.modules import ModulesHandler from synapse.handlers.pagination import PaginationHandler from synapse.handlers.password_policy import PasswordPolicyHandler from synapse.handlers.presence import ( @@ -683,6 +682,10 @@ def get_server_notices_sender(self) -> WorkerServerNoticesSender: def get_message_handler(self) -> MessageHandler: return MessageHandler(self) + @cache_in_self + def get_modules_handler(self) -> ModulesHandler: + return ModulesHandler(self) + @cache_in_self def get_pagination_handler(self) -> PaginationHandler: return PaginationHandler(self) From 11f525c9cf3c2db337a288d8617e0b167388a329 Mon Sep 17 00:00:00 2001 From: Brendan Abolivier Date: Wed, 26 May 2021 11:38:21 +0100 Subject: [PATCH 02/34] Don't use a centralised handler and let modules register what they need using the module API --- synapse/app/_base.py | 6 +++ synapse/app/homeserver.py | 3 +- synapse/config/modules.py | 6 +-- synapse/handlers/modules.py | 70 ---------------------------------- synapse/module_api/__init__.py | 68 ++++++++++++++++++++++++++++++++- synapse/server.py | 5 --- 6 files changed, 76 insertions(+), 82 deletions(-) delete mode 100644 synapse/handlers/modules.py diff --git a/synapse/app/_base.py b/synapse/app/_base.py index 59918d789ec5..7fb03ffaded4 100644 --- a/synapse/app/_base.py +++ b/synapse/app/_base.py @@ -337,6 +337,12 @@ def run_sighup(*args, **kwargs): # Start the tracer synapse.logging.opentracing.init_tracer(hs) # type: ignore[attr-defined] # noqa + # Instantiate the modules so they can register their web resources to the module API + # before we start the listeners. + module_api = hs.get_module_api() + for module, config in hs.config.modules.loaded_modules: + module(config=config, api=module_api) + # It is now safe to start your Synapse. hs.start_listening() hs.get_datastore().db_pool.start_profiling() diff --git a/synapse/app/homeserver.py b/synapse/app/homeserver.py index f5a85c486363..3fdf4c8d700d 100644 --- a/synapse/app/homeserver.py +++ b/synapse/app/homeserver.py @@ -125,8 +125,7 @@ def _listener_http(self, config: HomeServerConfig, listener_config: ListenerConf resources[path] = resource # Attach additional resources registered by modules. - module_handler = self.get_modules_handler() - resources.update(module_handler.get_registered_web_resources()) + resources.update(module_api.get_registered_web_resources()) # try to find something useful to redirect '/' to if WEB_CLIENT_PREFIX in resources: diff --git a/synapse/config/modules.py b/synapse/config/modules.py index ed936b73cc19..8af2b846b41a 100644 --- a/synapse/config/modules.py +++ b/synapse/config/modules.py @@ -31,10 +31,9 @@ def read_config(self, config: dict, **kwargs): self.loaded_modules.append(load_module(module, config_path)) def generate_config_section(self, **kwargs): - return ( - """ + return """ ## Modules ## - + # Server admins can expand Synapse's functionalities by using external modules # to complement certain operations. # @@ -48,4 +47,3 @@ def generate_config_section(self, **kwargs): # - module: my_other_super_module.SomeClass # config: {} """ - ) diff --git a/synapse/handlers/modules.py b/synapse/handlers/modules.py deleted file mode 100644 index fc1f3126d7f5..000000000000 --- a/synapse/handlers/modules.py +++ /dev/null @@ -1,70 +0,0 @@ -# Copyright 2021 The Matrix.org Foundation C.I.C. -# -# Licensed under the Apache License, Version 2.0 (the "License"); -# you may not use this file except in compliance with the License. -# You may obtain a copy of the License at -# -# http://www.apache.org/licenses/LICENSE-2.0 -# -# Unless required by applicable law or agreed to in writing, software -# distributed under the License is distributed on an "AS IS" BASIS, -# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -# See the License for the specific language governing permissions and -# limitations under the License. - -import logging -from typing import TYPE_CHECKING, Dict - -from twisted.web.resource import IResource - -if TYPE_CHECKING: - from synapse.server import HomeServer - -logger = logging.getLogger(__name__) - - -class ModulesHandler: - def __init__(self, hs: "HomeServer"): - self.modules = [] - - module_api = hs.get_module_api() - for module, config in hs.config.modules.loaded_modules: - self.modules.append(module(config=config, api=module_api)) - - self.hooks_cache: Dict[str, list] = {} - - def get_registered_web_resources(self) -> Dict[str, IResource]: - """Retrieve the custom resources registered by each module. - - If several modules attempt to register a resource for the same path, the module - defined the highest in the configuration file takes priority. - - Returns: - A dictionary associating paths to the resources to attach to them. - """ - resources = {} - - # We reverse the list of modules so that if two modules try to register the same - # path the highest one in the configuration file takes priority. - reversed_module_list = self.modules.copy() - reversed_module_list.reverse() - for module in reversed_module_list: - if hasattr(module, "register_web_resources"): - resources.update(module.register_web_resources()) - - return resources - - def _get_modules_for_hook(self, fn_name: str) -> list: - """Get the modules implementing a hook (function) with the given name - - Returns: - A list of modules implementing the given function. - """ - if fn_name not in self.hooks_cache.keys(): - self.hooks_cache[fn_name] = [] - - for module in self.modules: - if hasattr(module, fn_name): - self.hooks_cache[fn_name].append(module) - - return self.hooks_cache[fn_name] diff --git a/synapse/module_api/__init__.py b/synapse/module_api/__init__.py index cecdc96bf517..9c13ad7d87bf 100644 --- a/synapse/module_api/__init__.py +++ b/synapse/module_api/__init__.py @@ -13,9 +13,20 @@ # See the License for the specific language governing permissions and # limitations under the License. import logging -from typing import TYPE_CHECKING, Any, Generator, Iterable, List, Optional, Tuple +from typing import ( + TYPE_CHECKING, + Any, + Callable, + Dict, + Generator, + Iterable, + List, + Optional, + Tuple, +) from twisted.internet import defer +from twisted.web.resource import IResource from synapse.events import EventBase from synapse.http.client import SimpleHttpClient @@ -56,6 +67,9 @@ def __init__(self, hs, auth_handler): self._http_client = hs.get_simple_http_client() # type: SimpleHttpClient self._public_room_list_manager = PublicRoomListManager(hs) + self._callbacks: Dict[str, List[Callable]] = {} + self._web_resources: Dict[str, IResource] = {} + @property def http_client(self): """Allows making outbound HTTP requests to remote resources. @@ -73,6 +87,58 @@ def public_room_list_manager(self): """ return self._public_room_list_manager + def register_hook(self, fn_name: str, callback: Callable): + """Registers the given callback as a hook with the given name. + + If multiple modules register the same hook, their callbacks will be run in the + same order as the modules appear in the configuration file (highest first). + + Args: + fn_name: The hook name. + callback: The function to call for this hook. + """ + if fn_name not in self._callbacks.keys(): + self._callbacks[fn_name] = [callback] + return + + self._callbacks[fn_name].append(callback) + + def get_registered_hook(self, fn_name: str) -> List[Callable]: + """Get the callbacks registered by modules for the given hook name. + + Args: + fn_name: The name of the hook. + + Returns: + The callbacks implementing the hook, or an empty list if there is no callback + for this hook. + """ + return self._callbacks.get(fn_name, []) + + def register_web_resource(self, path: str, resource: IResource): + """Registers a web resource to be served at the given path. + + If multiple modules register a resource for the same path, the module that + appears the highest in the configuration file takes priority. + + Args: + path: The path to register the resource for. + resource: The resource to attach to this path. + """ + # Don't register a resource that's already been registered. Like with hooks, we + # want to give priority to the module that's the highest in the configuration + # file, so in case of a conflict we keep the resource registered by this module. + if path not in self._web_resources.keys(): + self._web_resources[path] = resource + + def get_registered_web_resources(self) -> Dict[str, IResource]: + """Get the web resources registered by modules. + + Returns: + A dict associating a path with the resource to attach to it. + """ + return self._web_resources + def get_user_by_req(self, req, allow_guest=False): """Check the access_token provided for a request diff --git a/synapse/server.py b/synapse/server.py index 8b68e4778e66..710731f736e8 100644 --- a/synapse/server.py +++ b/synapse/server.py @@ -82,7 +82,6 @@ from synapse.handlers.identity import IdentityHandler from synapse.handlers.initial_sync import InitialSyncHandler from synapse.handlers.message import EventCreationHandler, MessageHandler -from synapse.handlers.modules import ModulesHandler from synapse.handlers.pagination import PaginationHandler from synapse.handlers.password_policy import PasswordPolicyHandler from synapse.handlers.presence import ( @@ -682,10 +681,6 @@ def get_server_notices_sender(self) -> WorkerServerNoticesSender: def get_message_handler(self) -> MessageHandler: return MessageHandler(self) - @cache_in_self - def get_modules_handler(self) -> ModulesHandler: - return ModulesHandler(self) - @cache_in_self def get_pagination_handler(self) -> PaginationHandler: return PaginationHandler(self) From ceb9904c28ac9f4e81cec5ff2c8d23385f3604a8 Mon Sep 17 00:00:00 2001 From: Brendan Abolivier Date: Wed, 26 May 2021 11:44:09 +0100 Subject: [PATCH 03/34] Specify where the new methods need to be called from --- synapse/module_api/__init__.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/synapse/module_api/__init__.py b/synapse/module_api/__init__.py index 9c13ad7d87bf..f4322a920ada 100644 --- a/synapse/module_api/__init__.py +++ b/synapse/module_api/__init__.py @@ -90,6 +90,8 @@ def public_room_list_manager(self): def register_hook(self, fn_name: str, callback: Callable): """Registers the given callback as a hook with the given name. + This function must be called in the __init__ method of the modules using it. + If multiple modules register the same hook, their callbacks will be run in the same order as the modules appear in the configuration file (highest first). @@ -118,6 +120,8 @@ def get_registered_hook(self, fn_name: str) -> List[Callable]: def register_web_resource(self, path: str, resource: IResource): """Registers a web resource to be served at the given path. + This function must be called in the __init__ method of the modules using it. + If multiple modules register a resource for the same path, the module that appears the highest in the configuration file takes priority. From c4d09a8ce3a3556f5b1df18f997ace647fec44fd Mon Sep 17 00:00:00 2001 From: Brendan Abolivier Date: Wed, 26 May 2021 18:14:12 +0100 Subject: [PATCH 04/34] Implement new module interface for the spam checker --- synapse/events/spamcheck.py | 235 ++++++++++++++++++++++++------------ 1 file changed, 157 insertions(+), 78 deletions(-) diff --git a/synapse/events/spamcheck.py b/synapse/events/spamcheck.py index d5fa19509498..8a1902b86665 100644 --- a/synapse/events/spamcheck.py +++ b/synapse/events/spamcheck.py @@ -15,7 +15,17 @@ import inspect import logging -from typing import TYPE_CHECKING, Any, Collection, Dict, List, Optional, Tuple, Union +from typing import ( + TYPE_CHECKING, + Any, + Callable, + Collection, + Dict, + List, + Optional, + Tuple, + Union, +) from synapse.rest.media.v1._base import FileInfo from synapse.rest.media.v1.media_storage import ReadableFileWrapper @@ -30,19 +40,118 @@ logger = logging.getLogger(__name__) -class SpamChecker: - def __init__(self, hs: "synapse.server.HomeServer"): - self.spam_checkers = [] # type: List[Any] +class LegacySpamCheckerWrapper: + """Wrapper that loads spam checkers configured using the old configuration, and + registers the spam checker hooks they implement. + """ + @staticmethod + def load_modules(hs: "synapse.server.HomeServer"): + spam_checkers = [] # type: List[Any] api = hs.get_module_api() - for module, config in hs.config.spam_checkers: # Older spam checkers don't accept the `api` argument, so we # try and detect support. spam_args = inspect.getfullargspec(module) if "api" in spam_args.args: - self.spam_checkers.append(module(config=config, api=api)) + spam_checkers.append(module(config=config, api=api)) else: - self.spam_checkers.append(module(config=config)) + spam_checkers.append(module(config=config)) + + # The known spam checker hooks. If a spam checker module implements a method + # which name appears in this set, we'll want to register it. + spam_checker_methods = { + "check_event_for_spam", + "user_may_invite", + "user_may_create_room", + "user_may_create_room_alias", + "user_may_publish_room", + "check_username_for_spam", + "check_registration_for_spam", + "check_media_file_for_spam", + } + + for spam_checker in spam_checkers: + # Get the properties (attributes and methods) of the module and filter it + # down to the supported method names. + props = dir(spam_checker) + supported_hooks = list(spam_checker_methods.intersection(props)) + + # Register the hooks through the module API. + hooks = { + hook: getattr(spam_checker, hook) + for hook in supported_hooks + } + + api.register_spam_checker_callbacks(**hooks) + + +class SpamChecker: + def __init__(self, hs: "synapse.server.HomeServer"): + self._callbacks: Dict[str, List[Callable]] = {} + + def register_callbacks( + self, + check_event_for_spam: Callable[ + ["synapse.events.EventBase"], Union[bool, str] + ] = None, + user_may_invite: Callable[[str, str, str], bool] = None, + user_may_create_room: Callable[[str], bool] = None, + user_may_create_room_alias: Callable[[str, RoomAlias], bool] = None, + user_may_publish_room: Callable[[str, str], bool] = None, + check_username_for_spam: Callable[[Dict[str, str]], bool] = None, + check_registration_for_spam: Callable[ + [Optional[dict], Optional[str], Collection[Tuple[str, str]], Optional[str]], + bool, + ] = None, + check_media_file_for_spam: Callable[ + [ReadableFileWrapper, FileInfo], bool + ] = None, + ): + """Register callbacks from module for each hook.""" + if check_event_for_spam is not None: + self._register_callback("check_event_for_spam", check_event_for_spam) + + if user_may_invite is not None: + self._register_callback("user_may_invite", user_may_invite) + + if user_may_create_room is not None: + self._register_callback("user_may_create_room", user_may_create_room) + + if user_may_create_room_alias is not None: + self._register_callback( + "user_may_create_room_alias", + user_may_create_room_alias, + ) + + if user_may_publish_room is not None: + self._register_callback("user_may_publish_room", user_may_publish_room) + + if check_username_for_spam is not None: + self._register_callback("check_username_for_spam", check_username_for_spam) + + if check_registration_for_spam is not None: + self._register_callback( + "check_registration_for_spam", + check_registration_for_spam, + ) + + if check_media_file_for_spam is not None: + self._register_callback( + "check_media_file_for_spam", + check_media_file_for_spam, + ) + + def _register_callback(self, fn_name: str, callback: Callable): + """Registers the given callback for the given hook name. + + Args: + fn_name: The hook name. + callback: The function to attach to this hook name. + """ + if fn_name not in self._callbacks.keys(): + self._callbacks[fn_name] = [] + + self._callbacks[fn_name].append(callback) async def check_event_for_spam( self, event: "synapse.events.EventBase" @@ -60,8 +169,8 @@ async def check_event_for_spam( True or a string if the event is spammy. If a string is returned it will be used as the error message returned to the user. """ - for spam_checker in self.spam_checkers: - if await maybe_awaitable(spam_checker.check_event_for_spam(event)): + for checker in self._callbacks.get("check_event_for_spam", []): + if await maybe_awaitable(checker(event)): return True return False @@ -81,13 +190,9 @@ async def user_may_invite( Returns: True if the user may send an invite, otherwise False """ - for spam_checker in self.spam_checkers: + for checker in self._callbacks.get("user_may_invite", []): if ( - await maybe_awaitable( - spam_checker.user_may_invite( - inviter_userid, invitee_userid, room_id - ) - ) + await maybe_awaitable(checker(inviter_userid, invitee_userid, room_id)) is False ): return False @@ -105,11 +210,8 @@ async def user_may_create_room(self, userid: str) -> bool: Returns: True if the user may create a room, otherwise False """ - for spam_checker in self.spam_checkers: - if ( - await maybe_awaitable(spam_checker.user_may_create_room(userid)) - is False - ): + for checker in self._callbacks.get("user_may_create_room", []): + if await maybe_awaitable(checker(userid)) is False: return False return True @@ -128,13 +230,8 @@ async def user_may_create_room_alias( Returns: True if the user may create a room alias, otherwise False """ - for spam_checker in self.spam_checkers: - if ( - await maybe_awaitable( - spam_checker.user_may_create_room_alias(userid, room_alias) - ) - is False - ): + for checker in self._callbacks.get("user_may_create_room_alias", []): + if await maybe_awaitable(checker(userid, room_alias)) is False: return False return True @@ -151,13 +248,8 @@ async def user_may_publish_room(self, userid: str, room_id: str) -> bool: Returns: True if the user may publish the room, otherwise False """ - for spam_checker in self.spam_checkers: - if ( - await maybe_awaitable( - spam_checker.user_may_publish_room(userid, room_id) - ) - is False - ): + for checker in self._callbacks.get("user_may_publish_room", []): + if await maybe_awaitable(checker(userid, room_id)) is False: return False return True @@ -177,15 +269,11 @@ async def check_username_for_spam(self, user_profile: Dict[str, str]) -> bool: Returns: True if the user is spammy. """ - for spam_checker in self.spam_checkers: - # For backwards compatibility, only run if the method exists on the - # spam checker - checker = getattr(spam_checker, "check_username_for_spam", None) - if checker: - # Make a copy of the user profile object to ensure the spam checker - # cannot modify it. - if await maybe_awaitable(checker(user_profile.copy())): - return True + for checker in self._callbacks.get("check_username_for_spam", []): + # Make a copy of the user profile object to ensure the spam checker cannot + # modify it. + if await maybe_awaitable(checker(user_profile.copy())): + return True return False @@ -211,33 +299,28 @@ async def check_registration_for_spam( Enum for how the request should be handled """ - for spam_checker in self.spam_checkers: - # For backwards compatibility, only run if the method exists on the - # spam checker - checker = getattr(spam_checker, "check_registration_for_spam", None) - if checker: - # Provide auth_provider_id if the function supports it - checker_args = inspect.signature(checker) - if len(checker_args.parameters) == 4: - d = checker( - email_threepid, - username, - request_info, - auth_provider_id, - ) - elif len(checker_args.parameters) == 3: - d = checker(email_threepid, username, request_info) - else: - logger.error( - "Invalid signature for %s.check_registration_for_spam. Denying registration", - spam_checker.__module__, - ) - return RegistrationBehaviour.DENY - - behaviour = await maybe_awaitable(d) - assert isinstance(behaviour, RegistrationBehaviour) - if behaviour != RegistrationBehaviour.ALLOW: - return behaviour + for checker in self._callbacks.get("check_registration_for_spam", []): + # Provide auth_provider_id if the function supports it + checker_args = inspect.signature(checker) + if len(checker_args.parameters) == 4: + d = checker( + email_threepid, + username, + request_info, + auth_provider_id, + ) + elif len(checker_args.parameters) == 3: + d = checker(email_threepid, username, request_info) + else: + logger.error( + "Invalid signature for check_registration_for_spam. Denying registration", + ) + return RegistrationBehaviour.DENY + + behaviour = await maybe_awaitable(d) + assert isinstance(behaviour, RegistrationBehaviour) + if behaviour != RegistrationBehaviour.ALLOW: + return behaviour return RegistrationBehaviour.ALLOW @@ -275,13 +358,9 @@ async def check_media_file_for_spam( allowed. """ - for spam_checker in self.spam_checkers: - # For backwards compatibility, only run if the method exists on the - # spam checker - checker = getattr(spam_checker, "check_media_file_for_spam", None) - if checker: - spam = await maybe_awaitable(checker(file_wrapper, file_info)) - if spam: - return True + for checker in self._callbacks.get("check_registration_for_spam", []): + spam = await maybe_awaitable(checker(file_wrapper, file_info)) + if spam: + return True return False From f5098c9113e5d20ffdc8ef0abe3a09f33795773c Mon Sep 17 00:00:00 2001 From: Brendan Abolivier Date: Wed, 26 May 2021 18:15:13 +0100 Subject: [PATCH 05/34] Don't centralise registration of hooks and web resources --- synapse/app/_base.py | 3 ++ synapse/app/homeserver.py | 2 +- synapse/config/modules.py | 2 +- synapse/module_api/__init__.py | 52 +++++----------------------------- synapse/server.py | 17 +++++++++++ 5 files changed, 29 insertions(+), 47 deletions(-) diff --git a/synapse/app/_base.py b/synapse/app/_base.py index 7fb03ffaded4..a32f64daa536 100644 --- a/synapse/app/_base.py +++ b/synapse/app/_base.py @@ -35,6 +35,7 @@ from synapse.app.phone_stats_home import start_phone_stats_home from synapse.config.homeserver import HomeServerConfig from synapse.crypto import context_factory +from synapse.events.spamcheck import LegacySpamCheckerWrapper from synapse.logging.context import PreserveLoggingContext from synapse.metrics.background_process_metrics import wrap_as_background_process from synapse.metrics.jemalloc import setup_jemalloc_stats @@ -343,6 +344,8 @@ def run_sighup(*args, **kwargs): for module, config in hs.config.modules.loaded_modules: module(config=config, api=module_api) + LegacySpamCheckerWrapper.load_modules(hs) + # It is now safe to start your Synapse. hs.start_listening() hs.get_datastore().db_pool.start_profiling() diff --git a/synapse/app/homeserver.py b/synapse/app/homeserver.py index 3fdf4c8d700d..212ddbde1e55 100644 --- a/synapse/app/homeserver.py +++ b/synapse/app/homeserver.py @@ -125,7 +125,7 @@ def _listener_http(self, config: HomeServerConfig, listener_config: ListenerConf resources[path] = resource # Attach additional resources registered by modules. - resources.update(module_api.get_registered_web_resources()) + resources.update(self._module_web_resources) # try to find something useful to redirect '/' to if WEB_CLIENT_PREFIX in resources: diff --git a/synapse/config/modules.py b/synapse/config/modules.py index 8af2b846b41a..c6c126097a20 100644 --- a/synapse/config/modules.py +++ b/synapse/config/modules.py @@ -22,7 +22,7 @@ class ModulesConfig(Config): def read_config(self, config: dict, **kwargs): self.loaded_modules = [] - configured_modules = config.get("modules", []) + configured_modules = config.get("modules") or [] for i, module in enumerate(configured_modules): config_path = ("modules", "" % i) if not isinstance(module, dict): diff --git a/synapse/module_api/__init__.py b/synapse/module_api/__init__.py index f4322a920ada..3be325a1cee7 100644 --- a/synapse/module_api/__init__.py +++ b/synapse/module_api/__init__.py @@ -53,7 +53,7 @@ class ModuleApi: can register new users etc if necessary. """ - def __init__(self, hs, auth_handler): + def __init__(self, hs: "HomeServer", auth_handler): self._hs = hs self._store = hs.get_datastore() @@ -67,8 +67,7 @@ def __init__(self, hs, auth_handler): self._http_client = hs.get_simple_http_client() # type: SimpleHttpClient self._public_room_list_manager = PublicRoomListManager(hs) - self._callbacks: Dict[str, List[Callable]] = {} - self._web_resources: Dict[str, IResource] = {} + self._spam_checker = hs.get_spam_checker() @property def http_client(self): @@ -87,35 +86,10 @@ def public_room_list_manager(self): """ return self._public_room_list_manager - def register_hook(self, fn_name: str, callback: Callable): - """Registers the given callback as a hook with the given name. - - This function must be called in the __init__ method of the modules using it. - - If multiple modules register the same hook, their callbacks will be run in the - same order as the modules appear in the configuration file (highest first). - - Args: - fn_name: The hook name. - callback: The function to call for this hook. - """ - if fn_name not in self._callbacks.keys(): - self._callbacks[fn_name] = [callback] - return - - self._callbacks[fn_name].append(callback) - - def get_registered_hook(self, fn_name: str) -> List[Callable]: - """Get the callbacks registered by modules for the given hook name. - - Args: - fn_name: The name of the hook. - - Returns: - The callbacks implementing the hook, or an empty list if there is no callback - for this hook. - """ - return self._callbacks.get(fn_name, []) + @property + def register_spam_checker_callbacks(self): + """Registers callbacks for spam checking capabilities.""" + return self._spam_checker.register_callbacks def register_web_resource(self, path: str, resource: IResource): """Registers a web resource to be served at the given path. @@ -129,19 +103,7 @@ def register_web_resource(self, path: str, resource: IResource): path: The path to register the resource for. resource: The resource to attach to this path. """ - # Don't register a resource that's already been registered. Like with hooks, we - # want to give priority to the module that's the highest in the configuration - # file, so in case of a conflict we keep the resource registered by this module. - if path not in self._web_resources.keys(): - self._web_resources[path] = resource - - def get_registered_web_resources(self) -> Dict[str, IResource]: - """Get the web resources registered by modules. - - Returns: - A dict associating a path with the resource to attach to it. - """ - return self._web_resources + self._hs.register_module_web_resource(path, resource) def get_user_by_req(self, req, allow_guest=False): """Check the access_token provided for a request diff --git a/synapse/server.py b/synapse/server.py index 710731f736e8..c9672d4b9051 100644 --- a/synapse/server.py +++ b/synapse/server.py @@ -37,6 +37,7 @@ from twisted.internet import defer from twisted.mail.smtp import sendmail from twisted.web.iweb import IPolicyForHTTPS +from twisted.web.resource import IResource from synapse.api.auth import Auth from synapse.api.filtering import Filtering @@ -257,6 +258,22 @@ def __init__( self.datastores = None # type: Optional[Databases] + self._module_web_resources: Dict[str, IResource] = {} + + def register_module_web_resource(self, path: str, resource: IResource): + """Allows a module to register a web resource to be served at the given path. + + If multiple modules register a resource for the same path, the module that + appears the highest in the configuration file takes priority. + + Args: + path: The path to register the resource for. + resource: The resource to attach to this path. + """ + # Don't register a resource that's already been registered. + if path not in self._module_web_resources.keys(): + self._module_web_resources[path] = resource + def get_instance_id(self) -> str: """A unique ID for this synapse process instance. From 7da2fd37075517fe3e63b978bff9512189a7bfb2 Mon Sep 17 00:00:00 2001 From: Brendan Abolivier Date: Wed, 26 May 2021 18:20:28 +0100 Subject: [PATCH 06/34] Don't use a class if a simple function works just as well --- synapse/app/_base.py | 4 +- synapse/events/spamcheck.py | 74 ++++++++++++++++++------------------- 2 files changed, 38 insertions(+), 40 deletions(-) diff --git a/synapse/app/_base.py b/synapse/app/_base.py index a32f64daa536..e6cc235357d2 100644 --- a/synapse/app/_base.py +++ b/synapse/app/_base.py @@ -35,7 +35,7 @@ from synapse.app.phone_stats_home import start_phone_stats_home from synapse.config.homeserver import HomeServerConfig from synapse.crypto import context_factory -from synapse.events.spamcheck import LegacySpamCheckerWrapper +from synapse.events.spamcheck import load_legacy_spam_checkers from synapse.logging.context import PreserveLoggingContext from synapse.metrics.background_process_metrics import wrap_as_background_process from synapse.metrics.jemalloc import setup_jemalloc_stats @@ -344,7 +344,7 @@ def run_sighup(*args, **kwargs): for module, config in hs.config.modules.loaded_modules: module(config=config, api=module_api) - LegacySpamCheckerWrapper.load_modules(hs) + load_legacy_spam_checkers(hs) # It is now safe to start your Synapse. hs.start_listening() diff --git a/synapse/events/spamcheck.py b/synapse/events/spamcheck.py index 8a1902b86665..f68ee246a59d 100644 --- a/synapse/events/spamcheck.py +++ b/synapse/events/spamcheck.py @@ -40,49 +40,47 @@ logger = logging.getLogger(__name__) -class LegacySpamCheckerWrapper: +def load_legacy_spam_checkers(hs: "synapse.server.HomeServer"): """Wrapper that loads spam checkers configured using the old configuration, and registers the spam checker hooks they implement. """ - @staticmethod - def load_modules(hs: "synapse.server.HomeServer"): - spam_checkers = [] # type: List[Any] - api = hs.get_module_api() - for module, config in hs.config.spam_checkers: - # Older spam checkers don't accept the `api` argument, so we - # try and detect support. - spam_args = inspect.getfullargspec(module) - if "api" in spam_args.args: - spam_checkers.append(module(config=config, api=api)) - else: - spam_checkers.append(module(config=config)) - - # The known spam checker hooks. If a spam checker module implements a method - # which name appears in this set, we'll want to register it. - spam_checker_methods = { - "check_event_for_spam", - "user_may_invite", - "user_may_create_room", - "user_may_create_room_alias", - "user_may_publish_room", - "check_username_for_spam", - "check_registration_for_spam", - "check_media_file_for_spam", + spam_checkers = [] # type: List[Any] + api = hs.get_module_api() + for module, config in hs.config.spam_checkers: + # Older spam checkers don't accept the `api` argument, so we + # try and detect support. + spam_args = inspect.getfullargspec(module) + if "api" in spam_args.args: + spam_checkers.append(module(config=config, api=api)) + else: + spam_checkers.append(module(config=config)) + + # The known spam checker hooks. If a spam checker module implements a method + # which name appears in this set, we'll want to register it. + spam_checker_methods = { + "check_event_for_spam", + "user_may_invite", + "user_may_create_room", + "user_may_create_room_alias", + "user_may_publish_room", + "check_username_for_spam", + "check_registration_for_spam", + "check_media_file_for_spam", + } + + for spam_checker in spam_checkers: + # Get the properties (attributes and methods) of the module and filter it + # down to the supported method names. + props = dir(spam_checker) + supported_hooks = list(spam_checker_methods.intersection(props)) + + # Register the hooks through the module API. + hooks = { + hook: getattr(spam_checker, hook) + for hook in supported_hooks } - for spam_checker in spam_checkers: - # Get the properties (attributes and methods) of the module and filter it - # down to the supported method names. - props = dir(spam_checker) - supported_hooks = list(spam_checker_methods.intersection(props)) - - # Register the hooks through the module API. - hooks = { - hook: getattr(spam_checker, hook) - for hook in supported_hooks - } - - api.register_spam_checker_callbacks(**hooks) + api.register_spam_checker_callbacks(**hooks) class SpamChecker: From f1c08898be56bcb6b79897c7c31ec755d7a60cab Mon Sep 17 00:00:00 2001 From: Brendan Abolivier Date: Wed, 26 May 2021 18:38:32 +0100 Subject: [PATCH 07/34] Fix CI --- changelog.d/10062.feature | 1 + synapse/events/spamcheck.py | 36 ++++++++++++++++++++---------------- 2 files changed, 21 insertions(+), 16 deletions(-) create mode 100644 changelog.d/10062.feature diff --git a/changelog.d/10062.feature b/changelog.d/10062.feature new file mode 100644 index 000000000000..97474f030c5f --- /dev/null +++ b/changelog.d/10062.feature @@ -0,0 +1 @@ +Standardised the module interface. diff --git a/synapse/events/spamcheck.py b/synapse/events/spamcheck.py index f68ee246a59d..50f2d7a2ecc2 100644 --- a/synapse/events/spamcheck.py +++ b/synapse/events/spamcheck.py @@ -75,10 +75,7 @@ def load_legacy_spam_checkers(hs: "synapse.server.HomeServer"): supported_hooks = list(spam_checker_methods.intersection(props)) # Register the hooks through the module API. - hooks = { - hook: getattr(spam_checker, hook) - for hook in supported_hooks - } + hooks = {hook: getattr(spam_checker, hook) for hook in supported_hooks} api.register_spam_checker_callbacks(**hooks) @@ -89,20 +86,27 @@ def __init__(self, hs: "synapse.server.HomeServer"): def register_callbacks( self, - check_event_for_spam: Callable[ - ["synapse.events.EventBase"], Union[bool, str] + check_event_for_spam: Optional[ + Callable[["synapse.events.EventBase"], Union[bool, str]] ] = None, - user_may_invite: Callable[[str, str, str], bool] = None, - user_may_create_room: Callable[[str], bool] = None, - user_may_create_room_alias: Callable[[str, RoomAlias], bool] = None, - user_may_publish_room: Callable[[str, str], bool] = None, - check_username_for_spam: Callable[[Dict[str, str]], bool] = None, - check_registration_for_spam: Callable[ - [Optional[dict], Optional[str], Collection[Tuple[str, str]], Optional[str]], - bool, + user_may_invite: Optional[Callable[[str, str, str], bool]] = None, + user_may_create_room: Optional[Callable[[str], bool]] = None, + user_may_create_room_alias: Optional[Callable[[str, RoomAlias], bool]] = None, + user_may_publish_room: Optional[Callable[[str, str], bool]] = None, + check_username_for_spam: Optional[Callable[[Dict[str, str]], bool]] = None, + check_registration_for_spam: Optional[ + Callable[ + [ + Optional[dict], + Optional[str], + Collection[Tuple[str, str]], + Optional[str], + ], + bool, + ] ] = None, - check_media_file_for_spam: Callable[ - [ReadableFileWrapper, FileInfo], bool + check_media_file_for_spam: Optional[ + Callable[[ReadableFileWrapper, FileInfo], bool] ] = None, ): """Register callbacks from module for each hook.""" From 817fc7501c6ae6a325bea170557b15d0b874d971 Mon Sep 17 00:00:00 2001 From: Brendan Abolivier Date: Wed, 26 May 2021 18:44:01 +0100 Subject: [PATCH 08/34] Lint --- synapse/module_api/__init__.py | 12 +----------- 1 file changed, 1 insertion(+), 11 deletions(-) diff --git a/synapse/module_api/__init__.py b/synapse/module_api/__init__.py index 3be325a1cee7..df5d46b2484b 100644 --- a/synapse/module_api/__init__.py +++ b/synapse/module_api/__init__.py @@ -13,17 +13,7 @@ # See the License for the specific language governing permissions and # limitations under the License. import logging -from typing import ( - TYPE_CHECKING, - Any, - Callable, - Dict, - Generator, - Iterable, - List, - Optional, - Tuple, -) +from typing import TYPE_CHECKING, Any, Generator, Iterable, List, Optional, Tuple from twisted.internet import defer from twisted.web.resource import IResource From a988b8c0418fbc53ffae002c966adf9b98efb7d8 Mon Sep 17 00:00:00 2001 From: Brendan Abolivier Date: Thu, 27 May 2021 17:05:02 +0100 Subject: [PATCH 09/34] Incorporate comments --- synapse/app/homeserver.py | 1 + synapse/events/spamcheck.py | 176 +++++++++++++++++++----------------- synapse/server.py | 11 +++ 3 files changed, 105 insertions(+), 83 deletions(-) diff --git a/synapse/app/homeserver.py b/synapse/app/homeserver.py index 212ddbde1e55..87cb4cf64a30 100644 --- a/synapse/app/homeserver.py +++ b/synapse/app/homeserver.py @@ -126,6 +126,7 @@ def _listener_http(self, config: HomeServerConfig, listener_config: ListenerConf # Attach additional resources registered by modules. resources.update(self._module_web_resources) + self._module_web_resources_consumed = True # try to find something useful to redirect '/' to if WEB_CLIENT_PREFIX in resources: diff --git a/synapse/events/spamcheck.py b/synapse/events/spamcheck.py index 50f2d7a2ecc2..d981667a2870 100644 --- a/synapse/events/spamcheck.py +++ b/synapse/events/spamcheck.py @@ -27,6 +27,7 @@ Union, ) +from synapse.api.errors import SynapseError from synapse.rest.media.v1._base import FileInfo from synapse.rest.media.v1.media_storage import ReadableFileWrapper from synapse.spam_checker_api import RegistrationBehaviour @@ -39,6 +40,23 @@ logger = logging.getLogger(__name__) +CHECK_EVENT_FOR_SPAM_CALLBACK = Callable[["synapse.events.EventBase"], Union[bool, str]] +USER_MAY_INVITE_CALLBACK = Callable[[str, str, str], bool] +USER_MAY_CREATE_ROOM_CALLBACK = Callable[[str], bool] +USER_MAY_CREATE_ROOM_ALIAS_CALLBACK = Callable[[str, RoomAlias], bool] +USER_MAY_PUBLISH_ROOM_CALLBACK = Callable[[str, str], bool] +CHECK_USERNAME_FOR_SPAM_CALLBACK = Callable[[Dict[str, str]], bool] +CHECK_REGISTRATION_FOR_SPAM_CALLBACK = Callable[ + [ + Optional[dict], + Optional[str], + Collection[Tuple[str, str]], + Optional[str], + ], + RegistrationBehaviour, + ] +CHECK_MEDIA_FILE_FOR_SPAM_CALLBACK = Callable[[ReadableFileWrapper, FileInfo], bool] + def load_legacy_spam_checkers(hs: "synapse.server.HomeServer"): """Wrapper that loads spam checkers configured using the old configuration, and @@ -82,78 +100,85 @@ def load_legacy_spam_checkers(hs: "synapse.server.HomeServer"): class SpamChecker: def __init__(self, hs: "synapse.server.HomeServer"): - self._callbacks: Dict[str, List[Callable]] = {} + self._check_event_for_spam_callbacks: List[CHECK_EVENT_FOR_SPAM_CALLBACK] = [] + self._user_may_invite_callbacks: List[USER_MAY_INVITE_CALLBACK] = [] + self._user_may_create_room_callbacks: List[USER_MAY_CREATE_ROOM_CALLBACK] = [] + self._user_may_create_room_alias_callbacks: List[ + USER_MAY_CREATE_ROOM_ALIAS_CALLBACK + ] = [] + self._user_may_publish_room_callbacks: List[USER_MAY_PUBLISH_ROOM_CALLBACK] = [] + self._check_username_for_spam_callbacks: List[ + CHECK_USERNAME_FOR_SPAM_CALLBACK + ] = [] + self._check_registration_for_spam_callbacks: List[ + CHECK_REGISTRATION_FOR_SPAM_CALLBACK + ] = [] + self._check_media_file_for_spam_callbacks: List[ + CHECK_MEDIA_FILE_FOR_SPAM_CALLBACK + ] = [] def register_callbacks( self, - check_event_for_spam: Optional[ - Callable[["synapse.events.EventBase"], Union[bool, str]] - ] = None, - user_may_invite: Optional[Callable[[str, str, str], bool]] = None, - user_may_create_room: Optional[Callable[[str], bool]] = None, - user_may_create_room_alias: Optional[Callable[[str, RoomAlias], bool]] = None, - user_may_publish_room: Optional[Callable[[str, str], bool]] = None, - check_username_for_spam: Optional[Callable[[Dict[str, str]], bool]] = None, + check_event_for_spam: Optional[CHECK_EVENT_FOR_SPAM_CALLBACK] = None, + user_may_invite: Optional[USER_MAY_INVITE_CALLBACK] = None, + user_may_create_room: Optional[USER_MAY_CREATE_ROOM_CALLBACK] = None, + user_may_create_room_alias: Optional[USER_MAY_CREATE_ROOM_ALIAS_CALLBACK] = None, + user_may_publish_room: Optional[USER_MAY_PUBLISH_ROOM_CALLBACK] = None, + check_username_for_spam: Optional[CHECK_USERNAME_FOR_SPAM_CALLBACK] = None, check_registration_for_spam: Optional[ - Callable[ - [ - Optional[dict], - Optional[str], - Collection[Tuple[str, str]], - Optional[str], - ], - bool, - ] - ] = None, - check_media_file_for_spam: Optional[ - Callable[[ReadableFileWrapper, FileInfo], bool] + CHECK_REGISTRATION_FOR_SPAM_CALLBACK ] = None, + check_media_file_for_spam: Optional[CHECK_MEDIA_FILE_FOR_SPAM_CALLBACK] = None, ): """Register callbacks from module for each hook.""" if check_event_for_spam is not None: - self._register_callback("check_event_for_spam", check_event_for_spam) + self._check_event_for_spam_callbacks.append(check_event_for_spam) if user_may_invite is not None: - self._register_callback("user_may_invite", user_may_invite) + self._user_may_invite_callbacks.append(user_may_invite) if user_may_create_room is not None: - self._register_callback("user_may_create_room", user_may_create_room) + self._user_may_create_room_callbacks.append(user_may_create_room) if user_may_create_room_alias is not None: - self._register_callback( - "user_may_create_room_alias", - user_may_create_room_alias, - ) + self._user_may_create_room_alias_callbacks.append(user_may_create_room_alias) if user_may_publish_room is not None: - self._register_callback("user_may_publish_room", user_may_publish_room) + self._user_may_publish_room_callbacks.append(user_may_publish_room) if check_username_for_spam is not None: - self._register_callback("check_username_for_spam", check_username_for_spam) + self._check_username_for_spam_callbacks.append(check_username_for_spam) if check_registration_for_spam is not None: - self._register_callback( - "check_registration_for_spam", - check_registration_for_spam, - ) + checker_args = inspect.signature(check_registration_for_spam) + if len(checker_args.parameters) == 4: + self._check_registration_for_spam_callbacks.append( + check_registration_for_spam, + ) + elif len(checker_args.parameters) == 3: + # Backwards compatibility; some modules might implement a hook that + # doesn't expect a 4th argument. In this case, wrap it in a function that + # gives it only 3 arguments and drops the auth_provider_id on the floor. + def wrapper( + email_threepid: Optional[dict], + username: Optional[str], + request_info: Collection[Tuple[str, str]], + auth_provider_id: Optional[str] = None, + ) -> RegistrationBehaviour: + return check_registration_for_spam( + email_threepid, + username, + request_info, + ) + + self._check_registration_for_spam_callbacks.append(wrapper) + else: + raise SynapseError( + 500, "Bad signature for callback check_registration_for_spam", + ) if check_media_file_for_spam is not None: - self._register_callback( - "check_media_file_for_spam", - check_media_file_for_spam, - ) - - def _register_callback(self, fn_name: str, callback: Callable): - """Registers the given callback for the given hook name. - - Args: - fn_name: The hook name. - callback: The function to attach to this hook name. - """ - if fn_name not in self._callbacks.keys(): - self._callbacks[fn_name] = [] - - self._callbacks[fn_name].append(callback) + self._check_media_file_for_spam_callbacks.append(check_media_file_for_spam) async def check_event_for_spam( self, event: "synapse.events.EventBase" @@ -171,8 +196,8 @@ async def check_event_for_spam( True or a string if the event is spammy. If a string is returned it will be used as the error message returned to the user. """ - for checker in self._callbacks.get("check_event_for_spam", []): - if await maybe_awaitable(checker(event)): + for callback in self._check_event_for_spam_callbacks: + if await maybe_awaitable(callback(event)): return True return False @@ -192,9 +217,9 @@ async def user_may_invite( Returns: True if the user may send an invite, otherwise False """ - for checker in self._callbacks.get("user_may_invite", []): + for callback in self._user_may_invite_callbacks: if ( - await maybe_awaitable(checker(inviter_userid, invitee_userid, room_id)) + await maybe_awaitable(callback(inviter_userid, invitee_userid, room_id)) is False ): return False @@ -212,8 +237,8 @@ async def user_may_create_room(self, userid: str) -> bool: Returns: True if the user may create a room, otherwise False """ - for checker in self._callbacks.get("user_may_create_room", []): - if await maybe_awaitable(checker(userid)) is False: + for callback in self._user_may_create_room_callbacks: + if await maybe_awaitable(callback(userid)) is False: return False return True @@ -232,8 +257,8 @@ async def user_may_create_room_alias( Returns: True if the user may create a room alias, otherwise False """ - for checker in self._callbacks.get("user_may_create_room_alias", []): - if await maybe_awaitable(checker(userid, room_alias)) is False: + for callback in self._user_may_create_room_alias_callbacks: + if await maybe_awaitable(callback(userid, room_alias)) is False: return False return True @@ -250,8 +275,8 @@ async def user_may_publish_room(self, userid: str, room_id: str) -> bool: Returns: True if the user may publish the room, otherwise False """ - for checker in self._callbacks.get("user_may_publish_room", []): - if await maybe_awaitable(checker(userid, room_id)) is False: + for callback in self._user_may_publish_room_callbacks: + if await maybe_awaitable(callback(userid, room_id)) is False: return False return True @@ -271,10 +296,10 @@ async def check_username_for_spam(self, user_profile: Dict[str, str]) -> bool: Returns: True if the user is spammy. """ - for checker in self._callbacks.get("check_username_for_spam", []): + for callback in self._check_username_for_spam_callbacks: # Make a copy of the user profile object to ensure the spam checker cannot # modify it. - if await maybe_awaitable(checker(user_profile.copy())): + if await maybe_awaitable(callback(user_profile.copy())): return True return False @@ -301,25 +326,10 @@ async def check_registration_for_spam( Enum for how the request should be handled """ - for checker in self._callbacks.get("check_registration_for_spam", []): - # Provide auth_provider_id if the function supports it - checker_args = inspect.signature(checker) - if len(checker_args.parameters) == 4: - d = checker( - email_threepid, - username, - request_info, - auth_provider_id, - ) - elif len(checker_args.parameters) == 3: - d = checker(email_threepid, username, request_info) - else: - logger.error( - "Invalid signature for check_registration_for_spam. Denying registration", - ) - return RegistrationBehaviour.DENY - - behaviour = await maybe_awaitable(d) + for callback in self._check_registration_for_spam_callbacks: + behaviour = await maybe_awaitable( + callback(email_threepid, username, request_info, auth_provider_id) + ) assert isinstance(behaviour, RegistrationBehaviour) if behaviour != RegistrationBehaviour.ALLOW: return behaviour @@ -360,8 +370,8 @@ async def check_media_file_for_spam( allowed. """ - for checker in self._callbacks.get("check_registration_for_spam", []): - spam = await maybe_awaitable(checker(file_wrapper, file_info)) + for callback in self._check_media_file_for_spam_callbacks: + spam = await maybe_awaitable(callback(file_wrapper, file_info)) if spam: return True diff --git a/synapse/server.py b/synapse/server.py index c9672d4b9051..9c0c57805cec 100644 --- a/synapse/server.py +++ b/synapse/server.py @@ -40,6 +40,7 @@ from twisted.web.resource import IResource from synapse.api.auth import Auth +from synapse.api.errors import SynapseError from synapse.api.filtering import Filtering from synapse.api.ratelimiting import Ratelimiter from synapse.appservice.api import ApplicationServiceApi @@ -259,6 +260,7 @@ def __init__( self.datastores = None # type: Optional[Databases] self._module_web_resources: Dict[str, IResource] = {} + self._module_web_resources_consumed = False def register_module_web_resource(self, path: str, resource: IResource): """Allows a module to register a web resource to be served at the given path. @@ -269,7 +271,16 @@ def register_module_web_resource(self, path: str, resource: IResource): Args: path: The path to register the resource for. resource: The resource to attach to this path. + + Raises: + SynapseError(500): A module tried to register a web resource after the HTTP + listeners have been started. """ + if self._module_web_resources_consumed: + raise SynapseError( + 500, "Tried to register a web resource from a module after startup", + ) + # Don't register a resource that's already been registered. if path not in self._module_web_resources.keys(): self._module_web_resources[path] = resource From ba4e678f9a53b56c40a3ed702f54a0dbf4ed72d1 Mon Sep 17 00:00:00 2001 From: Brendan Abolivier Date: Thu, 27 May 2021 17:05:46 +0100 Subject: [PATCH 10/34] Lint --- synapse/events/spamcheck.py | 27 ++++++++++++++++----------- synapse/server.py | 3 ++- 2 files changed, 18 insertions(+), 12 deletions(-) diff --git a/synapse/events/spamcheck.py b/synapse/events/spamcheck.py index d981667a2870..41cc7966edb5 100644 --- a/synapse/events/spamcheck.py +++ b/synapse/events/spamcheck.py @@ -47,14 +47,14 @@ USER_MAY_PUBLISH_ROOM_CALLBACK = Callable[[str, str], bool] CHECK_USERNAME_FOR_SPAM_CALLBACK = Callable[[Dict[str, str]], bool] CHECK_REGISTRATION_FOR_SPAM_CALLBACK = Callable[ - [ - Optional[dict], - Optional[str], - Collection[Tuple[str, str]], - Optional[str], - ], - RegistrationBehaviour, - ] + [ + Optional[dict], + Optional[str], + Collection[Tuple[str, str]], + Optional[str], + ], + RegistrationBehaviour, +] CHECK_MEDIA_FILE_FOR_SPAM_CALLBACK = Callable[[ReadableFileWrapper, FileInfo], bool] @@ -122,7 +122,9 @@ def register_callbacks( check_event_for_spam: Optional[CHECK_EVENT_FOR_SPAM_CALLBACK] = None, user_may_invite: Optional[USER_MAY_INVITE_CALLBACK] = None, user_may_create_room: Optional[USER_MAY_CREATE_ROOM_CALLBACK] = None, - user_may_create_room_alias: Optional[USER_MAY_CREATE_ROOM_ALIAS_CALLBACK] = None, + user_may_create_room_alias: Optional[ + USER_MAY_CREATE_ROOM_ALIAS_CALLBACK + ] = None, user_may_publish_room: Optional[USER_MAY_PUBLISH_ROOM_CALLBACK] = None, check_username_for_spam: Optional[CHECK_USERNAME_FOR_SPAM_CALLBACK] = None, check_registration_for_spam: Optional[ @@ -141,7 +143,9 @@ def register_callbacks( self._user_may_create_room_callbacks.append(user_may_create_room) if user_may_create_room_alias is not None: - self._user_may_create_room_alias_callbacks.append(user_may_create_room_alias) + self._user_may_create_room_alias_callbacks.append( + user_may_create_room_alias + ) if user_may_publish_room is not None: self._user_may_publish_room_callbacks.append(user_may_publish_room) @@ -174,7 +178,8 @@ def wrapper( self._check_registration_for_spam_callbacks.append(wrapper) else: raise SynapseError( - 500, "Bad signature for callback check_registration_for_spam", + 500, + "Bad signature for callback check_registration_for_spam", ) if check_media_file_for_spam is not None: diff --git a/synapse/server.py b/synapse/server.py index 9c0c57805cec..0b0ccd2b7a67 100644 --- a/synapse/server.py +++ b/synapse/server.py @@ -278,7 +278,8 @@ def register_module_web_resource(self, path: str, resource: IResource): """ if self._module_web_resources_consumed: raise SynapseError( - 500, "Tried to register a web resource from a module after startup", + 500, + "Tried to register a web resource from a module after startup", ) # Don't register a resource that's already been registered. From a06649c668f4de6d03e6bcf45588648017318323 Mon Sep 17 00:00:00 2001 From: Brendan Abolivier Date: Fri, 28 May 2021 09:50:34 +0100 Subject: [PATCH 11/34] Don't inhibit rejection reason from spamchecker --- synapse/events/spamcheck.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/synapse/events/spamcheck.py b/synapse/events/spamcheck.py index 41cc7966edb5..ed1b103087c5 100644 --- a/synapse/events/spamcheck.py +++ b/synapse/events/spamcheck.py @@ -202,8 +202,9 @@ async def check_event_for_spam( will be used as the error message returned to the user. """ for callback in self._check_event_for_spam_callbacks: - if await maybe_awaitable(callback(event)): - return True + res = await maybe_awaitable(callback(event)) + if res: + return res return False From d55b17b8638fcdaeaa871888156fada4439a6115 Mon Sep 17 00:00:00 2001 From: Brendan Abolivier Date: Fri, 28 May 2021 14:06:36 +0100 Subject: [PATCH 12/34] Make mypy happy --- synapse/events/spamcheck.py | 29 +++++++++++++++++++++++++---- 1 file changed, 25 insertions(+), 4 deletions(-) diff --git a/synapse/events/spamcheck.py b/synapse/events/spamcheck.py index ed1b103087c5..10b2a414f1c2 100644 --- a/synapse/events/spamcheck.py +++ b/synapse/events/spamcheck.py @@ -25,6 +25,7 @@ Optional, Tuple, Union, + cast, ) from synapse.api.errors import SynapseError @@ -46,6 +47,14 @@ USER_MAY_CREATE_ROOM_ALIAS_CALLBACK = Callable[[str, RoomAlias], bool] USER_MAY_PUBLISH_ROOM_CALLBACK = Callable[[str, str], bool] CHECK_USERNAME_FOR_SPAM_CALLBACK = Callable[[Dict[str, str]], bool] +LEGACY_CHECK_REGISTRATION_FOR_SPAM_CALLBACK = Callable[ + [ + Optional[dict], + Optional[str], + Collection[Tuple[str, str]], + ], + RegistrationBehaviour, +] CHECK_REGISTRATION_FOR_SPAM_CALLBACK = Callable[ [ Optional[dict], @@ -128,7 +137,10 @@ def register_callbacks( user_may_publish_room: Optional[USER_MAY_PUBLISH_ROOM_CALLBACK] = None, check_username_for_spam: Optional[CHECK_USERNAME_FOR_SPAM_CALLBACK] = None, check_registration_for_spam: Optional[ - CHECK_REGISTRATION_FOR_SPAM_CALLBACK + Union[ + LEGACY_CHECK_REGISTRATION_FOR_SPAM_CALLBACK, + CHECK_REGISTRATION_FOR_SPAM_CALLBACK, + ] ] = None, check_media_file_for_spam: Optional[CHECK_MEDIA_FILE_FOR_SPAM_CALLBACK] = None, ): @@ -156,20 +168,29 @@ def register_callbacks( if check_registration_for_spam is not None: checker_args = inspect.signature(check_registration_for_spam) if len(checker_args.parameters) == 4: - self._check_registration_for_spam_callbacks.append( + # Let mypy know that at this point we're pretty sure which side of the + # Union we're on. + checker = cast( + CHECK_REGISTRATION_FOR_SPAM_CALLBACK, check_registration_for_spam, ) + self._check_registration_for_spam_callbacks.append(checker) elif len(checker_args.parameters) == 3: # Backwards compatibility; some modules might implement a hook that # doesn't expect a 4th argument. In this case, wrap it in a function that # gives it only 3 arguments and drops the auth_provider_id on the floor. + legacy_checker = cast( + LEGACY_CHECK_REGISTRATION_FOR_SPAM_CALLBACK, + check_registration_for_spam, + ) + def wrapper( email_threepid: Optional[dict], username: Optional[str], request_info: Collection[Tuple[str, str]], auth_provider_id: Optional[str] = None, ) -> RegistrationBehaviour: - return check_registration_for_spam( + return legacy_checker( email_threepid, username, request_info, @@ -202,7 +223,7 @@ async def check_event_for_spam( will be used as the error message returned to the user. """ for callback in self._check_event_for_spam_callbacks: - res = await maybe_awaitable(callback(event)) + res = await maybe_awaitable(callback(event)) # type: Union[bool, str] if res: return res From 2c8d6d58c5169efd679c3402d27efddc2603a174 Mon Sep 17 00:00:00 2001 From: Brendan Abolivier Date: Fri, 28 May 2021 15:50:16 +0100 Subject: [PATCH 13/34] Fix tests --- tests/handlers/test_register.py | 108 ++++++++++++++-------- tests/handlers/test_user_directory.py | 21 ++--- tests/rest/media/v1/test_media_storage.py | 3 + 3 files changed, 84 insertions(+), 48 deletions(-) diff --git a/tests/handlers/test_register.py b/tests/handlers/test_register.py index bd4319052338..9c5998eb3a13 100644 --- a/tests/handlers/test_register.py +++ b/tests/handlers/test_register.py @@ -27,6 +27,46 @@ from .. import unittest +class TestSpamChecker: + def __init__(self, config, api): + api.register_spam_checker_callbacks( + check_registration_for_spam=self.check_registration_for_spam, + ) + + @staticmethod + def parse_config(config): + return config + + def check_registration_for_spam( + self, email_threepid, username, request_info + ): + pass + + +class DenyAll(TestSpamChecker): + def check_registration_for_spam( + self, email_threepid, username, request_info + ): + return RegistrationBehaviour.DENY + + +class BanAll(TestSpamChecker): + def check_registration_for_spam( + self, email_threepid, username, request_info + ): + return RegistrationBehaviour.SHADOW_BAN + + +class BanBadIdPUser(TestSpamChecker): + def check_registration_for_spam( + self, email_threepid, username, request_info, auth_provider_id=None + ): + # Reject any user coming from CAS and whose username contains profanity + if auth_provider_id == "cas" and "flimflob" in username: + return RegistrationBehaviour.DENY + return RegistrationBehaviour.ALLOW + + class RegistrationTestCase(unittest.HomeserverTestCase): """ Tests the RegistrationHandler. """ @@ -42,6 +82,11 @@ def make_homeserver(self, reactor, clock): hs_config["limit_usage_by_mau"] = True hs = self.setup_test_homeserver(config=hs_config) + + module_api = hs.get_module_api() + for module, config in hs.config.modules.loaded_modules: + module(config=config, api=module_api) + return hs def prepare(self, reactor, clock, hs): @@ -465,34 +510,30 @@ def test_invalid_user_id_length(self): self.handler.register_user(localpart=invalid_user_id), SynapseError ) + @override_config( + { + "modules": [ + { + "module": TestSpamChecker.__module__ + ".DenyAll", + } + ] + } + ) def test_spam_checker_deny(self): """A spam checker can deny registration, which results in an error.""" - - class DenyAll: - def check_registration_for_spam( - self, email_threepid, username, request_info - ): - return RegistrationBehaviour.DENY - - # Configure a spam checker that denies all users. - spam_checker = self.hs.get_spam_checker() - spam_checker.spam_checkers = [DenyAll()] - self.get_failure(self.handler.register_user(localpart="user"), SynapseError) + @override_config( + { + "modules": [ + { + "module": TestSpamChecker.__module__ + ".BanAll", + } + ] + } + ) def test_spam_checker_shadow_ban(self): """A spam checker can choose to shadow-ban a user, which allows registration to succeed.""" - - class BanAll: - def check_registration_for_spam( - self, email_threepid, username, request_info - ): - return RegistrationBehaviour.SHADOW_BAN - - # Configure a spam checker that denies all users. - spam_checker = self.hs.get_spam_checker() - spam_checker.spam_checkers = [BanAll()] - user_id = self.get_success(self.handler.register_user(localpart="user")) # Get an access token. @@ -512,22 +553,17 @@ def check_registration_for_spam( self.assertTrue(requester.shadow_banned) + @override_config( + { + "modules": [ + { + "module": TestSpamChecker.__module__ + ".BanBadIdPUser", + } + ] + } + ) def test_spam_checker_receives_sso_type(self): """Test rejecting registration based on SSO type""" - - class BanBadIdPUser: - def check_registration_for_spam( - self, email_threepid, username, request_info, auth_provider_id=None - ): - # Reject any user coming from CAS and whose username contains profanity - if auth_provider_id == "cas" and "flimflob" in username: - return RegistrationBehaviour.DENY - return RegistrationBehaviour.ALLOW - - # Configure a spam checker that denies a certain user on a specific IdP - spam_checker = self.hs.get_spam_checker() - spam_checker.spam_checkers = [BanBadIdPUser()] - f = self.get_failure( self.handler.register_user(localpart="bobflimflob", auth_provider_id="cas"), SynapseError, diff --git a/tests/handlers/test_user_directory.py b/tests/handlers/test_user_directory.py index daac37abd876..4cca2ced2e63 100644 --- a/tests/handlers/test_user_directory.py +++ b/tests/handlers/test_user_directory.py @@ -312,15 +312,13 @@ def test_spam_checker(self): s = self.get_success(self.handler.search_users(u1, "user2", 10)) self.assertEqual(len(s["results"]), 1) + async def allow_all(user_profile): + # Allow all users. + return False + # Configure a spam checker that does not filter any users. spam_checker = self.hs.get_spam_checker() - - class AllowAll: - async def check_username_for_spam(self, user_profile): - # Allow all users. - return False - - spam_checker.spam_checkers = [AllowAll()] + spam_checker._check_username_for_spam_callbacks = [allow_all] # The results do not change: # We get one search result when searching for user2 by user1. @@ -328,12 +326,11 @@ async def check_username_for_spam(self, user_profile): self.assertEqual(len(s["results"]), 1) # Configure a spam checker that filters all users. - class BlockAll: - async def check_username_for_spam(self, user_profile): - # All users are spammy. - return True + async def block_all( user_profile): + # All users are spammy. + return True - spam_checker.spam_checkers = [BlockAll()] + spam_checker._check_username_for_spam_callbacks = [block_all] # User1 now gets no search results for any of the other users. s = self.get_success(self.handler.search_users(u1, "user2", 10)) diff --git a/tests/rest/media/v1/test_media_storage.py b/tests/rest/media/v1/test_media_storage.py index 4a213d13ddf9..95e707584160 100644 --- a/tests/rest/media/v1/test_media_storage.py +++ b/tests/rest/media/v1/test_media_storage.py @@ -27,6 +27,7 @@ from twisted.internet import defer from twisted.internet.defer import Deferred +from synapse.events.spamcheck import load_legacy_spam_checkers from synapse.logging.context import make_deferred_yieldable from synapse.rest import admin from synapse.rest.client.v1 import login @@ -535,6 +536,8 @@ def prepare(self, reactor, clock, hs): self.download_resource = self.media_repo.children[b"download"] self.upload_resource = self.media_repo.children[b"upload"] + load_legacy_spam_checkers(hs) + def default_config(self): config = default_config("test") From 10153fce53f1e7e40e9de03d40f7b0fab47493c0 Mon Sep 17 00:00:00 2001 From: Brendan Abolivier Date: Fri, 28 May 2021 15:54:42 +0100 Subject: [PATCH 14/34] Lint --- tests/handlers/test_register.py | 12 +++--------- tests/handlers/test_user_directory.py | 2 +- 2 files changed, 4 insertions(+), 10 deletions(-) diff --git a/tests/handlers/test_register.py b/tests/handlers/test_register.py index 9c5998eb3a13..1e1f09891972 100644 --- a/tests/handlers/test_register.py +++ b/tests/handlers/test_register.py @@ -37,23 +37,17 @@ def __init__(self, config, api): def parse_config(config): return config - def check_registration_for_spam( - self, email_threepid, username, request_info - ): + def check_registration_for_spam(self, email_threepid, username, request_info): pass class DenyAll(TestSpamChecker): - def check_registration_for_spam( - self, email_threepid, username, request_info - ): + def check_registration_for_spam(self, email_threepid, username, request_info): return RegistrationBehaviour.DENY class BanAll(TestSpamChecker): - def check_registration_for_spam( - self, email_threepid, username, request_info - ): + def check_registration_for_spam(self, email_threepid, username, request_info): return RegistrationBehaviour.SHADOW_BAN diff --git a/tests/handlers/test_user_directory.py b/tests/handlers/test_user_directory.py index 4cca2ced2e63..549876dc85c0 100644 --- a/tests/handlers/test_user_directory.py +++ b/tests/handlers/test_user_directory.py @@ -326,7 +326,7 @@ async def allow_all(user_profile): self.assertEqual(len(s["results"]), 1) # Configure a spam checker that filters all users. - async def block_all( user_profile): + async def block_all(user_profile): # All users are spammy. return True From 1c9e3d46386ba93ab298f059c51088e51836cb5b Mon Sep 17 00:00:00 2001 From: Brendan Abolivier Date: Fri, 4 Jun 2021 16:48:38 +0100 Subject: [PATCH 15/34] Document the new module interface --- docs/modules.md | 195 +++++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 194 insertions(+), 1 deletion(-) diff --git a/docs/modules.md b/docs/modules.md index cef4c1fb886b..8c028384a117 100644 --- a/docs/modules.md +++ b/docs/modules.md @@ -25,4 +25,197 @@ running malicious code on their instance. ## Writing a module -TODO \ No newline at end of file +A module is a Python class that uses Synapse's module API to interact with the +homeserver. It can register callbacks that Synapse will call on specific operations, as +well as web resources to attach to Synapse's web server. + +A module must implement the following static method: + +```python +@staticmethod +def parse_config(config: dict) -> dict +``` + +This method is given a dictionary resulting from parsing the YAML configuration for the +module. It may modify it (for example by parsing durations expressed as strings (e.g. +"5d") into milliseconds, etc.), and return the modified dictionary. If no change is +necessary, this method should just return `config`. + +When instantiated, a module is given its parsed configuration (i.e. the output of +`parse_config`) as well as an instance of the `synapse.module_api.ModuleApi` class. + +See the documentation for the `ModuleApi` class [here](/synapse/module_api/__init__.py). + +### Registering a web resource + +Modules can register web resources onto Synapse's web server using the following module +API method: + +```python +def ModuleApi.register_web_resource(path: str, resource: IResource) +``` + +The path is the full absolute path to register the resource at. For example, if you +register a resource for the path `/_synapse/client/my_super_module/say_hello`, Synapse +will serve it at `https://[HS_URL]/_synapse/client/my_super_module/say_hello`. Note +that Synapse does not allow registering resources for several sub-paths in the `/_matrix` +namespace (such as anything under `/_matrix/client` for example). It is strongly +recommended that modules register their web resources under the `/_synapse/client` +namespace. + +The provided resource is a Python class that implements Twisted's [IResource](https://twistedmatrix.com/documents/current/api/twisted.web.resource.IResource.html) +interface (such as [Resource](https://twistedmatrix.com/documents/current/api/twisted.web.resource.Resource.html)). + +Only one resource can be registered for a given path. If several modules attempt to +register a resource for the same path, the module that appears first in Synapse's +configuration file takes priority. + +Modules **must** register their web resources in their `__init__` method. + +### Registering a callback + +Modules can use Synapse's module API to register callbacks. Callbacks are functions that +Synapse will call when performing specific actions. Callbacks can be either asynchronous +or synchronous, and are split in categories. A single module may implement callbacks from +multiple categories, and is under no obligation to implement all callbacks from the +category(ies) it registers callbacks for. + +#### Spam checker callbacks + +To register one of the callbacks described in this section, a module needs to use the +module API's `register_spam_checker_callbacks` method. The callback functions are passed +to `register_spam_checker_callbacks` as keyword arguments, with the callback name as the +argument name and the function as its value. This is demonstrated in the example below. + +The available spam checker callbacks are: + +```python +def check_event_for_spam(event: "synapse.events.EventBase") -> Union[bool, str] +``` + +Called when receiving an event from a client or via federation. The module can return +either a `bool` to indicate whether the event must be rejected because of spam, or a `str` +to indicate the event must be rejected because of spam and to give a rejection reason to +forward to clients. + +```python +def user_may_invite(inviter: str, invitee: str, room_id: str) -> bool +``` + +Called when processing an invitation. The module must return a `bool` indicating whether +the inviter can invite the invitee to the given room. Both inviter and invitee are +represented by their Matrix user ID (i.e. `@alice:example.com`). + +```python +def user_may_create_room(user: str) -> bool +``` + +Called when processing a room creation request. The module must return a `bool` indicating +whether the given user (represented by their Matrix user ID) is allowed to create a room. + +```python +def user_may_create_room_alias(user: str, room_alias: "synapse.types.RoomAlias") -> bool +``` + +Called when trying to associate an alias with an existing room. The module must return a +`bool` indicating whether the given user (represented by their Matrix user ID) is allowed +to set the given alias. + +```python +def user_may_publish_room(user: str, room_id: str) -> bool +``` + +Called when trying to publish a room to the homeserver's public rooms directory. The +module must return a `bool` indicating whether the given user (represented by their +Matrix user ID) is allowed to publish the given room. + +```python +def check_username_for_spam(user_profile: Dict[str, str]) -> bool +``` + +Called when computing search results in the user directory. The module must return a +`bool` indicating whether the given user profile can appear in search results. The profile +is represented as a dictionary with the following keys: + +* `user_id`: The Matrix ID for this user. +* `display_name`: The user's display name. +* `avatar_url`: The `mxc://` URL to the user's avatar. + +The module is given a copy of the original dictionary, so modifying it from within the +module cannot modify a user's profile when included in user directory search results. + +```python +def check_registration_for_spam( + email_threepid: Optional[dict], + username: Optional[str], + request_info: Collection[Tuple[str, str]], + auth_provider_id: Optional[str] = None, +) -> "synapse.spam_checker_api.RegistrationBehaviour" +``` + +Called when registering a new user. The module must return a `RegistrationBehaviour` +indicating whether the registration can go through or must be denied, or whether the user +may be allowed to register but will be shadow banned. + +The arguments passed to this callback are: + +* `email_threepid`: The email address used for registering, if any. +* `username`: The username the user would like to register. Can be `None`, meaning that + Synapse will generate one later. +* `request_info`: A collection of tuples, which first item is a user agent, and which + second item is an IP address. These user agents and IP addresses are the ones that were + used during the registration process. +* `auth_provider_id`: The identifier of the SSO authentication provider, if any. + +```python +def check_media_file_for_spam( + file_wrapper: "synapse.rest.media.v1.media_storage.ReadableFileWrapper", + file_info: "synapse.rest.media.v1._base.FileInfo" +) -> bool +``` + +Called when storing a local or remote file. The module must return a boolean indicating +whether the given file can be stored in the homeserver's media store. + +### Example + +```python +import json + +from twisted.web.resource import Resource +from twisted.web.server import Request + +from synapse.module_api import ModuleApi + + +class DemoResource(Resource): + def __init__(self, config): + super(DemoResource, self).__init__() + self.config = config + + async def render_GET(self, request: Request): + name = request.args.get(b"name")[0] + return json.dumps({"name": name}) + + +class DemoModule: + def __init__(self, config: dict, api: ModuleApi): + self.config = config + self.api = api + + self.api.register_web_resource( + path="/_synapse/client/demo/hello", + resource=DemoResource(self.config), + ) + + self.api.register_spam_checker_callbacks( + user_may_create_room=self.user_may_create_room, + ) + + @staticmethod + def parse_config(config): + return config + + async def user_may_create_room(self, userid: str) -> bool: + pass +``` From b92965cdb7df04a1289d76c0f835812cac0b4a1f Mon Sep 17 00:00:00 2001 From: Brendan Abolivier Date: Fri, 4 Jun 2021 16:56:38 +0100 Subject: [PATCH 16/34] Add new doc to the summary, and add a deprecation notice to the spam checker doc --- docs/SUMMARY.md | 2 +- docs/spam_checker.md | 4 ++++ 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/docs/SUMMARY.md b/docs/SUMMARY.md index 8f39ae027001..1b571730bf9f 100644 --- a/docs/SUMMARY.md +++ b/docs/SUMMARY.md @@ -35,7 +35,7 @@ - [URL Previews](url_previews.md) - [User Directory](user_directory.md) - [Message Retention Policies](message_retention_policies.md) - - [Pluggable Modules]() + - [Pluggable Modules](modules.md) - [Third Party Rules]() - [Spam Checker](spam_checker.md) - [Presence Router](presence_router_module.md) diff --git a/docs/spam_checker.md b/docs/spam_checker.md index 52947f605e13..c16914e61d83 100644 --- a/docs/spam_checker.md +++ b/docs/spam_checker.md @@ -1,3 +1,7 @@ +**Note: this page of the Synapse documentation is now deprecated. For up to date +documentation on setting up or writing a spam checker module, please see +[this page](https://matrix-org.github.io/synapse/develop/modules.html).** + # Handling spam in Synapse Synapse has support to customize spam checking behavior. It can plug into a From d440297df147dd7d3623faf7f1d5d7d978c6eb31 Mon Sep 17 00:00:00 2001 From: Brendan Abolivier Date: Fri, 4 Jun 2021 16:56:58 +0100 Subject: [PATCH 17/34] Fix a typo in registration docs --- synapse/handlers/register.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/synapse/handlers/register.py b/synapse/handlers/register.py index 4ceef3fab3d9..ca1ed6a5c077 100644 --- a/synapse/handlers/register.py +++ b/synapse/handlers/register.py @@ -195,7 +195,7 @@ async def register_user( bind_emails: list of emails to bind to this account. by_admin: True if this registration is being made via the admin api, otherwise False. - user_agent_ips: Tuples of IP addresses and user-agents used + user_agent_ips: Tuples of user-agents and IP addresses used during the registration process. auth_provider_id: The SSO IdP the user used, if any. Returns: From ce4347b3abecb70b29abede4c33212246ac381cb Mon Sep 17 00:00:00 2001 From: Brendan Abolivier Date: Fri, 4 Jun 2021 17:02:34 +0100 Subject: [PATCH 18/34] Point to the new docs in the sample configuration --- docs/sample_config.yaml | 4 ++-- synapse/config/modules.py | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/docs/sample_config.yaml b/docs/sample_config.yaml index d7d04bc50822..1f7627402c8f 100644 --- a/docs/sample_config.yaml +++ b/docs/sample_config.yaml @@ -37,8 +37,8 @@ # Server admins can expand Synapse's functionalities by using external modules # to complement certain operations. # -# See https://github.com/matrix-org/synapse/tree/master/docs/modules.md for -# more documentation on how to configure or create custom modules for Synapse. +# See https://matrix-org.github.io/synapse/develop/modules.html for more +# documentation on how to configure or create custom modules for Synapse. # modules: # - module: my_super_module.MySuperClass diff --git a/synapse/config/modules.py b/synapse/config/modules.py index c6c126097a20..030ba648ff28 100644 --- a/synapse/config/modules.py +++ b/synapse/config/modules.py @@ -37,8 +37,8 @@ def generate_config_section(self, **kwargs): # Server admins can expand Synapse's functionalities by using external modules # to complement certain operations. # - # See https://github.com/matrix-org/synapse/tree/master/docs/modules.md for - # more documentation on how to configure or create custom modules for Synapse. + # See https://matrix-org.github.io/synapse/develop/modules.html for more + # documentation on how to configure or create custom modules for Synapse. # modules: # - module: my_super_module.MySuperClass From 79ee96701a7634b7ca09f10f634a65dd1cc65d29 Mon Sep 17 00:00:00 2001 From: Brendan Abolivier Date: Fri, 4 Jun 2021 17:14:16 +0100 Subject: [PATCH 19/34] Improve example --- docs/modules.md | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/docs/modules.md b/docs/modules.md index 8c028384a117..4fc8cd4e9434 100644 --- a/docs/modules.md +++ b/docs/modules.md @@ -57,7 +57,7 @@ def ModuleApi.register_web_resource(path: str, resource: IResource) The path is the full absolute path to register the resource at. For example, if you register a resource for the path `/_synapse/client/my_super_module/say_hello`, Synapse -will serve it at `https://[HS_URL]/_synapse/client/my_super_module/say_hello`. Note +will serve it at `http(s)://[HS_URL]/_synapse/client/my_super_module/say_hello`. Note that Synapse does not allow registering resources for several sub-paths in the `/_matrix` namespace (such as anything under `/_matrix/client` for example). It is strongly recommended that modules register their web resources under the `/_synapse/client` @@ -179,6 +179,10 @@ whether the given file can be stored in the homeserver's media store. ### Example +The example below is a module that implements the spam checker callback +`user_may_create_room` to deny room creation to user `@evilguy:example.com`, and registers +a web resource to the path `/_synapse/client/demo/hello` that returns a JSON object. + ```python import json @@ -195,7 +199,8 @@ class DemoResource(Resource): async def render_GET(self, request: Request): name = request.args.get(b"name")[0] - return json.dumps({"name": name}) + request.setHeader(b"Content-Type", b"application/json") + return json.dumps({"hello": name}) class DemoModule: @@ -216,6 +221,9 @@ class DemoModule: def parse_config(config): return config - async def user_may_create_room(self, userid: str) -> bool: - pass + async def user_may_create_room(self, user: str) -> bool: + if user == "@evilguy:example.com": + return False + + return True ``` From 7bf8fdb63bfc9172af5542d3a09fc671ef454d5e Mon Sep 17 00:00:00 2001 From: Brendan Abolivier Date: Wed, 16 Jun 2021 16:51:38 +0200 Subject: [PATCH 20/34] Apply suggestions from code review Co-authored-by: Richard van der Hoff <1389908+richvdh@users.noreply.github.com> --- docs/modules.md | 4 ++-- synapse/config/modules.py | 3 +-- synapse/module_api/__init__.py | 2 +- 3 files changed, 4 insertions(+), 5 deletions(-) diff --git a/docs/modules.md b/docs/modules.md index 4fc8cd4e9434..baeb97c91c05 100644 --- a/docs/modules.md +++ b/docs/modules.md @@ -1,6 +1,6 @@ # Modules -Synapse supports extending its functionalities by configuring external modules. +Synapse supports extending its functionality by configuring external modules. ## Using modules @@ -78,7 +78,7 @@ Modules can use Synapse's module API to register callbacks. Callbacks are functi Synapse will call when performing specific actions. Callbacks can be either asynchronous or synchronous, and are split in categories. A single module may implement callbacks from multiple categories, and is under no obligation to implement all callbacks from the -category(ies) it registers callbacks for. +categories it registers callbacks for. #### Spam checker callbacks diff --git a/synapse/config/modules.py b/synapse/config/modules.py index 030ba648ff28..803fa7818476 100644 --- a/synapse/config/modules.py +++ b/synapse/config/modules.py @@ -34,8 +34,7 @@ def generate_config_section(self, **kwargs): return """ ## Modules ## - # Server admins can expand Synapse's functionalities by using external modules - # to complement certain operations. + # Server admins can expand Synapse's functionality with external modules. # # See https://matrix-org.github.io/synapse/develop/modules.html for more # documentation on how to configure or create custom modules for Synapse. diff --git a/synapse/module_api/__init__.py b/synapse/module_api/__init__.py index df5d46b2484b..2ecd0e0c9d7b 100644 --- a/synapse/module_api/__init__.py +++ b/synapse/module_api/__init__.py @@ -84,7 +84,7 @@ def register_spam_checker_callbacks(self): def register_web_resource(self, path: str, resource: IResource): """Registers a web resource to be served at the given path. - This function must be called in the __init__ method of the modules using it. + This function should be called during initialisation of the module. If multiple modules register a resource for the same path, the module that appears the highest in the configuration file takes priority. From c6ed0495af64b85a0afa4b3b8810f29e3e033e74 Mon Sep 17 00:00:00 2001 From: Brendan Abolivier Date: Wed, 16 Jun 2021 19:10:31 +0100 Subject: [PATCH 21/34] Incorporate review comments --- CHANGES.md | 8 +++++ UPGRADE.rst | 18 +++++++++++ docs/modules.md | 55 ++++++++++++++++++++++++++-------- docs/sample_config.yaml | 16 +--------- synapse/app/generic_worker.py | 4 +++ synapse/config/modules.py | 3 +- synapse/config/spam_checker.py | 15 ---------- synapse/events/spamcheck.py | 51 ++++++++++++++++++------------- synapse/module_api/__init__.py | 38 +++++++++++++---------- synapse/module_api/errors.py | 1 + synapse/server.py | 12 +++++--- synapse/util/module_loader.py | 33 ++++++++++---------- 12 files changed, 155 insertions(+), 99 deletions(-) diff --git a/CHANGES.md b/CHANGES.md index 0f9798a4d3b7..cf7908ec148d 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -1,3 +1,11 @@ +Unreleased +========== + +The spam checker interface is now deprecated in favour of a new generic module system and +will be removed in Synapse 1.40 (planned for mid-August 2021). See the +[upgrade notes](https://github.com/matrix-org/synapse/blob/develop/UPGRADE.rst#deprecation-of-the-current-spam-checker-interface) +for more info. + Synapse 1.36.0 (2021-06-15) =========================== diff --git a/UPGRADE.rst b/UPGRADE.rst index 9f61aad4120d..2bae2058520e 100644 --- a/UPGRADE.rst +++ b/UPGRADE.rst @@ -85,6 +85,24 @@ for example: wget https://packages.matrix.org/debian/pool/main/m/matrix-synapse-py3/matrix-synapse-py3_1.3.0+stretch1_amd64.deb dpkg -i matrix-synapse-py3_1.3.0+stretch1_amd64.deb +Upgrading to v1.37.0 +==================== + +Deprecation of the current spam checker interface +------------------------------------------------- + +The current spam checker interface is deprecated in favour of a new generic modules system. +Authors of spam checker modules can refer to `this documentation `_ +to update their modules. Synapse administrators can refer to `this documentation `_ +to update their configuration once the modules they are using have been updated. + +Support for the current spam checker interface will be removed in Synapse 1.40 (planned for +mid-August 2021). + +More module interfaces will be ported over to this new generic system in future versions +of Synapse. + + Upgrading to v1.34.0 ==================== diff --git a/docs/modules.md b/docs/modules.md index baeb97c91c05..d64ec4493de8 100644 --- a/docs/modules.md +++ b/docs/modules.md @@ -23,13 +23,28 @@ custom code on your Synapse homeserver. Server admins are encouraged to verify t provenance of the modules they use on their homeserver and make sure the modules aren't running malicious code on their instance. +Also note that we are currently in the process of migrating module interfaces to this +system. While some interfaces might be compatible with it, others still require +configuring modules in another part of Synapse's configuration file. Currently, only the +spam checker interface is compatible with this new system. + ## Writing a module A module is a Python class that uses Synapse's module API to interact with the homeserver. It can register callbacks that Synapse will call on specific operations, as well as web resources to attach to Synapse's web server. -A module must implement the following static method: +When instantiated, a module is given its parsed configuration as well as an instance of +the `synapse.module_api.ModuleApi` class. The configuration is a dictionary, and is +either the output of the module's `parse_config` static method (see below), or the +configuration associated with the module in Synapse's configuration file. + +See the documentation for the `ModuleApi` class +[here](https://github.com/matrix-org/synapse/blob/master/synapse/module_api/__init__.py). + +### Handling the module's configuration + +A module can implement the following static method: ```python @staticmethod @@ -38,13 +53,9 @@ def parse_config(config: dict) -> dict This method is given a dictionary resulting from parsing the YAML configuration for the module. It may modify it (for example by parsing durations expressed as strings (e.g. -"5d") into milliseconds, etc.), and return the modified dictionary. If no change is -necessary, this method should just return `config`. - -When instantiated, a module is given its parsed configuration (i.e. the output of -`parse_config`) as well as an instance of the `synapse.module_api.ModuleApi` class. - -See the documentation for the `ModuleApi` class [here](/synapse/module_api/__init__.py). +"5d") into milliseconds, etc.), and return the modified dictionary. It may also verify +that the configuration is correct, and raise an instance of +`synapse.module_api.errors.ConfigError` if not. ### Registering a web resource @@ -75,10 +86,10 @@ Modules **must** register their web resources in their `__init__` method. ### Registering a callback Modules can use Synapse's module API to register callbacks. Callbacks are functions that -Synapse will call when performing specific actions. Callbacks can be either asynchronous -or synchronous, and are split in categories. A single module may implement callbacks from -multiple categories, and is under no obligation to implement all callbacks from the -categories it registers callbacks for. +Synapse will call when performing specific actions. Callbacks must be asynchronous, and +are split in categories. A single module may implement callbacks from multiple categories, +and is under no obligation to implement all callbacks from the categories it registers +callbacks for. #### Spam checker callbacks @@ -177,6 +188,24 @@ def check_media_file_for_spam( Called when storing a local or remote file. The module must return a boolean indicating whether the given file can be stored in the homeserver's media store. +### Porting an existing module that uses the old interface + +In order to port a module that uses Synapse's old module interface, its author needs to: + +* ensure the module's callbacks are all asynchronous. +* register their callbacks using one or more of the `register_[...]_callbacks` methods + from the `ModuleApi` class in the module's `__init__` method (see [this section](#registering-a-web-resource) + for more info). + +Additionally, if the module is packaged with an additional web resource, the module +should register this resource in its `__init__` method using the `register_web_resource` +method from the `ModuleApi` class (see [this section](#registering-a-web-resource) for +more info). + +The module's author should also update any example in the module's configuration to only +use the new `modules` section in Synapse's configuration file (see [this section](#using-modules) +for more info). + ### Example The example below is a module that implements the spam checker callback @@ -197,7 +226,7 @@ class DemoResource(Resource): super(DemoResource, self).__init__() self.config = config - async def render_GET(self, request: Request): + def render_GET(self, request: Request): name = request.args.get(b"name")[0] request.setHeader(b"Content-Type", b"application/json") return json.dumps({"hello": name}) diff --git a/docs/sample_config.yaml b/docs/sample_config.yaml index 6863b6e7c735..edb994566155 100644 --- a/docs/sample_config.yaml +++ b/docs/sample_config.yaml @@ -34,8 +34,7 @@ ## Modules ## -# Server admins can expand Synapse's functionalities by using external modules -# to complement certain operations. +# Server admins can expand Synapse's functionality with external modules. # # See https://matrix-org.github.io/synapse/develop/modules.html for more # documentation on how to configure or create custom modules for Synapse. @@ -2586,19 +2585,6 @@ push: #group_unread_count_by_room: false -# Spam checkers are third-party modules that can block specific actions -# of local users, such as creating rooms and registering undesirable -# usernames, as well as remote users by redacting incoming events. -# -spam_checker: - #- module: "my_custom_project.SuperSpamChecker" - # config: - # example_option: 'things' - #- module: "some_other_project.BadEventStopper" - # config: - # example_stop_events_from: ['@bad:example.com'] - - ## Rooms ## # Controls whether locally-created rooms should be end-to-end encrypted by diff --git a/synapse/app/generic_worker.py b/synapse/app/generic_worker.py index 57c2fc2e88b2..8e648c6ee077 100644 --- a/synapse/app/generic_worker.py +++ b/synapse/app/generic_worker.py @@ -354,6 +354,10 @@ def _listen_http(self, listener_config: ListenerConfig): if name == "replication": resources[REPLICATION_PREFIX] = ReplicationRestResource(self) + # Attach additional resources registered by modules. + resources.update(self._module_web_resources) + self._module_web_resources_consumed = True + root_resource = create_resource_tree(resources, OptionsResource()) _base.listen_tcp( diff --git a/synapse/config/modules.py b/synapse/config/modules.py index 803fa7818476..3209e1c492b6 100644 --- a/synapse/config/modules.py +++ b/synapse/config/modules.py @@ -11,6 +11,7 @@ # WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. # See the License for the specific language governing permissions and # limitations under the License. +from typing import Any, Dict, List, Tuple from synapse.config._base import Config, ConfigError from synapse.util.module_loader import load_module @@ -20,7 +21,7 @@ class ModulesConfig(Config): section = "modules" def read_config(self, config: dict, **kwargs): - self.loaded_modules = [] + self.loaded_modules: List[Tuple[Any, Dict]] = [] configured_modules = config.get("modules") or [] for i, module in enumerate(configured_modules): diff --git a/synapse/config/spam_checker.py b/synapse/config/spam_checker.py index 447ba3303b3a..c24165eb8a88 100644 --- a/synapse/config/spam_checker.py +++ b/synapse/config/spam_checker.py @@ -42,18 +42,3 @@ def read_config(self, config, **kwargs): self.spam_checkers.append(load_module(spam_checker, config_path)) else: raise ConfigError("spam_checker syntax is incorrect") - - def generate_config_section(self, **kwargs): - return """\ - # Spam checkers are third-party modules that can block specific actions - # of local users, such as creating rooms and registering undesirable - # usernames, as well as remote users by redacting incoming events. - # - spam_checker: - #- module: "my_custom_project.SuperSpamChecker" - # config: - # example_option: 'things' - #- module: "some_other_project.BadEventStopper" - # config: - # example_stop_events_from: ['@bad:example.com'] - """ diff --git a/synapse/events/spamcheck.py b/synapse/events/spamcheck.py index 10b2a414f1c2..b380e806f692 100644 --- a/synapse/events/spamcheck.py +++ b/synapse/events/spamcheck.py @@ -18,6 +18,7 @@ from typing import ( TYPE_CHECKING, Any, + Awaitable, Callable, Collection, Dict, @@ -28,7 +29,6 @@ cast, ) -from synapse.api.errors import SynapseError from synapse.rest.media.v1._base import FileInfo from synapse.rest.media.v1.media_storage import ReadableFileWrapper from synapse.spam_checker_api import RegistrationBehaviour @@ -41,19 +41,22 @@ logger = logging.getLogger(__name__) -CHECK_EVENT_FOR_SPAM_CALLBACK = Callable[["synapse.events.EventBase"], Union[bool, str]] -USER_MAY_INVITE_CALLBACK = Callable[[str, str, str], bool] -USER_MAY_CREATE_ROOM_CALLBACK = Callable[[str], bool] -USER_MAY_CREATE_ROOM_ALIAS_CALLBACK = Callable[[str, RoomAlias], bool] -USER_MAY_PUBLISH_ROOM_CALLBACK = Callable[[str, str], bool] -CHECK_USERNAME_FOR_SPAM_CALLBACK = Callable[[Dict[str, str]], bool] +CHECK_EVENT_FOR_SPAM_CALLBACK = Callable[ + ["synapse.events.EventBase"], + Awaitable[Union[bool, str]], +] +USER_MAY_INVITE_CALLBACK = Callable[[str, str, str], Awaitable[bool]] +USER_MAY_CREATE_ROOM_CALLBACK = Callable[[str], Awaitable[bool]] +USER_MAY_CREATE_ROOM_ALIAS_CALLBACK = Callable[[str, RoomAlias], Awaitable[bool]] +USER_MAY_PUBLISH_ROOM_CALLBACK = Callable[[str, str], Awaitable[bool]] +CHECK_USERNAME_FOR_SPAM_CALLBACK = Callable[[Dict[str, str]], Awaitable[bool]] LEGACY_CHECK_REGISTRATION_FOR_SPAM_CALLBACK = Callable[ [ Optional[dict], Optional[str], Collection[Tuple[str, str]], ], - RegistrationBehaviour, + Awaitable[RegistrationBehaviour], ] CHECK_REGISTRATION_FOR_SPAM_CALLBACK = Callable[ [ @@ -62,9 +65,12 @@ Collection[Tuple[str, str]], Optional[str], ], - RegistrationBehaviour, + Awaitable[RegistrationBehaviour], +] +CHECK_MEDIA_FILE_FOR_SPAM_CALLBACK = Callable[ + [ReadableFileWrapper, FileInfo], + Awaitable[bool], ] -CHECK_MEDIA_FILE_FOR_SPAM_CALLBACK = Callable[[ReadableFileWrapper, FileInfo], bool] def load_legacy_spam_checkers(hs: "synapse.server.HomeServer"): @@ -96,19 +102,25 @@ def load_legacy_spam_checkers(hs: "synapse.server.HomeServer"): } for spam_checker in spam_checkers: - # Get the properties (attributes and methods) of the module and filter it - # down to the supported method names. - props = dir(spam_checker) - supported_hooks = list(spam_checker_methods.intersection(props)) + # Methods on legacy spam checkers might not be async, so we wrap them around a + # wrapper that will call maybe_awaitable on the result. + def async_wrapper(f: Callable) -> Callable[..., Awaitable]: + def run(*args, **kwargs): + return maybe_awaitable(f(*args, **kwargs)) + + return run # Register the hooks through the module API. - hooks = {hook: getattr(spam_checker, hook) for hook in supported_hooks} + hooks = { + hook: async_wrapper(getattr(spam_checker, hook, None)) + for hook in spam_checker_methods + } api.register_spam_checker_callbacks(**hooks) class SpamChecker: - def __init__(self, hs: "synapse.server.HomeServer"): + def __init__(self): self._check_event_for_spam_callbacks: List[CHECK_EVENT_FOR_SPAM_CALLBACK] = [] self._user_may_invite_callbacks: List[USER_MAY_INVITE_CALLBACK] = [] self._user_may_create_room_callbacks: List[USER_MAY_CREATE_ROOM_CALLBACK] = [] @@ -188,8 +200,8 @@ def wrapper( email_threepid: Optional[dict], username: Optional[str], request_info: Collection[Tuple[str, str]], - auth_provider_id: Optional[str] = None, - ) -> RegistrationBehaviour: + auth_provider_id: Optional[str], + ) -> Awaitable[RegistrationBehaviour]: return legacy_checker( email_threepid, username, @@ -198,8 +210,7 @@ def wrapper( self._check_registration_for_spam_callbacks.append(wrapper) else: - raise SynapseError( - 500, + raise RuntimeError( "Bad signature for callback check_registration_for_spam", ) diff --git a/synapse/module_api/__init__.py b/synapse/module_api/__init__.py index 2ecd0e0c9d7b..58b255eb1b28 100644 --- a/synapse/module_api/__init__.py +++ b/synapse/module_api/__init__.py @@ -59,22 +59,8 @@ def __init__(self, hs: "HomeServer", auth_handler): self._spam_checker = hs.get_spam_checker() - @property - def http_client(self): - """Allows making outbound HTTP requests to remote resources. - - An instance of synapse.http.client.SimpleHttpClient - """ - return self._http_client - - @property - def public_room_list_manager(self): - """Allows adding to, removing from and checking the status of rooms in the - public room list. - - An instance of synapse.module_api.PublicRoomListManager - """ - return self._public_room_list_manager + ################################################################################# + # The following methods should only be called during the module's initialisation. @property def register_spam_checker_callbacks(self): @@ -95,6 +81,26 @@ def register_web_resource(self, path: str, resource: IResource): """ self._hs.register_module_web_resource(path, resource) + ######################################################################### + # The following methods can be called by the module at any point in time. + + @property + def http_client(self): + """Allows making outbound HTTP requests to remote resources. + + An instance of synapse.http.client.SimpleHttpClient + """ + return self._http_client + + @property + def public_room_list_manager(self): + """Allows adding to, removing from and checking the status of rooms in the + public room list. + + An instance of synapse.module_api.PublicRoomListManager + """ + return self._public_room_list_manager + def get_user_by_req(self, req, allow_guest=False): """Check the access_token provided for a request diff --git a/synapse/module_api/errors.py b/synapse/module_api/errors.py index d24864c5492a..02bbb0be39fc 100644 --- a/synapse/module_api/errors.py +++ b/synapse/module_api/errors.py @@ -15,3 +15,4 @@ """Exception types which are exposed as part of the stable module API""" from synapse.api.errors import RedirectException, SynapseError # noqa: F401 +from synapse.config._base import ConfigError # noqa: F401 diff --git a/synapse/server.py b/synapse/server.py index 0b0ccd2b7a67..39ff29023169 100644 --- a/synapse/server.py +++ b/synapse/server.py @@ -40,7 +40,6 @@ from twisted.web.resource import IResource from synapse.api.auth import Auth -from synapse.api.errors import SynapseError from synapse.api.filtering import Filtering from synapse.api.ratelimiting import Ratelimiter from synapse.appservice.api import ApplicationServiceApi @@ -277,14 +276,19 @@ def register_module_web_resource(self, path: str, resource: IResource): listeners have been started. """ if self._module_web_resources_consumed: - raise SynapseError( - 500, + raise RuntimeError( "Tried to register a web resource from a module after startup", ) # Don't register a resource that's already been registered. if path not in self._module_web_resources.keys(): self._module_web_resources[path] = resource + else: + logger.warning( + "Module tried to register a web resource for path %s but another module" + " has already registered a resource for this path.", + path, + ) def get_instance_id(self) -> str: """A unique ID for this synapse process instance. @@ -678,7 +682,7 @@ def get_stats_handler(self) -> StatsHandler: @cache_in_self def get_spam_checker(self) -> SpamChecker: - return SpamChecker(self) + return SpamChecker() @cache_in_self def get_third_party_event_rules(self) -> ThirdPartyEventRules: diff --git a/synapse/util/module_loader.py b/synapse/util/module_loader.py index cbfbd097f9a1..3cae8b18de2f 100644 --- a/synapse/util/module_loader.py +++ b/synapse/util/module_loader.py @@ -51,21 +51,24 @@ def load_module(provider: dict, config_path: Iterable[str]) -> Tuple[Type, Any]: # Load the module config. If None, pass an empty dictionary instead module_config = provider.get("config") or {} - try: - provider_config = provider_class.parse_config(module_config) - except jsonschema.ValidationError as e: - raise json_error_to_config_error(e, itertools.chain(config_path, ("config",))) - except ConfigError as e: - raise _wrap_config_error( - "Failed to parse config for module %r" % (modulename,), - prefix=itertools.chain(config_path, ("config",)), - e=e, - ) - except Exception as e: - raise ConfigError( - "Failed to parse config for module %r" % (modulename,), - path=itertools.chain(config_path, ("config",)), - ) from e + if hasattr(provider_class, "parse_config"): + try: + provider_config = provider_class.parse_config(module_config) + except jsonschema.ValidationError as e: + raise json_error_to_config_error(e, itertools.chain(config_path, ("config",))) + except ConfigError as e: + raise _wrap_config_error( + "Failed to parse config for module %r" % (modulename,), + prefix=itertools.chain(config_path, ("config",)), + e=e, + ) + except Exception as e: + raise ConfigError( + "Failed to parse config for module %r" % (modulename,), + path=itertools.chain(config_path, ("config",)), + ) from e + else: + provider_config = module_config return provider_class, provider_config From 39a02b1918f2371f6e452d75e7df6d094cafb0b8 Mon Sep 17 00:00:00 2001 From: Brendan Abolivier Date: Thu, 17 Jun 2021 11:09:02 +0100 Subject: [PATCH 22/34] Lint --- synapse/events/spamcheck.py | 19 ++++++++----------- synapse/util/module_loader.py | 4 +++- 2 files changed, 11 insertions(+), 12 deletions(-) diff --git a/synapse/events/spamcheck.py b/synapse/events/spamcheck.py index b380e806f692..39b1462144ed 100644 --- a/synapse/events/spamcheck.py +++ b/synapse/events/spamcheck.py @@ -234,7 +234,7 @@ async def check_event_for_spam( will be used as the error message returned to the user. """ for callback in self._check_event_for_spam_callbacks: - res = await maybe_awaitable(callback(event)) # type: Union[bool, str] + res = await callback(event) # type: Union[bool, str] if res: return res @@ -256,10 +256,7 @@ async def user_may_invite( True if the user may send an invite, otherwise False """ for callback in self._user_may_invite_callbacks: - if ( - await maybe_awaitable(callback(inviter_userid, invitee_userid, room_id)) - is False - ): + if await callback(inviter_userid, invitee_userid, room_id) is False: return False return True @@ -276,7 +273,7 @@ async def user_may_create_room(self, userid: str) -> bool: True if the user may create a room, otherwise False """ for callback in self._user_may_create_room_callbacks: - if await maybe_awaitable(callback(userid)) is False: + if await callback(userid) is False: return False return True @@ -296,7 +293,7 @@ async def user_may_create_room_alias( True if the user may create a room alias, otherwise False """ for callback in self._user_may_create_room_alias_callbacks: - if await maybe_awaitable(callback(userid, room_alias)) is False: + if await callback(userid, room_alias) is False: return False return True @@ -314,7 +311,7 @@ async def user_may_publish_room(self, userid: str, room_id: str) -> bool: True if the user may publish the room, otherwise False """ for callback in self._user_may_publish_room_callbacks: - if await maybe_awaitable(callback(userid, room_id)) is False: + if await callback(userid, room_id) is False: return False return True @@ -337,7 +334,7 @@ async def check_username_for_spam(self, user_profile: Dict[str, str]) -> bool: for callback in self._check_username_for_spam_callbacks: # Make a copy of the user profile object to ensure the spam checker cannot # modify it. - if await maybe_awaitable(callback(user_profile.copy())): + if await callback(user_profile.copy()): return True return False @@ -365,7 +362,7 @@ async def check_registration_for_spam( """ for callback in self._check_registration_for_spam_callbacks: - behaviour = await maybe_awaitable( + behaviour = await ( callback(email_threepid, username, request_info, auth_provider_id) ) assert isinstance(behaviour, RegistrationBehaviour) @@ -409,7 +406,7 @@ async def check_media_file_for_spam( """ for callback in self._check_media_file_for_spam_callbacks: - spam = await maybe_awaitable(callback(file_wrapper, file_info)) + spam = await callback(file_wrapper, file_info) if spam: return True diff --git a/synapse/util/module_loader.py b/synapse/util/module_loader.py index 3cae8b18de2f..5a638c6e9a48 100644 --- a/synapse/util/module_loader.py +++ b/synapse/util/module_loader.py @@ -55,7 +55,9 @@ def load_module(provider: dict, config_path: Iterable[str]) -> Tuple[Type, Any]: try: provider_config = provider_class.parse_config(module_config) except jsonschema.ValidationError as e: - raise json_error_to_config_error(e, itertools.chain(config_path, ("config",))) + raise json_error_to_config_error( + e, itertools.chain(config_path, ("config",)) + ) except ConfigError as e: raise _wrap_config_error( "Failed to parse config for module %r" % (modulename,), From 8e28b3e427e49b8d37fc5eeea245b1af16062504 Mon Sep 17 00:00:00 2001 From: Brendan Abolivier Date: Thu, 17 Jun 2021 13:10:09 +0100 Subject: [PATCH 23/34] Use async callbacks in tests --- tests/handlers/test_register.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/tests/handlers/test_register.py b/tests/handlers/test_register.py index 1e1f09891972..a2a8bb0768a6 100644 --- a/tests/handlers/test_register.py +++ b/tests/handlers/test_register.py @@ -37,22 +37,22 @@ def __init__(self, config, api): def parse_config(config): return config - def check_registration_for_spam(self, email_threepid, username, request_info): + async def check_registration_for_spam(self, email_threepid, username, request_info): pass class DenyAll(TestSpamChecker): - def check_registration_for_spam(self, email_threepid, username, request_info): + async def check_registration_for_spam(self, email_threepid, username, request_info): return RegistrationBehaviour.DENY class BanAll(TestSpamChecker): - def check_registration_for_spam(self, email_threepid, username, request_info): + async def check_registration_for_spam(self, email_threepid, username, request_info): return RegistrationBehaviour.SHADOW_BAN class BanBadIdPUser(TestSpamChecker): - def check_registration_for_spam( + async def check_registration_for_spam( self, email_threepid, username, request_info, auth_provider_id=None ): # Reject any user coming from CAS and whose username contains profanity From 9c5bffd72490435ff84a037445072a5935ddb232 Mon Sep 17 00:00:00 2001 From: Brendan Abolivier Date: Thu, 17 Jun 2021 15:37:16 +0100 Subject: [PATCH 24/34] Correctly wrap check_registration_for_spam --- synapse/events/spamcheck.py | 40 +++++++++++++++++++++++++++++++++++-- 1 file changed, 38 insertions(+), 2 deletions(-) diff --git a/synapse/events/spamcheck.py b/synapse/events/spamcheck.py index 39b1462144ed..2a263b527d52 100644 --- a/synapse/events/spamcheck.py +++ b/synapse/events/spamcheck.py @@ -104,11 +104,47 @@ def load_legacy_spam_checkers(hs: "synapse.server.HomeServer"): for spam_checker in spam_checkers: # Methods on legacy spam checkers might not be async, so we wrap them around a # wrapper that will call maybe_awaitable on the result. - def async_wrapper(f: Callable) -> Callable[..., Awaitable]: + def async_wrapper(f: Callable) -> Optional[Callable[..., Awaitable]]: + # f might be None if the callback isn't implemented by the module. In this + # case we don't want to register a callback at all so we return None. + if f is None: + return None + def run(*args, **kwargs): return maybe_awaitable(f(*args, **kwargs)) - return run + wrapper = run + + # register_spam_checker_callbacks does additional checks on the number of + # arguments for check_registration_for_spam, so we can't just pass run, + # instead we use another wrapper with the right number of arguments. + if f.__name__ == "check_registration_for_spam": + def wrap_legacy_registration_checker( + email_threepid: Optional[dict], + username: Optional[str], + request_info: Collection[Tuple[str, str]], + ): + return run(email_threepid, username, request_info) + + def wrap_registration_checker( + email_threepid: Optional[dict], + username: Optional[str], + request_info: Collection[Tuple[str, str]], + auth_provider_id: Optional[str], + ): + return run(email_threepid, username, request_info, auth_provider_id) + + checker_args = inspect.signature(f) + if len(checker_args.parameters) == 4: + wrapper = wrap_registration_checker + elif len(checker_args.parameters) == 3: + wrapper = wrap_legacy_registration_checker + else: + raise RuntimeError( + "Bad signature for callback check_registration_for_spam", + ) + + return wrapper # Register the hooks through the module API. hooks = { From 468b9000e750df2e5ad6871199ed852b569dc504 Mon Sep 17 00:00:00 2001 From: Brendan Abolivier Date: Thu, 17 Jun 2021 15:52:38 +0100 Subject: [PATCH 25/34] Lint --- synapse/events/spamcheck.py | 1 + 1 file changed, 1 insertion(+) diff --git a/synapse/events/spamcheck.py b/synapse/events/spamcheck.py index 2a263b527d52..ab279aa6c31f 100644 --- a/synapse/events/spamcheck.py +++ b/synapse/events/spamcheck.py @@ -119,6 +119,7 @@ def run(*args, **kwargs): # arguments for check_registration_for_spam, so we can't just pass run, # instead we use another wrapper with the right number of arguments. if f.__name__ == "check_registration_for_spam": + def wrap_legacy_registration_checker( email_threepid: Optional[dict], username: Optional[str], From 5a9f3918af579d67c9c159d71ccf5f4924353dc1 Mon Sep 17 00:00:00 2001 From: Brendan Abolivier Date: Fri, 18 Jun 2021 10:11:38 +0100 Subject: [PATCH 26/34] Move support for 3-arg check_registration_for_spam to legacy code --- synapse/events/spamcheck.py | 103 +++++++++++------------------------- 1 file changed, 31 insertions(+), 72 deletions(-) diff --git a/synapse/events/spamcheck.py b/synapse/events/spamcheck.py index ab279aa6c31f..a8c1f957417b 100644 --- a/synapse/events/spamcheck.py +++ b/synapse/events/spamcheck.py @@ -15,6 +15,7 @@ import inspect import logging +import functools from typing import ( TYPE_CHECKING, Any, @@ -104,48 +105,41 @@ def load_legacy_spam_checkers(hs: "synapse.server.HomeServer"): for spam_checker in spam_checkers: # Methods on legacy spam checkers might not be async, so we wrap them around a # wrapper that will call maybe_awaitable on the result. - def async_wrapper(f: Callable) -> Optional[Callable[..., Awaitable]]: + def async_wrapper(f: Optional[Callable]) -> Optional[Callable[..., Awaitable]]: # f might be None if the callback isn't implemented by the module. In this # case we don't want to register a callback at all so we return None. if f is None: return None - def run(*args, **kwargs): - return maybe_awaitable(f(*args, **kwargs)) - - wrapper = run - - # register_spam_checker_callbacks does additional checks on the number of - # arguments for check_registration_for_spam, so we can't just pass run, - # instead we use another wrapper with the right number of arguments. if f.__name__ == "check_registration_for_spam": - - def wrap_legacy_registration_checker( - email_threepid: Optional[dict], - username: Optional[str], - request_info: Collection[Tuple[str, str]], - ): - return run(email_threepid, username, request_info) - - def wrap_registration_checker( - email_threepid: Optional[dict], - username: Optional[str], - request_info: Collection[Tuple[str, str]], - auth_provider_id: Optional[str], - ): - return run(email_threepid, username, request_info, auth_provider_id) - checker_args = inspect.signature(f) - if len(checker_args.parameters) == 4: - wrapper = wrap_registration_checker - elif len(checker_args.parameters) == 3: - wrapper = wrap_legacy_registration_checker - else: + if len(checker_args.parameters) == 3: + # Backwards compatibility; some modules might implement a hook that + # doesn't expect a 4th argument. In this case, wrap it in a function + # that gives it only 3 arguments and drops the auth_provider_id on + # the floor. + def wrapper( + email_threepid: Optional[dict], + username: Optional[str], + request_info: Collection[Tuple[str, str]], + auth_provider_id: Optional[str], + ) -> Union[Awaitable[RegistrationBehaviour], RegistrationBehaviour]: + return f( + email_threepid, + username, + request_info, + ) + + f = wrapper + elif len(checker_args.parameters) != 4: raise RuntimeError( "Bad signature for callback check_registration_for_spam", ) - return wrapper + def run(*args, **kwargs): + return maybe_awaitable(f(*args, **kwargs)) + + return run # Register the hooks through the module API. hooks = { @@ -181,15 +175,12 @@ def register_callbacks( user_may_invite: Optional[USER_MAY_INVITE_CALLBACK] = None, user_may_create_room: Optional[USER_MAY_CREATE_ROOM_CALLBACK] = None, user_may_create_room_alias: Optional[ - USER_MAY_CREATE_ROOM_ALIAS_CALLBACK + USER_MAY_CREATE_ROOM_ALIAS_CALLBACK, ] = None, user_may_publish_room: Optional[USER_MAY_PUBLISH_ROOM_CALLBACK] = None, check_username_for_spam: Optional[CHECK_USERNAME_FOR_SPAM_CALLBACK] = None, check_registration_for_spam: Optional[ - Union[ - LEGACY_CHECK_REGISTRATION_FOR_SPAM_CALLBACK, - CHECK_REGISTRATION_FOR_SPAM_CALLBACK, - ] + CHECK_REGISTRATION_FOR_SPAM_CALLBACK, ] = None, check_media_file_for_spam: Optional[CHECK_MEDIA_FILE_FOR_SPAM_CALLBACK] = None, ): @@ -205,7 +196,7 @@ def register_callbacks( if user_may_create_room_alias is not None: self._user_may_create_room_alias_callbacks.append( - user_may_create_room_alias + user_may_create_room_alias, ) if user_may_publish_room is not None: @@ -215,41 +206,9 @@ def register_callbacks( self._check_username_for_spam_callbacks.append(check_username_for_spam) if check_registration_for_spam is not None: - checker_args = inspect.signature(check_registration_for_spam) - if len(checker_args.parameters) == 4: - # Let mypy know that at this point we're pretty sure which side of the - # Union we're on. - checker = cast( - CHECK_REGISTRATION_FOR_SPAM_CALLBACK, - check_registration_for_spam, - ) - self._check_registration_for_spam_callbacks.append(checker) - elif len(checker_args.parameters) == 3: - # Backwards compatibility; some modules might implement a hook that - # doesn't expect a 4th argument. In this case, wrap it in a function that - # gives it only 3 arguments and drops the auth_provider_id on the floor. - legacy_checker = cast( - LEGACY_CHECK_REGISTRATION_FOR_SPAM_CALLBACK, - check_registration_for_spam, - ) - - def wrapper( - email_threepid: Optional[dict], - username: Optional[str], - request_info: Collection[Tuple[str, str]], - auth_provider_id: Optional[str], - ) -> Awaitable[RegistrationBehaviour]: - return legacy_checker( - email_threepid, - username, - request_info, - ) - - self._check_registration_for_spam_callbacks.append(wrapper) - else: - raise RuntimeError( - "Bad signature for callback check_registration_for_spam", - ) + self._check_registration_for_spam_callbacks.append( + check_registration_for_spam, + ) if check_media_file_for_spam is not None: self._check_media_file_for_spam_callbacks.append(check_media_file_for_spam) From 6a326f9efceb352f44f5ff17a7ebffe27e423715 Mon Sep 17 00:00:00 2001 From: Brendan Abolivier Date: Fri, 18 Jun 2021 10:14:45 +0100 Subject: [PATCH 27/34] Remove unused import --- synapse/events/spamcheck.py | 1 - 1 file changed, 1 deletion(-) diff --git a/synapse/events/spamcheck.py b/synapse/events/spamcheck.py index a8c1f957417b..156eff727798 100644 --- a/synapse/events/spamcheck.py +++ b/synapse/events/spamcheck.py @@ -15,7 +15,6 @@ import inspect import logging -import functools from typing import ( TYPE_CHECKING, Any, From 575556f6b0de47a64ddb6b7eb6466205717434ea Mon Sep 17 00:00:00 2001 From: Brendan Abolivier Date: Fri, 18 Jun 2021 10:15:05 +0100 Subject: [PATCH 28/34] Remove other unused import --- synapse/events/spamcheck.py | 1 - 1 file changed, 1 deletion(-) diff --git a/synapse/events/spamcheck.py b/synapse/events/spamcheck.py index 156eff727798..3a81ba6db261 100644 --- a/synapse/events/spamcheck.py +++ b/synapse/events/spamcheck.py @@ -26,7 +26,6 @@ Optional, Tuple, Union, - cast, ) from synapse.rest.media.v1._base import FileInfo From 12774dcc1c770c90bf87353d97a0eb543b801dfc Mon Sep 17 00:00:00 2001 From: Brendan Abolivier Date: Fri, 18 Jun 2021 10:39:56 +0100 Subject: [PATCH 29/34] Explicitely type legacy callback as not None --- synapse/events/spamcheck.py | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/synapse/events/spamcheck.py b/synapse/events/spamcheck.py index 3a81ba6db261..ab78dc19501d 100644 --- a/synapse/events/spamcheck.py +++ b/synapse/events/spamcheck.py @@ -26,6 +26,7 @@ Optional, Tuple, Union, + cast, ) from synapse.rest.media.v1._base import FileInfo @@ -122,6 +123,11 @@ def wrapper( request_info: Collection[Tuple[str, str]], auth_provider_id: Optional[str], ) -> Union[Awaitable[RegistrationBehaviour], RegistrationBehaviour]: + # We've already made sure f is not None above, but mypy doesn't + # do well across function boundaries so we need to tell it f is + # definitely not None. + assert f is not None + return f( email_threepid, username, @@ -135,6 +141,11 @@ def wrapper( ) def run(*args, **kwargs): + # We've already made sure f is not None above, but mypy doesn't do well + # across function boundaries so we need to tell it f is definitely not + # None. + assert f is not None + return maybe_awaitable(f(*args, **kwargs)) return run From b12855c7396b5ea135732bfc6ba7c0fb1291b2af Mon Sep 17 00:00:00 2001 From: Brendan Abolivier Date: Fri, 18 Jun 2021 10:43:03 +0100 Subject: [PATCH 30/34] Don't import cast again --- synapse/events/spamcheck.py | 1 - 1 file changed, 1 deletion(-) diff --git a/synapse/events/spamcheck.py b/synapse/events/spamcheck.py index ab78dc19501d..bf57c605835b 100644 --- a/synapse/events/spamcheck.py +++ b/synapse/events/spamcheck.py @@ -26,7 +26,6 @@ Optional, Tuple, Union, - cast, ) from synapse.rest.media.v1._base import FileInfo From cd596f5dec2ebb8a0d61aff01611981b54388adc Mon Sep 17 00:00:00 2001 From: Brendan Abolivier Date: Fri, 18 Jun 2021 10:47:21 +0100 Subject: [PATCH 31/34] Be more vague in upgrade notes and add deprecation notice to changelog --- UPGRADE.rst | 3 +-- changelog.d/10062.removal | 1 + 2 files changed, 2 insertions(+), 2 deletions(-) create mode 100644 changelog.d/10062.removal diff --git a/UPGRADE.rst b/UPGRADE.rst index 2bae2058520e..22e1e0fbd8b3 100644 --- a/UPGRADE.rst +++ b/UPGRADE.rst @@ -96,8 +96,7 @@ Authors of spam checker modules can refer to `this documentation `_ to update their configuration once the modules they are using have been updated. -Support for the current spam checker interface will be removed in Synapse 1.40 (planned for -mid-August 2021). +Support for the current spam checker interface will be removed in August 2021. More module interfaces will be ported over to this new generic system in future versions of Synapse. diff --git a/changelog.d/10062.removal b/changelog.d/10062.removal new file mode 100644 index 000000000000..7f0cbdae2e57 --- /dev/null +++ b/changelog.d/10062.removal @@ -0,0 +1 @@ +The current spam checker interface is deprecated in favour of a new generic modules system. See the [upgrade notes](https://github.com/matrix-org/synapse/blob/master/UPGRADE.rst#deprecation-of-the-current-spam-checker-interface) for more information on how to update to the new system. \ No newline at end of file From 3a28f6a718b9c32fb3bfee54f35553500660909b Mon Sep 17 00:00:00 2001 From: Brendan Abolivier Date: Fri, 18 Jun 2021 10:48:44 +0100 Subject: [PATCH 32/34] Phrasing --- UPGRADE.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/UPGRADE.rst b/UPGRADE.rst index 22e1e0fbd8b3..cc088d0ccc47 100644 --- a/UPGRADE.rst +++ b/UPGRADE.rst @@ -96,7 +96,7 @@ Authors of spam checker modules can refer to `this documentation `_ to update their configuration once the modules they are using have been updated. -Support for the current spam checker interface will be removed in August 2021. +We will remove support for the current spam checker interface in August 2021. More module interfaces will be ported over to this new generic system in future versions of Synapse. From 387d41b3c8c18f853f4ec71f21011c5958dc1e60 Mon Sep 17 00:00:00 2001 From: Brendan Abolivier Date: Fri, 18 Jun 2021 11:28:08 +0100 Subject: [PATCH 33/34] Types don't like commas --- synapse/events/spamcheck.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/synapse/events/spamcheck.py b/synapse/events/spamcheck.py index bf57c605835b..45ec96dfc116 100644 --- a/synapse/events/spamcheck.py +++ b/synapse/events/spamcheck.py @@ -183,12 +183,12 @@ def register_callbacks( user_may_invite: Optional[USER_MAY_INVITE_CALLBACK] = None, user_may_create_room: Optional[USER_MAY_CREATE_ROOM_CALLBACK] = None, user_may_create_room_alias: Optional[ - USER_MAY_CREATE_ROOM_ALIAS_CALLBACK, + USER_MAY_CREATE_ROOM_ALIAS_CALLBACK ] = None, user_may_publish_room: Optional[USER_MAY_PUBLISH_ROOM_CALLBACK] = None, check_username_for_spam: Optional[CHECK_USERNAME_FOR_SPAM_CALLBACK] = None, check_registration_for_spam: Optional[ - CHECK_REGISTRATION_FOR_SPAM_CALLBACK, + CHECK_REGISTRATION_FOR_SPAM_CALLBACK ] = None, check_media_file_for_spam: Optional[CHECK_MEDIA_FILE_FOR_SPAM_CALLBACK] = None, ): From 249c60741ca91c6a315beb486ebf7dd6936f9a97 Mon Sep 17 00:00:00 2001 From: Brendan Abolivier Date: Fri, 18 Jun 2021 11:48:32 +0100 Subject: [PATCH 34/34] Fix tests and phrasing --- CHANGES.md | 8 -------- UPGRADE.rst | 2 +- tests/handlers/test_register.py | 24 +++++++++++++++++++++--- 3 files changed, 22 insertions(+), 12 deletions(-) diff --git a/CHANGES.md b/CHANGES.md index cf7908ec148d..0f9798a4d3b7 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -1,11 +1,3 @@ -Unreleased -========== - -The spam checker interface is now deprecated in favour of a new generic module system and -will be removed in Synapse 1.40 (planned for mid-August 2021). See the -[upgrade notes](https://github.com/matrix-org/synapse/blob/develop/UPGRADE.rst#deprecation-of-the-current-spam-checker-interface) -for more info. - Synapse 1.36.0 (2021-06-15) =========================== diff --git a/UPGRADE.rst b/UPGRADE.rst index cc088d0ccc47..ee8b4fa60b14 100644 --- a/UPGRADE.rst +++ b/UPGRADE.rst @@ -96,7 +96,7 @@ Authors of spam checker modules can refer to `this documentation `_ to update their configuration once the modules they are using have been updated. -We will remove support for the current spam checker interface in August 2021. +We plan to remove support for the current spam checker interface in August 2021. More module interfaces will be ported over to this new generic system in future versions of Synapse. diff --git a/tests/handlers/test_register.py b/tests/handlers/test_register.py index 04bb147c0241..a9fd3036dca5 100644 --- a/tests/handlers/test_register.py +++ b/tests/handlers/test_register.py @@ -37,17 +37,35 @@ def __init__(self, config, api): def parse_config(config): return config - async def check_registration_for_spam(self, email_threepid, username, request_info): + async def check_registration_for_spam( + self, + email_threepid, + username, + request_info, + auth_provider_id, + ): pass class DenyAll(TestSpamChecker): - async def check_registration_for_spam(self, email_threepid, username, request_info): + async def check_registration_for_spam( + self, + email_threepid, + username, + request_info, + auth_provider_id, + ): return RegistrationBehaviour.DENY class BanAll(TestSpamChecker): - async def check_registration_for_spam(self, email_threepid, username, request_info): + async def check_registration_for_spam( + self, + email_threepid, + username, + request_info, + auth_provider_id, + ): return RegistrationBehaviour.SHADOW_BAN