Send an email when creating an account via POST /register

core/templates/core/register.jinja

@@ -15,18 +15,11 @@
 {% block content %}
     <h1 class="title">{% trans %}Register{% endtrans %}</h1>
 
-    {% if user_registered %}
+    <form action="{{ url('core:register') }}" method="post">
-        {% trans user_name=user_registered.get_display_name() %}Welcome {{ user_name }}!{% endtrans %}<br>
+        {% csrf_token %}
-        {% trans %}You successfully registered and you will soon receive a confirmation mail.{% endtrans %}<br>
+        {% render_honeypot_field %}
-        {% trans username=user_registered.username %}Your username is {{ username }}.{% endtrans %}<br>
+        {{ form }}
-    
+        <input type="submit" value="{% trans %}Register{% endtrans %}" />
-    {% else %}
+    </form>
-        <form action="{{ url('core:register') }}" method="post">
-            {% csrf_token %}
-            {% render_honeypot_field %}
-            {{ form }}
-            <input type="submit" value="{% trans %}Register{% endtrans %}" />
-        </form>
-    {% endif %}
 {% endblock %}
  

core/templates/core/register_confirm_mail.jinja

@@ -0,0 +1,17 @@
+{% autoescape off %}
+{% trans %}You're receiving this email because you created an account on the AE website.{% endtrans %}
+
+{% trans %}Your username, in case it was not given to you: {% endtrans %} {{ username }}
+
+{% trans %}
+As this is the website of the students of the AE, by the students of the AE,
+for the students of the AE, you won't be able to do many things without subscribing to the AE.
+To make a contribution, contact a member of the association's board, either directly or by email at ae@utbm.fr.
+{% endtrans %}
+
+{% trans %}Wishing you a good experience among us! {% endtrans %}
+
+{% trans %}The AE team{% endtrans %}
+
+{% endautoescape %}
+

core/tests.py

@@ -15,11 +15,14 @@
 
 from datetime import date, timedelta
 from pathlib import Path
+from smtplib import SMTPException
 
 import freezegun
 import pytest
+from django.core import mail
 from django.core.cache import cache
-from django.test import TestCase
+from django.core.mail import EmailMessage
+from django.test import Client, TestCase
 from django.urls import reverse
 from django.utils.timezone import now
 from pytest_django.asserts import assertInHTML, assertRedirects
@@ -48,26 +51,35 @@ class TestUserRegistration:
 
     def test_register_user_form_ok(self, client, valid_payload):
         """Should register a user correctly."""
+        assert not User.objects.filter(email=valid_payload["email"]).exists()
         response = client.post(reverse("core:register"), valid_payload)
-        assert response.status_code == 200
+        assertRedirects(response, reverse("core:index"))
-        assert "TEST_REGISTER_USER_FORM_OK" in str(response.content)
+        assert len(mail.outbox) == 1
+        assert mail.outbox[0].subject == "Création de votre compte AE"
+        assert User.objects.filter(email=valid_payload["email"]).exists()
 
     @pytest.mark.parametrize(
-        "payload_edit",
+        ("payload_edit", "expected_error"),
         [
-            {"password2": "not the same as password1"},
+            (
-            {"email": "not-an-email"},
+                {"password2": "not the same as password1"},
-            {"first_name": ""},
+                "Les deux mots de passe ne correspondent pas.",
-            {"last_name": ""},
+            ),
-            {"captcha_1": "WRONG_CAPTCHA"},
+            ({"email": "not-an-email"}, "Saisissez une adresse e-mail valide."),
+            ({"first_name": ""}, "Ce champ est obligatoire."),
+            ({"last_name": ""}, "Ce champ est obligatoire."),
+            ({"captcha_1": "WRONG_CAPTCHA"}, "CAPTCHA invalide"),
         ],
     )
-    def test_register_user_form_fail(self, client, valid_payload, payload_edit):
+    def test_register_user_form_fail(
+        self, client, valid_payload, payload_edit, expected_error
+    ):
         """Should not register a user correctly."""
         payload = valid_payload | payload_edit
         response = client.post(reverse("core:register"), payload)
         assert response.status_code == 200
