Some security fixes and other fixes for file storage.

Also added unittests for creating files.

pillar/application/modules/file_storage.py

@@ -8,12 +8,14 @@ import eve.utils
 from bson import ObjectId
 from eve.methods.patch import patch_internal
 from eve.methods.put import put_internal
-from flask import Blueprint
+from flask import Blueprint, safe_join
 from flask import jsonify
 from flask import request
+from flask import abort
 from flask import send_from_directory
 from flask import url_for, helpers
 
+from application import utils
 from application.utils import remove_private_keys
 from application.utils.cdn import hash_file_path
 from application.utils.encoding import Encoder
@@ -75,7 +77,7 @@ def build_thumbnails(file_path=None, file_id=None):
         file_ = files_collection.find_one({"_id": ObjectId(file_id)})
         file_path = file_['name']
 
-    file_full_path = os.path.join(app.config['SHARED_DIR'], file_path[:2],
+    file_full_path = safe_join(safe_join(app.config['SHARED_DIR'], file_path[:2]),
                                file_path)
     # Does the original file exist?
     if not os.path.isfile(file_full_path):
@@ -147,26 +149,29 @@ def index(file_name=None):
     return jsonify({'url': url_for('file_storage.index', file_name=file_name)})
 
 
-def process_file(src_file):
+def process_file(file_id, src_file):
-    """Process the file
+    """Process the file.
+
+    :param file_id: '_id' key of the file
+    :param src_file: POSTed data of the file, lacks private properties.
     """
     from application import app
 
-    file_id = src_file['_id']
+    src_file = utils.remove_private_keys(src_file)
-    # Remove properties that do not belong in the collection
-    internal_fields = ['_id', '_etag', '_updated', '_created', '_status']
-    for field in internal_fields:
-        src_file.pop(field, None)
 
-    files_collection = app.data.driver.db['files']
+    filename = src_file['name']
-    file_abs_path = os.path.join(
+    file_abs_path = safe_join(safe_join(app.config['SHARED_DIR'], filename[:2]), filename)
-        app.config['SHARED_DIR'], src_file['name'][:2], src_file['name'])
+
+    if not os.path.exists(file_abs_path):
+        log.warning("POSTed file document %r refers to non-existant file on file system %s!",
+                    file_id, file_abs_path)
+        abort(422, "POSTed file document refers to non-existant file on file system!")
 
     src_file['length'] = os.stat(file_abs_path).st_size
     content_type = src_file['content_type'].split('/')
     src_file['format'] = content_type[1]
     mime_type = content_type[0]
-    src_file['file_path'] = src_file['name']
+    src_file['file_path'] = filename
 
     if mime_type == 'image':
         from PIL import Image
@@ -197,7 +202,7 @@ def process_file(src_file):
         src_file['variations'] = []
         # Create variations
         for v in variations:
-            root, ext = os.path.splitext(src_file['name'])
+            root, ext = os.path.splitext(filename)
             filename = "{0}-{1}p.{2}".format(root, res_y, v)
             video_duration = None
             if src_video_data['duration']:
@@ -250,6 +255,9 @@ def process_file(src_file):
 
         p = Process(target=encode, args=(file_abs_path, src_file, res_y))
         p.start()
+    else:
+        log.info("POSTed file was of type %r, which isn't thumbnailed/encoded.", mime_type)
+
     if mime_type != 'video':
         # Sync the whole subdir
         sync_path = os.path.split(file_abs_path)[0]
@@ -259,7 +267,7 @@ def process_file(src_file):
         p.start()
 
     # Update the original file with additional info, e.g. image resolution
-    r = put_internal('files', src_file, **{'_id': ObjectId(file_id)})
+    put_internal('files', src_file, _id=ObjectId(file_id))
 
 
 def delete_file(file_item):
@@ -383,7 +391,14 @@ def post_POST_files(request, payload):
     """After an file object has been created, we do the necessary processing
     and further update it.
     """
-    process_file(request.get_json())
+
+    if 200 <= payload.status_code < 300:
+        import json
+        posted_properties = json.loads(request.data)
+        private_properties = json.loads(payload.data)
+        file_id = private_properties['_id']
+
+        process_file(file_id, posted_properties)
 
 
 def before_deleting_file(item):

pillar/application/utils/__init__.py

@@ -1,4 +1,9 @@
 import copy
+import json
+import datetime
+
+import bson
+from eve import RFC1123_DATE_FORMAT
 
 
 def remove_private_keys(document):
@@ -11,3 +16,19 @@ def remove_private_keys(document):
             del patch_info[key]
 
     return patch_info
+
+
+class PillarJSONEncoder(json.JSONEncoder):
+    """JSON encoder with support for Pillar resources."""
+
+    def default(self, obj):
+        if isinstance(obj, datetime.datetime):
+            if obj.tzinfo is None:
+                raise ValueError('All datetime.datetime objects should be timezone-aware.')
+            return obj.strftime(RFC1123_DATE_FORMAT)
+
+        if isinstance(obj, bson.ObjectId):
+            return str(obj)
+
+        # Let the base class default method raise the TypeError
+        return json.JSONEncoder.default(self, obj)

pillar/application/utils/storage.py

@@ -70,7 +70,7 @@ def push_to_storage(project_id, full_path, backend='cgs'):
             # XXX Make public on the fly if it's an image and small preview.
             # This should happen by reading the database (push to storage
             # should change to accomodate it).
-            if full_path.endswith('-t.jpg'):
+            if blob is not None and full_path.endswith('-t.jpg'):
                 blob.make_public()
             os.remove(full_path)
 

tests/common_test_class.py

@@ -34,6 +34,7 @@ class AbstractPillarTest(TestMinimal):
         app.config['BLENDER_ID_ENDPOINT'] = BLENDER_ID_ENDPOINT
         logging.getLogger('application').setLevel(logging.DEBUG)
         logging.getLogger('werkzeug').setLevel(logging.DEBUG)
+        logging.getLogger('eve').setLevel(logging.DEBUG)
 
         self.app = app
         self.client = app.test_client()
@@ -59,7 +60,20 @@ class AbstractPillarTest(TestMinimal):
             projects_collection.insert_one(EXAMPLE_PROJECT)
             result = files_collection.insert_one(file)
             file_id = result.inserted_id
-        return file_id, EXAMPLE_FILE
+        return file_id, file
+
+    def ensure_project_exists(self, project_overrides=None):
+        with self.app.test_request_context():
+            projects_collection = self.app.data.driver.db['projects']
+            assert isinstance(projects_collection, pymongo.collection.Collection)
+
+            project = copy.deepcopy(EXAMPLE_PROJECT)
+            if project_overrides is not None:
+                project.update(project_overrides)
+
+            result = projects_collection.insert_one(project)
+            project_id = result.inserted_id
+        return project_id, project
 
     def htp_blenderid_validate_unhappy(self):
         """Sets up HTTPretty to mock unhappy validation flow."""

tests/test_file_storage.py

@@ -0,0 +1,58 @@
+"""Test cases for file handling."""
+
+from __future__ import print_function
+
+import os
+import shutil
+import copy
+import json
+
+from common_test_class import AbstractPillarTest, MY_PATH
+from common_test_data import EXAMPLE_FILE
+
+
+class FileUploadingTest(AbstractPillarTest):
+
+    def test_create_file_missing_on_fs(self):
+        from application import utils
+        from application.utils import PillarJSONEncoder
+
+        to_post = utils.remove_private_keys(EXAMPLE_FILE)
+        json_file = json.dumps(to_post, cls=PillarJSONEncoder)
+
+        with self.app.test_request_context():
+            self.ensure_project_exists()
+
+            resp = self.client.post('/files',
+                                    data=json_file,
+                                    headers={'Content-Type': 'application/json'})
+
+        self.assertEqual(422, resp.status_code)
+
+
+    def test_create_file_exists_on_fs(self):
+        from application import utils
+        from application.utils import PillarJSONEncoder
+
+        filename = 'BlenderDesktopLogo.png'
+        full_file = copy.deepcopy(EXAMPLE_FILE)
+        full_file[u'name'] = filename
+        to_post = utils.remove_private_keys(full_file)
+        json_file = json.dumps(to_post, cls=PillarJSONEncoder)
+
+        with self.app.test_request_context():
+            self.ensure_project_exists()
+
+            target_dir = os.path.join(self.app.config['SHARED_DIR'], filename[:2])
+            if os.path.exists(target_dir):
+                assert os.path.isdir(target_dir)
+            else:
+                os.makedirs(target_dir)
+            shutil.copy(os.path.join(MY_PATH, filename), target_dir)
+
+            resp = self.client.post('/files',
+                                    data=json_file,
+                                    headers={'Content-Type': 'application/json'})
+
+        self.assertEqual(201, resp.status_code)
+