diff --git a/allianceauth/authentication/admin.py b/allianceauth/authentication/admin.py index 16818d41..f39af894 100644 --- a/allianceauth/authentication/admin.py +++ b/allianceauth/authentication/admin.py @@ -1,26 +1,44 @@ from django.contrib import admin from django.contrib.auth.admin import UserAdmin as BaseUserAdmin -from django.contrib.auth.models import User as BaseUser, \ - Permission as BasePermission, Group +from django.contrib.auth.models import Group +from django.contrib.auth.models import Permission as BasePermission +from django.contrib.auth.models import User as BaseUser from django.db.models import Count, Q -from allianceauth.services.hooks import ServicesHook -from django.db.models.signals import pre_save, post_save, pre_delete, \ - post_delete, m2m_changed from django.db.models.functions import Lower +from django.db.models.signals import ( + m2m_changed, + post_delete, + post_save, + pre_delete, + pre_save +) from django.dispatch import receiver -from django.forms import ModelForm -from django.utils.html import format_html from django.urls import reverse +from django.utils.html import format_html from django.utils.text import slugify -from allianceauth.authentication.models import State, get_guest_state,\ - CharacterOwnership, UserProfile, OwnershipRecord -from allianceauth.hooks import get_hooks -from allianceauth.eveonline.models import EveCharacter, EveCorporationInfo,\ - EveAllianceInfo, EveFactionInfo +from allianceauth.authentication.models import ( + CharacterOwnership, + OwnershipRecord, + State, + UserProfile, + get_guest_state +) +from allianceauth.eveonline.models import ( + EveAllianceInfo, + EveCharacter, + EveCorporationInfo, + EveFactionInfo +) from allianceauth.eveonline.tasks import update_character -from .app_settings import AUTHENTICATION_ADMIN_USERS_MAX_GROUPS, \ - AUTHENTICATION_ADMIN_USERS_MAX_CHARS +from allianceauth.hooks import get_hooks +from allianceauth.services.hooks import ServicesHook + +from .app_settings import ( + AUTHENTICATION_ADMIN_USERS_MAX_CHARS, + AUTHENTICATION_ADMIN_USERS_MAX_GROUPS +) +from .forms import UserChangeForm, UserProfileForm def make_service_hooks_update_groups_action(service): @@ -59,19 +77,10 @@ def make_service_hooks_sync_nickname_action(service): return sync_nickname -class QuerysetModelForm(ModelForm): - # allows specifying FK querysets through kwarg - def __init__(self, querysets=None, *args, **kwargs): - querysets = querysets or {} - super().__init__(*args, **kwargs) - for field, qs in querysets.items(): - self.fields[field].queryset = qs - - class UserProfileInline(admin.StackedInline): model = UserProfile readonly_fields = ('state',) - form = QuerysetModelForm + form = UserProfileForm verbose_name = '' verbose_name_plural = 'Profile' @@ -99,6 +108,7 @@ class UserProfileInline(admin.StackedInline): return False +@admin.display(description="") def user_profile_pic(obj): """profile pic column data for user objects @@ -111,13 +121,10 @@ def user_profile_pic(obj): '', user_obj.profile.main_character.portrait_url(size=32) ) - else: - return None - - -user_profile_pic.short_description = '' + return None +@admin.display(description="user / main", ordering="username") def user_username(obj): """user column data for user objects @@ -139,18 +146,17 @@ def user_username(obj): user_obj.username, user_obj.profile.main_character.character_name ) - else: - return format_html( - '{}', - link, - user_obj.username, - ) - - -user_username.short_description = 'user / main' -user_username.admin_order_field = 'username' + return format_html( + '{}', + link, + user_obj.username, + ) +@admin.display( + description="Corporation / Alliance (Main)", + ordering="profile__main_character__corporation_name" +) def user_main_organization(obj): """main organization column data for user objects @@ -159,21 +165,15 @@ def user_main_organization(obj): """ user_obj = obj.user if hasattr(obj, 'user') else obj if not user_obj.profile.main_character: - result = '' - else: - result = user_obj.profile.main_character.corporation_name - if user_obj.profile.main_character.alliance_id: - result += f'
{user_obj.profile.main_character.alliance_name}' - elif user_obj.profile.main_character.faction_name: - result += f'
{user_obj.profile.main_character.faction_name}' + return '' + result = user_obj.profile.main_character.corporation_name + if user_obj.profile.main_character.alliance_id: + result += f'
{user_obj.profile.main_character.alliance_name}' + elif user_obj.profile.main_character.faction_name: + result += f'
{user_obj.profile.main_character.faction_name}' return format_html(result) -user_main_organization.short_description = 'Corporation / Alliance (Main)' -user_main_organization.admin_order_field = \ - 'profile__main_character__corporation_name' - - class MainCorporationsFilter(admin.SimpleListFilter): """Custom filter to filter on corporations from mains only @@ -196,15 +196,13 @@ class MainCorporationsFilter(admin.SimpleListFilter): def queryset(self, request, qs): if self.value() is None: return qs.all() - else: - if qs.model == User: - return qs.filter( - profile__main_character__corporation_id=self.value() - ) - else: - return qs.filter( - user__profile__main_character__corporation_id=self.value() - ) + if qs.model == User: + return qs.filter( + profile__main_character__corporation_id=self.value() + ) + return qs.filter( + user__profile__main_character__corporation_id=self.value() + ) class MainAllianceFilter(admin.SimpleListFilter): @@ -217,12 +215,14 @@ class MainAllianceFilter(admin.SimpleListFilter): parameter_name = 'main_alliance_id__exact' def lookups(self, request, model_admin): - qs = EveCharacter.objects\ - .exclude(alliance_id=None)\ - .exclude(userprofile=None)\ - .values('alliance_id', 'alliance_name')\ - .distinct()\ + qs = ( + EveCharacter.objects + .exclude(alliance_id=None) + .exclude(userprofile=None) + .values('alliance_id', 'alliance_name') + .distinct() .order_by(Lower('alliance_name')) + ) return tuple( (x['alliance_id'], x['alliance_name']) for x in qs ) @@ -230,13 +230,11 @@ class MainAllianceFilter(admin.SimpleListFilter): def queryset(self, request, qs): if self.value() is None: return qs.all() - else: - if qs.model == User: - return qs.filter(profile__main_character__alliance_id=self.value()) - else: - return qs.filter( - user__profile__main_character__alliance_id=self.value() - ) + if qs.model == User: + return qs.filter(profile__main_character__alliance_id=self.value()) + return qs.filter( + user__profile__main_character__alliance_id=self.value() + ) class MainFactionFilter(admin.SimpleListFilter): @@ -249,12 +247,14 @@ class MainFactionFilter(admin.SimpleListFilter): parameter_name = 'main_faction_id__exact' def lookups(self, request, model_admin): - qs = EveCharacter.objects\ - .exclude(faction_id=None)\ - .exclude(userprofile=None)\ - .values('faction_id', 'faction_name')\ - .distinct()\ + qs = ( + EveCharacter.objects + .exclude(faction_id=None) + .exclude(userprofile=None) + .values('faction_id', 'faction_name') + .distinct() .order_by(Lower('faction_name')) + ) return tuple( (x['faction_id'], x['faction_name']) for x in qs ) @@ -262,15 +262,14 @@ class MainFactionFilter(admin.SimpleListFilter): def queryset(self, request, qs): if self.value() is None: return qs.all() - else: - if qs.model == User: - return qs.filter(profile__main_character__faction_id=self.value()) - else: - return qs.filter( - user__profile__main_character__faction_id=self.value() - ) + if qs.model == User: + return qs.filter(profile__main_character__faction_id=self.value()) + return qs.filter( + user__profile__main_character__faction_id=self.value() + ) +@admin.display(description="Update main character model from ESI") def update_main_character_model(modeladmin, request, queryset): tasks_count = 0 for obj in queryset: @@ -279,21 +278,48 @@ def update_main_character_model(modeladmin, request, queryset): tasks_count += 1 modeladmin.message_user( - request, - f'Update from ESI started for {tasks_count} characters' + request, f'Update from ESI started for {tasks_count} characters' ) -update_main_character_model.short_description = \ - 'Update main character model from ESI' - - class UserAdmin(BaseUserAdmin): """Extending Django's UserAdmin model Behavior of groups and characters columns can be configured via settings """ + inlines = BaseUserAdmin.inlines + [UserProfileInline] + ordering = ('username', ) + list_select_related = ('profile__state', 'profile__main_character') + show_full_result_count = True + list_display = ( + user_profile_pic, + user_username, + '_state', + '_groups', + user_main_organization, + '_characters', + 'is_active', + 'date_joined', + '_role' + ) + list_display_links = None + list_filter = ( + 'profile__state', + 'groups', + MainCorporationsFilter, + MainAllianceFilter, + MainFactionFilter, + 'is_active', + 'date_joined', + 'is_staff', + 'is_superuser' + ) + search_fields = ('username', 'character_ownerships__character__character_name') + readonly_fields = ('date_joined', 'last_login') + filter_horizontal = ('groups', 'user_permissions',) + form = UserChangeForm + class Media: css = { "all": ("authentication/css/admin.css",) @@ -303,9 +329,21 @@ class UserAdmin(BaseUserAdmin): qs = super().get_queryset(request) return qs.prefetch_related("character_ownerships__character", "groups") - def get_actions(self, request): - actions = super(BaseUserAdmin, self).get_actions(request) + def get_form(self, request, obj=None, **kwargs): + """Inject current request into change form object.""" + MyForm = super().get_form(request, obj, **kwargs) + if obj: + class MyFormInjected(MyForm): + def __new__(cls, *args, **kwargs): + kwargs['request'] = request + return MyForm(*args, **kwargs) + + return MyFormInjected + return MyForm + + def get_actions(self, request): + actions = super().get_actions(request) actions[update_main_character_model.__name__] = ( update_main_character_model, update_main_character_model.__name__, @@ -349,39 +387,6 @@ class UserAdmin(BaseUserAdmin): ) return result - inlines = BaseUserAdmin.inlines + [UserProfileInline] - ordering = ('username', ) - list_select_related = ('profile__state', 'profile__main_character') - show_full_result_count = True - list_display = ( - user_profile_pic, - user_username, - '_state', - '_groups', - user_main_organization, - '_characters', - 'is_active', - 'date_joined', - '_role' - ) - list_display_links = None - list_filter = ( - 'profile__state', - 'groups', - MainCorporationsFilter, - MainAllianceFilter, - MainFactionFilter, - 'is_active', - 'date_joined', - 'is_staff', - 'is_superuser' - ) - search_fields = ( - 'username', - 'character_ownerships__character__character_name' - ) - readonly_fields = ('date_joined', 'last_login') - def _characters(self, obj): character_ownerships = list(obj.character_ownerships.all()) characters = [obj.character.character_name for obj in character_ownerships] @@ -390,22 +395,16 @@ class UserAdmin(BaseUserAdmin): AUTHENTICATION_ADMIN_USERS_MAX_CHARS ) - _characters.short_description = 'characters' - + @admin.display(ordering="profile__state") def _state(self, obj): return obj.profile.state.name - _state.short_description = 'state' - _state.admin_order_field = 'profile__state' - def _groups(self, obj): my_groups = sorted(group.name for group in list(obj.groups.all())) return self._list_2_html_w_tooltips( my_groups, AUTHENTICATION_ADMIN_USERS_MAX_GROUPS ) - _groups.short_description = 'groups' - def _role(self, obj): if obj.is_superuser: role = 'Superuser' @@ -415,8 +414,6 @@ class UserAdmin(BaseUserAdmin): role = 'User' return role - _role.short_description = 'role' - def has_change_permission(self, request, obj=None): return request.user.has_perm('auth.change_user') @@ -438,9 +435,16 @@ class UserAdmin(BaseUserAdmin): if obj_state: matching_groups_qs = Group.objects.filter(authgroup__states=obj_state) groups_qs = groups_qs | matching_groups_qs - kwargs["queryset"] = groups_qs.order_by(Lower('name')) + kwargs["queryset"] = groups_qs.order_by(Lower("name")) return super().formfield_for_manytomany(db_field, request, **kwargs) + def get_readonly_fields(self, request, obj=None): + if obj and not request.user.is_superuser: + return self.readonly_fields + ( + "is_staff", "is_superuser", "user_permissions" + ) + return self.readonly_fields + @admin.register(State) class StateAdmin(admin.ModelAdmin): @@ -451,10 +455,9 @@ class StateAdmin(admin.ModelAdmin): qs = super().get_queryset(request) return qs.annotate(user_count=Count("userprofile__id")) + @admin.display(description="Users", ordering="user_count") def _user_count(self, obj): return obj.user_count - _user_count.short_description = 'Users' - _user_count.admin_order_field = 'user_count' fieldsets = ( (None, { @@ -510,13 +513,13 @@ class StateAdmin(admin.ModelAdmin): ) return super().get_fieldsets(request, obj=obj) + def get_readonly_fields(self, request, obj=None): + if not request.user.is_superuser: + return self.readonly_fields + ("permissions",) + return self.readonly_fields + class BaseOwnershipAdmin(admin.ModelAdmin): - class Media: - css = { - "all": ("authentication/css/admin.css",) - } - list_select_related = ( 'user__profile__state', 'user__profile__main_character', 'character') list_display = ( @@ -537,6 +540,11 @@ class BaseOwnershipAdmin(admin.ModelAdmin): MainAllianceFilter, ) + class Media: + css = { + "all": ("authentication/css/admin.css",) + } + def get_readonly_fields(self, request, obj=None): if obj and obj.pk: return 'owner_hash', 'character' diff --git a/allianceauth/authentication/forms.py b/allianceauth/authentication/forms.py index ea0379ca..fabff9d9 100644 --- a/allianceauth/authentication/forms.py +++ b/allianceauth/authentication/forms.py @@ -1,8 +1,66 @@ from django import forms -from django.utils.translation import ugettext_lazy as _ +from django.contrib.auth.forms import UserChangeForm as BaseUserChangeForm +from django.contrib.auth.models import Group +from django.core.exceptions import ValidationError +from django.forms import ModelForm +from django.utils.translation import gettext_lazy as _ + from allianceauth.authentication.models import User + + class RegistrationForm(forms.Form): email = forms.EmailField(label=_('Email'), max_length=254, required=True) class _meta: model = User + + +class UserProfileForm(ModelForm): + """Allows specifying FK querysets through kwarg""" + + def __init__(self, querysets=None, *args, **kwargs): + querysets = querysets or {} + super().__init__(*args, **kwargs) + for field, qs in querysets.items(): + self.fields[field].queryset = qs + + +class UserChangeForm(BaseUserChangeForm): + """Add custom cleaning to UserChangeForm""" + + def __init__(self, *args, **kwargs): + self.request = kwargs.pop("request") # Inject current request into form object + super().__init__(*args, **kwargs) + + def clean(self): + cleaned_data = super().clean() + if not self.request.user.is_superuser: + if self.instance: + current_restricted = set( + self.instance.groups.filter( + authgroup__restricted=True + ).values_list("pk", flat=True) + ) + else: + current_restricted = set() + new_restricted = set( + cleaned_data["groups"].filter( + authgroup__restricted=True + ).values_list("pk", flat=True) + ) + if current_restricted != new_restricted: + restricted_removed = current_restricted - new_restricted + restricted_added = new_restricted - current_restricted + restricted_changed = restricted_removed | restricted_added + restricted_names_qs = Group.objects.filter( + pk__in=restricted_changed + ).values_list("name", flat=True) + restricted_names = ",".join(list(restricted_names_qs)) + raise ValidationError( + { + "groups": _( + "You are not allowed to add or remove these " + "restricted groups: %s" % restricted_names + ) + } + ) diff --git a/allianceauth/authentication/tests/test_admin.py b/allianceauth/authentication/tests/test_admin.py index 203707bc..0f43a14c 100644 --- a/allianceauth/authentication/tests/test_admin.py +++ b/allianceauth/authentication/tests/test_admin.py @@ -2,6 +2,8 @@ from bs4 import BeautifulSoup from urllib.parse import quote from unittest.mock import patch, MagicMock +from django_webtest import WebTest + from django.contrib.admin.sites import AdminSite from django.contrib.auth.models import Group from django.test import TestCase, RequestFactory, Client @@ -276,10 +278,10 @@ class TestOwnershipRecordAdmin(TestCaseWithTestData): class TestStateAdmin(TestCaseWithTestData): fixtures = ["disable_analytics"] - def setUp(self): - self.modeladmin = StateAdmin( - model=User, admin_site=AdminSite() - ) + @classmethod + def setUpClass(cls) -> None: + super().setUpClass() + cls.modeladmin = StateAdmin(model=User, admin_site=AdminSite()) def test_change_view_loads_normally(self): User.objects.create_superuser( @@ -543,7 +545,74 @@ class TestUserAdmin(TestCaseWithTestData): self.assertEqual(response.status_code, expected) +class TestStateAdminChangeFormSuperuserExclusiveEdits(WebTest): + @classmethod + def setUpClass(cls) -> None: + super().setUpClass() + cls.super_admin = User.objects.create_superuser("super_admin") + cls.staff_admin = User.objects.create_user("staff_admin") + cls.staff_admin.is_staff = True + cls.staff_admin.save() + cls.staff_admin = AuthUtils.add_permissions_to_user_by_name( + [ + "authentication.add_state", + "authentication.change_state", + "authentication.view_state", + ], + cls.staff_admin + ) + cls.superuser_exclusive_fields = ["permissions",] + + def test_should_show_all_fields_to_superuser_for_add(self): + # given + self.app.set_user(self.super_admin) + page = self.app.get("/admin/authentication/state/add/") + # when + form = page.forms["state_form"] + # then + for field in self.superuser_exclusive_fields: + with self.subTest(field=field): + self.assertIn(field, form.fields) + + def test_should_not_show_all_fields_to_staff_admins_for_add(self): + # given + self.app.set_user(self.staff_admin) + page = self.app.get("/admin/authentication/state/add/") + # when + form = page.forms["state_form"] + # then + for field in self.superuser_exclusive_fields: + with self.subTest(field=field): + self.assertNotIn(field, form.fields) + + def test_should_show_all_fields_to_superuser_for_change(self): + # given + self.app.set_user(self.super_admin) + state = AuthUtils.get_member_state() + page = self.app.get(f"/admin/authentication/state/{state.pk}/change/") + # when + form = page.forms["state_form"] + # then + for field in self.superuser_exclusive_fields: + with self.subTest(field=field): + self.assertIn(field, form.fields) + + def test_should_not_show_all_fields_to_staff_admin_for_change(self): + # given + self.app.set_user(self.staff_admin) + state = AuthUtils.get_member_state() + page = self.app.get(f"/admin/authentication/state/{state.pk}/change/") + # when + form = page.forms["state_form"] + # then + for field in self.superuser_exclusive_fields: + with self.subTest(field=field): + self.assertNotIn(field, form.fields) + + class TestUserAdminChangeForm(TestCase): + fixtures = ["disable_analytics"] + @classmethod def setUpClass(cls) -> None: super().setUpClass() @@ -552,7 +621,7 @@ class TestUserAdminChangeForm(TestCase): def test_should_show_groups_available_to_user_with_blue_state_only(self): # given superuser = User.objects.create_superuser("Super") - user = AuthUtils.create_user("Bruce Wayne") + user = AuthUtils.create_user("bruce_wayne") character = AuthUtils.add_main_character_2( user, name="Bruce Wayne", @@ -579,6 +648,126 @@ class TestUserAdminChangeForm(TestCase): self.assertSetEqual(group_ids, {group_1.pk, group_2.pk}) +class TestUserAdminChangeFormSuperuserExclusiveEdits(WebTest): + fixtures = ["disable_analytics"] + + @classmethod + def setUpClass(cls) -> None: + super().setUpClass() + cls.super_admin = User.objects.create_superuser("super_admin") + cls.staff_admin = User.objects.create_user("staff_admin") + cls.staff_admin.is_staff = True + cls.staff_admin.save() + cls.staff_admin = AuthUtils.add_permissions_to_user_by_name( + [ + "auth.change_user", + "auth.view_user", + "authentication.change_user", + "authentication.change_userprofile", + "authentication.view_user" + ], + cls.staff_admin + ) + cls.superuser_exclusive_fields = [ + "is_staff", "is_superuser", "user_permissions" + ] + + def setUp(self) -> None: + self.user = AuthUtils.create_user("bruce_wayne") + + def test_should_show_all_fields_to_superuser_for_change(self): + # given + self.app.set_user(self.super_admin) + + page = self.app.get(f"/admin/authentication/user/{self.user.pk}/change/") + # when + form = page.forms["user_form"] + # then + for field in self.superuser_exclusive_fields: + with self.subTest(field=field): + self.assertIn(field, form.fields) + + def test_should_not_show_all_fields_to_staff_admin_for_change(self): + # given + self.app.set_user(self.staff_admin) + page = self.app.get(f"/admin/authentication/user/{self.user.pk}/change/") + # when + form = page.forms["user_form"] + # then + for field in self.superuser_exclusive_fields: + with self.subTest(field=field): + self.assertNotIn(field, form.fields) + + def test_should_allow_super_admin_to_add_restricted_group_to_user(self): + # given + self.app.set_user(self.super_admin) + group_restricted = Group.objects.create(name="restricted group") + group_restricted.authgroup.restricted = True + group_restricted.authgroup.save() + page = self.app.get(f"/admin/authentication/user/{self.user.pk}/change/") + form = page.forms["user_form"] + # when + form["groups"].select_multiple(texts=["restricted group"]) + response = form.submit("save") + # then + self.assertEqual(response.status_code, 302) + self.user.refresh_from_db() + self.assertIn( + "restricted group", self.user.groups.values_list("name", flat=True) + ) + + def test_should_not_allow_staff_admin_to_add_restricted_group_to_user(self): + # given + self.app.set_user(self.staff_admin) + group_restricted = Group.objects.create(name="restricted group") + group_restricted.authgroup.restricted = True + group_restricted.authgroup.save() + page = self.app.get(f"/admin/authentication/user/{self.user.pk}/change/") + form = page.forms["user_form"] + # when + form["groups"].select_multiple(texts=["restricted group"]) + response = form.submit("save") + # then + self.assertEqual(response.status_code, 200) + self.assertIn( + "You are not allowed to add or remove these restricted groups", + response.text + ) + + def test_should_not_allow_staff_admin_to_remove_restricted_group_from_user(self): + # given + self.app.set_user(self.staff_admin) + group_restricted = Group.objects.create(name="restricted group") + group_restricted.authgroup.restricted = True + group_restricted.authgroup.save() + self.user.groups.add(group_restricted) + page = self.app.get(f"/admin/authentication/user/{self.user.pk}/change/") + form = page.forms["user_form"] + # when + form["groups"].select_multiple(texts=[]) + response = form.submit("save") + # then + self.assertEqual(response.status_code, 200) + self.assertIn( + "You are not allowed to add or remove these restricted groups", + response.text + ) + + def test_should_allow_staff_admin_to_add_normal_group_to_user(self): + # given + self.app.set_user(self.super_admin) + Group.objects.create(name="normal group") + page = self.app.get(f"/admin/authentication/user/{self.user.pk}/change/") + form = page.forms["user_form"] + # when + form["groups"].select_multiple(texts=["normal group"]) + response = form.submit("save") + # then + self.assertEqual(response.status_code, 302) + self.user.refresh_from_db() + self.assertIn("normal group", self.user.groups.values_list("name", flat=True)) + + class TestMakeServicesHooksActions(TestCaseWithTestData): class MyServicesHookTypeA(ServicesHook): diff --git a/allianceauth/groupmanagement/admin.py b/allianceauth/groupmanagement/admin.py index 0f38bfe7..3657fc73 100644 --- a/allianceauth/groupmanagement/admin.py +++ b/allianceauth/groupmanagement/admin.py @@ -1,19 +1,20 @@ -from django import forms from django.apps import apps -from django.contrib.auth.models import Permission from django.contrib import admin -from django.contrib.auth.models import Group as BaseGroup, User -from django.core.exceptions import ValidationError +from django.contrib.auth.models import Group as BaseGroup +from django.contrib.auth.models import Permission, User from django.db.models import Count from django.db.models.functions import Lower -from django.db.models.signals import pre_save, post_save, pre_delete, \ - post_delete, m2m_changed +from django.db.models.signals import ( + m2m_changed, + post_delete, + post_save, + pre_delete, + pre_save +) from django.dispatch import receiver -from django.utils.timezone import now -from django.utils.translation import gettext_lazy as _ -from .models import AuthGroup, ReservedGroupName -from .models import GroupRequest +from .forms import GroupAdminForm, ReservedGroupNameAdminForm +from .models import AuthGroup, GroupRequest, ReservedGroupName if 'eve_autogroups' in apps.app_configs: _has_auto_groups = True @@ -28,10 +29,12 @@ class AuthGroupInlineAdmin(admin.StackedInline): 'description', 'group_leaders', 'group_leader_groups', - 'states', 'internal', + 'states', + 'internal', 'hidden', 'open', - 'public' + 'public', + 'restricted', ) verbose_name_plural = 'Auth Settings' verbose_name = '' @@ -50,6 +53,11 @@ class AuthGroupInlineAdmin(admin.StackedInline): def has_change_permission(self, request, obj=None): return request.user.has_perm('auth.change_group') + def get_readonly_fields(self, request, obj=None): + if not request.user.is_superuser: + return self.readonly_fields + ("restricted",) + return self.readonly_fields + if _has_auto_groups: class IsAutoGroupFilter(admin.SimpleListFilter): @@ -96,17 +104,6 @@ class HasLeaderFilter(admin.SimpleListFilter): return queryset -class GroupAdminForm(forms.ModelForm): - def clean_name(self): - my_name = self.cleaned_data['name'] - if ReservedGroupName.objects.filter(name__iexact=my_name).exists(): - raise ValidationError( - _("This name has been reserved and can not be used for groups."), - code='reserved_name' - ) - return my_name - - class GroupAdmin(admin.ModelAdmin): form = GroupAdminForm list_select_related = ('authgroup',) @@ -143,17 +140,14 @@ class GroupAdmin(admin.ModelAdmin): def _description(self, obj): return obj.authgroup.description + @admin.display(description="Members", ordering="member_count") def _member_count(self, obj): return obj.member_count - _member_count.short_description = 'Members' - _member_count.admin_order_field = 'member_count' - + @admin.display(boolean=True) def has_leader(self, obj): return obj.authgroup.group_leaders.exists() or obj.authgroup.group_leader_groups.exists() - has_leader.boolean = True - def _properties(self, obj): properties = list() if _has_auto_groups and ( @@ -172,11 +166,10 @@ class GroupAdmin(admin.ModelAdmin): properties.append('Public') if not properties: properties.append('Default') - + if obj.authgroup.restricted: + properties.append('Restricted') return properties - _properties.short_description = "properties" - filter_horizontal = ('permissions',) inlines = (AuthGroupInlineAdmin,) @@ -192,6 +185,11 @@ class GroupAdmin(admin.ModelAdmin): ag_instance.save() formset.save() + def get_readonly_fields(self, request, obj=None): + if not request.user.is_superuser: + return self.readonly_fields + ("permissions",) + return self.readonly_fields + class Group(BaseGroup): class Meta: @@ -216,33 +214,10 @@ class GroupRequestAdmin(admin.ModelAdmin): 'leave_request', ) + @admin.display(boolean=True, description="is leave request") def _leave_request(self, obj) -> True: return obj.leave_request - _leave_request.short_description = 'is leave request' - _leave_request.boolean = True - - -class ReservedGroupNameAdminForm(forms.ModelForm): - def __init__(self, *args, **kwargs): - super().__init__(*args, **kwargs) - self.fields['created_by'].initial = self.current_user.username - self.fields['created_at'].initial = _("(auto)") - - created_by = forms.CharField(disabled=True) - created_at = forms.CharField(disabled=True) - - def clean_name(self): - my_name = self.cleaned_data['name'].lower() - if Group.objects.filter(name__iexact=my_name).exists(): - raise ValidationError( - _("There already exists a group with that name."), code='already_exists' - ) - return my_name - - def clean_created_at(self): - return now() - @admin.register(ReservedGroupName) class ReservedGroupNameAdmin(admin.ModelAdmin): diff --git a/allianceauth/groupmanagement/forms.py b/allianceauth/groupmanagement/forms.py new file mode 100644 index 00000000..1214a7c2 --- /dev/null +++ b/allianceauth/groupmanagement/forms.py @@ -0,0 +1,39 @@ +from django import forms +from django.contrib.auth.models import Group +from django.core.exceptions import ValidationError +from django.utils.timezone import now +from django.utils.translation import gettext_lazy as _ + +from .models import ReservedGroupName + + +class GroupAdminForm(forms.ModelForm): + def clean_name(self): + my_name = self.cleaned_data['name'] + if ReservedGroupName.objects.filter(name__iexact=my_name).exists(): + raise ValidationError( + _("This name has been reserved and can not be used for groups."), + code='reserved_name' + ) + return my_name + + +class ReservedGroupNameAdminForm(forms.ModelForm): + def __init__(self, *args, **kwargs): + super().__init__(*args, **kwargs) + self.fields['created_by'].initial = self.current_user.username + self.fields['created_at'].initial = _("(auto)") + + created_by = forms.CharField(disabled=True) + created_at = forms.CharField(disabled=True) + + def clean_name(self): + my_name = self.cleaned_data['name'].lower() + if Group.objects.filter(name__iexact=my_name).exists(): + raise ValidationError( + _("There already exists a group with that name."), code='already_exists' + ) + return my_name + + def clean_created_at(self): + return now() diff --git a/allianceauth/groupmanagement/migrations/0019_adding_restricted_to_groups.py b/allianceauth/groupmanagement/migrations/0019_adding_restricted_to_groups.py new file mode 100644 index 00000000..7d5f93d1 --- /dev/null +++ b/allianceauth/groupmanagement/migrations/0019_adding_restricted_to_groups.py @@ -0,0 +1,18 @@ +# Generated by Django 3.2.10 on 2022-04-08 19:30 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ('groupmanagement', '0018_reservedgroupname'), + ] + + operations = [ + migrations.AddField( + model_name='authgroup', + name='restricted', + field=models.BooleanField(default=False, help_text='Group is restricted. This means that adding or removing users for this group requires a superuser admin.'), + ), + ] diff --git a/allianceauth/groupmanagement/models.py b/allianceauth/groupmanagement/models.py index 9befa8d0..102f70a7 100644 --- a/allianceauth/groupmanagement/models.py +++ b/allianceauth/groupmanagement/models.py @@ -13,6 +13,7 @@ from allianceauth.notifications import notify class GroupRequest(models.Model): """Request from a user for joining or leaving a group.""" + leave_request = models.BooleanField(default=0) user = models.ForeignKey(User, on_delete=models.CASCADE) group = models.ForeignKey(Group, on_delete=models.CASCADE) @@ -44,6 +45,7 @@ class GroupRequest(models.Model): class RequestLog(models.Model): """Log entry about who joined and left a group and who approved it.""" + request_type = models.BooleanField(null=True) group = models.ForeignKey(Group, on_delete=models.CASCADE) request_info = models.CharField(max_length=254) @@ -95,6 +97,7 @@ class AuthGroup(models.Model): Open - Users are automatically accepted into the group Not Open - Users requests must be approved before they are added to the group """ + group = models.OneToOneField(Group, on_delete=models.CASCADE, primary_key=True) internal = models.BooleanField( default=True, @@ -126,6 +129,13 @@ class AuthGroup(models.Model): "are no longer authenticated." ) ) + restricted = models.BooleanField( + default=False, + help_text=_( + "Group is restricted. This means that adding or removing users " + "for this group requires a superuser admin." + ) + ) group_leaders = models.ManyToManyField( User, related_name='leads_groups', @@ -185,6 +195,7 @@ class ReservedGroupName(models.Model): This enables AA to ignore groups on other services (e.g. Discord) with that name. """ + name = models.CharField( _('name'), max_length=150, diff --git a/allianceauth/groupmanagement/tests/test_admin.py b/allianceauth/groupmanagement/tests/test_admin.py index 02c5a848..3b4a50c1 100644 --- a/allianceauth/groupmanagement/tests/test_admin.py +++ b/allianceauth/groupmanagement/tests/test_admin.py @@ -1,5 +1,7 @@ from unittest.mock import patch +from django_webtest import WebTest + from django.conf import settings from django.contrib import admin from django.contrib.admin.sites import AdminSite @@ -10,8 +12,10 @@ from allianceauth.authentication.models import CharacterOwnership, State from allianceauth.eveonline.models import ( EveCharacter, EveCorporationInfo, EveAllianceInfo ) -from ..admin import HasLeaderFilter, GroupAdmin, Group +from allianceauth.tests.auth_utils import AuthUtils + from . import get_admin_change_view_url +from ..admin import HasLeaderFilter, GroupAdmin, Group from ..models import ReservedGroupName @@ -33,7 +37,6 @@ class MockRequest: class TestGroupAdmin(TestCase): - @classmethod def setUpClass(cls): super().setUpClass() @@ -468,6 +471,74 @@ class TestGroupAdmin(TestCase): self.assertFalse(Group.objects.filter(name="new group").exists()) +class TestGroupAdminChangeFormSuperuserExclusiveEdits(WebTest): + @classmethod + def setUpClass(cls) -> None: + super().setUpClass() + cls.super_admin = User.objects.create_superuser("super_admin") + cls.staff_admin = User.objects.create_user("staff_admin") + cls.staff_admin.is_staff = True + cls.staff_admin.save() + cls.staff_admin = AuthUtils.add_permissions_to_user_by_name( + [ + "auth.add_group", + "auth.change_group", + "auth.view_group", + "groupmanagement.add_group", + "groupmanagement.change_group", + "groupmanagement.view_group", + ], + cls.staff_admin + ) + cls.superuser_exclusive_fields = ["permissions", "authgroup-0-restricted"] + + def test_should_show_all_fields_to_superuser_for_add(self): + # given + self.app.set_user(self.super_admin) + page = self.app.get("/admin/groupmanagement/group/add/") + # when + form = page.forms["group_form"] + # then + for field in self.superuser_exclusive_fields: + with self.subTest(field=field): + self.assertIn(field, form.fields) + + def test_should_not_show_all_fields_to_staff_admins_for_add(self): + # given + self.app.set_user(self.staff_admin) + page = self.app.get("/admin/groupmanagement/group/add/") + # when + form = page.forms["group_form"] + # then + for field in self.superuser_exclusive_fields: + with self.subTest(field=field): + self.assertNotIn(field, form.fields) + + def test_should_show_all_fields_to_superuser_for_change(self): + # given + self.app.set_user(self.super_admin) + group = Group.objects.create(name="Dummy group") + page = self.app.get(f"/admin/groupmanagement/group/{group.pk}/change/") + # when + form = page.forms["group_form"] + # then + for field in self.superuser_exclusive_fields: + with self.subTest(field=field): + self.assertIn(field, form.fields) + + def test_should_not_show_all_fields_to_staff_admin_for_change(self): + # given + self.app.set_user(self.staff_admin) + group = Group.objects.create(name="Dummy group") + page = self.app.get(f"/admin/groupmanagement/group/{group.pk}/change/") + # when + form = page.forms["group_form"] + # then + for field in self.superuser_exclusive_fields: + with self.subTest(field=field): + self.assertNotIn(field, form.fields) + + class TestReservedGroupNameAdmin(TestCase): @classmethod def setUpClass(cls): diff --git a/allianceauth/tests/auth_utils.py b/allianceauth/tests/auth_utils.py index 1ee824ec..7f759b84 100644 --- a/allianceauth/tests/auth_utils.py +++ b/allianceauth/tests/auth_utils.py @@ -1,3 +1,5 @@ +from typing import List + from django.contrib.auth.models import User, Group, Permission from django.db.models.signals import m2m_changed, pre_save, post_save from django.test import TestCase @@ -258,6 +260,23 @@ class AuthUtils: p = cls.get_permission_by_name(perm) return cls.add_permissions_to_user([p], user, disconnect_signals) + @classmethod + def add_permissions_to_user_by_name( + cls, perms: List[str], user: User, disconnect_signals: bool = True + ) -> User: + """Add permissions given by name to a user + + Args: + perms: List of permission names as 'app_label.codename' + user: user object + disconnect_signals: whether to run process without signals + + Returns: + Updated user object + """ + permissions = [cls.get_permission_by_name(perm) for perm in perms] + return cls.add_permissions_to_user(permissions, user, disconnect_signals) + @staticmethod def get_permission_by_name(perm: str) -> Permission: """returns permission specified by qualified name diff --git a/docs/_static/images/features/core/admin_site.png b/docs/_static/images/features/core/admin_site.png new file mode 100644 index 00000000..d26e5dbd Binary files /dev/null and b/docs/_static/images/features/core/admin_site.png differ diff --git a/docs/features/core/admin_site.md b/docs/features/core/admin_site.md new file mode 100644 index 00000000..2ab8d1ff --- /dev/null +++ b/docs/features/core/admin_site.md @@ -0,0 +1,96 @@ +# Admin Site + +The admin site allows administrators to configure, manage and trouble shoot Alliance Auth and all it's applications and services. E.g. you can create new groups and assign groups to users. + +You can open the admin site by clicking on "Admin" in the drop down menu for a user that has access. + +![Admin Site](/_static/images/features/core/admin_site.png) + +## Setup for small to medium size installations + +For small to medium size alliances it is often sufficient to have no more then two superuser admins (admins that also are superusers). Having two admins usually makes sense, so you can have one primary and one backup. + +```eval_rst +.. warning:: + Superusers have read & write access to everything on your AA installation. Superusers also automatically have all permissions and therefore access to all features of your apps. Therefore we recommend to be very careful to whom you give superuser privileges. +``` + +## Setup for large installations + +For large alliances and coalitions you may want to have a couple of administrators to be able to distribute and handle the work load. However, having a larger number of superusers may be a security concern. + +As an alternative to superusers admins you can define staff admins. Staff admins can perform most of the daily admin work, but are not superusers and therefore can be restricted in what they can access. + +To create a staff admin you need to do two things: + +1. Enable the `is_staff` property for a user +1. Give the user permissions for admin tasks + +```eval_rst +.. note:: + Note that staff admins have the following limitations: + + - Can not promote users to staff + - Can not promote users to superuser + - Can not add/remove permissions for users, groups and states + + These limitations exist to prevent staff admins to promote themselves to quasi superusers. Only superusers can perform these actions. +``` + +### Staff property + +Access to the admin site is restricted. Users needs to have the `is_staff` property to be able to open the site at all. The superuser that is created during the installation +process will automatically have access to the admin site. + +```eval_rst +.. hint:: + Without any permissions a "staff user" can open the admin site, but can neither view nor edit anything except for viewing the list of permissions. +``` + +### Permissions for common admin tasks + +Here is a list of permissions a staff admin would need to perform some common admin tasks: + +#### Edit users + +- auth | user | Can view user +- auth | user | Can change user +- authentication | user | Can view user +- authentication | user | Can change user +- authentication | user profile | Can change profile + +#### Delete users + +- auth | user | Can view user +- auth | user | Can delete user +- authentication | user | Can delete user +- authentication | user profile | Can delete user profile + +#### Add & edit states + +- authentication | state | Can add state +- authentication | state | Can change state +- authentication | state | Can view state + +#### Delete states + +- authentication | state | Can delete state +- authentication | state | Can view state + +#### Add & edit groups + +- auth | group | Can add group +- auth | group | Can change group +- auth | group | Can view group +- authentication | group | Can add group +- authentication | group | Can change group +- authentication | group | Can view group + +#### Delete groups + +- auth | group | Can delete group +- authentication | group | Can delete group + +### Permissions for other apps + +The permissions a staff admin needs to perform tasks for other applications depends on how the applications are configured. the default is to have four permissions (change, delete, edit view) for each model of the applications. The view permission is usually required to see the model list on the admin site and the other three permissions are required to perform the respective action to an object of that model. However, app developer can chose to define permissions differently. diff --git a/docs/features/core/groups.md b/docs/features/core/groups.md index 2c15cb91..2868ca4d 100644 --- a/docs/features/core/groups.md +++ b/docs/features/core/groups.md @@ -38,6 +38,10 @@ The key difference is that the group is completely unmanaged by Auth. **Once a m Most people won't have a use for public groups, though it can be useful if you wish to allow public access to some services. You can grant service permissions on a public group to allow this behavior. +### Restricted + +When a group is restricted only superuser admins can directly add or remove them to/from users. The purpose of this property is prevent staff admins from assigning themselves to groups that are security sensitive. The "restricted" property can be combined with all the other properties. + ```eval_rst .. _ref-reserved-group-names: ``` diff --git a/docs/features/core/index.md b/docs/features/core/index.md index 1c518e3a..02007bd6 100644 --- a/docs/features/core/index.md +++ b/docs/features/core/index.md @@ -11,4 +11,5 @@ Managing access to applications and services is one of the core functions of **A groups analytics notifications + admin_site ```