Skip to content

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

Merged
BeryJu merged 2 commits into
goauthentik:mainfrom
Souptik96:fix/24230-error-handler-method-not-allowed
Aug 10, 2026
Merged

core: return the intended status code from error views for all request methods#24902
BeryJu merged 2 commits into
goauthentik:mainfrom
Souptik96:fix/24230-error-handler-method-not-allowed

Conversation

@Souptik96

Copy link
Copy Markdown
Contributor

Details

What does this PR change?

The four views wired up as django's handler400/403/404/500 in authentik/root/urls.py
subclassed TemplateView directly. TemplateView only implements get, so its
dispatch() answers a failing POST/PUT/PATCH/DELETE with 405 Method Not Allowed and an Allow: GET, HEAD, OPTIONS header, discarding the status code the
handler stands for.

  • Adds a shared ErrorView base in authentik/core/views/error.py that renders the
    error page for every request method, so BadRequestView/ForbiddenView/
    NotFoundView/ServerErrorView return 400/403/404/500 respectively regardless of
    method. This also removes the duplicated template_name and the
    ServerErrorView.dispatch # pragma: no cover wrapper it made redundant.
  • Widens the dramatiq result handling in OutgoingSyncProviderStatusMixin.sync_object
    (authentik/lib/sync/outgoing/api.py) from ResultFailure to ResultError, so
    ResultTimeout/ResultMissing — a task that is queued but has not reported yet —
    returns the task's messages instead of escaping as an unhandled 500.
  • Adds authentik/core/tests/test_error_views.py covering both the handler status
    codes per method and the end-to-end unresolvable-API-path case.

Why is this change needed?

Reported in #24230: POST /api/v3/providers/scim/{id}/sync/object/ with an invalid ID
returns 405 Method Not Allowed instead of a descriptive error. The endpoint is not
actually at fault — it is the error handlers.

Two things combine:

  1. The DRF router matches a detail route's id with (?P<pk>[^/.]+), so an id
    containing a dot never resolves and django raises Http404 at the resolver, before
    any view runs. handler404 then answers the POST with 405 rather than 404.
  2. If the sync task is queued but has not reported a result yet, get_result() raises
    ResultTimeout/ResultMissing, which was not caught. That unhandled exception
    reaches handler500, which likewise answers the POST with 405 instead of 500.

The defect is repo-wide, not SCIM-specific: any non-GET request that reaches an error
handler on any endpoint was reported as 405. schema.yml declares only 200/400/403 for
this operation, so a 405 is not a documented outcome and the web UI's
SyncObjectForm.ts has no way to surface it usefully.

How was this tested?

