Просмотр исходного кода

Fixes #22346: Return a clean error message & redirect on SSO auth failure (#22420)

Jeremy Stretch 2 недель назад
Родитель
Сommit
8afbfc42d5
3 измененных файлов с 78 добавлено и 1 удалено
  1. 13 0
      netbox/netbox/middleware.py
  2. 8 0
      netbox/netbox/settings.py
  3. 57 1
      netbox/netbox/tests/test_authentication.py

+ 13 - 0
netbox/netbox/middleware.py

@@ -10,7 +10,9 @@ from django.db import ProgrammingError, connection
 from django.db.utils import InternalError
 from django.db.utils import InternalError
 from django.http import Http404, HttpResponseRedirect
 from django.http import Http404, HttpResponseRedirect
 from django.middleware.common import CommonMiddleware as DjangoCommonMiddleware
 from django.middleware.common import CommonMiddleware as DjangoCommonMiddleware
+from django.utils.translation import gettext_lazy as _
 from django_prometheus import middleware
 from django_prometheus import middleware
+from social_django.middleware import SocialAuthExceptionMiddleware as SocialAuthExceptionMiddleware_
 
 
 from netbox.config import clear_config, get_config
 from netbox.config import clear_config, get_config
 from netbox.metrics import Metrics
 from netbox.metrics import Metrics
@@ -26,6 +28,7 @@ __all__ = (
     'PrometheusAfterMiddleware',
     'PrometheusAfterMiddleware',
     'PrometheusBeforeMiddleware',
     'PrometheusBeforeMiddleware',
     'RemoteUserMiddleware',
     'RemoteUserMiddleware',
+    'SocialAuthExceptionMiddleware',
 )
 )
 
 
 
 
@@ -286,3 +289,13 @@ class MaintenanceModeMiddleware:
             messages.error(request, error_message)
             messages.error(request, error_message)
             return HttpResponseRedirect(request.path_info)
             return HttpResponseRedirect(request.path_info)
         return None
         return None
+
+
+class SocialAuthExceptionMiddleware(SocialAuthExceptionMiddleware_):
+    """
+    Subclass of python-social-auth's exception middleware which surfaces a generic, user-friendly
+    message rather than exposing the raw social_core exception text to (typically unauthenticated)
+    users when an SSO/SAML login fails.
+    """
+    def get_message(self, request, exception):
+        return _("Single sign-on failed. Please try again or contact your administrator.")

+ 8 - 0
netbox/netbox/settings.py

@@ -516,6 +516,7 @@ MIDDLEWARE = [
     'netbox.middleware.RemoteUserMiddleware',
     'netbox.middleware.RemoteUserMiddleware',
     'netbox.middleware.CoreMiddleware',
     'netbox.middleware.CoreMiddleware',
     'netbox.middleware.MaintenanceModeMiddleware',
     'netbox.middleware.MaintenanceModeMiddleware',
+    'netbox.middleware.SocialAuthExceptionMiddleware',
 ]
 ]
 
 
 if DEBUG:
 if DEBUG:
@@ -725,6 +726,13 @@ SOCIAL_AUTH_PIPELINE = (
     'social_core.pipeline.user.user_details',
     'social_core.pipeline.user.user_details',
 )
 )
 
 
+# Redirect users back to the login page (surfacing the error via the messages framework) when an
+# SSO/SAML authentication failure occurs, rather than raising an HTTP 500. Full exceptions are still
+# raised when DEBUG is enabled. LOGIN_URL is an absolute path which respects BASE_PATH; the social
+# auth middleware passes this value directly to an HttpResponseRedirect without reversing it.
+SOCIAL_AUTH_LOGIN_ERROR_URL = LOGIN_URL
+SOCIAL_AUTH_RAISE_EXCEPTIONS = DEBUG
+
 # Load all SOCIAL_AUTH_* settings from the user configuration
 # Load all SOCIAL_AUTH_* settings from the user configuration
 for param in dir(configuration):
 for param in dir(configuration):
     if param.startswith('SOCIAL_AUTH_'):
     if param.startswith('SOCIAL_AUTH_'):

