diff --git a/api/funkwhale_api/federation/permissions.py b/api/funkwhale_api/federation/permissions.py index 370328eaa..c6f0660b1 100644 --- a/api/funkwhale_api/federation/permissions.py +++ b/api/funkwhale_api/federation/permissions.py @@ -16,4 +16,5 @@ class LibraryFollower(BasePermission): return False library = actors.SYSTEM_ACTORS['library'].get_actor_instance() - return library.followers.filter(url=actor.url).exists() + return library.received_follows.filter( + approved=True, actor=actor).exists() diff --git a/api/funkwhale_api/federation/serializers.py b/api/funkwhale_api/federation/serializers.py index 6ae6abb78..735a101b4 100644 --- a/api/funkwhale_api/federation/serializers.py +++ b/api/funkwhale_api/federation/serializers.py @@ -616,10 +616,12 @@ class CollectionPageSerializer(serializers.Serializer): if not item_serializer: return v raw_items = [item_serializer(data=i, context=self.context) for i in v] + valid_items = [] for i in raw_items: - i.is_valid(raise_exception=True) + if i.is_valid(): + valid_items.append(i) - return raw_items + return valid_items def to_representation(self, conf): page = conf['page'] diff --git a/api/tests/federation/test_permissions.py b/api/tests/federation/test_permissions.py index 1a6977542..9b8683210 100644 --- a/api/tests/federation/test_permissions.py +++ b/api/tests/federation/test_permissions.py @@ -30,11 +30,26 @@ def test_library_follower_actor_non_follower( assert check is False +def test_library_follower_actor_follower_not_approved( + factories, api_request, anonymous_user, settings): + settings.FEDERATION_MUSIC_NEEDS_APPROVAL = True + library = actors.SYSTEM_ACTORS['library'].get_actor_instance() + follow = factories['federation.Follow'](target=library, approved=False) + view = APIView.as_view() + permission = permissions.LibraryFollower() + request = api_request.get('/') + setattr(request, 'user', anonymous_user) + setattr(request, 'actor', follow.actor) + check = permission.has_permission(request, view) + + assert check is False + + def test_library_follower_actor_follower( factories, api_request, anonymous_user, settings): settings.FEDERATION_MUSIC_NEEDS_APPROVAL = True library = actors.SYSTEM_ACTORS['library'].get_actor_instance() - follow = factories['federation.Follow'](target=library) + follow = factories['federation.Follow'](target=library, approved=True) view = APIView.as_view() permission = permissions.LibraryFollower() request = api_request.get('/') diff --git a/api/tests/federation/test_serializers.py b/api/tests/federation/test_serializers.py index 6d33a529d..85208fa49 100644 --- a/api/tests/federation/test_serializers.py +++ b/api/tests/federation/test_serializers.py @@ -431,8 +431,14 @@ def test_collection_page_serializer_validation(): def test_collection_page_serializer_can_validate_child(): - base = 'https://test.federation/test' data = { + 'type': 'CollectionPage', + 'id': 'https://test.page?page=2', + 'actor': 'https://test.actor', + 'first': 'https://test.page?page=1', + 'last': 'https://test.page?page=3', + 'partOf': 'https://test.page', + 'totalItems': 1, 'items': [{'in': 'valid'}], } @@ -441,8 +447,9 @@ def test_collection_page_serializer_can_validate_child(): context={'item_serializer': serializers.AudioSerializer} ) - assert serializer.is_valid() is False - assert 'items' in serializer.errors + # child are validated but not included in data if not valid + assert serializer.is_valid(raise_exception=True) is True + assert len(serializer.validated_data['items']) == 0 def test_collection_page_serializer(factories): diff --git a/api/tests/federation/test_views.py b/api/tests/federation/test_views.py index 8c5235b8b..ae94bcdc0 100644 --- a/api/tests/federation/test_views.py +++ b/api/tests/federation/test_views.py @@ -43,7 +43,7 @@ def test_instance_endpoints_405_if_federation_disabled( def test_wellknown_webfinger_validates_resource( - db, api_client, settings, mocker): + db, api_client, settings, mocker): clean = mocker.spy(webfinger, 'clean_resource') url = reverse('federation:well-known-webfinger') response = api_client.get(url, data={'resource': 'something'}) diff --git a/changes/changelog.d/federation-1.bugfix b/changes/changelog.d/federation-1.bugfix new file mode 100644 index 000000000..371208e0e --- /dev/null +++ b/changes/changelog.d/federation-1.bugfix @@ -0,0 +1 @@ +Fixed broken permission check on library scanning and too aggressive page validation