Merge branch '758-authenticate-fetches' into 'develop'

Fix #758: Ensure all our ActivityPub fetches are authenticated

Closes #758

See merge request funkwhale/funkwhale!674

api/funkwhale_api/federation/api_views.py

@@ -132,6 +132,7 @@ class LibraryViewSet(mixins.RetrieveModelMixin, viewsets.GenericViewSet):
         try:
             library = utils.retrieve_ap_object(
                 fid,
+                actor=request.user.actor,
                 queryset=self.queryset,
                 serializer_class=serializers.LibrarySerializer,
             )

api/funkwhale_api/federation/serializers.py

@@ -631,6 +631,7 @@ class LibrarySerializer(PaginatedCollectionSerializer):
     def create(self, validated_data):
         actor = utils.retrieve_ap_object(
             validated_data["actor"],
+            actor=self.context.get("fetch_actor"),
             queryset=models.Actor,
             serializer_class=ActorSerializer,
         )

api/funkwhale_api/federation/tasks.py

@@ -15,6 +15,7 @@ from funkwhale_api.common import utils as common_utils
 from funkwhale_api.music import models as music_models
 from funkwhale_api.taskapp import celery
 
+from . import actors
 from . import keys
 from . import models, signing
 from . import serializers
@@ -195,6 +196,7 @@ def update_domain_nodeinfo(domain):
         domain.service_actor = (
             utils.retrieve_ap_object(
                 service_actor_id,
+                actor=actors.get_service_actor(),
                 queryset=models.Actor,
                 serializer_class=serializers.ActorSerializer,
             )

api/funkwhale_api/federation/utils.py

@@ -61,7 +61,7 @@ def slugify_username(username):
 
 
 def retrieve_ap_object(
-    fid, actor=None, serializer_class=None, queryset=None, apply_instance_policies=True
+    fid, actor, serializer_class=None, queryset=None, apply_instance_policies=True
 ):
     from . import activity
 
@@ -104,6 +104,6 @@ def retrieve_ap_object(
             raise exceptions.BlockedActorOrDomain()
     if not serializer_class:
         return data
-    serializer = serializer_class(data=data)
+    serializer = serializer_class(data=data, context={"fetch_actor": actor})
     serializer.is_valid(raise_exception=True)
     return serializer.save()

api/funkwhale_api/music/models.py

@@ -703,12 +703,12 @@ class Upload(models.Model):
 
     objects = UploadQuerySet.as_manager()
 
-    def download_audio_from_remote(self, user):
+    def download_audio_from_remote(self, actor):
         from funkwhale_api.common import session
         from funkwhale_api.federation import signing
 
-        if user.is_authenticated and user.actor:
-            auth = signing.get_auth(user.actor.private_key, user.actor.private_key_id)
+        if actor:
+            auth = signing.get_auth(actor.private_key, actor.private_key_id)
         else:
             auth = None
 

api/funkwhale_api/music/views.py

@@ -21,6 +21,7 @@ from funkwhale_api.common import preferences
 from funkwhale_api.common import utils as common_utils
 from funkwhale_api.common import views as common_views
 from funkwhale_api.federation.authentication import SignatureAuthentication
+from funkwhale_api.federation import actors
 from funkwhale_api.federation import api_serializers as federation_api_serializers
 from funkwhale_api.federation import routes
 
@@ -303,7 +304,11 @@ def handle_serve(upload, user, format=None):
             # thus resulting in multiple downloads from the remote
             qs = f.__class__.objects.select_for_update()
             f = qs.get(pk=f.pk)
-            f.download_audio_from_remote(user=user)
+            if user.is_authenticated:
+                actor = user.actor
+            else:
+                actor = actors.get_service_actor()
+            f.download_audio_from_remote(actor=actor)
         data = f.get_audio_data()
         if data:
             f.duration = data["duration"]

api/tests/conftest.py

@@ -28,6 +28,7 @@ from rest_framework import fields as rest_fields
 from rest_framework.test import APIClient, APIRequestFactory
 
 from funkwhale_api.activity import record
+from funkwhale_api.federation import actors
 from funkwhale_api.users.permissions import HasUserPermission
 
 
@@ -426,3 +427,8 @@ def rsa_small_key(settings):
 def a_responses():
     with aioresponses() as m:
         yield m
+
+
+@pytest.fixture
+def service_actor(db):
+    return actors.get_service_actor()

api/tests/federation/test_tasks.py

@@ -5,7 +5,10 @@ import pytest
 
 from django.utils import timezone
 
+from funkwhale_api.federation import models
+from funkwhale_api.federation import serializers
 from funkwhale_api.federation import tasks
+from funkwhale_api.federation import utils
 
 
 def test_clean_federation_music_cache_if_no_listen(preferences, factories):
@@ -162,9 +165,11 @@ def test_fetch_nodeinfo(factories, r_mock, now):
     assert tasks.fetch_nodeinfo("test.test") == {"hello": "world"}
 
 
-def test_update_domain_nodeinfo(factories, mocker, now):
+def test_update_domain_nodeinfo(factories, mocker, now, service_actor):
     domain = factories["federation.Domain"](nodeinfo_fetch_date=None)
     actor = factories["federation.Actor"](fid="https://actor.id")
+    retrieve_ap_object = mocker.spy(utils, "retrieve_ap_object")
+
     mocker.patch.object(
         tasks,
         "fetch_nodeinfo",
@@ -186,6 +191,13 @@ def test_update_domain_nodeinfo(factories, mocker, now):
     }
     assert domain.service_actor == actor
 
+    retrieve_ap_object.assert_called_once_with(
+        "https://actor.id",
+        actor=service_actor,
+        queryset=models.Actor,
+        serializer_class=serializers.ActorSerializer,
+    )
+
 
 def test_update_domain_nodeinfo_error(factories, r_mock, now):
     domain = factories["federation.Domain"](nodeinfo_fetch_date=None)

api/tests/federation/test_utils.py

@@ -56,7 +56,7 @@ def test_extract_headers_from_meta():
 def test_retrieve_ap_object(db, r_mock):
     fid = "https://some.url"
     m = r_mock.get(fid, json={"hello": "world"})
-    result = utils.retrieve_ap_object(fid)
+    result = utils.retrieve_ap_object(fid, actor=None)
 
     assert result == {"hello": "world"}
     assert m.request_history[-1].headers["Accept"] == "application/activity+json"
@@ -69,7 +69,7 @@ def test_retrieve_ap_object_honor_instance_policy_domain(factories):
     fid = "https://{}/test".format(domain.name)
 
     with pytest.raises(exceptions.BlockedActorOrDomain):
-        utils.retrieve_ap_object(fid)
+        utils.retrieve_ap_object(fid, actor=None)
 
 
 def test_retrieve_ap_object_honor_instance_policy_different_url_and_id(
@@ -82,7 +82,7 @@ def test_retrieve_ap_object_honor_instance_policy_different_url_and_id(
     r_mock.get(fid, json={"id": "http://{}/test".format(domain.name)})
 
     with pytest.raises(exceptions.BlockedActorOrDomain):
-        utils.retrieve_ap_object(fid)
+        utils.retrieve_ap_object(fid, actor=None)
 
 
 def test_retrieve_with_actor(r_mock, factories):
@@ -99,7 +99,7 @@ def test_retrieve_with_actor(r_mock, factories):
 def test_retrieve_with_queryset(factories):
     actor = factories["federation.Actor"]()
 
-    assert utils.retrieve_ap_object(actor.fid, queryset=actor.__class__)
+    assert utils.retrieve_ap_object(actor.fid, actor=None, queryset=actor.__class__)
 
 
 def test_retrieve_with_serializer(db, r_mock):
@@ -109,6 +109,6 @@ def test_retrieve_with_serializer(db, r_mock):
 
     fid = "https://some.url"
     r_mock.get(fid, json={"hello": "world"})
-    result = utils.retrieve_ap_object(fid, serializer_class=S)
+    result = utils.retrieve_ap_object(fid, actor=None, serializer_class=S)
 
     assert result == {"persisted": "object"}

changes/changelog.d/758.bugfix

@@ -0,0 +1 @@
+Ensure all our ActivityPub fetches are authenticated (#758)