Code

Opened 5 years ago

Closed 3 years ago

Last modified 3 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 5 years ago.
11513_adming_change_view_redirect.diff (5.7 KB) - added by julien 3 years ago.

Download all attachments as: .zip

Change History (10)

Changed 5 years ago by rlaager

comment:1 Changed 5 years ago by Alex

  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset
  • Triage Stage changed from Unreviewed to Accepted

comment:2 Changed 5 years ago by SmileyChris

  • Component changed from Uncategorized to django.contrib.admin
  • Needs tests set
  • Version 1.0 deleted

Changed 3 years ago by julien

comment:3 Changed 3 years ago by julien

  • milestone set to 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 3 years ago by russellm

  • Triage Stage changed from Accepted to Ready 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 3 years ago by julien

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 3 years ago by russellm

  • Resolution set to fixed
  • Status changed from new to closed

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 3 years ago by russellm

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 3 years ago by jacob

  • milestone 1.3 deleted

Milestone 1.3 deleted

Add Comment

Modify Ticket

Change Properties
<Author field>
Action
as closed
as The resolution will be set. Next status will be 'closed'
The resolution will be deleted. Next status will be 'new'
Author


E-mail address and user name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.