mirror of
https://github.com/retspen/webvirtcloud
synced 2026-11-11 12:46:49 +00:00
fix(accounts): a username longer than the field is refused by the login form
Django's login form gives the username its max_length after the field is built, so a longer username was not refused and reached authentication. django-axes then stored it in its 255-character column, which PostgreSQL refuses: a login with a 300-character username was a 500 (SQLite does not enforce the length). Both login forms, with and without OTP, now check the length, so such a username is a form error and never reaches authentication, the lockout or the Logs page. This also turns the PostgreSQL leg of CI green again: the S-16 test from #167 hit exactly this.
This commit is contained in:
parent
6c080aa4a3
commit
3210ef2eee
5 changed files with 36 additions and 5 deletions
|
|
@ -1,5 +1,7 @@
|
||||||
from appsettings.settings import app_settings
|
from appsettings.settings import app_settings
|
||||||
from django.contrib.auth import get_user_model
|
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.forms import EmailField, Form, ModelForm, ValidationError
|
||||||
from django.utils.translation import gettext_lazy as _
|
from django.utils.translation import gettext_lazy as _
|
||||||
|
|
||||||
|
|
@ -74,3 +76,18 @@ class UserSSHKeyForm(ModelForm):
|
||||||
|
|
||||||
class EmailOTPForm(Form):
|
class EmailOTPForm(Form):
|
||||||
email = EmailField(label=_("Email"))
|
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
|
||||||
|
|
|
||||||
|
|
@ -15,8 +15,10 @@ from django.core.exceptions import ValidationError
|
||||||
from django_otp.forms import OTPAuthenticationForm
|
from django_otp.forms import OTPAuthenticationForm
|
||||||
from django_otp.plugins.otp_totp.models import TOTPDevice
|
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):
|
def _confirmed_totp(self, user, for_verify=False):
|
||||||
"""The user's oldest confirmed TOTP device, or None. With for_verify
|
"""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
|
the row is locked (select_for_update) so the token's replay and
|
||||||
|
|
|
||||||
|
|
@ -167,6 +167,12 @@ class OTPLoginHTTPTestCase(TestCase):
|
||||||
self._post(otp_token=token_for(device))
|
self._post(otp_token=token_for(device))
|
||||||
self.assertEqual(self.messages(), ["Login"])
|
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):
|
def test_a_signed_in_user_is_sent_on_from_the_login_page(self):
|
||||||
from django.urls import reverse
|
from django.urls import reverse
|
||||||
self.client.force_login(self.user)
|
self.client.force_login(self.user)
|
||||||
|
|
|
||||||
|
|
@ -3,6 +3,7 @@ from django.contrib.auth.views import LoginView, LogoutView
|
||||||
from django.urls import path
|
from django.urls import path
|
||||||
|
|
||||||
from . import views
|
from . import views
|
||||||
|
from .forms import LoginForm
|
||||||
from .otp import SingleDeviceOTPForm
|
from .otp import SingleDeviceOTPForm
|
||||||
|
|
||||||
app_name = "accounts"
|
app_name = "accounts"
|
||||||
|
|
@ -51,5 +52,6 @@ if settings.OTP_ENABLED:
|
||||||
]
|
]
|
||||||
else:
|
else:
|
||||||
urlpatterns += (
|
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"),
|
||||||
)
|
)
|
||||||
|
|
|
||||||
|
|
@ -58,9 +58,13 @@ class LoginRowInputTests(TestCase):
|
||||||
def login(self, username):
|
def login(self, username):
|
||||||
return self.client.post(reverse("accounts:login"), {"username": username, "password": "wrong"})
|
return self.client.post(reverse("accounts:login"), {"username": username, "password": "wrong"})
|
||||||
|
|
||||||
def test_an_oversized_username_is_cut_to_the_field(self):
|
def test_an_oversized_username_is_refused_by_the_form(self):
|
||||||
self.assertEqual(self.login("x" * 300).status_code, 200)
|
# longer than the username field: no login attempt, so nothing for
|
||||||
self.assertEqual(rows(), [("x" * 150, "-", "Login failed", False, "127.0.0.1")])
|
# 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):
|
def test_a_crlf_username_stays_in_its_row(self):
|
||||||
forged = "bob\r\nadmin - Login"
|
forged = "bob\r\nadmin - Login"
|
||||||
|
|
|
||||||
Loading…
Add table
Add a link
Reference in a new issue