Merge pull request #22 from ImperialCollegeLondon/fix/issue18

Fix for #18: 500 error when `BASE_DIR` is not set or file uploads set to a directory outside the app install base.

django_drf_filepond/drf_filepond_settings.py

@@ -50,3 +50,11 @@
 # django-drf-filepond's file management and the api.store_upload function
 # will not be usable - you will need to manage file storage in your code.
 FILE_STORE_PATH = getattr(settings, _app_prefix+'FILE_STORE_PATH', None)
+
+# If you want to use an external directory (a directory outside of your 
+# project directory) to store temporary uploads, this setting needs to be 
+# set to true. By default it is False to prevent uploads being stored 
+# elsewhere.
+ALLOW_EXTERNAL_UPLOAD_DIR = getattr(settings, 
+                                    _app_prefix+'ALLOW_EXTERNAL_UPLOAD_DIR',
+                                    False)

django_drf_filepond/views.py

@@ -28,6 +28,7 @@
 from django_drf_filepond.api import get_stored_upload,\
     get_stored_upload_file_data
 from django_drf_filepond.exceptions import ConfigurationError
+import django_drf_filepond
 
 LOG = logging.getLogger(__name__)
 
@@ -79,17 +80,33 @@ class ProcessView(APIView):
     def post(self, request):
         LOG.debug('Filepond API: Process view POST called...')
 
-        # Enforce that the upload location must be a sub-directory of
-        # the project base directory
+        # Check that the temporary upload directory has been set
+        if not hasattr(local_settings, 'UPLOAD_TMP'):
+            return Response('The file upload path settings are not '
+                            'configured correctly.',
+                            status=status.HTTP_500_INTERNAL_SERVER_ERROR)
+
+        # By default, enforce that the temporary upload location must be a
+        # sub-directory of the project base directory.
         # TODO: Check whether this is necessary - maybe add a security
         # parameter that can be disabled to turn off this check if the
         # developer wishes?
