Merge pull request #5909 from learningequality/hotfixes

Release 2026.05.14

Author: rtibbles

.pre-commit-config.yaml

@@ -81,6 +81,15 @@ repos:
           contentcuration/kolibri_public/migrations/0004_auto_20240612_1847.py|
           contentcuration/kolibri_public/migrations/0006_auto_20250417_1516.py|
           )$
+  # Only checks the root Makefile. Extend if nested Makefiles get added.
+  - repo: local
+    hooks:
+      - id: makefile-syntax
+        name: Makefile syntax check
+        entry: make -n
+        language: system
+        files: ^Makefile$
+        pass_filenames: false
   # Always keep black as the final hook so it reformats any other reformatting.
   - repo: https://github.com/python/black
     rev: 20.8b1

Makefile

@@ -39,7 +39,7 @@ migrate:
 # 4) Remove the management command from this `deploy-migrate` recipe
 # 5) Repeat!
 deploy-migrate:
-	python contentcuration/manage.py ensure_versioned_databases_exist & python contentcuration/manage.py create_channel_versions & wait
+	echo "Nothing to do here!"
 
 contentnodegc:
 	python contentcuration/manage.py garbage_collect

contentcuration/contentcuration/tests/utils/test_urls.py

@@ -0,0 +1,38 @@
+from django.contrib.sites.models import Site
+from django.test import override_settings
+
+from contentcuration.tests.base import StudioTestCase
+from contentcuration.utils.urls import canonical_url
+
+
+class CanonicalUrlTestCase(StudioTestCase):
+    @override_settings(SITE_ID=1)
+    def test_production_domain_uses_https(self):
+        self.assertEqual(
+            canonical_url("/settings/"),
+            "https://studio.learningequality.org/settings/",
+        )
+
+    @override_settings(SITE_ID=3)
+    def test_branch_subdomain_uses_https(self):
+        self.assertEqual(
+            canonical_url("/settings/"),
+            "https://unstable.studio.learningequality.org/settings/",
+        )
+
+    @override_settings(SITE_ID=2)
+    def test_local_dev_loopback_uses_http(self):
+        self.assertEqual(canonical_url("/settings/"), "http://127.0.0.1:8080/settings/")
+
+    def test_localhost_uses_http(self):
+        site = Site.objects.create(
+            pk=9999, domain="localhost:9000", name="Localhost test"
+        )
+        with override_settings(SITE_ID=site.pk):
+            self.assertEqual(
+                canonical_url("/settings/"), "http://localhost:9000/settings/"
+            )
+
+    @override_settings(SITE_ID=1)
+    def test_empty_path_returns_bare_url(self):
+        self.assertEqual(canonical_url(), "https://studio.learningequality.org")

contentcuration/contentcuration/tests/views/test_subscription.py

@@ -1,6 +1,7 @@
 from unittest import mock
 
 import stripe
+from django.contrib.sites.models import Site
 from django.test import override_settings
 from django.urls import reverse
 
@@ -57,6 +58,33 @@ def test_rejects_user_with_active_subscription(self, mock_create):
         self.assertEqual(response.status_code, 400)
         mock_create.assert_not_called()
 
+    @override_settings(SITE_ID=1)
+    @mock.patch("contentcuration.views.subscription.stripe.checkout.Session.create")
+    def test_checkout_urls_use_canonical_site_domain(self, mock_create):
+        Site.objects.update_or_create(
+            pk=1, defaults={"domain": "studio.learningequality.org", "name": "Studio"}
+        )
+        mock_create.return_value = mock.Mock(url="https://checkout.stripe.com/test")
+
+        self.client.force_authenticate(self.user)
+        response = self.client.post(
+            self.url,
+            data={"storage_gb": 10},
+            format="json",
+            HTTP_HOST="master.studio.learningequality.org",
+        )
+
+        self.assertEqual(response.status_code, 200)
+        call_kwargs = mock_create.call_args[1]
+        for key in ("success_url", "cancel_url"):
+            url = call_kwargs[key]
+            self.assertNotIn(
+                "master.studio.learningequality.org",
+                url,
+                f"{key} leaked internal hostname: {url}",
+            )
+            self.assertIn("studio.learningequality.org", url)
+
     @mock.patch("contentcuration.views.subscription.stripe.checkout.Session.create")
     def test_user_with_canceled_subscription_can_checkout_again(self, mock_create):
         """User whose subscription was canceled can create a new checkout session."""
