From 825f0beda804e48e9197fcf3b0d909f9f548aa47 Mon Sep 17 00:00:00 2001 From: olivierdalang Date: Wed, 2 May 2018 20:39:12 +1200 Subject: [PATCH] Fixed #8936 -- Added a view permission and a read-only admin. Co-authored-by: Petr Dlouhy Co-authored-by: Olivier Dalang --- django/contrib/admin/helpers.py | 27 +++- django/contrib/admin/options.py | 137 ++++++++++++---- django/contrib/admin/sites.py | 3 +- .../contrib/admin/static/admin/css/base.css | 5 + .../contrib/admin/static/admin/css/forms.css | 17 ++ .../admin/static/admin/css/responsive.css | 6 +- django/contrib/admin/static/admin/css/rtl.css | 2 +- .../admin/static/admin/img/icon-viewlink.svg | 3 + .../admin/js/admin/RelatedObjectLookups.js | 2 +- .../admin/templates/admin/change_form.html | 2 +- .../templates/admin/edit_inline/stacked.html | 4 +- .../templates/admin/edit_inline/tabular.html | 4 +- .../contrib/admin/templates/admin/index.html | 6 +- .../admin/related_widget_wrapper.html | 10 +- .../admin/templates/admin/submit_line.html | 3 +- .../admin/templatetags/admin_modify.py | 20 ++- django/contrib/admin/widgets.py | 28 ++-- django/contrib/auth/management/__init__.py | 2 +- django/db/models/options.py | 2 +- docs/ref/contrib/admin/actions.txt | 3 + docs/ref/contrib/admin/index.txt | 21 ++- docs/ref/models/options.txt | 16 +- docs/releases/2.1.txt | 43 +++++ docs/topics/auth/default.txt | 4 + tests/admin_changelist/test_date_hierarchy.py | 6 + tests/admin_changelist/tests.py | 39 ++++- tests/admin_filters/tests.py | 57 ++++++- tests/admin_views/admin.py | 4 + tests/admin_views/test_templatetags.py | 4 + tests/admin_views/tests.py | 147 +++++++++++++++++- tests/auth_tests/test_management.py | 4 +- tests/modeladmin/tests.py | 44 +++++- 32 files changed, 579 insertions(+), 96 deletions(-) create mode 100644 django/contrib/admin/static/admin/img/icon-viewlink.svg diff --git a/django/contrib/admin/helpers.py b/django/contrib/admin/helpers.py index f010ebb63e..8bb3df7c43 100644 --- a/django/contrib/admin/helpers.py +++ b/django/contrib/admin/helpers.py @@ -224,7 +224,9 @@ class InlineAdminFormSet: A wrapper around an inline formset for use in the admin system. """ def __init__(self, inline, formset, fieldsets, prepopulated_fields=None, - readonly_fields=None, model_admin=None): + readonly_fields=None, model_admin=None, has_add_permission=True, + has_change_permission=True, has_delete_permission=True, + has_view_permission=True): self.opts = inline self.formset = formset self.fieldsets = fieldsets @@ -236,13 +238,21 @@ class InlineAdminFormSet: prepopulated_fields = {} self.prepopulated_fields = prepopulated_fields self.classes = ' '.join(inline.classes) if inline.classes else '' + self.has_add_permission = has_add_permission + self.has_change_permission = has_change_permission + self.has_delete_permission = has_delete_permission + self.has_view_permission = has_view_permission def __iter__(self): + readonly_fields_for_editing = self.readonly_fields + if not self.has_change_permission: + readonly_fields_for_editing += flatten_fieldsets(self.fieldsets) + for form, original in zip(self.formset.initial_forms, self.formset.get_queryset()): view_on_site_url = self.opts.get_view_on_site_url(original) yield InlineAdminForm( self.formset, form, self.fieldsets, self.prepopulated_fields, - original, self.readonly_fields, model_admin=self.opts, + original, readonly_fields_for_editing, model_admin=self.opts, view_on_site_url=view_on_site_url, ) for form in self.formset.extra_forms: @@ -250,11 +260,12 @@ class InlineAdminFormSet: self.formset, form, self.fieldsets, self.prepopulated_fields, None, self.readonly_fields, model_admin=self.opts, ) - yield InlineAdminForm( - self.formset, self.formset.empty_form, - self.fieldsets, self.prepopulated_fields, None, - self.readonly_fields, model_admin=self.opts, - ) + if self.has_add_permission: + yield InlineAdminForm( + self.formset, self.formset.empty_form, + self.fieldsets, self.prepopulated_fields, None, + self.readonly_fields, model_admin=self.opts, + ) def fields(self): fk = getattr(self.formset, "fk", None) @@ -264,7 +275,7 @@ class InlineAdminFormSet: for i, field_name in enumerate(flatten_fieldsets(self.fieldsets)): if fk and fk.name == field_name: continue - if field_name in self.readonly_fields: + if not self.has_change_permission or field_name in self.readonly_fields: yield { 'label': meta_labels.get(field_name) or label_for_field(field_name, self.opts.model, self.opts), 'widget': {'is_hidden': False}, diff --git a/django/contrib/admin/options.py b/django/contrib/admin/options.py index 7cbfde4452..e78e99f9fb 100644 --- a/django/contrib/admin/options.py +++ b/django/contrib/admin/options.py @@ -167,6 +167,7 @@ class BaseModelAdmin(metaclass=forms.MediaDefiningClass): can_add_related=related_modeladmin.has_add_permission(request), can_change_related=related_modeladmin.has_change_permission(request), can_delete_related=related_modeladmin.has_delete_permission(request), + can_view_related=related_modeladmin.has_view_permission(request), ) formfield.widget = widgets.RelatedFieldWidgetWrapper( formfield.widget, db_field.remote_field, self.admin_site, **wrapper_kwargs @@ -497,6 +498,25 @@ class BaseModelAdmin(metaclass=forms.MediaDefiningClass): codename = get_permission_codename('delete', opts) return request.user.has_perm("%s.%s" % (opts.app_label, codename)) + def has_view_permission(self, request, obj=None): + """ + Return True if the given request has permission to view the given + Django model instance. The default implementation doesn't examine the + `obj` parameter. + + If overridden by the user in subclasses, it should return True if the + given request has permission to view the `obj` model instance. If `obj` + is None, it should return True if the request has permission to view + any object of the given type. + """ + opts = self.opts + codename_view = get_permission_codename('view', opts) + codename_change = get_permission_codename('change', opts) + return ( + request.user.has_perm('%s.%s' % (opts.app_label, codename_view)) or + request.user.has_perm('%s.%s' % (opts.app_label, codename_change)) + ) + def has_module_permission(self, request): """ Return True if the given request has any permission in the given @@ -567,7 +587,8 @@ class ModelAdmin(BaseModelAdmin): else: inline_has_add_permission = inline.has_add_permission(request) if request: - if not (inline_has_add_permission or + if not (inline.has_view_permission(request, obj) or + inline_has_add_permission or inline.has_change_permission(request, obj) or inline.has_delete_permission(request, obj)): continue @@ -624,19 +645,20 @@ class ModelAdmin(BaseModelAdmin): def get_model_perms(self, request): """ Return a dict of all perms for this model. This dict has the keys - ``add``, ``change``, and ``delete`` mapping to the True/False for each - of those actions. + ``add``, ``change``, ``delete``, and ``view`` mapping to the True/False + for each of those actions. """ return { 'add': self.has_add_permission(request), 'change': self.has_change_permission(request), 'delete': self.has_delete_permission(request), + 'view': self.has_view_permission(request), } def _get_form_for_get_fields(self, request, obj): return self.get_form(request, obj, fields=None) - def get_form(self, request, obj=None, **kwargs): + def get_form(self, request, obj=None, change=False, **kwargs): """ Return a Form class for use in the admin add view. This is used by add_view and change_view. @@ -649,6 +671,10 @@ class ModelAdmin(BaseModelAdmin): exclude = [] if excluded is None else list(excluded) readonly_fields = self.get_readonly_fields(request, obj) exclude.extend(readonly_fields) + # Exclude all fields if it's a change form and the user doesn't have + # the change permission. + if change and hasattr(request, 'user') and not self.has_change_permission(request, obj): + exclude.extend(fields) if excluded is None and hasattr(self.form, '_meta') and self.form._meta.exclude: # Take the custom ModelForm's Meta.exclude into account only if the # ModelAdmin doesn't define its own. @@ -834,6 +860,9 @@ class ModelAdmin(BaseModelAdmin): # want *any* actions enabled on this page. if self.actions is None or IS_POPUP_VAR in request.GET: return OrderedDict() + # The change permission is required to use actions. + if not self.has_change_permission(request): + return OrderedDict() actions = [] @@ -1082,12 +1111,19 @@ class ModelAdmin(BaseModelAdmin): preserved_filters = self.get_preserved_filters(request) form_url = add_preserved_filters({'preserved_filters': preserved_filters, 'opts': opts}, form_url) view_on_site_url = self.get_view_on_site_url(obj) + has_editable_inline_admin_formsets = False + for inline in context['inline_admin_formsets']: + if inline.has_add_permission or inline.has_change_permission or inline.has_delete_permission: + has_editable_inline_admin_formsets = True + break context.update({ 'add': add, 'change': change, + 'has_view_permission': self.has_view_permission(request, obj), 'has_add_permission': self.has_add_permission(request), 'has_change_permission': self.has_change_permission(request, obj), 'has_delete_permission': self.has_delete_permission(request, obj), + 'has_editable_inline_admin_formsets': has_editable_inline_admin_formsets, 'has_file_field': context['adminform'].form.is_multipart() or any( admin_formset.formset.form().is_multipart() for admin_formset in context['inline_admin_formsets'] @@ -1163,11 +1199,10 @@ class ModelAdmin(BaseModelAdmin): "_saveasnew" in request.POST and self.save_as_continue and self.has_change_permission(request, obj) ): - msg = format_html( - _('The {name} "{obj}" was added successfully. You may edit it again below.'), - **msg_dict - ) - self.message_user(request, msg, messages.SUCCESS) + msg = _('The {name} "{obj}" was added successfully.') + if self.has_change_permission(request, obj): + msg += ' ' + _('You may edit it again below.') + self.message_user(request, format_html(msg, **msg_dict), messages.SUCCESS) if post_url_continue is None: post_url_continue = obj_url post_url_continue = add_preserved_filters( @@ -1438,10 +1473,15 @@ class ModelAdmin(BaseModelAdmin): for inline, formset in zip(inline_instances, formsets): fieldsets = list(inline.get_fieldsets(request, obj)) readonly = list(inline.get_readonly_fields(request, obj)) + has_add_permission = inline.has_add_permission(request, obj) + has_change_permission = inline.has_change_permission(request, obj) + has_delete_permission = inline.has_delete_permission(request, obj) + has_view_permission = inline.has_view_permission(request, obj) prepopulated = dict(inline.get_prepopulated_fields(request, obj)) inline_admin_formset = helpers.InlineAdminFormSet( - inline, formset, fieldsets, prepopulated, readonly, - model_admin=self, + inline, formset, fieldsets, prepopulated, readonly, model_admin=self, + has_add_permission=has_add_permission, has_change_permission=has_change_permission, + has_delete_permission=has_delete_permission, has_view_permission=has_view_permission, ) inline_admin_formsets.append(inline_admin_formset) return inline_admin_formsets @@ -1500,13 +1540,13 @@ class ModelAdmin(BaseModelAdmin): else: obj = self.get_object(request, unquote(object_id), to_field) - if not self.has_change_permission(request, obj): + if not self.has_view_permission(request, obj) and not self.has_change_permission(request, obj): raise PermissionDenied if obj is None: return self._get_obj_does_not_exist_redirect(request, opts, object_id) - ModelForm = self.get_form(request, obj) + ModelForm = self.get_form(request, obj, change=not add) if request.method == 'POST': form = ModelForm(request.POST, request.FILES, instance=obj) form_validated = form.is_valid() @@ -1536,11 +1576,15 @@ class ModelAdmin(BaseModelAdmin): form = ModelForm(instance=obj) formsets, inline_instances = self._create_formsets(request, obj, change=True) + if not add and not self.has_change_permission(request): + readonly_fields = flatten_fieldsets(self.get_fieldsets(request, obj)) + else: + readonly_fields = self.get_readonly_fields(request, obj) adminForm = helpers.AdminForm( form, list(self.get_fieldsets(request, obj)), self.get_prepopulated_fields(request, obj), - self.get_readonly_fields(request, obj), + readonly_fields, model_admin=self) media = self.media + adminForm.media @@ -1591,7 +1635,7 @@ class ModelAdmin(BaseModelAdmin): from django.contrib.admin.views.main import ERROR_FLAG opts = self.model._meta app_label = opts.app_label - if not self.has_change_permission(request, None): + if not self.has_view_permission(request) and not self.has_change_permission(request): raise PermissionDenied try: @@ -1620,6 +1664,8 @@ class ModelAdmin(BaseModelAdmin): # Actions with no confirmation if (actions and request.method == 'POST' and 'index' in request.POST and '_save' not in request.POST): + if not self.has_change_permission(request): + raise PermissionDenied if selected: response = self.response_action(request, queryset=cl.get_queryset(request)) if response: @@ -1636,6 +1682,8 @@ class ModelAdmin(BaseModelAdmin): if (actions and request.method == 'POST' and helpers.ACTION_CHECKBOX_NAME in request.POST and 'index' not in request.POST and '_save' not in request.POST): + if not self.has_change_permission(request): + raise PermissionDenied if selected: response = self.response_action(request, queryset=cl.get_queryset(request)) if response: @@ -1656,6 +1704,8 @@ class ModelAdmin(BaseModelAdmin): # Handle POSTed bulk-edit data. if request.method == 'POST' and cl.list_editable and '_save' in request.POST: + if not self.has_change_permission(request): + raise PermissionDenied FormSet = self.get_changelist_formset(request) formset = cl.formset = FormSet(request.POST, request.FILES, queryset=self.get_queryset(request)) if formset.is_valid(): @@ -1683,7 +1733,7 @@ class ModelAdmin(BaseModelAdmin): return HttpResponseRedirect(request.get_full_path()) # Handle GET -- construct a formset for display. - elif cl.list_editable: + elif cl.list_editable and self.has_change_permission(request): FormSet = self.get_changelist_formset(request) formset = cl.formset = FormSet(queryset=cl.result_list) @@ -1814,7 +1864,7 @@ class ModelAdmin(BaseModelAdmin): if obj is None: return self._get_obj_does_not_exist_redirect(request, model._meta, object_id) - if not self.has_change_permission(request, obj): + if not self.has_view_permission(request, obj) and not self.has_change_permission(request, obj): raise PermissionDenied # Then get the history for this object. @@ -1961,8 +2011,17 @@ class InlineModelAdmin(BaseModelAdmin): } base_model_form = defaults['form'] + can_change = self.has_change_permission(request, obj) if request else True + can_add = self.has_add_permission(request, obj) if request else True class DeleteProtectedModelForm(base_model_form): + def __init__(self, *args, **kwargs): + super(DeleteProtectedModelForm, self).__init__(*args, **kwargs) + if not can_change and not self.instance._state.adding: + self.fields = {} + if not can_add and self.instance._state.adding: + self.fields = {} + def hand_clean_DELETE(self): """ We don't validate the 'DELETE' field itself because on @@ -1972,7 +2031,7 @@ class InlineModelAdmin(BaseModelAdmin): if self.cleaned_data.get(DELETION_FIELD_NAME, False): using = router.db_for_write(self._meta.model) collector = NestedObjects(using=using) - if self.instance.pk is None: + if self.instance._state.adding: return collector.collect([self.instance]) if collector.protected: @@ -2010,7 +2069,7 @@ class InlineModelAdmin(BaseModelAdmin): def get_queryset(self, request): queryset = super().get_queryset(request) - if not self.has_change_permission(request): + if not self.has_change_permission(request) and not self.has_view_permission(request): queryset = queryset.none() return queryset @@ -2018,32 +2077,44 @@ class InlineModelAdmin(BaseModelAdmin): 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 + # to have the view 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 self.has_view_permission(request, obj) return super().has_add_permission(request) 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.remote_field and field.remote_field.model != self.parent_model: - opts = field.remote_field.model._meta - break - codename = get_permission_codename('change', opts) - return request.user.has_perm("%s.%s" % (opts.app_label, codename)) + 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 view permission for the related model in order to + # be able to do anything with the intermediate model. + return self.has_view_permission(request, obj) + return super().has_change_permission(request) 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 + # to have the view 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 self.has_view_permission(request, obj) return super().has_delete_permission(request, obj) + def has_view_permission(self, request, obj=None): + if self.opts.auto_created: + opts = self.opts + # The model was auto-created as intermediary for a many-to-many + # Many-relationship; find the target model. + for field in opts.fields: + if field.remote_field and field.remote_field.model != self.parent_model: + opts = field.remote_field.model._meta + break + return ( + request.user.has_perm('%s.%s' % (opts.app_label, get_permission_codename('view', opts))) or + request.user.has_perm('%s.%s' % (opts.app_label, get_permission_codename('change', opts))) + ) + return super().has_view_permission(request) + class StackedInline(InlineModelAdmin): template = 'admin/edit_inline/stacked.html' diff --git a/django/contrib/admin/sites.py b/django/contrib/admin/sites.py index f7d0ac0fbc..0dafe9766b 100644 --- a/django/contrib/admin/sites.py +++ b/django/contrib/admin/sites.py @@ -432,7 +432,8 @@ class AdminSite: 'object_name': model._meta.object_name, 'perms': perms, } - if perms.get('change'): + if perms.get('change') or perms.get('view'): + model_dict['view_only'] = not perms.get('change') try: model_dict['admin_url'] = reverse('admin:%s_%s_changelist' % info, current_app=self.name) except NoReverseMatch: diff --git a/django/contrib/admin/static/admin/css/base.css b/django/contrib/admin/static/admin/css/base.css index 5dfeaffe81..6551e232a2 100644 --- a/django/contrib/admin/static/admin/css/base.css +++ b/django/contrib/admin/static/admin/css/base.css @@ -662,6 +662,11 @@ div.breadcrumbs a:focus, div.breadcrumbs a:hover { /* ACTION ICONS */ +.viewlink, .inlineviewlink { + padding-left: 16px; + background: url(../img/icon-viewlink.svg) 0 1px no-repeat; +} + .addlink { padding-left: 16px; background: url(../img/icon-addlink.svg) 0 1px no-repeat; diff --git a/django/contrib/admin/static/admin/css/forms.css b/django/contrib/admin/static/admin/css/forms.css index 82930a0cd6..5db927d6cf 100644 --- a/django/contrib/admin/static/admin/css/forms.css +++ b/django/contrib/admin/static/admin/css/forms.css @@ -291,12 +291,29 @@ body.popup .submit-row { color: #fff; } +.submit-row a.closelink { + display: inline-block; + background: #bbbbbb; + border-radius: 4px; + padding: 10px 15px; + height: 15px; + line-height: 15px; + margin: 0 0 0 5px; + color: #fff; +} + .submit-row a.deletelink:focus, .submit-row a.deletelink:hover, .submit-row a.deletelink:active { background: #a41515; } +.submit-row a.closelink:focus, +.submit-row a.closelink:hover, +.submit-row a.closelink:active { + background: #aaaaaa; +} + /* CUSTOM FORM FIELDS */ .vSelectMultipleField { diff --git a/django/contrib/admin/static/admin/css/responsive.css b/django/contrib/admin/static/admin/css/responsive.css index 2a4b2bbd40..05fd2c5123 100644 --- a/django/contrib/admin/static/admin/css/responsive.css +++ b/django/contrib/admin/static/admin/css/responsive.css @@ -810,12 +810,16 @@ input[type="submit"], button { width: 100%; } - .submit-row input, .submit-row input.default, .submit-row a { + .submit-row input, .submit-row input.default, .submit-row a, .submit-row a.closelink { float: none; margin: 0 0 10px; text-align: center; } + .submit-row a.closelink { + padding: 10px 0; + } + .submit-row p.deletelink-box { order: 4; } diff --git a/django/contrib/admin/static/admin/css/rtl.css b/django/contrib/admin/static/admin/css/rtl.css index f7514a5d38..d998e7ce0a 100644 --- a/django/contrib/admin/static/admin/css/rtl.css +++ b/django/contrib/admin/static/admin/css/rtl.css @@ -35,7 +35,7 @@ th { margin-right: 1.5em; } -.addlink, .changelink { +.viewlink, .addlink, .changelink { padding-left: 0; padding-right: 16px; background-position: 100% 1px; diff --git a/django/contrib/admin/static/admin/img/icon-viewlink.svg b/django/contrib/admin/static/admin/img/icon-viewlink.svg new file mode 100644 index 0000000000..a1ca1d3f4e --- /dev/null +++ b/django/contrib/admin/static/admin/img/icon-viewlink.svg @@ -0,0 +1,3 @@ + + + diff --git a/django/contrib/admin/static/admin/js/admin/RelatedObjectLookups.js b/django/contrib/admin/static/admin/js/admin/RelatedObjectLookups.js index e6118be668..f4c57c40e5 100644 --- a/django/contrib/admin/static/admin/js/admin/RelatedObjectLookups.js +++ b/django/contrib/admin/static/admin/js/admin/RelatedObjectLookups.js @@ -58,7 +58,7 @@ function updateRelatedObjectLinks(triggeringLink) { var $this = $(triggeringLink); - var siblings = $this.nextAll('.change-related, .delete-related'); + var siblings = $this.nextAll('.view-related, .change-related, .delete-related'); if (!siblings.length) { return; } diff --git a/django/contrib/admin/templates/admin/change_form.html b/django/contrib/admin/templates/admin/change_form.html index 604747e6d9..1d749f25d3 100644 --- a/django/contrib/admin/templates/admin/change_form.html +++ b/django/contrib/admin/templates/admin/change_form.html @@ -17,7 +17,7 @@
{% trans 'Home' %} › {{ opts.app_config.verbose_name }} -› {% if has_change_permission %}{{ opts.verbose_name_plural|capfirst }}{% else %}{{ opts.verbose_name_plural|capfirst }}{% endif %} +› {% if has_view_permission %}{{ opts.verbose_name_plural|capfirst }}{% else %}{{ opts.verbose_name_plural|capfirst }}{% endif %} › {% if add %}{% blocktrans with name=opts.verbose_name %}Add {{ name }}{% endblocktrans %}{% else %}{{ original|truncatewords:"18" }}{% endif %}
{% endblock %} diff --git a/django/contrib/admin/templates/admin/edit_inline/stacked.html b/django/contrib/admin/templates/admin/edit_inline/stacked.html index 65af259a21..507f69bc56 100644 --- a/django/contrib/admin/templates/admin/edit_inline/stacked.html +++ b/django/contrib/admin/templates/admin/edit_inline/stacked.html @@ -8,8 +8,8 @@ {{ inline_admin_formset.formset.management_form }} {{ inline_admin_formset.formset.non_form_errors }} -{% for inline_admin_form in inline_admin_formset %}
-

{{ inline_admin_formset.opts.verbose_name|capfirst }}: {% if inline_admin_form.original %}{{ inline_admin_form.original }}{% if inline_admin_form.model_admin.show_change_link and inline_admin_form.model_admin.has_registered_model %} {% trans "Change" %}{% endif %} +{% for inline_admin_form in inline_admin_formset %}
+

{{ inline_admin_formset.opts.verbose_name|capfirst }}: {% if inline_admin_form.original %}{{ inline_admin_form.original }}{% if inline_admin_form.model_admin.show_change_link and inline_admin_form.model_admin.has_registered_model %} {% if inline_admin_formset.has_change_permission %}{% trans "Change" %}{% else %}{% trans "View" %}{% endif %}{% endif %} {% else %}#{{ forloop.counter }}{% endif %} {% if inline_admin_form.show_url %}{% trans "View on site" %}{% endif %} {% if inline_admin_formset.formset.can_delete and inline_admin_form.original %}{{ inline_admin_form.deletion_field.field }} {{ inline_admin_form.deletion_field.label_tag }}{% endif %} diff --git a/django/contrib/admin/templates/admin/edit_inline/tabular.html b/django/contrib/admin/templates/admin/edit_inline/tabular.html index 2f449d67af..2584745ef9 100644 --- a/django/contrib/admin/templates/admin/edit_inline/tabular.html +++ b/django/contrib/admin/templates/admin/edit_inline/tabular.html @@ -25,13 +25,13 @@ {% if inline_admin_form.form.non_field_errors %} {{ inline_admin_form.form.non_field_errors }} {% endif %} - {% if inline_admin_form.original or inline_admin_form.show_url %}

{% if inline_admin_form.original %} {{ inline_admin_form.original }} - {% if inline_admin_form.model_admin.show_change_link and inline_admin_form.model_admin.has_registered_model %}{% trans "Change" %}{% endif %} + {% if inline_admin_form.model_admin.show_change_link and inline_admin_form.model_admin.has_registered_model %}{% if inline_admin_formset.has_change_permission %}{% trans "Change" %}{% else %}{% trans "View" %}{% endif %}{% endif %} {% endif %} {% if inline_admin_form.show_url %}{% trans "View on site" %}{% endif %}

{% endif %} diff --git a/django/contrib/admin/templates/admin/index.html b/django/contrib/admin/templates/admin/index.html index 03383db8ea..2b50015886 100644 --- a/django/contrib/admin/templates/admin/index.html +++ b/django/contrib/admin/templates/admin/index.html @@ -34,7 +34,11 @@ {% endif %} {% if model.admin_url %} + {% if model.view_only %} + {% trans 'View' %} + {% else %} {% trans 'Change' %} + {% endif %} {% else %}   {% endif %} @@ -44,7 +48,7 @@

{% endfor %} {% else %} -

{% trans "You don't have permission to edit anything." %}

+

{% trans "You don't have permission to view or edit anything." %}

{% endif %}

