Fixed #22295 -- Replaced permission check for displaying admin user-tools

django/contrib/admin/actions.py

@@ -64,7 +64,7 @@ def delete_selected(modeladmin, request, queryset):
         title = _("Are you sure?")
 
     context = dict(
-        modeladmin.admin_site.each_context(),
+        modeladmin.admin_site.each_context(request),
         title=title,
         objects_name=objects_name,
         deletable_objects=[deletable_objects],

django/contrib/admin/options.py

@@ -1461,7 +1461,7 @@ class ModelAdmin(BaseModelAdmin):
         for inline_formset in inline_formsets:
             media = media + inline_formset.media
 
-        context = dict(self.admin_site.each_context(),
+        context = dict(self.admin_site.each_context(request),
             title=(_('Add %s') if add else _('Change %s')) % force_text(opts.verbose_name),
             adminform=adminForm,
             object_id=object_id,
@@ -1617,7 +1617,7 @@ class ModelAdmin(BaseModelAdmin):
             'All %(total_count)s selected', cl.result_count)
 
         context = dict(
-            self.admin_site.each_context(),
+            self.admin_site.each_context(request),
             module_name=force_text(opts.verbose_name_plural),
             selection_note=_('0 of %(cnt)s selected') % {'cnt': len(cl.result_list)},
             selection_note_all=selection_note_all % {'total_count': cl.result_count},
@@ -1686,7 +1686,7 @@ class ModelAdmin(BaseModelAdmin):
             title = _("Are you sure?")
 
         context = dict(
-            self.admin_site.each_context(),
+            self.admin_site.each_context(request),
             title=title,
             object_name=object_name,
             object=obj,
@@ -1720,7 +1720,7 @@ class ModelAdmin(BaseModelAdmin):
             content_type=get_content_type_for_model(model)
         ).select_related().order_by('action_time')
 
-        context = dict(self.admin_site.each_context(),
+        context = dict(self.admin_site.each_context(request),
             title=_('Change history: %s') % force_text(obj),
             action_list=action_list,
             module_name=capfirst(force_text(opts.verbose_name_plural)),

django/contrib/admin/sites.py

@@ -272,7 +272,7 @@ class AdminSite(object):
     def urls(self):
         return self.get_urls(), 'admin', self.name
 
-    def each_context(self):
+    def each_context(self, request):
         """
         Returns a dictionary of variables to put in the template context for
         *every* page in the admin site.
@@ -281,6 +281,7 @@ class AdminSite(object):
             'site_title': self.site_title,
             'site_header': self.site_header,
             'site_url': self.site_url,
+            'has_permission': self.has_permission(request),
         }
 
     def password_change(self, request, extra_context=None):
@@ -294,7 +295,7 @@ class AdminSite(object):
             'current_app': self.name,
             'password_change_form': AdminPasswordChangeForm,
             'post_change_redirect': url,
-            'extra_context': dict(self.each_context(), **(extra_context or {})),
+            'extra_context': dict(self.each_context(request), **(extra_context or {})),
         }
         if self.password_change_template is not None:
             defaults['template_name'] = self.password_change_template
@@ -307,7 +308,7 @@ class AdminSite(object):
         from django.contrib.auth.views import password_change_done
         defaults = {
             'current_app': self.name,
-            'extra_context': dict(self.each_context(), **(extra_context or {})),
+            'extra_context': dict(self.each_context(request), **(extra_context or {})),
         }
         if self.password_change_done_template is not None:
             defaults['template_name'] = self.password_change_done_template
@@ -336,7 +337,7 @@ class AdminSite(object):
         from django.contrib.auth.views import logout
         defaults = {
             'current_app': self.name,
-            'extra_context': dict(self.each_context(), **(extra_context or {})),
+            'extra_context': dict(self.each_context(request), **(extra_context or {})),
         }
         if self.logout_template is not None:
             defaults['template_name'] = self.logout_template
@@ -357,7 +358,7 @@ class AdminSite(object):
         # it cannot import models from other applications at the module level,
         # and django.contrib.admin.forms eventually imports User.
         from django.contrib.admin.forms import AdminAuthenticationForm
-        context = dict(self.each_context(),
+        context = dict(self.each_context(request),
             title=_('Log in'),
             app_path=request.get_full_path(),
         )
@@ -431,7 +432,7 @@ class AdminSite(object):
             app['models'].sort(key=lambda x: x['name'])
 
         context = dict(
-            self.each_context(),
+            self.each_context(request),
             title=self.index_title,
             app_list=app_list,
         )
@@ -489,7 +490,7 @@ class AdminSite(object):
             raise Http404('The requested admin page does not exist.')
         # Sort the models alphabetically within each app.
         app_dict['models'].sort(key=lambda x: x['name'])
-        context = dict(self.each_context(),
+        context = dict(self.each_context(request),
             title=_('%(app)s administration') % {'app': app_name},
             app_list=[app_dict],
             app_label=app_label,

django/contrib/admin/templates/admin/base.html

@@ -24,7 +24,8 @@
         <div id="branding">
         {% block branding %}{% endblock %}
         </div>
-        {% if user.is_active and user.is_staff %}
+        {% block usertools %}
+        {% if has_permission %}
         <div id="user-tools">
             {% block welcome-msg %}
                 {% trans 'Welcome,' %}
@@ -34,9 +35,11 @@
                 {% if site_url %}
                     <a href="{{ site_url }}">{% trans 'View site' %}</a> /
                 {% endif %}
-                {% url 'django-admindocs-docroot' as docsroot %}
-                {% if docsroot %}
-                    <a href="{{ docsroot }}">{% trans 'Documentation' %}</a> /
+                {% if user.is_active and user.is_staff %}
+                    {% url 'django-admindocs-docroot' as docsroot %}
+                    {% if docsroot %}
+                        <a href="{{ docsroot }}">{% trans 'Documentation' %}</a> /
+                    {% endif %}
                 {% endif %}
                 {% if user.has_usable_password %}
                 <a href="{% url 'admin:password_change' %}">{% trans 'Change password' %}</a> /
@@ -45,6 +48,7 @@
             {% endblock %}
         </div>
         {% endif %}
+        {% endblock %}
         {% block nav-global %}{% endblock %}
     </div>
     <!-- END Header -->

django/contrib/admin/templates/admin/login.html

@@ -5,6 +5,8 @@
 
 {% block bodyclass %}{{ block.super }} login{% endblock %}
 
+{% block usertools %}{% endblock %}
+
 {% block nav-global %}{% endblock %}
 
 {% block content_title %}{% endblock %}

django/contrib/admindocs/views.py

@@ -30,16 +30,16 @@ class BaseAdminDocsView(TemplateView):
     Base view for admindocs views.
     """
     @method_decorator(staff_member_required)
-    def dispatch(self, *args, **kwargs):
+    def dispatch(self, request, *args, **kwargs):
         if not utils.docutils_is_available:
             # Display an error message for people without docutils
             self.template_name = 'admin_doc/missing_docutils.html'
-            return self.render_to_response(admin.site.each_context())
-        return super(BaseAdminDocsView, self).dispatch(*args, **kwargs)
+            return self.render_to_response(admin.site.each_context(request))
+        return super(BaseAdminDocsView, self).dispatch(request, *args, **kwargs)
 
     def get_context_data(self, **kwargs):
         kwargs.update({'root_path': urlresolvers.reverse('admin:index')})
-        kwargs.update(admin.site.each_context())
+        kwargs.update(admin.site.each_context(self.request))
         return super(BaseAdminDocsView, self).get_context_data(**kwargs)
 
 

django/contrib/auth/admin.py

@@ -157,7 +157,7 @@ class UserAdmin(admin.ModelAdmin):
             'save_as': False,
             'show_save': True,
         }
-        context.update(admin.site.each_context())
+        context.update(admin.site.each_context(request))
 
         request.current_app = self.admin_site.name
 

docs/ref/contrib/admin/index.txt

@@ -2520,6 +2520,24 @@ Templates can override or extend base admin templates as described in
 ``AdminSite`` methods
 ---------------------
 
+.. method:: AdminSite.each_context(request):
+
+    .. versionadded:: 1.7
+
+    Returns a dictionary of variables to put in the template context for
+    every page in the admin site.
+
+    Includes the following variables and values by default:
+
+    * ``site_header``: :attr:`AdminSite.site_header`
+    * ``site_title``: :attr:`AdminSite.site_title`
+    * ``site_url``: :attr:`AdminSite.site_url`
+    * ``has_permission``: :meth:`AdminSite.has_permission`
+
+    .. versionchanged:: 1.8
+
+        The ``request`` argument and the ``has_permission`` variable were added.
+
 .. method:: AdminSite.has_permission(request)
 
     Returns ``True`` if the user for the given ``HttpRequest`` has permission

docs/releases/1.8.txt

@@ -136,6 +136,15 @@ Minor features
 * The ``AdminSite.password_change()`` method now has an ``extra_context``
   parameter.
 
+* You can now control who may login to the admin site by overriding only
+  :meth:`AdminSite.has_permission()
+  <django.contrib.admin.AdminSite.has_permission>` and
+  :attr:`AdminSite.login_form <django.contrib.admin.AdminSite.login_form>`.
+  The ``base.html`` template has a new block ``usertools`` which contains the
+  user-specific header. A new context variable ``has_permission``, which gets
+  its value from :meth:`~django.contrib.admin.AdminSite.has_permission`,
+  indicates whether the user may access the site.
+
 :mod:`django.contrib.admindocs`
 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
 
@@ -899,6 +908,14 @@ Miscellaneous
   opposed to the instance name which you can still customize using
   ``AdminSite(name="...")``.
 
+* The block ``usertools`` in the ``base.html`` template of
+  :mod:`django.contrib.admin` now requires the ``has_permission`` context
+  variable to be set. If you have any custom admin views that use this
+  template, update them to pass :meth:`AdminSite.has_permission()
+  <django.contrib.admin.AdminSite.has_permission>` as this new variable's
+  value or simply include :meth:`AdminSite.each_context(request)
+  <django.contrib.admin.AdminSite.each_context>` in the context.
+
 * Internal changes were made to the :class:`~django.forms.ClearableFileInput`
   widget to allow more customization. The undocumented ``url_markup_template``
   attribute was removed in favor of ``template_with_initial``.

tests/admin_views/custom_has_permission_admin.py

@@ -0,0 +1,33 @@
+"""
+A custom AdminSite for AdminViewPermissionsTest.test_login_has_permission().
+"""
+from __future__ import unicode_literals
+
+from django.contrib import admin
+from django.contrib.auth import get_permission_codename
+from django.contrib.auth.forms import AuthenticationForm
+
+from . import models, admin as base_admin
+
+PERMISSION_NAME = 'admin_views.%s' % get_permission_codename('change', models.Article._meta)
+
+
+class PermissionAdminAuthenticationForm(AuthenticationForm):
+    def confirm_login_allowed(self, user):
+        from django import forms
+        if not user.is_active or not (user.is_staff or user.has_perm(PERMISSION_NAME)):
+            raise forms.ValidationError('permission denied')
+
+
+class HasPermissionAdmin(admin.AdminSite):
+    login_form = PermissionAdminAuthenticationForm
+
+    def has_permission(self, request):
+        return (
+            request.user.is_active and
+            (request.user.is_staff or request.user.has_perm(PERMISSION_NAME))
+        )
+
+
+site = HasPermissionAdmin(name="has_permission_admin")
+site.register(models.Article, base_admin.ArticleAdmin)

tests/admin_views/fixtures/admin-views-users.xml

@@ -70,6 +70,20 @@
         <field to="auth.group" name="groups" rel="ManyToManyRel"></field>
         <field to="auth.permission" name="user_permissions" rel="ManyToManyRel"></field>
     </object>
+    <object pk="106" model="auth.user">
+        <field type="CharField" name="username">nostaff</field>
+        <field type="CharField" name="first_name">No</field>
+        <field type="CharField" name="last_name">Staff</field>
+        <field type="CharField" name="email">nostaff@example.com</field>
+        <field type="CharField" name="password">sha1$995a3$6011485ea3834267d719b4c801409b8b1ddd0158</field>
+        <field type="BooleanField" name="is_staff">False</field>
+        <field type="BooleanField" name="is_active">True</field>
+        <field type="BooleanField" name="is_superuser">False</field>
+        <field type="DateTimeField" name="last_login">2007-05-30 13:20:10</field>
+        <field type="DateTimeField" name="date_joined">2007-05-30 13:20:10</field>
+        <field to="auth.group" name="groups" rel="ManyToManyRel"></field>
+        <field to="auth.permission" name="user_permissions" rel="ManyToManyRel"></field>
+    </object>
     <object pk="1" model="admin_views.section">
         <field type="CharField" name="name">Test section</field>
     </object>

tests/admin_views/tests.py

@@ -1091,6 +1091,9 @@ class AdminViewPermissionsTest(TestCase):
         change_user = User.objects.get(username='changeuser')
         change_user.user_permissions.add(get_perm(Article,
             get_permission_codename('change', opts)))
+        change_user2 = User.objects.get(username='nostaff')
+        change_user2.user_permissions.add(get_perm(Article,
+            get_permission_codename('change', opts)))
 
         # User who can delete Articles
         delete_user = User.objects.get(username='deleteuser')
@@ -1131,6 +1134,11 @@ class AdminViewPermissionsTest(TestCase):
             'username': 'deleteuser',
             'password': 'secret',
         }
+        self.nostaff_login = {
+            REDIRECT_FIELD_NAME: '/test_admin/has_permission_admin/',
+            'username': 'nostaff',
+            'password': 'secret',
+        }
         self.joepublic_login = {
             REDIRECT_FIELD_NAME: '/test_admin/admin/',
             'username': 'joepublic',
@@ -1211,6 +1219,34 @@ class AdminViewPermissionsTest(TestCase):
         form = login.context[0].get('form')
         self.assertEqual(form.errors['username'][0], 'This field is required.')
 
+    def test_login_has_permission(self):
+        # Regular User should not be able to login.
+        response = self.client.get('/test_admin/has_permission_admin/')
+        self.assertEqual(response.status_code, 302)
+        login = self.client.post('/test_admin/has_permission_admin/login/', self.joepublic_login)
+        self.assertEqual(login.status_code, 200)
+        self.assertContains(login, 'permission denied')
+
+        # User with permissions should be able to login.
+        response = self.client.get('/test_admin/has_permission_admin/')
+        self.assertEqual(response.status_code, 302)
+        login = self.client.post('/test_admin/has_permission_admin/login/', self.nostaff_login)
+        self.assertRedirects(login, '/test_admin/has_permission_admin/')
+        self.assertFalse(login.context)
+        self.client.get('/test_admin/has_permission_admin/logout/')
+
+        # Staff should be able to login.
+        response = self.client.get('/test_admin/has_permission_admin/')
+        self.assertEqual(response.status_code, 302)
+        login = self.client.post('/test_admin/has_permission_admin/login/', {
+            REDIRECT_FIELD_NAME: '/test_admin/has_permission_admin/',
+            'username': 'deleteuser',
+            'password': 'secret',
+        })
+        self.assertRedirects(login, '/test_admin/has_permission_admin/')
+        self.assertFalse(login.context)
+        self.client.get('/test_admin/has_permission_admin/logout/')
+
     def test_login_successfully_redirects_to_original_URL(self):
         response = self.client.get('/test_admin/admin/')
         self.assertEqual(response.status_code, 302)

tests/admin_views/urls.py

@@ -1,6 +1,6 @@
 from django.conf.urls import include, url
 
-from . import views, customadmin, admin
+from . import views, customadmin, custom_has_permission_admin, admin
 
 
 urlpatterns = [
@@ -12,4 +12,5 @@ urlpatterns = [
     url(r'^test_admin/admin4/', include(customadmin.simple_site.urls)),
     url(r'^test_admin/admin5/', include(admin.site2.urls)),
     url(r'^test_admin/admin7/', include(admin.site7.urls)),
+    url(r'^test_admin/has_permission_admin/', include(custom_has_permission_admin.site.urls)),
 ]