Pass room_config argument to user_may_create_room spam checker module callback (#18486)

hughns · web-flow · commit fbe7a898f038 · 2025-06-04T11:30:45.000+01:00
This PR adds an additional `room_config` argument to the
`user_may_create_room` spam checker module API callback.

It will continue to work with implementations of `user_may_create_room`
that do not expect the additional parameter.

A side affect is that on a room upgrade the spam checker callback is
called *after* doing some work to calculate the state rather than
before. However, I hope that this is acceptable given the relative
infrequency of room upgrades.
diff --git a/changelog.d/18486.feature b/changelog.d/18486.feature
@@ -0,0 +1 @@
+Pass room_config argument to user_may_create_room spam checker module callback.
diff --git a/docs/modules/spam_checker_callbacks.md b/docs/modules/spam_checker_callbacks.md
@@ -159,12 +159,19 @@ _First introduced in Synapse v1.37.0_
 
 _Changed in Synapse v1.62.0: `synapse.module_api.NOT_SPAM` and `synapse.module_api.errors.Codes` can be returned by this callback. Returning a boolean is now deprecated._ 
 
+_Changed in Synapse v1.132.0: Added the `room_config` argument. Callbacks that only expect a single `user_id` argument are still supported._
+
 ```python
-async def user_may_create_room(user_id: str) -> Union["synapse.module_api.NOT_SPAM", "synapse.module_api.errors.Codes", bool]
+async def user_may_create_room(user_id: str, room_config: synapse.module_api.JsonDict) -> Union["synapse.module_api.NOT_SPAM", "synapse.module_api.errors.Codes", bool]
 ```
 
 Called when processing a room creation request.
 
+The arguments passed to this callback are:
+
+* `user_id`: The Matrix user ID of the user (e.g. `@alice:example.com`).
+* `room_config`: The contents of the body of a [/createRoom request](https://spec.matrix.org/latest/client-server-api/#post_matrixclientv3createroom) as a dictionary.
+
 The callback must return one of:
   - `synapse.module_api.NOT_SPAM`, to allow the operation. Other callbacks may still 
     decide to reject it.
diff --git a/docs/spam_checker.md b/docs/spam_checker.md
@@ -63,7 +63,7 @@ class ExampleSpamChecker:
     async def user_may_invite(self, inviter_userid, invitee_userid, room_id):
         return True  # allow all invites
 
-    async def user_may_create_room(self, userid):
+    async def user_may_create_room(self, userid, room_config):
         return True  # allow all room creations
 
     async def user_may_create_room_alias(self, userid, room_alias):
diff --git a/synapse/handlers/room.py b/synapse/handlers/room.py
@@ -468,17 +468,6 @@ async def clone_existing_room(
         """
         user_id = requester.user.to_string()
 
-        spam_check = await self._spam_checker_module_callbacks.user_may_create_room(
-            user_id
-        )
-        if spam_check != self._spam_checker_module_callbacks.NOT_SPAM:
-            raise SynapseError(
-                403,
-                "You are not permitted to create rooms",
-                errcode=spam_check[0],
-                additional_fields=spam_check[1],
-            )
-
         creation_content: JsonDict = {
             "room_version": new_room_version.identifier,
             "predecessor": {"room_id": old_room_id, "event_id": tombstone_event_id},
@@ -585,6 +574,24 @@ async def clone_existing_room(
         if current_power_level_int < needed_power_level:
             user_power_levels[user_id] = needed_power_level
 
+        # We construct what the body of a call to /createRoom would look like for passing
+        # to the spam checker. We don't include a preset here, as we expect the
+        # initial state to contain everything we need.
+        spam_check = await self._spam_checker_module_callbacks.user_may_create_room(
+            user_id,
+            {
+                "creation_content": creation_content,
+                "initial_state": list(initial_state.items()),
+            },
+        )
+        if spam_check != self._spam_checker_module_callbacks.NOT_SPAM:
+            raise SynapseError(
+                403,
+                "You are not permitted to create rooms",
+                errcode=spam_check[0],
+                additional_fields=spam_check[1],
+            )
+
         await self._send_events_for_new_room(
             requester,
             new_room_id,
@@ -786,7 +793,7 @@ async def create_room(
 
         if not is_requester_admin:
             spam_check = await self._spam_checker_module_callbacks.user_may_create_room(
-                user_id
+                user_id, config
             )
             if spam_check != self._spam_checker_module_callbacks.NOT_SPAM:
                 raise SynapseError(
diff --git a/synapse/module_api/callbacks/spamchecker_callbacks.py b/synapse/module_api/callbacks/spamchecker_callbacks.py
@@ -22,6 +22,7 @@
 import functools
 import inspect
 import logging
+from copy import deepcopy
 from typing import (
     TYPE_CHECKING,
     Any,
@@ -120,20 +121,24 @@
         ]
     ],
 ]
-USER_MAY_CREATE_ROOM_CALLBACK = Callable[
-    [str],
-    Awaitable[
-        Union[
-            Literal["NOT_SPAM"],
-            Codes,
-            # Highly experimental, not officially part of the spamchecker API, may
-            # disappear without warning depending on the results of ongoing
-            # experiments.
-            # Use this to return additional information as part of an error.
-            Tuple[Codes, JsonDict],
-            # Deprecated
-            bool,
-        ]
+USER_MAY_CREATE_ROOM_CALLBACK_RETURN_VALUE = Union[
+    Literal["NOT_SPAM"],
+    Codes,
+    # Highly experimental, not officially part of the spamchecker API, may
+    # disappear without warning depending on the results of ongoing
+    # experiments.
+    # Use this to return additional information as part of an error.
+    Tuple[Codes, JsonDict],
+    # Deprecated
+    bool,
+]
+USER_MAY_CREATE_ROOM_CALLBACK = Union[
+    Callable[
+        [str, JsonDict],
+        Awaitable[USER_MAY_CREATE_ROOM_CALLBACK_RETURN_VALUE],
+    ],
+    Callable[  # Single argument variant for backwards compatibility
+        [str], Awaitable[USER_MAY_CREATE_ROOM_CALLBACK_RETURN_VALUE]
     ],
 ]
 USER_MAY_CREATE_ROOM_ALIAS_CALLBACK = Callable[
@@ -622,16 +627,41 @@ async def user_may_send_3pid_invite(
         return self.NOT_SPAM
 
     async def user_may_create_room(
-        self, userid: str
+        self, userid: str, room_config: JsonDict
     ) -> Union[Tuple[Codes, dict], Literal["NOT_SPAM"]]:
         """Checks if a given user may create a room
 
         Args:
             userid: The ID of the user attempting to create a room
+            room_config: The room creation configuration which is the body of the /createRoom request
         """
         for callback in self._user_may_create_room_callbacks:
             with Measure(self.clock, f"{callback.__module__}.{callback.__qualname__}"):
-                res = await delay_cancellation(callback(userid))
+                checker_args = inspect.signature(callback)
+                # Also ensure backwards compatibility with spam checker callbacks
+                # that don't expect the room_config argument.
+                if len(checker_args.parameters) == 2:
+                    callback_with_requester_id = cast(
+                        Callable[
+                            [str, JsonDict],
+                            Awaitable[USER_MAY_CREATE_ROOM_CALLBACK_RETURN_VALUE],
+                        ],
+                        callback,
+                    )
+                    # We make a copy of the config to ensure the spam checker cannot modify it.
+                    res = await delay_cancellation(
+                        callback_with_requester_id(userid, deepcopy(room_config))
+                    )
+                else:
+                    callback_without_requester_id = cast(
+                        Callable[
+                            [str], Awaitable[USER_MAY_CREATE_ROOM_CALLBACK_RETURN_VALUE]
+                        ],
+                        callback,
+                    )
+                    res = await delay_cancellation(
+                        callback_without_requester_id(userid)
+                    )
                 if res is True or res is self.NOT_SPAM:
                     continue
                 elif res is False:
diff --git a/tests/module_api/test_spamchecker.py b/tests/module_api/test_spamchecker.py
@@ -0,0 +1,155 @@
+#
+# This file is licensed under the Affero General Public License (AGPL) version 3.
+#
+# Copyright (C) 2025 New Vector, Ltd
+#
+# This program is free software: you can redistribute it and/or modify
+# it under the terms of the GNU Affero General Public License as
+# published by the Free Software Foundation, either version 3 of the
+# License, or (at your option) any later version.
+#
+# See the GNU Affero General Public License for more details:
+# <https://www.gnu.org/licenses/agpl-3.0.html>.
+#
+#
+from typing import Literal, Union
+
+from twisted.test.proto_helpers import MemoryReactor
+
+from synapse.config.server import DEFAULT_ROOM_VERSION
+from synapse.rest import admin, login, room, room_upgrade_rest_servlet
+from synapse.server import HomeServer
+from synapse.types import Codes, JsonDict
+from synapse.util import Clock
+
+from tests.server import FakeChannel
+from tests.unittest import HomeserverTestCase
+
+
+class SpamCheckerTestCase(HomeserverTestCase):
+    servlets = [
+        room.register_servlets,
+        admin.register_servlets,
+        login.register_servlets,
+        room_upgrade_rest_servlet.register_servlets,
+    ]
+
+    def prepare(
+        self, reactor: MemoryReactor, clock: Clock, homeserver: HomeServer
+    ) -> None:
+        self._module_api = homeserver.get_module_api()
+        self.user_id = self.register_user("user", "password")
+        self.token = self.login("user", "password")
+
+    def create_room(self, content: JsonDict) -> FakeChannel:
+        channel = self.make_request(
+            "POST",
+            "/_matrix/client/r0/createRoom",
+            content,
+            access_token=self.token,
+        )
+
+        return channel
+
+    def test_may_user_create_room(self) -> None:
+        """Test that the may_user_create_room callback is called when a user
+        creates a room, and that it receives the correct parameters.
+        """
+
+        async def user_may_create_room(
+            user_id: str, room_config: JsonDict
+        ) -> Union[Literal["NOT_SPAM"], Codes]:
+            self.last_room_config = room_config
+            self.last_user_id = user_id
+            return "NOT_SPAM"
+
+        self._module_api.register_spam_checker_callbacks(
+            user_may_create_room=user_may_create_room
+        )
+
+        channel = self.create_room({"foo": "baa"})
+        self.assertEqual(channel.code, 200)
+        self.assertEqual(self.last_user_id, self.user_id)
+        self.assertEqual(self.last_room_config["foo"], "baa")
+
+    def test_may_user_create_room_on_upgrade(self) -> None:
+        """Test that the may_user_create_room callback is called when a room is upgraded."""
+
+        # First, create a room to upgrade.
+        channel = self.create_room({"topic": "foo"})
+        self.assertEqual(channel.code, 200)
+        room_id = channel.json_body["room_id"]
+
+        async def user_may_create_room(
+            user_id: str, room_config: JsonDict
+        ) -> Union[Literal["NOT_SPAM"], Codes]:
+            self.last_room_config = room_config
+            self.last_user_id = user_id
+            return "NOT_SPAM"
+
+        # Register the callback for spam checking.
+        self._module_api.register_spam_checker_callbacks(
+            user_may_create_room=user_may_create_room
+        )
+
+        # Now upgrade the room.
+        channel = self.make_request(
+            "POST",
+            f"/_matrix/client/r0/rooms/{room_id}/upgrade",
+            # This will upgrade a room to the same version, but that's fine.
+            content={"new_version": DEFAULT_ROOM_VERSION},
+            access_token=self.token,
+        )
+
+        # Check that the callback was called and the room was upgraded.
+        self.assertEqual(channel.code, 200)
+        self.assertEqual(self.last_user_id, self.user_id)
+        # Check that the initial state received by callback contains the topic event.
+        self.assertTrue(
+            any(
+                event[0][0] == "m.room.topic" and event[1].get("topic") == "foo"
+                for event in self.last_room_config["initial_state"]
+            )
+        )
+
+    def test_may_user_create_room_disallowed(self) -> None:
+        """Test that the codes response from may_user_create_room callback is respected
+        and returned via the API.
+        """
+
+        async def user_may_create_room(
+            user_id: str, room_config: JsonDict
+        ) -> Union[Literal["NOT_SPAM"], Codes]:
+            self.last_room_config = room_config
+            self.last_user_id = user_id
+            return Codes.UNAUTHORIZED
+
+        self._module_api.register_spam_checker_callbacks(
+            user_may_create_room=user_may_create_room
+        )
+
+        channel = self.create_room({"foo": "baa"})
+        self.assertEqual(channel.code, 403)
+        self.assertEqual(channel.json_body["errcode"], Codes.UNAUTHORIZED)
+        self.assertEqual(self.last_user_id, self.user_id)
+        self.assertEqual(self.last_room_config["foo"], "baa")
+
+    def test_may_user_create_room_compatibility(self) -> None:
+        """Test that the may_user_create_room callback is called when a user
+        creates a room for a module that uses the old callback signature
+        (without the `room_config` parameter)
+        """
+
+        async def user_may_create_room(
+            user_id: str,
+        ) -> Union[Literal["NOT_SPAM"], Codes]:
+            self.last_user_id = user_id
+            return "NOT_SPAM"
+
+        self._module_api.register_spam_checker_callbacks(
+            user_may_create_room=user_may_create_room
+        )
+
+        channel = self.create_room({"foo": "baa"})
+        self.assertEqual(channel.code, 200)
+        self.assertEqual(self.last_user_id, self.user_id)

Original file line number	Diff line number	Diff line change
`@@ -0,0 +1 @@`
	`1`	`+Pass room_config argument to user_may_create_room spam checker module callback.`