add in token generation and storage (bug #643482)

apps/amo/tests/test_helpers.py

@@ -15,7 +15,7 @@ import test_utils
 
 import amo
 from amo import urlresolvers, utils, helpers
-from amo.utils import ImageCheck
+from amo.utils import ImageCheck, Token
 from versions.models import License
 
 
@@ -363,6 +363,42 @@ class TestAnimatedImages(test_utils.TestCase):
         assert img.is_image()
 
 
+class TestToken(test_utils.TestCase):
+
+    def test_token_pop(self):
+        new = Token()
+        new.save()
+        assert Token.pop(new.token)
+        assert not Token.pop(new.token)
+
+    def test_token_valid(self):
+        new = Token()
+        new.save()
+        assert Token.valid(new.token)
+
+    def test_token_fails(self):
+        assert not Token.pop('some-random-token')
+
+    def test_token_ip(self):
+        new = Token(data='127.0.0.1')
+        new.save()
+        assert Token.valid(new.token, '127.0.0.1')
+
+    def test_token_no_ip_invalid(self):
+        new = Token()
+        assert not Token.valid(new.token, '255.255.255.0')
+
+    def test_token_bad_ip_invalid(self):
+        new = Token(data='127.0.0.1')
+        new.save()
+        assert not Token.pop(new.token, '255.255.255.0')
+        assert Token.pop(new.token, '127.0.0.1')
+
+    def test_token_well_formed(self):
+        new = Token('some badly formed token')
+        assert not new.well_formed()
+
+
 def test_site_nav():
     r = Mock()
     r.APP = amo.FIREFOX

apps/amo/utils.py

@@ -8,6 +8,7 @@ import time
 import unicodedata
 import urllib
 import urlparse
+import uuid
 
 from django import http
 from django.conf import settings
@@ -447,3 +448,46 @@ def memoize(prefix, time=60):
         return wrapper
     return decorator
 
+
+class Token:
+    _well_formed = re.compile('^[a-z0-9-]+$')
+
+    def __init__(self, token=None, data=True):
+        if token is None:
+            token = str(uuid.uuid4())
+        self.token = token
+        self.data = data
+
+    def cache_key(self):
+        assert self.token, 'No token value set.'
+        return '%s:token:%s' % (settings.CACHE_PREFIX, self.token)
+
+    def save(self, time=60):
+        cache.set(self.cache_key(), self.data, time)
+
+    def well_formed(self):
+        return self._well_formed.match(self.token)
+
+    @classmethod
+    def valid(cls, key, data=True):
+        """Checks that the token is valid."""
+        token = cls(key)
+        if not token.well_formed():
+            return False
+        result = cache.get(token.cache_key())
+        if result is not None:
+            return result == data
+        return False
+
+    @classmethod
+    def pop(cls, key, data=True):
+        """Checks that the token is valid and deletes it."""
+        token = cls(key)
+        if not token.well_formed():
+            return False
+        result = cache.get(token.cache_key())
+        if result is not None:
+            if result == data:
+                cache.delete(token.cache_key())
+                return True
+        return False

apps/files/decorators.py

@@ -4,6 +4,7 @@ from django import http
 from django.core.exceptions import ObjectDoesNotExist
 from django.shortcuts import get_object_or_404
 
+from amo.utils import Token
 from access.acl import check_addon_ownership, action_allowed
 from files.helpers import DiffHelper, FileViewer
 from files.models import File
@@ -51,3 +52,17 @@ def compare_file_view(func, **kwargs):
 
         return func(request, DiffHelper(one, two), *args, **kw)
     return wrapper
+
+
+def file_view_token(func, **kwargs):
+    @functools.wraps(func)
+    def wrapper(request, file_id, key, *args, **kw):
+        viewer = FileViewer(get_object_or_404(File, pk=file_id))
+        token = request.GET.get('token')
+        if not token:
+            return http.HttpResponseForbidden()
+        if not Token.valid(token, [request.META.get('REMOTE_ADDR'),
+                                   viewer.file.id, key]):
+            return http.HttpResponseForbidden()
+        return func(request, viewer, key, *args, **kw)
+    return wrapper

apps/files/helpers.py

@@ -128,22 +128,20 @@ class FileViewer:
             filename = smart_unicode(filename)
             short = smart_unicode(path[len(self.dest) + 1:])
             mime, encoding = mimetypes.guess_type(filename)
-            binary = self.is_binary(mime, filename)
-            # TODO: handling for st.a.m.o will be going in here
-            url = reverse('files.list', args=[self.file.id, short])
             directory = os.path.isdir(path)
-            res[short] = {'full': path,
-                          'filename': filename,
-                          'truncated': self.truncate(filename),
-                          'short': short,
-                          'directory': directory,
-                          'url': url,
-                          'binary': binary,
+            args = [self.file.id, short]
+            res[short] = {'binary': self.is_binary(mime, filename),
                           'depth': short.count(os.sep),
-                          'modified': os.stat(path)[stat.ST_MTIME],
-                          'mimetype': mime or 'application/octet-stream',
-                          'encoding': encoding,
+                          'directory': directory,
+                          'filename': filename,
+                          'full': path,
                           'md5': get_md5(path) if not directory else '',
+                          'mimetype': mime or 'application/octet-stream',
+                          'modified': os.stat(path)[stat.ST_MTIME],
+                          'short': short,
+                          'truncated': self.truncate(filename),
+                          'url': reverse('files.list', args=args),
+                          'url_serve': reverse('files.redirect', args=args),
                           'parent': '/'.join(short.split(os.sep)[:-1])}
         return res
 
@@ -170,12 +168,12 @@ class DiffHelper:
     def is_extracted(self):
         return self.file_one.is_extracted and self.file_two.is_extracted
 
-    def primary_files(self):
+    def get_files(self, file_obj):
         """
         Get the files from the primary and remap any diffable ones
         to the compare url as opposed to the other url.
         """
-        files = self.file_one.get_files()
+        files = file_obj.get_files()
         for file in files.values():
             file['url'] = reverse('files.compare',
                                   args=[self.file_one.file.id,
@@ -186,8 +184,8 @@ class DiffHelper:
 
     def select(self, key):
         self.key = key
-        self.one = self.file_one.get_files().get(key)
-        self.two = self.file_two.get_files().get(key)
+        self.one = self.get_files(self.file_one).get(key)
+        self.two = self.get_files(self.file_two).get(key)
 
     def is_different(self):
         if self.one and self.two:

apps/files/templates/files/file.html

@@ -0,0 +1,7 @@
+<dl>
+    <dt>{{ _('Version:') }}</dt><dd>{{ file.version }}</dd>
+    {# L10n: {0} is the filename #}
+    <dt>{{ _('File:') }}</dt><dd><a href="{{ selected['url_serve'] }}">{{ _('Download {0}').format(selected['filename']) }}</a></dd>
+    <dt>{{ _('MD5 hash:') }}</dt><dd>{{ selected['md5'] }}</dd>
+    <dt>{{ _('Mimetype:') }}</dt><dd>{{ selected['mimetype'] }}</dd>
+</dl>

apps/files/templates/files/viewer.html

@@ -53,7 +53,7 @@
         <div>
         {% if viewer %}
             {% if selected['binary'] and not selected['directory'] %}
-                <p>{{ _('Binary file, hash:') }} {{ selected.md5 }}</p>
+                {% include "files/file.html" %}
             {% else %}
                 {% if selected and content %}
                     <pre id="content" class="wrapped">{{ content }}</pre>
@@ -70,10 +70,16 @@
         {% if diff and diff.is_binary() %}
             <p>
             {% if diff.is_different() %}
-                {{ _('Files are different.') }}
-            {% else %}
-                {{ _('Files are the same.') }}
+                {{ _('Files are different.') }}<br/>
+              {% else %}
+                {{ _('Files are the same.') }}<br/>
             {% endif %}
+            {% with selected = diff.one, file = diff.file_one.file %}
+                {% include "files/file.html" %}
+            {% endwith %}
+            {% with selected = diff.two, file = diff.file_two.file %}
+                {% include "files/file.html" %}
+            {% endwith %}
             </p>
         {% endif %}
         {% if diff and not diff.is_binary() %}

apps/files/tests/test_helpers.py

@@ -12,7 +12,6 @@ import test_utils
 
 from amo.urlresolvers import reverse
 from files.helpers import FileViewer, DiffHelper
-from files.utils import extract_zip
 
 root = os.path.join(settings.ROOT, 'apps/files/fixtures/files')
 dictionary = '%s/dictionary-test.xpi' % root
@@ -148,8 +147,9 @@ class TestDiffHelper(test_utils.TestCase):
         self.helper.extract()
         eq_(self.helper.is_extracted, True)
 
-    def test_primary_files(self):
-        eq_(self.helper.file_one.get_files(), self.helper.primary_files())
+    def test_get_files(self):
+        eq_(self.helper.file_one.get_files(),
+            self.helper.get_files(self.helper.file_one))
 
     def test_diffable(self):
         self.helper.extract()

apps/files/tests/test_views.py

@@ -2,6 +2,7 @@ import json
 import os
 import shutil
 import tempfile
+import urlparse
 
 from django.conf import settings
 from django.core.cache import cache
@@ -17,9 +18,10 @@ from files.helpers import FileViewer, DiffHelper
 from files.models import File
 from users.models import UserProfile
 
-
 dictionary = 'apps/files/fixtures/files/dictionary-test.xpi'
 unicode_filenames = 'apps/files/fixtures/files/unicode-filenames.xpi'
+not_binary = 'install.js'
+binary = 'dictionaries/ar.dic'
 
 
 class FilesBase:
@@ -62,6 +64,12 @@ class FilesBase:
         settings.TMP_PATH = self.old_tmp
         settings.ADDONS_PATH = self.old_addon
 
+    def files_redirect(self, file):
+        return reverse('files.redirect', args=[self.file.pk, file])
+
+    def files_serve(self, file):
+        return reverse('files.serve', args=[self.file.pk, file])
+
     def test_view_access_anon(self):
         self.client.logout()
         self.check_urls(403)
@@ -133,7 +141,7 @@ class FilesBase:
 
     def test_file_header(self):
         self.file_viewer.extract()
-        res = self.client.get(self.file_url('install.js'))
+        res = self.client.get(self.file_url(not_binary))
         url = res.context['file_url']
         eq_(url, reverse('editors.review', args=[self.version.pk]))
 
@@ -141,7 +149,7 @@ class FilesBase:
         self.client.logout()
         self.file_viewer.extract()
         self.addon.update(view_source=True)
-        res = self.client.get(self.file_url('install.js'))
+        res = self.client.get(self.file_url(not_binary))
         url = res.context['file_url']
         eq_(url, reverse('addons.detail', args=[self.addon.pk]))
 
@@ -169,7 +177,7 @@ class FilesBase:
 
     def test_files_file(self):
         self.file_viewer.extract()
-        res = self.client.get(self.file_url('install.js'))
+        res = self.client.get(self.file_url(not_binary))
         eq_(res.status_code, 200)
         assert 'selected' in res.context
 
@@ -250,6 +258,44 @@ class TestFileViewer(FilesBase, test_utils.TestCase):
         res = self.client.get(self.file_url(iri_to_uri(u'\u1109\u1161\u11a9')))
         eq_(res.status_code, 200)
 
+    def test_serve_no_token(self):
+        self.file_viewer.extract()
+        res = self.client.get(self.files_serve(binary))
+        eq_(res.status_code, 403)
+
+    def test_serve_fake_token(self):
+        self.file_viewer.extract()
+        res = self.client.get(self.files_serve(binary) + '?token=aasd')
+        eq_(res.status_code, 403)
+
+    def test_serve_bad_token(self):
+        self.file_viewer.extract()
+        res = self.client.get(self.files_serve(binary) + '?token=a asd')
+        eq_(res.status_code, 403)
+
+    def test_serve_get_token(self):
+        self.file_viewer.extract()
+        res = self.client.get(self.files_redirect(binary))
+        eq_(res.status_code, 302)
+        url = res['Location']
+        assert url.startswith(settings.STATIC_URL)
+        assert urlparse.urlparse(url).query.startswith('token=')
+
+    def test_memcache_goes_bye_bye(self):
+        self.file_viewer.extract()
+        res = self.client.get(self.files_redirect(binary))
+        url = res['Location'][len(settings.STATIC_URL):]
+        cache.clear()
+        res = self.client.get(url)
+        eq_(res.status_code, 403)
+
+    def test_bounce(self):
+        self.file_viewer.extract()
+        res = self.client.get(self.files_redirect(binary), follow=True)
+        eq_(res.status_code, 200)
+        eq_(res['X-SENDFILE'],
+            self.file_viewer.get_files().get(binary)['full'])
+
 
 class TestDiffViewer(FilesBase, test_utils.TestCase):
     fixtures = ['base/addon_3615', 'base/users']
@@ -283,7 +329,15 @@ class TestDiffViewer(FilesBase, test_utils.TestCase):
 
     def test_content_file(self):
         self.file_viewer.extract()
-        res = self.client.get(self.file_url('install.js'))
+        res = self.client.get(self.file_url(not_binary))
         doc = pq(res.content)
         eq_(len(doc('#file-one')), 1)
         eq_(len(doc('#file-two')), 1)
+
+    def test_binary_serve_links(self):
+        self.file_viewer.extract()
+        res = self.client.get(self.file_url(binary))
+        doc = pq(res.content)
+        node = doc('#content-wrapper a')
+        eq_(len(node), 2)
+        assert node[0].text.startswith('Download ar.dic')

apps/files/urls.py

@@ -5,6 +5,9 @@ from files import views
 file_patterns = patterns('',
     url(r'^$', views.files_list, name='files.list'),
     url(r'file/(?P<key>.*)$', views.files_list, name='files.list'),
+    url(r'file-redirect/(?P<key>.*)$', views.files_redirect,
+        name='files.redirect'),
+    url(r'file-serve/(?P<key>.*)$', views.files_serve, name='files.serve'),
     url(r'status$', views.files_poll, name='files.poll'),
 )
 

apps/files/views.py

@@ -10,7 +10,8 @@ import jingo
 from access import acl
 from amo.decorators import json_view
 from amo.urlresolvers import reverse
-from files.decorators import file_view, compare_file_view
+from amo.utils import HttpResponseSendFile, Token
+from files.decorators import file_view, compare_file_view, file_view_token
 from files.tasks import extract_file
 
 
@@ -80,7 +81,7 @@ def files_compare(request, diff, key='install.rdf'):
                                      diff.file_two.file.id])
 
     if diff.is_extracted:
-        files = diff.primary_files()
+        files = diff.get_files(diff.file_one)
         data.update({'status': True, 'files': files})
 
         if key:
@@ -106,3 +107,26 @@ def files_compare(request, diff, key='install.rdf'):
         response['Last-Modified'] = http_date(data['selected']['modified'] if
                                               data['selected'] else None)
     return response
+
+
+@file_view
+def files_redirect(request, viewer, key):
+    new = Token(data=[request.META.get('REMOTE_ADDR'), viewer.file.id, key])
+    new.save()
+    url = '%s%s?token=%s' % (settings.STATIC_URL,
+                             reverse('files.serve', args=[viewer, key]),
+                             new.token)
+    return http.HttpResponseRedirect(url)
+
+
+@file_view_token
+def files_serve(request, viewer, key):
+    """
+    This is to serve files off of st.a.m.o, not standard a.m.o. For this we
+    use token based authentication.
+    """
+    files = viewer.get_files()
+    obj = files.get(key)
+    if not obj:
+        raise http.Http404()
+    return HttpResponseSendFile(request, obj['full'])

media/css/zamboni/files.css

@@ -112,3 +112,12 @@ pre del {
 #file-viewer p.help {
     clear: both;
 }
+
+dl dt {
+    padding: 0;
+    margin: 0;
+}
+
+dl dd {
+    padding-left: 1em;
+}