Code review changes.

Changes:
  - Change token to code to be consistent with other models.
  - Ensure only one EmailVerification record is created per email.
  - Add email verification expiry.

web/domains/user/forms.py

@@ -134,8 +134,8 @@ class Meta:
         model = Email
         fields = ["email", "type"]
 
-    def __init__(self, *args, **kwargs):
-        self.user = kwargs.pop("user", None)
+    def __init__(self, *args: Any, user: User, **kwargs: Any) -> None:
+        self.user = user
         super().__init__(*args, **kwargs)
 
     def clean_email(self):
@@ -177,8 +177,8 @@ class Meta:
             "is_primary": "Preferred contact address",
         }
 
-    def __init__(self, *args, **kwargs):
-        self.user = kwargs.pop("user", None)
+    def __init__(self, *args: Any, user: User, **kwargs: Any) -> None:
+        self.user = user
         super().__init__(*args, **kwargs)
 
     def save(self, commit=True):

web/domains/user/models.py

@@ -97,12 +97,12 @@ class Email(models.Model):
     WORK = "WORK"
     HOME = "HOME"
     TYPES = ((WORK, "Work"), (HOME, "Home"))
-    email = models.EmailField(max_length=254, blank=False, null=False)
-    type = models.CharField(max_length=30, choices=TYPES, blank=False, null=False, default=WORK)
-    portal_notifications = models.BooleanField(blank=False, null=False, default=False)
+    email = models.EmailField(max_length=254)
+    type = models.CharField(max_length=30, choices=TYPES, default=WORK)
+    portal_notifications = models.BooleanField(default=False)
     comment = models.CharField(max_length=4000, blank=True, null=True)
-    is_primary = models.BooleanField(blank=False, null=False, default=False)
-    is_verified = models.BooleanField(blank=False, null=False, default=False)
+    is_primary = models.BooleanField(default=False)
+    is_verified = models.BooleanField(default=False)
 
     user = models.ForeignKey(
         settings.AUTH_USER_MODEL, on_delete=models.CASCADE, related_name="emails"
@@ -114,9 +114,10 @@ def __str__(self):
 
 class EmailVerification(models.Model):
     email = models.ForeignKey(Email, on_delete=models.CASCADE)
-    token = models.UUIDField(unique=True, default=uuid.uuid4, editable=False)
+    code = models.UUIDField(unique=True, default=uuid.uuid4, editable=False)
+    processed = models.BooleanField(default=False)
     created_at = models.DateTimeField(auto_now_add=True)
     verified_at = models.DateTimeField(auto_now=True)
 
     def get_email_verification_url(self):
-        return reverse("email-verify", kwargs={"token": self.token})
+        return reverse("email-verify", kwargs={"code": self.code})

web/domains/user/urls.py

@@ -7,7 +7,24 @@
     path("welcome/", views.NewUserWelcomeView.as_view(), name="user-welcome"),
     path("welcome/clear/", views.ClearNewUserWelcomeView.as_view(), name="user-welcome-clear"),
     path("new-user/", views.NewUserUpdateView.as_view(), name="new-user-edit"),
-    path("email/verifiy/<uuid:token>/", views.VerifyEmailView.as_view(), name="email-verify"),
+    path(
+        "email/",
+        include(
+            [
+                path("verifiy/<uuid:code>/", views.VerifyEmailView.as_view(), name="email-verify"),
+                path(
+                    "<int:email_pk>/send-verify/",
+                    views.SendVerifyEmailView.as_view(),
+                    name="user-send-verify-email",
+                ),
+                path(
+                    "<int:email_pk>/verify-email-expired/",
+                    views.VerifyEmailExpiredView.as_view(),
+                    name="user-verify-email-expired",
+                ),
+            ]
+        ),
+    ),
     path(
         "<int:user_pk>/",
         include(
@@ -65,11 +82,6 @@
                                             views.UserDeleteEmailView.as_view(),
                                             name="user-email-delete",
                                         ),
-                                        path(
-                                            "verify/",
-                                            views.SendVerifyEmailView.as_view(),
-                                            name="user-send-verify-email",
-                                        ),
                                     ]
                                 ),
                             ),

