core: return the intended status code from error views for all request methods

authentik/core/tests/test_error_views.py

@@ -0,0 +1,69 @@
+"""Test the views wired up as django's handler400/403/404/500"""
+
+from django.test import RequestFactory, TestCase
+
+from authentik.core.tests.utils import create_test_brand
+from authentik.core.views.error import (
+    BadRequestView,
+    ForbiddenView,
+    NotFoundView,
+    ServerErrorView,
+)
+
+HANDLERS = (
+    (BadRequestView, 400),
+    (ForbiddenView, 403),
+    (NotFoundView, 404),
+    (ServerErrorView, 500),
+)
+UNSAFE_METHODS = ("post", "put", "patch", "delete")
+
+
+class TestErrorViews(TestCase):
+    """The error handlers must answer with their own status code for every request
+    method, not just GET.
+
+    Django calls handler400/403/404/500 with the request that failed, whatever its
+    method. A handler that only implements `get` answers a failing POST with 405
+    Method Not Allowed and drops its own status code -- which made an error on a
+    POST-only API endpoint surface to clients as `405`."""
+
+    def setUp(self):
+        self.factory = RequestFactory()
+
+    def test_get(self):
+        """A GET keeps returning the handler's own status code"""
+        for view, expected in HANDLERS:
+            with self.subTest(view=view.__name__):
+                response = view.as_view()(self.factory.get("/"))
+                self.assertEqual(response.status_code, expected)
+
+    def test_unsafe_methods(self):
+        """A POST/PUT/PATCH/DELETE must not be answered with 405"""
+        for view, expected in HANDLERS:
+            for method in UNSAFE_METHODS:
+                with self.subTest(view=view.__name__, method=method):
+                    request = getattr(self.factory, method)("/")
+                    response = view.as_view()(request)
+                    self.assertEqual(response.status_code, expected)
+                    self.assertNotIn("Allow", response)
+
+
+class TestErrorViewsRouting(TestCase):
+    """End-to-end: an unresolvable path must 404 regardless of request method.
+
+    The DRF router matches a detail route's id with `[^/.]+`, so an id containing a
+    dot never resolves and django raises Http404 before any view runs -- the
+    "invalid ID" case that reported 405 rather than a descriptive error."""
+
+    def setUp(self):
+        create_test_brand()
+
+    def test_unresolvable_api_path(self):
+        """Every method gets 404, and the error page still renders"""
+        url = "/api/v3/providers/scim/1.5/sync/object/"
+        for method in ("get", *UNSAFE_METHODS):
+            with self.subTest(method=method):
+                response = getattr(self.client, method)(url)
+                self.assertEqual(response.status_code, 404)
+                self.assertTemplateUsed(response, "if/error.html")

authentik/core/views/error.py

@@ -26,41 +26,50 @@ class ServerErrorTemplateResponse(TemplateResponse, HttpResponseServerError):
     """Combine Template response with Http Code 500"""
 
 
-class BadRequestView(TemplateView):
+class ErrorView(TemplateView):
+    """Base for the views wired up as django's handler400/403/404/500.
+
+    Django invokes an error handler with the request that failed, whatever its
+    method. A plain TemplateView only implements `get`, so its `dispatch` answers
+    a failing POST/PUT/PATCH/DELETE with 405 Method Not Allowed and discards the
+    handler's own status code -- which is how an API error surfaced as a 405 with
+    an `Allow: GET, HEAD, OPTIONS` header. Render the page for every method so the
+    status code the handler stands for is the one the client sees.
+    """
+
+    template_name = "if/error.html"
+
+    def dispatch(self, request, *args, **kwargs):
+        return self.get(request, *args, **kwargs)
+
+
+class BadRequestView(ErrorView):
     """Show Bad Request message"""
 
     extra_context = {"title": "Bad Request"}
 
     response_class = BadRequestTemplateResponse
-    template_name = "if/error.html"
 
 
-class ForbiddenView(TemplateView):
+class ForbiddenView(ErrorView):
     """Show Forbidden message"""
 
     extra_context = {"title": "Forbidden"}
 
     response_class = ForbiddenTemplateResponse
-    template_name = "if/error.html"
 
 
-class NotFoundView(TemplateView):
+class NotFoundView(ErrorView):
     """Show Not Found message"""
 
     extra_context = {"title": "Not Found"}
 
     response_class = NotFoundTemplateResponse
-    template_name = "if/error.html"
 
 
-class ServerErrorView(TemplateView):
+class ServerErrorView(ErrorView):
     """Show Server Error message"""
 
     extra_context = {"title": "Server Error"}
 
     response_class = ServerErrorTemplateResponse
-    template_name = "if/error.html"
-
-    def dispatch(self, *args, **kwargs):  # pragma: no cover
-        """Little wrapper so django accepts this function"""
-        return super().dispatch(*args, **kwargs)

authentik/lib/sync/outgoing/api.py

@@ -1,6 +1,6 @@
 from django.db.models import Model
 from dramatiq.actor import Actor
-from dramatiq.results.errors import ResultFailure
+from dramatiq.results.errors import ResultError
 from drf_spectacular.utils import extend_schema
 from rest_framework.decorators import action
 from rest_framework.fields import BooleanField, CharField, ChoiceField
@@ -119,7 +119,7 @@ def sync_object(self, request: Request, body: SyncObjectSerializer, pk: int) ->
         )
         try:
             msg.get_result(block=True)
-        except ResultFailure:
+        except ResultError:
             pass
         task: Task = msg.options["task"]
         task.refresh_from_db()