search

dbkaplan / dry-rest-permissions

Rules based permissions for the Django Rest Framework
ISC License
374 stars 59 forks source link

Wrong behaviour on custom view that return an instance of a Model without has_read/write permissions #53

Open Sveder opened 6 years ago

Sveder commented 6 years ago

My use case is that I have an endpoint /v1/widget/100 that returns the Widget model which has has_object_read_permission. This works as expected. I've added a new custom route to the view set for /v1/widget/100/vendor that serializes a different model - Vendor, which doesn't have permission models.

When calling this method without the DRF Browsable API (https://www.django-rest-framework.org/topics/browsable-api/), it works. But when I do call it with the Browasble API, I get an assertion error. Following the chain of calls DRF wants to render an HTML form for the options call, so execution eventually hits get_rendered_html_form (https://github.com/encode/django-rest-framework/blob/master/rest_framework/renderers.py#L456) and it indeed recognizes I'm returning a Vendor model and finds the right serializer. Eventually it calls show_form_for_method -> check_object_permissions (https://github.com/encode/django-rest-framework/blob/master/rest_framework/views.py#L339) -> has_object_permission (https://github.com/dbkaplan/dry-rest-permissions/blob/master/dry_rest_permissions/generics.py#L130).

When it arrives there, dry-rest-permissions wrongly parses the serializer from the ViewSet (which in my case will be the WidgetViewSet and the WidgetSerializer instead of VendorSerializer, as DRF successfully parsed). Responsible code: https://github.com/dbkaplan/dry-rest-permissions/blob/master/dry_rest_permissions/generics.py#L137

It then goes to do an even weirder thing - it asserts on the obj for has_object_read/write_permissions and then shows an error with the class_name instead of the obj. This is super misleading - the obj is a Vendor and the class_name is the Widget so the error doesn't make sense.

I'm guessing this is not the wanted behaviour, but I'm not sure how to patch it and what is the right behaviour, but I'd love your feedback on what to do here.

To summarize:

  1. There is a weird (buggy?) behaviour where the serializer is taken from the ViewSet instead of from the object checking permissions on.

  2. The error message then doubles down on it by misleading you to think the problem is with the ViewSet model and not the object model.

WDYT? What to do?

Sveder commented 5 years ago

@dbkaplan Sorry for pinging again, but I'd love to get your thoughts here.