Skip to content

Commit 33f6e6a

Browse files
author
Matthew Fisher
committed
fix(controller): disallow unauthorized users from deleting apps
Authenticated users who were not given explicit permission to view or modify an application were still allowed to delete other user's applications, should the unauthorized user know what the ID of the application would be. This returns a 404 message if the user is unauthorized to see the application. Because we were calling get_object_or_404(), we were not performing any authorization against the API call. Calling self.get_object() is the preferred method to retrieve an object since it authenticates against the classes' permissions classes to ensure that the user is authorized to see the application. This also fixes up an issue where administrators were not explicitly given permission to an app. They fell through the same "unauthorized" issue as above.
1 parent f3489c3 commit 33f6e6a

3 files changed

Lines changed: 7 additions & 1 deletion

File tree

controller/api/permissions.py

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,8 @@ class IsAppUser(permissions.BasePermission):
3636
an app-related model.
3737
"""
3838
def has_object_permission(self, request, view, obj):
39+
if request.user.is_superuser:
40+
return True
3941
if isinstance(obj, models.App) and obj.owner == request.user:
4042
return True
4143
elif hasattr(obj, 'app') and obj.app.owner == request.user:

controller/api/tests/test_app.py

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -275,6 +275,10 @@ def test_unauthorized_user_cannot_see_app(self):
275275
url = '{}/{}/logs'.format(base_url, app_id)
276276
response = self.client.get(url, HTTP_AUTHORIZATION='token {}'.format(unauthorized_token))
277277
self.assertEqual(response.status_code, 404)
278+
url = '{}/{}'.format(base_url, app_id)
279+
response = self.client.delete(url,
280+
HTTP_AUTHORIZATION='token {}'.format(unauthorized_token))
281+
self.assertEqual(response.status_code, 404)
278282

279283
def test_app_info_not_showing_wrong_app(self):
280284
app_id = 'autotest'

controller/api/views.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -231,7 +231,7 @@ def run(self, request, **kwargs):
231231
content_type='text/plain')
232232

233233
def destroy(self, request, **kwargs):
234-
obj = get_object_or_404(self.model, id=kwargs['id'])
234+
obj = self.get_object()
235235
obj.delete()
236236
return Response(status=status.HTTP_204_NO_CONTENT)
237237

0 commit comments

Comments
 (0)