[Django] #26954: has_module_permission set to False on ModelAdmin raises 403 Forbidden error when visiting its app page
Django
when visiting its app page
-------------------------------+--------------------
Reporter: bhch | Owner: nobody
Type: Bug | Status: new
Component: contrib.admin | Version: 1.8
Severity: Normal | Keywords:
Triage Stage: Unreviewed | Has patch: 0
Easy pickings: 1 | UI/UX: 1
-------------------------------+--------------------
Suppose an app has 2 (or more) models, and we hide one model from the
admin index page by returning `False` in `has_module_permission` method of
`ModelAdmin`. Now, if we visit the admin index page, we see all the models
under our app except the one we hid. So far so good.
But if we visit the app page (at /admin/<app_label>/) we get a `403
Forbidden` error. Whereas one would expect to see the app page with the
models listed there, except of course the hidden ones.
--
Ticket URL: <https://code.djangoproject.com/ticket/26954>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
when visiting its app page
Reporter: bhch | Owner: nobody
Type: Bug | Status: new
Component: contrib.admin | Version: 1.8
Keywords: | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 1
Changes (by kezabelle):
* cc: django@… (added)
* needs_better_patch: => 0
* needs_tests: => 0
* needs_docs: => 0
Comment:
Chiming in as I believe I encouraged bhch to open this. I've not tested
the ticket's validity, but I'm familiar with the start of the whole
changeset, and looking at the code over time I can see how it may be
correct. YMMV.
The notion of raising {{{PermissionDenied}}} originated in #21063 and
landed in commit 0d74f9553c6acb8b53a502ca5e39542dcc4412c1, probably off
the back of #21056 (which landed in
170f72136758add6c9c0c59240cfce73d5672cc2). Both are my doing.
The idea was that, knowing an app label was valid (either because of the
urlconf changes having landed, or because Django held an idea of what
constituted a good app label anyway) it could run once before doing the
loop over the app's ModelAdmins and check whether there were any reason to
carry on.
In 504c89e8008c557a1e83c45535b549f77a3503b2 (referencing #6327) the check
was hoisted to be a member of a given ModelAdmin, which meant it had to
happen within the loop of ModelAdmins. This subtley regressed the original
check if one of the enumerated ModelAdmins returned a falsy value for
has_module_permission such that now a single ModelAdmin may be responsible
for causing a 403 for the whole app's index.
This looks to have happened between 1.7 and 1.8 and was ultimately brought
forward when the app dicts were combined in
bcf700b4e3393d8e72920ae64aa421c5651f8935 in 1.9. I can't find a ticket
number for this last change, though.
I'd argue that the original check (mine, regrettably, as it suggests a
bias :)) is correct, and the subsequent ones are incorrect for the
purposes of displaying the app index.
--
Ticket URL: <https://code.djangoproject.com/ticket/26954#comment:1>
Django
when visiting its app page
Reporter: bhch | Owner: nobody
Type: Bug | Status: new
Component: contrib.admin | Version: 1.8
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 1 | UI/UX: 0
-------------------------------+------------------------------------
Changes (by timgraham):
* ui_ux: 1 => 0
* stage: Unreviewed => Accepted
--
Ticket URL: <https://code.djangoproject.com/ticket/26954#comment:2>
Django
when visiting its app page
Reporter: bhch | Owner: halilkaya
Type: Bug | Status: assigned
Component: contrib.admin | Version: 1.8
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 1 | UI/UX: 0
-------------------------------+-------------------------------------
Changes (by halilkaya):
* owner: nobody => halilkaya
* status: new => assigned
--
Ticket URL: <https://code.djangoproject.com/ticket/26954#comment:3>
Django
when visiting its app page
Reporter: bhch | Owner: halilkaya
Type: Bug | Status: assigned
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 1 | UI/UX: 0
-------------------------------+-------------------------------------
Comment (by halilkaya):
I have opened a pull request for this ticket:
https://github.com/django/django/pull/6999
--
Ticket URL: <https://code.djangoproject.com/ticket/26954#comment:4>
Django
when visiting its app page
Reporter: bhch | Owner: halilkaya
Type: Bug | Status: assigned
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 1 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
-------------------------------+-------------------------------------
Changes (by timgraham):
* has_patch: 0 => 1
* needs_tests: 0 => 1
Comment:
Please uncheck "Needs tests" if you can add one.
--
Ticket URL: <https://code.djangoproject.com/ticket/26954#comment:5>
Django
when visiting its app page
Reporter: bhch | Owner: Halil Kaya
Type: Bug | Status: assigned
Component: contrib.admin | Version: 1.8
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------+--------------------------------------
Changes (by Tim Graham):
* needs_tests: 1 => 0
* easy: 1 => 0
Comment:
[https://github.com/django/django/pull/7367 Updated PR]. Keryn, can you
check it?
--
Ticket URL: <https://code.djangoproject.com/ticket/26954#comment:6>
Django
when visiting its app page
Reporter: bhch | Owner: Halil Kaya
Component: contrib.admin | Version: 1.8
Severity: Normal | Resolution: fixed
Keywords: | Triage Stage: Accepted
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:"2027d6acf79d6d1d18c804d942d63338fb6b8504" 2027d6a]:
{{{
#!CommitTicketReference repository=""
revision="2027d6acf79d6d1d18c804d942d63338fb6b8504"
Fixed #26954 -- Prevented ModelAdmin.has_module_permission()=False from
blocking access to the app index page.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/26954#comment:7>