Skip to content
This repository has been archived by the owner on Nov 30, 2021. It is now read-only.

Commit

Permalink
fix(controller): return 403 when a user does not have permission
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
Matthew Fisher committed Jan 21, 2015
1 parent 2359596 commit 861108a
Show file tree
Hide file tree
Showing 3 changed files with 17 additions and 6 deletions.
2 changes: 0 additions & 2 deletions controller/api/tests/test_app.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@
import mock
import os.path
import requests
import unittest

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

@unittest.expectedFailure
def test_unauthorized_user_cannot_see_app(self):
"""
An unauthorized user should not be able to access an app's resources.
Expand Down
2 changes: 0 additions & 2 deletions controller/api/tests/test_container.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@
import json
import mock
import requests
import unittest

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

@unittest.expectedFailure
def test_scale_with_unauthorized_user_returns_403(self):
"""An unauthorized user should not be able to access an app's resources.
Expand Down
19 changes: 17 additions & 2 deletions controller/api/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -107,9 +107,24 @@ class AppViewSet(BaseDeisViewSet):
model = models.App
serializer_class = serializers.AppSerializer

def get_queryset(self, **kwargs):
return super(AppViewSet, self).get_queryset(**kwargs) | \
def get_queryset(self, *args, **kwargs):
return self.model.objects.all(*args, **kwargs)

def list(self, request, *args, **kwargs):
"""
HACK: Instead of filtering by the queryset, we limit the queryset to list only the apps
which are owned by the user as well as any apps they have been given permission to
interact with.
"""
queryset = super(AppViewSet, self).get_queryset(**kwargs) | \
get_objects_for_user(self.request.user, 'api.use_app')
instance = self.filter_queryset(queryset)
page = self.paginate_queryset(instance)
if page is not None:
serializer = self.get_pagination_serializer(page)
else:
serializer = self.get_serializer(instance, many=True)
return Response(serializer.data)

def post_save(self, app):
app.create()
Expand Down

0 comments on commit 861108a

Please sign in to comment.