From 273cdd0b312324f05fcbebc5996ddfc25ca42cda Mon Sep 17 00:00:00 2001
From: Julian Baumann <jeriox@users.noreply.github.com>
Date: Thu, 4 Apr 2024 13:59:28 +0200
Subject: [PATCH] fix permissions for API user endpoint (#1256)

---
 ephios/api/permissions.py | 23 +++++++++++++++++++++++
 ephios/api/views/users.py |  9 +++------
 tests/api/test_user.py    | 10 ++++++++++
 3 files changed, 36 insertions(+), 6 deletions(-)
 create mode 100644 ephios/api/permissions.py

diff --git a/ephios/api/permissions.py b/ephios/api/permissions.py
new file mode 100644
index 000000000..134ffdf50
--- /dev/null
+++ b/ephios/api/permissions.py
@@ -0,0 +1,23 @@
+from rest_framework.permissions import DjangoModelPermissions, DjangoObjectPermissions
+
+
+class ViewPermissionsMixin:
+    # DjangoModelPermissions and DjangoObjectPermissions only check permissions for write/unsafe operations.
+    # This mixin adds permissions for read/safe operations.
+    perms_map = {
+        "GET": ["%(app_label)s.view_%(model_name)s"],
+        "OPTIONS": [],
+        "HEAD": [],
+        "POST": ["%(app_label)s.add_%(model_name)s"],
+        "PUT": ["%(app_label)s.change_%(model_name)s"],
+        "PATCH": ["%(app_label)s.change_%(model_name)s"],
+        "DELETE": ["%(app_label)s.delete_%(model_name)s"],
+    }
+
+
+class ViewPermissions(ViewPermissionsMixin, DjangoModelPermissions):
+    pass
+
+
+class ViewObjectPermissions(ViewPermissionsMixin, DjangoObjectPermissions):
+    pass
diff --git a/ephios/api/views/users.py b/ephios/api/views/users.py
index 84d79fe44..9e92d0eed 100644
--- a/ephios/api/views/users.py
+++ b/ephios/api/views/users.py
@@ -8,14 +8,13 @@
 from rest_framework.filters import SearchFilter
 from rest_framework.generics import RetrieveAPIView
 from rest_framework.mixins import RetrieveModelMixin
-from rest_framework.permissions import DjangoObjectPermissions
 from rest_framework.relations import SlugRelatedField
 from rest_framework.schemas.openapi import AutoSchema
 from rest_framework.serializers import ModelSerializer
 from rest_framework.viewsets import GenericViewSet
-from rest_framework_guardian.filters import ObjectPermissionsFilter
 
 from ephios.api.filters import ParticipationFilterSet, ParticipationPermissionFilter
+from ephios.api.permissions import ViewPermissions
 from ephios.api.views.events import ParticipationSerializer
 from ephios.core.models import LocalParticipation, Qualification, UserProfile
 from ephios.core.services.qualification import collect_all_included_qualifications
@@ -78,23 +77,21 @@ def get_object(self):
 class UserViewSet(viewsets.ReadOnlyModelViewSet):
     serializer_class = UserProfileSerializer
     queryset = UserProfile.objects.all()
-    permission_classes = [IsAuthenticatedOrTokenHasScope, DjangoObjectPermissions]
+    permission_classes = [IsAuthenticatedOrTokenHasScope, ViewPermissions]
     required_scopes = ["CONFIDENTIAL_READ"]
     search_fields = ["display_name", "email"]
 
     filter_backends = [
         DjangoFilterBackend,
         SearchFilter,
-        ObjectPermissionsFilter,
     ]
 
 
 class UserByMailView(RetrieveModelMixin, GenericViewSet):
     serializer_class = UserProfileSerializer
     queryset = UserProfile.objects.all()
-    permission_classes = [IsAuthenticatedOrTokenHasScope, DjangoObjectPermissions]
+    permission_classes = [IsAuthenticatedOrTokenHasScope, ViewPermissions]
     required_scopes = ["CONFIDENTIAL_READ"]
-    filter_backends = [ObjectPermissionsFilter]
     lookup_url_kwarg = "email"
     lookup_field = "email"
     lookup_value_regex = "[^/]+"  # customize to allow dots (".") in the lookup value
diff --git a/tests/api/test_user.py b/tests/api/test_user.py
index 8b671092e..1706dbe17 100644
--- a/tests/api/test_user.py
+++ b/tests/api/test_user.py
@@ -19,3 +19,13 @@ def test_api_user_profile_by_email(django_app, superuser):
         user=superuser,
     )
     assert superuser.email in response
+
+
+def test_manager_can_view_users(django_app, groups, manager):
+    response = django_app.get(reverse("api:userprofile-list"), user=manager)
+    assert manager.email in response
+
+
+def test_volunter_cannot_view_users(django_app, groups, volunteer):
+    response = django_app.get(reverse("api:userprofile-list"), user=volunteer, expect_errors=True)
+    assert response.status_code == 403