Generic permissions added, allowed_methods and anon_allowed_methods now defunct, dispatch now mirrors View.dispatch more nicely

8 files changed, 98 insertions, 130 deletions

diff --git a/djangorestframework/authenticators.py b/djangorestframework/authenticators.py
index e8331cc7..e6f51dd5 100644
--- a/djangorestframework/authenticators.py
+++ b/djangorestframework/authenticators.py
@@ -13,10 +13,10 @@ import base64
 class BaseAuthenticator(object):
     """All authenticators should extend BaseAuthenticator."""
 
-    def __init__(self, mixin):
+    def __init__(self, view):
         """Initialise the authenticator with the mixin instance as state,
         in case the authenticator needs to access any metadata on the mixin object."""
-        self.mixin = mixin
+        self.view = view
 
     def authenticate(self, request):
         """Authenticate the request and return the authentication context or None.
@@ -61,11 +61,13 @@ class BasicAuthenticator(BaseAuthenticator):
                 
 
 class UserLoggedInAuthenticator(BaseAuthenticator):
-    """Use Djagno's built-in request session for authentication."""
+    """Use Django's built-in request session for authentication."""
     def authenticate(self, request):
         if getattr(request, 'user', None) and request.user.is_active:
-            # Temporarily request.POST with .RAW_CONTENT, so that we use our more generic request parsing
-            request._post = self.mixin.RAW_CONTENT
+            # Temporarily set request.POST to view.RAW_CONTENT,
+            # so that we use our more generic request parsing,
+            # in preference to Django's form-only request parsing.
+            request._post = self.view.RAW_CONTENT
             resp = CsrfViewMiddleware().process_view(request, None, (), {})
             del(request._post)
             if resp is None:  # csrf passed
diff --git a/djangorestframework/mixins.py b/djangorestframework/mixins.py
index 9af79c66..53262366 100644
--- a/djangorestframework/mixins.py
+++ b/djangorestframework/mixins.py
@@ -396,9 +396,9 @@ class ResponseMixin(object):
 ########## Auth Mixin ##########
 
 class AuthMixin(object):
-    """Mixin class to provide authentication and permissions."""
+    """Mixin class to provide authentication and permission checking."""
     authenticators = ()
-    permitters = ()
+    permissions = ()
 
     @property
     def auth(self):
@@ -406,6 +406,14 @@ class AuthMixin(object):
             self._auth = self._authenticate()
         return self._auth
 
+    def _authenticate(self):
+        for authenticator_cls in self.authenticators:
+            authenticator = authenticator_cls(self)
+            auth = authenticator.authenticate(self.request)
+            if auth:
+                return auth
+        return None
+
     # TODO?
     #@property
     #def user(self):
@@ -421,15 +429,11 @@ class AuthMixin(object):
         if not self.permissions:
             return
 
-        auth = self.auth
-        for permitter_cls in self.permitters:
-            permitter = permission_cls(self)
-            permitter.permit(auth)
+        for permission_cls in self.permissions:
+            permission = permission_cls(self)
+            if not permission.has_permission(self.auth):
+                raise ErrorResponse(status.HTTP_403_FORBIDDEN,
+                                   {'detail': 'You do not have permission to access this resource. ' +
+                                    'You may need to login or otherwise authenticate the request.'})                
+
 
-    def _authenticate(self):
-        for authenticator_cls in self.authenticators:
-            authenticator = authenticator_cls(self)
-            auth = authenticator.authenticate(self.request)
-            if auth:
-                return auth
-        return None
diff --git a/djangorestframework/modelresource.py b/djangorestframework/modelresource.py
index 23a87e65..24fb62ab 100644
--- a/djangorestframework/modelresource.py
+++ b/djangorestframework/modelresource.py
@@ -410,13 +410,13 @@ class ModelResource(Resource):
 
 class RootModelResource(ModelResource):
     """A Resource which provides default operations for list and create."""
-    allowed_methods = ('GET', 'POST')
     queryset = None
 
     def get(self, request, *args, **kwargs):
         queryset = self.queryset if self.queryset else self.model.objects.all()
         return queryset.filter(**kwargs)
 
