Code

Opened 9 years ago

Closed 4 years ago

Last modified 17 months 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: SmileyChris
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> 9 years ago.
1035.diff (4.3 KB) - added by SmileyChris 5 years ago.

Download all attachments as: .zip

Change History (15)

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

comment:1 Changed 9 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 8 years ago by mtredinnick

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 8 years ago by mtredinnick

  • Summary changed from [patch] Link to popup for adding related objects should respect user's permissions to Link 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 7 years ago by Simon G. <dev@…>

  • Patch needs improvement set
  • Triage Stage changed from Unreviewed to Accepted

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

  • Has patch set

as per Malcolm's comments above.

comment:6 Changed 7 years ago by Esaj

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

comment:7 Changed 7 years ago by ubernostrum

  • Owner changed from nobody to xian

#2927 is a duplicate. Reassigning to xian.

comment:8 Changed 6 years ago by billsb

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

comment:9 Changed 6 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 5 years ago by SmileyChris

comment:10 Changed 5 years ago by SmileyChris

  • Owner changed from xian to SmileyChris
  • Patch needs improvement unset
  • Status changed from new to assigned
  • Version set to 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 5 years ago by jdunck

  • Keywords sprint200912 added
  • Triage Stage changed from Accepted to Ready for checkin

comment:12 Changed 4 years ago by mtredinnick

  • Resolution set to fixed
  • Status changed from assigned to closed

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

comment:13 Changed 17 months ago by dwightgunning

  • Easy pickings unset
  • UI/UX unset

Just marked #11136 as duplicate of this ticket.

Add Comment

Modify Ticket

Change Properties
<Author field>
Action
as closed
as The resolution will be set. Next status will be 'closed'
The resolution will be deleted. Next status will be 'new'
Author


E-mail address and user name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.