-        assert "TEST_REGISTER_USER_FORM_FAIL" in str(response.content)
+        error_html = f'<ul class="errorlist"><li>{expected_error}</li></ul>'
+        assertInHTML(error_html, str(response.content.decode()))
 
     def test_register_honeypot_fail(self, client, valid_payload):
         payload = valid_payload | {
@@ -76,13 +88,34 @@ class TestUserRegistration:
         response = client.post(reverse("core:register"), payload)
         assert response.status_code == 400
 
-    def test_register_user_form_fail_already_exists(self, client, valid_payload):
+    def test_register_user_form_fail_already_exists(
+        self, client: Client, valid_payload
+    ):
         """Should not register a user correctly if it already exists."""
         # create the user, then try to create it again
         client.post(reverse("core:register"), valid_payload)
         response = client.post(reverse("core:register"), valid_payload)
+
         assert response.status_code == 200
-        assert "TEST_REGISTER_USER_FORM_FAIL" in str(response.content)
+        error_html = "<li>Un objet User avec ce champ Adresse email existe déjà.</li>"
+        assertInHTML(error_html, str(response.content.decode()))
+
+    def test_register_fail_with_not_existing_email(
+        self, client: Client, valid_payload, monkeypatch
+    ):
+        """Test that, when email is valid but doesn't actually exist, registration fails"""
+
+        def always_fail(*_args, **_kwargs):
+            raise SMTPException
+
+        monkeypatch.setattr(EmailMessage, "send", always_fail)
+
+        response = client.post(reverse("core:register"), valid_payload)
+        assert response.status_code == 200
+        error_html = (
+            "<li>Nous n'avons pas réussi à vérifier que cette adresse mail existe.</li>"
+        )
+        assertInHTML(error_html, str(response.content.decode()))
 
 
 @pytest.mark.django_db

core/urls.py

@@ -75,7 +75,7 @@ urlpatterns = [
         SithPasswordResetCompleteView.as_view(),
         name="password_reset_complete",
     ),
-    path("register/", register, name="register"),
+    path("register/", UserCreationView.as_view(), name="register"),
     # Group handling
     path("group/", GroupListView.as_view(), name="group_list"),
     path("group/new/", GroupCreateView.as_view(), name="group_new"),

core/views/forms.py

@@ -194,14 +194,6 @@ class RegisteringForm(UserCreationForm):
         model = User
         fields = ("first_name", "last_name", "email")
 
-    def save(self, *, commit=True):
-        user = super().save(commit=False)
-        user.set_password(self.cleaned_data["password1"])
-        user.generate_username()
-        if commit:
-            user.save()
-        return user
-
 
 class UserProfileForm(forms.ModelForm):
     """

core/views/user.py

@@ -23,17 +23,18 @@
 #
 
 # This file contains all the views that concern the user model
-import logging
 from datetime import date, timedelta
+from smtplib import SMTPException
 
 from django.conf import settings
-from django.contrib.auth import views
+from django.contrib.auth import login, views
 from django.contrib.auth.forms import PasswordChangeForm
 from django.core.exceptions import PermissionDenied, ValidationError
 from django.forms import CheckboxSelectMultiple
 from django.forms.models import modelform_factory
 from django.http import Http404, HttpResponse
-from django.shortcuts import get_object_or_404, redirect, render
+from django.shortcuts import get_object_or_404, redirect
+from django.template.loader import render_to_string
 from django.template.response import TemplateResponse
 from django.urls import reverse, reverse_lazy
 from django.utils.decorators import method_decorator
@@ -46,7 +47,7 @@ from django.views.generic import (
     TemplateView,
 )
 from django.views.generic.dates import MonthMixin, YearMixin
-from django.views.generic.edit import UpdateView
+from django.views.generic.edit import FormView, UpdateView
 from honeypot.decorators import check_honeypot
 
 from api.views.sas import all_pictures_of_user
@@ -80,6 +81,7 @@ class SithLoginView(views.LoginView):
     template_name = "core/login.jinja"
     authentication_form = LoginForm
     form_class = PasswordChangeForm
+    redirect_authenticated_user = True
 
 
 class SithPasswordChangeView(views.PasswordChangeView):
@@ -163,28 +165,41 @@ class SithPasswordResetCompleteView(views.PasswordResetCompleteView):
     template_name = "core/password_reset_complete.jinja"
 
 
-@check_honeypot
+@method_decorator(check_honeypot, name="post")
-def register(request):
+class UserCreationView(FormView):
-    context = {}
+    success_url = reverse_lazy("core:index")
-    if request.method == "POST":
+    form_class = RegisteringForm
-        form = RegisteringForm(request.POST)
+    template_name = "core/register.jinja"
-        if form.is_valid():
+
-            logging.debug(
+    def form_valid(self, form):
-                "Registering "
+        # Just knowing that the user gave sound data isn't enough,
-                + form.cleaned_data["first_name"]
+        # we must also know if the given email actually exists.
-                + form.cleaned_data["last_name"]
+        # This step must happen after the whole validation has been made,
+        # but before saving the user, while being tightly coupled
+        # to the request/response cycle.
+        # Thus this is here.
+        user: User = form.save(commit=False)
+        username = user.generate_username()
+        try:
+            user.email_user(
+                "Création de votre compte AE",
+                render_to_string(
+                    "core/register_confirm_mail.jinja", context={"username": username}
+                ),
             )
-            u = form.save()
+        except SMTPException:
-            context["user_registered"] = u
+            # if the email couldn't be sent, it's likely to be
-            context["tests"] = "TEST_REGISTER_USER_FORM_OK"
+            # that the given email doesn't exist (which means it's either a typo or a bot).
-            form = RegisteringForm()
+            # It may also be a genuine bug, but that's less likely to happen
-        else:
+            # and wouldn't be critical as the favoured way to create an account
-            context["error"] = "Erreur"
+            # is to contact an AE board member
-            context["tests"] = "TEST_REGISTER_USER_FORM_FAIL"
+            form.add_error(
-    else:
+                "email", _("We couldn't verify that this email actually exists")
-        form = RegisteringForm()
+            )
-    context["form"] = form.as_p()
+            return super().form_invalid(form)
-    return render(request, "core/register.jinja", context)
+        user = form.save()
+        login(self.request, user)
+        return super().form_valid(form)
 
 
 class UserTabsMixin(TabedViewMixin):