From c3791a7179e433b1934bc6ed7327a63c72f9906c Mon Sep 17 00:00:00 2001 From: bobby-didcoding Date: Wed, 22 Jan 2025 17:18:30 +0000 Subject: [PATCH 01/19] Added new middleware to check the x-forward-for header for approved IPs --- conf/env.py | 2 ++ conf/settings.py | 2 ++ core/middleware.py | 18 +++++++++++++++ core/tests/test_middleware.py | 42 +++++++++++++++++++++++++++++++++++ 4 files changed, 64 insertions(+) diff --git a/conf/env.py b/conf/env.py index 250b6ac3..af607c61 100644 --- a/conf/env.py +++ b/conf/env.py @@ -28,6 +28,8 @@ class BaseSettings(PydanticBaseSettings): sentry_enable_tracing: bool = False sentry_traces_sample_rate: float = 1.0 + safelist_ips: str = '' + feature_enforce_staff_sso_enabled: bool = False staff_sso_authbroker_url: str diff --git a/conf/settings.py b/conf/settings.py index 77f59481..a5c02c1b 100644 --- a/conf/settings.py +++ b/conf/settings.py @@ -29,6 +29,8 @@ # PaaS, we can open ALLOWED_HOSTS ALLOWED_HOSTS = ['*'] +SAFELIST_IPS = [host.strip() for host in env.safelist_ips.split(',')] + INSTALLED_APPS = [ 'django.contrib.auth', diff --git a/core/middleware.py b/core/middleware.py index 377a7b1b..e836c30f 100644 --- a/core/middleware.py +++ b/core/middleware.py @@ -1,3 +1,4 @@ +from dbt_copilot_python.utility import is_copilot from django.conf import settings from django.http import HttpResponse from django.utils.deprecation import MiddlewareMixin @@ -37,3 +38,20 @@ def process_view(self, request, view_func, view_args, view_kwarg): if self.is_admin_name_space(request) or request.path_info.startswith('/admin/login'): if not request.user.is_staff: return HttpResponse(self.SSO_UNAUTHORISED_ACCESS_MESSAGE, status=401) + + +class XForwardForCheckMiddleware(MiddlewareMixin): + CLIENT_IP_ERROR_MESSAGE = 'X Forward For checks failed' + + def process_request(self, request): + if is_copilot(): + # 200 response if client IP in x-forwarded-for header from DBT platform + try: + client_ip = request.META['HTTP_X_FORWARDED_FOR'].split(',')[0] + if client_ip not in settings.SAFELIST_IPS: + return HttpResponse(self.CLIENT_IP_ERROR_MESSAGE, status=401) + except IndexError: + # Return forbidden if the x-forwarded-for header does not have 0 index + return HttpResponse(self.CLIENT_IP_ERROR_MESSAGE, status=401) + except KeyError: + pass diff --git a/core/tests/test_middleware.py b/core/tests/test_middleware.py index 73c8cfc4..78a6b2c2 100644 --- a/core/tests/test_middleware.py +++ b/core/tests/test_middleware.py @@ -1,4 +1,7 @@ +import os + import pytest +from dbt_copilot_python.utility import is_copilot from django.http import HttpResponse from django.test.client import Client from django.urls import reverse @@ -80,3 +83,42 @@ def test_admin_permission_middleware_authorised_with_staff(client, settings, adm response = client.get(reverse('admin:login')) assert response.status_code == 302 + + +@pytest.mark.django_db +def test_x_forward_for_middleware_with_expected_ip(client, settings): + os.environ["COPILOT_ENVIRONMENT_NAME"] = "dev" + settings.SAFELIST_IPS = [ + '1.2.3.4', + ] + reload_urlconf() + + # Middleware is for DBT only and should only trigger is is_copilot() is true + assert is_copilot() is True + + response = client.get( + reverse('pingdom'), + content_type='', + HTTP_X_FORWARDED_FOR='1.2.3.4, 123.123.123.123', + ) + + assert response.status_code == 200 + + +def test_x_forward_for_middleware_with_unexpected_ip(client, settings): + os.environ["COPILOT_ENVIRONMENT_NAME"] = "dev" + settings.SAFELIST_IPS = [ + '1.2.3.4.5', + ] + reload_urlconf() + + # Middleware is for DBT only and should only trigger is is_copilot() is true + assert is_copilot() is True + + response = client.get( + reverse('pingdom'), + content_type='', + HTTP_X_FORWARDED_FOR='1.2.3.4, 123.123.123.123', + ) + + assert response.status_code == 401 From 540cfa352fe081143d1f9a9a71fd7214efcd1856 Mon Sep 17 00:00:00 2001 From: Bobby Stearman <80459294+bobby-didcoding@users.noreply.github.com> Date: Thu, 23 Jan 2025 09:56:59 +0000 Subject: [PATCH 02/19] Added test decorator - copy error --- core/tests/test_middleware.py | 1 + 1 file changed, 1 insertion(+) diff --git a/core/tests/test_middleware.py b/core/tests/test_middleware.py index 78a6b2c2..dd789b6e 100644 --- a/core/tests/test_middleware.py +++ b/core/tests/test_middleware.py @@ -105,6 +105,7 @@ def test_x_forward_for_middleware_with_expected_ip(client, settings): assert response.status_code == 200 +@pytest.mark.django_db def test_x_forward_for_middleware_with_unexpected_ip(client, settings): os.environ["COPILOT_ENVIRONMENT_NAME"] = "dev" settings.SAFELIST_IPS = [ From bb2b8ee01680357d4dfd63b1c2ce39328b4f29de Mon Sep 17 00:00:00 2001 From: Bobby Stearman <80459294+bobby-didcoding@users.noreply.github.com> Date: Thu, 23 Jan 2025 10:30:04 +0000 Subject: [PATCH 03/19] Added middleware to settings --- conf/settings.py | 1 + 1 file changed, 1 insertion(+) diff --git a/conf/settings.py b/conf/settings.py index a5c02c1b..05a42c9e 100644 --- a/conf/settings.py +++ b/conf/settings.py @@ -73,6 +73,7 @@ 'django.middleware.cache.UpdateCacheMiddleware', 'directory_components.middleware.MaintenanceModeMiddleware', 'core.middleware.SSODisplayLoggedInCookieMiddleware', + 'core.middleware.XForwardForCheckMiddleware', 'django.middleware.security.SecurityMiddleware', 'whitenoise.middleware.WhiteNoiseMiddleware', 'conf.signature.SignatureCheckMiddleware', From 6e779a9aa00d296d1024b0773c5c69b09cd9706b Mon Sep 17 00:00:00 2001 From: Bobby Stearman <80459294+bobby-didcoding@users.noreply.github.com> Date: Thu, 23 Jan 2025 11:43:08 +0000 Subject: [PATCH 04/19] new x-forward-for testing to check all IPs --- core/tests/test_middleware.py | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/core/tests/test_middleware.py b/core/tests/test_middleware.py index dd789b6e..0597a540 100644 --- a/core/tests/test_middleware.py +++ b/core/tests/test_middleware.py @@ -88,8 +88,8 @@ def test_admin_permission_middleware_authorised_with_staff(client, settings, adm @pytest.mark.django_db def test_x_forward_for_middleware_with_expected_ip(client, settings): os.environ["COPILOT_ENVIRONMENT_NAME"] = "dev" - settings.SAFELIST_IPS = [ - '1.2.3.4', + settings.ALLOWED_IPS = [ + '1.2.3.4', '123.123.123.123' ] reload_urlconf() @@ -104,12 +104,11 @@ def test_x_forward_for_middleware_with_expected_ip(client, settings): assert response.status_code == 200 - @pytest.mark.django_db def test_x_forward_for_middleware_with_unexpected_ip(client, settings): os.environ["COPILOT_ENVIRONMENT_NAME"] = "dev" - settings.SAFELIST_IPS = [ - '1.2.3.4.5', + settings.ALLOWED_IPS = [ + '0.0.0.0', ] reload_urlconf() From 02e9e94d7a0cb992d61084ec34a4e37cceca4a7a Mon Sep 17 00:00:00 2001 From: Bobby Stearman <80459294+bobby-didcoding@users.noreply.github.com> Date: Thu, 23 Jan 2025 11:44:10 +0000 Subject: [PATCH 05/19] New middleware to loop through all IPs --- core/middleware.py | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/core/middleware.py b/core/middleware.py index e836c30f..cb7c9085 100644 --- a/core/middleware.py +++ b/core/middleware.py @@ -47,11 +47,10 @@ def process_request(self, request): if is_copilot(): # 200 response if client IP in x-forwarded-for header from DBT platform try: - client_ip = request.META['HTTP_X_FORWARDED_FOR'].split(',')[0] - if client_ip not in settings.SAFELIST_IPS: - return HttpResponse(self.CLIENT_IP_ERROR_MESSAGE, status=401) + client_ips = request.META['HTTP_X_FORWARDED_FOR'].split(',') + for ip in client_ips: + if ip.strip() not in settings.ALLOWED_IPS: + return HttpResponse(self.CLIENT_IP_ERROR_MESSAGE, status=401) except IndexError: # Return forbidden if the x-forwarded-for header does not have 0 index return HttpResponse(self.CLIENT_IP_ERROR_MESSAGE, status=401) - except KeyError: - pass From 6b0a1216466e12e765d5a758950e1c6d2bdc561d Mon Sep 17 00:00:00 2001 From: Bobby Stearman <80459294+bobby-didcoding@users.noreply.github.com> Date: Thu, 23 Jan 2025 11:44:52 +0000 Subject: [PATCH 06/19] updated setting name --- conf/settings.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/conf/settings.py b/conf/settings.py index 05a42c9e..51694625 100644 --- a/conf/settings.py +++ b/conf/settings.py @@ -29,7 +29,7 @@ # PaaS, we can open ALLOWED_HOSTS ALLOWED_HOSTS = ['*'] -SAFELIST_IPS = [host.strip() for host in env.safelist_ips.split(',')] +ALLOWED_IPS = [host.strip() for host in env.allowed_ips.split(',')] INSTALLED_APPS = [ From a23a79887e11248ecf04039be4287596474c5e10 Mon Sep 17 00:00:00 2001 From: Bobby Stearman <80459294+bobby-didcoding@users.noreply.github.com> Date: Thu, 23 Jan 2025 11:45:38 +0000 Subject: [PATCH 07/19] changed setting name --- conf/env.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/conf/env.py b/conf/env.py index af607c61..249baff8 100644 --- a/conf/env.py +++ b/conf/env.py @@ -28,7 +28,7 @@ class BaseSettings(PydanticBaseSettings): sentry_enable_tracing: bool = False sentry_traces_sample_rate: float = 1.0 - safelist_ips: str = '' + allowed_ips: str = '' feature_enforce_staff_sso_enabled: bool = False From 0726046834d058c1757ca68764489f75d7bc9899 Mon Sep 17 00:00:00 2001 From: Bobby Stearman <80459294+bobby-didcoding@users.noreply.github.com> Date: Thu, 23 Jan 2025 12:08:15 +0000 Subject: [PATCH 08/19] formatting --- core/tests/test_middleware.py | 1 + 1 file changed, 1 insertion(+) diff --git a/core/tests/test_middleware.py b/core/tests/test_middleware.py index 0597a540..12bc8f54 100644 --- a/core/tests/test_middleware.py +++ b/core/tests/test_middleware.py @@ -104,6 +104,7 @@ def test_x_forward_for_middleware_with_expected_ip(client, settings): assert response.status_code == 200 + @pytest.mark.django_db def test_x_forward_for_middleware_with_unexpected_ip(client, settings): os.environ["COPILOT_ENVIRONMENT_NAME"] = "dev" From 9abd34d6d34743d2057b072fff77ed48e8914dcd Mon Sep 17 00:00:00 2001 From: Bobby Stearman <80459294+bobby-didcoding@users.noreply.github.com> Date: Thu, 23 Jan 2025 12:11:53 +0000 Subject: [PATCH 09/19] Formatting --- core/tests/test_middleware.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/core/tests/test_middleware.py b/core/tests/test_middleware.py index 12bc8f54..b9143ba2 100644 --- a/core/tests/test_middleware.py +++ b/core/tests/test_middleware.py @@ -88,9 +88,7 @@ def test_admin_permission_middleware_authorised_with_staff(client, settings, adm @pytest.mark.django_db def test_x_forward_for_middleware_with_expected_ip(client, settings): os.environ["COPILOT_ENVIRONMENT_NAME"] = "dev" - settings.ALLOWED_IPS = [ - '1.2.3.4', '123.123.123.123' - ] + settings.ALLOWED_IPS = ['1.2.3.4', '123.123.123.123'] reload_urlconf() # Middleware is for DBT only and should only trigger is is_copilot() is true From 0e72a0aa149876ec06a08c6f94977359d3123377 Mon Sep 17 00:00:00 2001 From: Bobby Stearman <80459294+bobby-didcoding@users.noreply.github.com> Date: Thu, 23 Jan 2025 12:51:17 +0000 Subject: [PATCH 10/19] Re added KeyError exception --- core/middleware.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/core/middleware.py b/core/middleware.py index cb7c9085..a74e4199 100644 --- a/core/middleware.py +++ b/core/middleware.py @@ -54,3 +54,6 @@ def process_request(self, request): except IndexError: # Return forbidden if the x-forwarded-for header does not have 0 index return HttpResponse(self.CLIENT_IP_ERROR_MESSAGE, status=401) + except KeyError: + pass + From 68b9bb3e4bacb2e89fead65bf6a6d8f261c04c4d Mon Sep 17 00:00:00 2001 From: Bobby Stearman <80459294+bobby-didcoding@users.noreply.github.com> Date: Thu, 23 Jan 2025 12:56:05 +0000 Subject: [PATCH 11/19] reformatted --- core/middleware.py | 1 - 1 file changed, 1 deletion(-) diff --git a/core/middleware.py b/core/middleware.py index a74e4199..e1194b82 100644 --- a/core/middleware.py +++ b/core/middleware.py @@ -56,4 +56,3 @@ def process_request(self, request): return HttpResponse(self.CLIENT_IP_ERROR_MESSAGE, status=401) except KeyError: pass - From f09f8f1901d8a245c0ed2f5ae48747b658957c54 Mon Sep 17 00:00:00 2001 From: Bobby Stearman <80459294+bobby-didcoding@users.noreply.github.com> Date: Thu, 23 Jan 2025 14:10:07 +0000 Subject: [PATCH 12/19] removing COPILOT setting from os --- core/tests/test_middleware.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/core/tests/test_middleware.py b/core/tests/test_middleware.py index b9143ba2..4bdcbfc5 100644 --- a/core/tests/test_middleware.py +++ b/core/tests/test_middleware.py @@ -101,6 +101,7 @@ def test_x_forward_for_middleware_with_expected_ip(client, settings): ) assert response.status_code == 200 + os.environ.pop("COPILOT_ENVIRONMENT_NAME") @pytest.mark.django_db @@ -121,3 +122,5 @@ def test_x_forward_for_middleware_with_unexpected_ip(client, settings): ) assert response.status_code == 401 + os.environ.pop("COPILOT_ENVIRONMENT_NAME") + From 3c5a0d8a5af737d0b79c192670d89143e1b3eb5f Mon Sep 17 00:00:00 2001 From: Bobby Stearman <80459294+bobby-didcoding@users.noreply.github.com> Date: Thu, 23 Jan 2025 14:18:59 +0000 Subject: [PATCH 13/19] Formatted --- core/tests/test_middleware.py | 1 - 1 file changed, 1 deletion(-) diff --git a/core/tests/test_middleware.py b/core/tests/test_middleware.py index 4bdcbfc5..e5d4fb9d 100644 --- a/core/tests/test_middleware.py +++ b/core/tests/test_middleware.py @@ -123,4 +123,3 @@ def test_x_forward_for_middleware_with_unexpected_ip(client, settings): assert response.status_code == 401 os.environ.pop("COPILOT_ENVIRONMENT_NAME") - From 194838bd277f099dc7f8603371bcd25bb757224d Mon Sep 17 00:00:00 2001 From: Bobby Stearman <80459294+bobby-didcoding@users.noreply.github.com> Date: Thu, 23 Jan 2025 15:57:51 +0000 Subject: [PATCH 14/19] removed unused docstring and simplified middleware --- core/middleware.py | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/core/middleware.py b/core/middleware.py index e1194b82..bfc2a4e8 100644 --- a/core/middleware.py +++ b/core/middleware.py @@ -45,14 +45,11 @@ class XForwardForCheckMiddleware(MiddlewareMixin): def process_request(self, request): if is_copilot(): - # 200 response if client IP in x-forwarded-for header from DBT platform + # 200 response if client IP in x-forwarded-for header from DBT platform else 401 try: client_ips = request.META['HTTP_X_FORWARDED_FOR'].split(',') for ip in client_ips: if ip.strip() not in settings.ALLOWED_IPS: return HttpResponse(self.CLIENT_IP_ERROR_MESSAGE, status=401) - except IndexError: - # Return forbidden if the x-forwarded-for header does not have 0 index + except (IndexError, KeyError): return HttpResponse(self.CLIENT_IP_ERROR_MESSAGE, status=401) - except KeyError: - pass From e3530564a61a9252cbff92903e0fb2088bebd770 Mon Sep 17 00:00:00 2001 From: Bobby Stearman <80459294+bobby-didcoding@users.noreply.github.com> Date: Thu, 23 Jan 2025 15:59:17 +0000 Subject: [PATCH 15/19] Better doc string in middleware --- core/middleware.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/middleware.py b/core/middleware.py index bfc2a4e8..9c8a8559 100644 --- a/core/middleware.py +++ b/core/middleware.py @@ -45,7 +45,7 @@ class XForwardForCheckMiddleware(MiddlewareMixin): def process_request(self, request): if is_copilot(): - # 200 response if client IP in x-forwarded-for header from DBT platform else 401 + # 200 response if client IP from x-forwarded-for header in ALLOWED_IPS, else 401. try: client_ips = request.META['HTTP_X_FORWARDED_FOR'].split(',') for ip in client_ips: From 2f9b315a81abe8f985e45cc15ed25cf593f50862 Mon Sep 17 00:00:00 2001 From: Bobby Stearman <80459294+bobby-didcoding@users.noreply.github.com> Date: Thu, 23 Jan 2025 16:14:44 +0000 Subject: [PATCH 16/19] Removed unused except statement --- core/middleware.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/middleware.py b/core/middleware.py index 9c8a8559..4749051d 100644 --- a/core/middleware.py +++ b/core/middleware.py @@ -51,5 +51,5 @@ def process_request(self, request): for ip in client_ips: if ip.strip() not in settings.ALLOWED_IPS: return HttpResponse(self.CLIENT_IP_ERROR_MESSAGE, status=401) - except (IndexError, KeyError): + except KeyError: return HttpResponse(self.CLIENT_IP_ERROR_MESSAGE, status=401) From b36350c09ea558d93a966b4825e52eea15a023fa Mon Sep 17 00:00:00 2001 From: Bobby Stearman <80459294+bobby-didcoding@users.noreply.github.com> Date: Thu, 23 Jan 2025 16:24:43 +0000 Subject: [PATCH 17/19] Return env to is_copilot = False --- core/tests/test_middleware.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/core/tests/test_middleware.py b/core/tests/test_middleware.py index e5d4fb9d..2abfcb4e 100644 --- a/core/tests/test_middleware.py +++ b/core/tests/test_middleware.py @@ -102,6 +102,7 @@ def test_x_forward_for_middleware_with_expected_ip(client, settings): assert response.status_code == 200 os.environ.pop("COPILOT_ENVIRONMENT_NAME") + reload_urlconf() @pytest.mark.django_db @@ -123,3 +124,4 @@ def test_x_forward_for_middleware_with_unexpected_ip(client, settings): assert response.status_code == 401 os.environ.pop("COPILOT_ENVIRONMENT_NAME") + reload_urlconf() From 56d5dee8b64dcbef9a5194b89ef0bb1184164cdf Mon Sep 17 00:00:00 2001 From: Bobby Stearman <80459294+bobby-didcoding@users.noreply.github.com> Date: Thu, 23 Jan 2025 16:32:46 +0000 Subject: [PATCH 18/19] Removed KeyError exception --- core/middleware.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/middleware.py b/core/middleware.py index 4749051d..e51db5be 100644 --- a/core/middleware.py +++ b/core/middleware.py @@ -52,4 +52,4 @@ def process_request(self, request): if ip.strip() not in settings.ALLOWED_IPS: return HttpResponse(self.CLIENT_IP_ERROR_MESSAGE, status=401) except KeyError: - return HttpResponse(self.CLIENT_IP_ERROR_MESSAGE, status=401) + pass From 0e15a47956fe67e5ca4903c0ed25d824f4ed3c28 Mon Sep 17 00:00:00 2001 From: Bobby Stearman <80459294+bobby-didcoding@users.noreply.github.com> Date: Thu, 23 Jan 2025 16:33:13 +0000 Subject: [PATCH 19/19] Update test_middleware.py --- core/tests/test_middleware.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/core/tests/test_middleware.py b/core/tests/test_middleware.py index 2abfcb4e..e5d4fb9d 100644 --- a/core/tests/test_middleware.py +++ b/core/tests/test_middleware.py @@ -102,7 +102,6 @@ def test_x_forward_for_middleware_with_expected_ip(client, settings): assert response.status_code == 200 os.environ.pop("COPILOT_ENVIRONMENT_NAME") - reload_urlconf() @pytest.mark.django_db @@ -124,4 +123,3 @@ def test_x_forward_for_middleware_with_unexpected_ip(client, settings): assert response.status_code == 401 os.environ.pop("COPILOT_ENVIRONMENT_NAME") - reload_urlconf()