Skip to content

Commit

Permalink
request.user set in JwtAuthCookieMiddleware
Browse files Browse the repository at this point in the history
JWT cookie authentication uses DRF, which does not set `request.user`
until the Middleware call: `process_response`.  This change sets the
`request.user` in process_view, which happens after all
`process_request` calls.

This is has been added to the existing `JwtAuthCookieMiddleware`.

ARCH-1197
  • Loading branch information
robrap committed Oct 4, 2019
1 parent cca9fd4 commit c31cf4e
Show file tree
Hide file tree
Showing 3 changed files with 145 additions and 15 deletions.
2 changes: 1 addition & 1 deletion edx_rest_framework_extensions/__init__.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@
""" edx Django REST Framework extensions. """

__version__ = '2.4.0' # pragma: no cover
__version__ = '2.4.1' # pragma: no cover
49 changes: 49 additions & 0 deletions edx_rest_framework_extensions/auth/jwt/middleware.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,12 @@
import logging

from django.contrib.auth.decorators import login_required
from django.contrib.auth.middleware import get_user
from django.utils.deprecation import MiddlewareMixin
from django.utils.functional import SimpleLazyObject
from edx_django_utils import monitoring
from edx_django_utils.cache import RequestCache
from rest_framework.request import Request
from rest_framework_jwt.authentication import BaseJSONWebTokenAuthentication

