changed queryset and added tests and refactored

app/content/models/user.py

@@ -16,7 +16,7 @@
 from app.common.enums import AdminGroup, Groups
 from app.common.enums import NativeGroupType as GroupType
 from app.common.enums import NativeMembershipType as MembershipType
-from app.common.permissions import check_has_access
+from app.common.permissions import check_has_access, is_admin_user
 from app.util.models import BaseModel, OptionalImage
 from app.util.utils import disable_for_loaddata, now
 
@@ -221,10 +221,17 @@ def has_object_write_permission(self, request):
         )
 
     def has_object_retrieve_permission(self, request):
-        return self == request.user or check_has_access(
-            self.read_access,
-            request,
-        )
+        is_public = self.public_profile
+        is_user = self == request.user
+        is_admin = is_admin_user(request)
+
+        if is_public and check_has_access(self.read_access, request):
+            return True
+
+        if not is_public and (is_user or is_admin):
+            return True
+
+        return False
 
     def has_object_read_permission(self, request):
         return self.has_object_retrieve_permission(request)

app/content/views/user.py

@@ -4,6 +4,7 @@
 from django_filters.rest_framework import DjangoFilterBackend
 from rest_framework import filters, status
 from rest_framework.decorators import action
+from rest_framework.exceptions import PermissionDenied
 from rest_framework.response import Response
 
 from app.badge.models import Badge, UserBadge
@@ -43,10 +44,11 @@
 from app.kontres.models.reservation import Reservation
 from app.kontres.serializer.reservation_seralizer import ReservationSerializer
 from app.util.export_user_data import export_user_data
+from app.util.mixins import APIErrorsMixin
 from app.util.utils import CaseInsensitiveBooleanQueryParam
 
 
-class UserViewSet(BaseViewSet, ActionMixin):
+class UserViewSet(BaseViewSet, ActionMixin, APIErrorsMixin):
     """API endpoint to display one user"""
 
     serializer_class = UserSerializer
@@ -71,22 +73,21 @@ def get_serializer_class(self):
         return super().get_serializer_class()
 
     def retrieve(self, request, pk, *args, **kwargs):
-        try:
-            user = self._get_user(request, pk)
-
-            self.check_object_permissions(self.request, user)
+        if pk == "me":
+            user = request.user
+            if not user:
+                raise PermissionDenied("Du må være logget inn for å se din egen bruker")
+        else:
+            user = get_object_or_404(User, user_id=pk)
 
-            serializer = DefaultUserSerializer(user)
+        self.check_object_permissions(self.request, user)
 
-            if is_admin_user(self.request) or user == request.user:
-                serializer = UserSerializer(user, context={"request": self.request})
+        serializer = DefaultUserSerializer(user)
 
-            return Response(serializer.data, status=status.HTTP_200_OK)
+        if is_admin_user(self.request) or user == request.user:
+            serializer = UserSerializer(user, context={"request": self.request})
 
-        except Exception:
-            return Response(
-                {"message": "Error"}, status=status.HTTP_500_INTERNAL_SERVER_ERROR
-            )
+        return Response(serializer.data, status=status.HTTP_200_OK)
 
     def create(self, request, *args, **kwargs):
         serializer = UserCreateSerializer(data=self.request.data)
@@ -149,12 +150,21 @@ def destroy(self, request, pk, *args, **kwargs):
 
     def _get_user(self, request, pk):
         if pk == "me":
+            if not request.user:
+                raise PermissionDenied("Du må være logget inn for å se din egen bruker")
             return request.user
-        return get_object_or_404(User, user_id=pk)
+
+        user = get_object_or_404(User, user_id=pk)
+        if not user.public_profile and not is_admin_user(request):
+            raise PermissionDenied("Brukeren har ikke offentlig profil")
+        return user
 
     @action(detail=False, methods=["post"], url_path="me/slack")
     def connect_to_slack(self, request, *args, **kwargs):
         user = self.request.user
