Skip to content

Commit b198427

Browse files
[ENG-8526] Fixed write contributor permissions on affiliation actions (#11397)
* fixed affiliation update for write contributors in registrations * removed redundant blank line * fixed tests * fixed tests * fixed a test * don't expect errors in tests
1 parent bbdf608 commit b198427

3 files changed

Lines changed: 47 additions & 12 deletions

File tree

api/nodes/permissions.py

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -127,6 +127,22 @@ def has_object_permission(self, request, view, obj):
127127
return obj.has_permission(auth.user, osf_permissions.WRITE)
128128

129129

130+
class AdminOrWriteContributor(permissions.BasePermission):
131+
acceptable_models = (AbstractNode, OSFUser, Institution, BaseAddonSettings, DraftRegistration)
132+
133+
def has_object_permission(self, request, view, obj):
134+
if isinstance(obj, dict) and 'self' in obj:
135+
obj = obj['self']
136+
137+
assert_resource_type(obj, self.acceptable_models)
138+
auth = get_user_auth(request)
139+
140+
if request.method in permissions.SAFE_METHODS:
141+
return obj.is_public or obj.can_view(auth)
142+
143+
return obj.has_permission(auth.user, osf_permissions.ADMIN) or obj.has_permission(auth.user, osf_permissions.WRITE)
144+
145+
130146
class AdminOrPublic(permissions.BasePermission):
131147

132148
acceptable_models = (AbstractNode, OSFUser, Institution, BaseAddonSettings, DraftRegistration)

api/registrations/views.py

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -59,6 +59,7 @@
5959
AdminOrPublic,
6060
ExcludeWithdrawals,
6161
NodeLinksShowIfVersion,
62+
AdminOrWriteContributor,
6263
)
6364
from api.registrations.permissions import ContributorOrModerator, ContributorOrModeratorOrPublic
6465
from api.registrations.serializers import (
@@ -696,7 +697,7 @@ class RegistrationInstitutionsRelationship(NodeInstitutionsRelationship, Registr
696697
permission_classes = (
697698
drf_permissions.IsAuthenticatedOrReadOnly,
698699
base_permissions.TokenHasScope,
699-
AdminOrPublic,
700+
AdminOrWriteContributor,
700701
)
701702

702703

api_tests/registrations/views/test_registration_relationship_institutions.py

Lines changed: 29 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -43,11 +43,11 @@ def resource_factory(self):
4343
return RegistrationFactory
4444

4545
# test override, write contribs can't update institution
46-
def test_put_not_admin_but_affiliated(self, app, institution_one, node, node_institutions_url):
46+
def test_put_not_admin_but_affiliated_read_permission(self, app, institution_one, node, node_institutions_url):
4747
user = AuthUserFactory()
4848
user.add_or_update_affiliated_institution(institution_one)
4949
user.save()
50-
node.add_contributor(user)
50+
node.add_contributor(user, permissions=permissions.READ)
5151
node.save()
5252

5353
res = app.put_json_api(
@@ -61,7 +61,25 @@ def test_put_not_admin_but_affiliated(self, app, institution_one, node, node_ins
6161
assert res.status_code == 403
6262
assert institution_one not in node.affiliated_institutions.all()
6363

64-
# test override, write contribs cannot delete
64+
def test_put_not_admin_but_affiliated_and_write_permission(self, app, institution_one, node, node_institutions_url):
65+
user = AuthUserFactory()
66+
user.add_or_update_affiliated_institution(institution_one)
67+
user.save()
68+
node.add_contributor(user)
69+
node.save()
70+
71+
res = app.put_json_api(
72+
node_institutions_url,
73+
self.create_payload([institution_one]),
74+
expect_errors=False,
75+
auth=user.auth
76+
)
77+
78+
node.reload()
79+
assert res.status_code == 200
80+
assert institution_one in node.affiliated_institutions.all()
81+
82+
# test override, write contribs can delete
6583
def test_delete_user_is_read_write(self, app, institution_one, node, node_institutions_url):
6684
user = AuthUserFactory()
6785
user.add_or_update_affiliated_institution(institution_one)
@@ -74,10 +92,10 @@ def test_delete_user_is_read_write(self, app, institution_one, node, node_instit
7492
node_institutions_url,
7593
self.create_payload([institution_one]),
7694
auth=user.auth,
77-
expect_errors=True
95+
expect_errors=False
7896
)
7997

80-
assert res.status_code == 403
98+
assert res.status_code == 204
8199

82100
def test_read_write_contributor_can_add_affiliated_institution(
83101
self, app, write_contrib, write_contrib_institution, node, node_institutions_url):
@@ -92,10 +110,10 @@ def test_read_write_contributor_can_add_affiliated_institution(
92110
]
93111
},
94112
auth=write_contrib.auth,
95-
expect_errors=True
113+
expect_errors=False
96114
)
97-
assert res.status_code == 403
98-
assert write_contrib_institution not in node.affiliated_institutions.all()
115+
assert res.status_code == 201
116+
assert write_contrib_institution in node.affiliated_institutions.all()
99117

100118
def test_read_write_contributor_can_remove_affiliated_institution(
101119
self, app, write_contrib, write_contrib_institution, node, node_institutions_url):
@@ -112,10 +130,10 @@ def test_read_write_contributor_can_remove_affiliated_institution(
112130
]
113131
},
114132
auth=write_contrib.auth,
115-
expect_errors=True
133+
expect_errors=False
116134
)
117-
assert res.status_code == 403
118-
assert write_contrib_institution in node.affiliated_institutions.all()
135+
assert res.status_code == 204
136+
assert write_contrib_institution not in node.affiliated_institutions.all()
119137

120138
def test_user_with_institution_and_permissions_through_patch(self, app, user, institution_one, institution_two,
121139
node, node_institutions_url):

0 commit comments

Comments
 (0)