diff --git a/src/olympia/reviewers/serializers.py b/src/olympia/reviewers/serializers.py index 047724e3b7..fe85125028 100644 --- a/src/olympia/reviewers/serializers.py +++ b/src/olympia/reviewers/serializers.py @@ -9,13 +9,13 @@ import magic from rest_framework import serializers from rest_framework.exceptions import NotFound -from rest_framework.reverse import reverse as drf_reverse from django.utils.functional import cached_property from django.utils.encoding import force_text from django.utils.timezone import FixedOffset from olympia.amo.urlresolvers import reverse +from olympia.amo.templatetags.jinja_helpers import absolutify from olympia.addons.serializers import ( VersionSerializer, FileSerializer, SimpleAddonSerializer) from olympia.addons.models import AddonReviewerFlags @@ -196,8 +196,6 @@ class FileEntriesSerializer(FileSerializer): self.git_repo[blob_or_tree.oid].read_raw()) def get_download_url(self, obj): - request = self.context.get('request') - commit = self._get_commit(obj) tree = self.repo.get_root_tree(commit) selected_file = self.get_selected_file(obj) @@ -206,15 +204,13 @@ class FileEntriesSerializer(FileSerializer): if blob_or_tree.type == 'tree': return None - return drf_reverse( - 'reviewers-versions-download', - request=request, + return absolutify(reverse( + 'reviewers.download_git_file', kwargs={ - 'addon_pk': self.get_instance().version.addon.pk, - 'pk': self.get_instance().version.pk, + 'version_id': self.get_instance().version.pk, 'filename': selected_file } - ) + )) class AddonBrowseVersionSerializer(VersionSerializer): diff --git a/src/olympia/reviewers/tests/test_serializers.py b/src/olympia/reviewers/tests/test_serializers.py index 516d6b4b54..2be34fd80e 100644 --- a/src/olympia/reviewers/tests/test_serializers.py +++ b/src/olympia/reviewers/tests/test_serializers.py @@ -17,7 +17,7 @@ from olympia import amo from olympia.reviewers.serializers import ( AddonBrowseVersionSerializer, FileEntriesSerializer) from olympia.amo.urlresolvers import reverse -from olympia.amo.tests import TestCase, addon_factory, reverse_ns +from olympia.amo.tests import TestCase, addon_factory from olympia.amo.templatetags.jinja_helpers import absolutify from olympia.versions.tasks import extract_version_to_git from olympia.versions.models import License @@ -63,14 +63,13 @@ class TestFileEntriesSerializer(TestCase): '/notify-link-clicks-i18n.xpi?src=').format(file.pk) assert data['selected_file'] == 'manifest.json' - assert data['download_url'] == reverse_ns( - 'reviewers-versions-download', + assert data['download_url'] == absolutify(reverse( + 'reviewers.download_git_file', kwargs={ - 'addon_pk': self.addon.pk, - 'pk': self.addon.current_version.pk, + 'version_id': self.addon.current_version.pk, 'filename': 'manifest.json' } - ) + )) assert set(data['entries'].keys()) == { 'README.md', @@ -131,14 +130,13 @@ class TestFileEntriesSerializer(TestCase): assert data['selected_file'] == 'icons/LICENSE' assert data['content'].startswith( 'The "link-48.png" icon is taken from the Geomicons') - assert data['download_url'] == reverse_ns( - 'reviewers-versions-download', + assert data['download_url'] == absolutify(reverse( + 'reviewers.download_git_file', kwargs={ - 'addon_pk': self.addon.pk, - 'pk': self.addon.current_version.pk, + 'version_id': self.addon.current_version.pk, 'filename': 'icons/LICENSE' } - ) + )) def test_get_entries_cached(self): file = self.addon.current_version.current_file diff --git a/src/olympia/reviewers/tests/test_views.py b/src/olympia/reviewers/tests/test_views.py index 1b51c6cc3a..c3b1fe0940 100644 --- a/src/olympia/reviewers/tests/test_views.py +++ b/src/olympia/reviewers/tests/test_views.py @@ -35,6 +35,7 @@ from olympia.amo.tests import ( APITestClient, TestCase, addon_factory, check_links, file_factory, formset, initial, reverse_ns, user_factory, version_factory) from olympia.amo.urlresolvers import reverse +from olympia.amo.templatetags.jinja_helpers import absolutify from olympia.files.models import File, FileValidation, WebextPermission from olympia.ratings.models import Rating, RatingFlag from olympia.reviewers.models import ( @@ -5271,14 +5272,13 @@ class TestReviewAddonVersionViewSetDetail( assert result['file']['content'] == '# beastify\n' # make sure the correct download url is correctly generated - assert result['file']['download_url'] == reverse_ns( - 'reviewers-versions-download', + assert result['file']['download_url'] == absolutify(reverse( + 'reviewers.download_git_file', kwargs={ - 'addon_pk': self.addon.pk, - 'pk': self.version.pk, + 'version_id': self.version.pk, 'filename': 'README.md' } - ) + )) def test_version_get_not_found(self): user = UserProfile.objects.create(username='reviewer') @@ -5319,61 +5319,6 @@ class TestReviewAddonVersionViewSetDetail( response = self.client.get(url) assert response.status_code == 404 - def test_download_basic(self): - user = UserProfile.objects.create(username='reviewer') - self.grant_permission(user, 'Addons:Review') - self.client.login_api(user) - - url = reverse_ns('reviewers-versions-download', kwargs={ - 'addon_pk': self.addon.pk, - 'pk': self.version.pk, - 'filename': 'manifest.json' - }) - - response = self.client.get(url) - assert response.status_code == 200 - assert ( - response['Content-Disposition'] == - 'attachment; filename="manifest.json"') - - def test_download_emoji_filename(self): - new_version = version_factory( - addon=self.addon, file_kw={'filename': 'webextension_no_id.xpi'}) - - repo = AddonGitRepository.extract_and_commit_from_version(new_version) - - apply_changes(repo, new_version, u'\n', u'๐Ÿ˜€โค.txt') - - user = UserProfile.objects.create(username='reviewer') - self.grant_permission(user, 'Addons:Review') - self.client.login_api(user) - - url = reverse_ns('reviewers-versions-download', kwargs={ - 'addon_pk': self.addon.pk, - 'pk': new_version.pk, - 'filename': u'๐Ÿ˜€โค.txt' - }) - - response = self.client.get(url) - assert response.status_code == 200 - assert ( - response['Content-Disposition'] == - "attachment; filename*=utf-8''%F0%9F%98%80%E2%9D%A4.txt") - - def test_download_notfound(self): - user = UserProfile.objects.create(username='reviewer') - self.grant_permission(user, 'Addons:Review') - self.client.login_api(user) - - url = reverse_ns('reviewers-versions-download', kwargs={ - 'addon_pk': self.addon.pk, - 'pk': self.version.pk, - 'filename': 'doesnotexist.json' - }) - - response = self.client.get(url) - assert response.status_code == 404 - class TestReviewAddonVersionViewSetList(TestCase): client_class = APITestClient @@ -5605,6 +5550,163 @@ class TestReviewAddonVersionCompareViewSet( ] +class TestDownloadGitFileView(TestCase): + def setUp(self): + super(TestDownloadGitFileView, self).setUp() + + self.addon = addon_factory( + name=u'My Addรดn', slug='my-addon', + file_kw={'filename': 'webextension_no_id.xpi'}) + + extract_version_to_git(self.addon.current_version.pk) + + self.version = self.addon.current_version + self.version.refresh_from_db() + + def test_download_basic(self): + user = UserProfile.objects.create(username='reviewer') + self.grant_permission(user, 'Addons:Review') + self.client.login(email=user.email) + + url = reverse('reviewers.download_git_file', kwargs={ + 'version_id': self.version.pk, + 'filename': 'manifest.json' + }) + + response = self.client.get(url) + assert response.status_code == 200 + assert ( + response['Content-Disposition'] == + 'attachment; filename="manifest.json"') + + def test_download_emoji_filename(self): + new_version = version_factory( + addon=self.addon, file_kw={'filename': 'webextension_no_id.xpi'}) + + repo = AddonGitRepository.extract_and_commit_from_version(new_version) + + apply_changes(repo, new_version, u'\n', u'๐Ÿ˜€โค.txt') + + user = UserProfile.objects.create(username='reviewer') + self.grant_permission(user, 'Addons:Review') + self.client.login(email=user.email) + + url = reverse('reviewers.download_git_file', kwargs={ + 'version_id': new_version.pk, + 'filename': u'๐Ÿ˜€โค.txt' + }) + + response = self.client.get(url) + assert response.status_code == 200 + assert ( + response['Content-Disposition'] == + "attachment; filename*=utf-8''%F0%9F%98%80%E2%9D%A4.txt") + + def test_download_notfound(self): + user = UserProfile.objects.create(username='reviewer') + self.grant_permission(user, 'Addons:Review') + self.client.login(email=user.email) + + url = reverse('reviewers.download_git_file', kwargs={ + 'version_id': self.version.pk, + 'filename': 'doesnotexist.json' + }) + + response = self.client.get(url) + assert response.status_code == 404 + + def _test_url_success(self): + url = reverse('reviewers.download_git_file', kwargs={ + 'version_id': self.version.pk, + 'filename': 'manifest.json' + }) + + response = self.client.get(url) + assert response.status_code == 200 + + def test_disabled_version_reviewer(self): + user = UserProfile.objects.create(username='reviewer') + self.grant_permission(user, 'Addons:Review') + self.client.login(email=user.email) + self.version.files.update(status=amo.STATUS_DISABLED) + self._test_url_success() + + def test_disabled_version_author(self): + user = UserProfile.objects.create(username='author') + AddonUser.objects.create(user=user, addon=self.addon) + self.client.login(email=user.email) + self.version.files.update(status=amo.STATUS_DISABLED) + self._test_url_success() + + def test_disabled_version_admin(self): + user = UserProfile.objects.create(username='admin') + self.grant_permission(user, '*:*') + self.client.login(email=user.email) + self.version.files.update(status=amo.STATUS_DISABLED) + self._test_url_success() + + def test_disabled_version_user_but_not_author(self): + user = UserProfile.objects.create(username='simpleuser') + self.client.login(email=user.email) + self.version.files.update(status=amo.STATUS_DISABLED) + + url = reverse('reviewers.download_git_file', kwargs={ + 'version_id': self.version.pk, + 'filename': 'manifest.json' + }) + + response = self.client.get(url) + assert response.status_code == 403 + + def test_unlisted_version_reviewer(self): + user = UserProfile.objects.create(username='reviewer') + self.grant_permission(user, 'Addons:Review') + self.client.login(email=user.email) + self.version.update(channel=amo.RELEASE_CHANNEL_UNLISTED) + + url = reverse('reviewers.download_git_file', kwargs={ + 'version_id': self.version.pk, + 'filename': 'manifest.json' + }) + + response = self.client.get(url) + assert response.status_code == 404 + + def test_unlisted_version_unlisted_reviewer(self): + user = UserProfile.objects.create(username='reviewer') + self.grant_permission(user, 'Addons:ReviewUnlisted') + self.client.login(email=user.email) + self.version.update(channel=amo.RELEASE_CHANNEL_UNLISTED) + self._test_url_success() + + def test_unlisted_version_author(self): + user = UserProfile.objects.create(username='author') + AddonUser.objects.create(user=user, addon=self.addon) + self.client.login(email=user.email) + self.version.update(channel=amo.RELEASE_CHANNEL_UNLISTED) + self._test_url_success() + + def test_unlisted_version_admin(self): + user = UserProfile.objects.create(username='admin') + self.grant_permission(user, '*:*') + self.client.login(email=user.email) + self.version.update(channel=amo.RELEASE_CHANNEL_UNLISTED) + self._test_url_success() + + def test_unlisted_version_user_but_not_author(self): + user = UserProfile.objects.create(username='simpleuser') + self.client.login(email=user.email) + self.version.update(channel=amo.RELEASE_CHANNEL_UNLISTED) + + url = reverse('reviewers.download_git_file', kwargs={ + 'version_id': self.version.pk, + 'filename': 'manifest.json' + }) + + response = self.client.get(url) + assert response.status_code == 404 + + class TestThemeBackgroundImages(ReviewBase): def setUp(self): diff --git a/src/olympia/reviewers/urls.py b/src/olympia/reviewers/urls.py index fd809934a3..b3000dd30c 100644 --- a/src/olympia/reviewers/urls.py +++ b/src/olympia/reviewers/urls.py @@ -61,4 +61,7 @@ urlpatterns = ( url(r'^theme_background_images/(?P[^ /]+)?$', views.theme_background_images, name='reviewers.theme_background_images'), + url(r'^download-git-file/(?P\d+)/(?P.*)/', + views.download_git_stored_file, + name='reviewers.download_git_file'), ) diff --git a/src/olympia/reviewers/views.py b/src/olympia/reviewers/views.py index f10793e3f3..b1b3186b98 100644 --- a/src/olympia/reviewers/views.py +++ b/src/olympia/reviewers/views.py @@ -30,7 +30,8 @@ from olympia.abuse.models import AbuseReport from olympia.access import acl from olympia.accounts.views import API_TOKEN_COOKIE from olympia.activity.models import ActivityLog, AddonLog, CommentLog -from olympia.addons.decorators import addon_view, addon_view_factory +from olympia.addons.decorators import ( + addon_view, addon_view_factory, owner_or_unlisted_reviewer) from olympia.addons.models import ( Addon, AddonApprovalsCounter, AddonReviewerFlags) from olympia.amo.decorators import ( @@ -1161,6 +1162,63 @@ def theme_background_images(request, version_id): return version.get_background_images_encoded(header_only=False) +@login_required +def download_git_stored_file(request, version_id, filename): + version = get_object_or_404(Version.unfiltered, id=int(version_id)) + + try: + addon = version.addon + except Addon.DoesNotExist: + raise http.Http404 + + if version.channel == amo.RELEASE_CHANNEL_LISTED: + is_owner = acl.check_addon_ownership(request, addon, dev=True) + if not (acl.is_reviewer(request, addon) or is_owner): + raise PermissionDenied + else: + if not owner_or_unlisted_reviewer(request, addon): + raise http.Http404 + + file = version.current_file + + serializer = FileEntriesSerializer( + instance=file, context={ + 'file': filename, + 'request': request + } + ) + + commit = serializer._get_commit(file) + tree = serializer.repo.get_root_tree(commit) + + try: + blob_or_tree = tree[serializer.get_selected_file(file)] + + if blob_or_tree.type == 'tree': + return http.HttpResponseBadRequest('Can\'t serve directories') + selected_file = serializer.get_entries(file)[filename] + except KeyError: + raise http.Http404() + + actual_blob = serializer.git_repo[blob_or_tree.oid] + response = http.FileResponse( + memoryview(actual_blob), + content_type=selected_file['mimetype']) + + # Backported from Django 2.1 to handle unicode filenames properly + selected_filename = selected_file['filename'] + try: + selected_filename.encode('ascii') + file_expr = 'filename="{}"'.format(selected_filename) + except UnicodeEncodeError: + file_expr = "filename*=utf-8''{}".format(urlquote(selected_filename)) + + response['Content-Disposition'] = 'attachment; {}'.format(file_expr) + response['Content-Length'] = actual_blob.size + + return response + + class AddonReviewerViewSet(GenericViewSet): log = olympia.core.logger.getLogger('z.reviewers') @@ -1322,53 +1380,6 @@ class ReviewAddonVersionViewSet(ReviewAddonVersionMixin, ListModelMixin, ) return Response(serializer.data) - @action( - detail=True, - methods=['get'], - permission_classes=ReviewAddonVersionMixin.permission_classes, - url_path='download/(?P.*)') - def download(self, request, *args, **kwargs): - version = self.get_object() - file = version.current_file - file_param = kwargs['filename'] - - serializer = FileEntriesSerializer( - instance=file, context={ - 'file': file_param, - 'request': request - } - ) - - commit = serializer._get_commit(file) - tree = serializer.repo.get_root_tree(commit) - - try: - blob_or_tree = tree[serializer.get_selected_file(file)] - - if blob_or_tree.type == 'tree': - return http.HttpResponseBadRequest('Can\'t serve directories') - selected_file = serializer.get_entries(file)[file_param] - except KeyError: - raise http.Http404() - - actual_blob = serializer.git_repo[blob_or_tree.oid] - response = http.FileResponse( - memoryview(actual_blob), - content_type=selected_file['mimetype']) - - # Backported from Django 2.1 to handle unicode filenames properly - filename = selected_file['filename'] - try: - filename.encode('ascii') - file_expr = 'filename="{}"'.format(filename) - except UnicodeEncodeError: - file_expr = "filename*=utf-8''{}".format(urlquote(filename)) - - response['Content-Disposition'] = 'attachment; {}'.format(file_expr) - response['Content-Length'] = actual_blob.size - - return response - class ReviewAddonVersionCompareViewSet(ReviewAddonVersionMixin, RetrieveModelMixin, GenericViewSet):