From 2586444db235e384e9773af58ab563a24b3a033a Mon Sep 17 00:00:00 2001
From: Eliot Berriot <contact@eliotberriot.com>
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
-- 
GitLab