mirror of
https://github.com/makeplane/plane.git
synced 2026-08-29 10:08:51 +02:00
[WEB-7946] fix: add rate limiting to email/password sign-in and sign-up endpoints (#9335)
* fix: add rate limiting to email/password sign-in and sign-up endpoints All four password authentication views (app sign-in, app sign-up, space sign-in, space sign-up) extended django.views.View, so DRF's global AnonRateThrottle never ran and the endpoints accepted unlimited credential guesses with no friction (brute-force / credential stuffing, GHSA-349j). Add authentication_throttle_allows(request) at the top of each post() method — before any DB access — using the same AuthenticationThrottle already guarding the magic-code views. On rejection the view redirects with RATE_LIMIT_EXCEEDED, consistent with all other throttled auth endpoints. Default limit remains 10/minute, overridable via AUTHENTICATION_RATE_LIMIT. Co-authored-by: Plane AI <noreply@plane.so> * refactor: consolidate auth throttle into a decorator + add tests - Extract the repeated throttle-and-redirect block from the six redirect-flow auth views (email + magic, app + space) into a single throttle_auth_redirect decorator in rate_limit.py. Behaviour is unchanged: the throttle still runs before any DB access; brute-force traffic is rejected without a DB hit. - Add regression tests for the password sign-in/sign-up throttle on both app and space endpoints, mirroring the existing magic-code throttle tests. - Reset the shared AuthenticationThrottle bucket before every test in test_authentication.py. All auth endpoints share one per-IP throttle scope, so the newly-throttled password requests exhausted the budget mid-file and caused unrelated tests to trip RATE_LIMIT_EXCEEDED. --------- Co-authored-by: Plane AI <noreply@plane.so> Co-authored-by: sriramveeraghanta <veeraghanta.sriram@gmail.com>
This commit is contained in:
@@ -4,17 +4,23 @@
|
||||
|
||||
# Python imports
|
||||
import os
|
||||
from functools import wraps
|
||||
|
||||
# Third party imports
|
||||
from rest_framework.throttling import AnonRateThrottle, UserRateThrottle
|
||||
from rest_framework import status
|
||||
from rest_framework.response import Response
|
||||
|
||||
# Django imports
|
||||
from django.http import HttpResponseRedirect
|
||||
|
||||
# Module imports
|
||||
from plane.authentication.adapter.error import (
|
||||
AuthenticationException,
|
||||
AUTHENTICATION_ERROR_CODES,
|
||||
)
|
||||
from plane.authentication.utils.host import base_host
|
||||
from plane.utils.path_validator import get_safe_redirect_url
|
||||
|
||||
|
||||
class AuthenticationThrottle(AnonRateThrottle):
|
||||
@@ -49,6 +55,41 @@ def authentication_throttle_allows(request):
|
||||
return throttle.allow_request(request, None)
|
||||
|
||||
|
||||
def throttle_auth_redirect(*, is_app=False, is_space=False):
|
||||
"""
|
||||
Decorator for redirect-flow ``django.views.View`` POST handlers.
|
||||
|
||||
Applies AuthenticationThrottle before the wrapped handler runs; when the
|
||||
per-IP budget is exceeded it short-circuits with a RATE_LIMIT_EXCEEDED
|
||||
redirect to the auth error page (the same behaviour every throttled auth
|
||||
endpoint uses). The throttle runs before any DB access in the handler, so
|
||||
brute-force traffic is rejected without touching the database.
|
||||
|
||||
Pass ``is_app=True`` for /app auth views and ``is_space=True`` for /spaces
|
||||
auth views so the redirect targets the correct base host.
|
||||
"""
|
||||
|
||||
def decorator(post_method):
|
||||
@wraps(post_method)
|
||||
def wrapper(self, request, *args, **kwargs):
|
||||
if not authentication_throttle_allows(request):
|
||||
exc = AuthenticationException(
|
||||
error_code=AUTHENTICATION_ERROR_CODES["RATE_LIMIT_EXCEEDED"],
|
||||
error_message="RATE_LIMIT_EXCEEDED",
|
||||
)
|
||||
url = get_safe_redirect_url(
|
||||
base_url=base_host(request=request, is_app=is_app, is_space=is_space),
|
||||
next_path=request.POST.get("next_path"),
|
||||
params=exc.get_error_dict(),
|
||||
)
|
||||
return HttpResponseRedirect(url)
|
||||
return post_method(self, request, *args, **kwargs)
|
||||
|
||||
return wrapper
|
||||
|
||||
return decorator
|
||||
|
||||
|
||||
class EmailVerificationThrottle(UserRateThrottle):
|
||||
"""
|
||||
Throttle for email verification code generation.
|
||||
|
||||
@@ -11,6 +11,7 @@ from django.views import View
|
||||
# Module imports
|
||||
from plane.authentication.provider.credentials.email import EmailProvider
|
||||
from plane.authentication.utils.login import user_login
|
||||
from plane.authentication.rate_limit import throttle_auth_redirect
|
||||
from plane.license.models import Instance
|
||||
from plane.authentication.utils.host import base_host
|
||||
from plane.authentication.utils.redirection_path import get_redirection_path
|
||||
@@ -24,6 +25,10 @@ from plane.utils.path_validator import get_safe_redirect_url
|
||||
|
||||
|
||||
class SignInAuthEndpoint(View):
|
||||
# Rate-limit password auth attempts before any DB access, using the same
|
||||
# AuthenticationThrottle the magic-code views use (default 10/minute,
|
||||
# configurable via the AUTHENTICATION_RATE_LIMIT env var).
|
||||
@throttle_auth_redirect(is_app=True)
|
||||
def post(self, request):
|
||||
next_path = request.POST.get("next_path")
|
||||
# Check instance configuration
|
||||
@@ -133,6 +138,7 @@ class SignInAuthEndpoint(View):
|
||||
|
||||
|
||||
class SignUpAuthEndpoint(View):
|
||||
@throttle_auth_redirect(is_app=True)
|
||||
def post(self, request):
|
||||
next_path = request.POST.get("next_path")
|
||||
# Check instance configuration
|
||||
|
||||
@@ -28,7 +28,7 @@ from plane.authentication.adapter.error import (
|
||||
)
|
||||
from plane.authentication.rate_limit import (
|
||||
AuthenticationThrottle,
|
||||
authentication_throttle_allows,
|
||||
throttle_auth_redirect,
|
||||
)
|
||||
from plane.utils.path_validator import get_safe_redirect_url
|
||||
|
||||
@@ -62,24 +62,13 @@ class MagicGenerateEndpoint(APIView):
|
||||
|
||||
|
||||
class MagicSignInEndpoint(View):
|
||||
@throttle_auth_redirect(is_app=True)
|
||||
def post(self, request):
|
||||
# set the referer as session to redirect after login
|
||||
code = request.POST.get("code", "").strip()
|
||||
email = request.POST.get("email", "").strip().lower()
|
||||
next_path = request.POST.get("next_path")
|
||||
|
||||
if not authentication_throttle_allows(request):
|
||||
exc = AuthenticationException(
|
||||
error_code=AUTHENTICATION_ERROR_CODES["RATE_LIMIT_EXCEEDED"],
|
||||
error_message="RATE_LIMIT_EXCEEDED",
|
||||
)
|
||||
url = get_safe_redirect_url(
|
||||
base_url=base_host(request=request, is_app=True),
|
||||
next_path=next_path,
|
||||
params=exc.get_error_dict(),
|
||||
)
|
||||
return HttpResponseRedirect(url)
|
||||
|
||||
if code == "" or email == "":
|
||||
exc = AuthenticationException(
|
||||
error_code=AUTHENTICATION_ERROR_CODES["MAGIC_SIGN_IN_EMAIL_CODE_REQUIRED"],
|
||||
@@ -145,24 +134,13 @@ class MagicSignInEndpoint(View):
|
||||
|
||||
|
||||
class MagicSignUpEndpoint(View):
|
||||
@throttle_auth_redirect(is_app=True)
|
||||
def post(self, request):
|
||||
# set the referer as session to redirect after login
|
||||
code = request.POST.get("code", "").strip()
|
||||
email = request.POST.get("email", "").strip().lower()
|
||||
next_path = request.POST.get("next_path")
|
||||
|
||||
if not authentication_throttle_allows(request):
|
||||
exc = AuthenticationException(
|
||||
error_code=AUTHENTICATION_ERROR_CODES["RATE_LIMIT_EXCEEDED"],
|
||||
error_message="RATE_LIMIT_EXCEEDED",
|
||||
)
|
||||
url = get_safe_redirect_url(
|
||||
base_url=base_host(request=request, is_app=True),
|
||||
next_path=next_path,
|
||||
params=exc.get_error_dict(),
|
||||
)
|
||||
return HttpResponseRedirect(url)
|
||||
|
||||
if code == "" or email == "":
|
||||
exc = AuthenticationException(
|
||||
error_code=AUTHENTICATION_ERROR_CODES["MAGIC_SIGN_UP_EMAIL_CODE_REQUIRED"],
|
||||
|
||||
@@ -12,6 +12,7 @@ from django.utils.http import url_has_allowed_host_and_scheme
|
||||
# Module imports
|
||||
from plane.authentication.provider.credentials.email import EmailProvider
|
||||
from plane.authentication.utils.login import user_login
|
||||
from plane.authentication.rate_limit import throttle_auth_redirect
|
||||
from plane.license.models import Instance
|
||||
from plane.authentication.utils.host import base_host
|
||||
from plane.db.models import User
|
||||
@@ -23,6 +24,7 @@ from plane.utils.path_validator import get_safe_redirect_url, validate_next_path
|
||||
|
||||
|
||||
class SignInAuthSpaceEndpoint(View):
|
||||
@throttle_auth_redirect(is_space=True)
|
||||
def post(self, request):
|
||||
next_path = request.POST.get("next_path")
|
||||
# Check instance configuration
|
||||
@@ -108,6 +110,7 @@ class SignInAuthSpaceEndpoint(View):
|
||||
|
||||
|
||||
class SignUpAuthSpaceEndpoint(View):
|
||||
@throttle_auth_redirect(is_space=True)
|
||||
def post(self, request):
|
||||
next_path = request.POST.get("next_path")
|
||||
# Check instance configuration
|
||||
|
||||
@@ -27,7 +27,7 @@ from plane.authentication.adapter.error import (
|
||||
)
|
||||
from plane.authentication.rate_limit import (
|
||||
AuthenticationThrottle,
|
||||
authentication_throttle_allows,
|
||||
throttle_auth_redirect,
|
||||
)
|
||||
from plane.utils.path_validator import get_safe_redirect_url, validate_next_path, get_allowed_hosts
|
||||
|
||||
@@ -60,24 +60,13 @@ class MagicGenerateSpaceEndpoint(APIView):
|
||||
|
||||
|
||||
class MagicSignInSpaceEndpoint(View):
|
||||
@throttle_auth_redirect(is_space=True)
|
||||
def post(self, request):
|
||||
# set the referer as session to redirect after login
|
||||
code = request.POST.get("code", "").strip()
|
||||
email = request.POST.get("email", "").strip().lower()
|
||||
next_path = request.POST.get("next_path")
|
||||
|
||||
if not authentication_throttle_allows(request):
|
||||
exc = AuthenticationException(
|
||||
error_code=AUTHENTICATION_ERROR_CODES["RATE_LIMIT_EXCEEDED"],
|
||||
error_message="RATE_LIMIT_EXCEEDED",
|
||||
)
|
||||
url = get_safe_redirect_url(
|
||||
base_url=base_host(request=request, is_space=True),
|
||||
next_path=next_path,
|
||||
params=exc.get_error_dict(),
|
||||
)
|
||||
return HttpResponseRedirect(url)
|
||||
|
||||
if code == "" or email == "":
|
||||
exc = AuthenticationException(
|
||||
error_code=AUTHENTICATION_ERROR_CODES["MAGIC_SIGN_IN_EMAIL_CODE_REQUIRED"],
|
||||
@@ -131,24 +120,13 @@ class MagicSignInSpaceEndpoint(View):
|
||||
|
||||
|
||||
class MagicSignUpSpaceEndpoint(View):
|
||||
@throttle_auth_redirect(is_space=True)
|
||||
def post(self, request):
|
||||
# set the referer as session to redirect after login
|
||||
code = request.POST.get("code", "").strip()
|
||||
email = request.POST.get("email", "").strip().lower()
|
||||
next_path = request.POST.get("next_path")
|
||||
|
||||
if not authentication_throttle_allows(request):
|
||||
exc = AuthenticationException(
|
||||
error_code=AUTHENTICATION_ERROR_CODES["RATE_LIMIT_EXCEEDED"],
|
||||
error_message="RATE_LIMIT_EXCEEDED",
|
||||
)
|
||||
url = get_safe_redirect_url(
|
||||
base_url=base_host(request=request, is_space=True),
|
||||
next_path=next_path,
|
||||
params=exc.get_error_dict(),
|
||||
)
|
||||
return HttpResponseRedirect(url)
|
||||
|
||||
if code == "" or email == "":
|
||||
exc = AuthenticationException(
|
||||
error_code=AUTHENTICATION_ERROR_CODES["MAGIC_SIGN_UP_EMAIL_CODE_REQUIRED"],
|
||||
|
||||
@@ -74,6 +74,21 @@ def django_client():
|
||||
return client
|
||||
|
||||
|
||||
@pytest.fixture(autouse=True)
|
||||
def _reset_authentication_throttle():
|
||||
"""
|
||||
Reset the shared AuthenticationThrottle bucket around every test in this module.
|
||||
|
||||
All auth endpoints share one per-IP throttle bucket (scope "authentication")
|
||||
and the test client always presents the same IP, so without clearing the cache
|
||||
between tests the request count accumulates across tests and later ones trip
|
||||
RATE_LIMIT_EXCEEDED. Clearing here keeps each test's rate-limit budget isolated.
|
||||
"""
|
||||
cache.clear()
|
||||
yield
|
||||
cache.clear()
|
||||
|
||||
|
||||
@pytest.mark.contract
|
||||
class TestMagicLinkGenerate:
|
||||
"""Test magic link generation functionality"""
|
||||
@@ -619,7 +634,7 @@ class TestMagicSignUpVerifyAttempts:
|
||||
|
||||
@pytest.mark.contract
|
||||
class TestAuthenticationThrottle:
|
||||
"""Per-IP throttle on the redirect-flow magic-link endpoints."""
|
||||
"""Per-IP throttle on the redirect-flow magic-link and password endpoints."""
|
||||
|
||||
@pytest.fixture(autouse=True)
|
||||
def _clear_state(self):
|
||||
@@ -654,6 +669,56 @@ class TestAuthenticationThrottle:
|
||||
response = django_client.post(url, {"email": "throttle-up@plane.so", "code": "000000"}, follow=False)
|
||||
assert "RATE_LIMIT_EXCEEDED" in response.url
|
||||
|
||||
@pytest.mark.django_db
|
||||
def test_password_sign_in_throttled(self, django_client, setup_instance):
|
||||
"""The password sign-in endpoint is throttled per IP on the same scope."""
|
||||
url = reverse("sign-in")
|
||||
with patch.object(AuthenticationThrottle, "rate", "2/minute"):
|
||||
for _ in range(2):
|
||||
response = django_client.post(
|
||||
url, {"email": "throttle@plane.so", "password": "secret123"}, follow=False
|
||||
)
|
||||
assert response.status_code == 302
|
||||
assert "RATE_LIMIT_EXCEEDED" not in response.url
|
||||
|
||||
# The 3rd request from the same IP within the window trips the throttle.
|
||||
response = django_client.post(url, {"email": "throttle@plane.so", "password": "secret123"}, follow=False)
|
||||
assert response.status_code == 302
|
||||
assert "RATE_LIMIT_EXCEEDED" in response.url
|
||||
|
||||
@pytest.mark.django_db
|
||||
def test_password_sign_up_throttled(self, django_client, setup_instance):
|
||||
"""The password sign-up endpoint trips on the same per-IP budget."""
|
||||
url = reverse("sign-up")
|
||||
with patch.object(AuthenticationThrottle, "rate", "1/minute"):
|
||||
response = django_client.post(url, {"email": "throttle-up@plane.so", "password": "secret123"}, follow=False)
|
||||
assert "RATE_LIMIT_EXCEEDED" not in response.url
|
||||
|
||||
response = django_client.post(url, {"email": "throttle-up@plane.so", "password": "secret123"}, follow=False)
|
||||
assert "RATE_LIMIT_EXCEEDED" in response.url
|
||||
|
||||
@pytest.mark.django_db
|
||||
def test_space_password_sign_in_throttled(self, django_client, setup_instance):
|
||||
"""The spaces password sign-in endpoint is throttled per IP."""
|
||||
url = reverse("space-sign-in")
|
||||
with patch.object(AuthenticationThrottle, "rate", "1/minute"):
|
||||
response = django_client.post(url, {"email": "throttle@plane.so", "password": "secret123"}, follow=False)
|
||||
assert "RATE_LIMIT_EXCEEDED" not in response.url
|
||||
|
||||
response = django_client.post(url, {"email": "throttle@plane.so", "password": "secret123"}, follow=False)
|
||||
assert "RATE_LIMIT_EXCEEDED" in response.url
|
||||
|
||||
@pytest.mark.django_db
|
||||
def test_space_password_sign_up_throttled(self, django_client, setup_instance):
|
||||
"""The spaces password sign-up endpoint trips on the same per-IP budget."""
|
||||
url = reverse("space-sign-up")
|
||||
with patch.object(AuthenticationThrottle, "rate", "1/minute"):
|
||||
response = django_client.post(url, {"email": "throttle-up@plane.so", "password": "secret123"}, follow=False)
|
||||
assert "RATE_LIMIT_EXCEEDED" not in response.url
|
||||
|
||||
response = django_client.post(url, {"email": "throttle-up@plane.so", "password": "secret123"}, follow=False)
|
||||
assert "RATE_LIMIT_EXCEEDED" in response.url
|
||||
|
||||
|
||||
@pytest.mark.contract
|
||||
class TestBotUserLoginBlocked:
|
||||
|
||||
Reference in New Issue
Block a user