From 023c6f6f5e1355977d441c8c2cce68044dee6e91 Mon Sep 17 00:00:00 2001 From: David Magnus Henriques Date: Thu, 10 May 2018 13:24:26 +0200 Subject: [PATCH 01/38] Encode the Filepath utf-8 The filepath of a track gets broken when it contains special UTF-8-characters. Encoding it UTF-8 fixes this behaviour. --- api/funkwhale_api/music/views.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/api/funkwhale_api/music/views.py b/api/funkwhale_api/music/views.py index 98274e741..14900ca98 100644 --- a/api/funkwhale_api/music/views.py +++ b/api/funkwhale_api/music/views.py @@ -230,7 +230,7 @@ def get_file_path(audio_file): 'MUSIC_DIRECTORY_PATH to serve in-place imported files' ) path = '/music' + audio_file.replace(prefix, '', 1) - return settings.PROTECT_FILES_PATH + path + return (settings.PROTECT_FILES_PATH + path).encode('utf-8') if t == 'apache2': try: path = audio_file.path From d21a9616f9527ad28ae8a42864dc46036e02985b Mon Sep 17 00:00:00 2001 From: David Magnus Henriques Date: Thu, 10 May 2018 13:28:16 +0200 Subject: [PATCH 02/38] Changelog contains explanation of fix --- changes/changelog.d/196.bug | 1 + 1 file changed, 1 insertion(+) create mode 100644 changes/changelog.d/196.bug diff --git a/changes/changelog.d/196.bug b/changes/changelog.d/196.bug new file mode 100644 index 000000000..6f59a28d1 --- /dev/null +++ b/changes/changelog.d/196.bug @@ -0,0 +1 @@ +Tracks with special UTF-8 characters don't break anymore, fixing Bug #196 From 190a4357dc9aa386299074f073ceac97b5f69a78 Mon Sep 17 00:00:00 2001 From: Eliot Berriot Date: Thu, 10 May 2018 16:45:45 +0200 Subject: [PATCH 03/38] Fix #198: Removed Python 3.6 dependency (secrets module) --- api/funkwhale_api/users/models.py | 9 +++++++-- changes/changelog.d/198.bugfix | 1 + 2 files changed, 8 insertions(+), 2 deletions(-) create mode 100644 changes/changelog.d/198.bugfix diff --git a/api/funkwhale_api/users/models.py b/api/funkwhale_api/users/models.py index 773d60f38..f067a2a8b 100644 --- a/api/funkwhale_api/users/models.py +++ b/api/funkwhale_api/users/models.py @@ -1,8 +1,9 @@ # -*- coding: utf-8 -*- from __future__ import unicode_literals, absolute_import +import binascii +import os import uuid -import secrets from django.conf import settings from django.contrib.auth.models import AbstractUser @@ -14,6 +15,10 @@ from django.utils.translation import ugettext_lazy as _ from funkwhale_api.common import fields +def get_token(): + return binascii.b2a_hex(os.urandom(15)).decode('utf-8') + + @python_2_unicode_compatible class User(AbstractUser): @@ -58,7 +63,7 @@ class User(AbstractUser): return self.secret_key def update_subsonic_api_token(self): - self.subsonic_api_token = secrets.token_hex(32) + self.subsonic_api_token = get_token() return self.subsonic_api_token def set_password(self, raw_password): diff --git a/changes/changelog.d/198.bugfix b/changes/changelog.d/198.bugfix new file mode 100644 index 000000000..dd2f4e8fc --- /dev/null +++ b/changes/changelog.d/198.bugfix @@ -0,0 +1 @@ +Removed Python 3.6 dependency (secrets module) (#198) From d8cbed5a4b7b7e391a4e244585c962259abf11f0 Mon Sep 17 00:00:00 2001 From: Eliot Berriot Date: Thu, 10 May 2018 17:33:07 +0200 Subject: [PATCH 04/38] Fixed syntax error --- CHANGELOG | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/CHANGELOG b/CHANGELOG index ba9b9f1ae..a8d5d23de 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -110,9 +110,7 @@ We offer two settings to manage nodeinfo in your Funkwhale instance: and user activity. To make your instance fully compatible with the nodeinfo protocol, you need to -to edit your nginx configuration file: - -.. code-block:: +to edit your nginx configuration file:: # before ... @@ -130,9 +128,7 @@ to edit your nginx configuration file: } ... -You can do the same if you use apache: - -.. code-block:: +You can do the same if you use apache:: # before ... From 1937b8169911c60827cc65cb9f18938b80e0fb6c Mon Sep 17 00:00:00 2001 From: Eliot Berriot Date: Thu, 10 May 2018 18:52:00 +0200 Subject: [PATCH 05/38] Fix #196: In-place imported tracks non-ascii characters don't break reverse-proxy serving --- api/funkwhale_api/music/views.py | 2 +- api/tests/music/test_views.py | 18 ++++++++++++++++++ changes/changelog.d/196.bug | 1 - changes/changelog.d/196.bugfix | 1 + 4 files changed, 20 insertions(+), 2 deletions(-) delete mode 100644 changes/changelog.d/196.bug create mode 100644 changes/changelog.d/196.bugfix diff --git a/api/funkwhale_api/music/views.py b/api/funkwhale_api/music/views.py index 14900ca98..b104bf389 100644 --- a/api/funkwhale_api/music/views.py +++ b/api/funkwhale_api/music/views.py @@ -241,7 +241,7 @@ def get_file_path(audio_file): 'You need to specify MUSIC_DIRECTORY_SERVE_PATH and ' 'MUSIC_DIRECTORY_PATH to serve in-place imported files' ) - path = audio_file.replace(prefix, serve_path, 1) + path = audio_file.replace(prefix, serve_path, 1).encode('utf-8') return path diff --git a/api/tests/music/test_views.py b/api/tests/music/test_views.py index b22ab7fd5..e641b45d5 100644 --- a/api/tests/music/test_views.py +++ b/api/tests/music/test_views.py @@ -104,6 +104,24 @@ def test_serve_file_in_place( assert response[headers[proxy]] == expected +@pytest.mark.parametrize('proxy,serve_path,expected', [ + ('apache2', '/host/music', '/host/music/hello/worldéà.mp3'), + ('apache2', '/app/music', '/app/music/hello/worldéà.mp3'), + ('nginx', '/host/music', '/_protected/music/hello/worldéà.mp3'), + ('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 + settings.PROTECT_FILE_PATH = '/_protected/music' + settings.REVERSE_PROXY_TYPE = proxy + settings.MUSIC_DIRECTORY_PATH = '/app/music' + settings.MUSIC_DIRECTORY_SERVE_PATH = serve_path + path = views.get_file_path('/app/music/hello/worldéà.mp3') + + assert path == expected.encode('utf-8') + + @pytest.mark.parametrize('proxy,serve_path,expected', [ ('apache2', '/host/music', '/host/media/tracks/hello/world.mp3'), # apache with container not supported yet diff --git a/changes/changelog.d/196.bug b/changes/changelog.d/196.bug deleted file mode 100644 index 6f59a28d1..000000000 --- a/changes/changelog.d/196.bug +++ /dev/null @@ -1 +0,0 @@ -Tracks with special UTF-8 characters don't break anymore, fixing Bug #196 diff --git a/changes/changelog.d/196.bugfix b/changes/changelog.d/196.bugfix new file mode 100644 index 000000000..655df7d79 --- /dev/null +++ b/changes/changelog.d/196.bugfix @@ -0,0 +1 @@ +In-place imported tracks non-ascii characters don't break reverse-proxy serving (#196) From 6f79dd475dfaf9882d86fc2ac0b835b81d47960a Mon Sep 17 00:00:00 2001 From: Eliot Berriot Date: Thu, 10 May 2018 17:31:49 +0200 Subject: [PATCH 06/38] Now return proper error payload on subsonic API --- api/funkwhale_api/subsonic/renderers.py | 6 ++++ api/funkwhale_api/subsonic/views.py | 39 +++++++++++++++---------- 2 files changed, 29 insertions(+), 16 deletions(-) diff --git a/api/funkwhale_api/subsonic/renderers.py b/api/funkwhale_api/subsonic/renderers.py index 74cf13d88..3a5664501 100644 --- a/api/funkwhale_api/subsonic/renderers.py +++ b/api/funkwhale_api/subsonic/renderers.py @@ -15,6 +15,9 @@ class SubsonicJSONRenderer(renderers.JSONRenderer): } } final['subsonic-response'].update(data) + if 'error' in final: + # an error was returned + final['subsonic-response']['status'] = 'failed' return super().render(final, accepted_media_type, renderer_context) @@ -31,6 +34,9 @@ class SubsonicXMLRenderer(renderers.JSONRenderer): 'version': '1.16.0', } final.update(data) + if 'error' in final: + # an error was returned + final['status'] = 'failed' tree = dict_to_xml_tree('subsonic-response', final) return b'\n' + ET.tostring(tree, encoding='utf-8') diff --git a/api/funkwhale_api/subsonic/views.py b/api/funkwhale_api/subsonic/views.py index 475e61aa7..2692a3dda 100644 --- a/api/funkwhale_api/subsonic/views.py +++ b/api/funkwhale_api/subsonic/views.py @@ -31,15 +31,19 @@ def find_object(queryset, model_field='pk', field='id', cast=int): raw_value = data[field] except KeyError: return response.Response({ - 'code': 10, - 'message': "required parameter '{}' not present".format(field) + 'error': { + 'code': 10, + 'message': "required parameter '{}' not present".format(field) + } }) try: value = cast(raw_value) except (TypeError, ValidationError): return response.Response({ - 'code': 0, - 'message': 'For input string "{}"'.format(raw_value) + 'error': { + 'code': 0, + 'message': 'For input string "{}"'.format(raw_value) + } }) qs = queryset if hasattr(qs, '__call__'): @@ -48,9 +52,11 @@ def find_object(queryset, model_field='pk', field='id', cast=int): obj = qs.get(**{model_field: value}) except qs.model.DoesNotExist: return response.Response({ - 'code': 70, - 'message': '{} not found'.format( - qs.model.__class__.__name__) + 'error': { + 'code': 70, + 'message': '{} not found'.format( + qs.model.__class__.__name__) + } }) kwargs['obj'] = obj return func(self, request, *args, **kwargs) @@ -83,15 +89,14 @@ class SubsonicViewSet(viewsets.GenericViewSet): payload = { 'status': 'failed' } - try: + if exc.__class__ in mapping: code, message = mapping[exc.__class__] - except KeyError: - return super().handle_exception(exc) else: - payload['error'] = { - 'code': code, - 'message': message - } + return super().handle_exception(exc) + payload['error'] = { + 'code': code, + 'message': message + } return response.Response(payload, status=200) @@ -450,8 +455,10 @@ class SubsonicViewSet(viewsets.GenericViewSet): name = data.get('name', '') if not name: return response.Response({ - 'code': 10, - 'message': 'Playlist ID or name must be specified.' + 'error': { + 'code': 10, + 'message': 'Playlist ID or name must be specified.' + } }, data) playlist = request.user.playlists.create( From 2f5a13a339ef30e5ab06008edc9856638178ac67 Mon Sep 17 00:00:00 2001 From: Eliot Berriot Date: Thu, 10 May 2018 17:52:13 +0200 Subject: [PATCH 07/38] Fix #199: unplayable tracks are now properly disabled in the interface --- api/funkwhale_api/music/admin.py | 8 +++++++- changes/changelog.d/199.bugfix | 1 + front/src/components/audio/PlayButton.vue | 20 +++++++++++++++----- 3 files changed, 23 insertions(+), 6 deletions(-) create mode 100644 changes/changelog.d/199.bugfix diff --git a/api/funkwhale_api/music/admin.py b/api/funkwhale_api/music/admin.py index 219b40a91..667a7c2a1 100644 --- a/api/funkwhale_api/music/admin.py +++ b/api/funkwhale_api/music/admin.py @@ -38,6 +38,7 @@ class ImportBatchAdmin(admin.ModelAdmin): search_fields = [ 'import_request__name', 'source', 'batch__pk', 'mbid'] + @admin.register(models.ImportJob) class ImportJobAdmin(admin.ModelAdmin): list_display = ['source', 'batch', 'track_file', 'status', 'mbid'] @@ -77,5 +78,10 @@ class TrackFileAdmin(admin.ModelAdmin): list_select_related = [ 'track' ] - search_fields = ['source', 'acoustid_track_id'] + search_fields = [ + 'source', + 'acoustid_track_id', + 'track__title', + 'track__album__title', + 'track__artist__name'] list_filter = ['mimetype'] diff --git a/changes/changelog.d/199.bugfix b/changes/changelog.d/199.bugfix new file mode 100644 index 000000000..0dff4f9fe --- /dev/null +++ b/changes/changelog.d/199.bugfix @@ -0,0 +1 @@ +Uplayable tracks are now properly disabled in the interface (#199) diff --git a/front/src/components/audio/PlayButton.vue b/front/src/components/audio/PlayButton.vue index 2662f30b3..efa59a29d 100644 --- a/front/src/components/audio/PlayButton.vue +++ b/front/src/components/audio/PlayButton.vue @@ -1,8 +1,9 @@ + + + diff --git a/front/src/router/index.js b/front/src/router/index.js index b1e208023..f71dab7f9 100644 --- a/front/src/router/index.js +++ b/front/src/router/index.js @@ -28,6 +28,7 @@ import RequestsList from '@/components/requests/RequestsList' import PlaylistDetail from '@/views/playlists/Detail' import PlaylistList from '@/views/playlists/List' import Favorites from '@/components/favorites/List' +import AdminSettings from '@/views/admin/Settings' import FederationBase from '@/views/federation/Base' import FederationScan from '@/views/federation/Scan' import FederationLibraryDetail from '@/views/federation/LibraryDetail' @@ -117,6 +118,11 @@ export default new Router({ defaultPaginateBy: route.query.paginateBy }) }, + { + path: '/manage/settings', + name: 'manage.settings', + component: AdminSettings + }, { path: '/manage/federation', component: FederationBase, diff --git a/front/src/views/admin/Settings.vue b/front/src/views/admin/Settings.vue new file mode 100644 index 000000000..7174ab516 --- /dev/null +++ b/front/src/views/admin/Settings.vue @@ -0,0 +1,155 @@ + + + diff --git a/front/src/views/playlists/List.vue b/front/src/views/playlists/List.vue index 32ee5aafa..5001fb14d 100644 --- a/front/src/views/playlists/List.vue +++ b/front/src/views/playlists/List.vue @@ -76,7 +76,6 @@ export default { Pagination }, data () { - console.log('YOLO', this.$t) let defaultOrdering = this.getOrderingFromString(this.defaultOrdering || '-creation_date') return { isLoading: true, From f3ee282fb2fd2830929ea40b2d41b883df77cc4c Mon Sep 17 00:00:00 2001 From: Eliot Berriot Date: Thu, 17 May 2018 23:46:55 +0200 Subject: [PATCH 29/38] See #206: updated documentation --- docs/configuration.rst | 17 +++++++++++++---- 1 file changed, 13 insertions(+), 4 deletions(-) diff --git a/docs/configuration.rst b/docs/configuration.rst index bbc658e08..b7df2db42 100644 --- a/docs/configuration.rst +++ b/docs/configuration.rst @@ -27,15 +27,24 @@ Those settings are stored in database and do not require a restart of your instance after modification. They typically relate to higher level configuration, such your instance description, signup policy and so on. -There is no polished interface for those settings, yet, but you can view update -them using the administration interface provided by Django (the framework funkwhale is built on). - -The URL should be ``/api/admin/dynamic_preferences/globalpreferencemodel/`` (prepend your domain in front of it, of course). +You can edit those settings directly from the web application, assuming +you have the required permissions. The URL is ``/manage/settings``, and +you will also find a link to this page in the sidebar. If you plan to use acoustid and external imports (e.g. with the youtube backends), you should edit the corresponding settings in this interface. +.. note:: + + If you have any issue with the web application, a management interface is also + available for those settings from Django's administration interface. It's + less user friendly, though, and we recommend you use the web app interface + whenever possible. + + The URL should be ``/api/admin/dynamic_preferences/globalpreferencemodel/`` (prepend your domain in front of it, of course). + + Configuration reference ----------------------- From ed9971cf7f8e595c18735641474c9420b4c0abde Mon Sep 17 00:00:00 2001 From: Eliot Berriot Date: Thu, 17 May 2018 23:50:44 +0200 Subject: [PATCH 30/38] Fix #206: changelog --- changes/changelog.d/206.feature | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) create mode 100644 changes/changelog.d/206.feature diff --git a/changes/changelog.d/206.feature b/changes/changelog.d/206.feature new file mode 100644 index 000000000..b334554fe --- /dev/null +++ b/changes/changelog.d/206.feature @@ -0,0 +1,17 @@ +We now have a brand new instance settings interface in the front-end (#206) + + +Instance settings interface +^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +Prior to this release, the only way to update instance settings (such as +instance description, signup policy, federation configuration, etc.) was using +the admin interface provided by Django (the back-end framework which power the API). + +This interface worked, but was not really-user friendly and intuitive. + +Starting from this release, we now offer a dedicated interface directly +in the front-end. You can view and edit all your instance settings from here, +assuming you have the required permissions. + +This interface is available at ``/manage/settings` and via link in the sidebar. From f0a62fbad5dcf35cfd04d1a7e21f5bf0233cc2ce Mon Sep 17 00:00:00 2001 From: Thomas Citharel Date: Fri, 18 May 2018 11:58:32 +0000 Subject: [PATCH 31/38] Change approve instance button from x to check Closes #210 --- front/src/components/federation/LibraryFollowTable.vue | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/front/src/components/federation/LibraryFollowTable.vue b/front/src/components/federation/LibraryFollowTable.vue index fd16d8371..4a7fe59f0 100644 --- a/front/src/components/federation/LibraryFollowTable.vue +++ b/front/src/components/federation/LibraryFollowTable.vue @@ -55,7 +55,7 @@

- +

From ff65a4b93592d7da25a2693042656c89461e6d54 Mon Sep 17 00:00:00 2001 From: Eliot Berriot Date: Fri, 18 May 2018 18:47:35 +0200 Subject: [PATCH 32/38] See #152: added permission fields on user model and corresponding API permission --- .../migrations/0006_auto_20180517_2324.py | 28 ++++++++++ api/funkwhale_api/users/models.py | 39 +++++++------ api/funkwhale_api/users/permissions.py | 19 +++++++ api/tests/users/test_models.py | 34 +++++++++++ api/tests/users/test_permissions.py | 56 +++++++++++++++++++ 5 files changed, 159 insertions(+), 17 deletions(-) create mode 100644 api/funkwhale_api/users/migrations/0006_auto_20180517_2324.py create mode 100644 api/funkwhale_api/users/permissions.py create mode 100644 api/tests/users/test_permissions.py diff --git a/api/funkwhale_api/users/migrations/0006_auto_20180517_2324.py b/api/funkwhale_api/users/migrations/0006_auto_20180517_2324.py new file mode 100644 index 000000000..7c9ab0fad --- /dev/null +++ b/api/funkwhale_api/users/migrations/0006_auto_20180517_2324.py @@ -0,0 +1,28 @@ +# Generated by Django 2.0.4 on 2018-05-17 23:24 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ('users', '0005_user_subsonic_api_token'), + ] + + operations = [ + migrations.AddField( + model_name='user', + name='permission_federation', + field=models.BooleanField(default=False), + ), + migrations.AddField( + model_name='user', + name='permission_library', + field=models.BooleanField(default=False), + ), + migrations.AddField( + model_name='user', + name='permission_settings', + field=models.BooleanField(default=False), + ), + ] diff --git a/api/funkwhale_api/users/models.py b/api/funkwhale_api/users/models.py index 8273507c4..1de23092a 100644 --- a/api/funkwhale_api/users/models.py +++ b/api/funkwhale_api/users/models.py @@ -19,6 +19,13 @@ def get_token(): return binascii.b2a_hex(os.urandom(15)).decode('utf-8') +PERMISSIONS = [ + 'federation', + 'library', + 'settings', +] + + @python_2_unicode_compatible class User(AbstractUser): @@ -28,20 +35,6 @@ class User(AbstractUser): # updated on logout or password change, to invalidate JWT secret_key = models.UUIDField(default=uuid.uuid4, null=True) - # permissions that are used for API access and that worth serializing - relevant_permissions = { - # internal_codename : {external_codename} - 'music.add_importbatch': { - 'external_codename': 'import.launch', - }, - 'dynamic_preferences.change_globalpreferencemodel': { - 'external_codename': 'settings.change', - }, - 'federation.change_library': { - 'external_codename': 'federation.manage', - }, - } - privacy_level = fields.get_privacy_field() # Unfortunately, Subsonic API assumes a MD5/password authentication @@ -52,12 +45,24 @@ class User(AbstractUser): subsonic_api_token = models.CharField( blank=True, null=True, max_length=255) + # permissions + permission_federation = models.BooleanField(default=False) + permission_library = models.BooleanField(default=False) + permission_settings = models.BooleanField(default=False) + def __str__(self): return self.username - def add_permission(self, codename): - p = Permission.objects.get(codename=codename) - self.user_permissions.add(p) + def get_permissions(self): + perms = {} + for p in PERMISSIONS: + v = self.is_superuser or getattr(self, 'permission_{}'.format(p)) + perms[p] = v + return perms + + def has_permissions(self, *perms): + permissions = self.get_permissions() + return all([permissions[p] for p in perms]) def get_absolute_url(self): return reverse('users:detail', kwargs={'username': self.username}) diff --git a/api/funkwhale_api/users/permissions.py b/api/funkwhale_api/users/permissions.py new file mode 100644 index 000000000..2ff49ff3f --- /dev/null +++ b/api/funkwhale_api/users/permissions.py @@ -0,0 +1,19 @@ +from rest_framework.permissions import BasePermission + + +class HasUserPermission(BasePermission): + """ + Ensure the request user has the proper permissions. + + Usage: + + class MyView(APIView): + permission_classes = [HasUserPermission] + required_permissions = ['federation'] + """ + def has_permission(self, request, view): + if not hasattr(request, 'user') or not request.user: + return False + if request.user.is_anonymous: + return False + return request.user.has_permissions(*view.required_permissions) diff --git a/api/tests/users/test_models.py b/api/tests/users/test_models.py index c7cd12e9e..49199e0a7 100644 --- a/api/tests/users/test_models.py +++ b/api/tests/users/test_models.py @@ -1,3 +1,7 @@ +import pytest + +from funkwhale_api.users import models + def test__str__(factories): user = factories['users.User'](username='hello') @@ -16,3 +20,33 @@ def test_changing_password_updates_subsonic_api_token(factories): assert user.subsonic_api_token is not None assert user.subsonic_api_token != 'test' + + +def test_get_permissions_superuser(factories): + user = factories['users.User'](is_superuser=True) + + perms = user.get_permissions() + for p in models.PERMISSIONS: + assert perms[p] is True + + +def test_get_permissions_regular(factories): + user = factories['users.User'](permission_library=True) + + perms = user.get_permissions() + for p in models.PERMISSIONS: + if p == 'library': + assert perms[p] is True + else: + assert perms[p] is False + + +@pytest.mark.parametrize('args,perms,expected', [ + ({'is_superuser': True}, ['federation', 'library'], True), + ({'is_superuser': False}, ['federation'], False), + ({'permission_library': True}, ['library'], True), + ({'permission_library': True}, ['library', 'federation'], False), +]) +def test_has_permissions(args, perms, expected, factories): + user = factories['users.User'](**args) + assert user.has_permissions(*perms) is expected diff --git a/api/tests/users/test_permissions.py b/api/tests/users/test_permissions.py new file mode 100644 index 000000000..1564c761d --- /dev/null +++ b/api/tests/users/test_permissions.py @@ -0,0 +1,56 @@ +import pytest +from rest_framework.views import APIView + +from funkwhale_api.users import permissions + + +def test_has_user_permission_no_user(api_request): + view = APIView.as_view() + permission = permissions.HasUserPermission() + request = api_request.get('/') + assert permission.has_permission(request, view) is False + + +def test_has_user_permission_anonymous(anonymous_user, api_request): + view = APIView.as_view() + permission = permissions.HasUserPermission() + request = api_request.get('/') + setattr(request, 'user', anonymous_user) + assert permission.has_permission(request, view) is False + + +@pytest.mark.parametrize('value', [True, False]) +def test_has_user_permission_logged_in_single(value, factories, api_request): + user = factories['users.User'](permission_federation=value) + + class View(APIView): + required_permissions = ['federation'] + view = View() + permission = permissions.HasUserPermission() + request = api_request.get('/') + setattr(request, 'user', user) + result = permission.has_permission(request, view) + assert result == user.has_permissions('federation') == value + + +@pytest.mark.parametrize('federation,library,expected', [ + (True, False, False), + (False, True, False), + (False, False, False), + (True, True, True), +]) +def test_has_user_permission_logged_in_single( + federation, library, expected, factories, api_request): + user = factories['users.User']( + permission_federation=federation, + permission_library=library, + ) + + class View(APIView): + required_permissions = ['federation', 'library'] + view = View() + permission = permissions.HasUserPermission() + request = api_request.get('/') + setattr(request, 'user', user) + result = permission.has_permission(request, view) + assert result == user.has_permissions('federation', 'library') == expected From 6fc4275b681ac65e1ebd289b55e99856408af12f Mon Sep 17 00:00:00 2001 From: Eliot Berriot Date: Fri, 18 May 2018 18:48:46 +0200 Subject: [PATCH 33/38] See #152: use new user permissions on relevant viewsets --- api/funkwhale_api/common/permissions.py | 13 +--------- api/funkwhale_api/federation/views.py | 12 ++++----- api/funkwhale_api/instance/views.py | 4 ++- api/funkwhale_api/music/views.py | 18 ++++++-------- api/funkwhale_api/users/serializers.py | 9 ++----- api/tests/conftest.py | 9 +++++++ api/tests/federation/test_views.py | 9 +++++++ api/tests/instance/test_views.py | 14 ++++++++++- api/tests/music/test_views.py | 8 ++++++ api/tests/users/test_views.py | 33 ++++++++++--------------- 10 files changed, 71 insertions(+), 58 deletions(-) diff --git a/api/funkwhale_api/common/permissions.py b/api/funkwhale_api/common/permissions.py index cab4b699d..e9e8b8819 100644 --- a/api/funkwhale_api/common/permissions.py +++ b/api/funkwhale_api/common/permissions.py @@ -3,7 +3,7 @@ import operator from django.conf import settings from django.http import Http404 -from rest_framework.permissions import BasePermission, DjangoModelPermissions +from rest_framework.permissions import BasePermission from funkwhale_api.common import preferences @@ -16,17 +16,6 @@ class ConditionalAuthentication(BasePermission): return True -class HasModelPermission(DjangoModelPermissions): - """ - Same as DjangoModelPermissions, but we pin the model: - - class MyModelPermission(HasModelPermission): - model = User - """ - def get_required_permissions(self, method, model_cls): - return super().get_required_permissions(method, self.model) - - class OwnerPermission(BasePermission): """ Ensure the request user is the owner of the object. diff --git a/api/funkwhale_api/federation/views.py b/api/funkwhale_api/federation/views.py index 37ad9ebfd..06a2cd040 100644 --- a/api/funkwhale_api/federation/views.py +++ b/api/funkwhale_api/federation/views.py @@ -15,8 +15,8 @@ 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 +from funkwhale_api.users.permissions import HasUserPermission from . import activity from . import actors @@ -187,16 +187,13 @@ class MusicFilesViewSet(FederationMixin, viewsets.GenericViewSet): return response.Response(data) -class LibraryPermission(HasModelPermission): - model = models.Library - - class LibraryViewSet( mixins.RetrieveModelMixin, mixins.UpdateModelMixin, mixins.ListModelMixin, viewsets.GenericViewSet): - permission_classes = [LibraryPermission] + permission_classes = (HasUserPermission,) + required_permissions = ['federation'] queryset = models.Library.objects.all().select_related( 'actor', 'follow', @@ -291,7 +288,8 @@ class LibraryViewSet( class LibraryTrackViewSet( mixins.ListModelMixin, viewsets.GenericViewSet): - permission_classes = [LibraryPermission] + permission_classes = (HasUserPermission,) + required_permissions = ['federation'] queryset = models.LibraryTrack.objects.all().select_related( 'library__actor', 'library__follow', diff --git a/api/funkwhale_api/instance/views.py b/api/funkwhale_api/instance/views.py index e6725e248..b905acd3e 100644 --- a/api/funkwhale_api/instance/views.py +++ b/api/funkwhale_api/instance/views.py @@ -6,6 +6,7 @@ from dynamic_preferences.api import viewsets as preferences_viewsets from dynamic_preferences.registries import global_preferences_registry from funkwhale_api.common import preferences +from funkwhale_api.users.permissions import HasUserPermission from . import nodeinfo from . import stats @@ -18,7 +19,8 @@ NODEINFO_2_CONTENT_TYPE = ( class AdminSettings(preferences_viewsets.GlobalPreferencesViewSet): pagination_class = None - + permission_classes = (HasUserPermission,) + required_permissions = ['settings'] class InstanceSettings(views.APIView): permission_classes = [] diff --git a/api/funkwhale_api/music/views.py b/api/funkwhale_api/music/views.py index f2ab72c5a..e71d3555e 100644 --- a/api/funkwhale_api/music/views.py +++ b/api/funkwhale_api/music/views.py @@ -25,8 +25,8 @@ from rest_framework import permissions from musicbrainzngs import ResponseError from funkwhale_api.common import utils as funkwhale_utils -from funkwhale_api.common.permissions import ( - ConditionalAuthentication, HasModelPermission) +from funkwhale_api.common.permissions import ConditionalAuthentication +from funkwhale_api.users.permissions import HasUserPermission from taggit.models import Tag from funkwhale_api.federation import actors from funkwhale_api.federation.authentication import SignatureAuthentication @@ -107,25 +107,22 @@ class ImportBatchViewSet( .annotate(job_count=Count('jobs')) ) serializer_class = serializers.ImportBatchSerializer - permission_classes = (permissions.DjangoModelPermissions, ) + permission_classes = (HasUserPermission,) + required_permissions = ['library'] filter_class = filters.ImportBatchFilter def perform_create(self, serializer): serializer.save(submitted_by=self.request.user) -class ImportJobPermission(HasModelPermission): - # not a typo, perms on import job is proxied to import batch - model = models.ImportBatch - - class ImportJobViewSet( mixins.CreateModelMixin, mixins.ListModelMixin, viewsets.GenericViewSet): queryset = (models.ImportJob.objects.all().select_related()) serializer_class = serializers.ImportJobSerializer - permission_classes = (ImportJobPermission, ) + permission_classes = (HasUserPermission,) + required_permissions = ['library'] filter_class = filters.ImportJobFilter @list_route(methods=['get']) @@ -442,7 +439,8 @@ class Search(views.APIView): class SubmitViewSet(viewsets.ViewSet): queryset = models.ImportBatch.objects.none() - permission_classes = (permissions.DjangoModelPermissions, ) + permission_classes = (HasUserPermission,) + required_permissions = ['library'] @list_route(methods=['post']) @transaction.non_atomic_requests diff --git a/api/funkwhale_api/users/serializers.py b/api/funkwhale_api/users/serializers.py index eadce6154..3a095e78a 100644 --- a/api/funkwhale_api/users/serializers.py +++ b/api/funkwhale_api/users/serializers.py @@ -55,16 +55,11 @@ class UserReadSerializer(serializers.ModelSerializer): 'is_superuser', 'permissions', 'date_joined', - 'privacy_level' + 'privacy_level', ] def get_permissions(self, o): - perms = {} - for internal_codename, conf in o.relevant_permissions.items(): - perms[conf['external_codename']] = { - 'status': o.has_perm(internal_codename) - } - return perms + return o.get_permissions() class PasswordResetSerializer(PRS): diff --git a/api/tests/conftest.py b/api/tests/conftest.py index dda537801..b7a7d071a 100644 --- a/api/tests/conftest.py +++ b/api/tests/conftest.py @@ -14,6 +14,7 @@ from rest_framework.test import APIClient from rest_framework.test import APIRequestFactory from funkwhale_api.activity import record +from funkwhale_api.users.permissions import HasUserPermission from funkwhale_api.taskapp import celery @@ -224,3 +225,11 @@ def authenticated_actor(factories, mocker): 'funkwhale_api.federation.authentication.SignatureAuthentication.authenticate_actor', return_value=actor) yield actor + + +@pytest.fixture +def assert_user_permission(): + def inner(view, permissions): + assert HasUserPermission in view.permission_classes + assert set(view.required_permissions) == set(permissions) + return inner diff --git a/api/tests/federation/test_views.py b/api/tests/federation/test_views.py index fd6ac2eb2..10237ed9f 100644 --- a/api/tests/federation/test_views.py +++ b/api/tests/federation/test_views.py @@ -9,9 +9,18 @@ from funkwhale_api.federation import activity from funkwhale_api.federation import models from funkwhale_api.federation import serializers from funkwhale_api.federation import utils +from funkwhale_api.federation import views from funkwhale_api.federation import webfinger +@pytest.mark.parametrize('view,permissions', [ + (views.LibraryViewSet, ['federation']), + (views.LibraryTrackViewSet, ['federation']), +]) +def test_permissions(assert_user_permission, view, permissions): + assert_user_permission(view, permissions) + + @pytest.mark.parametrize('system_actor', actors.SYSTEM_ACTORS.keys()) def test_instance_actors(system_actor, db, api_client): actor = actors.SYSTEM_ACTORS[system_actor].get_actor_instance() diff --git a/api/tests/instance/test_views.py b/api/tests/instance/test_views.py index 6d8dcac3e..daf54db51 100644 --- a/api/tests/instance/test_views.py +++ b/api/tests/instance/test_views.py @@ -1,5 +1,16 @@ +import pytest + from django.urls import reverse +from funkwhale_api.instance import views + + +@pytest.mark.parametrize('view,permissions', [ + (views.AdminSettings, ['settings']), +]) +def test_permissions(assert_user_permission, view, permissions): + assert_user_permission(view, permissions) + def test_nodeinfo_endpoint(db, api_client, mocker): payload = { @@ -43,7 +54,8 @@ def test_admin_settings_restrict_access(db, logged_in_api_client, preferences): def test_admin_settings_correct_permission( db, logged_in_api_client, preferences): user = logged_in_api_client.user - user.add_permission('change_globalpreferencemodel') + user.permission_settings = True + user.save() url = reverse('api:v1:instance:admin-settings-list') response = logged_in_api_client.get(url) diff --git a/api/tests/music/test_views.py b/api/tests/music/test_views.py index e641b45d5..030fc3a73 100644 --- a/api/tests/music/test_views.py +++ b/api/tests/music/test_views.py @@ -8,6 +8,14 @@ from funkwhale_api.music import views from funkwhale_api.federation import actors +@pytest.mark.parametrize('view,permissions', [ + (views.ImportBatchViewSet, ['library']), + (views.ImportJobViewSet, ['library']), +]) +def test_permissions(assert_user_permission, view, permissions): + assert_user_permission(view, permissions) + + @pytest.mark.parametrize('param,expected', [ ('true', 'full'), ('false', 'empty'), diff --git a/api/tests/users/test_views.py b/api/tests/users/test_views.py index fffc762fd..1bbf8b9a2 100644 --- a/api/tests/users/test_views.py +++ b/api/tests/users/test_views.py @@ -53,33 +53,24 @@ def test_can_disable_registration_view(preferences, client, db): assert response.status_code == 403 -def test_can_fetch_data_from_api(client, factories): +def test_can_fetch_data_from_api(api_client, factories): url = reverse('api:v1:users:users-me') - response = client.get(url) + response = api_client.get(url) # login required assert response.status_code == 401 user = factories['users.User']( - is_staff=True, - perms=[ - 'music.add_importbatch', - 'dynamic_preferences.change_globalpreferencemodel', - ] + permission_library=True ) - assert user.has_perm('music.add_importbatch') - client.login(username=user.username, password='test') - response = client.get(url) + api_client.login(username=user.username, password='test') + response = api_client.get(url) assert response.status_code == 200 - - payload = json.loads(response.content.decode('utf-8')) - - assert payload['username'] == user.username - assert payload['is_staff'] == user.is_staff - assert payload['is_superuser'] == user.is_superuser - assert payload['email'] == user.email - assert payload['name'] == user.name - assert payload['permissions']['import.launch']['status'] - assert payload['permissions']['settings.change']['status'] + assert response.data['username'] == user.username + assert response.data['is_staff'] == user.is_staff + assert response.data['is_superuser'] == user.is_superuser + assert response.data['email'] == user.email + assert response.data['name'] == user.name + assert response.data['permissions'] == user.get_permissions() def test_can_get_token_via_api(client, factories): @@ -202,6 +193,8 @@ def test_user_can_get_new_subsonic_token(logged_in_api_client): assert response.data == { 'subsonic_api_token': 'test' } + + def test_user_can_request_new_subsonic_token(logged_in_api_client): user = logged_in_api_client.user user.subsonic_api_token = 'test' From 4ce6715dc7a3cf6946dc38b12e02790a9424d2fa Mon Sep 17 00:00:00 2001 From: Eliot Berriot Date: Fri, 18 May 2018 19:09:47 +0200 Subject: [PATCH 34/38] See #152: updated front-end to use new permissions --- front/src/components/About.vue | 2 +- front/src/components/Sidebar.vue | 14 +++++++------- front/src/components/library/Library.vue | 4 ++-- front/src/components/requests/Card.vue | 2 +- front/src/store/auth.js | 2 +- front/test/unit/specs/store/auth.spec.js | 4 +--- 6 files changed, 13 insertions(+), 15 deletions(-) diff --git a/front/src/components/About.vue b/front/src/components/About.vue index b0ae67ef7..59c8411ac 100644 --- a/front/src/components/About.vue +++ b/front/src/components/About.vue @@ -15,7 +15,7 @@

{{ $t('Edit instance info') }} diff --git a/front/src/components/Sidebar.vue b/front/src/components/Sidebar.vue index 9fbc5605c..9f3134c2a 100644 --- a/front/src/components/Sidebar.vue +++ b/front/src/components/Sidebar.vue @@ -60,7 +60,7 @@
{{ $t('Import requests') }}
{{ $t('Federation') }}
{{ $t('Settings') }} @@ -192,8 +192,8 @@ export default { }), showAdmin () { let adminPermissions = [ - this.$store.state.auth.availablePermissions['federation.manage'], - this.$store.state.auth.availablePermissions['import.launch'] + this.$store.state.auth.availablePermissions['federation'], + this.$store.state.auth.availablePermissions['library'] ] return adminPermissions.filter(e => { return e @@ -209,7 +209,7 @@ export default { this.fetchFederationImportRequestsCount() }, fetchFederationNotificationsCount () { - if (!this.$store.state.auth.availablePermissions['federation.manage']) { + if (!this.$store.state.auth.availablePermissions['federation']) { return } let self = this @@ -218,7 +218,7 @@ export default { }) }, fetchFederationImportRequestsCount () { - if (!this.$store.state.auth.availablePermissions['import.launch']) { + if (!this.$store.state.auth.availablePermissions['library']) { return } let self = this diff --git a/front/src/components/library/Library.vue b/front/src/components/library/Library.vue index 820d60a54..e360ccb1c 100644 --- a/front/src/components/library/Library.vue +++ b/front/src/components/library/Library.vue @@ -13,10 +13,10 @@ exact> - + - +
diff --git a/front/src/components/requests/Card.vue b/front/src/components/requests/Card.vue index 9d0bf0b77..7743bb6d4 100644 --- a/front/src/components/requests/Card.vue +++ b/front/src/components/requests/Card.vue @@ -22,7 +22,7 @@
diff --git a/front/src/store/auth.js b/front/src/store/auth.js index 68a15090b..87af081d2 100644 --- a/front/src/store/auth.js +++ b/front/src/store/auth.js @@ -112,7 +112,7 @@ export default { dispatch('playlists/fetchOwn', null, {root: true}) Object.keys(data.permissions).forEach(function (key) { // this makes it easier to check for permissions in templates - commit('permission', {key, status: data.permissions[String(key)].status}) + commit('permission', {key, status: data.permissions[String(key)]}) }) return response.data }, (response) => { diff --git a/front/test/unit/specs/store/auth.spec.js b/front/test/unit/specs/store/auth.spec.js index 3d175e9f9..46901cd16 100644 --- a/front/test/unit/specs/store/auth.spec.js +++ b/front/test/unit/specs/store/auth.spec.js @@ -164,9 +164,7 @@ describe('store/auth', () => { const profile = { username: 'bob', permissions: { - admin: { - status: true - } + admin: true } } moxios.stubRequest('users/users/me/', { From a57d975183e1fc3e73d3ed8b6de145c1697e8890 Mon Sep 17 00:00:00 2001 From: Eliot Berriot Date: Fri, 18 May 2018 19:18:40 +0200 Subject: [PATCH 35/38] See #152: updated admin interface --- api/funkwhale_api/users/admin.py | 17 ++++++++++++++++- api/funkwhale_api/users/models.py | 14 +++++++++++--- 2 files changed, 27 insertions(+), 4 deletions(-) diff --git a/api/funkwhale_api/users/admin.py b/api/funkwhale_api/users/admin.py index 89b67d3df..c772603e9 100644 --- a/api/funkwhale_api/users/admin.py +++ b/api/funkwhale_api/users/admin.py @@ -5,6 +5,7 @@ from django import forms from django.contrib import admin from django.contrib.auth.admin import UserAdmin as AuthUserAdmin from django.contrib.auth.forms import UserChangeForm, UserCreationForm +from django.utils.translation import ugettext_lazy as _ from .models import User @@ -41,8 +42,22 @@ class UserAdmin(AuthUserAdmin): 'email', 'date_joined', 'last_login', - 'privacy_level', + 'is_staff', + 'is_superuser', ] list_filter = [ + 'is_superuser', + 'is_staff', 'privacy_level', + 'permission_settings', + 'permission_library', + 'permission_federation', ] + + fieldsets = ( + (None, {'fields': ('username', 'password', 'privacy_level')}), + (_('Personal info'), {'fields': ('first_name', 'last_name', 'email')}), + (_('Permissions'), {'fields': ('is_active', 'is_staff', 'is_superuser', + 'permission_library', 'permission_settings', 'permission_federation')}), + (_('Important dates'), {'fields': ('last_login', 'date_joined')}), + ) diff --git a/api/funkwhale_api/users/models.py b/api/funkwhale_api/users/models.py index 1de23092a..c16cd62b3 100644 --- a/api/funkwhale_api/users/models.py +++ b/api/funkwhale_api/users/models.py @@ -46,9 +46,17 @@ class User(AbstractUser): blank=True, null=True, max_length=255) # permissions - permission_federation = models.BooleanField(default=False) - permission_library = models.BooleanField(default=False) - permission_settings = models.BooleanField(default=False) + permission_federation = models.BooleanField( + 'Manage library federation', + help_text='Follow other instances, accept/deny library follow requests...', + default=False) + permission_library = models.BooleanField( + 'Manage library', + help_text='Import new content, manage existing content', + default=False) + permission_settings = models.BooleanField( + 'Manage instance-level settings', + default=False) def __str__(self): return self.username From ac7db737857c1a070574174db4b90450a380a331 Mon Sep 17 00:00:00 2001 From: Eliot Berriot Date: Fri, 18 May 2018 21:19:20 +0200 Subject: [PATCH 36/38] See #152: added management command to execute one-time migration scripts --- .../common/management/__init__.py | 0 .../common/management/commands/__init__.py | 0 .../common/management/commands/script.py | 66 +++++++++++++++++++ api/funkwhale_api/common/scripts/__init__.py | 2 + .../django_permissions_to_user_permissions.py | 29 ++++++++ api/funkwhale_api/common/scripts/test.py | 8 +++ api/funkwhale_api/users/admin.py | 17 ++++- api/funkwhale_api/users/factories.py | 28 +++++++- api/tests/common/test_scripts.py | 56 ++++++++++++++++ 9 files changed, 202 insertions(+), 4 deletions(-) create mode 100644 api/funkwhale_api/common/management/__init__.py create mode 100644 api/funkwhale_api/common/management/commands/__init__.py create mode 100644 api/funkwhale_api/common/management/commands/script.py create mode 100644 api/funkwhale_api/common/scripts/__init__.py create mode 100644 api/funkwhale_api/common/scripts/django_permissions_to_user_permissions.py create mode 100644 api/funkwhale_api/common/scripts/test.py create mode 100644 api/tests/common/test_scripts.py diff --git a/api/funkwhale_api/common/management/__init__.py b/api/funkwhale_api/common/management/__init__.py new file mode 100644 index 000000000..e69de29bb diff --git a/api/funkwhale_api/common/management/commands/__init__.py b/api/funkwhale_api/common/management/commands/__init__.py new file mode 100644 index 000000000..e69de29bb diff --git a/api/funkwhale_api/common/management/commands/script.py b/api/funkwhale_api/common/management/commands/script.py new file mode 100644 index 000000000..9d26a5836 --- /dev/null +++ b/api/funkwhale_api/common/management/commands/script.py @@ -0,0 +1,66 @@ +from django.core.management.base import BaseCommand, CommandError + +from funkwhale_api.common import scripts + + +class Command(BaseCommand): + help = 'Run a specific script from funkwhale_api/common/scripts/' + + def add_arguments(self, parser): + parser.add_argument('script_name', nargs='?', type=str) + parser.add_argument( + '--noinput', '--no-input', action='store_false', dest='interactive', + help="Do NOT prompt the user for input of any kind.", + ) + + def handle(self, *args, **options): + name = options['script_name'] + if not name: + self.show_help() + + available_scripts = self.get_scripts() + try: + script = available_scripts[name] + except KeyError: + raise CommandError( + '{} is not a valid script. Run python manage.py script for a ' + 'list of available scripts'.format(name)) + + self.stdout.write('') + if options['interactive']: + message = ( + 'Are you sure you want to execute the script {}?\n\n' + "Type 'yes' to continue, or 'no' to cancel: " + ).format(name) + if input(''.join(message)) != 'yes': + raise CommandError("Script cancelled.") + script['entrypoint'](self, **options) + + def show_help(self): + indentation = 4 + self.stdout.write('') + self.stdout.write('Available scripts:') + self.stdout.write('Launch with: python manage.py ') + available_scripts = self.get_scripts() + for name, script in sorted(available_scripts.items()): + self.stdout.write('') + self.stdout.write(self.style.SUCCESS(name)) + self.stdout.write('') + for line in script['help'].splitlines(): + self.stdout.write('     {}'.format(line)) + self.stdout.write('') + + def get_scripts(self): + available_scripts = [ + k for k in sorted(scripts.__dict__.keys()) + if not k.startswith('__') + ] + data = {} + for name in available_scripts: + module = getattr(scripts, name) + data[name] = { + 'name': name, + 'help': module.__doc__.strip(), + 'entrypoint': module.main + } + return data diff --git a/api/funkwhale_api/common/scripts/__init__.py b/api/funkwhale_api/common/scripts/__init__.py new file mode 100644 index 000000000..4b2d52520 --- /dev/null +++ b/api/funkwhale_api/common/scripts/__init__.py @@ -0,0 +1,2 @@ +from . import django_permissions_to_user_permissions +from . import test diff --git a/api/funkwhale_api/common/scripts/django_permissions_to_user_permissions.py b/api/funkwhale_api/common/scripts/django_permissions_to_user_permissions.py new file mode 100644 index 000000000..1bc971f80 --- /dev/null +++ b/api/funkwhale_api/common/scripts/django_permissions_to_user_permissions.py @@ -0,0 +1,29 @@ +""" +Convert django permissions to user permissions in the database, +following the work done in #152. +""" +from django.db.models import Q +from funkwhale_api.users import models + +from django.contrib.auth.models import Permission + +mapping = { + 'dynamic_preferences.change_globalpreferencemodel': 'settings', + 'music.add_importbatch': 'library', + 'federation.change_library': 'federation', +} + + +def main(command, **kwargs): + for codename, user_permission in sorted(mapping.items()): + app_label, c = codename.split('.') + p = Permission.objects.get( + content_type__app_label=app_label, codename=c) + users = models.User.objects.filter( + Q(groups__permissions=p) | Q(user_permissions=p)).distinct() + total = users.count() + + command.stdout.write('Updating {} users with {} permission...'.format( + total, user_permission + )) + users.update(**{'permission_{}'.format(user_permission): True}) diff --git a/api/funkwhale_api/common/scripts/test.py b/api/funkwhale_api/common/scripts/test.py new file mode 100644 index 000000000..ab401dca4 --- /dev/null +++ b/api/funkwhale_api/common/scripts/test.py @@ -0,0 +1,8 @@ +""" +This is a test script that does nothing. +You can launch it just to check how it works. +""" + + +def main(command, **kwargs): + command.stdout.write('Test script run successfully') diff --git a/api/funkwhale_api/users/admin.py b/api/funkwhale_api/users/admin.py index c772603e9..7e9062a13 100644 --- a/api/funkwhale_api/users/admin.py +++ b/api/funkwhale_api/users/admin.py @@ -57,7 +57,18 @@ class UserAdmin(AuthUserAdmin): fieldsets = ( (None, {'fields': ('username', 'password', 'privacy_level')}), (_('Personal info'), {'fields': ('first_name', 'last_name', 'email')}), - (_('Permissions'), {'fields': ('is_active', 'is_staff', 'is_superuser', - 'permission_library', 'permission_settings', 'permission_federation')}), + (_('Permissions'), { + 'fields': ( + 'is_active', + 'is_staff', + 'is_superuser', + 'permission_library', + 'permission_settings', + 'permission_federation')}), (_('Important dates'), {'fields': ('last_login', 'date_joined')}), - ) + (_('Useless fields'), { + 'fields': ( + 'user_permissions', + 'groups', + )}) + ) diff --git a/api/funkwhale_api/users/factories.py b/api/funkwhale_api/users/factories.py index 12307f7fd..cd28f4407 100644 --- a/api/funkwhale_api/users/factories.py +++ b/api/funkwhale_api/users/factories.py @@ -1,15 +1,41 @@ import factory -from funkwhale_api.factories import registry +from funkwhale_api.factories import registry, ManyToManyFromList from django.contrib.auth.models import Permission +@registry.register +class GroupFactory(factory.django.DjangoModelFactory): + name = factory.Sequence(lambda n: 'group-{0}'.format(n)) + + class Meta: + model = 'auth.Group' + + @factory.post_generation + def perms(self, create, extracted, **kwargs): + if not create: + # Simple build, do nothing. + return + + if extracted: + perms = [ + Permission.objects.get( + content_type__app_label=p.split('.')[0], + codename=p.split('.')[1], + ) + for p in extracted + ] + # A list of permissions were passed in, use them + self.permissions.add(*perms) + + @registry.register class UserFactory(factory.django.DjangoModelFactory): username = factory.Sequence(lambda n: 'user-{0}'.format(n)) email = factory.Sequence(lambda n: 'user-{0}@example.com'.format(n)) password = factory.PostGenerationMethodCall('set_password', 'test') subsonic_api_token = None + groups = ManyToManyFromList('groups') class Meta: model = 'users.User' diff --git a/api/tests/common/test_scripts.py b/api/tests/common/test_scripts.py new file mode 100644 index 000000000..ce478ba04 --- /dev/null +++ b/api/tests/common/test_scripts.py @@ -0,0 +1,56 @@ +import pytest + +from funkwhale_api.common.management.commands import script +from funkwhale_api.common import scripts + + +@pytest.fixture +def command(): + return script.Command() + + +@pytest.mark.parametrize('script_name', [ + 'django_permissions_to_user_permissions', + 'test', +]) +def test_script_command_list(command, script_name, mocker): + mocked = mocker.patch( + 'funkwhale_api.common.scripts.{}.main'.format(script_name)) + + command.handle(script_name=script_name, interactive=False) + + mocked.assert_called_once_with( + command, script_name=script_name, interactive=False) + + +def test_django_permissions_to_user_permissions(factories, command): + group = factories['auth.Group']( + perms=[ + 'federation.change_library' + ] + ) + user1 = factories['users.User']( + perms=[ + 'dynamic_preferences.change_globalpreferencemodel', + 'music.add_importbatch', + ] + ) + user2 = factories['users.User']( + perms=[ + 'music.add_importbatch', + ], + groups=[group] + ) + + scripts.django_permissions_to_user_permissions.main(command) + + user1.refresh_from_db() + user2.refresh_from_db() + + assert user1.permission_settings is True + assert user1.permission_library is True + assert user1.permission_federation is False + + assert user2.permission_settings is False + assert user2.permission_library is True + assert user2.permission_federation is True From aee792ab478dda746feafe69dc0ed963cb48c7b5 Mon Sep 17 00:00:00 2001 From: Eliot Berriot Date: Fri, 18 May 2018 22:13:33 +0200 Subject: [PATCH 37/38] Fix #152: changelog and documentation --- changes/changelog.d/152.feature | 32 ++++++++++++++++++++++++++++++++ docs/configuration.rst | 25 +++++++++++++++++++++++++ 2 files changed, 57 insertions(+) create mode 100644 changes/changelog.d/152.feature diff --git a/changes/changelog.d/152.feature b/changes/changelog.d/152.feature new file mode 100644 index 000000000..a10225288 --- /dev/null +++ b/changes/changelog.d/152.feature @@ -0,0 +1,32 @@ +Simpler permission system (#152) + + +Simpler permission system +========================= + +Starting from this release, the permission system is much simpler. Up until now, +we were using Django's built-in permission system, which was working, but also +quite complex to deal with. + +The new implementation relies on simpler logic, which will make integration +on the front-end in upcoming releases faster and easier. + +If you have manually given permissions to users on your instance, +you can migrate those to the new system. + +On docker setups: + +.. code-block:: shell + + docker-compose run --rm api python manage.py script django_permissions_to_user_permissions --no-input + +On non-docker setups: + +.. code-block:: shell + + # in your virtualenv + python api/manage.py script django_permissions_to_user_permissions --no-input + +There is still no dedicated interface to manage user permissions, but you +can use the admin interface at ``/api/admin/users/user/`` for that purpose in +the meantime. diff --git a/docs/configuration.rst b/docs/configuration.rst index b7df2db42..46756bb26 100644 --- a/docs/configuration.rst +++ b/docs/configuration.rst @@ -117,3 +117,28 @@ Then, the value of :ref:`setting-MUSIC_DIRECTORY_SERVE_PATH` should be On non-docker setup, you don't need to configure this setting. .. note:: This path should not include any trailing slash + +User permissions +---------------- + +Funkwhale's permission model works as follows: + +- Anonymous users cannot do anything unless configured specifically +- Logged-in users can use the application, but cannot do things that affect + the whole instance +- Superusers can do anything + +To make things more granular and allow some delegation of responsability, +superusers can grant specific permissions to specific users. Available +permissions are: + +- **Manage instance-level settings**: users with this permission can edit instance + settings as described in :ref:`instance-settings` +- **Manage library**: users with this permission can import new music in the + instance +- **Manage library federation**: users with this permission can ask to federate with + other instances, and accept/deny federation requests from other intances + +There is no dedicated interface to manage users permissions, but superusers +can login on the Django's admin at ``/api/admin/`` and grant permissions +to users at ``/api/admin/users/user/``. From d299964c99960ecc460e808516aed2dbf3a07a6d Mon Sep 17 00:00:00 2001 From: Eliot Berriot Date: Sat, 19 May 2018 11:43:19 +0200 Subject: [PATCH 38/38] 0.13: Version bump and changelog --- CHANGELOG | 121 ++++++++++++++++++++++++++++ api/funkwhale_api/__init__.py | 2 +- changes/changelog.d/152.feature | 32 -------- changes/changelog.d/157.feature | 1 - changes/changelog.d/195.feature | 42 ---------- changes/changelog.d/196.bugfix | 1 - changes/changelog.d/197.bugfix | 1 - changes/changelog.d/198.bugfix | 1 - changes/changelog.d/199.bugfix | 1 - changes/changelog.d/200.enhancement | 1 - changes/changelog.d/201.bugfix | 1 - changes/changelog.d/204.bugfix | 1 - changes/changelog.d/205.enhancement | 1 - changes/changelog.d/206.feature | 17 ---- 14 files changed, 122 insertions(+), 101 deletions(-) delete mode 100644 changes/changelog.d/152.feature delete mode 100644 changes/changelog.d/157.feature delete mode 100644 changes/changelog.d/195.feature delete mode 100644 changes/changelog.d/196.bugfix delete mode 100644 changes/changelog.d/197.bugfix delete mode 100644 changes/changelog.d/198.bugfix delete mode 100644 changes/changelog.d/199.bugfix delete mode 100644 changes/changelog.d/200.enhancement delete mode 100644 changes/changelog.d/201.bugfix delete mode 100644 changes/changelog.d/204.bugfix delete mode 100644 changes/changelog.d/205.enhancement delete mode 100644 changes/changelog.d/206.feature diff --git a/CHANGELOG b/CHANGELOG index a8d5d23de..c53714488 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -10,6 +10,127 @@ This changelog is viewable on the web at https://docs.funkwhale.audio/changelog. .. towncrier +0.13 (2018-05-19) +----------------- + +Upgrade instructions are available at + https://docs.funkwhale.audio/upgrading.html + +Features: + +- Can now import and play flac files (#157) +- Simpler permission system (#152) +- Store file length, size and bitrate (#195) +- We now have a brand new instance settings interface in the front-end (#206) + + +Enhancements: + +- Disabled browsable HTML API in production (#205) +- Instances can now indicate on the nodeinfo endpoint if they want to remain + private (#200) + + +Bugfixes: + +- .well-known/nodeinfo endpoint can now answer to request with Accept: + application/json (#197) +- Fixed escaping issue of track name in playlist modal (#201) +- Fixed missing dot when downloading file (#204) +- In-place imported tracks with non-ascii characters don't break reverse-proxy + serving (#196) +- Removed Python 3.6 dependency (secrets module) (#198) +- Uplayable tracks are now properly disabled in the interface (#199) + + +Instance settings interface +^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +Prior to this release, the only way to update instance settings (such as +instance description, signup policy, federation configuration, etc.) was using +the admin interface provided by Django (the back-end framework which power the API). + +This interface worked, but was not really-user friendly and intuitive. + +Starting from this release, we now offer a dedicated interface directly +in the front-end. You can view and edit all your instance settings from here, +assuming you have the required permissions. + +This interface is available at ``/manage/settings` and via link in the sidebar. + + +Storage of bitrate, size and length in database +^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +Starting with this release, when importing files, Funkwhale will store +additional information about audio files: + +- Bitrate +- Size (in bytes) +- Duration + +This change is not retroactive, meaning already imported files will lack those +informations. The interface and API should work as before in such case, however, +we offer a command to deal with legacy files and populate the missing values. + +On docker setups: + +.. code-block:: shell + + docker-compose run --rm api python manage.py fix_track_files + + +On non-docker setups: + +.. code-block:: shell + + # from your activated virtualenv + python manage.py fix_track_files + +.. note:: + + The execution time for this command is proportional to the number of + audio files stored on your instance. This is because we need to read the + files from disk to fetch the data. You can run it in the background + while Funkwhale is up. + + It's also safe to interrupt this command and rerun it at a later point, or run + it multiple times. + + Use the --dry-run flag to check how many files would be impacted. + + +Simpler permission system +^^^^^^^^^^^^^^^^^^^^^^^^^ + +Starting from this release, the permission system is much simpler. Up until now, +we were using Django's built-in permission system, which was working, but also +quite complex to deal with. + +The new implementation relies on simpler logic, which will make integration +on the front-end in upcoming releases faster and easier. + +If you have manually given permissions to users on your instance, +you can migrate those to the new system. + +On docker setups: + +.. code-block:: shell + + docker-compose run --rm api python manage.py script django_permissions_to_user_permissions --no-input + +On non-docker setups: + +.. code-block:: shell + + # in your virtualenv + python api/manage.py script django_permissions_to_user_permissions --no-input + +There is still no dedicated interface to manage user permissions, but you +can use the admin interface at ``/api/admin/users/user/`` for that purpose in +the meantime. + + 0.12 (2018-05-09) ----------------- diff --git a/api/funkwhale_api/__init__.py b/api/funkwhale_api/__init__.py index f8b8af412..b0d7cc517 100644 --- a/api/funkwhale_api/__init__.py +++ b/api/funkwhale_api/__init__.py @@ -1,3 +1,3 @@ # -*- coding: utf-8 -*- -__version__ = '0.12' +__version__ = '0.13' __version_info__ = tuple([int(num) if num.isdigit() else num for num in __version__.replace('-', '.', 1).split('.')]) diff --git a/changes/changelog.d/152.feature b/changes/changelog.d/152.feature deleted file mode 100644 index a10225288..000000000 --- a/changes/changelog.d/152.feature +++ /dev/null @@ -1,32 +0,0 @@ -Simpler permission system (#152) - - -Simpler permission system -========================= - -Starting from this release, the permission system is much simpler. Up until now, -we were using Django's built-in permission system, which was working, but also -quite complex to deal with. - -The new implementation relies on simpler logic, which will make integration -on the front-end in upcoming releases faster and easier. - -If you have manually given permissions to users on your instance, -you can migrate those to the new system. - -On docker setups: - -.. code-block:: shell - - docker-compose run --rm api python manage.py script django_permissions_to_user_permissions --no-input - -On non-docker setups: - -.. code-block:: shell - - # in your virtualenv - python api/manage.py script django_permissions_to_user_permissions --no-input - -There is still no dedicated interface to manage user permissions, but you -can use the admin interface at ``/api/admin/users/user/`` for that purpose in -the meantime. diff --git a/changes/changelog.d/157.feature b/changes/changelog.d/157.feature deleted file mode 100644 index 0560541b5..000000000 --- a/changes/changelog.d/157.feature +++ /dev/null @@ -1 +0,0 @@ -Can now import and play flac files (#157) diff --git a/changes/changelog.d/195.feature b/changes/changelog.d/195.feature deleted file mode 100644 index 62411d8ef..000000000 --- a/changes/changelog.d/195.feature +++ /dev/null @@ -1,42 +0,0 @@ -Store file length, size and bitrate (#195) - - -Storage of bitrate, size and length in database -^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ - -Starting with this release, when importing files, Funkwhale will store -additional information about audio files: - -- Bitrate -- Size (in bytes) -- Duration - -This change is not retroactive, meaning already imported files will lack those -informations. The interface and API should work as before in such case, however, -we offer a command to deal with legacy files and populate the missing values. - -On docker setups: - -.. code-block:: shell - - docker-compose run --rm api python manage.py fix_track_files - - -On non-docker setups: - -.. code-block:: shell - - # from your activated virtualenv - python manage.py fix_track_files - -.. note:: - - The execution time for this command is proportional to the number of - audio files stored on your instance. This is because we need to read the - files from disk to fetch the data. You can run it in the background - while Funkwhale is up. - - It's also safe to interrupt this command and rerun it at a later point, or run - it multiple times. - - Use the --dry-run flag to check how many files would be impacted. diff --git a/changes/changelog.d/196.bugfix b/changes/changelog.d/196.bugfix deleted file mode 100644 index 655df7d79..000000000 --- a/changes/changelog.d/196.bugfix +++ /dev/null @@ -1 +0,0 @@ -In-place imported tracks non-ascii characters don't break reverse-proxy serving (#196) diff --git a/changes/changelog.d/197.bugfix b/changes/changelog.d/197.bugfix deleted file mode 100644 index d40182f54..000000000 --- a/changes/changelog.d/197.bugfix +++ /dev/null @@ -1 +0,0 @@ -.well-known/nodeinfo endpoint can now answer to request with Accept: application/json (#197) diff --git a/changes/changelog.d/198.bugfix b/changes/changelog.d/198.bugfix deleted file mode 100644 index dd2f4e8fc..000000000 --- a/changes/changelog.d/198.bugfix +++ /dev/null @@ -1 +0,0 @@ -Removed Python 3.6 dependency (secrets module) (#198) diff --git a/changes/changelog.d/199.bugfix b/changes/changelog.d/199.bugfix deleted file mode 100644 index 0dff4f9fe..000000000 --- a/changes/changelog.d/199.bugfix +++ /dev/null @@ -1 +0,0 @@ -Uplayable tracks are now properly disabled in the interface (#199) diff --git a/changes/changelog.d/200.enhancement b/changes/changelog.d/200.enhancement deleted file mode 100644 index 1e78b92f7..000000000 --- a/changes/changelog.d/200.enhancement +++ /dev/null @@ -1 +0,0 @@ -Instances can now indicate on the nodeinfo endpoint if they want to remain private (#200) diff --git a/changes/changelog.d/201.bugfix b/changes/changelog.d/201.bugfix deleted file mode 100644 index a0a25aa52..000000000 --- a/changes/changelog.d/201.bugfix +++ /dev/null @@ -1 +0,0 @@ -Fixed escaping issue of track name in playlist modal (#201) diff --git a/changes/changelog.d/204.bugfix b/changes/changelog.d/204.bugfix deleted file mode 100644 index 370f1d837..000000000 --- a/changes/changelog.d/204.bugfix +++ /dev/null @@ -1 +0,0 @@ -Fixed missing dot when downloading file (#204) diff --git a/changes/changelog.d/205.enhancement b/changes/changelog.d/205.enhancement deleted file mode 100644 index b44375b13..000000000 --- a/changes/changelog.d/205.enhancement +++ /dev/null @@ -1 +0,0 @@ -Disabled browsable HTML API in production (#205) diff --git a/changes/changelog.d/206.feature b/changes/changelog.d/206.feature deleted file mode 100644 index b334554fe..000000000 --- a/changes/changelog.d/206.feature +++ /dev/null @@ -1,17 +0,0 @@ -We now have a brand new instance settings interface in the front-end (#206) - - -Instance settings interface -^^^^^^^^^^^^^^^^^^^^^^^^^^^ - -Prior to this release, the only way to update instance settings (such as -instance description, signup policy, federation configuration, etc.) was using -the admin interface provided by Django (the back-end framework which power the API). - -This interface worked, but was not really-user friendly and intuitive. - -Starting from this release, we now offer a dedicated interface directly -in the front-end. You can view and edit all your instance settings from here, -assuming you have the required permissions. - -This interface is available at ``/manage/settings` and via link in the sidebar.