Split groups and ban groups

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",)

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="")

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
+        ),
+    ]

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.TextField(_("description"))
-    description = models.CharField(_("description"), max_length=60)
 
     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):
+    def is_banned_alcohol(self) -> bool:
-        return self.is_in_group(pk=settings.SITH_GROUP_BANNED_ALCOHOL_ID)
+        return self.ban_groups.filter(id=settings.SITH_GROUP_BANNED_ALCOHOL_ID).exists()
 
     @cached_property
-    def is_banned_counter(self):
+    def is_banned_counter(self) -> bool:
-        return self.is_in_group(pk=settings.SITH_GROUP_BANNED_COUNTER_ID)
+        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

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,

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(
+        cls.banned_alcohol_customer.ban_groups.add(
-            Group.objects.get(pk=settings.SITH_GROUP_BANNED_ALCOHOL_ID)
+            BanGroup.objects.get(pk=settings.SITH_GROUP_BANNED_ALCOHOL_ID)
         )
-        cls.banned_counter_customer.groups.add(
+        cls.banned_counter_customer.ban_groups.add(
-            Group.objects.get(pk=settings.SITH_GROUP_BANNED_COUNTER_ID)
+            BanGroup.objects.get(pk=settings.SITH_GROUP_BANNED_COUNTER_ID)
         )
 
         cls.beer = product_recipe.make(

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_SAS_ADMIN_ID = 8
-SITH_GROUP_BANNED_COUNTER_ID = 9
+SITH_GROUP_FORUM_ADMIN_ID = 9
-SITH_GROUP_BANNED_SUBSCRIPTION_ID = 10
+SITH_GROUP_PEDAGOGY_ADMIN_ID = 10
-SITH_GROUP_SAS_ADMIN_ID = 11
+
-SITH_GROUP_FORUM_ADMIN_ID = 12
+SITH_GROUP_BANNED_ALCOHOL_ID = 11
-SITH_GROUP_PEDAGOGY_ADMIN_ID = 13
+SITH_GROUP_BANNED_COUNTER_ID = 12
+SITH_GROUP_BANNED_SUBSCRIPTION_ID = 13
 
 SITH_CLUB_REFOUND_ID = 89
 SITH_COUNTER_REFOUND_ID = 38