[ENG-2814] Allow Read-only and Read/Write contributors to view a proj…

…ect's draft registrations (CenterForOpenScience#10660)

## Purpose

Change permissions for DraftRegistrations so Read/Write contributors can view all listed DraftRegistrations for a node.

GET for reading, POST for DR creation and PATCH for editing

## Changes

- Add new permissions class NodeDraftRegistrationsListPermission to NodeDraftRegistrationsList
- Break up test runner into separate classes
- Split apart large single test functions into individual cases
- add more to testing matrix test_<user_permission>_draft_not_node has all cases

CenterForOpenScience#10468

## QA Notes


## Ticket

https://openscience.atlassian.net/browse/ENG-2814

api/draft_registrations/permissions.py

@@ -8,6 +8,8 @@
     OSFUser,
 )
 from api.nodes.permissions import ContributorDetailPermissions
+from osf.utils.permissions import WRITE, ADMIN
+
 
 class IsContributorOrAdminContributor(permissions.BasePermission):
     """
@@ -57,3 +59,34 @@ class DraftContributorDetailPermissions(ContributorDetailPermissions):
 
     def load_resource(self, context, view):
         return DraftRegistration.load(context['draft_id'])
+
+
+class DraftRegistrationPermission(permissions.BasePermission):
+    """
+    Check permissions for draft and node, Admin can create (POST) or edit (PATCH, PUT) to a DraftRegistration, but write
+    users can only edit them. Node permissions are inherited by the DraftRegistration when they are higher.
+    """
+    acceptable_models = (DraftRegistration, AbstractNode)
+
+    def has_object_permission(self, request, view, obj):
+        auth = get_user_auth(request)
+
+        if not auth.user:
+            return False
+
+        if request.method in permissions.SAFE_METHODS:
+            if isinstance(obj, DraftRegistration):
+                return obj.can_view(auth)
+            elif isinstance(obj, AbstractNode):
+                return obj.can_view(auth)
+        elif request.method == 'POST':  # Only Admin can create a draft registration
+            if isinstance(obj, DraftRegistration):
+                return obj.is_contributor(auth.user) and obj.has_permission(auth.user, ADMIN)
+            elif isinstance(obj, AbstractNode):
+                return obj.has_permission(auth.user, ADMIN)
+        else:
+            if isinstance(obj, DraftRegistration):
+                return obj.is_contributor(auth.user) and obj.has_permission(auth.user, WRITE)
+            elif isinstance(obj, AbstractNode):
+                return obj.has_permission(auth.user, WRITE)
+        return False

api/draft_registrations/views.py

@@ -6,7 +6,7 @@
 from api.base.pagination import DraftRegistrationContributorPagination
 from api.draft_registrations.permissions import (
     DraftContributorDetailPermissions,
-    IsContributorOrAdminContributor,
+    DraftRegistrationPermission,
     IsAdminContributor,
 )
 from api.draft_registrations.serializers import (
@@ -50,9 +50,9 @@ def check_resource_permissions(self, resource):
 
 class DraftRegistrationList(NodeDraftRegistrationsList):
     permission_classes = (
-        IsContributorOrAdminContributor,
         drf_permissions.IsAuthenticatedOrReadOnly,
         base_permissions.TokenHasScope,
+        DraftRegistrationPermission,
     )
 
     view_category = 'draft_registrations'
@@ -70,10 +70,9 @@ def get_queryset(self):
         # Returns DraftRegistrations for which a user is a contributor
         return user.draft_registrations_active
 
-
 class DraftRegistrationDetail(NodeDraftRegistrationDetail, DraftRegistrationMixin):
     permission_classes = (
-        ContributorOrPublic,
+        DraftRegistrationPermission,
         AdminDeletePermissions,
         drf_permissions.IsAuthenticatedOrReadOnly,
         base_permissions.TokenHasScope,

api/nodes/views.py

@@ -5,7 +5,7 @@
 from django.db.models import F, Max, Q, Subquery
 from django.utils import timezone
 from django.contrib.contenttypes.models import ContentType
-from rest_framework import generics, permissions as drf_permissions
+from rest_framework import generics, permissions as drf_permissions, exceptions
 from rest_framework.exceptions import PermissionDenied, ValidationError, NotFound, MethodNotAllowed, NotAuthenticated
 from rest_framework.response import Response
 from rest_framework.status import HTTP_202_ACCEPTED, HTTP_204_NO_CONTENT
@@ -66,6 +66,7 @@
     NodeCommentSerializer,
 )
 from api.draft_registrations.serializers import DraftRegistrationSerializer, DraftRegistrationDetailSerializer
+from api.draft_registrations.permissions import DraftRegistrationPermission
 from api.files.serializers import FileSerializer, OsfStorageFileSerializer
 from api.files import annotations as file_annotations
 from api.identifiers.serializers import NodeIdentifierSerializer
@@ -75,7 +76,6 @@
 from api.nodes.filters import NodesFilterMixin
 from api.nodes.permissions import (
     IsAdmin,
-    IsAdminContributor,
     IsPublic,
     AdminOrPublic,
     WriteAdmin,
@@ -626,7 +626,7 @@ class NodeDraftRegistrationsList(JSONAPIBaseView, generics.ListCreateAPIView, No
     Use DraftRegistrationsList endpoint instead.
     """
     permission_classes = (
-        IsAdminContributor,
+        DraftRegistrationPermission,
         drf_permissions.IsAuthenticatedOrReadOnly,
         base_permissions.TokenHasScope,
     )
