Skip to content

Permission checks on related resources not executed #864

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

Closed
uliSchuster opened this issue Nov 18, 2020 · 4 comments · Fixed by #872
Closed

Permission checks on related resources not executed #864

uliSchuster opened this issue Nov 18, 2020 · 4 comments · Fixed by #872

Comments

@uliSchuster
Copy link
Contributor

I do have a ModelViewSet, say MyModelViewSet with custom permissions, and several related resources that are available via hyperlinks. The related resources have permissions that are different from the parent ViewSet. When I access the related resource via the provided hyperlinks, the permissions on the related resource are not enforced.

My hypothesis on what happens is as follows:
As recommended in the documentation, the related resource hyperlinks are made available through the parent view; i.e. through MyModelViewSet. The URL configuration is

  path(
        "mymode/<pk>/<related_field>/",
        MyModelViewSet.as_view({"get": "retrieve_related"}),
        name="mymodel-related",
    )

When I access the related resource, the method retrieve_related on the parent view is executed. Therefore, the permission checks on the parent view are run, but not the permission checks on the related view.

Is this behavior intended? It dod not find it referenced anywhere in the documentation. To me, it came as a surprise and would have opened up serious security problems.

@sliverc
Copy link
Member

sliverc commented Nov 20, 2020

We have discussed this at length in #496

Best summary is probably #496 (comment)

We should certainly note this in the documentation that related urls are passed through parent object permission (as they actually also would if included_serializers was used).

@uliSchuster
Copy link
Contributor Author

Thanks for the explanation and the link. I understand that this isa much deeper issue than apparent at first. For my case, it might be best to route the url to a separate view instead of handling it via the get_related method on the parent view. Which makes me wonder why the latter (using the GET/retrieve method on the related view) is not the default case?

@uliSchuster
Copy link
Contributor Author

uliSchuster commented Nov 21, 2020

OK, I figured out how to use the base serializer with the related link. However, it was quite a challenge to get it right. The following comment here in the source code tipped me off:

        # Assuming RelatedField will be declared in two ways:
        # 1. url(r'^authors/(?P<pk>[^/.]+)/(?P<related_field>\w+)/$',
        #         AuthorViewSet.as_view({'get': 'retrieve_related'}))
        # 2. url(r'^authors/(?P<author_pk>[^/.]+)/bio/$',
        #         AuthorBioViewSet.as_view({'get': 'retrieve'}))
        # So, if related_link_url_kwarg == 'pk' it will add 'related_field' parameter to reverse()

It seems quire magical to me that depending on the naming, the link is resolved differently. It would be great to have this information in the documentation.

@sliverc
Copy link
Member

sliverc commented Nov 22, 2020

Great that you were able to figure it out. Related urls are documented here but if you feel this needs to be improved a PR is always welcome.

Leaving this issue open anyhow for now so a note concerning the permission is added to the documentation.

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

Successfully merging a pull request may close this issue.

2 participants