See #152: use new user permissions on relevant viewsets

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.

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',

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 = []

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

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):

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

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()

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)
 

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'),

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'