[#912] Refactored case-views authorisation checks into a mixin

Author: Bart van der Schoor

src/open_inwoner/accounts/views/cases.py

@@ -1,7 +1,7 @@
 import dataclasses
 from typing import List
 
-from django.contrib.auth.mixins import LoginRequiredMixin, UserPassesTestMixin
+from django.contrib.auth.mixins import AccessMixin
 from django.core.cache import cache
 from django.core.exceptions import PermissionDenied
 from django.http import Http404, StreamingHttpResponse
@@ -14,6 +14,7 @@
 
 from view_breadcrumbs import BaseBreadcrumbMixin
 
+from open_inwoner.openzaak.api_models import Zaak
 from open_inwoner.openzaak.cases import (
     fetch_case_information_objects,
     fetch_case_information_objects_for_case_and_info,
@@ -37,24 +38,52 @@
 from open_inwoner.openzaak.utils import filter_info_object_visibility
 
 
-class CaseListView(
-    BaseBreadcrumbMixin, LoginRequiredMixin, UserPassesTestMixin, TemplateView
-):
-    template_name = "pages/cases/list.html"
+class CaseAccessMixin(AccessMixin):
+    """
+    Shared authorisation check
 
-    @cached_property
-    def crumbs(self):
-        return [(_("Mijn aanvragen"), reverse("accounts:my_cases"))]
+    Base checks:
+    - user is authenticated
+    - user has a BSN
+
+    When retrieving a case :
+    - users BSN has a role for this case
+    """
+
+    case: Zaak = None
+
+    def dispatch(self, request, *args, **kwargs):
+        if not request.user.is_authenticated:
+            return self.handle_no_permission()
+
+        if not request.user.bsn:
+            return self.handle_no_permission()
+
+        if "object_id" in kwargs:
+            case_uuid = kwargs["object_id"]
+            self.case = fetch_single_case(case_uuid)
+
+            if self.case:
+                # check if we have a role in this case
+                if not fetch_roles_for_case_and_bsn(self.case.url, request.user.bsn):
+                    return self.handle_no_permission()
 
-    def test_func(self):
-        return self.request.user.bsn is not None
+        return super().dispatch(request, *args, **kwargs)
 
     def handle_no_permission(self):
         if self.request.user.is_authenticated:
             return redirect(reverse("root"))
 
         return super().handle_no_permission()
 
+
+class CaseListView(BaseBreadcrumbMixin, CaseAccessMixin, TemplateView):
+    template_name = "pages/cases/list.html"
+
+    @cached_property
+    def crumbs(self):
+        return [(_("Mijn aanvragen"), reverse("accounts:my_cases"))]
+
     def get_context_data(self, **kwargs):
         context = super().get_context_data(**kwargs)
 
@@ -124,9 +153,7 @@ class SimpleFile:
     url: str
 
 
-class CaseDetailView(
-    BaseBreadcrumbMixin, LoginRequiredMixin, UserPassesTestMixin, TemplateView
-):
+class CaseDetailView(BaseBreadcrumbMixin, CaseAccessMixin, TemplateView):
     template_name = "pages/cases/status.html"
 
     @cached_property
@@ -139,44 +166,30 @@ def crumbs(self):
             ),
         ]
 
-    def test_func(self):
-        return self.request.user.bsn is not None
-
-    def handle_no_permission(self):
-        if self.request.user.is_authenticated:
-            return redirect(reverse("root"))
-
-        return super().handle_no_permission()
-
     def get_context_data(self, **kwargs):
         context = super().get_context_data(**kwargs)
 
-        case_uuid = context["object_id"]
-        case = fetch_single_case(case_uuid)
+        if self.case:
+            documents = self.get_case_document_files(self.case)
 
-        if case:
-            # check if we have a role in this case
-            if not fetch_roles_for_case_and_bsn(case.url, self.request.user.bsn):
-                raise PermissionDenied()
-
-            documents = self.get_case_document_files(case)
-
-            statuses = fetch_status_history(case.url)
+            statuses = fetch_status_history(self.case.url)
             statuses.sort(key=lambda status: status.datum_status_gezet)
 
