From cd109ddeb64374e08576f33f361659851e6c7788 Mon Sep 17 00:00:00 2001 From: Agate Date: Tue, 18 Aug 2020 20:40:02 +0200 Subject: [PATCH] Fix #858: Broadcast/handle rejected follows --- api/funkwhale_api/federation/api_views.py | 4 +- api/funkwhale_api/federation/routes.py | 31 +++++++++++++++ api/funkwhale_api/federation/serializers.py | 30 ++++++++++++--- api/tests/federation/test_api_views.py | 9 ++--- api/tests/federation/test_routes.py | 42 +++++++++++++++++++++ api/tests/federation/test_serializers.py | 16 ++++++++ changes/changelog.d/858.enhancement | 1 + 7 files changed, 120 insertions(+), 13 deletions(-) create mode 100644 changes/changelog.d/858.enhancement diff --git a/api/funkwhale_api/federation/api_views.py b/api/funkwhale_api/federation/api_views.py index 1cf7fb7f2..f49c51c35 100644 --- a/api/funkwhale_api/federation/api_views.py +++ b/api/funkwhale_api/federation/api_views.py @@ -34,6 +34,8 @@ def update_follow(follow, approved): follow.save(update_fields=["approved"]) if approved: routes.outbox.dispatch({"type": "Accept"}, context={"follow": follow}) + else: + routes.outbox.dispatch({"type": "Reject"}, context={"follow": follow}) class LibraryFollowViewSet( @@ -57,7 +59,7 @@ class LibraryFollowViewSet( def get_queryset(self): qs = super().get_queryset() - return qs.filter(actor=self.request.user.actor) + return qs.filter(actor=self.request.user.actor).exclude(approved=False) def perform_create(self, serializer): follow = serializer.save(actor=self.request.user.actor) diff --git a/api/funkwhale_api/federation/routes.py b/api/funkwhale_api/federation/routes.py index fef0d2b9a..fa98f736f 100644 --- a/api/funkwhale_api/federation/routes.py +++ b/api/funkwhale_api/federation/routes.py @@ -82,6 +82,37 @@ def outbox_accept(context): } +@outbox.register({"type": "Reject"}) +def outbox_reject_follow(context): + follow = context["follow"] + if follow._meta.label == "federation.LibraryFollow": + actor = follow.target.actor + else: + actor = follow.target + payload = serializers.RejectFollowSerializer(follow, context={"actor": actor}).data + yield { + "actor": actor, + "type": "Reject", + "payload": with_recipients(payload, to=[follow.actor]), + "object": follow, + "related_object": follow.target, + } + + +@inbox.register({"type": "Reject"}) +def inbox_reject_follow(payload, context): + serializer = serializers.RejectFollowSerializer(data=payload, context=context) + if not serializer.is_valid(raise_exception=context.get("raise_exception", False)): + logger.debug( + "Discarding invalid follow reject from %s: %s", + context["actor"].fid, + serializer.errors, + ) + return + + serializer.save() + + @inbox.register({"type": "Undo", "object.type": "Follow"}) def inbox_undo_follow(payload, context): serializer = serializers.UndoFollowSerializer(data=payload, context=context) diff --git a/api/funkwhale_api/federation/serializers.py b/api/funkwhale_api/federation/serializers.py index 9f68a2375..a026e8fb2 100644 --- a/api/funkwhale_api/federation/serializers.py +++ b/api/funkwhale_api/federation/serializers.py @@ -688,11 +688,10 @@ class APIFollowSerializer(serializers.ModelSerializer): ] -class AcceptFollowSerializer(serializers.Serializer): +class FollowActionSerializer(serializers.Serializer): id = serializers.URLField(max_length=500, required=False) actor = serializers.URLField(max_length=500) object = FollowSerializer() - type = serializers.ChoiceField(choices=["Accept"]) def validate_actor(self, v): expected = self.context.get("actor") @@ -720,12 +719,13 @@ class AcceptFollowSerializer(serializers.Serializer): follow_class.objects.filter( target=target, actor=validated_data["object"]["actor"] ) - .exclude(approved=True) .select_related() .get() ) except follow_class.DoesNotExist: - raise serializers.ValidationError("No follow to accept") + raise serializers.ValidationError( + "No follow to {}".format(self.action_type) + ) return validated_data def to_representation(self, instance): @@ -736,12 +736,18 @@ class AcceptFollowSerializer(serializers.Serializer): return { "@context": jsonld.get_default_context(), - "id": instance.get_federation_id() + "/accept", - "type": "Accept", + "id": instance.get_federation_id() + "/{}".format(self.action_type), + "type": self.action_type.title(), "actor": actor.fid, "object": FollowSerializer(instance).data, } + +class AcceptFollowSerializer(FollowActionSerializer): + + type = serializers.ChoiceField(choices=["Accept"]) + action_type = "accept" + def save(self): follow = self.validated_data["follow"] follow.approved = True @@ -751,6 +757,18 @@ class AcceptFollowSerializer(serializers.Serializer): return follow +class RejectFollowSerializer(FollowActionSerializer): + + type = serializers.ChoiceField(choices=["Reject"]) + action_type = "reject" + + def save(self): + follow = self.validated_data["follow"] + follow.approved = False + follow.save() + return follow + + class UndoFollowSerializer(serializers.Serializer): id = serializers.URLField(max_length=500) actor = serializers.URLField(max_length=500) diff --git a/api/tests/federation/test_api_views.py b/api/tests/federation/test_api_views.py index b9459c3d3..e0fcc6edf 100644 --- a/api/tests/federation/test_api_views.py +++ b/api/tests/federation/test_api_views.py @@ -126,12 +126,9 @@ def test_user_can_accept_or_reject_own_follows( assert follow.approved is expected - if action == "accept": - mocked_dispatch.assert_called_once_with( - {"type": "Accept"}, context={"follow": follow} - ) - if action == "reject": - mocked_dispatch.assert_not_called() + mocked_dispatch.assert_called_once_with( + {"type": action.title()}, context={"follow": follow} + ) def test_user_can_list_inbox_items(factories, logged_in_api_client): diff --git a/api/tests/federation/test_routes.py b/api/tests/federation/test_routes.py index 1bfd5e088..753f286b7 100644 --- a/api/tests/federation/test_routes.py +++ b/api/tests/federation/test_routes.py @@ -16,6 +16,7 @@ from funkwhale_api.moderation import serializers as moderation_serializers [ ({"type": "Follow"}, routes.inbox_follow), ({"type": "Accept"}, routes.inbox_accept), + ({"type": "Reject"}, routes.inbox_reject_follow), ({"type": "Create", "object": {"type": "Audio"}}, routes.inbox_create_audio), ( {"type": "Update", "object": {"type": "Library"}}, @@ -51,6 +52,7 @@ def test_inbox_routes(route, handler): ({"type": "Accept"}, routes.outbox_accept), ({"type": "Flag"}, routes.outbox_flag), ({"type": "Follow"}, routes.outbox_follow), + ({"type": "Reject"}, routes.outbox_reject_follow), ({"type": "Create", "object": {"type": "Audio"}}, routes.outbox_create_audio), ( {"type": "Update", "object": {"type": "Library"}}, @@ -669,6 +671,46 @@ def test_outbox_delete_follow_library(factories): assert activity["related_object"] == follow.target +def test_inbox_reject_follow_library(factories): + local_actor = factories["users.User"]().create_actor() + remote_actor = factories["federation.Actor"]() + follow = factories["federation.LibraryFollow"]( + actor=local_actor, target__actor=remote_actor, approved=True + ) + assert follow.approved is True + serializer = serializers.RejectFollowSerializer( + follow, context={"actor": remote_actor} + ) + ii = factories["federation.InboxItem"](actor=local_actor) + routes.inbox_reject_follow( + serializer.data, + context={"actor": remote_actor, "inbox_items": [ii], "raise_exception": True}, + ) + follow.refresh_from_db() + assert follow.approved is False + + +def test_outbox_reject_follow_library(factories): + remote_actor = factories["federation.Actor"]() + local_actor = factories["federation.Actor"](local=True) + follow = factories["federation.LibraryFollow"]( + actor=remote_actor, target__actor=local_actor + ) + + activity = list(routes.outbox_reject_follow({"follow": follow}))[0] + + serializer = serializers.RejectFollowSerializer( + follow, context={"actor": local_actor} + ) + expected = serializer.data + expected["to"] = [remote_actor] + + assert activity["payload"] == expected + assert activity["actor"] == local_actor + assert activity["object"] == follow + assert activity["related_object"] == follow.target + + def test_handle_library_entry_update_can_manage(factories, mocker): update_library_entity = mocker.patch( "funkwhale_api.music.tasks.update_library_entity" diff --git a/api/tests/federation/test_serializers.py b/api/tests/federation/test_serializers.py index 6617ba07d..af774ab03 100644 --- a/api/tests/federation/test_serializers.py +++ b/api/tests/federation/test_serializers.py @@ -318,6 +318,22 @@ def test_accept_follow_serializer_validates_on_context(factories): assert "object" in serializer.errors["object"] +def test_reject_follow_serializer_representation(factories): + follow = factories["federation.Follow"](approved=None) + + expected = { + "@context": jsonld.get_default_context(), + "id": follow.get_federation_id() + "/reject", + "type": "Reject", + "actor": follow.target.fid, + "object": serializers.FollowSerializer(follow).data, + } + + serializer = serializers.RejectFollowSerializer(follow) + + assert serializer.data == expected + + def test_undo_follow_serializer_representation(factories): follow = factories["federation.Follow"](approved=True) diff --git a/changes/changelog.d/858.enhancement b/changes/changelog.d/858.enhancement new file mode 100644 index 000000000..7e0135e7c --- /dev/null +++ b/changes/changelog.d/858.enhancement @@ -0,0 +1 @@ +Broadcast/handle rejected follows (#858) \ No newline at end of file