From 262163c63c8251562fbcc2c254fad9bb1929fa52 Mon Sep 17 00:00:00 2001
From: Brendan Abolivier <babolivier@matrix.org>
Date: Fri, 24 Sep 2021 16:38:23 +0200
Subject: [PATCH 01/10] Add a spamchecker callback to allow or deny room
 creation based on invites (#10898)

This is in the context of creating new module callbacks that modules in https://github.com/matrix-org/synapse-dinsic can use, in an effort to reconcile the spam checker API in synapse-dinsic with the one in mainline.

This adds a callback that's fairly similar to user_may_create_room except it also allows processing based on the invites sent at room creation.
---
 changelog.d/10898.feature              |   1 +
 docs/modules/spam_checker_callbacks.md | 189 +++++++++++++++++++++++++
 synapse/events/spamcheck.py            |  50 ++++++-
 synapse/handlers/room.py               |  16 +--
 tests/rest/client/v1/test_rooms.py     | 119 +++++++++++++++-
 5 files changed, 358 insertions(+), 17 deletions(-)
 create mode 100644 changelog.d/10898.feature
 create mode 100644 docs/modules/spam_checker_callbacks.md

diff --git a/changelog.d/10898.feature b/changelog.d/10898.feature
new file mode 100644
index 000000000..97fa39fd0
--- /dev/null
+++ b/changelog.d/10898.feature
@@ -0,0 +1 @@
+Add a `user_may_create_room_with_invites` spam checker callback to allow modules to allow or deny a room creation request based on the invites and/or 3PID invites it includes.
diff --git a/docs/modules/spam_checker_callbacks.md b/docs/modules/spam_checker_callbacks.md
new file mode 100644
index 000000000..7920ac5f8
--- /dev/null
+++ b/docs/modules/spam_checker_callbacks.md
@@ -0,0 +1,189 @@
+# Spam checker callbacks
+
+Spam checker callbacks allow module developers to implement spam mitigation actions for
+Synapse instances. Spam checker callbacks can be registered using the module API's
+`register_spam_checker_callbacks` method.
+
+## Callbacks
+
+The available spam checker callbacks are:
+
+### `check_event_for_spam`
+
+```python
+async 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.
+
+### `user_may_invite`
+
+```python
+async 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 (e.g. `@alice:example.com`).
+
+### `user_may_create_room`
+
+```python
+async 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.
+
+### `user_may_create_room_with_invites`
+
+```python
+async def user_may_create_room_with_invites(
+    user: str,
+    invites: List[str],
+    threepid_invites: List[Dict[str, str]],
+) -> bool
+```
+
+Called when processing a room creation request (right after `user_may_create_room`).
+The module is given the Matrix user ID of the user trying to create a room, as well as a
+list of Matrix users to invite and a list of third-party identifiers (3PID, e.g. email
+addresses) to invite.
+
+An invited Matrix user to invite is represented by their Matrix user IDs, and an invited
+3PIDs is represented by a dict that includes the 3PID medium (e.g. "email") through its
+`medium` key and its address (e.g. "alice@example.com") through its `address` key.
+
+See [the Matrix specification](https://matrix.org/docs/spec/appendices#pid-types) for more
+information regarding third-party identifiers.
+
+If no invite and/or 3PID invite were specified in the room creation request, the
+corresponding list(s) will be empty.
+
+**Note**: This callback is not called when a room is cloned (e.g. during a room upgrade)
+since no invites are sent when cloning a room. To cover this case, modules also need to
+implement `user_may_create_room`.
+
+### `user_may_create_room_alias`
+
+```python
+async 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.
+
+### `user_may_publish_room`
+
+```python
+async 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.
+
+### `check_username_for_spam`
+
+```python
+async 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.
+
+### `check_registration_for_spam`
+
+```python
+async 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.
+
+### `check_media_file_for_spam`
+
+```python
+async def check_media_file_for_spam(
+    file_wrapper: "synapse.rest.media.v1.media_storage.ReadableFileWrapper",
+    file_info: "synapse.rest.media.v1._base.FileInfo",
+) -> bool
+```
+
+Called when storing a local or remote file. The module must return a boolean indicating
+whether the given file can be stored in the homeserver's media store.
+
+## Example
+
+The example below is a module that implements the spam checker callback
+`check_event_for_spam` to deny any message sent by users whose Matrix user IDs are
+mentioned in a configured list, and registers a web resource to the path
+`/_synapse/client/list_spam_checker/is_evil` that returns a JSON object indicating
+whether the provided user appears in that list.
+
+```python
+import json
+from typing import Union
+
+from twisted.web.resource import Resource
+from twisted.web.server import Request
+
+from synapse.module_api import ModuleApi
+
+
+class IsUserEvilResource(Resource):
+    def __init__(self, config):
+        super(IsUserEvilResource, self).__init__()
+        self.evil_users = config.get("evil_users") or []
+
+    def render_GET(self, request: Request):
+        user = request.args.get(b"user")[0].decode()
+        request.setHeader(b"Content-Type", b"application/json")
+        return json.dumps({"evil": user in self.evil_users}).encode()
+
+
+class ListSpamChecker:
+    def __init__(self, config: dict, api: ModuleApi):
+        self.api = api
+        self.evil_users = config.get("evil_users") or []
+
+        self.api.register_spam_checker_callbacks(
+            check_event_for_spam=self.check_event_for_spam,
+        )
+
+        self.api.register_web_resource(
+            path="/_synapse/client/list_spam_checker/is_evil",
+            resource=IsUserEvilResource(config),
+        )
+
+    async def check_event_for_spam(self, event: "synapse.events.EventBase") -> Union[bool, str]:
+        return event.sender not in self.evil_users
+```
diff --git a/synapse/events/spamcheck.py b/synapse/events/spamcheck.py
index 67d66b5a1..de3a239e9 100644
--- a/synapse/events/spamcheck.py
+++ b/synapse/events/spamcheck.py
@@ -44,13 +44,10 @@
     ["synapse.events.EventBase"],
     Awaitable[Union[bool, str]],
 ]
-# FIXME: Callback signature differs from mainline
-USER_MAY_INVITE_CALLBACK = Callable[
-    [str, Optional[str], Optional[dict], str, bool, bool], Awaitable[bool]
-]
-# FIXME: Callback signature differs from mainline
-USER_MAY_CREATE_ROOM_CALLBACK = Callable[
-    [str, List[str], List[dict], bool], Awaitable[bool]
+USER_MAY_INVITE_CALLBACK = Callable[[str, str, str], Awaitable[bool]]
+USER_MAY_CREATE_ROOM_CALLBACK = Callable[[str], Awaitable[bool]]
+USER_MAY_CREATE_ROOM_WITH_INVITES_CALLBACK = Callable[
+    [str, List[str], List[Dict[str, str]]], Awaitable[bool]
 ]
 USER_MAY_CREATE_ROOM_ALIAS_CALLBACK = Callable[[str, RoomAlias], Awaitable[bool]]
 USER_MAY_PUBLISH_ROOM_CALLBACK = Callable[[str, str], Awaitable[bool]]
@@ -173,6 +170,9 @@ 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_with_invites_callbacks: List[
+            USER_MAY_CREATE_ROOM_WITH_INVITES_CALLBACK
+        ] = []
         self._user_may_create_room_alias_callbacks: List[
             USER_MAY_CREATE_ROOM_ALIAS_CALLBACK
         ] = []
@@ -193,6 +193,9 @@ def register_callbacks(
         check_event_for_spam: Optional[CHECK_EVENT_FOR_SPAM_CALLBACK] = None,
         user_may_invite: Optional[USER_MAY_INVITE_CALLBACK] = None,
         user_may_create_room: Optional[USER_MAY_CREATE_ROOM_CALLBACK] = None,
+        user_may_create_room_with_invites: Optional[
+            USER_MAY_CREATE_ROOM_WITH_INVITES_CALLBACK
+        ] = None,
         user_may_create_room_alias: Optional[
             USER_MAY_CREATE_ROOM_ALIAS_CALLBACK
         ] = None,
@@ -214,6 +217,11 @@ def register_callbacks(
         if user_may_create_room is not None:
             self._user_may_create_room_callbacks.append(user_may_create_room)
 
+        if user_may_create_room_with_invites is not None:
+            self._user_may_create_room_with_invites_callbacks.append(
+                user_may_create_room_with_invites,
+            )
+
         if user_may_create_room_alias is not None:
             self._user_may_create_room_alias_callbacks.append(
                 user_may_create_room_alias,
@@ -338,6 +346,34 @@ async def user_may_create_room(
 
         return True
 
+    async def user_may_create_room_with_invites(
+        self,
+        userid: str,
+        invites: List[str],
+        threepid_invites: List[Dict[str, str]],
+    ) -> bool:
+        """Checks if a given user may create a room with invites
+
+        If this method returns false, the creation request will be rejected.
+
+        Args:
+            userid: The ID of the user attempting to create a room
+            invites: The IDs of the Matrix users to be invited if the room creation is
+                allowed.
+            threepid_invites: The threepids to be invited if the room creation is allowed,
+                as a dict including a "medium" key indicating the threepid's medium (e.g.
+                "email") and an "address" key indicating the threepid's address (e.g.
+                "alice@example.com")
+
+        Returns:
+            True if the user may create the room, otherwise False
+        """
+        for callback in self._user_may_create_room_with_invites_callbacks:
+            if await callback(userid, invites, threepid_invites) is False:
+                return False
+
+        return True
+
     async def user_may_create_room_alias(
         self, userid: str, room_alias: RoomAlias
     ) -> bool:
diff --git a/synapse/handlers/room.py b/synapse/handlers/room.py
index 8ee88b6b8..d5db75aea 100644
--- a/synapse/handlers/room.py
+++ b/synapse/handlers/room.py
@@ -636,14 +636,16 @@ async def create_room(
             requester, config, is_requester_admin=is_requester_admin
         )
 
-        invite_list = config.get("invite", [])
         invite_3pid_list = config.get("invite_3pid", [])
+        invite_list = config.get("invite", [])
 
-        if not is_requester_admin and not await self.spam_checker.user_may_create_room(
-            user_id,
-            invite_list=invite_list,
-            third_party_invite_list=invite_3pid_list,
-            cloning=False,
+        if not is_requester_admin and not (
+            await self.spam_checker.user_may_create_room(user_id)
+            and await self.spam_checker.user_may_create_room_with_invites(
+                user_id,
+                invite_list,
+                invite_3pid_list,
+            )
         ):
             raise SynapseError(403, "You are not permitted to create rooms")
 
@@ -677,8 +679,6 @@ async def create_room(
             if mapping:
                 raise SynapseError(400, "Room alias already taken", Codes.ROOM_IN_USE)
 
-        invite_3pid_list = config.get("invite_3pid", [])
-        invite_list = config.get("invite", [])
         for i in invite_list:
             try:
                 uid = UserID.from_string(i)
diff --git a/tests/rest/client/v1/test_rooms.py b/tests/rest/client/v1/test_rooms.py
index 50100a5ae..f42a60380 100644
--- a/tests/rest/client/v1/test_rooms.py
+++ b/tests/rest/client/v1/test_rooms.py
@@ -18,7 +18,7 @@
 """Tests REST events for /rooms paths."""
 
 import json
-from typing import Iterable
+from typing import Dict, Iterable, List, Optional
 from unittest.mock import Mock, call
 from urllib import parse as urlparse
 
@@ -30,7 +30,7 @@
 from synapse.handlers.pagination import PurgeStatus
 from synapse.rest import admin
 from synapse.rest.client import account, directory, login, profile, room, sync
-from synapse.types import JsonDict, RoomAlias, UserID, create_requester
+from synapse.types import JsonDict, Requester, RoomAlias, UserID, create_requester
 from synapse.util.stringutils import random_string
 
 from tests import unittest
@@ -584,6 +584,121 @@ def test_post_room_invitees_ratelimit(self):
         channel = self.make_request("POST", "/createRoom", content)
         self.assertEqual(200, channel.code)
 
+    def test_spamchecker_invites(self):
+        """Tests the user_may_create_room_with_invites spam checker callback."""
+
+        # Mock do_3pid_invite, so we don't fail from failing to send a 3PID invite to an
+        # IS.
+        async def do_3pid_invite(
+            room_id: str,
+            inviter: UserID,
+            medium: str,
+            address: str,
+            id_server: str,
+            requester: Requester,
+            txn_id: Optional[str],
+            id_access_token: Optional[str] = None,
+        ) -> int:
+            return 0
+
+        do_3pid_invite_mock = Mock(side_effect=do_3pid_invite)
+        self.hs.get_room_member_handler().do_3pid_invite = do_3pid_invite_mock
+
+        # Add a mock callback for user_may_create_room_with_invites. Make it allow any
+        # room creation request for now.
+        return_value = True
+
+        async def user_may_create_room_with_invites(
+            user: str,
+            invites: List[str],
+            threepid_invites: List[Dict[str, str]],
+        ) -> bool:
+            return return_value
+
+        callback_mock = Mock(side_effect=user_may_create_room_with_invites)
+        self.hs.get_spam_checker()._user_may_create_room_with_invites_callbacks.append(
+            callback_mock,
+        )
+
+        # The MXIDs we'll try to invite.
+        invited_mxids = [
+            "@alice1:red",
+            "@alice2:red",
+            "@alice3:red",
+            "@alice4:red",
+        ]
+
+        # The 3PIDs we'll try to invite.
+        invited_3pids = [
+            {
+                "id_server": "example.com",
+                "id_access_token": "sometoken",
+                "medium": "email",
+                "address": "alice1@example.com",
+            },
+            {
+                "id_server": "example.com",
+                "id_access_token": "sometoken",
+                "medium": "email",
+                "address": "alice2@example.com",
+            },
+            {
+                "id_server": "example.com",
+                "id_access_token": "sometoken",
+                "medium": "email",
+                "address": "alice3@example.com",
+            },
+        ]
+
+        # Create a room and invite the Matrix users, and check that it succeeded.
+        channel = self.make_request(
+            "POST",
+            "/createRoom",
+            json.dumps({"invite": invited_mxids}).encode("utf8"),
+        )
+        self.assertEqual(200, channel.code)
+
+        # Check that the callback was called with the right arguments.
+        expected_call_args = ((self.user_id, invited_mxids, []),)
+        self.assertEquals(
+            callback_mock.call_args,
+            expected_call_args,
+            callback_mock.call_args,
+        )
+
+        # Create a room and invite the 3PIDs, and check that it succeeded.
+        channel = self.make_request(
+            "POST",
+            "/createRoom",
+            json.dumps({"invite_3pid": invited_3pids}).encode("utf8"),
+        )
+        self.assertEqual(200, channel.code)
+
+        # Check that do_3pid_invite was called the right amount of time
+        self.assertEquals(do_3pid_invite_mock.call_count, len(invited_3pids))
+
+        # Check that the callback was called with the right arguments.
+        expected_call_args = ((self.user_id, [], invited_3pids),)
+        self.assertEquals(
+            callback_mock.call_args,
+            expected_call_args,
+            callback_mock.call_args,
+        )
+
+        # Now deny any room creation.
+        return_value = False
+
+        # Create a room and invite the 3PIDs, and check that it failed.
+        channel = self.make_request(
+            "POST",
+            "/createRoom",
+            json.dumps({"invite_3pid": invited_3pids}).encode("utf8"),
+        )
+        self.assertEqual(403, channel.code)
+
+        # Check that do_3pid_invite wasn't called this time.
+        self.assertEquals(do_3pid_invite_mock.call_count, len(invited_3pids))
+
 
 class RoomTopicTestCase(RoomBase):
     """Tests /rooms/$room_id/topic REST events."""

From 6bcb28bfd1215b178603e48f0608f71416330c22 Mon Sep 17 00:00:00 2001
From: Brendan Abolivier <babolivier@matrix.org>
Date: Wed, 6 Oct 2021 16:32:16 +0200
Subject: [PATCH 02/10] Add a spamchecker callback to allow or deny room joins
 (#10910)

Co-authored-by: Erik Johnston <erik@matrix.org>
---
 changelog.d/10910.feature              |   1 +
 docs/modules/spam_checker_callbacks.md |  15 ++++
 synapse/events/spamcheck.py            |  24 ++++++
 synapse/handlers/room_member.py        |  21 +++--
 tests/rest/client/v1/test_rooms.py     | 101 +++++++++++++++++++++++++
 5 files changed, 156 insertions(+), 6 deletions(-)
 create mode 100644 changelog.d/10910.feature

diff --git a/changelog.d/10910.feature b/changelog.d/10910.feature
new file mode 100644
index 000000000..aee139f8b
--- /dev/null
+++ b/changelog.d/10910.feature
@@ -0,0 +1 @@
+Add a spam checker callback to allow or deny room joins.
diff --git a/docs/modules/spam_checker_callbacks.md b/docs/modules/spam_checker_callbacks.md
index 7920ac5f8..92376df99 100644
--- a/docs/modules/spam_checker_callbacks.md
+++ b/docs/modules/spam_checker_callbacks.md
@@ -19,6 +19,21 @@ either a `bool` to indicate whether the event must be rejected because of spam,
 to indicate the event must be rejected because of spam and to give a rejection reason to
 forward to clients.
 
+### `user_may_join_room`
+
+```python
+async def user_may_join_room(user: str, room: str, is_invited: bool) -> bool
+```
+
+Called when a user is trying to join a room. The module must return a `bool` to indicate
+whether the user can join the room. The user is represented by their Matrix user ID (e.g.
+`@alice:example.com`) and the room is represented by its Matrix ID (e.g.
+`!room:example.com`). The module is also given a boolean to indicate whether the user
+currently has a pending invite in the room.
+
+This callback isn't called if the join is performed by a server administrator, or in the
+context of a room creation.
+
 ### `user_may_invite`
 
 ```python
diff --git a/synapse/events/spamcheck.py b/synapse/events/spamcheck.py
index de3a239e9..94b4f78e3 100644
--- a/synapse/events/spamcheck.py
+++ b/synapse/events/spamcheck.py
@@ -44,6 +44,7 @@
     ["synapse.events.EventBase"],
     Awaitable[Union[bool, str]],
 ]
