From a3b2125d2a62c4c735f829e932ce2cb2a6c57b41 Mon Sep 17 00:00:00 2001
From: Eliot Berriot <contact@eliotberriot.com>
Date: Sat, 28 Apr 2018 06:11:50 +0200
Subject: [PATCH] See #186: moved api authentication required setting to
 preference

---
 api/config/settings/common.py                 | 11 ++++++----
 .../common/dynamic_preferences_registry.py    | 20 +++++++++++++++++++
 api/funkwhale_api/common/permissions.py       |  4 +++-
 api/funkwhale_api/music/permissions.py        |  4 ++++
 api/tests/activity/test_views.py              |  4 ++--
 api/tests/favorites/test_favorites.py         |  4 ++--
 api/tests/history/test_history.py             |  5 +++--
 api/tests/music/test_api.py                   | 12 +++++------
 api/tests/playlists/test_views.py             |  8 ++++----
 api/tests/radios/test_radios.py               |  8 ++++----
 api/tests/test_jwt_querystring.py             |  5 +++--
 api/tests/test_youtube.py                     |  8 ++++----
 deploy/env.prod.sample                        |  3 ---
 13 files changed, 62 insertions(+), 34 deletions(-)
 create mode 100644 api/funkwhale_api/common/dynamic_preferences_registry.py

diff --git a/api/config/settings/common.py b/api/config/settings/common.py
index 7eb40983..d2948064 100644
--- a/api/config/settings/common.py
+++ b/api/config/settings/common.py
@@ -48,16 +48,18 @@ else:
 FUNKWHALE_URL = '{}://{}'.format(FUNKWHALE_PROTOCOL, FUNKWHALE_HOSTNAME)
 
 
+# XXX: deprecated, see #186
 FEDERATION_ENABLED = env.bool('FEDERATION_ENABLED', default=True)
 FEDERATION_HOSTNAME = env('FEDERATION_HOSTNAME', default=FUNKWHALE_HOSTNAME)
+# XXX: deprecated, see #186
 FEDERATION_COLLECTION_PAGE_SIZE = env.int(
     'FEDERATION_COLLECTION_PAGE_SIZE', default=50
 )
+# XXX: deprecated, see #186
 FEDERATION_MUSIC_NEEDS_APPROVAL = env.bool(
     'FEDERATION_MUSIC_NEEDS_APPROVAL', default=True
 )
-# how much minutes to wait before refetching actor data
-# when authenticating
+# XXX: deprecated, see #186
 FEDERATION_ACTOR_FETCH_DELAY = env.int(
     'FEDERATION_ACTOR_FETCH_DELAY', default=60 * 12)
 ALLOWED_HOSTS = env.list('DJANGO_ALLOWED_HOSTS')
@@ -366,7 +368,6 @@ CORS_ORIGIN_ALLOW_ALL = True
 #     'funkwhale.localhost',
 # )
 CORS_ALLOW_CREDENTIALS = True
