Merge pull request #218 from dandi/lock-down-publish

Lock down publish

dandiapi/api/tests/conftest.py

@@ -31,15 +31,20 @@
 register(AssetBlobFactory)
 register(AssetMetadataFactory)
 register(DandisetFactory)
-register(PublishedVersionFactory)
-register(DraftVersionFactory)
+register(PublishedVersionFactory, _name='published_version')
+register(DraftVersionFactory, _name='draft_version')
 # registering DraftVersionFactory after PublishedVersionFactory means
 # the fixture `version` will always be a draft
 register(UserFactory)
 register(UploadFactory)
 register(VersionMetadataFactory)
 
 
+@pytest.fixture(params=[DraftVersionFactory, PublishedVersionFactory], ids=['draft', 'published'])
+def version(request):
+    return request.param()
+
+
 @pytest.fixture
 def api_client() -> APIClient:
     return APIClient()

dandiapi/api/tests/test_version.py

@@ -189,33 +189,35 @@ def test_version_rest_update_not_an_owner(api_client, user, version):
 
 
 @pytest.mark.django_db
-def test_version_rest_publish(api_client, user, version, asset):
-    assign_perm('owner', user, version.dandiset)
-    api_client.force_authenticate(user=user)
-    version.assets.add(asset)
+# TODO change admin_user back to a normal user once publish is allowed
+def test_version_rest_publish(api_client, admin_user, draft_version, asset):
+    assign_perm('owner', admin_user, draft_version.dandiset)
+    api_client.force_authenticate(user=admin_user)
+    draft_version.assets.add(asset)
 
     resp = api_client.post(
-        f'/api/dandisets/{version.dandiset.identifier}/versions/{version.version}/publish/'
+        f'/api/dandisets/{draft_version.dandiset.identifier}'
+        f'/versions/{draft_version.version}/publish/'
     )
     assert resp.data == {
         'dandiset': {
-            'identifier': version.dandiset.identifier,
+            'identifier': draft_version.dandiset.identifier,
             'created': TIMESTAMP_RE,
             'modified': TIMESTAMP_RE,
         },
         'version': VERSION_ID_RE,
-        'name': version.name,
+        'name': draft_version.name,
         'created': TIMESTAMP_RE,
         'modified': TIMESTAMP_RE,
         'asset_count': 1,
-        'size': version.size,
+        'size': draft_version.size,
     }
     published_version = Version.objects.get(version=resp.data['version'])
     assert published_version
-    assert version.dandiset.versions.count() == 2
+    assert draft_version.dandiset.versions.count() == 2
 
     # The original asset should now be in both versions
-    assert asset == version.assets.get()
+    assert asset == draft_version.assets.get()
     assert asset == published_version.assets.get()
     assert asset.versions.count() == 2
 
@@ -229,3 +231,31 @@ def test_version_rest_publish_not_an_owner(api_client, user, version, asset):
         f'/api/dandisets/{version.dandiset.identifier}/versions/{version.version}/publish/'
     )
     assert resp.status_code == 403
+
+
+# TODO remove this test once publish is allowed
+@pytest.mark.django_db
+def test_version_rest_publish_not_an_admin(api_client, user, version, asset):
+    assign_perm('owner', user, version.dandiset)
+    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
+    assert resp.data == 'Must be an admin to publish'
+
+
+@pytest.mark.django_db
+# TODO change admin_user back to a normal user once publish is allowed
+def test_version_rest_publish_not_a_draft(api_client, admin_user, published_version, asset):
+    assign_perm('owner', admin_user, published_version.dandiset)
+    api_client.force_authenticate(user=admin_user)
+    published_version.assets.add(asset)
+
+    resp = api_client.post(
+        f'/api/dandisets/{published_version.dandiset.identifier}'
+        f'/versions/{published_version.version}/publish/'
+    )
+    assert resp.status_code == 405

dandiapi/api/views/version.py

@@ -1,7 +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
 from rest_framework.permissions import IsAuthenticatedOrReadOnly
@@ -41,12 +40,6 @@ def update(self, request, **kwargs):
         """Update the metadata of a version."""
         version: 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'], version.dandiset, return_403=True)
-        if response:
-            return response
-
         serializer = VersionMetadataSerializer(data=request.data)
         serializer.is_valid(raise_exception=True)
 
@@ -68,7 +61,16 @@ def update(self, request, **kwargs):
     @action(detail=True, methods=['POST'])
     @method_decorator(permission_required_or_403('owner', (Dandiset, 'pk', 'dandiset__pk')))
     def publish(self, request, **kwargs):
+        # TODO remove this check once publish is allowed
+        if not request.user.is_superuser:
+            return Response('Must be an admin to publish', status=status.HTTP_403_FORBIDDEN)
+
         old_version = self.get_object()
+        if old_version.version != 'draft':
+            return Response(
+                'Only draft versions can be published',
+                status=status.HTTP_405_METHOD_NOT_ALLOWED,
+            )
 
         new_version = Version.copy(old_version)