+        if not user:
+            raise PermissionDenied("Du må være logget inn for å koble til Slack")
+
         self.check_object_permissions(self.request, user)
 
         code = request.data.get("code", None)
@@ -177,6 +187,9 @@ def connect_to_slack(self, request, *args, **kwargs):
 
     @action(detail=False, methods=["get"], url_path="me/permissions")
     def get_user_permissions(self, request, *args, **kwargs):
+        if not request.user:
+            raise PermissionDenied("Du må være logget inn for å se dine tillatelser")
+
         try:
             serializer = UserPermissionsSerializer(
                 request.user, context={"request": request}
@@ -295,6 +308,9 @@ def get_or_post_detail_user_badges(self, request, *args, **kwargs):
 
     @action(detail=False, methods=["get"], url_path="me/strikes")
     def get_user_strikes(self, request, *args, **kwargs):
+        if not request.user:
+            raise PermissionDenied("Du må være logget inn for å se dine prikker")
+
         strikes = request.user.strikes.active()
         serializer = UserInfoStrikeSerializer(
             instance=strikes, many=True, context={"request": request}
@@ -312,6 +328,9 @@ def get_user_detail_strikes(self, request, *args, **kwargs):
 
     @action(detail=False, methods=["get"], url_path="me/events")
     def get_user_events(self, request, *args, **kwargs):
+        if not request.user:
+            raise PermissionDenied("Du må være logget inn for å se dine arrangementer")
+
         filter_field = self.request.query_params.get("expired")
         event_has_ended = CaseInsensitiveBooleanQueryParam(filter_field).value
 
@@ -332,6 +351,9 @@ def get_user_events(self, request, *args, **kwargs):
 
     @action(detail=False, methods=["get"], url_path="me/forms")
     def get_user_forms(self, request, *args, **kwargs):
+        if not request.user:
+            raise PermissionDenied("Du må være logget inn for å se dine skjemaer")
+
         forms = request.user.forms
 
         filter_field = request.query_params.get("unanswered")
@@ -403,6 +425,9 @@ def declineTIHLDEMember(self, request, *args, **kwargs):
 
     @action(detail=False, methods=["get"], url_path="me/data")
     def export_user_data(self, request, *args, **kwargs):
+        if not request.user:
+            raise PermissionDenied("Du må være logget inn for å eksportere dine data")
+
         export_successfull = export_user_data(request, request.user)
 
         if export_successfull:
@@ -421,6 +446,9 @@ def export_user_data(self, request, *args, **kwargs):
     @action(detail=False, methods=["get"], url_path="me/reservations")
     def get_user_reservations(self, request, *args, **kwargs):
         user = request.user
+        if not user:
+            raise PermissionDenied("Du må være logget inn for å se dine reservasjoner")
+
         reservations = Reservation.objects.filter(author=user).order_by("start_time")
         serializer = ReservationSerializer(
             reservations, many=True, context={"request": request}

app/tests/content/test_user_integration.py

@@ -199,6 +199,30 @@ def test_user_actions_self(url, status_code, member, api_client):
     assert response.status_code == status_code
 
 
+@pytest.mark.django_db
+@pytest.mark.parametrize(
+    ("url", "status_code"),
+    [
+        ("/", status.HTTP_403_FORBIDDEN),
+        ("/memberships/", status.HTTP_403_FORBIDDEN),
+        ("/memberships-with-fines/", status.HTTP_403_FORBIDDEN),
+        ("/membership-histories/", status.HTTP_403_FORBIDDEN),
+        ("/badges/", status.HTTP_403_FORBIDDEN),
+        ("/events/", status.HTTP_403_FORBIDDEN),
+        ("/forms/", status.HTTP_403_FORBIDDEN),
+        ("/strikes/", status.HTTP_403_FORBIDDEN),
+        ("/data/", status.HTTP_403_FORBIDDEN),
+        ("/permissions/", status.HTTP_403_FORBIDDEN),
+    ],
+)
+def test_user_actions_self_as_anonymous_user(url, status_code, default_client):
+    """An anonymous user should not be able to access any user actions."""
+
+    url = f"{API_USER_BASE_URL}me{url}"
+    response = default_client.get(url)
+    assert response.status_code == status_code
+
+
 @pytest.mark.parametrize(
     ("url", "status_code"),
     [
@@ -246,10 +270,10 @@ def test_user_actions_get_user_as_member(url, status_code, user, member, api_cli
 @pytest.mark.parametrize(
     ("url", "status_code"),
     [
-        ("/", status.HTTP_403_FORBIDDEN),
-        ("/memberships/", status.HTTP_403_FORBIDDEN),
-        ("/membership-histories/", status.HTTP_403_FORBIDDEN),
-        ("/badges/", status.HTTP_403_FORBIDDEN),
+        ("/", status.HTTP_200_OK),
+        ("/memberships/", status.HTTP_200_OK),
+        ("/membership-histories/", status.HTTP_200_OK),
+        ("/badges/", status.HTTP_200_OK),
     ],
 )
 def test_user_actions_get_user_as_not_member(
@@ -309,7 +333,7 @@ def test_list_private_users_as_member(member, api_client):
     (
         AdminGroup.HS,
         AdminGroup.INDEX,
-    )
+    ),
 )
 def test_list_private_users_as_admin_user(member, api_client, group):
     """An admin should be able to list all users that is private."""
@@ -322,10 +346,66 @@ def test_list_private_users_as_admin_user(member, api_client, group):
     assert response.data["count"] == 1
 
 
-def test_list_as_member_of_admin_group(admin_user, api_client):
-    """An admin should be able to list all users."""
-    client = api_client(user=admin_user)
-    response = client.get(API_USER_BASE_URL)
+@pytest.mark.django_db
+def test_retrieve_private_user_as_anoymous_user(default_client, member):
+    """An anonymous user should not be able to retrieve a user that is private."""
+    member.public_profile = False
+    member.save()
+
+    url = _get_user_detail_url(member)
+    response = default_client.get(url)
+
+    assert response.status_code == status.HTTP_403_FORBIDDEN
+
+
+@pytest.mark.django_db
+def test_retrieve_public_person_as_anonymous_user(member, api_client):
+    """An anonymous user should be able to retrieve a user that is public."""
+    url = _get_user_detail_url(member)
+    response = api_client().get(url)
+
+    assert response.status_code == status.HTTP_200_OK
+
+
+@pytest.mark.django_db
+def test_retrieve_private_user_as_member(member, api_client):
+    """A member should not be able to retrieve a user that is private."""
+    user = UserFactory(public_profile=False)
+
+    url = _get_user_detail_url(user)
+    response = api_client(user=member).get(url)
+
+    assert response.status_code == status.HTTP_403_FORBIDDEN
+
+
+@pytest.mark.django_db
+@pytest.mark.parametrize(
+    "group",
+    (
+        AdminGroup.HS,
+        AdminGroup.INDEX,
+    ),
+)
+def test_retrieve_private_user_as_admin_user(member, api_client, group):
+    """An admin should be able to retrieve a user that is private."""
+    member.public_profile = False
+    member.save()
+    add_user_to_group_with_name(member, group)
+
+    url = _get_user_detail_url(member)
+    response = api_client(user=member).get(url)
+
+    assert response.status_code == status.HTTP_200_OK
+
+
+@pytest.mark.django_db
+def test_retrieve_own_private_user_as_me(member, api_client):
+    """A user should be able to retrieve self even if private, with the pk 'me'."""
+    member.public_profile = False
+    member.save()
+
+    url = f"{API_USER_BASE_URL}me/"
+    response = api_client(user=member).get(url)
 
     assert response.status_code == status.HTTP_200_OK