On Tue, Sep 7, 2010 at 4:08 AM, Patryk Zawadzki <pat...@pld-linux.org> wrote:
> Hi,
>
> Since CSRF is already being reafactored up-side down in the trunk, I
> thought it might be a good idea to propose a slight modification.
Erm... it is? That's news to me.
Django 1.2 introduced a bunch of very big changes, but those changes
are signed, sealed and delivered. I'm not aware of any major changes
that have already been made, or are planned.
> Currently CSRF either falls through to the resolved view function or
> calls settings.CSRF_FAILURE_VIEW. More than once I've found myself
> wanting something in between. In such cases I'd like the logic flow to
> be able to reach the view, just telling me that CSRF did not validate.
>
> Let's say we add a new attribute to the request, call it "validated"
> and make it default to True for GET and False for everything else. Now
> split the CSRF middleware into two separate pieces of code. One
> middleware that does the validation and sets request.validated to True
> on success. One middleware that checks for (request.method == 'POST'
> and not request.validated) and in such cases returns
> settings.CSRF_FAILURE_VIEW.
>
> "How is that useful?" I hear you ask.
>
> class SecureForm(forms.Form):
> def __init__(self, *args, **kwargs):
> self.secure = kwargs.pop('secure', False)
> return super(SecureForm, self).__init__(*args, **kwargs)
> def _clean_form(self, *args, **kwargs):
> if not self.secure:
> self._errors[NON_FIELD_ERRORS] = self.error_class([
> 'We could not confirm that the request originated from
> your machine. Please resubmit to continue.'
> ])
> else:
> super(SecureForm, self)._clean_form(self, *args, **kwargs)
>
> def MyForm(SecureForm):
> foo = forms.CharField()
>
> def my_view(request):
> myform = MyForm(request.POST or None, request.FILES or None,
> secure=request.validated)
> if myform.is_valid():
> # ...
> pass
> return direct_to_template(request, 'my.html', {'form': myform})
Hrm. I see what you're doing here.
Firstly, I'm not wild about "secure=request.validated". This looks
like a really simple way for people to say "secure=True" as a way of
"fixing" CSRF support that they can't get to work. The choice of
argument on the attribute isn't optional -- it *must* be
request.validated. So really, it's the request that is the argument
that needs to be passed in. The good news on this point is that a
"request aware form" is something that has been floated in other
discussions recently. I'll be sure to raise it at the DjangoCon
sprints this week as a topic for discussion.
Secondly, IMHO there is a lot of value in the fact that Django forces
the raising of a 403, rather than a 200 with an error message on the
page. This isn't a form input error. It's a catastrophic problem that
should only be observed when the user is actually under attack, or if
cookies aren't available. Displaying a CSRF failure in the same way as
an error for not putting 12 digits in your credit card number strikes
me as the wrong way to visualize the error case. It implies that the
problem can be fixed by user interaction when it can't -- at least,
not by fixing form inputs. In order to give instructions on why
cookies are needed and how to enable them, we need a lot more real
estate... like a CSRF view.
Yours,
Russ Magee %-)
--
You received this message because you are subscribed to the Google Groups
"Django developers" group.
To post to this group, send email to django-develop...@googlegroups.com.
To unsubscribe from this group, send email to
django-developers+unsubscr...@googlegroups.com.
For more options, visit this group at
http://groups.google.com/group/django-developers?hl=en.
