From b1b1da1eac93297503c04b8394fb98e38f552f5f Mon Sep 17 00:00:00 2001 From: Carl Meyer Date: Fri, 7 Oct 2011 00:41:25 +0000 Subject: [PATCH] Fixed #8060 - Added permissions-checking for admin inlines. Thanks p.patruno for report and Stephan Jaensch for work on the patch. git-svn-id: http://code.djangoproject.com/svn/django/trunk@16934 bcc190cf-cafb-0310-a4f2-bffc1f526a37 --- django/contrib/admin/options.py | 149 +++++++++----- django/contrib/contenttypes/generic.py | 3 +- docs/ref/contrib/admin/index.txt | 8 +- docs/releases/1.4.txt | 9 + tests/regressiontests/admin_inlines/tests.py | 183 +++++++++++++++++- tests/regressiontests/admin_ordering/tests.py | 22 ++- .../generic_inline_admin/tests.py | 22 ++- tests/regressiontests/modeladmin/tests.py | 17 +- 8 files changed, 341 insertions(+), 72 deletions(-) diff --git a/django/contrib/admin/options.py b/django/contrib/admin/options.py index f05b5cb3a7..6be2a64b7c 100644 --- a/django/contrib/admin/options.py +++ b/django/contrib/admin/options.py @@ -270,6 +270,41 @@ class BaseModelAdmin(object): clean_lookup = LOOKUP_SEP.join(parts) return clean_lookup in self.list_filter or clean_lookup == self.date_hierarchy + def has_add_permission(self, request): + """ + Returns True if the given request has permission to add an object. + Can be overriden by the user in subclasses. + """ + opts = self.opts + return request.user.has_perm(opts.app_label + '.' + opts.get_add_permission()) + + def has_change_permission(self, request, obj=None): + """ + Returns True if the given request has permission to change the given + Django model instance, the default implementation doesn't examine the + `obj` parameter. + + Can be overriden by the user in subclasses. In such case it should + return True if the given request has permission to change the `obj` + model instance. If `obj` is None, this should return True if the given + request has permission to change *any* object of the given type. + """ + opts = self.opts + return request.user.has_perm(opts.app_label + '.' + opts.get_change_permission()) + + def has_delete_permission(self, request, obj=None): + """ + Returns True if the given request has permission to change the given + Django model instance, the default implementation doesn't examine the + `obj` parameter. + + Can be overriden by the user in subclasses. In such case it should + return True if the given request has permission to delete the `obj` + model instance. If `obj` is None, this should return True if the given + request has permission to delete *any* object of the given type. + """ + opts = self.opts + return request.user.has_perm(opts.app_label + '.' + opts.get_delete_permission()) class ModelAdmin(BaseModelAdmin): "Encapsulates all admin options and functionality for a given model." @@ -307,10 +342,6 @@ class ModelAdmin(BaseModelAdmin): self.model = model self.opts = model._meta self.admin_site = admin_site - self.inline_instances = [] - for inline_class in self.inlines: - inline_instance = inline_class(self.model, self.admin_site) - self.inline_instances.append(inline_instance) if 'action_checkbox' not in self.list_display and self.actions is not None: self.list_display = ['action_checkbox'] + list(self.list_display) if not self.list_display_links: @@ -320,6 +351,21 @@ class ModelAdmin(BaseModelAdmin): break super(ModelAdmin, self).__init__() + def get_inline_instances(self, request): + inline_instances = [] + for inline_class in self.inlines: + inline = inline_class(self.model, self.admin_site) + if request: + if not (inline.has_add_permission(request) or + inline.has_change_permission(request) or + inline.has_delete_permission(request)): + continue + if not inline.has_add_permission(request): + inline.max_num = 0 + inline_instances.append(inline) + + return inline_instances + def get_urls(self): from django.conf.urls import patterns, url @@ -369,42 +415,6 @@ class ModelAdmin(BaseModelAdmin): js.extend(['getElementsBySelector.js', 'dom-drag.js' , 'admin/ordering.js']) return forms.Media(js=[static('admin/js/%s' % url) for url in js]) - def has_add_permission(self, request): - """ - Returns True if the given request has permission to add an object. - Can be overriden by the user in subclasses. - """ - opts = self.opts - return request.user.has_perm(opts.app_label + '.' + opts.get_add_permission()) - - def has_change_permission(self, request, obj=None): - """ - Returns True if the given request has permission to change the given - Django model instance, the default implementation doesn't examine the - `obj` parameter. - - Can be overriden by the user in subclasses. In such case it should - return True if the given request has permission to change the `obj` - model instance. If `obj` is None, this should return True if the given - request has permission to change *any* object of the given type. - """ - opts = self.opts - return request.user.has_perm(opts.app_label + '.' + opts.get_change_permission()) - - def has_delete_permission(self, request, obj=None): - """ - Returns True if the given request has permission to change the given - Django model instance, the default implementation doesn't examine the - `obj` parameter. - - Can be overriden by the user in subclasses. In such case it should - return True if the given request has permission to delete the `obj` - model instance. If `obj` is None, this should return True if the given - request has permission to delete *any* object of the given type. - """ - opts = self.opts - return request.user.has_perm(opts.app_label + '.' + opts.get_delete_permission()) - def get_model_perms(self, request): """ Returns a dict of all perms for this model. This dict has the keys @@ -500,7 +510,7 @@ class ModelAdmin(BaseModelAdmin): fields=self.list_editable, **defaults) def get_formsets(self, request, obj=None): - for inline in self.inline_instances: + for inline in self.get_inline_instances(request): yield inline.get_formset(request, obj) def get_paginator(self, request, queryset, per_page, orphans=0, allow_empty_first_page=True): @@ -914,6 +924,7 @@ class ModelAdmin(BaseModelAdmin): ModelForm = self.get_form(request) formsets = [] + inline_instances = self.get_inline_instances(request) if request.method == 'POST': form = ModelForm(request.POST, request.FILES) if form.is_valid(): @@ -923,7 +934,7 @@ class ModelAdmin(BaseModelAdmin): form_validated = False new_object = self.model() prefixes = {} - for FormSet, inline in zip(self.get_formsets(request), self.inline_instances): + for FormSet, inline in zip(self.get_formsets(request), inline_instances): prefix = FormSet.get_default_prefix() prefixes[prefix] = prefixes.get(prefix, 0) + 1 if prefixes[prefix] != 1 or not prefix: @@ -951,8 +962,7 @@ class ModelAdmin(BaseModelAdmin): initial[k] = initial[k].split(",") form = ModelForm(initial=initial) prefixes = {} - for FormSet, inline in zip(self.get_formsets(request), - self.inline_instances): + for FormSet, inline in zip(self.get_formsets(request), inline_instances): prefix = FormSet.get_default_prefix() prefixes[prefix] = prefixes.get(prefix, 0) + 1 if prefixes[prefix] != 1 or not prefix: @@ -968,7 +978,7 @@ class ModelAdmin(BaseModelAdmin): media = self.media + adminForm.media inline_admin_formsets = [] - for inline, formset in zip(self.inline_instances, formsets): + for inline, formset in zip(inline_instances, formsets): fieldsets = list(inline.get_fieldsets(request)) readonly = list(inline.get_readonly_fields(request)) prepopulated = dict(inline.get_prepopulated_fields(request)) @@ -1012,6 +1022,7 @@ class ModelAdmin(BaseModelAdmin): ModelForm = self.get_form(request, obj) formsets = [] + inline_instances = self.get_inline_instances(request) if request.method == 'POST': form = ModelForm(request.POST, request.FILES, instance=obj) if form.is_valid(): @@ -1021,8 +1032,7 @@ class ModelAdmin(BaseModelAdmin): form_validated = False new_object = obj prefixes = {} - for FormSet, inline in zip(self.get_formsets(request, new_object), - self.inline_instances): + for FormSet, inline in zip(self.get_formsets(request, new_object), inline_instances): prefix = FormSet.get_default_prefix() prefixes[prefix] = prefixes.get(prefix, 0) + 1 if prefixes[prefix] != 1 or not prefix: @@ -1043,7 +1053,7 @@ class ModelAdmin(BaseModelAdmin): else: form = ModelForm(instance=obj) prefixes = {} - for FormSet, inline in zip(self.get_formsets(request, obj), self.inline_instances): + for FormSet, inline in zip(self.get_formsets(request, obj), inline_instances): prefix = FormSet.get_default_prefix() prefixes[prefix] = prefixes.get(prefix, 0) + 1 if prefixes[prefix] != 1 or not prefix: @@ -1059,7 +1069,7 @@ class ModelAdmin(BaseModelAdmin): media = self.media + adminForm.media inline_admin_formsets = [] - for inline, formset in zip(self.inline_instances, formsets): + for inline, formset in zip(inline_instances, formsets): fieldsets = list(inline.get_fieldsets(request, obj)) readonly = list(inline.get_readonly_fields(request, obj)) prepopulated = dict(inline.get_prepopulated_fields(request, obj)) @@ -1377,6 +1387,7 @@ class InlineModelAdmin(BaseModelAdmin): # if exclude is an empty list we use None, since that's the actual # default exclude = exclude or None + can_delete = self.can_delete and self.has_delete_permission(request, obj) defaults = { "form": self.form, "formset": self.formset, @@ -1386,7 +1397,7 @@ class InlineModelAdmin(BaseModelAdmin): "formfield_callback": partial(self.formfield_for_dbfield, request=request), "extra": self.extra, "max_num": self.max_num, - "can_delete": self.can_delete, + "can_delete": can_delete, } defaults.update(kwargs) return inlineformset_factory(self.parent_model, self.model, **defaults) @@ -1398,6 +1409,44 @@ class InlineModelAdmin(BaseModelAdmin): fields = form.base_fields.keys() + list(self.get_readonly_fields(request, obj)) return [(None, {'fields': fields})] + def queryset(self, request): + queryset = super(InlineModelAdmin, self).queryset(request) + if not self.has_change_permission(request): + queryset = queryset.none() + return queryset + + def has_add_permission(self, request): + if self.opts.auto_created: + # We're checking the rights to an auto-created intermediate model, + # which doesn't have its own individual permissions. The user needs + # to have the change permission for the related model in order to + # be able to do anything with the intermediate model. + return self.has_change_permission(request) + return request.user.has_perm( + self.opts.app_label + '.' + self.opts.get_add_permission()) + + def has_change_permission(self, request, obj=None): + opts = self.opts + if opts.auto_created: + # The model was auto-created as intermediary for a + # ManyToMany-relationship, find the target model + for field in opts.fields: + if field.rel and field.rel.to != self.parent_model: + opts = field.rel.to._meta + break + return request.user.has_perm( + opts.app_label + '.' + opts.get_change_permission()) + + def has_delete_permission(self, request, obj=None): + if self.opts.auto_created: + # We're checking the rights to an auto-created intermediate model, + # which doesn't have its own individual permissions. The user needs + # to have the change permission for the related model in order to + # be able to do anything with the intermediate model. + return self.has_change_permission(request, obj) + return request.user.has_perm( + self.opts.app_label + '.' + self.opts.get_delete_permission()) + class StackedInline(InlineModelAdmin): template = 'admin/edit_inline/stacked.html' diff --git a/django/contrib/contenttypes/generic.py b/django/contrib/contenttypes/generic.py index f475c08812..9c6048a7ee 100644 --- a/django/contrib/contenttypes/generic.py +++ b/django/contrib/contenttypes/generic.py @@ -424,6 +424,7 @@ class GenericInlineModelAdmin(InlineModelAdmin): # GenericInlineModelAdmin doesn't define its own. exclude.extend(self.form._meta.exclude) exclude = exclude or None + can_delete = self.can_delete and self.has_delete_permission(request, obj) defaults = { "ct_field": self.ct_field, "fk_field": self.ct_fk_field, @@ -431,7 +432,7 @@ class GenericInlineModelAdmin(InlineModelAdmin): "formfield_callback": partial(self.formfield_for_dbfield, request=request), "formset": self.formset, "extra": self.extra, - "can_delete": self.can_delete, + "can_delete": can_delete, "can_order": False, "fields": fields, "max_num": self.max_num, diff --git a/docs/ref/contrib/admin/index.txt b/docs/ref/contrib/admin/index.txt index fbe4c2a741..02e539e6f2 100644 --- a/docs/ref/contrib/admin/index.txt +++ b/docs/ref/contrib/admin/index.txt @@ -1391,11 +1391,17 @@ adds some of its own (the shared features are actually defined in the - :attr:`~ModelAdmin.ordering` - :meth:`~ModelAdmin.queryset` +.. versionadded:: 1.4 + +- :meth:`~ModelAdmin.has_add_permission` +- :meth:`~ModelAdmin.has_change_permission` +- :meth:`~ModelAdmin.has_delete_permission` + The ``InlineModelAdmin`` class adds: .. attribute:: InlineModelAdmin.model - The model in which the inline is using. This is required. + The model which the inline is using. This is required. .. attribute:: InlineModelAdmin.fk_name diff --git a/docs/releases/1.4.txt b/docs/releases/1.4.txt index 5580b67dce..6a97060f40 100644 --- a/docs/releases/1.4.txt +++ b/docs/releases/1.4.txt @@ -128,6 +128,15 @@ A new :meth:`~django.contrib.admin.ModelAdmin.save_related` hook was added to :mod:`~django.contrib.admin.ModelAdmin` to ease the customization of how related objects are saved in the admin. +Admin inlines respect user permissions +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + +Admin inlines will now only allow those actions for which the user has +permission. For ``ManyToMany`` relationships with an auto-created intermediate +model (which does not have its own permissions), the change permission for the +related model determines if the user has the permission to add, change or +delete relationships. + Tools for cryptographic signing ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ diff --git a/tests/regressiontests/admin_inlines/tests.py b/tests/regressiontests/admin_inlines/tests.py index 955d6208de..7ab5ba6478 100644 --- a/tests/regressiontests/admin_inlines/tests.py +++ b/tests/regressiontests/admin_inlines/tests.py @@ -1,11 +1,12 @@ from django.contrib.admin.helpers import InlineAdminForm +from django.contrib.auth.models import User, Permission from django.contrib.contenttypes.models import ContentType from django.test import TestCase # local test models from models import (Holder, Inner, Holder2, Inner2, Holder3, Inner3, Person, OutfitItem, Fashionista, Teacher, Parent, Child, - CapoFamiglia, Consigliere, SottoCapo) + Author, Book) from admin import InnerInline @@ -141,7 +142,6 @@ class TestInline(TestCase): '') - class TestInlineMedia(TestCase): urls = "regressiontests.admin_inlines.urls" fixtures = ['admin-views-users.xml'] @@ -196,3 +196,182 @@ class TestInlineAdminForm(TestCase): iaf = InlineAdminForm(None, None, {}, {}, joe) parent_ct = ContentType.objects.get_for_model(Parent) self.assertEqual(iaf.original.content_type, parent_ct) + +class TestInlinePermissions(TestCase): + """ + Make sure the admin respects permissions for objects that are edited + inline. Refs #8060. + + """ + urls = "regressiontests.admin_inlines.urls" + + def setUp(self): + self.user = User(username='admin') + self.user.is_staff = True + self.user.is_active = True + self.user.set_password('secret') + self.user.save() + + self.author_ct = ContentType.objects.get_for_model(Author) + self.holder_ct = ContentType.objects.get_for_model(Holder2) + self.book_ct = ContentType.objects.get_for_model(Book) + self.inner_ct = ContentType.objects.get_for_model(Inner2) + + # User always has permissions to add and change Authors, and Holders, + # the main (parent) models of the inlines. Permissions on the inlines + # vary per test. + permission = Permission.objects.get(codename='add_author', content_type=self.author_ct) + self.user.user_permissions.add(permission) + permission = Permission.objects.get(codename='change_author', content_type=self.author_ct) + self.user.user_permissions.add(permission) + permission = Permission.objects.get(codename='add_holder2', content_type=self.holder_ct) + self.user.user_permissions.add(permission) + permission = Permission.objects.get(codename='change_holder2', content_type=self.holder_ct) + self.user.user_permissions.add(permission) + + author = Author.objects.create(pk=1, name=u'The Author') + author.books.create(name=u'The inline Book') + self.author_change_url = '/admin/admin_inlines/author/%i/' % author.id + + holder = Holder2.objects.create(dummy=13) + Inner2.objects.create(dummy=42, holder=holder) + self.holder_change_url = '/admin/admin_inlines/holder2/%i/' % holder.id + + self.assertEqual( + self.client.login(username='admin', password='secret'), + True) + + def tearDown(self): + self.client.logout() + + def test_inline_add_m2m_noperm(self): + response = self.client.get('/admin/admin_inlines/author/add/') + # No change permission on books, so no inline + self.assertNotContains(response, '

