Improved handling for HTTP 403 Errors

[Nicola]

in core/handlers/base.py is hardcoded an h1:

<h1>Permission denied</h1>

so this string not transable,

regards
drakkan

[Jacob]

(In [10346]) Fixed #9847: mark the permission denied message for translation.

[Jacob]

Reopening - the fix was a bad idea; see [10347].

[Adam Nelson]

core/base.diff

[Adam Nelson]

There is no need to have a message at all. 'Permission Denied' is presumed with the 403 HttpResponseForbidden error. If a message must be delivered, it should be added to django/http/init.py for all HttpResponseForbidden errors.

[James Bennett]

I think there should be content in the response, since otherwise you'd show a completely blank page to an end user, requiring them to know about HTTP status codes and how to find out what status code we sent back.

I can see a couple options for doing this:

1. Don't bother trying to translate the response content, and just use the language in the HTTP spec (so just "403 Forbidden").
2. Set a default content value for HttpResponseForbidden, and mark it for translation. I think we can do that safely since it's over in django.http and settings will already be set up by the time it gets imported.

[James Bennett]

Also, this is a minor enough issue that it really ought to happen on 1.3 or a 1.2.X bugfix release.

[Adam Nelson]

I take back what I said about not having a message at all.

1. It shouldn't be hardcoded in base.py, maybe it should be in source:django/trunk/django/http/__init__.py ?
2. All the non-200 HTTP errors should derive from a master error class. Right now it's kind of a hodge-podge it seems.

I agree that milestone:1.3 is best.

[vkryachko]

Why not create an exception subclass which would be raised in base.py and caught by a middleware to display a custom 403 page, like with Http404 ?

[vkryachko]

OK, I see middleware is not involved in this process, but it's possible to add a method to the resolver, resolver.resolve403() just like resolve404 and resolve500 which could render 403.html template and if that fails just return 'Permission Denied'

[vkryachko]

Sorry base.3.diff was wrong

[vkryachko]

So here's the patch, I think that this way it would be more flexible so that you can customize the content of 403 as much as you wish. Besides it makes exception handling more consistent, you just create 403.html in the root of your templates dir just like for 404 and 500. What do you think?

[Steven L Smith]

I've closed #5515 and #10360 and changed the title of this ticket to better reflect the problem we hope to solve. Having one ticket instead of three will greatly improve the chances of resolution.

[Steven L Smith]

I'm going to try and get this ready for checkin today at the djangocon sprint.

[Steven L Smith]

Added tests and docs... anyone care to test my tests?

[Steven L Smith]

Need some more feedback on this one, so we're going to hold off... in any case, the tests will be useful even if we do something other than a bunch of resolve4xx stuff. Slippery slope... we don't really want 17 different resolve401, resolve402 etc.

[Malcolm Tredinnick]

Tests and docs are fine. As noted, I'd like a way to avoid requiring the module-level resolver403 function. Those functions are really annoying (they require import * from the default url stuff to be used, for example).

[Łukasz Rekucki]

Just for the record, after [13590] the handlerXXX functions don't require an "import *". This still calls for a more general solution then just putting another function. Also, the current patch required a "403.html" template, but doesn't provide it. This gets catch by "except:"``, but that's at least bad style to me. If I make a syntax error in the ``"403.html" I would expect a 500 and a trace, so that my tests fail when checking the returned status code.

[James Bennett]

1.3 is feature-frozen now.

[luke crouch]

Bump. Is this in the running for 1.3.x?

[Luke Plant]

It can't go in 1.3.X, as we don't backport features. It could get into 1.4. It currently has 'Patch needs improvement' for some reason. If that is resolved, and it can promoted to 'Ready for checkin', it stands a good chance for 1.4. The way to get things included is to follow our ​contributing howto to make sure the ticket moves along nicely.

[Chris Beaven]

​https://github.com/SmileyChris/django/compare/master...9487-better-http403-errors

[Jannis Leidel]

In [16606]:

Fixed #9847 -- Added 403 response handler. Many thanks to kgrandis, adamnelson, vkryachko, fvox13 and Chris Beaven.

[GitHub <noreply@…>]

In c187417:

Refs #9847 -- Added tests for handler403 resolution.