From 06b2d5102c187718fbd0bfc480c766d2bec0bb45 Mon Sep 17 00:00:00 2001 From: shamoon <4887959+shamoon@users.noreply.github.com> Date: Sun, 15 Mar 2026 17:13:08 -0700 Subject: [PATCH] Fix GHSA-59xh-5vwx-4c4q --- src/documents/tests/test_api_permissions.py | 72 +++++++++++++++++++++ src/paperless/views.py | 53 +++++++++++++-- 2 files changed, 119 insertions(+), 6 deletions(-) diff --git a/src/documents/tests/test_api_permissions.py b/src/documents/tests/test_api_permissions.py index 31b860745..00a613cff 100644 --- a/src/documents/tests/test_api_permissions.py +++ b/src/documents/tests/test_api_permissions.py @@ -888,6 +888,19 @@ class TestApiUser(DirectoriesMixin, APITestCase): self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN) + response = self.client.post( + f"{self.ENDPOINT}", + json.dumps( + { + "username": "user4", + "is_superuser": "true", + }, + ), + content_type="application/json", + ) + + self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN) + self.client.force_authenticate(user2) response = self.client.patch( @@ -920,6 +933,65 @@ class TestApiUser(DirectoriesMixin, APITestCase): returned_user1 = User.objects.get(pk=user1.pk) self.assertEqual(returned_user1.is_superuser, False) + def test_only_superusers_can_create_or_alter_staff_status(self): + """ + GIVEN: + - Existing user account + WHEN: + - API request is made to add a user account with staff status + - API request is made to change staff status + THEN: + - Only superusers can change staff status + """ + + user1 = User.objects.create_user(username="user1") + user1.user_permissions.add(*Permission.objects.all()) + user2 = User.objects.create_superuser(username="user2") + + self.client.force_authenticate(user1) + + response = self.client.patch( + f"{self.ENDPOINT}{user1.pk}/", + json.dumps( + { + "is_staff": "true", + }, + ), + content_type="application/json", + ) + + self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN) + + response = self.client.post( + f"{self.ENDPOINT}", + json.dumps( + { + "username": "user3", + "is_staff": 1, + }, + ), + content_type="application/json", + ) + + self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN) + + self.client.force_authenticate(user2) + + response = self.client.patch( + f"{self.ENDPOINT}{user1.pk}/", + json.dumps( + { + "is_staff": True, + }, + ), + content_type="application/json", + ) + + self.assertEqual(response.status_code, status.HTTP_200_OK) + + returned_user1 = User.objects.get(pk=user1.pk) + self.assertEqual(returned_user1.is_staff, True) + class TestApiGroup(DirectoriesMixin, APITestCase): ENDPOINT = "/api/groups/" diff --git a/src/paperless/views.py b/src/paperless/views.py index e79c0e668..2a2ee9518 100644 --- a/src/paperless/views.py +++ b/src/paperless/views.py @@ -25,6 +25,8 @@ from drf_spectacular.utils import extend_schema_view from rest_framework.authtoken.models import Token from rest_framework.authtoken.views import ObtainAuthToken from rest_framework.decorators import action +from rest_framework.exceptions import ValidationError +from rest_framework.fields import BooleanField from rest_framework.filters import OrderingFilter from rest_framework.generics import GenericAPIView from rest_framework.pagination import PageNumberPagination @@ -103,6 +105,7 @@ class FaviconView(View): class UserViewSet(ModelViewSet): + _BOOL_NOT_PROVIDED = object() model = User queryset = User.objects.exclude( @@ -116,27 +119,65 @@ class UserViewSet(ModelViewSet): filterset_class = UserFilterSet ordering_fields = ("username",) + @staticmethod + def _parse_requested_bool(data, key: str): + if key not in data: + return UserViewSet._BOOL_NOT_PROVIDED + try: + return BooleanField().to_internal_value(data.get(key)) + except ValidationError: + # Let serializer validation report invalid values as 400 responses + return UserViewSet._BOOL_NOT_PROVIDED + def create(self, request, *args, **kwargs): - if not request.user.is_superuser and request.data.get("is_superuser") is True: - return HttpResponseForbidden( - "Superuser status can only be granted by a superuser", - ) + requested_is_superuser = self._parse_requested_bool( + request.data, + "is_superuser", + ) + requested_is_staff = self._parse_requested_bool(request.data, "is_staff") + + if not request.user.is_superuser: + if requested_is_superuser is True: + return HttpResponseForbidden( + "Superuser status can only be granted by a superuser", + ) + if requested_is_staff is True: + return HttpResponseForbidden( + "Staff status can only be granted by a superuser", + ) + return super().create(request, *args, **kwargs) def update(self, request, *args, **kwargs): user_to_update: User = self.get_object() + if not request.user.is_superuser and user_to_update.is_superuser: return HttpResponseForbidden( "Superusers can only be modified by other superusers", ) + + requested_is_superuser = self._parse_requested_bool( + request.data, + "is_superuser", + ) + requested_is_staff = self._parse_requested_bool(request.data, "is_staff") + if ( not request.user.is_superuser - and request.data.get("is_superuser") is not None - and request.data.get("is_superuser") != user_to_update.is_superuser + and requested_is_superuser is not self._BOOL_NOT_PROVIDED + and requested_is_superuser != user_to_update.is_superuser ): return HttpResponseForbidden( "Superuser status can only be changed by a superuser", ) + if ( + not request.user.is_superuser + and requested_is_staff is not self._BOOL_NOT_PROVIDED + and requested_is_staff != user_to_update.is_staff + ): + return HttpResponseForbidden( + "Staff status can only be changed by a superuser", + ) return super().update(request, *args, **kwargs) @extend_schema(