from edx_rest_framework_extensions.auth.jwt.cookies import (
Expand Down Expand Up @@ -153,6 +156,9 @@ class JwtAuthCookieMiddleware(MiddlewareMixin):
Reconstitutes JWT auth cookies for use by API views which use the JwtAuthentication
authentication class.
Has side effect of setting request.user to be available for Jwt Authentication
to Middleware using process_view, but not process_request.
We split the JWT across two separate cookies in the browser for security reasons. This
middleware reconstitutes the full JWT into a new cookie on the request object for use
by the JwtAuthentication class.
Expand Down Expand Up @@ -191,10 +197,17 @@ def process_view(self, request, view_func, view_args, view_kwargs): # pylint: d
"""
Reconstitute the full JWT and add a new cookie on the request object.
"""
assert hasattr(request, 'session'), "The Django authentication middleware requires session middleware to be installed. Edit your MIDDLEWARE_CLASSES setting to insert 'django.contrib.sessions.middleware.SessionMiddleware'." # noqa E501 line too long

use_jwt_cookie_requested = request.META.get(USE_JWT_COOKIE_HEADER)
header_payload_cookie = request.COOKIES.get(jwt_cookie_header_payload_name())
signature_cookie = request.COOKIES.get(jwt_cookie_signature_name())

if use_jwt_cookie_requested:
# DRF does not set request.user until process_response. This makes it available in process_view.
# For more info, see https://github.com/jpadilla/django-rest-framework-jwt/issues/45#issuecomment-74996698
request.user = SimpleLazyObject(lambda: _get_user_from_jwt(request, view_func))

if not use_jwt_cookie_requested:
metric_value = 'not-requested'
elif header_payload_cookie and signature_cookie:
Expand All @@ -219,10 +232,46 @@ def process_view(self, request, view_func, view_args, view_kwargs): # pylint: d
log.warning(log_message)
else:
metric_value = 'missing-both'
log.warning('Both JWT auth cookies missing. JWT auth cookies will not be reconstituted.')

monitoring.set_custom_metric('request_jwt_cookie', metric_value)


def _get_user_from_jwt(request, view_func):
user = get_user(request)
if user.is_authenticated():
return user

try:
jwt_authentication_class = _get_jwt_authentication_class(view_func)
if jwt_authentication_class:
user_jwt = jwt_authentication_class().authenticate(Request(request))
if user_jwt is not None:
return user_jwt[0]
else:
log.warn('Jwt Authentication failed and request.user could not be set.')
else:
log.warn('Jwt Authentication expected, but view %s is not using a JwtAuthentication class.', view_func)
except Exception as e:
log.exception('Unknown error attempting to complete Jwt Authentication.') # pragma: no cover

return user


def _get_jwt_authentication_class(view_func):
"""
Returns the first DRF Authentication class that is a subclass of BaseJSONWebTokenAuthentication
"""
view_class = _get_view_class(view_func)
view_authentication_classes = getattr(view_class, 'authentication_classes', tuple())
if _includes_base_class(view_authentication_classes, BaseJSONWebTokenAuthentication):
return next(
current_class for current_class in view_authentication_classes
if issubclass(current_class, BaseJSONWebTokenAuthentication)
)
return None


def _includes_base_class(iter_classes, base_class):
"""
Returns whether any class in iter_class is a subclass of the given base_class.
Expand Down
109 changes: 95 additions & 14 deletions edx_rest_framework_extensions/auth/jwt/tests/test_middleware.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,10 @@
from django.conf.urls import url
from django.http.cookie import SimpleCookie
from django.test import Client, RequestFactory, TestCase, override_settings
from django.utils.deprecation import MiddlewareMixin
from edx_django_utils.cache import RequestCache
from itertools import product
from mock import patch
from mock import patch, ANY
from rest_condition import C
from rest_framework.authentication import BaseAuthentication, SessionAuthentication
from rest_framework.permissions import IsAuthenticated
Expand Down Expand Up @@ -180,7 +181,7 @@ class HasNoCondPermView(APIView):
self.assertIn(NotJwtRestrictedApplication, HasNoCondPermView.permission_classes)


class MockJwtAuthentication(BaseAuthentication):
class MockJwtAuthentication(BaseJSONWebTokenAuthentication):
"""
Authenticates a user if the reconstituted jwt cookie contains the expected value.
Expand All @@ -195,6 +196,11 @@ def authenticate(self, request):
return None


class MockUnauthenticatedView(APIView):
def get(self, request): # pylint: disable=unused-argument
return Response({'success': True})


class MockJwtAuthenticationView(APIView):
authentication_classes = (MockJwtAuthentication,)

Expand All @@ -220,10 +226,11 @@ class NoPermissionsRequiredView(MockJwtAuthenticationView):


urlpatterns = [
url(r'^loginredirectifunauthenticated/$', LoginRedirectIfUnauthenticatedView.as_view(), name='payment'),
url(r'^isauthenticatedandloginredirect/$', IsAuthenticatedAndLoginRedirectIfUnauthenticatedView.as_view(), name='payment'), # noqa E501 line too long
url(r'^isauthenticated/$', IsAuthenticatedView.as_view(), name='payment'),
url(r'^nopermissionsrequired/$', NoPermissionsRequiredView.as_view(), name='payment'),
url(r'^loginredirectifunauthenticated/$', LoginRedirectIfUnauthenticatedView.as_view()),
url(r'^isauthenticatedandloginredirect/$', IsAuthenticatedAndLoginRedirectIfUnauthenticatedView.as_view()), # noqa E501 line too long
url(r'^isauthenticated/$', IsAuthenticatedView.as_view()),
url(r'^nopermissionsrequired/$', NoPermissionsRequiredView.as_view()),
url(r'^unauthenticated/$', MockUnauthenticatedView.as_view()),
]


Expand Down Expand Up @@ -263,17 +270,15 @@ def setUp(self):
@override_settings(
ROOT_URLCONF='edx_rest_framework_extensions.auth.jwt.tests.test_middleware',
MIDDLEWARE_CLASSES=(
'django.contrib.sessions.middleware.SessionMiddleware',
'edx_rest_framework_extensions.auth.jwt.middleware.JwtRedirectToLoginIfUnauthenticatedMiddleware',
'edx_rest_framework_extensions.auth.jwt.middleware.JwtAuthCookieMiddleware',
),
LOGIN_URL='/test/login/',
)
def test_login_required_middleware(self, url, has_jwt_cookies, expected_status):
if has_jwt_cookies:
self.client.cookies = SimpleCookie({
jwt_cookie_header_payload_name(): 'header.payload',
jwt_cookie_signature_name(): 'signature',
})
self.client.cookies = _get_test_cookie()
response = self.client.get(url)
self.assertEqual(expected_status, response.status_code)
if response.status_code == 302:
Expand All @@ -293,28 +298,45 @@ def test_login_required_middleware(self, url, has_jwt_cookies, expected_status):
@override_settings(
ROOT_URLCONF='edx_rest_framework_extensions.auth.jwt.tests.test_middleware',
MIDDLEWARE_CLASSES=(
'django.contrib.sessions.middleware.SessionMiddleware',
'edx_rest_framework_extensions.auth.jwt.tests.test_middleware.OverriddenJwtRedirectToLoginIfUnauthenticatedMiddleware', # noqa E501 line too long
'edx_rest_framework_extensions.auth.jwt.middleware.JwtAuthCookieMiddleware',
),
LOGIN_URL='/test/login/',
)
def test_login_required_overridden_middleware(self, url, has_jwt_cookies, expected_status):
if has_jwt_cookies:
self.client.cookies = SimpleCookie({
jwt_cookie_header_payload_name(): 'header.payload',
jwt_cookie_signature_name(): 'signature',
})
self.client.cookies = _get_test_cookie()
response = self.client.get(url)
self.assertEqual(expected_status, response.status_code)
if response.status_code == 302:
self.assertEqual('/overridden/login/?next=' + url, response.url)


class CheckRequestUserForJwtAuthMiddleware(MiddlewareMixin):
"""
This test middleware can be used to confirm that our Jwt Authentication related middleware correctly
set the request.user.
"""
def process_view(self, request, view_func, view_args, view_kwargs): # pylint: disable=unused-argument
assert request.user.is_authenticated(), 'Request.user was expected to be authenticated.'


class CheckRequestUserAnonymousForJwtAuthMiddleware(MiddlewareMixin):
"""
This test middleware can be used to confirm that when Jwt Authentication related middleware does not set
the user (e.g. a failed cookie).
"""
def process_view(self, request, view_func, view_args, view_kwargs): # pylint: disable=unused-argument
assert not request.user.is_authenticated(), 'Request.user was expected to be anonymous.'


@ddt.ddt
class TestJwtAuthCookieMiddleware(TestCase):
def setUp(self):
super(TestJwtAuthCookieMiddleware, self).setUp()
self.request = RequestFactory().get('/')
self.request.session = 'mock session'
self.middleware = JwtAuthCookieMiddleware()

@patch('edx_django_utils.monitoring.set_custom_metric')
Expand Down Expand Up @@ -358,3 +380,62 @@ def test_success(self, mock_set_custom_metric):
self.middleware.process_view(self.request, None, None, None)
self.assertEqual(self.request.COOKIES[jwt_cookie_name()], 'header.payload.signature')
mock_set_custom_metric.assert_called_once_with('request_jwt_cookie', 'success')

@override_settings(
ROOT_URLCONF='edx_rest_framework_extensions.auth.jwt.tests.test_middleware',
MIDDLEWARE_CLASSES=(
'django.contrib.sessions.middleware.SessionMiddleware',
'edx_rest_framework_extensions.auth.jwt.middleware.JwtAuthCookieMiddleware',
'django.contrib.auth.middleware.AuthenticationMiddleware',
'edx_rest_framework_extensions.auth.jwt.tests.test_middleware.CheckRequestUserForJwtAuthMiddleware',
),
)
def test_success_sets_request_user(self):
header = {USE_JWT_COOKIE_HEADER: 'true'}
self.client.cookies = _get_test_cookie()
response = self.client.get('/nopermissionsrequired/', **header)
self.assertEqual(200, response.status_code)

@override_settings(
ROOT_URLCONF='edx_rest_framework_extensions.auth.jwt.tests.test_middleware',
MIDDLEWARE_CLASSES=(
'django.contrib.sessions.middleware.SessionMiddleware',
'edx_rest_framework_extensions.auth.jwt.middleware.JwtAuthCookieMiddleware',
'django.contrib.auth.middleware.AuthenticationMiddleware',
'edx_rest_framework_extensions.auth.jwt.tests.test_middleware.CheckRequestUserAnonymousForJwtAuthMiddleware',
),
)
@patch('edx_rest_framework_extensions.auth.jwt.middleware.log')
def test_invalid_cookie_logs_warning(self, mock_log):
header = {USE_JWT_COOKIE_HEADER: 'true'}
self.client.cookies = _get_test_cookie(is_cookie_valid=False)
response = self.client.get('/nopermissionsrequired/', **header)
self.assertEqual(200, response.status_code)
mock_log.warn.assert_called_once_with('Jwt Authentication failed and request.user could not be set.')

@override_settings(
ROOT_URLCONF='edx_rest_framework_extensions.auth.jwt.tests.test_middleware',
MIDDLEWARE_CLASSES=(
'django.contrib.sessions.middleware.SessionMiddleware',
'edx_rest_framework_extensions.auth.jwt.middleware.JwtAuthCookieMiddleware',
'django.contrib.auth.middleware.AuthenticationMiddleware',
'edx_rest_framework_extensions.auth.jwt.tests.test_middleware.CheckRequestUserAnonymousForJwtAuthMiddleware',
),
)
@patch('edx_rest_framework_extensions.auth.jwt.middleware.log')
def test_unauthenticated_view_logs_warning(self, mock_log):
header = {USE_JWT_COOKIE_HEADER: 'true'}
self.client.cookies = _get_test_cookie()
response = self.client.get('/unauthenticated/', **header)
self.assertEqual(200, response.status_code)
mock_log.warn.assert_called_once_with(
'Jwt Authentication expected, but view %s is not using a JwtAuthentication class.', ANY
)


def _get_test_cookie(is_cookie_valid=True):
header_payload_value = 'header.payload' if is_cookie_valid else 'header.payload.invalid'
return SimpleCookie({
jwt_cookie_header_payload_name(): header_payload_value,
jwt_cookie_signature_name(): 'signature',
})

0 comments on commit c31cf4e

Please sign in to comment.