Skip to content

Commit

Permalink
enhance authentication metrics
Browse files Browse the repository at this point in the history
BREAKING CHANGE: Changed names of NewRelic metric names and values.
  Has potential to break NewRelic dashboards or alerts, but should not
  have an affect on deployed code.

- Renamed 'request_auth_type' to 'request_auth_type_guess'. This makes
  it more clear that this metric could be wrong in certain cases.
- Renamed value `session-or-unknown` to `session-or-other`. This name
  makes it more clear that it is the method of authentication that is in
  question, not whether or not the user is authenticated.
- Added 'jwt-cookie' as new value for 'request_auth_type_guess'.
- Added new 'request_authenticated_user_found_in_middleware' metric.
  Helps identify for what middleware step the request user was set, if
  it was set. Example values: 'process_request', 'process_view',
  'process_response', or 'process_exception'.
- Fixed/Added setting of authentication metrics for exceptions as well.
- Fixed values of 'unauthenticated' and 'no-user' for
  'request_auth_type_guess' to be more accurate.

ARCHBOM-128
  • Loading branch information
robrap committed May 5, 2020
1 parent a8960bd commit be52dd2
Show file tree
Hide file tree
Showing 3 changed files with 132 additions and 28 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__ = '5.1.0' # pragma: no cover
__version__ = '6.0.0' # pragma: no cover
89 changes: 74 additions & 15 deletions edx_rest_framework_extensions/middleware.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,14 +4,22 @@
from django.utils.deprecation import MiddlewareMixin
from edx_django_utils import monitoring

from edx_rest_framework_extensions.auth.jwt.constants import USE_JWT_COOKIE_HEADER
from edx_rest_framework_extensions.auth.jwt.cookies import jwt_cookie_name


class RequestMetricsMiddleware(MiddlewareMixin):
"""
Adds various request related metrics.
Possible metrics include:
request_auth_type: Example values include: no-user, unauthenticated,
jwt, bearer, other-token-type, or session-or-unknown
request_authenticated_user_set_in_middleware:
Example values: 'process_request', 'process_view', 'process_response',
or 'process_exception'. Metric won't exist if user is not authenticated.
request_auth_type_guess: Example values include: no-user, unauthenticated,
jwt, bearer, other-token-type, jwt-cookie, or session-or-other
Note: These are just guesses because if a token was expired, for example,
the user could have been authenticated by some other means.
request_client_name: The client name from edx-rest-api-client calls.
request_referer
request_user_agent: The user agent string from the request header.
Expand All @@ -28,17 +36,44 @@ class RequestMetricsMiddleware(MiddlewareMixin):
This middleware should also appear after any authentication middleware.
"""
_authenticated_user_found_in_middleware = None

def process_request(self, request):
"""
Caches if authenticated user was found.
"""
self._cache_if_authenticated_user_found_in_middleware(request, 'process_request')

def process_view(self, request, view_func, view_args, view_kwargs): # pylint: disable=unused-argument
"""
Caches if authenticated user was found.
"""
self._cache_if_authenticated_user_found_in_middleware(request, 'process_view')

def process_response(self, request, response):
"""
Add metrics for various details of the request.
"""
self._set_request_auth_type_metric(request)
self._cache_if_authenticated_user_found_in_middleware(request, 'process_response')
self._set_all_request_metrics(request)
return response

def process_exception(self, request, exception): # pylint: disable=unused-argument
"""
Django middleware handler to process an exception
"""
self._cache_if_authenticated_user_found_in_middleware(request, 'process_exception')
self._set_all_request_metrics(request)

def _set_all_request_metrics(self, request):
"""
Sets all the request metrics
"""
self._set_request_auth_type_guess_metric(request)
self._set_request_user_agent_metrics(request)
self._set_request_referer_metric(request)
self._set_request_user_id_metric(request)

return response
self._set_request_authenticated_user_found_in_middleware()

def _set_request_user_id_metric(self, request):
"""
Expand Down Expand Up @@ -76,28 +111,52 @@ def _set_request_user_agent_metrics(self, request):
if len(user_agent_parts) == 3 and user_agent_parts[1].startswith('edx-rest-api-client/'):
monitoring.set_custom_metric('request_client_name', user_agent_parts[2])

def _set_request_auth_type_metric(self, request):
def _set_request_auth_type_guess_metric(self, request):
"""
Add metric 'request_auth_type' for the authentication type used.
Add metric 'request_auth_type_guess' for the authentication type used.
NOTE: This is a best guess at this point. Possible values include:
no-user
unauthenticated
jwt/bearer/other-token-type
session-or-unknown (catch all)
jwt-cookie
session-or-other (catch all)
"""
if 'HTTP_AUTHORIZATION' in request.META and request.META['HTTP_AUTHORIZATION']:
if not hasattr(request, 'user') or not request.user:
auth_type = 'no-user'
elif not request.user.is_authenticated:
auth_type = 'unauthenticated'
elif 'HTTP_AUTHORIZATION' in request.META and request.META['HTTP_AUTHORIZATION']:
token_parts = request.META['HTTP_AUTHORIZATION'].split()
# Example: "JWT eyJhbGciO..."
if len(token_parts) == 2:
auth_type = token_parts[0].lower() # 'jwt' or 'bearer' (for example)
else:
auth_type = 'other-token-type'
elif not hasattr(request, 'user') or not request.user:
auth_type = 'no-user'
elif not request.user.is_authenticated:
auth_type = 'unauthenticated'
elif USE_JWT_COOKIE_HEADER in request.META and jwt_cookie_name() in request.COOKIES:
auth_type = 'jwt-cookie'
else:
auth_type = 'session-or-unknown'
monitoring.set_custom_metric('request_auth_type', auth_type)
auth_type = 'session-or-other'
monitoring.set_custom_metric('request_auth_type_guess', auth_type)

def _set_request_authenticated_user_found_in_middleware(self):
"""
Add metric 'request_authenticated_user_found_in_middleware' if authenticated user was found.
"""
if self._authenticated_user_found_in_middleware:
monitoring.set_custom_metric(
'request_authenticated_user_found_in_middleware',
self._authenticated_user_found_in_middleware
)

def _cache_if_authenticated_user_found_in_middleware(self, request, value):
"""
Updates the cached process step in which the authenticated user was found.
"""
if self._authenticated_user_found_in_middleware:
# value already set in earlier middleware step
return

if hasattr(request, 'user') and request.user and request.user.is_authenticated:
self._authenticated_user_found_in_middleware = value
69 changes: 57 additions & 12 deletions edx_rest_framework_extensions/tests/test_middleware.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@
from django.test import RequestFactory, TestCase
from mock import call, patch

from edx_rest_framework_extensions.auth.jwt.constants import USE_JWT_COOKIE_HEADER
from edx_rest_framework_extensions.auth.jwt.cookies import jwt_cookie_name
from edx_rest_framework_extensions.middleware import RequestMetricsMiddleware
from edx_rest_framework_extensions.tests.factories import UserFactory

Expand All @@ -17,11 +19,18 @@ def setUp(self):
self.request = RequestFactory().get('/')
self.middleware = RequestMetricsMiddleware()

@patch('edx_django_utils.monitoring.set_custom_metric')
def test_request_auth_type_guess_anonymous_metric(self, mock_set_custom_metric):
self.request.user = AnonymousUser()

self.middleware.process_response(self.request, None)
mock_set_custom_metric.assert_called_once_with('request_auth_type_guess', 'unauthenticated')

@patch('edx_django_utils.monitoring.set_custom_metric')
def test_request_no_headers(self, mock_set_custom_metric):
self.request.user = None
self.middleware.process_response(self.request, None)
mock_set_custom_metric.assert_called_once_with('request_auth_type', 'no-user')
mock_set_custom_metric.assert_called_once_with('request_auth_type_guess', 'no-user')

@patch('edx_django_utils.monitoring.set_custom_metric')
def test_request_blank_headers(self, mock_set_custom_metric):
Expand All @@ -30,7 +39,7 @@ def test_request_blank_headers(self, mock_set_custom_metric):
self.request.META['HTTP_AUTHORIZATION'] = ''

self.middleware.process_response(self.request, None)
mock_set_custom_metric.assert_called_once_with('request_auth_type', 'no-user')
mock_set_custom_metric.assert_called_once_with('request_auth_type_guess', 'no-user')

@patch('edx_django_utils.monitoring.set_custom_metric')
def test_request_referer_metric(self, mock_set_custom_metric):
Expand All @@ -39,7 +48,7 @@ def test_request_referer_metric(self, mock_set_custom_metric):
self.middleware.process_response(self.request, None)
expected_calls = [
call('request_referer', 'test-http-referer'),
call('request_auth_type', 'no-user'),
call('request_auth_type_guess', 'no-user'),
]
mock_set_custom_metric.assert_has_calls(expected_calls, any_order=True)

Expand All @@ -51,7 +60,7 @@ def test_request_rest_client_user_agent_metrics(self, mock_set_custom_metric):
expected_calls = [
call('request_user_agent', 'python-requests/2.9.1 edx-rest-api-client/1.7.2 test-client'),
call('request_client_name', 'test-client'),
call('request_auth_type', 'no-user'),
call('request_auth_type_guess', 'no-user'),
]
mock_set_custom_metric.assert_has_calls(expected_calls, any_order=True)

Expand All @@ -62,7 +71,7 @@ def test_request_standard_user_agent_metrics(self, mock_set_custom_metric):
self.middleware.process_response(self.request, None)
expected_calls = [
call('request_user_agent', 'test-user-agent'),
call('request_auth_type', 'no-user'),
call('request_auth_type_guess', 'no-user'),
]
mock_set_custom_metric.assert_has_calls(expected_calls, any_order=True)

Expand All @@ -73,29 +82,65 @@ def test_request_standard_user_agent_metrics(self, mock_set_custom_metric):
)
@ddt.unpack
@patch('edx_django_utils.monitoring.set_custom_metric')
def test_request_auth_type_token_metric(self, token, expected_token_type, mock_set_custom_metric):
def test_request_auth_type_guess_token_metric(self, token, expected_token_type, mock_set_custom_metric):
self.request.user = UserFactory()
self.request.META['HTTP_AUTHORIZATION'] = token

self.middleware.process_response(self.request, None)
mock_set_custom_metric.assert_called_once_with('request_auth_type', expected_token_type)
mock_set_custom_metric.assert_any_call('request_auth_type_guess', expected_token_type)

@patch('edx_django_utils.monitoring.set_custom_metric')
def test_request_auth_type_anonymous_metric(self, mock_set_custom_metric):
self.request.user = AnonymousUser()
def test_request_auth_type_guess_jwt_cookie_metric(self, mock_set_custom_metric):
self.request.user = UserFactory()
self.request.META[USE_JWT_COOKIE_HEADER] = True
self.request.COOKIES[jwt_cookie_name()] = 'reconstituted-jwt-cookie'

self.middleware.process_response(self.request, None)
mock_set_custom_metric.assert_called_once_with('request_auth_type', 'unauthenticated')
mock_set_custom_metric.assert_any_call('request_auth_type_guess', 'jwt-cookie')

@patch('edx_django_utils.monitoring.set_custom_metric')
def test_request_auth_type_session_metric(self, mock_set_custom_metric):
def test_request_auth_type_guess_session_metric(self, mock_set_custom_metric):
self.request.user = UserFactory()

self.middleware.process_response(self.request, None)
mock_set_custom_metric.assert_any_call('request_auth_type', 'session-or-unknown')
mock_set_custom_metric.assert_any_call('request_auth_type_guess', 'session-or-other')

@patch('edx_django_utils.monitoring.set_custom_metric')
def test_request_user_id_metric(self, mock_set_custom_metric):
self.request.user = UserFactory()

self.middleware.process_response(self.request, None)
mock_set_custom_metric.assert_any_call('request_user_id', self.request.user.id)
mock_set_custom_metric.assert_any_call(
'request_authenticated_user_found_in_middleware', 'process_response'
)

@patch('edx_django_utils.monitoring.set_custom_metric')
def test_request_user_id_metric_with_exception(self, mock_set_custom_metric):
self.request.user = UserFactory()

self.middleware.process_exception(self.request, None)
mock_set_custom_metric.assert_any_call('request_user_id', self.request.user.id)
mock_set_custom_metric.assert_any_call(
'request_authenticated_user_found_in_middleware', 'process_exception'
)

@patch('edx_django_utils.monitoring.set_custom_metric')
def test_authenticated_user_found_in_process_request(self, mock_set_custom_metric):
self.request.user = UserFactory()
self.middleware.process_request(self.request)
self.middleware.process_response(self.request, None)

mock_set_custom_metric.assert_any_call(
'request_authenticated_user_found_in_middleware', 'process_request'
)

@patch('edx_django_utils.monitoring.set_custom_metric')
def test_authenticated_user_found_in_process_view(self, mock_set_custom_metric):
self.request.user = UserFactory()
self.middleware.process_view(self.request, None, None, None)
self.middleware.process_response(self.request, None)

mock_set_custom_metric.assert_any_call(
'request_authenticated_user_found_in_middleware', 'process_view'
)

0 comments on commit be52dd2

Please sign in to comment.