Opened 9 years ago

Closed 6 years ago

#7167 closed New feature (wontfix)

Add a SafeForm / SecureForm to django.contrib.csrf

Reported by: mrts Owned by: nobody
Component: CSRF Version: master
Severity: Normal Keywords:
Cc: Triage Stage: Design decision needed
Has patch: yes Needs documentation: yes
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX:

Description

According to the discussion http://groups.google.com/group/django-developers/browse_thread/thread/832caf3d404af1eb/c4063b6303bce2f7 there seems to be a consensus that a secure form that embeds the security token into form data automatically as CsrfMiddleware does is should be added to Django.

Qouting Jacob:

... adding a SecureForm to django.contrib.csrf, and perhaps even de-emphasizing the middleware (which is a bit scary, frankly) in favor of the more explicit form.

I'm opening the ticket so that this will get the needed attention eventually.

Attachments (2)

7167-safeform.diff (5.1 KB) - added by ElliottM 8 years ago.
7167-safeform1.diff (5.7 KB) - added by ElliottM 8 years ago.
revised docstring

Download all attachments as: .zip

Change History (12)

comment:1 Changed 8 years ago by Simon Greenhill

Triage Stage: UnreviewedAccepted

comment:2 Changed 8 years ago by Michael Radziej

milestone: post-1.0

Changed 8 years ago by ElliottM

Attachment: 7167-safeform.diff added

comment:3 Changed 8 years ago by ElliottM

I made the patch on 2 assumptions:

  1. If the "session id" cookie has not been set in some way, either through the session middleware or manually by the developer, then SafeForm should always say the data submitted is invalid. The alternative is to have SafeForm act like a normal form. If you do this, you run into a situation where a developer that does not use sessions at all thinks his or her forms are safe when they are not.
  1. The SafeForm should be passed the entire HttpRequest object on top of everything you would normally send it. This one is kind of hard to decide on though. It looks funny as it is now, and people are inevitably going to ask "why do I need to pass data separately". The alternative is to only pass the cookies to the form. The advantage here is that people could then pass any dictionary, which would make it more flexible, and maybe usable outside of views. The downside is more problems later on as far as backwards-compatibility goes, because we'd be pretty much stuck with using cookies to generate the token.

There's one important decision to be made: Whether to use the clean() method or the clean_field() method for validation.

If you use the clean_field() way, a "(hidden field _csrf_token)" or something like that is added to error messages, which will not be customizable unless i'm mistaken.

That leaves us with the clean() method, but that will not be called unless the developer creating the subclass of SafeForm call is it, and you can GUARANTEE that someone will forget to call it and be left with a SafeForm that does not check the token to make sure it's correct. Ideally, most developers should be able to change the class that their form extends and magically have a secure form, so making them call the superclass' clean method isn't a good idea to me.

comment:4 Changed 8 years ago by ElliottM

Triage Stage: AcceptedDesign decision needed

comment:5 Changed 8 years ago by ElliottM

Has patch: set
Needs documentation: set

Changed 8 years ago by ElliottM

Attachment: 7167-safeform1.diff added

revised docstring

comment:6 Changed 8 years ago by Luke Plant

I have just beefed up the CsrfMiddleware a bit, in particular:

  • split it into two components, which allows for only using the 'incoming' bit, and the not the response processing.
  • added a 'csrf_exempt' decorator for views so that they are not checked in the incoming bit.

This allows for a combined approach:

  • We have CsrfMiddleware turned on by default
  • Use @csrf_exempt on the views that don't need it (e.g ones that use SafeForm)
  • Eventually replace CsrfMiddleware (which combines CsrfViewMiddleware and CsrfResponseMiddleware) with just CsrfViewMiddleware. Users are then responsible for added the token themselves, but the middleware will catch them if they ever forget it.

This assumes the middleware and SafeForm can cooperate e.g. use the same name/contents for the CSRF token.

comment:7 Changed 8 years ago by (none)

milestone: post-1.0

Milestone post-1.0 deleted

comment:8 Changed 6 years ago by Gabriel Hurley

Component: Contrib appscontrib.csrf

comment:9 Changed 6 years ago by Luke Plant

Severity: Normal
Type: New feature

comment:10 Changed 6 years ago by Luke Plant

Easy pickings: unset
Resolution: wontfix
Status: newclosed

We lost interest in this and went in another direction. With the lack of any interest in reviving it, I'm closing WONTFIX.

Note: See TracTickets for help on using tickets.
Back to Top