-            case_type = fetch_single_case_type(case.zaaktype)
-            status_types = fetch_status_types(case_type=case.zaaktype)
+            case_type = fetch_single_case_type(self.case.zaaktype)
+            status_types = fetch_status_types(case_type=self.case.zaaktype)
 
             status_types_mapping = {st.url: st for st in status_types}
             for status in statuses:
                 status_type = status_types_mapping[status.statustype]
                 status.statustype = status_type
 
             context["case"] = {
-                "identification": case.identificatie,
-                "start_date": case.startdatum,
-                "end_date": (case.einddatum if hasattr(case, "einddatum") else None),
-                "description": case.omschrijving,
+                "identification": self.case.identificatie,
+                "start_date": self.case.startdatum,
+                "end_date": (
+                    self.case.einddatum if hasattr(self.case, "einddatum") else None
+                ),
+                "description": self.case.omschrijving,
                 "type_description": (
                     case_type.omschrijving if case_type else _("No data available")
                 ),
@@ -248,34 +261,19 @@ def get_anchors(self, statuses, documents):
         return anchors
 
 
-class CaseDocumentDownloadView(LoginRequiredMixin, UserPassesTestMixin, View):
-    def test_func(self):
-        return self.request.user.bsn is not None
-
-    def handle_no_permission(self):
-        if self.request.user.is_authenticated:
-            return redirect(reverse("root"))
-
-        return super().handle_no_permission()
-
-    def get(self, *args, **kwargs):
-        case_uuid = kwargs["object_id"]
-        case = fetch_single_case(case_uuid)
-        if not case:
+class CaseDocumentDownloadView(CaseAccessMixin, View):
+    def get(self, request, *args, **kwargs):
+        if not self.case:
             raise Http404
 
-        # check if we have a role this case
-        if not fetch_roles_for_case_and_bsn(case.url, self.request.user.bsn):
-            raise PermissionDenied()
-
         info_object_uuid = kwargs["info_id"]
         info_object = fetch_single_information_object(uuid=info_object_uuid)
         if not info_object:
             raise Http404
 
         # check if this info_object belongs to this case
         if not fetch_case_information_objects_for_case_and_info(
-            case.url, info_object.url
+            self.case.url, info_object.url
         ):
             raise PermissionDenied()
 
@@ -286,6 +284,7 @@ def get(self, *args, **kwargs):
         ):
             raise PermissionDenied()
 
+        # retrieve and stream content
         content_stream = download_document(info_object.inhoud)
         if not content_stream:
             raise Http404
@@ -297,3 +296,7 @@ def get(self, *args, **kwargs):
         }
         response = StreamingHttpResponse(content_stream, headers=headers)
         return response
+
+    def handle_no_permission(self):
+        # plain error and no redirect
+        raise PermissionDenied()

src/open_inwoner/openzaak/tests/test_documents.py

@@ -197,26 +197,19 @@ def test_document_content_with_bad_confidentiality_is_http_403(self, m):
         )
         self.app.get(url, user=self.user, status=403)
 
-    def test_user_is_redirected_to_root_when_not_logged_in_via_digid(self, m):
+    def test_response_is_http_403_when_not_logged_in_via_digid(self, m):
         self._setUpMocks(m)
         user = UserFactory(
             first_name="",
             last_name="",
             login_type=LoginTypeChoices.default,
         )
-        response = self.app.get(self.informatie_object_file.url, user=user)
-
-        self.assertRedirects(response, reverse("root"))
+        self.app.get(self.informatie_object_file.url, user=user, status=403)
 
     def test_anonymous_user_has_no_access_to_download_page(self, m):
         self._setUpMocks(m)
         user = AnonymousUser()
-        response = self.app.get(self.informatie_object_file.url, user=user)
-
-        self.assertRedirects(
-            response,
-            f"{reverse('login')}?next={self.informatie_object_file.url}",
-        )
+        self.app.get(self.informatie_object_file.url, user=user, status=403)
 
     def test_no_data_is_retrieved_when_case_object_http_404(self, m):
         self._setUpOASMocks(m)

src/open_inwoner/openzaak/tests/test_statuses.py

@@ -357,14 +357,14 @@ def test_no_access_when_no_roles_are_found_for_user_bsn(self, m):
             # no roles found
             json=paginated_response([]),
         )
-        self.app.get(
+        response = self.app.get(
             reverse(
                 "accounts:case_status",
                 kwargs={"object_id": "d8bbdeb7-770f-4ca9-b1ea-77b4730bf67d"},
             ),
             user=self.user,
-            status=403,
         )
+        self.assertRedirects(response, reverse("root"))
 
     def test_no_data_is_retrieved_when_http_404(self, m):
         mock_service_oas_get(m, ZAKEN_ROOT, "zrc")