diff --git a/awx/api/views/__init__.py b/awx/api/views/__init__.py index 4d86b61320..1456e8bb16 100644 --- a/awx/api/views/__init__.py +++ b/awx/api/views/__init__.py @@ -55,6 +55,7 @@ from wsgiref.util import FileWrapper from drf_spectacular.utils import extend_schema_view, extend_schema # django-ansible-base +from ansible_base.lib.utils.requests import get_remote_hosts from ansible_base.rbac.models import RoleEvaluation from ansible_base.lib.utils.schema import extend_schema_if_available @@ -97,7 +98,6 @@ from awx.main.utils import ( from awx.main.utils.encryption import encrypt_value from awx.main.utils.filters import SmartFilter from awx.main.utils.plugins import compute_cloud_inventory_sources -from awx.main.utils.proxy import get_first_remote_host_from_headers from awx.main.utils.common import memoize from awx.main.redact import UriCleaner from awx.api.permissions import ( @@ -2877,8 +2877,7 @@ class JobTemplateCallback(GenericAPIView): host for the current request. """ # Find the list of remote host names/IPs to check. - # Only consider the first entry from each header (for comma-separated values like X-Forwarded-For) - remote_hosts = get_first_remote_host_from_headers(self.request, settings.REMOTE_HOST_HEADERS) + remote_hosts = set(get_remote_hosts(self.request)) # Add the reverse lookup of IP addresses. for rh in list(remote_hosts): try: diff --git a/awx/main/tests/functional/api/test_job_template.py b/awx/main/tests/functional/api/test_job_template.py index ac648b3f39..4cdf43d37b 100644 --- a/awx/main/tests/functional/api/test_job_template.py +++ b/awx/main/tests/functional/api/test_job_template.py @@ -485,47 +485,3 @@ class TestJobTemplateCallbackProxyIntegration: expect=400, **headers ) - - @override_settings(REMOTE_HOST_HEADERS=['HTTP_X_FROM_THE_LOAD_BALANCER', 'REMOTE_ADDR', 'REMOTE_HOST'], PROXY_IP_ALLOWED_LIST=[]) - def test_only_first_entry_in_comma_separated_header_is_considered(self, job_template, admin_user, post): - """ - Test that only the first entry in a comma-separated header value is used for host matching. - This is important for X-Forwarded-For style headers where the format is "client, proxy1, proxy2". - Only the original client (first entry) should be matched against inventory hosts. - """ - # Create host that matches the SECOND entry in the comma-separated list - job_template.inventory.hosts.create(name='second-host.example.com') - - headers = { - # First entry is 'first-host.example.com', second is 'second-host.example.com' - # Only the first should be considered, so this should NOT match - 'HTTP_X_FROM_THE_LOAD_BALANCER': 'first-host.example.com, second-host.example.com', - 'REMOTE_ADDR': 'unrelated-addr', - 'REMOTE_HOST': 'unrelated-host', - } - - # Should return 400 because only 'first-host.example.com' is considered, - # and that host is NOT in the inventory - r = post( - url=reverse('api:job_template_callback', kwargs={'pk': job_template.pk}), data={'host_config_key': 'abcd'}, user=admin_user, expect=400, **headers - ) - assert r.data['msg'] == 'No matching host could be found!' - - @override_settings(REMOTE_HOST_HEADERS=['HTTP_X_FROM_THE_LOAD_BALANCER', 'REMOTE_ADDR', 'REMOTE_HOST'], PROXY_IP_ALLOWED_LIST=[]) - def test_first_entry_in_comma_separated_header_matches(self, job_template, admin_user, post): - """ - Test that the first entry in a comma-separated header value correctly matches an inventory host. - """ - # Create host that matches the FIRST entry in the comma-separated list - job_template.inventory.hosts.create(name='first-host.example.com') - - headers = { - # First entry is 'first-host.example.com', second is 'second-host.example.com' - # The first entry matches the inventory host - 'HTTP_X_FROM_THE_LOAD_BALANCER': 'first-host.example.com, second-host.example.com', - 'REMOTE_ADDR': 'unrelated-addr', - 'REMOTE_HOST': 'unrelated-host', - } - - # Should return 201 because 'first-host.example.com' is the first entry and matches - post(url=reverse('api:job_template_callback', kwargs={'pk': job_template.pk}), data={'host_config_key': 'abcd'}, user=admin_user, expect=201, **headers) diff --git a/awx/main/tests/unit/utils/test_proxy.py b/awx/main/tests/unit/utils/test_proxy.py deleted file mode 100644 index 775e1fed66..0000000000 --- a/awx/main/tests/unit/utils/test_proxy.py +++ /dev/null @@ -1,207 +0,0 @@ -# Copyright (c) 2024 Ansible, Inc. -# All Rights Reserved. - -from unittest import mock - -from awx.main.utils.proxy import get_first_remote_host_from_headers, is_proxy_in_headers - - -class TestGetFirstRemoteHostFromHeaders: - """Tests for get_first_remote_host_from_headers function.""" - - def _make_mock_request(self, environ): - """Create a mock request with the given environ dict.""" - request = mock.MagicMock() - request.environ = environ - return request - - def test_single_value_headers(self): - """Test extraction from headers with single values (no commas).""" - request = self._make_mock_request( - { - "REMOTE_ADDR": "192.168.1.1", - "REMOTE_HOST": "client.example.com", - } - ) - headers = ["REMOTE_ADDR", "REMOTE_HOST"] - - result = get_first_remote_host_from_headers(request, headers) - - assert result == {"192.168.1.1", "client.example.com"} - - def test_comma_separated_only_first_entry(self): - """Test that only the first entry is extracted from comma-separated values.""" - request = self._make_mock_request( - { - "HTTP_X_FORWARDED_FOR": "10.0.0.1, 192.168.1.1, 172.16.0.1", - } - ) - headers = ["HTTP_X_FORWARDED_FOR"] - - result = get_first_remote_host_from_headers(request, headers) - - # Only the first IP should be included - assert result == {"10.0.0.1"} - # Subsequent IPs should NOT be included - assert "192.168.1.1" not in result - assert "172.16.0.1" not in result - - def test_comma_separated_with_whitespace(self): - """Test that whitespace is properly stripped from first entry.""" - request = self._make_mock_request( - { - "HTTP_X_FORWARDED_FOR": " 10.0.0.1 , 192.168.1.1", - } - ) - headers = ["HTTP_X_FORWARDED_FOR"] - - result = get_first_remote_host_from_headers(request, headers) - - assert result == {"10.0.0.1"} - - def test_multiple_headers_with_comma_separated(self): - """Test multiple headers where some have comma-separated values.""" - request = self._make_mock_request( - { - "HTTP_X_FORWARDED_FOR": "client.example.com, proxy1.example.com, proxy2.example.com", - "REMOTE_ADDR": "172.16.0.1", - "REMOTE_HOST": "proxy2.example.com", - } - ) - headers = ["HTTP_X_FORWARDED_FOR", "REMOTE_ADDR", "REMOTE_HOST"] - - result = get_first_remote_host_from_headers(request, headers) - - # Should have first entry from X-Forwarded-For plus the single values from other headers - assert result == {"client.example.com", "172.16.0.1", "proxy2.example.com"} - # Should NOT have subsequent entries from X-Forwarded-For - assert "proxy1.example.com" not in result - - def test_empty_header_value(self): - """Test handling of empty header values.""" - request = self._make_mock_request( - { - "HTTP_X_FORWARDED_FOR": "", - "REMOTE_ADDR": "192.168.1.1", - } - ) - headers = ["HTTP_X_FORWARDED_FOR", "REMOTE_ADDR"] - - result = get_first_remote_host_from_headers(request, headers) - - assert result == {"192.168.1.1"} - - def test_missing_header(self): - """Test handling of headers that don't exist in environ.""" - request = self._make_mock_request( - { - "REMOTE_ADDR": "192.168.1.1", - } - ) - headers = ["HTTP_X_FORWARDED_FOR", "REMOTE_ADDR", "REMOTE_HOST"] - - result = get_first_remote_host_from_headers(request, headers) - - assert result == {"192.168.1.1"} - - def test_empty_headers_list(self): - """Test with no headers specified.""" - request = self._make_mock_request( - { - "REMOTE_ADDR": "192.168.1.1", - } - ) - headers = [] - - result = get_first_remote_host_from_headers(request, headers) - - assert result == set() - - def test_whitespace_only_first_entry(self): - """Test handling when first entry is whitespace only.""" - request = self._make_mock_request( - { - "HTTP_X_FORWARDED_FOR": " , 192.168.1.1", - } - ) - headers = ["HTTP_X_FORWARDED_FOR"] - - result = get_first_remote_host_from_headers(request, headers) - - # Empty/whitespace first entry should be skipped - assert result == set() - - def test_single_entry_with_trailing_comma(self): - """Test single entry that happens to have a trailing comma.""" - request = self._make_mock_request( - { - "HTTP_X_FORWARDED_FOR": "10.0.0.1,", - } - ) - headers = ["HTTP_X_FORWARDED_FOR"] - - result = get_first_remote_host_from_headers(request, headers) - - assert result == {"10.0.0.1"} - - -class TestIsProxyInHeaders: - """Tests for is_proxy_in_headers function.""" - - def _make_mock_request(self, environ): - """Create a mock request with the given environ dict.""" - request = mock.MagicMock() - request.environ = environ - return request - - def test_proxy_found_in_single_value(self): - """Test proxy detection in single-value header.""" - request = self._make_mock_request( - { - "REMOTE_ADDR": "192.168.1.1", - } - ) - - result = is_proxy_in_headers(request, ["192.168.1.1"], ["REMOTE_ADDR"]) - - assert result is True - - def test_proxy_found_in_comma_separated(self): - """Test proxy detection in comma-separated header value.""" - request = self._make_mock_request( - { - "HTTP_X_FORWARDED_FOR": "10.0.0.1, 192.168.1.1, 172.16.0.1", - } - ) - - result = is_proxy_in_headers(request, ["192.168.1.1"], ["HTTP_X_FORWARDED_FOR"]) - - assert result is True - - def test_proxy_not_found(self): - """Test when proxy is not in any header.""" - request = self._make_mock_request( - { - "REMOTE_ADDR": "10.0.0.1", - } - ) - - result = is_proxy_in_headers(request, ["192.168.1.1"], ["REMOTE_ADDR"]) - - assert result is False - - def test_multiple_proxies_one_match(self): - """Test with multiple allowed proxies, one matches.""" - request = self._make_mock_request( - { - "REMOTE_HOST": "proxy.example.com", - } - ) - - result = is_proxy_in_headers( - request, - ["proxy1.example.com", "proxy.example.com", "proxy2.example.com"], - ["REMOTE_HOST"], - ) - - assert result is True diff --git a/awx/main/utils/proxy.py b/awx/main/utils/proxy.py index 89aa605094..0676e8f44a 100644 --- a/awx/main/utils/proxy.py +++ b/awx/main/utils/proxy.py @@ -45,38 +45,3 @@ def delete_headers_starting_with_http(request: Request, headers: list[str]): for header in headers: if header.startswith('HTTP_'): request.environ.pop(header, None) - - -def get_first_remote_host_from_headers(request: Request, headers: list[str]) -> set[str]: - """ - Extract remote host addresses from headers, considering only the first entry - in comma-separated values. - - For headers like X-Forwarded-For that may contain multiple IPs (e.g., "client, proxy1, proxy2"), - only the first entry (the original client) is considered. - - Example: - request.environ = { - "HTTP_X_FORWARDED_FOR": "10.0.0.1, 192.168.1.1, 172.16.0.1", - "REMOTE_ADDR": "192.168.1.1", - "REMOTE_HOST": "proxy.example.com" - } - headers = ["HTTP_X_FORWARDED_FOR", "REMOTE_ADDR", "REMOTE_HOST"] - - Returns: {"10.0.0.1", "192.168.1.1", "proxy.example.com"} - (Only the first IP "10.0.0.1" from X-Forwarded-For, not the full chain) - - request: The DRF/Django request. request.environ dict will be used for extracting hosts - headers: A list of header keys to check for remote host values - """ - remote_hosts = set() - - for header in headers: - header_value = request.environ.get(header, '') - if header_value: - # Only take the first entry if comma-separated - first_value = header_value.split(',')[0].strip() - if first_value: - remote_hosts.add(first_value) - - return remote_hosts