Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixed #6327 -- Added has_module_permission method to BaseModelAdmin #2730

Closed
wants to merge 3 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
7 changes: 7 additions & 0 deletions django/contrib/admin/options.py
Expand Up @@ -477,6 +477,13 @@ def has_delete_permission(self, request, obj=None):
codename = get_permission_codename('delete', opts) codename = get_permission_codename('delete', opts)
return request.user.has_perm("%s.%s" % (opts.app_label, codename)) return request.user.has_perm("%s.%s" % (opts.app_label, codename))


def has_module_permission(self, request):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The method needs to be used by the ModelAdmin... (and should add a test that overriding it achieves the desired functionality)

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure I understand where to use this method in ModelAdmin. Should I replace occurrences of user.has_module_perms by model_admin.has_module_permission (like in the index() and app_index() methods of AdminSite) ?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I believe that's the point of the suggestion -- so you can override the method if you want to customize the behavior.

"""
Returns True if the given request has any permission in the given
app label
"""
return request.user.has_module_perms(self.opts.app_label)



@python_2_unicode_compatible @python_2_unicode_compatible
class ModelAdmin(BaseModelAdmin): class ModelAdmin(BaseModelAdmin):
Expand Down
11 changes: 5 additions & 6 deletions django/contrib/admin/sites.py
Expand Up @@ -367,10 +367,9 @@ def index(self, request, extra_context=None):
apps that have been registered in this site. apps that have been registered in this site.
""" """
app_dict = {} app_dict = {}
user = request.user
for model, model_admin in self._registry.items(): for model, model_admin in self._registry.items():
app_label = model._meta.app_label app_label = model._meta.app_label
has_module_perms = user.has_module_perms(app_label) has_module_perms = model_admin.has_module_permission(request)


if has_module_perms: if has_module_perms:
perms = model_admin.get_model_perms(request) perms = model_admin.get_model_perms(request)
Expand Down Expand Up @@ -424,14 +423,14 @@ def index(self, request, extra_context=None):
current_app=self.name) current_app=self.name)


def app_index(self, request, app_label, extra_context=None): def app_index(self, request, app_label, extra_context=None):
user = request.user
app_name = apps.get_app_config(app_label).verbose_name app_name = apps.get_app_config(app_label).verbose_name
has_module_perms = user.has_module_perms(app_label)
if not has_module_perms:
raise PermissionDenied
app_dict = {} app_dict = {}
for model, model_admin in self._registry.items(): for model, model_admin in self._registry.items():
if app_label == model._meta.app_label: if app_label == model._meta.app_label:
has_module_perms = model_admin.has_module_permission(request)
if not has_module_perms:
raise PermissionDenied

perms = model_admin.get_model_perms(request) perms = model_admin.get_model_perms(request)


# Check whether user has any perm for this module. # Check whether user has any perm for this module.
Expand Down
14 changes: 14 additions & 0 deletions docs/ref/contrib/admin/index.txt
Expand Up @@ -1631,6 +1631,19 @@ templates used by the :class:`ModelAdmin` views:
be interpreted as meaning that the current user is not permitted to delete be interpreted as meaning that the current user is not permitted to delete
any object of this type). any object of this type).


.. method:: ModelAdmin.has_module_permission(request)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

.. versionadded:: 1.8 and add a mention in the 1.8 release notes

.. versionadded:: 1.8

Should return ``True`` if adding, editing, or deleting an object is
permitted, ``False`` otherwise. Uses :meth:`User.has_module_perms()
<django.contrib.auth.models.User.has_module_perms>` by default. It is
used for displaying the module on the admin index page and for accessing
the module's index page. Overriding it does not restrict access to the add,
change or delete views, :meth:`~ModelAdmin.has_add_permission`,
:meth:`~ModelAdmin.has_change_permission`, or
:meth:`~ModelAdmin.has_delete_permission` should be used for that.

.. method:: ModelAdmin.get_queryset(request) .. method:: ModelAdmin.get_queryset(request)


