Django

Code

Ticket #5515 (reopened)

Opened 2 years ago

Last modified 8 months ago

Custom Permission Denied Pages

Reported by: Piotr Lewandowski <django@icomputing.biz> Assigned to:
Milestone: Component: Contrib apps
Version: SVN Keywords:
Cc: ubanus@users.sf.net, alexkon@gmail.com, rlaager@wiktel.com Triage Stage: Design decision needed
Has patch: 1 Needs documentation: 0
Needs tests: 0 Patch needs improvement: 0

Description

CSFR Middleware has hard-encoded error page in _ERROR_MSG string. It should be located in a separate template -- just like Http500 or Http404 error pages are.

Attachments

5515_make_403_errors_customizable.diff (8.8 kB) - added by progprog on 12/01/07 02:40:19.
Patch to make 403 responses work like 404 and 500 responses (customizable via handler)

Change History

09/16/07 17:20:20 changed by ubernostrum

  • needs_better_patch changed.
  • stage changed from Unreviewed to Accepted.
  • needs_tests changed.
  • needs_docs changed.

11/30/07 22:00:44 changed by progprog

  • owner changed from nobody to progprog.
  • status changed from new to assigned.

12/01/07 02:40:19 changed by progprog

  • attachment 5515_make_403_errors_customizable.diff added.

Patch to make 403 responses work like 404 and 500 responses (customizable via handler)

12/01/07 02:45:42 changed by progprog

  • owner deleted.
  • status changed from assigned to new.
  • has_patch set to 1.
  • summary changed from CSFR has hard-encoded error page to CSRF has hard-encoded error page.

Patch with tests added.

I decided that in general Django should have customizable 403 pages, a la 404/500, so my patch deals with a larger scope than is described by this ticket. After making 403 customizable, the CSRF middleware simply raises the PermissionDenied? exception with a custom message, and get_response() handles the rest.

One consequence of this is that a 403.html template will have to be declared, similar to 404.html and 500.html.

12/01/07 22:17:06 changed by Simon G <dev@simon.net.nz>

  • stage changed from Accepted to Ready for checkin.

(follow-up: ↓ 6 ) 12/04/07 13:21:27 changed by jacob

  • stage changed from Ready for checkin to Design decision needed.

I'd like to have more discussion on this before checking it in. I don't like the feature creap of handler403... handler447 is only one more step down that slippery slope.

(in reply to: ↑ 5 ) 12/09/07 00:16:00 changed by progprog

Replying to jacob:

I'd like to have more discussion on this before checking it in. I don't like the feature creap of handler403... handler447 is only one more step down that slippery slope.

I agree - as I was doing the ticket I thought "Django needs a more generic way to do this." However, designing and implementing a system of generic hooks for different response codes could take weeks or months, and this patch does fulfill an immediate need, so I decided to forge ahead anyway.

08/26/08 12:52:52 changed by Jakub Wilk <ubanus@users.sf.net>

  • cc set to ubanus@users.sf.net.

09/03/08 06:03:28 changed by anonymous

  • milestone set to post-1.0.

01/29/09 08:05:11 changed by alexkon

  • cc changed from ubanus@users.sf.net to ubanus@users.sf.net, alexkon@gmail.com.

02/25/09 13:51:44 changed by

  • milestone deleted.

Milestone post-1.0 deleted

07/26/09 16:35:21 changed by anonymous

  • cc changed from ubanus@users.sf.net, alexkon@gmail.com to ubanus@users.sf.net, alexkon@gmail.com, rlaager@wiktel.com.

07/26/09 19:36:53 changed by lukeplant

  • status changed from new to closed.
  • resolution set to duplicate.

The patch in #9977 addresses this issue by allowing the user to provide their own view function for the CSRF failure view. This is more useful than just a template, as the view can do logic based on the contents of the request. So I'm closing as duplicate, since the other patch, which includes big changes to the CSRF functionality, is very likely to go in post 1.1

(follow-up: ↓ 14 ) 07/27/09 06:41:32 changed by anonymous

Does the patch in #9977 provide for a 403.html template for PermissionDenied? errors? I just installed this patch so I could create user-friendly Permimssion Denied pages.

(in reply to: ↑ 13 ) 07/27/09 11:58:37 changed by lukeplant

Replying to anonymous:

Does the patch in #9977 provide for a 403.html template for PermissionDenied? errors?

No, it doesn't. I guess if you removed the CSRF stuff from the patch, this would be useful in its own right, and the bug title would probably want to change. A problem I have with the patch, on top of the concerns Jacob raised, is the issue of i18n — if we want to solve this, we want to do it right.

07/29/09 02:41:04 changed by rlaager@wiktel.com

  • status changed from closed to reopened.
  • resolution deleted.
  • summary changed from CSRF has hard-encoded error page to Custom Permission Denied Pages.

What i18n concerns do you have? Enabling a 403.html wouldn't necessarily even involve shipping a 403.html. If you wanted to though, surely the text could be wrapped with trans like everything else?

07/29/09 05:49:30 changed by lukeplant

Sorry, I saw a non-i18n-ized template in the patch, but that was just for the tests.

Like Jacob, I've got concerns about whether this is the right thing. In favor of the ticket, it is probably a good idea to allow the user to customise any user visible response that is returned by Django core code, which means providing some way of giving a template for HttpPermissionDenied errors.

Also as the patch stands, I don't like the backwards incompatibility - developers have to create a template for 403s to continue to work. Hardcoding a simple fallback template (possibly in django/views/defaults.py itself) would solve that, even if it doesn't feel very nice.

To summarise, we need to:

  • discuss whether this is the right thing
  • remove CSRF stuff from patch
  • sort out backwards compatibility for 403s.

Add/Change #5515 (Custom Permission Denied Pages)




Change Properties
Action