From aa3da412a7857775fce32cc7e131d4de37363ae0 Mon Sep 17 00:00:00 2001 From: Eliot Berriot Date: Sat, 28 Apr 2018 05:28:51 +0200 Subject: [PATCH 1/5] #186: common utils for moving settings to preferences --- api/funkwhale_api/common/preferences.py | 12 ++++++++++++ 1 file changed, 12 insertions(+) create mode 100644 api/funkwhale_api/common/preferences.py diff --git a/api/funkwhale_api/common/preferences.py b/api/funkwhale_api/common/preferences.py new file mode 100644 index 000000000..e6eb8beda --- /dev/null +++ b/api/funkwhale_api/common/preferences.py @@ -0,0 +1,12 @@ +from django.conf import settings +from dynamic_preferences.registries import global_preferences_registry + + +class DefaultFromSettingMixin(object): + def get_default(self): + return getattr(settings, self.setting) + + +def get(pref): + manager = global_preferences_registry.manager() + return manager[pref] From 7222f7b710836b391f2e2556165bb976de67bb7a Mon Sep 17 00:00:00 2001 From: Eliot Berriot Date: Sat, 28 Apr 2018 05:30:23 +0200 Subject: [PATCH 2/5] See #186: moved PLAYLISTS_MAX_TRACKS to playlists__max_tracks --- api/config/settings/common.py | 1 + .../playlists/dynamic_preferences_registry.py | 15 +++++++++++++++ api/funkwhale_api/playlists/models.py | 6 ++++-- api/funkwhale_api/playlists/serializers.py | 6 ++++-- api/tests/playlists/test_models.py | 4 ++-- api/tests/playlists/test_serializers.py | 4 ++-- 6 files changed, 28 insertions(+), 8 deletions(-) create mode 100644 api/funkwhale_api/playlists/dynamic_preferences_registry.py diff --git a/api/config/settings/common.py b/api/config/settings/common.py index 2b9580b04..7eb409830 100644 --- a/api/config/settings/common.py +++ b/api/config/settings/common.py @@ -433,6 +433,7 @@ ADMIN_URL = env('DJANGO_ADMIN_URL', default='^api/admin/') CSRF_USE_SESSIONS = True # Playlist settings +# XXX: Deprecated, use playlists__max_tracks instead PLAYLISTS_MAX_TRACKS = env.int('PLAYLISTS_MAX_TRACKS', default=250) ACCOUNT_USERNAME_BLACKLIST = [ diff --git a/api/funkwhale_api/playlists/dynamic_preferences_registry.py b/api/funkwhale_api/playlists/dynamic_preferences_registry.py new file mode 100644 index 000000000..21140fa14 --- /dev/null +++ b/api/funkwhale_api/playlists/dynamic_preferences_registry.py @@ -0,0 +1,15 @@ +from dynamic_preferences import types +from dynamic_preferences.registries import global_preferences_registry + +from funkwhale_api.common import preferences + +playlists = types.Section('playlists') + + +@global_preferences_registry.register +class MaxTracks(preferences.DefaultFromSettingMixin, types.IntegerPreference): + show_in_api = True + section = playlists + name = 'max_tracks' + verbose_name = 'Max tracks per playlist' + setting = 'PLAYLISTS_MAX_TRACKS' diff --git a/api/funkwhale_api/playlists/models.py b/api/funkwhale_api/playlists/models.py index 6bb8fe178..a208a5fd0 100644 --- a/api/funkwhale_api/playlists/models.py +++ b/api/funkwhale_api/playlists/models.py @@ -6,6 +6,7 @@ from django.utils import timezone from rest_framework import exceptions from funkwhale_api.common import fields +from funkwhale_api.common import preferences class Playlist(models.Model): @@ -81,10 +82,11 @@ class Playlist(models.Model): existing = self.playlist_tracks.select_for_update() now = timezone.now() total = existing.filter(index__isnull=False).count() - if existing.count() + len(tracks) > settings.PLAYLISTS_MAX_TRACKS: + max_tracks = preferences.get('playlists__max_tracks') + if existing.count() + len(tracks) > max_tracks: raise exceptions.ValidationError( 'Playlist would reach the maximum of {} tracks'.format( - settings.PLAYLISTS_MAX_TRACKS)) + max_tracks)) self.save(update_fields=['modification_date']) start = total plts = [ diff --git a/api/funkwhale_api/playlists/serializers.py b/api/funkwhale_api/playlists/serializers.py index 6caf9aa4a..fcb2a412d 100644 --- a/api/funkwhale_api/playlists/serializers.py +++ b/api/funkwhale_api/playlists/serializers.py @@ -3,6 +3,7 @@ from django.db import transaction from rest_framework import serializers from taggit.models import Tag +from funkwhale_api.common import preferences from funkwhale_api.music.models import Track from funkwhale_api.music.serializers import TrackSerializerNested from funkwhale_api.users.serializers import UserBasicSerializer @@ -32,10 +33,11 @@ class PlaylistTrackWriteSerializer(serializers.ModelSerializer): raise serializers.ValidationError( 'You do not have the permission to edit this playlist') existing = value.playlist_tracks.count() - if existing >= settings.PLAYLISTS_MAX_TRACKS: + max_tracks = preferences.get('playlists__max_tracks') + if existing >= max_tracks: raise serializers.ValidationError( 'Playlist has reached the maximum of {} tracks'.format( - settings.PLAYLISTS_MAX_TRACKS)) + max_tracks)) return value @transaction.atomic diff --git a/api/tests/playlists/test_models.py b/api/tests/playlists/test_models.py index c9def4dab..fe5dd40a8 100644 --- a/api/tests/playlists/test_models.py +++ b/api/tests/playlists/test_models.py @@ -116,8 +116,8 @@ def test_can_insert_many(factories): assert plt.playlist == playlist -def test_insert_many_honor_max_tracks(factories, settings): - settings.PLAYLISTS_MAX_TRACKS = 4 +def test_insert_many_honor_max_tracks(preferences, factories): + preferences['playlists__max_tracks'] = 4 playlist = factories['playlists.Playlist']() plts = factories['playlists.PlaylistTrack'].create_batch( size=2, playlist=playlist) diff --git a/api/tests/playlists/test_serializers.py b/api/tests/playlists/test_serializers.py index 8e30919e6..908c1c796 100644 --- a/api/tests/playlists/test_serializers.py +++ b/api/tests/playlists/test_serializers.py @@ -2,8 +2,8 @@ from funkwhale_api.playlists import models from funkwhale_api.playlists import serializers -def test_cannot_max_500_tracks_per_playlist(factories, settings): - settings.PLAYLISTS_MAX_TRACKS = 2 +def test_cannot_max_500_tracks_per_playlist(factories, preferences): + preferences['playlists__max_tracks'] = 2 playlist = factories['playlists.Playlist']() plts = factories['playlists.PlaylistTrack'].create_batch( size=2, playlist=playlist) From 6100b106c0df000501ce1070b380e8b73a3037a0 Mon Sep 17 00:00:00 2001 From: Eliot Berriot Date: Sat, 28 Apr 2018 05:55:21 +0200 Subject: [PATCH 3/5] See #186: moved federation settings to preferences --- api/funkwhale_api/federation/actors.py | 5 +- .../dynamic_preferences_registry.py | 51 +++++++++++++++++++ api/funkwhale_api/federation/permissions.py | 3 +- api/funkwhale_api/federation/views.py | 8 +-- api/funkwhale_api/music/tasks.py | 8 ++- api/tests/federation/test_actors.py | 22 ++++---- api/tests/federation/test_permissions.py | 16 +++--- api/tests/federation/test_views.py | 36 ++++++------- api/tests/music/test_import.py | 4 +- deploy/env.prod.sample | 9 ---- 10 files changed, 103 insertions(+), 59 deletions(-) diff --git a/api/funkwhale_api/federation/actors.py b/api/funkwhale_api/federation/actors.py index a0e23d12f..7a209b1ff 100644 --- a/api/funkwhale_api/federation/actors.py +++ b/api/funkwhale_api/federation/actors.py @@ -12,6 +12,7 @@ from rest_framework.exceptions import PermissionDenied from dynamic_preferences.registries import global_preferences_registry +from funkwhale_api.common import preferences from funkwhale_api.common import session from funkwhale_api.common import utils as funkwhale_utils from funkwhale_api.music import models as music_models @@ -55,7 +56,7 @@ def get_actor(actor_url): except models.Actor.DoesNotExist: actor = None fetch_delta = datetime.timedelta( - minutes=settings.FEDERATION_ACTOR_FETCH_DELAY) + minutes=preferences.get('federation__actor_fetch_delay')) if actor and actor.last_fetch_date > timezone.now() - fetch_delta: # cache is hot, we can return as is return actor @@ -225,7 +226,7 @@ class LibraryActor(SystemActor): @property def manually_approves_followers(self): - return settings.FEDERATION_MUSIC_NEEDS_APPROVAL + return preferences.get('federation__music_needs_approval') @transaction.atomic def handle_create(self, ac, sender): diff --git a/api/funkwhale_api/federation/dynamic_preferences_registry.py b/api/funkwhale_api/federation/dynamic_preferences_registry.py index 43877c75c..e86b9f6f2 100644 --- a/api/funkwhale_api/federation/dynamic_preferences_registry.py +++ b/api/funkwhale_api/federation/dynamic_preferences_registry.py @@ -3,6 +3,7 @@ from django.forms import widgets from dynamic_preferences import types from dynamic_preferences.registries import global_preferences_registry +from funkwhale_api.common import preferences federation = types.Section('federation') @@ -18,3 +19,53 @@ class MusicCacheDuration(types.IntPreference): 'locally? Federated files that were not listened in this interval ' 'will be erased and refetched from the remote on the next listening.' ) + + +@global_preferences_registry.register +class Enabled(preferences.DefaultFromSettingMixin, types.BooleanPreference): + section = federation + name = 'enabled' + setting = 'FEDERATION_ENABLED' + verbose_name = 'Federation enabled' + help_text = ( + 'Use this setting to enable or disable federation logic and API' + ' globally' + ) + + +@global_preferences_registry.register +class CollectionPageSize( + preferences.DefaultFromSettingMixin, types.IntPreference): + section = federation + name = 'collection_page_size' + setting = 'FEDERATION_COLLECTION_PAGE_SIZE' + verbose_name = 'Federation collection page size' + help_text = ( + 'How much items to display in ActivityPub collections' + ) + + +@global_preferences_registry.register +class ActorFetchDelay( + preferences.DefaultFromSettingMixin, types.IntPreference): + section = federation + name = 'actor_fetch_delay' + setting = 'FEDERATION_ACTOR_FETCH_DELAY' + verbose_name = 'Federation actor fetch delay' + help_text = ( + 'How much minutes to wait before refetching actors on ' + 'request authentication' + ) + + +@global_preferences_registry.register +class MusicNeedsApproval( + preferences.DefaultFromSettingMixin, types.BooleanPreference): + section = federation + name = 'music_needs_approval' + setting = 'FEDERATION_MUSIC_NEEDS_APPROVAL' + verbose_name = 'Federation music needs approval' + help_text = ( + 'When true, other federation actors will require your approval' + ' before being able to browse your library.' + ) diff --git a/api/funkwhale_api/federation/permissions.py b/api/funkwhale_api/federation/permissions.py index c6f0660b1..438b675cb 100644 --- a/api/funkwhale_api/federation/permissions.py +++ b/api/funkwhale_api/federation/permissions.py @@ -2,13 +2,14 @@ from django.conf import settings from rest_framework.permissions import BasePermission +from funkwhale_api.common import preferences from . import actors class LibraryFollower(BasePermission): def has_permission(self, request, view): - if not settings.FEDERATION_MUSIC_NEEDS_APPROVAL: + if not preferences.get('federation__music_needs_approval'): return True actor = getattr(request, 'actor', None) diff --git a/api/funkwhale_api/federation/views.py b/api/funkwhale_api/federation/views.py index 381f87eff..9b51a534d 100644 --- a/api/funkwhale_api/federation/views.py +++ b/api/funkwhale_api/federation/views.py @@ -13,6 +13,7 @@ from rest_framework import viewsets from rest_framework.decorators import list_route, detail_route from rest_framework.serializers import ValidationError +from funkwhale_api.common import preferences from funkwhale_api.common import utils as funkwhale_utils from funkwhale_api.common.permissions import HasModelPermission from funkwhale_api.music.models import TrackFile @@ -33,7 +34,7 @@ from . import webfinger class FederationMixin(object): def dispatch(self, request, *args, **kwargs): - if not settings.FEDERATION_ENABLED: + if not preferences.get('federation__enabled'): return HttpResponse(status=405) return super().dispatch(request, *args, **kwargs) @@ -136,7 +137,8 @@ class MusicFilesViewSet(FederationMixin, viewsets.GenericViewSet): if page is None: conf = { 'id': utils.full_url(reverse('federation:music:files-list')), - 'page_size': settings.FEDERATION_COLLECTION_PAGE_SIZE, + 'page_size': preferences.get( + 'federation__collection_page_size'), 'items': qs, 'item_serializer': serializers.AudioSerializer, 'actor': library, @@ -150,7 +152,7 @@ class MusicFilesViewSet(FederationMixin, viewsets.GenericViewSet): return response.Response( {'page': ['Invalid page number']}, status=400) p = paginator.Paginator( - qs, settings.FEDERATION_COLLECTION_PAGE_SIZE) + qs, preferences.get('federation__collection_page_size')) try: page = p.page(page_number) conf = { diff --git a/api/funkwhale_api/music/tasks.py b/api/funkwhale_api/music/tasks.py index aaaa2cdca..4509c9a57 100644 --- a/api/funkwhale_api/music/tasks.py +++ b/api/funkwhale_api/music/tasks.py @@ -2,8 +2,7 @@ import os from django.core.files.base import ContentFile -from dynamic_preferences.registries import global_preferences_registry - +from funkwhale_api.common import preferences from funkwhale_api.federation import activity from funkwhale_api.federation import actors from funkwhale_api.federation import models as federation_models @@ -80,8 +79,7 @@ def _do_import(import_job, replace=False, use_acoustid=True): acoustid_track_id = None duration = None track = None - manager = global_preferences_registry.manager() - use_acoustid = use_acoustid and manager['providers_acoustid__api_key'] + use_acoustid = use_acoustid and preferences.get('providers_acoustid__api_key') if not mbid and use_acoustid and from_file: # we try to deduce mbid from acoustid client = get_acoustid_client() @@ -185,7 +183,7 @@ def fetch_content(lyrics): @celery.require_instance( models.ImportBatch.objects.filter(status='finished'), 'import_batch') def import_batch_notify_followers(import_batch): - if not settings.FEDERATION_ENABLED: + if not preferences.get('federation__enabled'): return if import_batch.source == 'federation': diff --git a/api/tests/federation/test_actors.py b/api/tests/federation/test_actors.py index f566e7a72..6f73a9b9b 100644 --- a/api/tests/federation/test_actors.py +++ b/api/tests/federation/test_actors.py @@ -39,8 +39,8 @@ def test_get_actor(factories, r_mock): assert serializers.ActorSerializer(new_actor).data == payload -def test_get_actor_use_existing(factories, settings, mocker): - settings.FEDERATION_ACTOR_FETCH_DELAY = 60 +def test_get_actor_use_existing(factories, preferences, mocker): + preferences['federation__actor_fetch_delay'] = 60 actor = factories['federation.Actor']() get_data = mocker.patch('funkwhale_api.federation.actors.get_actor_data') new_actor = actors.get_actor(actor.url) @@ -49,8 +49,8 @@ def test_get_actor_use_existing(factories, settings, mocker): get_data.assert_not_called() -def test_get_actor_refresh(factories, settings, mocker): - settings.FEDERATION_ACTOR_FETCH_DELAY = 0 +def test_get_actor_refresh(factories, preferences, mocker): + preferences['federation__actor_fetch_delay'] = 0 actor = factories['federation.Actor']() payload = serializers.ActorSerializer(actor).data # actor changed their username in the meantime @@ -274,9 +274,9 @@ def test_actor_is_system( @pytest.mark.parametrize('value', [False, True]) -def test_library_actor_manually_approves_based_on_setting( - value, settings): - settings.FEDERATION_MUSIC_NEEDS_APPROVAL = value +def test_library_actor_manually_approves_based_on_preference( + value, preferences): + preferences['federation__music_needs_approval'] = value library_conf = actors.SYSTEM_ACTORS['library'] assert library_conf.manually_approves_followers is value @@ -356,8 +356,8 @@ def test_test_actor_handles_undo_follow( def test_library_actor_handles_follow_manual_approval( - settings, mocker, factories): - settings.FEDERATION_MUSIC_NEEDS_APPROVAL = True + preferences, mocker, factories): + preferences['federation__music_needs_approval'] = True actor = factories['federation.Actor']() now = timezone.now() mocker.patch('django.utils.timezone.now', return_value=now) @@ -377,8 +377,8 @@ def test_library_actor_handles_follow_manual_approval( def test_library_actor_handles_follow_auto_approval( - settings, mocker, factories): - settings.FEDERATION_MUSIC_NEEDS_APPROVAL = False + preferences, mocker, factories): + preferences['federation__music_needs_approval'] = False actor = factories['federation.Actor']() accept_follow = mocker.patch( 'funkwhale_api.federation.activity.accept_follow') diff --git a/api/tests/federation/test_permissions.py b/api/tests/federation/test_permissions.py index 9b8683210..a87f26f1b 100644 --- a/api/tests/federation/test_permissions.py +++ b/api/tests/federation/test_permissions.py @@ -5,8 +5,8 @@ from funkwhale_api.federation import permissions def test_library_follower( - factories, api_request, anonymous_user, settings): - settings.FEDERATION_MUSIC_NEEDS_APPROVAL = True + factories, api_request, anonymous_user, preferences): + preferences['federation__music_needs_approval'] = True view = APIView.as_view() permission = permissions.LibraryFollower() request = api_request.get('/') @@ -17,8 +17,8 @@ def test_library_follower( def test_library_follower_actor_non_follower( - factories, api_request, anonymous_user, settings): - settings.FEDERATION_MUSIC_NEEDS_APPROVAL = True + factories, api_request, anonymous_user, preferences): + preferences['federation__music_needs_approval'] = True actor = factories['federation.Actor']() view = APIView.as_view() permission = permissions.LibraryFollower() @@ -31,8 +31,8 @@ def test_library_follower_actor_non_follower( def test_library_follower_actor_follower_not_approved( - factories, api_request, anonymous_user, settings): - settings.FEDERATION_MUSIC_NEEDS_APPROVAL = True + factories, api_request, anonymous_user, preferences): + preferences['federation__music_needs_approval'] = True library = actors.SYSTEM_ACTORS['library'].get_actor_instance() follow = factories['federation.Follow'](target=library, approved=False) view = APIView.as_view() @@ -46,8 +46,8 @@ def test_library_follower_actor_follower_not_approved( def test_library_follower_actor_follower( - factories, api_request, anonymous_user, settings): - settings.FEDERATION_MUSIC_NEEDS_APPROVAL = True + factories, api_request, anonymous_user, preferences): + preferences['federation__music_needs_approval'] = True library = actors.SYSTEM_ACTORS['library'].get_actor_instance() follow = factories['federation.Follow'](target=library, approved=True) view = APIView.as_view() diff --git a/api/tests/federation/test_views.py b/api/tests/federation/test_views.py index ae94bcdc0..09ecfc8ff 100644 --- a/api/tests/federation/test_views.py +++ b/api/tests/federation/test_views.py @@ -13,7 +13,7 @@ from funkwhale_api.federation import webfinger @pytest.mark.parametrize('system_actor', actors.SYSTEM_ACTORS.keys()) -def test_instance_actors(system_actor, db, settings, api_client): +def test_instance_actors(system_actor, db, api_client): actor = actors.SYSTEM_ACTORS[system_actor].get_actor_instance() url = reverse( 'federation:instance-actors-detail', @@ -34,8 +34,8 @@ def test_instance_actors(system_actor, db, settings, api_client): ('well-known-webfinger', {}), ]) def test_instance_endpoints_405_if_federation_disabled( - authenticated_actor, db, settings, api_client, route, kwargs): - settings.FEDERATION_ENABLED = False + authenticated_actor, db, preferences, api_client, route, kwargs): + preferences['federation__enabled'] = False url = reverse('federation:{}'.format(route), kwargs=kwargs) response = api_client.get(url) @@ -71,8 +71,8 @@ def test_wellknown_webfinger_system( def test_audio_file_list_requires_authenticated_actor( - db, settings, api_client): - settings.FEDERATION_MUSIC_NEEDS_APPROVAL = True + db, preferences, api_client): + preferences['federation__music_needs_approval'] = True url = reverse('federation:music:files-list') response = api_client.get(url) @@ -80,9 +80,9 @@ def test_audio_file_list_requires_authenticated_actor( def test_audio_file_list_actor_no_page( - db, settings, api_client, factories): - settings.FEDERATION_MUSIC_NEEDS_APPROVAL = False - settings.FEDERATION_COLLECTION_PAGE_SIZE = 2 + db, preferences, api_client, factories): + preferences['federation__music_needs_approval'] = False + preferences['federation__collection_page_size'] = 2 library = actors.SYSTEM_ACTORS['library'].get_actor_instance() tfs = factories['music.TrackFile'].create_batch(size=5) conf = { @@ -101,9 +101,9 @@ def test_audio_file_list_actor_no_page( def test_audio_file_list_actor_page( - db, settings, api_client, factories): - settings.FEDERATION_MUSIC_NEEDS_APPROVAL = False - settings.FEDERATION_COLLECTION_PAGE_SIZE = 2 + db, preferences, api_client, factories): + preferences['federation__music_needs_approval'] = False + preferences['federation__collection_page_size'] = 2 library = actors.SYSTEM_ACTORS['library'].get_actor_instance() tfs = factories['music.TrackFile'].create_batch(size=5) conf = { @@ -121,8 +121,8 @@ def test_audio_file_list_actor_page( def test_audio_file_list_actor_page_exclude_federated_files( - db, settings, api_client, factories): - settings.FEDERATION_MUSIC_NEEDS_APPROVAL = False + db, preferences, api_client, factories): + preferences['federation__music_needs_approval'] = False library = actors.SYSTEM_ACTORS['library'].get_actor_instance() tfs = factories['music.TrackFile'].create_batch(size=5, federation=True) @@ -134,8 +134,8 @@ def test_audio_file_list_actor_page_exclude_federated_files( def test_audio_file_list_actor_page_error( - db, settings, api_client, factories): - settings.FEDERATION_MUSIC_NEEDS_APPROVAL = False + db, preferences, api_client, factories): + preferences['federation__music_needs_approval'] = False url = reverse('federation:music:files-list') response = api_client.get(url, data={'page': 'nope'}) @@ -143,15 +143,15 @@ def test_audio_file_list_actor_page_error( def test_audio_file_list_actor_page_error_too_far( - db, settings, api_client, factories): - settings.FEDERATION_MUSIC_NEEDS_APPROVAL = False + db, preferences, api_client, factories): + preferences['federation__music_needs_approval'] = False url = reverse('federation:music:files-list') response = api_client.get(url, data={'page': 5000}) assert response.status_code == 404 -def test_library_actor_includes_library_link(db, settings, api_client): +def test_library_actor_includes_library_link(db, preferences, api_client): actor = actors.SYSTEM_ACTORS['library'].get_actor_instance() url = reverse( 'federation:instance-actors-detail', diff --git a/api/tests/music/test_import.py b/api/tests/music/test_import.py index fa1c98eb4..000e6a8b6 100644 --- a/api/tests/music/test_import.py +++ b/api/tests/music/test_import.py @@ -169,10 +169,10 @@ def test_import_job_run_triggers_notifies_followers( def test_import_batch_notifies_followers_skip_on_disabled_federation( - settings, factories, mocker): + preferences, factories, mocker): mocked_deliver = mocker.patch('funkwhale_api.federation.activity.deliver') batch = factories['music.ImportBatch'](finished=True) - settings.FEDERATION_ENABLED = False + preferences['federation__enabled'] = False tasks.import_batch_notify_followers(import_batch_id=batch.pk) mocked_deliver.assert_not_called() diff --git a/deploy/env.prod.sample b/deploy/env.prod.sample index 54f2e1ef0..e357d08d3 100644 --- a/deploy/env.prod.sample +++ b/deploy/env.prod.sample @@ -98,15 +98,6 @@ API_AUTHENTICATION_REQUIRED=True RAVEN_ENABLED=false RAVEN_DSN=https://44332e9fdd3d42879c7d35bf8562c6a4:0062dc16a22b41679cd5765e5342f716@sentry.eliotberriot.com/5 -# This settings enable/disable federation on the instance level -FEDERATION_ENABLED=True -# This setting decide wether music library is shared automatically -# to followers or if it requires manual approval before. -# FEDERATION_MUSIC_NEEDS_APPROVAL=False -# means anyone can subscribe to your library and import your file, -# use with caution. -FEDERATION_MUSIC_NEEDS_APPROVAL=True - # In-place import settings # You can safely leave those settings uncommented if you don't plan to use # in place imports. From a3b2125d2a62c4c735f829e932ce2cb2a6c57b41 Mon Sep 17 00:00:00 2001 From: Eliot Berriot Date: Sat, 28 Apr 2018 06:11:50 +0200 Subject: [PATCH 4/5] 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 7eb409830..d29480648 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 000000000..2374de7c7 --- /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 c99c275c1..cab4b699d 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 61fc65beb..77f95c477 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 bdc3c6339..9b24f3ad3 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 f4a045af8..591fe7c9c 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 ec8689e96..563cf2f08 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 cc6fe644b..53ee29f3e 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 f0fb6d0fd..44d060821 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 c8038a4db..e78bd0e2f 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 bd07e1dc3..f18e6b729 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 441179095..7ab6256da 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 e357d08d3..f1718ff7e 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. From 7ad21b7d25608eec9703fdf1d2c575247d2b4f2c Mon Sep 17 00:00:00 2001 From: Eliot Berriot Date: Sat, 28 Apr 2018 06:24:44 +0200 Subject: [PATCH 5/5] Fix #186: moved high-level settings to database preferences --- .env.dev | 1 - api/tests/music/test_permissions.py | 2 +- changes/changelog.d/186.enhancement | 24 ++++++++++++++++++++++++ docs/configuration.rst | 2 ++ docs/federation.rst | 5 ++--- 5 files changed, 29 insertions(+), 5 deletions(-) create mode 100644 changes/changelog.d/186.enhancement diff --git a/.env.dev b/.env.dev index c09262509..d9e2dd3ce 100644 --- a/.env.dev +++ b/.env.dev @@ -1,4 +1,3 @@ -API_AUTHENTICATION_REQUIRED=True RAVEN_ENABLED=false RAVEN_DSN=https://44332e9fdd3d42879c7d35bf8562c6a4:0062dc16a22b41679cd5765e5342f716@sentry.eliotberriot.com/5 DJANGO_ALLOWED_HOSTS=.funkwhale.test,localhost,nginx,0.0.0.0,127.0.0.1 diff --git a/api/tests/music/test_permissions.py b/api/tests/music/test_permissions.py index d36f37886..a5f0c4109 100644 --- a/api/tests/music/test_permissions.py +++ b/api/tests/music/test_permissions.py @@ -13,7 +13,7 @@ def test_list_permission_no_protect(anonymous_user, api_request, settings): def test_list_permission_protect_anonymous( - anonymous_user, api_request, settings): + db, anonymous_user, api_request, settings): settings.PROTECT_AUDIO_FILES = True view = APIView.as_view() permission = permissions.Listen() diff --git a/changes/changelog.d/186.enhancement b/changes/changelog.d/186.enhancement new file mode 100644 index 000000000..36777c786 --- /dev/null +++ b/changes/changelog.d/186.enhancement @@ -0,0 +1,24 @@ +Store high-level settings (such as federation or auth-related ones) in database (#186) + +Changelog +^^^^^^^^^ +Due to the work done in #186, the following environment variables have been +deprecated: + +- FEDERATION_ENABLED +- FEDERATION_COLLECTION_PAGE_SIZE +- FEDERATION_MUSIC_NEEDS_APPROVAL +- FEDERATION_ACTOR_FETCH_DELAY +- PLAYLISTS_MAX_TRACKS +- API_AUTHENTICATION_REQUIRED + +Configuration for this settings has been moved to database, as it will provide +a better user-experience, by allowing you to edit these values on-the-fly, +without restarting Funkwhale processes. + +You can leave those environment variables in your .env file for now, as the +values will be used to populate the database entries. We'll make a proper +announcement when the variables won't be used anymore. + +Please browse https://docs.funkwhale.audio/configuration.html#instance-settings +for more information about instance configuration using the web interface. diff --git a/docs/configuration.rst b/docs/configuration.rst index c0de76f56..1c89feeb8 100644 --- a/docs/configuration.rst +++ b/docs/configuration.rst @@ -18,6 +18,8 @@ and technical aspects of your instance, such as database credentials. on environment variables. +.. _instance-settings: + Instance settings ----------------- diff --git a/docs/federation.rst b/docs/federation.rst index 5b030074c..0f016ada9 100644 --- a/docs/federation.rst +++ b/docs/federation.rst @@ -12,8 +12,7 @@ Managing federation Federation management is only available to instance admins and users who have the proper permissions. You can disable federation completely -at the instance level by setting the FEDERATION_ENABLED environment variable -to False. +at the instance level by editing the ``federation__enabled`` :ref:`setting `. On the front end, assuming you have the proper permission, you will see a "Federation" link in the sidebar. @@ -52,6 +51,6 @@ each other instance asking for access to library. This is by design, to ensure your library is not shared publicly without your consent. However, if you're confident about federating publicly without manual approval, -you can set the FEDERATION_MUSIC_NEEDS_APPROVAL environment variable to false. +you can set the ``federation__music_needs_approval`` :ref:`setting ` to false. Follow requests will be accepted automatically and followers given access to your library without manual intervention.