Merge pull request #9981 from edx/bderusha/context-tracker-ip

Upgrade IP parsing in tracker middleware

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:

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)

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",