Updated DjangoAuthorization to disallow read unless a user has `chang…

…e` permission. (Closes #1407, PR #1409)

AUTHORS

@@ -97,7 +97,7 @@ Contributors:
 * Judit Novak (juditnovak) for related schema updates
 * Mike Bryant (mikebryant) for unit tests
 * Maxim Filipenko (prokaktus) for changing repr value to str in Python 2
-
+* Thomas Yip (thomasyip) PR #1409
 
 Thanks to Tav for providing validate_jsonp.py, placed in public domain.
 

docs/release_notes/dev.rst

@@ -7,4 +7,5 @@ copied to the release notes for the next release.
 Bugfixes
 --------
 
-* list of changes here
+* Updated DjangoAuthorization to disallow read unless a user has `change` permission. (#1407, PR #1409)
+

tastypie/authorization.py

@@ -139,6 +139,12 @@ class DjangoAuthorization(Authorization):
     Both the list & detail variants simply check the model they're based
     on, as that's all the more granular Django's permission setup gets.
     """
+
+    # By default, following `ModelAdmin` "convention", `app.change_model` is used
+    # `django.contrib.auth.models.Permission` as perm code for viewing and updating.
+    # https://docs.djangoproject.com/es/1.9/topics/auth/default/#permissions-and-authorization
+    READ_PERM_CODE = 'change'
+
     def base_checks(self, request, model_klass):
         # If it doesn't look like a model, we can't check permissions.
         if not model_klass or not getattr(model_klass, '_meta', None):
@@ -150,116 +156,61 @@ def base_checks(self, request, model_klass):
 
         return model_klass
 
-    def read_list(self, object_list, bundle):
-        klass = self.base_checks(bundle.request, object_list.model)
-
-        if klass is False:
-            return []
-
-        # GET-style methods are always allowed.
-        return object_list
-
-    def read_detail(self, object_list, bundle):
-        klass = self.base_checks(bundle.request, bundle.obj.__class__)
-
-        if klass is False:
-            raise Unauthorized("You are not allowed to access that resource.")
-
-        # GET-style methods are always allowed.
-        return True
-
-    def create_list(self, object_list, bundle):
-        klass = self.base_checks(bundle.request, object_list.model)
+    def check_user_perm(self, user, permission, obj_or_list):
+        return user.has_perm(permission)
 
+    def perm_list_checks(self, request, code, obj_list):
+        klass = self.base_checks(request, obj_list.model)
         if klass is False:
             return []
 
-        permission = '%s.add_%s' % (
+        permission = '%s.%s_%s' % (
             klass._meta.app_label,
+            code,
             get_module_name(klass._meta)
         )
 
-        if not bundle.request.user.has_perm(permission):
-            return []
+        if self.check_user_perm(request.user, permission, obj_list):
+            return obj_list
 
-        return object_list
-
-    def create_detail(self, object_list, bundle):
-        klass = self.base_checks(bundle.request, bundle.obj.__class__)
+        return obj_list.none()
 
+    def perm_obj_checks(self, request, code, obj):
+        klass = self.base_checks(request, obj.__class__)
         if klass is False:
             raise Unauthorized("You are not allowed to access that resource.")
 
-        permission = '%s.add_%s' % (
+        permission = '%s.%s_%s' % (
             klass._meta.app_label,
+            code,
             get_module_name(klass._meta)
         )
 
-        if not bundle.request.user.has_perm(permission):
-            raise Unauthorized("You are not allowed to access that resource.")
+        if self.check_user_perm(request.user, permission, obj):
+            return True
 
-        return True
+        raise Unauthorized("You are not allowed to access that resource.")
 
-    def update_list(self, object_list, bundle):
-        klass = self.base_checks(bundle.request, object_list.model)
+    def read_list(self, object_list, bundle):
+        return self.perm_list_checks(bundle.request, self.READ_PERM_CODE, object_list)
 
-        if klass is False:
-            return []
+    def read_detail(self, object_list, bundle):
+        return self.perm_obj_checks(bundle.request, self.READ_PERM_CODE, bundle.obj)
 
-        permission = '%s.change_%s' % (
-            klass._meta.app_label,
-            get_module_name(klass._meta)
-        )
+    def create_list(self, object_list, bundle):
+        return self.perm_list_checks(bundle.request, 'add', object_list)
 
-        if not bundle.request.user.has_perm(permission):
-            return []
+    def create_detail(self, object_list, bundle):
+        return self.perm_obj_checks(bundle.request, 'add', bundle.obj)
 
-        return object_list
+    def update_list(self, object_list, bundle):
+        return self.perm_list_checks(bundle.request, 'change', object_list)
 
     def update_detail(self, object_list, bundle):
-        klass = self.base_checks(bundle.request, bundle.obj.__class__)
-
-        if klass is False:
-            raise Unauthorized("You are not allowed to access that resource.")
-
-        permission = '%s.change_%s' % (
-            klass._meta.app_label,
-            get_module_name(klass._meta)
-        )
-
-        if not bundle.request.user.has_perm(permission):
-            raise Unauthorized("You are not allowed to access that resource.")
-
-        return True
+        return self.perm_obj_checks(bundle.request, 'change', bundle.obj)
 
     def delete_list(self, object_list, bundle):
-        klass = self.base_checks(bundle.request, object_list.model)
-
-        if klass is False:
-            return []
-
-        permission = '%s.delete_%s' % (
-            klass._meta.app_label,
-            get_module_name(klass._meta)
-        )
-
-        if not bundle.request.user.has_perm(permission):
-            return []
-
-        return object_list
+        return self.perm_list_checks(bundle.request, 'delete', object_list)
 
     def delete_detail(self, object_list, bundle):
-        klass = self.base_checks(bundle.request, bundle.obj.__class__)
-
-        if klass is False:
-            raise Unauthorized("You are not allowed to access that resource.")
-
-        permission = '%s.delete_%s' % (
-            klass._meta.app_label,
-            get_module_name(klass._meta)
-        )
-
-        if not bundle.request.user.has_perm(permission):
-            raise Unauthorized("You are not allowed to access that resource.")
-
-        return True
+        return self.perm_obj_checks(bundle.request, 'delete', bundle.obj)

tests/core/tests/authorization.py

@@ -113,8 +113,8 @@ def test_no_perms(self):
         bundle = resource.build_bundle(request=request)
 
         bundle.request.method = 'GET'
-        self.assertEqual(len(auth.read_list(resource.get_object_list(bundle.request), bundle)), 4)
-        self.assertTrue(auth.read_detail(resource.get_object_list(bundle.request)[0], bundle))
+        self.assertEqual(len(auth.read_list(resource.get_object_list(bundle.request), bundle)), 0)
+        self.assertRaises(Unauthorized, auth.read_detail, resource.get_object_list(bundle.request)[0], bundle)
 
         bundle.request.method = 'POST'
         self.assertEqual(len(auth.create_list(resource.get_object_list(bundle.request), bundle)), 0)
@@ -142,8 +142,8 @@ def test_add_perm(self):
         bundle = resource.build_bundle(request=request)
 
         bundle.request.method = 'GET'
-        self.assertEqual(len(auth.read_list(resource.get_object_list(bundle.request), bundle)), 4)
-        self.assertTrue(auth.read_detail(resource.get_object_list(bundle.request)[0], bundle))
+        self.assertEqual(len(auth.read_list(resource.get_object_list(bundle.request), bundle)), 0)
+        self.assertRaises(Unauthorized, auth.read_detail, resource.get_object_list(bundle.request)[0], bundle)
 
         bundle.request.method = 'POST'
         self.assertEqual(len(auth.create_list(resource.get_object_list(bundle.request), bundle)), 4)
@@ -196,8 +196,8 @@ def test_delete_perm(self):
         bundle = resource.build_bundle(request=request)
 
         bundle.request.method = 'GET'
-        self.assertEqual(len(auth.read_list(resource.get_object_list(bundle.request), bundle)), 4)
-        self.assertTrue(auth.read_detail(resource.get_object_list(bundle.request)[0], bundle))
+        self.assertEqual(len(auth.read_list(resource.get_object_list(bundle.request), bundle)), 0)
+        self.assertRaises(Unauthorized, auth.read_detail, resource.get_object_list(bundle.request)[0], bundle)
 
         bundle.request.method = 'POST'
         self.assertEqual(len(auth.create_list(resource.get_object_list(bundle.request), bundle)), 0)