From 11846dff8c667cbe6861ddc821ca7c53e3e2d890 Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Wed, 9 Jun 2021 07:05:32 -0400 Subject: Limit the number of in-flight /keys/query requests from a single device. (#10144) --- tests/handlers/test_e2e_keys.py | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) (limited to 'tests/handlers') diff --git a/tests/handlers/test_e2e_keys.py b/tests/handlers/test_e2e_keys.py index 61a00130b8..e0a24824cc 100644 --- a/tests/handlers/test_e2e_keys.py +++ b/tests/handlers/test_e2e_keys.py @@ -257,7 +257,9 @@ class E2eKeysHandlerTestCase(unittest.HomeserverTestCase): self.get_success(self.handler.upload_signing_keys_for_user(local_user, keys2)) devices = self.get_success( - self.handler.query_devices({"device_keys": {local_user: []}}, 0, local_user) + self.handler.query_devices( + {"device_keys": {local_user: []}}, 0, local_user, "device123" + ) ) self.assertDictEqual(devices["master_keys"], {local_user: keys2["master_key"]}) @@ -357,7 +359,9 @@ class E2eKeysHandlerTestCase(unittest.HomeserverTestCase): device_key_1["signatures"][local_user]["ed25519:abc"] = "base64+signature" device_key_2["signatures"][local_user]["ed25519:def"] = "base64+signature" devices = self.get_success( - self.handler.query_devices({"device_keys": {local_user: []}}, 0, local_user) + self.handler.query_devices( + {"device_keys": {local_user: []}}, 0, local_user, "device123" + ) ) del devices["device_keys"][local_user]["abc"]["unsigned"] del devices["device_keys"][local_user]["def"]["unsigned"] @@ -591,7 +595,10 @@ class E2eKeysHandlerTestCase(unittest.HomeserverTestCase): # fetch the signed keys/devices and make sure that the signatures are there ret = self.get_success( self.handler.query_devices( - {"device_keys": {local_user: [], other_user: []}}, 0, local_user + {"device_keys": {local_user: [], other_user: []}}, + 0, + local_user, + "device123", ) ) -- cgit 1.5.1 From 8070b893dbc7b9e17b4843f70e4f4e32a52ce58a Mon Sep 17 00:00:00 2001 From: Marcus Date: Thu, 17 Jun 2021 16:20:06 +0200 Subject: update black to 21.6b0 (#10197) Reformat all files with the new version. Signed-off-by: Marcus Hoffmann --- changelog.d/10197.misc | 1 + contrib/experiments/cursesio.py | 8 ++++---- setup.py | 2 +- synapse/handlers/federation.py | 2 +- synapse/http/servlet.py | 2 +- synapse/replication/tcp/handler.py | 2 +- synapse/types.py | 4 ++-- tests/handlers/test_appservice.py | 2 +- tests/handlers/test_directory.py | 2 +- tests/handlers/test_profile.py | 2 +- tests/handlers/test_register.py | 2 +- tests/handlers/test_sync.py | 2 +- tests/rest/client/v1/test_events.py | 2 +- tests/rest/client/v1/test_presence.py | 2 +- tests/rest/client/v1/test_rooms.py | 16 ++++++++-------- tests/rest/client/v1/test_typing.py | 2 +- tests/storage/test_base.py | 2 +- 17 files changed, 28 insertions(+), 27 deletions(-) create mode 100644 changelog.d/10197.misc (limited to 'tests/handlers') diff --git a/changelog.d/10197.misc b/changelog.d/10197.misc new file mode 100644 index 0000000000..cbb3b454be --- /dev/null +++ b/changelog.d/10197.misc @@ -0,0 +1 @@ +Upgrade `black` linting tool to 21.6b0. diff --git a/contrib/experiments/cursesio.py b/contrib/experiments/cursesio.py index cff73650e6..7695cc77ca 100644 --- a/contrib/experiments/cursesio.py +++ b/contrib/experiments/cursesio.py @@ -46,14 +46,14 @@ class CursesStdIO: self.callback = callback def fileno(self): - """ We want to select on FD 0 """ + """We want to select on FD 0""" return 0 def connectionLost(self, reason): self.close() def print_line(self, text): - """ add a line to the internal list of lines""" + """add a line to the internal list of lines""" self.lines.append(text) self.redraw() @@ -92,7 +92,7 @@ class CursesStdIO: ) def doRead(self): - """ Input is ready! """ + """Input is ready!""" curses.noecho() c = self.stdscr.getch() # read a character @@ -132,7 +132,7 @@ class CursesStdIO: return "CursesStdIO" def close(self): - """ clean up """ + """clean up""" curses.nocbreak() self.stdscr.keypad(0) diff --git a/setup.py b/setup.py index e2e488761d..1081548e00 100755 --- a/setup.py +++ b/setup.py @@ -97,7 +97,7 @@ CONDITIONAL_REQUIREMENTS["all"] = list(ALL_OPTIONAL_REQUIREMENTS) # We pin black so that our tests don't start failing on new releases. CONDITIONAL_REQUIREMENTS["lint"] = [ "isort==5.7.0", - "black==20.8b1", + "black==21.6b0", "flake8-comprehensions", "flake8-bugbear==21.3.2", "flake8", diff --git a/synapse/handlers/federation.py b/synapse/handlers/federation.py index 1ecdafaadd..0bfb25802a 100644 --- a/synapse/handlers/federation.py +++ b/synapse/handlers/federation.py @@ -1961,7 +1961,7 @@ class FederationHandler(BaseHandler): return event async def on_send_leave_request(self, origin: str, pdu: EventBase) -> None: - """ We have received a leave event for a room. Fully process it.""" + """We have received a leave event for a room. Fully process it.""" event = pdu logger.debug( diff --git a/synapse/http/servlet.py b/synapse/http/servlet.py index 3c43f32586..fda8da21b7 100644 --- a/synapse/http/servlet.py +++ b/synapse/http/servlet.py @@ -454,7 +454,7 @@ class RestServlet: """ def register(self, http_server): - """ Register this servlet with the given HTTP server. """ + """Register this servlet with the given HTTP server.""" patterns = getattr(self, "PATTERNS", None) if patterns: for method in ("GET", "PUT", "POST", "DELETE"): diff --git a/synapse/replication/tcp/handler.py b/synapse/replication/tcp/handler.py index 7ced4c543c..2ad7a200bb 100644 --- a/synapse/replication/tcp/handler.py +++ b/synapse/replication/tcp/handler.py @@ -571,7 +571,7 @@ class ReplicationCommandHandler: def on_REMOTE_SERVER_UP( self, conn: IReplicationConnection, cmd: RemoteServerUpCommand ): - """"Called when get a new REMOTE_SERVER_UP command.""" + """Called when get a new REMOTE_SERVER_UP command.""" self._replication_data_handler.on_remote_server_up(cmd.data) self._notifier.notify_remote_server_up(cmd.data) diff --git a/synapse/types.py b/synapse/types.py index e52cd7ffd4..0bdf32659c 100644 --- a/synapse/types.py +++ b/synapse/types.py @@ -284,14 +284,14 @@ class RoomAlias(DomainSpecificString): @attr.s(slots=True, frozen=True, repr=False) class RoomID(DomainSpecificString): - """Structure representing a room id. """ + """Structure representing a room id.""" SIGIL = "!" @attr.s(slots=True, frozen=True, repr=False) class EventID(DomainSpecificString): - """Structure representing an event id. """ + """Structure representing an event id.""" SIGIL = "$" diff --git a/tests/handlers/test_appservice.py b/tests/handlers/test_appservice.py index 5d6cc2885f..024c5e963c 100644 --- a/tests/handlers/test_appservice.py +++ b/tests/handlers/test_appservice.py @@ -26,7 +26,7 @@ from .. import unittest class AppServiceHandlerTestCase(unittest.TestCase): - """ Tests the ApplicationServicesHandler. """ + """Tests the ApplicationServicesHandler.""" def setUp(self): self.mock_store = Mock() diff --git a/tests/handlers/test_directory.py b/tests/handlers/test_directory.py index 1908d3c2c6..7a8041ab44 100644 --- a/tests/handlers/test_directory.py +++ b/tests/handlers/test_directory.py @@ -27,7 +27,7 @@ from tests.test_utils import make_awaitable class DirectoryTestCase(unittest.HomeserverTestCase): - """ Tests the directory service. """ + """Tests the directory service.""" def make_homeserver(self, reactor, clock): self.mock_federation = Mock() diff --git a/tests/handlers/test_profile.py b/tests/handlers/test_profile.py index 5330a9b34e..cdb41101b3 100644 --- a/tests/handlers/test_profile.py +++ b/tests/handlers/test_profile.py @@ -23,7 +23,7 @@ from tests.test_utils import make_awaitable class ProfileTestCase(unittest.HomeserverTestCase): - """ Tests profile management. """ + """Tests profile management.""" def make_homeserver(self, reactor, clock): self.mock_federation = Mock() diff --git a/tests/handlers/test_register.py b/tests/handlers/test_register.py index bd43190523..c51763f41a 100644 --- a/tests/handlers/test_register.py +++ b/tests/handlers/test_register.py @@ -28,7 +28,7 @@ from .. import unittest class RegistrationTestCase(unittest.HomeserverTestCase): - """ Tests the RegistrationHandler. """ + """Tests the RegistrationHandler.""" def make_homeserver(self, reactor, clock): hs_config = self.default_config() diff --git a/tests/handlers/test_sync.py b/tests/handlers/test_sync.py index c8b43305f4..84f05f6c58 100644 --- a/tests/handlers/test_sync.py +++ b/tests/handlers/test_sync.py @@ -22,7 +22,7 @@ import tests.utils class SyncTestCase(tests.unittest.HomeserverTestCase): - """ Tests Sync Handler. """ + """Tests Sync Handler.""" def prepare(self, reactor, clock, hs): self.hs = hs diff --git a/tests/rest/client/v1/test_events.py b/tests/rest/client/v1/test_events.py index 852bda408c..2789d51546 100644 --- a/tests/rest/client/v1/test_events.py +++ b/tests/rest/client/v1/test_events.py @@ -23,7 +23,7 @@ from tests import unittest class EventStreamPermissionsTestCase(unittest.HomeserverTestCase): - """ Tests event streaming (GET /events). """ + """Tests event streaming (GET /events).""" servlets = [ events.register_servlets, diff --git a/tests/rest/client/v1/test_presence.py b/tests/rest/client/v1/test_presence.py index 409f3949dc..597e4c67de 100644 --- a/tests/rest/client/v1/test_presence.py +++ b/tests/rest/client/v1/test_presence.py @@ -24,7 +24,7 @@ from tests import unittest class PresenceTestCase(unittest.HomeserverTestCase): - """ Tests presence REST API. """ + """Tests presence REST API.""" user_id = "@sid:red" diff --git a/tests/rest/client/v1/test_rooms.py b/tests/rest/client/v1/test_rooms.py index 5b1096d091..e94566ffd7 100644 --- a/tests/rest/client/v1/test_rooms.py +++ b/tests/rest/client/v1/test_rooms.py @@ -64,7 +64,7 @@ class RoomBase(unittest.HomeserverTestCase): class RoomPermissionsTestCase(RoomBase): - """ Tests room permissions. """ + """Tests room permissions.""" user_id = "@sid1:red" rmcreator_id = "@notme:red" @@ -377,7 +377,7 @@ class RoomPermissionsTestCase(RoomBase): class RoomsMemberListTestCase(RoomBase): - """ Tests /rooms/$room_id/members/list REST events.""" + """Tests /rooms/$room_id/members/list REST events.""" user_id = "@sid1:red" @@ -416,7 +416,7 @@ class RoomsMemberListTestCase(RoomBase): class RoomsCreateTestCase(RoomBase): - """ Tests /rooms and /rooms/$room_id REST events. """ + """Tests /rooms and /rooms/$room_id REST events.""" user_id = "@sid1:red" @@ -502,7 +502,7 @@ class RoomsCreateTestCase(RoomBase): class RoomTopicTestCase(RoomBase): - """ Tests /rooms/$room_id/topic REST events. """ + """Tests /rooms/$room_id/topic REST events.""" user_id = "@sid1:red" @@ -566,7 +566,7 @@ class RoomTopicTestCase(RoomBase): class RoomMemberStateTestCase(RoomBase): - """ Tests /rooms/$room_id/members/$user_id/state REST events. """ + """Tests /rooms/$room_id/members/$user_id/state REST events.""" user_id = "@sid1:red" @@ -790,7 +790,7 @@ class RoomJoinRatelimitTestCase(RoomBase): class RoomMessagesTestCase(RoomBase): - """ Tests /rooms/$room_id/messages/$user_id/$msg_id REST events. """ + """Tests /rooms/$room_id/messages/$user_id/$msg_id REST events.""" user_id = "@sid1:red" @@ -838,7 +838,7 @@ class RoomMessagesTestCase(RoomBase): class RoomInitialSyncTestCase(RoomBase): - """ Tests /rooms/$room_id/initialSync. """ + """Tests /rooms/$room_id/initialSync.""" user_id = "@sid1:red" @@ -879,7 +879,7 @@ class RoomInitialSyncTestCase(RoomBase): class RoomMessageListTestCase(RoomBase): - """ Tests /rooms/$room_id/messages REST events. """ + """Tests /rooms/$room_id/messages REST events.""" user_id = "@sid1:red" diff --git a/tests/rest/client/v1/test_typing.py b/tests/rest/client/v1/test_typing.py index 0aad48a162..44e22ca999 100644 --- a/tests/rest/client/v1/test_typing.py +++ b/tests/rest/client/v1/test_typing.py @@ -26,7 +26,7 @@ PATH_PREFIX = "/_matrix/client/api/v1" class RoomTypingTestCase(unittest.HomeserverTestCase): - """ Tests /rooms/$room_id/typing/$user_id REST API. """ + """Tests /rooms/$room_id/typing/$user_id REST API.""" user_id = "@sid:red" diff --git a/tests/storage/test_base.py b/tests/storage/test_base.py index 3b45a7efd8..ddad44bd6c 100644 --- a/tests/storage/test_base.py +++ b/tests/storage/test_base.py @@ -27,7 +27,7 @@ from tests.utils import TestHomeServer, default_config class SQLBaseStoreTestCase(unittest.TestCase): - """ Test the "simple" SQL generating methods in SQLBaseStore. """ + """Test the "simple" SQL generating methods in SQLBaseStore.""" def setUp(self): self.db_pool = Mock(spec=["runInteraction"]) -- cgit 1.5.1 From 1b3e398bea8129fa7ae6fe28fd3a395fcd427ad9 Mon Sep 17 00:00:00 2001 From: Brendan Abolivier Date: Fri, 18 Jun 2021 13:15:52 +0200 Subject: Standardise the module interface (#10062) This PR adds a common configuration section for all modules (see docs). These modules are then loaded at startup by the homeserver. Modules register their hooks and web resources using the new `register_[...]_callbacks` and `register_web_resource` methods of the module API. --- UPGRADE.rst | 17 ++ changelog.d/10062.feature | 1 + changelog.d/10062.removal | 1 + docs/SUMMARY.md | 2 +- docs/modules.md | 258 +++++++++++++++++++++++++ docs/sample_config.yaml | 29 +-- docs/spam_checker.md | 4 + synapse/app/_base.py | 9 + synapse/app/generic_worker.py | 4 + synapse/app/homeserver.py | 4 + synapse/config/_base.pyi | 2 + synapse/config/homeserver.py | 5 +- synapse/config/modules.py | 49 +++++ synapse/config/spam_checker.py | 15 -- synapse/events/spamcheck.py | 306 +++++++++++++++++++++--------- synapse/handlers/register.py | 2 +- synapse/module_api/__init__.py | 30 ++- synapse/module_api/errors.py | 1 + synapse/server.py | 39 +++- synapse/util/module_loader.py | 35 ++-- tests/handlers/test_register.py | 120 ++++++++---- tests/handlers/test_user_directory.py | 21 +- tests/rest/media/v1/test_media_storage.py | 3 + 23 files changed, 769 insertions(+), 188 deletions(-) create mode 100644 changelog.d/10062.feature create mode 100644 changelog.d/10062.removal create mode 100644 docs/modules.md create mode 100644 synapse/config/modules.py (limited to 'tests/handlers') diff --git a/UPGRADE.rst b/UPGRADE.rst index 9f61aad412..ee8b4fa60b 100644 --- a/UPGRADE.rst +++ b/UPGRADE.rst @@ -85,6 +85,23 @@ 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. + +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. + + Upgrading to v1.34.0 ==================== diff --git a/changelog.d/10062.feature b/changelog.d/10062.feature new file mode 100644 index 0000000000..97474f030c --- /dev/null +++ b/changelog.d/10062.feature @@ -0,0 +1 @@ +Standardised the module interface. diff --git a/changelog.d/10062.removal b/changelog.d/10062.removal new file mode 100644 index 0000000000..7f0cbdae2e --- /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 diff --git a/docs/SUMMARY.md b/docs/SUMMARY.md index 01ef4ff600..98969bdd2d 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/modules.md b/docs/modules.md new file mode 100644 index 0000000000..d64ec4493d --- /dev/null +++ b/docs/modules.md @@ -0,0 +1,258 @@ +# Modules + +Synapse supports extending its functionality 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. + +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. + +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 +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. 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 + +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 `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` +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 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 + +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. + +### 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 +`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 + +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 + + 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}) + + +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, user: str) -> bool: + if user == "@evilguy:example.com": + return False + + return True +``` diff --git a/docs/sample_config.yaml b/docs/sample_config.yaml index 307f8cd3c8..19505c7fd2 100644 --- a/docs/sample_config.yaml +++ b/docs/sample_config.yaml @@ -31,6 +31,22 @@ # # [1] https://docs.ansible.com/ansible/latest/reference_appendices/YAMLSyntax.html + +## Modules ## + +# 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. +# +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 @@ -2491,19 +2507,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/docs/spam_checker.md b/docs/spam_checker.md index 52947f605e..c16914e61d 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 diff --git a/synapse/app/_base.py b/synapse/app/_base.py index 1dde9d7173..00ab67e7e4 100644 --- a/synapse/app/_base.py +++ b/synapse/app/_base.py @@ -35,6 +35,7 @@ from synapse.app import check_bind_error 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 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 @@ -330,6 +331,14 @@ async def start(hs: "synapse.server.HomeServer"): # 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) + + load_legacy_spam_checkers(hs) + # It is now safe to start your Synapse. hs.start_listening() hs.get_datastore().db_pool.start_profiling() diff --git a/synapse/app/generic_worker.py b/synapse/app/generic_worker.py index 57c2fc2e88..8e648c6ee0 100644 --- a/synapse/app/generic_worker.py +++ b/synapse/app/generic_worker.py @@ -354,6 +354,10 @@ class GenericWorkerServer(HomeServer): 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/app/homeserver.py b/synapse/app/homeserver.py index fb16bceff8..f31467bde7 100644 --- a/synapse/app/homeserver.py +++ b/synapse/app/homeserver.py @@ -124,6 +124,10 @@ class SynapseHomeServer(HomeServer): ) resources[path] = resource + # 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: root_resource = RootOptionsRedirectResource(WEB_CLIENT_PREFIX) diff --git a/synapse/config/_base.pyi b/synapse/config/_base.pyi index 4e7bfa8b3b..844ecd4708 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 5ae0f55bcc..1f42a51857 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 .jwt import JWTConfig 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 @@ from .workers import WorkerConfig class HomeServerConfig(RootConfig): config_classes = [ + ModulesConfig, ServerConfig, TlsConfig, FederationConfig, diff --git a/synapse/config/modules.py b/synapse/config/modules.py new file mode 100644 index 0000000000..3209e1c492 --- /dev/null +++ b/synapse/config/modules.py @@ -0,0 +1,49 @@ +# 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 typing import Any, Dict, List, Tuple + +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: List[Tuple[Any, Dict]] = [] + + configured_modules = config.get("modules") or [] + 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 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. + # + modules: + # - module: my_super_module.MySuperClass + # config: + # do_thing: true + # - module: my_other_super_module.SomeClass + # config: {} + """ diff --git a/synapse/config/spam_checker.py b/synapse/config/spam_checker.py index 447ba3303b..c24165eb8a 100644 --- a/synapse/config/spam_checker.py +++ b/synapse/config/spam_checker.py @@ -42,18 +42,3 @@ class SpamCheckerConfig(Config): 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 d5fa195094..45ec96dfc1 100644 --- a/synapse/events/spamcheck.py +++ b/synapse/events/spamcheck.py @@ -15,7 +15,18 @@ import inspect import logging -from typing import TYPE_CHECKING, Any, Collection, Dict, List, Optional, Tuple, Union +from typing import ( + TYPE_CHECKING, + Any, + Awaitable, + Callable, + Collection, + Dict, + List, + Optional, + Tuple, + Union, +) from synapse.rest.media.v1._base import FileInfo from synapse.rest.media.v1.media_storage import ReadableFileWrapper @@ -29,20 +40,186 @@ if TYPE_CHECKING: logger = logging.getLogger(__name__) +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]], + ], + Awaitable[RegistrationBehaviour], +] +CHECK_REGISTRATION_FOR_SPAM_CALLBACK = Callable[ + [ + Optional[dict], + Optional[str], + Collection[Tuple[str, str]], + Optional[str], + ], + Awaitable[RegistrationBehaviour], +] +CHECK_MEDIA_FILE_FOR_SPAM_CALLBACK = Callable[ + [ReadableFileWrapper, FileInfo], + Awaitable[bool], +] + + +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. + """ + 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: + # 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: 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 + + if f.__name__ == "check_registration_for_spam": + checker_args = inspect.signature(f) + 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]: + # 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, + request_info, + ) + + f = wrapper + elif len(checker_args.parameters) != 4: + raise RuntimeError( + "Bad signature for callback check_registration_for_spam", + ) + + 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 + + # Register the hooks through the module API. + 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"): - self.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)) - else: - self.spam_checkers.append(module(config=config)) + 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] = [] + 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[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[ + 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._check_event_for_spam_callbacks.append(check_event_for_spam) + + if user_may_invite is not None: + self._user_may_invite_callbacks.append(user_may_invite) + + if user_may_create_room is not None: + 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, + ) + + if user_may_publish_room is not None: + self._user_may_publish_room_callbacks.append(user_may_publish_room) + + if check_username_for_spam is not None: + self._check_username_for_spam_callbacks.append(check_username_for_spam) + + if check_registration_for_spam is not None: + 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) async def check_event_for_spam( self, event: "synapse.events.EventBase" @@ -60,9 +237,10 @@ class SpamChecker: 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)): - return True + for callback in self._check_event_for_spam_callbacks: + res = await callback(event) # type: Union[bool, str] + if res: + return res return False @@ -81,15 +259,8 @@ class SpamChecker: Returns: True if the user may send an invite, otherwise False """ - for spam_checker in self.spam_checkers: - if ( - await maybe_awaitable( - spam_checker.user_may_invite( - inviter_userid, invitee_userid, room_id - ) - ) - is False - ): + for callback in self._user_may_invite_callbacks: + if await callback(inviter_userid, invitee_userid, room_id) is False: return False return True @@ -105,11 +276,8 @@ class SpamChecker: 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 callback in self._user_may_create_room_callbacks: + if await callback(userid) is False: return False return True @@ -128,13 +296,8 @@ class SpamChecker: 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 callback in self._user_may_create_room_alias_callbacks: + if await callback(userid, room_alias) is False: return False return True @@ -151,13 +314,8 @@ class SpamChecker: 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 callback in self._user_may_publish_room_callbacks: + if await callback(userid, room_id) is False: return False return True @@ -177,15 +335,11 @@ class SpamChecker: 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 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 callback(user_profile.copy()): + return True return False @@ -211,33 +365,13 @@ class SpamChecker: 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 callback in self._check_registration_for_spam_callbacks: + behaviour = await ( + callback(email_threepid, username, request_info, auth_provider_id) + ) + assert isinstance(behaviour, RegistrationBehaviour) + if behaviour != RegistrationBehaviour.ALLOW: + return behaviour return RegistrationBehaviour.ALLOW @@ -275,13 +409,9 @@ class SpamChecker: 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 callback in self._check_media_file_for_spam_callbacks: + spam = await callback(file_wrapper, file_info) + if spam: + return True return False diff --git a/synapse/handlers/register.py b/synapse/handlers/register.py index 4ceef3fab3..ca1ed6a5c0 100644 --- a/synapse/handlers/register.py +++ b/synapse/handlers/register.py @@ -195,7 +195,7 @@ class RegistrationHandler(BaseHandler): 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: diff --git a/synapse/module_api/__init__.py b/synapse/module_api/__init__.py index cecdc96bf5..58b255eb1b 100644 --- a/synapse/module_api/__init__.py +++ b/synapse/module_api/__init__.py @@ -16,6 +16,7 @@ import logging from typing import TYPE_CHECKING, Any, 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 @@ -42,7 +43,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() @@ -56,6 +57,33 @@ class ModuleApi: self._http_client = hs.get_simple_http_client() # type: SimpleHttpClient self._public_room_list_manager = PublicRoomListManager(hs) + self._spam_checker = hs.get_spam_checker() + + ################################################################################# + # The following methods should only be called during the module's initialisation. + + @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. + + 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. + + Args: + path: The path to register the resource for. + resource: The resource to attach to this path. + """ + 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. diff --git a/synapse/module_api/errors.py b/synapse/module_api/errors.py index d24864c549..02bbb0be39 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 e8dd2fa9f2..2c27d2a7e8 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. @@ -39,6 +37,7 @@ import twisted.internet.tcp 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 @@ -258,6 +257,38 @@ class HomeServer(metaclass=abc.ABCMeta): 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. + + 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. + + 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 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. @@ -646,7 +677,7 @@ class HomeServer(metaclass=abc.ABCMeta): @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 cbfbd097f9..5a638c6e9a 100644 --- a/synapse/util/module_loader.py +++ b/synapse/util/module_loader.py @@ -51,21 +51,26 @@ 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 diff --git a/tests/handlers/test_register.py b/tests/handlers/test_register.py index c51763f41a..a9fd3036dc 100644 --- a/tests/handlers/test_register.py +++ b/tests/handlers/test_register.py @@ -27,6 +27,58 @@ from tests.utils import mock_getRawHeaders 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 + + 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, + auth_provider_id, + ): + return RegistrationBehaviour.DENY + + +class BanAll(TestSpamChecker): + async def check_registration_for_spam( + self, + email_threepid, + username, + request_info, + auth_provider_id, + ): + return RegistrationBehaviour.SHADOW_BAN + + +class BanBadIdPUser(TestSpamChecker): + 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 + if auth_provider_id == "cas" and "flimflob" in username: + return RegistrationBehaviour.DENY + return RegistrationBehaviour.ALLOW + + class RegistrationTestCase(unittest.HomeserverTestCase): """Tests the RegistrationHandler.""" @@ -42,6 +94,11 @@ class RegistrationTestCase(unittest.HomeserverTestCase): 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 +522,30 @@ class RegistrationTestCase(unittest.HomeserverTestCase): 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 +565,17 @@ class RegistrationTestCase(unittest.HomeserverTestCase): 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 daac37abd8..549876dc85 100644 --- a/tests/handlers/test_user_directory.py +++ b/tests/handlers/test_user_directory.py @@ -312,15 +312,13 @@ class UserDirectoryTestCase(unittest.HomeserverTestCase): 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 @@ class UserDirectoryTestCase(unittest.HomeserverTestCase): 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 4a213d13dd..95e7075841 100644 --- a/tests/rest/media/v1/test_media_storage.py +++ b/tests/rest/media/v1/test_media_storage.py @@ -27,6 +27,7 @@ from PIL import Image as Image 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 @@ class SpamCheckerTestCase(unittest.HomeserverTestCase): 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") -- cgit 1.5.1 From 0bd968921c03dd61c1487f85dd884c4ed11ff486 Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Fri, 18 Jun 2021 13:41:33 -0400 Subject: Fix a missing await when in the spaces summary. (#10208) This could cause a minor data leak if someone defined a non-restricted join rule with an allow key or used a restricted join rule in an older room version, but this is unlikely. Additionally this starts adding unit tests to the spaces summary handler. --- changelog.d/10208.bugfix | 1 + synapse/handlers/space_summary.py | 3 +- tests/handlers/test_space_summary.py | 99 +++++++++++++++++++++++++++++++++++- 3 files changed, 100 insertions(+), 3 deletions(-) create mode 100644 changelog.d/10208.bugfix (limited to 'tests/handlers') diff --git a/changelog.d/10208.bugfix b/changelog.d/10208.bugfix new file mode 100644 index 0000000000..32b6465717 --- /dev/null +++ b/changelog.d/10208.bugfix @@ -0,0 +1 @@ +Fix a bug introduced in v1.35.1 where an `allow` key of a `m.room.join_rules` event could be applied for incorrect room versions and configurations. diff --git a/synapse/handlers/space_summary.py b/synapse/handlers/space_summary.py index e953a8afe6..17fc47ce16 100644 --- a/synapse/handlers/space_summary.py +++ b/synapse/handlers/space_summary.py @@ -445,14 +445,13 @@ class SpaceSummaryHandler: member_event_id = state_ids.get((EventTypes.Member, requester), None) # If they're in the room they can see info on it. - member_event = None if member_event_id: member_event = await self._store.get_event(member_event_id) if member_event.membership in (Membership.JOIN, Membership.INVITE): return True # Otherwise, check if they should be allowed access via membership in a space. - if self._event_auth_handler.has_restricted_join_rules( + if await self._event_auth_handler.has_restricted_join_rules( state_ids, room_version ): allowed_rooms = ( diff --git a/tests/handlers/test_space_summary.py b/tests/handlers/test_space_summary.py index 2c5e81531b..131d362ccc 100644 --- a/tests/handlers/test_space_summary.py +++ b/tests/handlers/test_space_summary.py @@ -11,10 +11,15 @@ # 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, Optional +from typing import Any, Iterable, Optional, Tuple from unittest import mock +from synapse.api.errors import AuthError from synapse.handlers.space_summary import _child_events_comparison_key +from synapse.rest import admin +from synapse.rest.client.v1 import login, room +from synapse.server import HomeServer +from synapse.types import JsonDict from tests import unittest @@ -79,3 +84,95 @@ class TestSpaceSummarySort(unittest.TestCase): ev1 = _create_event("!abc:test", "a" * 51) self.assertEqual([ev2, ev1], _order(ev1, ev2)) + + +class SpaceSummaryTestCase(unittest.HomeserverTestCase): + servlets = [ + admin.register_servlets_for_client_rest_resource, + room.register_servlets, + login.register_servlets, + ] + + def prepare(self, reactor, clock, hs: HomeServer): + self.hs = hs + self.handler = self.hs.get_space_summary_handler() + + self.user = self.register_user("user", "pass") + self.token = self.login("user", "pass") + + def _add_child(self, space_id: str, room_id: str, token: str) -> None: + """Add a child room to a space.""" + self.helper.send_state( + space_id, + event_type="m.space.child", + body={"via": [self.hs.hostname]}, + tok=token, + state_key=room_id, + ) + + def _assert_rooms(self, result: JsonDict, rooms: Iterable[str]) -> None: + """Assert that the expected room IDs are in the response.""" + self.assertCountEqual([room.get("room_id") for room in result["rooms"]], rooms) + + def _assert_events( + self, result: JsonDict, events: Iterable[Tuple[str, str]] + ) -> None: + """Assert that the expected parent / child room IDs are in the response.""" + self.assertCountEqual( + [ + (event.get("room_id"), event.get("state_key")) + for event in result["events"] + ], + events, + ) + + def test_simple_space(self): + """Test a simple space with a single room.""" + space = self.helper.create_room_as(self.user, tok=self.token) + room = self.helper.create_room_as(self.user, tok=self.token) + self._add_child(space, room, self.token) + + result = self.get_success(self.handler.get_space_summary(self.user, space)) + # The result should have the space and the room in it, along with a link + # from space -> room. + self._assert_rooms(result, [space, room]) + self._assert_events(result, [(space, room)]) + + def test_visibility(self): + """A user not in a space cannot inspect it.""" + space = self.helper.create_room_as(self.user, tok=self.token) + room = self.helper.create_room_as(self.user, tok=self.token) + self._add_child(space, room, self.token) + + user2 = self.register_user("user2", "pass") + token2 = self.login("user2", "pass") + + # The user cannot see the space. + self.get_failure(self.handler.get_space_summary(user2, space), AuthError) + + # Joining the room causes it to be visible. + self.helper.join(space, user2, tok=token2) + result = self.get_success(self.handler.get_space_summary(user2, space)) + + # The result should only have the space, but includes the link to the room. + self._assert_rooms(result, [space]) + self._assert_events(result, [(space, room)]) + + def test_world_readable(self): + """A world-readable room is visible to everyone.""" + space = self.helper.create_room_as(self.user, tok=self.token) + room = self.helper.create_room_as(self.user, tok=self.token) + self._add_child(space, room, self.token) + self.helper.send_state( + space, + event_type="m.room.history_visibility", + body={"history_visibility": "world_readable"}, + tok=self.token, + ) + + user2 = self.register_user("user2", "pass") + + # The space should be visible, as well as the link to the room. + result = self.get_success(self.handler.get_space_summary(user2, space)) + self._assert_rooms(result, [space]) + self._assert_events(result, [(space, room)]) -- cgit 1.5.1 From 96f6293de51c2fcf530bb6ca3705cf596c19656f Mon Sep 17 00:00:00 2001 From: Eric Eastwood Date: Tue, 22 Jun 2021 04:02:53 -0500 Subject: Add endpoints for backfilling history (MSC2716) (#9247) Work on https://github.com/matrix-org/matrix-doc/pull/2716 --- changelog.d/9247.feature | 1 + scripts-dev/complement.sh | 2 +- synapse/api/auth.py | 7 +- synapse/api/constants.py | 15 ++ synapse/config/experimental.py | 3 + synapse/events/__init__.py | 9 + synapse/events/builder.py | 17 +- synapse/handlers/message.py | 104 +++++++- synapse/handlers/room_member.py | 90 +++++++ synapse/rest/client/v1/room.py | 288 ++++++++++++++++++++- synapse/storage/databases/main/event_federation.py | 50 +++- tests/handlers/test_presence.py | 4 +- tests/replication/test_federation_sender_shard.py | 4 +- tests/storage/test_redaction.py | 13 +- 14 files changed, 584 insertions(+), 23 deletions(-) create mode 100644 changelog.d/9247.feature (limited to 'tests/handlers') diff --git a/changelog.d/9247.feature b/changelog.d/9247.feature new file mode 100644 index 0000000000..c687acf102 --- /dev/null +++ b/changelog.d/9247.feature @@ -0,0 +1 @@ +Add experimental support for backfilling history into rooms ([MSC2716](https://github.com/matrix-org/matrix-doc/pull/2716)). diff --git a/scripts-dev/complement.sh b/scripts-dev/complement.sh index 0043964673..ba060104c3 100755 --- a/scripts-dev/complement.sh +++ b/scripts-dev/complement.sh @@ -65,4 +65,4 @@ if [[ -n "$1" ]]; then fi # Run the tests! -go test -v -tags synapse_blacklist,msc2946,msc3083 -count=1 $EXTRA_COMPLEMENT_ARGS ./tests +go test -v -tags synapse_blacklist,msc2946,msc3083,msc2716 -count=1 $EXTRA_COMPLEMENT_ARGS ./tests diff --git a/synapse/api/auth.py b/synapse/api/auth.py index cf4333a923..edf1b918eb 100644 --- a/synapse/api/auth.py +++ b/synapse/api/auth.py @@ -92,11 +92,8 @@ class Auth: async def check_from_context( self, room_version: str, event, context, do_sig_check=True ) -> None: - prev_state_ids = await context.get_prev_state_ids() - auth_events_ids = self.compute_auth_events( - event, prev_state_ids, for_verification=True - ) - auth_events_by_id = await self.store.get_events(auth_events_ids) + auth_event_ids = event.auth_event_ids() + auth_events_by_id = await self.store.get_events(auth_event_ids) auth_events = {(e.type, e.state_key): e for e in auth_events_by_id.values()} room_version_obj = KNOWN_ROOM_VERSIONS[room_version] diff --git a/synapse/api/constants.py b/synapse/api/constants.py index 6c3958f7ab..414e4c019a 100644 --- a/synapse/api/constants.py +++ b/synapse/api/constants.py @@ -119,6 +119,9 @@ class EventTypes: SpaceChild = "m.space.child" SpaceParent = "m.space.parent" + MSC2716_INSERTION = "org.matrix.msc2716.insertion" + MSC2716_MARKER = "org.matrix.msc2716.marker" + class ToDeviceEventTypes: RoomKeyRequest = "m.room_key_request" @@ -185,6 +188,18 @@ class EventContentFields: # cf https://github.com/matrix-org/matrix-doc/pull/1772 ROOM_TYPE = "type" + # Used on normal messages to indicate they were historically imported after the fact + MSC2716_HISTORICAL = "org.matrix.msc2716.historical" + # For "insertion" events + MSC2716_NEXT_CHUNK_ID = "org.matrix.msc2716.next_chunk_id" + # Used on normal message events to indicate where the chunk connects to + MSC2716_CHUNK_ID = "org.matrix.msc2716.chunk_id" + # For "marker" events + MSC2716_MARKER_INSERTION = "org.matrix.msc2716.marker.insertion" + MSC2716_MARKER_INSERTION_PREV_EVENTS = ( + "org.matrix.msc2716.marker.insertion_prev_events" + ) + class RoomEncryptionAlgorithms: MEGOLM_V1_AES_SHA2 = "m.megolm.v1.aes-sha2" diff --git a/synapse/config/experimental.py b/synapse/config/experimental.py index 6ebce4b2f7..7fb1f7021f 100644 --- a/synapse/config/experimental.py +++ b/synapse/config/experimental.py @@ -29,3 +29,6 @@ class ExperimentalConfig(Config): # MSC3026 (busy presence state) self.msc3026_enabled = experimental.get("msc3026_enabled", False) # type: bool + + # MSC2716 (backfill existing history) + self.msc2716_enabled = experimental.get("msc2716_enabled", False) # type: bool diff --git a/synapse/events/__init__.py b/synapse/events/__init__.py index c8b52cbc7a..0cb9c1cc1e 100644 --- a/synapse/events/__init__.py +++ b/synapse/events/__init__.py @@ -119,6 +119,7 @@ class _EventInternalMetadata: redacted = DictProperty("redacted") # type: bool txn_id = DictProperty("txn_id") # type: str token_id = DictProperty("token_id") # type: str + historical = DictProperty("historical") # type: bool # XXX: These are set by StreamWorkerStore._set_before_and_after. # I'm pretty sure that these are never persisted to the database, so shouldn't @@ -204,6 +205,14 @@ class _EventInternalMetadata: """ return self._dict.get("redacted", False) + def is_historical(self) -> bool: + """Whether this is a historical message. + This is used by the batchsend historical message endpoint and + is needed to and mark the event as backfilled and skip some checks + like push notifications. + """ + return self._dict.get("historical", False) + class EventBase(metaclass=abc.ABCMeta): @property diff --git a/synapse/events/builder.py b/synapse/events/builder.py index 5793553a88..81bf8615b7 100644 --- a/synapse/events/builder.py +++ b/synapse/events/builder.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. +import logging from typing import Any, Dict, List, Optional, Tuple, Union import attr @@ -33,6 +34,8 @@ from synapse.types import EventID, JsonDict from synapse.util import Clock from synapse.util.stringutils import random_string +logger = logging.getLogger(__name__) + @attr.s(slots=True, cmp=False, frozen=True) class EventBuilder: @@ -100,6 +103,7 @@ class EventBuilder: self, prev_event_ids: List[str], auth_event_ids: Optional[List[str]], + depth: Optional[int] = None, ) -> EventBase: """Transform into a fully signed and hashed event @@ -108,6 +112,9 @@ class EventBuilder: auth_event_ids: The event IDs to use as the auth events. Should normally be set to None, which will cause them to be calculated based on the room state at the prev_events. + depth: Override the depth used to order the event in the DAG. + Should normally be set to None, which will cause the depth to be calculated + based on the prev_events. Returns: The signed and hashed event. @@ -131,8 +138,14 @@ class EventBuilder: auth_events = auth_event_ids prev_events = prev_event_ids - old_depth = await self._store.get_max_depth_of(prev_event_ids) - depth = old_depth + 1 + # Otherwise, progress the depth as normal + if depth is None: + ( + _, + most_recent_prev_event_depth, + ) = await self._store.get_max_depth_of(prev_event_ids) + + depth = most_recent_prev_event_depth + 1 # we cap depth of generated events, to ensure that they are not # rejected by other servers (and so that they can be persisted in diff --git a/synapse/handlers/message.py b/synapse/handlers/message.py index 4d2255bdf1..db12abd59d 100644 --- a/synapse/handlers/message.py +++ b/synapse/handlers/message.py @@ -482,6 +482,9 @@ class EventCreationHandler: prev_event_ids: Optional[List[str]] = None, auth_event_ids: Optional[List[str]] = None, require_consent: bool = True, + outlier: bool = False, + historical: bool = False, + depth: Optional[int] = None, ) -> Tuple[EventBase, EventContext]: """ Given a dict from a client, create a new event. @@ -508,6 +511,14 @@ class EventCreationHandler: require_consent: Whether to check if the requester has consented to the privacy policy. + + outlier: Indicates whether the event is an `outlier`, i.e. if + it's from an arbitrary point and floating in the DAG as + opposed to being inline with the current DAG. + depth: Override the depth used to order the event in the DAG. + Should normally be set to None, which will cause the depth to be calculated + based on the prev_events. + Raises: ResourceLimitError if server is blocked to some resource being exceeded @@ -563,11 +574,36 @@ class EventCreationHandler: if txn_id is not None: builder.internal_metadata.txn_id = txn_id + builder.internal_metadata.outlier = outlier + + builder.internal_metadata.historical = historical + + # Strip down the auth_event_ids to only what we need to auth the event. + # For example, we don't need extra m.room.member that don't match event.sender + if auth_event_ids is not None: + temp_event = await builder.build( + prev_event_ids=prev_event_ids, + auth_event_ids=auth_event_ids, + depth=depth, + ) + auth_events = await self.store.get_events_as_list(auth_event_ids) + # Create a StateMap[str] + auth_event_state_map = { + (e.type, e.state_key): e.event_id for e in auth_events + } + # Actually strip down and use the necessary auth events + auth_event_ids = self.auth.compute_auth_events( + event=temp_event, + current_state_ids=auth_event_state_map, + for_verification=False, + ) + event, context = await self.create_new_client_event( builder=builder, requester=requester, prev_event_ids=prev_event_ids, auth_event_ids=auth_event_ids, + depth=depth, ) # In an ideal world we wouldn't need the second part of this condition. However, @@ -724,9 +760,13 @@ class EventCreationHandler: self, requester: Requester, event_dict: dict, + prev_event_ids: Optional[List[str]] = None, + auth_event_ids: Optional[List[str]] = None, ratelimit: bool = True, txn_id: Optional[str] = None, ignore_shadow_ban: bool = False, + outlier: bool = False, + depth: Optional[int] = None, ) -> Tuple[EventBase, int]: """ Creates an event, then sends it. @@ -736,10 +776,24 @@ class EventCreationHandler: Args: requester: The requester sending the event. event_dict: An entire event. + prev_event_ids: + The event IDs to use as the prev events. + Should normally be left as None to automatically request them + from the database. + auth_event_ids: + The event ids to use as the auth_events for the new event. + Should normally be left as None, which will cause them to be calculated + based on the room state at the prev_events. ratelimit: Whether to rate limit this send. txn_id: The transaction ID. ignore_shadow_ban: True if shadow-banned users should be allowed to send this event. + outlier: Indicates whether the event is an `outlier`, i.e. if + it's from an arbitrary point and floating in the DAG as + opposed to being inline with the current DAG. + depth: Override the depth used to order the event in the DAG. + Should normally be set to None, which will cause the depth to be calculated + based on the prev_events. Returns: The event, and its stream ordering (if deduplication happened, @@ -779,7 +833,13 @@ class EventCreationHandler: return event, event.internal_metadata.stream_ordering event, context = await self.create_event( - requester, event_dict, txn_id=txn_id + requester, + event_dict, + txn_id=txn_id, + prev_event_ids=prev_event_ids, + auth_event_ids=auth_event_ids, + outlier=outlier, + depth=depth, ) assert self.hs.is_mine_id(event.sender), "User must be our own: %s" % ( @@ -811,6 +871,7 @@ class EventCreationHandler: requester: Optional[Requester] = None, prev_event_ids: Optional[List[str]] = None, auth_event_ids: Optional[List[str]] = None, + depth: Optional[int] = None, ) -> Tuple[EventBase, EventContext]: """Create a new event for a local client @@ -828,6 +889,10 @@ class EventCreationHandler: Should normally be left as None, which will cause them to be calculated based on the room state at the prev_events. + depth: Override the depth used to order the event in the DAG. + Should normally be set to None, which will cause the depth to be calculated + based on the prev_events. + Returns: Tuple of created event, context """ @@ -851,9 +916,24 @@ class EventCreationHandler: ), "Attempting to create an event with no prev_events" event = await builder.build( - prev_event_ids=prev_event_ids, auth_event_ids=auth_event_ids + prev_event_ids=prev_event_ids, + auth_event_ids=auth_event_ids, + depth=depth, ) - context = await self.state.compute_event_context(event) + + old_state = None + + # Pass on the outlier property from the builder to the event + # after it is created + if builder.internal_metadata.outlier: + event.internal_metadata.outlier = builder.internal_metadata.outlier + + # Calculate the state for outliers that pass in their own `auth_event_ids` + if auth_event_ids: + old_state = await self.store.get_events_as_list(auth_event_ids) + + context = await self.state.compute_event_context(event, old_state=old_state) + if requester: context.app_service = requester.app_service @@ -1018,7 +1098,13 @@ class EventCreationHandler: the arguments. """ - await self.action_generator.handle_push_actions_for_event(event, context) + # Skip push notification actions for historical messages + # because we don't want to notify people about old history back in time. + # The historical messages also do not have the proper `context.current_state_ids` + # and `state_groups` because they have `prev_events` that aren't persisted yet + # (historical messages persisted in reverse-chronological order). + if not event.internal_metadata.is_historical(): + await self.action_generator.handle_push_actions_for_event(event, context) try: # If we're a worker we need to hit out to the master. @@ -1317,13 +1403,21 @@ class EventCreationHandler: if prev_state_ids: raise AuthError(403, "Changing the room create event is forbidden") + # Mark any `m.historical` messages as backfilled so they don't appear + # in `/sync` and have the proper decrementing `stream_ordering` as we import + backfilled = False + if event.internal_metadata.is_historical(): + backfilled = True + # Note that this returns the event that was persisted, which may not be # the same as we passed in if it was deduplicated due transaction IDs. ( event, event_pos, max_stream_token, - ) = await self.storage.persistence.persist_event(event, context=context) + ) = await self.storage.persistence.persist_event( + event, context=context, backfilled=backfilled + ) if self._ephemeral_events_enabled: # If there's an expiry timestamp on the event, schedule its expiry. diff --git a/synapse/handlers/room_member.py b/synapse/handlers/room_member.py index a49a61a34c..1192591609 100644 --- a/synapse/handlers/room_member.py +++ b/synapse/handlers/room_member.py @@ -257,11 +257,42 @@ class RoomMemberHandler(metaclass=abc.ABCMeta): room_id: str, membership: str, prev_event_ids: List[str], + auth_event_ids: Optional[List[str]] = None, txn_id: Optional[str] = None, ratelimit: bool = True, content: Optional[dict] = None, require_consent: bool = True, + outlier: bool = False, ) -> Tuple[str, int]: + """ + Internal membership update function to get an existing event or create + and persist a new event for the new membership change. + + Args: + requester: + target: + room_id: + membership: + prev_event_ids: The event IDs to use as the prev events + + auth_event_ids: + The event ids to use as the auth_events for the new event. + Should normally be left as None, which will cause them to be calculated + based on the room state at the prev_events. + + txn_id: + ratelimit: + content: + require_consent: + + outlier: Indicates whether the event is an `outlier`, i.e. if + it's from an arbitrary point and floating in the DAG as + opposed to being inline with the current DAG. + + Returns: + Tuple of event ID and stream ordering position + """ + user_id = target.to_string() if content is None: @@ -298,7 +329,9 @@ class RoomMemberHandler(metaclass=abc.ABCMeta): }, txn_id=txn_id, prev_event_ids=prev_event_ids, + auth_event_ids=auth_event_ids, require_consent=require_consent, + outlier=outlier, ) prev_state_ids = await context.get_prev_state_ids() @@ -399,6 +432,9 @@ class RoomMemberHandler(metaclass=abc.ABCMeta): ratelimit: bool = True, content: Optional[dict] = None, require_consent: bool = True, + outlier: bool = False, + prev_event_ids: Optional[List[str]] = None, + auth_event_ids: Optional[List[str]] = None, ) -> Tuple[str, int]: """Update a user's membership in a room. @@ -413,6 +449,14 @@ class RoomMemberHandler(metaclass=abc.ABCMeta): ratelimit: Whether to rate limit the request. content: The content of the created event. require_consent: Whether consent is required. + outlier: Indicates whether the event is an `outlier`, i.e. if + it's from an arbitrary point and floating in the DAG as + opposed to being inline with the current DAG. + prev_event_ids: The event IDs to use as the prev events + auth_event_ids: + The event ids to use as the auth_events for the new event. + Should normally be left as None, which will cause them to be calculated + based on the room state at the prev_events. Returns: A tuple of the new event ID and stream ID. @@ -439,6 +483,9 @@ class RoomMemberHandler(metaclass=abc.ABCMeta): ratelimit=ratelimit, content=content, require_consent=require_consent, + outlier=outlier, + prev_event_ids=prev_event_ids, + auth_event_ids=auth_event_ids, ) return result @@ -455,10 +502,36 @@ class RoomMemberHandler(metaclass=abc.ABCMeta): ratelimit: bool = True, content: Optional[dict] = None, require_consent: bool = True, + outlier: bool = False, + prev_event_ids: Optional[List[str]] = None, + auth_event_ids: Optional[List[str]] = None, ) -> Tuple[str, int]: """Helper for update_membership. Assumes that the membership linearizer is already held for the room. + + Args: + requester: + target: + room_id: + action: + txn_id: + remote_room_hosts: + third_party_signed: + ratelimit: + content: + require_consent: + outlier: Indicates whether the event is an `outlier`, i.e. if + it's from an arbitrary point and floating in the DAG as + opposed to being inline with the current DAG. + prev_event_ids: The event IDs to use as the prev events + auth_event_ids: + The event ids to use as the auth_events for the new event. + Should normally be left as None, which will cause them to be calculated + based on the room state at the prev_events. + + Returns: + A tuple of the new event ID and stream ID. """ content_specified = bool(content) if content is None: @@ -543,6 +616,21 @@ class RoomMemberHandler(metaclass=abc.ABCMeta): if block_invite: raise SynapseError(403, "Invites have been disabled on this server") + if prev_event_ids: + return await self._local_membership_update( + requester=requester, + target=target, + room_id=room_id, + membership=effective_membership_state, + txn_id=txn_id, + ratelimit=ratelimit, + prev_event_ids=prev_event_ids, + auth_event_ids=auth_event_ids, + content=content, + require_consent=require_consent, + outlier=outlier, + ) + latest_event_ids = await self.store.get_prev_events_for_room(room_id) current_state_ids = await self.state_handler.get_current_state_ids( @@ -732,8 +820,10 @@ class RoomMemberHandler(metaclass=abc.ABCMeta): txn_id=txn_id, ratelimit=ratelimit, prev_event_ids=latest_event_ids, + auth_event_ids=auth_event_ids, content=content, require_consent=require_consent, + outlier=outlier, ) async def transfer_room_state_on_room_upgrade( diff --git a/synapse/rest/client/v1/room.py b/synapse/rest/client/v1/room.py index 16d087ea60..92ebe838fd 100644 --- a/synapse/rest/client/v1/room.py +++ b/synapse/rest/client/v1/room.py @@ -19,7 +19,7 @@ import re from typing import TYPE_CHECKING, Dict, List, Optional, Tuple from urllib import parse as urlparse -from synapse.api.constants import EventTypes, Membership +from synapse.api.constants import EventContentFields, EventTypes, Membership from synapse.api.errors import ( AuthError, Codes, @@ -266,6 +266,288 @@ class RoomSendEventRestServlet(TransactionRestServlet): ) +class RoomBatchSendEventRestServlet(TransactionRestServlet): + """ + API endpoint which can insert a chunk of events historically back in time + next to the given `prev_event`. + + `chunk_id` comes from `next_chunk_id `in the response of the batch send + endpoint and is derived from the "insertion" events added to each chunk. + It's not required for the first batch send. + + `state_events_at_start` is used to define the historical state events + needed to auth the events like join events. These events will float + outside of the normal DAG as outlier's and won't be visible in the chat + history which also allows us to insert multiple chunks without having a bunch + of `@mxid joined the room` noise between each chunk. + + `events` is chronological chunk/list of events you want to insert. + There is a reverse-chronological constraint on chunks so once you insert + some messages, you can only insert older ones after that. + tldr; Insert chunks from your most recent history -> oldest history. + + POST /_matrix/client/unstable/org.matrix.msc2716/rooms//batch_send?prev_event=&chunk_id= + { + "events": [ ... ], + "state_events_at_start": [ ... ] + } + """ + + PATTERNS = ( + re.compile( + "^/_matrix/client/unstable/org.matrix.msc2716" + "/rooms/(?P[^/]*)/batch_send$" + ), + ) + + def __init__(self, hs): + super().__init__(hs) + self.hs = hs + self.store = hs.get_datastore() + self.state_store = hs.get_storage().state + self.event_creation_handler = hs.get_event_creation_handler() + self.room_member_handler = hs.get_room_member_handler() + self.auth = hs.get_auth() + + async def inherit_depth_from_prev_ids(self, prev_event_ids) -> int: + ( + most_recent_prev_event_id, + most_recent_prev_event_depth, + ) = await self.store.get_max_depth_of(prev_event_ids) + + # We want to insert the historical event after the `prev_event` but before the successor event + # + # We inherit depth from the successor event instead of the `prev_event` + # because events returned from `/messages` are first sorted by `topological_ordering` + # which is just the `depth` and then tie-break with `stream_ordering`. + # + # We mark these inserted historical events as "backfilled" which gives them a + # negative `stream_ordering`. If we use the same depth as the `prev_event`, + # then our historical event will tie-break and be sorted before the `prev_event` + # when it should come after. + # + # We want to use the successor event depth so they appear after `prev_event` because + # it has a larger `depth` but before the successor event because the `stream_ordering` + # is negative before the successor event. + successor_event_ids = await self.store.get_successor_events( + [most_recent_prev_event_id] + ) + + # If we can't find any successor events, then it's a forward extremity of + # historical messages and we can just inherit from the previous historical + # event which we can already assume has the correct depth where we want + # to insert into. + if not successor_event_ids: + depth = most_recent_prev_event_depth + else: + ( + _, + oldest_successor_depth, + ) = await self.store.get_min_depth_of(successor_event_ids) + + depth = oldest_successor_depth + + return depth + + async def on_POST(self, request, room_id): + requester = await self.auth.get_user_by_req(request, allow_guest=False) + + if not requester.app_service: + raise AuthError( + 403, + "Only application services can use the /batchsend endpoint", + ) + + body = parse_json_object_from_request(request) + assert_params_in_dict(body, ["state_events_at_start", "events"]) + + prev_events_from_query = parse_strings_from_args(request.args, "prev_event") + chunk_id_from_query = parse_string(request, "chunk_id", default=None) + + if prev_events_from_query is None: + raise SynapseError( + 400, + "prev_event query parameter is required when inserting historical messages back in time", + errcode=Codes.MISSING_PARAM, + ) + + # For the event we are inserting next to (`prev_events_from_query`), + # find the most recent auth events (derived from state events) that + # allowed that message to be sent. We will use that as a base + # to auth our historical messages against. + ( + most_recent_prev_event_id, + _, + ) = await self.store.get_max_depth_of(prev_events_from_query) + # mapping from (type, state_key) -> state_event_id + prev_state_map = await self.state_store.get_state_ids_for_event( + most_recent_prev_event_id + ) + # List of state event ID's + prev_state_ids = list(prev_state_map.values()) + auth_event_ids = prev_state_ids + + for state_event in body["state_events_at_start"]: + assert_params_in_dict( + state_event, ["type", "origin_server_ts", "content", "sender"] + ) + + logger.debug( + "RoomBatchSendEventRestServlet inserting state_event=%s, auth_event_ids=%s", + state_event, + auth_event_ids, + ) + + event_dict = { + "type": state_event["type"], + "origin_server_ts": state_event["origin_server_ts"], + "content": state_event["content"], + "room_id": room_id, + "sender": state_event["sender"], + "state_key": state_event["state_key"], + } + + # Make the state events float off on their own + fake_prev_event_id = "$" + random_string(43) + + # TODO: This is pretty much the same as some other code to handle inserting state in this file + if event_dict["type"] == EventTypes.Member: + membership = event_dict["content"].get("membership", None) + event_id, _ = await self.room_member_handler.update_membership( + requester, + target=UserID.from_string(event_dict["state_key"]), + room_id=room_id, + action=membership, + content=event_dict["content"], + outlier=True, + prev_event_ids=[fake_prev_event_id], + # Make sure to use a copy of this list because we modify it + # later in the loop here. Otherwise it will be the same + # reference and also update in the event when we append later. + auth_event_ids=auth_event_ids.copy(), + ) + else: + # TODO: Add some complement tests that adds state that is not member joins + # and will use this code path. Maybe we only want to support join state events + # and can get rid of this `else`? + ( + event, + _, + ) = await self.event_creation_handler.create_and_send_nonmember_event( + requester, + event_dict, + outlier=True, + prev_event_ids=[fake_prev_event_id], + # Make sure to use a copy of this list because we modify it + # later in the loop here. Otherwise it will be the same + # reference and also update in the event when we append later. + auth_event_ids=auth_event_ids.copy(), + ) + event_id = event.event_id + + auth_event_ids.append(event_id) + + events_to_create = body["events"] + + # If provided, connect the chunk to the last insertion point + # The chunk ID passed in comes from the chunk_id in the + # "insertion" event from the previous chunk. + if chunk_id_from_query: + last_event_in_chunk = events_to_create[-1] + last_event_in_chunk["content"][ + EventContentFields.MSC2716_CHUNK_ID + ] = chunk_id_from_query + + # Add an "insertion" event to the start of each chunk (next to the oldest + # event in the chunk) so the next chunk can be connected to this one. + next_chunk_id = random_string(64) + insertion_event = { + "type": EventTypes.MSC2716_INSERTION, + "sender": requester.user.to_string(), + "content": { + EventContentFields.MSC2716_NEXT_CHUNK_ID: next_chunk_id, + EventContentFields.MSC2716_HISTORICAL: True, + }, + # Since the insertion event is put at the start of the chunk, + # where the oldest event is, copy the origin_server_ts from + # the first event we're inserting + "origin_server_ts": events_to_create[0]["origin_server_ts"], + } + # Prepend the insertion event to the start of the chunk + events_to_create = [insertion_event] + events_to_create + + inherited_depth = await self.inherit_depth_from_prev_ids(prev_events_from_query) + + event_ids = [] + prev_event_ids = prev_events_from_query + events_to_persist = [] + for ev in events_to_create: + assert_params_in_dict(ev, ["type", "origin_server_ts", "content", "sender"]) + + # Mark all events as historical + # This has important semantics within the Synapse internals to backfill properly + ev["content"][EventContentFields.MSC2716_HISTORICAL] = True + + event_dict = { + "type": ev["type"], + "origin_server_ts": ev["origin_server_ts"], + "content": ev["content"], + "room_id": room_id, + "sender": ev["sender"], # requester.user.to_string(), + "prev_events": prev_event_ids.copy(), + } + + event, context = await self.event_creation_handler.create_event( + requester, + event_dict, + prev_event_ids=event_dict.get("prev_events"), + auth_event_ids=auth_event_ids, + historical=True, + depth=inherited_depth, + ) + logger.debug( + "RoomBatchSendEventRestServlet inserting event=%s, prev_event_ids=%s, auth_event_ids=%s", + event, + prev_event_ids, + auth_event_ids, + ) + + assert self.hs.is_mine_id(event.sender), "User must be our own: %s" % ( + event.sender, + ) + + events_to_persist.append((event, context)) + event_id = event.event_id + + event_ids.append(event_id) + prev_event_ids = [event_id] + + # Persist events in reverse-chronological order so they have the + # correct stream_ordering as they are backfilled (which decrements). + # Events are sorted by (topological_ordering, stream_ordering) + # where topological_ordering is just depth. + for (event, context) in reversed(events_to_persist): + ev = await self.event_creation_handler.handle_new_client_event( + requester=requester, + event=event, + context=context, + ) + + return 200, { + "state_events": auth_event_ids, + "events": event_ids, + "next_chunk_id": next_chunk_id, + } + + def on_GET(self, request, room_id): + return 501, "Not implemented" + + def on_PUT(self, request, room_id): + return self.txns.fetch_or_execute_request( + request, self.on_POST, request, room_id + ) + + # TODO: Needs unit testing for room ID + alias joins class JoinRoomAliasServlet(TransactionRestServlet): def __init__(self, hs): @@ -1054,6 +1336,8 @@ class RoomSpaceSummaryRestServlet(RestServlet): def register_servlets(hs: "HomeServer", http_server, is_worker=False): + msc2716_enabled = hs.config.experimental.msc2716_enabled + RoomStateEventRestServlet(hs).register(http_server) RoomMemberListRestServlet(hs).register(http_server) JoinedRoomMemberListRestServlet(hs).register(http_server) @@ -1061,6 +1345,8 @@ def register_servlets(hs: "HomeServer", http_server, is_worker=False): JoinRoomAliasServlet(hs).register(http_server) RoomMembershipRestServlet(hs).register(http_server) RoomSendEventRestServlet(hs).register(http_server) + if msc2716_enabled: + RoomBatchSendEventRestServlet(hs).register(http_server) PublicRoomListRestServlet(hs).register(http_server) RoomStateRestServlet(hs).register(http_server) RoomRedactEventRestServlet(hs).register(http_server) diff --git a/synapse/storage/databases/main/event_federation.py b/synapse/storage/databases/main/event_federation.py index ff81d5cd17..c0ea445550 100644 --- a/synapse/storage/databases/main/event_federation.py +++ b/synapse/storage/databases/main/event_federation.py @@ -16,6 +16,7 @@ import logging from queue import Empty, PriorityQueue from typing import Collection, Dict, Iterable, List, Set, Tuple +from synapse.api.constants import MAX_DEPTH from synapse.api.errors import StoreError from synapse.events import EventBase from synapse.metrics.background_process_metrics import wrap_as_background_process @@ -670,8 +671,8 @@ class EventFederationWorkerStore(EventsWorkerStore, SignatureWorkerStore, SQLBas return dict(txn) - async def get_max_depth_of(self, event_ids: List[str]) -> int: - """Returns the max depth of a set of event IDs + async def get_max_depth_of(self, event_ids: List[str]) -> Tuple[str, int]: + """Returns the event ID and depth for the event that has the max depth from a set of event IDs Args: event_ids: The event IDs to calculate the max depth of. @@ -680,14 +681,53 @@ class EventFederationWorkerStore(EventsWorkerStore, SignatureWorkerStore, SQLBas table="events", column="event_id", iterable=event_ids, - retcols=("depth",), + retcols=( + "event_id", + "depth", + ), desc="get_max_depth_of", ) if not rows: - return 0 + return None, 0 else: - return max(row["depth"] for row in rows) + max_depth_event_id = "" + current_max_depth = 0 + for row in rows: + if row["depth"] > current_max_depth: + max_depth_event_id = row["event_id"] + current_max_depth = row["depth"] + + return max_depth_event_id, current_max_depth + + async def get_min_depth_of(self, event_ids: List[str]) -> Tuple[str, int]: + """Returns the event ID and depth for the event that has the min depth from a set of event IDs + + Args: + event_ids: The event IDs to calculate the max depth of. + """ + rows = await self.db_pool.simple_select_many_batch( + table="events", + column="event_id", + iterable=event_ids, + retcols=( + "event_id", + "depth", + ), + desc="get_min_depth_of", + ) + + if not rows: + return None, 0 + else: + min_depth_event_id = "" + current_min_depth = MAX_DEPTH + for row in rows: + if row["depth"] < current_min_depth: + min_depth_event_id = row["event_id"] + current_min_depth = row["depth"] + + return min_depth_event_id, current_min_depth async def get_prev_events_for_room(self, room_id: str) -> List[str]: """ diff --git a/tests/handlers/test_presence.py b/tests/handlers/test_presence.py index d90a9fec91..dfb9b3a0fa 100644 --- a/tests/handlers/test_presence.py +++ b/tests/handlers/test_presence.py @@ -863,7 +863,9 @@ class PresenceJoinTestCase(unittest.HomeserverTestCase): self.store.get_latest_event_ids_in_room(room_id) ) - event = self.get_success(builder.build(prev_event_ids, None)) + event = self.get_success( + builder.build(prev_event_ids=prev_event_ids, auth_event_ids=None) + ) self.get_success(self.federation_handler.on_receive_pdu(hostname, event)) diff --git a/tests/replication/test_federation_sender_shard.py b/tests/replication/test_federation_sender_shard.py index 48ab3aa4e3..584da58371 100644 --- a/tests/replication/test_federation_sender_shard.py +++ b/tests/replication/test_federation_sender_shard.py @@ -224,7 +224,9 @@ class FederationSenderTestCase(BaseMultiWorkerStreamTestCase): } builder = factory.for_room_version(room_version, event_dict) - join_event = self.get_success(builder.build(prev_event_ids, None)) + join_event = self.get_success( + builder.build(prev_event_ids=prev_event_ids, auth_event_ids=None) + ) self.get_success(federation.on_send_join_request(remote_server, join_event)) self.replicate() diff --git a/tests/storage/test_redaction.py b/tests/storage/test_redaction.py index bb31ab756d..dbacce4380 100644 --- a/tests/storage/test_redaction.py +++ b/tests/storage/test_redaction.py @@ -232,9 +232,14 @@ class RedactionTestCase(unittest.HomeserverTestCase): self._base_builder = base_builder self._event_id = event_id - async def build(self, prev_event_ids, auth_event_ids): + async def build( + self, + prev_event_ids, + auth_event_ids, + depth: Optional[int] = None, + ): built_event = await self._base_builder.build( - prev_event_ids, auth_event_ids + prev_event_ids=prev_event_ids, auth_event_ids=auth_event_ids ) built_event._event_id = self._event_id @@ -251,6 +256,10 @@ class RedactionTestCase(unittest.HomeserverTestCase): def type(self): return self._base_builder.type + @property + def internal_metadata(self): + return self._base_builder.internal_metadata + event_1, context_1 = self.get_success( self.event_creation_handler.create_new_client_event( EventIdManglingBuilder( -- cgit 1.5.1 From c955e378683708acd5b88e9cb1980291e06dd9a7 Mon Sep 17 00:00:00 2001 From: Brendan Abolivier Date: Wed, 23 Jun 2021 17:22:08 +0200 Subject: Fix wrapping of legacy check_registration_for_spam (#10238) Fixes #10234 --- changelog.d/10238.removal | 1 + synapse/events/spamcheck.py | 13 +++---- tests/handlers/test_register.py | 76 +++++++++++++++++++++++++++++++++++++++++ 3 files changed, 84 insertions(+), 6 deletions(-) create mode 100644 changelog.d/10238.removal (limited to 'tests/handlers') diff --git a/changelog.d/10238.removal b/changelog.d/10238.removal new file mode 100644 index 0000000000..5fb7bfb47e --- /dev/null +++ b/changelog.d/10238.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. diff --git a/synapse/events/spamcheck.py b/synapse/events/spamcheck.py index 45ec96dfc1..efec16c226 100644 --- a/synapse/events/spamcheck.py +++ b/synapse/events/spamcheck.py @@ -109,6 +109,8 @@ def load_legacy_spam_checkers(hs: "synapse.server.HomeServer"): if f is None: return None + wrapped_func = f + if f.__name__ == "check_registration_for_spam": checker_args = inspect.signature(f) if len(checker_args.parameters) == 3: @@ -133,19 +135,18 @@ def load_legacy_spam_checkers(hs: "synapse.server.HomeServer"): request_info, ) - f = wrapper + wrapped_func = wrapper elif len(checker_args.parameters) != 4: raise RuntimeError( "Bad signature for callback check_registration_for_spam", ) 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 + # mypy doesn't do well across function boundaries so we need to tell it + # wrapped_func is definitely not None. + assert wrapped_func is not None - return maybe_awaitable(f(*args, **kwargs)) + return maybe_awaitable(wrapped_func(*args, **kwargs)) return run diff --git a/tests/handlers/test_register.py b/tests/handlers/test_register.py index a9fd3036dc..c5f6bc3c75 100644 --- a/tests/handlers/test_register.py +++ b/tests/handlers/test_register.py @@ -17,6 +17,7 @@ from unittest.mock import Mock from synapse.api.auth import Auth from synapse.api.constants import UserTypes from synapse.api.errors import Codes, ResourceLimitError, SynapseError +from synapse.events.spamcheck import load_legacy_spam_checkers from synapse.spam_checker_api import RegistrationBehaviour from synapse.types import RoomAlias, UserID, create_requester @@ -79,6 +80,39 @@ class BanBadIdPUser(TestSpamChecker): return RegistrationBehaviour.ALLOW +class TestLegacyRegistrationSpamChecker: + def __init__(self, config, api): + pass + + async def check_registration_for_spam( + self, + email_threepid, + username, + request_info, + ): + pass + + +class LegacyAllowAll(TestLegacyRegistrationSpamChecker): + async def check_registration_for_spam( + self, + email_threepid, + username, + request_info, + ): + return RegistrationBehaviour.ALLOW + + +class LegacyDenyAll(TestLegacyRegistrationSpamChecker): + async def check_registration_for_spam( + self, + email_threepid, + username, + request_info, + ): + return RegistrationBehaviour.DENY + + class RegistrationTestCase(unittest.HomeserverTestCase): """Tests the RegistrationHandler.""" @@ -95,6 +129,8 @@ class RegistrationTestCase(unittest.HomeserverTestCase): hs = self.setup_test_homeserver(config=hs_config) + load_legacy_spam_checkers(hs) + module_api = hs.get_module_api() for module, config in hs.config.modules.loaded_modules: module(config=config, api=module_api) @@ -535,6 +571,46 @@ class RegistrationTestCase(unittest.HomeserverTestCase): """A spam checker can deny registration, which results in an error.""" self.get_failure(self.handler.register_user(localpart="user"), SynapseError) + @override_config( + { + "spam_checker": [ + { + "module": TestSpamChecker.__module__ + ".LegacyAllowAll", + } + ] + } + ) + def test_spam_checker_legacy_allow(self): + """Tests that a legacy spam checker implementing the legacy 3-arg version of the + check_registration_for_spam callback is correctly called. + + In this test and the following one we test both success and failure to make sure + any failure comes from the spam checker (and not something else failing in the + call stack) and any success comes from the spam checker (and not because a + misconfiguration prevented it from being loaded). + """ + self.get_success(self.handler.register_user(localpart="user")) + + @override_config( + { + "spam_checker": [ + { + "module": TestSpamChecker.__module__ + ".LegacyDenyAll", + } + ] + } + ) + def test_spam_checker_legacy_deny(self): + """Tests that a legacy spam checker implementing the legacy 3-arg version of the + check_registration_for_spam callback is correctly called. + + In this test and the previous one we test both success and failure to make sure + any failure comes from the spam checker (and not something else failing in the + call stack) and any success comes from the spam checker (and not because a + misconfiguration prevented it from being loaded). + """ + self.get_failure(self.handler.register_user(localpart="user"), SynapseError) + @override_config( { "modules": [ -- cgit 1.5.1