[Django] #17985: Add additional lookup_allowed whitelist functionality to ModelAdmin
Django
-------------------------------+--------------------
Reporter: 3point2 | Owner: nobody
Type: New feature | Status: new
Component: contrib.admin | Version: 1.4
Severity: Normal | Keywords:
Triage Stage: Unreviewed | Has patch: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------+--------------------
Right now, as a result of the security fix introduced in r15031, the only
way to allow querystring lookups across relationships in the admin is to
whitelist them by including them in list_filter.
However, in my application the lookup that needs to be whitelisted
generates a huge filter widget as it contains thousands of instances.
It would be helpful if I could whitelist the exact lookup I need to link
to without having to generate the filter widget itself.
Something like
{{{
class MyModelAdmin(ModelAdmin):
allow_lookup = ["fieldname__id__exact"]
}}}
would do. If the developers agree this is useful functionality, I could
write a patch.
--
Ticket URL: <https://code.djangoproject.com/ticket/17985>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
Reporter: 3point2 | Owner: nobody
Type: New feature | Status: closed
Component: contrib.admin | Version: 1.4
Severity: Normal | Resolution: wontfix
Keywords: | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Changes (by julien):
* status: new => closed
* needs_better_patch: => 0
* resolution: => wontfix
* needs_tests: => 0
* needs_docs: => 0
Comment:
Thanks for the suggestion, but you can already easily achieve this by
overriding the `ModelAdmin.lookup_allowed()` method. So there is no need
for introducing a new class attribute.
--
Ticket URL: <https://code.djangoproject.com/ticket/17985#comment:1>
Django
Reporter: 3point2 | Owner: nobody
Type: New feature | Status: reopened
Component: contrib.admin | Version: 1.4
Severity: Normal | Resolution:
Keywords: | Triage Stage: Unreviewed
Changes (by 3point2):
* status: closed => reopened
* resolution: wontfix =>
Comment:
Sorry to re-open. I'm fine with overriding lookup_allowed, but I opened
this ticket because I feel like this is a feature that is generally
useful, and lookup_allowed is undocumented. I feel like this functionality
should be officially supported, and overriding an undocumented method is
more of a work-around. Also see http://www.hoboes.com/Mimsy/hacks/fixing-
django-124s-suspiciousoperation-filtering/lookup_allowed-gets-new-
parameter-value/
At the very least, documenting lookup_allowed would be helpful.
If on the other hand you feel that this functionality is not a common use
case, I'm fine with closing the ticket and sticking with the work around.
--
Ticket URL: <https://code.djangoproject.com/ticket/17985#comment:2>
Django
-------------------------------+------------------------------------
Reporter: 3point2 | Owner: nobody
Type: New feature | Status: reopened
Component: contrib.admin | Version: 1.4
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 1
-------------------------------+------------------------------------
Changes (by Fandekasp):
* cc: lemaire.adrien@… (added)
* needs_docs: 0 => 1
* easy: 0 => 1
* stage: Unreviewed => Accepted
Comment:
Renamed the ticket: Improving the documentation is a good idea.
--
Ticket URL: <https://code.djangoproject.com/ticket/17985#comment:3>
Django
Reporter: 3point2 | Owner: nobody
Type: New feature | Status: reopened
Component: contrib.admin | Version: 1.4
Severity: Normal | Resolution:
Keywords: | Triage Stage: Design
Has patch: 0 | decision needed
Needs tests: 0 | Needs documentation: 1
Easy pickings: 1 | Patch needs improvement: 0
| UI/UX: 0
-------------------------------------+-------------------------------------
Changes (by julien):
* stage: Accepted => Design decision needed
Comment:
I'm not sure we want to document this method yet. It has been introduced
recently (in 1.2.4) to fix a security issue, and has been modified quite a
bit since then, so it's quite unstable. At the very least, this needs more
thought before we make it part of the official API.
--
Ticket URL: <https://code.djangoproject.com/ticket/17985#comment:4>
Django
-------------------------------------+-------------------------------------
Type: New feature | Status: reopened
Component: Documentation | Version: 1.4
Severity: Normal | Resolution:
Keywords: | Triage Stage: Design
Has patch: 0 | decision needed
Needs tests: 0 | Needs documentation: 1
Easy pickings: 1 | Patch needs improvement: 0
| UI/UX: 0
-------------------------------------+-------------------------------------
* component: contrib.admin => Documentation
--
Ticket URL: <https://code.djangoproject.com/ticket/17985#comment:5>
Django
Reporter: 3point2 | Owner: nobody
Type: New feature | Status: new
Severity: Normal | Resolution:
Has patch: 0 | Needs documentation: 1
Easy pickings: 0 | UI/UX: 0
Changes (by aaugustin):
* easy: 1 => 0
* stage: Design decision needed => Accepted
Comment:
Julien, do you think lookup_allowed can be considered stable now?
--
Ticket URL: <https://code.djangoproject.com/ticket/17985#comment:7>
Django
-------------------------------+------------------------------------
Type: New feature | Status: new
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 1
Easy pickings: 0 | UI/UX: 0
Changes (by Zach):
* cc: zborboa@… (added)
--
Ticket URL: <https://code.djangoproject.com/ticket/17985#comment:8>
Django
Reporter: 3point2 | Owner: Simon Meers
Type: New feature | Status: assigned
Component: Documentation | Version: 1.4
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 1
Easy pickings: 0 | UI/UX: 0
Changes (by Simon Meers):
* owner: nobody => Simon Meers
* status: new => assigned
Comment:
Discussed in https://github.com/django/django/pull/8856
--
Ticket URL: <https://code.djangoproject.com/ticket/17985#comment:9>
Django
-------------------------------+---------------------------------------
Reporter: 3point2 | Owner: Simon Meers
Type: New feature | Status: assigned
Component: Documentation | Version: 1.4
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 1
Easy pickings: 0 | UI/UX: 0
Comment (by Simon Meers):
See https://github.com/django/django/pull/8904
Though I suspect I didn't actually need to create three separate pull
requests for the three (dependent but distinct) commits?
--
Ticket URL: <https://code.djangoproject.com/ticket/17985#comment:10>
Django
-------------------------------+---------------------------------------
Reporter: 3point2 | Owner: Simon Meers
Type: New feature | Status: assigned
Component: Documentation | Version: 1.4
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Simon Meers):
* needs_docs: 1 => 0
* has_patch: 0 => 1
--
Ticket URL: <https://code.djangoproject.com/ticket/17985#comment:11>
Django
-------------------------------+---------------------------------------
Reporter: 3point2 | Owner: Simon Meers
Type: New feature | Status: assigned
Component: Documentation | Version: 1.4
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Tim Graham):
* needs_better_patch: 0 => 1
Comment:
I suppose your thinking was that since `ModelAdmin.lookup_allowed()` isn't
documented, it's okay to make a backwards-incompatible change in adding
the `request` argument without a deprecation? Lately, Django has been more
conservative about changing undocumented APIs without a deprecation if
there's evidence that the API is at least somewhat widely used by
projects. A
[https://github.com/search?l=Python&q=lookup_allowed&ref=simplesearch&type=Code&utf8=%E2%9C%93
GitHub code search] reveals that's probably the case here.
If I were writing a patch, I would first document
`ModelAdmin.lookup_allowed()` as it is now and backport that documentation
to 1.11. (A separate pull request would ease merging that incrementally).
Then I'd implement a deprecation and corresponding documentation changes
for adding the `request` argument as part of #22569.
--
Ticket URL: <https://code.djangoproject.com/ticket/17985#comment:12>
Django
-------------------------------+---------------------------------------
Reporter: 3point2 | Owner: Simon Meers
Type: New feature | Status: assigned
Component: Documentation | Version: 1.4
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 1
Comment (by Simon Meers):
Thanks Tim, sounds like a good approach. Here is a documentation patch for
the current implementation to start with:
https://github.com/django/django/pull/8906
--
Ticket URL: <https://code.djangoproject.com/ticket/17985#comment:13>
Django
-------------------------------+---------------------------------------
Reporter: 3point2 | Owner: Simon Meers
Type: New feature | Status: assigned
Component: Documentation | Version: 1.4
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Simon Meers):
* needs_better_patch: 1 => 0
--
Ticket URL: <https://code.djangoproject.com/ticket/17985#comment:14>
Django
-------------------------------+---------------------------------------
Reporter: 3point2 | Owner: Simon Meers
Type: New feature | Status: assigned
Component: Documentation | Version: 1.4
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Simon Meers):
So the deprecation would involve:
- Adding a heads-up in `internals/deprecation.txt` that `request` will be
required in `2.0` (or do we have to go further ahead -- `2.1`?)
- Adding code to `lookup_allowed` in the current implementation that
accepts both parameter formats (with/without request) and provides a
deprecation warning if request is absent -- can this be in `1.11`, or does
that need to be in `2.0`?
- Then performing the change in https://github.com/django/django/pull/8904
(and removing the deprecation warning)
- Hopefully https://github.com/django/django/pull/8903 can be merged in
the meanwhile to close #28496 and lay the foundation for the simpler patch
for #22569?
--
Ticket URL: <https://code.djangoproject.com/ticket/17985#comment:15>
Django
-------------------------------+---------------------------------------
Reporter: 3point2 | Owner: Simon Meers
Type: New feature | Status: assigned
Component: Documentation | Version: 1.4
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Tim Graham):
The deprecation should start in Django 2.0 and `request` would be required
in Django 3.0 (use `RemovedInDjango30Warning`). You may find the
checklist for
[https://docs.djangoproject.com/en/dev/internals/contributing/writing-code
/submitting-patches/#deprecating-a-feature Deprecating a feature] helpful.
I plan to look at the patch for #28496 tomorrow.
--
Ticket URL: <https://code.djangoproject.com/ticket/17985#comment:16>
Django
-------------------------------+---------------------------------------
Reporter: 3point2 | Owner: Simon Meers
Component: Documentation | Version: 1.4
Severity: Normal | Resolution: fixed
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Tim Graham <timograham@…>):
* status: assigned => closed
* resolution: => fixed
Comment:
In [changeset:"60443e84b38ea3a143b0ef9c05b1e1f39d91ddb5" 60443e84]:
{{{
#!CommitTicketReference repository=""
revision="60443e84b38ea3a143b0ef9c05b1e1f39d91ddb5"
Fixed #17985 -- Documented ModelAdmin.lookup_allowed().
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/17985#comment:17>
Django
-------------------------------+---------------------------------------
Reporter: 3point2 | Owner: Simon Meers
Type: New feature | Status: closed
Component: Documentation | Version: 1.4
Severity: Normal | Resolution: fixed
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Tim Graham <timograham@…>):
In [changeset:"07f73daf6ba2994e572776edbbf3266f2f09d7a5" 07f73da]:
{{{
#!CommitTicketReference repository=""
revision="07f73daf6ba2994e572776edbbf3266f2f09d7a5"
[1.11.x] Fixed #17985 -- Documented ModelAdmin.lookup_allowed().
Backport of 60443e84b38ea3a143b0ef9c05b1e1f39d91ddb5 from master
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/17985#comment:18>