Author-book relationships

') + self.assertNotContains(response, 'Add another Author-Book Relationship') + self.assertNotContains(response, 'id="id_Author_books-TOTAL_FORMS"') + + def test_inline_add_fk_noperm(self): + response = self.client.get('/admin/admin_inlines/holder2/add/') + # No permissions on Inner2s, so no inline + self.assertNotContains(response, '

Inner2s

') + self.assertNotContains(response, 'Add another Inner2') + self.assertNotContains(response, 'id="id_inner2_set-TOTAL_FORMS"') + + def test_inline_change_m2m_noperm(self): + response = self.client.get(self.author_change_url) + # No change permission on books, so no inline + self.assertNotContains(response, '

Author-book relationships

') + self.assertNotContains(response, 'Add another Author-Book Relationship') + self.assertNotContains(response, 'id="id_Author_books-TOTAL_FORMS"') + + def test_inline_change_fk_noperm(self): + response = self.client.get(self.holder_change_url) + # No permissions on Inner2s, so no inline + self.assertNotContains(response, '

Inner2s

') + self.assertNotContains(response, 'Add another Inner2') + self.assertNotContains(response, 'id="id_inner2_set-TOTAL_FORMS"') + + def test_inline_add_m2m_add_perm(self): + permission = Permission.objects.get(codename='add_book', content_type=self.book_ct) + self.user.user_permissions.add(permission) + response = self.client.get('/admin/admin_inlines/author/add/') + # No change permission on Books, so no inline + self.assertNotContains(response, '

