Opened 15 years ago
Closed 12 years ago
#7167 closed New feature (wontfix)
Add a SafeForm / SecureForm to django.contrib.csrf
Reported by: | mrts | Owned by: | nobody |
---|---|---|---|
Component: | CSRF | Version: | dev |
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: | no |
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)
Change History (12)
comment:1 Changed 15 years ago by
Triage Stage: | Unreviewed → Accepted |
---|
comment:2 Changed 15 years ago by
milestone: | → post-1.0 |
---|
Changed 14 years ago by
Attachment: | 7167-safeform.diff added |
---|
comment:3 Changed 14 years ago by
comment:4 Changed 14 years ago by
Triage Stage: | Accepted → Design decision needed |
---|
comment:5 Changed 14 years ago by
Has patch: | set |
---|---|
Needs documentation: | set |
comment:6 Changed 14 years ago by
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:8 Changed 12 years ago by
Component: | Contrib apps → contrib.csrf |
---|
comment:9 Changed 12 years ago by
Severity: | → Normal |
---|---|
Type: | → New feature |
comment:10 Changed 12 years ago by
Easy pickings: | unset |
---|---|
Resolution: | → wontfix |
Status: | new → closed |
We lost interest in this and went in another direction. With the lack of any interest in reviving it, I'm closing WONTFIX.
I made the patch on 2 assumptions:
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.