The ``get_queryset`` method on a ``ModelAdmin`` returns a The ``get_queryset`` method on a ``ModelAdmin`` returns a
Expand Down Expand Up @@ -1909,6 +1922,7 @@ adds some of its own (the shared features are actually defined in the
- :meth:`~ModelAdmin.has_add_permission` - :meth:`~ModelAdmin.has_add_permission`
- :meth:`~ModelAdmin.has_change_permission` - :meth:`~ModelAdmin.has_change_permission`
- :meth:`~ModelAdmin.has_delete_permission` - :meth:`~ModelAdmin.has_delete_permission`
- :meth:`~ModelAdmin.has_module_permission`


The ``InlineModelAdmin`` class adds: The ``InlineModelAdmin`` class adds:


Expand Down
4 changes: 3 additions & 1 deletion docs/releases/1.8.txt
Expand Up @@ -31,7 +31,9 @@ Minor features
:mod:`django.contrib.admin` :mod:`django.contrib.admin`
^^^^^^^^^^^^^^^^^^^^^^^^^^^ ^^^^^^^^^^^^^^^^^^^^^^^^^^^


* ... * :class:`~django.contrib.admin.ModelAdmin` now has a
:meth:`~django.contrib.admin.ModelAdmin.has_module_permission`
method to check if the request has any permission for this module.


:mod:`django.contrib.auth` :mod:`django.contrib.auth`
^^^^^^^^^^^^^^^^^^^^^^^^^^ ^^^^^^^^^^^^^^^^^^^^^^^^^^
Expand Down
3 changes: 3 additions & 0 deletions tests/admin_ordering/tests.py
Expand Up @@ -17,6 +17,9 @@ class MockSuperUser(object):
def has_perm(self, perm): def has_perm(self, perm):
return True return True


def has_module_perms(self, module):
return True

request = MockRequest() request = MockRequest()
request.user = MockSuperUser() request.user = MockSuperUser()


Expand Down
8 changes: 8 additions & 0 deletions tests/admin_views/admin.py
Expand Up @@ -124,6 +124,12 @@ def save_model(self, request, obj, form, change=True):
return super(ArticleAdmin, self).save_model(request, obj, form, change) return super(ArticleAdmin, self).save_model(request, obj, form, change)




class ArticleAdmin2(admin.ModelAdmin):

def has_module_permission(self, request):
return False


class RowLevelChangePermissionModelAdmin(admin.ModelAdmin): class RowLevelChangePermissionModelAdmin(admin.ModelAdmin):
def has_change_permission(self, request, obj=None): def has_change_permission(self, request, obj=None):
""" Only allow changing objects with even id number """ """ Only allow changing objects with even id number """
Expand Down Expand Up @@ -923,3 +929,5 @@ def get_changeform_initial_data(self, request):
site2 = admin.AdminSite(name="namespaced_admin") site2 = admin.AdminSite(name="namespaced_admin")
site2.register(User, UserAdmin) site2.register(User, UserAdmin)
site2.register(Group, GroupAdmin) site2.register(Group, GroupAdmin)
site7 = admin.AdminSite(name="admin7")
site7.register(Article, ArticleAdmin2)
64 changes: 64 additions & 0 deletions tests/admin_views/tests.py
Expand Up @@ -1494,6 +1494,70 @@ def test_shortcut_view_only_available_to_staff(self):
self.assertEqual(response.status_code, 302) self.assertEqual(response.status_code, 302)
self.assertEqual(response.url, 'http://example.com/dummy/foo/') self.assertEqual(response.url, 'http://example.com/dummy/foo/')


def test_has_module_permission(self):
"""
Ensure that has_module_permission() returns True for all users who
have any permission for that module (add, change, or delete), so that
the module is displayed on the admin index page.
"""
login_url = reverse('admin:login') + '?next=/test_admin/admin/'

self.client.post(login_url, self.super_login)
response = self.client.get('/test_admin/admin/')
self.assertContains(response, 'admin_views')
self.assertContains(response, 'Articles')
self.client.get('/test_admin/admin/logout/')

self.client.post(login_url, self.adduser_login)
response = self.client.get('/test_admin/admin/')
self.assertContains(response, 'admin_views')
self.assertContains(response, 'Articles')
self.client.get('/test_admin/admin/logout/')

self.client.post(login_url, self.changeuser_login)
response = self.client.get('/test_admin/admin/')
self.assertContains(response, 'admin_views')
self.assertContains(response, 'Articles')
self.client.get('/test_admin/admin/logout/')

self.client.post(login_url, self.deleteuser_login)
response = self.client.get('/test_admin/admin/')
self.assertContains(response, 'admin_views')
self.assertContains(response, 'Articles')
self.client.get('/test_admin/admin/logout/')

def test_overriding_has_module_permission(self):
"""
Ensure that overriding has_module_permission() has the desired effect.
In this case, it always returns False, so the module should not be
displayed on the admin index page for any users.
"""
login_url = reverse('admin:login') + '?next=/test_admin/admin7/'

self.client.post(login_url, self.super_login)
response = self.client.get('/test_admin/admin7/')
self.assertNotContains(response, 'admin_views')
self.assertNotContains(response, 'Articles')
self.client.get('/test_admin/admin7/logout/')

self.client.post(login_url, self.adduser_login)
response = self.client.get('/test_admin/admin7/')
self.assertNotContains(response, 'admin_views')
self.assertNotContains(response, 'Articles')
self.client.get('/test_admin/admin7/logout/')

self.client.post(login_url, self.changeuser_login)
response = self.client.get('/test_admin/admin7/')
self.assertNotContains(response, 'admin_views')
self.assertNotContains(response, 'Articles')
self.client.get('/test_admin/admin7/logout/')

self.client.post(login_url, self.deleteuser_login)
response = self.client.get('/test_admin/admin7/')
self.assertNotContains(response, 'admin_views')
self.assertNotContains(response, 'Articles')
self.client.get('/test_admin/admin7/logout/')



@override_settings(PASSWORD_HASHERS=('django.contrib.auth.hashers.SHA1PasswordHasher',), @override_settings(PASSWORD_HASHERS=('django.contrib.auth.hashers.SHA1PasswordHasher',),
ROOT_URLCONF="admin_views.urls") ROOT_URLCONF="admin_views.urls")
Expand Down
1 change: 1 addition & 0 deletions tests/admin_views/urls.py
Expand Up @@ -11,4 +11,5 @@
url(r'^test_admin/admin3/', include(admin.site.urls), dict(form_url='pony')), url(r'^test_admin/admin3/', include(admin.site.urls), dict(form_url='pony')),
url(r'^test_admin/admin4/', include(customadmin.simple_site.urls)), url(r'^test_admin/admin4/', include(customadmin.simple_site.urls)),
url(r'^test_admin/admin5/', include(admin.site2.urls)), url(r'^test_admin/admin5/', include(admin.site2.urls)),
url(r'^test_admin/admin7/', include(admin.site7.urls)),
] ]
90 changes: 90 additions & 0 deletions tests/modeladmin/tests.py
Expand Up @@ -1518,3 +1518,93 @@ class CustomModelAdmin(ModelAdmin):
validator_class = CustomValidator validator_class = CustomValidator


