From 86269c1b078c96a69a10d9c777de69664715ec62 Mon Sep 17 00:00:00 2001 From: Eliot Berriot Date: Thu, 9 May 2019 11:43:35 +0200 Subject: [PATCH] Can now serve audio content directly from S3 --- api/config/settings/common.py | 9 +++++-- api/funkwhale_api/music/views.py | 13 +++++++-- api/funkwhale_api/subsonic/views.py | 8 +++++- api/tests/music/test_views.py | 42 ++++++++++++++++++++++++----- api/tests/subsonic/test_views.py | 25 ++++++++++++++--- docs/admin/external-storages.rst | 20 ++++++++++++++ 6 files changed, 101 insertions(+), 16 deletions(-) diff --git a/api/config/settings/common.py b/api/config/settings/common.py index bf357e17c..2f4f6e9a4 100644 --- a/api/config/settings/common.py +++ b/api/config/settings/common.py @@ -306,9 +306,13 @@ STATIC_ROOT = env("STATIC_ROOT", default=str(ROOT_DIR("staticfiles"))) STATIC_URL = env("STATIC_URL", default="/staticfiles/") DEFAULT_FILE_STORAGE = "funkwhale_api.common.storage.ASCIIFileSystemStorage" +PROXY_MEDIA = env.bool("PROXY_MEDIA", default=True) AWS_DEFAULT_ACL = None -AWS_QUERYSTRING_AUTH = False - +AWS_QUERYSTRING_AUTH = env.bool("AWS_QUERYSTRING_AUTH", default=not PROXY_MEDIA) +AWS_S3_MAX_MEMORY_SIZE = env.int( + "AWS_S3_MAX_MEMORY_SIZE", default=1000 * 1000 * 1000 * 20 +) +AWS_QUERYSTRING_EXPIRE = env.int("AWS_QUERYSTRING_EXPIRE", default=3600) AWS_ACCESS_KEY_ID = env("AWS_ACCESS_KEY_ID", default=None) if AWS_ACCESS_KEY_ID: @@ -319,6 +323,7 @@ if AWS_ACCESS_KEY_ID: AWS_LOCATION = env("AWS_LOCATION", default="") DEFAULT_FILE_STORAGE = "storages.backends.s3boto3.S3Boto3Storage" + # See: https://docs.djangoproject.com/en/dev/ref/contrib/staticfiles/#std:setting-STATICFILES_DIRS STATICFILES_DIRS = (str(APPS_DIR.path("static")),) diff --git a/api/funkwhale_api/music/views.py b/api/funkwhale_api/music/views.py index 20e173d83..391a4b333 100644 --- a/api/funkwhale_api/music/views.py +++ b/api/funkwhale_api/music/views.py @@ -285,7 +285,7 @@ def should_transcode(upload, format, max_bitrate=None): return format_need_transcoding or bitrate_need_transcoding -def handle_serve(upload, user, format=None, max_bitrate=None): +def handle_serve(upload, user, format=None, max_bitrate=None, proxy_media=True): f = upload # we update the accessed_date now = timezone.now() @@ -329,6 +329,11 @@ def handle_serve(upload, user, format=None, max_bitrate=None): f = transcoded_version file_path = get_file_path(f.audio_file) mt = f.mimetype + if not proxy_media: + # we simply issue a 302 redirect to the real URL + response = Response(status=302) + response["Location"] = f.audio_file.url + return response if mt: response = Response(content_type=mt) else: @@ -380,7 +385,11 @@ class ListenViewSet(mixins.RetrieveModelMixin, viewsets.GenericViewSet): if max_bitrate: max_bitrate = max_bitrate * 1000 return handle_serve( - upload, user=request.user, format=format, max_bitrate=max_bitrate + upload, + user=request.user, + format=format, + max_bitrate=max_bitrate, + proxy_media=settings.PROXY_MEDIA, ) diff --git a/api/funkwhale_api/subsonic/views.py b/api/funkwhale_api/subsonic/views.py index 31bd00407..88d7ece7c 100644 --- a/api/funkwhale_api/subsonic/views.py +++ b/api/funkwhale_api/subsonic/views.py @@ -260,7 +260,13 @@ class SubsonicViewSet(viewsets.GenericViewSet): if max_bitrate: max_bitrate = max_bitrate * 1000 return music_views.handle_serve( - upload=upload, user=request.user, format=format, max_bitrate=max_bitrate + upload=upload, + user=request.user, + format=format, + max_bitrate=max_bitrate, + # Subsonic clients don't expect 302 redirection unfortunately, + # So we have to proxy media files + proxy_media=True, ) @action(detail=False, methods=["get", "post"], url_name="star", url_path="star") diff --git a/api/tests/music/test_views.py b/api/tests/music/test_views.py index 32d95e14f..102b5a790 100644 --- a/api/tests/music/test_views.py +++ b/api/tests/music/test_views.py @@ -331,7 +331,7 @@ def test_listen_correct_access(factories, logged_in_api_client): assert response.status_code == 200 -def test_listen_explicit_file(factories, logged_in_api_client, mocker): +def test_listen_explicit_file(factories, logged_in_api_client, mocker, settings): mocked_serve = mocker.spy(views, "handle_serve") upload1 = factories["music.Upload"]( library__privacy_level="everyone", import_status="finished" @@ -344,10 +344,26 @@ def test_listen_explicit_file(factories, logged_in_api_client, mocker): assert response.status_code == 200 mocked_serve.assert_called_once_with( - upload2, user=logged_in_api_client.user, format=None, max_bitrate=None + upload2, + user=logged_in_api_client.user, + format=None, + max_bitrate=None, + proxy_media=settings.PROXY_MEDIA, ) +def test_listen_no_proxy(factories, logged_in_api_client, settings): + settings.PROXY_MEDIA = False + upload = factories["music.Upload"]( + library__privacy_level="everyone", import_status="finished" + ) + url = reverse("api:v1:listen-detail", kwargs={"uuid": upload.track.uuid}) + response = logged_in_api_client.get(url, {"upload": upload.uuid}) + + assert response.status_code == 302 + assert response["Location"] == upload.audio_file.url + + @pytest.mark.parametrize( "mimetype,format,expected", [ @@ -409,7 +425,7 @@ def test_handle_serve_create_mp3_version(factories, now): assert response.status_code == 200 -def test_listen_transcode(factories, now, logged_in_api_client, mocker): +def test_listen_transcode(factories, now, logged_in_api_client, mocker, settings): upload = factories["music.Upload"]( import_status="finished", library__actor__user=logged_in_api_client.user ) @@ -420,7 +436,11 @@ def test_listen_transcode(factories, now, logged_in_api_client, mocker): assert response.status_code == 200 handle_serve.assert_called_once_with( - upload, user=logged_in_api_client.user, format="mp3", max_bitrate=None + upload, + user=logged_in_api_client.user, + format="mp3", + max_bitrate=None, + proxy_media=settings.PROXY_MEDIA, ) @@ -436,7 +456,7 @@ def test_listen_transcode(factories, now, logged_in_api_client, mocker): ], ) def test_listen_transcode_bitrate( - max_bitrate, expected, factories, now, logged_in_api_client, mocker + max_bitrate, expected, factories, now, logged_in_api_client, mocker, settings ): upload = factories["music.Upload"]( import_status="finished", library__actor__user=logged_in_api_client.user @@ -448,7 +468,11 @@ def test_listen_transcode_bitrate( assert response.status_code == 200 handle_serve.assert_called_once_with( - upload, user=logged_in_api_client.user, format=None, max_bitrate=expected + upload, + user=logged_in_api_client.user, + format=None, + max_bitrate=expected, + proxy_media=settings.PROXY_MEDIA, ) @@ -474,7 +498,11 @@ def test_listen_transcode_in_place( assert response.status_code == 200 handle_serve.assert_called_once_with( - upload, user=logged_in_api_client.user, format="mp3", max_bitrate=None + upload, + user=logged_in_api_client.user, + format="mp3", + max_bitrate=None, + proxy_media=settings.PROXY_MEDIA, ) diff --git a/api/tests/subsonic/test_views.py b/api/tests/subsonic/test_views.py index ec61b25fa..73f968ff4 100644 --- a/api/tests/subsonic/test_views.py +++ b/api/tests/subsonic/test_views.py @@ -217,7 +217,12 @@ def test_get_song( @pytest.mark.parametrize("f", ["json"]) -def test_stream(f, db, logged_in_api_client, factories, mocker, queryset_equal_queries): +def test_stream( + f, db, logged_in_api_client, factories, mocker, queryset_equal_queries, settings +): + # Even with this settings set to false, we proxy media in the subsonic API + # Because clients don't expect a 302 redirect + settings.PROXY_MEDIA = False url = reverse("api:subsonic-stream") mocked_serve = mocker.spy(music_views, "handle_serve") assert url.endswith("stream") is True @@ -226,7 +231,11 @@ def test_stream(f, db, logged_in_api_client, factories, mocker, queryset_equal_q response = logged_in_api_client.get(url, {"f": f, "id": upload.track.pk}) mocked_serve.assert_called_once_with( - upload=upload, user=logged_in_api_client.user, format=None, max_bitrate=None + upload=upload, + user=logged_in_api_client.user, + format=None, + max_bitrate=None, + proxy_media=True, ) assert response.status_code == 200 playable_by.assert_called_once_with(music_models.Track.objects.all(), None) @@ -242,7 +251,11 @@ def test_stream_format(format, expected, logged_in_api_client, factories, mocker response = logged_in_api_client.get(url, {"id": upload.track.pk, "format": format}) mocked_serve.assert_called_once_with( - upload=upload, user=logged_in_api_client.user, format=expected, max_bitrate=None + upload=upload, + user=logged_in_api_client.user, + format=expected, + max_bitrate=None, + proxy_media=True, ) assert response.status_code == 200 @@ -261,7 +274,11 @@ def test_stream_bitrate(max_bitrate, expected, logged_in_api_client, factories, ) mocked_serve.assert_called_once_with( - upload=upload, user=logged_in_api_client.user, format=None, max_bitrate=expected + upload=upload, + user=logged_in_api_client.user, + format=None, + max_bitrate=expected, + proxy_media=True, ) assert response.status_code == 200 diff --git a/docs/admin/external-storages.rst b/docs/admin/external-storages.rst index 73b888904..413f8cfb2 100644 --- a/docs/admin/external-storages.rst +++ b/docs/admin/external-storages.rst @@ -55,6 +55,26 @@ by hand (which is outside the scope of this guide). At the moment, we do not support S3 when using Apache as a reverse proxy. +Serving audio files directly from the bucket +******************************************** + +Depending on your setup, you may want to serve audio fils directly from the S3 bucket +instead of proxying them through Funkwhale, e.g to reduce the bandwidth consumption on your server, +or get better performance. + +You can achieve that by adding ``PROXY_MEDIA=false`` to your ``.env`` file. + +When receiving a request on the stream endpoint, Funkwhale will check for authentication and permissions, +then issue a 302 redirect to the file URL in the bucket. + +This URL is actually be visible by the client, but contains a signature valid only for one hour, to ensure +no one can reuse this URL or share it publicly to distribute unauthorized content. + +.. note:: + + Since some Subsonic clients don't support 302 redirections, Funkwhale will ignore + the ``PROXY_MEDIA`` setting and always proxy file when accessed through the Subsonic API. + Securing your S3 bucket ***********************