diff --git a/docs/topics/api/addons.rst b/docs/topics/api/addons.rst index 03b04bd4b9b1..7d319e03aa00 100644 --- a/docs/topics/api/addons.rst +++ b/docs/topics/api/addons.rst @@ -459,6 +459,7 @@ This endpoint allows you to fetch a single version belonging to a specific add-o :>json object|null release_notes: The release notes for this version (See :ref:`translated fields `). :>json string reviewed: The date the version was reviewed at. :>json boolean is_strict_compatibility_enabled: Whether or not this version has `strictCompatibility `_. set. + :>json string|null source: The (absolute) URL to download the submitted source for this version. This field is only present for authenticated users, for their own add-ons. :>json string version: The version number string for the version. @@ -493,6 +494,7 @@ This endpoint allows a submission of an upload to an existing add-on to create a :`). Custom licenses are not supported for themes. :`). :` to set/update the source file. ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ @@ -547,6 +549,32 @@ Shorthand, for when you only want to define compatible apps, but use the min/max } +~~~~~~~~~~~~~~~ +Version Sources +~~~~~~~~~~~~~~~ + +.. _version-sources: + +Version source files cannot be uploaded as JSON - the request must be sent as form-data instead. +Other fields can be set/updated at the same time as ``source`` if desired. + +.. http:post:: /api/v5/addons/addon/(int:addon_id|string:addon_slug|string:addon_guid)/versions/ + + .. _version-sources-request-create: + + :form source: The add-on file being uploaded. + :form compatibility: See :ref:`create ` for details. + :form upload: The uuid for the xpi upload to create this version with. + :reqheader Content-Type: multipart/form-data + + +.. http:patch:: /api/v5/addons/addon/(int:addon_id|string:addon_slug|string:addon_guid)/versions/(int:id)/ + + .. _version-sources-request-edit: + + :form source: The add-on file being uploaded. + :reqheader Content-Type: multipart/form-data + ------------ Version Edit ------------ @@ -574,6 +602,7 @@ This endpoint allows the metadata for an existing version to be edited. :`). Custom licenses are not supported for themes. :`). Custom licenses are not supported for themes. :`). + :` to set/update the source file. ------------- diff --git a/docs/topics/api/overview.rst b/docs/topics/api/overview.rst index 4ae19540d5ec..8846284597b6 100644 --- a/docs/topics/api/overview.rst +++ b/docs/topics/api/overview.rst @@ -435,6 +435,7 @@ These are `v5` specific changes - `v4` changes apply also. * 2021-12-09: enabled setting ``tags`` via addon submission and edit apis. https://github.com/mozilla/addons-server/issues/18268 * 2021-12-09: changed ``license`` in version create/update endpoints to accept a license slug rather than numeric ID, and documented supported licenses. https://github.com/mozilla/addons-server/issues/18361 * 2022-01-27: added ``ERROR_AUTHENTICATION_EXPIRED`` error code for authentication failures. https://github.com/mozilla/addons-server/issues/18669 +* 2022-02-03: added ``source`` to version detail responses, for developer's own add-ons. ``source`` can also be set via version create/update endpoints. https://github.com/mozilla/addons-server/issues/9913 .. _`#11380`: https://github.com/mozilla/addons-server/issues/11380/ .. _`#11379`: https://github.com/mozilla/addons-server/issues/11379/ diff --git a/src/olympia/addons/fields.py b/src/olympia/addons/fields.py new file mode 100644 index 000000000000..d5313ce56035 --- /dev/null +++ b/src/olympia/addons/fields.py @@ -0,0 +1,267 @@ +import os +import tarfile +import zipfile + +from urllib.parse import urlsplit, urlunsplit + +from django.http.request import QueryDict +from django.urls import reverse + +from rest_framework import fields, exceptions, serializers + +from olympia import amo +from olympia.amo.utils import sorted_groupby +from olympia.amo.templatetags.jinja_helpers import absolutify +from olympia.api.fields import ( + ESTranslationSerializerField, + GetTextTranslationSerializerField, + OutgoingURLField, + TranslationSerializerField, +) +from olympia.applications.models import AppVersion +from olympia.constants.applications import APPS +from olympia.constants.categories import CATEGORIES +from olympia.constants.licenses import LICENSES_BY_SLUG +from olympia.files.utils import SafeZip, archive_member_validator +from olympia.versions.models import ( + ApplicationsVersions, + License, + VALID_SOURCE_EXTENSIONS, +) + + +class CategoriesSerializerField(serializers.Field): + def to_internal_value(self, data): + try: + categories = [] + for app_name, category_names in data.items(): + if len(category_names) > amo.MAX_CATEGORIES: + raise exceptions.ValidationError( + 'Maximum number of categories per application ' + f'({amo.MAX_CATEGORIES}) exceeded' + ) + if len(category_names) > 1 and 'other' in category_names: + raise exceptions.ValidationError( + 'The "other" category cannot be combined with another category' + ) + app_cats = CATEGORIES[APPS[app_name].id] + # We don't know the addon_type at this point, so try them all and we'll + # drop anything that's wrong later in AddonSerializer.validate + all_cat_slugs = set() + for type_cats in app_cats.values(): + categories.extend( + type_cats[name] for name in category_names if name in type_cats + ) + all_cat_slugs.update(type_cats.keys()) + # Now double-check all the category names were found + if not all_cat_slugs.issuperset(category_names): + raise exceptions.ValidationError('Invalid category name.') + return categories + except KeyError: + raise exceptions.ValidationError('Invalid app name.') + + def to_representation(self, value): + grouped = sorted_groupby( + sorted(value), + key=lambda x: getattr(amo.APP_IDS.get(x.application), 'short', ''), + ) + return { + app_name: [cat.slug for cat in categories] + for app_name, categories in grouped + } + + +class ContributionSerializerField(OutgoingURLField): + def to_representation(self, value): + if not value: + # don't add anything when it's not set. + return value + parts = urlsplit(value) + query = QueryDict(parts.query, mutable=True) + query.update(amo.CONTRIBUTE_UTM_PARAMS) + return super().to_representation( + urlunsplit( + ( + parts.scheme, + parts.netloc, + parts.path, + query.urlencode(), + parts.fragment, + ) + ) + ) + + +class LicenseNameSerializerField(serializers.Field): + """Field to handle license name translations. + + Builtin licenses, for better or worse, don't necessarily have their name + translated in the database like custom licenses. Instead, the string is in + this repos, and translated using gettext. This field deals with that + difference, delegating the rendering to TranslationSerializerField or + GetTextTranslationSerializerField depending on what the license instance + is. + """ + + builtin_translation_field_class = GetTextTranslationSerializerField + custom_translation_field_class = TranslationSerializerField + + def __init__(self, *args, **kwargs): + super().__init__(*args, **kwargs) + self.builtin_translation_field = self.builtin_translation_field_class() + self.custom_translation_field = self.custom_translation_field_class() + + def bind(self, field_name, parent): + super().bind(field_name, parent) + self.builtin_translation_field.bind(field_name, parent) + self.custom_translation_field.bind(field_name, parent) + + def get_attribute(self, obj): + if obj._constant: + return self.builtin_translation_field.get_attribute(obj._constant) + else: + return self.custom_translation_field.get_attribute(obj) + + def to_representation(self, obj): + # Like TranslationSerializerField, the bulk of the logic is in + # get_attribute(), we just have to return the data at this point. + return obj + + def run_validation(self, data=fields.empty): + return self.custom_translation_field.run_validation(data) + + def to_internal_value(self, value): + return self.custom_translation_field.to_internal_value(value) + + +class ESLicenseNameSerializerField(LicenseNameSerializerField): + """Like LicenseNameSerializerField, but uses the data from ES to avoid + a database query for custom licenses. + + BaseESSerializer automatically changes + TranslationSerializerField to ESTranslationSerializerField for all base + fields on the serializer, but License name has its own special field to + handle builtin licences so it's done separately.""" + + custom_translation_field_class = ESTranslationSerializerField + + def attach_translations(self, obj, data, field_name): + return self.custom_translation_field.attach_translations(obj, data, field_name) + + +class LicenseSlugSerializerField(serializers.SlugRelatedField): + def __init__(self, **kwargs): + super().__init__( + slug_field='builtin', + queryset=License.objects.exclude(builtin=License.OTHER), + **kwargs, + ) + + def to_internal_value(self, data): + license_ = LICENSES_BY_SLUG.get(data) + if not license_: + self.fail('invalid') + return super().to_internal_value(license_.builtin) + + +class SourceFileField(serializers.FileField): + def to_internal_value(self, data): + data = super().to_internal_value(data) + + # Ensure the file type is one we support. + if not data.name.endswith(VALID_SOURCE_EXTENSIONS): + error_msg = ( + 'Unsupported file type, please upload an archive file ({extensions}).' + ) + raise exceptions.ValidationError( + error_msg.format(extensions=(', '.join(VALID_SOURCE_EXTENSIONS))) + ) + + # Check inside to see if the file extension matches the content. + try: + _, ext = os.path.splitext(data.name) + if ext == '.zip': + # testzip() returns None if there are no broken CRCs. + if SafeZip(data).zip_file.testzip() is not None: + raise zipfile.BadZipFile() + else: + # For tar files we need to do a little more work. + mode = 'r:bz2' if ext == '.bz2' else 'r:gz' + with tarfile.open(mode=mode, fileobj=data) as archive: + for member in archive.getmembers(): + archive_member_validator(archive, member) + except (zipfile.BadZipFile, tarfile.ReadError, OSError, EOFError): + raise exceptions.ValidationError('Invalid or broken archive.') + + return data + + def to_representation(self, value): + if not value: + return None + else: + return absolutify(reverse('downloads.source', args=(self.parent.id,))) + + +class VersionCompatabilityField(serializers.Field): + def to_internal_value(self, data): + """Note: this returns unsaved and incomplete ApplicationsVersions objects that + need to have version set, and may have missing min or max AppVersion instances + for new Version instances. (As intended - we want to be able to partially + specify min or max and have the manifest or defaults be instead used). + """ + try: + if isinstance(data, list): + # if it's a list of apps, normalize into a dict first + data = {key: {} for key in data} + if isinstance(data, dict): + version = self.parent.instance + existing = version.compatible_apps if version else {} + qs = AppVersion.objects + internal = {} + for app_name, min_max in data.items(): + app = amo.APPS[app_name] + apps_versions = existing.get( + app, ApplicationsVersions(application=app.id) + ) + + app_qs = qs.filter(application=app.id) + if 'max' in min_max: + apps_versions.max = app_qs.get(version=min_max['max']) + elif version: + apps_versions.max = app_qs.get( + version=amo.DEFAULT_WEBEXT_MAX_VERSION + ) + + app_qs = app_qs.exclude(version='*') + if 'min' in min_max: + apps_versions.min = app_qs.get(version=min_max['min']) + elif version: + apps_versions.min = app_qs.get( + version=amo.DEFAULT_WEBEXT_MIN_VERSIONS[app] + ) + + internal[app] = apps_versions + return internal + else: + # if it's neither it's not a valid input + raise exceptions.ValidationError('Invalid value') + except KeyError: + raise exceptions.ValidationError('Invalid app specified') + except AppVersion.DoesNotExist: + raise exceptions.ValidationError('Unknown app version specified') + + def to_representation(self, value): + return { + app.short: ( + { + 'min': compat.min.version, + 'max': compat.max.version, + } + if compat + else { + 'min': amo.D2C_MIN_VERSIONS.get(app.id, '1.0'), + 'max': amo.FAKE_MAX_VERSION, + } + ) + for app, compat in value.items() + } diff --git a/src/olympia/addons/serializers.py b/src/olympia/addons/serializers.py index 4440c71a57ba..07d91d1d823e 100644 --- a/src/olympia/addons/serializers.py +++ b/src/olympia/addons/serializers.py @@ -1,24 +1,20 @@ import re -from urllib.parse import urlsplit, urlunsplit -from django.http.request import QueryDict from django.urls import reverse -from rest_framework import exceptions, fields, serializers +from rest_framework import exceptions, serializers from olympia import amo from olympia.accounts.serializers import ( BaseUserSerializer, UserProfileBasketSyncSerializer, ) +from olympia.activity.utils import log_and_notify from olympia.amo.templatetags.jinja_helpers import absolutify -from olympia.amo.utils import slug_validator, sorted_groupby +from olympia.amo.utils import slug_validator from olympia.api.fields import ( EmailTranslationField, - ESTranslationSerializerField, - GetTextTranslationSerializerField, LazyChoiceField, - OutgoingURLField, OutgoingURLTranslationField, ReverseChoiceField, SplitField, @@ -29,10 +25,9 @@ from olympia.applications.models import AppVersion from olympia.bandwagon.models import Collection from olympia.blocklist.models import Block -from olympia.constants.applications import APPS, APPS_ALL, APP_IDS +from olympia.constants.applications import APPS_ALL, APP_IDS from olympia.constants.base import ADDON_TYPE_CHOICES_API -from olympia.constants.categories import CATEGORIES, CATEGORIES_BY_ID -from olympia.constants.licenses import LICENSES_BY_SLUG +from olympia.constants.categories import CATEGORIES_BY_ID from olympia.constants.promoted import PROMOTED_GROUPS, RECOMMENDED from olympia.files.models import File, FileUpload from olympia.files.utils import parse_addon @@ -48,7 +43,16 @@ VersionPreview, ) -from .models import Addon, DeniedSlug, Preview, ReplacementAddon +from .fields import ( + CategoriesSerializerField, + ContributionSerializerField, + ESLicenseNameSerializerField, + LicenseNameSerializerField, + LicenseSlugSerializerField, + SourceFileField, + VersionCompatabilityField, +) +from .models import Addon, AddonReviewerFlags, DeniedSlug, Preview, ReplacementAddon class FileSerializer(serializers.ModelSerializer): @@ -147,78 +151,6 @@ def fake_object(self, data): return data -class LicenseNameSerializerField(serializers.Field): - """Field to handle license name translations. - - Builtin licenses, for better or worse, don't necessarily have their name - translated in the database like custom licenses. Instead, the string is in - this repos, and translated using gettext. This field deals with that - difference, delegating the rendering to TranslationSerializerField or - GetTextTranslationSerializerField depending on what the license instance - is. - """ - - builtin_translation_field_class = GetTextTranslationSerializerField - custom_translation_field_class = TranslationSerializerField - - def __init__(self, *args, **kwargs): - super().__init__(*args, **kwargs) - self.builtin_translation_field = self.builtin_translation_field_class() - self.custom_translation_field = self.custom_translation_field_class() - - def bind(self, field_name, parent): - super().bind(field_name, parent) - self.builtin_translation_field.bind(field_name, parent) - self.custom_translation_field.bind(field_name, parent) - - def get_attribute(self, obj): - if obj._constant: - return self.builtin_translation_field.get_attribute(obj._constant) - else: - return self.custom_translation_field.get_attribute(obj) - - def to_representation(self, obj): - # Like TranslationSerializerField, the bulk of the logic is in - # get_attribute(), we just have to return the data at this point. - return obj - - def run_validation(self, data=fields.empty): - return self.custom_translation_field.run_validation(data) - - def to_internal_value(self, value): - return self.custom_translation_field.to_internal_value(value) - - -class ESLicenseNameSerializerField(LicenseNameSerializerField): - """Like LicenseNameSerializerField, but uses the data from ES to avoid - a database query for custom licenses. - - BaseESSerializer automatically changes - TranslationSerializerField to ESTranslationSerializerField for all base - fields on the serializer, but License name has its own special field to - handle builtin licences so it's done separately.""" - - custom_translation_field_class = ESTranslationSerializerField - - def attach_translations(self, obj, data, field_name): - return self.custom_translation_field.attach_translations(obj, data, field_name) - - -class LicenseSlugSerializerField(serializers.SlugRelatedField): - def __init__(self, **kwargs): - super().__init__( - slug_field='builtin', - queryset=License.objects.exclude(builtin=License.OTHER), - **kwargs, - ) - - def to_internal_value(self, data): - license_ = LICENSES_BY_SLUG.get(data) - if not license_: - self.fail('invalid') - return super().to_internal_value(license_.builtin) - - class LicenseSerializer(serializers.ModelSerializer): is_custom = serializers.SerializerMethodField() name = LicenseNameSerializerField() @@ -288,71 +220,6 @@ def to_representation(self, instance): return repr -class VersionCompatabilityField(serializers.Field): - def to_internal_value(self, data): - """Note: this returns unsaved and incomplete ApplicationsVersions objects that - need to have version set, and may have missing min or max AppVersion instances - for new Version instances. (As intended - we want to be able to partially - specify min or max and have the manifest or defaults be instead used). - """ - try: - if isinstance(data, list): - # if it's a list of apps, normalize into a dict first - data = {key: {} for key in data} - if isinstance(data, dict): - version = self.parent.instance - existing = version.compatible_apps if version else {} - qs = AppVersion.objects - internal = {} - for app_name, min_max in data.items(): - app = amo.APPS[app_name] - apps_versions = existing.get( - app, ApplicationsVersions(application=app.id) - ) - - app_qs = qs.filter(application=app.id) - if 'max' in min_max: - apps_versions.max = app_qs.get(version=min_max['max']) - elif version: - apps_versions.max = app_qs.get( - version=amo.DEFAULT_WEBEXT_MAX_VERSION - ) - - app_qs = app_qs.exclude(version='*') - if 'min' in min_max: - apps_versions.min = app_qs.get(version=min_max['min']) - elif version: - apps_versions.min = app_qs.get( - version=amo.DEFAULT_WEBEXT_MIN_VERSIONS[app] - ) - - internal[app] = apps_versions - return internal - else: - # if it's neither it's not a valid input - raise exceptions.ValidationError('Invalid value') - except KeyError: - raise exceptions.ValidationError('Invalid app specified') - except AppVersion.DoesNotExist: - raise exceptions.ValidationError('Unknown app version specified') - - def to_representation(self, value): - return { - app.short: ( - { - 'min': compat.min.version, - 'max': compat.max.version, - } - if compat - else { - 'min': amo.D2C_MIN_VERSIONS.get(app.id, '1.0'), - 'max': amo.FAKE_MAX_VERSION, - } - ) - for app, compat in value.items() - } - - class SimpleVersionSerializer(MinimalVersionSerializer): compatibility = VersionCompatabilityField( # default to just Desktop Firefox; most of the times developers don't develop @@ -408,11 +275,35 @@ class VersionSerializer(SimpleVersionSerializer): LicenseSlugSerializerField(required=False), LicenseSerializer(), ) + + class Meta: + model = Version + fields = ( + 'id', + 'channel', + 'compatibility', + 'edit_url', + 'file', + 'is_strict_compatibility_enabled', + 'license', + 'release_notes', + 'reviewed', + 'version', + ) + read_only_fields = fields + + def __init__(self, instance=None, data=serializers.empty, **kwargs): + self.addon = kwargs.pop('addon', None) + super().__init__(instance=instance, data=data, **kwargs) + + +class DeveloperVersionSerializer(VersionSerializer): custom_license = LicenseSerializer( write_only=True, required=False, source='license', ) + source = SourceFileField(required=False, allow_null=True) upload = serializers.SlugRelatedField( slug_field='uuid', queryset=FileUpload.objects.all(), write_only=True ) @@ -430,6 +321,7 @@ class Meta: 'license', 'release_notes', 'reviewed', + 'source', 'upload', 'version', ) @@ -438,16 +330,22 @@ class Meta: 'custom_license', 'license', 'release_notes', + 'source', 'upload', ) read_only_fields = tuple(set(fields) - set(writeable_fields)) def __init__(self, instance=None, data=serializers.empty, **kwargs): - self.addon = kwargs.pop('addon', None) if instance and isinstance(data, dict): data.pop('upload', None) # we only support upload field for create super().__init__(instance=instance, data=data, **kwargs) + def to_representation(self, instance): + # SourceFileField needs the version id to build the url. + if 'source' in self.fields and instance.source: + self.id = instance.id + return super().to_representation(instance) + def validate_upload(self, value): own_upload = (request := self.context.get('request')) and ( request.user == value.user @@ -563,6 +461,20 @@ def validate(self, data): return data + def _update_admin_review_flag_and_logging(self, version): + if version.source and not version.addon.needs_admin_code_review: + AddonReviewerFlags.objects.update_or_create( + addon=version.addon, defaults={'needs_admin_code_review': True} + ) + # Add Activity Log, notifying staff, relevant reviewers and other authors of + # the add-on. + log_and_notify( + amo.LOG.SOURCE_CODE_UPLOADED, + None, + self.context['request'].user, + version, + ) + def create(self, validated_data): upload = validated_data.get('upload') parsed_and_validated_data = { @@ -591,6 +503,12 @@ def create(self, validated_data): and upload.channel == amo.RELEASE_CHANNEL_LISTED ): self.addon.update(status=amo.STATUS_NOMINATED) + + if 'source' in validated_data: + version.source = validated_data['source'] + version.save() + self._update_admin_review_flag_and_logging(version) + return version def update(self, instance, validated_data): @@ -612,15 +530,22 @@ def update(self, instance, validated_data): instance.update( license=self.fields['custom_license'].create(custom_license) ) + if 'source' in validated_data: + self._update_admin_review_flag_and_logging(instance) return instance -class VersionListSerializer(VersionSerializer): +class ListVersionSerializer(VersionSerializer): # When we're listing versions, we don't want to include the full license # text every time: we only do this for the version detail endpoint. license = CompactLicenseSerializer() +class DeveloperListVersionSerializer(DeveloperVersionSerializer): + # As ListVersionSerializer, but based on DeveloperVersionSerializer instead + license = CompactLicenseSerializer() + + class CurrentVersionSerializer(SimpleVersionSerializer): def to_representation(self, obj): # If the add-on is a langpack, and `appversion` is passed, try to @@ -722,68 +647,6 @@ def get_apps(self, obj): return [app.short for app in obj.approved_applications] -class CategoriesSerializerField(serializers.Field): - def to_internal_value(self, data): - try: - categories = [] - for app_name, category_names in data.items(): - if len(category_names) > amo.MAX_CATEGORIES: - raise exceptions.ValidationError( - 'Maximum number of categories per application ' - f'({amo.MAX_CATEGORIES}) exceeded' - ) - if len(category_names) > 1 and 'other' in category_names: - raise exceptions.ValidationError( - 'The "other" category cannot be combined with another category' - ) - app_cats = CATEGORIES[APPS[app_name].id] - # We don't know the addon_type at this point, so try them all and we'll - # drop anything that's wrong later in AddonSerializer.validate - all_cat_slugs = set() - for type_cats in app_cats.values(): - categories.extend( - type_cats[name] for name in category_names if name in type_cats - ) - all_cat_slugs.update(type_cats.keys()) - # Now double-check all the category names were found - if not all_cat_slugs.issuperset(category_names): - raise exceptions.ValidationError('Invalid category name.') - return categories - except KeyError: - raise exceptions.ValidationError('Invalid app name.') - - def to_representation(self, value): - grouped = sorted_groupby( - sorted(value), - key=lambda x: getattr(amo.APP_IDS.get(x.application), 'short', ''), - ) - return { - app_name: [cat.slug for cat in categories] - for app_name, categories in grouped - } - - -class ContributionSerializerField(OutgoingURLField): - def to_representation(self, value): - if not value: - # don't add anything when it's not set. - return value - parts = urlsplit(value) - query = QueryDict(parts.query, mutable=True) - query.update(amo.CONTRIBUTE_UTM_PARAMS) - return super().to_representation( - urlunsplit( - ( - parts.scheme, - parts.netloc, - parts.path, - query.urlencode(), - parts.fragment, - ) - ) - ) - - class AddonSerializer(serializers.ModelSerializer): authors = AddonDeveloperSerializer( many=True, source='listed_authors', read_only=True @@ -829,7 +692,7 @@ class AddonSerializer(serializers.ModelSerializer): choices=list(amo.ADDON_TYPE_CHOICES_API.items()), read_only=True ) url = serializers.SerializerMethodField() - version = VersionSerializer(write_only=True) + version = DeveloperVersionSerializer(write_only=True) versions_url = serializers.SerializerMethodField() class Meta: diff --git a/src/olympia/addons/tests/test_serializers.py b/src/olympia/addons/tests/test_serializers.py index cfbc6fd2062f..f76c9c1ad941 100644 --- a/src/olympia/addons/tests/test_serializers.py +++ b/src/olympia/addons/tests/test_serializers.py @@ -20,10 +20,13 @@ AddonDeveloperSerializer, AddonSerializer, AddonSerializerWithUnlistedData, + DeveloperVersionSerializer, + DeveloperListVersionSerializer, ESAddonAutoCompleteSerializer, ESAddonSerializer, LanguageToolsSerializer, LicenseSerializer, + ListVersionSerializer, ReplacementAddonSerializer, SimpleVersionSerializer, VersionSerializer, @@ -1031,13 +1034,15 @@ def test_grouped_ratings(self): class TestVersionSerializerOutput(TestCase): + serializer_class = VersionSerializer + def setUp(self): super().setUp() self.request = APIRequestFactory().get('/') self.request.version = 'v5' def serialize(self): - serializer = VersionSerializer(context={'request': self.request}) + serializer = self.serializer_class(context={'request': self.request}) return serializer.to_representation(self.version) def test_basic(self): @@ -1284,20 +1289,37 @@ def test_version_files_or_file(self): assert 'files' in result assert result['files'] == [default_file_result] + +class TestDeveloperVersionSerializerOutput(TestVersionSerializerOutput): + serializer_class = DeveloperVersionSerializer + def test_readonly_fields(self): - serializer = VersionSerializer() + 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) + def test_source(self): + self.version = addon_factory().current_version + result = self.serialize() + assert result['source'] is None + + self.version.update(source='whatever.zip') + result = self.serialize() + assert result['source'] == absolutify( + reverse('downloads.source', args=(self.version.id,)) + ) + + +class TestListVersionSerializerOutput(TestCase): + serializer_class = ListVersionSerializer -class TestVersionListSerializerOutput(TestCase): def setUp(self): self.request = APIRequestFactory().get('/') def serialize(self): - serializer = SimpleVersionSerializer(context={'request': self.request}) + serializer = self.serializer_class(context={'request': self.request}) return serializer.to_representation(self.version) def test_basic(self): @@ -1306,6 +1328,10 @@ def test_basic(self): assert 'text' not in result['license'] +class TestDeveloperListVersionSerializerOutput(TestListVersionSerializerOutput): + serializer_class = DeveloperListVersionSerializer + + class TestSimpleVersionSerializerOutput(TestCase): def setUp(self): self.request = APIRequestFactory().get('/') diff --git a/src/olympia/addons/tests/test_views.py b/src/olympia/addons/tests/test_views.py index 98cccc751fb8..c5d4155194d4 100644 --- a/src/olympia/addons/tests/test_views.py +++ b/src/olympia/addons/tests/test_views.py @@ -1,9 +1,17 @@ +import io import json +import mimetypes +import os +import stat +import tarfile +import tempfile +import zipfile from unittest import mock from django.conf import settings from django.core.cache import cache +from django.core.files.uploadedfile import SimpleUploadedFile from django.test.utils import override_settings from django.urls import reverse from django.utils.encoding import force_bytes, force_str @@ -19,7 +27,7 @@ from olympia import amo from olympia.activity.models import ActivityLog -from olympia.addons.models import AddonCategory, DeniedSlug +from olympia.addons.models import AddonCategory, AddonReviewerFlags, DeniedSlug from olympia.amo.tests import ( APITestClientWebToken, ESTestCase, @@ -59,8 +67,8 @@ from ..serializers import ( AddonSerializer, AddonSerializerWithUnlistedData, + DeveloperVersionSerializer, LicenseSerializer, - VersionSerializer, ) from ..utils import generate_addon_guid from ..views import ( @@ -1680,8 +1688,134 @@ def test_unlisted_version_user_but_not_author(self): response = self.client.get(self.url) assert response.status_code == 403 + def test_developer_version_serializer_used_for_authors(self): + self.version.update(source='src.zip') + # not logged in + assert 'source' not in self.client.get(self.url).data -class TestVersionViewSetCreate(UploadMixin, TestCase): + user = UserProfile.objects.create(username='user') + self.client.login_api(user) + + # logged in but not an author + assert 'source' not in self.client.get(self.url).data + + AddonUser.objects.create(user=user, addon=self.addon) + + # the field is present when the user is an author of the add-on. + assert 'source' in self.client.get(self.url).data + + +class SubmitSourceMixin: + def _generate_source_tar(self, suffix='.tar.gz', data=b't' * (2 ** 21), mode=None): + source = tempfile.NamedTemporaryFile(suffix=suffix, dir=settings.TMP_PATH) + if mode is None: + mode = 'w:bz2' if suffix.endswith('.tar.bz2') else 'w:gz' + with tarfile.open(fileobj=source, mode=mode) as tar_file: + tar_info = tarfile.TarInfo('foo') + tar_info.size = len(data) + tar_file.addfile(tar_info, io.BytesIO(data)) + + source.seek(0) + return source + + def _generate_source_zip( + self, suffix='.zip', data='z' * (2 ** 21), compression=zipfile.ZIP_DEFLATED + ): + source = tempfile.NamedTemporaryFile(suffix=suffix, dir=settings.TMP_PATH) + with zipfile.ZipFile(source, 'w', compression=compression) as zip_file: + zip_file.writestr('foo', data) + source.seek(0) + return source + + def test_source_zip(self): + _, version = self._submit_source( + self.file_path('webextension_with_image.zip'), + ) + assert version.source + assert str(version.source).endswith('.zip') + assert self.addon.needs_admin_code_review + mode = '0%o' % (os.stat(version.source.path)[stat.ST_MODE]) + assert mode == '0100644' + + def test_source_targz(self): + _, version = self._submit_source(self.file_path('webextension_no_id.tar.gz')) + assert version.source + assert str(version.source).endswith('.tar.gz') + assert self.addon.needs_admin_code_review + mode = '0%o' % (os.stat(version.source.path)[stat.ST_MODE]) + assert mode == '0100644' + + def test_source_tgz(self): + _, version = self._submit_source(self.file_path('webextension_no_id.tgz')) + assert version.source + assert str(version.source).endswith('.tgz') + assert self.addon.needs_admin_code_review + mode = '0%o' % (os.stat(version.source.path)[stat.ST_MODE]) + assert mode == '0100644' + + def test_source_tarbz2(self): + _, version = self._submit_source( + self.file_path('webextension_no_id.tar.bz2'), + ) + assert version.source + assert str(version.source).endswith('.tar.bz2') + assert self.addon.needs_admin_code_review + mode = '0%o' % (os.stat(version.source.path)[stat.ST_MODE]) + assert mode == '0100644' + + def test_with_bad_source_extension(self): + response, version = self._submit_source( + self.file_path('webextension_crx3.crx'), + error=True, + ) + assert response.data['source'] == [ + 'Unsupported file type, please upload an archive file ' + '(.zip, .tar.gz, .tgz, .tar.bz2).' + ] + assert not version or not version.source + self.addon.reload() + assert not self.addon.needs_admin_code_review + + def test_with_bad_source_broken_archive(self): + source = self._generate_source_zip( + data='Hello World', compression=zipfile.ZIP_STORED + ) + data = source.read().replace(b'Hello World', b'dlroW olleH') + source.seek(0) # First seek to rewrite from the beginning + source.write(data) + source.seek(0) # Second seek to reset like it's fresh. + # Still looks like a zip at first glance. + assert zipfile.is_zipfile(source) + source.seek(0) # Last seek to reset source descriptor before posting. + with open(source.name, 'rb'): + response, version = self._submit_source( + source.name, + error=True, + ) + assert response.data['source'] == ['Invalid or broken archive.'] + self.addon.reload() + assert not version or not version.source + assert not self.addon.needs_admin_code_review + + def test_with_bad_source_broken_archive_compressed_tar(self): + source = self._generate_source_tar() + with open(source.name, 'r+b') as fobj: + fobj.truncate(512) + # Still looks like a tar at first glance. + assert tarfile.is_tarfile(source.name) + # Re-open and post. + with open(source.name, 'rb'): + response, version = self._submit_source( + source.name, + error=True, + ) + assert response.data['source'] == ['Invalid or broken archive.'] + self.addon.reload() + assert not version or not version.source + assert not self.addon.needs_admin_code_review + + +class TestVersionViewSetCreate(UploadMixin, SubmitSourceMixin, TestCase): client_class = APITestClientWebToken @classmethod @@ -1736,7 +1870,7 @@ def test_basic_unlisted(self): request = APIRequestFactory().get('/') request.version = 'v5' request.user = self.user - assert data == VersionSerializer( + assert data == DeveloperVersionSerializer( context={'request': request} ).to_representation(version) assert version.channel == amo.RELEASE_CHANNEL_UNLISTED @@ -1762,7 +1896,7 @@ def test_basic_listed(self): request = APIRequestFactory().get('/') request.version = 'v5' request.user = self.user - assert data == VersionSerializer( + assert data == DeveloperVersionSerializer( context={'request': request} ).to_representation(version) assert version.channel == amo.RELEASE_CHANNEL_LISTED @@ -2077,12 +2211,31 @@ def test_cannot_submit_listed_to_disabled_(self): ) assert response.status_code == 201, response.content + def _submit_source(self, filepath, error=False): + _, filename = os.path.split(filepath) + src = SimpleUploadedFile( + filename, + open(filepath, 'rb').read(), + content_type=mimetypes.guess_type(filename)[0], + ) + response = self.client.post( + self.url, data={**self.minimal_data, 'source': src}, format='multipart' + ) + if not error: + assert response.status_code == 201, response.content + self.addon.reload() + version = self.addon.find_latest_version(channel=None) + else: + assert response.status_code == 400 + version = None + return response, version + class TestVersionViewSetCreateJWTAuth(TestVersionViewSetCreate): client_class = APITestClientJWT -class TestVersionViewSetUpdate(UploadMixin, TestCase): +class TestVersionViewSetUpdate(UploadMixin, SubmitSourceMixin, TestCase): client_class = APITestClientWebToken @classmethod @@ -2133,7 +2286,7 @@ def test_basic(self): request = APIRequestFactory().get('/') request.version = 'v5' request.user = self.user - assert data == VersionSerializer( + assert data == DeveloperVersionSerializer( context={'request': request} ).to_representation(version) @@ -2453,6 +2606,35 @@ def test_cannot_supply_both_custom_and_license_id(self): ] } + def test_source_set_null_clears_field(self): + AddonReviewerFlags.objects.create( + addon=self.version.addon, needs_admin_code_review=True + ) + self.version.update(source='src.zip') + response = self.client.patch( + self.url, + data={'source': None}, + ) + assert response.status_code == 200, response.content + self.version.reload() + assert not self.version.source + assert self.addon.needs_admin_code_review # still set + + def _submit_source(self, filepath, error=False): + _, filename = os.path.split(filepath) + src = SimpleUploadedFile( + filename, + open(filepath, 'rb').read(), + content_type=mimetypes.guess_type(filename)[0], + ) + response = self.client.patch(self.url, data={'source': src}, format='multipart') + if not error: + assert response.status_code == 200, response.content + else: + assert response.status_code == 400 + self.version.reload() + return response, self.version + class TestVersionViewSetUpdateJWTAuth(TestVersionViewSetUpdate): client_class = APITestClientJWT @@ -2770,6 +2952,25 @@ def test_all_without_unlisted_when_no_listed_versions_for_viewer(self): result = json.loads(force_str(response.content)) assert result['results'] == [] + def test_developer_version_serializer_used_for_authors(self): + self.version.update(source='src.zip') + # not logged in + assert 'source' not in self.client.get(self.url).data['results'][0] + assert 'source' not in self.client.get(self.url).data['results'][1] + + user = UserProfile.objects.create(username='user') + self.client.login_api(user) + + # logged in but not an author + assert 'source' not in self.client.get(self.url).data['results'][0] + assert 'source' not in self.client.get(self.url).data['results'][1] + + AddonUser.objects.create(user=user, addon=self.addon) + + # the field is present when the user is an author of the add-on. + assert 'source' in self.client.get(self.url).data['results'][0] + assert 'source' in self.client.get(self.url).data['results'][1] + class TestAddonViewSetEulaPolicy(TestCase): client_class = APITestClientWebToken diff --git a/src/olympia/addons/views.py b/src/olympia/addons/views.py index e03388bc01e8..92756b2e93f0 100644 --- a/src/olympia/addons/views.py +++ b/src/olympia/addons/views.py @@ -73,13 +73,15 @@ AddonEulaPolicySerializer, AddonSerializer, AddonSerializerWithUnlistedData, + DeveloperVersionSerializer, + DeveloperListVersionSerializer, ESAddonAutoCompleteSerializer, ESAddonSerializer, LanguageToolsSerializer, ReplacementAddonSerializer, StaticCategorySerializer, VersionSerializer, - VersionListSerializer, + ListVersionSerializer, ) from .utils import ( get_addon_recommendations, @@ -381,14 +383,28 @@ class AddonVersionViewSet( throttle_classes = addon_submission_throttles def get_serializer_class(self): + use_developer_serializer = getattr( + self.request, 'user', None + ) and acl.author_or_unlisted_viewer_or_reviewer( + self.request, self.get_addon_object() + ) + if ( self.action == 'list' and self.request and not is_gate_active(self.request, 'keep-license-text-in-version-list') ): - serializer_class = VersionListSerializer + serializer_class = ( + ListVersionSerializer + if not use_developer_serializer + else DeveloperListVersionSerializer + ) else: - serializer_class = VersionSerializer + serializer_class = ( + VersionSerializer + if not use_developer_serializer + else DeveloperVersionSerializer + ) return serializer_class def get_serializer(self, *args, **kwargs): diff --git a/src/olympia/files/fixtures/files/webextension_no_id.tgz b/src/olympia/files/fixtures/files/webextension_no_id.tgz new file mode 100644 index 000000000000..a15d11c1eed9 Binary files /dev/null and b/src/olympia/files/fixtures/files/webextension_no_id.tgz differ