diff --git a/api/config/settings/common.py b/api/config/settings/common.py index f1a383c587b123c39a6cbf3dcd5ad0de445a30cf..2b9580b049458c75a70a5ec18236cabcda1ac0d1 100644 --- a/api/config/settings/common.py +++ b/api/config/settings/common.py @@ -56,6 +56,10 @@ FEDERATION_COLLECTION_PAGE_SIZE = env.int( FEDERATION_MUSIC_NEEDS_APPROVAL = env.bool( 'FEDERATION_MUSIC_NEEDS_APPROVAL', default=True ) +# how much minutes to wait before refetching actor data +# when authenticating +FEDERATION_ACTOR_FETCH_DELAY = env.int( + 'FEDERATION_ACTOR_FETCH_DELAY', default=60 * 12) ALLOWED_HOSTS = env.list('DJANGO_ALLOWED_HOSTS') # APP CONFIGURATION diff --git a/api/funkwhale_api/federation/actors.py b/api/funkwhale_api/federation/actors.py index 380bb23c01e38e0651e3a3fc7ee0244025adcf34..a0e23d12f864c9ef29fc1b39e3bd1819e0cb9c62 100644 --- a/api/funkwhale_api/federation/actors.py +++ b/api/funkwhale_api/federation/actors.py @@ -1,3 +1,4 @@ +import datetime import logging import uuid import xml @@ -49,11 +50,20 @@ def get_actor_data(actor_url): def get_actor(actor_url): + try: + actor = models.Actor.objects.get(url=actor_url) + except models.Actor.DoesNotExist: + actor = None + fetch_delta = datetime.timedelta( + minutes=settings.FEDERATION_ACTOR_FETCH_DELAY) + if actor and actor.last_fetch_date > timezone.now() - fetch_delta: + # cache is hot, we can return as is + return actor data = get_actor_data(actor_url) serializer = serializers.ActorSerializer(data=data) serializer.is_valid(raise_exception=True) - return serializer.build() + return serializer.save(last_fetch_date=timezone.now()) class SystemActor(object): diff --git a/api/funkwhale_api/federation/authentication.py b/api/funkwhale_api/federation/authentication.py index 7f8ad6653995b4a0f3efd72dc82e4303da5d8760..bfd46084c0dbd2e7d4b8fd3dac67a837809586bf 100644 --- a/api/funkwhale_api/federation/authentication.py +++ b/api/funkwhale_api/federation/authentication.py @@ -25,28 +25,19 @@ class SignatureAuthentication(authentication.BaseAuthentication): raise exceptions.AuthenticationFailed(str(e)) try: - actor_data = actors.get_actor_data(key_id) + actor = actors.get_actor(key_id.split('#')[0]) except Exception as e: raise exceptions.AuthenticationFailed(str(e)) - try: - public_key = actor_data['publicKey']['publicKeyPem'] - except KeyError: + if not actor.public_key: raise exceptions.AuthenticationFailed('No public key found') - serializer = serializers.ActorSerializer(data=actor_data) - if not serializer.is_valid(): - raise exceptions.AuthenticationFailed('Invalid actor payload: {}'.format(serializer.errors)) - try: - signing.verify_django(request, public_key.encode('utf-8')) + signing.verify_django(request, actor.public_key.encode('utf-8')) except cryptography.exceptions.InvalidSignature: raise exceptions.AuthenticationFailed('Invalid signature') - try: - return models.Actor.objects.get(url=actor_data['id']) - except models.Actor.DoesNotExist: - return serializer.save() + return actor def authenticate(self, request): setattr(request, 'actor', None) diff --git a/api/funkwhale_api/federation/serializers.py b/api/funkwhale_api/federation/serializers.py index 38efdd3bf76fad277b6a6d9ea7836b18619a9aa8..426aabd771b1e5caaaa683648a123ccbe00aa986 100644 --- a/api/funkwhale_api/federation/serializers.py +++ b/api/funkwhale_api/federation/serializers.py @@ -103,9 +103,10 @@ class ActorSerializer(serializers.Serializer): def save(self, **kwargs): d = self.prepare_missing_fields() d.update(kwargs) - return models.Actor.objects.create( - **d - ) + return models.Actor.objects.update_or_create( + url=d['url'], + defaults=d, + )[0] def validate_summary(self, value): if value: diff --git a/api/tests/federation/test_actors.py b/api/tests/federation/test_actors.py index 7281147a1b143a9d48adce68bf124b8a57009933..f566e7a72aa4fd5bf7f1417856903d39535e4f66 100644 --- a/api/tests/federation/test_actors.py +++ b/api/tests/federation/test_actors.py @@ -29,6 +29,42 @@ def test_actor_fetching(r_mock): assert r == payload +def test_get_actor(factories, r_mock): + actor = factories['federation.Actor'].build() + payload = serializers.ActorSerializer(actor).data + r_mock.get(actor.url, json=payload) + new_actor = actors.get_actor(actor.url) + + assert new_actor.pk is not None + assert serializers.ActorSerializer(new_actor).data == payload + + +def test_get_actor_use_existing(factories, settings, mocker): + settings.FEDERATION_ACTOR_FETCH_DELAY = 60 + actor = factories['federation.Actor']() + get_data = mocker.patch('funkwhale_api.federation.actors.get_actor_data') + new_actor = actors.get_actor(actor.url) + + assert new_actor == actor + get_data.assert_not_called() + + +def test_get_actor_refresh(factories, settings, mocker): + settings.FEDERATION_ACTOR_FETCH_DELAY = 0 + actor = factories['federation.Actor']() + payload = serializers.ActorSerializer(actor).data + # actor changed their username in the meantime + payload['preferredUsername'] = 'New me' + get_data = mocker.patch( + 'funkwhale_api.federation.actors.get_actor_data', + return_value=payload) + new_actor = actors.get_actor(actor.url) + + assert new_actor == actor + assert new_actor.last_fetch_date > actor.last_fetch_date + assert new_actor.preferred_username == 'New me' + + def test_get_library(db, settings, mocker): get_key_pair = mocker.patch( 'funkwhale_api.federation.keys.get_key_pair', diff --git a/changes/changelog.d/actor-fetch.enhancement b/changes/changelog.d/actor-fetch.enhancement new file mode 100644 index 0000000000000000000000000000000000000000..17f3a88df2d961d2fbc7d60fcf9c876fd4ba96c8 --- /dev/null +++ b/changes/changelog.d/actor-fetch.enhancement @@ -0,0 +1 @@ +Avoid fetching Actor object on every request authentication diff --git a/front/src/components/federation/LibraryCard.vue b/front/src/components/federation/LibraryCard.vue index a16c80f7eebb611cf3e1617a9fa4dcb6c25aac9d..e7ef7a516e1eab553583f965b3443d134c7169d4 100644 --- a/front/src/components/federation/LibraryCard.vue +++ b/front/src/components/federation/LibraryCard.vue @@ -3,10 +3,12 @@ <div class="content"> <div class="header ellipsis"> <router-link + v-if="library" :title="displayName" :to="{name: 'federation.libraries.detail', params: {id: library.uuid }}"> {{ displayName }} </router-link> + <span :title="displayName" v-else>{{ displayName }}</span> </div> </div> <div class="content"> diff --git a/front/src/views/federation/LibraryDetail.vue b/front/src/views/federation/LibraryDetail.vue index 64f2cc7c6534bd73c94bde57b485755038d92758..7a842b679a967ed288f9805d1dbf684f2256d859 100644 --- a/front/src/views/federation/LibraryDetail.vue +++ b/front/src/views/federation/LibraryDetail.vue @@ -150,6 +150,7 @@ export default { methods: { fetchData () { var self = this + this.scanTrigerred = false this.isLoading = true let url = 'federation/libraries/' + this.id + '/' logger.default.debug('Fetching library "' + this.id + '"')