Merge pull request #711 from ae-utbm/bot-filtering

Implement mechanisms to block bots on authentication views

core/templates/core/login.jinja

@@ -33,6 +33,7 @@
         {% endif %}
 
         {% csrf_token %}
+        {% render_honeypot_field %}
 
         <div>
             <label for="{{ form.username.name }}">{{ form.username.label }}</label>

core/templates/core/password_reset.jinja

@@ -3,6 +3,7 @@
 {% block content %}
 <form method="post" action="">
 {% csrf_token %}
+{% render_honeypot_field %}
 {{ form.as_p() }}
 <input type="submit" value="{% trans %}Reset{% endtrans %}" />
 </form>

core/templates/core/register.jinja

@@ -15,17 +15,11 @@
 {% block content %}
     <h1 class="title">{% trans %}Register{% endtrans %}</h1>
 
-    {% if user_registered %}
-        {% trans user_name=user_registered.get_display_name() %}Welcome {{ user_name }}!{% endtrans %}<br>
-        {% trans %}You successfully registered and you will soon receive a confirmation mail.{% endtrans %}<br>
-        {% trans username=user_registered.username %}Your username is {{ username }}.{% endtrans %}<br>
-    
-    {% else %}
     <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/templatetags/extensions.py

@@ -0,0 +1,58 @@
+#
+# Copyright 2024
+# - Sli <antoine@bartuccio.fr>
+#
+# Ce fichier fait partie du site de l'Association des Étudiants de l'UTBM,
+# http://ae.utbm.fr.
+#
+# This program is free software; you can redistribute it and/or modify it under
+# the terms of the GNU General Public License a published by the Free Software
+# Foundation; either version 3 of the License, or (at your option) any later
+# version.
+#
+# This program is distributed in the hope that it will be useful, but WITHOUT
+# ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS
+# FOR A PARTICULAR PURPOSE.  See the GNU General Public License for more
+# details.
+#
+# You should have received a copy of the GNU General Public License along with
+# this program; if not, write to the Free Software Foundation, Inc., 59 Temple
+# Place - Suite 330, Boston, MA 02111-1307, USA.
+#
+#
+from typing import Callable
+
+import honeypot.templatetags.honeypot as honeypot_filters
+from django.template.loader import render_to_string
+from jinja2 import Environment, nodes
+from jinja2.ext import Extension
+from jinja2.parser import Parser
+
+
+class HoneypotExtension(Extension):
+    """
+    Wrapper around the honeypot extension tag
+    Known limitation: doesn't support arguments
+
+    Usage: {% render_honeypot_field %}
+    """
+
+    tags = {"render_honeypot_field"}
+
+    def __init__(self, environment: Environment) -> None:
+        environment.globals["render_honeypot_field"] = (
+            honeypot_filters.render_honeypot_field
+        )
+        self.environment = environment
+
+    def parse(self, parser: Parser) -> nodes.Output:
+        lineno = parser.stream.expect("name:render_honeypot_field").lineno
+        call = self.call_method(
+            "_render",
+            [nodes.Name("render_honeypot_field", "load", lineno=lineno)],
+            lineno=lineno,
+        )
+        return nodes.Output([nodes.MarkSafe(call)])
+
+    def _render(self, render_honeypot_field: Callable[[str | None], str]):
+        return render_to_string("honeypot/honeypot_field.html", render_honeypot_field())

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
@@ -36,6 +39,7 @@ class TestUserRegistration:
     @pytest.fixture()
     def valid_payload(self):
         return {
+            settings.HONEYPOT_FIELD_NAME: settings.HONEYPOT_VALUE,
             "first_name": "this user does not exist (yet)",
             "last_name": "this user does not exist (yet)",
             "email": "i-dont-exist-yet@git.an",
@@ -47,34 +51,73 @@ 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"},
+                "Les deux mots de passe ne correspondent pas.",
-            {"first_name": ""},
+            ),
-            {"last_name": ""},
+            ({"email": "not-an-email"}, "Saisissez une adresse e-mail valide."),
-            {"captcha_1": "WRONG_CAPTCHA"},
+            ({"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()))
+        assert not User.objects.filter(email=payload["email"]).exists()
 
-    def test_register_user_form_fail_already_exists(self, client, valid_payload):
+    def test_register_honeypot_fail(self, client: Client, valid_payload):
+        payload = valid_payload | {
+            settings.HONEYPOT_FIELD_NAME: settings.HONEYPOT_VALUE + "random"
+        }
+        response = client.post(reverse("core:register"), payload)
+        assert response.status_code == 200
+        assert not User.objects.filter(email=payload["email"]).exists()
+
+    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
@@ -90,7 +133,11 @@ class TestUserLogin:
 
         response = client.post(
             reverse("core:login"),
-            {"username": user.username, "password": "wrong-password"},
+            {
+                "username": user.username,
+                "password": "wrong-password",
+                settings.HONEYPOT_FIELD_NAME: settings.HONEYPOT_VALUE,
+            },
         )
         assert response.status_code == 200
         assert (
@@ -98,14 +145,32 @@ class TestUserLogin:
             "et votre mot de passe ne correspondent pas. Merci de réessayer.</p>"
         ) in str(response.content.decode())
 
+    def test_login_honeypot(self, client, user):
+        response = client.post(
+            reverse("core:login"),
+            {
+                "username": user.username,
+                "password": "wrong-password",
+                settings.HONEYPOT_FIELD_NAME: settings.HONEYPOT_VALUE + "incorrect",
+            },
+        )
+        assert response.status_code == 200
+        assert response.wsgi_request.user.is_anonymous
+
     def test_login_success(self, client, user):
         """
         Should login a user correctly
         """
         response = client.post(
-            reverse("core:login"), {"username": user.username, "password": "plop"}
+            reverse("core:login"),
+            {
+                "username": user.username,
+                "password": "plop",
+                settings.HONEYPOT_FIELD_NAME: settings.HONEYPOT_VALUE,
+            },
         )
         assertRedirects(response, reverse("core:index"))
+        assert response.wsgi_request.user == user
 
 
 @pytest.mark.parametrize(

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,19 +23,21 @@
 #
 
 # 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
 from django.utils.translation import gettext as _
 from django.views.generic import (
     CreateView,
@@ -45,7 +47,8 @@ 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
 from core.models import Gift, Preferences, SithFile, User
@@ -69,6 +72,7 @@ from subscription.models import Subscription
 from trombi.views import UserTrombiForm
 
 
+@method_decorator(check_honeypot, name="post")
 class SithLoginView(views.LoginView):
     """
     The login View
@@ -77,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):
@@ -124,9 +129,10 @@ def password_root_change(request, user_id):
     )
 
 
+@method_decorator(check_honeypot, name="post")
 class SithPasswordResetView(views.PasswordResetView):
     """
-    Allows someone to enter an email adresse for resetting password
+    Allows someone to enter an email address for resetting password
     """
 
     template_name = "core/password_reset.jinja"
@@ -153,33 +159,47 @@ class SithPasswordResetConfirmView(views.PasswordResetConfirmView):
 
 class SithPasswordResetCompleteView(views.PasswordResetCompleteView):
     """
-    Confirm the password has sucessfully been reset
+    Confirm the password has successfully been reset
     """
 
     template_name = "core/password_reset_complete.jinja"
 
 
-def register(request):
+@method_decorator(check_honeypot, name="post")
-    context = {}
+class UserCreationView(FormView):
-    if request.method == "POST":
+    success_url = reverse_lazy("core:index")
-        form = RegisteringForm(request.POST)
+    form_class = RegisteringForm
-        if form.is_valid():
+    template_name = "core/register.jinja"
-            logging.debug(
+
-                "Registering "
+    def form_valid(self, form):
-                + form.cleaned_data["first_name"]
+        # Just knowing that the user gave sound data isn't enough,
-                + form.cleaned_data["last_name"]
+        # we must also know if the given email actually exists.
+        # 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):

poetry.lock

@@ -426,13 +426,13 @@ files = [
 
 [[package]]
 name = "django"
-version = "4.2.13"
+version = "4.2.14"
 description = "A high-level Python web framework that encourages rapid development and clean, pragmatic design."
 optional = false
 python-versions = ">=3.8"
 files = [
-    {file = "Django-4.2.13-py3-none-any.whl", hash = "sha256:a17fcba2aad3fc7d46fdb23215095dbbd64e6174bf4589171e732b18b07e426a"},
+    {file = "Django-4.2.14-py3-none-any.whl", hash = "sha256:3ec32bc2c616ab02834b9cac93143a7dc1cdcd5b822d78ac95fc20a38c534240"},
-    {file = "Django-4.2.13.tar.gz", hash = "sha256:837e3cf1f6c31347a1396a3f6b65688f2b4bb4a11c580dcb628b5afe527b68a5"},
+    {file = "Django-4.2.14.tar.gz", hash = "sha256:fc6919875a6226c7ffcae1a7d51e0f2ceaf6f160393180818f6c95f51b1e7b96"},
 ]
 
 [package.dependencies]
@@ -477,17 +477,17 @@ test = ["djangorestframework", "graphene-django", "pytest", "pytest-cov", "pytes
 
 [[package]]
 name = "django-debug-toolbar"
-version = "4.3.0"
+version = "4.4.5"
 description = "A configurable set of panels that display various debug information about the current request/response."
 optional = false
 python-versions = ">=3.8"
 files = [
-    {file = "django_debug_toolbar-4.3.0-py3-none-any.whl", hash = "sha256:e09b7dcb8417b743234dfc57c95a7c1d1d87a88844abd13b4c5387f807b31bf6"},
+    {file = "django_debug_toolbar-4.4.5-py3-none-any.whl", hash = "sha256:91425606673ee674d780f7aeedf3595c264eb382dcf41f55c6779577900904c0"},
-    {file = "django_debug_toolbar-4.3.0.tar.gz", hash = "sha256:0b0dddee5ea29b9cb678593bc0d7a6d76b21d7799cb68e091a2148341a80f3c4"},
+    {file = "django_debug_toolbar-4.4.5.tar.gz", hash = "sha256:8298ce966b4c8fc71430082dd4739ef2badb5f867734e1973a413c4ab2ea81b7"},
 ]
 
 [package.dependencies]
-django = ">=3.2.4"
+django = ">=4.2.9"
 sqlparse = ">=0.2"
 
 [[package]]
@@ -508,6 +508,20 @@ packaging = "*"
 elasticsearch = ["elasticsearch (>=5,<8)"]
 testing = ["coverage", "geopy (==2)", "pysolr (>=3.7)", "python-dateutil", "requests", "whoosh (>=2.5.4,<3.0)"]
 
+[[package]]
+name = "django-honeypot"
+version = "1.2.0"
+description = "Django honeypot field utilities"
+optional = false
+python-versions = "<4.0,>=3.8"
+files = [
+    {file = "django_honeypot-1.2.0-py3-none-any.whl", hash = "sha256:53dd5f8dd96ef1bb7e31b5514c0dc2caae9577e78ebdf03ca4e0f304a7422aba"},
+    {file = "django_honeypot-1.2.0.tar.gz", hash = "sha256:25fca02e786aec26649bd13b37a95c846e09ab3cfc10f28db2f7dfaa77b9b9c6"},
+]
+
+[package.dependencies]
+Django = ">=3.2,<5.1"
+
 [[package]]
 name = "django-jinja"
 version = "2.11.0"
@@ -669,13 +683,13 @@ python-dateutil = ">=2.7"
 
 [[package]]
 name = "identify"
-version = "2.5.36"
+version = "2.6.0"
 description = "File identification library for Python"
 optional = false
 python-versions = ">=3.8"
 files = [
-    {file = "identify-2.5.36-py2.py3-none-any.whl", hash = "sha256:37d93f380f4de590500d9dba7db359d0d3da95ffe7f9de1753faa159e71e7dfa"},
+    {file = "identify-2.6.0-py2.py3-none-any.whl", hash = "sha256:e79ae4406387a9d300332b5fd366d8994f1525e8414984e1a59e058b2eda2dd0"},
-    {file = "identify-2.5.36.tar.gz", hash = "sha256:e5e00f54165f9047fbebeb4a560f9acfb8af4c88232be60a488e9b68d122745d"},
+    {file = "identify-2.6.0.tar.gz", hash = "sha256:cb171c685bdc31bcc4c1734698736a7d5b6c8bf2e0c15117f4d469c8640ae5cf"},
 ]
 
 [package.extras]
@@ -1867,4 +1881,4 @@ filelock = ">=3.4"
 [metadata]
 lock-version = "2.0"
 python-versions = "^3.10"
-content-hash = "8524ed5f593973edf05b3c01010c1f2345b7e799089c3e38274304bdedf8b3cb"
+content-hash = "51820883f41bdf40f00296b722ebdd9ac386e43ef1424ef990b29bac579ecbab"

pyproject.toml

@@ -46,6 +46,7 @@ django-countries = "^7.5.1"
 dict2xml = "^1.7.3"
 Sphinx = "^5" # Needed for building xapian
 tomli = "^2.0.1"
+django-honeypot = "^1.2.0"
 
 [tool.poetry.group.dev.dependencies]
 django-debug-toolbar = "^4.0.0"

sith/honeypot.py

@@ -0,0 +1,12 @@
+import logging
+from typing import Any
+
+from django.http import HttpResponse
+from django.test.client import WSGIRequest
+
+
+def custom_honeypot_error(
+    request: WSGIRequest, context: dict[str, Any]
+) -> HttpResponse:
+    logging.warning(f"HoneyPot blocked user with ip {request.META.get('REMOTE_ADDR')}")
+    return HttpResponse("Upon reading this, the http client was enlightened.")

sith/settings.py

@@ -1,5 +1,5 @@
 #
-# Copyright 2016,2017
+# Copyright 2016,2017,2024
 # - Skia <skia@libskia.so>
 # - Sli <antoine@bartuccio.fr>
 #
@@ -17,7 +17,7 @@
 # details.
 #
 # You should have received a copy of the GNU General Public License along with
-# this program; if not, write to the Free Sofware Foundation, Inc., 59 Temple
+# this program; if not, write to the Free Software Foundation, Inc., 59 Temple
 # Place - Suite 330, Boston, MA 02111-1307, USA.
 #
 #
@@ -44,6 +44,8 @@ import sentry_sdk
 from django.utils.translation import gettext_lazy as _
 from sentry_sdk.integrations.django import DjangoIntegration
 
+from .honeypot import custom_honeypot_error
+
 BASE_DIR = os.path.dirname(os.path.dirname(os.path.abspath(__file__)))
 
 os.environ["HTTPS"] = "off"
@@ -75,6 +77,7 @@ INSTALLED_APPS = (
     "django.contrib.messages",
     "django.contrib.staticfiles",
     "django.contrib.sites",
+    "honeypot",
     "django_jinja",
     "rest_framework",
     "ajax_select",
@@ -143,6 +146,7 @@ TEMPLATES = [
                 "django_jinja.builtins.extensions.UrlsExtension",
                 "django_jinja.builtins.extensions.StaticFilesExtension",
                 "django_jinja.builtins.extensions.DjangoFiltersExtension",
+                "core.templatetags.extensions.HoneypotExtension",
             ],
             "filters": {
                 "markdown": "core.templatetags.renderer.markdown",
@@ -280,6 +284,11 @@ LOGIN_REDIRECT_URL = "/"
 DEFAULT_FROM_EMAIL = "bibou@git.an"
 SITH_COM_EMAIL = "bibou_com@git.an"
 REST_FRAMEWORK["UNAUTHENTICATED_USER"] = "core.models.AnonymousUser"
+# Those values are to be changed in production to be more effective
+HONEYPOT_FIELD_NAME = "body2"
+HONEYPOT_VALUE = "content"
+HONEYPOT_RESPONDER = custom_honeypot_error  # Make honeypot errors less suspicious
+
 
 # Email
 EMAIL_BACKEND = "django.core.mail.backends.console.EmailBackend"