[#2187] Always check is_authenticated, tolerate non-reverse-able urls

src/open_inwoner/accounts/middleware.py

@@ -2,10 +2,10 @@
 
 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 open_inwoner.utils.middleware import BaseConditionalUserRedirectMiddleware
 
 
-class NecessaryFieldsMiddleware(BaseForcedRedirectMiddleware):
+class NecessaryFieldsMiddleware(BaseConditionalUserRedirectMiddleware):
     """
     Redirect the user to a view to fill in necessary fields
     """
@@ -14,14 +14,10 @@ class NecessaryFieldsMiddleware(BaseForcedRedirectMiddleware):
 
     def requires_redirect(self, request) -> bool:
         user = request.user
-        return (
-            user.is_authenticated
-            and user.require_necessary_fields()
-            and profile_page_is_published()
-        )
+        return user.require_necessary_fields() and profile_page_is_published()
 
 
-class EmailVerificationMiddleware(BaseForcedRedirectMiddleware):
+class EmailVerificationMiddleware(BaseConditionalUserRedirectMiddleware):
     """
     Redirect the user to a view to verify email
     """
@@ -32,8 +28,7 @@ class EmailVerificationMiddleware(BaseForcedRedirectMiddleware):
     def requires_redirect(self, request) -> bool:
         user = request.user
         return (
-            user.is_authenticated
-            and not user.has_verified_email()
+            not user.has_verified_email()
             and profile_page_is_published()
             and SiteConfiguration.get_solo().email_verification_required
         )

src/open_inwoner/kvk/middleware.py

@@ -3,23 +3,22 @@
 from django.urls import NoReverseMatch, reverse
 
 from open_inwoner.kvk.branches import kvk_branch_selected_done
-from open_inwoner.utils.middleware import BaseForcedRedirectMiddleware
+from open_inwoner.utils.middleware import BaseConditionalUserRedirectMiddleware
 
 logger = logging.getLogger(__name__)
 
 
-class KvKLoginMiddleware(BaseForcedRedirectMiddleware):
+class KvKLoginMiddleware(BaseConditionalUserRedirectMiddleware):
     """Redirect authenticated eHerkenning users to select a company branch"""
 
     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)
+        return user.is_eherkenning_user and not kvk_branch_selected_done(
+            request.session
         )
 
     def get_redirect_url(self, request):
+        # TODO we don't need this
         try:
             return reverse("kvk:branches")
         except NoReverseMatch:

src/open_inwoner/utils/middleware.py

@@ -3,6 +3,7 @@
 from django.conf import settings
 from django.shortcuts import redirect
 from django.urls import reverse
+from django.urls.exceptions import NoReverseMatch
 
 from furl import furl
 
@@ -28,7 +29,7 @@ def get_always_pass_prefixes() -> tuple[str, ...]:
     )
 
 
-class BaseForcedRedirectMiddleware(abc.ABC):
+class BaseConditionalUserRedirectMiddleware(abc.ABC):
     """
     Base class for middleware that redirects users to another view based on conditions
 
@@ -46,28 +47,47 @@ def requires_redirect(self, request) -> bool:
         return False
 
     def get_redirect_url(self, request) -> str:
-        if not self.redirect_url:
+        # redirect_url could be reverse_lazy
+        if self.redirect_url is None:
             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)
+        return 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))
+        prefixes = get_always_pass_prefixes()
+        # str() to resolve lazy reverses
+        prefixes += (str(self.get_redirect_url(request)),)
+        prefixes += self.get_extra_pass_prefixes()
         return prefixes
 
+    def get_extra_pass_prefixes(self) -> tuple[str, ...]:
+        prefixes = []
+        if self.extra_pass_prefixes:
+            for e in self.extra_pass_prefixes:
+                try:
+                    # str() to resolve lazy reverses
+                    prefixes.append(str(e))
+                except NoReverseMatch:
+                    # support testing without cms app mounted
+                    pass
+        return tuple(prefixes)
+
     def __call__(self, request):
+        # we only trigger for authenticated users
+        if not request.user.is_authenticated:
+            return self.get_response(request)
+
+        try:
+            target = str(self.get_redirect_url(request))
+        except NoReverseMatch:
+            return self.get_response(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?