diff --git a/api/nodes/permissions.py b/api/nodes/permissions.py index 5fc16f6cf16..e7d2b773280 100644 --- a/api/nodes/permissions.py +++ b/api/nodes/permissions.py @@ -127,6 +127,22 @@ def has_object_permission(self, request, view, obj): return obj.has_permission(auth.user, osf_permissions.WRITE) +class AdminOrWriteContributor(permissions.BasePermission): + acceptable_models = (AbstractNode, OSFUser, Institution, BaseAddonSettings, DraftRegistration) + + def has_object_permission(self, request, view, obj): + if isinstance(obj, dict) and 'self' in obj: + obj = obj['self'] + + assert_resource_type(obj, self.acceptable_models) + auth = get_user_auth(request) + + if request.method in permissions.SAFE_METHODS: + return obj.is_public or obj.can_view(auth) + + return obj.has_permission(auth.user, osf_permissions.ADMIN) or obj.has_permission(auth.user, osf_permissions.WRITE) + + class AdminOrPublic(permissions.BasePermission): acceptable_models = (AbstractNode, OSFUser, Institution, BaseAddonSettings, DraftRegistration) diff --git a/api/registrations/views.py b/api/registrations/views.py index b2026d5f4b8..2b1030d9629 100644 --- a/api/registrations/views.py +++ b/api/registrations/views.py @@ -59,6 +59,7 @@ AdminOrPublic, ExcludeWithdrawals, NodeLinksShowIfVersion, + AdminOrWriteContributor, ) from api.registrations.permissions import ContributorOrModerator, ContributorOrModeratorOrPublic from api.registrations.serializers import ( @@ -682,7 +683,7 @@ class RegistrationInstitutionsRelationship(NodeInstitutionsRelationship, Registr permission_classes = ( drf_permissions.IsAuthenticatedOrReadOnly, base_permissions.TokenHasScope, - AdminOrPublic, + AdminOrWriteContributor, ) diff --git a/api_tests/registrations/views/test_registration_relationship_institutions.py b/api_tests/registrations/views/test_registration_relationship_institutions.py index b1a482c914a..07d71b3ca48 100644 --- a/api_tests/registrations/views/test_registration_relationship_institutions.py +++ b/api_tests/registrations/views/test_registration_relationship_institutions.py @@ -40,11 +40,11 @@ def resource_factory(self): return RegistrationFactory # test override, write contribs can't update institution - def test_put_not_admin_but_affiliated(self, app, institution_one, node, node_institutions_url): + def test_put_not_admin_but_affiliated_read_permission(self, app, institution_one, node, node_institutions_url): user = AuthUserFactory() user.add_or_update_affiliated_institution(institution_one) user.save() - node.add_contributor(user) + node.add_contributor(user, permissions=permissions.READ) node.save() res = app.put_json_api( @@ -58,7 +58,25 @@ def test_put_not_admin_but_affiliated(self, app, institution_one, node, node_ins assert res.status_code == 403 assert institution_one not in node.affiliated_institutions.all() - # test override, write contribs cannot delete + def test_put_not_admin_but_affiliated_and_write_permission(self, app, institution_one, node, node_institutions_url): + user = AuthUserFactory() + user.add_or_update_affiliated_institution(institution_one) + user.save() + node.add_contributor(user) + node.save() + + res = app.put_json_api( + node_institutions_url, + self.create_payload([institution_one]), + expect_errors=True, + auth=user.auth + ) + + node.reload() + assert res.status_code == 200 + assert institution_one in node.affiliated_institutions.all() + + # test override, write contribs can delete def test_delete_user_is_read_write(self, app, institution_one, node, node_institutions_url): user = AuthUserFactory() user.add_or_update_affiliated_institution(institution_one) @@ -74,7 +92,7 @@ def test_delete_user_is_read_write(self, app, institution_one, node, node_instit expect_errors=True ) - assert res.status_code == 403 + assert res.status_code == 204 def test_read_write_contributor_can_add_affiliated_institution( self, app, write_contrib, write_contrib_institution, node, node_institutions_url): @@ -91,8 +109,8 @@ def test_read_write_contributor_can_add_affiliated_institution( auth=write_contrib.auth, expect_errors=True ) - assert res.status_code == 403 - assert write_contrib_institution not in node.affiliated_institutions.all() + assert res.status_code == 201 + assert write_contrib_institution in node.affiliated_institutions.all() def test_read_write_contributor_can_remove_affiliated_institution( self, app, write_contrib, write_contrib_institution, node, node_institutions_url): @@ -111,8 +129,8 @@ def test_read_write_contributor_can_remove_affiliated_institution( auth=write_contrib.auth, expect_errors=True ) - assert res.status_code == 403 - assert write_contrib_institution in node.affiliated_institutions.all() + assert res.status_code == 204 + assert write_contrib_institution not in node.affiliated_institutions.all() def test_user_with_institution_and_permissions_through_patch(self, app, user, institution_one, institution_two, node, node_institutions_url):