diff --git a/src/olympia/addons/api_urls.py b/src/olympia/addons/api_urls.py index 8bd9e1ebebc7..d9b1cdc1418a 100644 --- a/src/olympia/addons/api_urls.py +++ b/src/olympia/addons/api_urls.py @@ -4,6 +4,7 @@ from rest_framework_nested.routers import NestedSimpleRouter from olympia.activity.views import VersionReviewNotesViewSet +from olympia.files.views import FileUploadViewSet from .views import ( AddonAutoCompleteSearchView, @@ -30,6 +31,9 @@ r'reviewnotes', VersionReviewNotesViewSet, basename='version-reviewnotes' ) +submissions = SimpleRouter() +submissions.register(r'submission', FileUploadViewSet, basename='addon-submission') + urls = [ re_path(r'', include(addons.urls)), re_path(r'', include(sub_addons.urls)), @@ -66,3 +70,7 @@ ] addons_v4 = urls + +addons_v5 = addons_v4 + [ + re_path(r'', include(submissions.urls)), +] diff --git a/src/olympia/addons/serializers.py b/src/olympia/addons/serializers.py index 1571b73580b5..591c35ef6dfd 100644 --- a/src/olympia/addons/serializers.py +++ b/src/olympia/addons/serializers.py @@ -6,7 +6,8 @@ from rest_framework import exceptions, serializers -from olympia import amo +import olympia.core.logger +from olympia import activity, amo from olympia.accounts.serializers import ( BaseUserSerializer, UserProfileBasketSyncSerializer, @@ -18,17 +19,20 @@ OutgoingTranslationField, OutgoingURLField, ReverseChoiceField, + SplitField, TranslationSerializerField, ) from olympia.api.serializers import BaseESSerializer from olympia.api.utils import is_gate_active from olympia.applications.models import AppVersion from olympia.bandwagon.models import Collection -from olympia.constants.applications import APPS_ALL, APP_IDS +from olympia.blocklist.models import Block +from olympia.constants.applications import APPS, APPS_ALL, APP_IDS from olympia.constants.base import ADDON_TYPE_CHOICES_API -from olympia.constants.categories import CATEGORIES_BY_ID +from olympia.constants.categories import CATEGORIES, CATEGORIES_BY_ID from olympia.constants.promoted import PROMOTED_GROUPS, RECOMMENDED -from olympia.files.models import File +from olympia.files.models import File, FileUpload +from olympia.files.utils import parse_addon from olympia.promoted.models import PromotedAddon from olympia.search.filters import AddonAppVersionQueryParam from olympia.ratings.utils import get_grouped_ratings @@ -40,7 +44,7 @@ VersionPreview, ) -from .models import Addon, Preview, ReplacementAddon, attach_tags +from .models import Addon, AddonCategory, Preview, ReplacementAddon, attach_tags class FileSerializer(serializers.ModelSerializer): @@ -225,11 +229,12 @@ class Meta: class MinimalVersionSerializer(serializers.ModelSerializer): - files = FileSerializer(source='file') + files = FileSerializer(source='file', read_only=True) class Meta: model = Version fields = ('id', 'files', 'reviewed', 'version') + read_only_fields = fields def to_representation(self, instance): repr = super().to_representation(instance) @@ -239,14 +244,43 @@ def to_representation(self, instance): return repr +class VersionCompatabilityField(serializers.Field): + def to_internal_value(self, data): + if isinstance(data, dict): + # if it's a dict, just translate the app name + return {amo.APPS[key]: value for key, value in data.items()} + elif isinstance(data, list): + # if it's a list of apps, normalize into a dict + return {amo.APPS[key]: None for key in data} + else: + # if it's neither it's not a valid input + raise exceptions.ValidationError() + + def to_representation(self, value): + return { + app.short: { + 'min': compat.min.version + if compat + else (amo.D2C_MIN_VERSIONS.get(app.id, '1.0')), + 'max': compat.max.version if compat else amo.FAKE_MAX_VERSION, + } + for app, compat in value.items() + } + + class SimpleVersionSerializer(MinimalVersionSerializer): - compatibility = serializers.SerializerMethodField() + compatibility = VersionCompatabilityField( + # default to just Desktop Firefox; most of the times developers don't develop + # their WebExtensions for Android. See https://bit.ly/2QaMicU + source='compatible_apps', + default={amo.APPS['firefox']: None}, + ) edit_url = serializers.SerializerMethodField() is_strict_compatibility_enabled = serializers.BooleanField( - source='file.strict_compatibility' + source='file.strict_compatibility', read_only=True ) license = CompactLicenseSerializer() - release_notes = TranslationSerializerField() + release_notes = TranslationSerializerField(required=False) class Meta: model = Version @@ -261,25 +295,14 @@ class Meta: 'reviewed', 'version', ) + read_only_fields = fields def to_representation(self, instance): - # Help the LicenseSerializer find the version we're currently - # serializing. + # Help the LicenseSerializer find the version we're currently serializing. if 'license' in self.fields and instance.license: instance.license.version_instance = instance return super().to_representation(instance) - def get_compatibility(self, obj): - return { - app.short: { - 'min': compat.min.version - if compat - else (amo.D2C_MIN_VERSIONS.get(app.id, '1.0')), - 'max': compat.max.version if compat else amo.FAKE_MAX_VERSION, - } - for app, compat in obj.compatible_apps.items() - } - def get_edit_url(self, obj): return absolutify( obj.addon.get_dev_url('versions.edit', args=[obj.pk], prefix_only=True) @@ -287,8 +310,16 @@ def get_edit_url(self, obj): class VersionSerializer(SimpleVersionSerializer): - channel = ReverseChoiceField(choices=list(amo.CHANNEL_CHOICES_API.items())) - license = LicenseSerializer() + channel = ReverseChoiceField( + choices=list(amo.CHANNEL_CHOICES_API.items()), read_only=True + ) + license = SplitField( + serializers.PrimaryKeyRelatedField(queryset=License.objects.builtins()), + LicenseSerializer(), + ) + upload = serializers.SlugRelatedField( + slug_field='uuid', queryset=FileUpload.objects.all(), write_only=True + ) class Meta: model = Version @@ -302,8 +333,74 @@ class Meta: 'license', 'release_notes', 'reviewed', + 'upload', 'version', ) + writeable_fields = ( + 'compatibility', + 'license', + 'release_notes', + 'upload', + ) + read_only_fields = tuple(set(fields) - set(writeable_fields)) + + def __init__(self, *args, **kwargs): + self.addon = kwargs.pop('addon', None) + super().__init__(*args, **kwargs) + + def validate_upload(self, value): + own_upload = (request := self.context.get('request')) and ( + request.user == value.user + ) + if not own_upload or not value.valid or value.validation_timeout: + raise exceptions.ValidationError('Upload is not valid.') + return value + + def _check_blocklist(self, guid, version_string): + # check the guid/version isn't in the addon blocklist + block_qs = Block.objects.filter(guid=guid) if guid else () + if block_qs and block_qs.first().is_version_blocked(version_string): + msg = ( + 'Version {version} matches {block_link} for this add-on. ' + 'You can contact {amo_admins} for additional information.' + ) + raise exceptions.ValidationError( + msg.format( + version=version_string, + block_link=absolutify(reverse('blocklist.block', args=[guid])), + amo_admins='amo-admins@mozilla.com', + ), + ) + + def validate(self, data): + if not self.instance: + # Parse the file to get and validate package data with the addon. + self.parsed_data = parse_addon( + data.get('upload'), addon=self.addon, user=self.context['request'].user + ) + guid = self.addon.guid if self.addon else self.parsed_data.get('guid') + self._check_blocklist(guid, self.parsed_data.get('version')) + else: + data.pop('upload', None) # upload can only be set during create + return data + + def create(self, validated_data): + upload = validated_data.get('upload') + parsed_and_validated_data = { + **self.parsed_data, + **validated_data, + 'license_id': validated_data['license'].id, + } + version = Version.from_upload( + upload=upload, + addon=self.addon or validated_data.get('addon'), + # TODO: change Version.from_upload to take the compat values into account + selected_apps=[app.id for app in validated_data.get('compatible_apps')], + channel=upload.channel, + parsed_data=parsed_and_validated_data, + ) + upload.update(addon=version.addon) + return version class VersionListSerializer(VersionSerializer): @@ -413,6 +510,18 @@ def get_apps(self, obj): return [app.short for app in obj.approved_applications] +class CategoriesSerializerField(serializers.Field): + def to_internal_value(self, data): + # Can't do any transformation/validation here because we don't know addon_type + return data + + def to_representation(self, value): + return { + app_short_name: [cat.slug for cat in categories] + for app_short_name, categories in value.items() + } + + class ContributionSerializerField(OutgoingURLField): def to_representation(self, value): if not value: @@ -435,33 +544,42 @@ def to_representation(self, value): class AddonSerializer(serializers.ModelSerializer): - authors = AddonDeveloperSerializer(many=True, source='listed_authors') - categories = serializers.SerializerMethodField() - contributions_url = ContributionSerializerField(source='contributions') - current_version = CurrentVersionSerializer() - description = TranslationSerializerField() - developer_comments = TranslationSerializerField() + authors = AddonDeveloperSerializer( + many=True, source='listed_authors', read_only=True + ) + categories = CategoriesSerializerField(source='app_categories') + contributions_url = ContributionSerializerField( + source='contributions', read_only=True + ) + current_version = CurrentVersionSerializer(read_only=True) + description = TranslationSerializerField(required=False) + developer_comments = TranslationSerializerField(required=False) edit_url = serializers.SerializerMethodField() has_eula = serializers.SerializerMethodField() has_privacy_policy = serializers.SerializerMethodField() - homepage = OutgoingTranslationField() + homepage = OutgoingTranslationField(required=False) icon_url = serializers.SerializerMethodField() icons = serializers.SerializerMethodField() is_source_public = serializers.SerializerMethodField() is_featured = serializers.SerializerMethodField() - name = TranslationSerializerField() - previews = PreviewSerializer(many=True, source='current_previews') - promoted = PromotedAddonSerializer() + name = TranslationSerializerField(required=False) + previews = PreviewSerializer(many=True, source='current_previews', read_only=True) + promoted = PromotedAddonSerializer(read_only=True) ratings = serializers.SerializerMethodField() ratings_url = serializers.SerializerMethodField() review_url = serializers.SerializerMethodField() - status = ReverseChoiceField(choices=list(amo.STATUS_CHOICES_API.items())) - summary = TranslationSerializerField() - support_email = TranslationSerializerField() - support_url = OutgoingTranslationField() + status = ReverseChoiceField( + choices=list(amo.STATUS_CHOICES_API.items()), read_only=True + ) + summary = TranslationSerializerField(required=False) + support_email = TranslationSerializerField(required=False) + support_url = OutgoingTranslationField(required=False) tags = serializers.SerializerMethodField() - type = ReverseChoiceField(choices=list(amo.ADDON_TYPE_CHOICES_API.items())) + type = ReverseChoiceField( + choices=list(amo.ADDON_TYPE_CHOICES_API.items()), read_only=True + ) url = serializers.SerializerMethodField() + version = VersionSerializer(write_only=True) versions_url = serializers.SerializerMethodField() class Meta: @@ -504,9 +622,23 @@ class Meta: 'tags', 'type', 'url', + 'version', 'versions_url', 'weekly_downloads', ) + writeable_fields = ( + 'categories', + 'description', + 'developer_comments', + 'homepage', + 'name', + 'slug', + 'summary', + 'support_email', + 'support_url', + 'version', + ) + read_only_fields = tuple(set(fields) - set(writeable_fields)) def to_representation(self, obj): data = super().to_representation(obj) @@ -520,12 +652,6 @@ def to_representation(self, obj): data.pop('is_featured', None) return data - def get_categories(self, obj): - return { - app_short_name: [cat.slug for cat in categories] - for app_short_name, categories in obj.app_categories.items() - } - def get_has_eula(self, obj): return bool(getattr(obj, 'has_eula', obj.eula)) @@ -586,13 +712,68 @@ def get_ratings(self, obj): def get_is_source_public(self, obj): return False + def validate(self, data): + if not self.instance: + addon_type = self.fields['version'].parsed_data['type'] + else: + addon_type = self.instance.type + if 'app_categories' in data: + try: + category_ids = [] + for app_name, category_names in data['app_categories'].items(): + app = APPS[app_name] + category_ids.extend( + CATEGORIES[app.id][addon_type][name] for name in category_names + ) + data['app_categories'] = category_ids + except KeyError: + raise exceptions.ValidationError( + {'categories': 'Invalid app or category name.'} + ) + + return data + + def create(self, validated_data): + upload = validated_data.get('version').get('upload') + + addon = Addon.initialize_addon_from_upload( + data={**self.fields['version'].parsed_data, **validated_data}, + upload=upload, + channel=upload.channel, + user=self.context['request'].user, + ) + # Add categories + for category in validated_data.get('app_categories', ()): + AddonCategory.objects.create(addon=addon, category_id=category.id) + + self.fields['version'].create( + {**validated_data.get('version', {}), 'addon': addon} + ) + + activity.log_create(amo.LOG.CREATE_ADDON, addon) + olympia.core.logger.getLogger('z.addons').info( + f'New addon {addon!r} from {upload!r}' + ) + + if ( + addon.status == amo.STATUS_NULL + and addon.has_complete_metadata() + and upload.channel == amo.RELEASE_CHANNEL_LISTED + ): + addon.update(status=amo.STATUS_NOMINATED) + + return addon + class AddonSerializerWithUnlistedData(AddonSerializer): - latest_unlisted_version = SimpleVersionSerializer() + latest_unlisted_version = SimpleVersionSerializer(read_only=True) class Meta: model = Addon fields = AddonSerializer.Meta.fields + ('latest_unlisted_version',) + read_only_fields = tuple( + set(fields) - set(AddonSerializer.Meta.writeable_fields) + ) class SimpleAddonSerializer(AddonSerializer): diff --git a/src/olympia/addons/tests/test_serializers.py b/src/olympia/addons/tests/test_serializers.py index fbfc1691860e..c8114d85ea9d 100644 --- a/src/olympia/addons/tests/test_serializers.py +++ b/src/olympia/addons/tests/test_serializers.py @@ -949,6 +949,13 @@ def test_latest_unlisted_version_with_right_serializer(self): self.addon.latest_unlisted_version, result['latest_unlisted_version'] ) + def test_readonly_fields(self): + serializer = self.serializer_class() + fields_read_only = { + name for name, field in serializer.get_fields().items() if field.read_only + } + assert fields_read_only == set(serializer.Meta.read_only_fields) + class TestESAddonSerializerOutput(AddonSerializerOutputTestMixin, ESTestCase): serializer_class = ESAddonSerializer diff --git a/src/olympia/addons/tests/test_views.py b/src/olympia/addons/tests/test_views.py index e5bef6914cda..370915ae3793 100644 --- a/src/olympia/addons/tests/test_views.py +++ b/src/olympia/addons/tests/test_views.py @@ -18,22 +18,10 @@ from waffle.testutils import override_switch from olympia import amo -from olympia.addons.models import ( - Addon, - AddonRegionalRestrictions, - AddonUser, - ReplacementAddon, -) -from olympia.addons.utils import generate_addon_guid -from olympia.addons.views import ( - DEFAULT_FIND_REPLACEMENT_PATH, - FIND_REPLACEMENT_SRC, - AddonAutoCompleteSearchView, - AddonSearchView, -) from olympia.amo.tests import ( APITestClient, ESTestCase, + JWTAPITestClient, TestCase, addon_factory, collection_factory, @@ -43,6 +31,7 @@ ) from olympia.amo.urlresolvers import get_outgoing_url from olympia.bandwagon.models import CollectionAddon +from olympia.blocklist.models import Block from olympia.constants.categories import CATEGORIES, CATEGORIES_BY_ID from olympia.constants.promoted import ( LINE, @@ -52,8 +41,24 @@ SPONSORED, VERIFIED, ) +from olympia.files.tests.test_models import UploadMixin from olympia.users.models import UserProfile -from olympia.versions.models import ApplicationsVersions, AppVersion +from olympia.versions.models import ApplicationsVersions, AppVersion, License + +from ..models import ( + Addon, + AddonRegionalRestrictions, + AddonUser, + ReplacementAddon, +) +from ..serializers import AddonSerializer, LicenseSerializer, VersionSerializer +from ..utils import generate_addon_guid +from ..views import ( + DEFAULT_FIND_REPLACEMENT_PATH, + FIND_REPLACEMENT_SRC, + AddonAutoCompleteSearchView, + AddonSearchView, +) class TestStatus(TestCase): @@ -686,6 +691,143 @@ def test_with_grouped_ratings(self): assert data == {'detail': 'show_grouped_ratings parameter should be a boolean'} +class TestAddonViewSetCreate(UploadMixin, TestCase): + client_class = APITestClient + + def setUp(self): + super().setUp() + self.user = user_factory(read_dev_agreement=self.days_ago(0)) + self.upload = self.get_upload( + 'webextension.xpi', user=self.user, source=amo.UPLOAD_SOURCE_ADDON_API + ) + self.url = reverse_ns('addon-list', api_version='v5') + self.client.login_api(self.user) + self.license = License.objects.create(builtin=1) + self.minimal_data = { + 'version': {'upload': self.upload.uuid, 'license': self.license.id}, + 'categories': {'firefox': ['bookmarks']}, + } + + def test_basic(self): + response = self.client.post( + self.url, + data=self.minimal_data, + ) + assert response.status_code == 201, response.content + data = response.data + assert data['name'] == {'en-US': 'My WebExtension Addon'} + assert data['status'] == 'nominated' + addon = Addon.objects.get() + request = APIRequestFactory().get('/') + request.version = 'v5' + request.user = self.user + assert data == AddonSerializer(context={'request': request}).to_representation( + addon + ) + + def test_not_authenticated(self): + self.client.logout_api() + response = self.client.post( + self.url, + data=self.minimal_data, + ) + assert response.status_code == 401 + assert response.data == { + 'detail': 'Authentication credentials were not provided.' + } + assert not Addon.objects.all() + + def test_not_read_agreement(self): + self.user.update(read_dev_agreement=None) + response = self.client.post( + self.url, + data=self.minimal_data, + ) + assert response.status_code in [401, 403] # JWT auth is a 401; web auth is 401 + assert 'agreement' in response.data['detail'].lower() + assert not Addon.objects.all() + + def test_waffle_flag_disabled(self): + gates = { + 'v5': ( + gate + for gate in settings.DRF_API_GATES['v5'] + if gate != 'addon-submission-api' + ) + } + with override_settings(DRF_API_GATES=gates): + response = self.client.post( + self.url, + data=self.minimal_data, + ) + assert response.status_code == 403 + assert response.data == { + 'detail': 'You do not have permission to perform this action.' + } + assert not Addon.objects.all() + + def test_missing_version(self): + response = self.client.post( + self.url, + data={'categories': {'firefox': ['bookmarks']}}, + ) + assert response.status_code == 400, response.content + assert response.data == {'version': ['This field is required.']} + assert not Addon.objects.all() + + def test_invalid_categories(self): + data = {**self.minimal_data, 'categories': {'firefox': ['performance']}} + response = self.client.post( + self.url, + data=data, + ) + assert response.status_code == 400, response.content + assert response.data == {'categories': ['Invalid app or category name.']} + assert not Addon.objects.all() + + def test_set_extra_data(self): + data = { + **self.minimal_data, + 'description': {'en-US': 'new description'}, + 'developer_comments': {'en-US': 'comments'}, + 'homepage': {'en-US': 'https://my.home.page/'}, + # 'name' # don't update - should retain name from the manifest + 'slug': 'addon-slug', + 'summary': {'en-US': 'new summary'}, + 'support_email': {'en-US': 'email@me'}, + 'support_url': {'en-US': 'https://my.home.page/support/'}, + } + response = self.client.post( + self.url, + data=data, + ) + addon = Addon.objects.get() + + assert response.status_code == 201, response.content + data = response.data + assert data['description'] == {'en-US': 'new description'} + assert addon.description == 'new description' + assert data['developer_comments'] == {'en-US': 'comments'} + assert addon.developer_comments == 'comments' + assert data['homepage']['url'] == {'en-US': 'https://my.home.page/'} + assert addon.homepage == 'https://my.home.page/' + assert data['name'] == {'en-US': 'My WebExtension Addon'} + assert addon.name == 'My WebExtension Addon' + assert data['slug'] == 'addon-slug' == addon.slug + assert data['summary'] == {'en-US': 'new summary'} + assert addon.summary == 'new summary' + assert data['support_email'] == {'en-US': 'email@me'} + assert addon.support_email == 'email@me' + assert data['support_url']['url'] == {'en-US': 'https://my.home.page/support/'} + assert addon.support_url == 'https://my.home.page/support/' + assert data['status'] == 'nominated' + assert addon.status == amo.STATUS_NOMINATED + + +class TestAddonViewSetCreateJWTAuth(TestAddonViewSetCreate): + client_class = JWTAPITestClient + + class TestVersionViewSetDetail(AddonAndVersionViewSetDetailMixin, TestCase): client_class = APITestClient @@ -844,6 +986,149 @@ def test_unlisted_version_user_but_not_author(self): assert response.status_code == 403 +class TestVersionViewSetCreate(UploadMixin, TestCase): + client_class = APITestClient + + def setUp(self): + super().setUp() + self.user = user_factory(read_dev_agreement=self.days_ago(0)) + self.upload = self.get_upload( + 'webextension.xpi', user=self.user, source=amo.UPLOAD_SOURCE_ADDON_API + ) + self.addon = addon_factory(users=(self.user,), guid='@webextension-guid') + self.url = reverse_ns( + 'addon-version-list', + kwargs={'addon_pk': self.addon.slug}, + api_version='v5', + ) + self.client.login_api(self.user) + self.license = License.objects.create(builtin=1) + self.minimal_data = {'upload': self.upload.uuid, 'license': self.license.id} + + def test_basic(self): + response = self.client.post( + self.url, + data=self.minimal_data, + ) + assert response.status_code == 201, response.content + data = response.data + assert data['license'] == LicenseSerializer().to_representation(self.license) + assert data['compatibility'] == { + 'firefox': {'max': '*', 'min': '42.0'}, + } + self.addon.reload() + assert self.addon.versions.count() == 2 + version = self.addon.find_latest_version(channel=None) + request = APIRequestFactory().get('/') + request.version = 'v5' + request.user = self.user + assert data == VersionSerializer( + context={'request': request} + ).to_representation(version) + + def test_not_authenticated(self): + self.client.logout_api() + response = self.client.post( + self.url, + data=self.minimal_data, + ) + assert response.status_code == 401 + assert response.data == { + 'detail': 'Authentication credentials were not provided.' + } + assert self.addon.reload().versions.count() == 1 + + def test_not_your_addon(self): + self.addon.addonuser_set.get(user=self.user).update( + role=amo.AUTHOR_ROLE_DELETED + ) + response = self.client.post( + self.url, + data=self.minimal_data, + ) + assert response.status_code == 403 + assert response.data['detail'] == ( + 'You do not have permission to perform this action.' + ) + assert self.addon.reload().versions.count() == 1 + + def test_not_read_agreement(self): + self.user.update(read_dev_agreement=None) + response = self.client.post( + self.url, + data=self.minimal_data, + ) + assert response.status_code in [401, 403] # JWT auth is a 401; web auth is 403 + assert 'agreement' in response.data['detail'].lower() + assert self.addon.reload().versions.count() == 1 + + def test_waffle_flag_disabled(self): + gates = { + 'v5': ( + gate + for gate in settings.DRF_API_GATES['v5'] + if gate != 'addon-submission-api' + ) + } + with override_settings(DRF_API_GATES=gates): + response = self.client.post( + self.url, + data=self.minimal_data, + ) + assert response.status_code == 403 + assert response.data == { + 'detail': 'You do not have permission to perform this action.' + } + assert self.addon.reload().versions.count() == 1 + + def test_missing_license(self): + response = self.client.post( + self.url, + data={'upload': self.upload.uuid}, + ) + assert response.status_code == 400, response.content + assert response.data == {'license': ['This field is required.']} + assert self.addon.reload().versions.count() == 1 + + def test_set_extra_data(self): + data = { + **self.minimal_data, + 'compatibility': ['firefox', 'android'], + 'release_notes': {'en-US': 'dsdsdsd'}, + } + response = self.client.post( + self.url, + data=data, + ) + + assert response.status_code == 201, response.content + data = response.data + self.addon.reload() + assert self.addon.versions.count() == 2 + version = self.addon.find_latest_version(channel=None) + assert data['compatibility'] == { + 'android': {'max': '*', 'min': '48.0'}, + 'firefox': {'max': '*', 'min': '42.0'}, + } + assert list(version.compatible_apps.keys()) == [amo.FIREFOX, amo.ANDROID] + assert data['release_notes'] == {'en-US': 'dsdsdsd'} + assert version.release_notes == 'dsdsdsd' + + def test_check_blocklist(self): + Block.objects.create(guid=self.addon.guid, updated_by=self.user) + response = self.client.post( + self.url, + data=self.minimal_data, + ) + assert response.status_code == 400 + assert 'Version 0.0.1 matches ' in str(response.data['non_field_errors']) + assert self.addon.reload().versions.count() == 1 + + +class TestVersionViewSetCreateJWTAuth(TestVersionViewSetCreate): + client_class = JWTAPITestClient + + class TestVersionViewSetList(AddonAndVersionViewSetDetailMixin, TestCase): client_class = APITestClient diff --git a/src/olympia/addons/views.py b/src/olympia/addons/views.py index 24672e6a4538..72da17b2ca7d 100644 --- a/src/olympia/addons/views.py +++ b/src/olympia/addons/views.py @@ -12,7 +12,8 @@ from rest_framework import exceptions, serializers from rest_framework.decorators import action from rest_framework.generics import ListAPIView -from rest_framework.mixins import ListModelMixin, RetrieveModelMixin +from rest_framework.mixins import CreateModelMixin, ListModelMixin, RetrieveModelMixin +from rest_framework.permissions import IsAuthenticated from rest_framework.response import Response from rest_framework.settings import api_settings from rest_framework.views import APIView @@ -23,6 +24,7 @@ from olympia import amo from olympia.access import acl from olympia.amo.urlresolvers import get_outgoing_url +from olympia.api.authentication import JWTKeyAuthentication, WebTokenAuthentication from olympia.api.exceptions import UnavailableForLegalReasons from olympia.api.pagination import ESPageNumberPagination from olympia.api.permissions import ( @@ -32,11 +34,13 @@ AllowListedViewerOrReviewer, AllowUnlistedViewerOrReviewer, AnyOf, + APIGatePermission, GroupPermission, RegionalRestriction, ) from olympia.api.utils import is_gate_active from olympia.constants.categories import CATEGORIES_BY_ID +from olympia.devhub.permissions import IsSubmissionAllowedFor from olympia.search.filters import ( AddonAppQueryParam, AddonAppVersionQueryParam, @@ -174,7 +178,7 @@ def find_replacement_addon(request): return redirect(replace_url, permanent=False) -class AddonViewSet(RetrieveModelMixin, GenericViewSet): +class AddonViewSet(CreateModelMixin, RetrieveModelMixin, GenericViewSet): permission_classes = [ AnyOf( AllowReadOnlyIfPublic, @@ -183,6 +187,7 @@ class AddonViewSet(RetrieveModelMixin, GenericViewSet): AllowUnlistedViewerOrReviewer, ), ] + authentication_classes = [JWTKeyAuthentication, WebTokenAuthentication] georestriction_classes = [ RegionalRestriction | GroupPermission(amo.permissions.ADDONS_EDIT) ] @@ -216,7 +221,7 @@ def get_queryset(self): def get_serializer_class(self): # Override serializer to use serializer_class_with_unlisted_data if # we are allowed to access unlisted data. - obj = getattr(self, 'instance') + obj = getattr(self, 'instance', None) request = self.request if acl.check_unlisted_addons_viewer_or_reviewer(request) or ( obj @@ -240,7 +245,12 @@ def check_permissions(self, request): for restriction in self.get_georestrictions(): if not restriction.has_permission(request, self): raise UnavailableForLegalReasons() - + if self.action == 'create': + self.permission_classes = [ + APIGatePermission('addon-submission-api'), + IsAuthenticated, + IsSubmissionAllowedFor, + ] super().check_permissions(request) def check_object_permissions(self, request, obj): @@ -285,6 +295,13 @@ def eula_policy(self, request, pk=None): ) return Response(serializer.data) + def create(self, request, *args, **kwargs): + from olympia.signing.views import VersionView # circular import + + # TODO: consolidate/replicate this behaviour. + VersionView().check_throttles(request) + return super().create(request, *args, **kwargs) + class AddonChildMixin: """Mixin containing method to retrieve the parent add-on object.""" @@ -322,7 +339,11 @@ def get_addon_object( class AddonVersionViewSet( - AddonChildMixin, RetrieveModelMixin, ListModelMixin, GenericViewSet + AddonChildMixin, + CreateModelMixin, + RetrieveModelMixin, + ListModelMixin, + GenericViewSet, ): # Permissions are always checked against the parent add-on in # get_addon_object() using AddonViewSet.permission_classes so we don't need @@ -330,6 +351,7 @@ class AddonVersionViewSet( # below in check_permissions() and check_object_permissions() depending on # what the client is requesting to see. permission_classes = [] + authentication_classes = [JWTKeyAuthentication, WebTokenAuthentication] def get_serializer_class(self): if ( @@ -342,9 +364,14 @@ def get_serializer_class(self): serializer_class = VersionSerializer return serializer_class + def get_serializer(self, *args, **kwargs): + return super().get_serializer( + *args, **{**kwargs, 'addon': self.get_addon_object()} + ) + def check_permissions(self, request): - requested = self.request.GET.get('filter') if self.action == 'list': + requested = self.request.GET.get('filter') if requested == 'all_with_deleted': # To see deleted versions, you need Addons:ViewDeleted. self.permission_classes = [ @@ -372,6 +399,12 @@ def check_permissions(self, request): # super + check_object_permission() ourselves, passing down the # addon object directly. return super().check_object_permissions(request, self.get_addon_object()) + elif self.action == 'create': + self.permission_classes = [ + APIGatePermission('addon-submission-api'), + IsAuthenticated, + IsSubmissionAllowedFor, + ] super().check_permissions(request) def check_object_permissions(self, request, obj): @@ -452,6 +485,13 @@ def get_queryset(self): queryset = queryset.transform(Version.transformer_license) return queryset + def create(self, request, *args, **kwargs): + from olympia.signing.views import VersionView # circular import + + # TODO: consolidate/replicate this behaviour. + VersionView().check_throttles(request) + return super().create(request, *args, **kwargs) + class AddonSearchView(ListAPIView): authentication_classes = [] diff --git a/src/olympia/api/permissions.py b/src/olympia/api/permissions.py index 672e9fd56b85..7a6c55f3f566 100644 --- a/src/olympia/api/permissions.py +++ b/src/olympia/api/permissions.py @@ -5,6 +5,8 @@ from olympia.addons.models import AddonRegionalRestrictions from olympia.amo import permissions +from .utils import is_gate_active + # Most of these classes come from zamboni, check out # https://github.com/mozilla/zamboni/blob/master/mkt/api/permissions.py for @@ -354,3 +356,17 @@ def has_object_permission(self, request, view, obj): def __call__(self, *a): return self + + +class APIGatePermission(BasePermission): + def __init__(self, gate): + self.gate = gate + + def has_permission(self, request, view): + return is_gate_active(request, self.gate) + + def has_object_permission(self, request, view, obj): + return self.has_permission(request, view) + + def __call__(self, *a): + return self diff --git a/src/olympia/api/urls.py b/src/olympia/api/urls.py index aa7af224cf97..b48eed42c9b3 100644 --- a/src/olympia/api/urls.py +++ b/src/olympia/api/urls.py @@ -2,7 +2,7 @@ from olympia.accounts.urls import accounts_v3, accounts_v4, auth_callback_patterns from olympia.amo.urls import api_patterns as amo_api_patterns -from olympia.addons.api_urls import addons_v3, addons_v4 +from olympia.addons.api_urls import addons_v3, addons_v4, addons_v5 from olympia.ratings.api_urls import ratings_v3, ratings_v4 @@ -33,7 +33,20 @@ re_path(r'^scanner/', include('olympia.scanners.api_urls')), ] -v5_api_urls = v4_api_urls + [ +v5_api_urls = [ + re_path(r'^abuse/', include('olympia.abuse.urls')), + re_path(r'^accounts/', include(accounts_v4)), + re_path(r'^activity/', include('olympia.activity.urls')), + re_path(r'^addons/', include(addons_v5)), + re_path(r'^applications/', include('olympia.applications.api_urls')), + re_path(r'^blocklist/', include('olympia.blocklist.urls')), + re_path(r'^', include('olympia.discovery.api_urls')), + re_path(r'^hero/', include('olympia.hero.urls')), + re_path(r'^ratings/', include(ratings_v4.urls)), + re_path(r'^reviewers/', include('olympia.reviewers.api_urls')), + re_path(r'^', include('olympia.signing.urls')), + re_path(r'^', include(amo_api_patterns)), + re_path(r'^scanner/', include('olympia.scanners.api_urls')), re_path(r'^shelves/', include('olympia.shelves.urls')), ] diff --git a/src/olympia/conf/prod/settings.py b/src/olympia/conf/prod/settings.py index 2a919997c466..1579df2c3e03 100644 --- a/src/olympia/conf/prod/settings.py +++ b/src/olympia/conf/prod/settings.py @@ -102,3 +102,6 @@ # See: https://bugzilla.mozilla.org/show_bug.cgi?id=1633746 BIGQUERY_AMO_DATASET = 'amo_prod' + +# We don't want to enable the new addon submission on prod yet +DRF_API_GATES['v5'].pop('addon-submission-api', None) diff --git a/src/olympia/files/models.py b/src/olympia/files/models.py index 7ddf11011d16..afcae9c5712c 100644 --- a/src/olympia/files/models.py +++ b/src/olympia/files/models.py @@ -576,6 +576,10 @@ def from_post(cls, chunks, filename, size, **params): def processed(self): return bool(self.valid or self.validation) + @property + def submitted(self): + return bool(self.addon) + @property def validation_timeout(self): if self.processed: diff --git a/src/olympia/files/serializers.py b/src/olympia/files/serializers.py new file mode 100644 index 000000000000..435746a73ca2 --- /dev/null +++ b/src/olympia/files/serializers.py @@ -0,0 +1,48 @@ +from rest_framework import serializers +from rest_framework.reverse import reverse as drf_reverse + +from olympia import amo +from olympia.amo.templatetags.jinja_helpers import absolutify +from olympia.api.fields import ReverseChoiceField + +from .models import FileUpload + + +class FileUploadSerializer(serializers.ModelSerializer): + uuid = serializers.UUIDField(format='hex') + channel = ReverseChoiceField( + choices=[ + (False, amo.CHANNEL_CHOICES_API[amo.RELEASE_CHANNEL_UNLISTED]), + (True, amo.CHANNEL_CHOICES_API[amo.RELEASE_CHANNEL_LISTED]), + ], + source='automated_signing', + ) + processed = serializers.BooleanField() + valid = serializers.BooleanField(source='passed_all_validations') + validation = serializers.SerializerMethodField() + url = serializers.SerializerMethodField() + + class Meta: + model = FileUpload + fields = [ + 'uuid', + 'channel', + 'processed', + 'submitted', + 'url', + 'valid', + 'validation', + 'version', + ] + + def get_validation(self, instance): + return instance.load_validation() if instance.validation else None + + def get_url(self, instance): + return absolutify( + drf_reverse( + 'addon-submission-detail', + request=self.context.get('request'), + args=[instance.uuid.hex], + ) + ) diff --git a/src/olympia/files/tests/test_serializers.py b/src/olympia/files/tests/test_serializers.py new file mode 100644 index 000000000000..b72f07dc5f3d --- /dev/null +++ b/src/olympia/files/tests/test_serializers.py @@ -0,0 +1,50 @@ +import json + +from django.conf import settings + +from rest_framework.settings import api_settings +from rest_framework.test import APIRequestFactory + +from olympia.amo.tests import TestCase +from olympia.amo.reverse import reverse + +from ..models import FileUpload +from ..serializers import FileUploadSerializer + + +class TestFileUploadSerializer(TestCase): + def setUp(self): + api_version = api_settings.DEFAULT_VERSION + self.request = APIRequestFactory().get('/api/%s/' % api_version) + self.request.versioning_scheme = api_settings.DEFAULT_VERSIONING_CLASS() + self.request.version = api_version + + def test_basic(self): + file_upload = FileUpload(version='123') + data = FileUploadSerializer( + instance=file_upload, context={'request': self.request} + ).data + assert data == { + 'uuid': file_upload.uuid.hex, + 'channel': 'unlisted', + 'processed': False, + 'submitted': False, + 'url': settings.EXTERNAL_SITE_URL + + reverse( + 'v5:addon-submission-detail', + args=[file_upload.uuid.hex], + ), + 'valid': False, + 'validation': None, + 'version': '123', + } + + file_upload.update( + automated_signing=True, + validation=json.dumps([{'something': 'happened'}]), + ) + data = FileUploadSerializer( + instance=file_upload, context={'request': self.request} + ).data + assert data['channel'] == 'listed' + assert data['validation'] == [{'something': 'happened'}] diff --git a/src/olympia/files/tests/test_views.py b/src/olympia/files/tests/test_views.py index c21ab4c657a8..a5455f1c870e 100644 --- a/src/olympia/files/tests/test_views.py +++ b/src/olympia/files/tests/test_views.py @@ -1,9 +1,20 @@ +import os + from django.conf import settings +from django.test.utils import override_settings from django.urls import reverse -from olympia.amo.tests import TestCase +from olympia import amo +from olympia.amo.tests import ( + APITestClient, + JWTAPITestClient, + reverse_ns, + TestCase, + user_factory, +) from .test_models import UploadMixin +from ..models import FileUpload files_fixtures = 'src/olympia/files/fixtures/files/' @@ -43,3 +54,128 @@ def test_returns_410_when_upload_path_is_falsey(self): resp = self.client.get(self.upload.get_authenticated_download_url()) assert resp.status_code == 410 + + +class FileUploadTestMixin: + def setUp(self): + super().setUp() + self.list_url = reverse_ns('addon-submission-list', api_version='v5') + self.user = user_factory(read_dev_agreement=self.days_ago(0)) + # Add a file upload + self.upload = FileUpload.objects.create(user=self.user) + # Add some other ones from other users + self.other_user_upload = FileUpload.objects.create(user=user_factory()) + FileUpload.objects.create() + + self.detail_url = reverse_ns( + 'addon-submission-detail', + kwargs={'uuid': self.upload.uuid.hex}, + api_version='v5', + ) + self.client.login_api(self.user) + + def _xpi_filepath(self, guid, version): + return os.path.join( + 'src', + 'olympia', + 'signing', + 'fixtures', + f'{guid}-{version}.xpi', + ) + + def test_not_authenticated(self): + self.client.logout_api() + response = self.client.get( + self.list_url, + ) + assert response.status_code == 401 + + def test_no_developer_agreement(self): + self.user.update(read_dev_agreement=None) + filepath = self._xpi_filepath('@upload-version', '3.0') + + with open(filepath, 'rb') as upload: + data = { + 'upload': upload, + 'channel': 'listed', + } + + response = self.client.post( + self.list_url, + data, + format='multipart', + REMOTE_ADDR='127.0.3.1', + ) + assert response.status_code in [401, 403] # JWT auth is a 401; web auth is 403 + + def test_create(self): + upload_count_before = FileUpload.objects.count() + filepath = self._xpi_filepath('@upload-version', '3.0') + + with open(filepath, 'rb') as upload: + data = { + 'upload': upload, + 'channel': 'listed', + } + + response = self.client.post( + self.list_url, + data, + format='multipart', + REMOTE_ADDR='127.0.3.1', + ) + + assert response.status_code == 201 + + assert FileUpload.objects.count() != upload_count_before + upload = FileUpload.objects.last() + + assert upload.name == f'{upload.uuid.hex}_@upload-version-3.0.xpi' + assert upload.source == amo.UPLOAD_SOURCE_ADDON_API + assert upload.user == self.user + assert upload.version == '3.0' + assert upload.ip_address == '127.0.3.1' + + data = response.json() + assert data['uuid'] == upload.uuid.hex + + def test_list(self): + response = self.client.get( + self.list_url, + ) + data = response.json()['results'] + assert len(data) == 1 # only the users own uploads + assert data[0]['uuid'] == self.upload.uuid.hex + assert data[0]['url'] == self.detail_url + + def test_api_unavailable(self): + with override_settings(DRF_API_GATES={'v5': []}): + response = self.client.get( + self.list_url, + ) + assert response.status_code == 403 + + def test_retrieve(self): + response = self.client.get(self.detail_url) + data = response.json() + assert data['uuid'] == self.upload.uuid.hex + assert data['url'] == self.detail_url + + def test_cannot_retrieve_other_uploads(self): + detail_url = reverse_ns( + 'addon-submission-detail', + kwargs={'uuid': self.other_user_upload.uuid.hex}, + api_version='v5', + ) + response = self.client.get( + detail_url, + ) + assert response.status_code == 404 + + +class TestFileUploadViewSetJWTAuth(FileUploadTestMixin, TestCase): + client_class = JWTAPITestClient + + +class TestFileUploadViewSetWebTokenAuth(FileUploadTestMixin, TestCase): + client_class = APITestClient diff --git a/src/olympia/files/views.py b/src/olympia/files/views.py index d625cc59523e..e3f15a6fd276 100644 --- a/src/olympia/files/views.py +++ b/src/olympia/files/views.py @@ -2,12 +2,23 @@ from django import http, shortcuts from django.utils.crypto import constant_time_compare +from rest_framework import exceptions, status +from rest_framework.mixins import CreateModelMixin +from rest_framework.response import Response +from rest_framework.viewsets import ReadOnlyModelViewSet + import olympia.core.logger +from olympia import amo from olympia.amo.decorators import use_primary_db from olympia.amo.utils import HttpResponseXSendFile +from olympia.api.authentication import JWTKeyAuthentication, WebTokenAuthentication +from olympia.api.permissions import AllowOwner, APIGatePermission +from olympia.devhub import tasks as devhub_tasks +from olympia.devhub.permissions import IsSubmissionAllowedFor from .models import FileUpload +from .serializers import FileUploadSerializer log = olympia.core.logger.getLogger('z.addons') @@ -37,3 +48,48 @@ def serve_file_upload(request, uuid): return HttpResponseXSendFile( request, upload.path, content_type='application/octet-stream' ) + + +class FileUploadViewSet(CreateModelMixin, ReadOnlyModelViewSet): + queryset = FileUpload.objects.all() + serializer_class = FileUploadSerializer + permission_classes = [ + APIGatePermission('addon-submission-api'), + AllowOwner, + IsSubmissionAllowedFor, + ] + authentication_classes = [JWTKeyAuthentication, WebTokenAuthentication] + lookup_field = 'uuid' + + def get_queryset(self): + return super().get_queryset().filter(user=self.request.user) + + def create(self, request): + # TODO: move most/all of this logic to the serializer + if 'upload' in request.FILES: + filedata = request.FILES['upload'] + else: + raise exceptions.ValidationError( + 'Missing "upload" key in multipart file data.', + status.HTTP_400_BAD_REQUEST, + ) + channel = amo.CHANNEL_CHOICES_LOOKUP.get(request.POST.get('channel')) + if not channel: + raise exceptions.ValidationError( + 'Missing "channel" arg.', + status.HTTP_400_BAD_REQUEST, + ) + + upload = FileUpload.from_post( + filedata, + filedata.name, + filedata.size, + channel=channel, + source=amo.UPLOAD_SOURCE_ADDON_API, + user=request.user, + ) + + devhub_tasks.validate(upload, listed=(channel == amo.RELEASE_CHANNEL_LISTED)) + headers = self.get_success_headers({}) + data = self.get_serializer(instance=upload).data + return Response(data, status=status.HTTP_201_CREATED, headers=headers) diff --git a/src/olympia/lib/settings_base.py b/src/olympia/lib/settings_base.py index e2bc8baad4a7..04fd83685b1f 100644 --- a/src/olympia/lib/settings_base.py +++ b/src/olympia/lib/settings_base.py @@ -1612,6 +1612,7 @@ def read_only_mode(env): 'addons-search-_score-field', 'ratings-can_reply', 'ratings-score-filter', + 'addon-submission-api', ), } diff --git a/src/olympia/versions/models.py b/src/olympia/versions/models.py index 6da6031fd02b..3af500ab3bd4 100644 --- a/src/olympia/versions/models.py +++ b/src/olympia/versions/models.py @@ -256,8 +256,8 @@ def from_upload(cls, upload, addon, selected_apps, channel, parsed_data=None): 'FileUpload user does not have some required fields' ) - license_id = None - if channel == amo.RELEASE_CHANNEL_LISTED: + license_id = parsed_data.get('license_id') + if not license_id and channel == amo.RELEASE_CHANNEL_LISTED: previous_version = addon.find_latest_version(channel=channel, exclude=()) if previous_version and previous_version.license_id: license_id = previous_version.license_id @@ -272,6 +272,7 @@ def from_upload(cls, upload, addon, selected_apps, channel, parsed_data=None): version=parsed_data['version'], license_id=license_id, channel=channel, + release_notes=parsed_data.get('release_notes'), ) email = upload.user.email if upload.user and upload.user.email else '' with core.override_remote_addr(upload.ip_address):