Merge pull request #216 from dandi/method-decorator-permissions

Method decorator permissions

dandiapi/api/tests/test_version.py

@@ -171,6 +171,23 @@ def test_version_rest_update_large(api_client, user, version):
     assert version.metadata.name == new_name
 
 
+@pytest.mark.django_db
+def test_version_rest_update_not_an_owner(api_client, user, version):
+    api_client.force_authenticate(user=user)
+
+    new_name = 'A unique and special name!'
+    new_metadata = {'foo': 'bar', 'num': 123, 'list': ['a', 'b', 'c']}
+
+    assert (
+        api_client.put(
+            f'/api/dandisets/{version.dandiset.identifier}/versions/{version.version}/',
+            {'metadata': new_metadata, 'name': new_name},
+            format='json',
+        ).status_code
+        == 403
+    )
+
+
 @pytest.mark.django_db
 def test_version_rest_publish(api_client, user, version, asset):
     assign_perm('owner', user, version.dandiset)
@@ -201,3 +218,14 @@ def test_version_rest_publish(api_client, user, version, asset):
     assert asset == version.assets.get()
     assert asset == published_version.assets.get()
     assert asset.versions.count() == 2
+
+
+@pytest.mark.django_db
+def test_version_rest_publish_not_an_owner(api_client, user, version, asset):
+    api_client.force_authenticate(user=user)
+    version.assets.add(asset)
+
+    resp = api_client.post(
+        f'/api/dandisets/{version.dandiset.identifier}/versions/{version.version}/publish/'
+    )
+    assert resp.status_code == 403

dandiapi/api/views/asset.py

@@ -14,18 +14,19 @@
 from django.http import HttpResponseRedirect
 from django.shortcuts import get_object_or_404
 from django.urls import reverse
+from django.utils.decorators import method_decorator
 from django_filters import rest_framework as filters
 from drf_yasg import openapi
 from drf_yasg.utils import swagger_auto_schema
-from guardian.utils import get_40x_or_None
+from guardian.decorators import permission_required_or_403
 from rest_framework import serializers, status
 from rest_framework.decorators import action
 from rest_framework.permissions import IsAuthenticatedOrReadOnly
 from rest_framework.response import Response
 from rest_framework.viewsets import ReadOnlyModelViewSet
 from rest_framework_extensions.mixins import DetailSerializerMixin, NestedViewSetMixin
 
-from dandiapi.api.models import Asset, AssetBlob, AssetMetadata, Version
+from dandiapi.api.models import Asset, AssetBlob, AssetMetadata, Dandiset, Version
 from dandiapi.api.views.common import DandiPagination
 from dandiapi.api.views.serializers import AssetDetailSerializer, AssetSerializer
 
@@ -89,20 +90,16 @@ def retrieve(self, request, versions__dandiset__pk, versions__version, asset_id)
             404: 'If a blob with the given checksum has not been validated',
         },
     )
-    # @permission_required_or_403('owner', (Dandiset, 'pk', 'version__dandiset__pk'))
+    @method_decorator(
+        permission_required_or_403('owner', (Dandiset, 'pk', 'versions__dandiset__pk'))
+    )
     def create(self, request, versions__dandiset__pk, versions__version):
         version: Version = get_object_or_404(
             Version,
             dandiset=versions__dandiset__pk,
             version=versions__version,
         )
 
-        # TODO @permission_required doesn't work on methods
-        # https://github.com/django-guardian/django-guardian/issues/723
-        response = get_40x_or_None(request, ['owner'], version.dandiset, return_403=True)
-        if response:
-            return response
-
         serializer = AssetRequestSerializer(data=request.data)
         serializer.is_valid(raise_exception=True)
 
@@ -134,7 +131,9 @@ def create(self, request, versions__dandiset__pk, versions__version):
         request_body=AssetRequestSerializer(),
         responses={200: AssetDetailSerializer()},
     )
-    # @permission_required_or_403('owner', (Dandiset, 'pk', 'version__dandiset__pk'))
+    @method_decorator(
+        permission_required_or_403('owner', (Dandiset, 'pk', 'versions__dandiset__pk'))
+    )
     def update(self, request, versions__dandiset__pk, versions__version, **kwargs):
         """Update the metadata of an asset."""
         old_asset = self.get_object()
@@ -143,12 +142,6 @@ def update(self, request, versions__dandiset__pk, versions__version, **kwargs):
             version=versions__version,
         )
 
-        # TODO @permission_required doesn't work on methods
-        # https://github.com/django-guardian/django-guardian/issues/723
-        response = get_40x_or_None(request, ['owner'], version.dandiset, return_403=True)
-        if response:
-            return response
-
         serializer = AssetRequestSerializer(data=request.data)
         serializer.is_valid(raise_exception=True)
 
