Skip to content

Commit

Permalink
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Ensure Django{Model,Object}Permissions don't hide exceptions.
Browse files Browse the repository at this point in the history
Quietly catching `AttributeError` and `TypeError` when calling
`get_queryset()` is rather insidious, as those exceptions get caught no
matter where they might happen in the call stack.
akx committed Nov 25, 2015

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
1 parent d8e3f8c commit 5c181b1
Showing 2 changed files with 39 additions and 8 deletions.
16 changes: 8 additions & 8 deletions rest_framework/permissions.py
Original file line number Diff line number Diff line change
@@ -112,15 +112,15 @@ def has_permission(self, request, view):
if getattr(view, '_ignore_model_permissions', False):
return True

try:
if hasattr(view, 'get_queryset'):
queryset = view.get_queryset()
except AttributeError:
else:
queryset = getattr(view, 'queryset', None)

assert queryset is not None, (
'Cannot apply DjangoModelPermissions on a view that '
'does not have `.queryset` property or overrides the '
'`.get_queryset()` method.')
'does not set `.queryset` or have `.get_queryset()` method.'
)

perms = self.get_required_permissions(request.method, queryset.model)

@@ -169,15 +169,15 @@ def get_required_object_permissions(self, method, model_cls):
return [perm % kwargs for perm in self.perms_map[method]]

def has_object_permission(self, request, view, obj):
try:
if hasattr(view, 'get_queryset'):
queryset = view.get_queryset()
except AttributeError:
else:
queryset = getattr(view, 'queryset', None)

assert queryset is not None, (
'Cannot apply DjangoObjectPermissions on a view that '
'does not have `.queryset` property or overrides the '
'`.get_queryset()` method.')
'does not set `.queryset` or have `.get_queryset()` method.'
)

model_cls = queryset.model
user = request.user
31 changes: 31 additions & 0 deletions tests/test_permissions.py
Original file line number Diff line number Diff line change
@@ -3,6 +3,7 @@
import base64
import unittest

import pytest
from django.contrib.auth.models import Group, Permission, User
from django.core.urlresolvers import ResolverMatch
from django.db import models
@@ -14,6 +15,9 @@
)
from rest_framework.compat import guardian
from rest_framework.filters import DjangoObjectPermissionsFilter
from rest_framework.permissions import (
DjangoModelPermissions, DjangoObjectPermissions
)
from rest_framework.routers import DefaultRouter
from rest_framework.test import APIRequestFactory
from tests.models import BasicModel
@@ -468,3 +472,30 @@ def test_permission_denied_for_object_with_custom_detail(self):
detail = response.data.get('detail')
self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN)
self.assertEqual(detail, self.custom_message)


class CheckAttributeError(AttributeError):
pass


class GetQuerysetRaises(generics.ListAPIView):
def get_queryset(self):
raise CheckAttributeError("Something terrible occurred deep down in the call stack")


class DjangoObjectPermissionsList(GetQuerysetRaises):
permission_classes = (DjangoObjectPermissions,)


class DjangoModelPermissionsList(GetQuerysetRaises):
permission_classes = (DjangoModelPermissions,)


def test_DjangoObjectPermissions_hiding():
with pytest.raises(CheckAttributeError):
DjangoObjectPermissionsList().check_object_permissions(request=None, obj=None)


def test_DjangoModelPermissions_hiding():
with pytest.raises(CheckAttributeError):
DjangoModelPermissionsList().check_permissions(request=None)

0 comments on commit 5c181b1

Please sign in to comment.