mirror of
https://mau.dev/maunium/synapse.git
synced 2024-10-01 01:36:05 -04:00
Do not apply ratelimiting on joins to appservices (#8139)
Add new method ratelimiter.can_requester_do_action and ensure that appservices are exempt from being ratelimited. Co-authored-by: Patrick Cloke <clokep@users.noreply.github.com> Co-authored-by: Erik Johnston <erik@matrix.org>
This commit is contained in:
parent
3234d5c305
commit
2df82ae451
1
changelog.d/8139.bugfix
Normal file
1
changelog.d/8139.bugfix
Normal file
@ -0,0 +1 @@
|
||||
Fixes a bug where appservices with ratelimiting disabled would still be ratelimited when joining rooms. This bug was introduced in v1.19.0.
|
@ -17,6 +17,7 @@ from collections import OrderedDict
|
||||
from typing import Any, Optional, Tuple
|
||||
|
||||
from synapse.api.errors import LimitExceededError
|
||||
from synapse.types import Requester
|
||||
from synapse.util import Clock
|
||||
|
||||
|
||||
@ -43,6 +44,42 @@ class Ratelimiter(object):
|
||||
# * 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(
|
||||
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]:
|
||||
"""Can the requester perform the action?
|
||||
|
||||
Args:
|
||||
requester: The requester to key off when rate limiting. The user property
|
||||
will be used.
|
||||
rate_hz: The long term number of actions that can be performed in a second.
|
||||
Overrides the value set during instantiation if set.
|
||||
burst_count: How many actions that can be performed before being limited.
|
||||
Overrides the value set during instantiation if set.
|
||||
update: Whether to count this check as performing the action
|
||||
_time_now_s: The current time. Optional, defaults to the current time according
|
||||
to self.clock. Only used by tests.
|
||||
|
||||
Returns:
|
||||
A tuple containing:
|
||||
* A bool indicating if they can perform the action now
|
||||
* The reactor timestamp for when the action can be performed next.
|
||||
-1 if rate_hz is less than or equal to zero
|
||||
"""
|
||||
# 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,
|
||||
|
@ -459,9 +459,10 @@ class RoomMemberHandler(object):
|
||||
|
||||
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(
|
||||
@ -470,9 +471,10 @@ class RoomMemberHandler(object):
|
||||
|
||||
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(
|
||||
|
@ -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
|
||||
|
||||
@ -18,6 +20,77 @@ class TestRatelimiter(unittest.TestCase):
|
||||
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)
|
||||
|
||||
|
Loading…
Reference in New Issue
Block a user