From 8a2eee113a775fb8894714fc8f1e2fc885aba8f0 Mon Sep 17 00:00:00 2001 From: Sli Date: Sat, 25 Apr 2026 01:06:23 +0200 Subject: [PATCH] Security fix for image rotations. Add proper permissions, tests and use a form to avoid cross domain forgery attacks --- locale/fr/LC_MESSAGES/django.po | 151 +++++++++++++++++--------------- sas/forms.py | 7 ++ sas/models.py | 28 +++--- sas/static/sas/css/picture.scss | 25 ++++-- sas/templates/sas/picture.jinja | 21 ++++- sas/tests/test_views.py | 90 +++++++++++++++++++ sas/urls.py | 2 + sas/views.py | 38 ++++++-- 8 files changed, 263 insertions(+), 99 deletions(-) diff --git a/locale/fr/LC_MESSAGES/django.po b/locale/fr/LC_MESSAGES/django.po index 2a250802..a8de5b8c 100644 --- a/locale/fr/LC_MESSAGES/django.po +++ b/locale/fr/LC_MESSAGES/django.po @@ -6,7 +6,7 @@ msgid "" msgstr "" "Report-Msgid-Bugs-To: \n" -"POT-Creation-Date: 2026-03-23 22:21+0100\n" +"POT-Creation-Date: 2026-04-25 01:00+0200\n" "PO-Revision-Date: 2016-07-18\n" "Last-Translator: Maréchal \n" @@ -181,6 +181,22 @@ msgstr "Vous devez être cotisant pour faire partie d'un club" msgid "You are already a member of this club" msgstr "Vous êtes déjà membre de ce club." +#: club/forms.py +msgid "Club status" +msgstr "État du club" + +#: club/forms.py +msgid "Active" +msgstr "Actif" + +#: club/forms.py +msgid "Inactive" +msgstr "Inactif" + +#: club/forms.py +msgid "All clubs" +msgstr "Tous les clubs" + #: club/models.py msgid "slug name" msgstr "nom slug" @@ -301,37 +317,22 @@ msgstr "Cet email est déjà abonné à cette mailing" msgid "Unregistered user" msgstr "Utilisateur non enregistré" -#: club/templates/club/club_list.jinja -msgid "Club list" -msgstr "Liste des clubs" - #: club/templates/club/club_list.jinja msgid "The list of all clubs existing at UTBM." msgstr "La liste de tous les clubs existants à l'UTBM" +#: club/templates/club/club_list.jinja +msgid "Club list" +msgstr "Liste des clubs" + #: club/templates/club/club_list.jinja msgid "Filters" msgstr "Filtres" -#: club/templates/club/club_list.jinja -msgid "Name" -msgstr "Nom" - -#: club/templates/club/club_list.jinja -msgid "Club state" -msgstr "Etat du club" - -#: club/templates/club/club_list.jinja -msgid "Active" -msgstr "Actif" - -#: club/templates/club/club_list.jinja -msgid "Inactive" -msgstr "Inactif" - -#: club/templates/club/club_list.jinja -msgid "All clubs" -msgstr "Tous les clubs" +#: club/templates/club/club_list.jinja core/templates/core/base/header.jinja +#: forum/templates/forum/macros.jinja matmat/templates/matmat/search_form.jinja +msgid "Search" +msgstr "Recherche" #: club/templates/club/club_list.jinja core/templates/core/user_tools.jinja msgid "New club" @@ -1863,11 +1864,6 @@ msgstr "Connexion" msgid "Register" msgstr "Inscription" -#: core/templates/core/base/header.jinja forum/templates/forum/macros.jinja -#: matmat/templates/matmat/search_form.jinja -msgid "Search" -msgstr "Recherche" - #: core/templates/core/base/header.jinja msgid "Logout" msgstr "Déconnexion" @@ -4212,6 +4208,47 @@ msgstr "" msgid "this page" msgstr "cette page" +#: eboutic/templates/eboutic/eboutic_main.jinja +msgid "Eurockéennes 2025 partnership" +msgstr "Partenariat Eurockéennes 2025" + +#: eboutic/templates/eboutic/eboutic_main.jinja +msgid "" +"Our partner uses Weezevent to sell tickets. Weezevent may collect user info " +"according to its own privacy policy. By clicking the accept button you " +"consent to their terms of services." +msgstr "" +"Notre partenaire utilises Wezevent pour vendre ses billets. Weezevent peut " +"collecter des informations utilisateur conformément à sa propre politique de " +"confidentialité. En cliquant sur le bouton d'acceptation vous consentez à " +"leurs termes de service." + +#: eboutic/templates/eboutic/eboutic_main.jinja +msgid "Privacy policy" +msgstr "Politique de confidentialité" + +#: eboutic/templates/eboutic/eboutic_main.jinja +#: trombi/templates/trombi/comment_moderation.jinja +msgid "Accept" +msgstr "Accepter" + +#: eboutic/templates/eboutic/eboutic_main.jinja +msgid "" +"You must be subscribed to benefit from the partnership with the Eurockéennes." +msgstr "" +"Vous devez être cotisant pour bénéficier du partenariat avec les " +"Eurockéennes." + +#: eboutic/templates/eboutic/eboutic_main.jinja +#, python-format +msgid "" +"This partnership offers a discount of up to 33%% on tickets for Friday, " +"Saturday and Sunday, as well as the 3-day package from Friday to Sunday." +msgstr "" +"Ce partenariat permet de profiter d'une réduction jusqu'à 33%% sur les " +"billets du vendredi, du samedi et du dimanche, ainsi qu'au forfait 3 jours, " +"du vendredi au dimanche." + #: eboutic/templates/eboutic/eboutic_main.jinja msgid "There are no items available for sale" msgstr "Aucun article n'est disponible à la vente" @@ -4985,6 +5022,14 @@ msgstr "Envoyer les images" msgid "You already requested moderation for this picture." msgstr "Vous avez déjà déposé une demande de retrait pour cette photo." +#: sas/forms.py +msgid "Left" +msgstr "" + +#: sas/forms.py +msgid "Right" +msgstr "Droite" + #: sas/models.py msgid "picture" msgstr "photo" @@ -5106,6 +5151,14 @@ msgstr "Photos de %(user_name)s" msgid "Download all my pictures" msgstr "Télécharger toutes mes photos" +#: sas/views.py +msgid "" +"Newly rotated image might not be immediately displayed due to your web " +"browser's cache" +msgstr "" +"Les images nouvellements pivotées peuvent ne pas s'afficher immédiatement " +"à cause du cache de votre navigateur internet" + #: sith/settings.py msgid "English" msgstr "Anglais" @@ -5638,10 +5691,6 @@ msgstr "fin" msgid "Moderate Trombi comments" msgstr "Modérer les commentaires du Trombi" -#: trombi/templates/trombi/comment_moderation.jinja -msgid "Accept" -msgstr "Accepter" - #: trombi/templates/trombi/comment_moderation.jinja msgid "Reject" msgstr "Refuser" @@ -5883,39 +5932,3 @@ msgstr "Vous ne pouvez plus écrire de commentaires, la date est passée." #, python-format msgid "Maximum characters: %(max_length)s" msgstr "Nombre de caractères max: %(max_length)s" - -#: eboutic/templates/eboutic/eboutic_main.jinja -msgid "Eurockéennes 2025 partnership" -msgstr "Partenariat Eurockéennes 2025" - -#: eboutic/templates/eboutic/eboutic_main.jinja -msgid "" -"Our partner uses Weezevent to sell tickets. Weezevent may collect user info " -"according to its own privacy policy. By clicking the accept button you " -"consent to their terms of services." -msgstr "" -"Notre partenaire utilises Wezevent pour vendre ses billets. Weezevent peut " -"collecter des informations utilisateur conformément à sa propre politique de " -"confidentialité. En cliquant sur le bouton d'acceptation vous consentez à " -"leurs termes de service." - -#: eboutic/templates/eboutic/eboutic_main.jinja -msgid "Privacy policy" -msgstr "Politique de confidentialité" - -#: eboutic/templates/eboutic/eboutic_main.jinja -msgid "" -"You must be subscribed to benefit from the partnership with the Eurockéennes." -msgstr "" -"Vous devez être cotisant pour bénéficier du partenariat avec les " -"Eurockéennes." - -#: eboutic/templates/eboutic/eboutic_main.jinja -#, python-format -msgid "" -"This partnership offers a discount of up to 33%% on tickets for Friday, " -"Saturday and Sunday, as well as the 3-day package from Friday to Sunday." -msgstr "" -"Ce partenariat permet de profiter d'une réduction jusqu'à 33%% sur les " -"billets du vendredi, du samedi et du dimanche, ainsi qu'au forfait 3 jours, " -"du vendredi au dimanche." diff --git a/sas/forms.py b/sas/forms.py index a478ce43..57662bdf 100644 --- a/sas/forms.py +++ b/sas/forms.py @@ -90,3 +90,10 @@ class PictureModerationRequestForm(forms.ModelForm): self.instance.author = self.user self.instance.picture = self.picture return super().save(commit) + + +class PictureRotationForm(forms.Form): + picture = forms.ModelChoiceField(Picture.objects.all(), required=True) + direction = forms.ChoiceField( + choices=[("LEFT", _("Left")), ("RIGHT", _("Right"))], required=True + ) diff --git a/sas/models.py b/sas/models.py index 64f6c15b..eefa943a 100644 --- a/sas/models.py +++ b/sas/models.py @@ -139,20 +139,20 @@ class Picture(SasFile): self.compressed.name = new_extension_name def rotate(self, degree): - for attr in ["file", "compressed", "thumbnail"]: - name = self.__getattribute__(attr).name - with open(settings.MEDIA_ROOT / name, "r+b") as file: - if file: - im = Image.open(BytesIO(file.read())) - file.seek(0) - im = im.rotate(degree, expand=True) - im.save( - fp=file, - format=self.mime_type.split("/")[-1].upper(), - quality=90, - optimize=True, - progressive=True, - ) + im = Image.open(BytesIO(self.file.read())) + self.file.seek(0) + with open(self.file.path, "r+b") as f: + im = im.rotate(degree, expand=True) + im.save( + fp=f, + format=self.mime_type.split("/")[-1].upper(), + quality=90, + optimize=True, + progressive=True, + ) + self.file.seek(0) + self.generate_thumbnails(overwrite=True) + self.save() def get_next(self): if self.is_moderated: diff --git a/sas/static/sas/css/picture.scss b/sas/static/sas/css/picture.scss index 03103a25..7d5536a0 100644 --- a/sas/static/sas/css/picture.scss +++ b/sas/static/sas/css/picture.scss @@ -191,7 +191,9 @@ } >form { - input, .ts-wrapper { + + input, + .ts-wrapper { min-width: 100%; max-width: 100%; margin: 5px; @@ -212,22 +214,27 @@ flex-direction: column; } - .infos, .tools { + .infos, + .tools { flex: 1; display: flex; flex-direction: column; gap: .5em; + @media (min-width: 700px) { max-width: 350px; } } - .infos > div, .tools > div > div { + + .infos>div, + .tools>div>div { display: flex; flex-direction: column; gap: .35em; } - .tools > div, >.infos >div>div { + .tools>div, + >.infos>div>div { display: flex; flex-direction: row; justify-content: space-between; @@ -237,7 +244,15 @@ flex: 1; >div>div { - >a.btn { + + >form { + margin: 0; + padding: 0; + display: flex; + } + + >a.btn, + >form>button { background-color: $primary-neutral-light-color; display: flex; justify-content: center; diff --git a/sas/templates/sas/picture.jinja b/sas/templates/sas/picture.jinja index 5113a3f9..66f1d8d5 100644 --- a/sas/templates/sas/picture.jinja +++ b/sas/templates/sas/picture.jinja @@ -122,7 +122,8 @@ {% trans %}Ask for removal{% endtrans %} -
+
- - +
+ {% csrf_token %} + + + +
+
+ {% csrf_token %} + + + +
diff --git a/sas/tests/test_views.py b/sas/tests/test_views.py index ffa48beb..66f6966e 100644 --- a/sas/tests/test_views.py +++ b/sas/tests/test_views.py @@ -12,16 +12,19 @@ # OR WITHIN THE LOCAL FILE "LICENSE" # # +from io import BytesIO from typing import Callable import pytest from bs4 import BeautifulSoup from django.conf import settings from django.core.cache import cache +from django.core.files.base import ContentFile from django.test import Client, TestCase from django.urls import reverse from django.utils.timezone import localdate from model_bakery import baker +from PIL import Image from pytest_django.asserts import assertHTMLEqual, assertInHTML, assertRedirects from core.baker_recipes import old_subscriber_user, subscriber_user @@ -297,6 +300,93 @@ class TestAlbumEdit: assert localdate(album.date) == localdate() +@pytest.mark.django_db +class TestPictureRotation: + @pytest.fixture + def picture(self) -> Picture: + # Creating a fake image from scratch is painful + # One of the base image in the test set is good enough + return Picture.objects.get(name="sli.jpg") + + def load_image(self, file: ContentFile) -> Image.Image: + file.seek(0) + im = Image.open(BytesIO(file.read())) + file.seek(0) + return im + + @pytest.mark.parametrize( + "user", + [ + None, + lambda: baker.make(User), + subscriber_user.make, + old_subscriber_user.make, + ], + ) + def test_permission_denied( + self, + client: Client, + picture: Picture, + user: Callable[[], User] | None, + ): + if user: + client.force_login(user()) + + payload = { + "picture": picture.pk, + "direction": "LEFT", + } + url = reverse("sas:picture_rotate") + response = client.post(url, payload) + if user: + assert response.status_code == 403 + else: + assertRedirects( + response, + reverse( + "core:login", + query={ + "next": url, + }, + ), + ) + + @pytest.mark.parametrize( + "user", + [ + lambda: baker.make(User, is_superuser=True), + lambda: baker.make( + User, groups=[Group.objects.get(pk=settings.SITH_GROUP_SAS_ADMIN_ID)] + ), + ], + ) + def test_rotation( + self, + client: Client, + picture: Picture, + user: Callable[[], User], + ): + client.force_login(user()) + + payload = { + "picture": picture.pk, + "direction": "LEFT", + } + response = client.post(reverse("sas:picture_rotate"), payload) + assertRedirects( + response, reverse("sas:picture", kwargs={"picture_id": picture.pk}) + ) + + payload = { + "picture": picture.pk, + "direction": "RIGHT", + } + response = client.post(reverse("sas:picture_rotate"), payload) + assertRedirects( + response, reverse("sas:picture", kwargs={"picture_id": picture.pk}) + ) + + class TestSasModeration(TestCase): @classmethod def setUpTestData(cls): diff --git a/sas/urls.py b/sas/urls.py index 2a3ba89f..3400bba8 100644 --- a/sas/urls.py +++ b/sas/urls.py @@ -22,6 +22,7 @@ from sas.views import ( ModerationView, PictureAskRemovalView, PictureEditView, + PictureRotateView, PictureView, SASMainView, UserPicturesView, @@ -52,6 +53,7 @@ urlpatterns = [ send_compressed, name="download_compressed", ), + path("picture/rotate", PictureRotateView.as_view(), name="picture_rotate"), path("picture//download/thumb/", send_thumb, name="download_thumb"), path( "user//pictures/", UserPicturesView.as_view(), name="user_pictures" diff --git a/sas/views.py b/sas/views.py index 160182e4..409f7d1f 100644 --- a/sas/views.py +++ b/sas/views.py @@ -15,12 +15,14 @@ from typing import Any from django.conf import settings +from django.contrib import messages from django.contrib.auth.mixins import PermissionRequiredMixin from django.db.models import Count, OuterRef, Subquery from django.http import Http404, HttpResponseRedirect from django.shortcuts import get_object_or_404, redirect from django.urls import reverse from django.utils.safestring import SafeString +from django.utils.translation import gettext_lazy as _ from django.views.generic import CreateView, DetailView, TemplateView from django.views.generic.edit import FormView, UpdateView @@ -35,6 +37,7 @@ from sas.forms import ( AlbumEditForm, PictureEditForm, PictureModerationRequestForm, + PictureRotationForm, PictureUploadForm, ) from sas.models import Album, PeoplePictureRelation, Picture @@ -96,20 +99,39 @@ class PictureView(CanViewMixin, DetailView): pk_url_kwarg = "picture_id" template_name = "sas/picture.jinja" - def get(self, request, *args, **kwargs): - self.object = self.get_object() - if "rotate_right" in request.GET: - self.object.rotate(270) - if "rotate_left" in request.GET: - self.object.rotate(90) - return super().get(request, *args, **kwargs) - def get_context_data(self, **kwargs): return super().get_context_data(**kwargs) | { "album": Album.objects.get(children=self.object) } +class PictureRotateView(PermissionRequiredMixin, FormView): + form_class = PictureRotationForm + template_name = "core/edit.jinja" + permission_required = "sas.moderate_sasfile" + + def form_valid(self, form: PictureRotationForm): + angles = {"RIGHT": 270, "LEFT": 90} + cleaned = form.clean() + cleaned["picture"].rotate(angles[cleaned["direction"]]) + self._success_url = reverse( + "sas:picture", + kwargs={ + "picture_id": cleaned["picture"].pk, + }, + ) + messages.warning( + self.request, + _( + "Newly rotated image might not be immediately displayed due to your web browser's cache" + ), + ) + return super().form_valid(form) + + def get_success_url(self): + return self._success_url + + def send_album(request, album_id): return send_file(request, album_id, Album)