From 654e0e45279b5023cea7574bda1ebd63dae96347 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Padilla?= Date: Thu, 27 Aug 2015 13:09:08 -0400 Subject: [PATCH 1/4] Update ModelSerializer fields behavior --- rest_framework/serializers.py | 21 +++++++++++++++++++-- tests/test_model_serializer.py | 15 +++++++++++++++ 2 files changed, 34 insertions(+), 2 deletions(-) diff --git a/rest_framework/serializers.py b/rest_framework/serializers.py index c7d4405c5..acaf3bef7 100644 --- a/rest_framework/serializers.py +++ b/rest_framework/serializers.py @@ -12,6 +12,8 @@ response content is handled by parsers and renderers. """ from __future__ import unicode_literals +import warnings + from django.db import models from django.db.models.fields import Field as DjangoModelField from django.db.models.fields import FieldDoesNotExist @@ -51,6 +53,8 @@ LIST_SERIALIZER_KWARGS = ( 'instance', 'data', 'partial', 'context', 'allow_null' ) +ALL_FIELDS = '__all__' + # BaseSerializer # -------------- @@ -943,13 +947,13 @@ class ModelSerializer(Serializer): fields = getattr(self.Meta, 'fields', None) exclude = getattr(self.Meta, 'exclude', None) - if fields and not isinstance(fields, (list, tuple)): + if fields and fields != ALL_FIELDS and not isinstance(fields, (list, tuple)): raise TypeError( 'The `fields` option must be a list or tuple. Got %s.' % type(fields).__name__ ) - if exclude and not isinstance(exclude, (list, tuple)): + if exclude and exclude != ALL_FIELDS and not isinstance(exclude, (list, tuple)): raise TypeError( 'The `exclude` option must be a list or tuple. Got %s.' % type(exclude).__name__ @@ -962,6 +966,19 @@ class ModelSerializer(Serializer): ) ) + if fields is None and exclude is None: + warnings.warn( + "Creating a ModelSerializer without either the 'fields' attribute " + "or the 'exclude' attribute will be prohibited. " + "The {serializer_class} serializer needs updating.".format( + serializer_class=self.__class__.__name__ + ), + PendingDeprecationWarning + ) + + if fields == ALL_FIELDS: + fields = None + if fields is not None: # Ensure that all declared fields have also been included in the # `Meta.fields` option. diff --git a/tests/test_model_serializer.py b/tests/test_model_serializer.py index 777b956c4..89557fa1d 100644 --- a/tests/test_model_serializer.py +++ b/tests/test_model_serializer.py @@ -321,6 +321,21 @@ class TestRegularFieldMappings(TestCase): ExampleSerializer() + def test_fields_and_exclude_behavior(self): + class ImplicitFieldsSerializer(serializers.ModelSerializer): + class Meta: + model = RegularFieldsModel + + class ExplicitFieldsSerializer(serializers.ModelSerializer): + class Meta: + model = RegularFieldsModel + fields = '__all__' + + implicit = ImplicitFieldsSerializer() + explicit = ExplicitFieldsSerializer() + + assert implicit.data == explicit.data + @pytest.mark.skipif(django.VERSION < (1, 8), reason='DurationField is only available for django1.8+') From f3ef13ab59c1ae9504e830ea3f45210fab5e973d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Padilla?= Date: Fri, 28 Aug 2015 08:05:20 -0400 Subject: [PATCH 2/4] Update to match docs on ModelForm fields --- docs/api-guide/serializers.md | 22 ++++++++++++++++++++-- 1 file changed, 20 insertions(+), 2 deletions(-) diff --git a/docs/api-guide/serializers.md b/docs/api-guide/serializers.md index abdb67afa..5138c6ce0 100644 --- a/docs/api-guide/serializers.md +++ b/docs/api-guide/serializers.md @@ -453,14 +453,31 @@ To do so, open the Django shell, using `python manage.py shell`, then import the ## Specifying which fields to include -If you only want a subset of the default fields to be used in a model serializer, you can do so using `fields` or `exclude` options, just as you would with a `ModelForm`. +It is strongly recommended that you explicitly set all fields that should be edited in the serializer using the `fields` attribute. Failure to do so can easily lead to security problems when a serializer unexpectedly allows a user to set certain fields, especially when new fields are added to a model. + +The alternative approach would be to include all fields automatically, or blacklist only some. This fundamental approach is known to be much less secure and has led to serious exploits on major websites (e.g. [GitHub][github-vuln-blog]). + +There are, however, two shortcuts available for cases where you can guarantee these security concerns do not apply to you: + +1. Set the `fields` attribute to the special value `'__all__'` to indicate that all fields in the model should be used. For example: class AccountSerializer(serializers.ModelSerializer): class Meta: model = Account - fields = ('id', 'account_name', 'users', 'created') + fields = '__all__' + +2. Set the exclude attribute of the ModelForm’s inner Meta class to a list of fields to be excluded from the form. + +For example: + + class AccountSerializer(serializers.ModelSerializer): + class Meta: + model = Account + exclude = 'users' + +In the example above, if the `Account` model had 3 fields `account_name`, `users`, and `created`, this will result in the fields `account_name` and `created` to be serialized. The names in the `fields` option will normally map to model fields on the model class. @@ -1035,6 +1052,7 @@ The [django-rest-framework-gis][django-rest-framework-gis] package provides a `G The [django-rest-framework-hstore][django-rest-framework-hstore] package provides an `HStoreSerializer` to support [django-hstore][django-hstore] `DictionaryField` model field and its `schema-mode` feature. [cite]: https://groups.google.com/d/topic/django-users/sVFaOfQi4wY/discussion +[github-vuln-blog]: https://github.com/blog/1068-public-key-security-vulnerability-and-mitigation [relations]: relations.md [model-managers]: https://docs.djangoproject.com/en/dev/topics/db/managers/ [encapsulation-blogpost]: http://www.dabapps.com/blog/django-models-and-encapsulation/ From 1fe8e9a0bf37e045457aa91a19748ea1a6479a1b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Padilla?= Date: Fri, 28 Aug 2015 08:11:07 -0400 Subject: [PATCH 3/4] Add note on deprecation path --- docs/api-guide/serializers.md | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/docs/api-guide/serializers.md b/docs/api-guide/serializers.md index 5138c6ce0..ed1ada92c 100644 --- a/docs/api-guide/serializers.md +++ b/docs/api-guide/serializers.md @@ -483,6 +483,12 @@ The names in the `fields` option will normally map to model fields on the model Alternatively names in the `fields` options can map to properties or methods which take no arguments that exist on the model class. +--- + +**Note**: Before version 3.3, the `'__all__'` shortcut did not exist, but omitting the fields attribute had the same effect. Omitting both fields and exclude is now deprecated, but will continue to work as before until version 3.5 + +--- + ## Specifying nested serialization The default `ModelSerializer` uses primary keys for relationships, but you can also easily generate nested representations using the `depth` option: From 9dd1b2516be3b1993219ad93e5802f5b8d25e324 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Padilla?= Date: Fri, 28 Aug 2015 09:51:11 -0400 Subject: [PATCH 4/4] Update ModelSerializer fields docs --- docs/api-guide/serializers.md | 25 +++++++++++-------------- 1 file changed, 11 insertions(+), 14 deletions(-) diff --git a/docs/api-guide/serializers.md b/docs/api-guide/serializers.md index ed1ada92c..cefcfa097 100644 --- a/docs/api-guide/serializers.md +++ b/docs/api-guide/serializers.md @@ -432,6 +432,7 @@ Declaring a `ModelSerializer` looks like this: class AccountSerializer(serializers.ModelSerializer): class Meta: model = Account + fields = ('id', 'account_name', 'users', 'created') By default, all the model fields on the class will be mapped to a corresponding serializer fields. @@ -453,13 +454,16 @@ To do so, open the Django shell, using `python manage.py shell`, then import the ## Specifying which fields to include -It is strongly recommended that you explicitly set all fields that should be edited in the serializer using the `fields` attribute. Failure to do so can easily lead to security problems when a serializer unexpectedly allows a user to set certain fields, especially when new fields are added to a model. +If you only want a subset of the default fields to be used in a model serializer, you can do so using `fields` or `exclude` options, just as you would with a `ModelForm`. It is strongly recommended that you explicitly set all fields that should be serialized using the `fields` attribute. This will make it less likely to result in unintentionally exposing data when your models change. -The alternative approach would be to include all fields automatically, or blacklist only some. This fundamental approach is known to be much less secure and has led to serious exploits on major websites (e.g. [GitHub][github-vuln-blog]). +For example: -There are, however, two shortcuts available for cases where you can guarantee these security concerns do not apply to you: + class AccountSerializer(serializers.ModelSerializer): + class Meta: + model = Account + fields = ('id', 'account_name', 'users', 'created') -1. Set the `fields` attribute to the special value `'__all__'` to indicate that all fields in the model should be used. +You can also set the `fields` attribute to the special value `'__all__'` to indicate that all fields in the model should be used. For example: @@ -468,27 +472,21 @@ For example: model = Account fields = '__all__' -2. Set the exclude attribute of the ModelForm’s inner Meta class to a list of fields to be excluded from the form. +You can set the `exclude` attribute of the to a list of fields to be excluded from the serializer. For example: class AccountSerializer(serializers.ModelSerializer): class Meta: model = Account - exclude = 'users' + exclude = ('users',) In the example above, if the `Account` model had 3 fields `account_name`, `users`, and `created`, this will result in the fields `account_name` and `created` to be serialized. -The names in the `fields` option will normally map to model fields on the model class. +The names in the `fields` and `exclude` attributes will normally map to model fields on the model class. Alternatively names in the `fields` options can map to properties or methods which take no arguments that exist on the model class. ---- - -**Note**: Before version 3.3, the `'__all__'` shortcut did not exist, but omitting the fields attribute had the same effect. Omitting both fields and exclude is now deprecated, but will continue to work as before until version 3.5 - ---- - ## Specifying nested serialization The default `ModelSerializer` uses primary keys for relationships, but you can also easily generate nested representations using the `depth` option: @@ -1058,7 +1056,6 @@ The [django-rest-framework-gis][django-rest-framework-gis] package provides a `G The [django-rest-framework-hstore][django-rest-framework-hstore] package provides an `HStoreSerializer` to support [django-hstore][django-hstore] `DictionaryField` model field and its `schema-mode` feature. [cite]: https://groups.google.com/d/topic/django-users/sVFaOfQi4wY/discussion -[github-vuln-blog]: https://github.com/blog/1068-public-key-security-vulnerability-and-mitigation [relations]: relations.md [model-managers]: https://docs.djangoproject.com/en/dev/topics/db/managers/ [encapsulation-blogpost]: http://www.dabapps.com/blog/django-models-and-encapsulation/