chore: upgrade django-registration to latest version (#531)

Signed-off-by: tdruez <tdruez@aboutcode.org>

Author: tdruez

dejacode/urls.py

@@ -102,16 +102,16 @@
         ),
         name="login",
     ),
-    # Activation and password views are required for the user creation flow.
-    # registration_activation_complete needs to be register before registration_activate
-    # so the 'complete/' segment is not caught as the activation_key
+    # User activation.
+    # Activation views are required for the user creation flow, even when
+    # self-registration (ENABLE_SELF_REGISTRATION) is turned off.
     path(
         "account/activate/complete/",
         TemplateView.as_view(template_name="django_registration/activation_complete.html"),
         name="django_registration_activation_complete",
     ),
     path(
-        "account/activate/<str:activation_key>/",
+        "account/activate/",
         DejaCodeActivationView.as_view(),
         name="django_registration_activate",
     ),
@@ -180,11 +180,13 @@
     from django_registration.backends.activation.views import RegistrationView
 
     urlpatterns += [
+        # Override the registration view to use our custom form
         path(
             "account/register/",
             RegistrationView.as_view(form_class=DejaCodeRegistrationForm),
             name="django_registration_register",
         ),
+        # Include the rest (complete, disallowed, etc.) from the default backend
         path("account/", include("django_registration.backends.activation.urls")),
     ]
 

dje/registration.py

@@ -46,6 +46,12 @@
 class DejaCodeActivationView(ActivationView):
     def get_success_url(self, user=None):
         """Add support for 'Sign Up' registration and User creation in admin."""
+        # In django-registration 5.x, get_success_url is called with user=user
+        # as a keyword argument. The default ``user=None`` keeps it safe when
+        # called without a user (e.g. from base FormView code paths).
+        if user is None:
+            return self.success_url
+
         if user.has_usable_password():
             # User created from registration process
             return self.success_url

dje/templates/django_registration/activation_email_body.txt

@@ -4,8 +4,9 @@ Your DejaCode {{ user.dataspace.name }} account is pending activation.
 
 Username: {{ user.username }}
 
-To activate the account please click on the link below:
-https://{{ site }}{% url 'django_registration_activate' activation_key %}
+To activate the account, please click on the link below and confirm the activation on the page that opens:
+
+https://{{ site }}{% url 'django_registration_activate' %}?activation_key={{ activation_key }}
 
 If you cannot click on the link, please copy it to your browser.
 
@@ -15,5 +16,4 @@ Please note that you have {{ expiration_days }} days to activate your account.
 {% endif %}
 
 Thank You,
-
 The DejaCode Team.

dje/templates/django_registration/activation_failed.html

@@ -0,0 +1,33 @@
+{% extends "bootstrap_base.html" %}
+{% load i18n %}
+{% block page_title %}{% trans 'Account activation' %}{% endblock %}
+{% block bodyclass %}bg-body-tertiary{% endblock %}
+{% block content %}
+  {% if activation_error %}
+    {% include 'includes/header_title.html' with pretitle='Account' title='Error during your account activation' %}
+    <p>{{ activation_error.message }}</p>
+    {% if DEJACODE_SUPPORT_EMAIL %}
+      <p>
+        If the problem persists, send us an email at <a href="mailto:{{ DEJACODE_SUPPORT_EMAIL }}">{{ DEJACODE_SUPPORT_EMAIL }}</a>
+      </p>
+    {% endif %}
+  {% else %}
+    {% include 'includes/header_title.html' with pretitle='Account' title='Confirm your account activation' %}
+    {% if form.activation_key.errors %}
+      <div class="alert alert-danger">
+        {% for error in form.activation_key.errors %}
+          <p class="mb-0">{{ error }}</p>
+        {% endfor %}
+      </div>
+    {% else %}
+      <p>Click the button below to activate your DejaCode account.</p>
+    {% endif %}
+    <form method="post">
+      {% csrf_token %}
+      <input type="hidden" name="activation_key" value="{{ form.activation_key.value|default:'' }}">
+      <button type="submit" class="btn btn-warning">
+        {% trans 'Activate my account' %}
+      </button>
+    </form>
+  {% endif %}
+{% endblock %}

dje/templates/django_registration/activation_form.html

@@ -10,7 +10,7 @@
   <p>
     Thank you for creating your account for DejaCode.
     An activation email will be sent shortly to the email address you provided.<br>
-    In order to activate your account you must click on the activation link inside the email you receive.
+    To activate your account, click on the link inside the email and confirm the activation on the page that opens.
   </p>
   <p>
     <a href="{% url 'index_dispatch' %}">Back to homepage.</a>

dje/templates/django_registration/registration_complete.html

@@ -6,17 +6,21 @@
 # See https://aboutcode.org for more information about AboutCode FOSS projects.
 #
 
+from datetime import timedelta
 from unittest.mock import patch
 
 from django.conf import settings
 from django.contrib.auth import get_user_model
 from django.contrib.auth.models import Group
 from django.core import mail
+from django.core import signing
 from django.test import TestCase
 from django.test import override_settings
 from django.urls import reverse