web/domains/user/utils.py

@@ -1,5 +1,8 @@
+import datetime as dt
+
 from django.contrib.sites.models import Site
 from django.db.models import QuerySet
+from django.utils import timezone
 
 from web.auth.backends import ANONYMOUS_USER_PK
 from web.mail.emails import send_email_verification_email
@@ -12,5 +15,13 @@ def user_list_view_qs() -> QuerySet[User]:
 
 
 def send_and_create_email_verification(email: Email, site: Site) -> None:
-    verification = EmailVerification.objects.create(email=email)
+    verification, created = EmailVerification.objects.get_or_create(email=email, processed=False)
+
+    two_weeks_ago = timezone.now() - dt.timedelta(weeks=2)
+    if not created and verification.created_at < two_weeks_ago:
+        verification.processed = True
+        verification.save()
+
+        verification = EmailVerification.objects.create(email=email)
+
     send_email_verification_email(verification, site)

web/domains/user/views.py

@@ -1,3 +1,4 @@
+import datetime as dt
 from typing import Any
 
 from django.conf import settings
@@ -13,7 +14,9 @@
 from django.http import Http404, HttpResponse, HttpResponseRedirect
 from django.shortcuts import get_object_or_404, redirect
 from django.urls import reverse
+from django.utils import timezone
 from django.utils.decorators import method_decorator
+from django.views import View
 from django.views.generic import (
     CreateView,
     DetailView,
@@ -23,6 +26,8 @@
     UpdateView,
 )
 from django.views.generic.detail import SingleObjectMixin
+from django_ratelimit import UNSAFE
+from django_ratelimit.decorators import ratelimit
 
 from web.domains.template.context import UserManagementContext
 from web.domains.template.utils import replace_template_values
@@ -178,25 +183,58 @@ def post(self, request: AuthenticatedHttpRequest, *args: Any, **kwargs: Any) ->
         return super().post(request, *args, **kwargs)
 
 
+@method_decorator(ratelimit(key="ip", rate="5/m", method=UNSAFE, block=True), name="post")
+class SendVerifyEmailView(LoginRequiredMixin, SingleObjectMixin, View):
+    http_method_names = ["post"]
+    model = Email
+    pk_url_kwarg = "email_pk"
+
+    def get_queryset(self) -> QuerySet[Email]:
+        qs = super().get_queryset()
+        return qs.filter(user=self.request.user)
+
+    def post(self, request: AuthenticatedHttpRequest, *args: Any, **kwargs: Any) -> HttpResponse:
+        email = self.get_object()
+        send_and_create_email_verification(email, self.request.site)
+        messages.info(
+            self.request,
+            f"Email sent to {email.email}, Please check your inbox to complete adding the email address to your account",
+        )
+
+        return redirect(reverse("user-edit", kwargs={"user_pk": self.request.user.pk}))
+
+
 class VerifyEmailView(LoginRequiredMixin, SingleObjectMixin, RedirectView):
     http_method_names = ["get"]
-    slug_url_kwarg = "token"
+    slug_url_kwarg = "code"
     model = EmailVerification
-    slug_field = "token"
+    slug_field = "code"
     context_object_name = "email_verification"
 
     def get_queryset(self) -> QuerySet[EmailVerification]:
         qs = super().get_queryset()
 
-        return qs.filter(email__user=self.request.user)
+        return qs.filter(email__user=self.request.user, processed=False)
 
     def get(self, request: AuthenticatedHttpRequest, *args: Any, **kwargs: Any) -> HttpResponse:
         self.object = self.get_object()
