diff --git a/api/funkwhale_api/common/management/__init__.py b/api/funkwhale_api/common/management/__init__.py
new file mode 100644
index 0000000000000000000000000000000000000000..e69de29bb2d1d6434b8b29ae775ad8c2e48c5391
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 0000000000000000000000000000000000000000..e69de29bb2d1d6434b8b29ae775ad8c2e48c5391
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 0000000000000000000000000000000000000000..9d26a5836d46906967e24662a94bab553fa15880
--- /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 <script_name>')
+ 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/permissions.py b/api/funkwhale_api/common/permissions.py
index cab4b699d2a518cffc52aaf0cbad4473dcc28a6f..e9e8b8819f4b70e9ada3a0bcf71c709c9a5ef1de 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/common/scripts/__init__.py b/api/funkwhale_api/common/scripts/__init__.py
new file mode 100644
index 0000000000000000000000000000000000000000..4b2d525202218c43483dae60b6df4f4c6090723e
--- /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 0000000000000000000000000000000000000000..1bc971f80911de276ef2c210fafddfc052d71ee6
--- /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 0000000000000000000000000000000000000000..ab401dca4a7a46be53df6ffc1a09cd6657407ea8
--- /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/federation/views.py b/api/funkwhale_api/federation/views.py
index 37ad9ebfd1a481ae0d882d064ea285f6b7fd4424..06a2cd040cfc0d94fe51ab419bf6159c6e14f631 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 e6725e24846500f86bfbf9105d55b2a6780dc5dd..b905acd3e6c1ce78811e44f691b17506b041ab6f 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 f2ab72c5a020f4745a932dbbb50c3233030c9497..e71d3555e67a21012ee34d79e2fd56b28354fcf8 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/admin.py b/api/funkwhale_api/users/admin.py
index 89b67d3df96de05b9ffd8fff9ce027f67f101318..7e9062a1308a298fb83453fa0cb1c53c5740dd51 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,33 @@ 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')}),
+ (_('Useless fields'), {
+ 'fields': (
+ 'user_permissions',
+ 'groups',
+ )})
+ )
diff --git a/api/funkwhale_api/users/factories.py b/api/funkwhale_api/users/factories.py
index 12307f7fd109407f025b05dd364146204a59ae27..cd28f44073ded3ad425ee3c1f84a7c4b6dcabec6 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/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 0000000000000000000000000000000000000000..7c9ab0fadc99016e8a62f609ace117ea0b941965
--- /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 8273507c49bb23f1986b6b691fe90cc1fc8fea45..c16cd62b319c01d4a9f1802c531d279f0ef802ed 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,32 @@ class User(AbstractUser):
subsonic_api_token = models.CharField(
blank=True, null=True, max_length=255)
+ # permissions
+ 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
- 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 0000000000000000000000000000000000000000..2ff49ff3fa6661aecd0616fdb87b5b43f89a95c2
--- /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/funkwhale_api/users/serializers.py b/api/funkwhale_api/users/serializers.py
index eadce6154fa12c385f0655d3667b1634442716ec..3a095e78aa727b52ed35153cfffc8419b6142172 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/common/test_scripts.py b/api/tests/common/test_scripts.py
new file mode 100644
index 0000000000000000000000000000000000000000..ce478ba048043b6c1da61d0febf72fd91e8c2444
--- /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
diff --git a/api/tests/conftest.py b/api/tests/conftest.py
index dda537801f3cf66efb7eec4e823816e5dcec8207..b7a7d071ab6e8e548b7026b84ff815c0f2e6e43a 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 fd6ac2eb2ef07e581cd8b6bd473a0e65a0e30125..10237ed9fd76d156656238edbce11495dc08934b 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 6d8dcac3eebe1470da291a783bb7ad97a8b0c127..daf54db51cb32181c380fca8719bc951c97404c6 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 e641b45d517068520101651a98fd6f1318a2954d..030fc3a73eeeabaf0507d5bdbc9949e3d5c4bff5 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_models.py b/api/tests/users/test_models.py
index c7cd12e9e3ba19a457fa0d66d68e6b3679925c84..49199e0a781b990268431f109ac0c8804f8391ea 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 0000000000000000000000000000000000000000..1564c761db59239eee5169bffd6ee92c2c148301
--- /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
diff --git a/api/tests/users/test_views.py b/api/tests/users/test_views.py
index fffc762fde7cb6f6533f3781bc73e828fa8e5d56..1bbf8b9a2d065735a017802bba369e817d016992 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'
diff --git a/changes/changelog.d/152.feature b/changes/changelog.d/152.feature
new file mode 100644
index 0000000000000000000000000000000000000000..a10225288fd67723d2af1e337dfe7ad399317e81
--- /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 b7df2db42079304fe9956a5460ea6f9beb4e746d..46756bb266ccf918314f57b8ef8b01dcb68409ce 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/``.
diff --git a/front/src/components/About.vue b/front/src/components/About.vue
index b0ae67ef7ae554aa70a2ed6cc595b45fea81b32d..59c8411ac131fe65ec72fa6ed1e4769eed5b361a 100644
--- a/front/src/components/About.vue
+++ b/front/src/components/About.vue
@@ -15,7 +15,7 @@
</p>
<router-link
class="ui button"
- v-if="$store.state.auth.availablePermissions['settings.change']"
+ v-if="$store.state.auth.availablePermissions['settings']"
:to="{path: '/manage/settings', hash: 'instance'}">
<i class="pencil icon"></i>{{ $t('Edit instance info') }}
</router-link>
diff --git a/front/src/components/Sidebar.vue b/front/src/components/Sidebar.vue
index 9fbc5605c8e13923379e879b4b1a6b3322d14333..9f3134c2a1cd38a64a1761c505eac57fa6970666 100644
--- a/front/src/components/Sidebar.vue
+++ b/front/src/components/Sidebar.vue
@@ -60,7 +60,7 @@
<div class="menu">
<router-link
class="item"
- v-if="$store.state.auth.availablePermissions['import.launch']"
+ v-if="$store.state.auth.availablePermissions['library']"
:to="{name: 'library.requests', query: {status: 'pending' }}">
<i class="download icon"></i>{{ $t('Import requests') }}
<div
@@ -70,7 +70,7 @@
</router-link>
<router-link
class="item"
- v-if="$store.state.auth.availablePermissions['federation.manage']"
+ v-if="$store.state.auth.availablePermissions['federation']"
:to="{path: '/manage/federation/libraries'}">
<i class="sitemap icon"></i>{{ $t('Federation') }}
<div
@@ -80,7 +80,7 @@
</router-link>
<router-link
class="item"
- v-if="$store.state.auth.availablePermissions['settings.change']"
+ v-if="$store.state.auth.availablePermissions['settings']"
:to="{path: '/manage/settings'}">
<i class="settings icon"></i>{{ $t('Settings') }}
</router-link>
@@ -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 820d60a5438545f1af49bd9bcf597b8c227534b9..e360ccb1c5c80985e5adc33f203ff316c610f504 100644
--- a/front/src/components/library/Library.vue
+++ b/front/src/components/library/Library.vue
@@ -13,10 +13,10 @@
exact>
<i18next path="Requests"/>
</router-link>
- <router-link v-if="$store.state.auth.availablePermissions['import.launch']" class="ui item" to="/library/import/launch" exact>
+ <router-link v-if="$store.state.auth.availablePermissions['library']" class="ui item" to="/library/import/launch" exact>
<i18next path="Import"/>
</router-link>
- <router-link v-if="$store.state.auth.availablePermissions['import.launch']" class="ui item" to="/library/import/batches">
+ <router-link v-if="$store.state.auth.availablePermissions['library']" class="ui item" to="/library/import/batches">
<i18next path="Import batches"/>
</router-link>
</div>
diff --git a/front/src/components/requests/Card.vue b/front/src/components/requests/Card.vue
index 9d0bf0b771bdaa398c882e84be4f07afb642c6ce..7743bb6d4403653fedbc355983ec0bb4e7adecf1 100644
--- a/front/src/components/requests/Card.vue
+++ b/front/src/components/requests/Card.vue
@@ -22,7 +22,7 @@
</span>
<button
@click="createImport"
- v-if="request.status === 'pending' && importAction && $store.state.auth.availablePermissions['import.launch']"
+ v-if="request.status === 'pending' && importAction && $store.state.auth.availablePermissions['library']"
class="ui mini basic green right floated button">{{ $t('Create import') }}</button>
</div>
diff --git a/front/src/store/auth.js b/front/src/store/auth.js
index 68a15090b5c289d2825563743f4cad7f2d3cdbf0..87af081d26fdfd09d2d02b16db6196f91d6cee95 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 3d175e9f9fa0e31b3b51b79314fe182e0516549c..46901cd16edd25ee9be4f7137a8835f1336174fb 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/', {