From 2586444db235e384e9773af58ab563a24b3a033a Mon Sep 17 00:00:00 2001 From: Eliot Berriot Date: Thu, 24 May 2018 21:34:59 +0200 Subject: [PATCH] Fix #229: removed last hardcoded settings to protect audio files --- api/config/settings/common.py | 6 ---- api/funkwhale_api/music/permissions.py | 3 -- api/tests/music/test_permissions.py | 29 ++++++---------- api/tests/music/test_views.py | 47 ++++++++++++++++++-------- changes/changelog.d/229.bugfix | 1 + demo/setup.sh | 2 -- 6 files changed, 44 insertions(+), 44 deletions(-) create mode 100644 changes/changelog.d/229.bugfix diff --git a/api/config/settings/common.py b/api/config/settings/common.py index 59aa93117..f376781b0 100644 --- a/api/config/settings/common.py +++ b/api/config/settings/common.py @@ -433,12 +433,6 @@ USE_X_FORWARDED_PORT = True REVERSE_PROXY_TYPE = env('REVERSE_PROXY_TYPE', default='nginx') assert REVERSE_PROXY_TYPE in ['apache2', 'nginx'], 'Unsupported REVERSE_PROXY_TYPE' -# 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/permissions.py b/api/funkwhale_api/music/permissions.py index 77f95c477..d31e1c5d5 100644 --- a/api/funkwhale_api/music/permissions.py +++ b/api/funkwhale_api/music/permissions.py @@ -10,9 +10,6 @@ from funkwhale_api.federation import models class Listen(BasePermission): def has_permission(self, request, view): - if not settings.PROTECT_AUDIO_FILES: - return True - if not preferences.get('common__api_authentication_required'): return True diff --git a/api/tests/music/test_permissions.py b/api/tests/music/test_permissions.py index a5f0c4109..825d1731d 100644 --- a/api/tests/music/test_permissions.py +++ b/api/tests/music/test_permissions.py @@ -4,26 +4,17 @@ from funkwhale_api.federation import actors from funkwhale_api.music import permissions -def test_list_permission_no_protect(anonymous_user, api_request, settings): - settings.PROTECT_AUDIO_FILES = False +def test_list_permission_no_protect(preferences, anonymous_user, api_request): + preferences['common__api_authentication_required'] = False view = APIView.as_view() permission = permissions.Listen() request = api_request.get('/') assert permission.has_permission(request, view) is True -def test_list_permission_protect_anonymous( - db, anonymous_user, api_request, settings): - settings.PROTECT_AUDIO_FILES = True - view = APIView.as_view() - permission = permissions.Listen() - request = api_request.get('/') - assert permission.has_permission(request, view) is False - - def test_list_permission_protect_authenticated( - factories, api_request, settings): - settings.PROTECT_AUDIO_FILES = True + factories, api_request, preferences): + preferences['common__api_authentication_required'] = True user = factories['users.User']() view = APIView.as_view() permission = permissions.Listen() @@ -33,8 +24,8 @@ def test_list_permission_protect_authenticated( def test_list_permission_protect_not_following_actor( - factories, api_request, settings): - settings.PROTECT_AUDIO_FILES = True + factories, api_request, preferences): + preferences['common__api_authentication_required'] = True actor = factories['federation.Actor']() view = APIView.as_view() permission = permissions.Listen() @@ -44,8 +35,8 @@ def test_list_permission_protect_not_following_actor( def test_list_permission_protect_following_actor( - factories, api_request, settings): - settings.PROTECT_AUDIO_FILES = True + factories, api_request, preferences): + preferences['common__api_authentication_required'] = True library_actor = actors.SYSTEM_ACTORS['library'].get_actor_instance() follow = factories['federation.Follow']( approved=True, target=library_actor) @@ -58,8 +49,8 @@ def test_list_permission_protect_following_actor( def test_list_permission_protect_following_actor_not_approved( - factories, api_request, settings): - settings.PROTECT_AUDIO_FILES = True + factories, api_request, preferences): + preferences['common__api_authentication_required'] = True library_actor = actors.SYSTEM_ACTORS['library'].get_actor_instance() follow = factories['federation.Follow']( approved=False, target=library_actor) diff --git a/api/tests/music/test_views.py b/api/tests/music/test_views.py index 9328ba329..042c1f607 100644 --- a/api/tests/music/test_views.py +++ b/api/tests/music/test_views.py @@ -119,8 +119,8 @@ def test_album_view_filter_listenable( def test_can_serve_track_file_as_remote_library( - factories, authenticated_actor, settings, api_client): - settings.PROTECT_AUDIO_FILES = True + factories, authenticated_actor, api_client, settings, preferences): + preferences['common__api_authentication_required'] = True library_actor = actors.SYSTEM_ACTORS['library'].get_actor_instance() follow = factories['federation.Follow']( approved=True, @@ -137,8 +137,8 @@ def test_can_serve_track_file_as_remote_library( def test_can_serve_track_file_as_remote_library_deny_not_following( - factories, authenticated_actor, settings, api_client): - settings.PROTECT_AUDIO_FILES = True + factories, authenticated_actor, settings, api_client, preferences): + preferences['common__api_authentication_required'] = True track_file = factories['music.TrackFile']() response = api_client.get(track_file.path) @@ -152,12 +152,18 @@ def test_can_serve_track_file_as_remote_library_deny_not_following( ('nginx', '/app/music', '/_protected/music/hello/world.mp3'), ]) def test_serve_file_in_place( - proxy, serve_path, expected, factories, api_client, settings): + proxy, + serve_path, + expected, + factories, + api_client, + preferences, + settings): headers = { 'apache2': 'X-Sendfile', 'nginx': 'X-Accel-Redirect', } - settings.PROTECT_AUDIO_FILES = False + preferences['common__api_authentication_required'] = False settings.PROTECT_FILE_PATH = '/_protected/music' settings.REVERSE_PROXY_TYPE = proxy settings.MUSIC_DIRECTORY_PATH = '/app/music' @@ -179,8 +185,14 @@ def test_serve_file_in_place( ('nginx', '/app/music', '/_protected/music/hello/worldéà.mp3'), ]) def test_serve_file_in_place_utf8( - proxy, serve_path, expected, factories, api_client, settings): - settings.PROTECT_AUDIO_FILES = False + proxy, + serve_path, + expected, + factories, + api_client, + settings, + preferences): + preferences['common__api_authentication_required'] = False settings.PROTECT_FILE_PATH = '/_protected/music' settings.REVERSE_PROXY_TYPE = proxy settings.MUSIC_DIRECTORY_PATH = '/app/music' @@ -198,12 +210,18 @@ def test_serve_file_in_place_utf8( ('nginx', '/app/music', '/_protected/media/tracks/hello/world.mp3'), ]) def test_serve_file_media( - proxy, serve_path, expected, factories, api_client, settings): + proxy, + serve_path, + expected, + factories, + api_client, + settings, + preferences): headers = { 'apache2': 'X-Sendfile', 'nginx': 'X-Accel-Redirect', } - settings.PROTECT_AUDIO_FILES = False + preferences['common__api_authentication_required'] = False settings.MEDIA_ROOT = '/host/media' settings.PROTECT_FILE_PATH = '/_protected/music' settings.REVERSE_PROXY_TYPE = proxy @@ -220,8 +238,8 @@ def test_serve_file_media( def test_can_proxy_remote_track( - factories, settings, api_client, r_mock): - settings.PROTECT_AUDIO_FILES = False + factories, settings, api_client, r_mock, preferences): + preferences['common__api_authentication_required'] = False track_file = factories['music.TrackFile'](federation=True) r_mock.get(track_file.library_track.audio_url, body=io.BytesIO(b'test')) @@ -236,8 +254,9 @@ def test_can_proxy_remote_track( assert library_track.audio_file.read() == b'test' -def test_serve_updates_access_date(factories, settings, api_client): - settings.PROTECT_AUDIO_FILES = False +def test_serve_updates_access_date( + factories, settings, api_client, preferences): + preferences['common__api_authentication_required'] = False track_file = factories['music.TrackFile']() now = timezone.now() assert track_file.accessed_date is None diff --git a/changes/changelog.d/229.bugfix b/changes/changelog.d/229.bugfix new file mode 100644 index 000000000..118ed6af2 --- /dev/null +++ b/changes/changelog.d/229.bugfix @@ -0,0 +1 @@ +Ensure anonymous users can use the app if the instance is configured accordingly (#229) diff --git a/demo/setup.sh b/demo/setup.sh index b96f517b3..e33bdf290 100644 --- a/demo/setup.sh +++ b/demo/setup.sh @@ -23,8 +23,6 @@ echo "DJANGO_SECRET_KEY=demo" >> .env echo "DJANGO_ALLOWED_HOSTS=demo.funkwhale.audio" >> .env echo "FUNKWHALE_VERSION=$version" >> .env echo "FUNKWHALE_API_PORT=5001" >> .env -echo "FEDERATION_MUSIC_NEEDS_APPROVAL=False" >>.env -echo "PROTECT_AUDIO_FILES=False" >> .env /usr/local/bin/docker-compose pull /usr/local/bin/docker-compose up -d postgres redis sleep 5