diff --git a/website/photos/models.py b/website/photos/models.py index 8f4db3e4cdb6d32d5c21126f21fbaab2cea7b3a9..b829d245131d253aa20c2768b68a0b79a8559966 100644 --- a/website/photos/models.py +++ b/website/photos/models.py @@ -65,7 +65,7 @@ class Photo(models.Model): self._orig_file = "" def __str__(self): - return self.file.name + return os.path.basename(self.file.name) def save(self, *args, **kwargs): super().save(*args, **kwargs) diff --git a/website/photos/services.py b/website/photos/services.py index 7ea8622ba30883bae0ef8d1ace9420f4b1c41743..32fda49c2a6a14ff340b0e346b4f448c0d3efa81 100644 --- a/website/photos/services.py +++ b/website/photos/services.py @@ -1,5 +1,6 @@ -from django.db.models import When, Value, BooleanField, ExpressionWrapper, Q, \ - Case +from django.db.models import (When, Value, BooleanField, ExpressionWrapper, Q, + Case) +from django.http import Http404 from PIL.JpegImagePlugin import JpegImageFile from PIL import ExifTags @@ -28,6 +29,11 @@ def photo_determine_rotation(pil_image): return 0 +def check_shared_album_token(album, token): + if token != album.access_token: + raise Http404("Invalid token.") + + def is_album_accessible(request, album): if request.member and request.member.current_membership is not None: return True diff --git a/website/photos/templates/photos/album.html b/website/photos/templates/photos/album.html index 35379a355b7e739466b0e022884336fa8797ee0c..2302776345635bed46c894acd57a9548cf60768b 100644 --- a/website/photos/templates/photos/album.html +++ b/website/photos/templates/photos/album.html @@ -11,19 +11,25 @@

{{ album.title }} -

+

{{ album.date|date:"d-m-Y" }}

{% if album.shareable %}

{% trans "Note: This album can be shared with people outside the association by sending them the following link:" %}
- - {{ request.get_host }}{% url 'photos:shared_album' album.slug album.access_token %} + + {{ request.get_host }}{% url 'photos:shared-album' album.slug album.access_token %}

