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

Do not apply ratelimiting on joins to appservices #8139

Merged
merged 10 commits into from
Aug 21, 2020
Merged
Show file tree
Hide file tree
Changes from 7 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/8139.bugfix
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Appservices which are exempt from ratelimiting are no longer ratelimited when joining rooms.
Half-Shot marked this conversation as resolved.
Show resolved Hide resolved
18 changes: 18 additions & 0 deletions synapse/api/ratelimiting.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
from typing import Any, Optional, Tuple

from synapse.api.errors import LimitExceededError
from synapse.types import Requester
from synapse.util import Clock


Expand All @@ -43,6 +44,23 @@ def __init__(self, clock: Clock, rate_hz: float, burst_count: int):
# * The rate_hz of this particular entry. This can vary per request
self.actions = OrderedDict() # type: OrderedDict[Any, Tuple[float, int, float]]

def can_requester_do_action(
Half-Shot marked this conversation as resolved.
Show resolved Hide resolved
self,
requester: Requester,
rate_hz: Optional[float] = None,
burst_count: Optional[int] = None,
update: bool = True,
_time_now_s: Optional[int] = None,
) -> Tuple[bool, float]:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, can you also add a docstring here, which mostly just refers to can_do_action

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

# Disable rate limiting of users belonging to any AS that is configured
# not to be rate limited in its registration file (rate_limited: true|false).
if requester.app_service and not requester.app_service.is_rate_limited():
return True, -1.0

return self.can_do_action(
requester.user.to_string(), rate_hz, burst_count, update, _time_now_s
)

def can_do_action(
self,
key: Any,
Expand Down
14 changes: 8 additions & 6 deletions synapse/handlers/room_member.py
Original file line number Diff line number Diff line change
Expand Up @@ -459,9 +459,10 @@ async def _update_membership(

if is_host_in_room:
time_now_s = self.clock.time()
allowed, time_allowed = self._join_rate_limiter_local.can_do_action(
requester.user.to_string(),
)
(
allowed,
time_allowed,
) = self._join_rate_limiter_local.can_requester_do_action(requester,)

if not allowed:
raise LimitExceededError(
Expand All @@ -470,9 +471,10 @@ async def _update_membership(

else:
time_now_s = self.clock.time()
allowed, time_allowed = self._join_rate_limiter_remote.can_do_action(
requester.user.to_string(),
)
(
allowed,
time_allowed,
) = self._join_rate_limiter_remote.can_requester_do_action(requester,)

if not allowed:
raise LimitExceededError(
Expand Down
73 changes: 73 additions & 0 deletions tests/api/test_ratelimiting.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
from synapse.api.ratelimiting import LimitExceededError, Ratelimiter
from synapse.appservice import ApplicationService
from synapse.types import create_requester

from tests import unittest

Expand All @@ -18,6 +20,77 @@ def test_allowed_via_can_do_action(self):
self.assertTrue(allowed)
self.assertEquals(20.0, time_allowed)

def test_allowed_user_via_can_requester_do_action(self):
user_requester = create_requester("@user:example.com")
limiter = Ratelimiter(clock=None, rate_hz=0.1, burst_count=1)
allowed, time_allowed = limiter.can_requester_do_action(
user_requester, _time_now_s=0
)
self.assertTrue(allowed)
self.assertEquals(10.0, time_allowed)

allowed, time_allowed = limiter.can_requester_do_action(
user_requester, _time_now_s=5
)
self.assertFalse(allowed)
self.assertEquals(10.0, time_allowed)

allowed, time_allowed = limiter.can_requester_do_action(
user_requester, _time_now_s=10
)
self.assertTrue(allowed)
self.assertEquals(20.0, time_allowed)

def test_allowed_appservice_ratelimited_via_can_requester_do_action(self):
appservice = ApplicationService(
None, "example.com", id="foo", rate_limited=True,
)
as_requester = create_requester("@user:example.com", app_service=appservice)

limiter = Ratelimiter(clock=None, rate_hz=0.1, burst_count=1)
allowed, time_allowed = limiter.can_requester_do_action(
as_requester, _time_now_s=0
)
self.assertTrue(allowed)
self.assertEquals(10.0, time_allowed)

allowed, time_allowed = limiter.can_requester_do_action(
as_requester, _time_now_s=5
)
self.assertFalse(allowed)
self.assertEquals(10.0, time_allowed)

allowed, time_allowed = limiter.can_requester_do_action(
as_requester, _time_now_s=10
)
self.assertTrue(allowed)
self.assertEquals(20.0, time_allowed)

def test_allowed_appservice_via_can_requester_do_action(self):
appservice = ApplicationService(
None, "example.com", id="foo", rate_limited=False,
)
as_requester = create_requester("@user:example.com", app_service=appservice)

limiter = Ratelimiter(clock=None, rate_hz=0.1, burst_count=1)
allowed, time_allowed = limiter.can_requester_do_action(
as_requester, _time_now_s=0
)
self.assertTrue(allowed)
self.assertEquals(-1, time_allowed)

allowed, time_allowed = limiter.can_requester_do_action(
as_requester, _time_now_s=5
)
self.assertTrue(allowed)
self.assertEquals(-1, time_allowed)

allowed, time_allowed = limiter.can_requester_do_action(
as_requester, _time_now_s=10
)
self.assertTrue(allowed)
self.assertEquals(-1, time_allowed)

def test_allowed_via_ratelimit(self):
limiter = Ratelimiter(clock=None, rate_hz=0.1, burst_count=1)

Expand Down