From 242327d339fe1193a45c64cb20a2ba4c56044c3b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A9bastien=20Piquemal?= Date: Thu, 23 Feb 2012 08:54:25 +0200 Subject: [PATCH 1/4] hack to fix ImmediateResponse rendering --- djangorestframework/response.py | 20 +++++++++++++ djangorestframework/tests/mixins.py | 2 +- djangorestframework/tests/response.py | 43 ++++++++++++++++++++++++--- 3 files changed, 60 insertions(+), 5 deletions(-) diff --git a/djangorestframework/response.py b/djangorestframework/response.py index be2c3ebe4..a352531f8 100644 --- a/djangorestframework/response.py +++ b/djangorestframework/response.py @@ -53,6 +53,14 @@ class Response(SimpleTemplateResponse): if renderers is not None: self.renderers = renderers + def render(self): + #TODO: see ImmediateResponse + try: + return super(Response, self).render() + except ImmediateResponse as response: + response.renderers = self.renderers + return response.render() + @property def rendered_content(self): """ @@ -166,6 +174,18 @@ class ImmediateResponse(Response, Exception): """ A subclass of :class:`Response` used to abort the current request handling. """ + #TODO: this is just a temporary fix, the whole rendering/support for ImmediateResponse, should be remade : see issue #163 + + def render(self): + try: + return super(Response, self).render() + except ImmediateResponse as exc: + renderer, media_type = self._determine_renderer() + self.renderers.remove(renderer) + if len(self.renderers) == 0: + raise RuntimeError('Caught an ImmediateResponse while '\ + 'trying to render an ImmediateResponse') + return self.render() def __str__(self): """ diff --git a/djangorestframework/tests/mixins.py b/djangorestframework/tests/mixins.py index 85c95d61c..25c57bd6e 100644 --- a/djangorestframework/tests/mixins.py +++ b/djangorestframework/tests/mixins.py @@ -281,6 +281,6 @@ class TestPagination(TestCase): paginated URLs. So page 1 should contain ?page=2, not ?page=1&page=2 """ request = self.req.get('/paginator/?page=1') response = MockPaginatorView.as_view()(request) - content = json.loads(response.content) + content = response.raw_content self.assertTrue('page=2' in content['next']) self.assertFalse('page=1' in content['next']) diff --git a/djangorestframework/tests/response.py b/djangorestframework/tests/response.py index 956036801..ccf6de345 100644 --- a/djangorestframework/tests/response.py +++ b/djangorestframework/tests/response.py @@ -95,21 +95,56 @@ class TestResponseDetermineRenderer(TestCase): class TestResponseRenderContent(TestCase): - def get_response(self, url='', accept_list=[], content=None): + def get_response(self, url='', accept_list=[], content=None, + renderer_classes=DEFAULT_RENDERERS): + accept_list = accept_list[0:] request = RequestFactory().get(url, HTTP_ACCEPT=','.join(accept_list)) - return Response(request=request, content=content, renderers=[r() for r in DEFAULT_RENDERERS]) + return Response(request=request, content=content, + renderers=[r() for r in renderer_classes]) def test_render(self): """ - Test rendering simple data to json. + Test rendering simple data to json. """ content = {'a': 1, 'b': [1, 2, 3]} content_type = 'application/json' response = self.get_response(accept_list=[content_type], content=content) - response.render() + response = response.render() self.assertEqual(json.loads(response.content), content) self.assertEqual(response['Content-Type'], content_type) + def test_render_no_renderer(self): + """ + Test rendering response when no renderer can satisfy accept. + """ + content = 'bla' + content_type = 'weirdcontenttype' + response = self.get_response(accept_list=[content_type], content=content) + response = response.render() + self.assertEqual(response.status_code, 406) + self.assertIsNotNone(response.content) + + def test_render_renderer_raises_ImmediateResponse(self): + """ + Test rendering response when renderer raises ImmediateResponse + """ + class PickyJSONRenderer(BaseRenderer): + """ + A renderer that doesn't make much sense, just to try + out raising an ImmediateResponse + """ + media_type = 'application/json' + def render(self, obj=None, media_type=None): + raise ImmediateResponse({'error': '!!!'}, status=400) + + response = self.get_response( + accept_list=['application/json'], + renderer_classes=[PickyJSONRenderer, JSONRenderer] + ) + response = response.render() + self.assertEqual(response.status_code, 400) + self.assertEqual(response.content, json.dumps({'error': '!!!'})) + DUMMYSTATUS = status.HTTP_200_OK DUMMYCONTENT = 'dummycontent' From afd490238a38c5445013f030547b1019f484f0bc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A9bastien=20Piquemal?= Date: Thu, 23 Feb 2012 22:47:45 +0200 Subject: [PATCH 2/4] authentication refactor : request.user + tests pass --- djangorestframework/authentication.py | 5 +- djangorestframework/mixins.py | 63 +++++++++------------ djangorestframework/request.py | 41 +++++++++++++- djangorestframework/tests/authentication.py | 2 +- djangorestframework/tests/throttling.py | 8 +-- djangorestframework/views.py | 14 ++--- 6 files changed, 79 insertions(+), 54 deletions(-) diff --git a/djangorestframework/authentication.py b/djangorestframework/authentication.py index e326c15ae..00a61e3d8 100644 --- a/djangorestframework/authentication.py +++ b/djangorestframework/authentication.py @@ -88,13 +88,14 @@ class UserLoggedInAuthentication(BaseAuthentication): Otherwise returns :const:`None`. """ request.DATA # Make sure our generic parsing runs first + user = getattr(request.request, 'user', None) - if getattr(request, 'user', None) and request.user.is_active: + if user and user.is_active: # Enforce CSRF validation for session based authentication. resp = CsrfViewMiddleware().process_view(request, None, (), {}) if resp is None: # csrf passed - return request.user + return user return None diff --git a/djangorestframework/mixins.py b/djangorestframework/mixins.py index 51c859cd2..398ed28ad 100644 --- a/djangorestframework/mixins.py +++ b/djangorestframework/mixins.py @@ -3,7 +3,6 @@ The :mod:`mixins` module provides a set of reusable `mixin` classes that can be added to a `View`. """ -from django.contrib.auth.models import AnonymousUser from django.core.paginator import Paginator from django.db.models.fields.related import ForeignKey from urlobject import URLObject @@ -19,7 +18,7 @@ __all__ = ( # Base behavior mixins 'RequestMixin', 'ResponseMixin', - 'AuthMixin', + 'PermissionsMixin', 'ResourceMixin', # Reverse URL lookup behavior 'InstanceMixin', @@ -45,6 +44,13 @@ class RequestMixin(object): Should be a tuple/list of classes as described in the :mod:`parsers` module. """ + authentication_classes = () + """ + The set of authentication types that this view can handle. + + Should be a tuple/list of classes as described in the :mod:`authentication` module. + """ + request_class = Request """ The class to use as a wrapper for the original request object. @@ -56,6 +62,12 @@ class RequestMixin(object): """ return [p(self) for p in self.parser_classes] + def get_authentications(self): + """ + Instantiates and returns the list of authentications the request will use. + """ + return [a(self) for a in self.authentication_classes] + def create_request(self, request): """ Creates and returns an instance of :class:`request.Request`. @@ -63,7 +75,9 @@ class RequestMixin(object): parsers set on the view. """ parsers = self.get_parsers() - return self.request_class(request, parsers=parsers) + authentications = self.get_authentications() + return self.request_class(request, parsers=parsers, + authentications=authentications) @property def _parsed_media_types(self): @@ -134,57 +148,32 @@ class ResponseMixin(object): return [renderer.format for renderer in self.get_renderers()] -########## Auth Mixin ########## +########## Permissions Mixin ########## -class AuthMixin(object): +class PermissionsMixin(object): """ - Simple :class:`mixin` class to add authentication and permission checking to a :class:`View` class. + Simple :class:`mixin` class to add permission checking to a :class:`View` class. """ - authentication = () - """ - The set of authentication types that this view can handle. - - Should be a tuple/list of classes as described in the :mod:`authentication` module. - """ - - permissions = () + permissions_classes = () """ The set of permissions that will be enforced on this view. Should be a tuple/list of classes as described in the :mod:`permissions` module. """ - @property - def user(self): + def get_permissions(self): """ - Returns the :obj:`user` for the current request, as determined by the set of - :class:`authentication` classes applied to the :class:`View`. + Instantiates and returns the list of permissions that this view requires. """ - if not hasattr(self, '_user'): - self._user = self._authenticate() - return self._user - - def _authenticate(self): - """ - Attempt to authenticate the request using each authentication class in turn. - Returns a ``User`` object, which may be ``AnonymousUser``. - """ - for authentication_cls in self.authentication: - authentication = authentication_cls(self) - user = authentication.authenticate(self.request) - if user: - return user - return AnonymousUser() + return [p(self) for p in self.permissions_classes] # TODO: wrap this behavior around dispatch() - def _check_permissions(self): + def check_permissions(self, user): """ Check user permissions and either raise an ``ImmediateResponse`` or return. """ - user = self.user - for permission_cls in self.permissions: - permission = permission_cls(self) + for permission in self.get_permissions(): permission.check_permission(user) diff --git a/djangorestframework/request.py b/djangorestframework/request.py index e8f2b8c3c..964231ba4 100644 --- a/djangorestframework/request.py +++ b/djangorestframework/request.py @@ -8,14 +8,15 @@ The wrapped request then offers a richer API, in particular : - full support of PUT method, including support for file uploads - form overloading of HTTP method, content type and content """ +from StringIO import StringIO + +from django.contrib.auth.models import AnonymousUser from djangorestframework.response import ImmediateResponse from djangorestframework import status from djangorestframework.utils.mediatypes import is_form_media_type from djangorestframework.utils import as_tuple -from StringIO import StringIO - __all__ = ('Request',) @@ -27,6 +28,7 @@ class Request(object): Kwargs: - request(HttpRequest). The original request instance. - parsers(list/tuple). The parsers to use for parsing the request content. + - authentications(list/tuple). The authentications used to try authenticating the request's user. """ _USE_FORM_OVERLOADING = True @@ -34,10 +36,12 @@ class Request(object): _CONTENTTYPE_PARAM = '_content_type' _CONTENT_PARAM = '_content' - def __init__(self, request=None, parsers=None): + def __init__(self, request, parsers=None, authentications=None): self.request = request if parsers is not None: self.parsers = parsers + if authentications is not None: + self.authentications = authentications @property def method(self): @@ -87,6 +91,16 @@ class Request(object): self._load_data_and_files() return self._files + @property + def user(self): + """ + Returns the :obj:`user` for the current request, authenticated + with the set of :class:`authentication` instances applied to the :class:`Request`. + """ + if not hasattr(self, '_user'): + self._user = self._authenticate() + return self._user + def _load_data_and_files(self): """ Parses the request content into self.DATA and self.FILES. @@ -192,6 +206,27 @@ class Request(object): parsers = property(_get_parsers, _set_parsers) + def _authenticate(self): + """ + Attempt to authenticate the request using each authentication instance in turn. + Returns a ``User`` object, which may be ``AnonymousUser``. + """ + for authentication in self.authentications: + user = authentication.authenticate(self) + if user: + return user + return AnonymousUser() + + def _get_authentications(self): + if hasattr(self, '_authentications'): + return self._authentications + return () + + def _set_authentications(self, value): + self._authentications = value + + authentications = property(_get_authentications, _set_authentications) + def __getattr__(self, name): """ When an attribute is not present on the calling instance, try to get it diff --git a/djangorestframework/tests/authentication.py b/djangorestframework/tests/authentication.py index 25410b040..5debc79a6 100644 --- a/djangorestframework/tests/authentication.py +++ b/djangorestframework/tests/authentication.py @@ -12,7 +12,7 @@ import base64 class MockView(View): - permissions = (permissions.IsAuthenticated,) + permissions_classes = (permissions.IsAuthenticated,) def post(self, request): return HttpResponse({'a': 1, 'b': 2, 'c': 3}) diff --git a/djangorestframework/tests/throttling.py b/djangorestframework/tests/throttling.py index 393c3ec89..73a4c02b1 100644 --- a/djangorestframework/tests/throttling.py +++ b/djangorestframework/tests/throttling.py @@ -13,17 +13,17 @@ from djangorestframework.resources import FormResource from djangorestframework.response import Response class MockView(View): - permissions = ( PerUserThrottling, ) + permissions_classes = ( PerUserThrottling, ) throttle = '3/sec' def get(self, request): return Response('foo') class MockView_PerViewThrottling(MockView): - permissions = ( PerViewThrottling, ) + permissions_classes = ( PerViewThrottling, ) class MockView_PerResourceThrottling(MockView): - permissions = ( PerResourceThrottling, ) + permissions_classes = ( PerResourceThrottling, ) resource = FormResource class MockView_MinuteThrottling(MockView): @@ -54,7 +54,7 @@ class ThrottlingTests(TestCase): """ Explicitly set the timer, overriding time.time() """ - view.permissions[0].timer = lambda self: value + view.permissions_classes[0].timer = lambda self: value def test_request_throttling_expires(self): """ diff --git a/djangorestframework/views.py b/djangorestframework/views.py index 6bfc41927..509d1471f 100644 --- a/djangorestframework/views.py +++ b/djangorestframework/views.py @@ -69,7 +69,7 @@ _resource_classes = ( ) -class View(ResourceMixin, RequestMixin, ResponseMixin, AuthMixin, DjangoView): +class View(ResourceMixin, RequestMixin, ResponseMixin, PermissionsMixin, DjangoView): """ Handles incoming requests and maps them to REST operations. Performs request deserialization, response serialization, authentication and input validation. @@ -91,13 +91,13 @@ class View(ResourceMixin, RequestMixin, ResponseMixin, AuthMixin, DjangoView): List of parser classes the resource can parse the request with. """ - authentication = (authentication.UserLoggedInAuthentication, + authentication_classes = (authentication.UserLoggedInAuthentication, authentication.BasicAuthentication) """ List of all authenticating methods to attempt. """ - permissions = (permissions.FullAnonAccess,) + permissions_classes = (permissions.FullAnonAccess,) """ List of all permissions that must be checked. """ @@ -206,15 +206,15 @@ class View(ResourceMixin, RequestMixin, ResponseMixin, AuthMixin, DjangoView): # all other authentication is CSRF exempt. @csrf_exempt def dispatch(self, request, *args, **kwargs): - self.request = self.create_request(request) + self.request = request = self.create_request(request) self.args = args self.kwargs = kwargs try: self.initial(request, *args, **kwargs) - - # Authenticate and check request has the relevant permissions - self._check_permissions() + + # check that user has the relevant permissions + self.check_permissions(request.user) # Get the appropriate handler method if request.method.lower() in self.http_method_names: From 023c008939c81ba8c33b4344b2c7756687e3be0b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A9bastien=20Piquemal?= Date: Thu, 23 Feb 2012 23:19:51 +0200 Subject: [PATCH 3/4] fixed permissions examples + sanity test --- examples/permissionsexample/tests.py | 27 +++++++++++++++++++++++++++ examples/permissionsexample/views.py | 4 ++-- examples/sandbox/views.py | 2 +- 3 files changed, 30 insertions(+), 3 deletions(-) create mode 100644 examples/permissionsexample/tests.py diff --git a/examples/permissionsexample/tests.py b/examples/permissionsexample/tests.py new file mode 100644 index 000000000..5434437af --- /dev/null +++ b/examples/permissionsexample/tests.py @@ -0,0 +1,27 @@ +from django.test import TestCase +from django.core.urlresolvers import reverse +from django.test.client import Client + + +class NaviguatePermissionsExamples(TestCase): + """ + Sanity checks for permissions examples + """ + + def test_throttled_resource(self): + url = reverse('throttled-resource') + for i in range(0, 10): + response = self.client.get(url) + self.assertEqual(response.status_code, 200) + response = self.client.get(url) + self.assertEqual(response.status_code, 503) + + + def test_loggedin_resource(self): + url = reverse('loggedin-resource') + response = self.client.get(url) + self.assertEqual(response.status_code, 403) + loggedin_client = Client() + loggedin_client.login(username='test', password='test') + response = loggedin_client.get(url) + self.assertEqual(response.status_code, 200) diff --git a/examples/permissionsexample/views.py b/examples/permissionsexample/views.py index 13384c9f5..f3dafcd47 100644 --- a/examples/permissionsexample/views.py +++ b/examples/permissionsexample/views.py @@ -30,7 +30,7 @@ class ThrottlingExampleView(View): throttle will be applied until 60 seconds have passed since the first request. """ - permissions = (PerUserThrottling,) + permissions_classes = (PerUserThrottling,) throttle = '10/min' def get(self, request): @@ -47,7 +47,7 @@ class LoggedInExampleView(View): `curl -X GET -H 'Accept: application/json' -u test:test http://localhost:8000/permissions-example` """ - permissions = (IsAuthenticated, ) + permissions_classes = (IsAuthenticated, ) def get(self, request): return Response('You have permission to view this resource') diff --git a/examples/sandbox/views.py b/examples/sandbox/views.py index a9b824475..6bc92d722 100644 --- a/examples/sandbox/views.py +++ b/examples/sandbox/views.py @@ -54,7 +54,7 @@ class Sandbox(View): 'url': reverse('model-resource-root', request)}, {'name': 'Simple Mixin-only example', 'url': reverse('mixin-view', request)}, - {'name': 'Object store API' + {'name': 'Object store API', 'url': reverse('object-store-root', request)}, {'name': 'Code highlighting API', 'url': reverse('pygments-root', request)}, From 1ff741d1ccc38f099a7159bdef787e5c04dc4f79 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A9bastien=20Piquemal?= Date: Thu, 23 Feb 2012 23:34:20 +0200 Subject: [PATCH 4/4] updated docs --- djangorestframework/authentication.py | 5 +---- djangorestframework/permissions.py | 7 ++++--- 2 files changed, 5 insertions(+), 7 deletions(-) diff --git a/djangorestframework/authentication.py b/djangorestframework/authentication.py index 00a61e3d8..904853e76 100644 --- a/djangorestframework/authentication.py +++ b/djangorestframework/authentication.py @@ -1,10 +1,7 @@ """ The :mod:`authentication` module provides a set of pluggable authentication classes. -Authentication behavior is provided by mixing the :class:`mixins.AuthMixin` class into a :class:`View` class. - -The set of authentication methods which are used is then specified by setting the -:attr:`authentication` attribute on the :class:`View` class, and listing a set of :class:`authentication` classes. +Authentication behavior is provided by mixing the :class:`mixins.RequestMixin` class into a :class:`View` class. """ from django.contrib.auth import authenticate diff --git a/djangorestframework/permissions.py b/djangorestframework/permissions.py index 335a72134..207a57b10 100644 --- a/djangorestframework/permissions.py +++ b/djangorestframework/permissions.py @@ -1,7 +1,8 @@ """ -The :mod:`permissions` module bundles a set of permission classes that are used -for checking if a request passes a certain set of constraints. You can assign a permission -class to your view by setting your View's :attr:`permissions` class attribute. +The :mod:`permissions` module bundles a set of permission classes that are used +for checking if a request passes a certain set of constraints. + +Permission behavior is provided by mixing the :class:`mixins.PermissionsMixin` class into a :class:`View` class. """ from django.core.cache import cache