diff --git a/api/funkwhale_api/federation/api_views.py b/api/funkwhale_api/federation/api_views.py index 549bac917f843fda93ac4b6056845c64ef6cda5b..6b3f30e6660f578a216454e1f90f17a5ba4494fd 100644 --- a/api/funkwhale_api/federation/api_views.py +++ b/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, ) diff --git a/api/funkwhale_api/federation/serializers.py b/api/funkwhale_api/federation/serializers.py index 6ee219288c3c1f0f62c9c172aae293f6d396abc9..754df60a3fdb04403c6fbd93b46866a6c71a482a 100644 --- a/api/funkwhale_api/federation/serializers.py +++ b/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, ) diff --git a/api/funkwhale_api/federation/tasks.py b/api/funkwhale_api/federation/tasks.py index 4714003cf7883df22dc80e720c28e4ab5e47f54f..8aebcb27a8f5840821ae0aa6dfed6cbc043deffa 100644 --- a/api/funkwhale_api/federation/tasks.py +++ b/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, ) diff --git a/api/funkwhale_api/federation/utils.py b/api/funkwhale_api/federation/utils.py index 7351763e31e4cd758ac16b05f86daa5fd2024730..a32256921a55f8eee1a5c406255d224b433eb5b0 100644 --- a/api/funkwhale_api/federation/utils.py +++ b/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() diff --git a/api/funkwhale_api/music/models.py b/api/funkwhale_api/music/models.py index e40483c80c31c59911efaa21c44037772f768dbb..fb7af3adbf138c144685a9e5f7174b872383e231 100644 --- a/api/funkwhale_api/music/models.py +++ b/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 diff --git a/api/funkwhale_api/music/views.py b/api/funkwhale_api/music/views.py index e7897edbb0faf2ea70a5fe0cf3dd74b53a027533..d23f07ce6943778e3fecdfcff2cc2d4acd04a119 100644 --- a/api/funkwhale_api/music/views.py +++ b/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"] diff --git a/api/tests/conftest.py b/api/tests/conftest.py index 7a15c6d3883d713e1e6fd7eb07cc52d350e7e49b..c4f65ea183b822ca10d3dffcdd302aea5b2c1a72 100644 --- a/api/tests/conftest.py +++ b/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() diff --git a/api/tests/federation/test_tasks.py b/api/tests/federation/test_tasks.py index a643a12d8dbdce4df8bc142d1f95e582305e1778..5c699cd7216dd038cc3ce091cb37504ddd4a6ce3 100644 --- a/api/tests/federation/test_tasks.py +++ b/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) diff --git a/api/tests/federation/test_utils.py b/api/tests/federation/test_utils.py index a8d02cdb9cc30ae7468a01bbc5b787ce1da73b2e..9aa850728ef969ee8a7e764050e3846650d21358 100644 --- a/api/tests/federation/test_utils.py +++ b/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"} diff --git a/changes/changelog.d/758.bugfix b/changes/changelog.d/758.bugfix new file mode 100644 index 0000000000000000000000000000000000000000..3275ddbe1a32255b69fa4bc3ba2c6c7a44559d01 --- /dev/null +++ b/changes/changelog.d/758.bugfix @@ -0,0 +1 @@ +Ensure all our ActivityPub fetches are authenticated (#758)