From c3bc68abc1641e45c45d5160c12c0107fb055ba0 Mon Sep 17 00:00:00 2001 From: Tim McCormack Date: Fri, 18 Mar 2022 15:31:27 +0000 Subject: [PATCH] feat: Add monitoring for X-Forwarded-For header length (#30090) --- .../core/lib/x_forwarded_for/middleware.py | 7 ++ .../lib/x_forwarded_for/tests/__init__.py | 0 .../x_forwarded_for/tests/test_middleware.py | 75 +++++++++++++++++++ 3 files changed, 82 insertions(+) create mode 100644 openedx/core/lib/x_forwarded_for/tests/__init__.py create mode 100644 openedx/core/lib/x_forwarded_for/tests/test_middleware.py diff --git a/openedx/core/lib/x_forwarded_for/middleware.py b/openedx/core/lib/x_forwarded_for/middleware.py index 98a8bf7bf9..7d4447a8af 100644 --- a/openedx/core/lib/x_forwarded_for/middleware.py +++ b/openedx/core/lib/x_forwarded_for/middleware.py @@ -3,7 +3,9 @@ Middleware to use the X-Forwarded-For header as the request IP. Updated the libray to use HTTP_HOST and X-Forwarded-Port as SERVER_NAME and SERVER_PORT. """ + from django.utils.deprecation import MiddlewareMixin +from edx_django_utils.monitoring import set_custom_attribute class XForwardedForMiddleware(MiddlewareMixin): @@ -19,6 +21,11 @@ class XForwardedForMiddleware(MiddlewareMixin): Process the given request, update the value of REMOTE_ADDR, SERVER_NAME and SERVER_PORT based on X-Forwarded-For, HTTP_HOST and X-Forwarded-Port headers """ + # Give some observability into X-Forwarded-For length. Useful + # for monitoring in case of unexpected changes, etc. + xff = request.META.get('HTTP_X_FORWARDED_FOR') + xff_len = xff.count(',') + 1 if xff else 0 + set_custom_attribute('header.x-forwarded-for.count', xff_len) for field, header in [("HTTP_X_FORWARDED_FOR", "REMOTE_ADDR"), ("HTTP_HOST", "SERVER_NAME"), ("HTTP_X_FORWARDED_PORT", "SERVER_PORT")]: diff --git a/openedx/core/lib/x_forwarded_for/tests/__init__.py b/openedx/core/lib/x_forwarded_for/tests/__init__.py new file mode 100644 index 0000000000..e69de29bb2 diff --git a/openedx/core/lib/x_forwarded_for/tests/test_middleware.py b/openedx/core/lib/x_forwarded_for/tests/test_middleware.py new file mode 100644 index 0000000000..52d1489eb6 --- /dev/null +++ b/openedx/core/lib/x_forwarded_for/tests/test_middleware.py @@ -0,0 +1,75 @@ +""" +Tests for XForwardedForMiddleware. +""" + +from unittest.mock import call, patch + +import ddt +from django.test import TestCase +from django.test.client import RequestFactory + +from openedx.core.lib.x_forwarded_for.middleware import XForwardedForMiddleware + + +@ddt.ddt +class TestXForwardedForMiddleware(TestCase): + """Tests for middleware's overrides.""" + + @ddt.unpack + @ddt.data( + # With no added headers, just see the test server's defaults. + ( + {}, + { + 'SERVER_NAME': 'testserver', + 'SERVER_PORT': '80', + 'REMOTE_ADDR': '127.0.0.1', + }, + ), + # With headers supplied by the request (Host) and a proxy + # (X-Forwarded-Port), see the name and port overridden. + ( + { + 'HTTP_HOST': 'example.com', + 'HTTP_X_FORWARDED_PORT': '443', + }, + { + 'SERVER_NAME': 'example.com', + 'SERVER_PORT': '443', + }, + ), + + # REMOTE_ADDR can also be overridden + ( + {'HTTP_X_FORWARDED_FOR': '7.8.9.0, 1.2.3.4'}, + { + 'REMOTE_ADDR': '7.8.9.0', + }, + ), + ) + def test_overrides(self, add_meta, expected_meta_include): + """ + Test that parts of request.META can be overridden by HTTP headers. + """ + request = RequestFactory().get('/somewhere') + request.META.update(add_meta) + + XForwardedForMiddleware().process_request(request) + + assert request.META.items() >= expected_meta_include.items() + + @ddt.unpack + @ddt.data( + (None, 0), + ('1.2.3.4', 1), + ('7.8.9.0, 1.2.3.4, 5.5.5.5', 3), + ) + @patch("openedx.core.lib.x_forwarded_for.middleware.set_custom_attribute") + def test_xff_metrics(self, xff, expected_count, mock_set_custom_attribute): + request = RequestFactory().get('/somewhere') + if xff is not None: + request.META['HTTP_X_FORWARDED_FOR'] = xff + + XForwardedForMiddleware().process_request(request) + + mock_set_custom_attribute.assert_has_calls([call('header.x-forwarded-for.count', expected_count)])