[ENG-2814] Allow Read-only and Read/Write contributors to view a project's draft registrations

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}/'