feat(nimbus): simplify GrafanaProxyView to accept only slug, construct URL from settings

Yashika Khurana · Yashika Khurana · commit e39bc28c4dc7 · 2026-05-19T16:56:50.000-07:00
Addresses security review: removes path parameter injection risk by validating
slug against DB and building the Grafana URL entirely from settings constants.
diff --git a/experimenter/experimenter/nimbus_ui/templates/nimbus_experiments/features.html b/experimenter/experimenter/nimbus_ui/templates/nimbus_experiments/features.html
@@ -545,7 +545,7 @@ <h5 class="fw-semibold mb-0 d-flex align-items-center">
           </div>
           <div class="card shadow-sm border-0 rounded-3 mb-3">
             <div class="card-body p-0">
-              <iframe src="/nimbus/grafana-proxy/?slug={{ selected_feature_config.slug }}&kiosk"
+              <iframe src="/nimbus/grafana-proxy/?slug={{ selected_feature_config.slug }}"
                       width="100%"
                       height="800"
                       frameborder="0"
diff --git a/experimenter/experimenter/nimbus_ui/tests/test_views.py b/experimenter/experimenter/nimbus_ui/tests/test_views.py
@@ -5842,69 +5842,27 @@ def test_post_clears_all_tags_when_none_selected(self):
 
 class TestGrafanaProxyView(AuthTestCase):
     def _dashboard_url(self, slug):
-        return reverse("nimbus-ui-grafana-proxy", kwargs={"path": ""}) + f"?slug={slug}"
+        return reverse("nimbus-ui-grafana-proxy") + f"?slug={slug}"
 
     @patch("experimenter.nimbus_ui.views.requests.get")
-    def test_proxy_returns_grafana_html(self, mock_get):
+    def test_proxy_returns_grafana_response(self, mock_get):
         feature = NimbusFeatureConfigFactory.create(
             slug="my-feature",
             application=NimbusExperiment.Application.DESKTOP,
         )
         mock_get.return_value.status_code = 200
         mock_get.return_value.headers = {"Content-Type": "text/html; charset=utf-8"}
-        mock_get.return_value.text = (
-            '<html><head><base href="/"></head><body></body></html>'
-        )
+        mock_get.return_value.content = b"<html></html>"
 
         response = self.client.get(self._dashboard_url(feature.slug))
 
         self.assertEqual(response.status_code, 200)
         mock_get.assert_called_once()
         call_url = mock_get.call_args[0][0]
         self.assertIn("nimbus-feature-monitoring", call_url)
-
-    @patch("experimenter.nimbus_ui.views.requests.get")
-    def test_proxy_rewrites_base_href(self, mock_get):
-        feature = NimbusFeatureConfigFactory.create(
-            slug="my-feature",
-            application=NimbusExperiment.Application.DESKTOP,
-        )
-        mock_get.return_value.status_code = 200
-        mock_get.return_value.headers = {"Content-Type": "text/html"}
-        mock_get.return_value.text = '<base href="/">'
-
-        response = self.client.get(self._dashboard_url(feature.slug))
-
-        self.assertIn(b"/nimbus/grafana-proxy/", response.content)
-        self.assertNotIn(b'base href="/"', response.content)
-
-    @patch("experimenter.nimbus_ui.views.requests.get")
-    def test_proxy_rewrites_app_url_in_boot_data(self, mock_get):
-        feature = NimbusFeatureConfigFactory.create(
-            slug="my-feature",
-            application=NimbusExperiment.Application.DESKTOP,
-        )
-        mock_get.return_value.status_code = 200
-        mock_get.return_value.headers = {"Content-Type": "text/html"}
-        mock_get.return_value.text = '"appUrl":"https://yardstick.mozilla.org/"'
-
-        response = self.client.get(self._dashboard_url(feature.slug))
-
-        self.assertIn(b"/nimbus/grafana-proxy/", response.content)
-        self.assertNotIn(b"yardstick.mozilla.org", response.content)
-
-    @patch("experimenter.nimbus_ui.views.requests.get")
-    def test_proxy_returns_binary_content_unchanged(self, mock_get):
-        mock_get.return_value.status_code = 200
-        mock_get.return_value.headers = {"Content-Type": "application/javascript"}
-        mock_get.return_value.content = b"console.log('hello');"
-
-        response = self.client.get(
-            reverse("nimbus-ui-grafana-proxy", kwargs={"path": "public/build/app.js"}),
-        )
-
-        self.assertEqual(response.status_code, 200)
-        self.assertEqual(response.content, b"console.log('hello');")
+        call_params = mock_get.call_args[1]["params"]
+        self.assertEqual(call_params["var-feature"], "my-feature")
+        self.assertEqual(call_params["var-application"], "firefox_desktop")
 
     @patch(
         "experimenter.nimbus_ui.views.requests.get",
@@ -5921,9 +5879,7 @@ def test_proxy_returns_503_when_grafana_unavailable(self, mock_get):
 
     def test_proxy_requires_login(self):
         self.client.defaults.pop(settings.OPENIDC_EMAIL_HEADER)
-        response = self.client.get(
-            reverse("nimbus-ui-grafana-proxy", kwargs={"path": "d/abc/my-dashboard"}),
-        )
+        response = self.client.get(reverse("nimbus-ui-grafana-proxy"))
         self.assertNotEqual(response.status_code, 200)
 
     @override_settings(GRAFANA_SERVICE_ACCOUNT_TOKEN="test-token")
@@ -5935,17 +5891,15 @@ def test_proxy_sends_service_account_token(self, mock_get):
         )
         mock_get.return_value.status_code = 200
         mock_get.return_value.headers = {"Content-Type": "text/html"}
