Skip to content
This repository has been archived by the owner on Apr 26, 2024. It is now read-only.

Honour ratelimit flag for application services for invite ratelimiting #9302

Merged
merged 2 commits into from
Feb 3, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions changelog.d/9302.bugfix
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Fix new ratelimiting for invites to respect the `ratelimit` flag on application services. Introduced in v1.27.0rc1.
4 changes: 3 additions & 1 deletion synapse/handlers/federation.py
Original file line number Diff line number Diff line change
Expand Up @@ -1619,7 +1619,9 @@ async def on_invite_request(

# We retrieve the room member handler here as to not cause a cyclic dependency
member_handler = self.hs.get_room_member_handler()
member_handler.ratelimit_invite(event.room_id, event.state_key)
# We don't rate limit based on room ID, as that should be done by
# sending server.
member_handler.ratelimit_invite(None, event.state_key)

# keep a record of the room version, if we don't yet know it.
# (this may get overwritten if we later get a different room version in a
Expand Down
12 changes: 9 additions & 3 deletions synapse/handlers/room_member.py
Original file line number Diff line number Diff line change
Expand Up @@ -155,10 +155,14 @@ async def _user_left_room(self, target: UserID, room_id: str) -> None:
"""
raise NotImplementedError()

def ratelimit_invite(self, room_id: str, invitee_user_id: str):
def ratelimit_invite(self, room_id: Optional[str], invitee_user_id: str):
"""Ratelimit invites by room and by target user.

If room ID is missing then we just rate limit by target user.
"""
self._invites_per_room_limiter.ratelimit(room_id)
if room_id:
self._invites_per_room_limiter.ratelimit(room_id)

self._invites_per_user_limiter.ratelimit(invitee_user_id)

async def _local_membership_update(
Expand Down Expand Up @@ -406,7 +410,9 @@ async def update_membership_locked(
if effective_membership_state == Membership.INVITE:
target_id = target.to_string()
if ratelimit:
self.ratelimit_invite(room_id, target_id)
# Don't ratelimit application services.
if not requester.app_service or requester.app_service.is_rate_limited():
self.ratelimit_invite(room_id, target_id)

# block any attempts to invite the server notices mxid
if target_id == self._server_notices_mxid:
Expand Down
47 changes: 0 additions & 47 deletions tests/handlers/test_federation.py
Original file line number Diff line number Diff line change
Expand Up @@ -191,53 +191,6 @@ def test_rejected_state_event_state(self):

self.assertEqual(sg, sg2)

@unittest.override_config(
{"rc_invites": {"per_room": {"per_second": 0.5, "burst_count": 3}}}
)
def test_invite_by_room_ratelimit(self):
"""Tests that invites from federation in a room are actually rate-limited.
"""
other_server = "otherserver"
other_user = "@otheruser:" + other_server

# create the room
user_id = self.register_user("kermit", "test")
tok = self.login("kermit", "test")
room_id = self.helper.create_room_as(room_creator=user_id, tok=tok)
room_version = self.get_success(self.store.get_room_version(room_id))

def create_invite_for(local_user):
return event_from_pdu_json(
{
"type": EventTypes.Member,
"content": {"membership": "invite"},
"room_id": room_id,
"sender": other_user,
"state_key": local_user,
"depth": 32,
"prev_events": [],
"auth_events": [],
"origin_server_ts": self.clock.time_msec(),
},
room_version,
)

for i in range(3):
self.get_success(
self.handler.on_invite_request(
other_server,
create_invite_for("@user-%d:test" % (i,)),
room_version,
)
)

self.get_failure(
self.handler.on_invite_request(
other_server, create_invite_for("@user-4:test"), room_version,
),
exc=LimitExceededError,
)

@unittest.override_config(
{"rc_invites": {"per_user": {"per_second": 0.5, "burst_count": 3}}}
)
Expand Down