[Django] #34172: Documentation of AdminSite.get_urls() encourages security vulnerabilities
Django
----------------------------------------------+------------------------
Reporter: Sylvain Fankhauser | Owner: nobody
Type: Uncategorized | Status: new
Component: contrib.admin | Version: 4.1
Severity: Normal | Keywords:
Triage Stage: Unreviewed | Has patch: 0
Needs documentation: 0 | Needs tests: 0
Patch needs improvement: 0 | Easy pickings: 0
UI/UX: 0 |
----------------------------------------------+------------------------
The documentation for AdminSite.get_urls()
(https://docs.djangoproject.com/en/dev/ref/contrib/admin/#django.contrib.admin.ModelAdmin.get_urls)
starts with an example that doesn’t use `self.admin_site.admin_view` and
only mentions later that this code doesn’t actually have any permission
check applied.
I think showing vulnerable code is a bad idea, as some people might stop
reading there and end up with admin views publicly reachable. Also the
docs themselves say below the example "this is usually not what you want".
My proposal would be to change the default example and show the code with
`admin_site.admin_view` first, with an explanation below of what it does
(without any code that would make the view publicly reachable).
--
Ticket URL: <https://code.djangoproject.com/ticket/34172>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
Reporter: Sylvain Fankhauser | Owner: nobody
Type: Uncategorized | Status: new
Component: contrib.admin | Version: 4.1
Keywords: | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by David Sanders):
I think you have a valid point 👍
Interested in submitting a documentation PR?
Just FYI small documentation fixes don't require a ticket, I think this
could fall under that category of not needing one 🙂
For reference the documentation follows the Diátaxis framework:
https://diataxis.fr/
--
Ticket URL: <https://code.djangoproject.com/ticket/34172#comment:1>
Django
------------------------------------+--------------------------------------
Reporter: Sylvain Fankhauser | Owner: nobody
Type: Uncategorized | Status: new
Component: contrib.admin | Version: 4.1
Severity: Normal | Resolution:
Keywords: | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
------------------------------------+--------------------------------------
Comment (by Mariusz Felisiak):
`get_urls()` docs contains a step by step example with further required
elements, first an example without `admin_view()`, than comments what is
missing:
> ''However, the self.my_view function registered above suffers from two
problems:''
> ''- It will not perform any permission checks, so it will be accessible
to the general public.''
and a second example with the `admin_view()` wrapper. I wouldn't change
anything, IMO it's nicely constructed.
--
Ticket URL: <https://code.djangoproject.com/ticket/34172#comment:2>
Django
Reporter: Sylvain Fankhauser | Owner: nobody
Component: contrib.admin | Version: 4.1
Severity: Normal | Resolution:
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Carlton Gibson):
* type: Uncategorized => Cleanup/optimization
* stage: Unreviewed => Accepted
Comment:
I tend to agree with the report here:
> ... as some people might stop reading there ...
I think that's likely very common. Folks just copy and paste without
really reading.
I take Mariusz' point that it's explained, but if a re-phrase is on offer,
having one correct example with a ''couple of things to note... '' below,
I think we should have a look at that.
I'll Accept on that basis (assuming that's why Mariusz left it unreviewed)
> Interested in submitting a documentation PR?
Sylvain, if you wanted to assign it to yourself and open a PR, that would
be great.
--
Ticket URL: <https://code.djangoproject.com/ticket/34172#comment:3>
Django
Reporter: Sylvain Fankhauser | Owner: Sylvain
Type: | Fankhauser
Cleanup/optimization | Status: assigned
Component: contrib.admin | Version: 4.1
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Sylvain Fankhauser):
* owner: nobody => Sylvain Fankhauser
* status: new => assigned
Comment:
Thanks for your feedback! I’ll work on a proposal soon.
--
Ticket URL: <https://code.djangoproject.com/ticket/34172#comment:4>
Django
-------------------------------------+-------------------------------------
Reporter: Sylvain Fankhauser | Owner: Sylvain
Type: | Fankhauser
Cleanup/optimization | Status: assigned
Component: contrib.admin | Version: 4.1
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Changes (by Sylvain Fankhauser):
* has_patch: 0 => 1
Comment:
[https://github.com/django/django/pull/16355 PR]
--
Ticket URL: <https://code.djangoproject.com/ticket/34172#comment:5>
Django
-------------------------------------+-------------------------------------
Reporter: Sylvain Fankhauser | Owner: Sylvain
Type: | Fankhauser
Cleanup/optimization | Status: assigned
Component: contrib.admin | Version: 4.1
Severity: Normal | Resolution:
| checkin
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* stage: Accepted => Ready for checkin
--
Ticket URL: <https://code.djangoproject.com/ticket/34172#comment:6>
Django
-------------------------------------+-------------------------------------
Reporter: Sylvain Fankhauser | Owner: Sylvain
Type: | Fankhauser
Component: contrib.admin | Version: 4.1
Severity: Normal | Resolution: fixed
Keywords: | Triage Stage: Ready for
| checkin
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* status: assigned => closed
* resolution: => fixed
Comment:
In [changeset:"0036bcdcb65874f63fff8139fe86574fa155eb26" 0036bcd]:
{{{
#!CommitTicketReference repository=""
revision="0036bcdcb65874f63fff8139fe86574fa155eb26"
Fixed #34172 -- Improved ModelAdmin.get_urls example.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/34172#comment:7>
Django
-------------------------------------+-------------------------------------
Reporter: Sylvain Fankhauser | Owner: Sylvain
Type: | Fankhauser
Cleanup/optimization | Status: closed
Component: contrib.admin | Version: 4.1
Severity: Normal | Resolution: fixed
Keywords: | Triage Stage: Ready for
| checkin
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Carlton Gibson <carlton.gibson@…>):
In [changeset:"3137174344775fa2358e39cd90e6137f292f8daa" 3137174]:
{{{
#!CommitTicketReference repository=""
revision="3137174344775fa2358e39cd90e6137f292f8daa"
[4.1.x] Fixed #34172 -- Improved ModelAdmin.get_urls example.
Backport of 0036bcdcb65874f63fff8139fe86574fa155eb26 from main
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/34172#comment:8>