Code

Opened 3 years ago

Closed 3 years ago

Last modified 3 years ago

#15261 closed Uncategorized (wontfix)

Admin querystring filters should work for superusers

Reported by: cdestigter Owned by: nobody
Component: contrib.admin Version: 1.2
Severity: Normal Keywords:
Cc: adehnert Triage Stage: Unreviewed
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

Our staff users are all superusers, and we actively use the manual querystring filters in the admin.

So the changes in 1.2.4+ to prevent those URLs from working were quite frustrating. I've written a patch which allows superusers to use these filters again.

It includes fixes to the tests, to test both the positive behavior for superusers and negative for non-superusers.

Patch is -p1. Applies to trunk but should backport nicely to 1.2.X.

Attachments (2)

django-allow-superuser-filters.diff (7.0 KB) - added by cdestigter 3 years ago.
django-allow-superuser-filters-1.2.Xbackport.diff (5.8 KB) - added by cdestigter 3 years ago.

Download all attachments as: .zip

Change History (7)

Changed 3 years ago by cdestigter

Changed 3 years ago by cdestigter

comment:1 Changed 3 years ago by cdestigter

  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset

also attached my 1.2.X backport patch

comment:2 Changed 3 years ago by russellm

  • Resolution set to wontfix
  • Status changed from new to closed

I'm not convinced this is a good idea. The list of allowed filters can be modified by overriding ModelAdmin.lookup_allowed, which strikes me as a much better approach to this problem.

comment:3 follow-up: Changed 3 years ago by cdestigter

Yes, I noticed that a similar thing could be achieved by doing something like:

    def lookup_allowed(self, lookup, value):
        return True

But I see some problems with that approach:

  1. lookup_allowed() doesn't have access to the request, so can't tell if the user is a superuser. It can only be overridden for everyone, which could be a problem in setups with non-superuser staff members.
  2. This requires we use a custom ModelAdmin base for every model in our project. And then, it still won't work for Django builtin models like User unless we monkey-patch the django BaseModelAdmin class...

Since superusers have implicit permissions for every model anyway, doesn't it seem logical that they should be able to filter by anything they like?

comment:4 in reply to: ↑ 3 Changed 3 years ago by adehnert

  • Easy pickings unset
  • Severity set to Normal
  • Type set to Uncategorized
  • UI/UX unset

Replying to cdestigter:

Since superusers have implicit permissions for every model anyway, doesn't it seem logical that they should be able to filter by anything they like?

That's not quite true, I believe. They implicitly have all Django permissions. I believe if you don't add the model to the admin and don't have a view for querying it, even admins won't be able to see it. (This is totally believable for, say, a logging table, which gets aggregated but isn't directly viewable, I think.) I can believe that Django doesn't actually try to maintain this, though I don't think I've seen docs either way.

comment:5 Changed 3 years ago by adehnert

  • Cc adehnert added

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.