mirror of https://github.com/zulip/zulip.git
remote_billing: Add rate-limiting for confirmation email sending.
These should be rate-limited by both IP using our regular sends_email_by_ip bucket as well as by server, using a new bucket dedicated to this.
This commit is contained in:
parent
07c4291749
commit
649b4885e8
|
@ -25,13 +25,16 @@ from corporate.models import (
|
||||||
from corporate.views.remote_billing_page import generate_confirmation_link_for_server_deactivation
|
from corporate.views.remote_billing_page import generate_confirmation_link_for_server_deactivation
|
||||||
from zerver.actions.realm_settings import do_deactivate_realm
|
from zerver.actions.realm_settings import do_deactivate_realm
|
||||||
from zerver.lib.exceptions import RemoteRealmServerMismatchError
|
from zerver.lib.exceptions import RemoteRealmServerMismatchError
|
||||||
|
from zerver.lib.rate_limiter import RateLimitedIPAddr
|
||||||
from zerver.lib.remote_server import send_server_data_to_push_bouncer
|
from zerver.lib.remote_server import send_server_data_to_push_bouncer
|
||||||
from zerver.lib.test_classes import BouncerTestCase
|
from zerver.lib.test_classes import BouncerTestCase
|
||||||
|
from zerver.lib.test_helpers import ratelimit_rule
|
||||||
from zerver.lib.timestamp import datetime_to_timestamp
|
from zerver.lib.timestamp import datetime_to_timestamp
|
||||||
from zerver.models import Realm, UserProfile
|
from zerver.models import Realm, UserProfile
|
||||||
from zilencer.models import (
|
from zilencer.models import (
|
||||||
PreregistrationRemoteRealmBillingUser,
|
PreregistrationRemoteRealmBillingUser,
|
||||||
PreregistrationRemoteServerBillingUser,
|
PreregistrationRemoteServerBillingUser,
|
||||||
|
RateLimitedRemoteZulipServer,
|
||||||
RemoteRealm,
|
RemoteRealm,
|
||||||
RemoteRealmBillingUser,
|
RemoteRealmBillingUser,
|
||||||
RemoteServerBillingUser,
|
RemoteServerBillingUser,
|
||||||
|
@ -92,6 +95,11 @@ class RemoteRealmBillingTestCase(BouncerTestCase):
|
||||||
{"email": user.delivery_email},
|
{"email": user.delivery_email},
|
||||||
subdomain="selfhosting",
|
subdomain="selfhosting",
|
||||||
)
|
)
|
||||||
|
if result.status_code == 429:
|
||||||
|
# Return rate limit errors early, since they occur in rate limiting tests
|
||||||
|
# that want to verify them.
|
||||||
|
return result
|
||||||
|
|
||||||
self.assertEqual(result.status_code, 200)
|
self.assertEqual(result.status_code, 200)
|
||||||
self.assert_in_success_response(
|
self.assert_in_success_response(
|
||||||
[
|
[
|
||||||
|
@ -250,6 +258,55 @@ class RemoteBillingAuthenticationTest(RemoteRealmBillingTestCase):
|
||||||
result = self.client_get(result["Location"], subdomain="selfhosting")
|
result = self.client_get(result["Location"], subdomain="selfhosting")
|
||||||
self.assert_in_success_response(["showing-self-hosted", "Retain full control"], result)
|
self.assert_in_success_response(["showing-self-hosted", "Retain full control"], result)
|
||||||
|
|
||||||
|
@ratelimit_rule(10, 3, domain="sends_email_by_remote_server")
|
||||||
|
@ratelimit_rule(10, 2, domain="sends_email_by_ip")
|
||||||
|
@responses.activate
|
||||||
|
def test_remote_billing_authentication_flow_rate_limited(self) -> None:
|
||||||
|
RateLimitedIPAddr("127.0.0.1", domain="sends_email_by_ip").clear_history()
|
||||||
|
RateLimitedRemoteZulipServer(
|
||||||
|
self.server, domain="sends_email_by_remote_server"
|
||||||
|
).clear_history()
|
||||||
|
|
||||||
|
self.login("desdemona")
|
||||||
|
desdemona = self.example_user("desdemona")
|
||||||
|
|
||||||
|
self.add_mock_response()
|
||||||
|
send_server_data_to_push_bouncer(consider_usage_statistics=False)
|
||||||
|
|
||||||
|
for i in range(2):
|
||||||
|
result = self.execute_remote_billing_authentication_flow(
|
||||||
|
desdemona, return_without_clicking_confirmation_link=True
|
||||||
|
)
|
||||||
|
self.assertEqual(result.status_code, 200)
|
||||||
|
|
||||||
|
result = self.execute_remote_billing_authentication_flow(
|
||||||
|
desdemona, return_without_clicking_confirmation_link=True
|
||||||
|
)
|
||||||
|
self.assertEqual(result.status_code, 429)
|
||||||
|
self.assert_in_response("You have exceeded the limit", result)
|
||||||
|
|
||||||
|
# Reset the IP rate limit so that we trigger the server-based one.
|
||||||
|
RateLimitedIPAddr("127.0.0.1", domain="sends_email_by_ip").clear_history()
|
||||||
|
|
||||||
|
result = self.execute_remote_billing_authentication_flow(
|
||||||
|
desdemona, return_without_clicking_confirmation_link=True
|
||||||
|
)
|
||||||
|
self.assertEqual(result.status_code, 200)
|
||||||
|
|
||||||
|
with self.assertLogs("zilencer.auth", "WARN") as mock_log:
|
||||||
|
result = self.execute_remote_billing_authentication_flow(
|
||||||
|
desdemona, return_without_clicking_confirmation_link=True
|
||||||
|
)
|
||||||
|
self.assertEqual(result.status_code, 429)
|
||||||
|
self.assert_in_response("Your server has exceeded the limit", result)
|
||||||
|
self.assertEqual(
|
||||||
|
mock_log.output,
|
||||||
|
[
|
||||||
|
f"WARNING:zilencer.auth:Remote server {self.server.hostname} {str(self.server.uuid)[:12]} exceeded "
|
||||||
|
"rate limits on domain sends_email_by_remote_server"
|
||||||
|
],
|
||||||
|
)
|
||||||
|
|
||||||
@responses.activate
|
@responses.activate
|
||||||
def test_remote_billing_authentication_flow_realm_not_registered(self) -> None:
|
def test_remote_billing_authentication_flow_realm_not_registered(self) -> None:
|
||||||
RemoteRealm.objects.all().delete()
|
RemoteRealm.objects.all().delete()
|
||||||
|
@ -1042,6 +1099,11 @@ class RemoteServerTestCase(BouncerTestCase):
|
||||||
payload,
|
payload,
|
||||||
subdomain="selfhosting",
|
subdomain="selfhosting",
|
||||||
)
|
)
|
||||||
|
if result.status_code == 429:
|
||||||
|
# Return rate limit errors early, since they occur in rate limiting tests
|
||||||
|
# that want to verify them.
|
||||||
|
return result
|
||||||
|
|
||||||
self.assertEqual(result.status_code, 200)
|
self.assertEqual(result.status_code, 200)
|
||||||
self.assert_in_success_response(
|
self.assert_in_success_response(
|
||||||
["We have sent", "a log in", "link will expire in", email],
|
["We have sent", "a log in", "link will expire in", email],
|
||||||
|
@ -1101,6 +1163,59 @@ class RemoteServerTestCase(BouncerTestCase):
|
||||||
|
|
||||||
|
|
||||||
class LegacyServerLoginTest(RemoteServerTestCase):
|
class LegacyServerLoginTest(RemoteServerTestCase):
|
||||||
|
@ratelimit_rule(10, 3, domain="sends_email_by_remote_server")
|
||||||
|
@ratelimit_rule(10, 2, domain="sends_email_by_ip")
|
||||||
|
def test_remote_billing_authentication_flow_rate_limited(self) -> None:
|
||||||
|
RateLimitedIPAddr("127.0.0.1", domain="sends_email_by_ip").clear_history()
|
||||||
|
RateLimitedRemoteZulipServer(
|
||||||
|
self.server, domain="sends_email_by_remote_server"
|
||||||
|
).clear_history()
|
||||||
|
|
||||||
|
self.login("desdemona")
|
||||||
|
desdemona = self.example_user("desdemona")
|
||||||
|
|
||||||
|
for i in range(2):
|
||||||
|
result = self.execute_remote_billing_authentication_flow(
|
||||||
|
desdemona.delivery_email,
|
||||||
|
desdemona.full_name,
|
||||||
|
return_without_clicking_confirmation_link=True,
|
||||||
|
)
|
||||||
|
self.assertEqual(result.status_code, 200)
|
||||||
|
|
||||||
|
result = self.execute_remote_billing_authentication_flow(
|
||||||
|
desdemona.delivery_email,
|
||||||
|
desdemona.full_name,
|
||||||
|
return_without_clicking_confirmation_link=True,
|
||||||
|
)
|
||||||
|
self.assertEqual(result.status_code, 429)
|
||||||
|
self.assert_in_response("You have exceeded the limit", result)
|
||||||
|
|
||||||
|
# Reset the IP rate limit so that we trigger the server-based one.
|
||||||
|
RateLimitedIPAddr("127.0.0.1", domain="sends_email_by_ip").clear_history()
|
||||||
|
|
||||||
|
result = self.execute_remote_billing_authentication_flow(
|
||||||
|
desdemona.delivery_email,
|
||||||
|
desdemona.full_name,
|
||||||
|
return_without_clicking_confirmation_link=True,
|
||||||
|
)
|
||||||
|
self.assertEqual(result.status_code, 200)
|
||||||
|
|
||||||
|
with self.assertLogs("zilencer.auth", "WARN") as mock_log:
|
||||||
|
result = self.execute_remote_billing_authentication_flow(
|
||||||
|
desdemona.delivery_email,
|
||||||
|
desdemona.full_name,
|
||||||
|
return_without_clicking_confirmation_link=True,
|
||||||
|
)
|
||||||
|
self.assertEqual(result.status_code, 429)
|
||||||
|
self.assert_in_response("Your server has exceeded the limit", result)
|
||||||
|
self.assertEqual(
|
||||||
|
mock_log.output,
|
||||||
|
[
|
||||||
|
f"WARNING:zilencer.auth:Remote server {self.server.hostname} {str(self.server.uuid)[:12]} exceeded "
|
||||||
|
"rate limits on domain sends_email_by_remote_server"
|
||||||
|
],
|
||||||
|
)
|
||||||
|
|
||||||
def test_server_login_get(self) -> None:
|
def test_server_login_get(self) -> None:
|
||||||
result = self.client_get("/serverlogin/", subdomain="selfhosting")
|
result = self.client_get("/serverlogin/", subdomain="selfhosting")
|
||||||
self.assertEqual(result.status_code, 200)
|
self.assertEqual(result.status_code, 200)
|
||||||
|
|
|
@ -44,14 +44,17 @@ from corporate.models import (
|
||||||
from zerver.lib.exceptions import (
|
from zerver.lib.exceptions import (
|
||||||
JsonableError,
|
JsonableError,
|
||||||
MissingRemoteRealmError,
|
MissingRemoteRealmError,
|
||||||
|
RateLimitedError,
|
||||||
RemoteBillingAuthenticationError,
|
RemoteBillingAuthenticationError,
|
||||||
RemoteRealmServerMismatchError,
|
RemoteRealmServerMismatchError,
|
||||||
)
|
)
|
||||||
|
from zerver.lib.rate_limiter import rate_limit_request_by_ip
|
||||||
from zerver.lib.remote_server import RealmDataForAnalytics, UserDataForRemoteBilling
|
from zerver.lib.remote_server import RealmDataForAnalytics, UserDataForRemoteBilling
|
||||||
from zerver.lib.response import json_success
|
from zerver.lib.response import json_success
|
||||||
from zerver.lib.send_email import FromAddress, send_email
|
from zerver.lib.send_email import FromAddress, send_email
|
||||||
from zerver.lib.timestamp import datetime_to_timestamp
|
from zerver.lib.timestamp import datetime_to_timestamp
|
||||||
from zerver.lib.typed_endpoint import PathOnly, typed_endpoint
|
from zerver.lib.typed_endpoint import PathOnly, typed_endpoint
|
||||||
|
from zilencer.auth import rate_limit_remote_server
|
||||||
from zilencer.models import (
|
from zilencer.models import (
|
||||||
PreregistrationRemoteRealmBillingUser,
|
PreregistrationRemoteRealmBillingUser,
|
||||||
PreregistrationRemoteServerBillingUser,
|
PreregistrationRemoteServerBillingUser,
|
||||||
|
@ -374,6 +377,10 @@ def remote_realm_billing_confirm_email(
|
||||||
except ObjectDoesNotExist:
|
except ObjectDoesNotExist:
|
||||||
raise AssertionError
|
raise AssertionError
|
||||||
|
|
||||||
|
rate_limit_error_response = check_rate_limits(request, remote_server)
|
||||||
|
if rate_limit_error_response is not None:
|
||||||
|
return rate_limit_error_response
|
||||||
|
|
||||||
obj = PreregistrationRemoteRealmBillingUser.objects.create(
|
obj = PreregistrationRemoteRealmBillingUser.objects.create(
|
||||||
email=email,
|
email=email,
|
||||||
remote_realm=remote_realm,
|
remote_realm=remote_realm,
|
||||||
|
@ -600,6 +607,10 @@ def remote_billing_legacy_server_confirm_login(
|
||||||
reverse("remote_billing_legacy_server_login") + f"?next_page={next_page}"
|
reverse("remote_billing_legacy_server_login") + f"?next_page={next_page}"
|
||||||
)
|
)
|
||||||
|
|
||||||
|
rate_limit_error_response = check_rate_limits(request, remote_server)
|
||||||
|
if rate_limit_error_response is not None:
|
||||||
|
return rate_limit_error_response
|
||||||
|
|
||||||
obj = PreregistrationRemoteServerBillingUser.objects.create(
|
obj = PreregistrationRemoteServerBillingUser.objects.create(
|
||||||
email=email,
|
email=email,
|
||||||
remote_server=remote_server,
|
remote_server=remote_server,
|
||||||
|
@ -796,3 +807,36 @@ def generate_confirmation_link_for_server_deactivation(
|
||||||
validity_in_minutes=validity_in_minutes,
|
validity_in_minutes=validity_in_minutes,
|
||||||
)
|
)
|
||||||
return url
|
return url
|
||||||
|
|
||||||
|
|
||||||
|
def check_rate_limits(
|
||||||
|
request: HttpRequest, remote_server: RemoteZulipServer
|
||||||
|
) -> Optional[HttpResponse]:
|
||||||
|
try:
|
||||||
|
rate_limit_request_by_ip(request, domain="sends_email_by_ip")
|
||||||
|
except RateLimitedError as e:
|
||||||
|
# Our generic error response is good enough here, since this is
|
||||||
|
# about the user's IP address, not their entire server.
|
||||||
|
assert e.secs_to_freedom is not None
|
||||||
|
return render(
|
||||||
|
request,
|
||||||
|
"zerver/rate_limit_exceeded.html",
|
||||||
|
context={"retry_after": int(e.secs_to_freedom)},
|
||||||
|
status=429,
|
||||||
|
)
|
||||||
|
|
||||||
|
try:
|
||||||
|
rate_limit_remote_server(request, remote_server, "sends_email_by_remote_server")
|
||||||
|
except RateLimitedError as e:
|
||||||
|
# In this case it's the limit for the entire server the user belongs to
|
||||||
|
# that was exceeded, so we need to show an error page explaining
|
||||||
|
# that specific situation.
|
||||||
|
assert e.secs_to_freedom is not None
|
||||||
|
return render(
|
||||||
|
request,
|
||||||
|
"corporate/remote_server_rate_limit_exceeded.html",
|
||||||
|
context={"retry_after": int(e.secs_to_freedom)},
|
||||||
|
status=429,
|
||||||
|
)
|
||||||
|
|
||||||
|
return None
|
||||||
|
|
|
@ -0,0 +1,27 @@
|
||||||
|
{% extends "zerver/portico.html" %}
|
||||||
|
|
||||||
|
{% block title %}
|
||||||
|
<title>{{ _("Rate limit exceeded") }} | Zulip</title>
|
||||||
|
{% endblock %}
|
||||||
|
|
||||||
|
{% block portico_content %}
|
||||||
|
|
||||||
|
<div class="error_page">
|
||||||
|
<div class="container">
|
||||||
|
<div class="row-fluid">
|
||||||
|
<img src="{{ static('images/errors/500art.svg') }}" alt=""/>
|
||||||
|
<div class="errorbox">
|
||||||
|
<div class="errorcontent">
|
||||||
|
<h1 class="lead">{{ _("Rate limit exceeded.") }}</h1>
|
||||||
|
<p>
|
||||||
|
{% trans %}Your server has exceeded the limit for how
|
||||||
|
often this action can be performed.{% endtrans %}
|
||||||
|
{% trans %}You can try again in {{retry_after}} seconds.{% endtrans %}
|
||||||
|
</p>
|
||||||
|
</div>
|
||||||
|
</div>
|
||||||
|
</div>
|
||||||
|
</div>
|
||||||
|
</div>
|
||||||
|
|
||||||
|
{% endblock %}
|
|
@ -291,6 +291,12 @@ DEFAULT_RATE_LIMITING_RULES = {
|
||||||
# 1000 per day per file
|
# 1000 per day per file
|
||||||
(86400, 1000),
|
(86400, 1000),
|
||||||
],
|
],
|
||||||
|
# A zilencer-only limit that applies to requests to the
|
||||||
|
# remote billing system that trigger the sending of an email.
|
||||||
|
"sends_email_by_remote_server": [
|
||||||
|
# 10 emails per day
|
||||||
|
(86400, 10),
|
||||||
|
],
|
||||||
}
|
}
|
||||||
# Rate limiting defaults can be individually overridden by adding
|
# Rate limiting defaults can be individually overridden by adding
|
||||||
# entries in this object, which is merged with
|
# entries in this object, which is merged with
|
||||||
|
|
|
@ -256,6 +256,7 @@ RATE_LIMITING_RULES: Dict[str, List[Tuple[int, int]]] = {
|
||||||
"sends_email_by_ip": [],
|
"sends_email_by_ip": [],
|
||||||
"email_change_by_user": [],
|
"email_change_by_user": [],
|
||||||
"password_reset_form_by_email": [],
|
"password_reset_form_by_email": [],
|
||||||
|
"sends_email_by_remote_server": [],
|
||||||
}
|
}
|
||||||
|
|
||||||
CLOUD_FREE_TRIAL_DAYS: Optional[int] = None
|
CLOUD_FREE_TRIAL_DAYS: Optional[int] = None
|
||||||
|
|
Loading…
Reference in New Issue