Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

core: add additional RBAC permission to restrict setting the superuser status on groups #12900

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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 @@
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
rissson marked this conversation as resolved.
Show resolved Hide resolved
# 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 @@

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 @@
},
)
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
Loading