Skip to content

Commit

Permalink
Refactorise le login
Browse files Browse the repository at this point in the history
* 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
  • Loading branch information
Arnaud-D committed Apr 3, 2022
1 parent 3580e4b commit 054137f
Show file tree
Hide file tree
Showing 5 changed files with 87 additions and 122 deletions.
67 changes: 41 additions & 26 deletions zds/member/forms.py
Original file line number Diff line number Diff line change
@@ -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 _

Expand Down Expand Up @@ -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):
Expand Down
15 changes: 9 additions & 6 deletions zds/member/tests/views/tests_login.py
Original file line number Diff line number Diff line change
@@ -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):
Expand Down Expand Up @@ -57,7 +60,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):
"""
Expand All @@ -76,7 +79,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):
"""
Expand All @@ -92,7 +95,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):
"""
Expand All @@ -113,7 +116,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'."""
Expand Down Expand Up @@ -143,7 +146,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,
Expand Down
4 changes: 2 additions & 2 deletions zds/member/urls.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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"),
Expand Down
120 changes: 34 additions & 86 deletions zds/member/views/login.py
Original file line number Diff line number Diff line change
@@ -1,96 +1,44 @@
from django.conf import settings
from django.contrib import messages
from django.contrib.auth import authenticate, login
from django.contrib.auth.models import User
from django.template.context_processors import csrf
from django.urls import reverse, resolve, Resolver404, NoReverseMatch
from django.shortcuts import redirect, render, get_object_or_404
from django.utils.translation import gettext_lazy as _
from django.contrib.auth.views import LoginView
from django.urls import reverse, is_valid_path

from zds.member.forms import LoginForm
from zds.member.models import Profile
from zds.member.views import get_client_ip
from zds.utils.tokens import generate_token


def login_view(request):
"""Logs user in."""
next_page = request.GET.get("next", "/")
if next_page in [reverse("member-login"), reverse("register-member"), reverse("member-logout")]:
next_page = "/"
csrf_tk = {"next_page": next_page}
csrf_tk.update(csrf(request))
error = False
class LoginView(LoginView):
form_class = LoginForm
template_name = "member/login.html"

if request.method != "POST":
form = LoginForm()
else:
form = LoginForm(request.POST)
def dispatch(self, request, *args, **kwargs):
self.request = request
return super().dispatch(request, *args, **kwargs)

if form.is_valid():
username = form.cleaned_data["username"]
password = form.cleaned_data["password"]
user = User.objects.filter(username=username).first()
if user is None:
messages.error(
request,
_("Ce nom d’utilisateur est inconnu. Si vous ne possédez pas de compte, vous pouvez vous inscrire."),
)
else:
if not user.is_active:
messages.error(
request,
_(
"Vous n'avez pas encore activé votre compte, "
"vous devez le faire pour pouvoir vous "
"connecter sur le site. Regardez dans vos "
"mails : {}."
).format(user.email),
)
else:
user = authenticate(username=username, password=password)
if user is None:
messages.error(
request,
_(
"Le mot de passe saisi est incorrect. Cliquez sur le lien « Mot de passe oublié ? »"
"si vous ne vous en souvenez plus."
),
)
initial = {"username": username}
def form_invalid(self, form):
# Display errors in error/info bars instead of the form built-in display.
for error in form.errors.values():
messages.error(self.request, error[0])
form.form_show_errors = False
return super().form_invalid(form)

form = LoginForm(initial=initial)
form.helper.form_action += "?next=" + next_page
csrf_tk["error"] = error
csrf_tk["form"] = form
return render(request, "member/login.html", {"form": form, "csrf_tk": csrf_tk})
else:
profile = get_object_or_404(Profile, user=user)
if not profile.can_read_now():
messages.error(
request,
_(
"Vous n'êtes pas autorisé à vous connecter sur le site, vous avez été banni par un "
"modérateur."
),
)
else:
login(request, user)
request.session["get_token"] = generate_token()
if "remember" not in request.POST:
request.session.set_expiry(0)
profile.last_ip_address = get_client_ip(request)
profile.save()
# Redirect the user if needed.
# Set the cookie for Clem smileys.
# (For people switching account or clearing cookies
# after a browser session.)
try:
response = redirect(resolve(next_page).url_name)
except Resolver404:
response = redirect(reverse("homepage"))
return response
def form_valid(self, form):
if "remember" not in self.request.POST:
self.request.session.set_expiry(0)
form.user_cache.profile.last_ip_address = get_client_ip(self.request)
form.user_cache.profile.save()
return super().form_valid(form)

form.helper.form_action += "?next=" + next_page
csrf_tk["error"] = error
csrf_tk["form"] = form
return render(request, "member/login.html", {"form": form, "csrf_tk": csrf_tk})
def get_success_url(self):
"""In case of success, redirect to homepage for some special 'next' targets or non-existing pages."""
url = self.get_redirect_url()
if self.is_special(url) or not is_valid_path(url):
url = settings.LOGIN_REDIRECT_URL
return url

@staticmethod
def is_special(url):
"""Determine if `url` is a special case for redirection."""
names = ["member-login", "register-member", "member-logout"]
urls = [reverse(name) for name in names]
return url in urls
3 changes: 1 addition & 2 deletions zds/settings/abstract_base/django.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
from os.path import join
from urllib.parse import quote

from django.contrib.messages import constants as message_constants
Expand Down Expand Up @@ -313,7 +312,7 @@
AUTHENTICATION_BACKENDS = (
"social_core.backends.facebook.FacebookOAuth2",
"social_core.backends.google.GoogleOAuth2",
"django.contrib.auth.backends.ModelBackend",
"django.contrib.auth.backends.AllowAllUsersModelBackend",
)

# To remove a useless warning in Django 1.7.
Expand Down

0 comments on commit 054137f

Please sign in to comment.