Opened 7 years ago

Closed 6 years ago

Last modified 5 years ago

#11513 closed (fixed)

Admin Change Page Redirects to Permission Denied

Reported by: rlaager Owned by: nobody
Component: contrib.admin Version:
Severity: Keywords:
Cc: Triage Stage: Ready for checkin
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: UI/UX:

Description

If ModelAdmin is subclassed to override has_change_permission() to provide row-level change permissions, it's possible to edit an object, click save, and be redirected to the list of all such objects and get a Permission Denied error page. The attached patch checks for the appropriate permissions and does the right thing. This sort of check exists elsewhere in the file, just not here.

Attachments (2)

admin-change-redirect-permissions-check.diff (626 bytes) - added by rlaager 7 years ago.
11513_adming_change_view_redirect.diff (5.7 KB) - added by Julien Phalip 6 years ago.

Download all attachments as: .zip

Change History (10)

Changed 7 years ago by rlaager

comment:1 Changed 7 years ago by Alex Gaynor

Needs documentation: unset
Needs tests: unset
Patch needs improvement: unset
Triage Stage: UnreviewedAccepted

comment:2 Changed 7 years ago by Chris Beaven

Component: Uncategorizeddjango.contrib.admin
Needs tests: set
Version: 1.0

Changed 6 years ago by Julien Phalip

comment:3 Changed 6 years ago by Julien Phalip

milestone: 1.3
Needs tests: unset

Thanks for the report rlaager. The behaviour should actually be slightly different from that in your patch. Like the 'add' view, it should redirect to the admin's root page if there are no appropriate 'change' permissions. The new patch fixes that and also contains thorougher tests for row level 'change' permissions.

comment:4 Changed 6 years ago by Russell Keith-Magee

Triage Stage: AcceptedReady for checkin

Looks good; my only suggested improvement would be to use named URL lookup instead of ../ and ../../../. However, that's easy enough to pick up on commit, so if you don't get around to updating the patch, I'll do it when I commit.

comment:5 Changed 6 years ago by Julien Phalip

Good point about the hard coded urls. There are still lots of occurrences like this, and perhaps it's best to tackle them all at once. See #15294.

comment:6 Changed 6 years ago by Russell Keith-Magee

Resolution: fixed
Status: newclosed

In [15584]:

Fixed #11513 -- Ensure that the redirect at the end of an object change won't redirect to a page for which the user doesn't have permission. Thanks to rlaager for the report and draft patch, and to Julien Phalip for the final patch.

comment:7 Changed 6 years ago by Russell Keith-Magee

In [15586]:

[1.2.X] Fixed #11513 -- Ensure that the redirect at the end of an object change won't redirect to a page for which the user doesn't have permission. Thanks to rlaager for the report and draft patch, and to Julien Phalip for the final patch.

Backport of r15584 from trunk.

comment:8 Changed 5 years ago by Jacob

milestone: 1.3

Milestone 1.3 deleted

Note: See TracTickets for help on using tickets.
Back to Top