From af47587116606b30ff407d57ae2ebdc4a417259b Mon Sep 17 00:00:00 2001 From: imperosol Date: Tue, 31 Dec 2024 16:09:08 +0100 Subject: [PATCH] Split groups and ban groups --- core/admin.py | 22 ++- core/management/commands/populate.py | 13 +- ..._description_alter_user_groups_and_more.py | 164 ++++++++++++++++++ core/models.py | 89 +++++++++- core/tests/test_core.py | 4 - counter/tests/test_counter.py | 10 +- sith/settings.py | 13 +- 7 files changed, 285 insertions(+), 30 deletions(-) create mode 100644 core/migrations/0043_bangroup_alter_group_description_alter_user_groups_and_more.py diff --git a/core/admin.py b/core/admin.py index 601ba636..5de89ada 100644 --- a/core/admin.py +++ b/core/admin.py @@ -17,7 +17,7 @@ from django.contrib import admin from django.contrib.auth.models import Group as AuthGroup from django.contrib.auth.models import Permission -from core.models import Group, OperationLog, Page, SithFile, User +from core.models import BanGroup, Group, OperationLog, Page, SithFile, User, UserBan admin.site.unregister(AuthGroup) @@ -30,6 +30,19 @@ class GroupAdmin(admin.ModelAdmin): autocomplete_fields = ("permissions",) +@admin.register(BanGroup) +class BanGroupAdmin(admin.ModelAdmin): + list_display = ("name", "description") + search_fields = ("name",) + autocomplete_fields = ("permissions",) + + +class UserBanInline(admin.TabularInline): + model = UserBan + extra = 0 + autocomplete_fields = ("ban_group",) + + @admin.register(User) class UserAdmin(admin.ModelAdmin): list_display = ("first_name", "last_name", "username", "email", "nick_name") @@ -42,9 +55,16 @@ class UserAdmin(admin.ModelAdmin): "user_permissions", "groups", ) + inlines = (UserBanInline,) search_fields = ["first_name", "last_name", "username"] +@admin.register(UserBan) +class UserBanAdmin(admin.ModelAdmin): + list_display = ("user", "ban_group", "created_at", "expires_at") + autocomplete_fields = ("user", "ban_group") + + @admin.register(Permission) class PermissionAdmin(admin.ModelAdmin): search_fields = ("codename",) diff --git a/core/management/commands/populate.py b/core/management/commands/populate.py index 3ed1025d..e3d6d8e4 100644 --- a/core/management/commands/populate.py +++ b/core/management/commands/populate.py @@ -48,7 +48,7 @@ from accounting.models import ( from club.models import Club, Membership from com.calendar import IcsCalendar from com.models import News, NewsDate, Sith, Weekmail -from core.models import Group, Page, PageRev, SithFile, User +from core.models import BanGroup, Group, Page, PageRev, SithFile, User from core.utils import resize_image from counter.models import Counter, Product, ProductType, StudentCard from election.models import Candidature, Election, ElectionList, Role @@ -94,6 +94,7 @@ class Command(BaseCommand): Sith.objects.create(weekmail_destinations="etudiants@git.an personnel@git.an") Site.objects.create(domain=settings.SITH_URL, name=settings.SITH_NAME) groups = self._create_groups() + self._create_ban_groups() root = User.objects.create_superuser( id=0, @@ -951,11 +952,6 @@ Welcome to the wiki page! ) ) ) - Group.objects.create( - name="Banned from buying alcohol", is_manually_manageable=True - ) - Group.objects.create(name="Banned from counters", is_manually_manageable=True) - Group.objects.create(name="Banned to subscribe", is_manually_manageable=True) sas_admin = Group.objects.create(name="SAS admin", is_manually_manageable=True) sas_admin.permissions.add( *list( @@ -995,3 +991,8 @@ Welcome to the wiki page! sas_admin=sas_admin, pedagogy_admin=pedagogy_admin, ) + + def _create_ban_groups(self): + BanGroup.objects.create(name="Banned from buying alcohol", description="") + BanGroup.objects.create(name="Banned from counters", description="") + BanGroup.objects.create(name="Banned to subscribe", description="") diff --git a/core/migrations/0043_bangroup_alter_group_description_alter_user_groups_and_more.py b/core/migrations/0043_bangroup_alter_group_description_alter_user_groups_and_more.py new file mode 100644 index 00000000..daba4097 --- /dev/null +++ b/core/migrations/0043_bangroup_alter_group_description_alter_user_groups_and_more.py @@ -0,0 +1,164 @@ +# Generated by Django 4.2.17 on 2024-12-31 13:30 + +import django.contrib.auth.models +import django.db.models.deletion +from django.conf import settings +from django.db import migrations, models +from django.db.migrations.state import StateApps + + +def migrate_ban_groups(apps: StateApps, schema_editor): + Group = apps.get_model("core", "Group") + BanGroup = apps.get_model("core", "BanGroup") + ban_group_ids = [ + settings.SITH_GROUP_BANNED_ALCOHOL_ID, + settings.SITH_GROUP_BANNED_COUNTER_ID, + settings.SITH_GROUP_BANNED_SUBSCRIPTION_ID, + ] + # this is a N+1 Queries, but the prod database has a grand total of 3 ban groups + for group in Group.objects.filter(id__in=ban_group_ids): + # auth_group, which both Group and BanGroup inherit, + # is unique by name. + # If we tried give the exact same name to the migrated BanGroup + # before deleting the corresponding Group, + # we would have an IntegrityError. + # So we append a space to the name, in order to create a name + # that will look the same, but that isn't really the same. + ban_group = BanGroup.objects.create( + name=f"{group.name} ", + description=group.description, + ) + perms = list(group.permissions.values_list("id", flat=True)) + if perms: + ban_group.permissions.add(*perms) + ban_group.users.add( + *group.users.values_list("id", flat=True), through_defaults={"reason": ""} + ) + group.delete() + # now that the original group is no longer there, + # we can remove the appended space + ban_group.name = ban_group.name.strip() + ban_group.save() + + +class Migration(migrations.Migration): + dependencies = [ + ("auth", "0012_alter_user_first_name_max_length"), + ("core", "0042_invert_is_manually_manageable_20250104_1742"), + ] + + operations = [ + migrations.CreateModel( + name="BanGroup", + fields=[ + ( + "group_ptr", + models.OneToOneField( + auto_created=True, + on_delete=django.db.models.deletion.CASCADE, + parent_link=True, + primary_key=True, + serialize=False, + to="auth.group", + ), + ), + ("description", models.TextField(verbose_name="description")), + ], + bases=("auth.group",), + managers=[ + ("objects", django.contrib.auth.models.GroupManager()), + ], + options={ + "verbose_name": "ban group", + "verbose_name_plural": "ban groups", + }, + ), + migrations.AlterField( + model_name="group", + name="description", + field=models.TextField(verbose_name="description"), + ), + migrations.AlterField( + model_name="user", + name="groups", + field=models.ManyToManyField( + help_text="The groups this user belongs to. A user will get all permissions granted to each of their groups.", + related_name="users", + to="core.group", + verbose_name="groups", + ), + ), + migrations.CreateModel( + name="UserBan", + fields=[ + ( + "id", + models.AutoField( + auto_created=True, + primary_key=True, + serialize=False, + verbose_name="ID", + ), + ), + ( + "created_at", + models.DateTimeField(auto_now_add=True, verbose_name="created at"), + ), + ( + "expires_at", + models.DateTimeField( + blank=True, + help_text="When the ban should be removed. Currently, there is no automatic removal, so this is purely indicative. Automatic ban removal may be implemented later on.", + null=True, + verbose_name="expires at", + ), + ), + ("reason", models.TextField(verbose_name="reason")), + ( + "ban_group", + models.ForeignKey( + on_delete=django.db.models.deletion.CASCADE, + related_name="user_bans", + to="core.bangroup", + verbose_name="ban type", + ), + ), + ( + "user", + models.ForeignKey( + on_delete=django.db.models.deletion.CASCADE, + related_name="bans", + to=settings.AUTH_USER_MODEL, + verbose_name="user", + ), + ), + ], + ), + migrations.AddField( + model_name="user", + name="ban_groups", + field=models.ManyToManyField( + help_text="The bans this user has received.", + related_name="users", + through="core.UserBan", + to="core.bangroup", + verbose_name="ban groups", + ), + ), + migrations.AddConstraint( + model_name="userban", + constraint=models.UniqueConstraint( + fields=("ban_group", "user"), name="unique_ban_type_per_user" + ), + ), + migrations.AddConstraint( + model_name="userban", + constraint=models.CheckConstraint( + check=models.Q(("expires_at__gte", models.F("created_at"))), + name="user_ban_end_after_start", + ), + ), + migrations.RunPython( + migrate_ban_groups, reverse_code=migrations.RunPython.noop, elidable=True + ), + ] diff --git a/core/models.py b/core/models.py index 945fd7d0..278182ac 100644 --- a/core/models.py +++ b/core/models.py @@ -42,7 +42,7 @@ from django.core.cache import cache from django.core.exceptions import PermissionDenied, ValidationError from django.core.mail import send_mail from django.db import models, transaction -from django.db.models import Exists, OuterRef, Q +from django.db.models import Exists, F, OuterRef, Q from django.urls import reverse from django.utils import timezone from django.utils.functional import cached_property @@ -65,8 +65,7 @@ class Group(AuthGroup): default=False, help_text=_("If False, this shouldn't be shown on group management pages"), ) - #: Description of the group - description = models.CharField(_("description"), max_length=60) + description = models.TextField(_("description")) def get_absolute_url(self) -> str: return reverse("core:group_list") @@ -134,6 +133,28 @@ def get_group(*, pk: int | None = None, name: str | None = None) -> Group | None return group +class BanGroup(AuthGroup): + """An anti-group, that removes permissions instead of giving them. + + Users are linked to BanGroups through UserBan objects. + + Example: + ```python + user = User.objects.get(username="...") + ban_group = BanGroup.objects.first() + UserBan.objects.create(user=user, ban_group=ban_group, reason="...") + + assert user.ban_groups.contains(ban_group) + ``` + """ + + description = models.TextField(_("description")) + + class Meta: + verbose_name = _("ban group") + verbose_name_plural = _("ban groups") + + class UserQuerySet(models.QuerySet): def filter_inactive(self) -> Self: from counter.models import Refilling, Selling @@ -184,7 +205,13 @@ class User(AbstractUser): "granted to each of their groups." ), related_name="users", - blank=True, + ) + ban_groups = models.ManyToManyField( + BanGroup, + verbose_name=_("ban groups"), + through="UserBan", + help_text=_("The bans this user has received."), + related_name="users", ) home = models.OneToOneField( "SithFile", @@ -424,12 +451,12 @@ class User(AbstractUser): ) @cached_property - def is_banned_alcohol(self): - return self.is_in_group(pk=settings.SITH_GROUP_BANNED_ALCOHOL_ID) + def is_banned_alcohol(self) -> bool: + return self.ban_groups.filter(id=settings.SITH_GROUP_BANNED_ALCOHOL_ID).exists() @cached_property - def is_banned_counter(self): - return self.is_in_group(pk=settings.SITH_GROUP_BANNED_COUNTER_ID) + def is_banned_counter(self) -> bool: + return self.ban_groups.filter(id=settings.SITH_GROUP_BANNED_COUNTER_ID).exists() @cached_property def age(self) -> int: @@ -731,6 +758,52 @@ class AnonymousUser(AuthAnonymousUser): return _("Visitor") +class UserBan(models.Model): + """A ban of a user. + + A user can be banned for a specific reason, for a specific duration. + The expiration date is indicative, and the ban should be removed manually. + """ + + ban_group = models.ForeignKey( + BanGroup, + verbose_name=_("ban type"), + related_name="user_bans", + on_delete=models.CASCADE, + ) + user = models.ForeignKey( + User, verbose_name=_("user"), related_name="bans", on_delete=models.CASCADE + ) + created_at = models.DateTimeField(_("created at"), auto_now_add=True) + expires_at = models.DateTimeField( + _("expires at"), + null=True, + blank=True, + help_text=_( + "When the ban should be removed. " + "Currently, there is no automatic removal, so this is purely indicative. " + "Automatic ban removal may be implemented later on." + ), + ) + reason = models.TextField(_("reason")) + + class Meta: + verbose_name = _("user ban") + verbose_name_plural = _("user bans") + constraints = [ + models.UniqueConstraint( + fields=["ban_group", "user"], name="unique_ban_type_per_user" + ), + models.CheckConstraint( + check=Q(expires_at__gte=F("created_at")), + name="user_ban_end_after_start", + ), + ] + + def __str__(self): + return f"Ban of user {self.user.id}" + + class Preferences(models.Model): user = models.OneToOneField( User, related_name="_preferences", on_delete=models.CASCADE diff --git a/core/tests/test_core.py b/core/tests/test_core.py index a152b579..878db4e4 100644 --- a/core/tests/test_core.py +++ b/core/tests/test_core.py @@ -358,9 +358,6 @@ class TestUserIsInGroup(TestCase): cls.accounting_admin = Group.objects.get(name="Accounting admin") cls.com_admin = Group.objects.get(name="Communication admin") cls.counter_admin = Group.objects.get(name="Counter admin") - cls.banned_alcohol = Group.objects.get(name="Banned from buying alcohol") - cls.banned_counters = Group.objects.get(name="Banned from counters") - cls.banned_subscription = Group.objects.get(name="Banned to subscribe") cls.sas_admin = Group.objects.get(name="SAS admin") cls.club = baker.make(Club) cls.main_club = Club.objects.get(id=1) @@ -373,7 +370,6 @@ class TestUserIsInGroup(TestCase): self.assert_in_public_group(user) for group in ( self.root_group, - self.banned_counters, self.accounting_admin, self.sas_admin, self.subscribers, diff --git a/counter/tests/test_counter.py b/counter/tests/test_counter.py index 1b378d5b..3fdc7099 100644 --- a/counter/tests/test_counter.py +++ b/counter/tests/test_counter.py @@ -31,7 +31,7 @@ from model_bakery import baker from club.models import Club, Membership from core.baker_recipes import board_user, subscriber_user, very_old_subscriber_user -from core.models import Group, User +from core.models import BanGroup, User from counter.baker_recipes import product_recipe from counter.models import ( Counter, @@ -229,11 +229,11 @@ class TestCounterClick(TestFullClickBase): cls.set_age(cls.banned_alcohol_customer, 20) cls.set_age(cls.underage_customer, 17) - cls.banned_alcohol_customer.groups.add( - Group.objects.get(pk=settings.SITH_GROUP_BANNED_ALCOHOL_ID) + cls.banned_alcohol_customer.ban_groups.add( + BanGroup.objects.get(pk=settings.SITH_GROUP_BANNED_ALCOHOL_ID) ) - cls.banned_counter_customer.groups.add( - Group.objects.get(pk=settings.SITH_GROUP_BANNED_COUNTER_ID) + cls.banned_counter_customer.ban_groups.add( + BanGroup.objects.get(pk=settings.SITH_GROUP_BANNED_COUNTER_ID) ) cls.beer = product_recipe.make( diff --git a/sith/settings.py b/sith/settings.py index a88734d3..42e46603 100644 --- a/sith/settings.py +++ b/sith/settings.py @@ -363,12 +363,13 @@ SITH_GROUP_OLD_SUBSCRIBERS_ID = 4 SITH_GROUP_ACCOUNTING_ADMIN_ID = 5 SITH_GROUP_COM_ADMIN_ID = 6 SITH_GROUP_COUNTER_ADMIN_ID = 7 -SITH_GROUP_BANNED_ALCOHOL_ID = 8 -SITH_GROUP_BANNED_COUNTER_ID = 9 -SITH_GROUP_BANNED_SUBSCRIPTION_ID = 10 -SITH_GROUP_SAS_ADMIN_ID = 11 -SITH_GROUP_FORUM_ADMIN_ID = 12 -SITH_GROUP_PEDAGOGY_ADMIN_ID = 13 +SITH_GROUP_SAS_ADMIN_ID = 8 +SITH_GROUP_FORUM_ADMIN_ID = 9 +SITH_GROUP_PEDAGOGY_ADMIN_ID = 10 + +SITH_GROUP_BANNED_ALCOHOL_ID = 11 +SITH_GROUP_BANNED_COUNTER_ID = 12 +SITH_GROUP_BANNED_SUBSCRIPTION_ID = 13 SITH_CLUB_REFOUND_ID = 89 SITH_COUNTER_REFOUND_ID = 38