From 0d74f9553c6acb8b53a502ca5e39542dcc4412c1 Mon Sep 17 00:00:00 2001 From: Keryn Knight Date: Sat, 7 Sep 2013 13:32:12 +0100 Subject: [PATCH] Fixed #21063 -- AdminSite app_index should be fail early if the user has no permissions. --- django/contrib/admin/sites.py | 69 ++++++++++++++++++----------------- tests/admin_views/tests.py | 21 +++++++++++ 2 files changed, 56 insertions(+), 34 deletions(-) diff --git a/django/contrib/admin/sites.py b/django/contrib/admin/sites.py index b4f45c1e17..bb767a6dfa 100644 --- a/django/contrib/admin/sites.py +++ b/django/contrib/admin/sites.py @@ -6,7 +6,7 @@ from django.contrib.auth import logout as auth_logout, REDIRECT_FIELD_NAME from django.contrib.contenttypes import views as contenttype_views from django.views.decorators.csrf import csrf_protect from django.db.models.base import ModelBase -from django.core.exceptions import ImproperlyConfigured +from django.core.exceptions import ImproperlyConfigured, PermissionDenied from django.core.urlresolvers import reverse, NoReverseMatch from django.template.response import TemplateResponse from django.utils import six @@ -399,44 +399,45 @@ class AdminSite(object): def app_index(self, request, app_label, extra_context=None): user = request.user has_module_perms = user.has_module_perms(app_label) + if not has_module_perms: + raise PermissionDenied app_dict = {} for model, model_admin in self._registry.items(): if app_label == model._meta.app_label: - if has_module_perms: - perms = model_admin.get_model_perms(request) + perms = model_admin.get_model_perms(request) - # Check whether user has any perm for this module. - # If so, add the module to the model_list. - if True in perms.values(): - info = (app_label, model._meta.model_name) - model_dict = { - 'name': capfirst(model._meta.verbose_name_plural), - 'object_name': model._meta.object_name, - 'perms': perms, + # Check whether user has any perm for this module. + # If so, add the module to the model_list. + if True in perms.values(): + info = (app_label, model._meta.model_name) + model_dict = { + 'name': capfirst(model._meta.verbose_name_plural), + 'object_name': model._meta.object_name, + 'perms': perms, + } + if perms.get('change'): + try: + model_dict['admin_url'] = reverse('admin:%s_%s_changelist' % info, current_app=self.name) + except NoReverseMatch: + pass + if perms.get('add'): + try: + model_dict['add_url'] = reverse('admin:%s_%s_add' % info, current_app=self.name) + except NoReverseMatch: + pass + if app_dict: + app_dict['models'].append(model_dict), + else: + # First time around, now that we know there's + # something to display, add in the necessary meta + # information. + app_dict = { + 'name': app_label.title(), + 'app_label': app_label, + 'app_url': '', + 'has_module_perms': has_module_perms, + 'models': [model_dict], } - if perms.get('change', False): - try: - model_dict['admin_url'] = reverse('admin:%s_%s_changelist' % info, current_app=self.name) - except NoReverseMatch: - pass - if perms.get('add', False): - try: - model_dict['add_url'] = reverse('admin:%s_%s_add' % info, current_app=self.name) - except NoReverseMatch: - pass - if app_dict: - app_dict['models'].append(model_dict), - else: - # First time around, now that we know there's - # something to display, add in the necessary meta - # information. - app_dict = { - 'name': app_label.title(), - 'app_label': app_label, - 'app_url': '', - 'has_module_perms': has_module_perms, - 'models': [model_dict], - } if not app_dict: raise Http404('The requested admin page does not exist.') # Sort the models alphabetically within each app. diff --git a/tests/admin_views/tests.py b/tests/admin_views/tests.py index ac289c550a..19bf00f302 100644 --- a/tests/admin_views/tests.py +++ b/tests/admin_views/tests.py @@ -1309,6 +1309,27 @@ class AdminViewPermissionsTest(TestCase): response = self.client.get('/test_admin/admin/secure-view/') self.assertContains(response, 'id="login-form"') + def testAppIndexFailEarly(self): + """ + If a user has no module perms, avoid iterating over all the modeladmins + in the registry. + """ + opts = Article._meta + change_user = User.objects.get(username='changeuser') + permission = get_perm(Article, get_permission_codename('change', opts)) + + self.client.post('/test_admin/admin/', self.changeuser_login) + + # the user has no module permissions, because this module doesn't exist + change_user.user_permissions.remove(permission) + response = self.client.get('/test_admin/admin/admin_views/') + self.assertEqual(response.status_code, 403) + + # the user now has module permissions + change_user.user_permissions.add(permission) + response = self.client.get('/test_admin/admin/admin_views/') + self.assertEqual(response.status_code, 200) + @override_settings(PASSWORD_HASHERS=('django.contrib.auth.hashers.SHA1PasswordHasher',)) class AdminViewsNoUrlTest(TestCase):