+    put = delete = http_method_not_allowed
 
 class QueryModelResource(ModelResource):
     """Resource with default operations for list.
@@ -424,10 +424,8 @@ class QueryModelResource(ModelResource):
     allowed_methods = ('GET',)
     queryset = None
 
-    def get_form(self, data=None):
-        return None
-
     def get(self, request, *args, **kwargs):
         queryset = self.queryset if self.queryset else self.model.objects.all()
         return queryset.filer(**kwargs)
 
+    post = put = delete = http_method_not_allowed
\ No newline at end of file
diff --git a/djangorestframework/resource.py b/djangorestframework/resource.py
index 1e79c79f..55a9b57d 100644
--- a/djangorestframework/resource.py
+++ b/djangorestframework/resource.py
@@ -4,7 +4,7 @@ from django.views.decorators.csrf import csrf_exempt
 from djangorestframework.compat import View
 from djangorestframework.response import Response, ErrorResponse
 from djangorestframework.mixins import RequestMixin, ResponseMixin, AuthMixin
-from djangorestframework import emitters, parsers, authenticators, validators, status
+from djangorestframework import emitters, parsers, authenticators, permissions, validators, status
 
 
 # TODO: Figure how out references and named urls need to work nicely
@@ -19,11 +19,7 @@ class Resource(RequestMixin, ResponseMixin, AuthMixin, View):
     """Handles incoming requests and maps them to REST operations,
     performing authentication, input deserialization, input validation, output serialization."""
 
-    # List of RESTful operations which may be performed on this resource.
-    # These are going to get dropped at some point, the allowable methods will be defined simply by
-    # which methods are present on the request (in the same way as Django's generic View)
-    allowed_methods = ('GET',)
-    anon_allowed_methods = ()
+    http_method_names = ['get', 'post', 'put', 'delete', 'head', 'options', 'trace', 'patch']
 
     # List of emitters the resource can serialize the response with, ordered by preference.
     emitters = ( emitters.JSONEmitter,
@@ -37,12 +33,15 @@ class Resource(RequestMixin, ResponseMixin, AuthMixin, View):
                 parsers.FormParser,
                 parsers.MultipartParser )
 
-    # List of validators to validate, cleanup and type-ify the request content    
+    # List of validators to validate, cleanup and normalize the request content    
     validators = ( validators.FormValidator, )
 
     # List of all authenticating methods to attempt.
     authenticators = ( authenticators.UserLoggedInAuthenticator,
                        authenticators.BasicAuthenticator )
+    
+    # List of all permissions required to access the resource
+    permissions = ( permissions.DeleteMePermission, )
 
     # Optional form for input validation and presentation of HTML formatted responses.
     form = None
@@ -53,52 +52,14 @@ class Resource(RequestMixin, ResponseMixin, AuthMixin, View):
     name = None
     description = None
 
-    # Map standard HTTP methods to function calls
-    callmap = { 'GET': 'get', 'POST': 'post', 
-                'PUT': 'put', 'DELETE': 'delete' }
-
-    def get(self, request, *args, **kwargs):
-        """Must be subclassed to be implemented."""
-        self.not_implemented('GET')
-
-
-    def post(self, request, *args, **kwargs):
-        """Must be subclassed to be implemented."""
-        self.not_implemented('POST')
-
-
-    def put(self, request, *args, **kwargs):
-        """Must be subclassed to be implemented."""
-        self.not_implemented('PUT')
-
-
-    def delete(self, request, *args, **kwargs):
-        """Must be subclassed to be implemented."""
-        self.not_implemented('DELETE')
+    @property
+    def allowed_methods(self):
+        return [method.upper() for method in self.http_method_names if hasattr(self, method)]
 
-
-    def not_implemented(self, operation):
-        """Return an HTTP 500 server error if an operation is called which has been allowed by
-        allowed_methods, but which has not been implemented."""
-        raise ErrorResponse(status.HTTP_500_INTERNAL_SERVER_ERROR,
-                                {'detail': '%s operation on this resource has not been implemented' % (operation, )})
-
-
-    def check_method_allowed(self, method, auth):
-        """Ensure the request method is permitted for this resource, raising a ResourceException if it is not."""
-
-        if not method in self.callmap.keys():
-            raise ErrorResponse(status.HTTP_501_NOT_IMPLEMENTED,
-                                    {'detail': 'Unknown or unsupported method \'%s\'' % method})
-
-        if not method in self.allowed_methods:
-            raise ErrorResponse(status.HTTP_405_METHOD_NOT_ALLOWED,
-                                    {'detail': 'Method \'%s\' not allowed on this resource.' % method})
-
-        if auth is None and not method in self.anon_allowed_methods:
-            raise ErrorResponse(status.HTTP_403_FORBIDDEN,
-                                    {'detail': 'You do not have permission to access this resource. ' +
-                                     'You may need to login or otherwise authenticate the request.'})
+    def http_method_not_allowed(self, request, *args, **kwargs):
+        """Return an HTTP 405 error if an operation is called which does not have a handler method."""
+        raise ErrorResponse(status.HTTP_405_METHOD_NOT_ALLOWED,
+                                {'detail': 'Method \'%s\' not allowed on this resource.' % self.method})
 
 
     def cleanup_response(self, data):
@@ -111,6 +72,7 @@ class Resource(RequestMixin, ResponseMixin, AuthMixin, View):
         the EmitterMixin and Emitter classes."""
         return data
 
