From e30605d4199ba21cbb46527a0b2351c3ea41d51c Mon Sep 17 00:00:00 2001 From: Manish Gupta <59428681+mguptahub@users.noreply.github.com> Date: Fri, 28 Aug 2026 00:13:58 +0530 Subject: [PATCH] [WEB-7946] fix: add rate limiting to email/password sign-in and sign-up endpoints (#9335) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * 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 * 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 Co-authored-by: sriramveeraghanta --- apps/api/plane/authentication/rate_limit.py | 41 ++++++++++++ .../plane/authentication/views/app/email.py | 6 ++ .../plane/authentication/views/app/magic.py | 28 +------- .../plane/authentication/views/space/email.py | 3 + .../plane/authentication/views/space/magic.py | 28 +------- .../tests/contract/app/test_authentication.py | 67 ++++++++++++++++++- 6 files changed, 122 insertions(+), 51 deletions(-) diff --git a/apps/api/plane/authentication/rate_limit.py b/apps/api/plane/authentication/rate_limit.py index bfadf82b70..898e52bd9f 100644 --- a/apps/api/plane/authentication/rate_limit.py +++ b/apps/api/plane/authentication/rate_limit.py @@ -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. diff --git a/apps/api/plane/authentication/views/app/email.py b/apps/api/plane/authentication/views/app/email.py index 3d1954875c..0527c115c8 100644 --- a/apps/api/plane/authentication/views/app/email.py +++ b/apps/api/plane/authentication/views/app/email.py @@ -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 diff --git a/apps/api/plane/authentication/views/app/magic.py b/apps/api/plane/authentication/views/app/magic.py index de77c13c80..63e2686d44 100644 --- a/apps/api/plane/authentication/views/app/magic.py +++ b/apps/api/plane/authentication/views/app/magic.py @@ -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"], diff --git a/apps/api/plane/authentication/views/space/email.py b/apps/api/plane/authentication/views/space/email.py index 827348cef2..b4a36aecb6 100644 --- a/apps/api/plane/authentication/views/space/email.py +++ b/apps/api/plane/authentication/views/space/email.py @@ -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 diff --git a/apps/api/plane/authentication/views/space/magic.py b/apps/api/plane/authentication/views/space/magic.py index 13001991b2..e4e8061270 100644 --- a/apps/api/plane/authentication/views/space/magic.py +++ b/apps/api/plane/authentication/views/space/magic.py @@ -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"], diff --git a/apps/api/plane/tests/contract/app/test_authentication.py b/apps/api/plane/tests/contract/app/test_authentication.py index 7a7d9640b7..c83a673e37 100644 --- a/apps/api/plane/tests/contract/app/test_authentication.py +++ b/apps/api/plane/tests/contract/app/test_authentication.py @@ -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: