diff --git a/docs/api-guide/authentication.md b/docs/api-guide/authentication.md index a7a24029b..63a789dfc 100644 --- a/docs/api-guide/authentication.md +++ b/docs/api-guide/authentication.md @@ -291,6 +291,12 @@ You *may* also override the `.authenticate_header(self, request)` method. If im If the `.authenticate_header()` method is not overridden, the authentication scheme will return `HTTP 403 Forbidden` responses when an unauthenticated request is denied access. +--- + +**Note:** When your custom authenticator is invoked by the request object's `.user` or `.auth` properties, you may see an `AttributeError` re-raised as a `WrappedAttributeError`. This is necessary to prevent the original exception from being suppressed by the outer property access. Python will not recognize that the `AttributeError` orginates from your custom authenticator and will instead assume that the request object does not have a `.user` or `.auth` property. These errors should be fixed or otherwise handled by your authenticator. + +--- + ## Example The following example will authenticate any incoming request as the user given by the username in a custom request header named 'X_USERNAME'. diff --git a/docs/api-guide/requests.md b/docs/api-guide/requests.md index 393b1ab9a..83a38d1c3 100644 --- a/docs/api-guide/requests.md +++ b/docs/api-guide/requests.md @@ -90,6 +90,10 @@ You won't typically need to access this property. --- +**Note:** You may see a `WrappedAttributeError` raised when calling the `.user` or `.auth` properties. These errors originate from an authenticator as a standard `AttributeError`, however it's necessary that they be re-raised as a different exception type in order to prevent them from being suppressed by the outer property access. Python will not recognize that the `AttributeError` orginates from the authenticator and will instaed assume that the request object does not have a `.user` or `.auth` property. The authenticator will need to be fixed. + +--- + # Browser enhancements REST framework supports a few browser enhancements such as browser-based `PUT`, `PATCH` and `DELETE` forms. diff --git a/rest_framework/request.py b/rest_framework/request.py index 5c9cb6136..7e4daf274 100644 --- a/rest_framework/request.py +++ b/rest_framework/request.py @@ -10,6 +10,9 @@ The wrapped request then offers a richer API, in particular : """ from __future__ import unicode_literals +import sys +from contextlib import contextmanager + from django.conf import settings from django.http import HttpRequest, QueryDict from django.http.multipartparser import parse_header @@ -59,6 +62,24 @@ class override_method(object): self.view.action = self.action +class WrappedAttributeError(Exception): + pass + + +@contextmanager +def wrap_attributeerrors(): + """ + Used to re-raise AttributeErrors caught during authentication, preventing + these errors from otherwise being handled by the attribute access protocol. + """ + try: + yield + except AttributeError: + info = sys.exc_info() + exc = WrappedAttributeError(str(info[1])) + six.reraise(type(exc), exc, info[2]) + + class Empty(object): """ Placeholder for unset attributes. @@ -197,7 +218,8 @@ class Request(object): by the authentication classes provided to the request. """ if not hasattr(self, '_user'): - self._authenticate() + with wrap_attributeerrors(): + self._authenticate() return self._user @user.setter @@ -220,7 +242,8 @@ class Request(object): request, such as an authentication token. """ if not hasattr(self, '_auth'): - self._authenticate() + with wrap_attributeerrors(): + self._authenticate() return self._auth @auth.setter @@ -239,7 +262,8 @@ class Request(object): to authenticate the request, or `None`. """ if not hasattr(self, '_authenticator'): - self._authenticate() + with wrap_attributeerrors(): + self._authenticate() return self._authenticator def _load_data_and_files(self): @@ -322,7 +346,7 @@ class Request(object): try: parsed = parser.parse(stream, media_type, self.parser_context) - except: + except Exception: # If we get an exception during parsing, fill in empty data and # re-raise. Ensures we don't simply repeat the error when # attempting to render the browsable renderer response, or when diff --git a/tests/test_request.py b/tests/test_request.py index c14233a6f..76589a440 100644 --- a/tests/test_request.py +++ b/tests/test_request.py @@ -6,8 +6,10 @@ from __future__ import unicode_literals import os.path import tempfile +import pytest from django.conf.urls import url from django.contrib.auth import authenticate, login, logout +from django.contrib.auth.middleware import AuthenticationMiddleware from django.contrib.auth.models import User from django.contrib.sessions.middleware import SessionMiddleware from django.core.files.uploadedfile import SimpleUploadedFile @@ -17,7 +19,7 @@ from django.utils import six from rest_framework import status from rest_framework.authentication import SessionAuthentication from rest_framework.parsers import BaseParser, FormParser, MultiPartParser -from rest_framework.request import Request +from rest_framework.request import Request, WrappedAttributeError from rest_framework.response import Response from rest_framework.test import APIClient, APIRequestFactory from rest_framework.views import APIView @@ -197,7 +199,8 @@ class TestUserSetter(TestCase): # available to login and logout functions self.wrapped_request = factory.get('/') self.request = Request(self.wrapped_request) - SessionMiddleware().process_request(self.request) + SessionMiddleware().process_request(self.wrapped_request) + AuthenticationMiddleware().process_request(self.wrapped_request) User.objects.create_user('ringo', 'starr@thebeatles.com', 'yellow') self.user = authenticate(username='ringo', password='yellow') @@ -212,9 +215,9 @@ class TestUserSetter(TestCase): def test_user_can_logout(self): self.request.user = self.user - self.assertFalse(self.request.user.is_anonymous) + assert not self.request.user.is_anonymous logout(self.request) - self.assertTrue(self.request.user.is_anonymous) + assert self.request.user.is_anonymous def test_logged_in_user_is_set_on_wrapped_request(self): login(self.request, self.user) @@ -227,22 +230,27 @@ class TestUserSetter(TestCase): """ class AuthRaisesAttributeError(object): def authenticate(self, request): - import rest_framework - rest_framework.MISSPELLED_NAME_THAT_DOESNT_EXIST + self.MISSPELLED_NAME_THAT_DOESNT_EXIST - self.request = Request(factory.get('/'), authenticators=(AuthRaisesAttributeError(),)) - SessionMiddleware().process_request(self.request) + request = Request(self.wrapped_request, authenticators=(AuthRaisesAttributeError(),)) - login(self.request, self.user) - try: - self.request.user - except AttributeError as error: - assert str(error) in ( - "'module' object has no attribute 'MISSPELLED_NAME_THAT_DOESNT_EXIST'", # Python < 3.5 - "module 'rest_framework' has no attribute 'MISSPELLED_NAME_THAT_DOESNT_EXIST'", # Python >= 3.5 - ) - else: - assert False, 'AttributeError not raised' + # The middleware processes the underlying Django request, sets anonymous user + assert self.wrapped_request.user.is_anonymous + + # The DRF request object does not have a user and should run authenticators + expected = r"no attribute 'MISSPELLED_NAME_THAT_DOESNT_EXIST'" + with pytest.raises(WrappedAttributeError, match=expected): + request.user + + # python 2 hasattr fails for *any* exception, not just AttributeError + if six.PY2: + return + + with pytest.raises(WrappedAttributeError, match=expected): + hasattr(request, 'user') + + with pytest.raises(WrappedAttributeError, match=expected): + login(request, self.user) class TestAuthSetter(TestCase):