+ 57 - 1
netbox/netbox/tests/test_authentication.py

@@ -1,13 +1,16 @@
 import datetime
 import datetime
 
 
 from django.conf import settings
 from django.conf import settings
-from django.test import Client
+from django.contrib.messages.storage.fallback import FallbackStorage
+from django.test import Client, RequestFactory, SimpleTestCase
 from django.test.utils import override_settings
 from django.test.utils import override_settings
 from django.urls import reverse
 from django.urls import reverse
 from rest_framework.test import APIClient
 from rest_framework.test import APIClient
+from social_core.exceptions import AuthFailed
 
 
 from core.models import ObjectType
 from core.models import ObjectType
 from dcim.models import Rack, Site
 from dcim.models import Rack, Site
+from netbox.middleware import SocialAuthExceptionMiddleware
 from users.constants import TOKEN_PREFIX
 from users.constants import TOKEN_PREFIX
 from users.models import Group, ObjectPermission, Token, User
 from users.models import Group, ObjectPermission, Token, User
 from utilities.testing import TestCase
 from utilities.testing import TestCase
@@ -697,3 +700,56 @@ class ObjectPermissionAPIViewTestCase(TestCase):
         url = reverse('dcim-api:rack-detail', kwargs={'pk': self.racks[0].pk})
         url = reverse('dcim-api:rack-detail', kwargs={'pk': self.racks[0].pk})
         response = self.client.delete(url, format='json', **self.header)
         response = self.client.delete(url, format='json', **self.header)
         self.assertEqual(response.status_code, 204)
         self.assertEqual(response.status_code, 204)
+
+
+class SocialAuthExceptionMiddlewareTestCase(SimpleTestCase):
+    """
+    Verify that SSO/SAML authentication failures are surfaced as a login-page message rather than
+    bubbling up as an HTTP 500 (see #22346).
+    """
+    GENERIC_MESSAGE = "Single sign-on failed. Please try again or contact your administrator."
+
+    class FakeStrategy:
+        # Mirror social_core's DjangoStrategy.setting(), which reads SOCIAL_AUTH_<NAME> from Django
+        # settings. This ensures the test exercises the real configured values (e.g.
+        # SOCIAL_AUTH_LOGIN_ERROR_URL) rather than hardcoded stand-ins.
+        def setting(self, name, default=None, backend=None):
+            return getattr(settings, f'SOCIAL_AUTH_{name}', default)
+
+    class FakeBackend:
+        name = 'saml'
+
+    def setUp(self):
+        self.factory = RequestFactory()
+        self.middleware = SocialAuthExceptionMiddleware(lambda request: None)
+
+    def _make_request(self):
+        request = self.factory.get('/')
+        request.social_strategy = self.FakeStrategy()
+        request.backend = self.FakeBackend()
+        # Attach message storage (normally provided by MessageMiddleware)
+        setattr(request, 'session', {})
+        request._messages = FallbackStorage(request)
+        return request
+
+    def test_generic_message(self):
+        """
+        The raw exception text should never be surfaced to the user.
+        """
+        request = self._make_request()
+        exception = AuthFailed(self.FakeBackend(), 'raw internal SAML detail')
+        self.assertEqual(self.middleware.get_message(request, exception), self.GENERIC_MESSAGE)
+
+    def test_redirect_on_failure(self):
+        """
+        A SocialAuthBaseException should redirect to the login page with the generic message set.
+        """
+        request = self._make_request()
+        exception = AuthFailed(self.FakeBackend(), 'raw internal SAML detail')
+        response = self.middleware.process_exception(request, exception)
+
+        self.assertEqual(response.status_code, 302)
+        self.assertEqual(response.url, settings.SOCIAL_AUTH_LOGIN_ERROR_URL)
+        self.assertEqual(response.url, settings.LOGIN_URL)
+        messages = [str(m) for m in request._messages]
+        self.assertEqual(messages, [self.GENERIC_MESSAGE])