From a3505d2099ae4a7816b79b826c445b6f0eca5cae Mon Sep 17 00:00:00 2001 From: Eliot Berriot Date: Fri, 14 Feb 2020 11:50:30 +0100 Subject: [PATCH 1/3] See #170: limit the amount of channels allowed per user --- .../audio/dynamic_preferences_registry.py | 9 ++++++ api/funkwhale_api/audio/serializers.py | 6 ++++ api/funkwhale_api/audio/views.py | 2 ++ api/tests/audio/test_serializers.py | 28 +++++++++++++++++-- front/src/views/admin/Settings.vue | 9 ++++++ 5 files changed, 52 insertions(+), 2 deletions(-) diff --git a/api/funkwhale_api/audio/dynamic_preferences_registry.py b/api/funkwhale_api/audio/dynamic_preferences_registry.py index 8f9b096b0..3dfcc8558 100644 --- a/api/funkwhale_api/audio/dynamic_preferences_registry.py +++ b/api/funkwhale_api/audio/dynamic_preferences_registry.py @@ -14,3 +14,12 @@ class ChannelsEnabled(types.BooleanPreference): "If disabled, the channels feature will be completely switched off, " "and users won't be able to create channels or subscribe to them." ) + + +@global_preferences_registry.register +class MaxChannels(types.IntegerPreference): + show_in_api = True + section = audio + default = 20 + name = "max_channels" + verbose_name = "Max channels allowed per user" diff --git a/api/funkwhale_api/audio/serializers.py b/api/funkwhale_api/audio/serializers.py index 3f953436c..70c51587e 100644 --- a/api/funkwhale_api/audio/serializers.py +++ b/api/funkwhale_api/audio/serializers.py @@ -5,6 +5,7 @@ from rest_framework import serializers from funkwhale_api.common import serializers as common_serializers from funkwhale_api.common import utils as common_utils from funkwhale_api.common import locales +from funkwhale_api.common import preferences from funkwhale_api.federation import serializers as federation_serializers from funkwhale_api.federation import utils as federation_utils from funkwhale_api.music import models as music_models @@ -59,6 +60,11 @@ class ChannelCreateSerializer(serializers.Serializer): metadata = serializers.DictField(required=False) def validate(self, validated_data): + existing_channels = self.context["actor"].owned_channels.count() + if existing_channels >= preferences.get("audio__max_channels"): + raise serializers.ValidationError( + "You have reached the maximum amount of allowed channels" + ) validated_data = super().validate(validated_data) metadata = validated_data.pop("metadata", {}) if validated_data["content_category"] == "podcast": diff --git a/api/funkwhale_api/audio/views.py b/api/funkwhale_api/audio/views.py index 09ae6d0cf..91331d2f2 100644 --- a/api/funkwhale_api/audio/views.py +++ b/api/funkwhale_api/audio/views.py @@ -138,6 +138,8 @@ class ChannelViewSet( "update", "partial_update", ] + if self.request.user.is_authenticated: + context["actor"] = self.request.user.actor return context diff --git a/api/tests/audio/test_serializers.py b/api/tests/audio/test_serializers.py index 430673d63..2ada89653 100644 --- a/api/tests/audio/test_serializers.py +++ b/api/tests/audio/test_serializers.py @@ -23,7 +23,9 @@ def test_channel_serializer_create(factories): "content_category": "other", } - serializer = serializers.ChannelCreateSerializer(data=data) + serializer = serializers.ChannelCreateSerializer( + data=data, context={"actor": attributed_to} + ) assert serializer.is_valid(raise_exception=True) is True channel = serializer.save(attributed_to=attributed_to) @@ -49,6 +51,26 @@ def test_channel_serializer_create(factories): assert channel.library.actor == attributed_to +def test_channel_serializer_create_honor_max_channels_setting(factories, preferences): + preferences["audio__max_channels"] = 1 + attributed_to = factories["federation.Actor"](local=True) + factories["audio.Channel"](attributed_to=attributed_to) + data = { + # TODO: cover + "name": "My channel", + "username": "mychannel", + "description": {"text": "This is my channel", "content_type": "text/markdown"}, + "tags": ["hello", "world"], + "content_category": "other", + } + + serializer = serializers.ChannelCreateSerializer( + data=data, context={"actor": attributed_to} + ) + with pytest.raises(serializers.serializers.ValidationError, match=r".*max.*"): + assert serializer.is_valid(raise_exception=True) + + def test_channel_serializer_create_podcast(factories): attributed_to = factories["federation.Actor"](local=True) @@ -62,7 +84,9 @@ def test_channel_serializer_create_podcast(factories): "metadata": {"itunes_category": "Sports", "language": "en"}, } - serializer = serializers.ChannelCreateSerializer(data=data) + serializer = serializers.ChannelCreateSerializer( + data=data, context={"actor": attributed_to} + ) assert serializer.is_valid(raise_exception=True) is True channel = serializer.save(attributed_to=attributed_to) diff --git a/front/src/views/admin/Settings.vue b/front/src/views/admin/Settings.vue index fdab614ea..7f40a00e3 100644 --- a/front/src/views/admin/Settings.vue +++ b/front/src/views/admin/Settings.vue @@ -80,6 +80,7 @@ export default { let instanceLabel = this.$pgettext('Content/Admin/Menu','Instance information') let usersLabel = this.$pgettext('*/*/*/Noun', 'Users') let musicLabel = this.$pgettext('*/*/*/Noun', 'Music') + let channelsLabel = this.$pgettext('*/*/*', 'Channels') let playlistsLabel = this.$pgettext('*/*/*', 'Playlists') let federationLabel = this.$pgettext('*/*/*', 'Federation') let moderationLabel = this.$pgettext('*/Moderation/*', 'Moderation') @@ -120,6 +121,14 @@ export default { {name: "music__transcoding_cache_duration"}, ] }, + { + label: channelsLabel, + id: "channels", + settings: [ + {name: "audio__channels_enabled"}, + {name: "audio__max_channels"}, + ] + }, { label: playlistsLabel, id: "playlists", From 581c531fca973a2112c8856c52d9ea4d05bb83b3 Mon Sep 17 00:00:00 2001 From: Eliot Berriot Date: Fri, 14 Feb 2020 11:56:53 +0100 Subject: [PATCH 2/3] See #170: proper error handling for username uniqueness in channels --- api/funkwhale_api/audio/serializers.py | 9 +++++++++ api/tests/audio/test_serializers.py | 18 +++++++++++++++++- 2 files changed, 26 insertions(+), 1 deletion(-) diff --git a/api/funkwhale_api/audio/serializers.py b/api/funkwhale_api/audio/serializers.py index 70c51587e..aaf8a1e68 100644 --- a/api/funkwhale_api/audio/serializers.py +++ b/api/funkwhale_api/audio/serializers.py @@ -6,6 +6,7 @@ from funkwhale_api.common import serializers as common_serializers from funkwhale_api.common import utils as common_utils from funkwhale_api.common import locales from funkwhale_api.common import preferences +from funkwhale_api.federation import models as federation_models from funkwhale_api.federation import serializers as federation_serializers from funkwhale_api.federation import utils as federation_utils from funkwhale_api.music import models as music_models @@ -74,6 +75,14 @@ class ChannelCreateSerializer(serializers.Serializer): validated_data["metadata"] = metadata return validated_data + def validate_username(self, value): + matching = federation_models.Actor.objects.local().filter( + preferred_username__iexact=value + ) + if matching.exists(): + raise serializers.ValidationError("This username is already taken") + return value + @transaction.atomic def create(self, validated_data): from . import views diff --git a/api/tests/audio/test_serializers.py b/api/tests/audio/test_serializers.py index 2ada89653..ee5a9c1b9 100644 --- a/api/tests/audio/test_serializers.py +++ b/api/tests/audio/test_serializers.py @@ -56,7 +56,6 @@ def test_channel_serializer_create_honor_max_channels_setting(factories, prefere attributed_to = factories["federation.Actor"](local=True) factories["audio.Channel"](attributed_to=attributed_to) data = { - # TODO: cover "name": "My channel", "username": "mychannel", "description": {"text": "This is my channel", "content_type": "text/markdown"}, @@ -71,6 +70,23 @@ def test_channel_serializer_create_honor_max_channels_setting(factories, prefere assert serializer.is_valid(raise_exception=True) +def test_channel_serializer_create_validates_username(factories): + attributed_to = factories["federation.Actor"](local=True) + data = { + "name": "My channel", + "username": attributed_to.preferred_username.upper(), + "description": {"text": "This is my channel", "content_type": "text/markdown"}, + "tags": ["hello", "world"], + "content_category": "other", + } + + serializer = serializers.ChannelCreateSerializer( + data=data, context={"actor": attributed_to} + ) + with pytest.raises(serializers.serializers.ValidationError, match=r".*username.*"): + assert serializer.is_valid(raise_exception=True) + + def test_channel_serializer_create_podcast(factories): attributed_to = factories["federation.Actor"](local=True) From dfaff270abe9119035ec75b18744680e8d4d74b6 Mon Sep 17 00:00:00 2001 From: Eliot Berriot Date: Fri, 14 Feb 2020 13:59:53 +0100 Subject: [PATCH 3/3] See #170: apply proper special chars and username blacklist to channel names --- api/funkwhale_api/audio/serializers.py | 10 +++++- api/tests/audio/test_serializers.py | 45 ++++++++++++++++++++++++-- 2 files changed, 52 insertions(+), 3 deletions(-) diff --git a/api/funkwhale_api/audio/serializers.py b/api/funkwhale_api/audio/serializers.py index aaf8a1e68..df98bde32 100644 --- a/api/funkwhale_api/audio/serializers.py +++ b/api/funkwhale_api/audio/serializers.py @@ -1,3 +1,4 @@ +from django.conf import settings from django.db import transaction from rest_framework import serializers @@ -13,6 +14,7 @@ from funkwhale_api.music import models as music_models from funkwhale_api.music import serializers as music_serializers from funkwhale_api.tags import models as tags_models from funkwhale_api.tags import serializers as tags_serializers +from funkwhale_api.users import serializers as users_serializers from . import categories from . import models @@ -52,7 +54,10 @@ class ChannelMetadataSerializer(serializers.Serializer): class ChannelCreateSerializer(serializers.Serializer): name = serializers.CharField(max_length=music_models.MAX_LENGTHS["ARTIST_NAME"]) - username = serializers.CharField(max_length=music_models.MAX_LENGTHS["ARTIST_NAME"]) + username = serializers.CharField( + max_length=music_models.MAX_LENGTHS["ARTIST_NAME"], + validators=[users_serializers.ASCIIUsernameValidator()], + ) description = common_serializers.ContentSerializer(allow_null=True) tags = tags_serializers.TagsListField() content_category = serializers.ChoiceField( @@ -76,6 +81,9 @@ class ChannelCreateSerializer(serializers.Serializer): return validated_data def validate_username(self, value): + if value.lower() in [n.lower() for n in settings.ACCOUNT_USERNAME_BLACKLIST]: + raise serializers.ValidationError("This username is already taken") + matching = federation_models.Actor.objects.local().filter( preferred_username__iexact=value ) diff --git a/api/tests/audio/test_serializers.py b/api/tests/audio/test_serializers.py index ee5a9c1b9..0d664969d 100644 --- a/api/tests/audio/test_serializers.py +++ b/api/tests/audio/test_serializers.py @@ -70,7 +70,7 @@ def test_channel_serializer_create_honor_max_channels_setting(factories, prefere assert serializer.is_valid(raise_exception=True) -def test_channel_serializer_create_validates_username(factories): +def test_channel_serializer_create_validates_username_uniqueness(factories): attributed_to = factories["federation.Actor"](local=True) data = { "name": "My channel", @@ -83,7 +83,48 @@ def test_channel_serializer_create_validates_username(factories): serializer = serializers.ChannelCreateSerializer( data=data, context={"actor": attributed_to} ) - with pytest.raises(serializers.serializers.ValidationError, match=r".*username.*"): + with pytest.raises( + serializers.serializers.ValidationError, match=r".*username is already taken.*" + ): + assert serializer.is_valid(raise_exception=True) + + +def test_channel_serializer_create_validates_username_chars(factories): + attributed_to = factories["federation.Actor"](local=True) + data = { + "name": "My channel", + "username": "hello world", + "description": {"text": "This is my channel", "content_type": "text/markdown"}, + "tags": ["hello", "world"], + "content_category": "other", + } + + serializer = serializers.ChannelCreateSerializer( + data=data, context={"actor": attributed_to} + ) + with pytest.raises( + serializers.serializers.ValidationError, match=r".*Enter a valid username.*" + ): + assert serializer.is_valid(raise_exception=True) + + +def test_channel_serializer_create_validates_blacklisted_username(factories, settings): + settings.ACCOUNT_USERNAME_BLACKLIST = ["forBidden"] + attributed_to = factories["federation.Actor"](local=True) + data = { + "name": "My channel", + "username": "FORBIDDEN", + "description": {"text": "This is my channel", "content_type": "text/markdown"}, + "tags": ["hello", "world"], + "content_category": "other", + } + + serializer = serializers.ChannelCreateSerializer( + data=data, context={"actor": attributed_to} + ) + with pytest.raises( + serializers.serializers.ValidationError, match=r".*username is already taken.*" + ): assert serializer.is_valid(raise_exception=True)