diff --git a/docs/productionsetup.rst b/docs/productionsetup.rst index ec5812899..40d979212 100644 --- a/docs/productionsetup.rst +++ b/docs/productionsetup.rst @@ -39,8 +39,7 @@ Acces to Documents Nginx is able to serve your uploads behind authentication/authorization. Activate the following settings:: # Use nginx to serve uploads authenticated - USE_X_ACCEL_REDIRECT = True - X_ACCEL_REDIRECT_PREFIX = '/protected' + INTERNAL_MEDIA_PREFIX = '/protected/' Nginx will forward the request to Froide which will in turn check for authentication and authorization. If everything is good Froide replies to Nginx with an internal redirect and Nginx will then serve the file to the user. diff --git a/froide/foirequest/api_views.py b/froide/foirequest/api_views.py index f5dab29c4..017ee21a4 100644 --- a/froide/foirequest/api_views.py +++ b/froide/foirequest/api_views.py @@ -130,7 +130,7 @@ class Meta: ) def get_file_url(self, obj): - return obj.get_absolute_domain_file_url(authenticated=True) + return obj.get_absolute_domain_file_url(authorized=True) def get_pending(self, obj): return obj.pending diff --git a/froide/foirequest/auth.py b/froide/foirequest/auth.py index 51623d81b..717dc309e 100644 --- a/froide/foirequest/auth.py +++ b/froide/foirequest/auth.py @@ -1,6 +1,10 @@ from functools import lru_cache from django.utils.crypto import salted_hmac, constant_time_compare +from django.urls import reverse +from django.conf import settings + +from crossdomainmedia import CrossDomainMediaAuth from froide.helper.auth import ( can_read_object, can_write_object, @@ -93,3 +97,68 @@ def clear_lru_caches(): for f in (can_write_foirequest, can_read_foirequest, can_read_foirequest_authenticated): f.cache_clear() + + +def has_attachment_access(request, foirequest, attachment): + if not can_read_foirequest(foirequest, request): + return False + if not attachment.approved: + # allow only approved attachments to be read + # do not allow anonymous authentication here + allowed = can_read_foirequest_authenticated( + foirequest, request, allow_code=False + ) + if not allowed: + return False + return True + + +def get_accessible_attachment_url(foirequest, attachment): + needs_authorization = not is_attachment_public(foirequest, attachment) + return attachment.get_absolute_domain_file_url( + authorized=needs_authorization + ) + + +class AttachmentCrossDomainMediaAuth(CrossDomainMediaAuth): + ''' + Create your own custom CrossDomainMediaAuth class + and implement at least these methods + ''' + TOKEN_MAX_AGE_SECONDS = settings.FOI_MEDIA_TOKEN_EXPIRY + SITE_URL = settings.SITE_URL + DEBUG = False + + def is_media_public(self): + ''' + Determine if the media described by self.context + needs authentication/authorization at all + ''' + ctx = self.context + return is_attachment_public(ctx['foirequest'], ctx['object']) + + def has_perm(self, request): + ctx = self.context + obj = ctx['object'] + foirequest = ctx['foirequest'] + return has_attachment_access(request, foirequest, obj) + + def get_auth_url(self): + ''' + Give URL path to authenticating view + for the media described in context + ''' + obj = self.context['object'] + + return reverse('foirequest-auth_message_attachment', + kwargs={ + 'message_id': obj.belongs_to_id, + 'attachment_name': obj.name + }) + + def get_media_file_path(self): + ''' + Return the URL path relative to MEDIA_ROOT for debug mode + ''' + obj = self.context['object'] + return obj.file.name diff --git a/froide/foirequest/models/attachment.py b/froide/foirequest/models/attachment.py index 0ed28c605..e049537a3 100644 --- a/froide/foirequest/models/attachment.py +++ b/froide/foirequest/models/attachment.py @@ -5,9 +5,6 @@ from django.dispatch import Signal from django.urls import reverse from django.utils.translation import ugettext_lazy as _ -from django.core.signing import ( - TimestampSigner, SignatureExpired, BadSignature -) from django.utils import timezone from froide.helper.redaction import can_redact_file @@ -31,6 +28,14 @@ def upload_to(instance, filename): return "%s/%s" % (settings.FOI_MEDIA_PATH, instance.name) +class FoiAttachmentManager(models.Manager): + def get_for_message(self, message, name): + return FoiAttachment.objects.filter( + belongs_to=message, + name=name + ).exclude(file='').get() + + class FoiAttachment(models.Model): belongs_to = models.ForeignKey( FoiMessage, null=True, @@ -65,6 +70,8 @@ class FoiAttachment(models.Model): on_delete=models.SET_NULL ) + objects = FoiAttachmentManager() + attachment_published = Signal(providing_args=[]) class Meta: @@ -83,9 +90,6 @@ def index_content(self): def get_html_id(self): return _("attachment-%(id)d") % {"id": self.id} - def get_internal_url(self): - return settings.FOI_MEDIA_URL + self.file.name - def get_bytes(self): self.file.open(mode='rb') try: @@ -169,53 +173,29 @@ def get_absolute_url(self): } ) - def get_absolute_domain_url(self): - return '%s%s' % (settings.SITE_URL, self.get_absolute_url()) + def get_crossdomain_auth(self): + from ..auth import AttachmentCrossDomainMediaAuth - def get_absolute_file_url(self, authenticated=False): - if not self.name: - return '' - url = reverse('foirequest-auth_message_attachment', - kwargs={ - 'message_id': self.belongs_to_id, - 'attachment_name': self.name - }) - if settings.FOI_MEDIA_TOKENS and authenticated: - signer = TimestampSigner() - value = signer.sign(url).split(':', 1)[1] - return '%s?token=%s' % (url, value) - return url + return AttachmentCrossDomainMediaAuth({ + 'object': self, + }) - def get_file_url(self): - return self.get_absolute_domain_file_url() + def send_internal_file(self): + return self.get_crossdomain_auth().send_internal_file() - def get_file_path(self): - if self.file: - return self.file.path - return '' + def get_absolute_domain_url(self): + return '%s%s' % (settings.SITE_URL, self.get_absolute_url()) - def get_authenticated_absolute_domain_file_url(self): - return self.get_absolute_domain_file_url(authenticated=True) + def get_absolute_file_url(self): + return self.get_crossdomain_auth().get_auth_url() - def get_absolute_domain_file_url(self, authenticated=False): - return '%s%s' % ( - settings.FOI_MEDIA_DOMAIN, - self.get_absolute_file_url(authenticated=authenticated) - ) + def get_authorized_absolute_domain_file_url(self): + return self.get_absolute_domain_file_url(authorized=True) - def check_token(self, request): - token = request.GET.get('token') - if token is None: - return None - original = '%s:%s' % (self.get_absolute_file_url(), token) - signer = TimestampSigner() - try: - signer.unsign(original, max_age=settings.FOI_MEDIA_TOKEN_EXPIRY) - except SignatureExpired: - return None - except BadSignature: - return False - return True + def get_absolute_domain_file_url(self, authorized=False): + return self.get_crossdomain_auth().get_full_media_url( + authorized=authorized + ) def approve_and_save(self): self.approved = True diff --git a/froide/foirequest/templates/foirequest/snippets/attachment_approved.html b/froide/foirequest/templates/foirequest/snippets/attachment_approved.html index e48242377..1a84e8ee0 100644 --- a/froide/foirequest/templates/foirequest/snippets/attachment_approved.html +++ b/froide/foirequest/templates/foirequest/snippets/attachment_approved.html @@ -24,7 +24,7 @@ {% endif %} {% if attachment.is_mail_decoration %} - + {% endif %} diff --git a/froide/foirequest/templates/foirequest/snippets/attachment_unapproved.html b/froide/foirequest/templates/foirequest/snippets/attachment_unapproved.html index eee1cb85b..1304421f0 100644 --- a/froide/foirequest/templates/foirequest/snippets/attachment_unapproved.html +++ b/froide/foirequest/templates/foirequest/snippets/attachment_unapproved.html @@ -32,7 +32,7 @@ {% blocktrans %}Not public!{% endblocktrans %} {% if attachment.is_mail_decoration and object|can_write_foirequest:request %} - + {% endif %} diff --git a/froide/foirequest/tests/test_web.py b/froide/foirequest/tests/test_web.py index 4b939e9ed..caa2b220d 100644 --- a/froide/foirequest/tests/test_web.py +++ b/froide/foirequest/tests/test_web.py @@ -1,5 +1,7 @@ import unittest +from mock import patch + from django.test import TestCase from django.urls import reverse from django.conf import settings @@ -299,34 +301,51 @@ def test_search(self): self.assertIn(reverse('foirequest-list'), response['Location']) +MEDIA_DOMAIN = 'media.frag-den-staat.de' + + class MediaServingTest(TestCaseHelpers, TestCase): def setUp(self): clear_lru_caches() self.site = factories.make_world() @override_settings( - USE_X_ACCEL_REDIRECT=True + SITE_URL='https://fragdenstaat.de', + MEDIA_URL='https://' + MEDIA_DOMAIN + '/files/', + ALLOWED_HOSTS=('fragdenstaat.de', MEDIA_DOMAIN), ) + @patch('froide.foirequest.auth.AttachmentCrossDomainMediaAuth.SITE_URL', + 'https://fragdenstaat.de') def test_request_not_public(self): att = FoiAttachment.objects.filter(approved=True)[0] req = att.belongs_to.request req.visibility = 1 req.save() - response = self.client.get(att.get_absolute_file_url()) + response = self.client.get( + att.get_absolute_file_url(), + HTTP_HOST='fragdenstaat.de' + ) self.assertForbidden(response) self.client.login(email='info@fragdenstaat.de', password='froide') - response = self.client.get(att.get_absolute_file_url()) + response = self.client.get( + att.get_absolute_file_url(), + HTTP_HOST='fragdenstaat.de' + ) + self.assertEqual(response.status_code, 302) + self.assertIn(MEDIA_DOMAIN, response['Location']) + response = self.client.get( + response['Location'], + HTTP_HOST=MEDIA_DOMAIN + ) self.assertEqual(response.status_code, 200) self.assertIn('X-Accel-Redirect', response) self.assertEqual(response['X-Accel-Redirect'], '%s%s' % ( - settings.X_ACCEL_REDIRECT_PREFIX, att.file.url)) + settings.INTERNAL_MEDIA_PREFIX, att.file.name)) @override_settings( - USE_X_ACCEL_REDIRECT=True, - FOI_MEDIA_TOKENS=True, SITE_URL='https://fragdenstaat.de', - FOI_MEDIA_DOMAIN='https://media.frag-den-staat.de', - ALLOWED_HOSTS=('fragdenstaat.de', 'media.frag-den-staat.de') + MEDIA_URL='https://' + MEDIA_DOMAIN + '/files/', + ALLOWED_HOSTS=('fragdenstaat.de', MEDIA_DOMAIN) ) def test_request_media_tokens(self): att = FoiAttachment.objects.filter(approved=True)[0] @@ -354,7 +373,6 @@ def test_request_media_tokens(self): HTTP_HOST=domain, ) self.assertEqual(response.status_code, 403) - response = self.client.get( '/' + path, follow=False, @@ -362,16 +380,18 @@ def test_request_media_tokens(self): ) self.assertEqual(response.status_code, 200) self.assertEqual(response['X-Accel-Redirect'], '%s%s' % ( - settings.X_ACCEL_REDIRECT_PREFIX, att.file.url)) + settings.INTERNAL_MEDIA_PREFIX, att.file.name)) @override_settings( - USE_X_ACCEL_REDIRECT=True, - FOI_MEDIA_TOKENS=True, SITE_URL='https://fragdenstaat.de', - FOI_MEDIA_DOMAIN='https://media.frag-den-staat.de', - ALLOWED_HOSTS=('fragdenstaat.de', 'media.frag-den-staat.de'), + MEDIA_URL='https://' + MEDIA_DOMAIN + '/files/', + ALLOWED_HOSTS=('fragdenstaat.de', MEDIA_DOMAIN), FOI_MEDIA_TOKEN_EXPIRY=0 ) + @patch('froide.foirequest.auth.AttachmentCrossDomainMediaAuth.TOKEN_MAX_AGE_SECONDS', + 0) + @patch('froide.foirequest.auth.AttachmentCrossDomainMediaAuth.SITE_URL', + 'https://fragdenstaat.de') def test_request_media_tokens_expired(self): att = FoiAttachment.objects.filter(approved=True)[0] req = att.belongs_to.request @@ -416,6 +436,13 @@ def test_request_public(self): response = self.client.get(att.get_absolute_url() + 'a') self.assertEqual(response.status_code, 404) + def test_attachment_pending(self): + att = FoiAttachment.objects.filter(approved=True)[0] + att.file = '' + att.save() + response = self.client.get(att.get_absolute_file_url()) + self.assertEqual(response.status_code, 404) + class PerformanceTest(TestCase): def setUp(self): diff --git a/froide/foirequest/urls/__init__.py b/froide/foirequest/urls/__init__.py index 1ed86c673..7d5d158d4 100644 --- a/froide/foirequest/urls/__init__.py +++ b/froide/foirequest/urls/__init__.py @@ -3,7 +3,7 @@ from django.utils.translation import pgettext_lazy from ..views import ( - search, auth, shortlink, auth_message_attachment, + search, auth, shortlink, AttachmentFileDetailView, project_shortlink ) @@ -42,8 +42,16 @@ auth, name="foirequest-auth"), ] +MEDIA_PATH = settings.MEDIA_URL +# Split off domain and leading slash +if MEDIA_PATH.startswith('http'): + MEDIA_PATH = MEDIA_PATH.split('/', 3)[-1] +else: + MEDIA_PATH = MEDIA_PATH[1:] + + urlpatterns += [ url(r'^%s%s/(?P\d+)/(?P.+)' % ( - settings.FOI_MEDIA_URL[1:], settings.FOI_MEDIA_PATH - ), auth_message_attachment, name='foirequest-auth_message_attachment') + MEDIA_PATH, settings.FOI_MEDIA_PATH + ), AttachmentFileDetailView.as_view(), name='foirequest-auth_message_attachment') ] diff --git a/froide/foirequest/views/__init__.py b/froide/foirequest/views/__init__.py index 439d9f7d8..abb94b547 100644 --- a/froide/foirequest/views/__init__.py +++ b/froide/foirequest/views/__init__.py @@ -5,7 +5,7 @@ ) from .attachment import ( show_attachment, delete_attachment, - approve_attachment, auth_message_attachment, redact_attachment + approve_attachment, AttachmentFileDetailView, redact_attachment ) from .draft import delete_draft, claim_draft from .list_requests import ( @@ -40,7 +40,7 @@ MyRequestsView, DraftRequestsView, FollowingRequestsView, FoiProjectListView, RequestSubscriptionsView, user_calendar, show_attachment, delete_attachment, - approve_attachment, auth_message_attachment, redact_attachment, + approve_attachment, AttachmentFileDetailView, redact_attachment, delete_draft, claim_draft, ListRequestView, search, list_unchecked, UserRequestFeedView, MakeRequestView, DraftRequestView, RequestSentView, diff --git a/froide/foirequest/views/attachment.py b/froide/foirequest/views/attachment.py index 1112f82a3..59b00ebc4 100644 --- a/froide/foirequest/views/attachment.py +++ b/froide/foirequest/views/attachment.py @@ -2,56 +2,39 @@ import json import logging -from django.conf import settings from django.urls import reverse -from django.shortcuts import render, get_object_or_404, redirect +from django.shortcuts import render, get_object_or_404, Http404, redirect from django.views.decorators.http import require_POST +from django.views.generic import DetailView from django.utils.translation import ugettext as _ from django.http import HttpResponse, JsonResponse from django.contrib import messages -from django.views.static import serve from django.templatetags.static import static +from crossdomainmedia import CrossDomainMediaMixin from froide.helper.utils import render_400, render_403 from ..models import FoiRequest, FoiMessage, FoiAttachment -from ..auth import (can_read_foirequest, can_read_foirequest_authenticated, - can_write_foirequest, is_attachment_public) +from ..auth import ( + can_write_foirequest, get_accessible_attachment_url, + AttachmentCrossDomainMediaAuth, has_attachment_access +) from ..tasks import redact_attachment_task logger = logging.getLogger(__name__) -X_ACCEL_REDIRECT_PREFIX = getattr(settings, 'X_ACCEL_REDIRECT_PREFIX', '') - - -def has_attachment_access(request, foirequest, attachment): - if not can_read_foirequest(foirequest, request): - return False - if not attachment.approved: - # allow only approved attachments to be read - # do not allow anonymous authentication here - allowed = can_read_foirequest_authenticated( - foirequest, request, allow_code=False - ) - if not allowed: - return False - return True - - -def get_accessible_attachment_url(foirequest, attachment): - needs_authentication = not is_attachment_public(foirequest, attachment) - return attachment.get_absolute_domain_file_url( - authenticated=needs_authentication - ) - def show_attachment(request, slug, message_id, attachment_name): foirequest = get_object_or_404(FoiRequest, slug=slug) message = get_object_or_404(FoiMessage, id=int(message_id), request=foirequest) - attachment = get_object_or_404(FoiAttachment, belongs_to=message, - name=attachment_name) + try: + attachment = FoiAttachment.objects.get_for_message( + message, attachment_name + ) + except FoiAttachment.DoesNotExist: + raise Http404 if not has_attachment_access(request, foirequest, attachment): if attachment.redacted and has_attachment_access( @@ -150,62 +133,34 @@ def create_document(request, slug, attachment): return redirect(att.get_anchor_url()) -def auth_message_attachment(request, message_id, attachment_name): +class AttachmentFileDetailView(CrossDomainMediaMixin, DetailView): ''' - nginx auth view + Add the CrossDomainMediaMixin + and set your custom media_auth_class ''' - message = get_object_or_404(FoiMessage, id=int(message_id)) - attachment = get_object_or_404(FoiAttachment, belongs_to=message, - name=attachment_name) - foirequest = message.request + media_auth_class = AttachmentCrossDomainMediaAuth + + def get_object(self): + self.message = get_object_or_404( + FoiMessage, id=int(self.kwargs['message_id']) + ) + try: + return FoiAttachment.objects.get_for_message( + self.message, self.kwargs['attachment_name'] + ) + except FoiAttachment.DoesNotExist: + raise Http404 - if settings.FOI_MEDIA_TOKENS: - return auth_attachment_with_token(request, foirequest, attachment) + def get_context_data(self, **kwargs): + ctx = super().get_context_data(**kwargs) + ctx['foirequest'] = self.message.request + return ctx - if not has_attachment_access(request, foirequest, attachment): - return render_403(request) + def invalid_token(self, mauth): + return render_403(self.request) - if not settings.USE_X_ACCEL_REDIRECT: - if not settings.DEBUG: - logger.warn('Django should not serve files in production!') - return serve(request, attachment.file.name, settings.MEDIA_ROOT) - - return send_attachment_file(attachment) - - -def auth_attachment_with_token(request, foirequest, attachment): - if request.get_host() not in settings.SITE_URL: - if is_attachment_public(foirequest, attachment): - return send_attachment_file(attachment) - # media domain internal NGINX check - result = attachment.check_token(request) - if not result: - if result is None: - # Redirect back to get new signature - app_url = settings.SITE_URL + attachment.get_absolute_file_url() - return redirect(app_url) - return render_403(request) - return send_attachment_file(attachment) - else: - # main domain: always deny or redirect - # in order not to render content on main domain - if is_attachment_public(foirequest, attachment): - url = attachment.get_absolute_domain_file_url(authenticated=False) - return redirect(url) - - if not has_attachment_access(request, foirequest, attachment): - # Deny access early - return render_403(request) - - url = attachment.get_absolute_domain_file_url(authenticated=True) - return redirect(url) - - -def send_attachment_file(attachment): - response = HttpResponse() - response['Content-Type'] = "" - response['X-Accel-Redirect'] = X_ACCEL_REDIRECT_PREFIX + attachment.get_internal_url() - return response + def unauthorized(self, mauth): + return render_403(self.request) def get_redact_context(foirequest, attachment): diff --git a/froide/foirequest/views/misc_views.py b/froide/foirequest/views/misc_views.py index 92fa1d143..343a8f09d 100644 --- a/froide/foirequest/views/misc_views.py +++ b/froide/foirequest/views/misc_views.py @@ -21,8 +21,6 @@ from ..auth import can_read_foirequest_authenticated from ..pdf_generator import FoiRequestPDFGenerator - -X_ACCEL_REDIRECT_PREFIX = getattr(settings, 'X_ACCEL_REDIRECT_PREFIX', '') User = get_user_model() diff --git a/froide/local_settings.py.example b/froide/local_settings.py.example index 3f4f27fef..1613ae0ad 100644 --- a/froide/local_settings.py.example +++ b/froide/local_settings.py.example @@ -100,8 +100,7 @@ class Dev(Base): # STATIC_URL = "/static/" # # Use nginx to serve uploads authenticated - # USE_X_ACCEL_REDIRECT = True - # X_ACCEL_REDIRECT_PREFIX = '/protected' + # INTERNAL_MEDIA_PREFIX = '/protected/' ### URLs that can be translated to a secret value diff --git a/froide/settings.py b/froide/settings.py index ae2b92bd6..c5de436a1 100644 --- a/froide/settings.py +++ b/froide/settings.py @@ -115,10 +115,8 @@ class Base(Configuration): # Sub path in MEDIA_ROOT that will hold FOI attachments FOI_MEDIA_PATH = values.Value('foi') - FOI_MEDIA_URL = values.Value('/files/') - FOI_MEDIA_DOMAIN = values.Value('') - FOI_MEDIA_TOKENS = False FOI_MEDIA_TOKEN_EXPIRY = 2 * 60 + INTERNAL_MEDIA_PREFIX = values.Value('/protected/') # Absolute path to the directory static files should be collected to. # Don't put anything in this directory yourself; store your static files @@ -142,9 +140,6 @@ class Base(Configuration): # Example: "http://media.lawrence.com" STATIC_URL = values.Value('/static/') - USE_X_ACCEL_REDIRECT = values.BooleanValue(False) - X_ACCEL_REDIRECT_PREFIX = values.Value('/protected') - # ## URLs that can be translated to a secret value SECRET_URLS = values.DictValue({ @@ -697,15 +692,6 @@ class SSLSite(object): SESSION_COOKIE_SECURE = True -class NginxSecureStatic(object): - USE_X_ACCEL_REDIRECT = True - X_ACCEL_REDIRECT_PREFIX = values.Value('/protected') - - -class SSLNginxProduction(SSLSite, NginxSecureStatic, Production): - pass - - class AmazonS3(object): STATICFILES_STORAGE = values.Value('storages.backends.s3boto.S3BotoStorage') diff --git a/requirements.txt b/requirements.txt index dd8d92ac8..cbe7d1c9e 100644 --- a/requirements.txt +++ b/requirements.txt @@ -10,6 +10,7 @@ dj-database-url==0.5.0 django-cache-url==3.0.0 django-configurations==2.1 django-contrib-comments==1.9.1 +django-crossdomainmedia==0.0.1 django-filter==2.1.0 django-elasticsearch-dsl==0.5.1 elasticsearch==6.3.1 diff --git a/setup.py b/setup.py index ed459da1d..09716c10e 100644 --- a/setup.py +++ b/setup.py @@ -48,6 +48,7 @@ def find_version(*file_paths): 'djangorestframework-jsonp', 'python-mimeparse', 'django-configurations', + 'django-crossdomainmedia', 'django-storages', 'django-wikidata', 'dj-database-url',