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/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/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/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_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'