paulmataruso/zulip
1
0
Fork 0
mirror of https://github.com/zulip/zulip.git synced 2025-11-02 04:53:36 +00:00
Code Issues Packages Projects Releases Wiki Activity

rate_limit: Implement IP-based rate limiting.

Browse Source 
If the user is logged in, we'll stick to rate limiting by the
UserProfile. In case of requests without authentication, we'll apply the
same limits but to the IP address.
This commit is contained in:
Mateusz Mandera
2021-07-08 14:46:47 +02:00
committed by Tim Abbott
parent df96e02732
commit b9056d193d
7 changed files with 109 additions and 22 deletions
Download Patch File Download Diff File Expand all files Collapse all files

22
zerver/decorator.py
View File

@@ -38,7 +38,7 @@ from zerver.lib.exceptions import (
UserDeactivatedError,
)
from zerver.lib.queue import queue_json_publish
from zerver.lib.rate_limiter import RateLimitedUser
from zerver.lib.rate_limiter import RateLimitedIPAddr, RateLimitedUser
from zerver.lib.request import REQ, has_request_variables
from zerver.lib.response import json_method_not_allowed, json_success, json_unauthorized
from zerver.lib.subdomains import get_subdomain, user_matches_subdomain
@@ -847,6 +847,10 @@ def rate_limit_user(request: HttpRequest, user: UserProfile, domain: str) -> Non
RateLimitedUser(user, domain=domain).rate_limit_request(request)
def rate_limit_ip(request: HttpRequest, ip_addr: str, domain: str) -> None:
RateLimitedIPAddr(ip_addr, domain=domain).rate_limit_request(request)
def rate_limit() -> Callable[[ViewFuncT], ViewFuncT]:
"""Rate-limits a view. Returns a decorator"""
@@ -868,14 +872,16 @@ def rate_limit() -> Callable[[ViewFuncT], ViewFuncT]:
if isinstance(user, AnonymousUser) or (
settings.ZILENCER_ENABLED and isinstance(user, RemoteZulipServer)
):
# We can only rate-limit logged-in users for now.
# We also only support rate-limiting authenticated
# views right now.
# TODO: implement per-IP non-authed rate limiting
# REMOTE_ADDR is set by SetRemoteAddrFromRealIpHeader in conjunction
# with the nginx configuration to guarantee this to be *the* correct
# IP address to use - without worrying we'll grab the IP of a proxy.
ip_addr = request.META["REMOTE_ADDR"]
assert ip_addr
rate_limit_ip(request, ip_addr, domain="api_by_ip")
return func(request, *args, **kwargs)
assert isinstance(user, UserProfile)
rate_limit_user(request, user, domain="api_by_user")
else:
assert isinstance(user, UserProfile)
rate_limit_user(request, user, domain="api_by_user")
return func(request, *args, **kwargs)

18
zerver/lib/rate_limiter.py
View File

@@ -132,6 +132,24 @@ class RateLimitedUser(RateLimitedObject):
return rules[self.domain]
class RateLimitedIPAddr(RateLimitedObject):
def __init__(self, ip_addr: str, domain: str = "api_by_ip") -> None:
self.ip_addr = ip_addr
self.domain = domain
if settings.RUNNING_INSIDE_TORNADO and domain in settings.RATE_LIMITING_DOMAINS_FOR_TORNADO:
backend: Optional[Type[RateLimiterBackend]] = TornadoInMemoryRateLimiterBackend
else:
backend = None
super().__init__(backend=backend)
def key(self) -> str:
# The angle brackets are important since IPv6 addresses contain :.
return f"{type(self).__name__}:<{self.ip_addr}>:{self.domain}"
def rules(self) -> List[Tuple[int, int]]:
return rules[self.domain]
def bounce_redis_key_prefix_for_testing(test_name: str) -> None:
global KEY_PREFIX
KEY_PREFIX = test_name + ":" + str(os.getpid()) + ":"

50
zerver/tests/test_decorators.py
View File

