New style object-level permission checks

8 files changed, 78 insertions, 22 deletions

diff --git a/docs/api-guide/permissions.md b/docs/api-guide/permissions.md
index 1814b811..5cb3ec3c 100644
--- a/docs/api-guide/permissions.md
+++ b/docs/api-guide/permissions.md
@@ -106,22 +106,55 @@ The `DjangoModelPermissions` class also supports object-level permissions.  Thir
 
 # Custom permissions
 
-To implement a custom permission, override `BasePermission` and implement the `.has_permission(self, request, view, obj=None)` method.
+To implement a custom permission, override `BasePermission` and implement either, or both, of the `.has_permission(self, request, view)` and `.has_object_permission(self, request, view, obj)` methods.
 
-The method should return `True` if the request should be granted access, and `False` otherwise.
+The methods should return `True` if the request should be granted access, and `False` otherwise.
 
-## Example
+---
+
+**Note**: In versions 2.0 and 2.1, the signature for the permission checks always included an optional `obj` parameter, like so: `.has_permission(self, request, view, obj=None)`.  The method would be called twice, first for the global permission checks, with no object supplied, and second for the object-level check when required.
+
+As of version 2.2 this signature has now been replaced with two seperate method calls, which is more explict, and obvious.  The old style signature continues to work, but it's use will result in a `PendingDeprecationWarning`, which is silent by default.  In 2.3 this will be escalated to a `DeprecationWarning`, and in 2.4 the old-style signature will be removed.
+
+For more details see the [2.2 release announcement][2.2-announcement].
+
+---
+
+## Examples
 
 The following is an example of a permission class that checks the incoming request's IP address against a blacklist, and denies the request if the IP has been blacklisted.
 
     class BlacklistPermission(permissions.BasePermission):
+        """
+        Global permission check for blacklisted IPs.
+        """
+
         def has_permission(self, request, view, obj=None):
             ip_addr = request.META['REMOTE_ADDR']
             blacklisted = Blacklist.objects.filter(ip_addr=ip_addr).exists()
             return not blacklisted
 
+As well as global permissions, that are run against all incoming requests, you can also create object-level permissions, that are only run against operations that affect a particular object instance.  For example:
+
+    class IsOwnerOrReadOnly(permissions.BasePermission):
+        """
+        Object-level permission to only allow owners of an object to edit it.
+        """
+
+        def has_object_permission(self, request, view, obj):
+            # Read permissions are allowed to any request,
+            # so we'll always allow GET, HEAD or OPTIONS requests.
+            if request.method in permissions.SAFE_METHODS:            
+                return True
+    
+            # Instance must have an attribute named `owner`.
+            return obj.owner == request.user
+
+Note that the generic views will check the appropriate object level permissions, but if you're writing your own custom views, you'll need to make sure you check the object level permission checks yourself, by calling `self.has_object_permission(request, obj)` from the view.
+
 [cite]: https://developer.apple.com/library/mac/#documentation/security/Conceptual/AuthenticationAndAuthorizationGuide/Authorization/Authorization.html
 [authentication]: authentication.md
 [throttling]: throttling.md
 [contribauth]: https://docs.djangoproject.com/en/1.0/topics/auth/#permissions
 [guardian]: https://github.com/lukaszb/django-guardian
+[2.2-announcement]: ../topics/2.2-announcement.md
diff --git a/docs/tutorial/4-authentication-and-permissions.md b/docs/tutorial/4-authentication-and-permissions.md
index e9e5246a..979421ea 100644
--- a/docs/tutorial/4-authentication-and-permissions.md
+++ b/docs/tutorial/4-authentication-and-permissions.md
@@ -161,11 +161,7 @@ In the snippets app, create a new file, `permissions.py`
         Custom permission to only allow owners of an object to edit it.
         """
 
-        def has_permission(self, request, view, obj=None):
-            # Skip the check unless this is an object-level test
-            if obj is None:
-                return True
-    
+        def has_object_permission(self, request, view, obj):
             # Read permissions are allowed to any request,
             # so we'll always allow GET, HEAD or OPTIONS requests.
             if request.method in permissions.SAFE_METHODS:            
diff --git a/rest_framework/generics.py b/rest_framework/generics.py
index 5abb915b..19dca7e6 100644
--- a/rest_framework/generics.py
+++ b/rest_framework/generics.py
@@ -131,7 +131,7 @@ class SingleObjectAPIView(SingleObjectMixin, GenericAPIView):
         Override default to add support for object-level permissions.
         """
         obj = super(SingleObjectAPIView, self).get_object(queryset)
-        if not self.has_permission(self.request, obj):
+        if not self.has_object_permission(self.request, obj):
             self.permission_denied(self.request)
         return obj
 
diff --git a/rest_framework/permissions.py b/rest_framework/permissions.py
index c9bbf4c4..306f00ca 100644
--- a/rest_framework/permissions.py
+++ b/rest_framework/permissions.py
@@ -2,6 +2,8 @@
 Provides a set of pluggable permission policies.
 """
 from __future__ import unicode_literals
+import inspect
+import warnings
 
 SAFE_METHODS = ['GET', 'HEAD', 'OPTIONS']
 
@@ -11,11 +13,22 @@ class BasePermission(object):
     A base class from which all permission classes should inherit.
     """
 