+USER_MAY_JOIN_ROOM_CALLBACK = Callable[[str, str, bool], Awaitable[bool]]
 USER_MAY_INVITE_CALLBACK = Callable[[str, str, str], Awaitable[bool]]
 USER_MAY_CREATE_ROOM_CALLBACK = Callable[[str], Awaitable[bool]]
 USER_MAY_CREATE_ROOM_WITH_INVITES_CALLBACK = Callable[
@@ -168,6 +169,7 @@ def run(*args, **kwargs):
 class SpamChecker:
     def __init__(self):
         self._check_event_for_spam_callbacks: List[CHECK_EVENT_FOR_SPAM_CALLBACK] = []
+        self._user_may_join_room_callbacks: List[USER_MAY_JOIN_ROOM_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_with_invites_callbacks: List[
@@ -191,6 +193,7 @@ def __init__(self):
     def register_callbacks(
         self,
         check_event_for_spam: Optional[CHECK_EVENT_FOR_SPAM_CALLBACK] = None,
+        user_may_join_room: Optional[USER_MAY_JOIN_ROOM_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_with_invites: Optional[
@@ -211,6 +214,9 @@ def register_callbacks(
         if check_event_for_spam is not None:
             self._check_event_for_spam_callbacks.append(check_event_for_spam)
 
+        if user_may_join_room is not None:
+            self._user_may_join_room_callbacks.append(user_may_join_room)
+
         if user_may_invite is not None:
             self._user_may_invite_callbacks.append(user_may_invite)
 
@@ -267,6 +273,24 @@ async def check_event_for_spam(
 
         return False
 
+    async def user_may_join_room(self, user_id: str, room_id: str, is_invited: bool):
+        """Checks if a given users is allowed to join a room.
+        Not called when a user creates a room.
+
+        Args:
+            userid: The ID of the user wanting to join the room
+            room_id: The ID of the room the user wants to join
+            is_invited: Whether the user is invited into the room
+
+        Returns:
+            bool: Whether the user may join the room
+        """
+        for callback in self._user_may_join_room_callbacks:
+            if await callback(user_id, room_id, is_invited) is False:
+                return False
+
+        return True
+
     async def user_may_invite(
         self,
         inviter_userid: str,
diff --git a/synapse/handlers/room_member.py b/synapse/handlers/room_member.py
index 362cd0cdd..edbd83889 100644
--- a/synapse/handlers/room_member.py
+++ b/synapse/handlers/room_member.py
@@ -449,6 +449,8 @@ async def update_membership(
             third_party_signed: Information from a 3PID invite.
             ratelimit: Whether to rate limit the request.
             content: The content of the created event.
+            new_room: Whether the membership update is happening in the context of a room
+                creation.
             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
@@ -523,6 +525,8 @@ async def update_membership_locked(
             third_party_signed:
             ratelimit:
             content:
+            new_room: Whether the membership update is happening in the context of a room
+                creation.
             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
@@ -711,24 +715,29 @@ async def update_membership_locked(
                     # so don't really fit into the general auth process.
                     raise AuthError(403, "Guest access not allowed")
 
+            # Figure out whether the user is a server admin to determine whether they
+            # should be able to bypass the spam checker.
             if (
                 self._server_notices_mxid is not None
                 and requester.user.to_string() == self._server_notices_mxid
             ):
                 # allow the server notices mxid to join rooms
-                is_requester_admin = True
+                bypass_spam_checker = True
 
             else:
-                is_requester_admin = await self.auth.is_server_admin(requester.user)
+                bypass_spam_checker = await self.auth.is_server_admin(requester.user)
 
             inviter = await self._get_inviter(target.to_string(), room_id)
-            if not is_requester_admin:
+            if (
+                not bypass_spam_checker
                 # We assume that if the spam checker allowed the user to create
                 # a room then they're allowed to join it.
-                if not new_room and not await self.spam_checker.user_may_join_room(
+                and not new_room
+                and not await self.spam_checker.user_may_join_room(
                     target.to_string(), room_id, is_invited=inviter is not None
-                ):
-                    raise SynapseError(403, "Not allowed to join this room")
+                )
+            ):
+                raise SynapseError(403, "Not allowed to join this room")
 
             # Check if a remote join should be performed.
             remote_join, remote_room_hosts = await self._should_perform_remote_join(
diff --git a/tests/rest/client/v1/test_rooms.py b/tests/rest/client/v1/test_rooms.py
index f42a60380..216615dc8 100644
--- a/tests/rest/client/v1/test_rooms.py
+++ b/tests/rest/client/v1/test_rooms.py
@@ -699,6 +699,30 @@ async def user_may_create_room_with_invites(
         # Check that do_3pid_invite wasn't called this time.
         self.assertEquals(do_3pid_invite_mock.call_count, len(invited_3pids))
 
+    def test_spam_checker_may_join_room(self):
+        """Tests that the user_may_join_room spam checker callback is correctly bypassed
+        when creating a new room.
+        """
+
+        async def user_may_join_room(
+            mxid: str,
+            room_id: str,
+            is_invite: bool,
+        ) -> bool:
+            return False
+
+        join_mock = Mock(side_effect=user_may_join_room)
+        self.hs.get_spam_checker()._user_may_join_room_callbacks.append(join_mock)
+
+        channel = self.make_request(
+            "POST",
+            "/createRoom",
+            {},
+        )
+        self.assertEquals(channel.code, 200, channel.json_body)
+
+        self.assertEquals(join_mock.call_count, 0)
+
 
 class RoomTopicTestCase(RoomBase):
     """Tests /rooms/$room_id/topic REST events."""
@@ -890,6 +914,83 @@ def test_invites_by_users_ratelimit(self):
         self.helper.invite(room_id, self.user_id, "@other-users:red", expect_code=429)
 
 
+class RoomJoinTestCase(RoomBase):
+
+    servlets = [
+        admin.register_servlets,
+        login.register_servlets,
+        room.register_servlets,
+    ]
+
+    def prepare(self, reactor, clock, homeserver):
+        self.user1 = self.register_user("thomas", "hackme")
+        self.tok1 = self.login("thomas", "hackme")
+
+        self.user2 = self.register_user("teresa", "hackme")
+        self.tok2 = self.login("teresa", "hackme")
+
+        self.room1 = self.helper.create_room_as(room_creator=self.user1, tok=self.tok1)
+        self.room2 = self.helper.create_room_as(room_creator=self.user1, tok=self.tok1)
+        self.room3 = self.helper.create_room_as(room_creator=self.user1, tok=self.tok1)
+
+    def test_spam_checker_may_join_room(self):
+        """Tests that the user_may_join_room spam checker callback is correctly called
+        and blocks room joins when needed.
+        """
+
+        # Register a dummy callback. Make it allow all room joins for now.
+        return_value = True
+
+        async def user_may_join_room(
+            userid: str,
+            room_id: str,
+            is_invited: bool,
+        ) -> bool:
+            return return_value
+
+        callback_mock = Mock(side_effect=user_may_join_room)
+        self.hs.get_spam_checker()._user_may_join_room_callbacks.append(callback_mock)
+
+        # Join a first room, without being invited to it.
+        self.helper.join(self.room1, self.user2, tok=self.tok2)
+
+        # Check that the callback was called with the right arguments.
+        expected_call_args = (
+            (
+                self.user2,
+                self.room1,
+                False,
+            ),
+        )
+        self.assertEquals(
+            callback_mock.call_args,
+            expected_call_args,
+            callback_mock.call_args,
+        )
+
+        # Join a second room, this time with an invite for it.
+        self.helper.invite(self.room2, self.user1, self.user2, tok=self.tok1)
+        self.helper.join(self.room2, self.user2, tok=self.tok2)
+
+        # Check that the callback was called with the right arguments.
+        expected_call_args = (
+            (
+                self.user2,
+                self.room2,
+                True,
+            ),
+        )
+        self.assertEquals(
+            callback_mock.call_args,
+            expected_call_args,
+            callback_mock.call_args,
+        )
+
+        # Now make the callback deny all room joins, and check that a join actually fails.
+        return_value = False
+        self.helper.join(self.room3, self.user2, expect_code=403, tok=self.tok2)
+
+
 class RoomJoinRatelimitTestCase(RoomBase):
     user_id = "@sid1:red"
 

From dfcb52c24f4162cf6f2fae47c6566d4605ffae3e Mon Sep 17 00:00:00 2001
From: Brendan Abolivier <babolivier@matrix.org>
Date: Wed, 6 Oct 2021 17:18:13 +0200
Subject: [PATCH 03/10] Add a spamchecker method to allow or deny 3pid invites
 (#10894)

This is in the context of creating new module callbacks that modules in https://github.com/matrix-org/synapse-dinsic can use, in an effort to reconcile the spam checker API in synapse-dinsic with the one in mainline.

Note that a module callback already exists for 3pid invites (https://matrix-org.github.io/synapse/develop/modules/third_party_rules_callbacks.html#check_threepid_can_be_invited) but it doesn't check whether the sender of the invite is allowed to send it.
---
 changelog.d/10894.feature              |  1 +
 docs/modules/spam_checker_callbacks.md | 35 +++++++++++++
 synapse/events/spamcheck.py            | 39 ++++++++++++--
 synapse/handlers/room_member.py        | 12 +++++
 tests/rest/client/v1/test_rooms.py     | 70 ++++++++++++++++++++++++++
 5 files changed, 153 insertions(+), 4 deletions(-)
 create mode 100644 changelog.d/10894.feature

diff --git a/changelog.d/10894.feature b/changelog.d/10894.feature
new file mode 100644
index 000000000..a4f968bed
--- /dev/null
+++ b/changelog.d/10894.feature
@@ -0,0 +1 @@
+Add a `user_may_send_3pid_invite` spam checker callback for modules to allow or deny 3PID invites.
diff --git a/docs/modules/spam_checker_callbacks.md b/docs/modules/spam_checker_callbacks.md
index 92376df99..787e99074 100644
--- a/docs/modules/spam_checker_callbacks.md
+++ b/docs/modules/spam_checker_callbacks.md
@@ -44,6 +44,41 @@ Called when processing an invitation. The module must return a `bool` indicating
 the inviter can invite the invitee to the given room. Both inviter and invitee are
 represented by their Matrix user ID (e.g. `@alice:example.com`).
 
+### `user_may_send_3pid_invite`
+
+```python
+async def user_may_send_3pid_invite(
+    inviter: str,
+    medium: str,
+    address: str,
+    room_id: str,
+) -> bool
+```
+
+Called when processing an invitation using a third-party identifier (also called a 3PID,
+e.g. an email address or a phone number). The module must return a `bool` indicating
+whether the inviter can invite the invitee to the given room.
+
+The inviter is represented by their Matrix user ID (e.g. `@alice:example.com`), and the
+invitee is represented by its medium (e.g. "email") and its address
+(e.g. `alice@example.com`). See [the Matrix specification](https://matrix.org/docs/spec/appendices#pid-types)
+for more information regarding third-party identifiers.
+
+For example, a call to this callback to send an invitation to the email address
+`alice@example.com` would look like this:
+
+```python
+await user_may_send_3pid_invite(
+    "@bob:example.com",  # The inviter's user ID
+    "email",  # The medium of the 3PID to invite
+    "alice@example.com",  # The address of the 3PID to invite
+    "!some_room:example.com",  # The ID of the room to send the invite into
+)
+```
+
+**Note**: If the third-party identifier is already associated with a matrix user ID,
+[`user_may_invite`](#user_may_invite) will be used instead.
+
 ### `user_may_create_room`
 
 ```python
diff --git a/synapse/events/spamcheck.py b/synapse/events/spamcheck.py
index 94b4f78e3..b4cd78ff3 100644
--- a/synapse/events/spamcheck.py
+++ b/synapse/events/spamcheck.py
@@ -46,6 +46,7 @@
 ]
 USER_MAY_JOIN_ROOM_CALLBACK = Callable[[str, str, bool], Awaitable[bool]]
 USER_MAY_INVITE_CALLBACK = Callable[[str, str, str], Awaitable[bool]]
+USER_MAY_SEND_3PID_INVITE_CALLBACK = Callable[[str, str, str, str], Awaitable[bool]]
 USER_MAY_CREATE_ROOM_CALLBACK = Callable[[str], Awaitable[bool]]
 USER_MAY_CREATE_ROOM_WITH_INVITES_CALLBACK = Callable[
     [str, List[str], List[Dict[str, str]]], Awaitable[bool]
@@ -74,9 +75,6 @@
     [ReadableFileWrapper, FileInfo],
     Awaitable[bool],
 ]
-# FIXME: This callback only exists on the DINUM fork and not in mainline.
-USER_MAY_JOIN_ROOM_CALLBACK = Callable[[str, str, bool], Awaitable[bool]]
-
 
 def load_legacy_spam_checkers(hs: "synapse.server.HomeServer"):
     """Wrapper that loads spam checkers configured using the old configuration, and
@@ -171,6 +169,9 @@ def __init__(self):
         self._check_event_for_spam_callbacks: List[CHECK_EVENT_FOR_SPAM_CALLBACK] = []
         self._user_may_join_room_callbacks: List[USER_MAY_JOIN_ROOM_CALLBACK] = []
         self._user_may_invite_callbacks: List[USER_MAY_INVITE_CALLBACK] = []
+        self._user_may_send_3pid_invite_callbacks: List[
+            USER_MAY_SEND_3PID_INVITE_CALLBACK
+        ] = []
         self._user_may_create_room_callbacks: List[USER_MAY_CREATE_ROOM_CALLBACK] = []
         self._user_may_create_room_with_invites_callbacks: List[
             USER_MAY_CREATE_ROOM_WITH_INVITES_CALLBACK
@@ -195,6 +196,7 @@ def register_callbacks(
         check_event_for_spam: Optional[CHECK_EVENT_FOR_SPAM_CALLBACK] = None,
         user_may_join_room: Optional[USER_MAY_JOIN_ROOM_CALLBACK] = None,
         user_may_invite: Optional[USER_MAY_INVITE_CALLBACK] = None,
+        user_may_send_3pid_invite: Optional[USER_MAY_SEND_3PID_INVITE_CALLBACK] = None,
         user_may_create_room: Optional[USER_MAY_CREATE_ROOM_CALLBACK] = None,
         user_may_create_room_with_invites: Optional[
             USER_MAY_CREATE_ROOM_WITH_INVITES_CALLBACK
@@ -208,7 +210,6 @@ def register_callbacks(
             CHECK_REGISTRATION_FOR_SPAM_CALLBACK
         ] = None,
         check_media_file_for_spam: Optional[CHECK_MEDIA_FILE_FOR_SPAM_CALLBACK] = None,
-        user_may_join_room: Optional[USER_MAY_JOIN_ROOM_CALLBACK] = None,
     ):
         """Register callbacks from module for each hook."""
         if check_event_for_spam is not None:
@@ -220,6 +221,11 @@ def register_callbacks(
         if user_may_invite is not None:
             self._user_may_invite_callbacks.append(user_may_invite)
 
+        if user_may_send_3pid_invite is not None:
+            self._user_may_send_3pid_invite_callbacks.append(
+                user_may_send_3pid_invite,
+            )
+
         if user_may_create_room is not None:
             self._user_may_create_room_callbacks.append(user_may_create_room)
 
@@ -338,6 +344,31 @@ async def user_may_invite(
 
         return True
 
+    async def user_may_send_3pid_invite(
+        self, inviter_userid: str, medium: str, address: str, room_id: str
+    ) -> bool:
+        """Checks if a given user may invite a given threepid into the room
+
+        If this method returns false, the threepid invite will be rejected.
+
+        Note that if the threepid is already associated with a Matrix user ID, Synapse
+        will call user_may_invite with said user ID instead.
+
+        Args:
+            inviter_userid: The user ID of the sender of the invitation
+            medium: The 3PID's medium (e.g. "email")
+            address: The 3PID's address (e.g. "alice@example.com")
+            room_id: The room ID
+
+        Returns:
+            True if the user may send the invite, otherwise False
+        """
+        for callback in self._user_may_send_3pid_invite_callbacks:
+            if await callback(inviter_userid, medium, address, room_id) is False:
+                return False
+
+        return True
+
     async def user_may_create_room(
         self,
         userid: str,
diff --git a/synapse/handlers/room_member.py b/synapse/handlers/room_member.py
index edbd83889..e5da79ca4 100644
--- a/synapse/handlers/room_member.py
+++ b/synapse/handlers/room_member.py
@@ -1253,10 +1253,22 @@ async def do_3pid_invite(
         if invitee:
             # Note that update_membership with an action of "invite" can raise
             # a ShadowBanError, but this was done above already.
+            # We don't check the invite against the spamchecker(s) here (through
+            # user_may_invite) because we'll do it further down the line anyway (in
+            # update_membership_locked).
             _, stream_id = await self.update_membership(
                 requester, UserID.from_string(invitee), room_id, "invite", txn_id=txn_id
             )
         else:
+            # Check if the spamchecker(s) allow this invite to go through.
+            if not await self.spam_checker.user_may_send_3pid_invite(
+                inviter_userid=requester.user.to_string(),
+                medium=medium,
+                address=address,
+                room_id=room_id,
+            ):
+                raise SynapseError(403, "Cannot send threepid invite")
+
             stream_id = await self._make_and_store_3pid_invite(
                 requester,
                 id_server,
diff --git a/tests/rest/client/v1/test_rooms.py b/tests/rest/client/v1/test_rooms.py
index 216615dc8..b726310b4 100644
--- a/tests/rest/client/v1/test_rooms.py
+++ b/tests/rest/client/v1/test_rooms.py
@@ -2446,3 +2446,73 @@ def test_bad_alias(self):
         """An alias which does not point to the room raises a SynapseError."""
         self._set_canonical_alias({"alias": "@unknown:test"}, expected_code=400)
         self._set_canonical_alias({"alt_aliases": ["@unknown:test"]}, expected_code=400)
+
+
+class ThreepidInviteTestCase(unittest.HomeserverTestCase):
+
+    servlets = [
+        admin.register_servlets,
+        login.register_servlets,
+        room.register_servlets,
+    ]
+
+    def prepare(self, reactor, clock, homeserver):
+        self.user_id = self.register_user("thomas", "hackme")
+        self.tok = self.login("thomas", "hackme")
+
+        self.room_id = self.helper.create_room_as(self.user_id, tok=self.tok)
+
+    def test_threepid_invite_spamcheck(self):
+        # Mock a few functions to prevent the test from failing due to failing to talk to
+        # a remote IS. We keep the mock for _mock_make_and_store_3pid_invite around so we
+        # can check its call_count later on during the test.
+        make_invite_mock = Mock(return_value=make_awaitable(0))
+        self.hs.get_room_member_handler()._make_and_store_3pid_invite = make_invite_mock
+        self.hs.get_identity_handler().lookup_3pid = Mock(
+            return_value=make_awaitable(None),
+        )
+
+        # Add a mock to the spamchecker callbacks for user_may_send_3pid_invite. Make it
+        # allow everything for now.
+        mock = Mock(return_value=make_awaitable(True))
+        self.hs.get_spam_checker()._user_may_send_3pid_invite_callbacks.append(mock)
+
+        # Send a 3PID invite into the room and check that it succeeded.
+        email_to_invite = "teresa@example.com"
+        channel = self.make_request(
+            method="POST",
+            path="/rooms/" + self.room_id + "/invite",
+            content={
+                "id_server": "example.com",
+                "id_access_token": "sometoken",
+                "medium": "email",
+                "address": email_to_invite,
+            },
+            access_token=self.tok,
+        )
+        self.assertEquals(channel.code, 200)
+
+        # Check that the callback was called with the right params.
+        mock.assert_called_with(self.user_id, "email", email_to_invite, self.room_id)
+
+        # Check that the call to send the invite was made.
+        make_invite_mock.assert_called_once()
+
+        # Now change the return value of the callback to deny any invite and test that
+        # we can't send the invite.
+        mock.return_value = make_awaitable(False)
+        channel = self.make_request(
+            method="POST",
+            path="/rooms/" + self.room_id + "/invite",
+            content={
+                "id_server": "example.com",
+                "id_access_token": "sometoken",
+                "medium": "email",
+                "address": email_to_invite,
+            },
+            access_token=self.tok,
+        )
+        self.assertEquals(channel.code, 403)
+
+        # Also check that it stopped before calling _make_and_store_3pid_invite.
+        make_invite_mock.assert_called_once()

From c9c1bed22c868262f23646a13d85fa634f88410b Mon Sep 17 00:00:00 2001
From: Brendan Abolivier <babolivier@matrix.org>
Date: Tue, 26 Oct 2021 17:26:59 +0100
Subject: [PATCH 04/10] Converge with mainline

Bring other callbacks to party with mainline, and fixup code calling to
the various callbacks.
---
 synapse/events/spamcheck.py     | 107 +++++++-------------------------
 synapse/handlers/federation.py  |   9 +--
 synapse/handlers/room.py        |  15 +----
 synapse/handlers/room_member.py |  23 +------
 synapse/rest/client/room.py     |   1 -
 5 files changed, 26 insertions(+), 129 deletions(-)

diff --git a/synapse/events/spamcheck.py b/synapse/events/spamcheck.py
index b4cd78ff3..3134beb8d 100644
--- a/synapse/events/spamcheck.py
+++ b/synapse/events/spamcheck.py
@@ -76,13 +76,14 @@
     Awaitable[bool],
 ]
 
-def load_legacy_spam_checkers(hs: "synapse.server.HomeServer"):
+
+def load_legacy_spam_checkers(hs: "synapse.server.HomeServer") -> None:
     """Wrapper that loads spam checkers configured using the old configuration, and
     registers the spam checker hooks they implement.
     """
     spam_checkers: List[Any] = []
     api = hs.get_module_api()
-    for module, config in hs.config.spam_checkers:
+    for module, config in hs.config.spamchecker.spam_checkers:
         # Older spam checkers don't accept the `api` argument, so we
         # try and detect support.
         spam_args = inspect.getfullargspec(module)
@@ -102,7 +103,6 @@ def load_legacy_spam_checkers(hs: "synapse.server.HomeServer"):
         "check_username_for_spam",
         "check_registration_for_spam",
         "check_media_file_for_spam",
-        "user_may_join_room",
     }
 
     for spam_checker in spam_checkers:
@@ -129,9 +129,9 @@ def wrapper(
                         request_info: Collection[Tuple[str, str]],
                         auth_provider_id: Optional[str],
                     ) -> Union[Awaitable[RegistrationBehaviour], RegistrationBehaviour]:
-                        # We've already made sure f is not None above, but mypy doesn't
-                        # do well across function boundaries so we need to tell it f is
-                        # definitely not None.
+                        # Assertion required because mypy can't prove we won't
+                        # change `f` back to `None`. See
+                        # https://mypy.readthedocs.io/en/latest/common_issues.html#narrowing-and-inner-functions
                         assert f is not None
 
                         return f(
@@ -146,9 +146,10 @@ def wrapper(
                         "Bad signature for callback check_registration_for_spam",
                     )
 
-            def run(*args, **kwargs):
-                # mypy doesn't do well across function boundaries so we need to tell it
-                # wrapped_func is definitely not None.
+            def run(*args: Any, **kwargs: Any) -> Awaitable:
+                # Assertion required because mypy can't prove we won't change `f`
+                # back to `None`. See
+                # https://mypy.readthedocs.io/en/latest/common_issues.html#narrowing-and-inner-functions
                 assert wrapped_func is not None
 
                 return maybe_awaitable(wrapped_func(*args, **kwargs))
@@ -165,7 +166,7 @@ def run(*args, **kwargs):
 
 
 class SpamChecker:
-    def __init__(self):
+    def __init__(self) -> None:
         self._check_event_for_spam_callbacks: List[CHECK_EVENT_FOR_SPAM_CALLBACK] = []
         self._user_may_join_room_callbacks: List[USER_MAY_JOIN_ROOM_CALLBACK] = []
         self._user_may_invite_callbacks: List[USER_MAY_INVITE_CALLBACK] = []
@@ -189,7 +190,6 @@ def __init__(self):
         self._check_media_file_for_spam_callbacks: List[
             CHECK_MEDIA_FILE_FOR_SPAM_CALLBACK
         ] = []
-        self._user_may_join_room_callbacks: List[USER_MAY_JOIN_ROOM_CALLBACK] = []
 
     def register_callbacks(
         self,
@@ -210,7 +210,7 @@ def register_callbacks(
             CHECK_REGISTRATION_FOR_SPAM_CALLBACK
         ] = None,
         check_media_file_for_spam: Optional[CHECK_MEDIA_FILE_FOR_SPAM_CALLBACK] = None,
-    ):
+    ) -> 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)
@@ -253,9 +253,6 @@ def register_callbacks(
         if check_media_file_for_spam is not None:
             self._check_media_file_for_spam_callbacks.append(check_media_file_for_spam)
 
-        if user_may_join_room is not None:
-            self._user_may_join_room_callbacks.append(user_may_join_room)
-
     async def check_event_for_spam(
         self, event: "synapse.events.EventBase"
     ) -> Union[bool, str]:
@@ -279,7 +276,9 @@ async def check_event_for_spam(
 
         return False
 
-    async def user_may_join_room(self, user_id: str, room_id: str, is_invited: bool):
+    async def user_may_join_room(
+        self, user_id: str, room_id: str, is_invited: bool
+    ) -> bool:
         """Checks if a given users is allowed to join a room.
         Not called when a user creates a room.
 
@@ -289,7 +288,7 @@ async def user_may_join_room(self, user_id: str, room_id: str, is_invited: bool)
             is_invited: Whether the user is invited into the room
 
         Returns:
-            bool: Whether the user may join the room
+            Whether the user may join the room
         """
         for callback in self._user_may_join_room_callbacks:
             if await callback(user_id, room_id, is_invited) is False:
@@ -298,48 +297,22 @@ async def user_may_join_room(self, user_id: str, room_id: str, is_invited: bool)
         return True
 
     async def user_may_invite(
-        self,
-        inviter_userid: str,
-        invitee_userid: Optional[str],
-        third_party_invite: Optional[Dict],
-        room_id: str,
-        new_room: bool,
-        published_room: bool,
+        self, inviter_userid: str, invitee_userid: str, room_id: str
     ) -> bool:
         """Checks if a given user may send an invite
 
         If this method returns false, the invite will be rejected.
 
         Args:
-            inviter_userid:
-            invitee_userid: The user ID of the invitee. Is None
-                if this is a third party invite and the 3PID is not bound to a
-                user ID.
-            third_party_invite: If a third party invite then is a
-                dict containing the medium and address of the invitee.
-            room_id:
-            new_room: Whether the user is being invited to the room as
-                part of a room creation, if so the invitee would have been
-                included in the call to `user_may_create_room`.
-            published_room: Whether the room the user is being invited
-                to has been published in the local homeserver's public room
-                directory.
+            inviter_userid: The user ID of the sender of the invitation
+            invitee_userid: The user ID targeted in the invitation
+            room_id: The room ID
 
         Returns:
             True if the user may send an invite, otherwise False
         """
         for callback in self._user_may_invite_callbacks:
-            if (
-                await callback(
-                    inviter_userid,
-                    invitee_userid,
-                    third_party_invite,
-                    room_id,
-                    new_room,
-                    published_room,
-                )
-                is False
-            ):
+            if await callback(inviter_userid, invitee_userid, room_id) is False:
                 return False
 
         return True
@@ -369,34 +342,19 @@ async def user_may_send_3pid_invite(
 
         return True
 
-    async def user_may_create_room(
-        self,
-        userid: str,
-        invite_list: List[str],
-        third_party_invite_list: List[Dict],
-        cloning: bool,
-    ) -> bool:
+    async def user_may_create_room(self, userid: str) -> bool:
         """Checks if a given user may create a room
 
         If this method returns false, the creation request will be rejected.
 
         Args:
             userid: The ID of the user attempting to create a room
-            invite_list: List of user IDs that would be invited to
-                the new room.
-            third_party_invite_list: List of third party invites
-                for the new room.
-            cloning: Whether the user is cloning an existing room, e.g.
-                upgrading a room.
 
         Returns:
             True if the user may create a room, otherwise False
         """
         for callback in self._user_may_create_room_callbacks:
-            if (
-                await callback(userid, invite_list, third_party_invite_list, cloning)
-                is False
-            ):
+            if await callback(userid) is False:
                 return False
 
         return True
@@ -467,25 +425,6 @@ async def user_may_publish_room(self, userid: str, room_id: str) -> bool:
 
         return True
 
-    async def user_may_join_room(self, userid: str, room_id: str, is_invited: bool):
-        """Checks if a given users is allowed to join a room.
-
-        Not called when a user creates a room.
-
-        Args:
-            userid:
-            room_id:
-            is_invited: Whether the user is invited into the room
-
-        Returns:
-            bool: Whether the user may join the room
-        """
-        for callback in self._user_may_join_room_callbacks:
-            if await callback(userid, room_id, is_invited) is False:
-                return False
-
-        return True
-
     async def check_username_for_spam(self, user_profile: Dict[str, str]) -> bool:
         """Checks if a user ID or display name are considered "spammy" by this server.
 
diff --git a/synapse/handlers/federation.py b/synapse/handlers/federation.py
index 5f5ee1d30..110b6bdf7 100644
--- a/synapse/handlers/federation.py
+++ b/synapse/handlers/federation.py
@@ -1893,15 +1893,8 @@ async def on_invite_request(
         if self.hs.config.block_non_admin_invites:
             raise SynapseError(403, "This server does not accept room invites")
 
-        is_published = await self.store.is_room_published(event.room_id)
-
         if not await self.spam_checker.user_may_invite(
-            event.sender,
-            event.state_key,
-            None,
-            room_id=event.room_id,
-            new_room=False,
-            published_room=is_published,
+            event.sender, event.state_key, event.room_id
         ):
             raise SynapseError(
                 403, "This user is not permitted to send invites to this server/user"
diff --git a/synapse/handlers/room.py b/synapse/handlers/room.py
index d5db75aea..5030542af 100644
--- a/synapse/handlers/room.py
+++ b/synapse/handlers/room.py
@@ -374,19 +374,7 @@ async def clone_existing_room(
         """
         user_id = requester.user.to_string()
 
-        if (
-            self._server_notices_mxid is not None
-            and requester.user.to_string() == self._server_notices_mxid
-        ):
-            # allow the server notices mxid to create rooms
-            is_requester_admin = True
-
-        else:
-            is_requester_admin = await self.auth.is_server_admin(requester.user)
-
-        if not is_requester_admin and not await self.spam_checker.user_may_create_room(
-            user_id, invite_list=[], third_party_invite_list=[], cloning=True
-        ):
+        if not await self.spam_checker.user_may_create_room(user_id):
             raise SynapseError(403, "You are not permitted to create rooms")
 
         creation_content: JsonDict = {
@@ -861,7 +849,6 @@ async def create_room(
                 id_server,
                 requester,
                 txn_id=None,
-                new_room=True,
                 id_access_token=id_access_token,
             )
 
diff --git a/synapse/handlers/room_member.py b/synapse/handlers/room_member.py
index e5da79ca4..9655d7f0a 100644
--- a/synapse/handlers/room_member.py
+++ b/synapse/handlers/room_member.py
@@ -614,15 +614,8 @@ async def update_membership_locked(
                     )
                     block_invite = True
 
-                is_published = await self.store.is_room_published(room_id)
-
                 if not await self.spam_checker.user_may_invite(
-                    requester.user.to_string(),
-                    target_id,
-                    third_party_invite=None,
-                    room_id=room_id,
-                    new_room=new_room,
-                    published_room=is_published,
+                    requester.user.to_string(), target_id, room_id
                 ):
                     logger.info("Blocking invite due to spam checker")
                     block_invite = True
@@ -1170,7 +1163,6 @@ async def do_3pid_invite(
         id_server: str,
         requester: Requester,
         txn_id: Optional[str],
-        new_room: bool = False,
         id_access_token: Optional[str] = None,
     ) -> int:
         """Invite a 3PID to a room.
@@ -1237,19 +1229,6 @@ async def do_3pid_invite(
             id_server, medium, address, id_access_token
         )
 
-        is_published = await self.store.is_room_published(room_id)
-
-        if not await self.spam_checker.user_may_invite(
-            requester.user.to_string(),
-            invitee,
-            third_party_invite={"medium": medium, "address": address},
-            room_id=room_id,
-            new_room=new_room,
-            published_room=is_published,
-        ):
-            logger.info("Blocking invite due to spam checker")
-            raise SynapseError(403, "Invites have been disabled on this server")
-
         if invitee:
             # Note that update_membership with an action of "invite" can raise
             # a ShadowBanError, but this was done above already.
diff --git a/synapse/rest/client/room.py b/synapse/rest/client/room.py
index 98c3a5e33..bd6885e5d 100644
--- a/synapse/rest/client/room.py
+++ b/synapse/rest/client/room.py
@@ -734,7 +734,6 @@ async def on_POST(self, request, room_id, membership_action, txn_id=None):
                     content["id_server"],
                     requester,
                     txn_id,
-                    new_room=False,
                     id_access_token=content.get("id_access_token"),
                 )
             except ShadowBanError:

From 23e48a1f8ef61de97abd7a5c00b38e601fd6e9a4 Mon Sep 17 00:00:00 2001
From: Brendan Abolivier <babolivier@matrix.org>
Date: Fri, 29 Oct 2021 18:28:29 +0200
Subject: [PATCH 05/10] Add a module API method to retrieve state from a room
 (#11204)

---
 changelog.d/11204.feature      |  1 +
 synapse/module_api/__init__.py | 57 +++++++++++++++++++++++++++++++++-
 tests/module_api/test_api.py   | 25 ++++++++++++++-
 3 files changed, 81 insertions(+), 2 deletions(-)
 create mode 100644 changelog.d/11204.feature

diff --git a/changelog.d/11204.feature b/changelog.d/11204.feature
new file mode 100644
index 000000000..f58ed4b3d
--- /dev/null
+++ b/changelog.d/11204.feature
@@ -0,0 +1 @@
+Add a module API method to retrieve the current state of a room.
diff --git a/synapse/module_api/__init__.py b/synapse/module_api/__init__.py
index 2d2ed229e..739939b6e 100644
--- a/synapse/module_api/__init__.py
+++ b/synapse/module_api/__init__.py
@@ -45,7 +45,14 @@
 from synapse.storage.database import DatabasePool, LoggingTransaction
 from synapse.storage.databases.main.roommember import ProfileInfo
 from synapse.storage.state import StateFilter
-from synapse.types import JsonDict, Requester, UserID, UserInfo, create_requester
+from synapse.types import (
+    JsonDict,
+    Requester,
+    StateMap,
+    UserID,
+    UserInfo,
+    create_requester,
+)
 from synapse.util import Clock
 from synapse.util.caches.descriptors import cached
 
@@ -70,6 +77,8 @@
     "DirectServeHtmlResource",
     "DirectServeJsonResource",
     "ModuleApi",
+    "EventBase",
+    "StateMap",
 ]
 
 logger = logging.getLogger(__name__)
@@ -690,6 +699,52 @@ def read_templates(
             (td for td in (self.custom_template_dir, custom_template_directory) if td),
         )
 
+    async def get_room_state(
+        self,
+        room_id: str,
+        event_filter: Optional[Iterable[Tuple[str, Optional[str]]]] = None,
+    ) -> StateMap[EventBase]:
+        """Returns the current state of the given room.
+
+        The events are returned as a mapping, in which the key for each event is a tuple
+        which first element is the event's type and the second one is its state key.
+
+        Added in Synapse v1.47.0
+
+        Args:
+            room_id: The ID of the room to get state from.
+            event_filter: A filter to apply when retrieving events. None if no filter
+                should be applied. If provided, must be an iterable of tuples. A tuple's
+                first element is the event type and the second is the state key, or is
+                None if the state key should not be filtered on.
+                An example of a filter is:
+                    [
+                        ("m.room.member", "@alice:example.com"),  # Member event for @alice:example.com
+                        ("org.matrix.some_event", ""),  # State event of type "org.matrix.some_event"
+                                                        # with an empty string as its state key
+                        ("org.matrix.some_other_event", None),  # State events of type "org.matrix.some_other_event"
+                                                                # regardless of their state key
+                    ]
+        """
+        if event_filter:
+            # If a filter was provided, turn it into a StateFilter and retrieve a filtered
+            # view of the state.
+            state_filter = StateFilter.from_types(event_filter)
+            state_ids = await self._store.get_filtered_current_state_ids(
+                room_id,
+                state_filter,
+            )
+        else:
+            # If no filter was provided, get the whole state. We could also reuse the call
+            # to get_filtered_current_state_ids above, with `state_filter = StateFilter.all()`,
+            # but get_filtered_current_state_ids isn't cached and `get_current_state_ids`
+            # is, so using the latter when we can is better for perf.
+            state_ids = await self._store.get_current_state_ids(room_id)
+
+        state_events = await self._store.get_events(state_ids.values())
+
+        return {key: state_events[event_id] for key, event_id in state_ids.items()}
+
 
 class PublicRoomListManager:
     """Contains methods for adding to, removing from and querying whether a room
diff --git a/tests/module_api/test_api.py b/tests/module_api/test_api.py
index 7dd519cd4..9904e1bd1 100644
--- a/tests/module_api/test_api.py
+++ b/tests/module_api/test_api.py
@@ -15,7 +15,7 @@
 
 from twisted.internet import defer
 
-from synapse.api.constants import EduTypes
+from synapse.api.constants import EduTypes, EventTypes
 from synapse.events import EventBase
 from synapse.federation.units import Transaction
 from synapse.handlers.presence import UserPresenceState
@@ -308,6 +308,29 @@ def test_send_local_online_presence_to_federation(self):
 
         self.assertTrue(found_update)
 
+    def test_get_room_state(self):
+        """Tests that a module can retrieve the state of a room through the module API."""
+        user_id = self.register_user("peter", "hackme")
+        tok = self.login("peter", "hackme")
+
+        # Create a room and send some custom state in it.
+        room_id = self.helper.create_room_as(tok=tok)
+        self.helper.send_state(room_id, "org.matrix.test", {}, tok=tok)
+
+        # Check that the module API can successfully fetch state for the room.
+        state = self.get_success(
+            defer.ensureDeferred(self.module_api.get_room_state(room_id))
+        )
+
+        # Check that a few standard events are in the returned state.
+        self.assertIn((EventTypes.Create, ""), state)
+        self.assertIn((EventTypes.Member, user_id), state)
+
+        # Check that our custom state event is in the returned state.
+        self.assertEqual(state[("org.matrix.test", "")].sender, user_id)
+        self.assertEqual(state[("org.matrix.test", "")].state_key, "")
+        self.assertEqual(state[("org.matrix.test", "")].content, {})
+
 
 class ModuleApiWorkerTestCase(BaseMultiWorkerStreamTestCase):
     """For testing ModuleApi functionality in a multi-worker setup"""

From 481fbb0a670410bc8283137c7bac509cc63f38d1 Mon Sep 17 00:00:00 2001
From: Brendan Abolivier <babolivier@matrix.org>
Date: Wed, 3 Nov 2021 17:14:23 +0100
Subject: [PATCH 06/10] Update DomainRuleChecker to use the new interface

---
 synapse/rulecheck/domain_rule_checker.py | 140 +++++++++++++++++------
 1 file changed, 108 insertions(+), 32 deletions(-)

diff --git a/synapse/rulecheck/domain_rule_checker.py b/synapse/rulecheck/domain_rule_checker.py
index 11e7cb59d..96424505e 100644
--- a/synapse/rulecheck/domain_rule_checker.py
+++ b/synapse/rulecheck/domain_rule_checker.py
@@ -14,8 +14,11 @@
 # limitations under the License.
 
 import logging
+from typing import Dict, Optional
 
+from synapse.api.constants import EventTypes, Membership
 from synapse.config._base import ConfigError
+from synapse.module_api import ModuleApi
 
 logger = logging.getLogger(__name__)
 
@@ -56,7 +59,7 @@ class DomainRuleChecker(object):
     Don't forget to consider if you can invite users from your own domain.
     """
 
-    def __init__(self, config):
+    def __init__(self, config, api: ModuleApi):
         self.domain_mapping = config["domain_mapping"] or {}
         self.default = config["default"]
 
@@ -76,30 +79,103 @@ def __init__(self, config):
             "domains_prevented_from_being_invited_to_published_rooms", []
         )
 
-    def check_event_for_spam(self, event):
-        """Implements synapse.events.SpamChecker.check_event_for_spam"""
-        return False
+        self._api = api
 
-    def user_may_invite(
+        self._api.register_spam_checker_callbacks(
+            user_may_invite=self.user_may_invite,
+            user_may_send_3pid_invite=self.user_may_send_3pid_invite,
+            user_may_create_room=self.user_may_create_room,
+            user_may_join_room=self.user_may_join_room,
+        )
+
+    async def _is_new_room(self, room_id: str) -> bool:
+        """Checks if the room provided looks new according to its state.
+
+        The module will consider a room to look new if the only m.room.member events in
+        its state are either for the room's creator (i.e. its join event) or invites sent
+        by the room's creator.
+
+        Args:
+            room_id: The ID of the room to check.
+
+        Returns:
+            Whether the room looks new.
+        """
+        state_event_filter = [
+            (EventTypes.Create, None),
+            (EventTypes.Member, None),
+        ]
+
+        events = await self._api.get_room_state(room_id, state_event_filter)
+
+        room_creator = events[(EventTypes.Create, "")].sender
+
+        for key, event in events.items():
+            if key[0] == EventTypes.Create:
+                continue
+
+            if key[1] != room_creator:
+                if (
+                    event.sender != room_creator
+                    and event.membership != Membership.INVITE
+                ):
+                    return False
+
+        return True
+
+    async def user_may_invite(
         self,
-        inviter_userid,
-        invitee_userid,
-        third_party_invite,
-        room_id,
-        new_room,
-        published_room=False,
-    ):
-        """Implements synapse.events.SpamChecker.user_may_invite"""
-        if self.can_only_invite_during_room_creation and not new_room:
-            return False
+        inviter_userid: str,
+        invitee_userid: str,
+        room_id: str,
+    ) -> bool:
+        """Implements the user_may_invite spam checker callback."""
+        return await self._user_may_invite(
+            room_id=room_id,
+            inviter_userid=inviter_userid,
+            invitee_userid=invitee_userid,
+        )
 
-        if not self.can_invite_by_third_party_id and third_party_invite:
+    async def user_may_send_3pid_invite(
+        self,
+        inviter_userid: str,
+        medium: str,
+        address: str,
+        room_id: str,
+    ) -> bool:
+        """Implements the user_may_send_3pid_invite spam checker callback."""
+        return await self._user_may_invite(
+            room_id=room_id, inviter_userid=inviter_userid, invitee_userid=None,
+        )
+
+    async def _user_may_invite(
+        self,
+        room_id: str,
+        inviter_userid: str,
+        invitee_userid: Optional[str],
+    ) -> bool:
+        """Processes any incoming invite, both normal Matrix invites and 3PID ones, and
+        check if they should be allowed.
+
+        Args:
+            room_id: The ID of the room the invite is happening in.
+            inviter_userid: The MXID of the user sending the invite.
+            invitee_userid: The MXID of the user being invited, or None if this is a 3PID
+                invite (in which case no MXID exists for this user yet).
+
+        Returns:
+            Whether the invite can be allowed to go through.
+        """
+        new_room = await self._is_new_room(room_id)
+
+        if self.can_only_invite_during_room_creation and not new_room:
             return False
 
-        # This is a third party invite (without a bound mxid), so unless we have
-        # banned all third party invites (above) we allow it.
-        if not invitee_userid:
-            return True
+        # If invitee_userid is None, then this means this is a 3PID invite (without a
+        # bound MXID), so we allow it unless the configuration mandates blocking all 3PID
+        # invites.
+        if invitee_userid is None:
+            return self.can_invite_by_third_party_id
 
         inviter_domain = self._get_domain_from_id(inviter_userid)
         invitee_domain = self._get_domain_from_id(invitee_userid)
@@ -107,6 +183,12 @@ def user_may_invite(
         if inviter_domain not in self.domain_mapping:
             return self.default
 
+        published_room = (
+            await self._api.public_room_list_manager.room_is_in_public_room_list(
+                room_id
+            )
+    )
+
         if (
             published_room
             and invitee_domain
@@ -116,7 +198,7 @@ def user_may_invite(
 
         return invitee_domain in self.domain_mapping[inviter_domain]
 
-    def user_may_create_room(
+    async def user_may_create_room(
         self, userid, invite_list, third_party_invite_list, cloning
     ):
         """Implements synapse.events.SpamChecker.user_may_create_room"""
@@ -134,16 +216,8 @@ def user_may_create_room(
 
         return True
 
-    def user_may_create_room_alias(self, userid, room_alias):
-        """Implements synapse.events.SpamChecker.user_may_create_room_alias"""
-        return True
-
-    def user_may_publish_room(self, userid, room_id):
-        """Implements synapse.events.SpamChecker.user_may_publish_room"""
-        return True
-
-    def user_may_join_room(self, userid, room_id, is_invited):
-        """Implements synapse.events.SpamChecker.user_may_join_room"""
+    async def user_may_join_room(self, userid, room_id, is_invited):
+        """Implements the user_may_join_room spam checker callback."""
         if self.can_only_join_rooms_with_invite and not is_invited:
             return False
 
@@ -151,7 +225,9 @@ def user_may_join_room(self, userid, room_id, is_invited):
 
     @staticmethod
     def parse_config(config):
-        """Implements synapse.events.SpamChecker.parse_config"""
+        """Checks whether required fields exist in the provided configuration for the
+        module.
+        """
         if "default" in config:
             return config
         else:

From 58d6cdd10d9abaa9fffdfc273631178905a105b6 Mon Sep 17 00:00:00 2001
From: Brendan Abolivier <babolivier@matrix.org>
Date: Wed, 3 Nov 2021 17:17:50 +0100
Subject: [PATCH 07/10] Update tests

---
 tests/rulecheck/test_domainrulecheck.py | 170 +++++++++++++++++-------
 1 file changed, 123 insertions(+), 47 deletions(-)

diff --git a/tests/rulecheck/test_domainrulecheck.py b/tests/rulecheck/test_domainrulecheck.py
index eee980c9a..b66fffc4a 100644
--- a/tests/rulecheck/test_domainrulecheck.py
+++ b/tests/rulecheck/test_domainrulecheck.py
@@ -15,17 +15,85 @@
 
 
 import json
+from typing import Optional
+
+import attr
 
 import synapse.rest.admin
+from synapse.api.constants import EventTypes
 from synapse.config._base import ConfigError
-from synapse.events.spamcheck import load_legacy_spam_checkers
 from synapse.rest.client import login, room
 from synapse.rulecheck.domain_rule_checker import DomainRuleChecker
 
 from tests import unittest
 
 
-class DomainRuleCheckerTestCase(unittest.TestCase):
+@attr.s(auto_attribs=True)
+class MockEvent:
+    """Mock of an event, only implementing the fields the DomainRuleChecker module will
+    use.
+    """
+    sender: str
+    membership: Optional[str] = None
+
+
+@attr.s(auto_attribs=True)
+class MockPublicRoomListManager:
+    """Mock of a synapse.module_api.PublicRoomListManager, only implementing the method
+    the DomainRuleChecker module will use.
+    """
+    _published: bool
+
+    async def room_is_in_public_room_list(self, room_id: str) -> bool:
+        return self._published
+
+
+@attr.s(auto_attribs=True)
+class MockModuleApi:
+    """Mock of a synapse.module_api.ModuleApi, only implementing the methods the
+    DomainRuleChecker module will use.
+    """
+    _new_room: bool
+    _published: bool
+
+    def register_spam_checker_callbacks(self, *args, **kwargs):
+        """Don't fail when the module tries to register its callbacks."""
+        pass
+
+    @property
+    def public_room_list_manager(self):
+        """Returns a mock public room list manager. We could in theory return a Mock with
+        a return value of make_awaitable(self._published), but local testing seems to show
+        this doesn't work on all versions of Python.
+        """
+        return MockPublicRoomListManager(self._published)
+
+    async def get_room_state(self, *args, **kwargs):
+        """Mocks the ModuleApi's get_room_state method, by returning mock events. The
+        number of events depends on whether we're testing for a new room or not (if the
+        room is not new it will have an extra user joined to it).
+        """
+        state = {
+            (EventTypes.Create, ""): MockEvent("room_creator"),
+            (EventTypes.Member, "room_creator"): MockEvent("room_creator", "join"),
+            (EventTypes.Member, "invitee"): MockEvent("room_creator", "invite"),
+        }
+
+        if not self._new_room:
+            state[(EventTypes.Member, "joinee")] = MockEvent("joinee", "join")
+
+        return state
+
+
+# We use a HomeserverTestCase despite not using the homeserver itself because we need a
+# reactor to run asynchronous code.
+class DomainRuleCheckerTestCase(unittest.HomeserverTestCase):
+    def _test_user_may_invite(
+        self, config, inviter, invitee, new_room, published,
+    ) -> bool:
+        check = DomainRuleChecker(config, MockModuleApi(new_room, published))
+        return self.get_success(check.user_may_invite(inviter, invitee, "room"))
+
     def test_allowed(self):
         config = {
             "default": False,
@@ -35,35 +103,37 @@ def test_allowed(self):
             },
             "domains_prevented_from_being_invited_to_published_rooms": ["target_two"],
         }
-        check = DomainRuleChecker(config)
+
         self.assertTrue(
-            check.user_may_invite(
-                "test:source_one", "test:target_one", None, "room", False
-            )
+            self._test_user_may_invite(
+                config, "test:source_one", "test:target_one", False, False,
+            ),
         )
+
         self.assertTrue(
-            check.user_may_invite(
-                "test:source_one", "test:target_two", None, "room", False
-            )
+            self._test_user_may_invite(
+                config, "test:source_one", "test:target_two", False, False,
+            ),
         )
+
         self.assertTrue(
-            check.user_may_invite(
-                "test:source_two", "test:target_two", None, "room", False
-            )
+            self._test_user_may_invite(
+                config, "test:source_two", "test:target_two", False, False,
+            ),
         )
 
         # User can invite internal user to a published room
         self.assertTrue(
-            check.user_may_invite(
-                "test:source_one", "test1:target_one", None, "room", False, True
-            )
+            self._test_user_may_invite(
+                config, "test:source_one", "test1:target_one", False, True,
+            ),
         )
 
         # User can invite external user to a non-published room
         self.assertTrue(
-            check.user_may_invite(
-                "test:source_one", "test:target_two", None, "room", False, False
-            )
+            self._test_user_may_invite(
+                config, "test:source_one", "test:target_two", False, False,
+            ),
         )
 
     def test_disallowed(self):
@@ -75,32 +145,32 @@ def test_disallowed(self):
                 "source_four": [],
             },
         }
-        check = DomainRuleChecker(config)
+
         self.assertFalse(
-            check.user_may_invite(
-                "test:source_one", "test:target_three", None, "room", False
+            self._test_user_may_invite(
+                config, "test:source_one", "test:target_three", False, False,
             )
         )
         self.assertFalse(
-            check.user_may_invite(
-                "test:source_two", "test:target_three", None, "room", False
+            self._test_user_may_invite(
+                config, "test:source_two", "test:target_three", False, False,
             )
         )
         self.assertFalse(
-            check.user_may_invite(
-                "test:source_two", "test:target_one", None, "room", False
+            self._test_user_may_invite(
+                config, "test:source_two", "test:target_one", False, False
             )
         )
         self.assertFalse(
-            check.user_may_invite(
-                "test:source_four", "test:target_one", None, "room", False
+            self._test_user_may_invite(
+                config, "test:source_four", "test:target_one", False, False
             )
         )
 
         # User cannot invite external user to a published room
         self.assertTrue(
-            check.user_may_invite(
-                "test:source_one", "test:target_two", None, "room", False, True
+            self._test_user_may_invite(
+                config, "test:source_one", "test:target_two", False, True
             )
         )
 
@@ -112,10 +182,10 @@ def test_default_allow(self):
                 "source_two": ["target_two"],
             },
         }
-        check = DomainRuleChecker(config)
+
         self.assertTrue(
-            check.user_may_invite(
-                "test:source_three", "test:target_one", None, "room", False
+            self._test_user_may_invite(
+                config, "test:source_three", "test:target_one", False, False,
             )
         )
 
@@ -127,10 +197,10 @@ def test_default_deny(self):
                 "source_two": ["target_two"],
             },
         }
-        check = DomainRuleChecker(config)
+
         self.assertFalse(
-            check.user_may_invite(
-                "test:source_three", "test:target_one", None, "room", False
+            self._test_user_may_invite(
+                config, "test:source_three", "test:target_one", False, False,
             )
         )
 
@@ -167,20 +237,26 @@ def make_homeserver(self, reactor, clock):
         config = self.default_config()
         config["trusted_third_party_id_servers"] = ["localhost"]
 
-        config["spam_checker"] = {
-            "module": "synapse.rulecheck.domain_rule_checker.DomainRuleChecker",
-            "config": {
-                "default": True,
-                "domain_mapping": {},
-                "can_only_join_rooms_with_invite": True,
-                "can_only_create_one_to_one_rooms": True,
-                "can_only_invite_during_room_creation": True,
-                "can_invite_by_third_party_id": False,
-            },
-        }
+        config["modules"] = [
+            {
+                "module": "synapse.rulecheck.domain_rule_checker.DomainRuleChecker",
+                "config": {
+                    "default": True,
+                    "domain_mapping": {},
+                    "can_only_join_rooms_with_invite": True,
+                    "can_only_create_one_to_one_rooms": True,
+                    "can_only_invite_during_room_creation": True,
+                    "can_invite_by_third_party_id": False,
+                },
+            }
+        ]
 
         hs = self.setup_test_homeserver(config=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)
+
         return hs
 
     def prepare(self, reactor, clock, hs):

From 726f13ee79f52dbe153300c406eba67a82cb5f5c Mon Sep 17 00:00:00 2001
From: Brendan Abolivier <babolivier@matrix.org>
Date: Wed, 3 Nov 2021 17:24:29 +0100
Subject: [PATCH 08/10] Delete custom user_may_create_room

Because we already do these checks with RoomAccessRules
---
 synapse/rulecheck/domain_rule_checker.py | 21 +-------------
 tests/rulecheck/test_domainrulecheck.py  | 37 ------------------------
 2 files changed, 1 insertion(+), 57 deletions(-)

diff --git a/synapse/rulecheck/domain_rule_checker.py b/synapse/rulecheck/domain_rule_checker.py
index 96424505e..da0d8aee3 100644
--- a/synapse/rulecheck/domain_rule_checker.py
+++ b/synapse/rulecheck/domain_rule_checker.py
@@ -14,7 +14,7 @@
 # limitations under the License.
 
 import logging
-from typing import Dict, Optional
+from typing import Optional
 
 from synapse.api.constants import EventTypes, Membership
 from synapse.config._base import ConfigError
@@ -84,7 +84,6 @@ def __init__(self, config, api: ModuleApi):
         self._api.register_spam_checker_callbacks(
             user_may_invite=self.user_may_invite,
             user_may_send_3pid_invite=self.user_may_send_3pid_invite,
-            user_may_create_room=self.user_may_create_room,
             user_may_join_room=self.user_may_join_room,
         )
 
@@ -198,24 +197,6 @@ async def _user_may_invite(
 
         return invitee_domain in self.domain_mapping[inviter_domain]
 
-    async def user_may_create_room(
-        self, userid, invite_list, third_party_invite_list, cloning
-    ):
-        """Implements synapse.events.SpamChecker.user_may_create_room"""
-
-        if cloning:
-            return True
-
-        if not self.can_invite_by_third_party_id and third_party_invite_list:
-            return False
-
-        number_of_invites = len(invite_list) + len(third_party_invite_list)
-
-        if self.can_only_create_one_to_one_rooms and number_of_invites != 1:
-            return False
-
-        return True
-
     async def user_may_join_room(self, userid, room_id, is_invited):
         """Implements the user_may_join_room spam checker callback."""
         if self.can_only_join_rooms_with_invite and not is_invited:
diff --git a/tests/rulecheck/test_domainrulecheck.py b/tests/rulecheck/test_domainrulecheck.py
index b66fffc4a..ba4be8135 100644
--- a/tests/rulecheck/test_domainrulecheck.py
+++ b/tests/rulecheck/test_domainrulecheck.py
@@ -272,43 +272,6 @@ def test_admin_can_create_room(self):
         channel = self._create_room(self.admin_access_token)
         assert channel.result["code"] == b"200", channel.result
 
-    def test_normal_user_cannot_create_empty_room(self):
-        channel = self._create_room(self.normal_access_token)
-        assert channel.result["code"] == b"403", channel.result
-
-    def test_normal_user_cannot_create_room_with_multiple_invites(self):
-        channel = self._create_room(
-            self.normal_access_token,
-            content={"invite": [self.other_user_id, self.admin_user_id]},
-        )
-        assert channel.result["code"] == b"403", channel.result
-
-        # Test that it correctly counts both normal and third party invites
-        channel = self._create_room(
-            self.normal_access_token,
-            content={
-                "invite": [self.other_user_id],
-                "invite_3pid": [{"medium": "email", "address": "foo@example.com"}],
-            },
-        )
-        assert channel.result["code"] == b"403", channel.result
-
-        # Test that it correctly rejects third party invites
-        channel = self._create_room(
-            self.normal_access_token,
-            content={
-                "invite": [],
-                "invite_3pid": [{"medium": "email", "address": "foo@example.com"}],
-            },
-        )
-        assert channel.result["code"] == b"403", channel.result
-
-    def test_normal_user_can_room_with_single_invites(self):
-        channel = self._create_room(
-            self.normal_access_token, content={"invite": [self.other_user_id]}
-        )
-        assert channel.result["code"] == b"200", channel.result
-
     def test_cannot_join_public_room(self):
         channel = self._create_room(self.admin_access_token)
         assert channel.result["code"] == b"200", channel.result

From 1e73b591262bb3d838f2c7916befda1646c1425f Mon Sep 17 00:00:00 2001
From: Brendan Abolivier <babolivier@matrix.org>
Date: Wed, 3 Nov 2021 17:37:28 +0100
Subject: [PATCH 09/10] Lint

---
 synapse/rulecheck/domain_rule_checker.py |  6 ++-
 tests/rulecheck/test_domainrulecheck.py  | 64 ++++++++++++++++++++----
 2 files changed, 58 insertions(+), 12 deletions(-)

diff --git a/synapse/rulecheck/domain_rule_checker.py b/synapse/rulecheck/domain_rule_checker.py
index da0d8aee3..645fa64a9 100644
--- a/synapse/rulecheck/domain_rule_checker.py
+++ b/synapse/rulecheck/domain_rule_checker.py
@@ -144,7 +144,9 @@ async def user_may_send_3pid_invite(
     ) -> bool:
         """Implements the user_may_send_3pid_invite spam checker callback."""
         return await self._user_may_invite(
-            room_id=room_id, inviter_userid=inviter_userid, invitee_userid=None,
+            room_id=room_id,
+            inviter_userid=inviter_userid,
+            invitee_userid=None,
         )
 
     async def _user_may_invite(
@@ -186,7 +188,7 @@ async def _user_may_invite(
             await self._api.public_room_list_manager.room_is_in_public_room_list(
                 room_id
             )
-    )
+        )
 
         if (
             published_room
diff --git a/tests/rulecheck/test_domainrulecheck.py b/tests/rulecheck/test_domainrulecheck.py
index ba4be8135..ab4023a72 100644
--- a/tests/rulecheck/test_domainrulecheck.py
+++ b/tests/rulecheck/test_domainrulecheck.py
@@ -33,6 +33,7 @@ class MockEvent:
     """Mock of an event, only implementing the fields the DomainRuleChecker module will
     use.
     """
+
     sender: str
     membership: Optional[str] = None
 
@@ -42,6 +43,7 @@ class MockPublicRoomListManager:
     """Mock of a synapse.module_api.PublicRoomListManager, only implementing the method
     the DomainRuleChecker module will use.
     """
+
     _published: bool
 
     async def room_is_in_public_room_list(self, room_id: str) -> bool:
@@ -53,6 +55,7 @@ class MockModuleApi:
     """Mock of a synapse.module_api.ModuleApi, only implementing the methods the
     DomainRuleChecker module will use.
     """
+
     _new_room: bool
     _published: bool
 
@@ -89,7 +92,12 @@ async def get_room_state(self, *args, **kwargs):
 # reactor to run asynchronous code.
 class DomainRuleCheckerTestCase(unittest.HomeserverTestCase):
     def _test_user_may_invite(
-        self, config, inviter, invitee, new_room, published,
+        self,
+        config,
+        inviter,
+        invitee,
+        new_room,
+        published,
     ) -> bool:
         check = DomainRuleChecker(config, MockModuleApi(new_room, published))
         return self.get_success(check.user_may_invite(inviter, invitee, "room"))
@@ -106,33 +114,53 @@ def test_allowed(self):
 
         self.assertTrue(
             self._test_user_may_invite(
-                config, "test:source_one", "test:target_one", False, False,
+                config,
+                "test:source_one",
+                "test:target_one",
+                False,
+                False,
             ),
         )
 
         self.assertTrue(
             self._test_user_may_invite(
-                config, "test:source_one", "test:target_two", False, False,
+                config,
+                "test:source_one",
+                "test:target_two",
+                False,
+                False,
             ),
         )
 
         self.assertTrue(
             self._test_user_may_invite(
-                config, "test:source_two", "test:target_two", False, False,
+                config,
+                "test:source_two",
+                "test:target_two",
+                False,
+                False,
             ),
         )
 
         # User can invite internal user to a published room
         self.assertTrue(
             self._test_user_may_invite(
-                config, "test:source_one", "test1:target_one", False, True,
+                config,
+                "test:source_one",
+                "test1:target_one",
+                False,
+                True,
             ),
         )
 
         # User can invite external user to a non-published room
         self.assertTrue(
             self._test_user_may_invite(
-                config, "test:source_one", "test:target_two", False, False,
+                config,
+                "test:source_one",
+                "test:target_two",
+                False,
+                False,
             ),
         )
 
@@ -148,12 +176,20 @@ def test_disallowed(self):
 
         self.assertFalse(
             self._test_user_may_invite(
-                config, "test:source_one", "test:target_three", False, False,
+                config,
+                "test:source_one",
+                "test:target_three",
+                False,
+                False,
             )
         )
         self.assertFalse(
             self._test_user_may_invite(
-                config, "test:source_two", "test:target_three", False, False,
+                config,
+                "test:source_two",
+                "test:target_three",
+                False,
+                False,
             )
         )
         self.assertFalse(
@@ -185,7 +221,11 @@ def test_default_allow(self):
 
         self.assertTrue(
             self._test_user_may_invite(
-                config, "test:source_three", "test:target_one", False, False,
+                config,
+                "test:source_three",
+                "test:target_one",
+                False,
+                False,
             )
         )
 
@@ -200,7 +240,11 @@ def test_default_deny(self):
 
         self.assertFalse(
             self._test_user_may_invite(
-                config, "test:source_three", "test:target_one", False, False,
+                config,
+                "test:source_three",
+                "test:target_one",
+                False,
+                False,
             )
         )
 

From ed31c6b1f8e035b520812be9ad101ae53d7525c6 Mon Sep 17 00:00:00 2001
From: Brendan Abolivier <babolivier@matrix.org>
Date: Wed, 3 Nov 2021 18:04:20 +0100
Subject: [PATCH 10/10] Remove now useless config option

---
 synapse/rulecheck/domain_rule_checker.py | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/synapse/rulecheck/domain_rule_checker.py b/synapse/rulecheck/domain_rule_checker.py
index 645fa64a9..b4d659b06 100644
--- a/synapse/rulecheck/domain_rule_checker.py
+++ b/synapse/rulecheck/domain_rule_checker.py
@@ -66,9 +66,6 @@ def __init__(self, config, api: ModuleApi):
         self.can_only_join_rooms_with_invite = config.get(
             "can_only_join_rooms_with_invite", False
         )
-        self.can_only_create_one_to_one_rooms = config.get(
-            "can_only_create_one_to_one_rooms", False
-        )
         self.can_only_invite_during_room_creation = config.get(
             "can_only_invite_during_room_creation", False
         )