@@ -649,8 +649,11 @@ def get_serializer_class(self):
 
     # overrides ListCreateAPIView
     def get_queryset(self):
+        user = self.request.user
         node = self.get_node()
-        return node.draft_registrations_active
+        if user.is_anonymous:
+            raise exceptions.NotAuthenticated()
+        return user.draft_registrations_active.filter(branched_from=node)
 
 
 class NodeDraftRegistrationDetail(JSONAPIBaseView, generics.RetrieveUpdateDestroyAPIView, DraftMixin):
@@ -660,9 +663,9 @@ class NodeDraftRegistrationDetail(JSONAPIBaseView, generics.RetrieveUpdateDestro
     Use DraftRegistrationDetail endpoint instead.
     """
     permission_classes = (
+        DraftRegistrationPermission,
         drf_permissions.IsAuthenticatedOrReadOnly,
         base_permissions.TokenHasScope,
-        IsAdminContributor,
     )
     parser_classes = (JSONAPIMultipleRelationshipsParser, JSONAPIMultipleRelationshipsParserForRegularJSON)
 

api_tests/draft_registrations/views/test_draft_registration_detail.py

@@ -2,10 +2,10 @@
 
 from api.base.settings.defaults import API_BASE
 from api_tests.nodes.views.test_node_draft_registration_detail import (
-    TestDraftRegistrationDetail,
     TestDraftRegistrationUpdate,
     TestDraftRegistrationPatch,
     TestDraftRegistrationDelete,
+    AbstractDraftRegistrationTestCase
 )
 from osf.models import DraftNode, Node, NodeLicense, RegistrationSchema
 from osf.utils.permissions import ADMIN, READ, WRITE
@@ -16,58 +16,34 @@
     SubjectFactory,
     ProjectFactory,
 )
+from website.settings import API_DOMAIN
 
 
 @pytest.mark.django_db
-class TestDraftRegistrationDetailEndpoint(TestDraftRegistrationDetail):
+class TestDraftRegistrationDetailEndpoint(AbstractDraftRegistrationTestCase):
     @pytest.fixture()
     def url_draft_registrations(self, project_public, draft_registration):
-        return '/{}draft_registrations/{}/'.format(
-            API_BASE, draft_registration._id)
-
-    # Overrides TestDraftRegistrationDetail
-    def test_admin_group_member_can_view(self, app, user, draft_registration, project_public,
-            schema, url_draft_registrations, group_mem):
-
-        res = app.get(url_draft_registrations, auth=group_mem.auth, expect_errors=True)
-        assert res.status_code == 403
+        return f'/{API_BASE}draft_registrations/{draft_registration._id}/'
 
-    def test_can_view_draft(
-            self, app, user_write_contrib, project_public,
-            user_read_contrib, user_non_contrib,
-            url_draft_registrations, group, group_mem):
-
-        #   test_read_only_contributor_can_view_draft
-        res = app.get(
-            url_draft_registrations,
-            auth=user_read_contrib.auth,
-            expect_errors=False)
+    def test_read_only_contributor_can_view_draft(self, app, user_read_contrib, url_draft_registrations):
+        res = app.get(url_draft_registrations, auth=user_read_contrib.auth)
         assert res.status_code == 200
 
-        #   test_read_write_contributor_can_view_draft
-        res = app.get(
-            url_draft_registrations,
-            auth=user_write_contrib.auth,
-            expect_errors=False)
+    def test_read_write_contributor_can_view_draft(self, app, user_write_contrib, url_draft_registrations):
+        res = app.get(url_draft_registrations, auth=user_write_contrib.auth)
         assert res.status_code == 200
 
-    def test_cannot_view_draft(
-            self, app, project_public,
-            user_non_contrib, url_draft_registrations):
-
-        #   test_logged_in_non_contributor_cannot_view_draft
-        res = app.get(
-            url_draft_registrations,
-            auth=user_non_contrib.auth,
-            expect_errors=True)
+    def test_logged_in_non_contributor_cannot_view_draft(self, app, user_non_contrib, url_draft_registrations):
+        res = app.get(url_draft_registrations, auth=user_non_contrib.auth, expect_errors=True)
         assert res.status_code == 403
 
-        #   test_unauthenticated_user_cannot_view_draft
+    def test_unauthenticated_user_cannot_view_draft(self, app, url_draft_registrations):
         res = app.get(url_draft_registrations, expect_errors=True)
         assert res.status_code == 401
 
-    def test_detail_view_returns_editable_fields(self, app, user, draft_registration,
-            url_draft_registrations, project_public):
+    def test_detail_view_returns_editable_fields(
+            self, app, user, draft_registration, url_draft_registrations, project_public
+    ):
 
         res = app.get(url_draft_registrations, auth=user.auth, expect_errors=True)
         attributes = res.json['data']['attributes']
@@ -77,7 +53,7 @@ def test_detail_view_returns_editable_fields(self, app, user, draft_registration
         assert attributes['category'] == project_public.category
         assert attributes['has_project']
 
-        res.json['data']['links']['self'] == url_draft_registrations
+        assert res.json['data']['links']['self'] == f'{API_DOMAIN}{url_draft_registrations.lstrip("/")}'
 
         relationships = res.json['data']['relationships']
         assert Node.load(relationships['branched_from']['data']['id']) == draft_registration.branched_from
@@ -89,8 +65,7 @@ def test_detail_view_returns_editable_fields(self, app, user, draft_registration
     def test_detail_view_returns_editable_fields_no_specified_node(self, app, user):
 
         draft_registration = DraftRegistrationFactory(initiator=user, branched_from=None)
-        url = '/{}draft_registrations/{}/'.format(
-            API_BASE, draft_registration._id)
+        url = f'{API_DOMAIN}{API_BASE}draft_registrations/{draft_registration._id}/'
 
         res = app.get(url, auth=user.auth, expect_errors=True)
         attributes = res.json['data']['attributes']
@@ -101,7 +76,7 @@ def test_detail_view_returns_editable_fields_no_specified_node(self, app, user):
         assert attributes['node_license'] is None
         assert not attributes['has_project']
 
-        res.json['data']['links']['self'] == url
+        assert res.json['data']['links']['self'] == url
         relationships = res.json['data']['relationships']
 
         assert 'affiliated_institutions' in relationships
@@ -112,44 +87,88 @@ def test_detail_view_returns_editable_fields_no_specified_node(self, app, user):
         res = app.get(draft_node_link, auth=user.auth)
         assert DraftNode.load(res.json['data']['id']) == draft_registration.branched_from
 
-    def test_draft_registration_perms_checked_on_draft_not_node(self, app, user, project_public,
-            draft_registration, url_draft_registrations):
-
-        # Admin on node and draft
+    def test_admin_node_and_draft(self, app, user, project_public, draft_registration, url_draft_registrations):
         assert project_public.has_permission(user, ADMIN) is True
         assert draft_registration.has_permission(user, ADMIN) is True
         res = app.get(url_draft_registrations, auth=user.auth)
         assert res.status_code == 200
 
-        # Admin on node but not draft
+    def test_admin_node_not_draft(self, app, user, project_public, draft_registration, url_draft_registrations):
         node_admin = AuthUserFactory()
         project_public.add_contributor(node_admin, ADMIN)
         assert project_public.has_permission(node_admin, ADMIN) is True
         assert draft_registration.has_permission(node_admin, ADMIN) is False
         res = app.get(url_draft_registrations, auth=node_admin.auth, expect_errors=True)
         assert res.status_code == 403
 
-        # Admin on draft but not node
+    def test_admin_draft_not_node(self, app, user, project_public, draft_registration, url_draft_registrations):
         draft_admin = AuthUserFactory()
         draft_registration.add_contributor(draft_admin, ADMIN)
         assert project_public.has_permission(draft_admin, ADMIN) is False
         assert draft_registration.has_permission(draft_admin, ADMIN) is True
         res = app.get(url_draft_registrations, auth=draft_admin.auth)
         assert res.status_code == 200
 
-    # Overwrites TestDraftRegistrationDetail
-    def test_can_view_after_added(
-            self, app, schema, draft_registration, url_draft_registrations):
-        # Draft Registration permissions are no longer based on the branched from project
+    def test_write_node_and_draft(self, app, user, project_public, draft_registration, url_draft_registrations):
+        assert project_public.has_permission(user, WRITE) is True
+        assert draft_registration.has_permission(user, WRITE) is True
+        res = app.get(url_draft_registrations, auth=user.auth)
+        assert res.status_code == 200
+
+    def test_write_node_not_draft(self, app, user, project_public, draft_registration, url_draft_registrations):
+        node_admin = AuthUserFactory()
+        project_public.add_contributor(node_admin, WRITE)
+        assert project_public.has_permission(node_admin, WRITE) is True
+        assert draft_registration.has_permission(node_admin, WRITE) is False
+        res = app.get(url_draft_registrations, auth=node_admin.auth, expect_errors=True)
+        assert res.status_code == 403
+
+    def test_write_draft_not_node(self, app, user, project_public, draft_registration, url_draft_registrations):
+        draft_admin = AuthUserFactory()
+        draft_registration.add_contributor(draft_admin, WRITE)
+        assert project_public.has_permission(draft_admin, WRITE) is False
+        assert draft_registration.has_permission(draft_admin, WRITE) is True
+        res = app.get(url_draft_registrations, auth=draft_admin.auth)
+        assert res.status_code == 200
+
+    def test_read_node_and_draft(self, app, user, project_public, draft_registration, url_draft_registrations):
+        assert project_public.has_permission(user, READ) is True
+        assert draft_registration.has_permission(user, READ) is True
+        res = app.get(url_draft_registrations, auth=user.auth)
+        assert res.status_code == 200
 
+    def test_read_node_not_draft(self, app, user, project_public, draft_registration, url_draft_registrations):
+        node_admin = AuthUserFactory()
+        project_public.add_contributor(node_admin, READ)
+        assert project_public.has_permission(node_admin, READ) is True
+        assert draft_registration.has_permission(node_admin, READ) is False
+        res = app.get(url_draft_registrations, auth=node_admin.auth, expect_errors=True)
+        assert res.status_code == 403
+
+    def test_read_draft_not_node(self, app, user, project_public, draft_registration, url_draft_registrations):
+        draft_admin = AuthUserFactory()
+        draft_registration.add_contributor(draft_admin, READ)
+        assert project_public.has_permission(draft_admin, READ) is False
+        assert draft_registration.has_permission(draft_admin, READ) is True
+        res = app.get(url_draft_registrations, auth=draft_admin.auth)
+        assert res.status_code == 200
+
+    def test_can_view_after_added(self, app, schema, draft_registration, url_draft_registrations):
+        """
+        Ensure Draft Registration permissions are no longer based on the branched from project
+        """
         user = AuthUserFactory()
         project = draft_registration.branched_from
         project.add_contributor(user, ADMIN)
         res = app.get(url_draft_registrations, auth=user.auth, expect_errors=True)
         assert res.status_code == 403
+        draft_registration.add_contributor(user, ADMIN)
+        res = app.get(url_draft_registrations, auth=user.auth)
+        assert res.status_code == 200
 
-    def test_current_permissions_field(self, app, user_read_contrib,
-            user_write_contrib, user, draft_registration, url_draft_registrations):
+    def test_current_permissions_field(
+            self, app, user_read_contrib, user_write_contrib, user, draft_registration, url_draft_registrations
+    ):
         res = app.get(url_draft_registrations, auth=user_read_contrib.auth, expect_errors=False)
         assert res.json['data']['attributes']['current_user_permissions'] == [READ]
 
@@ -548,9 +567,8 @@ def test_write_contributor_can_update_draft(
         assert data['attributes']['registration_metadata'] == payload['data']['attributes']['registration_metadata']
 
 
-class TestDraftRegistrationDelete(TestDraftRegistrationDelete):
+class TestDraftRegistrationDeleteDetail(TestDraftRegistrationDelete):
     @pytest.fixture()
     def url_draft_registrations(self, project_public, draft_registration):
         # Overrides TestDraftRegistrationDelete
-        return '/{}draft_registrations/{}/'.format(
-            API_BASE, draft_registration._id)
+        return f'/{API_BASE}draft_registrations/{draft_registration._id}/'