Merge pull request #1534 from ae-utbm/preference-perm

fix: permission check on `UserVisibilityFormFragment`

core/tests/test_user.py

@@ -472,6 +472,36 @@ def test_user_preferences(client: Client):
     assert user.preferences.notify_on_click is True
 
 
+@pytest.mark.django_db
+@pytest.mark.parametrize("can_edit", [True, False])
+def test_user_visibility(client: Client, monkeypatch, can_edit):
+    # The view is a CanView, so just patch User.can_be_edited_by for the test
+    monkeypatch.setattr("core.models.User.can_be_edited_by", lambda *args: can_edit)
+    client.force_login(subscriber_user.make())
+
+    user, other_user = subscriber_user.make(_quantity=2)
+    url = reverse("core:user_visibility_fragment", kwargs={"user_id": user.id})
+    response = client.get(url)
+    assert response.status_code == 200 if can_edit else 403
+    response = client.post(
+        url,
+        {
+            "show_my_stats": False,
+            "is_viewable": False,
+            "whitelisted_users": [other_user.id],
+        },
+    )
+    if not can_edit:
+        assert response.status_code == 403
+        return
+    assertRedirects(response, url)
+    user.refresh_from_db()
+    user.preferences.refresh_from_db()
+    assert user.is_viewable is False
+    assert list(user.whitelisted_users.all()) == [other_user]
+    assert user.preferences.show_my_stats is False
+
+
 @pytest.mark.django_db
 def test_user_stats(client: Client):
     user = subscriber_user.make()

core/views/mixins.py