+
     # Session based authentication is explicitly CSRF validated, all other authentication is CSRF exempt.
     @csrf_exempt
     def dispatch(self, request, *args, **kwargs):
@@ -125,57 +87,54 @@ class Resource(RequestMixin, ResponseMixin, AuthMixin, View):
         4. cleanup the response data
         5. serialize response data into response content, using standard HTTP content negotiation
         """
-
-        self.request = request
-
-        # Calls to 'reverse' will not be fully qualified unless we set the scheme/host/port here.
-        prefix = '%s://%s' % (request.is_secure() and 'https' or 'http', request.get_host())
-        set_script_prefix(prefix)
-
         try:
-            # Authenticate the request, and store any context so that the resource operations can
-            # do more fine grained authentication if required.
+            self.request = request
+            self.args = args
+            self.kwargs = kwargs
+    
+            # Calls to 'reverse' will not be fully qualified unless we set the scheme/host/port here.
+            prefix = '%s://%s' % (request.is_secure() and 'https' or 'http', request.get_host())
+            set_script_prefix(prefix)
+    
+            try:
+                # If using a form POST with '_method'/'_content'/'_content_type' overrides, then alter
+                # self.method, self.content_type, self.RAW_CONTENT & self.CONTENT appropriately.
+                self.perform_form_overloading()
+    
+                # Authenticate and check request is has the relevant permissions
+                self.check_permissions()
+    
+                # Get the appropriate handler method
+                if self.method.lower() in self.http_method_names:
+                    handler = getattr(self, self.method.lower(), self.http_method_not_allowed)
+                else:
+                    handler = self.http_method_not_allowed
+    
+                response_obj = handler(request, *args, **kwargs)
+    
+                # Allow return value to be either Response, or an object, or None
+                if isinstance(response_obj, Response):
+                    response = response_obj
+                elif response_obj is not None:
+                    response = Response(status.HTTP_200_OK, response_obj)
+                else:
+                    response = Response(status.HTTP_204_NO_CONTENT)
+    
+                # Pre-serialize filtering (eg filter complex objects into natively serializable types)
+                response.cleaned_content = self.cleanup_response(response.raw_content)
+    
+            except ErrorResponse, exc:
+                response = exc.response
+    
+            # Always add these headers.
             #
-            # Typically the context will be a user, or None if this is an anonymous request,
-            # but it could potentially be more complex (eg the context of a request key which
-            # has been signed against a particular set of permissions)
-            auth_context = self.auth
-
-            # If using a form POST with '_method'/'_content'/'_content_type' overrides, then alter
-            # self.method, self.content_type, self.CONTENT appropriately.
-            self.perform_form_overloading()
-
-            # Ensure the requested operation is permitted on this resource
-            self.check_method_allowed(self.method, auth_context)
-
-            # Get the appropriate create/read/update/delete function
-            func = getattr(self, self.callmap.get(self.method, None))
+            # TODO - this isn't actually the correct way to set the vary header,
+            # also it's currently sub-obtimal for HTTP caching - need to sort that out. 
+            response.headers['Allow'] = ', '.join(self.allowed_methods)
+            response.headers['Vary'] = 'Authenticate, Accept'
     
-            # Either generate the response data, deserializing and validating any request data
-            # TODO: This is going to change to: func(request, *args, **kwargs)
-            # That'll work out now that we have the lazily evaluated self.CONTENT property.
-            response_obj = func(request, *args, **kwargs)
-
-            # Allow return value to be either Response, or an object, or None
-            if isinstance(response_obj, Response):
-                response = response_obj
-            elif response_obj is not None:
-                response = Response(status.HTTP_200_OK, response_obj)
-            else:
-                response = Response(status.HTTP_204_NO_CONTENT)
-
-            # Pre-serialize filtering (eg filter complex objects into natively serializable types)
-            response.cleaned_content = self.cleanup_response(response.raw_content)
-
-        except ErrorResponse, exc:
-            response = exc.response
-
-        # Always add these headers.
-        #
-        # TODO - this isn't actually the correct way to set the vary header,
-        # also it's currently sub-obtimal for HTTP caching - need to sort that out. 
-        response.headers['Allow'] = ', '.join(self.allowed_methods)
-        response.headers['Vary'] = 'Authenticate, Accept'
-
-        return self.emit(response)
+            return self.emit(response)
+        except:
+            import traceback
+            traceback.print_exc()
 
diff --git a/djangorestframework/tests/accept.py b/djangorestframework/tests/accept.py
index 726e1252..b12dc757 100644
--- a/djangorestframework/tests/accept.py
+++ b/djangorestframework/tests/accept.py
@@ -18,10 +18,13 @@ class UserAgentMungingTest(TestCase):
     http://www.gethifi.com/blog/browser-rest-http-accept-headers"""
 
     def setUp(self):
