From 8e9edeebd8dbaee2272c1d6a320dff0be263d339 Mon Sep 17 00:00:00 2001
From: Jorran de Wit <jorrandewit@outlook.com>
Date: Sat, 30 Sep 2017 15:27:38 +0200
Subject: [PATCH] Important file handling improvements

Files can now be auto-saved into a secure folder on the server.
This will prevent people from opening files that do not copmly with
their permissions.

Further, it's now possible to open all types of files in the partner dashboard.
---
 SciPost_v1/settings/base.py |  6 +++++-
 partners/models.py          |  4 +++-
 partners/storage.py         | 25 +++++++++++++++++++++++++
 partners/views.py           | 10 ++++++++--
 scipost/urls.py             |  3 ++-
 scipost/views.py            | 15 +++++++++++++++
 6 files changed, 58 insertions(+), 5 deletions(-)
 create mode 100644 partners/storage.py

diff --git a/SciPost_v1/settings/base.py b/SciPost_v1/settings/base.py
index 760615bd5..f79b99fad 100644
--- a/SciPost_v1/settings/base.py
+++ b/SciPost_v1/settings/base.py
@@ -227,9 +227,13 @@ USE_TZ = True
 
 # MEDIA
 MEDIA_URL = '/media/'
-MEDIA_ROOT = 'local_files/media/'
+MEDIA_URL_SECURE = '/files/secure/'
 MAX_UPLOAD_SIZE = "1310720"  # Default max attachment size in Bytes
 
+# -- These MEDIA settings are machine-dependent
+MEDIA_ROOT = 'local_files/media/'
+MEDIA_ROOT_SECURE = 'local_files/secure/media/'
+
 # Static files (CSS, JavaScript, Images)
 STATIC_URL = '/static/'
 STATIC_ROOT = 'local_files/static/'
diff --git a/partners/models.py b/partners/models.py
index 1d7f39a88..ff95d0799 100644
--- a/partners/models.py
+++ b/partners/models.py
@@ -26,6 +26,7 @@ from .constants import PROSPECTIVE_PARTNER_EVENT_EMAIL_SENT,\
 
 from .managers import MembershipAgreementManager, ProspectivePartnerManager, PartnerManager,\
                       ContactRequestManager, PartnersAttachmentManager
+from .storage import SecureFileStorage
 
 from scipost.constants import TITLE_CHOICES
 from scipost.fields import ChoiceArrayField
