Django

Code

Ticket #7167 (new)

Opened 2 years ago

Last modified 1 year ago

Add a SafeForm / SecureForm to django.contrib.csrf

Reported by: mrts Assigned to: nobody
Milestone: Component: Contrib apps
Version: SVN Keywords:
Cc: Triage Stage: Design decision needed
Has patch: 1 Needs documentation: 1
Needs tests: 0 Patch needs improvement: 0

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

7167-safeform.diff (5.1 kB) - added by ElliottM on 10/27/08 11:49:36.
7167-safeform1.diff (5.7 kB) - added by ElliottM on 10/27/08 21:09:32.
revised docstring

Change History

06/14/08 07:05:14 changed by Simon Greenhill

  • needs_better_patch changed.
  • stage changed from Unreviewed to Accepted.
  • needs_tests changed.
  • needs_docs changed.

07/22/08 12:23:25 changed by mir

  • milestone set to post-1.0.

10/27/08 11:49:36 changed by ElliottM

  • attachment 7167-safeform.diff added.

10/27/08 12:12:52 changed 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.

2. 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.

10/27/08 12:13:37 changed by ElliottM

  • stage changed from Accepted to Design decision needed.

10/27/08 12:15:19 changed by ElliottM

  • needs_docs set to 1.
  • has_patch set to 1.

10/27/08 21:09:32 changed by ElliottM

  • attachment 7167-safeform1.diff added.

revised docstring

12/02/08 19:01:55 changed 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.

02/25/09 13:51:44 changed by

  • milestone deleted.

Milestone post-1.0 deleted


Add/Change #7167 (Add a SafeForm / SecureForm to django.contrib.csrf)




Change Properties
Action