Changeset 15031 breaks filtering to objects which are subclassed.

[rene]

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.

[rene]

Sample models.py file

[rene]

Sample admin.py file

[Ramiro Morales]

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.

[Ramiro Morales]

Patch for the Django test suite

[Ramiro Morales]

Standalone project

[rene]

Stand alone project with test-suite.

[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.

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.

[rene]

Transscript of run of unit-test

[rene]

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'.

[rene]

Explanation of source of error

[Ramiro Morales]

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.

[rene]

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.

[Russell Keith-Magee]

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.

[Ramiro Morales]

Fix plus tests for 1.2.X branch

[Ramiro Morales]

Addition to regression tests for trunk

[Ramiro Morales]

(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.

[Ramiro Morales]

In [15555]:

[1.1.X] Fixed #15306 -- Replaced 1.1.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 dbenamy for reporting this. Refs #15032.

[Jacob]

Milestone 1.3 deleted