Merge pull request #1332 from makeabilitylab/1236-https-scheme-djangoenv

fix(seo): https still broken on -test — key site_scheme off DJANGO_ENV not DEBUG (#1236)

Author: jonfroehlich

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):
     """

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, '<link rel="canonical" href="https://testserver/">')
+        self.assertContains(resp, '<meta property="og:url" content="https://testserver/">')
+
+    @override_settings(DJANGO_ENV='PROD', DEBUG=False)
+    def test_prod_uses_https(self):
+        resp = self.client.get(reverse("website:index"))
+        self.assertContains(resp, '<meta property="og:url" content="https://testserver/">')
+
+    @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, '<link rel="canonical" href="http://testserver/">')
         self.assertContains(resp, '<meta property="og:url" content="http://testserver/">')

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):