fix(csrf): accept same-host Origin regardless of scheme (#2867)#2868
Merged
Conversation
* Custom SameHostOriginCsrfMiddleware falls back to a host-only Origin match when the stock scheme+host check rejects, fixing 403s behind a TLS-terminating proxy (Caddy sidecar, Cloudflare Tunnel, Tailscale Serve) that hands plain HTTP to uvicorn. * Also covers HSTS / HTTPS-First leftovers after disabling SSL. * Cross-host Origin and bad/missing tokens still 403. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
This PR addresses CSRF 403 failures caused by Django’s strict Origin scheme+host validation when Anthias is deployed behind TLS-terminating proxies that don’t propagate (or aren’t configured to trust) the original scheme (issue #2867). It introduces a custom CSRF middleware that preserves Django’s default behavior first, then falls back to allowing same-host Origin values regardless of scheme while still requiring valid CSRF tokens.
Changes:
- Adds
SameHostOriginCsrfMiddlewareto relax same-host Origin checks across scheme mismatches. - Replaces Django’s default CSRF middleware with the custom middleware in Django settings.
- Adds regression tests covering same-host scheme drift and ensuring cross-host / missing-token / bad-token requests still fail.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
src/anthias_server/lib/csrf.py |
Adds the custom CSRF middleware with a same-host fallback Origin check. |
src/anthias_server/django_project/settings.py |
Registers the custom middleware in MIDDLEWARE and updates CSRF-related comments. |
tests/test_csrf.py |
Adds regression tests for the new middleware behavior and CSRF invariants. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
* Suppress mypy ``misc`` on the ``super()._origin_verified`` call —
django-stubs doesn't model private hooks — and ``bool(...)`` the
final comparison for ``no-any-return``.
* NOSONAR the MIDDLEWARE entry: S4502 pattern-matches the bare
class name, but SameHostOriginCsrfMiddleware extends
CsrfViewMiddleware so CSRF protection is still wired in.
* Centralise the test Origin literals so S5332 ("use https") fires
once per constant instead of per call site, with NOSONAR on the
intentional ``http://`` ones.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Sonar's S4502 anchors on the closing ``]`` of the MIDDLEWARE list, not on the individual middleware entries — the comment on the custom-middleware line wasn't suppressing it. Move NOSONAR to the bracket S4502 actually flags. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* Pull a ``_hostname()`` helper that runs both ``Origin`` and ``request.get_host()`` through ``urlsplit().hostname`` so the comparison ignores port and IPv6-bracket drift. Common shape this fixes: proxy passes ``Host: device.local:443`` upstream while the browser's ``Origin`` is ``https://device.local`` with the default port elided. * Regression test ``test_host_with_port_origin_without_port_passes`` pins the host-with-port + origin-without-port case. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The ``http://`` prefix is only there so ``urlsplit`` will parse a
bare ``Host`` header value into a hostname; nothing leaves the
process on that scheme. Sonar's S5332 ("use https") doesn't apply.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Use a protocol-relative ``//host[:port]`` prefix so ``urlsplit`` parses a bare ``Host`` header value as a netloc — no literal scheme baked into the file, no NOSONAR suppression needed for S5332. IPv6 and ``user:pw@host`` shapes still hostname-extract cleanly (verified). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Two follow-ups from Copilot review on e9794f4: * Tighten the fallback so ``Origin: http://device:8080`` posting to ``Host: device:8000`` (distinct web origins) still 403s. Resolve each side to ``(host, port)`` with default-port substitution for the scheme, accept exact-port matches, and only tolerate port drift when at least one side is on the scheme's default — that's the proxy/HSTS shape the fallback actually exists for. * Drop the regex-on-rendered-HTML token extraction in the tests. Use the ``csrftoken`` cookie value directly as ``csrfmiddlewaretoken`` (Django accepts the unmasked secret) so the suite is decoupled from template markup. * Regression test ``test_same_host_distinct_non_default_ports_rejected`` pins the cross-origin distinct-port case. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Copilot pointed out the previous rule was still too broad: with ``Origin: https://anthias.local`` (port 443) posting to ``Host: anthias.local:8000``, my "either side on a default port" test accepted it even though 443 and 8000 are distinct web origins. Tighten to ``{origin_port, target_port} == {80, 443}`` — exactly the scheme-drift pair the proxy/HSTS case needs, nothing else. Add ``test_default_port_origin_vs_non_default_host_port_rejected`` to pin the case Copilot flagged. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The earlier comment said the final comparison needed ``bool(...)`` to satisfy ``no-any-return``. The body has since been rewritten around ``_split_origin`` and the explicit ``True``/``False`` / set-equality returns make that wrapping unnecessary — the line was just lying. Drop it; the ``type: ignore`` rationale is the only part still load-bearing. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Issues Fixed
Closes #2867.
Description
Django 4.x+ rejects POSTs where the
Originheader'sscheme://hostdoesn't match what uvicorn sees. That 403s every form submit on Anthias when the device sits behind a TLS-terminating proxy (Caddy sidecar, Cloudflare Tunnel, Tailscale Serve, home-router HTTPS rewrite) withoutFORWARDED_ALLOW_IPSalso being set, and after disabling a previously-enabled TLS proxy while the browser's HSTS / HTTPS-First cache still sendshttps://. Confirmed in dev:Origin checking failed - https://localhost:8000 does not match any trusted origins.CSRF_TRUSTED_ORIGINS = ['http://*', 'https://*']looks like the answer but is a Django no-op (only subdomain wildcards likehttps://*.example.comare honoured).The fix is a thin
CsrfViewMiddlewaresubclass that defers to the stock check first, then falls back to accepting anyOriginwhose host equalsrequest.get_host()regardless of scheme. The masked-token check still runs on top, so the relaxation can't bypass CSRF — cross-hostOriginand missing / bad tokens still 403.Checklist
🤖 Generated with Claude Code