From 1c2b06521ed01c542feac15b287c52acb5806d88 Mon Sep 17 00:00:00 2001 From: shamoon <4887959+shamoon@users.noreply.github.com> Date: Mon, 20 Jan 2025 09:41:50 -0800 Subject: [PATCH] Change: restrict altering and creation of superusers to superusers --- .../user-edit-dialog.component.spec.ts | 19 +++++ .../user-edit-dialog.component.ts | 5 ++ src/documents/tests/test_api_permissions.py | 74 +++++++++++++++++++ src/paperless/views.py | 19 +++++ 4 files changed, 117 insertions(+) diff --git a/src-ui/src/app/components/common/edit-dialog/user-edit-dialog/user-edit-dialog.component.spec.ts b/src-ui/src/app/components/common/edit-dialog/user-edit-dialog/user-edit-dialog.component.spec.ts index 560a259f4..9ffa1ea95 100644 --- a/src-ui/src/app/components/common/edit-dialog/user-edit-dialog/user-edit-dialog.component.spec.ts +++ b/src-ui/src/app/components/common/edit-dialog/user-edit-dialog/user-edit-dialog.component.spec.ts @@ -160,4 +160,23 @@ describe('UserEditDialogComponent', () => { }) expect(component.currentUserIsSuperUser).toBeTruthy() }) + + it('should disable superuser option if current user is not superuser', () => { + const control: AbstractControl = component.objectForm.get('is_superuser') + permissionsService.initialize([], { + id: 99, + username: 'user99', + is_superuser: false, + }) + component.ngOnInit() + expect(control.disabled).toBeTruthy() + + permissionsService.initialize([], { + id: 99, + username: 'user99', + is_superuser: true, + }) + component.ngOnInit() + expect(control.disabled).toBeFalsy() + }) }) diff --git a/src-ui/src/app/components/common/edit-dialog/user-edit-dialog/user-edit-dialog.component.ts b/src-ui/src/app/components/common/edit-dialog/user-edit-dialog/user-edit-dialog.component.ts index 819d075b5..7ba0f5ceb 100644 --- a/src-ui/src/app/components/common/edit-dialog/user-edit-dialog/user-edit-dialog.component.ts +++ b/src-ui/src/app/components/common/edit-dialog/user-edit-dialog/user-edit-dialog.component.ts @@ -60,6 +60,11 @@ export class UserEditDialogComponent ngOnInit(): void { super.ngOnInit() this.onToggleSuperUser() + if (!this.currentUserIsSuperUser) { + this.objectForm.get('is_superuser').disable() + } else { + this.objectForm.get('is_superuser').enable() + } } getCreateTitle() { diff --git a/src/documents/tests/test_api_permissions.py b/src/documents/tests/test_api_permissions.py index ef50c55f7..5de1887b2 100644 --- a/src/documents/tests/test_api_permissions.py +++ b/src/documents/tests/test_api_permissions.py @@ -681,6 +681,80 @@ class TestApiUser(DirectoriesMixin, APITestCase): ) self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN) + def test_only_superusers_can_create_or_alter_superuser_status(self): + """ + GIVEN: + - Existing user account + WHEN: + - API request is made to add a user account with superuser status + - API request is made to change superuser status + THEN: + - Only superusers can change superuser 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_superuser": 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_superuser": True, + }, + ), + 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_superuser": 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_superuser, True) + + response = self.client.patch( + f"{self.ENDPOINT}{user1.pk}/", + json.dumps( + { + "is_superuser": False, + }, + ), + 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_superuser, False) + class TestApiGroup(DirectoriesMixin, APITestCase): ENDPOINT = "/api/groups/" diff --git a/src/paperless/views.py b/src/paperless/views.py index b5142ed62..03721adf2 100644 --- a/src/paperless/views.py +++ b/src/paperless/views.py @@ -109,6 +109,25 @@ class UserViewSet(ModelViewSet): filterset_class = UserFilterSet ordering_fields = ("username",) + 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", + ) + 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 request.data.get("is_superuser") is not None + and request.data.get("is_superuser") != user_to_update.is_superuser + ): + return HttpResponseForbidden( + "Superuser status can only be changed by a superuser", + ) + return super().update(request, *args, **kwargs) + @action(detail=True, methods=["post"]) def deactivate_totp(self, request, pk=None): request_user = request.user