@@ -291,7 +292,8 @@ class PartnersAttachment(models.Model):
     An Attachment which can (in the future) be related to a Partner, Contact, MembershipAgreement,
     etc.
     """
-    attachment = models.FileField(upload_to='UPLOADS/PARTNERS/ATTACHMENTS')
+    attachment = models.FileField(upload_to='UPLOADS/PARTNERS/ATTACHMENTS',
+                                  storage=SecureFileStorage())
     name = models.CharField(max_length=128)
     agreement = models.ForeignKey('partners.MembershipAgreement', related_name='attachments',
                                   blank=True)
diff --git a/partners/storage.py b/partners/storage.py
new file mode 100644
index 000000000..1821e4f48
--- /dev/null
+++ b/partners/storage.py
@@ -0,0 +1,25 @@
+from django.conf import settings
+from django.core.files.storage import FileSystemStorage
+from django.utils.functional import cached_property
+
+
+class SecureFileStorage(FileSystemStorage):
+    """
+    Inherit default FileStorage system to prevent files from being publicly accessible
+    from an server location that is permitted to be opened without explicit permissions.
+    """
+    @cached_property
+    def location(self):
+        """
+        This method determines the storage location for a new file. To secure the file from
+        'the public', we'll store it outside the publicly accessible MEDIA_ROOT folder.
+
+        This also means you need to explicitly handle the file reading/opening!
+        """
+        if hasattr(settings, 'MEDIA_ROOT_SECURE'):
+            return self._value_or_setting(self._location, settings.MEDIA_ROOT_SECURE)
+        return super().location
+
+    @cached_property
+    def base_url(self):
+        return settings.MEDIA_URL_SECURE
diff --git a/partners/views.py b/partners/views.py
index 04a4c5f1e..a4367d7fd 100644
--- a/partners/views.py
+++ b/partners/views.py
@@ -1,8 +1,10 @@
+import mimetypes
+
 from django.contrib import messages
 from django.contrib.auth.decorators import login_required
 from django.db import transaction
 from django.forms import modelformset_factory
-from django.http import HttpResponse
+from django.http import FileResponse, HttpResponse
 from django.shortcuts import get_object_or_404, render, reverse, redirect
 from django.utils import timezone
 
@@ -386,7 +388,11 @@ def agreement_details(request, agreement_id):
 def agreement_attachments(request, agreement_id, attachment_id):
     attachment = get_object_or_404(PartnersAttachment.objects.my_attachments(request.user),
                                    agreement__id=agreement_id, id=attachment_id)
-    response = HttpResponse(attachment.attachment.read(), content_type='application/pdf')
+
+    content_type, encoding = mimetypes.guess_type(attachment.attachment.path)
+    content_type = content_type or 'application/octet-stream'
+    response = HttpResponse(attachment.attachment.read(), content_type=content_type)
+    response["Content-Encoding"] = encoding
     response['Content-Disposition'] = ('filename=%s' % attachment.name)
     return response
 
diff --git a/scipost/urls.py b/scipost/urls.py
index f3af0c4a6..fe4aaefa2 100644
--- a/scipost/urls.py
+++ b/scipost/urls.py
@@ -1,4 +1,4 @@
-from django.conf.urls import include, url
+from django.conf.urls import url
 from django.views.generic import TemplateView
 
 from . import views
@@ -14,6 +14,7 @@ JOURNAL_REGEX = '(?P<doi_label>%s)' % REGEX_CHOICES
 
 urlpatterns = [
     url(r'^$', views.index, name='index'),
+    url(r'^files/secure/(?P<path>.*)$', views.protected_serve, name='secure_file'),
 
     # General use pages
     url(r'^error$', TemplateView.as_view(template_name='scipost/error.html'), name='error'),
diff --git a/scipost/views.py b/scipost/views.py
index 5c09fe3e6..ce592b942 100644
--- a/scipost/views.py
+++ b/scipost/views.py
@@ -1,5 +1,6 @@
 from django.utils import timezone
 from django.shortcuts import get_object_or_404, render
+from django.conf import settings
 from django.contrib import messages
 from django.contrib.auth import login, logout, update_session_auth_hash
 from django.contrib.auth.decorators import login_required, user_passes_test
@@ -10,10 +11,12 @@ from django.core.mail import EmailMessage, EmailMultiAlternatives
 from django.core.paginator import Paginator
 from django.core.urlresolvers import reverse
 from django.db.models import Prefetch
+from django.http import Http404
 from django.shortcuts import redirect
 from django.template import Context, Template
 from django.views.decorators.http import require_POST
 from django.views.generic.list import ListView
+from django.views.static import serve
 
 from guardian.decorators import permission_required
 from guardian.shortcuts import assign_perm, get_objects_for_user
@@ -85,6 +88,18 @@ def index(request):
     return render(request, 'scipost/index.html', context)
 
 
+def protected_serve(request, path, show_indexes=False):
+    """
+    Serve files that are saved outside the default MEDIA_ROOT folder for superusers only!
+    This will be usefull eg. in the admin pages.
+    """
+    if not request.user.is_authenticated or not request.user.is_superuser:
+        # Only superusers may get to see secure files without an explicit serve method!
+        raise Http404
+    document_root = settings.MEDIA_ROOT_SECURE
+    return serve(request, path, document_root, show_indexes)
+
+
 ###############
 # Information
 ###############
-- 
GitLab