Drop sitemap https pin; verify SECURE_PROXY_SSL_HEADER via request.scheme (#1329)#1338
Merged
Merged
Conversation
Now that SECURE_PROXY_SSL_HEADER lets Django see the real https scheme behind UW CSE's Apache proxy (#1329), the sitemap no longer needs to pin protocol="https". Remove the _HttpsSitemap base so every sitemap inherits Django's default behavior, where <loc> URLs use request.scheme. This doubles as the on-server verification for #1329: /sitemap.xml on -test now reflects request.scheme directly, so if its <loc> URLs stay https the proxy header is being honored end-to-end (the meta-tag site_scheme workaround masks this everywhere else). Tests: replace the old "always https" assertion with one that drives the scheme via an X-Forwarded-Proto header under SECURE_PROXY_SSL_HEADER (the real #1329 path), plus a check that a plain request now reflects the request scheme rather than a hardcoded https. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
jonfroehlich
added a commit
that referenced
this pull request
Jun 18, 2026
…workaround, Pa11y CI) Removes the site_scheme context-processor workaround now that SECURE_PROXY_SSL_HEADER is trusted on TEST/PROD; https is derived from request.scheme behind UW CSE's TLS proxy (#1329/#1236, #1338). Wires the Pa11y accessibility sweep into CI and excludes django-debug-toolbar from the scan (#1278 item 6). No schema change. Co-Authored-By: Claude Opus 4.8 (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.
Follow-up to #1336 (which enabled
SECURE_PROXY_SSL_HEADERon TEST/PROD). Part of #1329 — does not close it (thesite_schememeta-tag workaround removal is still pending).Why
After #1336, Django sees the real
httpsscheme behind UW CSE's Apache proxy. But every scheme-dependent output had already been pinned to https as a workaround, so there was nothing left to passively verify against. The sitemap was one such workaround (_HttpsSitemaphardcodedprotocol = "https", #1252).Removing that pin makes
/sitemap.xmlreflectrequest.schemedirectly — turning it into the one observable that provesSECURE_PROXY_SSL_HEADERis working end-to-end.Change
_HttpsSitemapbase; the four sitemap classes now extendSitemapdirectly and inherit Django's default scheme behavior (<loc>usesrequest.scheme).test_sitemap_honors_forwarded_proto_header— drives the scheme via anX-Forwarded-Proto: httpsheader under@override_settings(SECURE_PROXY_SSL_HEADER=...), i.e. the real Enable SECURE_PROXY_SSL_HEADER so Django sees the real https scheme (supersedes in-app workaround) #1329 path; andtest_sitemap_scheme_follows_request— a plain request now reflects the request scheme (http) rather than a hardcoded https.Verification (the point of this PR)
After merge → deploy to
-test, fetchhttps://makeabilitylab-test.cs.washington.edu/sitemap.xml:<loc>URLs stillhttps://→SECURE_PROXY_SSL_HEADERis honored; safe to proceed with removing thesite_schememeta-tag workaround.<loc>URLs showhttp://→ the proxy header isn't reaching Django as expected; revert this PR and keep the workarounds.Tests
Full suite green locally in-container:
279 tests, OK (skipped=1)(--settings=makeabilitylab.settings_test).🤖 Generated with Claude Code