Merge pull request #55 from taigaio/attachments

Check permissions when accessing attachments

settings/common.py

@@ -324,3 +324,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

@@ -266,6 +266,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')