Refactorise le login

* utilise la LoginView de Django
* change le backend standard pour afficher une erreur spécifique pour les comptes inactifs (nécessaire en utilisant la LoginView de Django)
* simplifie en fusionnant les erreurs pour mauvais nom d'utilisateur ou mot de passe
* conserve le comportement de redirection originel
* ne teste plus le formulaire seul (tout est couvert par les tests de la vue)
* corrige des tests annexes qui ne testaient pas la bonne chose

zds/member/forms.py

@@ -1,7 +1,9 @@
 from django import forms
 from django.conf import settings
 from django.contrib.auth import authenticate
+from django.contrib.auth.forms import AuthenticationForm
 from django.contrib.auth.models import User, Group
+from django.core.exceptions import ValidationError
 from django.urls import reverse
 from django.utils.translation import gettext_lazy as _
 
@@ -29,45 +31,58 @@
 MIN_PASSWORD_LENGTH = 6
 
 
-class LoginForm(forms.Form):
-    """
-    The login form, including the "remember me" checkbox.
-    """
-
-    username = forms.CharField(
-        label=_("Nom d'utilisateur"),
-        max_length=User._meta.get_field("username").max_length,
-        required=True,
-        widget=forms.TextInput(attrs={"autofocus": ""}),
-    )
-
-    password = forms.CharField(
-        label=_("Mot de passe"),
-        required=True,
-        widget=forms.PasswordInput,
-    )
-
+class LoginForm(AuthenticationForm):
     remember = forms.BooleanField(
         label=_("Se souvenir de moi"),
         initial=True,
         required=False,
     )
 
-    def __init__(self, *args, **kwargs):
-        super().__init__(*args, **kwargs)
-        self.helper = FormHelper()
-        self.helper.form_action = reverse("member-login")
-        self.helper.form_method = "post"
-        self.helper.form_class = "content-wrapper"
-
-        self.helper.layout = Layout(
+    error_messages = {
+        "invalid_login": _(
+            "Merci de saisir un nom d'utilisateur et un mot de passe corrects. Faites attention aux majuscules et "
+            "minuscules !"
+        ),
+        "inactive": _(
+            "Vous n’avez pas encore activé votre compte, vous devez le faire pour pouvoir vous connecter sur le site."
+            " Regardez dans vos mails : %(email)s."
+        ),
+        "banned": _("Vous n’êtes pas autorisé à vous connecter sur le site, vous avez été banni par un modérateur."),
+    }
+
+    def __init__(self, request=None, *args, **kwargs):
+        super().__init__(request, *args, **kwargs)
+        self.helper = self.get_helper()
+        # Errors are displayed using info bars (see LoginView) instead of the form built-in error rendering
+        self.helper.form_show_errors = False
+
+    def get_helper(self):
+        """Return the FormHelper expected by cripsy."""
+        helper = FormHelper()
+        helper.form_action = reverse("member-login")
+        helper.form_method = "post"
+        helper.form_class = "content-wrapper"
+        helper.layout = Layout(
             Field("username"),
             Field("password"),
             Field("remember"),
             ButtonHolder(
                 StrictButton(_("Se connecter"), type="submit"),
             ),
         )
+        return helper
+
+    def confirm_login_allowed(self, user):
+        """Override the parent method to change the error for inactive users and prevent login of banned users."""
+        if not user.is_active:
+            raise ValidationError(
+                self.error_messages["inactive"], code="inactive", params={"email": self.user_cache.email}
+            )
+        elif not user.profile.can_read:
+            raise ValidationError(
+                self.error_messages["banned"],
+                code="banned",
+            )
 
 
 class RegisterForm(forms.Form):

zds/member/tests/tests_forms.py

@@ -2,7 +2,6 @@
 
 from zds.member.tests.factories import ProfileFactory, NonAsciiProfileFactory
 from zds.member.forms import (
-    LoginForm,
     RegisterForm,
     MiniProfileForm,
     ProfileForm,
@@ -29,30 +28,6 @@
     stringof2001chars += "0123456789"
 stringof2001chars += "12.jpg"
 
-# This form is tricky to test as it needs a tuto to be done
-# class OldTutoFormTest(TestCase):
-
-
-class LoginFormTest(TestCase):
-    """
-    Check the form to login.
-    """
-
-    def test_valid_login_form(self):
-        data = {"username": "Tester", "password": "hostel77", "remember": True}
-        form = LoginForm(data=data)
-        self.assertTrue(form.is_valid())
-
-    def test_missing_username_form(self):
-        data = {"username": "", "password": "hostel77", "remember": True}
-        form = LoginForm(data=data)
-        self.assertFalse(form.is_valid())
-
-    def test_missing_password_form(self):
-        data = {"username": "Tester", "password": "", "remember": True}
-        form = LoginForm(data=data)
-        self.assertFalse(form.is_valid())
-
 
 class RegisterFormTest(TestCase):
     """

zds/member/tests/views/tests_admin.py

@@ -1,5 +1,5 @@
 from django.conf import settings
-from django.contrib.auth.models import Group
+from django.contrib.auth.models import Group, User
 from django.urls import reverse
 from django.test import TestCase
 
@@ -55,9 +55,8 @@ def test_promote_interface(self):
 
         # LET THE TEST BEGIN !
 
-        # tester shouldn't be able to connect
-        login_check = self.client.login(username=tester.user.username, password="hostel77")
-        self.assertEqual(login_check, False)
+        # At this point, the user cannot log in because the account is inactive.
+        # This behavior is tested in the tests of LoginView.
 
         # connect as staff (superuser)
         self.client.force_login(staff.user)
@@ -87,7 +86,7 @@ def test_promote_interface(self):
 
         self.assertEqual(len(TopicAnswerSubscription.objects.get_objects_followed_by(tester.user)), 3)
 
-        # retract all right, keep one group only and activate account
+        # retract all rights, keep one group only and activate account
         result = self.client.post(
             reverse("member-settings-promote", kwargs={"user_pk": tester.user.id}),
             {"groups": [group.id], "activation": "on"},
@@ -108,9 +107,10 @@ def test_promote_interface(self):
         tester = Profile.objects.get(id=tester.id)  # refresh
         self.assertEqual(len(TopicAnswerSubscription.objects.get_objects_followed_by(tester.user)), 1)
 
-        # Finally, check that user can connect and can not access the interface
-        login_check = self.client.login(username=tester.user.username, password="hostel77")
-        self.assertEqual(login_check, True)
+        # Finally, check that the user cannot access the interface.
+        tester_from_db = User.objects.get(username=tester.user.username)
+        self.assertTrue(tester_from_db.is_active)
+        self.client.force_login(tester.user)
         result = self.client.post(
             reverse("member-settings-promote", kwargs={"user_pk": staff.user.id}), {"activation": "on"}, follow=False
         )

zds/member/tests/views/tests_login.py

@@ -1,10 +1,13 @@
 from django.urls import reverse
 from django.test import TestCase
-from django.utils.translation import gettext_lazy as _
 from django.utils.html import escape
 
+from zds.member.forms import LoginForm
 from zds.member.tests.factories import ProfileFactory, NonAsciiProfileFactory
 
+# TODO: test correct update of IP
+# TODO: test session expiration with/without "remember me"
+
 
 class LoginTests(TestCase):
     def setUp(self):
@@ -43,6 +46,22 @@ def test_nonascii(self):
         )
         self.assertEqual(result.status_code, 200)
 
+    def test_empty_username_or_password(self):
+        """
+        Error case: bad username, password not relevant.
+        Expected: cannot log in, errors associated to empty username and password.
+        """
+        result = self.client.post(
+            self.login_url,
+            {
+                "username": "",
+                "password": "",
+                "remember": "remember",
+            },
+            follow=False,
+        )
+        self.assertContains(result, escape("Ce champ est obligatoire"), count=2)
+
     def test_bad_username(self):
         """
         Error case: bad username, password not relevant.
@@ -57,7 +76,7 @@ def test_bad_username(self):
             },
             follow=False,
         )
