diff --git a/core/auth/backends.py b/core/auth/backends.py index 7d6fd76c..fced226a 100644 --- a/core/auth/backends.py +++ b/core/auth/backends.py @@ -1,15 +1,16 @@ from __future__ import annotations -from typing import TYPE_CHECKING +import typing from django.conf import settings from django.contrib.auth.backends import ModelBackend from django.contrib.auth.models import Permission +from django.db.models import Exists, OuterRef, Q, QuerySet -from core.models import Group +from core.models import Group, User -if TYPE_CHECKING: - from core.models import User +if typing.TYPE_CHECKING: + from django.db.models.base import Model class SithModelBackend(ModelBackend): @@ -40,3 +41,53 @@ class SithModelBackend(ModelBackend): return Permission.objects.filter( group__group__in=groups.values_list("pk", flat=True) ) + + @typing.override + def with_perm( + self, + perm: str | Permission, + is_active: bool | None = True, + include_superusers: bool = False, + obj: Model | None = None, + ) -> QuerySet[User]: + """Return users that have permission "perm". + + Contrary to the base django method, superusers aren't included in the + result. + This is because the OR operation to include superusers in the query result + utterly destroy the query performances on postgres + (it makes it like 1000x slower, and I'm not even kidding). + To overcome that, we could use a UNION instead, but then we wouldn't + be able to perform further filter operations on the queryset. + Thus, the `include_superusers` argument is not used at all. + + Because of that, it is useless to set `include_superusers`, + as it will be silently ignored. + The only reason it's still there is not to break the interface + of the base class. + """ + if isinstance(perm, str): + try: + app_label, codename = perm.split(".") + except ValueError as e: + raise ValueError( + "Permission name should be in the form " + "app_label.permission_codename." + ) from e + permission_q = Q(codename=codename, content_type__app_label=app_label) + elif isinstance(perm, Permission): + permission_q = Q(pk=perm.pk) + else: + raise TypeError( + "The `perm` argument must be a string or a permission instance." + ) + + user_q = Exists( + Permission.objects.filter( + Q(group__group__users=OuterRef("pk")) | Q(user=OuterRef("pk")), + permission_q, + ) + ) + if is_active is not None: + user_q &= Q(is_active=is_active) + return User.objects.filter(user_q) diff --git a/core/management/commands/populate.py b/core/management/commands/populate.py index 05a650d0..a85da98e 100644 --- a/core/management/commands/populate.py +++ b/core/management/commands/populate.py @@ -747,6 +747,7 @@ class Command(BaseCommand): "add_subscription", "add_membership", "view_hidden_user", + "add_refilling", ] ) ) diff --git a/core/tests/test_auth_backend.py b/core/tests/test_auth_backend.py new file mode 100644 index 00000000..97eaac1b --- /dev/null +++ b/core/tests/test_auth_backend.py @@ -0,0 +1,27 @@ +import pytest +from django.contrib.auth.models import Permission +from model_bakery import baker + +from core.models import Group, User + + +@pytest.mark.django_db +def test_with_perm(): + """Test that `SithModelBackend.with_perm` works as intended.""" + perms = baker.make(Permission, _quantity=4) + groups = baker.make(Group, _quantity=2) + groups[0].permissions.set(perms[0:2]) + groups[1].permissions.set(perms[2:4]) + users = [ + baker.make(User), + baker.make(User, groups=[groups[0]]), + baker.make(User, groups=[groups[1]]), + baker.make(User, user_permissions=[perms[0]]), + baker.make(User, user_permissions=[perms[2]]), + baker.make(User, groups=[groups[1]], user_permissions=[perms[0]]), + ] + + expected = [users[1], users[3], users[5]] + assert list(User.objects.with_perm(perms[0])) == expected + str_repr = f"{perms[0].content_type.app_label}.{perms[0].codename}" + assert list(User.objects.with_perm(str_repr)) == expected diff --git a/counter/models.py b/counter/models.py index 7f6db9b4..1ccf1d8f 100644 --- a/counter/models.py +++ b/counter/models.py @@ -669,13 +669,18 @@ class Counter(models.Model): """Update the barman activity to prevent timeout.""" self.permanencies.filter(end=None).update(activity=timezone.now()) + @cached_property def can_refill(self) -> bool: - """Show if the counter authorize the refilling with physic money.""" - if self.type != "BAR": - return False - # at least one of the barmen is in the AE board - ae = Club.objects.get(id=settings.SITH_MAIN_CLUB_ID) - return any(ae.get_membership_for(barman) for barman in self.barmen_list) + """Show if the counter authorize the refilling with physic money. + + Refills are authorized if a user having the required permission + is currently logged in. + """ + return self.type == "BAR" and ( + User.objects.with_perm("counter.add_refilling") + .filter(id__in=[u.id for u in self.barmen_list]) + .exists() + ) def get_top_barmen(self) -> QuerySet: """Return a QuerySet querying the office hours stats of all the barmen of all time diff --git a/counter/tests/test_counter.py b/counter/tests/test_counter.py index 581b5b07..83b65021 100644 --- a/counter/tests/test_counter.py +++ b/counter/tests/test_counter.py @@ -108,6 +108,13 @@ class TestFullClickBase(TestCase): class TestRefilling(TestFullClickBase): + @classmethod + def setUpTestData(cls): + super().setUpTestData() + cls.board_admin.user_permissions.add( + Permission.objects.get(codename="add_refilling") + ) + def login_in_bar(self, barmen: User | None = None): used_barman = barmen if barmen is not None else self.board_admin self.client.post( @@ -147,7 +154,7 @@ class TestRefilling(TestFullClickBase): assert self.updated_amount(self.customer) == 0 def test_refilling_no_refer_fail(self): - """Check that the refill fails is the HTTP_REFERER header is missing""" + """Check that the refill fails if the HTTP_REFERER header is missing""" def refill(): return self.client.post( diff --git a/counter/views/click.py b/counter/views/click.py index f13444ef..5398eb92 100644 --- a/counter/views/click.py +++ b/counter/views/click.py @@ -204,7 +204,7 @@ class CounterClick( res["student_card_fragment"] = StudentCardFormFragment.as_fragment()( self.request, customer=self.customer ) - if self.object.can_refill(): + if self.object.can_refill: res["refilling_fragment"] = RefillingCreateView.as_fragment()( self.request, customer=self.customer, counter=self.object ) @@ -250,7 +250,7 @@ class RefillingCreateView(FragmentMixin, CreateView): if not ( request.barmen and request.barmen.issubset(self.counter.barmen_list) - and self.counter.can_refill() + and self.counter.can_refill ): raise PermissionDenied