Commit bab3981d authored by Eliot Berriot's avatar Eliot Berriot 💬

Fixed #15: Ensure we check for authorization for serving audio files, meaning...

Fixed #15: Ensure we check for authorization for serving audio files, meaning we don't leak the absolute URL anymore
parent e2507598
Pipeline #119 passed with stage
in 26 seconds
FROM nginx:latest
ADD nginx.conf /etc/nginx/nginx.conf
\ No newline at end of file
......@@ -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')
......
......@@ -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')
......@@ -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')
......
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'
......@@ -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)
)
......@@ -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
......
......@@ -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/;
}
}
......@@ -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"
......@@ -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/;
}
}
}
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
-------
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment