diff --git a/api/compose/nginx/Dockerfile b/api/compose/nginx/Dockerfile deleted file mode 100644 index 196395763a7b93c4842d68bf2bca8f9c9d1e635b..0000000000000000000000000000000000000000 --- a/api/compose/nginx/Dockerfile +++ /dev/null @@ -1,2 +0,0 @@ -FROM nginx:latest -ADD nginx.conf /etc/nginx/nginx.conf \ No newline at end of file diff --git a/api/config/api_urls.py b/api/config/api_urls.py index b56944d4eed8b75faa0c51202c286ef4662d16ee..9c612c94d92d742318cf6b60cb359fff45127105 100644 --- a/api/config/api_urls.py +++ b/api/config/api_urls.py @@ -8,6 +8,7 @@ from rest_framework_jwt import views as jwt_views router = routers.SimpleRouter() router.register(r'tags', views.TagViewSet, 'tags') router.register(r'tracks', views.TrackViewSet, 'tracks') +router.register(r'trackfiles', views.TrackFileViewSet, 'trackfiles') router.register(r'artists', views.ArtistViewSet, 'artists') router.register(r'albums', views.AlbumViewSet, 'albums') router.register(r'import-batches', views.ImportBatchViewSet, 'import-batches') diff --git a/api/config/settings/common.py b/api/config/settings/common.py index 93381c4f5399425e340961bbde35500884df88bc..5ba197145330a7a29a3e9e03d7d33db9087af223 100644 --- a/api/config/settings/common.py +++ b/api/config/settings/common.py @@ -217,7 +217,6 @@ STATICFILES_FINDERS = ( # See: https://docs.djangoproject.com/en/dev/ref/settings/#media-root MEDIA_ROOT = str(APPS_DIR('media')) -USE_SAMPLE_TRACK = env.bool("USE_SAMPLE_TRACK", False) # See: https://docs.djangoproject.com/en/dev/ref/settings/#media-url @@ -261,7 +260,6 @@ BROKER_URL = env("CELERY_BROKER_URL", default='django://') # Location of root django.contrib.admin URL, use {% url 'admin:index' %} ADMIN_URL = r'^admin/' -SESSION_SAVE_EVERY_REQUEST = True # Your common stuff: Below this line define 3rd party library settings CELERY_DEFAULT_RATE_LIMIT = 1 CELERYD_TASK_TIME_LIMIT = 300 @@ -305,3 +303,13 @@ FUNKWHALE_PROVIDERS = { } } ATOMIC_REQUESTS = False + +# Wether we should check user permission before serving audio files (meaning +# return an obfuscated url) +# This require a special configuration on the reverse proxy side +# See https://wellfire.co/learn/nginx-django-x-accel-redirects/ for example +PROTECT_AUDIO_FILES = env.bool('PROTECT_AUDIO_FILES', default=True) + +# Which path will be used to process the internal redirection +# **DO NOT** put a slash at the end +PROTECT_FILES_PATH = env('PROTECT_FILES_PATH', default='/_protected') diff --git a/api/funkwhale_api/music/models.py b/api/funkwhale_api/music/models.py index 70230847700b9b6b788b8d36ea22b9774a963947..6a55dfc00c385f18da92c7f6c0da49458c16402e 100644 --- a/api/funkwhale_api/music/models.py +++ b/api/funkwhale_api/music/models.py @@ -8,7 +8,6 @@ import markdown from django.conf import settings from django.db import models -from django.contrib.staticfiles.templatetags.staticfiles import static from django.core.files.base import ContentFile from django.core.files import File from django.core.urlresolvers import reverse @@ -354,10 +353,12 @@ class TrackFile(models.Model): @property def path(self): - if settings.USE_SAMPLE_TRACK: - return static('music/sample1.ogg') + if settings.PROTECT_AUDIO_FILES: + return reverse( + 'api:v1:trackfiles-serve', kwargs={'pk': self.pk}) return self.audio_file.url + class ImportBatch(models.Model): creation_date = models.DateTimeField(default=timezone.now) submitted_by = models.ForeignKey('users.User', related_name='imports') diff --git a/api/funkwhale_api/music/tests/factories.py b/api/funkwhale_api/music/tests/factories.py new file mode 100644 index 0000000000000000000000000000000000000000..dfa7a75e2814c82e276f692da07d1411da2b3e95 --- /dev/null +++ b/api/funkwhale_api/music/tests/factories.py @@ -0,0 +1,39 @@ +import factory + + +class ArtistFactory(factory.django.DjangoModelFactory): + name = factory.Sequence(lambda n: 'artist-{0}'.format(n)) + mbid = factory.Faker('uuid4') + + class Meta: + model = 'music.Artist' + + +class AlbumFactory(factory.django.DjangoModelFactory): + title = factory.Sequence(lambda n: 'album-{0}'.format(n)) + mbid = factory.Faker('uuid4') + release_date = factory.Faker('date') + cover = factory.django.ImageField() + artist = factory.SubFactory(ArtistFactory) + + class Meta: + model = 'music.Album' + + +class TrackFactory(factory.django.DjangoModelFactory): + title = factory.Sequence(lambda n: 'track-{0}'.format(n)) + mbid = factory.Faker('uuid4') + album = factory.SubFactory(AlbumFactory) + artist = factory.SelfAttribute('album.artist') + position = 1 + + class Meta: + model = 'music.Track' + + +class TrackFileFactory(factory.django.DjangoModelFactory): + track = factory.SubFactory(TrackFactory) + audio_file = factory.django.FileField() + + class Meta: + model = 'music.TrackFile' diff --git a/api/funkwhale_api/music/tests/test_api.py b/api/funkwhale_api/music/tests/test_api.py index d8f56eeb92604b9b9399c61c0124b9b9d8256fed..21a567084e25a4d3ade7a27fe878d227839f4bbf 100644 --- a/api/funkwhale_api/music/tests/test_api.py +++ b/api/funkwhale_api/music/tests/test_api.py @@ -10,6 +10,8 @@ from funkwhale_api.music import serializers from funkwhale_api.users.models import User from . import data as api_data +from . import factories + class TestAPI(TMPDirTestCaseMixin, TestCase): @@ -214,3 +216,26 @@ class TestAPI(TMPDirTestCaseMixin, TestCase): with self.settings(API_AUTHENTICATION_REQUIRED=False): response = getattr(self.client, method)(url) self.assertEqual(response.status_code, 200) + + def test_track_file_url_is_restricted_to_authenticated_users(self): + f = factories.TrackFileFactory() + self.assertNotEqual(f.audio_file, None) + url = f.path + + with self.settings(API_AUTHENTICATION_REQUIRED=True): + response = self.client.get(url) + + self.assertEqual(response.status_code, 401) + + user = User.objects.create_superuser( + username='test', email='test@test.com', password='test') + self.client.login(username=user.username, password='test') + with self.settings(API_AUTHENTICATION_REQUIRED=True): + response = self.client.get(url) + + self.assertEqual(response.status_code, 200) + + self.assertEqual( + response['X-Accel-Redirect'], + '/_protected{}'.format(f.audio_file.url) + ) diff --git a/api/funkwhale_api/music/views.py b/api/funkwhale_api/music/views.py index 772f4173ea0a5de52ff3e5b75293c8d6affb0ecd..506db123923ce9b706a46dcbff5771ec40473e60 100644 --- a/api/funkwhale_api/music/views.py +++ b/api/funkwhale_api/music/views.py @@ -3,6 +3,7 @@ import json from django.core.urlresolvers import reverse from django.db import models, transaction from django.db.models.functions import Length +from django.conf import settings from rest_framework import viewsets, views from rest_framework.decorators import detail_route, list_route from rest_framework.response import Response @@ -51,6 +52,7 @@ class ArtistViewSet(SearchMixin, viewsets.ReadOnlyModelViewSet): search_fields = ['name'] ordering_fields = ('creation_date',) + class AlbumViewSet(SearchMixin, viewsets.ReadOnlyModelViewSet): queryset = ( models.Album.objects.all() @@ -63,6 +65,7 @@ class AlbumViewSet(SearchMixin, viewsets.ReadOnlyModelViewSet): search_fields = ['title'] ordering_fields = ('creation_date',) + class ImportBatchViewSet(viewsets.ReadOnlyModelViewSet): queryset = models.ImportBatch.objects.all().order_by('-creation_date') serializer_class = serializers.ImportBatchSerializer @@ -70,6 +73,7 @@ class ImportBatchViewSet(viewsets.ReadOnlyModelViewSet): def get_queryset(self): return super().get_queryset().filter(submitted_by=self.request.user) + class TrackViewSet(TagViewSetMixin, SearchMixin, viewsets.ReadOnlyModelViewSet): """ A simple ViewSet for viewing and editing accounts. @@ -120,6 +124,27 @@ class TrackViewSet(TagViewSetMixin, SearchMixin, viewsets.ReadOnlyModelViewSet): return Response(serializer.data) +class TrackFileViewSet(viewsets.ReadOnlyModelViewSet): + queryset = (models.TrackFile.objects.all().order_by('-id')) + serializer_class = serializers.TrackFileSerializer + permission_classes = [ConditionalAuthentication] + + @detail_route(methods=['get']) + def serve(self, request, *args, **kwargs): + try: + f = models.TrackFile.objects.get(pk=kwargs['pk']) + except models.TrackFile.DoesNotExist: + return Response(status=404) + + response = Response() + response["Content-Disposition"] = "attachment; filename={0}".format( + f.audio_file.name) + response['X-Accel-Redirect'] = "{}{}".format( + settings.PROTECT_FILES_PATH, + f.audio_file.url) + return response + + class TagViewSet(viewsets.ReadOnlyModelViewSet): queryset = Tag.objects.all().order_by('name') serializer_class = serializers.TagSerializer diff --git a/deploy/nginx.conf b/deploy/nginx.conf index 7395e37d90a4fa5b1ce800ce5c4afdfff2dfb5a4..6a0a9f50936f32dfc9c29cbf29ac58ed1658e473 100644 --- a/deploy/nginx.conf +++ b/deploy/nginx.conf @@ -47,7 +47,17 @@ server { location /media/ { alias /srv/funkwhale/data/media/; } + + location /_protected/media { + # this is an internal location that is used to serve + # audio files once correct permission / authentication + # has been checked on API side + internal; + alias /srv/funkwhale/data/media; + } + location /staticfiles/ { + # django static files alias /srv/funkwhale/data/static/; } } diff --git a/dev.yml b/dev.yml index 21b0912e397d554793b387892fa9a7c816f5f090..712288492000dc93b4e43ad93a759c0cfb6d3e1d 100644 --- a/dev.yml +++ b/dev.yml @@ -53,12 +53,14 @@ services: - redis - celeryworker - # nginx: - # env_file: .env.dev - # build: ./api/compose/nginx - # links: - # - api - # volumes: - # - ./api/funkwhale_api/media:/staticfiles/media - # ports: - # - "0.0.0.0:6001:80" + nginx: + env_file: .env.dev + image: nginx + links: + - api + - front + volumes: + - ./docker/nginx/conf.dev:/etc/nginx/nginx.conf + - ./api/funkwhale_api/media:/protected/media + ports: + - "0.0.0.0:6001:80" diff --git a/api/compose/nginx/nginx.conf b/docker/nginx/conf.dev similarity index 72% rename from api/compose/nginx/nginx.conf rename to docker/nginx/conf.dev index 331d8d45f3c2311cac0aafd01ded6bb22f0454bd..6ca395fb1aaf47fbfb1a09bfd491daddc5f28875 100644 --- a/api/compose/nginx/nginx.conf +++ b/docker/nginx/conf.dev @@ -27,27 +27,21 @@ http { #gzip on; - upstream app { - server django:12081; - } - server { listen 80; charset utf-8; - root /staticfiles; - location / { - # checks for static file, if not found proxy to app - try_files $uri @proxy_to_app; + location /_protected/media { + internal; + alias /protected/media; } - - location @proxy_to_app { + location / { + proxy_set_header Host $host; + proxy_set_header X-Real-IP $remote_addr; proxy_set_header X-Forwarded-For $proxy_add_x_forwarded_for; - proxy_set_header Host $http_host; + proxy_set_header X-Forwarded-Proto $scheme; proxy_redirect off; - - proxy_pass http://app; + proxy_pass http://api:12081/; } - } } diff --git a/docs/changelog.rst b/docs/changelog.rst index c4092dc8b549d8157b77500da46731d7b2cbb103..ced5c0a7ea9d761a2bebcadc5f7f6114f68cc8a5 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -1,6 +1,20 @@ Changelog ========= +next +------- + +* [breaking] we now check for user permission before serving audio files, which requires +a specific configuration block in your reverse proxy configuration: + +.. code-block:: + + location /_protected/media { + internal; + alias /srv/funkwhale/data/media; + } + + 0.1 -------