-        mock_get.return_value.text = ""
+        mock_get.return_value.content = b""
 
         self.client.get(self._dashboard_url(feature.slug))
 
         call_headers = mock_get.call_args[1]["headers"]
         self.assertEqual(call_headers["Authorization"], "Bearer test-token")
 
     def test_proxy_returns_400_when_slug_missing(self):
-        response = self.client.get(
-            reverse("nimbus-ui-grafana-proxy", kwargs={"path": ""}),
-        )
+        response = self.client.get(reverse("nimbus-ui-grafana-proxy"))
         self.assertEqual(response.status_code, 400)
 
     def test_proxy_returns_404_when_slug_invalid(self):
diff --git a/experimenter/experimenter/nimbus_ui/urls.py b/experimenter/experimenter/nimbus_ui/urls.py
@@ -23,6 +23,7 @@
     DraftToReviewView,
     EditOutcomeSummaryView,
     FeatureSubscribersUpdateView,
+    GrafanaProxyView,
     LiveToCompleteView,
     LiveToEndEnrollmentView,
     LiveToUpdateRolloutView,
@@ -79,6 +80,11 @@
         NimbusFeaturesView.as_view(),
         name="nimbus-ui-features",
     ),
+    re_path(
+        r"^grafana-proxy/$",
+        GrafanaProxyView.as_view(),
+        name="nimbus-ui-grafana-proxy",
+    ),
     re_path(
         r"^tags/manage/$",
         TagsManageView.as_view(),
diff --git a/experimenter/experimenter/nimbus_ui/views.py b/experimenter/experimenter/nimbus_ui/views.py
@@ -1,6 +1,5 @@
 import json
-import re
-from urllib.parse import parse_qs, urlparse
+from django.shortcuts import get_object_or_404
 
 import requests
 from deepdiff import DeepDiff
@@ -1171,35 +1170,35 @@ def get_context_data(self, **kwargs):
 # Grafana is behind Google IAP, which blocks third-party cookies in iframes.
 # Both Experimenter and Grafana run in the same k8s cluster, so the backend
 # can reach Grafana via internal DNS (bypassing IAP) and proxy it to the browser.
-_GRAFANA_PROXY_PREFIX = "/nimbus/grafana-proxy/"
 _SAFE_RESPONSE_HEADERS = ("Cache-Control", "ETag", "Last-Modified", "Content-Type")
 
 
 class GrafanaProxyView(LoginRequiredMixin, View):
-    def get(self, request, path=""):
-        if not path:
-            # Dashboard request: validate slug against the DB and construct the
-            # Grafana URL server-side so user-supplied input never reaches the
-            # upstream target directly.
-            slug = request.GET.get("slug")
-            if not slug:
-                return HttpResponse("Missing slug parameter", status=400)
-            try:
-                feature_config = NimbusFeatureConfig.objects.get(slug=slug)
-            except NimbusFeatureConfig.DoesNotExist:
-                return HttpResponse("Feature not found", status=404)
-            parsed = urlparse(feature_config.feature_monitoring_url)
-            path = parsed.path.lstrip("/")
-            # Merge the monitoring URL's own query params with any extra params
-            # from the request (e.g. kiosk), excluding the slug itself.
-            params = parse_qs(parsed.query, keep_blank_values=True)
-            for key, value in request.GET.items():
-                if key != "slug":
-                    params[key] = [value]
-        else:
-            params = request.GET
-
-        target = f"{settings.GRAFANA_INTERNAL_URL.rstrip('/')}/{path}"
+    def get(self, request):
+        slug = request.GET.get("slug")
+        if not slug:
+            return HttpResponse("Missing slug parameter", status=400)
+
+        feature_config = get_object_or_404(NimbusFeatureConfig, slug=slug)
+
+        # Construct the Grafana URL entirely from known-safe components.
+        # User input (slug) is validated against the DB before use;
+        # the host and dashboard path come from settings only.
+        application = (feature_config.application or "").replace("-", "_")
+        target = "{base}/{path}".format(
+            base=settings.GRAFANA_INTERNAL_URL.rstrip("/"),
+            path=settings.GRAFANA_FEATURE_MONITORING_DASHBOARD_PATH,
+        )
+        params = {
+            "orgId": "1",
+            "from": "now-7d",
+            "to": "now",
+            "timezone": "utc",
+            "var-application": application,
+            "var-feature": feature_config.slug,
+            "kiosk": "",
+        }
+
         headers = {}
         if settings.GRAFANA_SERVICE_ACCOUNT_TOKEN:
             headers["Authorization"] = (
@@ -1216,17 +1215,13 @@ def get(self, request, path=""):
         except requests.exceptions.RequestException:
             return HttpResponse("Grafana is unavailable", status=503)
 
-        content_type = upstream.headers.get("Content-Type", "application/octet-stream")
-
-        if "text/html" in content_type:
-            body = self._rewrite_html(upstream.text)
-            response = HttpResponse(
-                body, content_type=content_type, status=upstream.status_code
-            )
-        else:
-            response = HttpResponse(
-                upstream.content, content_type=content_type, status=upstream.status_code
-            )
+        response = HttpResponse(
+            upstream.content,
+            content_type=upstream.headers.get(
+                "Content-Type", "application/octet-stream"
+            ),
+            status=upstream.status_code,
+        )
 
         for header in _SAFE_RESPONSE_HEADERS:
             if value := upstream.headers.get(header):
@@ -1235,23 +1230,6 @@ def get(self, request, path=""):
         response["X-Frame-Options"] = "SAMEORIGIN"
         return response
 
-    def _rewrite_html(self, html):
-        # Rewrite <base href> so relative asset paths (JS, CSS, fonts) resolve
-        # through our proxy instead of directly to Grafana's public URL.
-        html = re.sub(
-            r'(<base\s+href=")[^"]*(")',
-            rf"\g<1>{_GRAFANA_PROXY_PREFIX}\2",
-            html,
-        )
-        # Rewrite appUrl in window.grafanaBootData so Grafana's JS routes
-        # API calls (datasources, dashboards) through our proxy too.
-        html = re.sub(
-            r'("appUrl"\s*:\s*")[^"]*(")',
-            rf"\g<1>{_GRAFANA_PROXY_PREFIX}\2",
-            html,
-        )
-        return html
-
 
 class NimbusExperimentsHomeView(FilterView):
     template_name = "nimbus_experiments/home.html"
diff --git a/experimenter/experimenter/settings.py b/experimenter/experimenter/settings.py
@@ -444,6 +444,10 @@
     "GRAFANA_INTERNAL_URL",
     default="http://grafana.grafana-prod.svc.cluster.local:8080",
 )
+GRAFANA_FEATURE_MONITORING_DASHBOARD_PATH = config(
+    "GRAFANA_FEATURE_MONITORING_DASHBOARD_PATH",
+    default="d/dtfz7xv/nimbus-feature-monitoring",
+)
 GRAFANA_SERVICE_ACCOUNT_TOKEN = config("GRAFANA_SERVICE_ACCOUNT_TOKEN", default="")
 
 # Statsd via Markus
