Skip to content

Commit c5fa35e

Browse files
svader0claude
andauthored
Restrict configuration permission assignment to superusers in the user API (#15296)
Assigning or changing a user's configuration permissions through the user API is now limited to superusers, matching how the serializer already treats the other privilege-bearing fields (staff and superuser status). Adds a regression test. No functional change for superusers. Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent 22e2519 commit c5fa35e

2 files changed

Lines changed: 150 additions & 0 deletions

File tree

dojo/user/api/serializer.py

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -155,6 +155,28 @@ def validate(self, data):
155155
msg = "Only superusers are allowed to add or edit staff users."
156156
raise ValidationError(msg)
157157

158+
# Configuration permissions are privilege-bearing: they grant the
159+
# ability to manage users, groups, tool configurations, and so on.
160+
# Only superusers may assign or change them, which keeps the set of
161+
# grantable capabilities under superuser control and mirrors the
162+
# is_staff / is_superuser guards above. The "configuration_permissions"
163+
# field maps to the "user_permissions" source.
164+
if "user_permissions" in data and not self.context["request"].user.is_superuser:
165+
requested_permissions = set(data.get("user_permissions") or [])
166+
if self.instance is not None:
167+
allowed_configuration_permissions = set(
168+
self.fields["configuration_permissions"].child_relation.queryset.all(),
169+
)
170+
current_permissions = (
171+
set(self.instance.user_permissions.all())
172+
& allowed_configuration_permissions
173+
)
174+
else:
175+
current_permissions = set()
176+
if requested_permissions != current_permissions:
177+
msg = "Only superusers are allowed to change configuration permissions."
178+
raise ValidationError(msg)
179+
158180
if self.context["request"].method in {"PATCH", "PUT"} and "password" in data:
159181
msg = "Update of password though API is not allowed"
160182
raise ValidationError(msg)

unittests/test_apiv2_user.py

Lines changed: 128 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -314,6 +314,134 @@ def test_superuser_can_set_is_staff_via_api(self):
314314
self.assertEqual(r.status_code, 200, r.content[:1000])
315315
self.assertTrue(User.objects.get(id=user_id).is_staff)
316316

317+
def test_non_superuser_cannot_grant_configuration_permissions_via_api(self):
318+
"""
319+
Only superusers may assign configuration permissions. A non-superuser,
320+
even one holding the delegated change_user permission, must not be able
321+
to grant configuration permissions to their own account or to another
322+
user, whether on update or at create time. Configuration permissions
323+
are privilege-bearing (managing users, groups, tool configurations, and
324+
so on), so assigning them is a superuser-only action.
325+
"""
326+
password = "testTEST1234!@#$"
327+
r = self.client.post(reverse("user-list"), {
328+
"username": "api-cfgperm-mgr",
329+
"email": "admin@dojo.com",
330+
"password": password,
331+
}, format="json")
332+
self.assertEqual(r.status_code, 201, r.content[:1000])
333+
mgr = User.objects.get(username="api-cfgperm-mgr")
334+
# Delegated user-manager: may change and create users, nothing more.
335+
mgr.user_permissions.add(
336+
Permission.objects.get(codename="change_user"),
337+
Permission.objects.get(codename="add_user"),
338+
)
339+
delete_user = Permission.objects.get(codename="delete_user")
340+
add_group = Permission.objects.get(codename="add_group")
341+
342+
token_resp = self.client.post(reverse("api-token-auth"), {
343+
"username": "api-cfgperm-mgr",
344+
"password": password,
345+
}, format="json")
346+
self.assertEqual(token_resp.status_code, 200, token_resp.content[:1000])
347+
mgr_client = APIClient()
348+
mgr_client.credentials(HTTP_AUTHORIZATION="Token " + token_resp.json()["token"])
349+
350+
# Self-escalation: granting themselves additional configuration
351+
# permissions must be rejected.
352+
r = mgr_client.patch("{}{}/".format(reverse("user-list"), mgr.id), {
353+
"configuration_permissions": [delete_user.id, add_group.id],
354+
}, format="json")
355+
self.assertEqual(r.status_code, 400, r.content[:1000])
356+
self.assertIn(
357+
"Only superusers are allowed to change configuration permissions.",
358+
r.content.decode("utf-8"),
359+
)
360+
self.assertFalse(User.objects.get(id=mgr.id).has_perm("auth.delete_user"))
361+
self.assertFalse(User.objects.get(id=mgr.id).has_perm("auth.add_group"))
362+
363+
# Target-escalation: granting configuration permissions to another user
364+
# must be rejected.
365+
r = self.client.post(reverse("user-list"), {
366+
"username": "api-cfgperm-target",
367+
"email": "admin@dojo.com",
368+
"password": password,
369+
}, format="json")
370+
self.assertEqual(r.status_code, 201, r.content[:1000])
371+
target_id = r.json()["id"]
372+
373+
r = mgr_client.patch("{}{}/".format(reverse("user-list"), target_id), {
374+
"configuration_permissions": [delete_user.id],
375+
}, format="json")
376+
self.assertEqual(r.status_code, 400, r.content[:1000])
377+
self.assertFalse(User.objects.get(id=target_id).has_perm("auth.delete_user"))
378+
379+
# Create-time escalation must also be rejected.
380+
r = mgr_client.post(reverse("user-list"), {
381+
"username": "api-cfgperm-on-create",
382+
"email": "admin@dojo.com",
383+
"password": password,
384+
"configuration_permissions": [delete_user.id],
385+
}, format="json")
386+
self.assertEqual(r.status_code, 400, r.content[:1000])
387+
self.assertFalse(User.objects.filter(username="api-cfgperm-on-create").exists())
388+
389+
def test_non_superuser_can_resend_unchanged_configuration_permissions(self):
390+
"""
391+
Negative control: the guard only fires when configuration permissions
392+
actually change, so a delegated user-manager can still PATCH their own
393+
account (including re-sending the configuration permissions they already
394+
hold) without being blocked.
395+
"""
396+
password = "testTEST1234!@#$"
397+
r = self.client.post(reverse("user-list"), {
398+
"username": "api-cfgperm-mgr2",
399+
"email": "admin@dojo.com",
400+
"password": password,
401+
}, format="json")
402+
self.assertEqual(r.status_code, 201, r.content[:1000])
403+
mgr = User.objects.get(username="api-cfgperm-mgr2")
404+
change_user = Permission.objects.get(codename="change_user")
405+
mgr.user_permissions.add(change_user)
406+
407+
token_resp = self.client.post(reverse("api-token-auth"), {
408+
"username": "api-cfgperm-mgr2",
409+
"password": password,
410+
}, format="json")
411+
self.assertEqual(token_resp.status_code, 200, token_resp.content[:1000])
412+
mgr_client = APIClient()
413+
mgr_client.credentials(HTTP_AUTHORIZATION="Token " + token_resp.json()["token"])
414+
415+
# Re-sending the same configuration permission the user already holds is
416+
# a no-op and must be allowed.
417+
r = mgr_client.patch("{}{}/".format(reverse("user-list"), mgr.id), {
418+
"configuration_permissions": [change_user.id],
419+
}, format="json")
420+
self.assertEqual(r.status_code, 200, r.content[:1000])
421+
422+
# Editing an unrelated field is likewise unaffected.
423+
r = mgr_client.patch("{}{}/".format(reverse("user-list"), mgr.id), {
424+
"first_name": "Renamed",
425+
}, format="json")
426+
self.assertEqual(r.status_code, 200, r.content[:1000])
427+
428+
def test_superuser_can_set_configuration_permissions_via_api(self):
429+
"""Positive control: a superuser may still assign configuration permissions."""
430+
r = self.client.post(reverse("user-list"), {
431+
"username": "api-cfgperm-grantable",
432+
"email": "admin@dojo.com",
433+
"password": "testTEST1234!@#$",
434+
}, format="json")
435+
self.assertEqual(r.status_code, 201, r.content[:1000])
436+
user_id = r.json()["id"]
437+
delete_user = Permission.objects.get(codename="delete_user")
438+
439+
r = self.client.patch("{}{}/".format(reverse("user-list"), user_id), {
440+
"configuration_permissions": [delete_user.id],
441+
}, format="json")
442+
self.assertEqual(r.status_code, 200, r.content[:1000])
443+
self.assertTrue(User.objects.get(id=user_id).has_perm("auth.delete_user"))
444+
317445
def test_user_reset_api_token_denies_global_owner_legacy(self):
318446
"""
319447
Legacy: Global_Role(role=Owner) is inert. Resetting another

0 commit comments

Comments
 (0)