@@ -94,8 +94,8 @@ class FragmentMixin(TemplateResponseMixin, AllowFragment, ContextMixin):
             return render(
                 request,
                 "app/template.jinja",
-                context={"fragment": fragment(request)
+                context={"fragment": fragment(request)}
-            }
+            )
 
         # in urls.py
         urlpatterns = [

core/views/user.py

@@ -502,7 +502,9 @@ class UserClubView(UserTabsMixin, CanViewMixin, DetailView):
     current_tab = "clubs"
 
 
-class UserVisibilityFormFragment(FragmentMixin, SuccessMessageMixin, UpdateView):
+class UserVisibilityFormFragment(
+    FragmentMixin, SuccessMessageMixin, CanEditMixin, UpdateView
+):
     model = User
     form_class = UserVisibilityForm
     template_name = "core/fragment/user_visibility.jinja"

counter/tests/test_counter.py

@@ -21,8 +21,7 @@ import pytest
 from bs4 import BeautifulSoup
 from dateutil.relativedelta import relativedelta
 from django.conf import settings
-from django.contrib.auth.hashers import make_password
+from django.contrib.auth.models import Permission, make_password
-from django.contrib.auth.models import Permission
 from django.contrib.messages import DEFAULT_LEVELS, get_messages
 from django.shortcuts import resolve_url
 from django.test import Client, TestCase
@@ -66,9 +65,8 @@ class TestFullClickBase(TestCase):
     @classmethod
     def setUpTestData(cls):
         cls.customer = subscriber_user.make()
-        password = make_password("plop")
+        cls.barmen = subscriber_user.make(password=make_password("plop"))
-        cls.barmen = subscriber_user.make(password=password)
+        cls.board_admin = board_user.make(password=make_password("plop"))
-        cls.board_admin = board_user.make(password=password)
         cls.club_admin = subscriber_user.make()
         cls.root = baker.make(User, is_superuser=True)
         cls.subscriber = subscriber_user.make()

counter/utils.py

@@ -6,10 +6,8 @@ from django.urls import resolve
 if TYPE_CHECKING:
     from django.http import HttpRequest
 
-    from counter.models import Counter
 
-
+def is_logged_in_counter(request: HttpRequest) -> bool:
-def is_logged_in_counter(request: HttpRequest, counter: Counter | None = None) -> bool:
     """Check if the request is sent from a device logged to a counter.
 
     The request must also be sent within the frame of a counter's activity.
@@ -32,6 +30,4 @@ def is_logged_in_counter(request: HttpRequest, counter: Counter | None = None) -
     if not referer_ok and request.resolver_match.app_name != "counter":
         return False
 
-    if counter is None:
     return bool(request.barmen)
-    return bool(request.barmen) and request.barmen.issubset(set(counter.barmen_list))

counter/views/admin.py

@@ -306,41 +306,52 @@ class ReturnableProductDeleteView(
         }
 
 
-class CounterTransactionDeletionView(DeleteView):
+class RefillingDeleteView(DeleteView):
+    """Delete a refilling (for the admins)."""
+
+    model = Refilling
+    pk_url_kwarg = "refilling_id"
     template_name = "core/delete_confirm.jinja"
 
-    def get_queryset(self):
-        return super().get_queryset().select_related("counter", "customer")
-
     def dispatch(self, request, *args, **kwargs):
-        self.object: Selling | Refilling = self.get_object()
+        self.object = self.get_object()
         if timezone.now() - self.object.date <= timedelta(
             minutes=settings.SITH_LAST_OPERATIONS_LIMIT
-        ) and is_logged_in_counter(request, counter=self.object.counter):
+        ) and is_logged_in_counter(request):
             self.success_url = reverse(
-                "counter:details", kwargs={"counter_id": self.object.counter_id}
+                "counter:details", kwargs={"counter_id": self.object.counter.id}
             )
             return super().dispatch(request, *args, **kwargs)
         elif self.object.is_owned_by(request.user):
             self.success_url = reverse(
-                "core:user_account", kwargs={"user_id": self.object.customer.user_id}
+                "core:user_account", kwargs={"user_id": self.object.customer.user.id}
             )
             return super().dispatch(request, *args, **kwargs)
         raise PermissionDenied
 
 
-class RefillingDeleteView(CounterTransactionDeletionView):
+class SellingDeleteView(DeleteView):
-    """Delete a refilling (for the admins)."""
-
-    model = Refilling
-    pk_url_kwarg = "refilling_id"
-
-
-class SellingDeleteView(CounterTransactionDeletionView):
     """Delete a selling (for the admins)."""
 
     model = Selling
     pk_url_kwarg = "selling_id"
+    template_name = "core/delete_confirm.jinja"
+
+    def dispatch(self, request, *args, **kwargs):
+        self.object = self.get_object()
+        if timezone.now() - self.object.date <= timedelta(
+            minutes=settings.SITH_LAST_OPERATIONS_LIMIT
+        ) and is_logged_in_counter(request):
+            self.success_url = reverse(
+                "counter:details", kwargs={"counter_id": self.object.counter.id}
+            )
+            return super().dispatch(request, *args, **kwargs)
+        elif self.object.is_owned_by(request.user):
+            self.success_url = reverse(
+                "core:user_account", kwargs={"user_id": self.object.customer.user.id}
+            )
+            return super().dispatch(request, *args, **kwargs)
+        raise PermissionDenied
 
 
 class CounterStatView(PermissionRequiredMixin, DetailView):

counter/views/click.py

@@ -242,11 +242,15 @@ class RefillingCreateView(FragmentMixin, CreateView):
         if not self.customer.can_buy:
             raise Http404
 
+        if not is_logged_in_counter(request):
+            raise PermissionDenied
+
         self.counter: Counter = get_object_or_404(Counter, id=self.kwargs["counter_id"])
 
-        if (
+        if not (
-            not is_logged_in_counter(request, counter=self.counter)
+            request.barmen
-            or not self.counter.can_refill
+            and request.barmen.issubset(self.counter.barmen_list)
+            and self.counter.can_refill
         ):
             raise PermissionDenied
 

counter/views/home.py

@@ -16,9 +16,9 @@ from datetime import timedelta
 from typing import TYPE_CHECKING
 
 from django.conf import settings
-from django.contrib.auth.mixins import UserPassesTestMixin
 from django.core.exceptions import PermissionDenied
 from django.db.models import F
+from django.http import HttpRequest, HttpResponseRedirect
 from django.shortcuts import redirect
 from django.urls import reverse
 from django.utils import timezone
@@ -36,7 +36,6 @@ from counter.utils import is_logged_in_counter
 from counter.views.mixins import CounterTabsMixin
 
 if TYPE_CHECKING:
-    from django.http import HttpRequest, HttpResponseRedirect
     from django.utils.safestring import SafeString
 
 
@@ -156,7 +155,7 @@ class CounterMain(
         return super().form_valid(form)
 
 
-class CounterLastOperationsView(CounterTabsMixin, UserPassesTestMixin, DetailView):
+class CounterLastOperationsView(CounterTabsMixin, CanViewMixin, DetailView):
     """Provide the last operations to allow barmen to delete them."""
 
     model = Counter
@@ -164,12 +163,14 @@ class CounterLastOperationsView(CounterTabsMixin, UserPassesTestMixin, DetailVie
     template_name = "counter/last_ops.jinja"
     current_tab = "last_ops"
 
-    def test_func(self):
+    def dispatch(self, request, *args, **kwargs):
+        """We have here again a very particular right handling."""
         self.object = self.get_object()
-        return is_logged_in_counter(
+        if is_logged_in_counter(request) and self.object.barmen_list:
-            self.request, counter=self.object
+            return super().dispatch(request, *args, **kwargs)
-        ) or self.request.user.has_perms(
+        return HttpResponseRedirect(
-            ["counter.view_selling", "counter.view_refilling"]
+            reverse("counter:details", kwargs={"counter_id": self.object.id})
+            + "?bad_location"
         )
 
     def get_context_data(self, **kwargs):

counter/views/mixins.py

@@ -22,7 +22,7 @@ from counter.utils import is_logged_in_counter
 
 class CounterTabsMixin(TabedViewMixin):
     def get_tabs_title(self):
-        return self.object.name
+        return self.object
 
     def get_list_of_tabs(self):
         if self.object.type != "BAR":
@@ -46,11 +46,7 @@ class CounterTabsMixin(TabedViewMixin):
                     "name": _("Cash summary"),
                 }
             )
-        if is_logged_in_counter(
+        if is_logged_in_counter(self.request):
-            self.request, counter=self.object
-        ) or self.request.user.has_perms(
-            ["counter.view_selling", "counter.view_refilling"]
-        ):
             tab_list.append(
                 {
                     "url": reverse(