Move download url view from api to regular view. (#11027)
* Move download url view from api to regular view. This is needed to allow us to authenticate a user based on already set cookies instead of unnecessarily complicated through an API. Fixes #10994 * Fix tests
This commit is contained in:
Christopher Grebs
GitHub
Родитель
234c3068d8
Коммит
e67eeccaf8
|
|
@ -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):
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
|
|
|||
|
|
@ -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):
|
||||
|
|
|
|||
|
|
@ -61,4 +61,7 @@ urlpatterns = (
|
|||
url(r'^theme_background_images/(?P<version_id>[^ /]+)?$',
|
||||
views.theme_background_images,
|
||||
name='reviewers.theme_background_images'),
|
||||
url(r'^download-git-file/(?P<version_id>\d+)/(?P<filename>.*)/',
|
||||
views.download_git_stored_file,
|
||||
name='reviewers.download_git_file'),
|
||||
)
|
||||
|
|
|
|||
|
|
@ -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<filename>.*)')
|
||||
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):
|
||||
|
|
|
|||
Загрузка…
Ссылка в новой задаче