Author-book relationships

') + self.assertNotContains(response, 'Add another Author-Book Relationship') + self.assertNotContains(response, 'id="id_Author_books-TOTAL_FORMS"') + + def test_inline_add_fk_add_perm(self): + permission = Permission.objects.get(codename='add_inner2', content_type=self.inner_ct) + self.user.user_permissions.add(permission) + response = self.client.get('/admin/admin_inlines/holder2/add/') + # Add permission on inner2s, so we get the inline + self.assertContains(response, '

Inner2s

') + self.assertContains(response, 'Add another Inner2') + self.assertContains(response, 'value="3" id="id_inner2_set-TOTAL_FORMS"') + + def test_inline_change_m2m_add_perm(self): + permission = Permission.objects.get(codename='add_book', content_type=self.book_ct) + self.user.user_permissions.add(permission) + response = self.client.get(self.author_change_url) + # No change permission on books, so no inline + self.assertNotContains(response, '

Author-book relationships

') + self.assertNotContains(response, 'Add another Author-Book Relationship') + self.assertNotContains(response, 'id="id_Author_books-TOTAL_FORMS"') + self.assertNotContains(response, 'id="id_Author_books-0-DELETE"') + + def test_inline_change_m2m_change_perm(self): + permission = Permission.objects.get(codename='change_book', content_type=self.book_ct) + self.user.user_permissions.add(permission) + response = self.client.get(self.author_change_url) + # We have change perm on books, so we can add/change/delete inlines + self.assertContains(response, '

Author-book relationships

') + self.assertContains(response, 'Add another Author-Book Relationship') + self.assertContains(response, 'value="4" id="id_Author_books-TOTAL_FORMS"') + self.assertContains(response, 'Inner2s') + self.assertContains(response, 'Add another Inner2') + # 3 extra forms only, not the existing instance form + self.assertContains(response, 'value="3" id="id_inner2_set-TOTAL_FORMS"') + self.assertNotContains(response, 'Inner2s') + # Just the one form for existing instances + self.assertContains(response, 'value="1" id="id_inner2_set-TOTAL_FORMS"') + self.assertContains(response, 'Inner2s') + # One form for existing instance and three extra for new + self.assertContains(response, 'value="4" id="id_inner2_set-TOTAL_FORMS"') + self.assertContains(response, 'Inner2s') + # One form for existing instance only, no new + self.assertContains(response, 'value="1" id="id_inner2_set-TOTAL_FORMS"') + self.assertContains(response, 'Inner2s') + # One form for existing instance only, three for new + self.assertContains(response, 'value="4" id="id_inner2_set-TOTAL_FORMS"') + self.assertContains(response, '