Admin: 403 link with missing view-permission

Hi folks!

A while ago I discovered a bug in the admin and raised a ticket.

  1. My user can see model “mymodel” via permisson “view_mymodel”
  2. My user doesnt have the “view_foreignkerelatedmodel” permission
  3. The readonly-foreign key is rendered as a link on the mymodel-detail page. Clicking on it leads to a 403 page

The problem exists for the regular admin and as well for inlines.

I created repo to reproduce the problem:

​https://github.com/GitRon/django_admin_readonly_link_field

The AdminReadonlyField has a method contents which creates the link. This method doesn’t know anything about the user or his/her permissions. And that’s ok.

What I’ve tried:

  • Give AdminReadonlyField attributes like view-permission etc to determine in the template what I want to show - doesn’t work because all the magic is in the contents method
  • Pass the user object to the contains method from the template
  • 27 different approaches with filters and template tags

Does anybody has an idea on how I can solve this? I’ve tried for hours now to solve this but I failed :disappointed:

Best from Cologne
Ronny

Hi @tom, I talked with @sarahboyce about this bug / bad UX and she mentioned you as a somebody I could ask for some input on admin topics.

I tried hard to find a solution but I’m missing something conceptually since withouth a large refactoring, I’m just not able to bring together the permission check and the link creation.

Thx!
Ronny

PS: Saw your talk in Vigo about improving accessability in the admin. This might to in the same direction, more or less.

1 Like

If you can make it that in the template you know the permissions, you may be able to use {{ field.contents|striptags }} (see Built-in template tags and filters | Django documentation | Django)

1 Like

Thx @sarahboyce for the suggestion! I had a look and the problem is that I don’t have the related model in the template. I do have the instance of the edit pages model but not the related one. Thus, I can’t do the check their. If I had it and could put together the permission check somehow, striptag would work!

Any follow-up ideas? :sweat_smile:

Not tested so unlikely to work and certainly isn’t “nice” but maybe you can have the required perm be a property on AdminReadonlyField

--- a/django/contrib/admin/helpers.py
+++ b/django/contrib/admin/helpers.py
@@ -263,6 +263,14 @@ class AdminReadonlyField:
         except NoReverseMatch:
             return str(remote_obj)
 
+    def required_perm(self):
+        try:
+            f, _, _ = lookup_field(self.field["field"], self.form.instance, self.model_admin)
+        except (AttributeError, ValueError, ObjectDoesNotExist):
+            return None
+        if f is not None and isinstance(f.remote_field, (ForeignObjectRel, OneToOneField)):
+            return f"perms.{f.model._meta.app_label}.view_{f.model._meta.model_name}"
+
     def contents(self):
--- a/django/contrib/admin/templates/admin/edit_inline/tabular.html
+++ b/django/contrib/admin/templates/admin/edit_inline/tabular.html
@@ -49,7 +49,7 @@
             {% for field in line %}
               <td class="{% if field.field.name %}field-{{ field.field.name }}{% endif %}{% if field.field.is_hidden %} hidden{% endif %}">
               {% if field.is_readonly %}
-                  <p>{{ field.contents }}</p>
+                  <p>{% if field.required_perm and field.required_perm not in perms %}{{ field.contents|striptags }}{% else %}{{ field.contents }}{% endif %}</p>
               {% else %}
--- a/django/contrib/admin/templates/admin/includes/fieldset.html
+++ b/django/contrib/admin/templates/admin/includes/fieldset.html
@@ -20,7 +20,7 @@
                             {% else %}
                                 {{ field.label_tag }}
                                 {% if field.is_readonly %}
-                                    <div class="readonly">{{ field.contents }}</div>
+                                    <div class="readonly">{% if field.required_perm and field.required_perm not in perms %}{{ field.contents|striptags }}{% else %}{{ field.contents }}{% endif %}</div>
                                 {% else %}
1 Like