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

Browseable API does not check view permissions when rendering permission denied response ? #5127

Closed
5 of 6 tasks
zemanel opened this issue May 6, 2017 · 6 comments
Closed
5 of 6 tasks

Comments

@zemanel
Copy link

zemanel commented May 6, 2017

Checklist

  • I have verified that that issue exists against the master branch of Django REST framework.
  • I have searched for similar issues in both open and closed tickets and cannot find a duplicate.
  • This is not a usage question. (Those should be directed to the discussion group instead.)
  • 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.)
  • 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

With IsAuthenticated permission class added to a ViewSet, accessing the Browseable API (not an issue with other formats) with an anonymouys user causes
´get_queryset()´ to be called and if, for example, ´get_queryset()´ expects an authenticated user, call can fail with exception.

I believe this happens when calculating the template context of a permission denied response, as currently

get_context() (https://github.com/encode/django-rest-framework/blob/master/rest_framework/renderers.py#L626) calls

get_filter_form() (https://github.com/encode/django-rest-framework/blob/master/rest_framework/renderers.py#L611) which calls ´get_queryset()´.

Example code:

from rest_framework.permissions import IsAuthenticated

class OrganizationPermissions(permissions.BasePermission):
    def has_object_permission(self, request, view, obj):
        return request.user.is_staff or (request.user == obj.owner.organization_user.user)


class OrganizationViewSet(viewsets.ModelViewSet):
    queryset = Organization.objects.all()
    serializer_class = OrganizationSerializer
    permission_classes = [IsAuthenticated, OrganizationPermissions]


    def get_queryset(self):
    	# query will fail with Exception:
    	# 	TypeError: int() argument must be a string, a bytes-like object or a number, not 'AnonymousUser'
		qs = super().get_queryset().filter(users=self.request.user)
        return qs

Traceback:

Traceback (most recent call last):
  File "/Users/zemanel/.virtualenvs/teammarks-api/lib/python3.5/site-packages/django/contrib/staticfiles/handlers.py", line 63, in __call__
    return self.application(environ, start_response)
  File "/Users/zemanel/.virtualenvs/teammarks-api/lib/python3.5/site-packages/django/core/handlers/wsgi.py", line 157, in __call__
    response = self.get_response(request)
  File "/Users/zemanel/.virtualenvs/teammarks-api/lib/python3.5/site-packages/django/core/handlers/base.py", line 124, in get_response
    response = self._middleware_chain(request)
  File "/Users/zemanel/.virtualenvs/teammarks-api/lib/python3.5/site-packages/django/core/handlers/exception.py", line 43, in inner
    response = response_for_exception(request, exc)
  File "/Users/zemanel/.virtualenvs/teammarks-api/lib/python3.5/site-packages/django/core/handlers/exception.py", line 93, in response_for_exception
    response = handle_uncaught_exception(request, get_resolver(get_urlconf()), sys.exc_info())
  File "/Users/zemanel/.virtualenvs/teammarks-api/lib/python3.5/site-packages/django/core/handlers/exception.py", line 139, in handle_uncaught_exception
    return debug.technical_500_response(request, *exc_info)
  File "/Users/zemanel/.virtualenvs/teammarks-api/lib/python3.5/site-packages/django_extensions/management/technical_response.py", line 6, in null_technical_500_response
    six.reraise(exc_type, exc_value, tb)
  File "/Users/zemanel/.virtualenvs/teammarks-api/lib/python3.5/site-packages/six.py", line 686, in reraise
    raise value
  File "/Users/zemanel/.virtualenvs/teammarks-api/lib/python3.5/site-packages/django/core/handlers/exception.py", line 41, in inner
    response = get_response(request)
  File "/Users/zemanel/.virtualenvs/teammarks-api/lib/python3.5/site-packages/django/core/handlers/base.py", line 249, in _legacy_get_response
    response = self._get_response(request)
  File "/Users/zemanel/.virtualenvs/teammarks-api/lib/python3.5/site-packages/django/core/handlers/base.py", line 217, in _get_response
    response = self.process_exception_by_middleware(e, request)
  File "/Users/zemanel/.virtualenvs/teammarks-api/lib/python3.5/site-packages/django/core/handlers/base.py", line 215, in _get_response
    response = response.render()
  File "/Users/zemanel/.virtualenvs/teammarks-api/lib/python3.5/site-packages/django/template/response.py", line 107, in render
    self.content = self.rendered_content
  File "/Users/zemanel/.virtualenvs/teammarks-api/lib/python3.5/site-packages/rest_framework/response.py", line 72, in rendered_content
    ret = renderer.render(self.data, accepted_media_type, context)
  File "/Users/zemanel/.virtualenvs/teammarks-api/lib/python3.5/site-packages/rest_framework/renderers.py", line 703, in render
    context = self.get_context(data, accepted_media_type, renderer_context)
  File "/Users/zemanel/.virtualenvs/teammarks-api/lib/python3.5/site-packages/rest_framework/renderers.py", line 680, in get_context
    'filter_form': self.get_filter_form(data, view, request),
  File "/Users/zemanel/.virtualenvs/teammarks-api/lib/python3.5/site-packages/rest_framework/renderers.py", line 611, in get_filter_form
    queryset = view.get_queryset()
  File "/Users/zemanel/Dropbox/Projects/teammarks/teammarks-api/src/teams/views.py", line 43, in get_queryset
    qs = super().get_queryset().filter(users=self.request.user)
  File "/Users/zemanel/.virtualenvs/teammarks-api/lib/python3.5/site-packages/django/db/models/query.py", line 781, in filter
    return self._filter_or_exclude(False, *args, **kwargs)
  File "/Users/zemanel/.virtualenvs/teammarks-api/lib/python3.5/site-packages/django/db/models/query.py", line 799, in _filter_or_exclude
    clone.query.add_q(Q(*args, **kwargs))
  File "/Users/zemanel/.virtualenvs/teammarks-api/lib/python3.5/site-packages/django/db/models/sql/query.py", line 1260, in add_q
    clause, _ = self._add_q(q_object, self.used_aliases)
  File "/Users/zemanel/.virtualenvs/teammarks-api/lib/python3.5/site-packages/django/db/models/sql/query.py", line 1286, in _add_q
    allow_joins=allow_joins, split_subq=split_subq,
  File "/Users/zemanel/.virtualenvs/teammarks-api/lib/python3.5/site-packages/django/db/models/sql/query.py", line 1216, in build_filter
    condition = lookup_class(lhs, value)
  File "/Users/zemanel/.virtualenvs/teammarks-api/lib/python3.5/site-packages/django/db/models/lookups.py", line 24, in __init__
    self.rhs = self.get_prep_lookup()
  File "/Users/zemanel/.virtualenvs/teammarks-api/lib/python3.5/site-packages/django/db/models/fields/related_lookups.py", line 110, in get_prep_lookup
    self.rhs = target_field.get_prep_value(self.rhs)
  File "/Users/zemanel/.virtualenvs/teammarks-api/lib/python3.5/site-packages/django/db/models/fields/__init__.py", line 962, in get_prep_value
    return int(value)
TypeError: int() argument must be a string, a bytes-like object or a number, not 'AnonymousUser'

Expected behavior

HTTP 403 Response

Actual behavior

HTTP 500 Response

@tomchristie
Copy link
Member

Seems a valid issue, yup. Side-effect of the rather wooly way that the browsable API has to infer what filters are present etc...

Not typically going to raise an error, but in this particular case of filtering against the user, causes an exception when an authenticated user request is made, since the field is attempt to filter against a PK, but the object is being coerced into the AnonymousUser string.

We probably want to double check if this is still an issue, as it's possible that it's been resolved by something else along the way. Otherwise, we could wrap quiet exception handling around get_queryset in that case.

busla added a commit to busla/django-rest-framework that referenced this issue Apr 13, 2019
@rpkilby
Copy link
Member

rpkilby commented Jul 3, 2019

For anyone looking to reproduce this issue, they may want to take a look at #6592, although it wasn't able to reproduce the failure.

@uliSchuster
Copy link

I can confirm that this is still an issue with

  • DRF 3.12.1
  • Django 3.1.2
  • Python 3.9

itsdkey added a commit to itsdkey/django-rest-framework that referenced this issue Feb 13, 2021
itsdkey added a commit to itsdkey/django-rest-framework that referenced this issue Feb 21, 2021
itsdkey added a commit to itsdkey/django-rest-framework that referenced this issue Feb 21, 2021
@itsdkey
Copy link
Contributor

itsdkey commented Feb 25, 2021

I've added my 3 quarters to this issue. Please check out my pull request with the explanation of why I think this isn't a bug at all.

@stale
Copy link

stale bot commented Apr 30, 2022

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale label Apr 30, 2022
@tomchristie
Copy link
Member

Closed via #5127 - thanks @itsdkey

sigvef pushed a commit to sigvef/django-rest-framework that referenced this issue Dec 3, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants