fix empty notification middleware query param

Author: felixrindt

ephios/extra/middleware.py

@@ -22,17 +22,22 @@ def __init__(self, get_response):
         self.get_response = get_response
 
     def __call__(self, request):
-        from ephios.core.models import Notification
-
         response = self.get_response(request)
-        if NOTIFICATION_READ_PARAM_NAME in request.GET:
+        if getattr(request.user, "is_authenticated", False) and (
+            notification_id := request.GET.get(NOTIFICATION_READ_PARAM_NAME)
+        ):
+            from ephios.core.models import Notification
+
             try:
                 notification = Notification.objects.get(
-                    pk=request.GET[NOTIFICATION_READ_PARAM_NAME]
+                    pk=notification_id,
+                    user=request.user,
+                    read=False,
                 )
-                if notification.user == request.user and not notification.read:
-                    notification.read = True
-                    notification.save()
-            except Notification.DoesNotExist:
+            except (Notification.DoesNotExist, ValueError):
+                # ValueError if `notification_id` is not an integer
                 pass
+            else:
+                notification.read = True
+                notification.save(update_fields=["read"])
         return response

tests/core/test_notifications.py

@@ -7,6 +7,7 @@
 from ephios.core.models import AbstractParticipation, LocalParticipation, Notification
 from ephios.core.services.notifications.backends import EmailNotificationBackend
 from ephios.core.services.notifications.types import (
+    NOTIFICATION_READ_PARAM_NAME,
     ConsequenceApprovedNotification,
     ConsequenceDeniedNotification,
     CustomEventParticipantNotification,
@@ -201,11 +202,34 @@ def test_middleware_marks_notification_as_read(django_app, qualified_volunteer,
         user=planner, slug=ResponsibleParticipationAwaitsDispositionNotification.slug
     )
     assert not notification.read
-    response = django_app.get(notification.get_actions()[0][1], user=planner)
+    django_app.get(notification.get_actions()[0][1], user=planner)
     notification.refresh_from_db()
     assert notification.read
 
 
+def test_broken_middleware_query_param(django_app, planner):
+    django_app.get(
+        f"/?{NOTIFICATION_READ_PARAM_NAME}=1",
+        user=None,  # anonymous user
+    )
+    django_app.get(
+        f"/?{NOTIFICATION_READ_PARAM_NAME}",
+        user=planner,
+    )
+    django_app.get(
+        f"/?{NOTIFICATION_READ_PARAM_NAME}=123",
+        user=planner,
+    )
+    django_app.get(
+        f"/?{NOTIFICATION_READ_PARAM_NAME}=abc",
+        user=planner,
+    )
+    django_app.get(
+        f"/?{NOTIFICATION_READ_PARAM_NAME}=1&{NOTIFICATION_READ_PARAM_NAME}=2",
+        user=planner,
+    )
+
+
 def test_notification_doesnotexist_gets_deleted(django_app, qualified_volunteer, event):
     participation = LocalParticipation.objects.create(
         shift=event.shifts.first(),