On 13 kesä, 02:16, Luke Plant <[email protected]> wrote:
> For the record, I brought up the issue initially, and favour the most
> secure of the options outlined below, but I'll try to summarise with as
> little bias as I can!
<SNIP>
> = Option 1: Leave things as they are, just document the problem. This
> was the least popular view on django-core.
>
> = Option 2: Deprecate Meta.exclude, but still allow a missing
> Meta.fields attribute to indicate that all fields should be editable.
>
> The policy here is essentially this: if any fields the model are
> sensitive, assume all are potentially, and require whitelisting, not
> blacklisting.
>
> = Option 3: Make the 'Meta.fields' attribute a required attribute, which
> must list all the model fields that should be editable. Without it,
> ModelForms would not work at all.
I support option 2. I will try to keep the reasoning part short. The
end of post is devoted to another idea.
Here are the reasons why I think Option 3 isn't worth the trouble:
1. Users will learn to use fields = [f.attname for f in
Permission._meta.fields]. At that point we haven't gained anything in
security, but made modelforms less convenient to use.
2. The fix doesn't actually fix the issue for all cases. You are still
allowed to use the same form in a way that renders additional fields
based on user permissions or if the user is authenticated. If you
reuse the same form in multiple views for example, or your template
contains some logic to add/remove fields dynamically.
Yes, option 3 is a bit more secure than option 2, but I don't find the
cost-benefit ratio correct. It will still not protect users from this
admittedly easy to do security mistake. So, I am -0 to option 3.
The short part is now over. I will throw in another idea I think is at
least worth presenting. The idea is that ModelForms contain attribute
contains_secure_data. Defaults to True. If True, then one must use
allowed_fields when initialising the form from request. So, you could
not do this for secure form:
MyForm(request.POST)
instead you would need to do:
MyForm(request.POST, allowed_fields=('field1', 'field2'))
Assume your model Document has three fields: title, body and creator.
If the creator edits his document, he would likely be able to change
the creator to somebody else when editing his document if the form is
defined like this using 1.4 behavior:
class DocumentForm:
class Meta:
model = Document
But, the developer could create this form:
class DocumentForm:
class Meta:
model = Document
fields = ('title', 'body')
contains_secure_data = False
This would would not need allowed_fields for form.__init__().
Alternatively, assume title shall not be changed on document edit.
Then the developer could use this form:
class DocumentForm:
class Meta:
model = Document
contains_secure_data = True
The developer would then use this form in create as:
DocumentForm(..., allowed_fields=('title', 'body'))
and in update as:
DocumentForm(..., allowed_fields=('body',))
My opinion is that the form.Meta isn't actually the right place to set
the security restriction. The same form can be used in different
places with different security restriction needs. The security
restriction should be done when the request data is added to the form.
The contains_secure_data could be thrown out for simplicity. However,
I do think it has its place. If allowed_fields (or form.meta.fields)
is always required, it leads to bad practices like using the "all
model.meta._fields". Ask the user if the form is security sensitive.
Only if it is, then ask for the field restrictions.
I'm not saying the above is the correct option. Just throwing in
ideas...
- Anssi
--
You received this message because you are subscribed to the Google Groups
"Django developers" group.
To post to this group, send email to [email protected].
To unsubscribe from this group, send email to
[email protected].
For more options, visit this group at
http://groups.google.com/group/django-developers?hl=en.
