diff --git a/AUTHORS b/AUTHORS index 8682341b8..b8b098766 100644 --- a/AUTHORS +++ b/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. diff --git a/docs/release_notes/dev.rst b/docs/release_notes/dev.rst index c0f80b292..6aa39eda3 100644 --- a/docs/release_notes/dev.rst +++ b/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) + diff --git a/tastypie/authorization.py b/tastypie/authorization.py index 1d6f5aaf9..8996137f1 100644 --- a/tastypie/authorization.py +++ b/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) diff --git a/tests/core/tests/authorization.py b/tests/core/tests/authorization.py index 0c31f5f5f..de0be62fa 100644 --- a/tests/core/tests/authorization.py +++ b/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)