From 55ad1f99fd66a04d41ac1439515a00278208bb6f Mon Sep 17 00:00:00 2001 From: thomas girod Date: Mon, 9 Sep 2024 21:37:28 +0200 Subject: [PATCH] fix undeletable SAS pictures --- core/models.py | 37 ++++----------------- core/views/files.py | 25 +------------- core/views/user.py | 2 ++ sas/tests/test_views.py | 72 +++++++++++++++++++++++++++++++++++++++-- sas/views.py | 7 ++-- 5 files changed, 82 insertions(+), 61 deletions(-) diff --git a/core/models.py b/core/models.py index 1f5b2109..171221f5 100644 --- a/core/models.py +++ b/core/models.py @@ -944,40 +944,15 @@ class SithFile(models.Model): param="1", ).save() - def can_be_managed_by(self, user: User) -> bool: - """Tell if the user can manage the file (edit, delete, etc.) or not. - Apply the following rules: - - If the file is not in the SAS nor in the profiles directory, it can be "managed" by anyone -> return True - - If the file is in the SAS, only the SAS admins (or roots) can manage it -> return True if the user is in the SAS admin group or is a root - - If the file is in the profiles directory, only the roots can manage it -> return True if the user is a root. - - Returns: - True if the file is managed by the SAS or within the profiles directory, False otherwise - """ - # If the file is not in the SAS nor in the profiles directory, it can be "managed" by anyone - profiles_dir = SithFile.objects.filter(name="profiles").first() - if not self.is_in_sas and not profiles_dir in self.get_parent_list(): - return True - - # If the file is in the SAS, only the SAS admins (or roots) can manage it - if self.is_in_sas and ( - user.is_in_group(settings.SITH_GROUP_SAS_ADMIN_ID) or user.is_root - ): - return True - - # If the file is in the profiles directory, only the roots can manage it - if profiles_dir in self.get_parent_list() and ( - user.is_root or user.is_board_member - ): - return True - - return False - def is_owned_by(self, user): if user.is_anonymous: return False - if hasattr(self, "profile_of") and user.is_board_member: + if user.is_root: return True + if hasattr(self, "profile_of"): + # if the `profile_of` attribute is set, this file is a profile picture + # and profile pictures may only be edited by board members + return user.is_board_member if user.is_com_admin: return True if self.is_in_sas and user.is_in_group(pk=settings.SITH_GROUP_SAS_ADMIN_ID): @@ -993,7 +968,7 @@ class SithFile(models.Model): return user.can_view(self.scrub_of) return False - def delete(self): + def delete(self, *args, **kwargs): for c in self.children.all(): c.delete() self.file.delete() diff --git a/core/views/files.py b/core/views/files.py index 3d1151d0..eb202991 100644 --- a/core/views/files.py +++ b/core/views/files.py @@ -190,13 +190,6 @@ class FileEditView(CanEditMixin, UpdateView): template_name = "core/file_edit.jinja" context_object_name = "file" - def get(self, request, *args, **kwargs): - self.object = self.get_object() - if not self.object.can_be_managed_by(request.user): - raise PermissionDenied - - return super().get(request, *args, **kwargs) - def get_form_class(self): fields = ["name", "is_moderated"] if self.object.is_file: @@ -242,13 +235,6 @@ class FileEditPropView(CanEditPropMixin, UpdateView): context_object_name = "file" form_class = FileEditPropForm - def get(self, request, *args, **kwargs): - self.object = self.get_object() - if not self.object.can_be_managed_by(request.user): - raise PermissionDenied - - return super().get(request, *args, **kwargs) - def get_form(self, form_class=None): form = super().get_form(form_class) form.fields["parent"].queryset = SithFile.objects.filter(is_folder=True) @@ -322,9 +308,6 @@ class FileView(CanViewMixin, DetailView, FormMixin): def get(self, request, *args, **kwargs): self.form = self.get_form() - if not self.object.can_be_managed_by(request.user): - raise PermissionDenied - if "clipboard" not in request.session.keys(): request.session["clipboard"] = [] return super().get(request, *args, **kwargs) @@ -372,13 +355,6 @@ class FileDeleteView(CanEditPropMixin, DeleteView): template_name = "core/file_delete_confirm.jinja" context_object_name = "file" - def get(self, request, *args, **kwargs): - self.object = self.get_object() - if not self.object.can_be_managed_by(request.user): - raise PermissionDenied - - return super().get(request, *args, **kwargs) - def get_success_url(self): self.object.file.delete() # Doing it here or overloading delete() is the same, so let's do it here if "next" in self.request.GET.keys(): @@ -416,6 +392,7 @@ class FileModerateView(CanEditPropMixin, SingleObjectMixin): model = SithFile pk_url_kwarg = "file_id" + # FIXME : wrong http method. This should be a POST or a DELETE request def get(self, request, *args, **kwargs): self.object = self.get_object() self.object.is_moderated = True diff --git a/core/views/user.py b/core/views/user.py index 0c695e8c..641ebb65 100644 --- a/core/views/user.py +++ b/core/views/user.py @@ -561,6 +561,8 @@ class UserListView(ListView, CanEditPropMixin): template_name = "core/user_list.jinja" +# FIXME: the edit_once fields aren't displayed to the user (as expected). +# However, if the user re-add them manually in the form, they are saved. class UserUpdateProfileView(UserTabsMixin, CanEditMixin, UpdateView): """Edit a user's profile.""" diff --git a/sas/tests/test_views.py b/sas/tests/test_views.py index ac83f199..7cce8269 100644 --- a/sas/tests/test_views.py +++ b/sas/tests/test_views.py @@ -17,14 +17,15 @@ from typing import Callable import pytest from django.conf import settings from django.core.cache import cache -from django.test import Client +from django.test import Client, TestCase from django.urls import reverse from model_bakery import baker +from pytest_django.asserts import assertRedirects from core.baker_recipes import old_subscriber_user, subscriber_user from core.models import RealGroup, User from sas.baker_recipes import picture_recipe -from sas.models import Album +from sas.models import Album, Picture # Create your tests here. @@ -64,3 +65,70 @@ def test_album_access_non_subscriber(client: Client): cache.clear() res = client.get(reverse("sas:album", kwargs={"album_id": album.id})) assert res.status_code == 200 + + +class TestSasModeration(TestCase): + @classmethod + def setUpTestData(cls): + album = baker.make(Album, parent_id=settings.SITH_SAS_ROOT_DIR_ID) + cls.pictures = picture_recipe.make( + parent=album, _quantity=10, _bulk_create=True + ) + cls.to_moderate = cls.pictures[0] + cls.to_moderate.is_moderated = False + cls.to_moderate.save() + cls.moderator = baker.make( + User, groups=[RealGroup.objects.get(pk=settings.SITH_GROUP_SAS_ADMIN_ID)] + ) + cls.simple_user = subscriber_user.make() + + def test_moderation_page_sas_admin(self): + """Test that a moderator can see the pictures needing moderation.""" + self.client.force_login(self.moderator) + res = self.client.get(reverse("sas:moderation")) + assert res.status_code == 200 + assert len(res.context_data["pictures"]) == 1 + assert res.context_data["pictures"][0] == self.to_moderate + + res = self.client.post( + reverse("sas:moderation"), + data={"album_id": self.to_moderate.id, "picture_id": self.to_moderate.id}, + ) + + def test_moderation_page_forbidden(self): + self.client.force_login(self.simple_user) + res = self.client.get(reverse("sas:moderation")) + assert res.status_code == 403 + + def test_moderate_picture(self): + self.client.force_login(self.moderator) + res = self.client.get( + reverse("core:file_moderate", kwargs={"file_id": self.to_moderate.id}), + data={"next": self.pictures[1].get_absolute_url()}, + ) + assertRedirects(res, self.pictures[1].get_absolute_url()) + self.to_moderate.refresh_from_db() + assert self.to_moderate.is_moderated + + def test_delete_picture(self): + self.client.force_login(self.moderator) + res = self.client.post( + reverse("core:file_delete", kwargs={"file_id": self.to_moderate.id}) + ) + assert res.status_code == 302 + assert not Picture.objects.filter(pk=self.to_moderate.id).exists() + + def test_moderation_action_non_authorized_user(self): + """Test that a non-authorized user cannot moderate a picture.""" + self.client.force_login(self.simple_user) + res = self.client.post( + reverse("core:file_moderate", kwargs={"file_id": self.to_moderate.id}), + ) + assert res.status_code == 403 + self.to_moderate.refresh_from_db() + assert not self.to_moderate.is_moderated + res = self.client.post( + reverse("core:file_delete", kwargs={"file_id": self.to_moderate.id}), + ) + assert res.status_code == 403 + assert Picture.objects.filter(pk=self.to_moderate.id).exists() diff --git a/sas/views.py b/sas/views.py index 575448e0..fe419c01 100644 --- a/sas/views.py +++ b/sas/views.py @@ -333,10 +333,9 @@ class ModerationView(TemplateView): kwargs["albums_to_moderate"] = Album.objects.filter( is_moderated=False, is_in_sas=True, is_folder=True ).order_by("id") - kwargs["pictures"] = Picture.objects.filter(is_moderated=False) - kwargs["albums"] = Album.objects.filter( - id__in=kwargs["pictures"].values("parent").distinct("parent") - ) + pictures = Picture.objects.filter(is_moderated=False).select_related("parent") + kwargs["pictures"] = pictures + kwargs["albums"] = [p.parent for p in pictures] return kwargs