{% endif %} diff --git a/website/photos/templatetags/photos_cards.py b/website/photos/templatetags/photos_cards.py index 0402f4d7d983b1c4c987001f445b6f5008febd95..5432de72c4f283dd98786613b3e230baeb65d3d6 100644 --- a/website/photos/templatetags/photos_cards.py +++ b/website/photos/templatetags/photos_cards.py @@ -45,7 +45,7 @@ def photo_card(photo): args=[photo.album.slug, photo.album.access_token, photo])) else: anchor_attrs += ' data-download={}'.format( - reverse('photos:download', args=[photo])) + reverse('photos:download', args=[photo.album.slug, photo])) image_url = thumbnail(photo.file, settings.THUMBNAIL_SIZES['medium']) if photo.album.shareable: diff --git a/website/photos/templatetags/shared_thumbnail.py b/website/photos/templatetags/shared_thumbnail.py index 2bc2dc9124aa53dac41a0e91741c74fd4bd6f80b..b993097572ab898545d4de5f882c6369c7ffdd7f 100644 --- a/website/photos/templatetags/shared_thumbnail.py +++ b/website/photos/templatetags/shared_thumbnail.py @@ -1,6 +1,8 @@ from django import template from django.urls import resolve, reverse +from os.path import basename + from utils.templatetags.thumbnail import thumbnail register = template.Library() @@ -9,5 +11,6 @@ register = template.Library() @register.simple_tag def shared_thumbnail(slug, token, path, size, fit=True): thumb = resolve(thumbnail(path, size, fit)) - args = [slug, token, thumb.kwargs['size_fit'], thumb.kwargs['path']] + filename = basename(thumb.kwargs['path']) + args = [slug, thumb.kwargs['size_fit'], token, filename] return reverse('photos:shared-thumbnail', args=args) diff --git a/website/photos/tests/test_views.py b/website/photos/tests/test_views.py index 5347f09f3641ef6c2e4635cff21434a5a1c67624..36185581c2f14c15e351bc0118ad4fc48f13ccf4 100644 --- a/website/photos/tests/test_views.py +++ b/website/photos/tests/test_views.py @@ -192,7 +192,7 @@ class SharedAlbumTest(TestCase): photo.save() response = self.client.get(reverse( - 'photos:shared_album', + 'photos:shared-album', args=(self.album.slug, self.album.access_token,))) self.assertEqual(response.status_code, 200) self.assertEqual(response.context['album'], self.album) @@ -229,13 +229,13 @@ class DownloadTest(TestCase): self.client.force_login(self.member) response = self.client.get(reverse( - 'photos:download', args=(self.photo,))) + 'photos:download', args=(self.album.slug, self.photo,))) self.assertEqual(response.status_code, 200) self.assertEqual(response['Content-Type'], 'image/jpeg') def test_logged_out(self): response = self.client.get(reverse( - 'photos:download', args=(self.photo,))) + 'photos:download', args=(self.album.slug, self.photo,))) self.assertEqual(response.status_code, 302) @@ -269,7 +269,8 @@ class SharedDownloadTest(TestCase): with self.subTest(): response = self.client.get(reverse( 'photos:shared-download', - args=(self.album.slug, self.album.access_token, self.photo,))) + args=(self.album.slug, self.album.access_token, + self.photo,))) self.assertEqual(response.status_code, 200) self.assertEqual(response['Content-Type'], 'image/jpeg') @@ -278,7 +279,8 @@ class SharedDownloadTest(TestCase): with self.subTest(): response = self.client.get(reverse( 'photos:shared-download', - args=(self.album.slug, self.album.access_token, self.photo,))) + args=(self.album.slug, self.album.access_token, + self.photo,))) self.assertEqual(response.status_code, 200) self.assertEqual(response['Content-Type'], 'image/jpeg') diff --git a/website/photos/urls.py b/website/photos/urls.py index 6b99648ef86c83ac4628dd035ef9065a3c8fd567..3c96458f2c0c6669db8b1cc29f91bb3ef1ea98e9 100644 --- a/website/photos/urls.py +++ b/website/photos/urls.py @@ -1,15 +1,26 @@ -from django.conf.urls import url +from django.urls import path, include, re_path from . import views app_name = "photos" urlpatterns = [ - url(r'^download/(?P.*)', views.download, name='download'), - url(r'^shared-download/(?P[-\w]+)/(?P[a-zA-Z0-9]+)/(?P.*)', views.shared_download, name='shared-download'), - url(r'^shared-thumbnail/(?P[-\w]+)/(?P[a-zA-Z0-9]+)/(?P\d+x\d+_[01])/(?P.*)', views.shared_thumbnail, name='shared-thumbnail'), - url(r'^(?P[-\w]+)/$', views.album, name='album'), - url(r'^(?P[-\w]+)/download$', views.album_download, name='album-download'), - url(r'^(?P[-\w]+)/(?P[a-zA-Z0-9]+)$', views.shared_album, name='shared_album'), - url(r'^$', views.index, name='index'), + path('', views.index, name='index'), + path('/', include([ + path('', views.album, name='album'), + path('download/', include([ + path('', views.album_download, name='album-download'), + path('', views.download, name='download'), + path('/', include([ + path('', views.shared_album_download, name='shared-album-download'), + path('', views.shared_download, name='shared-download'), + ])), + ])), + path('thumbnail/', include([ + re_path(r'(?P\d+x\d+_[01])/', include([ + path('//', views.shared_thumbnail, name='shared-thumbnail'), + ])), + ])), + path('/', views.shared_album, name='shared-album'), + ])), ] diff --git a/website/photos/views.py b/website/photos/views.py index 83b463c80580c3dfb621d5040f10c87746c6009c..3f19063d60c1b38cb69df7a9ed0bf6d85a92a096 100644 --- a/website/photos/views.py +++ b/website/photos/views.py @@ -2,17 +2,17 @@ import os from tempfile import gettempdir from zipfile import ZipFile -from django.conf import settings from django.contrib.auth.decorators import login_required -from django.core.exceptions import SuspiciousFileOperation from django.core.paginator import EmptyPage, Paginator from django.http import Http404 from django.shortcuts import get_object_or_404, render from sendfile import sendfile -from photos import services +from photos.models import Album, Photo +from photos.services import (check_shared_album_token, + get_annotated_accessible_albums, + is_album_accessible) from utils.views import _private_thumbnails_unauthed -from .models import Album COVER_FILENAME = 'cover.jpg' @@ -22,7 +22,7 @@ def index(request): # Only show published albums albums = Album.objects.filter(hidden=False) - albums = services.get_annotated_accessible_albums(request, albums) + albums = get_annotated_accessible_albums(request, albums) albums = albums.order_by('-date') paginator = Paginator(albums, 12) @@ -65,78 +65,77 @@ def _render_album_page(request, album): @login_required def album(request, slug): album = get_object_or_404(Album, slug=slug) - if services.is_album_accessible(request, album): + if is_album_accessible(request, album): return _render_album_page(request, album) raise Http404("Sorry, you're not allowed to view this album") -def _checked_shared_album(slug, token): - album = get_object_or_404(Album, slug=slug) - if token != album.access_token: - raise Http404("Invalid token.") - return album - - def shared_album(request, slug, token): - album = _checked_shared_album(slug, token) + album = get_object_or_404(Album, slug=slug) + check_shared_album_token(album, token) return _render_album_page(request, album) -def _download(request, original_path): - """This function provides a layer of indirection for shared albums - - Checks for some path traversal: +def _photo_path(album, filename): + photoname = os.path.basename(filename) + albumpath = os.path.join(album.photosdir, album.dirname) + photopath = os.path.join(albumpath, photoname) + get_object_or_404(Photo.objects.filter(album=album, file=photopath)) + return photopath - >>> from django.test import RequestFactory - >>> r = RequestFactory().get('/photos/download/../../../../../etc/passwd') - >>> _download(r, '../../../../../../../etc/passwd') #doctest: +ELLIPSIS - Traceback (most recent call last): - ... - django.core.exceptions.SuspiciousFileOperation: ... - """ - photopath = os.path.join(settings.MEDIA_ROOT, 'photos') - path = os.path.normpath( - os.path.join(photopath, *original_path.split('/')[1:])) - - if not os.path.commonpath([photopath, path]) == photopath: - raise SuspiciousFileOperation( - "Path traversal detected: someone tried to download " - "{}, input: {}".format(path, original_path)) - if not os.path.isfile(path): - raise Http404("Photo not found.") - return sendfile(request, path, attachment=True) +def _download(request, album, filename): + """This function provides a layer of indirection for shared albums""" + photopath = _photo_path(album, filename) + photo = get_object_or_404( + Photo.objects.filter(album=album, file=photopath)) + return sendfile(request, photo.file.path, attachment=True) -def _album_download(request, slug): +def _album_download(request, album): """This function provides a layer of indirection for shared albums""" - album = get_object_or_404(Album, slug=slug) albumpath = os.path.join(album.photospath, album.dirname) - pictures = [os.path.join(albumpath, x) for x in os.listdir(albumpath)] zipfilename = os.path.join(gettempdir(), '{}.zip'.format(album.dirname)) if not os.path.exists(zipfilename): with ZipFile(zipfilename, 'w') as f: + pictures = [os.path.join(albumpath, x) + for x in os.listdir(albumpath)] for picture in pictures: f.write(picture, arcname=os.path.basename(picture)) return sendfile(request, zipfilename, attachment=True) @login_required -def download(request, path): - return _download(request, path) +def download(request, slug, filename): + album = get_object_or_404(Album, slug=slug) + if is_album_accessible(request, album): + return _download(request, album, filename) + raise Http404("Sorry, you're not allowed to view this album") @login_required def album_download(request, slug): - return _album_download(request, slug) + album = get_object_or_404(Album, slug=slug) + if is_album_accessible(request, album): + return _album_download(request, album) + raise Http404("Sorry, you're not allowed to view this album") + + +def shared_download(request, slug, token, filename): + album = get_object_or_404(Album, slug=slug) + check_shared_album_token(album, token) + return _download(request, album, filename) -def shared_download(request, slug, token, path): - _checked_shared_album(slug, token) - return _download(request, path) +def shared_album_download(request, slug, token): + album = get_object_or_404(Album, slug=slug) + check_shared_album_token(album, token) + return _album_download(request, album) -def shared_thumbnail(request, slug, token, size_fit, path): - _checked_shared_album(slug, token) - return _private_thumbnails_unauthed(request, size_fit, path) +def shared_thumbnail(request, slug, size_fit, token, filename): + album = get_object_or_404(Album, slug=slug) + check_shared_album_token(album, token) + photopath = _photo_path(album, filename) + return _private_thumbnails_unauthed(request, size_fit, photopath)