@@ -124,6 +152,35 @@ def test_creates_portal_session(self, mock_create):
         data = response.json()
         self.assertEqual(data["portal_url"], "https://billing.stripe.com/test")
 
+    @override_settings(SITE_ID=1)
+    @mock.patch(
+        "contentcuration.views.subscription.stripe.billing_portal.Session.create"
+    )
+    def test_portal_return_url_uses_canonical_site_domain(self, mock_create):
+        Site.objects.update_or_create(
+            pk=1, defaults={"domain": "studio.learningequality.org", "name": "Studio"}
+        )
+        UserSubscription.objects.create(
+            user=self.user,
+            stripe_customer_id="cus_test123",
+            stripe_subscription_status="active",
+        )
+        mock_create.return_value = mock.Mock(url="https://billing.stripe.com/test")
+
+        self.client.force_authenticate(self.user)
+        response = self.client.post(
+            self.url, HTTP_HOST="master.studio.learningequality.org"
+        )
+
+        self.assertEqual(response.status_code, 200)
+        return_url = mock_create.call_args[1]["return_url"]
+        self.assertNotIn(
+            "master.studio.learningequality.org",
+            return_url,
+            f"return_url leaked internal hostname: {return_url}",
+        )
+        self.assertIn("studio.learningequality.org", return_url)
+
 
 @override_settings(
     STRIPE_SECRET_KEY="sk_test_fake",

contentcuration/contentcuration/utils/urls.py

@@ -0,0 +1,20 @@
+from django.contrib.sites.models import Site
+from django.contrib.sites.shortcuts import get_current_site
+
+_LOCAL_HOSTS = ("127.0.0.1", "localhost", "::1")
+
+
+def canonical_url(path="", request=None):
+    """Absolute URL on the Site framework's canonical domain.
+
+    Prefer this over ``request.build_absolute_uri`` for URLs handed to
+    external systems: the latter reflects the Host header Django actually
+    received, which on production is the internal pod hostname rather
+    than the public canonical.
+    """
+    if request is not None:
+        domain = get_current_site(request).domain
+    else:
+        domain = Site.objects.get_current().domain
+    scheme = "http" if domain.startswith(_LOCAL_HOSTS) else "https"
+    return f"{scheme}://{domain}{path}"

contentcuration/contentcuration/views/subscription.py

@@ -14,6 +14,7 @@
 
 from contentcuration.models import User
 from contentcuration.models import UserSubscription
+from contentcuration.utils.urls import canonical_url
 
 BYTES_PER_GB = 10 ** 9
 MIN_STORAGE_GB = 1
@@ -52,10 +53,10 @@ def post(self, request):
         storage_gb = serializer.validated_data["storage_gb"]
 
         try:
-            success_url = request.build_absolute_uri(
-                "/settings/#/storage?upgrade=success"
+            success_url = canonical_url(
+                "/settings/#/storage?upgrade=success", request=request
             )
-            cancel_url = request.build_absolute_uri("/settings/#/storage")
+            cancel_url = canonical_url("/settings/#/storage", request=request)
 
             checkout_session_params = {
                 "mode": "subscription",
@@ -101,7 +102,7 @@ def post(self, request):
         try:
             session = stripe.billing_portal.Session.create(
                 customer=subscription.stripe_customer_id,
-                return_url=request.build_absolute_uri("/settings/#/storage"),
+                return_url=canonical_url("/settings/#/storage", request=request),
             )
             return Response({"portal_url": session.url})