@@ -591,11 +591,14 @@ class RateLimitTestCase(ZulipTestCase):
f = rate_limit()(f)
with self.settings(RATE_LIMITING=True):
with mock.patch("zerver.decorator.rate_limit_user") as rate_limit_mock:
with mock.patch("zerver.decorator.rate_limit_user") as rate_limit_user_mock, mock.patch(
"zerver.decorator.rate_limit_ip"
) as rate_limit_ip_mock:
with self.errors_disallowed():
self.assertEqual(f(req), "some value")
self.assertFalse(rate_limit_mock.called)
self.assertFalse(rate_limit_ip_mock.called)
self.assertFalse(rate_limit_user_mock.called)
def test_debug_clients_skip_rate_limiting(self) -> None:
class Client:
@@ -613,12 +616,15 @@ class RateLimitTestCase(ZulipTestCase):
f = rate_limit()(f)
with self.settings(RATE_LIMITING=True):
with mock.patch("zerver.decorator.rate_limit_user") as rate_limit_mock:
with mock.patch("zerver.decorator.rate_limit_user") as rate_limit_user_mock, mock.patch(
"zerver.decorator.rate_limit_ip"
) as rate_limit_ip_mock:
with self.errors_disallowed():
with self.settings(DEBUG_RATE_LIMITING=True):
self.assertEqual(f(req), "some value")
self.assertFalse(rate_limit_mock.called)
self.assertFalse(rate_limit_ip_mock.called)
self.assertFalse(rate_limit_user_mock.called)
def test_rate_limit_setting_of_false_bypasses_rate_limiting(self) -> None:
class Client:
@@ -636,11 +642,14 @@ class RateLimitTestCase(ZulipTestCase):
f = rate_limit()(f)
with self.settings(RATE_LIMITING=False):
with mock.patch("zerver.decorator.rate_limit_user") as rate_limit_mock:
with mock.patch("zerver.decorator.rate_limit_user") as rate_limit_user_mock, mock.patch(
"zerver.decorator.rate_limit_ip"
) as rate_limit_ip_mock:
with self.errors_disallowed():
self.assertEqual(f(req), "some value")
self.assertFalse(rate_limit_mock.called)
self.assertFalse(rate_limit_ip_mock.called)
self.assertFalse(rate_limit_user_mock.called)
def test_rate_limiting_happens_in_normal_case(self) -> None:
class Client:
@@ -665,7 +674,7 @@ class RateLimitTestCase(ZulipTestCase):
self.assertTrue(rate_limit_mock.called)
@skipUnless(settings.ZILENCER_ENABLED, "requires zilencer")
def test_rate_limiting_skipped_if_remote_server(self) -> None:
def test_rate_limiting_happens_by_ip_if_remote_server(self) -> None:
server_uuid = "1234-abcd"
server = RemoteZulipServer(
uuid=server_uuid,
@@ -689,11 +698,34 @@ class RateLimitTestCase(ZulipTestCase):
f = rate_limit()(f)
with self.settings(RATE_LIMITING=True):
with mock.patch("zerver.decorator.rate_limit_user") as rate_limit_mock:
with mock.patch("zerver.decorator.rate_limit_ip") as rate_limit_mock:
with self.errors_disallowed():
self.assertEqual(f(req), "some value")
self.assertFalse(rate_limit_mock.called)
self.assertTrue(rate_limit_mock.called)
def test_rate_limiting_happens_by_ip_if_unauthed(self) -> None:
class Client:
name = "external"
class Request:
client = Client()
META = {"REMOTE_ADDR": "3.3.3.3"}
user = AnonymousUser()
req = Request()
def f(req: Any) -> str:
return "some value"
f = rate_limit()(f)
with self.settings(RATE_LIMITING=True):
with mock.patch("zerver.decorator.rate_limit_ip") as rate_limit_mock:
with self.errors_disallowed():
self.assertEqual(f(req), "some value")
self.assertTrue(rate_limit_mock.called)
class ValidatorTestCase(ZulipTestCase):

21
zerver/tests/test_external.py
View File

@@ -9,6 +9,7 @@ from django.http import HttpResponse
from zerver.forms import email_is_not_mit_mailing_list
from zerver.lib.rate_limiter import (
RateLimitedIPAddr,
RateLimitedUser,
RateLimiterLockingException,
add_ratelimit_rule,
@@ -100,6 +101,16 @@ class RateLimitTests(ZulipTestCase):
},
)
def send_unauthed_api_request(self) -> HttpResponse:
result = self.client_get("/json/messages")
# We're not making a correct request here, but rate-limiting is supposed
# to happen before the request fails due to not being correctly made. Thus
# we expect either an 400 error if the request is allowed by the rate limiter,
# or 429 if we're above the limit. We don't expect to see other status codes here,
# so we assert for safety.
self.assertIn(result.status_code, [400, 429])
return result
def test_headers(self) -> None:
user = self.example_user("hamlet")
RateLimitedUser(user).clear_history()
@@ -147,6 +158,16 @@ class RateLimitTests(ZulipTestCase):
self.do_test_hit_ratelimits(lambda: self.send_api_message(user, "some stuff"))
def test_hit_ratelimits_as_ip(self) -> None:
add_ratelimit_rule(1, 5, domain="api_by_ip")
try:
RateLimitedIPAddr("127.0.0.1").clear_history()
self.do_test_hit_ratelimits(self.send_unauthed_api_request)
finally:
# We need this in a finally block to ensure the test cleans up after itself
# even in case of failure, to avoid polluting the rules state.
remove_ratelimit_rule(1, 5, domain="api_by_ip")
def test_hit_ratelimiterlockingexception(self) -> None:
user = self.example_user("cordelia")
RateLimitedUser(user).clear_history()