-        if self.object and not self.object.email.is_verified:
-            self.object.email.is_verified = True
-            self.object.email.save()
-            self.object.save()
-            messages.info(self.request, f"Email address {self.object.email} has now been verified.")
+
+        two_weeks_ago = timezone.now() - dt.timedelta(weeks=2)
+        if self.object:
+            if self.object.created_at < two_weeks_ago:
+                return redirect(
+                    reverse("user-verify-email-expired", kwargs={"email_pk": self.object.email.pk})
+                )
+
+            if not self.object.email.is_verified:
+                self.object.email.is_verified = True
+                self.object.email.save()
+                self.object.processed = True
+                self.object.save()
+                messages.info(
+                    self.request, f"Email address {self.object.email} has now been verified."
+                )
+
         return super().get(request, *args, **kwargs)
 
     def get_object(self, *args: Any, **kwargs: Any) -> EmailVerification | None:
@@ -215,23 +253,25 @@ def get_redirect_url(self, *args: Any, **kwargs: Any) -> str:
             return reverse("user-edit", kwargs={"user_pk": self.request.user.pk})
 
 
-class SendVerifyEmailView(UserBaseMixin, SingleObjectMixin, RedirectView):
-    http_method_names = ["post"]
+class VerifyEmailExpiredView(LoginRequiredMixin, SingleObjectMixin, TemplateView):
+    # SingleObjectMixin config
     model = Email
     pk_url_kwarg = "email_pk"
+    context_object_name = "email"
 
-    def get_queryset(self) -> QuerySet[PhoneNumber]:
+    # TemplateView config
+    http_method_names = ["get"]
+    template_name = "web/domains/user/verify_email_expired.html"
+
+    def get_queryset(self) -> QuerySet[EmailVerification]:
         qs = super().get_queryset()
+
         return qs.filter(user=self.request.user)
 
-    def post(self, request: AuthenticatedHttpRequest, *args: Any, **kwargs: Any) -> HttpResponse:
-        email = self.get_object()
-        send_and_create_email_verification(email, self.request.site)
-        messages.info(
-            self.request,
-            f"Email sent to {email.email}, Please check your inbox to complete adding the email address to your account",
-        )
-        return super().post(request, *args, **kwargs)
+    def get(self, request: AuthenticatedHttpRequest, *args: Any, **kwargs: Any) -> HttpResponse:
+        self.object = self.get_object()
+
+        return super().get(request, *args, **kwargs)
 
 
 class UserCreateEmailView(UserBaseMixin, CreateView):

web/migrations/0048_email_is_verified_alter_emailtemplate_name_and_more.py → web/migrations/0048_user_email_verification.py

@@ -1,4 +1,4 @@
-# Generated by Django 5.1.3 on 2024-11-22 11:01
+# Generated by Django 5.1.3 on 2024-12-06 12:19
 
 import uuid
 
@@ -154,7 +154,8 @@ class Migration(migrations.Migration):
                         auto_created=True, primary_key=True, serialize=False, verbose_name="ID"
                     ),
                 ),
