Browsable API crashes on extra action in viewset.

[sanchaz]

Checklist

• [ X ] I have verified that that issue exists against the master branch of Django REST framework.
• [ X ] I have searched for similar issues in both open and closed tickets and cannot find a duplicate.
• [ X ] This is not a usage question. (Those should be directed to the discussion group instead.)
• [ X ] This cannot be dealt with as a third party library. (We prefer new functionality to be in the form of third party libraries where possible.)
• [ X ] I have reduced the issue to the simplest possible case.
• [ ] I have included a failing test as a pull request. (If you are unable to do so we can still accept the issue.)

Steps to reproduce

Please see my example project https://github.com/sanchaz/django-drf-demo clone ./manage migrate ./manage test ./manage runserver Go to http://127.0.0.1:8000/admin Login with user: demo password: demo Go to http://127.0.0.1:8000/api/demo/thisone/demo2/ Go to http://127.0.0.1:8000/api/demo/thisone/demo/

Expected behavior

http://127.0.0.1:8000/api/demo/thisone/demo/ should instead show the data.

The tests written call endpoint demo/ and demo2/ without any issue. demo2/ is the same as demo/ but the serializer is passed an array instead of an object and many=True

Actual behavior

Crashes because it tries to check for object permission on the serializer object used in the Response.

This seems like a bug in the browsable api. I don't understand why it calls check_object_permissions on the serializer obj used in the response.

[rpkilby]

The browsable API is trying to determine whether or not it should show you a form for modifying the serialized object. The idea being that the user may have permission to view the object, but not permissions to edit the object.

In your case, the extra action is serializing an object that is incompatible with your permissions class (TestDemo has no user attribute). If the extra action doesn't need these permissions, or needs different permissions, then different permission_classes should be passed to the @action.

---

That said, I'm a little confused by what's happening in the browsable API renderer. We attempt to render forms for OPTIONS and DELETE requests.

https://github.com/encode/django-rest-framework/blob/0cc09f0c0dbe4a6552b1a5bbaa4f7f921270698a/rest_framework/renderers.py#L698-L699

However, get_rendered_html_form bails for these methods anyway.

https://github.com/encode/django-rest-framework/blob/0cc09f0c0dbe4a6552b1a5bbaa4f7f921270698a/rest_framework/renderers.py#L480-L481

A few questions:

• Why do we return True for this specific case instead of returning None?
• Could we move the OPTIONS/DELETE bail above the show_form_for_method check? This would preempt the otherwise unnecessary permissions check.
• Should we just remove delete_form and options_form? It doesn't seem like these would ever have content?

[sanchaz]

I get that, but the object should't be the serializer...

When you make a request the object is checked when the viewset does get_object. https://github.com/encode/django-rest-framework/blob/0407a0df8a16fdac94bbd08d49143a74a88001cd/rest_framework/generics.py#L75-L100

From my understanding it assumes (incorrectly?) the serializer is a model serializer and will have the same fields. But the check for whether you should show the form or not, should be done on the model object, not the serializer. The model object is what we will be potentially modifying. Also, if I'm incorrect, then I'd argue the same check should be done aswell when we have the param many=True, which it isn't.

imo, which I admit might be wrong, I'm new to DRF, the serializer should not be checked for object permissions, the Model object should.

As for you mentioning it should bail It fails in the method call before https://github.com/encode/django-rest-framework/blob/0cc09f0c0dbe4a6552b1a5bbaa4f7f921270698a/rest_framework/renderers.py#L424-L434 it throws because the attribute does not exist in the serializer... so it never reaches the statement https://github.com/encode/django-rest-framework/blob/0cc09f0c0dbe4a6552b1a5bbaa4f7f921270698a/rest_framework/renderers.py#L477-L481