Opened 11 years ago

Closed 6 years ago

Last modified 4 years ago

#1035 closed defect (fixed)

Link to popup for adding related objects should respect user's permissions

Reported by: Esaj <jason at jasondavies.com> Owned by: Chris Beaven
Component: contrib.admin Version: master
Severity: normal Keywords: sprint200912
Cc: Triage Stage: Ready for checkin
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

The little link for adding additional related objects (e.g. related by foreign key) should only show if the user has sufficient permissions to add these objects.

Index: django/contrib/admin/templatetags/admin_modify.py
===================================================================
--- django/contrib/admin/templatetags/admin_modify.py   (revision 1587)
+++ django/contrib/admin/templatetags/admin_modify.py   (working copy)
@@ -246,6 +246,7 @@

     return {
         'add': context['add'],
+        'app_permission': context['app_permission'],
         'change': context['change'],
         'bound_fields': bound_fields,
         'class_names': " ".join(class_names),
@@ -257,3 +258,11 @@
     return bound_manip.get_ordered_object_pk(ordered_obj)

 object_pk = register.simple_tag(object_pk)
+
+#@register.filter
+def has_perm(perm_obj, perm):
+    if perm_obj:
+        return perm_obj[perm]
+    return False
+
+has_perm = register.filter(has_perm)
Index: django/contrib/admin/views/main.py
===================================================================
--- django/contrib/admin/views/main.py  (revision 1587)
+++ django/contrib/admin/views/main.py  (working copy)
@@ -286,6 +286,7 @@
         self.is_date_time = isinstance(field, meta.DateTimeField)
         self.is_file_field = isinstance(field, meta.FileField)
         self.needs_add_label = field.rel and isinstance(field.rel, meta.ManyToOne) or isinstance(field.rel, meta.ManyToMany) and field.rel.to.admin
+        self.add_permission = self.needs_add_label and "can_add_%s" % (field.rel.to.verbose_name)
         self.hidden = isinstance(self.field, meta.AutoField)
         self.first = False

@@ -375,11 +376,13 @@
         return ""

 def render_change_form(opts, manipulator, app_label, context, add=False, change=False, show_delete=False, form_url=''):
+    app_permission = context['perms'][app_label]
     extra_context = {
         'add': add,
         'change': change,
         'bound_manipulator': AdminBoundManipulator(opts, manipulator, context['form']),
-        'has_delete_permission': context['perms'][app_label][opts.get_delete_permission()],
+        'has_delete_permission': app_permission[opts.get_delete_permission()],
+        'app_permission': app_permission,
         'form_url': form_url,
         'app_label': app_label,
     }
Index: django/contrib/admin/templates/widget/foreign.html
===================================================================
--- django/contrib/admin/templates/widget/foreign.html  (revision 1587)
+++ django/contrib/admin/templates/widget/foreign.html  (working copy)
@@ -4,5 +4,6 @@
     <a href="../../../{{ bound_field.field.rel.to.app_label }}/{{ bound_field.field.rel.to.module_name }}/" class="related-lookup" id="lookup_{{ bound_field.element_id }}" onclick="return showRelatedObjectLookupPopup(this);"> <img src="{% admin_media_prefix %}img/admin/selector-search.gif" width="16" height="16" alt="Lookup"></a>
 {% else %}
 {% if bound_field.needs_add_label %}
+{% if app_permission|has_perm:bound_field.add_permission %}
     <a href="../../../{{ bound_field.field.rel.to.app_label }}/{{ bound_field.field.rel.to.module_name }}/add/" class="add-another" id="add_{{ bound_field.element_id }}" onclick="return showAddAnotherPopup(this);"> <img src="{% admin_media_prefix %}img/admin/icon_addlink.gif" width="10" height="10" alt="Add Another"/></a>
-{% endif %}{% endif %}
+{% endif %}{% endif %}{% endif %}

Attachments (2)

add_popup.diff (3.2 KB) - added by Esaj <jason at jasondavies.com> 11 years ago.
1035.diff (4.3 KB) - added by Chris Beaven 7 years ago.

Download all attachments as: .zip

Change History (15)

Changed 11 years ago by Esaj <jason at jasondavies.com>

Attachment: add_popup.diff added

comment:1 Changed 11 years ago by Esaj <jason at jasondavies.com>

Note: The lookup filter from #959 would come in handy here, instead of the has_perm filter I added in the patch.

comment:2 Changed 10 years ago by Malcolm Tredinnick

This is a reasonable fix to make, but the patch no longer cleanly applies and needs a bit of reworking. The way self.add_permission is constructed in views/main.py does not work and doesn't look like the write approach. Something Options.using get_add_permission() feels like it would be cleaner.

comment:3 Changed 10 years ago by Malcolm Tredinnick

Summary: [patch] Link to popup for adding related objects should respect user's permissionsLink to popup for adding related objects should respect user's permissions

Removing "patch" keyword so that it doesn't show up on the patch report. This needs a fresh patch to be written.

comment:4 Changed 10 years ago by Simon G. <dev@…>

Patch needs improvement: set
Triage Stage: UnreviewedAccepted

comment:5 Changed 10 years ago by Gary Wilson <gary.wilson@…>

Has patch: set

as per Malcolm's comments above.

comment:6 Changed 9 years ago by Esaj

I'll write a better patch once newforms-admin is done...

comment:7 Changed 9 years ago by James Bennett

Owner: changed from nobody to xian

#2927 is a duplicate. Reassigning to xian.

comment:8 Changed 8 years ago by billsb

I believe this patch won't work at all with trunk...

comment:9 Changed 8 years ago by vincent

For the trunk, you can use this : http://code.djangoproject.com/ticket/9071

Doesn't work with permissions but can do the trick (it did for me).

Changed 7 years ago by Chris Beaven

Attachment: 1035.diff added

comment:10 Changed 7 years ago by Chris Beaven

Owner: changed from xian to Chris Beaven
Patch needs improvement: unset
Status: newassigned
Version: SVN

New patch against current admin (i.e. the new newforms-admin). Left backwards compatible in the odd case which someone may have subclassed the wrapper widget.

comment:11 Changed 7 years ago by Jeremy Dunck

Keywords: sprint200912 added
Triage Stage: AcceptedReady for checkin

comment:12 Changed 6 years ago by Malcolm Tredinnick

Resolution: fixed
Status: assignedclosed

(In [13708]) Adding related objects in the admin (via popup) respects user
permissions. Patch from SmileyChris. Fixed #1035.

comment:13 Changed 4 years ago by Dwight Gunning

Easy pickings: unset
UI/UX: unset

Just marked #11136 as duplicate of this ticket.

Note: See TracTickets for help on using tickets.
Back to Top