diff --git a/web/rap_web/security_middleware.py b/web/rap_web/security_middleware.py index ad63066..e9fd881 100644 --- a/web/rap_web/security_middleware.py +++ b/web/rap_web/security_middleware.py @@ -110,6 +110,8 @@ def get_client_ip(self, request): class BasicRateLimitMiddleware(MiddlewareMixin): """ Basic rate limiting middleware as fallback when django-ratelimit is not available. + Rate limits login attempts per username to avoid locking out all users behind + the same proxy/load balancer when using IP-based rate limiting. """ def process_request(self, request): @@ -119,26 +121,45 @@ def process_request(self, request): ): return None - client_ip = self.get_client_ip(request) - - # Rate limit login attempts + # Rate limit login attempts by username (not IP, to avoid proxy issues) if request.path == "/users/login/" and request.method == "POST": - cache_key = f"login_attempts_{client_ip}" - attempts = cache.get(cache_key, 0) + failure_limit = getattr(settings, "AXES_FAILURE_LIMIT", 5) + username = request.POST.get("username", "").strip().lower() + if username: + cache_key = f"login_attempts_user_{username}" + attempts = cache.get(cache_key, 0) - if attempts >= 5: # Max 5 attempts per 5 minutes - logger.warning( - f"Rate limit exceeded for login attempts from IP {client_ip}" - ) - return HttpResponse( - "Te veel inlogpogingen. Probeer het over 5 minuten opnieuw.", - status=429, - ) + if attempts >= failure_limit: + logger.warning( + f"Rate limit exceeded for login attempts for username '{username}'" + ) + return HttpResponse( + "Te veel inlogpogingen. Probeer het over 5 minuten opnieuw.", + status=429, + ) + + cache.set(cache_key, attempts + 1, 300) # 5 minutes + else: + # No username provided — fall back to IP-based limiting to block + # empty-username probes from the same source. + client_ip = self.get_client_ip(request) + cache_key = f"login_attempts_nouser_{client_ip}" + attempts = cache.get(cache_key, 0) + + if attempts >= failure_limit: + logger.warning( + f"Rate limit exceeded for username-less login attempts from IP {client_ip}" + ) + return HttpResponse( + "Te veel inlogpogingen. Probeer het over 5 minuten opnieuw.", + status=429, + ) - cache.set(cache_key, attempts + 1, 300) # 5 minutes + cache.set(cache_key, attempts + 1, 300) # 5 minutes - # Rate limit registration attempts + # Rate limit registration attempts by IP (registration has no username yet) if request.path == "/users/register/" and request.method == "POST": + client_ip = self.get_client_ip(request) cache_key = f"register_attempts_{client_ip}" attempts = cache.get(cache_key, 0) diff --git a/web/rap_web/settings.py b/web/rap_web/settings.py index ae09452..6a1b518 100644 --- a/web/rap_web/settings.py +++ b/web/rap_web/settings.py @@ -121,6 +121,7 @@ def load_file_secret(file_path: str | None) -> str | None: "django.middleware.security.SecurityMiddleware", "whitenoise.middleware.WhiteNoiseMiddleware", # Static files security "django.contrib.sessions.middleware.SessionMiddleware", + "axes.middleware.AxesMiddleware", # Account lockout (must be after SessionMiddleware) "django.middleware.common.CommonMiddleware", "django.middleware.csrf.CsrfViewMiddleware", "django.contrib.auth.middleware.AuthenticationMiddleware", # Must be before custom middleware that uses request.user @@ -133,8 +134,6 @@ def load_file_secret(file_path: str | None) -> str | None: "rap_web.middleware.LoginRequiredMiddleware", "django.contrib.messages.middleware.MessageMiddleware", "django.middleware.clickjacking.XFrameOptionsMiddleware", - # Will be enabled when packages are installed: - "axes.middleware.AxesMiddleware", # Account lockout "corsheaders.middleware.CorsMiddleware", # CORS "csp.middleware.CSPMiddleware", # Content Security Policy ] @@ -405,6 +404,19 @@ def load_file_secret(file_path: str | None) -> str | None: "django.contrib.auth.backends.ModelBackend", # Default Django backend ] +# ============================================================================= +# ACCOUNT LOCKOUT CONFIGURATION (django-axes) +# ============================================================================= +# Lock accounts by username only — NOT by IP address. +# Locking by IP causes all users behind the same proxy/load balancer to be +# locked out simultaneously when any one user exceeds the failure limit. +AXES_ENABLED = True +AXES_LOCKOUT_PARAMETERS = ["username"] # Username-only lockout (not IP-based) +AXES_FAILURE_LIMIT = 5 # Lock after 5 failed attempts +AXES_COOLOFF_TIME = 1 # Lockout duration in hours +AXES_RESET_ON_SUCCESS = True # Clear failed attempts on successful login +AXES_HANDLER = "axes.handlers.database.AxesDatabaseHandler" + # ============================================================================= # CELERY CONFIGURATION # ============================================================================= diff --git a/web/users/tests.py b/web/users/tests.py index f8d1a3f..8c53b8c 100644 --- a/web/users/tests.py +++ b/web/users/tests.py @@ -1,4 +1,4 @@ -from django.test import TestCase, Client +from django.test import TestCase, Client, override_settings from django.urls import reverse from django.contrib.auth import get_user_model from django.core.files.uploadedfile import SimpleUploadedFile @@ -304,3 +304,104 @@ def test_regular_player_cannot_access_invaller_matches(self): # Should be redirected when trying to access invaller matches response = self.client.get(reverse('events:invaller_matches')) self.assertEqual(response.status_code, 302) # Redirect + + +# Settings overrides to enable django-axes for lockout tests +_AXES_TEST_MIDDLEWARE = [ + 'django.middleware.security.SecurityMiddleware', + 'whitenoise.middleware.WhiteNoiseMiddleware', + 'django.contrib.sessions.middleware.SessionMiddleware', + 'axes.middleware.AxesMiddleware', + 'django.middleware.common.CommonMiddleware', + 'django.middleware.csrf.CsrfViewMiddleware', + 'django.contrib.auth.middleware.AuthenticationMiddleware', + 'django.contrib.messages.middleware.MessageMiddleware', + 'django.middleware.clickjacking.XFrameOptionsMiddleware', +] +_AXES_TEST_AUTH_BACKENDS = [ + 'axes.backends.AxesStandaloneBackend', + 'django.contrib.auth.backends.ModelBackend', +] + + +@override_settings( + MIDDLEWARE=_AXES_TEST_MIDDLEWARE, + AUTHENTICATION_BACKENDS=_AXES_TEST_AUTH_BACKENDS, + AXES_ENABLED=True, + AXES_LOCKOUT_PARAMETERS=["username"], + AXES_FAILURE_LIMIT=3, + AXES_COOLOFF_TIME=1, + AXES_RESET_ON_SUCCESS=True, + AXES_HANDLER="axes.handlers.database.AxesDatabaseHandler", +) +class AccountLockoutTestCase(TestCase): + """ + Tests to verify account lockout is per-username and that one user's + failed login attempts do not lock out other users (proxy/IP isolation). + """ + + def setUp(self): + from django.core.cache import cache as _cache + _cache.clear() + self.client = Client() + self.user1 = User.objects.create_user( + username='lockout_user1', + email='user1@example.com', + password='correctpass123!', + is_active=True, + ) + self.user2 = User.objects.create_user( + username='lockout_user2', + email='user2@example.com', + password='correctpass456!', + is_active=True, + ) + + def _post_login(self, username, password, ip='127.0.0.1'): + return self.client.post( + reverse('users:login'), + {'username': username, 'password': password}, + REMOTE_ADDR=ip, + ) + + def test_failed_logins_lock_only_the_targeted_user(self): + """ + Failing to log in for user1 should not lock out user2, + even when requests come from the same IP address. + """ + shared_ip = '10.0.0.1' + + # Exhaust the failure limit for user1 from shared IP + for _ in range(3): + self._post_login('lockout_user1', 'wrongpassword', ip=shared_ip) + + # user1 should now be locked out (axes returns 429 Too Many Requests) + response = self._post_login('lockout_user1', 'correctpass123!', ip=shared_ip) + self.assertEqual(response.status_code, 429, + "user1 should be locked out after exceeding failure limit") + + # user2 should still be able to log in from the same IP + response = self._post_login('lockout_user2', 'correctpass456!', ip=shared_ip) + self.assertEqual(response.status_code, 302, + "user2 should NOT be locked out by user1's failed attempts") + + def test_lockout_is_per_user_not_per_ip(self): + """Each user has an independent lockout counter.""" + # Fail login 2 times for user1 (below limit) + for _ in range(2): + self._post_login('lockout_user1', 'wrong', ip='192.168.1.1') + + # user2 should still be able to log in + response = self._post_login('lockout_user2', 'correctpass456!', ip='192.168.1.1') + self.assertEqual(response.status_code, 302, + "user2 should be unaffected by user1's partial failure count") + + def test_user_locked_after_failure_limit(self): + """A user is locked after exceeding AXES_FAILURE_LIMIT failed attempts.""" + for _ in range(3): + self._post_login('lockout_user1', 'badpassword') + + # Correct password but account is locked (axes returns 429 Too Many Requests) + response = self._post_login('lockout_user1', 'correctpass123!') + self.assertEqual(response.status_code, 429, + "User should be locked after exceeding failure limit") diff --git a/web/users/views.py b/web/users/views.py index 82a8cbf..e48c1b5 100644 --- a/web/users/views.py +++ b/web/users/views.py @@ -50,7 +50,7 @@ def register(request: HttpRequest): return render(request, "users/register.html", {"form": form}) -@ratelimit(key="ip", rate="10/m", method="POST", block=True) +@ratelimit(key="post:username", rate="6/m", method="POST", block=True) @require_http_methods(["GET", "POST"]) @csrf_protect @never_cache