{% endblock %} diff --git a/django/contrib/admin/templates/admin/related_widget_wrapper.html b/django/contrib/admin/templates/admin/related_widget_wrapper.html index 7b0a809392..658a7b547b 100644 --- a/django/contrib/admin/templates/admin/related_widget_wrapper.html +++ b/django/contrib/admin/templates/admin/related_widget_wrapper.html @@ -3,11 +3,17 @@ {{ widget }} {% block links %} {% spaceless %} - {% if can_change_related %} - {% trans 'Change' %} + {% else %} + title="{% blocktrans %}View selected {{ model }}{% endblocktrans %}"> + {% trans 'View' %} + {% endif %} {% endif %} {% if can_add_related %} diff --git a/django/contrib/admin/templates/admin/submit_line.html b/django/contrib/admin/templates/admin/submit_line.html index 26f3920ffa..b9467e82b7 100644 --- a/django/contrib/admin/templates/admin/submit_line.html +++ b/django/contrib/admin/templates/admin/submit_line.html @@ -8,6 +8,7 @@ {% endif %} {% if show_save_as_new %}{% endif %} {% if show_save_and_add_another %}{% endif %} -{% if show_save_and_continue %}{% endif %} +{% if show_save_and_continue %}{% endif %} +{% if show_close %}{% trans 'Close' %}{% endif %} {% endblock %} diff --git a/django/contrib/admin/templatetags/admin_modify.py b/django/contrib/admin/templatetags/admin_modify.py index 82bb6c9be2..60bc560df0 100644 --- a/django/contrib/admin/templatetags/admin_modify.py +++ b/django/contrib/admin/templatetags/admin_modify.py @@ -49,24 +49,34 @@ def submit_row(context): """ Display the row of buttons for delete and save. """ + add = context['add'] change = context['change'] is_popup = context['is_popup'] save_as = context['save_as'] show_save = context.get('show_save', True) show_save_and_continue = context.get('show_save_and_continue', True) + has_add_permission = context['has_add_permission'] + has_change_permission = context['has_change_permission'] + has_view_permission = context['has_view_permission'] + has_editable_inline_admin_formsets = context['has_editable_inline_admin_formsets'] + can_save = (has_change_permission and change) or (has_add_permission and add) or has_editable_inline_admin_formsets + can_save_and_continue = not is_popup and can_save and has_view_permission and show_save_and_continue + can_change = has_change_permission or has_editable_inline_admin_formsets ctx = Context(context) ctx.update({ + 'can_change': can_change, 'show_delete_link': ( not is_popup and context['has_delete_permission'] and change and context.get('show_delete', True) ), - 'show_save_as_new': not is_popup and change and save_as, + 'show_save_as_new': not is_popup and has_change_permission and change and save_as, 'show_save_and_add_another': ( - context['has_add_permission'] and not is_popup and - (not save_as or context['add']) + has_add_permission and not is_popup and + (not save_as or add) and can_save ), - 'show_save_and_continue': not is_popup and context['has_change_permission'] and show_save_and_continue, - 'show_save': show_save, + 'show_save_and_continue': can_save_and_continue, + 'show_save': show_save and can_save, + 'show_close': not(show_save and can_save) }) return ctx diff --git a/django/contrib/admin/widgets.py b/django/contrib/admin/widgets.py index 5af187a5c3..4ce3e053f6 100644 --- a/django/contrib/admin/widgets.py +++ b/django/contrib/admin/widgets.py @@ -239,7 +239,8 @@ class RelatedFieldWidgetWrapper(forms.Widget): template_name = 'admin/widgets/related_widget_wrapper.html' def __init__(self, widget, rel, admin_site, can_add_related=None, - can_change_related=False, can_delete_related=False): + can_change_related=False, can_delete_related=False, + can_view_related=False): self.needs_multipart_form = widget.needs_multipart_form self.attrs = widget.attrs self.choices = widget.choices @@ -256,6 +257,7 @@ class RelatedFieldWidgetWrapper(forms.Widget): # XXX: The deletion UX can be confusing when dealing with cascading deletion. cascade = getattr(rel, 'on_delete', None) is CASCADE self.can_delete_related = not multiple and not cascade and can_delete_related + self.can_view_related = not multiple and can_view_related # so we can check if the related object is registered with this AdminSite self.admin_site = admin_site @@ -292,25 +294,17 @@ class RelatedFieldWidgetWrapper(forms.Widget): 'name': name, 'url_params': url_params, 'model': rel_opts.verbose_name, + 'can_add_related': self.can_add_related, + 'can_change_related': self.can_change_related, + 'can_delete_related': self.can_delete_related, + 'can_view_related': self.can_view_related, } - if self.can_change_related: - change_related_template_url = self.get_related_url(info, 'change', '__fk__') - context.update( - can_change_related=True, - change_related_template_url=change_related_template_url, - ) if self.can_add_related: - add_related_url = self.get_related_url(info, 'add') - context.update( - can_add_related=True, - add_related_url=add_related_url, - ) + context['add_related_url'] = self.get_related_url(info, 'add') if self.can_delete_related: - delete_related_template_url = self.get_related_url(info, 'delete', '__fk__') - context.update( - can_delete_related=True, - delete_related_template_url=delete_related_template_url, - ) + context['delete_related_template_url'] = self.get_related_url(info, 'delete', '__fk__') + if self.can_view_related or self.can_change_related: + context['change_related_template_url'] = self.get_related_url(info, 'change', '__fk__') return context def value_from_datadict(self, data, files, name): diff --git a/django/contrib/auth/management/__init__.py b/django/contrib/auth/management/__init__.py index 36a2a7038f..cdf7203a9d 100644 --- a/django/contrib/auth/management/__init__.py +++ b/django/contrib/auth/management/__init__.py @@ -22,7 +22,7 @@ def _get_all_permissions(opts): def _get_builtin_permissions(opts): """ Return (codename, name) for all autogenerated permissions. - By default, this is ('add', 'change', 'delete') + By default, this is ('add', 'change', 'delete', 'view') """ perms = [] for action in opts.default_permissions: diff --git a/django/db/models/options.py b/django/db/models/options.py index 5364383076..c0c925375f 100644 --- a/django/db/models/options.py +++ b/django/db/models/options.py @@ -92,7 +92,7 @@ class Options: self.unique_together = [] self.index_together = [] self.select_on_save = False - self.default_permissions = ('add', 'change', 'delete') + self.default_permissions = ('add', 'change', 'delete', 'view') self.permissions = [] self.object_name = None self.app_label = app_label diff --git a/docs/ref/contrib/admin/actions.txt b/docs/ref/contrib/admin/actions.txt index 0eb6de5b11..88fcd60751 100644 --- a/docs/ref/contrib/admin/actions.txt +++ b/docs/ref/contrib/admin/actions.txt @@ -340,6 +340,9 @@ Conditionally enabling or disabling actions Finally, you can conditionally enable or disable actions on a per-request (and hence per-user basis) by overriding :meth:`ModelAdmin.get_actions`. + This doesn't return any actions if the user doesn't have the "change" + permission for the model. + This returns a dictionary of actions allowed. The keys are action names, and the values are ``(function, name, short_description)`` tuples. diff --git a/docs/ref/contrib/admin/index.txt b/docs/ref/contrib/admin/index.txt index abaeeb5c22..9b0a7cc8a4 100644 --- a/docs/ref/contrib/admin/index.txt +++ b/docs/ref/contrib/admin/index.txt @@ -1623,7 +1623,7 @@ templates used by the :class:`ModelAdmin` views: a ``list`` or ``tuple`` of :class:`~django.contrib.admin.InlineModelAdmin` objects, as described below in the :class:`~django.contrib.admin.InlineModelAdmin` section. For example, the following would return inlines without the default - filtering based on add, change, and delete permissions:: + filtering based on add, change, delete, and view permissions:: class MyModelAdmin(admin.ModelAdmin): inlines = (MyInline,) @@ -1887,6 +1887,19 @@ templates used by the :class:`ModelAdmin` views: Override this method to customize the lookups permitted for your :class:`~django.contrib.admin.ModelAdmin` subclass. +.. method:: ModelAdmin.has_view_permission(request, obj=None) + + .. versionadded:: 2.1 + + Should return ``True`` if viewing ``obj`` is permitted, ``False`` otherwise. + If obj is ``None``, should return ``True`` or ``False`` to indicate whether + viewing of objects of this type is permitted in general (e.g., ``False`` + will be interpreted as meaning that the current user is not permitted to + view any object of this type). + + The default implementation returns ``True`` if the user has either the + "change" or "view" permission. + .. method:: ModelAdmin.has_add_permission(request) Should return ``True`` if adding an object is permitted, ``False`` @@ -1914,7 +1927,8 @@ templates used by the :class:`ModelAdmin` views: accessing the module's index page is permitted, ``False`` otherwise. Uses :meth:`User.has_module_perms() ` by default. Overriding - it does not restrict access to the add, change or delete views, + it does not restrict access to the view, add, change, or delete views, + :meth:`~ModelAdmin.has_view_permission`, :meth:`~ModelAdmin.has_add_permission`, :meth:`~ModelAdmin.has_change_permission`, and :meth:`~ModelAdmin.has_delete_permission` should be used for that. @@ -2862,7 +2876,8 @@ Templates can override or extend base admin templates as described in * ``object_name``: class name of the model * ``name``: plural name of the model - * ``perms``: a ``dict`` tracking ``add``, ``change``, and ``delete`` permissions + * ``perms``: a ``dict`` tracking ``add``, ``change``, ``delete``, and + ``view`` permissions * ``admin_url``: admin changelist URL for the model * ``add_url``: admin URL to add a new model instance diff --git a/docs/ref/models/options.txt b/docs/ref/models/options.txt index fe51a1a362..51a87fc632 100644 --- a/docs/ref/models/options.txt +++ b/docs/ref/models/options.txt @@ -313,7 +313,7 @@ Django quotes column and table names behind the scenes. .. attribute:: Options.permissions Extra permissions to enter into the permissions table when creating this object. - Add, delete and change permissions are automatically created for each + Add, change, delete, and view permissions are automatically created for each model. This example specifies an extra permission, ``can_deliver_pizzas``:: permissions = (("can_deliver_pizzas", "Can deliver pizzas"),) @@ -326,11 +326,15 @@ Django quotes column and table names behind the scenes. .. attribute:: Options.default_permissions - Defaults to ``('add', 'change', 'delete')``. You may customize this list, - for example, by setting this to an empty list if your app doesn't require - any of the default permissions. It must be specified on the model before - the model is created by :djadmin:`migrate` in order to prevent any omitted - permissions from being created. + Defaults to ``('add', 'change', 'delete', 'view')``. You may customize this + list, for example, by setting this to an empty list if your app doesn't + require any of the default permissions. It must be specified on the model + before the model is created by :djadmin:`migrate` in order to prevent any + omitted permissions from being created. + + .. versionchanged:: 2.1 + + The ``view`` permission was added. ``proxy`` --------- diff --git a/docs/releases/2.1.txt b/docs/releases/2.1.txt index 083488491c..16641d1923 100644 --- a/docs/releases/2.1.txt +++ b/docs/releases/2.1.txt @@ -26,6 +26,21 @@ latest release of each series. What's new in Django 2.1 ======================== +Model "view" permission +----------------------- + +A "view" permission is added to the model :attr:`Meta.default_permissions +`. The new permissions will be +create automatically when running :djadmin:`migrate`. + +This allows giving users read-only access to models in the admin. +:meth:`.ModelAdmin.has_view_permission` is new. The implementation is backwards +compatible in that there isn't a need to assign the "view" permission to allow +users who have the "change" permission to edit objects. + +There are a couple of :ref:`backwards incompatible considerations +`. + Minor features -------------- @@ -372,6 +387,34 @@ cross-origin requests. If you rely on the old behavior, set the :setting:`SESSION_COOKIE_SAMESITE` and/or :setting:`CSRF_COOKIE_SAMESITE` setting to ``None``. +.. _view_permission_backwards_incompatible: + +Considerations for the new model "view" permission +-------------------------------------------------- + +Custom admin forms need to take the view-only case into account +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + +With the new "view" permission, existing custom admin forms may raise errors +when a user doesn't have the change permission because the form might access +nonexistent fields. Fix this by overriding :meth:`.ModelAdmin.get_form` and +checking if the user has the "change" permissions and returning the default +form if not:: + + class MyAdmin(admin.ModelAdmin): + def get_form(self, request, obj=None, **kwargs): + if not self.has_change_permission(request, obj): + return super().get_form(request, obj, **kwargs) + return CustomForm + +New default view permission could allow unwanted access to admin views +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + +If you have a custom permission with a codename of the form +``can_view_``, the new view permission handling in the admin will +allow view access to the changelist and detail pages for those models. If this +is unwanted, you must change your custom permission codename. + Miscellaneous ------------- diff --git a/docs/topics/auth/default.txt b/docs/topics/auth/default.txt index 9f864c64d7..94a8fc592f 100644 --- a/docs/topics/auth/default.txt +++ b/docs/topics/auth/default.txt @@ -158,6 +158,8 @@ code. The Django admin site uses permissions as follows: +* Access to view objects is limited to users with the "view" or "change" + permission for that type of object. * Access to view the "add" form and add an object is limited to users with the "add" permission for that type of object. * Access to view the change list, view the "change" form and change an @@ -168,6 +170,7 @@ The Django admin site uses permissions as follows: Permissions can be set not only per type of object, but also per specific object instance. By using the +:meth:`~django.contrib.admin.ModelAdmin.has_view_permission`, :meth:`~django.contrib.admin.ModelAdmin.has_add_permission`, :meth:`~django.contrib.admin.ModelAdmin.has_change_permission` and :meth:`~django.contrib.admin.ModelAdmin.has_delete_permission` methods provided @@ -213,6 +216,7 @@ to test for basic permissions you should use: * add: ``user.has_perm('foo.add_bar')`` * change: ``user.has_perm('foo.change_bar')`` * delete: ``user.has_perm('foo.delete_bar')`` +* view: ``user.has_perm('foo.view_bar')`` The :class:`~django.contrib.auth.models.Permission` model is rarely accessed directly. diff --git a/tests/admin_changelist/test_date_hierarchy.py b/tests/admin_changelist/test_date_hierarchy.py index 96590ccc82..f19e38f9bf 100644 --- a/tests/admin_changelist/test_date_hierarchy.py +++ b/tests/admin_changelist/test_date_hierarchy.py @@ -1,6 +1,7 @@ from datetime import datetime from django.contrib.admin.options import IncorrectLookupParameters +from django.contrib.auth.models import User from django.test import RequestFactory, TestCase from django.utils.timezone import make_aware @@ -11,9 +12,14 @@ from .models import Event class DateHierarchyTests(TestCase): factory = RequestFactory() + @classmethod + def setUpTestData(cls): + cls.superuser = User.objects.create_superuser(username='super', email='a@b.com', password='xxx') + def assertDateParams(self, query, expected_from_date, expected_to_date): query = {'date__%s' % field: val for field, val in query.items()} request = self.factory.get('/', query) + request.user = self.superuser changelist = EventAdmin(Event, custom_site).get_changelist_instance(request) _, _, lookup_params, _ = changelist.get_filters(request) self.assertEqual(lookup_params['date__gte'], expected_from_date) diff --git a/tests/admin_changelist/tests.py b/tests/admin_changelist/tests.py index 68aaa78996..20587613a7 100644 --- a/tests/admin_changelist/tests.py +++ b/tests/admin_changelist/tests.py @@ -50,6 +50,7 @@ class ChangeListTests(TestCase): def setUp(self): self.factory = RequestFactory() + self.superuser = User.objects.create_superuser(username='super', email='a@b.com', password='xxx') def _create_superuser(self, username): return User.objects.create_superuser(username=username, email='a@b.com', password='xxx') @@ -70,6 +71,7 @@ class ChangeListTests(TestCase): m = OrderedByFBandAdmin(Band, custom_site) request = self.factory.get('/band/') + request.user = self.superuser cl = m.get_changelist_instance(request) self.assertEqual(cl.get_ordering_field_columns(), {3: 'desc', 2: 'asc'}) @@ -80,12 +82,14 @@ class ChangeListTests(TestCase): """ m = ChildAdmin(Child, custom_site) request = self.factory.get('/child/') + request.user = self.superuser cl = m.get_changelist_instance(request) self.assertEqual(cl.queryset.query.select_related, {'parent': {}}) def test_select_related_as_tuple(self): ia = InvitationAdmin(Invitation, custom_site) request = self.factory.get('/invitation/') + request.user = self.superuser cl = ia.get_changelist_instance(request) self.assertEqual(cl.queryset.query.select_related, {'player': {}}) @@ -93,6 +97,7 @@ class ChangeListTests(TestCase): ia = InvitationAdmin(Invitation, custom_site) ia.list_select_related = () request = self.factory.get('/invitation/') + request.user = self.superuser cl = ia.get_changelist_instance(request) self.assertIs(cl.queryset.query.select_related, False) @@ -105,6 +110,7 @@ class ChangeListTests(TestCase): ia = GetListSelectRelatedAdmin(Invitation, custom_site) request = self.factory.get('/invitation/') + request.user = self.superuser cl = ia.get_changelist_instance(request) self.assertEqual(cl.queryset.query.select_related, {'player': {}, 'band': {}}) @@ -115,6 +121,7 @@ class ChangeListTests(TestCase): """ new_child = Child.objects.create(name='name', parent=None) request = self.factory.get('/child/') + request.user = self.superuser m = ChildAdmin(Child, custom_site) cl = m.get_changelist_instance(request) cl.formset = None @@ -131,6 +138,7 @@ class ChangeListTests(TestCase): """ new_child = Child.objects.create(name='name', parent=None) request = self.factory.get('/child/') + request.user = self.superuser # Set a new empty display value on AdminSite. admin.site.empty_value_display = '???' m = ChildAdmin(Child, admin.site) @@ -149,6 +157,7 @@ class ChangeListTests(TestCase): """ new_child = Child.objects.create(name='name', parent=None) request = self.factory.get('/child/') + request.user = self.superuser m = EmptyValueChildAdmin(Child, admin.site) cl = m.get_changelist_instance(request) cl.formset = None @@ -172,6 +181,7 @@ class ChangeListTests(TestCase): new_parent = Parent.objects.create(name='parent') new_child = Child.objects.create(name='name', parent=new_parent) request = self.factory.get('/child/') + request.user = self.superuser m = ChildAdmin(Child, custom_site) cl = m.get_changelist_instance(request) cl.formset = None @@ -194,6 +204,7 @@ class ChangeListTests(TestCase): new_parent = Parent.objects.create(name='parent') new_child = Child.objects.create(name='name', parent=new_parent) request = self.factory.get('/child/') + request.user = self.superuser m = ChildAdmin(Child, custom_site) # Test with list_editable fields @@ -233,6 +244,7 @@ class ChangeListTests(TestCase): for i in range(200): Child.objects.create(name='name %s' % i, parent=new_parent) request = self.factory.get('/child/', data={'p': -1}) # Anything outside range + request.user = self.superuser m = ChildAdmin(Child, custom_site) # Test with list_editable fields @@ -248,6 +260,7 @@ class ChangeListTests(TestCase): Child.objects.create(name='name %s' % i, parent=new_parent) request = self.factory.get('/child/') + request.user = self.superuser m = CustomPaginationAdmin(Child, custom_site) cl = m.get_changelist_instance(request) @@ -267,6 +280,7 @@ class ChangeListTests(TestCase): m = BandAdmin(Band, custom_site) request = self.factory.get('/band/', data={'genres': blues.pk}) + request.user = self.superuser cl = m.get_changelist_instance(request) cl.get_results(request) @@ -286,6 +300,7 @@ class ChangeListTests(TestCase): m = GroupAdmin(Group, custom_site) request = self.factory.get('/group/', data={'members': lead.pk}) + request.user = self.superuser cl = m.get_changelist_instance(request) cl.get_results(request) @@ -307,6 +322,7 @@ class ChangeListTests(TestCase): m = ConcertAdmin(Concert, custom_site) request = self.factory.get('/concert/', data={'group__members': lead.pk}) + request.user = self.superuser cl = m.get_changelist_instance(request) cl.get_results(request) @@ -327,6 +343,7 @@ class ChangeListTests(TestCase): m = QuartetAdmin(Quartet, custom_site) request = self.factory.get('/quartet/', data={'members': lead.pk}) + request.user = self.superuser cl = m.get_changelist_instance(request) cl.get_results(request) @@ -347,6 +364,7 @@ class ChangeListTests(TestCase): m = ChordsBandAdmin(ChordsBand, custom_site) request = self.factory.get('/chordsband/', data={'members': lead.pk}) + request.user = self.superuser cl = m.get_changelist_instance(request) cl.get_results(request) @@ -366,6 +384,7 @@ class ChangeListTests(TestCase): m = ParentAdmin(Parent, custom_site) request = self.factory.get('/parent/', data={'child__name': 'Daniel'}) + request.user = self.superuser cl = m.get_changelist_instance(request) # Make sure distinct() was called @@ -382,6 +401,7 @@ class ChangeListTests(TestCase): m = ParentAdmin(Parent, custom_site) request = self.factory.get('/parent/', data={SEARCH_VAR: 'daniel'}) + request.user = self.superuser cl = m.get_changelist_instance(request) # Make sure distinct() was called @@ -401,6 +421,7 @@ class ChangeListTests(TestCase): m = ConcertAdmin(Concert, custom_site) request = self.factory.get('/concert/', data={SEARCH_VAR: 'vox'}) + request.user = self.superuser cl = m.get_changelist_instance(request) # There's only one Concert instance @@ -414,10 +435,12 @@ class ChangeListTests(TestCase): m.search_fields = ['group__pk'] request = self.factory.get('/concert/', data={SEARCH_VAR: band.pk}) + request.user = self.superuser cl = m.get_changelist_instance(request) self.assertEqual(cl.queryset.count(), 1) request = self.factory.get('/concert/', data={SEARCH_VAR: band.pk + 5}) + request.user = self.superuser cl = m.get_changelist_instance(request) self.assertEqual(cl.queryset.count(), 0) @@ -429,10 +452,12 @@ class ChangeListTests(TestCase): m.search_fields = ['name__iexact'] request = self.factory.get('/', data={SEARCH_VAR: 'woodstock'}) + request.user = self.superuser cl = m.get_changelist_instance(request) self.assertCountEqual(cl.queryset, [concert]) request = self.factory.get('/', data={SEARCH_VAR: 'wood'}) + request.user = self.superuser cl = m.get_changelist_instance(request) self.assertCountEqual(cl.queryset, []) @@ -445,10 +470,12 @@ class ChangeListTests(TestCase): Field.register_lookup(Contains, 'cc') try: request = self.factory.get('/', data={SEARCH_VAR: 'Hype'}) + request.user = self.superuser cl = m.get_changelist_instance(request) self.assertCountEqual(cl.queryset, [concert]) request = self.factory.get('/', data={SEARCH_VAR: 'Woodstock'}) + request.user = self.superuser cl = m.get_changelist_instance(request) self.assertCountEqual(cl.queryset, []) finally: @@ -467,10 +494,12 @@ class ChangeListTests(TestCase): m.search_fields = ['group__members__age__exactly'] request = self.factory.get('/', data={SEARCH_VAR: '20'}) + request.user = self.superuser cl = m.get_changelist_instance(request) self.assertCountEqual(cl.queryset, [concert]) request = self.factory.get('/', data={SEARCH_VAR: '21'}) + request.user = self.superuser cl = m.get_changelist_instance(request) self.assertCountEqual(cl.queryset, []) finally: @@ -486,10 +515,12 @@ class ChangeListTests(TestCase): m.search_fields = ['pk__exact'] request = self.factory.get('/', data={SEARCH_VAR: 'abc'}) + request.user = self.superuser cl = m.get_changelist_instance(request) self.assertCountEqual(cl.queryset, [abc]) request = self.factory.get('/', data={SEARCH_VAR: 'abcd'}) + request.user = self.superuser cl = m.get_changelist_instance(request) self.assertCountEqual(cl.queryset, [abcd]) @@ -501,11 +532,13 @@ class ChangeListTests(TestCase): m = BandAdmin(Band, custom_site) for lookup_params in ({}, {'name': 'test'}): request = self.factory.get('/band/', lookup_params) + request.user = self.superuser cl = m.get_changelist_instance(request) self.assertFalse(cl.queryset.query.distinct) # A ManyToManyField in params does have distinct applied. request = self.factory.get('/band/', {'genres': '0'}) + request.user = self.superuser cl = m.get_changelist_instance(request) self.assertTrue(cl.queryset.query.distinct) @@ -520,6 +553,7 @@ class ChangeListTests(TestCase): Child.objects.create(name='filtered %s' % i, parent=parent) request = self.factory.get('/child/') + request.user = self.superuser # Test default queryset m = ChildAdmin(Child, custom_site) @@ -540,8 +574,7 @@ class ChangeListTests(TestCase): Regression test for #13196: output of functions should be localized in the changelist. """ - superuser = User.objects.create_superuser(username='super', email='super@localhost', password='secret') - self.client.force_login(superuser) + self.client.force_login(self.superuser) event = Event.objects.create(date=datetime.date.today()) response = self.client.get(reverse('admin:admin_changelist_event_changelist')) self.assertContains(response, formats.localize(event.date)) @@ -597,6 +630,7 @@ class ChangeListTests(TestCase): # Add "show all" parameter to request request = self.factory.get('/child/', data={ALL_VAR: ''}) + request.user = self.superuser # Test valid "show all" request (number of total objects is under max) m = ChildAdmin(Child, custom_site) @@ -856,6 +890,7 @@ class ChangeListTests(TestCase): # instantiating and setting up ChangeList object m = GroupAdmin(Group, custom_site) request = self.factory.get('/group/') + request.user = self.superuser cl = m.get_changelist_instance(request) per_page = cl.list_per_page = 10 diff --git a/tests/admin_filters/tests.py b/tests/admin_filters/tests.py index 554b1a4e23..2e16909dc3 100644 --- a/tests/admin_filters/tests.py +++ b/tests/admin_filters/tests.py @@ -263,7 +263,7 @@ class ListFiltersTests(TestCase): self.request_factory = RequestFactory() # Users - self.alfred = User.objects.create_user('alfred', 'alfred@example.com') + self.alfred = User.objects.create_superuser('alfred', 'alfred@example.com', 'password') self.bob = User.objects.create_user('bob', 'bob@example.com') self.lisa = User.objects.create_user('lisa', 'lisa@example.com') @@ -308,6 +308,7 @@ class ListFiltersTests(TestCase): modeladmin = BookmarkChoicesAdmin(Bookmark, site) request = self.request_factory.get('/', {}) + request.user = self.alfred changelist = modeladmin.get_changelist_instance(request) filterspec = changelist.get_filters(request)[0][0] choices = list(filterspec.choices(changelist)) @@ -318,10 +319,12 @@ class ListFiltersTests(TestCase): modeladmin = BookAdmin(Book, site) request = self.request_factory.get('/') + request.user = self.alfred changelist = modeladmin.get_changelist(request) request = self.request_factory.get('/', {'date_registered__gte': self.today, 'date_registered__lt': self.tomorrow}) + request.user = self.alfred changelist = modeladmin.get_changelist_instance(request) # Make sure the correct queryset is returned @@ -343,6 +346,7 @@ class ListFiltersTests(TestCase): request = self.request_factory.get('/', {'date_registered__gte': self.today.replace(day=1), 'date_registered__lt': self.next_month}) + request.user = self.alfred changelist = modeladmin.get_changelist_instance(request) # Make sure the correct queryset is returned @@ -368,6 +372,7 @@ class ListFiltersTests(TestCase): request = self.request_factory.get('/', {'date_registered__gte': self.today.replace(month=1, day=1), 'date_registered__lt': self.next_year}) + request.user = self.alfred changelist = modeladmin.get_changelist_instance(request) # Make sure the correct queryset is returned @@ -395,6 +400,7 @@ class ListFiltersTests(TestCase): 'date_registered__gte': str(self.one_week_ago), 'date_registered__lt': str(self.tomorrow), }) + request.user = self.alfred changelist = modeladmin.get_changelist_instance(request) # Make sure the correct queryset is returned @@ -416,6 +422,7 @@ class ListFiltersTests(TestCase): # Null/not null queries request = self.request_factory.get('/', {'date_registered__isnull': 'True'}) + request.user = self.alfred changelist = modeladmin.get_changelist_instance(request) # Make sure the correct queryset is returned @@ -431,6 +438,7 @@ class ListFiltersTests(TestCase): self.assertEqual(choice['query_string'], '?date_registered__isnull=True') request = self.request_factory.get('/', {'date_registered__isnull': 'False'}) + request.user = self.alfred changelist = modeladmin.get_changelist_instance(request) # Make sure the correct queryset is returned @@ -459,6 +467,7 @@ class ListFiltersTests(TestCase): modeladmin = BookAdmin(Book, site) request = self.request_factory.get('/', {'year__isnull': 'True'}) + request.user = self.alfred changelist = modeladmin.get_changelist_instance(request) # Make sure the correct queryset is returned @@ -473,6 +482,7 @@ class ListFiltersTests(TestCase): self.assertEqual(choices[-1]['query_string'], '?year__isnull=True') request = self.request_factory.get('/', {'year': '2002'}) + request.user = self.alfred changelist = modeladmin.get_changelist_instance(request) # Make sure the correct choice is selected @@ -486,6 +496,7 @@ class ListFiltersTests(TestCase): # Make sure that correct filters are returned with custom querysets modeladmin = BookAdminWithCustomQueryset(self.alfred, Book, site) request = self.request_factory.get('/') + request.user = self.alfred changelist = modeladmin.get_changelist_instance(request) filterspec = changelist.get_filters(request)[0][0] @@ -502,6 +513,7 @@ class ListFiltersTests(TestCase): modeladmin = BookAdmin(Book, site) request = self.request_factory.get('/') + request.user = self.alfred changelist = modeladmin.get_changelist_instance(request) # Make sure that all users are present in the author's list filter @@ -510,6 +522,7 @@ class ListFiltersTests(TestCase): self.assertEqual(sorted(filterspec.lookup_choices), sorted(expected)) request = self.request_factory.get('/', {'author__isnull': 'True'}) + request.user = self.alfred changelist = modeladmin.get_changelist_instance(request) # Make sure the correct queryset is returned @@ -524,6 +537,7 @@ class ListFiltersTests(TestCase): self.assertEqual(choices[-1]['query_string'], '?author__isnull=True') request = self.request_factory.get('/', {'author__id__exact': self.alfred.pk}) + request.user = self.alfred changelist = modeladmin.get_changelist_instance(request) # Make sure the correct choice is selected @@ -538,6 +552,7 @@ class ListFiltersTests(TestCase): modeladmin = BookAdmin(Book, site) request = self.request_factory.get('/') + request.user = self.alfred changelist = modeladmin.get_changelist_instance(request) # Make sure that all users are present in the contrib's list filter @@ -546,6 +561,7 @@ class ListFiltersTests(TestCase): self.assertEqual(sorted(filterspec.lookup_choices), sorted(expected)) request = self.request_factory.get('/', {'contributors__isnull': 'True'}) + request.user = self.alfred changelist = modeladmin.get_changelist_instance(request) # Make sure the correct queryset is returned @@ -560,6 +576,7 @@ class ListFiltersTests(TestCase): self.assertEqual(choices[-1]['query_string'], '?contributors__isnull=True') request = self.request_factory.get('/', {'contributors__id__exact': self.bob.pk}) + request.user = self.alfred changelist = modeladmin.get_changelist_instance(request) # Make sure the correct choice is selected @@ -574,6 +591,7 @@ class ListFiltersTests(TestCase): # FK relationship ----- request = self.request_factory.get('/', {'books_authored__isnull': 'True'}) + request.user = self.alfred changelist = modeladmin.get_changelist_instance(request) # Make sure the correct queryset is returned @@ -588,6 +606,7 @@ class ListFiltersTests(TestCase): self.assertEqual(choices[-1]['query_string'], '?books_authored__isnull=True') request = self.request_factory.get('/', {'books_authored__id__exact': self.bio_book.pk}) + request.user = self.alfred changelist = modeladmin.get_changelist_instance(request) # Make sure the correct choice is selected @@ -599,6 +618,7 @@ class ListFiltersTests(TestCase): # M2M relationship ----- request = self.request_factory.get('/', {'books_contributed__isnull': 'True'}) + request.user = self.alfred changelist = modeladmin.get_changelist_instance(request) # Make sure the correct queryset is returned @@ -613,6 +633,7 @@ class ListFiltersTests(TestCase): self.assertEqual(choices[-1]['query_string'], '?books_contributed__isnull=True') request = self.request_factory.get('/', {'books_contributed__id__exact': self.django_book.pk}) + request.user = self.alfred changelist = modeladmin.get_changelist_instance(request) # Make sure the correct choice is selected @@ -636,6 +657,7 @@ class ListFiltersTests(TestCase): modeladmin = BookAdminRelatedOnlyFilter(Book, site) request = self.request_factory.get('/') + request.user = self.alfred changelist = modeladmin.get_changelist_instance(request) # Make sure that only actual authors are present in author's list filter @@ -652,6 +674,7 @@ class ListFiltersTests(TestCase): modeladmin = BookAdminRelatedOnlyFilter(Book, site) request = self.request_factory.get('/') + request.user = self.alfred changelist = modeladmin.get_changelist_instance(request) # Only actual departments should be present in employee__department's @@ -667,6 +690,7 @@ class ListFiltersTests(TestCase): modeladmin = BookAdminRelatedOnlyFilter(Book, site) request = self.request_factory.get('/') + request.user = self.alfred changelist = modeladmin.get_changelist_instance(request) # Make sure that only actual contributors are present in contrib's list filter @@ -686,6 +710,7 @@ class ListFiltersTests(TestCase): modeladmin = BookmarkAdminGenericRelation(Bookmark, site) request = self.request_factory.get('/', {'tags__tag': 'python'}) + request.user = self.alfred changelist = modeladmin.get_changelist_instance(request) queryset = changelist.get_queryset(request) @@ -702,9 +727,11 @@ class ListFiltersTests(TestCase): def verify_booleanfieldlistfilter(self, modeladmin): request = self.request_factory.get('/') + request.user = self.alfred changelist = modeladmin.get_changelist_instance(request) request = self.request_factory.get('/', {'is_best_seller__exact': 0}) + request.user = self.alfred changelist = modeladmin.get_changelist_instance(request) # Make sure the correct queryset is returned @@ -719,6 +746,7 @@ class ListFiltersTests(TestCase): self.assertEqual(choice['query_string'], '?is_best_seller__exact=0') request = self.request_factory.get('/', {'is_best_seller__exact': 1}) + request.user = self.alfred changelist = modeladmin.get_changelist_instance(request) # Make sure the correct queryset is returned @@ -733,6 +761,7 @@ class ListFiltersTests(TestCase): self.assertEqual(choice['query_string'], '?is_best_seller__exact=1') request = self.request_factory.get('/', {'is_best_seller__isnull': 'True'}) + request.user = self.alfred changelist = modeladmin.get_changelist_instance(request) # Make sure the correct queryset is returned @@ -750,9 +779,11 @@ class ListFiltersTests(TestCase): modeladmin = BookAdmin2(Book, site) request = self.request_factory.get('/') + request.user = self.alfred changelist = modeladmin.get_changelist_instance(request) request = self.request_factory.get('/', {'is_best_seller2__exact': 0}) + request.user = self.alfred changelist = modeladmin.get_changelist_instance(request) # Make sure the correct queryset is returned @@ -767,6 +798,7 @@ class ListFiltersTests(TestCase): self.assertEqual(choice['query_string'], '?is_best_seller2__exact=0') request = self.request_factory.get('/', {'is_best_seller2__exact': 1}) + request.user = self.alfred changelist = modeladmin.get_changelist_instance(request) # Make sure the correct queryset is returned @@ -781,6 +813,7 @@ class ListFiltersTests(TestCase): self.assertEqual(choice['query_string'], '?is_best_seller2__exact=1') request = self.request_factory.get('/', {'is_best_seller2__isnull': 'True'}) + request.user = self.alfred changelist = modeladmin.get_changelist_instance(request) # Make sure the correct queryset is returned @@ -801,9 +834,11 @@ class ListFiltersTests(TestCase): """ modeladmin = BookAdminWithUnderscoreLookupAndTuple(Book, site) request = self.request_factory.get('/') + request.user = self.alfred changelist = modeladmin.get_changelist_instance(request) request = self.request_factory.get('/', {'author__email': 'alfred@example.com'}) + request.user = self.alfred changelist = modeladmin.get_changelist_instance(request) # Make sure the correct queryset is returned @@ -814,6 +849,7 @@ class ListFiltersTests(TestCase): """Filtering by an invalid value.""" modeladmin = BookAdmin(Book, site) request = self.request_factory.get('/', {'author__id__exact': 'StringNotInteger!'}) + request.user = self.alfred with self.assertRaises(IncorrectLookupParameters): modeladmin.get_changelist_instance(request) @@ -822,6 +858,7 @@ class ListFiltersTests(TestCase): # Make sure that the first option is 'All' --------------------------- request = self.request_factory.get('/', {}) + request.user = self.alfred changelist = modeladmin.get_changelist_instance(request) # Make sure the correct queryset is returned @@ -838,6 +875,7 @@ class ListFiltersTests(TestCase): # Look for books in the 1980s ---------------------------------------- request = self.request_factory.get('/', {'publication-decade': 'the 80s'}) + request.user = self.alfred changelist = modeladmin.get_changelist_instance(request) # Make sure the correct queryset is returned @@ -854,6 +892,7 @@ class ListFiltersTests(TestCase): # Look for books in the 1990s ---------------------------------------- request = self.request_factory.get('/', {'publication-decade': 'the 90s'}) + request.user = self.alfred changelist = modeladmin.get_changelist_instance(request) # Make sure the correct queryset is returned @@ -870,6 +909,7 @@ class ListFiltersTests(TestCase): # Look for books in the 2000s ---------------------------------------- request = self.request_factory.get('/', {'publication-decade': 'the 00s'}) + request.user = self.alfred changelist = modeladmin.get_changelist_instance(request) # Make sure the correct queryset is returned @@ -886,6 +926,7 @@ class ListFiltersTests(TestCase): # Combine multiple filters ------------------------------------------- request = self.request_factory.get('/', {'publication-decade': 'the 00s', 'author__id__exact': self.alfred.pk}) + request.user = self.alfred changelist = modeladmin.get_changelist_instance(request) # Make sure the correct queryset is returned @@ -915,6 +956,7 @@ class ListFiltersTests(TestCase): """ modeladmin = DecadeFilterBookAdminWithoutTitle(Book, site) request = self.request_factory.get('/', {}) + request.user = self.alfred msg = "The list filter 'DecadeListFilterWithoutTitle' does not specify a 'title'." with self.assertRaisesMessage(ImproperlyConfigured, msg): modeladmin.get_changelist_instance(request) @@ -925,6 +967,7 @@ class ListFiltersTests(TestCase): """ modeladmin = DecadeFilterBookAdminWithoutParameter(Book, site) request = self.request_factory.get('/', {}) + request.user = self.alfred msg = "The list filter 'DecadeListFilterWithoutParameter' does not specify a 'parameter_name'." with self.assertRaisesMessage(ImproperlyConfigured, msg): modeladmin.get_changelist_instance(request) @@ -936,6 +979,7 @@ class ListFiltersTests(TestCase): """ modeladmin = DecadeFilterBookAdminWithNoneReturningLookups(Book, site) request = self.request_factory.get('/', {}) + request.user = self.alfred changelist = modeladmin.get_changelist_instance(request) filterspec = changelist.get_filters(request)[0] self.assertEqual(len(filterspec), 0) @@ -947,12 +991,14 @@ class ListFiltersTests(TestCase): """ modeladmin = DecadeFilterBookAdminWithFailingQueryset(Book, site) request = self.request_factory.get('/', {}) + request.user = self.alfred with self.assertRaises(ZeroDivisionError): modeladmin.get_changelist_instance(request) def test_simplelistfilter_with_queryset_based_lookups(self): modeladmin = DecadeFilterBookAdminWithQuerysetBasedLookups(Book, site) request = self.request_factory.get('/', {}) + request.user = self.alfred changelist = modeladmin.get_changelist_instance(request) filterspec = changelist.get_filters(request)[0][0] @@ -978,6 +1024,7 @@ class ListFiltersTests(TestCase): """ modeladmin = BookAdmin(Book, site) request = self.request_factory.get('/', {'no': '207'}) + request.user = self.alfred changelist = modeladmin.get_changelist_instance(request) # Make sure the correct queryset is returned @@ -998,6 +1045,7 @@ class ListFiltersTests(TestCase): # When it ends with '__in' ----------------------------------------- modeladmin = DecadeFilterBookAdminParameterEndsWith__In(Book, site) request = self.request_factory.get('/', {'decade__in': 'the 90s'}) + request.user = self.alfred changelist = modeladmin.get_changelist_instance(request) # Make sure the correct queryset is returned @@ -1015,6 +1063,7 @@ class ListFiltersTests(TestCase): # When it ends with '__isnull' --------------------------------------- modeladmin = DecadeFilterBookAdminParameterEndsWith__Isnull(Book, site) request = self.request_factory.get('/', {'decade__isnull': 'the 90s'}) + request.user = self.alfred changelist = modeladmin.get_changelist_instance(request) # Make sure the correct queryset is returned @@ -1036,6 +1085,7 @@ class ListFiltersTests(TestCase): """ modeladmin = DepartmentFilterEmployeeAdmin(Employee, site) request = self.request_factory.get('/', {'department': self.john.department.pk}) + request.user = self.alfred changelist = modeladmin.get_changelist_instance(request) queryset = changelist.get_queryset(request) @@ -1056,6 +1106,7 @@ class ListFiltersTests(TestCase): """ modeladmin = DepartmentFilterUnderscoredEmployeeAdmin(Employee, site) request = self.request_factory.get('/', {'department__whatever': self.john.department.pk}) + request.user = self.alfred changelist = modeladmin.get_changelist_instance(request) queryset = changelist.get_queryset(request) @@ -1076,6 +1127,7 @@ class ListFiltersTests(TestCase): modeladmin = EmployeeAdmin(Employee, site) request = self.request_factory.get('/', {}) + request.user = self.alfred changelist = modeladmin.get_changelist_instance(request) # Make sure the correct queryset is returned @@ -1101,6 +1153,7 @@ class ListFiltersTests(TestCase): # Filter by Department=='Development' -------------------------------- request = self.request_factory.get('/', {'department__code__exact': 'DEV'}) + request.user = self.alfred changelist = modeladmin.get_changelist_instance(request) # Make sure the correct queryset is returned @@ -1130,6 +1183,7 @@ class ListFiltersTests(TestCase): modeladmin = DepartmentFilterDynamicValueBookAdmin(Book, site) def _test_choices(request, expected_displays): + request.user = self.alfred changelist = modeladmin.get_changelist_instance(request) filterspec = changelist.get_filters(request)[0][0] self.assertEqual(filterspec.title, 'publication decade') @@ -1152,6 +1206,7 @@ class ListFiltersTests(TestCase): """ modeladmin = NotNinetiesListFilterAdmin(Book, site) request = self.request_factory.get('/', {}) + request.user = self.alfred changelist = modeladmin.get_changelist_instance(request) changelist.get_results(request) self.assertEqual(changelist.full_result_count, 4) diff --git a/tests/admin_views/admin.py b/tests/admin_views/admin.py index 9e6ce63077..51791e961e 100644 --- a/tests/admin_views/admin.py +++ b/tests/admin_views/admin.py @@ -156,6 +156,10 @@ class RowLevelChangePermissionModelAdmin(admin.ModelAdmin): """ Only allow changing objects with even id number """ return request.user.is_staff and (obj is not None) and (obj.id % 2 == 0) + def has_view_permission(self, request, obj=None): + """Only allow viewing objects if id is a multiple of 3.""" + return request.user.is_staff and obj is not None and obj.id % 3 == 0 + class CustomArticleAdmin(admin.ModelAdmin): """ diff --git a/tests/admin_views/test_templatetags.py b/tests/admin_views/test_templatetags.py index 41bb8c37eb..929ff7e045 100644 --- a/tests/admin_views/test_templatetags.py +++ b/tests/admin_views/test_templatetags.py @@ -72,6 +72,9 @@ class AdminTemplateTagsTest(AdminViewBasicTestCase): class DateHierarchyTests(TestCase): factory = RequestFactory() + def setUp(self): + self.superuser = User.objects.create_superuser(username='super', password='secret', email='super@example.com') + def test_choice_links(self): modeladmin = ModelAdmin(Question, site) modeladmin.date_hierarchy = 'posted' @@ -97,6 +100,7 @@ class DateHierarchyTests(TestCase): with self.subTest(query=query): query = {'posted__%s' % q: val for q, val in query.items()} request = self.factory.get('/', query) + request.user = self.superuser changelist = modeladmin.get_changelist_instance(request) spec = date_hierarchy(changelist) choices = [choice['link'] for choice in spec['choices']] diff --git a/tests/admin_views/tests.py b/tests/admin_views/tests.py index f8f3b3c269..f7f247fd37 100644 --- a/tests/admin_views/tests.py +++ b/tests/admin_views/tests.py @@ -1394,6 +1394,7 @@ class AdminViewPermissionsTest(TestCase): @classmethod def setUpTestData(cls): cls.superuser = User.objects.create_superuser(username='super', password='secret', email='super@example.com') + cls.viewuser = User.objects.create_user(username='viewuser', password='secret', is_staff=True) cls.adduser = User.objects.create_user(username='adduser', password='secret', is_staff=True) cls.changeuser = User.objects.create_user(username='changeuser', password='secret', is_staff=True) cls.deleteuser = User.objects.create_user(username='deleteuser', password='secret', is_staff=True) @@ -1415,6 +1416,8 @@ class AdminViewPermissionsTest(TestCase): # Setup permissions, for our users who can add, change, and delete. opts = Article._meta + # User who can view Articles + cls.viewuser.user_permissions.add(get_perm(Article, get_permission_codename('view', opts))) # User who can add Articles cls.adduser.user_permissions.add(get_perm(Article, get_permission_codename('add', opts))) # User who can change Articles @@ -1467,6 +1470,11 @@ class AdminViewPermissionsTest(TestCase): 'username': 'joepublic', 'password': 'secret', } + cls.viewuser_login = { + REDIRECT_FIELD_NAME: cls.index_url, + 'username': 'viewuser', + 'password': 'secret', + } cls.no_username_login = { REDIRECT_FIELD_NAME: cls.index_url, 'password': 'secret', @@ -1503,6 +1511,14 @@ class AdminViewPermissionsTest(TestCase): login = self.client.post(login_url, self.super_email_login) self.assertContains(login, ERROR_MESSAGE) + # View User + response = self.client.get(self.index_url) + self.assertEqual(response.status_code, 302) + login = self.client.post(login_url, self.viewuser_login) + self.assertRedirects(login, self.index_url) + self.assertFalse(login.context) + self.client.get(reverse('admin:logout')) + # Add User response = self.client.get(self.index_url) self.assertEqual(response.status_code, 302) @@ -1657,6 +1673,27 @@ class AdminViewPermissionsTest(TestCase): self.assertEqual(Article.objects.count(), 3) self.client.get(reverse('admin:logout')) + # View User should not have access to add articles + self.client.force_login(self.viewuser) + response = self.client.get(reverse('admin:admin_views_article_add')) + self.assertEqual(response.status_code, 403) + # Try POST just to make sure + post = self.client.post(reverse('admin:admin_views_article_add'), add_dict) + self.assertEqual(post.status_code, 403) + self.assertEqual(Article.objects.count(), 3) + # Now give the user permission to add but not change. + self.viewuser.user_permissions.add(get_perm(Article, get_permission_codename('add', Article._meta))) + response = self.client.get(reverse('admin:admin_views_article_add')) + self.assertContains(response, '') + post = self.client.post(reverse('admin:admin_views_article_add'), add_dict, follow=False) + self.assertEqual(post.status_code, 302) + self.assertEqual(Article.objects.count(), 4) + article = Article.objects.latest('pk') + response = self.client.get(reverse('admin:admin_views_article_change', args=(article.pk,))) + self.assertContains(response, '
  • The article "Døm ikke" was added successfully.
    • ') + article.delete() + self.client.get(reverse('admin:logout')) + # Add user may login and POST to add view, then redirect to admin root self.client.force_login(self.adduser) addpage = self.client.get(reverse('admin:admin_views_article_add')) @@ -1668,7 +1705,7 @@ class AdminViewPermissionsTest(TestCase): post = self.client.post(reverse('admin:admin_views_article_add'), add_dict) self.assertRedirects(post, self.index_url) self.assertEqual(Article.objects.count(), 4) - self.assertEqual(len(mail.outbox), 1) + self.assertEqual(len(mail.outbox), 2) self.assertEqual(mail.outbox[0].subject, 'Greetings from a created object') self.client.get(reverse('admin:logout')) @@ -1722,6 +1759,19 @@ class AdminViewPermissionsTest(TestCase): self.assertEqual(post.status_code, 403) self.client.get(reverse('admin:logout')) + # view user should be able to view the article but not change any of them + # (the POST can be sent, but no modification occures) + self.client.force_login(self.viewuser) + response = self.client.get(article_changelist_url) + self.assertEqual(response.status_code, 200) + response = self.client.get(article_change_url) + self.assertEqual(response.status_code, 200) + self.assertContains(response, 'Close') + post = self.client.post(article_change_url, change_dict) + self.assertEqual(post.status_code, 302) + self.assertEqual(Article.objects.get(pk=self.a1.pk).content, '

    Middle content

    ') + self.client.get(reverse('admin:logout')) + # change user can view all items and edit them self.client.force_login(self.changeuser) response = self.client.get(article_changelist_url) @@ -1751,9 +1801,14 @@ class AdminViewPermissionsTest(TestCase): # Test redirection when using row-level change permissions. Refs #11513. r1 = RowLevelChangePermissionModel.objects.create(id=1, name="odd id") r2 = RowLevelChangePermissionModel.objects.create(id=2, name="even id") + r3 = RowLevelChangePermissionModel.objects.create(id=3, name='odd id mult 3') + r6 = RowLevelChangePermissionModel.objects.create(id=6, name='even id mult 3') change_url_1 = reverse('admin:admin_views_rowlevelchangepermissionmodel_change', args=(r1.pk,)) change_url_2 = reverse('admin:admin_views_rowlevelchangepermissionmodel_change', args=(r2.pk,)) - for login_user in [self.superuser, self.adduser, self.changeuser, self.deleteuser]: + change_url_3 = reverse('admin:admin_views_rowlevelchangepermissionmodel_change', args=(r3.pk,)) + change_url_6 = reverse('admin:admin_views_rowlevelchangepermissionmodel_change', args=(r6.pk,)) + logins = [self.superuser, self.viewuser, self.adduser, self.changeuser, self.deleteuser] + for login_user in logins: self.client.force_login(login_user) response = self.client.get(change_url_1) self.assertEqual(response.status_code, 403) @@ -1765,6 +1820,18 @@ class AdminViewPermissionsTest(TestCase): response = self.client.post(change_url_2, {'name': 'changed'}) self.assertEqual(RowLevelChangePermissionModel.objects.get(id=2).name, 'changed') self.assertRedirects(response, self.index_url) + response = self.client.get(change_url_3) + self.assertEqual(response.status_code, 200) + response = self.client.post(change_url_3, {'name': 'changed'}) + self.assertEqual(response.status_code, 302) + self.assertRedirects(response, self.index_url) + self.assertEqual(RowLevelChangePermissionModel.objects.get(id=3).name, 'odd id mult 3') + response = self.client.get(change_url_6) + self.assertEqual(response.status_code, 200) + response = self.client.post(change_url_6, {'name': 'changed'}) + self.assertEqual(RowLevelChangePermissionModel.objects.get(id=6).name, 'changed') + self.assertRedirects(response, self.index_url) + self.client.get(reverse('admin:logout')) for login_user in [self.joepublicuser, self.nostaffuser]: @@ -1833,6 +1900,15 @@ class AdminViewPermissionsTest(TestCase): self.assertEqual(Article.objects.count(), 3) self.client.logout() + # view user should not be able to delete articles + self.client.force_login(self.viewuser) + response = self.client.get(delete_url) + self.assertEqual(response.status_code, 403) + post = self.client.post(delete_url, delete_dict) + self.assertEqual(post.status_code, 403) + self.assertEqual(Article.objects.count(), 3) + self.client.logout() + # Delete user can delete self.client.force_login(self.deleteuser) response = self.client.get(reverse('admin:admin_views_section_delete', args=(self.s1.pk,))) @@ -1890,6 +1966,12 @@ class AdminViewPermissionsTest(TestCase): self.assertEqual(response.status_code, 403) self.client.get(reverse('admin:logout')) + # view user can view all items + self.client.force_login(self.viewuser) + response = self.client.get(reverse('admin:admin_views_article_history', args=(self.a1.pk,))) + self.assertEqual(response.status_code, 200) + self.client.get(reverse('admin:logout')) + # change user can view all items and edit them self.client.force_login(self.changeuser) response = self.client.get(reverse('admin:admin_views_article_history', args=(self.a1.pk,))) @@ -1898,7 +1980,8 @@ class AdminViewPermissionsTest(TestCase): # Test redirection when using row-level change permissions. Refs #11513. rl1 = RowLevelChangePermissionModel.objects.create(name="odd id") rl2 = RowLevelChangePermissionModel.objects.create(name="even id") - for login_user in [self.superuser, self.adduser, self.changeuser, self.deleteuser]: + logins = [self.superuser, self.viewuser, self.adduser, self.changeuser, self.deleteuser] + for login_user in logins: self.client.force_login(login_user) url = reverse('admin:admin_views_rowlevelchangepermissionmodel_history', args=(rl1.pk,)) response = self.client.get(url) @@ -2072,6 +2155,12 @@ class AdminViewPermissionsTest(TestCase): self.assertContains(response, 'Articles') self.client.logout() + self.client.force_login(self.viewuser) + response = self.client.get(self.index_url) + self.assertContains(response, 'admin_views') + self.assertContains(response, 'Articles') + self.client.logout() + self.client.force_login(self.adduser) response = self.client.get(self.index_url) self.assertContains(response, 'admin_views') @@ -2104,6 +2193,12 @@ class AdminViewPermissionsTest(TestCase): self.assertNotContains(response, articles) self.client.logout() + self.client.force_login(self.viewuser) + response = self.client.get(index_url) + self.assertNotContains(response, 'admin_views') + self.assertNotContains(response, articles) + self.client.logout() + self.client.force_login(self.adduser) response = self.client.get(index_url) self.assertNotContains(response, 'admin_views') @@ -3745,6 +3840,52 @@ class AdminInlineTests(TestCase): self.assertEqual(Widget.objects.count(), 1) self.assertEqual(Widget.objects.all()[0].name, "Widget 1 Updated") + def test_simple_inline_permissions(self): + """ + Changes aren't allowed without change permissions for the inline object. + """ + # User who can view Articles + permissionuser = User.objects.create_user( + username='permissionuser', password='secret', + email='vuser@example.com', is_staff=True, + ) + permissionuser.user_permissions.add(get_perm(Collector, get_permission_codename('view', Collector._meta))) + permissionuser.user_permissions.add(get_perm(Widget, get_permission_codename('view', Widget._meta))) + self.client.force_login(permissionuser) + # Without add permission, a new inline can't be added. + self.post_data['widget_set-0-name'] = 'Widget 1' + collector_url = reverse('admin:admin_views_collector_change', args=(self.collector.pk,)) + response = self.client.post(collector_url, self.post_data) + self.assertEqual(response.status_code, 302) + self.assertEqual(Widget.objects.count(), 0) + # But after adding the permisson it can. + permissionuser.user_permissions.add(get_perm(Widget, get_permission_codename('add', Widget._meta))) + self.post_data['widget_set-0-name'] = "Widget 1" + collector_url = reverse('admin:admin_views_collector_change', args=(self.collector.pk,)) + response = self.client.post(collector_url, self.post_data) + self.assertEqual(response.status_code, 302) + self.assertEqual(Widget.objects.count(), 1) + self.assertEqual(Widget.objects.first().name, 'Widget 1') + widget_id = Widget.objects.first().id + # Without the change permission, a POST doesn't change the object. + self.post_data['widget_set-INITIAL_FORMS'] = '1' + self.post_data['widget_set-0-id'] = str(widget_id) + self.post_data['widget_set-0-name'] = 'Widget 1 Updated' + response = self.client.post(collector_url, self.post_data) + self.assertEqual(response.status_code, 302) + self.assertEqual(Widget.objects.count(), 1) + self.assertEqual(Widget.objects.first().name, 'Widget 1') + # Now adding the change permission and editing works. + permissionuser.user_permissions.remove(get_perm(Widget, get_permission_codename('add', Widget._meta))) + permissionuser.user_permissions.add(get_perm(Widget, get_permission_codename('change', Widget._meta))) + self.post_data['widget_set-INITIAL_FORMS'] = '1' + self.post_data['widget_set-0-id'] = str(widget_id) + self.post_data['widget_set-0-name'] = 'Widget 1 Updated' + response = self.client.post(collector_url, self.post_data) + self.assertEqual(response.status_code, 302) + self.assertEqual(Widget.objects.count(), 1) + self.assertEqual(Widget.objects.first().name, 'Widget 1 Updated') + def test_explicit_autofield_inline(self): "A model with an explicit autofield primary key can be saved as inlines. Regression for #8093" # First add a new inline diff --git a/tests/auth_tests/test_management.py b/tests/auth_tests/test_management.py index 4e60333c9d..b0c5c6204b 100644 --- a/tests/auth_tests/test_management.py +++ b/tests/auth_tests/test_management.py @@ -767,10 +767,10 @@ class CreatePermissionsTests(TestCase): ] create_permissions(self.app_config, verbosity=0) - # add/change/delete permission by default + custom permission + # view/add/change/delete permission by default + custom permission self.assertEqual(Permission.objects.filter( content_type=permission_content_type, - ).count(), 4) + ).count(), 5) Permission.objects.filter(content_type=permission_content_type).delete() Permission._meta.default_permissions = [] diff --git a/tests/modeladmin/tests.py b/tests/modeladmin/tests.py index d0123de722..db6e9e8864 100644 --- a/tests/modeladmin/tests.py +++ b/tests/modeladmin/tests.py @@ -11,7 +11,7 @@ from django.contrib.admin.widgets import ( AdminDateWidget, AdminRadioSelect, AutocompleteSelect, AutocompleteSelectMultiple, ) -from django.contrib.auth.models import User +from django.contrib.auth.models import Permission, User from django.db import models from django.forms.widgets import Select from django.test import SimpleTestCase, TestCase @@ -676,6 +676,18 @@ class ModelAdminTests(TestCase): self.assertEqual(perms_needed, set()) self.assertEqual(protected, []) + def test_get_actions_requires_change_perm(self): + user = User.objects.create_user(username='bob', email='bob@test.com', password='test') + mock_request = MockRequest() + mock_request.user = user + mock_request.GET = {} + ma = ModelAdmin(Band, self.site) + self.assertEqual(list(ma.get_actions(mock_request).keys()), []) + p = Permission.objects.get(codename='change_band', content_type=get_content_type_for_model(Band())) + user.user_permissions.add(p) + mock_request.user = User.objects.get(pk=user.pk) + self.assertEqual(list(ma.get_actions(mock_request).keys()), ['delete_selected']) + class ModelAdminPermissionTests(SimpleTestCase): @@ -683,6 +695,10 @@ class ModelAdminPermissionTests(SimpleTestCase): def has_module_perms(self, app_label): return app_label == 'modeladmin' + class MockViewUser(MockUser): + def has_perm(self, perm): + return perm == 'modeladmin.view_band' + class MockAddUser(MockUser): def has_perm(self, perm): return perm == 'modeladmin.add_band' @@ -695,6 +711,22 @@ class ModelAdminPermissionTests(SimpleTestCase): def has_perm(self, perm): return perm == 'modeladmin.delete_band' + def test_has_view_permission(self): + """ + has_view_permission() returns True for users who can view objects and + False for users who can't. + """ + ma = ModelAdmin(Band, AdminSite()) + request = MockRequest() + request.user = self.MockViewUser() + self.assertIs(ma.has_view_permission(request), True) + request.user = self.MockAddUser() + self.assertIs(ma.has_view_permission(request), False) + request.user = self.MockChangeUser() + self.assertIs(ma.has_view_permission(request), True) + request.user = self.MockDeleteUser() + self.assertIs(ma.has_view_permission(request), False) + def test_has_add_permission(self): """ has_add_permission returns True for users who can add objects and @@ -702,6 +734,8 @@ class ModelAdminPermissionTests(SimpleTestCase): """ ma = ModelAdmin(Band, AdminSite()) request = MockRequest() + request.user = self.MockViewUser() + self.assertFalse(ma.has_add_permission(request)) request.user = self.MockAddUser() self.assertTrue(ma.has_add_permission(request)) request.user = self.MockChangeUser() @@ -735,6 +769,8 @@ class ModelAdminPermissionTests(SimpleTestCase): """ ma = ModelAdmin(Band, AdminSite()) request = MockRequest() + request.user = self.MockViewUser() + self.assertIs(ma.has_change_permission(request), False) request.user = self.MockAddUser() self.assertFalse(ma.has_change_permission(request)) request.user = self.MockChangeUser() @@ -749,6 +785,8 @@ class ModelAdminPermissionTests(SimpleTestCase): """ ma = ModelAdmin(Band, AdminSite()) request = MockRequest() + request.user = self.MockViewUser() + self.assertIs(ma.has_delete_permission(request), False) request.user = self.MockAddUser() self.assertFalse(ma.has_delete_permission(request)) request.user = self.MockChangeUser() @@ -763,6 +801,8 @@ class ModelAdminPermissionTests(SimpleTestCase): """ ma = ModelAdmin(Band, AdminSite()) request = MockRequest() + request.user = self.MockViewUser() + self.assertIs(ma.has_module_permission(request), True) request.user = self.MockAddUser() self.assertTrue(ma.has_module_permission(request)) request.user = self.MockChangeUser() @@ -773,6 +813,8 @@ class ModelAdminPermissionTests(SimpleTestCase): original_app_label = ma.opts.app_label ma.opts.app_label = 'anotherapp' try: + request.user = self.MockViewUser() + self.assertIs(ma.has_module_permission(request), False) request.user = self.MockAddUser() self.assertFalse(ma.has_module_permission(request)) request.user = self.MockChangeUser()