From 92c92d60e7d83141d5f9febae848d313638fcf73 Mon Sep 17 00:00:00 2001 From: Matthias Mair Date: Tue, 1 Jul 2025 01:12:48 +0200 Subject: [PATCH] [FR] Add proactive system check for site_url (#9895) * [FR] Add proactive system check for site_url Fixes #7847 * fix enviroment for tests * fix admin tests and make them more robust * improve tests for admin actions * add test for all scenarios * use right http error code * fix error code * remove dependency on bundle * fix test --- docs/docs/settings/error_codes.md | 6 +++ src/backend/InvenTree/InvenTree/middleware.py | 41 ++++++++++++++++- src/backend/InvenTree/InvenTree/settings.py | 1 + .../InvenTree/InvenTree/test_middleware.py | 46 +++++++++++++++++++ src/backend/InvenTree/InvenTree/test_views.py | 7 +++ src/backend/InvenTree/InvenTree/tests.py | 3 ++ src/backend/InvenTree/InvenTree/unit_test.py | 6 ++- .../plugin/samples/integration/test_sample.py | 4 ++ src/backend/InvenTree/plugin/test_api.py | 10 ++++ .../InvenTree/templates/config_error.html | 13 ++++++ 10 files changed, 135 insertions(+), 2 deletions(-) create mode 100644 src/backend/InvenTree/templates/config_error.html diff --git a/docs/docs/settings/error_codes.md b/docs/docs/settings/error_codes.md index b64927df05..8fbaec0ded 100644 --- a/docs/docs/settings/error_codes.md +++ b/docs/docs/settings/error_codes.md @@ -44,6 +44,12 @@ This might be caused by an addition or removal of models to the code base. Runni The scopes used for oAuth permissions have an issue and do not match the rulesets. This might be caused by an addition or removal of models to the code base or changes to the rulesets. Running the test suit should surface more logs with the error code indicating the exact infractions. +#### INVE-E7 +**The host used does not match settings - Backend** + +The settings for SITE_URL and ALLOWED_HOSTS do not match the host used to access the server. This might lead to issues with CSRF protection, CORS and other security features. +The settings must be adjusted. + ### INVE-W (InvenTree Warning) Warnings - These are non-critical errors which should be addressed when possible. diff --git a/src/backend/InvenTree/InvenTree/middleware.py b/src/backend/InvenTree/InvenTree/middleware.py index df14ba3810..336e03aabf 100644 --- a/src/backend/InvenTree/InvenTree/middleware.py +++ b/src/backend/InvenTree/InvenTree/middleware.py @@ -5,7 +5,7 @@ import sys from django.conf import settings from django.contrib.auth.middleware import PersistentRemoteUserMiddleware from django.http import HttpResponse -from django.shortcuts import redirect +from django.shortcuts import redirect, render from django.urls import resolve, reverse_lazy from django.utils.deprecation import MiddlewareMixin @@ -213,3 +213,42 @@ class InvenTreeRequestCacheMiddleware(MiddlewareMixin): """Clear the cache object.""" delete_session_cache() return response + + +class InvenTreeHostSettingsMiddleware(MiddlewareMixin): + """Middleware to check the host settings. + + Especially SITE_URL, trusted_origins. + """ + + def process_request(self, request): + """Check the host settings.""" + # Debug setups do not enforce these checks so we ignore that case + if settings.DEBUG: + return None + + # Handle commonly ignored paths that might also work without a correct setup (api, auth) + path = request.path_info + if path in urls or any(path.startswith(p) for p in paths_ignore): + return None + + # Ensure that the settings are set correctly with the current request + accessed_scheme = request._current_scheme_host + if accessed_scheme and not accessed_scheme.startswith(settings.SITE_URL): + msg = f'INVE-E7: The used path `{accessed_scheme}` does not match the SITE_URL `{settings.SITE_URL}`' + logger.error(msg) + return render( + request, 'config_error.html', {'error_message': msg}, status=500 + ) + + # Check trusted origins + if not any( + accessed_scheme.startswith(origin) + for origin in settings.CSRF_TRUSTED_ORIGINS + ): + msg = f'INVE-E7: The used path `{accessed_scheme}` is not in the TRUSTED_ORIGINS' + logger.error(msg) + return render( + request, 'config_error.html', {'error_message': msg}, status=500 + ) + return None diff --git a/src/backend/InvenTree/InvenTree/settings.py b/src/backend/InvenTree/InvenTree/settings.py index 548b0cb6a5..1465dc1982 100644 --- a/src/backend/InvenTree/InvenTree/settings.py +++ b/src/backend/InvenTree/InvenTree/settings.py @@ -340,6 +340,7 @@ MIDDLEWARE = CONFIG.get( 'maintenance_mode.middleware.MaintenanceModeMiddleware', 'InvenTree.middleware.InvenTreeExceptionProcessor', # Error reporting 'InvenTree.middleware.InvenTreeRequestCacheMiddleware', # Request caching + 'InvenTree.middleware.InvenTreeHostSettingsMiddleware', # Ensuring correct hosting/security settings 'django_structlog.middlewares.RequestMiddleware', # Structured logging ], ) diff --git a/src/backend/InvenTree/InvenTree/test_middleware.py b/src/backend/InvenTree/InvenTree/test_middleware.py index a3952f5d7f..cb334875c2 100644 --- a/src/backend/InvenTree/InvenTree/test_middleware.py +++ b/src/backend/InvenTree/InvenTree/test_middleware.py @@ -87,3 +87,49 @@ class MiddlewareTests(InvenTreeTestCase): except Http404: log_error('testpath') check(1) + + def test_site_url_checks(self): + """Test that the site URL check is correctly working.""" + # correctly set + with self.settings( + SITE_URL='http://testserver', CSRF_TRUSTED_ORIGINS=['http://testserver'] + ): + response = self.client.get(reverse('web')) + self.assertEqual(response.status_code, 200) + self.assertNotContains(response, 'INVE-E7') + self.assertContains(response, 'window.INVENTREE_SETTINGS') + + # wrongly set site URL + with self.settings(SITE_URL='https://example.com'): + response = self.client.get(reverse('web')) + self.assertEqual(response.status_code, 500) + self.assertContains( + response, + 'INVE-E7: The used path `http://testserver` does not match', + status_code=500, + ) + self.assertNotContains( + response, 'window.INVENTREE_SETTINGS', status_code=500 + ) + + # wrongly set but in debug -> is ignored + with self.settings(SITE_URL='https://example.com', DEBUG=True): + response = self.client.get(reverse('web')) + self.assertEqual(response.status_code, 200) + self.assertNotContains(response, 'INVE-E7') + self.assertContains(response, 'window.INVENTREE_SETTINGS') + + # wrongly set cors + with self.settings( + SITE_URL='http://testserver', + CORS_ORIGIN_ALLOW_ALL=False, + CSRF_TRUSTED_ORIGINS=['https://example.com'], + ): + response = self.client.get(reverse('web')) + self.assertEqual(response.status_code, 500) + self.assertContains( + response, 'is not in the TRUSTED_ORIGINS', status_code=500 + ) + self.assertNotContains( + response, 'window.INVENTREE_SETTINGS', status_code=500 + ) diff --git a/src/backend/InvenTree/InvenTree/test_views.py b/src/backend/InvenTree/InvenTree/test_views.py index 73ec672f73..d135a57e5c 100644 --- a/src/backend/InvenTree/InvenTree/test_views.py +++ b/src/backend/InvenTree/InvenTree/test_views.py @@ -2,6 +2,7 @@ import os +from django.test import override_settings from django.urls import reverse from InvenTree.unit_test import InvenTreeTestCase @@ -13,6 +14,9 @@ class ViewTests(InvenTreeTestCase): username = 'test_user' password = 'test_pass' + @override_settings( + SITE_URL='http://testserver', CSRF_TRUSTED_ORIGINS=['http://testserver'] + ) def test_api_doc(self): """Test that the api-doc view works.""" api_url = os.path.join(reverse('index'), 'api-doc') + '/' @@ -20,6 +24,9 @@ class ViewTests(InvenTreeTestCase): response = self.client.get(api_url) self.assertEqual(response.status_code, 200) + @override_settings( + SITE_URL='http://testserver', CSRF_TRUSTED_ORIGINS=['http://testserver'] + ) def test_index_redirect(self): """Top-level URL should redirect to "index" page.""" response = self.client.get('/') diff --git a/src/backend/InvenTree/InvenTree/tests.py b/src/backend/InvenTree/InvenTree/tests.py index 84b21e85e9..1d0c3d5938 100644 --- a/src/backend/InvenTree/InvenTree/tests.py +++ b/src/backend/InvenTree/InvenTree/tests.py @@ -1809,6 +1809,9 @@ class URLCompatibilityTest(InvenTreeTestCase): ('/stock/item/1/', '/web/stock/item/1/'), ] + @override_settings( + SITE_URL='http://testserver', CSRF_TRUSTED_ORIGINS=['http://testserver'] + ) def test_legacy_urls(self): """Test legacy URLs.""" for old_url, new_url in self.URL_MAPPINGS: diff --git a/src/backend/InvenTree/InvenTree/unit_test.py b/src/backend/InvenTree/InvenTree/unit_test.py index 23600e81ca..2836bbf198 100644 --- a/src/backend/InvenTree/InvenTree/unit_test.py +++ b/src/backend/InvenTree/InvenTree/unit_test.py @@ -15,7 +15,7 @@ from django.contrib.auth.models import Group, Permission, User from django.db import connections, models from django.http.response import StreamingHttpResponse from django.test import TestCase -from django.test.utils import CaptureQueriesContext +from django.test.utils import CaptureQueriesContext, override_settings from django.urls import reverse from djmoney.contrib.exchange.models import ExchangeBackend, Rate @@ -659,6 +659,9 @@ class InvenTreeAPITestCase(ExchangeRateMixin, UserMixin, APITestCase): self.assertEqual(b, b | a) +@override_settings( + SITE_URL='http://testserver', CSRF_TRUSTED_ORIGINS=['http://testserver'] +) class AdminTestCase(InvenTreeAPITestCase): """Tests for the admin interface integration.""" @@ -682,6 +685,7 @@ class AdminTestCase(InvenTreeAPITestCase): reverse(f'admin:{app_app}_{app_mdl}_change', kwargs={'object_id': obj.pk}) ) self.assertEqual(response.status_code, 200) + self.assertContains(response, 'Django site admin') return obj diff --git a/src/backend/InvenTree/plugin/samples/integration/test_sample.py b/src/backend/InvenTree/plugin/samples/integration/test_sample.py index 0d7ac58893..1da0bb9800 100644 --- a/src/backend/InvenTree/plugin/samples/integration/test_sample.py +++ b/src/backend/InvenTree/plugin/samples/integration/test_sample.py @@ -1,6 +1,7 @@ """Unit tests for action plugins.""" from django.core.exceptions import ValidationError +from django.test import override_settings from InvenTree.unit_test import InvenTreeTestCase from plugin import registry @@ -9,6 +10,9 @@ from plugin import registry class SampleIntegrationPluginTests(InvenTreeTestCase): """Tests for SampleIntegrationPlugin.""" + @override_settings( + SITE_URL='http://testserver', CSRF_TRUSTED_ORIGINS=['http://testserver'] + ) def test_view(self): """Check the function of the custom sample plugin.""" from common.models import InvenTreeSetting diff --git a/src/backend/InvenTree/plugin/test_api.py b/src/backend/InvenTree/plugin/test_api.py index 0857433112..bb5625d80a 100644 --- a/src/backend/InvenTree/plugin/test_api.py +++ b/src/backend/InvenTree/plugin/test_api.py @@ -1,6 +1,7 @@ """Tests for general API tests for the plugin app.""" from django.conf import settings +from django.test import override_settings from django.urls import reverse from rest_framework.exceptions import NotFound @@ -150,6 +151,9 @@ class PluginDetailAPITest(PluginMixin, InvenTreeAPITestCase): str(response.data['detail']), ) + @override_settings( + SITE_URL='http://testserver', CSRF_TRUSTED_ORIGINS=['http://testserver'] + ) def test_admin_action(self): """Test the PluginConfig action commands.""" url = reverse('admin:plugin_pluginconfig_changelist') @@ -168,6 +172,7 @@ class PluginDetailAPITest(PluginMixin, InvenTreeAPITestCase): follow=True, ) self.assertEqual(response.status_code, 200) + self.assertContains(response, 'Select Plugin Configuration to change') # deactivate plugin - deactivate again -> nothing will happen but the nothing 'changed' function is triggered response = self.client.post( @@ -180,6 +185,7 @@ class PluginDetailAPITest(PluginMixin, InvenTreeAPITestCase): follow=True, ) self.assertEqual(response.status_code, 200) + self.assertContains(response, 'Select Plugin Configuration to change') # activate plugin response = self.client.post( @@ -192,6 +198,7 @@ class PluginDetailAPITest(PluginMixin, InvenTreeAPITestCase): follow=True, ) self.assertEqual(response.status_code, 200) + self.assertContains(response, 'Select Plugin Configuration to change') # save to deactivate a plugin response = self.client.post( @@ -200,6 +207,9 @@ class PluginDetailAPITest(PluginMixin, InvenTreeAPITestCase): follow=True, ) self.assertEqual(response.status_code, 200) + self.assertContains( + response, '(not active) | Change Plugin Configuration | Django site admin' + ) def test_model(self): """Test the PluginConfig model.""" diff --git a/src/backend/InvenTree/templates/config_error.html b/src/backend/InvenTree/templates/config_error.html new file mode 100644 index 0000000000..2765add532 --- /dev/null +++ b/src/backend/InvenTree/templates/config_error.html @@ -0,0 +1,13 @@ +{% extends "base.html" %} +{% load i18n %} +{% load inventree_extras %} + +{% block page_title %} +{% inventree_title %} | {% trans "Configuration Error" %} +{% endblock page_title %} + +{% block content %} +

{% trans "Configuration Error" %}

+

{% blocktrans %}The {{ inventree_title }} server raised a configuration error{% endblocktrans %}

+

{{ error_message }}

+{% endblock content %}