+from django.utils import timezone
 
 from django_altcha import AltchaField
+from django_registration.backends.activation.views import REGISTRATION_SALT
 from django_registration.backends.activation.views import RegistrationView
 
 from dje.registration import REGISTRATION_DEFAULT_GROUPS
@@ -26,6 +30,7 @@
 @override_settings(
     ENABLE_SELF_REGISTRATION=True,
     ADMINS=[("admin", "admin@nexb.com")],
+    ALTCHA_HMAC_KEY="abcdef123456",
 )
 class DejaCodeUserRegistrationTestCase(TestCase):
     """Tests for the dejacode.com registration workflow."""
@@ -48,7 +53,6 @@ def setUp(self):
     def tearDown(self):
         self.captcha_patch.stop()
 
-    @override_settings(ALTCHA_HMAC_KEY="abcdef123456")
     def test_user_registration_form_submit(self):
         url = reverse("django_registration_register")
         response = self.client.get(url)
@@ -67,6 +71,8 @@ def test_user_registration_form_submit(self):
         self.assertTrue("Your DejaCode Evaluation account is pending activation." in body)
         self.assertTrue("Username: {}".format(self.registration_data["username"]) in body)
         self.assertTrue("{} days to activate".format(settings.ACCOUNT_ACTIVATION_DAYS) in body)
+        # Verify the activation URL uses the querystring format
+        self.assertTrue("?activation_key=" in body)
 
         new_user = get_user_model().objects.get(username=self.registration_data["username"])
         self.assertEqual(new_user.email, self.registration_data["email"])
@@ -84,7 +90,6 @@ def test_user_registration_form_submit(self):
         body = mail.outbox[0].body
         self.assertTrue("New registration for user username username@company.com" in body)
 
-    @override_settings(ALTCHA_HMAC_KEY="abcdef123456")
     def test_user_registration_form_validators(self):
         self.captcha_patch.stop()
 
@@ -126,15 +131,19 @@ def test_user_registration_account_activation(self):
 
         self.assertEqual("[DejaCode] Please activate your account", mail.outbox[1].subject)
         activation_key = RegistrationView().get_activation_key(user)
-        activation_url = reverse("django_registration_activate", args=[activation_key])
+        activation_url = reverse("django_registration_activate")
         # Check the validity of URL in activation email
         # WARNING: The key of the URL set in the email may be different since it is not
-        # generated at the same time as the `activation_key` and the results is based on
+        # generated at the same time as the `activation_key` and the result is based on
         # the timestamp.
-        self.assertTrue(activation_url in mail.outbox[1].body)
-
-        # Call the url to activate the account
-        response = self.client.get(activation_url, follow=True)
+        expected_url_in_email = f"{activation_url}?activation_key={activation_key}"
+        self.assertTrue(expected_url_in_email in mail.outbox[1].body)
+        # Activate the account via POST (django-registration 5.x requires POST)
+        response = self.client.post(
+            activation_url,
+            data={"activation_key": activation_key},
+            follow=True,
+        )
         self.assertRedirects(response, reverse("django_registration_activation_complete"))
         self.assertContains(response, "account is now active")
         # Now the user is active
@@ -143,16 +152,100 @@ def test_user_registration_account_activation(self):
         self.assertTrue(user.has_usable_password())
         self.assertTrue(user.is_staff)
         self.assertFalse(user.is_superuser)
-
-        # Make sure the activation link can be use multiple time within the
+        # Make sure the activation link can be used multiple times within the
         # `ACCOUNT_ACTIVATION_DAYS` period.
-        response = self.client.get(activation_url, follow=True)
+        response = self.client.post(
+            activation_url,
+            data={"activation_key": activation_key},
+            follow=True,
+        )
         self.assertContains(response, "account is now active")
 
+    def test_user_registration_activation_form_displayed_on_get(self):
+        url = reverse("django_registration_register")
+        self.client.post(url, self.registration_data)
+        user = get_user_model().objects.get(username=self.registration_data["username"])
+        activation_key = RegistrationView().get_activation_key(user)
+        activation_url = reverse("django_registration_activate")
+
+        response = self.client.get(f"{activation_url}?activation_key={activation_key}")
+        self.assertEqual(response.status_code, 200)
+        # The page should display the activation form, not redirect or auto-activate
+        self.assertContains(response, "Activate my account")
+        # User should NOT be active yet (security: GET should not activate)
+        user.refresh_from_db()
+        self.assertFalse(user.is_active)
+
     def test_user_registration_activate_wrong_key(self):
