Check permissions when accessing attachments

Attachment files dispatching is now done through `RawAttachmentView`
view that checks for appropiate permissions.

When using the development server this view just redirects to the real
media path of the file.

When using the production server the special redirection header
`X-Accel-Redirect` is used instead to improve efficiency by instructing
the server to dispatch the file instead of django, but you also need the
following configuration (Nginx):

location /attachment-files {
    internal;
    alias /path/to/taiga/media/attachment-files;
}

It's recommended to also restrict the direct access from outside to the
`attachment-files` directory by using some configuration like this:

location /media/attachment-files {
    deny all;
}

settings/common.py

@@ -76,7 +76,7 @@ SITES = {
 SITE_ID = "api"
 
 # Session configuration (only used for admin)
-SESSION_ENGINE="django.contrib.sessions.backends.db"
+SESSION_ENGINE = "django.contrib.sessions.backends.db"
 SESSION_COOKIE_AGE = 1209600 # (2 weeks)
 
 # MAIL OPTIONS
@@ -325,3 +325,8 @@ GRAVATAR_DEFAULT_OPTIONS = {
     'default': DEFAULT_AVATAR_URL, # default avatar to show if there's no gravatar image
     'size': DEFAULT_AVATAR_SIZE
 }
+
+try:
+    IN_DEVELOPMENT_SERVER = sys.argv[1] == 'runserver'
+except IndexError:
+    IN_DEVELOPMENT_SERVER = False

settings/testing.py

@@ -20,3 +20,5 @@ SKIP_SOUTH_TESTS = True
 SOUTH_TESTS_MIGRATE = False
 
 CELERY_ALWAYS_EAGER = True
+
+MEDIA_ROOT = "/tmp"

taiga/base/utils/urls.py

@@ -1,4 +1,5 @@
 import django_sites as sites
+from django.core.urlresolvers import reverse as django_reverse
 
 URL_TEMPLATE = "{scheme}://{domain}/{path}"
 
@@ -18,3 +19,8 @@ def get_absolute_url(path):
         return path
     site = sites.get_current()
     return build_url(path, scheme=site.scheme, domain=site.domain)
+
+
+def reverse(viewname, *args, **kwargs):
+    """Same behavior as django's reverse but uses django_sites to compute absolute url."""
+    return get_absolute_url(django_reverse(viewname, *args, **kwargs))

taiga/projects/attachments/serializers.py

@@ -13,13 +13,13 @@
 #
 # You should have received a copy of the GNU Affero General Public License
 # along with this program.  If not, see <http://www.gnu.org/licenses/>.
+from os import path
 
 from rest_framework import serializers
 
+from taiga.base.utils.urls import reverse
 from . import models
 
-from os import path
-
 
 class AttachmentSerializer(serializers.ModelSerializer):
     name = serializers.SerializerMethodField("get_name")
@@ -39,7 +39,7 @@ class AttachmentSerializer(serializers.ModelSerializer):
         return ""
 
     def get_url(self, obj):
-        return obj.attached_file.url if obj and obj.attached_file else ""
+        return reverse("attachment-url", kwargs={"pk": obj.pk})
 
     def get_size(self, obj):
         if obj.attached_file:

taiga/projects/attachments/views.py

@@ -0,0 +1,32 @@
+import os
+
+from django.conf import settings
+from django import http
+
+from rest_framework import generics
+from rest_framework.permissions import IsAuthenticated
+
+from . import permissions
+from . import models
+
+
+def serve_attachment(request, attachment):
+    if settings.IN_DEVELOPMENT_SERVER:
+        return http.HttpResponseRedirect(attachment.url)
+
+    name = attachment.name
+    response = http.HttpResponse()
+    response['X-Accel-Redirect'] = "/{filepath}".format(filepath=name)
+    response['Content-Disposition'] = 'attachment;filename={filename}'.format(
+        filename=os.path.basename(name))
+
+    return response
+
+
+class RawAttachmentView(generics.RetrieveAPIView):
+    queryset = models.Attachment.objects.all()
+    permission_classes = (IsAuthenticated, permissions.AttachmentPermission,)
+
+    def retrieve(self, request, *args, **kwargs):
+        self.object = self.get_object()
+        return serve_attachment(request, self.object.attached_file)

taiga/urls.py

@@ -20,11 +20,14 @@ from django.contrib.staticfiles.urls import staticfiles_urlpatterns
 from django.contrib import admin
 
 from .routers import router
+from .projects.attachments.views import RawAttachmentView
+
 
 
 admin.autodiscover()
 
 urlpatterns = patterns('',
+    url(r'^attachments/(?P<pk>\d+)/$', RawAttachmentView.as_view(), name="attachment-url"),
     url(r'^api/v1/', include(router.urls)),
     url(r'^api/v1/api-auth/', include('rest_framework.urls', namespace='rest_framework')),
     url(r'^admin/', include(admin.site.urls)),

tests/factories.py

@@ -258,6 +258,16 @@ class ContentTypeFactory(Factory):
     model = factory.LazyAttribute(lambda obj: ContentTypeFactory.FACTORY_FOR._meta.model_name)
 
 
+class AttachmentFactory(Factory):
+    FACTORY_FOR = get_model("attachments", "Attachment")
+
+    owner = factory.SubFactory("tests.factories.UserFactory")
+    project = factory.SubFactory("tests.factories.ProjectFactory")
+    content_type = factory.SubFactory("tests.factories.ContentTypeFactory")
+    object_id = factory.Sequence(lambda n: n)
+    attached_file = factory.django.FileField(data=b"File contents")
+
+
 def create_issue(**kwargs):
     "Create an issue and along with its dependencies."
     owner = kwargs.pop("owner", UserFactory())

tests/integration/test_attachments.py

@@ -0,0 +1,58 @@
+import pytest
+
+from django.core.urlresolvers import reverse
+from django.core.files.base import File
+
+from .. import factories as f
+from ..utils import set_settings
+
+pytestmark = pytest.mark.django_db
+
+def test_authentication(client):
+    "User can't access an attachment if not authenticated"
+    attachment = f.AttachmentFactory.create()
+    url = reverse("attachment-url", kwargs={"pk": attachment.pk})
+
+    response = client.get(url)
+
+    assert response.status_code == 401
+
+
+def test_authorization(client):
+    "User can't access an attachment if not authorized"
+    attachment = f.AttachmentFactory.create()
+    user = f.UserFactory.create()
+
+    url = reverse("attachment-url", kwargs={"pk": attachment.pk})
+
+    client.login(user)
+    response = client.get(url)
+
+    assert response.status_code == 403
+
+
+@set_settings(IN_DEVELOPMENT_SERVER=True)
+def test_attachment_redirect_in_devserver(client):
+    "When accessing the attachment in devserver redirect to the real attachment url"
+    attachment = f.AttachmentFactory.create()
+
+    url = reverse("attachment-url", kwargs={"pk": attachment.pk})
+
+    client.login(attachment.owner)
+    response = client.get(url)
+
+    assert response.status_code == 302
+
+
+@set_settings(IN_DEVELOPMENT_SERVER=False)
+def test_attachment_redirect(client):
+    "When accessing the attachment redirect using X-Accel-Redirect header"
+    attachment = f.AttachmentFactory.create()
+
+    url = reverse("attachment-url", kwargs={"pk": attachment.pk})
+
+    client.login(attachment.owner)
+    response = client.get(url)
+
+    assert response.status_code == 200
+    assert response.has_header('x-accel-redirect')