fix(security): fall back to allowed_origins for CSRF origin validation behind reverse proxies

[jonpspri]

Summary

• Fixes CSRF origin validation failure behind layered reverse proxies where X-Forwarded-Proto / X-Forwarded-Host headers reflect internal hops rather than the external scheme/host
• Extends _request_origin_matches() to fall back to settings.allowed_origins when the forwarded-header same-origin comparison fails
• Excludes wildcard entries (*, null, "") from the fallback to preserve fail-closed behavior

Closes #3431

[Copilot]

Pull request overview

This PR fixes admin CSRF origin validation in reverse-proxy deployments by allowing a safe fallback comparison against settings.allowed_origins when forwarded-header-based same-origin checks fail (while keeping wildcard origins fail-closed). It also adds unit tests covering the new fallback behavior.

Changes:

• Extend _request_origin_matches() to fall back to settings.allowed_origins when same-origin (via forwarded headers) does not match.
• Explicitly exclude wildcard-ish entries (*, null, "") from the fallback.
• Add unit tests for allowed-origins fallback acceptance/rejection and port normalization behavior.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File	Description
mcpgateway/admin.py	Adds allowed_origins fallback path to CSRF origin validation when forwarded-header same-origin fails.
tests/unit/mcpgateway/test_admin.py	Adds regression tests for the new allowed_origins fallback behavior in enforce_admin_csrf.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

settings.allowed_origins entries can include leading/trailing whitespace when provided via JSON env (e.g., ["https://a.com "]), and validate_cors_origins doesn’t strip it. In this fallback, a whitespace-padded origin may be silently ignored or mis-parsed, causing CSRF validation to fail unexpectedly. Consider normalizing each allowed entry (e.g., str(...).strip(), plus a case-insensitive check for "null") before the wildcard/parse checks (and ideally precomputing normalized allowed-origin parts once to avoid repeated urlparse work per request).

Suggested change

	if allowed in ("*", "null", ""):
	continue
	try:
	allowed_parsed = urllib.parse.urlparse(allowed)
	# Normalize each allowed origin to avoid config surprises such as
	# ["https://a.com "] or [" null "], which could otherwise be
	# mis-parsed or skipped.
	allowed_normalized = str(allowed).strip()
	if not allowed_normalized or allowed_normalized == "*" or allowed_normalized.casefold() == "null":
	continue
	try:
	allowed_parsed = urllib.parse.urlparse(allowed_normalized)

[madhav165]

Reviewed the changes — code looks good. The allowed_origins fallback in _request_origin_matches() is clean, wildcard/null exclusion preserves fail-closed behavior, and test coverage is solid (6 new cases covering accept, reject, port normalization, empty set, and regression). Nginx header pass-through changes are consistent across all location blocks.

[crivetimihai]

Thanks @jonpspri — critical fix for #3431. The fallback to allowed_origins when forwarded-header comparison fails is the right approach for reverse proxy deployments. Good that wildcard entries are excluded from the fallback to maintain fail-closed behavior. LGTM.

[crivetimihai]

Review — Approve

Rebased onto main (clean, no conflicts) and reviewed thoroughly.

What this PR does

1. Nginx configs: Preserves upstream X-Forwarded-Proto via map directive instead of overwriting with $scheme. Adds X-Forwarded-Host $http_host. Applied to both nginx.conf and the Helm configmap.
2. CSRF origin fallback (admin.py): When the forwarded-header same-origin check fails, falls back to settings.allowed_origins. Wildcards (*, null, "") are excluded to preserve fail-closed behavior.
3. Tests: 12 new tests covering all fallback branches.

Changes I made on top of the original commits

• Aligned schemeless origin handling with SSO: _request_origin_matches() now prepends https:// for allowed_origins entries without a scheme (e.g. bare example.com), consistent with sso.py:_validate_redirect_uri(). Previously these were silently skipped, meaning ALLOWED_ORIGINS=example.com would work for SSO but not for CSRF fallback.
• Added 3 new tests: null entry rejection, blank "" entry rejection, and schemeless origin positive-path acceptance.
• Renamed test: test_enforce_admin_csrf_skips_schemeless_allowed_origin → test_enforce_admin_csrf_rejects_non_matching_bare_host with updated description to reflect the new behavior.

Security analysis

• No CSRF bypass: fallback requires exact normalized match against non-wildcard allowlist entries, and the double-submit CSRF token check still gates all mutations independently.
• Wildcard/null/blank filtering is correct and tested.
• Fail-closed behavior preserved: empty allowed_origins or only-wildcard sets result in rejection.

Out-of-scope follow-ups

• Other nginx configs (nginx-tls.conf, nginx-performance.conf, tls-stack.yaml) still use $scheme — should be addressed in a separate PR.
• The map directive trusts inbound X-Forwarded-Proto unconditionally — pre-existing architectural concern, not introduced by this PR.