@@ -182,19 +175,15 @@ def update(self, request, versions__dandiset__pk, versions__version, **kwargs):
         serializer = AssetDetailSerializer(instance=new_asset)
         return Response(serializer.data, status=status.HTTP_200_OK)
 
-    # @permission_required_or_403('owner', (Dandiset, 'pk', 'version__dandiset__pk'))
+    @method_decorator(
+        permission_required_or_403('owner', (Dandiset, 'pk', 'versions__dandiset__pk'))
+    )
     def destroy(self, request, versions__dandiset__pk, versions__version, **kwargs):
         asset = self.get_object()
         version = Version.objects.get(
             dandiset__pk=versions__dandiset__pk, version=versions__version
         )
 
-        # TODO @permission_required doesn't work on methods
-        # https://github.com/django-guardian/django-guardian/issues/723
-        response = get_40x_or_None(request, ['owner'], version.dandiset, return_403=True)
-        if response:
-            return response
-
         version.assets.remove(asset)
         return Response(None, status=status.HTTP_204_NO_CONTENT)
 

dandiapi/api/views/dandiset.py

@@ -3,7 +3,9 @@
 from django.db.utils import IntegrityError
 from django.http import Http404
 from django.shortcuts import get_object_or_404
+from django.utils.decorators import method_decorator
 from drf_yasg.utils import swagger_auto_schema
+from guardian.decorators import permission_required_or_403
 from guardian.shortcuts import assign_perm, get_objects_for_user
 from guardian.utils import get_40x_or_None
 from rest_framework import filters, status
@@ -130,16 +132,10 @@ def create(self, request):
         serializer = DandisetDetailSerializer(instance=dandiset)
         return Response(serializer.data, status=status.HTTP_200_OK)
 
-    # @permission_required_or_403('owner', (Dandiset, 'dandiset__pk'))
+    @method_decorator(permission_required_or_403('owner', (Dandiset, 'pk', 'dandiset__pk')))
     def destroy(self, request, dandiset__pk):
         dandiset: Dandiset = get_object_or_404(Dandiset, pk=dandiset__pk)
 
-        # TODO @permission_required doesn't work on methods
-        # https://github.com/django-guardian/django-guardian/issues/723
-        response = get_40x_or_None(request, ['owner'], dandiset, return_403=True)
-        if response:
-            return response
-
         dandiset.delete()
         return Response(None, status=status.HTTP_204_NO_CONTENT)
 

dandiapi/api/views/version.py

@@ -1,4 +1,6 @@
+from django.utils.decorators import method_decorator
 from drf_yasg.utils import no_body, swagger_auto_schema
+from guardian.decorators import permission_required_or_403
 from guardian.utils import get_40x_or_None
 from rest_framework import status
 from rest_framework.decorators import action
@@ -8,7 +10,7 @@
 from rest_framework_extensions.mixins import DetailSerializerMixin, NestedViewSetMixin
 
 from dandiapi.api import doi
-from dandiapi.api.models import Version, VersionMetadata
+from dandiapi.api.models import Dandiset, Version, VersionMetadata
 from dandiapi.api.tasks import write_yamls
 from dandiapi.api.views.common import DandiPagination
 from dandiapi.api.views.serializers import (
@@ -34,7 +36,7 @@ class VersionViewSet(NestedViewSetMixin, DetailSerializerMixin, ReadOnlyModelVie
         request_body=VersionMetadataSerializer(),
         responses={200: VersionDetailSerializer()},
     )
-    # @permission_required_or_403('owner', (Dandiset, 'pk', 'dandiset__pk'))
+    @method_decorator(permission_required_or_403('owner', (Dandiset, 'pk', 'dandiset__pk')))
     def update(self, request, **kwargs):
         """Update the metadata of a version."""
         version: Version = self.get_object()
@@ -64,16 +66,10 @@ def update(self, request, **kwargs):
 
     @swagger_auto_schema(request_body=no_body, responses={200: VersionSerializer()})
     @action(detail=True, methods=['POST'])
-    # @permission_required_or_403('owner', (Dandiset, 'pk', 'dandiset__pk'))
+    @method_decorator(permission_required_or_403('owner', (Dandiset, 'pk', 'dandiset__pk')))
     def publish(self, request, **kwargs):
         old_version = self.get_object()
 
-        # TODO @permission_required doesn't work on methods
-        # https://github.com/django-guardian/django-guardian/issues/723
-        response = get_40x_or_None(request, ['owner'], old_version.dandiset, return_403=True)
-        if response:
-            return response
-
         new_version = Version.copy(old_version)
 
         new_version.doi = doi.create_doi(new_version)