self.assertIsInvalid(CustomModelAdmin, ValidationTestModel, 'error!') self.assertIsInvalid(CustomModelAdmin, ValidationTestModel, 'error!')


class ModelAdminPermissionTests(TestCase):

class MockUser(object):
def has_module_perms(self, app_label):
if app_label == "modeladmin":
return True
return False

class MockAddUser(MockUser):
def has_perm(self, perm):
if perm == "modeladmin.add_band":
return True
return False

class MockChangeUser(MockUser):
def has_perm(self, perm):
if perm == "modeladmin.change_band":
return True
return False

class MockDeleteUser(MockUser):
def has_perm(self, perm):
if perm == "modeladmin.delete_band":
return True
return False

def test_has_add_permission(self):
"""
Ensure that has_add_permission returns True for users who can add
objects and False for users who can't.
"""
ma = ModelAdmin(Band, AdminSite())
request = MockRequest()
request.user = self.MockAddUser()
self.assertTrue(ma.has_add_permission(request))
request.user = self.MockChangeUser()
self.assertFalse(ma.has_add_permission(request))
request.user = self.MockDeleteUser()
self.assertFalse(ma.has_add_permission(request))

def test_has_change_permission(self):
"""
Ensure that has_change_permission returns True for users who can edit
objects and False for users who can't.
"""
ma = ModelAdmin(Band, AdminSite())
request = MockRequest()
request.user = self.MockAddUser()
self.assertFalse(ma.has_change_permission(request))
request.user = self.MockChangeUser()
self.assertTrue(ma.has_change_permission(request))
request.user = self.MockDeleteUser()
self.assertFalse(ma.has_change_permission(request))

def test_has_delete_permission(self):
"""
Ensure that has_delete_permission returns True for users who can delete
objects and False for users who can't.
"""
ma = ModelAdmin(Band, AdminSite())
request = MockRequest()
request.user = self.MockAddUser()
self.assertFalse(ma.has_delete_permission(request))
request.user = self.MockChangeUser()
self.assertFalse(ma.has_delete_permission(request))
request.user = self.MockDeleteUser()
self.assertTrue(ma.has_delete_permission(request))

def test_has_module_permission(self):
"""
Ensure that has_module_permission returns True for users who have any
permission for the module and False for users who don't.
"""
ma = ModelAdmin(Band, AdminSite())
request = MockRequest()
request.user = self.MockAddUser()
self.assertTrue(ma.has_module_permission(request))
request.user = self.MockChangeUser()
self.assertTrue(ma.has_module_permission(request))
request.user = self.MockDeleteUser()
self.assertTrue(ma.has_module_permission(request))
ma.opts.app_label = "anotherapp"
request.user = self.MockAddUser()
self.assertFalse(ma.has_module_permission(request))
request.user = self.MockChangeUser()
self.assertFalse(ma.has_module_permission(request))
request.user = self.MockDeleteUser()
self.assertFalse(ma.has_module_permission(request))