-        if ((not hasattr(local_settings, 'UPLOAD_TMP')) or
-                (not (storage.location).startswith(local_settings.BASE_DIR))):
-            return Response('The file upload path settings are not '
+        if ((not (storage.location).startswith(local_settings.BASE_DIR)) and
+                (local_settings.BASE_DIR != 
+                 os.path.dirname(django_drf_filepond.__file__))):
+            if not local_settings.ALLOW_EXTERNAL_UPLOAD_DIR:
+                return Response('The file upload path settings are not '
                             'configured correctly.',
                             status=status.HTTP_500_INTERNAL_SERVER_ERROR)
 
+        # Check that a relative path is not being used to store the
+        # upload outside the specified UPLOAD_TMP directory.
+        if not getattr(local_settings, 'UPLOAD_TMP').startswith(
+                os.path.abspath(storage.location)):
+            return Response('An invalid storage location has been '
+                    'specified.',
+                    status=status.HTTP_500_INTERNAL_SERVER_ERROR)
+
         # Check that we've received a file and then generate a unique ID
         # for it. Also generate a unique UD for the temp upload dir
         file_id = _get_file_id()
@@ -127,6 +144,9 @@ def post(self, request):
         #             '<%s>...' % storage.location)
         #    os.makedirs(storage.location, mode=0o700)
 
+        LOG.debug('About to store uploaded temp file with filename: %s' 
+                  % (upload_filename))
+
         # We now need to create the temporary upload object and store the
         # file and metadata.
         tu = TemporaryUpload(upload_id=upload_id, file_id=file_id,

docs/installation.rst

@@ -56,6 +56,15 @@ variable to your settings file, e.g.::
 	``filepond_uploads`` directory in the ``django-drf-filepond`` app  
 	directory, wherever that is located. Note that this may be within the  
 	``lib`` directory of a virtualenv.
+
+.. note:: If you wish to set your file upload location to an directory
+	outside of your Django application, i.e. something that is not below
+	BASE_DIR, you should consider any security implications that may result
+	from letting the app write to another location on disk and set the
+	``DJANGO_DRF_FILEPOND_ALLOW_EXTERNAL_UPLOAD_DIR`` to ``True`` to confirm
+	that you want to use a directory external to your application to store
+	temporary uploads.
+
 
 3. Include the app urls into your main url configuration
 ========================================================

setup.py

@@ -22,7 +22,7 @@
     include_package_data=True,
     install_requires=[
         "Django<2.0.0;python_version=='2.7'",
-        "Django>=2.0.0;python_version>='3.5'",
+        "Django<3.0.0;python_version>='3.5'",
         "djangorestframework==3.9.3;python_version=='2.7'",
         "djangorestframework>=3.9.3;python_version>='3.5'",
         "shortuuid>=0.5.0",

tests/test_process_view.py

@@ -15,7 +15,17 @@
 from tests.utils import remove_file_upload_dir_if_required
 
 LOG = logging.getLogger(__name__)
-
+#
+# New tests for checking file storage outside of BASE_DIR (see #18) 
+#
+# test_store_upload_with_storage_outside_BASE_DIR_without_enable: Set a 
+#    FileSystemStorage store location that is outside of BASE_DIR and check 
+#    that the upload fails.
+# 
+# test_store_upload_with_storage_outside_BASE_DIR_with_enable: Set a
+#    FileSystemStorage store location outside of BASE_DIR and also set the
+#    ALLOW_EXTERNAL_UPLOAD_DIR setting to True and check the upload succeeds
+#
 
 class ProcessTestCase(TestCase):
 
@@ -107,6 +117,47 @@ def test_process_data_invalid_different_field_name(self):
         self.assertIn(response.data['detail'], ('Invalid request data has '
                                                 'been provided.'))
 
+    def test_store_upload_with_storage_outside_BASE_DIR_without_enable(self):
+        old_storage = views.storage
+        views.storage = FileSystemStorage(location='/tmp/uploads')
+        (encoded_form, content_type) = self._get_encoded_form('testfile.dat')
+
+        rf = RequestFactory()
+        req = rf.post(reverse('process'),
+                      data=encoded_form, content_type=content_type)
+        pv = views.ProcessView.as_view()
+        response = pv(req)
+        views.storage = old_storage
+        self.assertEqual(response.status_code, 500, 'Expecting 500 error due'
+                         ' to invalid storage location.')
+        self.assertEqual(
+            response.data,
+            'The file upload path settings are not configured correctly.',
+            ('Expecting error showing path settings are configured '
+             'incorrectly.'))
+
+    def test_store_upload_with_storage_outside_BASE_DIR_with_enable(self):
+        old_storage = views.storage
+        old_UPLOAD_TMP = drf_filepond_settings.UPLOAD_TMP
+
+        drf_filepond_settings.ALLOW_EXTERNAL_UPLOAD_DIR = True
+
+        views.storage = FileSystemStorage(location='/tmp/uploads')
+        drf_filepond_settings.UPLOAD_TMP = '/tmp/uploads'
+
+        (encoded_form, content_type) = self._get_encoded_form('testfile.dat')
+
+        rf = RequestFactory()
+        req = rf.post(reverse('process'),
+                      data=encoded_form, content_type=content_type)
+        pv = views.ProcessView.as_view()
+        response = pv(req)
+        views.storage = old_storage
+        drf_filepond_settings.UPLOAD_TMP = old_UPLOAD_TMP
+        drf_filepond_settings.ALLOW_EXTERNAL_UPLOAD_DIR = False
+        self.assertEqual(response.status_code, 200, 'Expecting upload to be '
+                         'successful.')
+
     def _process_data(self, upload_field_name=None):
         tmp_upload_dir = drf_filepond_settings.UPLOAD_TMP
         self.uploaddir_exists_pre_test = os.path.exists(tmp_upload_dir)