-        activation_url = reverse("django_registration_activate", args=["wrong_key"])
-        response = self.client.get(activation_url)
-        self.assertContains(response, "Error during your account activation")
+        activation_url = reverse("django_registration_activate")
+        response = self.client.post(activation_url, data={"activation_key": "wrong_key"})
+        self.assertEqual(response.status_code, 200)
+        # The form should reject the invalid activation key
+        self.assertContains(response, "invalid")
+        # No user should have been created or activated
+        self.assertEqual(get_user_model().objects.count(), 0)
+
+    def test_user_registration_activate_empty_key(self):
+        activation_url = reverse("django_registration_activate")
+        response = self.client.post(activation_url, data={"activation_key": ""})
+        self.assertEqual(response.status_code, 200)
+        # Form should reject the empty key
+        self.assertContains(response, "required")
+
+    def test_user_registration_activate_expired_key(self):
+        url = reverse("django_registration_register")
+        self.client.post(url, self.registration_data)
+        user = get_user_model().objects.get(username=self.registration_data["username"])
+
+        # Generate a key with a timestamp older than ACCOUNT_ACTIVATION_DAYS
+        expired_days = settings.ACCOUNT_ACTIVATION_DAYS + 1
+        with patch("django.core.signing.time.time") as mock_time:
+            past_timestamp = (timezone.now() - timedelta(days=expired_days)).timestamp()
+            mock_time.return_value = past_timestamp
+            expired_key = signing.dumps(obj=user.username, salt=REGISTRATION_SALT)
+
+        activation_url = reverse("django_registration_activate")
+        response = self.client.post(activation_url, data={"activation_key": expired_key})
+        self.assertEqual(response.status_code, 200)
+        self.assertContains(response, "expired")
+        # User should remain inactive
+        user.refresh_from_db()
+        self.assertFalse(user.is_active)
+
+    def test_user_registration_activate_unknown_user(self):
+        # Generate a key for a user that doesn't exist
+        bogus_key = signing.dumps(obj="nonexistent_user", salt=REGISTRATION_SALT)
+        activation_url = reverse("django_registration_activate")
+        response = self.client.post(activation_url, data={"activation_key": bogus_key}, follow=True)
+        self.assertEqual(response.status_code, 200)
+        # Should display the activation error template content
+        self.assertContains(response, "The account you attempted to activate is invalid")
+
+    def test_user_registration_unique_email(self):
+        url = reverse("django_registration_register")
+        # First registration succeeds
+        self.client.post(url, self.registration_data)
+        # Second registration with same email but different username
+        duplicate_data = dict(self.registration_data)
+        duplicate_data["username"] = "different_username"
+        response = self.client.post(url, duplicate_data)
+        self.assertEqual(response.status_code, 200)
+        self.assertIn("email", response.context["form"].errors)
+
+    def test_user_registration_unique_username(self):
+        url = reverse("django_registration_register")
+        self.client.post(url, self.registration_data)
+        duplicate_data = dict(self.registration_data)
+        duplicate_data["email"] = "different@company.com"
+        response = self.client.post(url, duplicate_data)
+        self.assertEqual(response.status_code, 200)
+        self.assertIn("username", response.context["form"].errors)
+
+    def test_user_registration_default_dataspace_assigned(self):
+        url = reverse("django_registration_register")
+        self.client.post(url, self.registration_data)
+        new_user = get_user_model().objects.get(username=self.registration_data["username"])
+        self.assertEqual(new_user.dataspace.name, "Evaluation")
 
     def test_user_registration_default_groups(self):
         for group_name in REGISTRATION_DEFAULT_GROUPS:
@@ -164,7 +257,41 @@ def test_user_registration_default_groups(self):
         new_user = get_user_model().objects.get(username=self.registration_data["username"])
         self.assertEqual(len(REGISTRATION_DEFAULT_GROUPS), new_user.groups.count())
 
+    def test_user_registration_default_groups_missing(self):
+        # Don't create any groups
+        url = reverse("django_registration_register")
+        response = self.client.post(url, self.registration_data, follow=True)
+        # Registration should succeed
+        self.assertRedirects(response, reverse("django_registration_complete"))
+        new_user = get_user_model().objects.get(username=self.registration_data["username"])
+        self.assertEqual(0, new_user.groups.count())
+
+    def test_user_registration_password_field_only_password1(self):
+        url = reverse("django_registration_register")
+        response = self.client.get(url)
+        self.assertContains(response, 'name="password1"')
+        self.assertNotContains(response, 'name="password2"')
+
     @override_settings(REGISTRATION_OPEN=False)
     def test_user_registration_closed(self):
         resp = self.client.get(reverse("django_registration_register"))
         self.assertRedirects(resp, reverse("django_registration_disallowed"))
+
+    @override_settings(REGISTRATION_OPEN=False)
+    def test_user_registration_closed_post_blocked(self):
+        resp = self.client.post(reverse("django_registration_register"), self.registration_data)
+        self.assertRedirects(resp, reverse("django_registration_disallowed"))
+        # No user should have been created
+        self.assertEqual(get_user_model().objects.count(), 0)
+
+    def test_user_registration_admin_notification_email_sent(self):
+        url = reverse("django_registration_register")
+        self.client.post(url, self.registration_data)
+
+        admin_email = mail.outbox[0]
+        self.assertEqual("[DejaCode] New User registration", admin_email.subject)
+        # Check that admin@nexb.com is in any of the recipient tuples or strings
+        recipients_str = str(admin_email.to)
+        self.assertIn("admin@nexb.com", recipients_str)
+        self.assertIn(self.registration_data["username"], admin_email.body)
+        self.assertIn(self.registration_data["email"], admin_email.body)