-    def has_permission(self, request, view, obj=None):
+    def has_permission(self, request, view):
         """
         Return `True` if permission is granted, `False` otherwise.
         """
-        raise NotImplementedError(".has_permission() must be overridden.")
+        return True
+
+    def has_object_permission(self, request, view, obj):
+        """
+        Return `True` if permission is granted, `False` otherwise.
+        """
+        if len(inspect.getargspec(self.has_permission)[0]) == 4:
+            warnings.warn('The `obj` argument in `has_permission` is due to be deprecated. '
+                      'Use `has_object_permission()` instead for object permissions.',
+                       PendingDeprecationWarning, stacklevel=2)
+            return self.has_permission(request, view, obj)
+        return True
 
 
 class AllowAny(BasePermission):
@@ -25,7 +38,7 @@ class AllowAny(BasePermission):
     permission_classes list, but it's useful because it makes the intention
     more explicit.
     """
-    def has_permission(self, request, view, obj=None):
+    def has_permission(self, request, view):
         return True
 
 
@@ -34,7 +47,7 @@ class IsAuthenticated(BasePermission):
     Allows access only to authenticated users.
     """
 
-    def has_permission(self, request, view, obj=None):
+    def has_permission(self, request, view):
         if request.user and request.user.is_authenticated():
             return True
         return False
@@ -45,7 +58,7 @@ class IsAdminUser(BasePermission):
     Allows access only to admin users.
     """
 
-    def has_permission(self, request, view, obj=None):
+    def has_permission(self, request, view):
         if request.user and request.user.is_staff:
             return True
         return False
@@ -56,7 +69,7 @@ class IsAuthenticatedOrReadOnly(BasePermission):
     The request is authenticated as a user, or is a read-only request.
     """
 
-    def has_permission(self, request, view, obj=None):
+    def has_permission(self, request, view):
         if (request.method in SAFE_METHODS or
             request.user and
             request.user.is_authenticated()):
@@ -100,7 +113,7 @@ class DjangoModelPermissions(BasePermission):
         }
         return [perm % kwargs for perm in self.perms_map[method]]
 
-    def has_permission(self, request, view, obj=None):
+    def has_permission(self, request, view):
         model_cls = getattr(view, 'model', None)
         if not model_cls:
             return True
diff --git a/rest_framework/renderers.py b/rest_framework/renderers.py
index 960d4849..e7df8758 100644
--- a/rest_framework/renderers.py
+++ b/rest_framework/renderers.py
@@ -301,7 +301,7 @@ class BrowsableAPIRenderer(BaseRenderer):
 
         request = clone_request(request, method)
         try:
-            if not view.has_permission(request, obj):
+            if not view.has_permission(request):
                 return  # Don't have permission
         except Exception:
             return  # Don't have permission and exception explicitly raise
diff --git a/rest_framework/tests/permissions.py b/rest_framework/tests/permissions.py
index 26a34319..b8e1d89c 100644
--- a/rest_framework/tests/permissions.py
+++ b/rest_framework/tests/permissions.py
@@ -115,9 +115,7 @@ class OwnerModel(models.Model):
 
 
 class IsOwnerPermission(permissions.BasePermission):
-    def has_permission(self, request, view, obj=None):
-        if not obj:
-            return True
+    def has_object_permission(self, request, view, obj):
         return request.user == obj.owner
 
 
diff --git a/rest_framework/tests/renderers.py b/rest_framework/tests/renderers.py
index 72405336..e3f45ce6 100644
--- a/rest_framework/tests/renderers.py
+++ b/rest_framework/tests/renderers.py
@@ -95,7 +95,7 @@ urlpatterns = patterns('',
 
 
 class POSTDeniedPermission(permissions.BasePermission):
-    def has_permission(self, request, view, obj=None):
+    def has_permission(self, request, view):
         return request.method != 'POST'
 
 
diff --git a/rest_framework/views.py b/rest_framework/views.py
index fd6b4313..dd8889ae 100644
--- a/rest_framework/views.py
+++ b/rest_framework/views.py
@@ -13,6 +13,7 @@ from rest_framework.response import Response
 from rest_framework.request import Request
 from rest_framework.settings import api_settings
 import re
+import warnings
 
 
 def _remove_trailing_string(content, trailing):
@@ -261,8 +262,23 @@ class APIView(View):
         """
         Return `True` if the request should be permitted.
         """
+        if obj is not None:
+            warnings.warn('The `obj` argument in `has_permission` is due to be deprecated. '
+                      'Use `has_object_permission()` instead for object permissions.',
+                       PendingDeprecationWarning, stacklevel=2)
+            return self.has_object_permission(request, obj)
+
+        for permission in self.get_permissions():
+            if not permission.has_permission(request, self):
+                return False
+        return True
+
+    def has_object_permission(self, request, obj):
+        """
+        Return `True` if the request should be permitted for a given object.
+        """
         for permission in self.get_permissions():
-            if not permission.has_permission(request, self, obj):
+            if not permission.has_object_permission(request, self, obj):
                 return False
         return True