-API_AUTHENTICATION_REQUIRED = env.bool("API_AUTHENTICATION_REQUIRED", True)
 REST_FRAMEWORK = {
     'DEFAULT_PERMISSION_CLASSES': (
         'rest_framework.permissions.IsAuthenticated',
@@ -433,7 +434,7 @@ ADMIN_URL = env('DJANGO_ADMIN_URL', default='^api/admin/')
 CSRF_USE_SESSIONS = True
 
 # Playlist settings
-# XXX: Deprecated, use playlists__max_tracks instead
+# XXX: deprecated, see #186
 PLAYLISTS_MAX_TRACKS = env.int('PLAYLISTS_MAX_TRACKS', default=250)
 
 ACCOUNT_USERNAME_BLACKLIST = [
@@ -453,6 +454,8 @@ EXTERNAL_REQUESTS_VERIFY_SSL = env.bool(
     'EXTERNAL_REQUESTS_VERIFY_SSL',
     default=True
 )
+# XXX: deprecated, see #186
+API_AUTHENTICATION_REQUIRED = env.bool("API_AUTHENTICATION_REQUIRED", True)
 
 MUSIC_DIRECTORY_PATH = env('MUSIC_DIRECTORY_PATH', default=None)
 # on Docker setup, the music directory may not match the host path,
diff --git a/api/funkwhale_api/common/dynamic_preferences_registry.py b/api/funkwhale_api/common/dynamic_preferences_registry.py
new file mode 100644
index 00000000..2374de7c
--- /dev/null
+++ b/api/funkwhale_api/common/dynamic_preferences_registry.py
@@ -0,0 +1,20 @@
+from dynamic_preferences import types
+from dynamic_preferences.registries import global_preferences_registry
+
+from funkwhale_api.common import preferences
+
+common = types.Section('common')
+
+
+@global_preferences_registry.register
+class APIAutenticationRequired(
+        preferences.DefaultFromSettingMixin, types.BooleanPreference):
+    section = common
+    name = 'api_authentication_required'
+    verbose_name = 'API Requires authentication'
+    setting = 'API_AUTHENTICATION_REQUIRED'
+    help_text = (
+        'If disabled, anonymous users will be able to query the API'
+        'and access music data (as well as other data exposed in the API '
+        'without specific permissions)'
+    )
diff --git a/api/funkwhale_api/common/permissions.py b/api/funkwhale_api/common/permissions.py
index c99c275c..cab4b699 100644
--- a/api/funkwhale_api/common/permissions.py
+++ b/api/funkwhale_api/common/permissions.py
@@ -5,11 +5,13 @@ from django.http import Http404
 
 from rest_framework.permissions import BasePermission, DjangoModelPermissions
 
+from funkwhale_api.common import preferences
+
 
 class ConditionalAuthentication(BasePermission):
 
     def has_permission(self, request, view):
-        if settings.API_AUTHENTICATION_REQUIRED:
+        if preferences.get('common__api_authentication_required'):
             return request.user and request.user.is_authenticated
         return True
 
diff --git a/api/funkwhale_api/music/permissions.py b/api/funkwhale_api/music/permissions.py
index 61fc65be..77f95c47 100644
--- a/api/funkwhale_api/music/permissions.py
+++ b/api/funkwhale_api/music/permissions.py
@@ -2,6 +2,7 @@ from django.conf import settings
 
 from rest_framework.permissions import BasePermission
 
+from funkwhale_api.common import preferences
 from funkwhale_api.federation import actors
 from funkwhale_api.federation import models
 
@@ -12,6 +13,9 @@ class Listen(BasePermission):
         if not settings.PROTECT_AUDIO_FILES:
             return True
 
+        if not preferences.get('common__api_authentication_required'):
+            return True
+
         user = getattr(request, 'user', None)
         if user and user.is_authenticated:
             return True
diff --git a/api/tests/activity/test_views.py b/api/tests/activity/test_views.py
index bdc3c633..9b24f3ad 100644
--- a/api/tests/activity/test_views.py
+++ b/api/tests/activity/test_views.py
@@ -4,8 +4,8 @@ from funkwhale_api.activity import serializers
 from funkwhale_api.activity import utils
 
 
-def test_activity_view(factories, api_client, settings, anonymous_user):
-    settings.API_AUTHENTICATION_REQUIRED = False
+def test_activity_view(factories, api_client, preferences, anonymous_user):
+    preferences['common__api_authentication_required'] = False
     favorite = factories['favorites.TrackFavorite'](
         user__privacy_level='everyone')
     listening = factories['history.Listening']()
diff --git a/api/tests/favorites/test_favorites.py b/api/tests/favorites/test_favorites.py
index f4a045af..591fe7c9 100644
--- a/api/tests/favorites/test_favorites.py
+++ b/api/tests/favorites/test_favorites.py
@@ -99,8 +99,8 @@ def test_user_can_remove_favorite_via_api_using_track_id(
 @pytest.mark.parametrize('url,method', [
     ('api:v1:favorites:tracks-list', 'get'),
 ])
-def test_url_require_auth(url, method, db, settings, client):
-    settings.API_AUTHENTICATION_REQUIRED = True
+def test_url_require_auth(url, method, db, preferences, client):
+    preferences['common__api_authentication_required'] = True
     url = reverse(url)
     response = getattr(client, method)(url)
     assert response.status_code == 401
diff --git a/api/tests/history/test_history.py b/api/tests/history/test_history.py
index ec8689e9..563cf2f0 100644
--- a/api/tests/history/test_history.py
+++ b/api/tests/history/test_history.py
@@ -14,8 +14,9 @@ def test_can_create_listening(factories):
     l = models.Listening.objects.create(user=user, track=track)
 
 
-def test_anonymous_user_can_create_listening_via_api(client, factories, settings):
-    settings.API_AUTHENTICATION_REQUIRED = False
+def test_anonymous_user_can_create_listening_via_api(
+        client, factories, preferences):
+    preferences['common__api_authentication_required'] = False
     track = factories['music.Track']()
     url = reverse('api:v1:history:listenings-list')
     response = client.post(url, {
diff --git a/api/tests/music/test_api.py b/api/tests/music/test_api.py
index cc6fe644..53ee29f3 100644
--- a/api/tests/music/test_api.py
+++ b/api/tests/music/test_api.py
@@ -265,16 +265,16 @@ def test_can_search_tracks(factories, logged_in_client):
     ('api:v1:albums-list', 'get'),
 ])
 def test_can_restrict_api_views_to_authenticated_users(
-        db, route, method, settings, client):
+        db, route, method, preferences, client):
     url = reverse(route)
-    settings.API_AUTHENTICATION_REQUIRED = True
+    preferences['common__api_authentication_required'] = True
     response = getattr(client, method)(url)
     assert response.status_code == 401
 
 
 def test_track_file_url_is_restricted_to_authenticated_users(
-        api_client, factories, settings):
-    settings.API_AUTHENTICATION_REQUIRED = True
+        api_client, factories, preferences):
+    preferences['common__api_authentication_required'] = True
     f = factories['music.TrackFile']()
     assert f.audio_file is not None
     url = f.path
@@ -283,8 +283,8 @@ def test_track_file_url_is_restricted_to_authenticated_users(
 
 
 def test_track_file_url_is_accessible_to_authenticated_users(
-        logged_in_api_client, factories, settings):
-    settings.API_AUTHENTICATION_REQUIRED = True
+        logged_in_api_client, factories, preferences):
+    preferences['common__api_authentication_required'] = True
     f = factories['music.TrackFile']()
     assert f.audio_file is not None
     url = f.path
diff --git a/api/tests/playlists/test_views.py b/api/tests/playlists/test_views.py
index f0fb6d0f..44d06082 100644
--- a/api/tests/playlists/test_views.py
+++ b/api/tests/playlists/test_views.py
@@ -107,8 +107,8 @@ def test_deleting_plt_updates_indexes(
 
 @pytest.mark.parametrize('level', ['instance', 'me', 'followers'])
 def test_playlist_privacy_respected_in_list_anon(
-        settings, level, factories, api_client):
-    settings.API_AUTHENTICATION_REQUIRED = False
+        preferences, level, factories, api_client):
+    preferences['common__api_authentication_required'] = False
     factories['playlists.Playlist'](privacy_level=level)
     url = reverse('api:v1:playlists-list')
     response = api_client.get(url)
@@ -137,8 +137,8 @@ def test_only_owner_can_edit_playlist_track(
 
 @pytest.mark.parametrize('level', ['instance', 'me', 'followers'])
 def test_playlist_track_privacy_respected_in_list_anon(
-        level, factories, api_client, settings):
-    settings.API_AUTHENTICATION_REQUIRED = False
+        level, factories, api_client, preferences):
+    preferences['common__api_authentication_required'] = False
     factories['playlists.PlaylistTrack'](playlist__privacy_level=level)
     url = reverse('api:v1:playlist-tracks-list')
     response = api_client.get(url)
diff --git a/api/tests/radios/test_radios.py b/api/tests/radios/test_radios.py
index c8038a4d..e78bd0e2 100644
--- a/api/tests/radios/test_radios.py
+++ b/api/tests/radios/test_radios.py
@@ -151,8 +151,8 @@ def test_can_start_radio_for_logged_in_user(logged_in_client):
     assert session.user == logged_in_client.user
 
 
-def test_can_start_radio_for_anonymous_user(api_client, db, settings):
-    settings.API_AUTHENTICATION_REQUIRED = False
+def test_can_start_radio_for_anonymous_user(api_client, db, preferences):
+    preferences['common__api_authentication_required'] = False
     url = reverse('api:v1:radios:sessions-list')
     response = api_client.post(url, {'radio_type': 'random'})
 
@@ -232,8 +232,8 @@ def test_can_start_tag_radio(factories):
         assert radio.pick() in good_tracks
 
 
-def test_can_start_artist_radio_from_api(api_client, settings, factories):
-    settings.API_AUTHENTICATION_REQUIRED = False
+def test_can_start_artist_radio_from_api(api_client, preferences, factories):
+    preferences['common__api_authentication_required'] = False
     artist = factories['music.Artist']()
     url = reverse('api:v1:radios:sessions-list')
 
diff --git a/api/tests/test_jwt_querystring.py b/api/tests/test_jwt_querystring.py
index bd07e1dc..f18e6b72 100644
--- a/api/tests/test_jwt_querystring.py
+++ b/api/tests/test_jwt_querystring.py
@@ -5,9 +5,10 @@ jwt_payload_handler = api_settings.JWT_PAYLOAD_HANDLER
 jwt_encode_handler = api_settings.JWT_ENCODE_HANDLER
 
 
-def test_can_authenticate_using_token_param_in_url(factories, settings, client):
+def test_can_authenticate_using_token_param_in_url(
+        factories, preferences, client):
     user = factories['users.User']()
-    settings.API_AUTHENTICATION_REQUIRED = True
+    preferences['common__api_authentication_required'] = True
     url = reverse('api:v1:tracks-list')
     response = client.get(url)
 
diff --git a/api/tests/test_youtube.py b/api/tests/test_youtube.py
index 44117909..7ab6256d 100644
--- a/api/tests/test_youtube.py
+++ b/api/tests/test_youtube.py
@@ -18,8 +18,8 @@ def test_can_get_search_results_from_youtube(mocker):
 
 
 def test_can_get_search_results_from_funkwhale(
-        settings, mocker, api_client, db):
-    settings.API_AUTHENTICATION_REQUIRED = False
+        preferences, mocker, api_client, db):
+    preferences['common__api_authentication_required'] = False
     mocker.patch(
         'funkwhale_api.providers.youtube.client._do_search',
         return_value=api_data.search['8 bit adventure'])
@@ -70,8 +70,8 @@ def test_can_send_multiple_queries_at_once(mocker):
 
 
 def test_can_send_multiple_queries_at_once_from_funwkhale(
-        settings, mocker, db, api_client):
-    settings.API_AUTHENTICATION_REQUIRED = False
+        preferences, mocker, db, api_client):
+    preferences['common__api_authentication_required'] = False
     mocker.patch(
         'funkwhale_api.providers.youtube.client._do_search',
         return_value=api_data.search['8 bit adventure'])
diff --git a/deploy/env.prod.sample b/deploy/env.prod.sample
index e357d08d..f1718ff7 100644
--- a/deploy/env.prod.sample
+++ b/deploy/env.prod.sample
@@ -88,9 +88,6 @@ DJANGO_SECRET_KEY=
 # want to
 # DJANGO_ADMIN_URL=^api/admin/
 
-# If True, unauthenticated users won't be able to query the API
-API_AUTHENTICATION_REQUIRED=True
-
 # Sentry/Raven error reporting (server side)
 # Enable Raven if you want to help improve funkwhale by
 # automatically sending error reports our Sentry instance.
-- 
GitLab