diff --git a/changelog.d/14314.bugfix b/changelog.d/14314.bugfix
new file mode 100644
index 0000000000..8be47ee083
--- /dev/null
+++ b/changelog.d/14314.bugfix
@@ -0,0 +1 @@
+Fix room creation being rate limited too aggressively since Synapse v1.69.0.
\ No newline at end of file
diff --git a/synapse/api/ratelimiting.py b/synapse/api/ratelimiting.py
index 044c7d4926..511790c7c5 100644
--- a/synapse/api/ratelimiting.py
+++ b/synapse/api/ratelimiting.py
@@ -343,6 +343,7 @@ class RequestRatelimiter:
requester: Requester,
update: bool = True,
is_admin_redaction: bool = False,
+ n_actions: int = 1,
) -> None:
"""Ratelimits requests.
@@ -355,6 +356,8 @@ class RequestRatelimiter:
is_admin_redaction: Whether this is a room admin/moderator
redacting an event. If so then we may apply different
ratelimits depending on config.
+ n_actions: Multiplier for the number of actions to apply to the
+ rate limiter at once.
Raises:
LimitExceededError if the request should be ratelimited
@@ -383,7 +386,9 @@ class RequestRatelimiter:
if is_admin_redaction and self.admin_redaction_ratelimiter:
# If we have separate config for admin redactions, use a separate
# ratelimiter as to not have user_ids clash
- await self.admin_redaction_ratelimiter.ratelimit(requester, update=update)
+ await self.admin_redaction_ratelimiter.ratelimit(
+ requester, update=update, n_actions=n_actions
+ )
else:
# Override rate and burst count per-user
await self.request_ratelimiter.ratelimit(
@@ -391,4 +396,5 @@ class RequestRatelimiter:
rate_hz=messages_per_second,
burst_count=burst_count,
update=update,
+ n_actions=n_actions,
)
diff --git a/synapse/handlers/room.py b/synapse/handlers/room.py
index 638f54051a..d74b675adc 100644
--- a/synapse/handlers/room.py
+++ b/synapse/handlers/room.py
@@ -559,7 +559,6 @@ class RoomCreationHandler:
invite_list=[],
initial_state=initial_state,
creation_content=creation_content,
- ratelimit=False,
)
# Transfer membership events
@@ -753,6 +752,10 @@ class RoomCreationHandler:
)
if ratelimit:
+ # Rate limit once in advance, but don't rate limit the individual
+ # events in the room — room creation isn't atomic and it's very
+ # janky if half the events in the initial state don't make it because
+ # of rate limiting.
await self.request_ratelimiter.ratelimit(requester)
room_version_id = config.get(
@@ -913,7 +916,6 @@ class RoomCreationHandler:
room_alias=room_alias,
power_level_content_override=power_level_content_override,
creator_join_profile=creator_join_profile,
- ratelimit=ratelimit,
)
if "name" in config:
@@ -1037,7 +1039,6 @@ class RoomCreationHandler:
room_alias: Optional[RoomAlias] = None,
power_level_content_override: Optional[JsonDict] = None,
creator_join_profile: Optional[JsonDict] = None,
- ratelimit: bool = True,
) -> Tuple[int, str, int]:
"""Sends the initial events into a new room. Sends the room creation, membership,
and power level events into the room sequentially, then creates and batches up the
@@ -1046,6 +1047,8 @@ class RoomCreationHandler:
`power_level_content_override` doesn't apply when initial state has
power level state event content.
+ Rate limiting should already have been applied by this point.
+
Returns:
A tuple containing the stream ID, event ID and depth of the last
event sent to the room.
@@ -1144,7 +1147,7 @@ class RoomCreationHandler:
creator.user,
room_id,
"join",
- ratelimit=ratelimit,
+ ratelimit=False,
content=creator_join_profile,
new_room=True,
prev_event_ids=[last_sent_event_id],
@@ -1269,7 +1272,10 @@ class RoomCreationHandler:
events_to_send.append((encryption_event, encryption_context))
last_event = await self.event_creation_handler.handle_new_client_event(
- creator, events_to_send, ignore_shadow_ban=True
+ creator,
+ events_to_send,
+ ignore_shadow_ban=True,
+ ratelimit=False,
)
assert last_event.internal_metadata.stream_ordering is not None
return last_event.internal_metadata.stream_ordering, last_event.event_id, depth
diff --git a/tests/rest/client/test_rooms.py b/tests/rest/client/test_rooms.py
index 716366eb90..1084d4ad9d 100644
--- a/tests/rest/client/test_rooms.py
+++ b/tests/rest/client/test_rooms.py
@@ -54,6 +54,7 @@ from tests.http.server._base import make_request_with_cancellation_test
from tests.storage.test_stream import PaginationTestCase
from tests.test_utils import make_awaitable
from tests.test_utils.event_injection import create_event
+from tests.unittest import override_config
PATH_PREFIX = b"/_matrix/client/api/v1"
@@ -871,6 +872,41 @@ class RoomsCreateTestCase(RoomBase):
self.assertEqual(channel.code, HTTPStatus.OK, channel.json_body)
self.assertEqual(join_mock.call_count, 0)
+ def _create_basic_room(self) -> Tuple[int, object]:
+ """
+ Tries to create a basic room and returns the response code.
+ """
+ channel = self.make_request(
+ "POST",
+ "/createRoom",
+ {},
+ )
+ return channel.code, channel.json_body
+
+ @override_config(
+ {
+ "rc_message": {"per_second": 0.2, "burst_count": 10},
+ }
+ )
+ def test_room_creation_ratelimiting(self) -> None:
+ """
+ Regression test for #14312, where ratelimiting was made too strict.
+ Clients should be able to create 10 rooms in a row
+ without hitting rate limits, using default rate limit config.
+ (We override rate limiting config back to its default value.)
+
+ To ensure we don't make ratelimiting too generous accidentally,
+ also check that we can't create an 11th room.
+ """
+
+ for _ in range(10):
+ code, json_body = self._create_basic_room()
+ self.assertEqual(code, HTTPStatus.OK, json_body)
+
+ # The 6th room hits the rate limit.
+ code, json_body = self._create_basic_room()
+ self.assertEqual(code, HTTPStatus.TOO_MANY_REQUESTS, json_body)
+
class RoomTopicTestCase(RoomBase):
"""Tests /rooms/$room_id/topic REST events."""
@@ -1390,10 +1426,22 @@ class RoomJoinRatelimitTestCase(RoomBase):
)
def test_join_local_ratelimit(self) -> None:
"""Tests that local joins are actually rate-limited."""
- for _ in range(3):
- self.helper.create_room_as(self.user_id)
+ # Create 4 rooms
+ room_ids = [
+ self.helper.create_room_as(self.user_id, is_public=True) for _ in range(4)
+ ]
+
+ joiner_user_id = self.register_user("joiner", "secret")
+ # Now make a new user try to join some of them.
- self.helper.create_room_as(self.user_id, expect_code=429)
+ # The user can join 3 rooms
+ for room_id in room_ids[0:3]:
+ self.helper.join(room_id, joiner_user_id)
+
+ # But the user cannot join a 4th room
+ self.helper.join(
+ room_ids[3], joiner_user_id, expect_code=HTTPStatus.TOO_MANY_REQUESTS
+ )
@unittest.override_config(
{"rc_joins": {"local": {"per_second": 0.5, "burst_count": 3}}}
|
