Merge pull request #2853 from bacongobbler/2845-unauthorized-api-usage

fix(controller): disallow unauthorized users from deleting apps

Author: Matthew Fisher

controller/api/permissions.py

@@ -36,6 +36,8 @@ class IsAppUser(permissions.BasePermission):
     an app-related model.
     """
     def has_object_permission(self, request, view, obj):
+        if request.user.is_superuser:
+            return True
         if isinstance(obj, models.App) and obj.owner == request.user:
             return True
         elif hasattr(obj, 'app') and obj.app.owner == request.user:

controller/api/tests/test_app.py

@@ -275,6 +275,10 @@ def test_unauthorized_user_cannot_see_app(self):
         url = '{}/{}/logs'.format(base_url, app_id)
         response = self.client.get(url, HTTP_AUTHORIZATION='token {}'.format(unauthorized_token))
         self.assertEqual(response.status_code, 404)
+        url = '{}/{}'.format(base_url, app_id)
+        response = self.client.delete(url,
+                                      HTTP_AUTHORIZATION='token {}'.format(unauthorized_token))
+        self.assertEqual(response.status_code, 404)
 
     def test_app_info_not_showing_wrong_app(self):
         app_id = 'autotest'

controller/api/tests/test_release.py

@@ -254,19 +254,25 @@ def test_unauthorized_user_cannot_modify_release(self):
         requests should return a 404.
         """
         app_id = 'autotest'
-        url = '/v1/apps'
+        base_url = '/v1/apps'
         body = {'id': app_id}
-        response = self.client.post(url, json.dumps(body), content_type='application/json',
+        response = self.client.post(base_url, json.dumps(body), content_type='application/json',
                                     HTTP_AUTHORIZATION='token {}'.format(self.token))
+        # push a new build
+        url = '{base_url}/{app_id}/builds'.format(**locals())
+        body = {'image': 'test'}
+        response = self.client.post(
+            url, json.dumps(body), content_type='application/json',
+            HTTP_AUTHORIZATION='token {}'.format(self.token))
         # update config to roll a new release
-        url = '/v1/apps/{app_id}/config'.format(**locals())
+        url = '{base_url}/{app_id}/config'.format(**locals())
         body = {'values': json.dumps({'NEW_URL1': 'http://localhost:8080/'})}
         response = self.client.post(
             url, json.dumps(body), content_type='application/json',
             HTTP_AUTHORIZATION='token {}'.format(self.token))
         unauthorized_user = User.objects.get(username='autotest2')
         unauthorized_token = Token.objects.get(user=unauthorized_user).key
         # try to rollback
-        url = '{}/{}/releases/rollback'.format(url, app_id)
+        url = '{base_url}/{app_id}/releases/rollback/'.format(**locals())
         response = self.client.post(url, HTTP_AUTHORIZATION='token {}'.format(unauthorized_token))
-        self.assertEqual(response.status_code, 404)
+        self.assertEqual(response.status_code, 403)

controller/api/views.py

@@ -231,7 +231,7 @@ def run(self, request, **kwargs):
                         content_type='text/plain')
 
     def destroy(self, request, **kwargs):
-        obj = get_object_or_404(self.model, id=kwargs['id'])
+        obj = self.get_object()
         obj.delete()
         return Response(status=status.HTTP_204_NO_CONTENT)
 
@@ -368,6 +368,7 @@ def rollback(self, request, *args, **kwargs):
         """
         try:
             app = get_object_or_404(models.App, id=self.kwargs['id'])
+            self.check_object_permissions(self.request, app)
             release = app.release_set.latest()
             version_to_rollback_to = release.version - 1
             if request.DATA.get('version'):