4 ms·

I like DRF, but I think its allow-by-default mentality is a security risk. If you use it, keep in mind that: * ModelViewSet is read/write, you should use the m

I like DRF, but I think its allow-by-default mentality is a security risk. If you use it, keep in mind that: * ModelViewSet is read/write, you should use the more verbose ReadOnlyModelViewSet until you know you want to allow writing. * Fields specified in the "fields" member are read/write by default. You have to explicitly declare the field on the ViewSet and pass "readonly=true" to make it read-only. This is especially dangerous for ForeignKey fields, which can be used to change object ownership if you aren't careful. When I was responsible for a DRF-based API I wrote some custom Fields and ViewSets to use safer defaults, and I recommend others do the same.

dopeboy 10y ago
If you want to get even more granular, I would suggest using the mixins. I almost never use the broad viewsets anymore. Example: class FooViewset( mixins.CreateModelMixin, mixins.ListModelMixin, mixins.DestroyModelMixin, mixins.RetrieveModelMixin, viewsets.GenericViewSet) Subtract as necessary.
- alexbecker 10y ago
  This is a good way to control what methods are allowed on the ViewSet, but still doesn't address the problem of fields being writable by default when the ViewSet allows writing.
  - deleted 10y ago
    [deleted]
  - chc 10y ago
    This seems like the expected behavior. If allowing for writing didn't actually allow you to write anything, that would be pretty strange, wouldn't it? Or do I misunderstand?
    - alexbecker 10y ago
      You may want to expose some fields that you don't allow changing, such as what account owns the resource. When fields are writable by default, it is easy for someone to miss that they've made a field writable when they just meant to expose it for reading.
      - Kpourdeilami 10y ago
        I think it's better to define field specific read/write permissions through the serializers. In the serializer's Meta class, you can define a readonly_fields tuple containing the string names of the read only fields
        qnrq 10y ago
        You can also route only read-only methods in urls.py, for example via: url( r'path/$', TheModelViewSetView.as_view({"get": "list"}), name="thename" ) or "retrieve" instead of "list" for a route which includes PK. This of course makes the entire path read-only so it's not a way to make some fields writable and others not.
        rtpg 10y ago
        yeah, I think they're saying instead of having the Meta fields list create writable fields by default, it should be read-only by default, after which you could add a "writable_fields" list
Alex3917 10y ago
> I like DRF, but I think its allow-by-default mentality is a security risk. Completely agree. But DRF is useful even if you don't use any of the serializers. E.g. you can just use it to handle auth, requests, and responses, and then validate things by hand and save them directly to the Django model. You still pretty much need serializers when validating image files because you can't trigger the Django code to do this by saving to the model directly, but for everything else it's often more readable just to serialize and deserialize by hand.

dopeboy 10y ago

If you want to get even more granular, I would suggest using the mixins. I almost never use the broad viewsets anymore. Example: class FooViewset( mixins.CreateModelMixin, mixins.ListModelMixin, mixins.DestroyModelMixin, mixins.RetrieveModelMixin, viewsets.GenericViewSet) Subtract as necessary.

alexbecker 10y ago
This is a good way to control what methods are allowed on the ViewSet, but still doesn't address the problem of fields being writable by default when the ViewSet allows writing.
- deleted 10y ago
  [deleted]
- chc 10y ago
  This seems like the expected behavior. If allowing for writing didn't actually allow you to write anything, that would be pretty strange, wouldn't it? Or do I misunderstand?
  - alexbecker 10y ago
    You may want to expose some fields that you don't allow changing, such as what account owns the resource. When fields are writable by default, it is easy for someone to miss that they've made a field writable when they just meant to expose it for reading.
    - Kpourdeilami 10y ago
      I think it's better to define field specific read/write permissions through the serializers. In the serializer's Meta class, you can define a readonly_fields tuple containing the string names of the read only fields
      - qnrq 10y ago
        You can also route only read-only methods in urls.py, for example via: url( r'path/$', TheModelViewSetView.as_view({"get": "list"}), name="thename" ) or "retrieve" instead of "list" for a route which includes PK. This of course makes the entire path read-only so it's not a way to make some fields writable and others not.
      - rtpg 10y ago
        yeah, I think they're saying instead of having the Meta fields list create writable fields by default, it should be read-only by default, after which you could add a "writable_fields" list

Alex3917 10y ago

> I like DRF, but I think its allow-by-default mentality is a security risk. Completely agree. But DRF is useful even if you don't use any of the serializers. E.g. you can just use it to handle auth, requests, and responses, and then validate things by hand and save them directly to the Django model. You still pretty much need serializers when validating image files because you can't trigger the Django code to do this by saving to the model directly, but for everything else it's often more readable just to serialize and deserialize by hand.