diff --git a/sas/api.py b/sas/api.py index dffded87..717025c1 100644 --- a/sas/api.py +++ b/sas/api.py @@ -41,17 +41,12 @@ class PicturesController(ControllerBase): cf. https://ae.utbm.fr/user/32663/pictures/) """ user: User = self.context.request.user - if not user.was_subscribed and filters.users_identified != {user.id}: - # User can view any moderated picture if he/she is subscribed. - # If not, he/she can view only the one he/she has been identified on - raise PermissionDenied - pictures = list( + return ( filters.filter(Picture.objects.viewable_by(user)) .distinct() .order_by("-parent__date", "date") .annotate(album=F("parent__name")) ) - return pictures @api_controller("/sas/relation", tags="User identification on SAS pictures") diff --git a/sas/models.py b/sas/models.py index f7ed8b02..cbb669e6 100644 --- a/sas/models.py +++ b/sas/models.py @@ -20,6 +20,7 @@ from io import BytesIO from django.conf import settings from django.core.cache import cache from django.db import models +from django.db.models import Exists, OuterRef from django.urls import reverse from django.utils import timezone from django.utils.translation import gettext_lazy as _ @@ -29,6 +30,36 @@ from core.models import SithFile, User from core.utils import exif_auto_rotate, resize_image +class SasFile(SithFile): + """Proxy model for any file in the SAS. + + May be used to have logic that should be shared by both + [Picture][sas.models.Picture] and [Album][sas.models.Album]. + """ + + class Meta: + proxy = True + + def can_be_viewed_by(self, user): + if user.is_anonymous: + return False + cache_key = ( + f"sas:{self._meta.model_name}_viewable_by_{user.id}_in_{self.parent_id}" + ) + viewable: list[int] | None = cache.get(cache_key) + if viewable is None: + viewable = list( + self.__class__.objects.filter(parent_id=self.parent_id) + .viewable_by(user) + .values_list("pk", flat=True) + ) + cache.set(cache_key, viewable, timeout=10) + return self.id in viewable + + def can_be_edited_by(self, user): + return user.is_root or user.is_in_group(pk=settings.SITH_GROUP_SAS_ADMIN_ID) + + class PictureQuerySet(models.QuerySet): def viewable_by(self, user: User) -> PictureQuerySet: """Filter the pictures that this user can view. @@ -45,17 +76,10 @@ class PictureQuerySet(models.QuerySet): class SASPictureManager(models.Manager): def get_queryset(self): - return PictureQuerySet(self.model, using=self._db).filter( - is_in_sas=True, is_folder=False - ) + return super().get_queryset().filter(is_in_sas=True, is_folder=False) -class SASAlbumManager(models.Manager): - def get_queryset(self): - return super().get_queryset().filter(is_in_sas=True, is_folder=True) - - -class Picture(SithFile): +class Picture(SasFile): class Meta: proxy = True @@ -68,29 +92,6 @@ class Picture(SithFile): (w, h) = im.size return (w / h) < 1 - def can_be_edited_by(self, user): - perm = cache.get("%d_can_edit_pictures" % (user.id), None) - if perm is None: - perm = user.is_root or user.is_in_group(pk=settings.SITH_GROUP_SAS_ADMIN_ID) - - cache.set("%d_can_edit_pictures" % (user.id), perm, timeout=4) - return perm - - def can_be_viewed_by(self, user: User) -> bool: - if user.is_anonymous: - return False - - cache_key = f"sas:pictures_viewable_by_{user.id}_in_{self.parent_id}" - viewable: list[int] | None = cache.get(cache_key) - if viewable is None: - viewable = list( - Picture.objects.filter(parent_id=self.parent_id) - .viewable_by(user) - .values_list("pk", flat=True) - ) - cache.set(cache_key, viewable, timeout=10) - return self.id in viewable - def get_download_url(self): return reverse("sas:download", kwargs={"picture_id": self.id}) @@ -142,48 +143,53 @@ class Picture(SithFile): def get_next(self): if self.is_moderated: - return ( - self.parent.children.filter( - is_moderated=True, - asked_for_removal=False, - is_folder=False, - id__gt=self.id, - ) - .order_by("id") - .first() + pictures_qs = self.parent.children.filter( + is_moderated=True, + asked_for_removal=False, + is_folder=False, + id__gt=self.id, ) else: - return ( - Picture.objects.filter(id__gt=self.id, is_moderated=False) - .order_by("id") - .first() - ) + pictures_qs = Picture.objects.filter(id__gt=self.id, is_moderated=False) + return pictures_qs.order_by("id").first() def get_previous(self): if self.is_moderated: - return ( - self.parent.children.filter( - is_moderated=True, - asked_for_removal=False, - is_folder=False, - id__lt=self.id, - ) - .order_by("id") - .last() + pictures_qs = self.parent.children.filter( + is_moderated=True, + asked_for_removal=False, + is_folder=False, + id__lt=self.id, ) else: - return ( - Picture.objects.filter(id__lt=self.id, is_moderated=False) - .order_by("-id") - .first() - ) + pictures_qs = Picture.objects.filter(id__lt=self.id, is_moderated=False) + return pictures_qs.order_by("-id").first() -class Album(SithFile): +class AlbumQuerySet(models.QuerySet): + def viewable_by(self, user: User) -> PictureQuerySet: + """Filter the albums that this user can view. + + Warnings: + Calling this queryset method may add several additional requests. + """ + if user.is_root or user.is_in_group(pk=settings.SITH_GROUP_SAS_ADMIN_ID): + return self.all() + return self.filter( + Exists(Picture.objects.filter(parent_id=OuterRef("pk")).viewable_by(user)) + ) + + +class SASAlbumManager(models.Manager): + def get_queryset(self): + return super().get_queryset().filter(is_in_sas=True, is_folder=True) + + +class Album(SasFile): class Meta: proxy = True - objects = SASAlbumManager() + objects = SASAlbumManager.from_queryset(AlbumQuerySet)() @property def children_pictures(self): @@ -193,15 +199,6 @@ class Album(SithFile): def children_albums(self): return Album.objects.filter(parent=self) - def can_be_edited_by(self, user): - return user.is_in_group(pk=settings.SITH_GROUP_SAS_ADMIN_ID) - - def can_be_viewed_by(self, user): - # file = SithFile.objects.filter(id=self.id).first() - return self.can_be_edited_by(user) or ( - self.is_in_sas and self.is_moderated and user.was_subscribed - ) # or user.can_view(file) - def get_absolute_url(self): return reverse("sas:album", kwargs={"album_id": self.id}) diff --git a/sas/templates/sas/picture.jinja b/sas/templates/sas/picture.jinja index a2a300af..7a2ddc77 100644 --- a/sas/templates/sas/picture.jinja +++ b/sas/templates/sas/picture.jinja @@ -4,22 +4,6 @@ {%- endblock -%} -{% block head %} - {{ super() }} - - {% if picture.get_previous() %} - - {% endif %} - {% if picture.get_next() %} - - {% endif %} - -{% endblock %} - {% block title %} {% trans %}SAS{% endtrans %} {% endblock %} @@ -124,16 +108,16 @@
diff --git a/sas/tests/test_api.py b/sas/tests/test_api.py index 3a21ef12..0f84e99f 100644 --- a/sas/tests/test_api.py +++ b/sas/tests/test_api.py @@ -89,19 +89,33 @@ class TestPictureSearch(TestSas): ) assert [i["id"] for i in res.json()["results"]] == expected - # trying to access the pictures of someone else - res = self.client.get( - reverse("api:pictures") + f"?users_identified={self.user_b.id}" - ) - assert res.status_code == 403 - - # trying to access the pictures of someone else shouldn't success, - # even if mixed with owned pictures + # trying to access the pictures of someone else mixed with owned pictures + # should return only owned pictures res = self.client.get( reverse("api:pictures") + f"?users_identified={self.user_a.id}&users_identified={self.user_b.id}" ) - assert res.status_code == 403 + assert res.status_code == 200 + assert [i["id"] for i in res.json()["results"]] == expected + + # trying to fetch everything should be the same + # as fetching its own pictures for a non-subscriber + res = self.client.get(reverse("api:pictures")) + assert res.status_code == 200 + assert [i["id"] for i in res.json()["results"]] == expected + + # trying to access the pictures of someone else should return only + # the ones where the non-subscribed user is identified too + res = self.client.get( + reverse("api:pictures") + f"?users_identified={self.user_b.id}" + ) + assert res.status_code == 200 + expected = list( + self.user_b.pictures.intersection(self.user_a.pictures.all()) + .order_by("-picture__parent__date", "picture__date") + .values_list("picture_id", flat=True) + ) + assert [i["id"] for i in res.json()["results"]] == expected class TestPictureRelation(TestSas): diff --git a/sas/views.py b/sas/views.py index d45c13d1..e39201ef 100644 --- a/sas/views.py +++ b/sas/views.py @@ -119,10 +119,11 @@ class SASMainView(FormView): def get_context_data(self, **kwargs): kwargs = super().get_context_data(**kwargs) - kwargs["categories"] = Album.objects.filter( - parent__id=settings.SITH_SAS_ROOT_DIR_ID - ).order_by("id") - kwargs["latest"] = Album.objects.filter(is_moderated=True).order_by("-id")[:5] + albums_qs = Album.objects.viewable_by(self.request.user) + kwargs["categories"] = list( + albums_qs.filter(parent_id=settings.SITH_SAS_ROOT_DIR_ID).order_by("id") + ) + kwargs["latest"] = list(albums_qs.order_by("-id")[:5]) return kwargs @@ -180,7 +181,14 @@ class PictureView(CanViewMixin, DetailView, FormMixin): def get_context_data(self, **kwargs): kwargs = super().get_context_data(**kwargs) + pictures_qs = Picture.objects.viewable_by(self.request.user) kwargs["form"] = self.form + kwargs["next_pict"] = ( + pictures_qs.filter(id__gt=self.object.id).order_by("id").first() + ) + kwargs["previous_pict"] = ( + pictures_qs.filter(id__lt=self.object.id).order_by("-id").first() + ) return kwargs def get_success_url(self):