From a8c5f363efc4776b218ddcba7b4e98fc13660a3e Mon Sep 17 00:00:00 2001 From: Anton Krytskyi Date: Thu, 18 Dec 2025 17:02:52 +0200 Subject: [PATCH] restrict R/W users from changing institutions they are not affiliated with --- api/institutions/views.py | 8 ++++++-- .../test_institution_relationship_nodes.py | 18 +++++++++++++++++- 2 files changed, 23 insertions(+), 3 deletions(-) diff --git a/api/institutions/views.py b/api/institutions/views.py index 60f8d31b88f..a3c0f93d0c8 100644 --- a/api/institutions/views.py +++ b/api/institutions/views.py @@ -338,7 +338,8 @@ class InstitutionNodesRelationship(JSONAPIBaseView, generics.RetrieveDestroyAPIV } Success: 204 - This requires write permissions in the nodes requested. + This requires write permissions in the nodes requested. If the user has admin permissions on the node, + the institution does not need to be affiliated in their account. """ permission_classes = ( drf_permissions.IsAuthenticatedOrReadOnly, @@ -367,16 +368,19 @@ def get_object(self): def perform_destroy(self, instance): data = self.request.data['data'] user = self.request.user + inst = instance['self'] ids = [datum['id'] for datum in data] nodes = [] for id_ in ids: node = Node.load(id_) if not node.has_permission(user, osf_permissions.WRITE): raise exceptions.PermissionDenied(detail=f'Write permission on node {id_} required') + if not user.is_affiliated_with_institution(inst) and not node.has_permission(user, osf_permissions.ADMIN): + raise exceptions.PermissionDenied(detail=f'User needs to be affiliated with {inst.name}') nodes.append(node) for node in nodes: - node.remove_affiliated_institution(inst=instance['self'], user=user) + node.remove_affiliated_institution(inst=inst, user=user) node.save() def create(self, *args, **kwargs): diff --git a/api_tests/institutions/views/test_institution_relationship_nodes.py b/api_tests/institutions/views/test_institution_relationship_nodes.py index c62d760710d..c2241095c47 100644 --- a/api_tests/institutions/views/test_institution_relationship_nodes.py +++ b/api_tests/institutions/views/test_institution_relationship_nodes.py @@ -289,7 +289,7 @@ def test_delete_user_is_admin(self, app, url_institution_nodes, node_one, user, assert res.status_code == 204 assert institution not in node_one.affiliated_institutions.all() - def test_delete_user_is_read_write(self, app, node_private, user, url_institution_nodes, institution): + def test_delete_user_is_read_write_and_affiliated(self, app, node_private, user, url_institution_nodes, institution): node_private.add_contributor(user) node_private.save() @@ -303,6 +303,22 @@ def test_delete_user_is_read_write(self, app, node_private, user, url_institutio assert res.status_code == 204 assert institution not in node_private.affiliated_institutions.all() + def test_delete_user_is_read_write_but_not_affiliated(self, app, node_private, url_institution_nodes, institution): + user_not_affiliated = AuthUserFactory() + node_private.add_contributor(user_not_affiliated, permissions=permissions.WRITE) + node_private.save() + + res = app.delete_json_api( + url_institution_nodes, + make_payload(node_private._id), + auth=user_not_affiliated.auth, + expect_errors=True + ) + node_private.reload() + + assert res.status_code == 403 + assert institution in node_private.affiliated_institutions.all() + def test_delete_user_is_read_only(self, node_private, user, app, url_institution_nodes, institution): node_private.add_contributor(user, permissions=permissions.READ) node_private.save()