From 22bd1512c7877c4c6c5c38b08a257d91e02dbc5d Mon Sep 17 00:00:00 2001 From: Eliot Berriot Date: Thu, 18 Jul 2019 12:08:20 +0200 Subject: [PATCH] Ensure owner of tracks/albums/artists can approve suggestions --- api/funkwhale_api/music/mutations.py | 7 ++++- api/funkwhale_api/music/serializers.py | 22 ++++++++++++++++ api/funkwhale_api/music/views.py | 12 ++++++--- api/tests/music/test_mutations.py | 36 ++++++++++++++++++++++++++ api/tests/music/test_serializers.py | 16 +++++++++--- 5 files changed, 86 insertions(+), 7 deletions(-) diff --git a/api/funkwhale_api/music/mutations.py b/api/funkwhale_api/music/mutations.py index ebbe00f8d..b149f1963 100644 --- a/api/funkwhale_api/music/mutations.py +++ b/api/funkwhale_api/music/mutations.py @@ -11,7 +11,12 @@ def can_suggest(obj, actor): def can_approve(obj, actor): - return obj.is_local and actor.user and actor.user.get_permissions()["library"] + if not obj.is_local or not actor.user: + return False + + return ( + actor.id is not None and actor.id == obj.attributed_to_id + ) or actor.user.get_permissions()["library"] class TagMutation(mutations.UpdateMutationSerializer): diff --git a/api/funkwhale_api/music/serializers.py b/api/funkwhale_api/music/serializers.py index 5a01772ee..dac50960c 100644 --- a/api/funkwhale_api/music/serializers.py +++ b/api/funkwhale_api/music/serializers.py @@ -19,6 +19,16 @@ from . import filters, models, tasks cover_field = VersatileImageFieldSerializer(allow_null=True, sizes="square") +def serialize_attributed_to(self, obj): + # Import at runtime to avoid a circular import issue + from funkwhale_api.federation import serializers as federation_serializers + + if not obj.attributed_to_id: + return + + return federation_serializers.APIActorSerializer(obj.attributed_to).data + + class LicenseSerializer(serializers.Serializer): id = serializers.SerializerMethodField() url = serializers.URLField() @@ -68,6 +78,7 @@ class ArtistAlbumSerializer(serializers.ModelSerializer): class ArtistWithAlbumsSerializer(serializers.ModelSerializer): albums = ArtistAlbumSerializer(many=True, read_only=True) tags = serializers.SerializerMethodField() + attributed_to = serializers.SerializerMethodField() class Meta: model = models.Artist @@ -80,12 +91,15 @@ class ArtistWithAlbumsSerializer(serializers.ModelSerializer): "albums", "is_local", "tags", + "attributed_to", ) def get_tags(self, obj): tagged_items = getattr(obj, "_prefetched_tagged_items", []) return [ti.tag.name for ti in tagged_items] + get_attributed_to = serialize_attributed_to + class ArtistSimpleSerializer(serializers.ModelSerializer): class Meta: @@ -139,6 +153,7 @@ class AlbumSerializer(serializers.ModelSerializer): cover = cover_field is_playable = serializers.SerializerMethodField() tags = serializers.SerializerMethodField() + attributed_to = serializers.SerializerMethodField() class Meta: model = models.Album @@ -155,8 +170,11 @@ class AlbumSerializer(serializers.ModelSerializer): "is_playable", "is_local", "tags", + "attributed_to", ) + get_attributed_to = serialize_attributed_to + def get_tracks(self, o): ordered_tracks = o.tracks.all() return AlbumTrackSerializer(ordered_tracks, many=True).data @@ -213,6 +231,7 @@ class TrackSerializer(serializers.ModelSerializer): uploads = serializers.SerializerMethodField() listen_url = serializers.SerializerMethodField() tags = serializers.SerializerMethodField() + attributed_to = serializers.SerializerMethodField() class Meta: model = models.Track @@ -232,8 +251,11 @@ class TrackSerializer(serializers.ModelSerializer): "license", "is_local", "tags", + "attributed_to", ) + get_attributed_to = serialize_attributed_to + def get_listen_url(self, obj): return obj.listen_url diff --git a/api/funkwhale_api/music/views.py b/api/funkwhale_api/music/views.py index bc405571d..90b833a2b 100644 --- a/api/funkwhale_api/music/views.py +++ b/api/funkwhale_api/music/views.py @@ -60,7 +60,7 @@ def get_libraries(filter_uploads): class ArtistViewSet(common_views.SkipFilterForGetObject, viewsets.ReadOnlyModelViewSet): - queryset = models.Artist.objects.all() + queryset = models.Artist.objects.all().select_related("attributed_to") serializer_class = serializers.ArtistWithAlbumsSerializer permission_classes = [oauth_permissions.ScopePermission] required_scope = "libraries" @@ -92,7 +92,9 @@ class ArtistViewSet(common_views.SkipFilterForGetObject, viewsets.ReadOnlyModelV class AlbumViewSet(common_views.SkipFilterForGetObject, viewsets.ReadOnlyModelViewSet): queryset = ( - models.Album.objects.all().order_by("artist", "release_date").select_related() + models.Album.objects.all() + .order_by("artist", "release_date") + .select_related("artist", "attributed_to") ) serializer_class = serializers.AlbumSerializer permission_classes = [oauth_permissions.ScopePermission] @@ -188,7 +190,11 @@ class TrackViewSet(common_views.SkipFilterForGetObject, viewsets.ReadOnlyModelVi A simple ViewSet for viewing and editing accounts. """ - queryset = models.Track.objects.all().for_nested_serialization() + queryset = ( + models.Track.objects.all() + .for_nested_serialization() + .select_related("attributed_to") + ) serializer_class = serializers.TrackSerializer permission_classes = [oauth_permissions.ScopePermission] required_scope = "libraries" diff --git a/api/tests/music/test_mutations.py b/api/tests/music/test_mutations.py index 260916bfa..3a86f3bf8 100644 --- a/api/tests/music/test_mutations.py +++ b/api/tests/music/test_mutations.py @@ -2,6 +2,8 @@ import datetime import pytest from funkwhale_api.music import licenses +from funkwhale_api.music import mutations + from funkwhale_api.tags import models as tags_models @@ -140,3 +142,37 @@ def test_mutation_set_tags(factory_name, factories, now, mocker): {"type": "Update", "object": {"type": obj_type}}, context={obj_type.lower(): obj}, ) + + +@pytest.mark.parametrize("is_local, expected", [(True, True), (False, False)]) +def test_perm_checkers_can_suggest(factories, is_local, expected): + obj = factories["music.Track"](local=is_local) + assert mutations.can_suggest(obj, actor=None) is expected + + +@pytest.mark.parametrize( + "is_local, permission_library, actor_is_attributed, expected", + [ + # Not local object, so local users can't edit + (False, False, False, False), + (False, True, False, False), + # Local but no specific conditions met for permission + (True, False, False, False), + # Local and attributed_to -> ok + (True, False, True, True), + # Local and library permission -> ok + (True, True, False, True), + ], +) +def test_perm_checkers_can_approve( + factories, is_local, permission_library, actor_is_attributed, expected +): + actor = factories["users.User"]( + permission_library=permission_library + ).create_actor() + obj_kwargs = {"local": is_local} + if actor_is_attributed: + obj_kwargs["attributed_to"] = actor + obj = factories["music.Track"](**obj_kwargs) + + assert mutations.can_approve(obj, actor=actor) is expected diff --git a/api/tests/music/test_serializers.py b/api/tests/music/test_serializers.py index af33e05ed..8582a761c 100644 --- a/api/tests/music/test_serializers.py +++ b/api/tests/music/test_serializers.py @@ -1,5 +1,6 @@ import pytest +from funkwhale_api.federation import serializers as federation_serializers from funkwhale_api.music import licenses from funkwhale_api.music import models from funkwhale_api.music import serializers @@ -56,7 +57,8 @@ def test_artist_album_serializer(factories, to_api_date): def test_artist_with_albums_serializer(factories, to_api_date): - track = factories["music.Track"]() + actor = factories["federation.Actor"]() + track = factories["music.Track"](album__artist__attributed_to=actor) artist = track.artist artist = artist.__class__.objects.with_albums().get(pk=artist.pk) album = list(artist.albums.all())[0] @@ -70,6 +72,7 @@ def test_artist_with_albums_serializer(factories, to_api_date): "creation_date": to_api_date(artist.creation_date), "albums": [serializers.ArtistAlbumSerializer(album).data], "tags": [], + "attributed_to": federation_serializers.APIActorSerializer(actor).data, } serializer = serializers.ArtistWithAlbumsSerializer(artist) assert serializer.data == expected @@ -156,7 +159,8 @@ def test_upload_owner_serializer(factories, to_api_date): def test_album_serializer(factories, to_api_date): - track1 = factories["music.Track"](position=2) + actor = factories["federation.Actor"]() + track1 = factories["music.Track"](position=2, album__attributed_to=actor) track2 = factories["music.Track"](position=1, album=track1.album) album = track1.album expected = { @@ -177,6 +181,7 @@ def test_album_serializer(factories, to_api_date): "tracks": serializers.AlbumTrackSerializer([track2, track1], many=True).data, "is_local": album.is_local, "tags": [], + "attributed_to": federation_serializers.APIActorSerializer(actor).data, } serializer = serializers.AlbumSerializer(album) @@ -184,8 +189,12 @@ def test_album_serializer(factories, to_api_date): def test_track_serializer(factories, to_api_date): + actor = factories["federation.Actor"]() upload = factories["music.Upload"]( - track__license="cc-by-4.0", track__copyright="test", track__disc_number=2 + track__license="cc-by-4.0", + track__copyright="test", + track__disc_number=2, + track__attributed_to=actor, ) track = upload.track setattr(track, "playable_uploads", [upload]) @@ -205,6 +214,7 @@ def test_track_serializer(factories, to_api_date): "copyright": upload.track.copyright, "is_local": upload.track.is_local, "tags": [], + "attributed_to": federation_serializers.APIActorSerializer(actor).data, } serializer = serializers.TrackSerializer(track) assert serializer.data == expected