From 7a6154bbbd849248c640382eb94844fb6fbe1e11 Mon Sep 17 00:00:00 2001 From: Ihor Sokhan Date: Thu, 7 Aug 2025 18:38:24 +0300 Subject: [PATCH 1/6] 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 368348c05f9..caab1e134de 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 ( @@ -696,7 +697,7 @@ class RegistrationInstitutionsRelationship(NodeInstitutionsRelationship, Registr permission_classes = ( drf_permissions.IsAuthenticatedOrReadOnly, base_permissions.TokenHasScope, - AdminOrPublic, + AdminOrWriteContributor, ) From b8db0944f3451e3cf578ee2835f6cb117642dc60 Mon Sep 17 00:00:00 2001 From: Ihor Sokhan Date: Thu, 7 Aug 2025 18:40:37 +0300 Subject: [PATCH 2/6] 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 20a29cc07ae338b7cc36cb9228fa44b4bbeb3b92 Mon Sep 17 00:00:00 2001 From: Ihor Sokhan Date: Fri, 8 Aug 2025 16:14:34 +0300 Subject: [PATCH 3/6] 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 033d59af697..864703eb2f6 100644 --- a/api_tests/registrations/views/test_registration_relationship_institutions.py +++ b/api_tests/registrations/views/test_registration_relationship_institutions.py @@ -43,11 +43,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( @@ -61,7 +61,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) @@ -77,7 +95,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): @@ -114,8 +132,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 2b54e98a5a05295c5f19700aea6d08ed45c776c0 Mon Sep 17 00:00:00 2001 From: Ihor Sokhan Date: Fri, 8 Aug 2025 16:29:27 +0300 Subject: [PATCH 4/6] 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 864703eb2f6..50fe721da26 100644 --- a/api_tests/registrations/views/test_registration_relationship_institutions.py +++ b/api_tests/registrations/views/test_registration_relationship_institutions.py @@ -112,7 +112,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 b6fc13d65ca799b075e84afacbf69b58902d2da5 Mon Sep 17 00:00:00 2001 From: Ihor Sokhan Date: Fri, 8 Aug 2025 16:44:49 +0300 Subject: [PATCH 5/6] 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 50fe721da26..39499788105 100644 --- a/api_tests/registrations/views/test_registration_relationship_institutions.py +++ b/api_tests/registrations/views/test_registration_relationship_institutions.py @@ -113,7 +113,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): From 0ba2e666951ab042a571ba25bab7a7cc07e1f4b0 Mon Sep 17 00:00:00 2001 From: Ihor Sokhan Date: Fri, 21 Nov 2025 15:31:23 +0200 Subject: [PATCH 6/6] don't expect errors in tests --- .../views/test_registration_relationship_institutions.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/api_tests/registrations/views/test_registration_relationship_institutions.py b/api_tests/registrations/views/test_registration_relationship_institutions.py index 39499788105..3ee6f8c0486 100644 --- a/api_tests/registrations/views/test_registration_relationship_institutions.py +++ b/api_tests/registrations/views/test_registration_relationship_institutions.py @@ -71,7 +71,7 @@ def test_put_not_admin_but_affiliated_and_write_permission(self, app, institutio res = app.put_json_api( node_institutions_url, self.create_payload([institution_one]), - expect_errors=True, + expect_errors=False, auth=user.auth ) @@ -92,7 +92,7 @@ def test_delete_user_is_read_write(self, app, institution_one, node, node_instit node_institutions_url, self.create_payload([institution_one]), auth=user.auth, - expect_errors=True + expect_errors=False ) assert res.status_code == 204 @@ -110,7 +110,7 @@ def test_read_write_contributor_can_add_affiliated_institution( ] }, auth=write_contrib.auth, - expect_errors=True + expect_errors=False ) assert res.status_code == 201 assert write_contrib_institution in node.affiliated_institutions.all() @@ -130,7 +130,7 @@ def test_read_write_contributor_can_remove_affiliated_institution( ] }, auth=write_contrib.auth, - expect_errors=True + expect_errors=False ) assert res.status_code == 204 assert write_contrib_institution not in node.affiliated_institutions.all()