Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
Sveder opened this issue Oct 16, 2018 · 1 comment

Comments

@Sveder
Copy link

Sveder commented Oct 16, 2018

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
Copy link
Author

Sveder commented Dec 23, 2018

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant