#15032 closed (fixed)
Changeset 15031 breaks filtering to objects which are subclassed.
Reported by: | rene | Owned by: | nobody |
---|---|---|---|
Component: | contrib.admin | Version: | 1.2 |
Severity: | Keywords: | filtering subclassed object not allowed regression blocker | |
Cc: | Triage Stage: | Accepted | |
Has patch: | yes | Needs documentation: | no |
Needs tests: | no | Patch needs improvement: | no |
Easy pickings: | no | UI/UX: | no |
Description
In changeset 15031 a security check is implemented which checks if the parameters in the URL query are indeed field names specified in the 'list_filter' attribute of the AdminModel.
This breaks the filtering of a inherintanced model object. See attached two files for a sample code.
I have a 'Employee' class in models.py which is a subclass of 'django.contrib.auth.models.User'.
I have a WorkHour class in models.py which has a foreign key to 'Employee'.
In admin.py I have WorkHourAdmin which defines a list_filter attribute which includes the field 'employee'. This field is the foreign key to Employee.
The employee filter on 'WorkHour' admin object will generate a lookup key like this: 'employee user_ptr exact'
In changeset 15031 this does not the pass the check in 'django/contrib/admin/options.py' line 243
The field 'employee user ptr' is not defined in the 'list_filter' attribute on class WorkHourAdmin. But according to me this is a valid filtering.
Attachments (9)
Change History (23)
by , 14 years ago
comment:1 by , 14 years ago
Component: | Uncategorized → django.contrib.admin |
---|---|
Keywords: | filtering subclassed object not allowed added |
comment:2 by , 14 years ago
Summary: | Changeset 15031 breaks filtering to objects which are are subclassed. → Changeset 15031 breaks filtering to objects which are subclassed. |
---|
comment:3 by , 14 years ago
Keywords: | regression blocker added |
---|---|
milestone: | → 1.3 |
Triage Stage: | Unreviewed → Accepted |
comment:4 by , 14 years ago
Resolution: | → worksforme |
---|---|
Status: | new → closed |
I have added a simplified test case that instead of having Employee inheriting from User makes it inherit from a Person model and it shows the filtering of WorkHour instances based on Employee is working (it also works if I undo the change introduced in r15139.)
Unfortunately you don't tell us what is the error you see in the admin.
Find attached the modification for the Django test suite as a patch, I've also implemented this as a standalone test app so we can test this interactively, it is in the attached tarball.
I'm going to close this ticket, please reopen if you can give us more details about what's the error condition you see.
follow-up: 8 comment:5 by , 14 years ago
Hello,
Your testsuite contains an error according to me. On line 392 you should reassign the variable 'response'. On line 393 you are still testing the 'response' assigined on line 389 which returned a '200' status code.
Attached you will a modified version of the standalone project you provided. I added a unit test, based upon your unit-test. A transscript of this unit-test is also added.
comment:6 by , 14 years ago
Please run the attached standalone project with Django 1.2.4. The included test-suite is not compatible with the current development version of Django.
--
For the source and explanation of the error, please see attached file 'explanation.txt'.
comment:7 by , 14 years ago
Resolution: | worksforme |
---|---|
Status: | closed → reopened |
comment:8 by , 14 years ago
milestone: | 1.3 |
---|
Replying to rene:
Hello,
Your testsuite contains an error according to me. On line 392 you should reassign the variable 'response'. On line 393 you are still testing the 'response' assigined on line 389 which returned a '200' status code.
You are completely right, thanks for noting this. I can reproduce what you report using 1.2.4 and with a checkout of the 1.2.X branch as of now.
And there lies the critical difference. I assumed this report was against the SVN development trunk (possibly because it references commit r15031 (trunk) instead of r15033 (1.2.X branch) and because the milestone for the ticket had been set to 1.3), but from a latter comment you posted I can see now you are reporting this against the 1.2.4 release.
My reports about not seeing this problem both with the automated test suite (even if I correct that response =
assignment) and when playing interactively with the admin app with the attached project were also correct. Because I'm testing against the SVN trunk.
So, in conclusion, this an error that isn't present in trunk but it is in the 1.2.X bug-fix branch. Updating ticket fields accordingly.
comment:9 by , 14 years ago
Oke, sorry my mistake for wrong ticket headers. I am glad you could reproduce the error.
Do you have any idea about a solution? For now I just overwrite the method in my AdminClass and do some additional testing for this special case. But it is not a generic solution.
comment:10 by , 14 years ago
milestone: | → 1.3 |
---|
Ramiro - this still needs to be on the 1.3 milestone because it needs to be addressed before we can release 1.3. If it isn't addressed, 1.2 will go into security-fix mode with this issue lingering.
comment:11 by , 14 years ago
Has patch: | set |
---|
comment:12 by , 14 years ago
Resolution: | → fixed |
---|---|
Status: | reopened → closed |
(In [15177]) [1.2.X] Fixed #15032 -- Replaced 1.2.X implementation of admin changelist filtering security fix (r15031/r15033) with the one from trunk so another valid filter usage scenario (using model inheritance) is still possible. Thanks rene for reporting this.
Sample models.py file