Merge pull request #1248 from ae-utbm/fix/api-barman-auth

fix: user search for anonymous sessions with logged barmen

api/hashers.py

@@ -8,7 +8,7 @@ from django.utils.crypto import constant_time_compare
 
 class Sha512ApiKeyHasher(BasePasswordHasher):
     """
-    An API key hasher using the sha256 algorithm.
+    An API key hasher using the sha512 algorithm.
 
     This hasher shouldn't be used in Django's `PASSWORD_HASHERS` setting.
     It is insecure for use in hashing passwords, but is safe for hashing

com/tests/test_api.py

@@ -1,4 +1,3 @@
-from dataclasses import dataclass
 from datetime import timedelta
 from pathlib import Path
 
@@ -18,16 +17,6 @@ from core.markdown import markdown
 from core.models import User
 
 
-@dataclass
-class MockResponse:
-    ok: bool
-    value: str
-
-    @property
-    def content(self):
-        return self.value.encode("utf8")
-
-
 def accel_redirect_to_file(response: HttpResponse) -> Path | None:
     redirect = Path(response.headers.get("X-Accel-Redirect", ""))
     if not redirect.is_relative_to(Path("/") / settings.MEDIA_ROOT.stem):

core/api.py

@@ -1,6 +1,6 @@
 from typing import Annotated, Any, Literal
 
-import annotated_types
+from annotated_types import Ge, Le, MinLen
 from django.conf import settings
 from django.db.models import F
 from django.http import HttpResponse
@@ -28,6 +28,7 @@ from core.schemas import (
     UserSchema,
 )
 from core.templatetags.renderer import markdown
+from counter.utils import is_logged_in_counter
 
 
 @api_controller("/markdown")
@@ -72,7 +73,7 @@ class MailingListController(ControllerBase):
 
 @api_controller("/user")
 class UserController(ControllerBase):
-    @route.get("", response=list[UserProfileSchema], permissions=[CanAccessLookup])
+    @route.get("", response=list[UserProfileSchema])
     def fetch_profiles(self, pks: Query[set[int]]):
         return User.objects.viewable_by(self.context.request.user).filter(pk__in=pks)
 
@@ -85,15 +86,18 @@ class UserController(ControllerBase):
         "/search",
         response=PaginatedResponseSchema[UserProfileSchema],
         url_name="search_users",
-        permissions=[CanAccessLookup],
+        # logged in barmen aren't authenticated stricto sensu, so no auth here
+        auth=None,
     )
     @paginate(PageNumberPaginationExtra, page_size=20)
     def search_users(self, filters: Query[UserFilterSchema]):
-        return filters.filter(
+        qs = User.objects
-            User.objects.viewable_by(self.context.request.user).order_by(
+        # the logged in barmen can see all users (even the hidden one),
-                F("last_login").desc(nulls_last=True)
+        # because they have a temporary administrative function during
-            )
+        # which they may have to deal with hidden users
-        )
+        if not is_logged_in_counter(self.context.request):
+            qs = qs.viewable_by(self.context.request.user)
+        return filters.filter(qs.order_by(F("last_login").desc(nulls_last=True)))
 
 
 @api_controller("/file")
@@ -105,7 +109,7 @@ class SithFileController(ControllerBase):
         permissions=[CanAccessLookup],
     )
     @paginate(PageNumberPaginationExtra, page_size=50)
-    def search_files(self, search: Annotated[str, annotated_types.MinLen(1)]):
+    def search_files(self, search: Annotated[str, MinLen(1)]):
         return SithFile.objects.filter(is_in_sas=False).filter(name__icontains=search)
 
 
@@ -118,11 +122,11 @@ class GroupController(ControllerBase):
         permissions=[CanAccessLookup],
     )
     @paginate(PageNumberPaginationExtra, page_size=50)
-    def search_group(self, search: Annotated[str, annotated_types.MinLen(1)]):
+    def search_group(self, search: Annotated[str, MinLen(1)]):
         return Group.objects.filter(name__icontains=search).values()
 
 
-DepthValue = Annotated[int, annotated_types.Ge(0), annotated_types.Le(10)]
+DepthValue = Annotated[int, Ge(0), Le(10)]
 DEFAULT_DEPTH = 4
 
 

core/tests/test_user.py

@@ -1,4 +1,5 @@
 from datetime import timedelta
+from unittest import mock
 
 import pytest
 from django.conf import settings
@@ -23,6 +24,7 @@ from core.models import AnonymousUser, Group, User
 from core.views import UserTabsMixin
 from counter.baker_recipes import sale_recipe
 from counter.models import Counter, Customer, Refilling, Selling
+from counter.utils import is_logged_in_counter
 from eboutic.models import Invoice, InvoiceItem
 
 
@@ -60,7 +62,9 @@ class TestSearchUsersAPI(TestSearchUsers):
         """Test that users are ordered by last login date."""
         self.client.force_login(subscriber_user.make())
 
-        response = self.client.get(reverse("api:search_users") + "?search=First")
+        response = self.client.get(
+            reverse("api:search_users", query={"search": "First"})
+        )
         assert response.status_code == 200
         assert response.json()["count"] == 11
         # The users are ordered by last login date, so we need to reverse the list
@@ -69,7 +73,7 @@ class TestSearchUsersAPI(TestSearchUsers):
         ]
 
     def test_search_case_insensitive(self):
-        """Test that the search is case insensitive."""
+        """Test that the search is case-insensitive."""
         self.client.force_login(subscriber_user.make())
 
         expected = [u.id for u in self.users[::-1]]
@@ -82,14 +86,19 @@ class TestSearchUsersAPI(TestSearchUsers):
             assert [r["id"] for r in response.json()["results"]] == expected
 
     def test_search_nick_name(self):
-        """Test that the search can be done on the nick name."""
+        """Test that the search can be done on the nickname."""
+        # hidden users should not be in the final result,
+        # even when the nickname matches
+        self.users[10].is_viewable = False
+        self.users[10].save()
         self.client.force_login(subscriber_user.make())
 
         # this should return users with nicknames Nick11, Nick10 and Nick1
-        response = self.client.get(reverse("api:search_users") + "?search=Nick1")
+        response = self.client.get(
+            reverse("api:search_users", query={"search": "Nick1"})
+        )
         assert response.status_code == 200
         assert [r["id"] for r in response.json()["results"]] == [
-            self.users[10].id,
             self.users[9].id,
             self.users[0].id,
         ]
@@ -101,10 +110,25 @@ class TestSearchUsersAPI(TestSearchUsers):
         self.client.force_login(subscriber_user.make())
 
         # this should return users with first names First1 and First10
-        response = self.client.get(reverse("api:search_users") + "?search=bél")
+        response = self.client.get(reverse("api:search_users", query={"search": "bél"}))
         assert response.status_code == 200
         assert [r["id"] for r in response.json()["results"]] == [belix.id]
 
+    @mock.create_autospec(is_logged_in_counter, return_value=True)
+    def test_search_as_barman(self):
+        # barmen should also see hidden users
+        self.users[10].is_viewable = False
+        self.users[10].save()
+        response = self.client.get(
+            reverse("api:search_users", query={"search": "Nick1"})
+        )
+        assert response.status_code == 200
+        assert [r["id"] for r in response.json()["results"]] == [
+            self.users[10].id,
+            self.users[9].id,
+            self.users[0].id,
+        ]
+
 
 class TestSearchUsersView(TestSearchUsers):
     """Test the search user view (`GET /search`)."""