Skip to content

Commit

Permalink
core: add additional RBAC permission to restrict setting the superuse…
Browse files Browse the repository at this point in the history
…r status on groups

Signed-off-by: Jens Langhammer <jens@goauthentik.io>
  • Loading branch information
BeryJu committed Jan 31, 2025
1 parent 427a8c9 commit 5927381
Show file tree
Hide file tree
Showing 5 changed files with 120 additions and 3 deletions.
31 changes: 30 additions & 1 deletion authentik/core/api/groups.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

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 (
Expand Down Expand Up @@ -81,9 +82,37 @@ def validate_parent(self, parent: Group | None):
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."))

Check warning on line 85 in authentik/core/api/groups.py

View check run for this annotation

Codecov / codecov/patch

authentik/core/api/groups.py#L85

Added line #L85 was not covered by tests
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

Check warning on line 92 in authentik/core/api/groups.py

View check run for this annotation

Codecov / codecov/patch

authentik/core/api/groups.py#L90-L92

Added lines #L90 - L92 were not covered by tests
# 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 = (

Check warning on line 97 in authentik/core/api/groups.py

View check run for this annotation

Codecov / codecov/patch

authentik/core/api/groups.py#L94-L97

Added lines #L94 - L97 were not covered by tests
"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(

Check warning on line 106 in authentik/core/api/groups.py

View check run for this annotation

Codecov / codecov/patch

authentik/core/api/groups.py#L102-L106

Added lines #L102 - L106 were not covered by tests
_(
(
"User does not have permission to set "
"superuser status to {superuser_status}."
).format_map({"superuser_status": superuser})
)
)
return superuser

Check warning on line 114 in authentik/core/api/groups.py

View check run for this annotation

Codecov / codecov/patch

authentik/core/api/groups.py#L114

Added line #L114 was not covered by tests

class Meta:
model = Group
fields = [
Expand Down
26 changes: 26 additions & 0 deletions authentik/core/migrations/0043_alter_group_options.py
Original file line number Diff line number Diff line change
@@ -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",
},
),
]
2 changes: 2 additions & 0 deletions authentik/core/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -204,6 +204,8 @@ class Meta:
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):
Expand Down
58 changes: 56 additions & 2 deletions authentik/core/tests/test_groups_api.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
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

Check warning on line 7 in authentik/core/tests/test_groups_api.py

View check run for this annotation

Codecov / codecov/patch

authentik/core/tests/test_groups_api.py#L7

Added line #L7 was not covered by tests
from authentik.core.tests.utils import create_test_admin_user, create_test_user
from authentik.lib.generators import generate_id

Expand All @@ -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()

Check warning on line 17 in authentik/core/tests/test_groups_api.py

View check run for this annotation

Codecov / codecov/patch

authentik/core/tests/test_groups_api.py#L17

Added line #L17 was not covered by tests

def test_list_with_users(self):
"""Test listing with users"""
Expand Down Expand Up @@ -109,3 +109,57 @@ def test_parent_self(self):
},
)
self.assertEqual(res.status_code, 400)

def test_superuser_no_perm(self):

Check warning on line 113 in authentik/core/tests/test_groups_api.py

View check run for this annotation

Codecov / codecov/patch

authentik/core/tests/test_groups_api.py#L113

Added line #L113 was not covered by tests
"""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(

Check warning on line 117 in authentik/core/tests/test_groups_api.py

View check run for this annotation

Codecov / codecov/patch

authentik/core/tests/test_groups_api.py#L115-L117

Added lines #L115 - L117 were not covered by tests
reverse("authentik_api:group-list"),
data={"name": generate_id(), "is_superuser": True},
)
self.assertEqual(res.status_code, 400)
self.assertJSONEqual(

Check warning on line 122 in authentik/core/tests/test_groups_api.py

View check run for this annotation

Codecov / codecov/patch

authentik/core/tests/test_groups_api.py#L121-L122

Added lines #L121 - L122 were not covered by tests
res.content,
{"is_superuser": ["User does not have permission to set superuser status to True."]},
)

def test_superuser_update_no_perm(self):

Check warning on line 127 in authentik/core/tests/test_groups_api.py

View check run for this annotation

Codecov / codecov/patch

authentik/core/tests/test_groups_api.py#L127

Added line #L127 was not covered by tests
"""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(

Check warning on line 133 in authentik/core/tests/test_groups_api.py

View check run for this annotation

Codecov / codecov/patch

authentik/core/tests/test_groups_api.py#L129-L133

Added lines #L129 - L133 were not covered by tests
reverse("authentik_api:group-detail", kwargs={"pk": group.pk}),
data={"is_superuser": False},
)
self.assertEqual(res.status_code, 400)
self.assertJSONEqual(

Check warning on line 138 in authentik/core/tests/test_groups_api.py

View check run for this annotation

Codecov / codecov/patch

authentik/core/tests/test_groups_api.py#L137-L138

Added lines #L137 - L138 were not covered by tests
res.content,
{"is_superuser": ["User does not have permission to set superuser status to False."]},
)

def test_superuser_update_no_change(self):

Check warning on line 143 in authentik/core/tests/test_groups_api.py

View check run for this annotation

Codecov / codecov/patch

authentik/core/tests/test_groups_api.py#L143

Added line #L143 was not covered by tests
"""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(

Check warning on line 150 in authentik/core/tests/test_groups_api.py

View check run for this annotation

Codecov / codecov/patch

authentik/core/tests/test_groups_api.py#L146-L150

Added lines #L146 - L150 were not covered by tests
reverse("authentik_api:group-detail", kwargs={"pk": group.pk}),
data={"name": generate_id(), "is_superuser": True},
)
self.assertEqual(res.status_code, 200)

Check warning on line 154 in authentik/core/tests/test_groups_api.py

View check run for this annotation

Codecov / codecov/patch

authentik/core/tests/test_groups_api.py#L154

Added line #L154 was not covered by tests

def test_superuser_create(self):

Check warning on line 156 in authentik/core/tests/test_groups_api.py

View check run for this annotation

Codecov / codecov/patch

authentik/core/tests/test_groups_api.py#L156

Added line #L156 was not covered by tests
"""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(

Check warning on line 161 in authentik/core/tests/test_groups_api.py

View check run for this annotation

Codecov / codecov/patch

authentik/core/tests/test_groups_api.py#L158-L161

Added lines #L158 - L161 were not covered by tests
reverse("authentik_api:group-list"),
data={"name": generate_id(), "is_superuser": True},
)
self.assertEqual(res.status_code, 201)

Check warning on line 165 in authentik/core/tests/test_groups_api.py

View check run for this annotation

Codecov / codecov/patch

authentik/core/tests/test_groups_api.py#L165

Added line #L165 was not covered by tests
6 changes: 6 additions & 0 deletions blueprints/schema.json
Original file line number Diff line number Diff line change
Expand Up @@ -6440,6 +6440,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",
Expand Down Expand Up @@ -12557,6 +12559,8 @@
"enum": [
"add_user_to_group",
"remove_user_from_group",
"enable_group_superuser",
"disable_group_superuser",
"add_group",
"change_group",
"delete_group",
Expand Down Expand Up @@ -12689,6 +12693,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",
Expand Down

0 comments on commit 5927381

Please sign in to comment.