Opened 7 years ago

Closed 4 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 6 years ago.
7167-safeform1.diff (5.7 KB) - added by ElliottM 6 years ago.
revised docstring

Download all attachments as: .zip

Change History (12)

comment:1 Changed 7 years ago by Simon Greenhill

  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset
  • Triage Stage changed from Unreviewed to Accepted

comment:2 Changed 7 years ago by mir

  • milestone set to post-1.0

Changed 6 years ago by ElliottM

comment:3 Changed 6 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 6 years ago by ElliottM

  • Triage Stage changed from Accepted to Design decision needed

comment:5 Changed 6 years ago by ElliottM

  • Has patch set
  • Needs documentation set

Changed 6 years ago by ElliottM

revised docstring

comment:6 Changed 6 years ago by lukeplant

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 6 years ago by anonymous

  • milestone post-1.0 deleted

Milestone post-1.0 deleted

comment:8 Changed 4 years ago by gabrielhurley

  • Component changed from Contrib apps to contrib.csrf

comment:9 Changed 4 years ago by lukeplant

  • Severity set to Normal
  • Type set to New feature

comment:10 Changed 4 years ago by lukeplant

  • Easy pickings unset
  • Resolution set to wontfix
  • Status changed from new to closed

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