-                ("token", models.UUIDField(default=uuid.uuid4, editable=False, unique=True)),
+                ("code", models.UUIDField(default=uuid.uuid4, editable=False, unique=True)),
+                ("processed", models.BooleanField(default=False)),
                 ("created_at", models.DateTimeField(auto_now_add=True)),
                 ("verified_at", models.DateTimeField(auto_now=True)),
                 (

web/templates/web/domains/user/user_edit.html

@@ -113,7 +113,7 @@ <h3>General Details</h3>
           {% if email.is_verified  %}
             <a href="{{ icms_url('user-email-edit', kwargs={'user_pk':request.user.pk, 'email_pk': email.pk}) }}" class="button small-button icon-pencil">Edit</a>
           {% elif email.email != request.user.email %}
-            <form method="post" action="{{ icms_url('user-send-verify-email', kwargs={'user_pk':request.user.pk, 'email_pk': email.pk}) }}" class="form-inline">
+            <form method="post" action="{{ icms_url('user-send-verify-email', kwargs={'email_pk': email.pk}) }}" class="form-inline">
               {{ csrf_input }}
               <button type="submit" class="button small-button icon-mail2">Resend Verification email</button>
             </form>

web/templates/web/domains/user/verify_email_expired.html

@@ -0,0 +1,16 @@
+{% extends "layout/sidebar.html" %}
+{% import "forms/forms.html" as forms %}
+
+{% block page_title %}Email Verification Expired{% endblock %}
+{#{% block context_header %}Email Verification Expired{% endblock %}#}
+
+
+{% block main_content %}
+  <h3>This link has now expired</h3>
+  <div class="info-box info-box-warning">
+    This email verification link has now expired.
+  </div>
+  {%- call forms.form(action=icms_url('user-send-verify-email',kwargs={'email_pk': email.pk}), method='post', csrf_input=csrf_input) -%}
+    {{ forms.submit_button(padding_cols="zero", btn_label="Resend verification email") }}
+  {%- endcall -%}
+{% endblock %}

web/tests/domains/user/test_utils.py

@@ -0,0 +1,81 @@
+import datetime as dt
+
+from django.contrib.sites.models import Site
+from django.utils import timezone
+from freezegun import freeze_time
+
+from web.domains.user.utils import send_and_create_email_verification
+from web.mail.constants import EmailTypes
+from web.mail.url_helpers import get_email_verification_url
+from web.models import Email, EmailVerification
+from web.sites import SiteName, get_importer_site_domain
+from web.tests.helpers import check_gov_notify_email_was_sent
+
+
+def test_send_and_create_email_verification(importer_one_contact):
+    email = Email.objects.create(
+        email="[email protected]", type=Email.WORK, user=importer_one_contact  # /PS-IGNORE
+    )
+    site = Site.objects.get(name=SiteName.IMPORTER)
+
+    send_and_create_email_verification(email, site)
+
+    email.refresh_from_db()
+    assert email.emailverification_set.count() == 1
+    _assert_email_sent(email.emailverification_set.first())
+
+
+def test_resend_and_create_email_verification(importer_one_contact):
+    email = Email.objects.create(
+        email="[email protected]", type=Email.WORK, user=importer_one_contact  # /PS-IGNORE
+    )
+    verification = EmailVerification.objects.create(email=email)
+
+    site = Site.objects.get(name=SiteName.IMPORTER)
+
+    send_and_create_email_verification(email, site)
+
+    email.refresh_from_db()
+
+    assert email.emailverification_set.count() == 1
+    assert email.emailverification_set.first() == verification
+    _assert_email_sent(verification)
+
+
+def test_resend_and_create_email_verification_for_expired_email_verification(importer_one_contact):
+    email = Email.objects.create(
+        email="[email protected]", type=Email.WORK, user=importer_one_contact  # /PS-IGNORE
+    )
+    older_than_two_weeks = timezone.now() - dt.timedelta(weeks=2, microseconds=1)
+    with freeze_time(older_than_two_weeks):
+        verification = EmailVerification.objects.create(email=email)
+
+    site = Site.objects.get(name=SiteName.IMPORTER)
+
+    send_and_create_email_verification(email, site)
+
+    latest_verification = email.emailverification_set.last()
+
+    email.refresh_from_db()
+    verification.refresh_from_db()
+
+    assert email.emailverification_set.count() == 2
+    assert email.emailverification_set.last() != verification
+    assert verification.processed is True
+    assert latest_verification.processed is False
+    _assert_email_sent(latest_verification)
+
+
+def _assert_email_sent(verification: EmailVerification) -> None:
+    check_gov_notify_email_was_sent(
+        1,
+        ["[email protected]"],  # /PS-IGNORE
+        EmailTypes.EMAIL_VERIFICATION,
+        {
+            "icms_url": get_importer_site_domain(),
+            "service_name": SiteName.IMPORTER.label,
+            "email_verification_url": get_email_verification_url(
+                verification, get_importer_site_domain()
+            ),
+        },
+    )