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

Forbid deleting the last staff user #1036

Merged
merged 8 commits into from
Aug 29, 2023
Merged
Show file tree
Hide file tree
Changes from 7 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
2 changes: 1 addition & 1 deletion ephios/core/forms/events.py
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,7 @@ def save(self, commit=True):
# delete existing permissions
# (better implement https://github.com/django-guardian/django-guardian/issues/654)
for group in get_groups_with_perms(
event, only_with_perms_in=["view_event", "change_event"]
event, only_with_perms_in=["view_event", "change_event"], must_have_all_perms=False
):
remove_perm("view_event", group, event)
remove_perm("change_event", group, event)
Expand Down
72 changes: 70 additions & 2 deletions ephios/core/forms/users.py
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@
from ephios.core.signals import register_group_permission_fields
from ephios.core.widgets import MultiUserProfileWidget
from ephios.extra.crispy import AbortLink
from ephios.extra.permissions import PermissionField, PermissionFormMixin
from ephios.extra.permissions import PermissionField, PermissionFormMixin, get_groups_with_perms
from ephios.extra.widgets import CustomDateInput
from ephios.modellogging.log import add_log_recorder
from ephios.modellogging.recorders import DerivedFieldsLogRecorder
Expand Down Expand Up @@ -176,6 +176,24 @@ def __init__(self, **kwargs):
),
)

def clean_is_management_group(self):
is_management_group = self.cleaned_data["is_management_group"]
if self.fields["is_management_group"].initial and not is_management_group:
other_management_groups = get_groups_with_perms(
only_with_perms_in=CORE_MANAGEMENT_PERMISSIONS,
must_have_all_perms=True,
).exclude(pk=self.instance.pk)
if not other_management_groups.exists():
raise ValidationError(
_(
"At least one group with management permissions must exist. "
"Please promote another group before demoting this one."
)
)
if is_management_group:
self.cleaned_data["is_hr_group"] = True
return is_management_group

def save(self, commit=True):
add_log_recorder(self.instance, DerivedFieldsLogRecorder(get_group_permission_log_fields))
group = super().save(commit)
Expand All @@ -200,14 +218,22 @@ def save(self, commit=True):
return group


class UserProfileForm(ModelForm):
class UserProfileForm(PermissionFormMixin, ModelForm):
groups = ModelMultipleChoiceField(
label=_("Groups"),
queryset=Group.objects.all(),
widget=Select2MultipleWidget,
required=False,
disabled=True, # explicitly enable for users with `change_group` permission
)
is_staff = PermissionField(
felixrindt marked this conversation as resolved.
Show resolved Hide resolved
label=_("Administrator"),
help_text=_(
"If checked, this user can change technical ephios settings as well as edit all user profiles, "
"groups, qualifications, events and event types."
),
permissions=CORE_MANAGEMENT_PERMISSIONS,
)

def __init__(self, *args, **kwargs):
request = kwargs.pop("request")
Expand All @@ -230,6 +256,10 @@ def __init__(self, *args, **kwargs):
"Only other technical administrators can change this."
)

@property
def permission_target(self):
return self.instance

