Support authenticated upload URLs.

Trac #1734

This is implemented by bouncing uploaded file links through a view
that checks authentication and redirects to an expiring S3 URL.

This makes file uploads return a domain-relative URI. The client converts
this to an absolute URI when it's in the composebox, then back to relative
when it's submitted to the server.

We need the relative URI because the same message may be viewed across
{staging,www,zephyr}.zulip.com, which have different cookies.

(imported from commit 33acb2abaa3002325f389d5198fb20ee1b30f5fa)

static/js/compose.js

@@ -5,6 +5,23 @@ var is_composing_message = false;
 var message_snapshot;
 var empty_subject_placeholder = "(no topic)";
 
+var uploads_domain = document.location.protocol + '//' + document.location.host;
+var uploads_path = '/user_uploads';
+var uploads_re = new RegExp("\\]\\(" + uploads_domain + "(" + uploads_path + "[^\\)]+)\\)", 'g');
+
+function make_upload_absolute(uri) {
+    if (uri.indexOf(uploads_path) === 0) {
+        // Rewrite the URI to a usable link
+        return uploads_domain + uri;
+    }
+    return uri;
+}
+
+function make_uploads_relative(content) {
+    // Rewrite uploads in markdown links back to domain-relative form
+    return content.replace(uploads_re, "]($1)");
+}
+
 function client() {
     if ((window.bridge !== undefined) &&
         (window.bridge.desktopAppVersion !== undefined)) {
@@ -248,12 +265,15 @@ function create_message_object() {
     if (subject === "") {
         subject = compose.empty_subject_placeholder();
     }
+
+    var content = make_uploads_relative(compose.message_content());
+
     var message = {client: client(),
                    type: compose.composing(),
                    subject: subject,
                    stream: compose.stream_name(),
                    private_message_recipient: compose.recipient(),
-                   content: compose.message_content()};
+                   content: content};
 
     if (message.type === "private") {
         // TODO: this should be collapsed with the code in composebox_typeahead.js
@@ -757,12 +777,15 @@ $(function () {
             if (!compose.composing()) {
                 compose.start('stream');
             }
+
+            var uri = make_upload_absolute(response.uri);
+
             if (i === -1) {
                 // This is a paste, so there's no filename. Show the image directly
-                textbox.val(textbox.val() + "[pasted image](" + response.uri + ") ");
+                textbox.val(textbox.val() + "[pasted image](" + uri + ") ");
             } else {
                 // This is a dropped file, so make the filename a link to the image
-                textbox.val(textbox.val() + "[" + filename + "](" + response.uri + ")" + " ");
+                textbox.val(textbox.val() + "[" + filename + "](" + uri + ")" + " ");
             }
             autosize_textarea();
             $("#compose-send-button").removeAttr("disabled");

zerver/lib/upload.py

@@ -66,9 +66,25 @@ def get_file_info(request, user_file):
         uploaded_file_name = uploaded_file_name + guess_extension(content_type)
     return uploaded_file_name, content_type
 
-def upload_message_image(uploaded_file_name, content_type, file_data, user_profile):
+def authed_upload_enabled(user_profile):
+    return user_profile.realm.domain == 'zulip.com'
+
+def upload_message_image(uploaded_file_name, content_type, file_data, user_profile, private=None):
+    if private is None:
+        private = authed_upload_enabled(user_profile)
+    if private:
+        bucket_name = settings.S3_AUTH_UPLOADS_BUCKET
+        s3_file_name = "/".join([
+            str(user_profile.realm.id),
+            random_name(18),
+            sanitize_name(uploaded_file_name)
+        ])
+        url = "/user_uploads/%s" % (s3_file_name)
+    else:
         bucket_name = settings.S3_BUCKET
         s3_file_name = "/".join([random_name(60), sanitize_name(uploaded_file_name)])
+        url = "https://%s.s3.amazonaws.com/%s" % (bucket_name, s3_file_name)
+
     upload_image_to_s3(
             bucket_name,
             s3_file_name,
@@ -76,11 +92,15 @@ def upload_message_image(uploaded_file_name, content_type, file_data, user_profi
             user_profile,
             file_data
     )
-    return "https://%s.s3.amazonaws.com/%s" % (bucket_name, s3_file_name)
+    return url
 
-def upload_message_image_through_web_client(request, user_file, user_profile):
+def upload_message_image_through_web_client(request, user_file, user_profile, private=None):
     uploaded_file_name, content_type = get_file_info(request, user_file)
-    return upload_message_image(uploaded_file_name, content_type, user_file.read(), user_profile)
+    return upload_message_image(uploaded_file_name, content_type, user_file.read(), user_profile, private)
+
+def get_signed_upload_url(path):
+    conn = S3Connection(settings.S3_KEY, settings.S3_SECRET_KEY)
+    return conn.generate_url(15, 'GET', bucket=settings.S3_AUTH_UPLOADS_BUCKET, key=path)
 
 def upload_avatar_image(user_file, user_profile, email):
     content_type = guess_type(user_file.name)[0]

zerver/tests.py

@@ -2253,7 +2253,8 @@ class MITNameTest(TestCase):
         self.assertTrue(not_mit_mailing_list("sipbexch@mit.edu"))
 
 class S3Test(AuthedTestCase):
-    test_uris = []
+    test_uris = [] # full URIs in public bucket
+    test_keys = [] # keys in authed bucket
 
     @slow(2.6, "has to contact external S3 service")
     def test_file_upload(self):
@@ -2264,7 +2265,7 @@ class S3Test(AuthedTestCase):
         fp = StringIO("zulip!")
         fp.name = "zulip.txt"
 
-        result = self.client.post("/json/upload_file", {'file': fp})
+        result = self.client.post("/json/upload_file", {'file': fp, 'private':'false'})
         self.assert_json_success(result)
         json = ujson.loads(result.content)
         self.assertIn("uri", json)
@@ -2272,6 +2273,29 @@ class S3Test(AuthedTestCase):
         self.test_uris.append(uri)
         self.assertEquals("zulip!", urllib2.urlopen(uri).read().strip())
 
+    @slow(2.6, "has to contact external S3 service")
+    def test_file_upload_authed(self):
+        """
+        A call to /json/upload_file should return a uri and actually create an object.
+        """
+        self.login("hamlet@zulip.com")
+        fp = StringIO("zulip!")
+        fp.name = "zulip.txt"
+
+        result = self.client.post("/json/upload_file", {'file': fp, 'private':'true'})
+        self.assert_json_success(result)
+        json = ujson.loads(result.content)
+        self.assertIn("uri", json)
+        uri = json["uri"]
+        base = '/user_uploads/'
+        self.assertEquals(base, uri[:len(base)])
+        self.test_keys.append(uri[len(base):])
+
+        response = self.client.get(uri)
+        redirect_url = response['Location']
+
+        self.assertEquals("zulip!", urllib2.urlopen(redirect_url).read().strip())
+
     def test_multiple_upload_failure(self):
         """
         Attempting to upload two files should fail.
@@ -2303,6 +2327,12 @@ class S3Test(AuthedTestCase):
             key.delete()
             self.test_uris.remove(uri)
 
+        for path in self.test_keys:
+            key = Key(conn.get_bucket(settings.S3_AUTH_UPLOADS_BUCKET))
+            key.name = path
+            key.delete()
+            self.test_keys.remove(path)
+
 
 class DummyStream:
     def closed(self):

zerver/views/__init__.py

@@ -4,7 +4,7 @@ from django.conf import settings
 from django.contrib.auth import authenticate, login
 from django.contrib.auth.decorators import login_required
 from django.core.urlresolvers import reverse
-from django.http import HttpResponseRedirect
+from django.http import HttpResponseRedirect, HttpResponseForbidden
 from django.shortcuts import render_to_response, redirect
 from django.template import RequestContext, loader
 from django.utils.timezone import now
@@ -61,7 +61,8 @@ from zerver.decorator import require_post, \
     zulip_internal
 from zerver.lib.query import last_n
 from zerver.lib.avatar import avatar_url
-from zerver.lib.upload import upload_message_image_through_web_client, upload_avatar_image
+from zerver.lib.upload import upload_message_image_through_web_client, upload_avatar_image, \
+    get_signed_upload_url
 from zerver.lib.response import json_success, json_error, json_response, json_method_not_allowed
 from zerver.lib.cache import cache_get_many, cache_set_many, \
     generic_bulk_cached_fetch
@@ -1699,16 +1700,26 @@ def json_get_subscribers(request, user_profile):
     return get_subscribers_backend(request, user_profile)
 
 @authenticated_json_post_view
-def json_upload_file(request, user_profile):
+@has_request_variables
+def json_upload_file(request, user_profile, private=REQ(converter=json_to_bool, default=None)):
     if len(request.FILES) == 0:
         return json_error("You must specify a file to upload")
     if len(request.FILES) != 1:
         return json_error("You may only upload one file at a time")
 
     user_file = request.FILES.values()[0]
-    uri = upload_message_image_through_web_client(request, user_file, user_profile)
+    uri = upload_message_image_through_web_client(request, user_file, user_profile, private=private)
     return json_success({'uri': uri})
 
+@authenticated_json_view
+def get_uploaded_file(request, user_profile, realm_id, filename):
+    # Internal users can access all uploads so we can receive attachments in cross-realm messages
+    if user_profile.realm.id == int(realm_id) or user_profile.realm.domain == 'zulip.com':
+        url = get_signed_upload_url("%s/%s" % (realm_id, filename))
+        return redirect(url)
+    else:
+        return HttpResponseForbidden()
+
 @has_request_variables
 def get_subscribers_backend(request, user_profile, stream_name=REQ('stream')):
     stream = get_stream(stream_name, user_profile.realm)

zproject/local_settings.py

@@ -69,6 +69,7 @@ if DEPLOYED and not LOCALSERVER:
     S3_KEY="xxxxxxxxxxxxxxxxxxxx"
     S3_SECRET_KEY="xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx"
     S3_BUCKET="humbug-user-uploads"
+    S3_AUTH_UPLOADS_BUCKET = "zulip-user-uploads"
     S3_AVATAR_BUCKET="humbug-user-avatars"
 
     MIXPANEL_TOKEN="xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx"
@@ -76,6 +77,7 @@ else:
     S3_KEY="xxxxxxxxxxxxxxxxxxxx"
     S3_SECRET_KEY="xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx"
     S3_BUCKET="humbug-user-uploads-test"
+    S3_AUTH_UPLOADS_BUCKET = "zulip-user-uploads-test"
     S3_AVATAR_BUCKET="humbug-user-avatars-test"
 
     MIXPANEL_TOKEN="xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx"

zproject/urls.py

@@ -41,6 +41,8 @@ urlpatterns = patterns('',
 
     url(r'^activity$', 'zerver.views.get_activity'),
 
+    url(r'^user_uploads/(?P<realm_id>\d*)/(?P<filename>.*)', 'zerver.views.get_uploaded_file'),
+
     # Registration views, require a confirmation ID.
     url(r'^accounts/home/', 'zerver.views.accounts_home'),
     url(r'^accounts/send_confirm/(?P<email>[\S]+)?',