auth_permission column lengths
I wanted to bring up this bug: https://code.djangoproject.com/ticket/17763
as it's bitten me on a couple of projects recently
Summarizing the ticket:
==
Django automatically generates a name that is longer than what the default
field length can hold.
INSERT INTO "auth_permission" ("name", "content_type_id", "codename")
VALUES (%s, %s, %s) (u'Can add study plan practice assessment tutorial
question', 52, u'add_studyplanpracticeassessmenttutorialquestion')
This will fail with a error about it not being able to fit in a varchar(50)
Table "public.auth_permission"
Column | Type | Modifiers
-++--
id | integer| not null default
nextval('auth_permission_id_seq'::regclass)
name| character varying(50 ) | not null
content_type_id | integer| not null
codename| character varying(100) | not null
This was under PostgreSQL and after modifying the field to have a length of
200, I was able to perform the insert.
==
This is easy to run into, especially when other developers who might make a
particular model are working in SQLite3, where validation at the db level
does not happen and values are silently truncated. It can also happen
pretty easily when working with legacy database tables with overly verbose
table names.
The 50/100 char length constraint on name and codename seem to be rather
arbitrary to me. I think it seems reasonable to bump these up to 200 or
255, and provide validation on the lengths to be entered when performing
syncdb.
I'm happy to work on a patch/tests if there is interest. For now, when a
slightly longer class name is needed, the only real way around it is to
alter the model & the db, which is not ideal.
Thanks in advance!
-greg
--
You received this message because you are subscribed to the Google Groups
"Django developers" group.
To view this discussion on the web visit
https://groups.google.com/d/msg/django-developers/-/yEXJxLQj_V0J.
To post to this group, send email to django-developers@googlegroups.com.
To unsubscribe from this group, send email to
django-developers+unsubscr...@googlegroups.com.
For more options, visit this group at
http://groups.google.com/group/django-developers?hl=en.
Re: ModelForms and the Rails input handling vulnerability
Thanks Luke for writing this up and representing all views. I am the proponent of "we are all adults here". On Thu, Jun 14, 2012 at 6:48 AM, Anssi Kääriäinen wrote: > ... > I hadn't realized all fields present on the Python form but not in the > HTML form will get overwritten to NULL... The above makes me a tad > more "-" on the require fields always proposal. The problem is there > only for forms used for model creation. If you use the form both for > update and creation, you will soon enough see that something funny is > going on as part of your fields are getting set to NULL on > form.save(). > > So, to hit the problem the user would need to: > 1. Have a ModelForm with no field restrictions in Python. > 2. Render only part of the fields. > 3. All non-rendered fields must be null/blank=True for the form to > work at all. > 4. Not use the form in update views. Exactly - the combination of things that need to happen for this is so complicated that I am perfectly ok with leaving things as they are. (add 5. No tests for the form/view) I believe exclude is way more useful than fields (I do see the security advantage but in my mind this is the case where convenience beats security, also still afraid of the fields = [f.name for f in ...]). I would be most happy with a modified Option 1 - require exclude OR fields on ModelForm.Meta. This should force the user to think about this issue, when these attributes are missing we can throw a reasonable error and refer the user to the docs for more info on how their choices affect the overall security of the app. so: -1 on options 2 and 3, +1 on option 1 + require exclude or fields -- You received this message because you are subscribed to the Google Groups "Django developers" group. To post to this group, send email to django-developers@googlegroups.com. To unsubscribe from this group, send email to django-developers+unsubscr...@googlegroups.com. For more options, visit this group at http://groups.google.com/group/django-developers?hl=en.
Re: ModelForms and the Rails input handling vulnerability
Hi, I'm with Carl in supporting option 3. I think have always thought that ModelForms were unsafe and requiring the fields option would go a long way to making them safer. I don't think I'm stupid and I've personally run into this issue. I have almost *NEVER* run into a case where I want all fields on a Model to be updatable by the user and I now always define the fields Meta attribute. However, I'm not necessarily against leaving the current functionality in Django for a few releases but with warning messages. > So, to hit the problem the user would need to: > 1. Have a ModelForm with no field restrictions in Python. > 2. Render only part of the fields. > 3. All non-rendered fields must be null/blank=True for the form to > work at all. > 4. Not use the form in update views. No, the user would have to not miss of those things when developing. Having a field get updated to NULL when not in the HTML does not guarantee the developer will notice it. Maybe the field's default is NULL? Or NULL is a perfectly valid value and they just don't notice? I have been doing Django and Python for years at a high level and I have done this. On Tue, Jun 19, 2012 at 8:42 PM, Honza Král wrote: > I believe exclude is way more useful than fields (I do see the > security advantage but in my mind this is the case where convenience > beats security, also still afraid of the fields = [f.name for f in > ...]). Personally, I don't think convenience EVER beats security in a framework like Django (if at all). This is the common "Oh but that will never happen to me!" syndrome. Sane defaults that can be overridden are going to always be better. -- Ian http://www.ianlewis.org/ -- You received this message because you are subscribed to the Google Groups "Django developers" group. To post to this group, send email to django-developers@googlegroups.com. To unsubscribe from this group, send email to django-developers+unsubscr...@googlegroups.com. For more options, visit this group at http://groups.google.com/group/django-developers?hl=en.
Re: ModelForms and the Rails input handling vulnerability
> On Tue, Jun 19, 2012 at 8:42 PM, Honza Král wrote: >> I believe exclude is way more useful than fields (I do see the >> security advantage but in my mind this is the case where convenience >> beats security, also still afraid of the fields = [f.name for f in >> ...]). > > Personally, I don't think convenience EVER beats security in a > framework like Django (if at all). This is the common "Oh but that > will never happen to me!" syndrome. Sane defaults that can be > overridden are going to always be better. TL;DR: Convenience beats security when it would otherwise result in people circumventing the security alltogether (== using fields = [f.name for MyModel._meta.fields]) I do agree that security is important and a framework worth it's name should do everything it can to help people develop secure apps. However I believe (from talking to people around me and looking at my own code) that if we force people to use fields and nothing else the majority of people will do [f.name for f in MyModel._meta.fields] which is both terrible and insecure thus completely negating the desired effect (worst of both worlds - insecure and ugly/hard to use). If we instead just force users to specify one of fields or exclude (even if just saying exclude = ()) we accomplish our goal - the goal is not to make the app secure no matter what the developer thinks, it's forcing the user to think about the implications of their choices and pointing them to the right place. (if you don't supply exclude of fields you get an error with a link to docs) For me requiring fields falls on the same level as requiring super strong password (one lowercase, one uppercase, a digit and a special character) and requiring users to change them weekly while not repeating 10 last passwords. In theory that's very good security (I know it's not perfect and that http://xkcd.com/936/ is valid, I have seen this requirement quite often though), but if you just force it on people without the necessary education etc it will just result in majority of people writing their password down on the other side of their keyboard (I have seen this personally way too many times). We cannot force this on users, we must make them aware and educate them - I cannot stress this enough that I don't believe we can strong arm people into doing security right, we need to make them aware of the possible problem and provide them with good tools to help them deal with it. -- You received this message because you are subscribed to the Google Groups "Django developers" group. To post to this group, send email to django-developers@googlegroups.com. To unsubscribe from this group, send email to django-developers+unsubscr...@googlegroups.com. For more options, visit this group at http://groups.google.com/group/django-developers?hl=en.
Re: auth_permission column lengths
Hi Greg, Django itself can't change that currently since there is no support for schema alteration in Core. Once we get that we can tackle issues like that and increase to a sensible limit. (both name and codename might cause problems…). Cheers, Florian -- You received this message because you are subscribed to the Google Groups "Django developers" group. To view this discussion on the web visit https://groups.google.com/d/msg/django-developers/-/tILW48L1y_IJ. To post to this group, send email to django-developers@googlegroups.com. To unsubscribe from this group, send email to django-developers+unsubscr...@googlegroups.com. For more options, visit this group at http://groups.google.com/group/django-developers?hl=en.
Re: auth_permission column lengths
Hi Florian, Am 19.06.2012 um 16:12 schrieb Florian Apolloner: > Django itself can't change that currently since there is no support for > schema alteration in Core. Once we get that we can tackle issues like that > and increase to a sensible limit. (both name and codename might cause > problems…). How is this a factor if the limit exists only at the database level and is not enforced or considered in Django code? Django code already creates values that are too long for the fields. You would just eliminate this bug on new installations. In the case of usernames and email addresses, the limit was enforced in Django code, so increasing the field length could break existing installations since we can't do automatic schema migrations. But in this case, things already break since Django does not care about the field length. Cheers, Stephan signature.asc Description: Message signed with OpenPGP using GPGMail
Re: ModelForms and the Rails input handling vulnerability
On 14 kesä, 08:53, Torsten Bronger wrote: > Hall chen! > > Alex Ogier writes: > > [...] > > > That suggests an idea to me. Perhaps the best way to check this > > isn't on the way out in the template renderer, but rather on the > > way back in in the form validation. If the form doesn't get back > > exactly those fields it sent out then you know that for whatever > > reason, the field was unable to make a round trip. > > But can one guarantee that fields rendered in the browser are also > sent back in the POST request? Even worse, how about non-browser > requests? One way is to have a .is_valid(safe_fields=list_of_fields) bypass. You could conveniently use form.is_valid(safe_field=form.fields) if you want to take chances with security. In normal use you would need to manually whitelist checkboxes, otherwise this should just work. This is safer than requiring fields in form.meta, and protects against hidden overwrites to NULL, too. Even safer is to require always whitelisting all allowed data elements in the is_valid() call. This is the most secure approach. It is pointless to debate about the approaches on the amount of security they give. This is all about the amount of convenience we are willing to sacrifice for the amount of security gained. To me it seems there isn't much security to be gained by always requiring fields, and there is much convenience lost. Still, there isn't any one right opinion here. BDFL decision seems likely here. The validation stage checking seems worth more investigation to me. If we can pull a nice usable and secure API, this would be the best choice in my opinion. Checkboxes are the real problem here. Sacrificing some security for convenience and one could just skip checkbox fields from the checking. If more security is wanted, then require manual whitelisting of checkbox fields. - Anssi -- You received this message because you are subscribed to the Google Groups "Django developers" group. To post to this group, send email to django-developers@googlegroups.com. To unsubscribe from this group, send email to django-developers+unsubscr...@googlegroups.com. For more options, visit this group at http://groups.google.com/group/django-developers?hl=en.
Re: auth_permission column lengths
Florian: I don't think waiting for migrations in the Django core is totally necessary to fix a bug like this (or others that might be similar). With proper documentation in the release/upgrade notes, I think it's completely reasonable to expect someone working with Django to be able to run a manual SQL query to alter those columns. If this is a core philosophy not to ask users to run manual queries on updates, is starting with a patch to enforce limits here a good thing? Thanks, -greg On Tuesday, June 19, 2012 9:12:47 AM UTC-5, Florian Apolloner wrote: > > Hi Greg, > > Django itself can't change that currently since there is no support for > schema alteration in Core. Once we get that we can tackle issues like that > and increase to a sensible limit. (both name and codename might cause > problems…). > > Cheers, > Florian > -- You received this message because you are subscribed to the Google Groups "Django developers" group. To view this discussion on the web visit https://groups.google.com/d/msg/django-developers/-/OygzSFWV74wJ. To post to this group, send email to django-developers@googlegroups.com. To unsubscribe from this group, send email to django-developers+unsubscr...@googlegroups.com. For more options, visit this group at http://groups.google.com/group/django-developers?hl=en.
Re: auth_permission column lengths
On 19/06/12 15:25, Stephan Jaensch wrote: > Hi Florian, > > Am 19.06.2012 um 16:12 schrieb Florian Apolloner: > >> Django itself can't change that currently since there is no >> support for schema alteration in Core. Once we get that we can >> tackle issues like that and increase to a sensible limit. (both >> name and codename might cause problems…). > > How is this a factor if the limit exists only at the database level > and is not enforced or considered in Django code? Django code > already creates values that are too long for the fields. You would > just eliminate this bug on new installations. In the case of > usernames and email addresses, the limit was enforced in Django > code, so increasing the field length could break existing > installations since we can't do automatic schema migrations. But in > this case, things already break since Django does not care about > the field length. I agree, this is one of the few cases where it's possible to get away without a schema migration, as the behaviour if we made this change would be exactly the same on old installations and it would be fixed on newer ones. Of course, this just leads to a higher limit after which you have values which are too long, but if your class names are getting over 200 characters long I suggest you have other issues. Andrew -- You received this message because you are subscribed to the Google Groups "Django developers" group. To post to this group, send email to django-developers@googlegroups.com. To unsubscribe from this group, send email to django-developers+unsubscr...@googlegroups.com. For more options, visit this group at http://groups.google.com/group/django-developers?hl=en.
Re: auth_permission column lengths
On 19/06/12 16:13, Greg Aker wrote: > Florian: > > I don't think waiting for migrations in the Django core is totally > necessary to fix a bug like this (or others that might be similar). > With proper documentation in the release/upgrade notes, I think it's > completely reasonable to expect someone working with Django to be able > to run a manual SQL query to alter those columns. > > If this is a core philosophy not to ask users to run manual queries on > updates, is starting with a patch to enforce limits here a good thing? It's messy to ask people to manually run SQL queries to change this stuff in general - we'd have to provide four or five different queries, and the operation isn't even possible on SQLite (you'd have to make a new table with the right schema and copy things over). I'm working studiously on getting schema migration stuff in, it'll just take a release or two till it's ready. As for the interim solution, I replied to Stephan's post about that - I think just increasing max_length will work well enough for now in this case, as it's one of the few situations where the schema is slightly decoupled from the models. Andrew -- You received this message because you are subscribed to the Google Groups "Django developers" group. To post to this group, send email to django-developers@googlegroups.com. To unsubscribe from this group, send email to django-developers+unsubscr...@googlegroups.com. For more options, visit this group at http://groups.google.com/group/django-developers?hl=en.
Re: auth_permission column lengths
Good to know about native migrations, and the interim solution seems reasonable. Thanks so much! On Tuesday, June 19, 2012 10:56:38 AM UTC-5, Andrew Godwin wrote: > > On 19/06/12 16:13, Greg Aker wrote: > > Florian: > > > > I don't think waiting for migrations in the Django core is totally > > necessary to fix a bug like this (or others that might be similar). > > With proper documentation in the release/upgrade notes, I think it's > > completely reasonable to expect someone working with Django to be able > > to run a manual SQL query to alter those columns. > > > > If this is a core philosophy not to ask users to run manual queries on > > updates, is starting with a patch to enforce limits here a good thing? > > It's messy to ask people to manually run SQL queries to change this > stuff in general - we'd have to provide four or five different queries, > and the operation isn't even possible on SQLite (you'd have to make a > new table with the right schema and copy things over). I'm working > studiously on getting schema migration stuff in, it'll just take a > release or two till it's ready. > > As for the interim solution, I replied to Stephan's post about that - I > think just increasing max_length will work well enough for now in this > case, as it's one of the few situations where the schema is slightly > decoupled from the models. > > Andrew > -- You received this message because you are subscribed to the Google Groups "Django developers" group. To view this discussion on the web visit https://groups.google.com/d/msg/django-developers/-/fuhiKTazQHoJ. To post to this group, send email to django-developers@googlegroups.com. To unsubscribe from this group, send email to django-developers+unsubscr...@googlegroups.com. For more options, visit this group at http://groups.google.com/group/django-developers?hl=en.
Re: Customizable Serialization check-in
Hi!
This week I wrote simple serialization and deserialization for json
format so it's possible now to encode objects from and to json:
import django.core.serializers as s
class Foo(object):
def __init__(self):
self.bar = [Bar(), Bar(), Bar()]
self.x = "X"
class Bar(object):
def __init__(self):
self.six = 6
class MyField2(s.Field):
def deserialized_value(self, obj, instance, field_name):
pass
class MyField(s.Field):
x = MyField2(label="my_attribute", attribute=True)
def serialized_value(self, obj, field_name):
return getattr(obj, field_name, "No field like this")
def deserialized_value(self, obj, instance, field_name):
pass
class BarSerializer(s.ObjectSerializer):
class Meta:
class_name = Bar
class FooSerializer(s.ObjectSerializer):
my_field=MyField(label="MYFIELD")
bar = BarSerializer()
class Meta:
class_name = Foo
foos = [Foo(), Foo(), Foo()]
ser = s.serialize('json', foos, serializer=FooSerializer, indent=4)
new_foos = s.deserialize('json', ser, deserializer=FooSerializer)
There are cases that I don't like:
* deserialized_value function with empty content - what to do with
fields that we don't want to deserialize. Should be better way to
handle this,
* I put list foos but return generator new_foos, also bar in Foo
object is generator, not list like in input. Generators are better
for performance but if I put list in input I want list in output,
not generator. I don't know what to do with this.
Next week I will handle rest of issues that I mentioned in my last week
check-in and refactor json format (de)serialization - usage of streams
and proper parameters handling (like indent, etc.)
--
Piotr Grabowski
--
You received this message because you are subscribed to the Google Groups "Django
developers" group.
To post to this group, send email to django-developers@googlegroups.com.
To unsubscribe from this group, send email to
django-developers+unsubscr...@googlegroups.com.
For more options, visit this group at
http://groups.google.com/group/django-developers?hl=en.
