From 40a8608b0b2f0e942f7cb7958e732298dda5f163 Mon Sep 17 00:00:00 2001 From: Bill DeRusha Date: Wed, 30 Sep 2015 11:13:10 -0400 Subject: [PATCH] Upgrade IP parsing in tracker middleware --- common/djangoapps/track/middleware.py | 11 ++++++- .../djangoapps/track/tests/test_middleware.py | 31 +++++++++++++++++++ common/djangoapps/track/views/__init__.py | 11 ++++++- 3 files changed, 51 insertions(+), 2 deletions(-) diff --git a/common/djangoapps/track/middleware.py b/common/djangoapps/track/middleware.py index f8715bcc0e..d8dbcbc25c 100644 --- a/common/djangoapps/track/middleware.py +++ b/common/djangoapps/track/middleware.py @@ -14,6 +14,7 @@ import re import sys from django.conf import settings +from ipware.ip import get_ip from track import views from track import contexts @@ -24,7 +25,6 @@ log = logging.getLogger(__name__) CONTEXT_NAME = 'edx.request' META_KEY_TO_CONTEXT_KEY = { - 'REMOTE_ADDR': 'ip', 'SERVER_NAME': 'host', 'HTTP_USER_AGENT': 'agent', 'PATH_INFO': 'path', @@ -137,6 +137,7 @@ class TrackMiddleware(object): 'session': self.get_session_key(request), 'user_id': self.get_user_primary_key(request), 'username': self.get_username(request), + 'ip': self.get_request_ip_address(request), } for header_name, context_key in META_KEY_TO_CONTEXT_KEY.iteritems(): context[context_key] = request.META.get(header_name, '') @@ -196,6 +197,14 @@ class TrackMiddleware(object): except AttributeError: return '' + def get_request_ip_address(self, request): + """Gets the IP address of the request""" + ip_address = get_ip(request) + if ip_address is not None: + return ip_address + else: + return '' + def process_response(self, _request, response): """Exit the context if it exists.""" try: diff --git a/common/djangoapps/track/tests/test_middleware.py b/common/djangoapps/track/tests/test_middleware.py index 93be67d911..bcb8d6a817 100644 --- a/common/djangoapps/track/tests/test_middleware.py +++ b/common/djangoapps/track/tests/test_middleware.py @@ -68,6 +68,37 @@ class TrackMiddlewareTestCase(TestCase): 'client_id': None, }) + def test_no_forward_for_header_ip_context(self): + request = self.request_factory.get('/courses/') + remote_addr = '127.0.0.1' + + request.META['REMOTE_ADDR'] = remote_addr + context = self.get_context_for_request(request) + + self.assertEquals(context['ip'], remote_addr) + + def test_single_forward_for_header_ip_context(self): + request = self.request_factory.get('/courses/') + remote_addr = '127.0.0.1' + forwarded_ip = '11.22.33.44' + + request.META['REMOTE_ADDR'] = remote_addr + request.META['HTTP_X_FORWARDED_FOR'] = forwarded_ip + context = self.get_context_for_request(request) + + self.assertEquals(context['ip'], forwarded_ip) + + def test_multiple_forward_for_header_ip_context(self): + request = self.request_factory.get('/courses/') + remote_addr = '127.0.0.1' + forwarded_ip = '11.22.33.44, 10.0.0.1, 127.0.0.1' + + request.META['REMOTE_ADDR'] = remote_addr + request.META['HTTP_X_FORWARDED_FOR'] = forwarded_ip + context = self.get_context_for_request(request) + + self.assertEquals(context['ip'], '11.22.33.44') + def get_context_for_path(self, path): """Extract the generated event tracking context for a given request for the given path.""" request = self.request_factory.get(path) diff --git a/common/djangoapps/track/views/__init__.py b/common/djangoapps/track/views/__init__.py index c9a37963c5..3babc9c9df 100644 --- a/common/djangoapps/track/views/__init__.py +++ b/common/djangoapps/track/views/__init__.py @@ -10,6 +10,7 @@ from django.shortcuts import redirect from django.views.decorators.csrf import ensure_csrf_cookie from edxmako.shortcuts import render_to_response +from ipware.ip import get_ip from track import tracker from track import contexts @@ -31,6 +32,14 @@ def _get_request_header(request, header_name, default=''): return default +def _get_request_ip(request, default=''): + """Helper method to get IP from a request's META dict, if present.""" + if request is not None and hasattr(request, 'META'): + return get_ip(request) + else: + return default + + def _get_request_value(request, value_name, default=''): """Helper method to get header values from a request's REQUEST dict, if present.""" if request is not None and hasattr(request, 'REQUEST') and value_name in request.REQUEST: @@ -89,7 +98,7 @@ def server_track(request, event_type, event, page=None): # define output: event = { "username": username, - "ip": _get_request_header(request, 'REMOTE_ADDR'), + "ip": _get_request_ip(request), "referer": _get_request_header(request, 'HTTP_REFERER'), "accept_language": _get_request_header(request, 'HTTP_ACCEPT_LANGUAGE'), "event_source": "server",