core: add additional RBAC permission to restrict setting the superuser status on groups (#12900)
Signed-off-by: Jens Langhammer <jens@goauthentik.io>
This commit is contained in:
@ -4,6 +4,7 @@ from json import loads
|
||||
|
||||
from django.db.models import Prefetch
|
||||
from django.http import Http404
|
||||
from django.utils.translation import gettext as _
|
||||
from django_filters.filters import CharFilter, ModelMultipleChoiceFilter
|
||||
from django_filters.filterset import FilterSet
|
||||
from drf_spectacular.utils import (
|
||||
@ -81,9 +82,37 @@ class GroupSerializer(ModelSerializer):
|
||||
if not self.instance or not parent:
|
||||
return parent
|
||||
if str(parent.group_uuid) == str(self.instance.group_uuid):
|
||||
raise ValidationError("Cannot set group as parent of itself.")
|
||||
raise ValidationError(_("Cannot set group as parent of itself."))
|
||||
return parent
|
||||
|
||||
def validate_is_superuser(self, superuser: bool):
|
||||
"""Ensure that the user creating this group has permissions to set the superuser flag"""
|
||||
request: Request = self.context.get("request", None)
|
||||
if not request:
|
||||
return superuser
|
||||
# If we're updating an instance, and the state hasn't changed, we don't need to check perms
|
||||
if self.instance and superuser == self.instance.is_superuser:
|
||||
return superuser
|
||||
user: User = request.user
|
||||
perm = (
|
||||
"authentik_core.enable_group_superuser"
|
||||
if superuser
|
||||
else "authentik_core.disable_group_superuser"
|
||||
)
|
||||
has_perm = user.has_perm(perm)
|
||||
if self.instance and not has_perm:
|
||||
has_perm = user.has_perm(perm, self.instance)
|
||||
if not has_perm:
|
||||
raise ValidationError(
|
||||
_(
|
||||
(
|
||||
"User does not have permission to set "
|
||||
"superuser status to {superuser_status}."
|
||||
).format_map({"superuser_status": superuser})
|
||||
)
|
||||
)
|
||||
return superuser
|
||||
|
||||
class Meta:
|
||||
model = Group
|
||||
fields = [
|
||||
|
||||
26
authentik/core/migrations/0043_alter_group_options.py
Normal file
26
authentik/core/migrations/0043_alter_group_options.py
Normal file
@ -0,0 +1,26 @@
|
||||
# Generated by Django 5.0.11 on 2025-01-30 23:55
|
||||
|
||||
from django.db import migrations
|
||||
|
||||
|
||||
class Migration(migrations.Migration):
|
||||
|
||||
dependencies = [
|
||||
("authentik_core", "0042_authenticatedsession_authentik_c_expires_08251d_idx_and_more"),
|
||||
]
|
||||
|
||||
operations = [
|
||||
migrations.AlterModelOptions(
|
||||
name="group",
|
||||
options={
|
||||
"permissions": [
|
||||
("add_user_to_group", "Add user to group"),
|
||||
("remove_user_from_group", "Remove user from group"),
|
||||
("enable_group_superuser", "Enable superuser status"),
|
||||
("disable_group_superuser", "Disable superuser status"),
|
||||
],
|
||||
"verbose_name": "Group",
|
||||
"verbose_name_plural": "Groups",
|
||||
},
|
||||
),
|
||||
]
|
||||
@ -204,6 +204,8 @@ class Group(SerializerModel, AttributesMixin):
|
||||
permissions = [
|
||||
("add_user_to_group", _("Add user to group")),
|
||||
("remove_user_from_group", _("Remove user from group")),
|
||||
("enable_group_superuser", _("Enable superuser status")),
|
||||
("disable_group_superuser", _("Disable superuser status")),
|
||||
]
|
||||
|
||||
def __str__(self):
|
||||
|
||||
@ -4,7 +4,7 @@ from django.urls.base import reverse
|
||||
from guardian.shortcuts import assign_perm
|
||||
from rest_framework.test import APITestCase
|
||||
|
||||
from authentik.core.models import Group, User
|
||||
from authentik.core.models import Group
|
||||
from authentik.core.tests.utils import create_test_admin_user, create_test_user
|
||||
from authentik.lib.generators import generate_id
|
||||
|
||||
@ -14,7 +14,7 @@ class TestGroupsAPI(APITestCase):
|
||||
|
||||
def setUp(self) -> None:
|
||||
self.login_user = create_test_user()
|
||||
self.user = User.objects.create(username="test-user")
|
||||
self.user = create_test_user()
|
||||
|
||||
def test_list_with_users(self):
|
||||
"""Test listing with users"""
|
||||
@ -109,3 +109,57 @@ class TestGroupsAPI(APITestCase):
|
||||
},
|
||||
)
|
||||
self.assertEqual(res.status_code, 400)
|
||||
|
||||
def test_superuser_no_perm(self):
|
||||
"""Test creating a superuser group without permission"""
|
||||
assign_perm("authentik_core.add_group", self.login_user)
|
||||
self.client.force_login(self.login_user)
|
||||
res = self.client.post(
|
||||
reverse("authentik_api:group-list"),
|
||||
data={"name": generate_id(), "is_superuser": True},
|
||||
)
|
||||
self.assertEqual(res.status_code, 400)
|
||||
self.assertJSONEqual(
|
||||
res.content,
|
||||
{"is_superuser": ["User does not have permission to set superuser status to True."]},
|
||||
)
|
||||
|
||||
def test_superuser_update_no_perm(self):
|
||||
"""Test updating a superuser group without permission"""
|
||||
group = Group.objects.create(name=generate_id(), is_superuser=True)
|
||||
assign_perm("view_group", self.login_user, group)
|
||||
assign_perm("change_group", self.login_user, group)
|
||||
self.client.force_login(self.login_user)
|
||||
res = self.client.patch(
|
||||
reverse("authentik_api:group-detail", kwargs={"pk": group.pk}),
|
||||
data={"is_superuser": False},
|
||||
)
|
||||
self.assertEqual(res.status_code, 400)
|
||||
self.assertJSONEqual(
|
||||
res.content,
|
||||
{"is_superuser": ["User does not have permission to set superuser status to False."]},
|
||||
)
|
||||
|
||||
def test_superuser_update_no_change(self):
|
||||
"""Test updating a superuser group without permission
|
||||
and without changing the superuser status"""
|
||||
group = Group.objects.create(name=generate_id(), is_superuser=True)
|
||||
assign_perm("view_group", self.login_user, group)
|
||||
assign_perm("change_group", self.login_user, group)
|
||||
self.client.force_login(self.login_user)
|
||||
res = self.client.patch(
|
||||
reverse("authentik_api:group-detail", kwargs={"pk": group.pk}),
|
||||
data={"name": generate_id(), "is_superuser": True},
|
||||
)
|
||||
self.assertEqual(res.status_code, 200)
|
||||
|
||||
def test_superuser_create(self):
|
||||
"""Test creating a superuser group with permission"""
|
||||
assign_perm("authentik_core.add_group", self.login_user)
|
||||
assign_perm("authentik_core.enable_group_superuser", self.login_user)
|
||||
self.client.force_login(self.login_user)
|
||||
res = self.client.post(
|
||||
reverse("authentik_api:group-list"),
|
||||
data={"name": generate_id(), "is_superuser": True},
|
||||
)
|
||||
self.assertEqual(res.status_code, 201)
|
||||
|
||||
@ -6565,6 +6565,8 @@
|
||||
"authentik_core.delete_token",
|
||||
"authentik_core.delete_user",
|
||||
"authentik_core.delete_usersourceconnection",
|
||||
"authentik_core.disable_group_superuser",
|
||||
"authentik_core.enable_group_superuser",
|
||||
"authentik_core.impersonate",
|
||||
"authentik_core.preview_user",
|
||||
"authentik_core.remove_user_from_group",
|
||||
@ -12936,6 +12938,8 @@
|
||||
"enum": [
|
||||
"add_user_to_group",
|
||||
"remove_user_from_group",
|
||||
"enable_group_superuser",
|
||||
"disable_group_superuser",
|
||||
"add_group",
|
||||
"change_group",
|
||||
"delete_group",
|
||||
@ -13068,6 +13072,8 @@
|
||||
"authentik_core.delete_token",
|
||||
"authentik_core.delete_user",
|
||||
"authentik_core.delete_usersourceconnection",
|
||||
"authentik_core.disable_group_superuser",
|
||||
"authentik_core.enable_group_superuser",
|
||||
"authentik_core.impersonate",
|
||||
"authentik_core.preview_user",
|
||||
"authentik_core.remove_user_from_group",
|
||||
|
||||
Reference in New Issue
Block a user