make all could not be run in my environment, and I want to be upfront about that:
uv sync --frozen fails locally while building dumb-init ('Compiler' object has no attribute 'linker_exe') and then psycopg-c (couldn't run 'pg_config' --includedir), and I have no docker/postgres available. So the django test suite and
the lint matrix could not execute against the real project. Please rely on CI for
make all.

What I did instead, to avoid submitting an unverified guess:

  • Built a standalone django + DRF harness that imports the real error.py by path and
    drives the handlers through RequestFactory, plus a DRF APIClient reproducing the
    router-level 404 and the unhandled-exception paths.
  • Confirmed the bug on unmodified source: each handler returns its intended code for
    GET but 405 for POST/PUT/PATCH/DELETE; a pk containing a dot and an
    unhandled exception both surface as 405.
  • Confirmed the fix: all methods return the handler's own status code, and the
    Allow header is gone.
  • Negative control: with the new test file kept and only error.py reverted, the test
    fails with 16 assertion failures, the first being AssertionError: 405 != 500.
  • ruff check and black --check pass on the three touched files.

No serializers or viewsets changed shape, so no make gen output changes are expected.

Linked issues

closes #24230

Checklist

  • The project has been linted, built, and tested (make all) — see "How was this
    tested?"; blocked locally by the uv sync build failures described above.
  • The documentation has been updated (n/a — no user-facing documentation change)
  • The documentation has been formatted (make docs) — n/a, no docs touched

Disclosure: yes, I used agentic AI assistance while preparing this change. The
diagnosis, the repros, and the negative control described above are real and were run;
please review accordingly, and let me know if you would rather I close this.

…t methods

The views wired up as django's handler400/403/404/500 subclassed TemplateView
directly. TemplateView only implements `get`, so its `dispatch` answers a failing
POST/PUT/PATCH/DELETE with 405 Method Not Allowed and an `Allow: GET, HEAD,
OPTIONS` header, discarding the status code the handler stands for. Any non-GET
request that reached an error handler -- on any endpoint -- was reported as a 405
rather than the actual 400/403/404/500.

Add a shared ErrorView base that renders the page for every method, and widen the
dramatiq result handling in the outgoing sync mixin from ResultFailure to
ResultError so a queued-but-not-yet-reported task returns the task log instead of
escaping as an unhandled 500.

closes goauthentik#24230
@Souptik96
Souptik96 requested a review from a team as a code owner August 9, 2026 05:02
@netlify

netlify Bot commented Aug 9, 2026

Copy link
Copy Markdown

Deploy Preview for authentik-docs ready!

Name Link
🔨 Latest commit 641319c
🔍 Latest deploy log https://app.netlify.com/projects/authentik-docs/deploys/6a7809f21fa6e60008b1cfe9
😎 Deploy Preview https://deploy-preview-24902--authentik-docs.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify project configuration.

Comment thread authentik/lib/sync/outgoing/api.py Outdated
Co-authored-by: Marc 'risson' Schmitt <marc.schmitt@risson.space>
Signed-off-by: Jens L. <jens@beryju.org>
@BeryJu BeryJu added backport/version-2026.2 Add this label to PRs to backport changes to version-2026.2 backport/version-2026.5 Add this label to PRs to backport changes to version-2026.5 backport/version-2026.8 Add this label to PRs to backport changes to version-2026.8 labels Aug 10, 2026
@netlify

netlify Bot commented Aug 10, 2026

Copy link
Copy Markdown

Deploy Preview for authentik-storybook ready!

Name Link
🔨 Latest commit 443d508
🔍 Latest deploy log https://app.netlify.com/projects/authentik-storybook/deploys/6a79cd9e2fdf8d0009ef94ca
😎 Deploy Preview https://deploy-preview-24902--authentik-storybook.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify project configuration.

@netlify

netlify Bot commented Aug 10, 2026

Copy link
Copy Markdown

Deploy Preview for authentik-integrations ready!

Name Link
🔨 Latest commit 443d508
🔍 Latest deploy log https://app.netlify.com/projects/authentik-integrations/deploys/6a79cd9e430ecb0008996c07
😎 Deploy Preview https://deploy-preview-24902--authentik-integrations.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify project configuration.

@codecov

codecov Bot commented Aug 10, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 97.56098% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 91.37%. Comparing base (3197973) to head (443d508).
⚠️ Report is 35 commits behind head on main.
✅ All tests successful. No failed tests found.

Files with missing lines Patch % Lines
authentik/lib/sync/outgoing/api.py 50.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main   #24902      +/-   ##
==========================================
- Coverage   91.38%   91.37%   -0.01%     
==========================================
  Files        1137     1147      +10     
  Lines       71626    71858     +232     
  Branches     3875     3875              
==========================================
+ Hits        65452    65663     +211     
- Misses       6139     6160      +21     
  Partials       35       35              
Flag Coverage Δ
conformance 34.84% <21.95%> (+0.03%) ⬆️
e2e 39.53% <21.95%> (+0.01%) ⬆️
integration 30.93% <2.43%> (-0.46%) ⬇️
rust 38.89% <ø> (ø)
unit 93.07% <97.56%> (+0.03%) ⬆️
unit-migrate 93.09% <97.56%> (+0.03%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

@BeryJu
BeryJu merged commit aca619f into goauthentik:main Aug 10, 2026
139 checks passed
@authentik-cherry-pick

Copy link
Copy Markdown
Contributor

🍒 Cherry-pick to version-2026.2 created: #24956

@authentik-cherry-pick

Copy link
Copy Markdown
Contributor

🍒 Cherry-pick to version-2026.5 created: #24957

@authentik-cherry-pick

Copy link
Copy Markdown
Contributor

🍒 Cherry-pick to version-2026.8 created: #24958

kensternberg-authentik pushed a commit that referenced this pull request Aug 10, 2026
…t methods (cherry-pick #24902 to version-2026.8) (#24958)

core: return the intended status code from error views for all request methods (#24902)

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

The views wired up as django's handler400/403/404/500 subclassed TemplateView
directly. TemplateView only implements `get`, so its `dispatch` answers a failing
POST/PUT/PATCH/DELETE with 405 Method Not Allowed and an `Allow: GET, HEAD,
OPTIONS` header, discarding the status code the handler stands for. Any non-GET
request that reached an error handler -- on any endpoint -- was reported as a 405
rather than the actual 400/403/404/500.

Add a shared ErrorView base that renders the page for every method, and widen the
dramatiq result handling in the outgoing sync mixin from ResultFailure to
ResultError so a queued-but-not-yet-reported task returns the task log instead of
escaping as an unhandled 500.

closes #24230

* Apply suggestion from @rissson




---------

Signed-off-by: Jens L. <jens@beryju.org>
Co-authored-by: Souptik Chakraborty <62941615+Souptik96@users.noreply.github.com>
Co-authored-by: Jens L. <jens@beryju.org>
Co-authored-by: Marc 'risson' Schmitt <marc.schmitt@risson.space>
BeryJu pushed a commit that referenced this pull request Aug 10, 2026
BeryJu pushed a commit that referenced this pull request Aug 10, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backport/version-2026.2 Add this label to PRs to backport changes to version-2026.2 backport/version-2026.5 Add this label to PRs to backport changes to version-2026.5 backport/version-2026.8 Add this label to PRs to backport changes to version-2026.8

Projects

None yet

Development

Successfully merging this pull request may close these issues.

/providers/scim/{id}/sync/object/ returns 405 when invalid ID is used or task errors for other reason

3 participants