Skip to content

Commit

Permalink
clean-up serializer/permission code to validate better, 409s instead …
Browse files Browse the repository at this point in the history
…of 403s etc
  • Loading branch information
John Tordoff committed Dec 6, 2024
1 parent ec0d147 commit cd45df2
Show file tree
Hide file tree
Showing 3 changed files with 16 additions and 39 deletions.
40 changes: 8 additions & 32 deletions api/users/permissions.py
Original file line number Diff line number Diff line change
Expand Up @@ -68,41 +68,17 @@ def has_permission(self, request, view) -> bool:
if not user or user.is_anonymous:
return False

message_type = request.data.get('message_type')
if message_type == MessageTypes.INSTITUTIONAL_REQUEST:
return self._validate_institutional_request(request, user)

return False

def _validate_institutional_request(self, request, user: OSFUser) -> bool:
"""
Validate the user's permissions for creating an `INSTITUTIONAL_REQUEST` message.
Args:
request: The HTTP request containing the institution ID.
user: The user making the request.
Returns:
bool: True if the user has the required permission.
"""
institution_id = request.data.get('institution')
if not institution_id:
raise exceptions.ValidationError({'institution': 'Institution ID is required.'})

institution = self._get_institution(institution_id)

if not user.is_institutional_admin(institution):
raise exceptions.PermissionDenied('You are not an admin of the specified institution.')
raise exceptions.ValidationError({'institution': 'Institution is required.'})

return True

def _get_institution(self, institution_id: str) -> Institution:
"""
Retrieve the institution by its ID.
Args:
institution_id (str): The ID of the institution.
Returns:
Institution: The retrieved institution.
"""
try:
return Institution.objects.get(_id=institution_id)
institution = Institution.objects.get(_id=institution_id)
except Institution.DoesNotExist:
raise exceptions.ValidationError({'institution': 'Specified institution does not exist.'})

message_type = request.data.get('message_type')
if message_type == MessageTypes.INSTITUTIONAL_REQUEST:
return user.is_institutional_admin(institution)
else:
raise exceptions.ValidationError('Not valid message type.')
9 changes: 5 additions & 4 deletions api/users/serializers.py
Original file line number Diff line number Diff line change
Expand Up @@ -707,9 +707,10 @@ def get_absolute_url(self, obj: UserMessage) -> str:
)

def to_internal_value(self, data):
instituion_id = data.pop('institution')
instituion_id = data.pop('institution', None)
data = super().to_internal_value(data)
data['institution'] = instituion_id
if instituion_id:
data['institution'] = instituion_id
return data

class Meta:
Expand Down Expand Up @@ -751,10 +752,10 @@ def create(self, validated_data: Dict[str, Any]) -> UserMessage:
)

if not sender.is_institutional_admin(institution):
raise exceptions.ValidationError({'sender': 'Only institutional administrators can create messages.'})
raise Conflict({'sender': 'Only institutional administrators can create messages.'})

if not recipient.is_affiliated_with_institution(institution):
raise exceptions.ValidationError(
raise Conflict(
{'user': 'Can not send to recipient that is not affiliated with the provided institution.'},
)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,7 @@ def test_request_without_institution(self, app, institutional_admin, user, url_w
'source': {
'pointer': '/data/relationships/institution'
},
'detail': 'Institution ID is required.'
'detail': 'Institution is required.'
}
]

Expand Down Expand Up @@ -150,6 +150,6 @@ def test_admin_cannot_message_user_outside_institution(
Ensure that an institutional admin cannot create a `UserMessage` for a user who is not affiliated with their institution.
"""
res = app.post_json_api(url_without_affiliation, payload, auth=institutional_admin.auth, expect_errors=True)
assert res.status_code == 400
assert res.status_code == 409
assert 'Can not send to recipient that is not affiliated with the provided institution.'\
in res.json['errors'][0]['detail']
in res.json['errors'][0]['detail']['user']

0 comments on commit cd45df2

Please sign in to comment.