From 4f8fce856f7044019f69937df04e44f6b196fb94 Mon Sep 17 00:00:00 2001 From: Bart van der Schoor Date: Tue, 19 Mar 2024 13:35:38 +0100 Subject: [PATCH] [#2187] Implemented email-verification views, refactored middleware's --- src/open_inwoner/accounts/admin.py | 5 +- src/open_inwoner/accounts/middleware.py | 79 +++++++----------- .../accounts/email_verification.html | 24 ++++++ .../accounts/views/registration.py | 46 ++++++++--- src/open_inwoner/cms/profile/urls.py | 6 ++ src/open_inwoner/conf/base.py | 1 + src/open_inwoner/kvk/middleware.py | 49 +++--------- src/open_inwoner/mail/urls.py | 4 +- src/open_inwoner/mail/views.py | 42 ++++++---- src/open_inwoner/utils/middleware.py | 80 +++++++++++++++++++ src/open_inwoner/utils/url.py | 14 ++++ 11 files changed, 230 insertions(+), 120 deletions(-) create mode 100644 src/open_inwoner/accounts/templates/accounts/email_verification.html create mode 100644 src/open_inwoner/utils/middleware.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 44d8d9441a..e79e5a1471 100644 --- a/src/open_inwoner/accounts/middleware.py +++ b/src/open_inwoner/accounts/middleware.py @@ -1,64 +1,39 @@ -import logging +from django.urls import reverse_lazy -from django.conf import settings -from django.http import HttpResponseRedirect -from django.urls import NoReverseMatch, reverse +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 BaseForcedRedirectMiddleware -from furl import furl -logger = logging.getLogger(__name__) - - -class NecessaryFieldsMiddleware: +class NecessaryFieldsMiddleware(BaseForcedRedirectMiddleware): """ Redirect the user to a view to fill in necessary fields """ - def __init__(self, get_response): - self.get_response = get_response - - def __call__(self, request): - try: - necessary_fields_url = reverse("profile:registration_necessary") - except NoReverseMatch: - logger.warning( - "cannot reverse 'profile:registration_necessary' URL: apphook not active" - ) - return self.get_response(request) - - if request.path.startswith(settings.MEDIA_URL) or request.path.startswith( - settings.PRIVATE_MEDIA_URL - ): - return self.get_response(request) + redirect_url = reverse_lazy("profile:registration_necessary") + def requires_redirect(self, request) -> bool: user = request.user - if user.is_authenticated: + return ( + user.is_authenticated + and user.require_necessary_fields() + and profile_page_is_published() + ) - # If the user is currently not editing their information, but it is required - # redirect to that view. - try: - digid_logout = reverse("digid:logout") - digid_slo_redirect = reverse("digid:slo-redirect") - except NoReverseMatch: - # temporary fix to make tests pass in case reverse fails - digid_logout = "/digid/logout/" - digid_slo_redirect = "/digid/slo/redirect/" - if ( - not request.path.startswith( - ( - necessary_fields_url, - reverse("logout"), - digid_logout, - digid_slo_redirect, - reverse("kvk:branches"), - ) - ) - and request.user.require_necessary_fields() - ): - redirect = furl(reverse("profile:registration_necessary")) - if request.path != settings.LOGIN_REDIRECT_URL: - redirect.set({"next": request.path}) - return HttpResponseRedirect(redirect.url) +class EmailVerificationMiddleware(BaseForcedRedirectMiddleware): + """ + Redirect the user to a view to verify email + """ - return self.get_response(request) + 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 ( + user.is_authenticated + and not user.has_verified_email() + and profile_page_is_published() + and SiteConfiguration.get_solo().email_verification_required + ) 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 76b611f29b..833ab0c7c2 100644 --- a/src/open_inwoner/accounts/views/registration.py +++ b/src/open_inwoner/accounts/views/registration.py @@ -2,13 +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.encoding import iri_to_uri -from django.utils.http import url_has_allowed_host_and_scheme 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 @@ -20,6 +19,8 @@ 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 ..forms import CustomRegistrationForm, NecessaryUserForm from ..models import Invite, User @@ -146,15 +147,10 @@ def get_object(self, queryset=None): return self.request.user def get_success_url(self): - messages.add_message(self.request, messages.SUCCESS, "Registratie is voltooid") - if "next" in self.request.GET and url_has_allowed_host_and_scheme( - url=self.request.GET["next"], - allowed_hosts=[ - self.request.get_host(), - ], - ): - return iri_to_uri(self.request.GET["next"]) - return reverse("pages-root") + messages.add_message( + self.request, messages.SUCCESS, _("Registratie is voltooid") + ) + return get_next_url(self.request, default=reverse("pages-root")) def get_form_kwargs(self): kwargs = super().get_form_kwargs() @@ -185,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(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 c84b501600..23158d078b 100644 --- a/src/open_inwoner/cms/profile/urls.py +++ b/src/open_inwoner/cms/profile/urls.py @@ -25,6 +25,7 @@ NewsletterSubscribeView, ) from open_inwoner.accounts.views.actions import ActionDeleteView +from open_inwoner.accounts.views.registration import EmailVerificationUserView app_name = "profile" @@ -102,6 +103,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 96e0e257ad..f51be806cc 100644 --- a/src/open_inwoner/conf/base.py +++ b/src/open_inwoner/conf/base.py @@ -263,6 +263,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/kvk/middleware.py b/src/open_inwoner/kvk/middleware.py index 80a7d3bb8c..e6a104a14c 100644 --- a/src/open_inwoner/kvk/middleware.py +++ b/src/open_inwoner/kvk/middleware.py @@ -1,52 +1,27 @@ import logging -from django.conf import settings -from django.http import HttpResponseRedirect from django.urls import NoReverseMatch, reverse -from furl import furl - from open_inwoner.kvk.branches import kvk_branch_selected_done +from open_inwoner.utils.middleware import BaseForcedRedirectMiddleware logger = logging.getLogger(__name__) -class KvKLoginMiddleware: +class KvKLoginMiddleware(BaseForcedRedirectMiddleware): """Redirect authenticated eHerkenning users to select a company branch""" - def __init__(self, get_response): - self.get_response = get_response - - def __call__(self, request): + def requires_redirect(self, request): user = request.user + return ( + user.is_authenticated + and user.is_eherkenning_user + and not kvk_branch_selected_done(request.session) + ) - # pass through - if ( - not user.is_authenticated - or not user.is_eherkenning_user - or kvk_branch_selected_done(request.session) - or request.path.startswith(settings.MEDIA_URL) - or request.path.startswith(settings.PRIVATE_MEDIA_URL) - ): - return self.get_response(request) - - # let the user logout and avoid redirect circles + def get_redirect_url(self, request): try: - logout = reverse("logout") - eherkenning_logout = reverse("eherkenning:logout") - branches = reverse("kvk:branches") + return reverse("kvk:branches") except NoReverseMatch: - logout = "/accounts/logout/" - eherkenning_logout = "/eherkenning/logout/" - branches = "/kvk/branches/" - - if request.path.startswith((logout, eherkenning_logout, branches)): - return self.get_response(request) - - # redirect to company branch choice - redirect = furl(reverse("kvk:branches")) - if request.path != settings.LOGIN_REDIRECT_URL: - redirect.set({"next": request.path}) - redirect.args.update(request.GET) - - return HttpResponseRedirect(redirect.url) + # temporary fallback for tests + return "/kvk/branches/" diff --git a/src/open_inwoner/mail/urls.py b/src/open_inwoner/mail/urls.py index ff47288b7f..7459fe957e 100644 --- a/src/open_inwoner/mail/urls.py +++ b/src/open_inwoner/mail/urls.py @@ -1,6 +1,6 @@ from django.urls import path -from open_inwoner.mail.views import EmailVerificationView +from open_inwoner.mail.views import EmailVerificationTokenView app_name = "mail" @@ -8,7 +8,7 @@ urlpatterns = [ path( "verification/", - EmailVerificationView.as_view(), + EmailVerificationTokenView.as_view(), name="verification", ), ] diff --git a/src/open_inwoner/mail/views.py b/src/open_inwoner/mail/views.py index 91908d030f..38cdee8b60 100644 --- a/src/open_inwoner/mail/views.py +++ b/src/open_inwoner/mail/views.py @@ -1,18 +1,16 @@ +from django.contrib import messages from django.contrib.auth.mixins import LoginRequiredMixin from django.core.exceptions import PermissionDenied -from django.shortcuts import redirect +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 ..utils.views import LogMixin from .verification import VERIFY_GET_PARAM, validate_email_verification_token -class EmailVerificationView(LoginRequiredMixin, View): - # TODO determine where we want to go - # TODO grab next? - - token_success_url = "profile:detail" - token_failure_url = "profile:detail" - +class EmailVerificationTokenView(LoginRequiredMixin, LogMixin, View): def get(self, request): if not request.user.is_authenticated: raise PermissionDenied("not authenticated") @@ -22,12 +20,24 @@ def get(self, request): raise PermissionDenied("missing token parameter") if validate_email_verification_token(request.user, token): - return redirect(self.get_token_success_url()) - else: - return redirect(self.get_token_failure_url()) + messages.add_message( + self.request, messages.SUCCESS, _("E-mailadres is bevestigd") + ) + self.log_user_action(request.user, _("user verified e-mail address")) - def get_token_success_url(self): - return self.token_success_url - - def get_token_failure_url(self): - return self.token_failure_url + return redirect(get_next_url(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/utils/middleware.py b/src/open_inwoner/utils/middleware.py new file mode 100644 index 0000000000..fcf0ff1d63 --- /dev/null +++ b/src/open_inwoner/utils/middleware.py @@ -0,0 +1,80 @@ +import abc + +from django.conf import settings +from django.shortcuts import redirect +from django.urls import reverse + +from furl import furl + + +def get_always_pass_prefixes() -> tuple[str, ...]: + return ( + settings.MEDIA_URL, + settings.PRIVATE_MEDIA_URL, + settings.STATIC_URL, + reverse("login"), + # prefixes from urls.py + "/admin/", + "/csp/", + "/reset/", + "/api/", + "/digid/", + "/eherkenning/", + "/oidc/", + "/digid-oidc/", + "/eherkenning-oidc/", + "/cms_login/", + "/cms_wizard/", + ) + + +class BaseForcedRedirectMiddleware(abc.ABC): + """ + Base class for middleware that redirects users to another view based on conditions + + Typical use case is forced registration, verification + """ + + redirect_url: str = None + extra_pass_prefixes: tuple[str, ...] = None + + def __init__(self, get_response): + self.get_response = get_response + + @abc.abstractmethod + def requires_redirect(self, request) -> bool: + return False + + def get_redirect_url(self, request) -> str: + if not self.redirect_url: + n = self.__class__.__name__ + raise ValueError( + f"configure {n}.redirect_url or override {n}.get_redirect_url()" + ) + # str() to resolve lazy reverses + return str(self.redirect_url) + + def get_pass_prefixes(self, request) -> tuple[str, ...]: + # extend and override for more + prefixes = get_always_pass_prefixes() + (self.get_redirect_url(request),) + if self.extra_pass_prefixes: + # str() to resolve lazy reverses + prefixes += tuple(map(str, self.extra_pass_prefixes)) + return prefixes + + def __call__(self, request): + if request.path.startswith(self.get_pass_prefixes(request)): + return self.get_response(request) + + if self.requires_redirect(request): + target = self.get_redirect_url(request) + if target: + url = furl(target) + # TODO do we need this? + if request.path != settings.LOGIN_REDIRECT_URL: + url.set({"next": request.path}) + url.args.update(request.GET) + return redirect(str(url)) + + # fallthrough + return self.get_response(request) diff --git a/src/open_inwoner/utils/url.py b/src/open_inwoner/utils/url.py index a4a9b2e3fc..341af7fffb 100644 --- a/src/open_inwoner/utils/url.py +++ b/src/open_inwoner/utils/url.py @@ -1,8 +1,22 @@ from django.conf import settings from django.contrib.sites.models import Site +from django.utils.encoding import iri_to_uri +from django.utils.http import url_has_allowed_host_and_scheme def build_absolute_url(path: str) -> str: domain = Site.objects.get_current().domain protocol = "https" if settings.IS_HTTPS else "http" return f"{protocol}://{domain}{path}" + + +def get_next_url(request, *, default: str = "") -> str: + next_url = request.GET.get("next") + if next_url and url_has_allowed_host_and_scheme( + url=next_url, + allowed_hosts=[ + request.get_host(), + ], + ): + return iri_to_uri(next_url) + return default