Skip to content

Commit 861108a

Browse files
author
Matthew Fisher
committed
fix(controller): return 403 when a user does not have permission
In our AppViewSet, we set .get_queryset() to give us only the applications that we have either created (i.e. we are the owner) or ones which we were given permission to use (use_app from django-guardian). This is such that when we list apps via /v1/apps, it will only show us that filtered viewset. However, that also limits the scope on what applications we can act upon. Since .get_queryset() returns a queryset of applications that we only know about, it returns a 404 when we try to "ping" an application we were not given access to. To fix this, I have modified `.list()` to display the limited queryset of applications which ther user is the owner or has been given permission to use, and changing the queryset to all applications such that responses from applications which we do not have access to will return a 403 FORBIDDEN.
1 parent 2359596 commit 861108a

3 files changed

Lines changed: 17 additions & 6 deletions

File tree

controller/api/tests/test_app.py

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,6 @@
1010
import mock
1111
import os.path
1212
import requests
13-
import unittest
1413

1514
from django.conf import settings
1615
from django.contrib.auth.models import User
@@ -268,7 +267,6 @@ def test_run_without_release_should_error(self):
268267
self.assertEqual(response.data, "No build associated with this release "
269268
"to run this command")
270269

271-
@unittest.expectedFailure
272270
def test_unauthorized_user_cannot_see_app(self):
273271
"""
274272
An unauthorized user should not be able to access an app's resources.

controller/api/tests/test_container.py

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,6 @@
99
import json
1010
import mock
1111
import requests
12-
import unittest
1312

1413
from django.contrib.auth.models import User
1514
from django.test import TransactionTestCase
@@ -541,7 +540,6 @@ def test_run_command_good(self):
541540
rc, output = c.run('echo hi')
542541
self.assertEqual(json.loads(output)['entrypoint'], '/runner/init')
543542

544-
@unittest.expectedFailure
545543
def test_scale_with_unauthorized_user_returns_403(self):
546544
"""An unauthorized user should not be able to access an app's resources.
547545

controller/api/views.py

Lines changed: 17 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -107,9 +107,24 @@ class AppViewSet(BaseDeisViewSet):
107107
model = models.App
108108
serializer_class = serializers.AppSerializer
109109

110-
def get_queryset(self, **kwargs):
111-
return super(AppViewSet, self).get_queryset(**kwargs) | \
110+
def get_queryset(self, *args, **kwargs):
111+
return self.model.objects.all(*args, **kwargs)
112+
113+
def list(self, request, *args, **kwargs):
114+
"""
115+
HACK: Instead of filtering by the queryset, we limit the queryset to list only the apps
116+
which are owned by the user as well as any apps they have been given permission to
117+
interact with.
118+
"""
119+
queryset = super(AppViewSet, self).get_queryset(**kwargs) | \
112120
get_objects_for_user(self.request.user, 'api.use_app')
121+
instance = self.filter_queryset(queryset)
122+
page = self.paginate_queryset(instance)
123+
if page is not None:
124+
serializer = self.get_pagination_serializer(page)
125+
else:
126+
serializer = self.get_serializer(instance, many=True)
127+
return Response(serializer.data)
113128

114129
def post_save(self, app):
115130
app.create()

0 commit comments

Comments
 (0)