From 25d65f9c8ed49dfe4f6398d666eccada05f34b85 Mon Sep 17 00:00:00 2001 From: Bart van der Schoor Date: Thu, 28 Mar 2024 16:30:04 +0100 Subject: [PATCH] [#2187] Implemented email-verification --- src/open_inwoner/accounts/admin.py | 5 +- src/open_inwoner/accounts/middleware.py | 21 +- .../migrations/0075_user_verified_email.py | 20 ++ src/open_inwoner/accounts/models.py | 9 + .../accounts/email_verification.html | 24 +++ .../accounts/views/registration.py | 30 ++- src/open_inwoner/cms/profile/urls.py | 6 + src/open_inwoner/conf/base.py | 1 + .../conf/parts/email_verification.html | 5 + src/open_inwoner/conf/parts/maileditor.py | 22 +++ src/open_inwoner/configurations/admin.py | 1 + ...nfiguration_email_verification_required.py | 22 +++ src/open_inwoner/configurations/models.py | 5 + .../mail/tests/test_verification.py | 186 ++++++++++++++++++ src/open_inwoner/mail/urls.py | 14 ++ src/open_inwoner/mail/verification.py | 94 +++++++++ src/open_inwoner/mail/views.py | 45 +++++ src/open_inwoner/urls.py | 1 + 18 files changed, 508 insertions(+), 3 deletions(-) create mode 100644 src/open_inwoner/accounts/migrations/0075_user_verified_email.py create mode 100644 src/open_inwoner/accounts/templates/accounts/email_verification.html create mode 100644 src/open_inwoner/conf/parts/email_verification.html create mode 100644 src/open_inwoner/configurations/migrations/0065_siteconfiguration_email_verification_required.py create mode 100644 src/open_inwoner/mail/tests/test_verification.py create mode 100644 src/open_inwoner/mail/urls.py create mode 100644 src/open_inwoner/mail/verification.py create mode 100644 src/open_inwoner/mail/views.py diff --git a/src/open_inwoner/accounts/admin.py b/src/open_inwoner/accounts/admin.py index f9ead9c390..ff128dcd1a 100644 --- a/src/open_inwoner/accounts/admin.py +++ b/src/open_inwoner/accounts/admin.py @@ -90,7 +90,10 @@ class _UserAdmin(ImageCroppingMixin, UserAdmin): "first_name", ) fieldsets = ( - (None, {"fields": ("uuid", "email", "password", "login_type")}), + ( + None, + {"fields": ("uuid", "email", "verified_email", "password", "login_type")}, + ), ( _("Personal info"), { diff --git a/src/open_inwoner/accounts/middleware.py b/src/open_inwoner/accounts/middleware.py index 6798361a98..f4a46f2030 100644 --- a/src/open_inwoner/accounts/middleware.py +++ b/src/open_inwoner/accounts/middleware.py @@ -1,5 +1,7 @@ from django.urls import reverse_lazy +from open_inwoner.cms.utils.page_display import profile_page_is_published +from open_inwoner.configurations.models import SiteConfiguration from open_inwoner.utils.middleware import BaseConditionalUserRedirectMiddleware @@ -12,4 +14,21 @@ class NecessaryFieldsMiddleware(BaseConditionalUserRedirectMiddleware): def requires_redirect(self, request) -> bool: user = request.user - return user.require_necessary_fields() # and profile_page_is_published() + return user.require_necessary_fields() and profile_page_is_published() + + +class EmailVerificationMiddleware(BaseConditionalUserRedirectMiddleware): + """ + Redirect the user to a view to verify email + """ + + redirect_url = reverse_lazy("profile:email_verification_user") + extra_pass_prefixes = (reverse_lazy("mail:verification"),) + + def requires_redirect(self, request) -> bool: + user = request.user + return ( + not user.has_verified_email() + and profile_page_is_published() + and SiteConfiguration.get_solo().email_verification_required + ) diff --git a/src/open_inwoner/accounts/migrations/0075_user_verified_email.py b/src/open_inwoner/accounts/migrations/0075_user_verified_email.py new file mode 100644 index 0000000000..b296367fc8 --- /dev/null +++ b/src/open_inwoner/accounts/migrations/0075_user_verified_email.py @@ -0,0 +1,20 @@ +# Generated by Django 4.2.11 on 2024-03-28 15:25 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ("accounts", "0074_merge_20240228_1544"), + ] + + operations = [ + migrations.AddField( + model_name="user", + name="verified_email", + field=models.EmailField( + blank=True, default="", max_length=254, verbose_name="Verified email" + ), + ), + ] diff --git a/src/open_inwoner/accounts/models.py b/src/open_inwoner/accounts/models.py index 0cda1ee583..37d903ac34 100644 --- a/src/open_inwoner/accounts/models.py +++ b/src/open_inwoner/accounts/models.py @@ -85,6 +85,15 @@ class User(AbstractBaseUser, PermissionsMixin): email = models.EmailField( verbose_name=_("Email address"), ) + verified_email = models.EmailField( + verbose_name=_("Verified email"), + blank=True, + default="", + ) + + def has_verified_email(self): + return self.verified_email != "" and self.email == self.verified_email + phonenumber = models.CharField( verbose_name=_("Phonenumber"), blank=True, diff --git a/src/open_inwoner/accounts/templates/accounts/email_verification.html b/src/open_inwoner/accounts/templates/accounts/email_verification.html new file mode 100644 index 0000000000..48553cd20e --- /dev/null +++ b/src/open_inwoner/accounts/templates/accounts/email_verification.html @@ -0,0 +1,24 @@ +{% extends 'master.html' %} +{% load i18n static form_tags card_tags grid_tags solo_tags %} + +{% block content %} +
+{% render_grid %} + {% render_column span=9 %} + {% render_card tinted=True %} + {% get_solo 'configurations.SiteConfiguration' as config %} +

{% trans "E-mailadres bevestigen" %}


+ {% if config.email_verification_text %}

{{ config.email_verification_text|urlize|linebreaksbr }}


{% endif %} +
+ {% csrf_token %} +{# {% for field in form.fields %}#} +{# {% autorender_field form field %}#} +{# {% endfor %}#} + {% trans "Verficatie email verzenden" as button_text %} + {% form_actions primary_icon='arrow_forward' primary_text=button_text %} +
+ {% endrender_card %} + {% endrender_column %} +{% endrender_grid %} +
+{% endblock content %} \ No newline at end of file diff --git a/src/open_inwoner/accounts/views/registration.py b/src/open_inwoner/accounts/views/registration.py index 1ea27292c4..a19f739842 100644 --- a/src/open_inwoner/accounts/views/registration.py +++ b/src/open_inwoner/accounts/views/registration.py @@ -2,11 +2,12 @@ from django.contrib import messages from django.contrib.auth.mixins import LoginRequiredMixin +from django.forms import Form from django.http import HttpResponseRedirect from django.shortcuts import get_object_or_404 from django.urls import NoReverseMatch, reverse from django.utils.translation import gettext as _ -from django.views.generic import UpdateView +from django.views.generic import FormView, UpdateView from django_registration.backends.one_step.views import RegistrationView from furl import furl @@ -18,6 +19,7 @@ from open_inwoner.utils.hash import generate_email_from_string from open_inwoner.utils.views import CommonPageMixin, LogMixin +from ...mail.verification import send_user_email_verification_mail from ...utils.url import get_next_url_from from ..forms import CustomRegistrationForm, NecessaryUserForm from ..models import Invite, User @@ -179,3 +181,29 @@ def get_initial(self): initial["email"] = "" return initial + + +class EmailVerificationUserView(LogMixin, LoginRequiredMixin, FormView): + model = User + # dummy form + form_class = Form + template_name = "accounts/email_verification.html" + + def page_title(self): + return _("E-mailadres bevestigen") + + def form_valid(self, form): + user = self.request.user + + send_user_email_verification_mail(user) + + messages.add_message( + self.request, messages.SUCCESS, _("Bevestigings e-mail is verzonden") + ) + + self.log_user_action(user, _("user requested e-mail address verification")) + + return super().form_valid(form) + + def get_success_url(self): + return get_next_url_from(self.request, default=reverse("pages-root")) diff --git a/src/open_inwoner/cms/profile/urls.py b/src/open_inwoner/cms/profile/urls.py index ea5f33cfa4..c530926593 100644 --- a/src/open_inwoner/cms/profile/urls.py +++ b/src/open_inwoner/cms/profile/urls.py @@ -26,6 +26,7 @@ NewsletterSubscribeView, ) from open_inwoner.accounts.views.actions import ActionDeleteView +from open_inwoner.accounts.views.registration import EmailVerificationUserView app_name = "profile" @@ -103,6 +104,11 @@ NecessaryFieldsUserView.as_view(), name="registration_necessary", ), + path( + "register/email/verification/", + EmailVerificationUserView.as_view(), + name="email_verification_user", + ), path( "newsletters", NewsletterSubscribeView.as_view(), diff --git a/src/open_inwoner/conf/base.py b/src/open_inwoner/conf/base.py index d6d5480f64..ef60b1dda1 100644 --- a/src/open_inwoner/conf/base.py +++ b/src/open_inwoner/conf/base.py @@ -264,6 +264,7 @@ "open_inwoner.extended_sessions.middleware.SessionTimeoutMiddleware", "open_inwoner.kvk.middleware.KvKLoginMiddleware", "open_inwoner.accounts.middleware.NecessaryFieldsMiddleware", + "open_inwoner.accounts.middleware.EmailVerificationMiddleware", "open_inwoner.cms.utils.middleware.AnonymousHomePageRedirectMiddleware", "mozilla_django_oidc_db.middleware.SessionRefresh", ] diff --git a/src/open_inwoner/conf/parts/email_verification.html b/src/open_inwoner/conf/parts/email_verification.html new file mode 100644 index 0000000000..f45e7f4e7b --- /dev/null +++ b/src/open_inwoner/conf/parts/email_verification.html @@ -0,0 +1,5 @@ +

Beste

+

Om door te gaan moet je je email-adres bevestigen →

+

Mocht je geen behoefte hieraan hebben dan staat het je vrij om dit bericht te negeren

+

Met vriendelijke groet, + {{ site_name }}

diff --git a/src/open_inwoner/conf/parts/maileditor.py b/src/open_inwoner/conf/parts/maileditor.py index 317965e8dd..714d6048cf 100644 --- a/src/open_inwoner/conf/parts/maileditor.py +++ b/src/open_inwoner/conf/parts/maileditor.py @@ -470,6 +470,28 @@ def _readfile(name): }, ], }, + "email_verification": { + "name": _("Email adress verification"), + "description": _("This email is used by users to verify their email address"), + "subject_default": _("Bevestig je email address voor {{ site_name }}"), + "body_default": _readfile("email_verification.html"), + "subject": [ + { + "name": "site_name", + "description": _("Name of the site."), + }, + ], + "body": [ + { + "name": "verification_link", + "description": _("Link to confirm email address"), + }, + { + "name": "site_name", + "description": _("Name of the site"), + }, + ], + }, } MAIL_EDITOR_BASE_CONTEXT = {"site_name": "Open Inwoner Platform"} MAIL_EDITOR_DYNAMIC_CONTEXT = "open_inwoner.mail.context.mail_context" diff --git a/src/open_inwoner/configurations/admin.py b/src/open_inwoner/configurations/admin.py index b41796c9ce..93d8f38e78 100644 --- a/src/open_inwoner/configurations/admin.py +++ b/src/open_inwoner/configurations/admin.py @@ -235,6 +235,7 @@ class SiteConfigurationAdmin(OrderedInlineModelAdminMixin, SingletonModelAdmin): "email_new_message", "recipients_email_digest", "contact_phonenumber", + "email_verification_required", ) }, ), diff --git a/src/open_inwoner/configurations/migrations/0065_siteconfiguration_email_verification_required.py b/src/open_inwoner/configurations/migrations/0065_siteconfiguration_email_verification_required.py new file mode 100644 index 0000000000..6d9c1e3742 --- /dev/null +++ b/src/open_inwoner/configurations/migrations/0065_siteconfiguration_email_verification_required.py @@ -0,0 +1,22 @@ +# Generated by Django 4.2.11 on 2024-03-28 15:25 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ("configurations", "0064_auto_20240304_1200"), + ] + + operations = [ + migrations.AddField( + model_name="siteconfiguration", + name="email_verification_required", + field=models.BooleanField( + default=False, + help_text="Whether to require users to verify their email address", + verbose_name="Email verification required", + ), + ), + ] diff --git a/src/open_inwoner/configurations/models.py b/src/open_inwoner/configurations/models.py index 7669e6c934..a4357b66ce 100644 --- a/src/open_inwoner/configurations/models.py +++ b/src/open_inwoner/configurations/models.py @@ -372,6 +372,11 @@ class SiteConfiguration(SingletonModel): blank=True, default=list, ) + email_verification_required = models.BooleanField( + verbose_name=_("Email verification required"), + default=False, + help_text=_("Whether to require users to verify their email address"), + ) # contact info contact_phonenumber = models.CharField( diff --git a/src/open_inwoner/mail/tests/test_verification.py b/src/open_inwoner/mail/tests/test_verification.py new file mode 100644 index 0000000000..d2820bd050 --- /dev/null +++ b/src/open_inwoner/mail/tests/test_verification.py @@ -0,0 +1,186 @@ +from django.core import mail +from django.test import TestCase, override_settings +from django.urls import reverse + +from django_webtest import WebTest +from freezegun.api import freeze_time +from pyquery.pyquery import PyQuery + +from open_inwoner.accounts.tests.factories import UserFactory +from open_inwoner.mail.verification import ( + BadToken, + decode_email_verification_token, + generate_email_verification_token, + generate_email_verification_url, + send_user_email_verification_mail, + validate_email_verification_token, +) + + +class TestTokenVerification(TestCase): + @freeze_time("2024-01-01 00:00") + def test_generate_and_decode_token__basic(self): + user = UserFactory.create(email="foo@example.com") + + token = generate_email_verification_token(user) + payload = decode_email_verification_token(token) + + self.assertEqual(payload["u"], str(user.uuid)) + self.assertEqual(payload["e"], user.email) + + with freeze_time("2024-01-01 02:00"): + with self.assertRaises(BadToken): + decode_email_verification_token(token) + + def test_generate_email_verification_token__checks(self): + with self.subTest("not active"): + user = UserFactory.create() + user.is_active = False + with self.assertRaises(BadToken): + generate_email_verification_token(user) + + with self.subTest("no email"): + user = UserFactory.create(email="") + with self.assertRaises(BadToken): + generate_email_verification_token(user) + + with self.subTest("already verified"): + user = UserFactory.create( + email="foo@example.com", verified_email="foo@example.com" + ) + with self.assertRaises(BadToken): + generate_email_verification_token(user) + + with self.subTest("verified email changed"): + user = UserFactory.create( + email="bar@example.com", verified_email="not_bar@example.com" + ) + # got a token! + self.assertNotEqual("", generate_email_verification_token(user)) + + @freeze_time("2024-01-01 00:00") + def test_validate_email_verification_token__basic(self): + user = UserFactory.create(email="foo@example.com") + self.assertFalse(user.has_verified_email()) + + token = generate_email_verification_token(user) + self.assertTrue(validate_email_verification_token(user, token)) + + self.assertTrue(user.has_verified_email()) + self.assertEqual(user.email, user.verified_email) + + @freeze_time("2024-01-01 00:00") + def test_validate_email_verification_token__combinations(self): + user_one = UserFactory.create(email="foo@example.com") + user_two = UserFactory.create(email="bar@example.com") + + token_one = generate_email_verification_token(user_one) + token_two = generate_email_verification_token(user_two) + + with self.subTest("our own tokens are valid"): + self.assertTrue(validate_email_verification_token(user_one, token_one)) + self.assertTrue(validate_email_verification_token(user_two, token_two)) + + with self.subTest("expired tokens are invalid"): + with freeze_time("2024-01-01 02:00"): + self.assertFalse(validate_email_verification_token(user_one, token_one)) + self.assertFalse(validate_email_verification_token(user_two, token_two)) + + with self.subTest("other tokens are invalid"): + self.assertFalse(validate_email_verification_token(user_one, token_two)) + self.assertFalse(validate_email_verification_token(user_two, token_one)) + + with self.subTest("broken tokens are invalid"): + self.assertFalse( + validate_email_verification_token(user_one, token_one[1:-1]) + ) + self.assertFalse(validate_email_verification_token(user_one, "dsfsfsfd")) + self.assertFalse(validate_email_verification_token(user_one, "")) + + def test_validate_email_verification_token__checks(self): + with self.subTest("deactivated"): + user = UserFactory.create() + token = generate_email_verification_token(user) + user.is_active = False + self.assertFalse(validate_email_verification_token(user, token)) + + with self.subTest("no email"): + user = UserFactory.create() + token = generate_email_verification_token(user) + user.email = "" + self.assertFalse(validate_email_verification_token(user, token)) + + with self.subTest("other email"): + user = UserFactory.create(email="bar@example.com") + token = generate_email_verification_token(user) + user.email = "bazz@example.com" + self.assertFalse(validate_email_verification_token(user, token)) + + def test_user_has_verified_email(self): + with self.subTest("verified"): + user = UserFactory.create( + email="foo@example.com", verified_email="foo@example.com" + ) + self.assertTrue(user.has_verified_email()) + + with self.subTest("no email"): + user = UserFactory.create(email="", verified_email="") + self.assertFalse(user.has_verified_email()) + + with self.subTest("not verified"): + user = UserFactory.create(email="bar@example.com", verified_email="") + self.assertFalse(user.has_verified_email()) + + with self.subTest("email changed"): + user = UserFactory.create( + email="bazz@example.com", verified_email="not_bazz@example.com" + ) + self.assertFalse(user.has_verified_email()) + + +@override_settings(ROOT_URLCONF="open_inwoner.cms.tests.urls") +class TestSendVerificationEmail(WebTest): + def test_send_user_email_verification_mail(self): + user = UserFactory(email="foo@example.com") + + send_user_email_verification_mail(user) + + self.assertEqual(len(mail.outbox), 1) + email = mail.outbox[0] + self.assertEqual(email.to, [user.email]) + body = email.alternatives[0][0] # html version of the email body + + url_path = reverse("mail:verification") + self.assertIn(url_path, body) + + pq = PyQuery(body) + for elem in pq.find("a"): + url = elem.attrib.get("href") + if url and url_path in url: + # check url works + response = self.app.get(url, user=user) + + user.refresh_from_db() + self.assertTrue(user.has_verified_email()) + break + else: + self.fail("could not locate URL in html") + + +@override_settings(ROOT_URLCONF="open_inwoner.cms.tests.urls") +class TestMailVerificationView(WebTest): + def test_mail_verification_view(self): + user = UserFactory(email="foo@example.com") + self.assertFalse(user.has_verified_email()) + + url = generate_email_verification_url(user) + response = self.app.get(url, user=user) + + user.refresh_from_db() + self.assertTrue(user.has_verified_email()) + + def test_mail_verification_view__login_required(self): + user = UserFactory(email="foo@example.com") + url = generate_email_verification_url(user) + # TODO more here + self.app.get(url, status=302) diff --git a/src/open_inwoner/mail/urls.py b/src/open_inwoner/mail/urls.py new file mode 100644 index 0000000000..7459fe957e --- /dev/null +++ b/src/open_inwoner/mail/urls.py @@ -0,0 +1,14 @@ +from django.urls import path + +from open_inwoner.mail.views import EmailVerificationTokenView + +app_name = "mail" + + +urlpatterns = [ + path( + "verification/", + EmailVerificationTokenView.as_view(), + name="verification", + ), +] diff --git a/src/open_inwoner/mail/verification.py b/src/open_inwoner/mail/verification.py new file mode 100644 index 0000000000..2447ba78b0 --- /dev/null +++ b/src/open_inwoner/mail/verification.py @@ -0,0 +1,94 @@ +from datetime import timedelta +from typing import TypedDict + +from django.core import signing +from django.urls import reverse + +from furl import furl +from mail_editor.helpers import find_template + +from open_inwoner.accounts.models import User + + +class BadToken(Exception): + pass + + +VERIFY_SALT = "email_verify" +VERIFY_MAX_AGE = timedelta(hours=1) +VERIFY_GET_PARAM = "t" + + +class Payload(TypedDict): + u: str + e: str + + +def generate_email_verification_token(user: User) -> str: + if not user.email: + raise BadToken("user doesn't have email") + if not user.is_active: + raise BadToken("user not active") + if user.email == user.verified_email: + raise BadToken("already verified") + + payload: Payload = { + "u": str(user.uuid), + "e": user.email, + } + signer = signing.TimestampSigner(salt=VERIFY_SALT) + token = signer.sign_object(payload, compress=True) + return token + + +def decode_email_verification_token(token: str) -> Payload: + signer = signing.TimestampSigner(salt=VERIFY_SALT) + try: + payload: Payload = signer.unsign_object(token, max_age=VERIFY_MAX_AGE) + except signing.BadSignature: + raise BadToken("invalid signature") + else: + return payload + + +def generate_email_verification_url(user: User) -> str: + token = generate_email_verification_token(user) + f = furl(reverse("mail:verification")) + f.args[VERIFY_GET_PARAM] = token + return f.url + + +def validate_email_verification_token(user: User, token: str) -> bool: + # check condition + if user.is_anonymous or not user.is_authenticated: + return False + if not user.is_active or not user.email: + return False + + # check payload + try: + payload = decode_email_verification_token(token) + except BadToken: + return False + if not payload: + return False + if payload["u"] != str(user.uuid): + return False + if payload["e"] != user.email: + return False + + # payload is good, let's apply it + if user.verified_email != user.email: + user.verified_email = payload["e"] + user.save(update_fields=["verified_email"]) + + return True + + +def send_user_email_verification_mail(user: User) -> bool: + url = generate_email_verification_url(user) + template = find_template("email_verification") + context = { + "verification_link": url, + } + template.send_email([user.email], context) diff --git a/src/open_inwoner/mail/views.py b/src/open_inwoner/mail/views.py new file mode 100644 index 0000000000..f9c952367c --- /dev/null +++ b/src/open_inwoner/mail/views.py @@ -0,0 +1,45 @@ +from django.contrib import messages +from django.contrib.auth.mixins import LoginRequiredMixin +from django.core.exceptions import PermissionDenied +from django.shortcuts import redirect, reverse +from django.utils.translation import gettext as _ +from django.views import View + +from ..utils.url import get_next_url_from +from ..utils.views import LogMixin +from .verification import VERIFY_GET_PARAM, validate_email_verification_token + + +class EmailVerificationTokenView(LoginRequiredMixin, LogMixin, View): + def get(self, request): + if not request.user.is_authenticated: + raise PermissionDenied("not authenticated") + + token = request.GET.get(VERIFY_GET_PARAM) + if not token: + raise PermissionDenied("missing token parameter") + + if validate_email_verification_token(request.user, token): + messages.add_message( + self.request, messages.SUCCESS, _("E-mailadres is bevestigd") + ) + self.log_user_action(request.user, _("user verified e-mail address")) + + return redirect( + get_next_url_from(self.request, default=reverse("pages-root")) + ) + else: + messages.add_message( + self.request, + messages.ERROR, + _( + "Er is iets misgegaan met het valideren van de link. Probeer het opnieuw" + ), + ) + + from django.urls.exceptions import NoReverseMatch + + try: + return redirect("profile:email_verification_user") + except NoReverseMatch: + return redirect("pages-root") diff --git a/src/open_inwoner/urls.py b/src/open_inwoner/urls.py index 43522a1fa7..2e7b3c0ce6 100644 --- a/src/open_inwoner/urls.py +++ b/src/open_inwoner/urls.py @@ -95,6 +95,7 @@ AddPhoneNumberWizardView.as_view(), name="add_phone_number", ), + path("accounts/mail/", include("open_inwoner.mail.urls")), path("accounts/", include("django_registration.backends.one_step.urls")), path("accounts/", include("django.contrib.auth.urls")), path("api/", include("open_inwoner.api.urls", namespace="api")),