field_order = [
"email",
"first_name",
Expand Down Expand Up @@ -280,6 +310,44 @@ def save(self, commit=True):
return userprofile


class DeleteUserProfileForm(Form):
def __init__(self, *args, **kwargs):
self.instance = kwargs.pop("instance")
super().__init__(*args, **kwargs)

def clean(self):
other_staff = UserProfile.objects.filter(is_staff=True).exclude(pk=self.instance.pk)
if self.instance.is_staff and not other_staff.exists():
raise ValidationError(
_(
"At least one user must be technical administrator. "
"Please promote another user before deleting this one."
)
)


class DeleteGroupForm(Form):
def __init__(self, *args, **kwargs):
self.instance = kwargs.pop("instance")
super().__init__(*args, **kwargs)

def clean(self):
management_groups = get_groups_with_perms(
only_with_perms_in=CORE_MANAGEMENT_PERMISSIONS, must_have_all_perms=True
)

if (
self.instance in management_groups
and not management_groups.exclude(pk=self.instance.pk).exists()
):
raise ValidationError(
_(
"At least one group with management permissions must exist. "
"Please promote another group before deleting this one."
)
)


class QualificationGrantForm(ModelForm):
model = QualificationGrant

Expand Down
3 changes: 1 addition & 2 deletions ephios/core/models/users.py
Original file line number Diff line number Diff line change
Expand Up @@ -104,8 +104,7 @@ class UserProfile(guardian.mixins.GuardianUserMixin, PermissionsMixin, AbstractB
is_visible = BooleanField(default=True, verbose_name=_("Visible"))
is_staff = BooleanField(
default=False,
verbose_name=_("Technical Administrator"),
help_text=_("If checked, this user can change technical ephios settings."),
verbose_name=_("Administrator"),
)
first_name = CharField(_("first name"), max_length=254)
last_name = CharField(_("last name"), max_length=254)
Expand Down
2 changes: 2 additions & 0 deletions ephios/core/templates/core/group_confirm_delete.html
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
{% extends "base.html" %}
{% load crispy_forms_filters %}
{% load i18n %}
{% load static %}

Expand All @@ -12,6 +13,7 @@ <h1>{% trans "Delete group" %}</h1>
</div>

<form method="post">{% csrf_token %}
{{ form|as_crispy_errors }}
<p>{% blocktrans %}Are you sure you want to delete the group "{{ group }}"?{% endblocktrans %}</p>
<a role="button" class="btn btn-secondary" href="{% url "core:group_list" %}">{% trans "Back" %}</a>
<button type="submit" class="btn btn-danger">{% trans "Delete" %}</button>
Expand Down
2 changes: 2 additions & 0 deletions ephios/core/templates/core/userprofile_confirm_delete.html
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
{% extends "base.html" %}
{% load crispy_forms_filters %}
{% load i18n %}
{% load static %}

Expand All @@ -12,6 +13,7 @@ <h1>{% translate "Delete user" %}</h1>
</div>

<form method="post">{% csrf_token %}
{{ form|as_crispy_errors }}
<p>{% blocktranslate trimmed with name=userprofile.get_full_name %}
Are you sure you want to delete the user {{ name }} ({{ userprofile }})?
{% endblocktranslate %}</p>
Expand Down
28 changes: 23 additions & 5 deletions ephios/core/views/accounts.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,13 @@
from dynamic_preferences.registries import global_preferences_registry

from ephios.api.access.auth import revoke_all_access_tokens
from ephios.core.forms.users import GroupForm, QualificationGrantFormset, UserProfileForm
from ephios.core.forms.users import (
DeleteGroupForm,
DeleteUserProfileForm,
GroupForm,
QualificationGrantFormset,
UserProfileForm,
)
from ephios.core.models import QualificationGrant, UserProfile
from ephios.core.services.notifications.types import (
NewProfileNotification,
Expand Down Expand Up @@ -111,8 +117,8 @@ def post(self, request, *args, **kwargs):
qualification_formset.save()
messages.success(
self.request,
_("User {name} ({user}) updated successfully.").format(
name=self.object.get_full_name(), user=self.object
_("User {name} ({email}) updated successfully.").format(
name=self.object.get_full_name(), email=self.object.email
),
)
if request.user != userprofile:
Expand All @@ -130,12 +136,18 @@ class UserProfileDeleteView(CustomPermissionRequiredMixin, DeleteView):
model = UserProfile
permission_required = "core.delete_userprofile"
template_name = "core/userprofile_confirm_delete.html"
form_class = DeleteUserProfileForm

def get_form_kwargs(self):
kwargs = super().get_form_kwargs()
kwargs["instance"] = self.object
return kwargs

def get_success_url(self):
messages.info(
self.request,
_("The user {name} ({user}) was deleted.").format(
name=self.object.get_full_name(), user=self.object
_("The user {name} ({email}) was deleted.").format(
name=self.object.get_full_name(), email=self.object.email
),
)
return reverse("core:userprofile_list")
Expand Down Expand Up @@ -233,6 +245,12 @@ class GroupDeleteView(CustomPermissionRequiredMixin, DeleteView):
model = Group
permission_required = "auth.delete_group"
template_name = "core/group_confirm_delete.html"
form_class = DeleteGroupForm

def get_form_kwargs(self):
kwargs = super().get_form_kwargs()
kwargs["instance"] = self.object
return kwargs

def get_success_url(self):
messages.info(self.request, _('The group "{group}" was deleted.').format(group=self.object))
Expand Down
12 changes: 9 additions & 3 deletions ephios/core/views/event.py
Original file line number Diff line number Diff line change
Expand Up @@ -412,15 +412,21 @@ def form_valid(self, form):
event.pk = None
event.save()
assign_perm(
"view_event", get_groups_with_perms(self.get_object(), ["view_event"]), event
"view_event",
get_groups_with_perms(self.get_object(), only_with_perms_in=["view_event"]),
event,
)
assign_perm(
"change_event", get_groups_with_perms(self.get_object(), ["change_event"]), event
"change_event",
get_groups_with_perms(self.get_object(), only_with_perms_in=["change_event"]),
event,
)
assign_perm(
"change_event",
get_users_with_perms(
self.get_object(), only_with_perms_in=["change_event"], with_group_users=False
self.get_object(),
only_with_perms_in=["change_event"],
with_group_users=False,
),
event,
)
Expand Down
109 changes: 84 additions & 25 deletions ephios/extra/permissions.py
Original file line number Diff line number Diff line change
@@ -1,12 +1,17 @@
from functools import wraps
from typing import Optional

from django.contrib.auth import get_user_model
from django.contrib.auth.models import Group, Permission
from django.core.exceptions import PermissionDenied
from django.db.models import Q
from django.forms import BooleanField
from guardian.ctypes import get_content_type
from guardian.shortcuts import assign_perm, remove_perm
from guardian.utils import get_group_obj_perms_model

Q_FALSE = Q(pk__in=[])


def staff_required(view_func):
@wraps(view_func)
Expand All @@ -22,29 +27,78 @@ def _wrapped_view(request, *args, **kwargs):
return _wrapped_view


def get_groups_with_perms(obj, only_with_perms_in):
ctype = get_content_type(obj)
group_model = get_group_obj_perms_model(obj)

group_rel_name = group_model.group.field.related_query_name()

if group_model.objects.is_generic():
group_filters = {
f"{group_rel_name}__content_type": ctype,
f"{group_rel_name}__object_pk": obj.pk,
}
def _make_permission_names_qualified(permission_names, obj: Optional):
"""
Return a list of permission names in the form "app_label.codename" for the given permissions codenames.
If obj is given, the app_label is inferred from the object's meta.
Otherwise, the permission name must already be qualified.
"""
qualified_permission_names = []
for name in permission_names or []:
if "." in name:
qualified_permission_names.append(name)
elif obj is not None:
app_label = obj._meta.app_label # pylint: disable=protected-access
qualified_permission_names.append(f"{app_label}.{name}")
else:
raise ValueError("Cannot infer permission app_label from name without obj")
return qualified_permission_names


def get_groups_with_perms(obj=None, *, only_with_perms_in=None, must_have_all_perms=False):
qs = Group.objects.all()
qualified_permission_names = _make_permission_names_qualified(only_with_perms_in or [], obj)
required_perms = get_permissions_from_qualified_names(qualified_permission_names)

group_perms_model = get_group_obj_perms_model(obj)
group_perms_rel_name = group_perms_model.group.field.related_query_name()
obj_filter = {}
if obj is not None:
ctype = get_content_type(obj)
if group_perms_model.objects.is_generic():
obj_filter = {
f"{group_perms_rel_name}__content_type": ctype,
f"{group_perms_rel_name}__object_pk": obj.pk,
}
else:
obj_filter = {f"{group_perms_rel_name}__content_object": obj}

if must_have_all_perms:
for perm in required_perms:
if obj is not None:
qs = qs.filter(
Q(permissions=perm)
| Q(
**{
f"{group_perms_rel_name}__permission": perm,
**obj_filter,
}
)
)
else:
qs = qs.filter(permissions=perm)
else:
group_filters = {f"{group_rel_name}__content_object": obj}

permission_ids = Permission.objects.filter(
content_type=ctype, codename__in=only_with_perms_in
).values_list("id", flat=True)
group_filters.update(
{
f"{group_rel_name}__permission_id__in": permission_ids,
}
)
return Group.objects.filter(**group_filters).distinct()
if obj is not None:
qs = qs.filter(
Q(permissions__in=required_perms)
| Q(
**{
f"{group_perms_rel_name}__permission__in": required_perms,
**obj_filter,
}
)
)
else:
qs = qs.filter(Q(permissions__in=required_perms))
return qs.distinct()


def get_permissions_from_qualified_names(qualified_names):
perms_filter = Q_FALSE
for qualified_name in qualified_names:
app_label, codename = qualified_name.split(".")
perms_filter = perms_filter | Q(content_type__app_label=app_label, codename=codename)
return Permission.objects.filter(perms_filter)


class PermissionField(BooleanField):
Expand All @@ -60,11 +114,16 @@ def __init__(self, *args, **kwargs):

def set_initial_value(self, user_or_group):
self.target = user_or_group
codename_set = set(map(lambda perm: perm.split(".")[-1], self.permission_set))
self.initial = codename_set <= set(
self.target.permissions.values_list("codename", flat=True)
wanted_permission_objects = get_permissions_from_qualified_names(self.permission_set)
self.initial = set(wanted_permission_objects) <= set(
self._get_permissions_from_user_or_group(self.target)
)

def _get_permissions_from_user_or_group(self, user_or_group):
if isinstance(user_or_group, get_user_model()):
return user_or_group.user_permissions.all()
return user_or_group.permissions.all()

def assign_permissions(self, target):
for permission in self.permission_set:
assign_perm(permission, target)
Expand Down
Loading