diff --git a/djangorestframework/authentication.py b/djangorestframework/authentication.py index cb95fb810..3ddd9e45f 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 @@ -23,12 +20,6 @@ class BaseAuthentication(object): All authentication classes should extend BaseAuthentication. """ - def __init__(self, view): - """ - :class:`Authentication` classes are always passed the current view on creation. - """ - self.view = view - def authenticate(self, request): """ Authenticate the :obj:`request` and return a :obj:`User` or :const:`None`. [*]_ @@ -87,12 +78,14 @@ class UserLoggedInAuthentication(BaseAuthentication): Returns a :obj:`User` if the request session currently has a logged in user. Otherwise returns :const:`None`. """ - if getattr(request, 'user', None) and request.user.is_active: + user = getattr(request._request, 'user', None) + + 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 f95ec60ff..3142b093b 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', # Model behavior mixins 'ReadModelMixin', @@ -49,7 +48,7 @@ class RequestMixin(object): This new instance wraps the `request` passed as a parameter, and use the parsers set on the view. """ - return self.request_class(request, parsers=self.parsers) + return self.request_class(request, parsers=self.parsers, authentication=self.authentication) @property def _parsed_media_types(self): @@ -101,57 +100,32 @@ class ResponseMixin(object): return self.renderers[0] -########## 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/permissions.py b/djangorestframework/permissions.py index 335a72134..4a2b1b00d 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 @@ -126,7 +127,7 @@ class DjangoModelPermissions(BasePermission): try: return [perm % kwargs for perm in self.perms_map[method]] except KeyError: - ErrorResponse(status.HTTP_405_METHOD_NOT_ALLOWED) + ImmediateResponse(status.HTTP_405_METHOD_NOT_ALLOWED) def check_permission(self, user): method = self.view.method diff --git a/djangorestframework/request.py b/djangorestframework/request.py index a6c23fb8e..82aed1e0c 100644 --- a/djangorestframework/request.py +++ b/djangorestframework/request.py @@ -9,12 +9,13 @@ 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 import status from djangorestframework.utils.mediatypes import is_form_media_type -from StringIO import StringIO - __all__ = ('Request',) @@ -34,6 +35,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 @@ -41,9 +43,10 @@ class Request(object): _CONTENTTYPE_PARAM = '_content_type' _CONTENT_PARAM = '_content' - def __init__(self, request=None, parsers=None): + def __init__(self, request=None, parsers=None, authentication=None): self._request = request self.parsers = parsers or () + self.authentication = authentication or () self._data = Empty self._files = Empty self._method = Empty @@ -56,6 +59,12 @@ class Request(object): """ return [parser() for parser in self.parsers] + def get_authentications(self): + """ + Instantiates and returns the list of parsers the request will use. + """ + return [authentication() for authentication in self.authentication] + @property def method(self): """ @@ -113,6 +122,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. @@ -205,6 +224,17 @@ class Request(object): }, status=status.HTTP_415_UNSUPPORTED_MEDIA_TYPE) + 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.get_authentications(): + user = authentication.authenticate(self) + if user: + return user + return AnonymousUser() + def __getattr__(self, name): """ Proxy other attributes to the underlying HttpRequest object. diff --git a/djangorestframework/response.py b/djangorestframework/response.py index bedeb6c5f..ea9a938c6 100644 --- a/djangorestframework/response.py +++ b/djangorestframework/response.py @@ -168,6 +168,18 @@ class ImmediateResponse(Response, Exception): An exception representing an Response that should be returned immediately. Any content should be serialized as-is, without being filtered. """ + #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: + 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 __init__(self, *args, **kwargs): self.response = Response(*args, **kwargs) 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/mixins.py b/djangorestframework/tests/mixins.py index bf0f29f75..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.rendered_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 fd83da293..07d0f4fbf 100644 --- a/djangorestframework/tests/response.py +++ b/djangorestframework/tests/response.py @@ -4,7 +4,7 @@ import unittest from django.conf.urls.defaults import patterns, url, include from django.test import TestCase -from djangorestframework.response import Response, NotAcceptable +from djangorestframework.response import Response, NotAcceptable, ImmediateResponse from djangorestframework.views import View from djangorestframework.compat import RequestFactory from djangorestframework import status @@ -95,10 +95,9 @@ class TestResponseDetermineRenderer(TestCase): class TestResponseRenderContent(TestCase): - - def get_response(self, url='', accept_list=[], content=None): + def get_response(self, url='', accept_list=[], content=None, renderers=None): request = RequestFactory().get(url, HTTP_ACCEPT=','.join(accept_list)) - return Response(request=request, content=content, renderers=DEFAULT_RENDERERS) + return Response(request=request, content=content, renderers=renderers or DEFAULT_RENDERERS) def test_render(self): """ @@ -107,10 +106,43 @@ class TestResponseRenderContent(TestCase): 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'], + # renderers=[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' 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 46223a3ff..8fe5f99a6 100644 --- a/djangorestframework/views.py +++ b/djangorestframework/views.py @@ -67,7 +67,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. @@ -90,7 +90,7 @@ class View(ResourceMixin, RequestMixin, ResponseMixin, AuthMixin, DjangoView): """ authentication = (authentication.UserLoggedInAuthentication, - authentication.BasicAuthentication) + authentication.BasicAuthentication) """ List of all authenticating methods to attempt. """ @@ -215,6 +215,7 @@ class View(ResourceMixin, RequestMixin, ResponseMixin, AuthMixin, DjangoView): def dispatch(self, request, *args, **kwargs): request = self.create_request(request) self.request = request + self.args = args self.kwargs = kwargs self.headers = self.default_response_headers @@ -222,8 +223,8 @@ class View(ResourceMixin, RequestMixin, ResponseMixin, AuthMixin, DjangoView): 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: 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')