On Sep 19, 4:56 pm, Russell Keith-Magee <freakboy3
...@gmail.com>
wrote:
> End users should be allowed to be as lazy as they like, but
> their laziness shouldn't open security holes in an app that Django
> ships, since the contrib apps (and admin in particular) are the
> obvious first port of call for any systematic CSRF attack.
> So - let's have both. A middleware enabled by default protects the
> rest of the views. However, we can also have a view decorator lets us
> protect admin (and other contrib apps) explicitly.
I'm a big +1 for that.
> * The SafeForm method of reporting errors as part of form validation
> is much nicer than the 403 page, IMHO. However, I'm not sure what we
> can do to provide form-level CSRF error handling without introducing a
> whole bunch of other fragility.
I've been scratching my head over this one - inline error messages and
(in particular) not using a form submission are the single thing that
makes me favour SafeForm over the template tag approach for my own
stuff. That's why I designed SafeForm to complement the forms
framework - correctly handling CSRF feels like a form validation
problem to me, and reusing Django's redisplay-the-form-with-errors
flow is a really neat thing to be able to do.
The main reason I really like preserving form data is that it means
CSRF failures are less of a problem, allowing us to be much more
strict about how they work (setting form tokens to expire after a few
hours, tying tokens to individual forms etc). This means we can reduce
the damage caused in the case of a token leaking somehow. This becomes
particularly important if tokens are going to be used to protect GET
requests, which some applications may want to do (Flickr have CSRF-
protected GETs for their logout and change-language links, for
example) - GET tokens are more likely to leak in other people's
referrer logs or to intermediate proxies.
There is a way we could achieve form validation, but it isn't
particularly pretty. Say the CSRF check is performed by a decorator
(which has an accompanying middleware for applying that decorator to
the entire site). The problem we have is that in the case of a
failure, the fact that the check failed needs to be communicated
through to the form /inside/ that view. The form doesn't have access
to the request, so we're stuck.
But... the form does have access to request.POST. The dirty solution
would be for the decorator to rewrite request.POST to include a new
key:
request.POST.mutable = True
request.POST['__validation_must_fail'] = True
request.POST.mutable = False
django.forms.Form.is_valid() could then be hard-coded to always fail
if '__validation_must_fail' is present in the data dictionary. I've
avoided including the string "csrf" in the form key because this
feature feels like it could be useful for other things as well (I
might use it for testing out my form error styles). I don't think it's
a security vulnerability - if someone fakes a "__validation_must_fail"
key in their own submission I don't see that there's anything bad they
can do.
It's a pretty monstrous hack, but it would allow a CSRF protecting
decorator to inform a form inside a view that the check had failed and
the form should not validate.
I wouldn't suggest this as the default behaviour for the overall CSRF
protection scheme - it would leave hand-rolled forms unprotected, and
I imagine there are new error cases it would introduce. That said, it
would placate users like myself who want the nicer invalidation
messages and are willing to do a bit of work to achieve them. Maybe
something like this:
@csrf_protect
def view(request):
"""A view that fails the CSRF check with a full page 403,
as seen in Luke's current middleware"""
@csrf_protect_form
def view(request):
"""A view that won't 403 directly, but will ensure that any
calls to form.is_valid() fail due to the presence of a
__validation_must_fail key"""
Thinking more pie-in-the-sky-ly, maybe this approach could be
generalised further, to specify a mechanism by which request.POST can
include a set of validation errors that should be shown by the form.
This might have other uses in things like wizards where validation
errors may want to persist between different pages.
> * The requirement for using RequestContext is somewhat annoying. I
> can't see any obvious way to piggyback CSRF data onto the base Context
> itself, but perhaps we can make RequestContext a more prominent
> default? The most obvious solution here is to make render_to_response
> use RequestContext by default.
Yes, yes and thrice yes - my only complaint about RequestContext is
how horrible the syntax for using it is:
render_to_response('template.html', {
'name': 'Foo',
'blah': 'baoenuth',
}, context_instance=RequestContext(request))
<shudder>
Yeah, I'd like a builtin shortcut like that - used like this:
render(request, 'template_name.html', {'foo':bar })
The biggest problem, for me, is finding a decent name - since
'render_to_response' is already taken. Maybe something as vague as
'standard_response' would work.
On Sep 19, 7:57 pm, Luke Plant <L.Plant...@cantab.net> wrote:
> Yeah, I'd like a builtin shortcut like that - used like this:
> render(request, 'template_name.html', {'foo':bar })
> The biggest problem, for me, is finding a decent name - since
> 'render_to_response' is already taken. Maybe something as vague as
> 'standard_response' would work.
Every single one of my Django projects has its own "render(request,
template, dictionary)" function somewhere as a result.
I'd be enormously happy to see that go in as django.shortcuts.render -
the name may not be as descriptive as render_to_response but for
something used in practically every view I think terseness should beat
descriptiveness. Alternatively, I've been experimenting recently with
a TemplateResponse class which lets you do this:
def view(request):
return TemplateResponse(request, 'my_template.html', {'foo':
'bar'})
It's an HttpResponse class that's designed to be lazily evaluated.
This makes it really useful for subclassing - you can do things like
this:
class MyGenericView(object):
def view(self, request):
# Do something complicated
return TemplateResponse(request, 'my_template.html', {'foo':
'bar'})
class MyCustomisedView(object):
def view(self, request):
response = super(MyCustomisedView, self).view(request)
response.context['foo'] = 'baz'
if is_mobile_phone(request):
response.template = 'my_mobile_template.html'
return response
This pattern would be particularly valuable for customising the admin,
which currently uses nasty extra_context argument hacks to achieve
something that isn't nearly as useful.
My current implementation of TemplateResponse is here:
http://github.com/simonw/django-openid/blob/master/django_openid/resp...
- there's a SimpleTemplateResponse as well which doesn't use
RequestContext (but has a more verbose name to discourage its use).
That was a bit of a tangent... but to conclude:
1. I've grudgingly accepted that a template tag based solution is
much, much more practical for inclusion in Django than the SafeForm
option, purely due to the difficulty in porting existing code and the
differences between the two form APIs.
2. I'm not at all keen on the implementation as a middleware
(especially a view middleware, which doesn't play well with generic
views and redispatching to other view functions, both patterns I like
a lot). I'd be perfectly happy with a decorator that is also available
as a middleware.
3. I'd like to include support for CSRF form tokens to expire and be
locked down to individual forms, as seen in SafeForm. Whether or not
we actually go for that depends on how likely we are to find a
solution to the next point...
4. I'd love it if we could find a way for developers who care (such as
myself) to opt-in to CSRF-failing gracefully at the form validation
level. I'm confident this should be possible, but I don't think it's
necessary to make it the default behaviour (the CSRF 403 screen is
probably fine for most people).
Luke: any thoughts on the above? I'd be happy to contribute code.
Cheers,
Simon
