Change: restrict altering and creation of superusers to superusers only (#8837)

This commit is contained in:
shamoon 2025-01-20 11:57:22 -08:00 committed by GitHub
parent 475c231c6f
commit 41bcc12cc2
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
7 changed files with 194 additions and 1 deletions

View File

@ -252,7 +252,7 @@ permissions can be granted to limit access to certain parts of the UI (and corre
#### Superusers
Superusers can access all parts of the front and backend application as well as any and all objects.
Superusers can access all parts of the front and backend application as well as any and all objects. Superuser status can only be granted by another superuser.
#### Admin Status

View File

@ -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()
})
})

View File

@ -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() {

View File

@ -1,4 +1,7 @@
import types
from django.contrib.admin.sites import AdminSite
from django.contrib.auth.models import User
from django.test import TestCase
from django.utils import timezone
@ -6,6 +9,7 @@ from documents import index
from documents.admin import DocumentAdmin
from documents.models import Document
from documents.tests.utils import DirectoriesMixin
from paperless.admin import PaperlessUserAdmin
class TestDocumentAdmin(DirectoriesMixin, TestCase):
@ -64,3 +68,22 @@ class TestDocumentAdmin(DirectoriesMixin, TestCase):
created=timezone.make_aware(timezone.datetime(2020, 4, 12)),
)
self.assertEqual(self.doc_admin.created_(doc), "2020-04-12")
class TestPaperlessAdmin(DirectoriesMixin, TestCase):
def setUp(self) -> None:
super().setUp()
self.user_admin = PaperlessUserAdmin(model=User, admin_site=AdminSite())
def test_only_superuser_can_change_superuser(self):
non_superuser = User.objects.create(username="requestuser")
user = User.objects.create(username="test", is_superuser=False)
data = {"is_superuser": True}
form = self.user_admin.form(data, instance=user)
form.request = types.SimpleNamespace(user=non_superuser)
self.assertFalse(form.is_valid())
self.assertEqual(
form.errors.get("__all__"),
["Superuser status can only be changed by a superuser"],
)

View File

@ -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/"

53
src/paperless/admin.py Normal file
View File

@ -0,0 +1,53 @@
from django import forms
from django.contrib import admin
from django.contrib.auth.admin import UserAdmin
from django.contrib.auth.models import User
class PaperlessUserForm(forms.ModelForm):
"""
Custom form for the User model that adds validation to prevent non-superusers
from changing the superuser status of a user.
"""
class Meta:
model = User
fields = [
"username",
"first_name",
"last_name",
"email",
"is_staff",
"is_active",
"is_superuser",
"groups",
"user_permissions",
]
def clean(self):
cleaned_data = super().clean()
user_being_edited = self.instance
is_superuser = cleaned_data.get("is_superuser")
if (
not self.request.user.is_superuser
and is_superuser != user_being_edited.is_superuser
):
raise forms.ValidationError(
"Superuser status can only be changed by a superuser",
)
return cleaned_data
class PaperlessUserAdmin(UserAdmin):
form = PaperlessUserForm
def get_form(self, request, obj=None, **kwargs):
form = super().get_form(request, obj, **kwargs)
form.request = request
return form
admin.site.unregister(User)
admin.site.register(User, PaperlessUserAdmin)

View File

@ -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