14
zerver/tests/test_rate_limiter.py
View File

@@ -4,6 +4,7 @@ from typing import Dict, List, Tuple, Type
from unittest import mock
from zerver.lib.rate_limiter import (
RateLimitedIPAddr,
RateLimitedObject,
RateLimitedUser,
RateLimiterBackend,
@@ -193,13 +194,18 @@ class TornadoInMemoryRateLimiterBackendTest(RateLimiterBackendBase):
def test_used_in_tornado(self) -> None:
user_profile = self.example_user("hamlet")
ip_addr = "192.168.0.123"
with self.settings(RUNNING_INSIDE_TORNADO=True):
obj = RateLimitedUser(user_profile, domain="api_by_user")
self.assertEqual(obj.backend, TornadoInMemoryRateLimiterBackend)
user_obj = RateLimitedUser(user_profile, domain="api_by_user")
ip_obj = RateLimitedIPAddr(ip_addr, domain="api_by_ip")
self.assertEqual(user_obj.backend, TornadoInMemoryRateLimiterBackend)
self.assertEqual(ip_obj.backend, TornadoInMemoryRateLimiterBackend)
with self.settings(RUNNING_INSIDE_TORNADO=True):
obj = RateLimitedUser(user_profile, domain="some_domain")
self.assertEqual(obj.backend, RedisRateLimiterBackend)
user_obj = RateLimitedUser(user_profile, domain="some_domain")
ip_obj = RateLimitedIPAddr(ip_addr, domain="some_domain")
self.assertEqual(user_obj.backend, RedisRateLimiterBackend)
self.assertEqual(ip_obj.backend, RedisRateLimiterBackend)
def test_block_access(self) -> None:
obj = self.create_object("test", [(2, 5)])

5
zproject/computed_settings.py
View File

@@ -375,6 +375,9 @@ RATE_LIMITING_RULES = {
"api_by_user": [
(60, 200), # 200 requests max every minute
],
"api_by_ip": [
(60, 100),
],
"authenticate_by_username": [
(1800, 5), # 5 login attempts within 30 minutes
],
@@ -389,7 +392,7 @@ RATE_LIMITING_RULES = {
# which has its own buckets separate from the default backend.
# In principle, it should be impossible to make requests to tornado that fall into
# other domains, but we use this list as an extra precaution.
RATE_LIMITING_DOMAINS_FOR_TORNADO = ["api_by_user"]
RATE_LIMITING_DOMAINS_FOR_TORNADO = ["api_by_user", "api_by_ip"]
# These ratelimits are also documented publicly at
# https://zulip.readthedocs.io/en/latest/production/email-gateway.html

1
zproject/test_extra_settings.py
View File

@@ -261,6 +261,7 @@ SOCIAL_AUTH_SAML_ENABLED_IDPS: Dict[str, SAMLIdPConfigDict] = {
RATE_LIMITING_RULES: Dict[str, List[Tuple[int, int]]] = {
"api_by_user": [],
"api_by_ip": [],
"authenticate_by_username": [],
"password_reset_form_by_email": [],
}