upload: Include filename at the end of temporary access URLs.

zerver/lib/upload.py

@@ -670,7 +670,9 @@ LOCAL_FILE_ACCESS_TOKEN_SALT = "local_file_"
 def generate_unauthed_file_access_url(path_id: str) -> str:
     signed_data = TimestampSigner(salt=LOCAL_FILE_ACCESS_TOKEN_SALT).sign(path_id)
     token = base64.b16encode(signed_data.encode('utf-8')).decode('utf-8')
-    return reverse('zerver.views.upload.serve_local_file_unauthed', args=[token, ])
+
+    filename = path_id.split('/')[-1]
+    return reverse('zerver.views.upload.serve_local_file_unauthed', args=[token, filename])
 
 def get_local_file_path_id_from_token(token: str) -> Optional[str]:
     signer = TimestampSigner(salt=LOCAL_FILE_ACCESS_TOKEN_SALT)

zerver/tests/test_upload.py

@@ -212,6 +212,7 @@ class FileUploadTest(UploadSerializeMixin, ZulipTestCase):
         # Ensure this is different from the original uri:
         self.assertNotEqual(url_only_url, uri)
         self.assertIn('user_uploads/temporary/', url_only_url)
+        self.assertTrue(url_only_url.endswith('zulip.txt'))
         # The generated url has a token authorizing the requestor to access the file
         # without being logged in.
         self.logout()
@@ -221,9 +222,26 @@ class FileUploadTest(UploadSerializeMixin, ZulipTestCase):
         self.assertEqual(result.status_code, 401)
 
     def test_serve_local_file_unauthed_invalid_token(self) -> None:
-        result = self.client_get('/user_uploads/temporary/badtoken')
+        result = self.client_get('/user_uploads/temporary/badtoken/file.png')
         self.assert_json_error(result, "Invalid token")
 
+    def test_serve_local_file_unauthed_altered_filename(self) -> None:
+        self.login('hamlet')
+        fp = StringIO("zulip!")
+        fp.name = "zulip.txt"
+        result = self.client_post("/json/user_uploads", {'file': fp})
+        url = '/json' + result.json()["uri"]
+
+        result = self.client_get(url)
+        self.assert_json_success(result)
+        data = result.json()
+        url_only_url = data['url']
+
+        self.assertTrue(url_only_url.endswith('zulip.txt'))
+        url_only_url_changed_filename = url_only_url.split('zulip.txt')[0] + 'differentname.exe'
+        result = self.client_get(url_only_url_changed_filename)
+        self.assert_json_error(result, "Invalid filename")
+
     def test_serve_local_file_unauthed_token_expires(self) -> None:
         self.login('hamlet')
         fp = StringIO("zulip!")

zerver/views/upload.py

@@ -82,10 +82,12 @@ def serve_file(request: HttpRequest, user_profile: UserProfile,
 
     return serve_s3(request, path_id, url_only)
 
-def serve_local_file_unauthed(request: HttpRequest, token: str) -> HttpResponse:
+def serve_local_file_unauthed(request: HttpRequest, token: str, filename: str) -> HttpResponse:
     path_id = get_local_file_path_id_from_token(token)
     if path_id is None:
         return json_error(_("Invalid token"))
+    if path_id.split('/')[-1] != filename:
+        return json_error(_("Invalid filename"))
 
     return serve_local(request, path_id, url_only=False)
 

zproject/urls.py

@@ -593,7 +593,7 @@ urls += [
 # having to rewrite URLs, and is implemented using the
 # 'override_api_url_scheme' flag passed to rest_dispatch
 urls += [
-    url(r'^user_uploads/temporary/([0-9A-Za-z]+)$',
+    url(r'^user_uploads/temporary/([0-9A-Za-z]+)/([^/]+)$',
         zerver.views.upload.serve_local_file_unauthed,
         name='zerver.views.upload.serve_local_file_unauthed'),
     url(r'^user_uploads/(?P<realm_id_str>(\d*|unk))/(?P<filename>.*)$',