+
         class MockResource(Resource):
-            anon_allowed_methods = allowed_methods = ('GET',)
+            permissions = ()
+
             def get(self, request):
                 return {'a':1, 'b':2, 'c':3}
+
         self.req = RequestFactory()
         self.MockResource = MockResource
         self.view = MockResource.as_view()
diff --git a/djangorestframework/tests/authentication.py b/djangorestframework/tests/authentication.py
index b2bc4446..c825883d 100644
--- a/djangorestframework/tests/authentication.py
+++ b/djangorestframework/tests/authentication.py
@@ -13,8 +13,6 @@ except ImportError:
     import simplejson as json
 
 class MockResource(Resource):
-    allowed_methods = ('POST',)
-
     def post(self, request):
         return {'a':1, 'b':2, 'c':3}
 
diff --git a/djangorestframework/tests/files.py b/djangorestframework/tests/files.py
index dd4689a6..4dc3aa40 100644
--- a/djangorestframework/tests/files.py
+++ b/djangorestframework/tests/files.py
@@ -16,7 +16,7 @@ class UploadFilesTests(TestCase):
             file = forms.FileField
 
         class MockResource(Resource):
-            allowed_methods = anon_allowed_methods = ('POST',)
+            permissions = ()
             form = FileForm
 
             def post(self, request, *args, **kwargs):
diff --git a/djangorestframework/tests/reverse.py b/djangorestframework/tests/reverse.py
index f6a3ea51..1f9071b3 100644
--- a/djangorestframework/tests/reverse.py
+++ b/djangorestframework/tests/reverse.py
@@ -12,7 +12,7 @@ except ImportError:
 
 class MockResource(Resource):
     """Mock resource which simply returns a URL, so that we can ensure that reversed URLs are fully qualified"""
-    anon_allowed_methods = ('GET',)
+    permissions = ()
 
     def get(self, request):
         return reverse('another')
@@ -28,5 +28,9 @@ class ReverseTests(TestCase):
     urls = 'djangorestframework.tests.reverse'
 
     def test_reversed_urls_are_fully_qualified(self):
-        response = self.client.get('/')
+        try:
+            response = self.client.get('/')
+        except:
+            import traceback
+            traceback.print_exc()
         self.assertEqual(json.loads(response.content), 'http://testserver/another')