From da3710ff086885c75c159e49db8e2509d23a0a86 Mon Sep 17 00:00:00 2001
From: Eliot Berriot <contact@eliotberriot.com>
Date: Fri, 15 Mar 2019 12:08:45 +0100
Subject: [PATCH] Fix #758: Ensure all our ActivityPub fetches are
authenticated
---
api/funkwhale_api/federation/api_views.py | 1 +
api/funkwhale_api/federation/serializers.py | 1 +
api/funkwhale_api/federation/tasks.py | 2 ++
api/funkwhale_api/federation/utils.py | 4 ++--
api/funkwhale_api/music/models.py | 6 +++---
api/funkwhale_api/music/views.py | 7 ++++++-
api/tests/conftest.py | 6 ++++++
api/tests/federation/test_tasks.py | 14 +++++++++++++-
api/tests/federation/test_utils.py | 10 +++++-----
changes/changelog.d/758.bugfix | 1 +
10 files changed, 40 insertions(+), 12 deletions(-)
create mode 100644 changes/changelog.d/758.bugfix
diff --git a/api/funkwhale_api/federation/api_views.py b/api/funkwhale_api/federation/api_views.py
index 549bac917f..6b3f30e666 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 6ee219288c..754df60a3f 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 4714003cf7..8aebcb27a8 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 7351763e31..a32256921a 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 e40483c80c..fb7af3adbf 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 e7897edbb0..d23f07ce69 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 7a15c6d388..c4f65ea183 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 a643a12d8d..5c699cd721 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 a8d02cdb9c..9aa850728e 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 0000000000..3275ddbe1a
--- /dev/null
+++ b/changes/changelog.d/758.bugfix
@@ -0,0 +1 @@
+Ensure all our ActivityPub fetches are authenticated (#758)
--
GitLab