From 74b052a9d8e2b1ab2b6599838b8a2967e81bfb78 Mon Sep 17 00:00:00 2001 From: Ihor Sokhan Date: Thu, 7 Aug 2025 18:38:24 +0300 Subject: [PATCH 1/5] fixed affiliation update for write contributors in registrations --- api/nodes/permissions.py | 17 +++++++++++++++++ api/registrations/views.py | 3 ++- 2 files changed, 19 insertions(+), 1 deletion(-) diff --git a/api/nodes/permissions.py b/api/nodes/permissions.py index 5fc16f6cf16..c15919379b0 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) @@ -141,6 +157,7 @@ def has_object_permission(self, request, view, obj): if request.method in permissions.SAFE_METHODS: return obj.is_public or obj.can_view(auth) else: + return obj.has_permission(auth.user, osf_permissions.ADMIN) class AdminContributorOrPublic(permissions.BasePermission): 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, ) From 02f1bdb505b511afa050a1c8d4a22301e427c59d Mon Sep 17 00:00:00 2001 From: Ihor Sokhan Date: Thu, 7 Aug 2025 18:40:37 +0300 Subject: [PATCH 2/5] removed redundant blank line --- api/nodes/permissions.py | 1 - 1 file changed, 1 deletion(-) diff --git a/api/nodes/permissions.py b/api/nodes/permissions.py index c15919379b0..e7d2b773280 100644 --- a/api/nodes/permissions.py +++ b/api/nodes/permissions.py @@ -157,7 +157,6 @@ def has_object_permission(self, request, view, obj): if request.method in permissions.SAFE_METHODS: return obj.is_public or obj.can_view(auth) else: - return obj.has_permission(auth.user, osf_permissions.ADMIN) class AdminContributorOrPublic(permissions.BasePermission): From cf67c7d6072ad860004294efe818b99d5c7aeb10 Mon Sep 17 00:00:00 2001 From: Ihor Sokhan Date: Fri, 8 Aug 2025 16:14:34 +0300 Subject: [PATCH 3/5] fixed tests --- ..._registration_relationship_institutions.py | 30 +++++++++++++++---- 1 file changed, 24 insertions(+), 6 deletions(-) diff --git a/api_tests/registrations/views/test_registration_relationship_institutions.py b/api_tests/registrations/views/test_registration_relationship_institutions.py index b1a482c914a..aca7b3f52a3 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): @@ -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): From dd26ec11113a1793cff98469089a435f19d55e09 Mon Sep 17 00:00:00 2001 From: Ihor Sokhan Date: Fri, 8 Aug 2025 16:29:27 +0300 Subject: [PATCH 4/5] fixed tests --- .../views/test_registration_relationship_institutions.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/api_tests/registrations/views/test_registration_relationship_institutions.py b/api_tests/registrations/views/test_registration_relationship_institutions.py index aca7b3f52a3..bd281ac8806 100644 --- a/api_tests/registrations/views/test_registration_relationship_institutions.py +++ b/api_tests/registrations/views/test_registration_relationship_institutions.py @@ -109,7 +109,7 @@ def test_read_write_contributor_can_add_affiliated_institution( auth=write_contrib.auth, expect_errors=True ) - assert res.status_code == 403 + assert res.status_code == 201 assert write_contrib_institution not in node.affiliated_institutions.all() def test_read_write_contributor_can_remove_affiliated_institution( From 8c222ffcfff4b2129ff5f141f280a895f122e1fa Mon Sep 17 00:00:00 2001 From: Ihor Sokhan Date: Fri, 8 Aug 2025 16:44:49 +0300 Subject: [PATCH 5/5] fixed a test --- .../views/test_registration_relationship_institutions.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/api_tests/registrations/views/test_registration_relationship_institutions.py b/api_tests/registrations/views/test_registration_relationship_institutions.py index bd281ac8806..07d71b3ca48 100644 --- a/api_tests/registrations/views/test_registration_relationship_institutions.py +++ b/api_tests/registrations/views/test_registration_relationship_institutions.py @@ -110,7 +110,7 @@ def test_read_write_contributor_can_add_affiliated_institution( expect_errors=True ) assert res.status_code == 201 - assert write_contrib_institution not in node.affiliated_institutions.all() + 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):