From e2ca8b2d9c0ba831316248d19a0c38292a78bbe4 Mon Sep 17 00:00:00 2001 From: Jon Froehlich Date: Thu, 18 Jun 2026 07:39:18 -0700 Subject: [PATCH] =?UTF-8?q?fix(seo):=20key=20site=5Fscheme=20off=20DJANGO?= =?UTF-8?q?=5FENV,=20not=20DEBUG=20=E2=80=94=20https=20was=20still=20broke?= =?UTF-8?q?n=20on=20-test=20(#1236)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The #1236 fix pinned https only when `not DEBUG`, assuming DEBUG=True meant local dev. But the TEST server runs DEBUG=True (config-test.ini) while sitting behind the same TLS-terminating Apache proxy as prod, so `site_scheme` returned `request.scheme` (http) there — every canonical/og:url/og:image on makeabilitylab-test emitted http://, i.e. #1236 was only fixed on prod, not test. (Confirmed by inspecting the live test pages after deploy.) Key off `DJANGO_ENV` instead: 'PROD'/'TEST' on the servers (set by rebuildanddeploy.sh) → https; 'DEBUG'/unset locally → request.scheme. Make the `site_scheme` context processor delegate to `website.utils.metadata.site_scheme` so the template (canonical/OG) and view (JSON-LD) URL schemes share one definition and can't drift. Tests: replace the DEBUG-based assertions with DJANGO_ENV-based ones and add an explicit regression test that DJANGO_ENV=TEST + DEBUG=True still emits https. Full suite 267 green. Co-Authored-By: Claude Opus 4.8 (1M context) --- website/context_processors.py | 31 ++++++++++------------------- website/tests/test_page_metadata.py | 27 ++++++++++++++++++++----- website/utils/metadata.py | 21 ++++++++++++------- 3 files changed, 46 insertions(+), 33 deletions(-) diff --git a/website/context_processors.py b/website/context_processors.py index b2290390..90053933 100644 --- a/website/context_processors.py +++ b/website/context_processors.py @@ -9,6 +9,7 @@ from .models import News from django.conf import settings +from website.utils.metadata import site_scheme as metadata_site_scheme def recent_news(request): """ context processors returning recent news """ @@ -19,28 +20,16 @@ def recent_news(request): def site_scheme(request): """ - Expose the canonical URL scheme (``http`` / ``https``) to every template. - - The site runs behind UW CSE's Apache TLS-terminating proxy, which talks to - the Django container over plain HTTP. Because ``SECURE_PROXY_SSL_HEADER`` is - not (yet) configured, ``request.scheme`` reports ``http`` in production/test - even though visitors arrive over HTTPS. Building absolute URLs (canonical, - Open Graph ``og:url``/``og:image``, Twitter Card images) from - ``request.scheme`` therefore advertises ``http://`` links to crawlers and - social scrapers — the root cause of issue #1236. - - This processor pins the scheme to ``https`` whenever the site is not in - DEBUG (i.e. on the test/prod servers), while leaving local dev on whatever - ``request.scheme`` reports (``http`` over localhost). Templates should build - absolute URLs as ``{{ site_scheme }}://{{ request.get_host }}{{ path }}``. - - NOTE: This is the in-repo workaround. The cleaner long-term fix is for IT to - set ``SECURE_PROXY_SSL_HEADER`` on the proxy (tracked in #1329); once that - lands, ``request.scheme`` will be correct and this can fall back to it. + Expose the canonical URL scheme (``http`` / ``https``) to every template, so + templates can build absolute URLs as + ``{{ site_scheme }}://{{ request.get_host }}{{ path }}`` that aren't ``http://`` + behind UW CSE's TLS-terminating proxy (issue #1236). + + Delegates to :func:`website.utils.metadata.site_scheme` (the single source of + truth, keyed on ``DJANGO_ENV``) so the scheme used by view-built JSON-LD URLs + and template-built canonical/OG URLs can't drift. """ - return { - 'site_scheme': request.scheme if settings.DEBUG else 'https', - } + return {'site_scheme': metadata_site_scheme(request)} def admin_version_info(request): """ diff --git a/website/tests/test_page_metadata.py b/website/tests/test_page_metadata.py index 62a7565c..031e8b87 100644 --- a/website/tests/test_page_metadata.py +++ b/website/tests/test_page_metadata.py @@ -46,9 +46,11 @@ def _position(person, title=None): ) -# On the servers DEBUG is False, so site_scheme pins https — assert against that -# (the test client's host is "testserver", auto-added to ALLOWED_HOSTS). -@override_settings(DEBUG=False) +# On the servers (DJANGO_ENV TEST/PROD) site_scheme pins https — assert against +# that (the test client's host is "testserver", auto-added to ALLOWED_HOSTS). +# NB: DJANGO_ENV, not DEBUG — the test server runs DEBUG=True behind the proxy +# (see PageMetadataSchemeTests for the regression that motivated this). +@override_settings(DJANGO_ENV='PROD') class PageMetadataHttpsTests(DatabaseTestCase): def test_home_has_core_metadata(self): @@ -204,10 +206,25 @@ def test_jsonld_escapes_script_breakout(self): class PageMetadataSchemeTests(DatabaseTestCase): + """site_scheme keys off DJANGO_ENV, not DEBUG. The test server runs DEBUG=True + behind the same TLS proxy as prod, so a DEBUG-based check would emit http:// + there (regression for the #1236 follow-up fix).""" - @override_settings(DEBUG=True) + @override_settings(DJANGO_ENV='TEST', DEBUG=True) + def test_test_server_uses_https_even_with_debug_true(self): + """The crux: DJANGO_ENV=TEST + DEBUG=True must still emit https.""" + resp = self.client.get(reverse("website:index")) + self.assertContains(resp, '') + self.assertContains(resp, '') + + @override_settings(DJANGO_ENV='PROD', DEBUG=False) + def test_prod_uses_https(self): + resp = self.client.get(reverse("website:index")) + self.assertContains(resp, '') + + @override_settings(DJANGO_ENV='DEBUG', DEBUG=True) def test_local_dev_uses_request_scheme(self): - """In DEBUG (local dev over http) the absolute URLs follow request.scheme.""" + """Local dev (DJANGO_ENV=DEBUG/unset over http) follows request.scheme.""" resp = self.client.get(reverse("website:index")) self.assertContains(resp, '') self.assertContains(resp, '') diff --git a/website/utils/metadata.py b/website/utils/metadata.py index a7052c83..e1189bc3 100644 --- a/website/utils/metadata.py +++ b/website/utils/metadata.py @@ -45,14 +45,21 @@ def meta_description(html, max_chars=META_DESCRIPTION_MAX_CHARS): def site_scheme(request): """ - The canonical scheme for absolute URLs built in views (https on the servers, - request.scheme in local dev). Mirrors the ``site_scheme`` context processor - (website/context_processors.py) so view-built JSON-LD URLs match the - template-built canonical/OG URLs. See #1236 (and #1329 for the IT-side - SECURE_PROXY_SSL_HEADER follow-up that would let this fall back to - request.scheme). + The canonical scheme for absolute URLs (https on the test/prod servers, + ``request.scheme`` in local dev). Single source of truth — the ``site_scheme`` + context processor delegates here, so view-built JSON-LD URLs and template-built + canonical/OG URLs always agree. + + We key off ``DJANGO_ENV``, NOT ``DEBUG``: the **test** server runs with + ``DEBUG=True`` (config-test.ini) yet sits behind the same TLS-terminating Apache + proxy as prod, so a DEBUG-based check emits ``http://`` on test — the exact #1236 + bug. ``DJANGO_ENV`` is ``'TEST'``/``'PROD'`` on the servers (set by + rebuildanddeploy.sh) and ``'DEBUG'``/unset locally, which is the signal we want. + + NOTE: in-repo workaround. #1329 (IT enabling SECURE_PROXY_SSL_HEADER) would make + request.scheme correct everywhere and let this fall back to it. """ - return request.scheme if settings.DEBUG else 'https' + return 'https' if settings.DJANGO_ENV in ('PROD', 'TEST') else request.scheme def absolute_url(request, path):