-        self.assertContains(result, _("Ce nom d’utilisateur est inconnu."))
+        self.assertContains(result, escape(LoginForm.error_messages["invalid_login"]))
 
     def test_inactive_account(self):
         """
@@ -76,7 +95,7 @@ def test_inactive_account(self):
             },
             follow=False,
         )
-        self.assertContains(result, escape(_("Vous n'avez pas encore activé votre compte")))
+        self.assertContains(result, escape(LoginForm.error_messages["inactive"][:20]))
 
     def test_correct_username_bad_password(self):
         """
@@ -92,7 +111,7 @@ def test_correct_username_bad_password(self):
             },
             follow=False,
         )
-        self.assertContains(result, _("Le mot de passe saisi est incorrect. "))
+        self.assertContains(result, escape(LoginForm.error_messages["invalid_login"]))
 
     def test_banned_user(self):
         """
@@ -113,7 +132,7 @@ def test_banned_user(self):
             },
             follow=False,
         )
-        self.assertContains(result, escape(_("Vous n'êtes pas autorisé à vous connecter")))
+        self.assertContains(result, escape(LoginForm.error_messages["banned"]))
 
     def test_redirection_good_target(self):
         """Nominal case: redirection to an existing page with the parameter 'next'."""
@@ -143,7 +162,7 @@ def test_redirection_bad_target(self):
 
     def test_redirection_loop_avoidance(self):
         """
-        Case failing gracefully: redirection to homeage when 'next' risk creating a redirection loop.
+        Case failing gracefully: redirection to homepage when 'next' risks creating a redirection loop.
         """
         result = self.client.post(
             self.login_url + "?next=" + self.login_url,

zds/member/urls.py

@@ -17,7 +17,7 @@
     member_from_ip,
     modify_profile,
 )
-from zds.member.views.login import login_view
+from zds.member.views.login import LoginView
 from zds.member.views.hats import (
     HatsSettings,
     RequestedHatsList,
@@ -103,7 +103,7 @@
     re_path(r"^casquettes/ajouter/(?P<user_pk>\d+)/$", add_hat, name="add-hat"),
     re_path(r"^casquettes/retirer/(?P<user_pk>\d+)/(?P<hat_pk>\d+)/$", remove_hat, name="remove-hat"),
     # membership
-    re_path(r"^connexion/$", login_view, name="member-login"),
+    re_path(r"^connexion/$", LoginView.as_view(), name="member-login"),
     re_path(r"^deconnexion/$", LogoutView.as_view(), name="member-logout"),
     re_path(r"^inscription/$", RegisterView.as_view(), name="register-member"),
     re_path(r"^reinitialisation/$", forgot_password, name="member-forgot-password"),