diff --git a/accounts/forms.py b/accounts/forms.py index f9ed733d..f458a03a 100644 --- a/accounts/forms.py +++ b/accounts/forms.py @@ -1,5 +1,7 @@ from appsettings.settings import app_settings from django.contrib.auth import get_user_model +from django.contrib.auth.forms import AuthenticationForm +from django.core.validators import MaxLengthValidator from django.forms import EmailField, Form, ModelForm, ValidationError from django.utils.translation import gettext_lazy as _ @@ -74,3 +76,18 @@ class UserSSHKeyForm(ModelForm): class EmailOTPForm(Form): email = EmailField(label=_("Email")) + + +class BoundedUsernameMixin: + """Django's login form gives the username its max_length after the field + is built, so a longer one is not refused and reaches authentication; + django-axes then cannot store it (PostgreSQL refuses it: 500).""" + + def __init__(self, *args, **kwargs): + super().__init__(*args, **kwargs) + username = self.fields["username"] + username.validators.append(MaxLengthValidator(username.max_length)) + + +class LoginForm(BoundedUsernameMixin, AuthenticationForm): + pass diff --git a/accounts/otp.py b/accounts/otp.py index 34ae3f35..fc95da79 100644 --- a/accounts/otp.py +++ b/accounts/otp.py @@ -15,8 +15,10 @@ from django.core.exceptions import ValidationError from django_otp.forms import OTPAuthenticationForm from django_otp.plugins.otp_totp.models import TOTPDevice +from accounts.forms import BoundedUsernameMixin -class SingleDeviceOTPForm(OTPAuthenticationForm): + +class SingleDeviceOTPForm(BoundedUsernameMixin, OTPAuthenticationForm): def _confirmed_totp(self, user, for_verify=False): """The user's oldest confirmed TOTP device, or None. With for_verify the row is locked (select_for_update) so the token's replay and diff --git a/accounts/tests/test_otp_login.py b/accounts/tests/test_otp_login.py index 7ec48bf9..540c5f59 100644 --- a/accounts/tests/test_otp_login.py +++ b/accounts/tests/test_otp_login.py @@ -167,6 +167,12 @@ class OTPLoginHTTPTestCase(TestCase): self._post(otp_token=token_for(device)) self.assertEqual(self.messages(), ["Login"]) + def test_an_oversized_username_is_refused_by_the_form(self): + response = self._post(username="x" * 300, otp_token="") + self.assertEqual(response.status_code, 200) + self.assertIn("username", response.context["form"].errors) + self.assertEqual(self.messages(), []) + def test_a_signed_in_user_is_sent_on_from_the_login_page(self): from django.urls import reverse self.client.force_login(self.user) diff --git a/accounts/urls.py b/accounts/urls.py index 55024737..59d04163 100644 --- a/accounts/urls.py +++ b/accounts/urls.py @@ -3,6 +3,7 @@ from django.contrib.auth.views import LoginView, LogoutView from django.urls import path from . import views +from .forms import LoginForm from .otp import SingleDeviceOTPForm app_name = "accounts" @@ -51,5 +52,6 @@ if settings.OTP_ENABLED: ] else: urlpatterns += ( - path("login/", LoginView.as_view(template_name="login.html", redirect_authenticated_user=True), name="login"), + path("login/", LoginView.as_view(template_name="login.html", authentication_form=LoginForm, + redirect_authenticated_user=True), name="login"), ) diff --git a/logs/tests/test_login_rows.py b/logs/tests/test_login_rows.py index 7958c387..91c9c8df 100644 --- a/logs/tests/test_login_rows.py +++ b/logs/tests/test_login_rows.py @@ -58,9 +58,13 @@ class LoginRowInputTests(TestCase): def login(self, username): return self.client.post(reverse("accounts:login"), {"username": username, "password": "wrong"}) - def test_an_oversized_username_is_cut_to_the_field(self): - self.assertEqual(self.login("x" * 300).status_code, 200) - self.assertEqual(rows(), [("x" * 150, "-", "Login failed", False, "127.0.0.1")]) + def test_an_oversized_username_is_refused_by_the_form(self): + # longer than the username field: no login attempt, so nothing for + # django-axes to store (its 255-character column refused it on PostgreSQL) + response = self.login("x" * 300) + self.assertEqual(response.status_code, 200) + self.assertIn("username", response.context["form"].errors) + self.assertFalse(Logs.objects.exists()) def test_a_crlf_username_stays_in_its_row(self): forged = "bob\r\nadmin - Login"