✨ Optionally validate DigiD mock IDP callback urls

digid_eherkenning/mock/conf.py

@@ -1,4 +1,5 @@
 from django.conf import settings
+from django.core.exceptions import ImproperlyConfigured
 from django.shortcuts import resolve_url
 from django.urls import reverse
 from django.utils.encoding import force_str
@@ -45,3 +46,16 @@ def get_idp_login_url():
         "digid-mock:login"
     )
     return resolve_url(url)
+
+
+def should_validate_idp_callback_urls() -> bool:
+    """
+    Whether to validate that the `next_url` and `cancel_urls` are safe
+    """
+    flag = getattr(settings, "DIGID_MOCK_IDP_VALIDATE_CALLBACK_URLS", False)
+    if not isinstance(flag, bool):
+        raise ImproperlyConfigured(
+            "DIGID_MOCK_IDP_VALIDATE_CALLBACK_URLS should be a boolean"
+        )
+
+    return flag

digid_eherkenning/mock/idp/views/digid.py

@@ -1,21 +1,36 @@
 import logging
 from urllib.parse import urlencode
 
-from django.http import HttpResponseBadRequest
+from django.http import HttpRequest, HttpResponseBadRequest
 from django.urls import reverse
+from django.utils.http import url_has_allowed_host_and_scheme
 from django.views.generic import FormView, TemplateView
 
 from furl import furl
 
 from digid_eherkenning.mock import conf
 from digid_eherkenning.mock.idp.forms import PasswordLoginForm
+from digid_eherkenning.views.base import get_redirect_url, is_relative_path
 
 logger = logging.getLogger(__name__)
 
 
 class _BaseIDPViewMixin(TemplateView):
     page_title = "DigiD: Inloggen"
 
+    @staticmethod
+    def _is_safe_callback(request: HttpRequest, url: str):
+        if is_relative_path(url):
+            return True
+
+        return url_has_allowed_host_and_scheme(
+            url=url,
+            allowed_hosts=[
+                request.get_host(),
+            ],
+            require_https=False,
+        )
+
     def dispatch(self, request, *args, **kwargs):
         # we pass these variables through the URL instead of dealing with POST and sessions
         self.acs_url = self.request.GET.get("acs")
@@ -34,6 +49,21 @@ def dispatch(self, request, *args, **kwargs):
             logger.debug("missing 'cancel' parameter")
             return HttpResponseBadRequest("missing 'cancel' parameter")
 
+        # The principal use-case is that the mock IDP redirect flow all takes place
+        # within the same service. This should generally only be used in development,
+        # but out of an abundance of caution, and also to please security scanners, we
+        # only allow redirects to the same host that is serving the mock IDP should the
+        # IDP be active in world-facing environments (e.g. in acceptance). This behavior
+        # can be disabled with the DIGID_MOCK_IDP_VALIDATE_CALLBACK_URLS setting.
+        if conf.should_validate_idp_callback_urls():
+            if not self._is_safe_callback(request, self.next_url):
+                return HttpResponseBadRequest("'next_url' parameter must be a safe url")
+
+            if not self._is_safe_callback(request, self.cancel_url):
+                return HttpResponseBadRequest(
+                    "'cancel_url' parameter must be a safe url"
+                )
+
         return super().dispatch(request, *args, **kwargs)
 
     def get_context_data(self, **kwargs):

digid_eherkenning/views/base.py

@@ -1,3 +1,5 @@
+from urllib.parse import urlparse
+
 from django.utils.http import url_has_allowed_host_and_scheme
 
 
@@ -18,3 +20,9 @@ def get_redirect_url(request, redirect_to, require_https=True):
         return redirect_to
 
     return ""
+
+
+def is_relative_path(uri: str) -> bool:
+    """Check if `uri` is a relative path."""
+    parsed = urlparse(uri)
+    return not (parsed.scheme or parsed.netloc)

tests/test_mock_views.py

@@ -6,6 +6,11 @@
 
 from furl import furl
 
+from digid_eherkenning.mock.conf import (
+    ImproperlyConfigured,
+    should_validate_idp_callback_urls,
+)
+
 
 class DigidMockTestCase(TestCase):
     def assertNoDigidURLS(self, response):
@@ -71,6 +76,67 @@ def test_get_returns_valid_response(self):
         self.assertContains(response, reverse("digid-mock:password"))
         self.assertNoDigidURLS(response)
 
+    @override_settings(DIGID_MOCK_IDP_VALIDATE_CALLBACK_URLS=True)
+    def test_cancel_url_cannot_have_different_host(self):
+        url = reverse("digid-mock:login")
+        data = {
+            "acs": reverse("digid:acs"),
+            "next": reverse("test-success"),
+            "cancel": "http://some-other-testserver" + reverse("test-index"),
+        }
+        response = self.client.get(url, data=data)
+
+        self.assertEqual(response.status_code, 400)
+        self.assertEqual(response.content, b"'cancel_url' parameter must be a safe url")
+
+        with override_settings(DIGID_MOCK_IDP_VALIDATE_CALLBACK_URLS=False):
+            response = self.client.get(url, data=data)
+            self.assertEqual(response.status_code, 200)
+
+    @override_settings(DIGID_MOCK_IDP_VALIDATE_CALLBACK_URLS=True)
+    def test_next_url_cannot_have_different_host(self):
+        url = reverse("digid-mock:login")
+        data = {
+            "acs": reverse("digid:acs"),
+            "next": "some-other-http://testserver" + reverse("test-success"),
+            "cancel": reverse("test-index"),
+        }
+        response = self.client.get(url, data=data)
+
+        self.assertEqual(response.status_code, 400)
+        self.assertEqual(response.content, b"'next_url' parameter must be a safe url")
+
+        with override_settings(DIGID_MOCK_IDP_VALIDATE_CALLBACK_URLS=False):
+            response = self.client.get(url, data=data)
+            self.assertEqual(response.status_code, 200)
+
+    @override_settings(DIGID_MOCK_IDP_VALIDATE_CALLBACK_URLS=True)
+    def test_next_and_cancel_url_can_be_relative(self):
+        url = reverse("digid-mock:login")
+        data = {
+            "acs": reverse("digid:acs"),
+            "next": reverse("test-success"),
+            "cancel": reverse("test-index"),
+        }
+        response = self.client.get(url, data=data)
+        self.assertEqual(response.status_code, 200)
+
+    @override_settings(DIGID_MOCK_IDP_VALIDATE_CALLBACK_URLS=True)
+    def test_next_and_cancel_url_can_be_absolute_if_same_host(self):
+        url = reverse("digid-mock:login")
+        data = {
+            "acs": reverse("digid:acs"),
+            "next": "http://testserver" + reverse("test-success"),
+            "cancel": "http://testserver" + reverse("test-index"),
+        }
+        response = self.client.get(url, data=data)
+        self.assertEqual(response.status_code, 200)
+
+    @override_settings(DIGID_MOCK_IDP_VALIDATE_CALLBACK_URLS="True")
+    def test_conf_setting_must_be_a_bool(self):
+        with self.assertRaises(ImproperlyConfigured):
+            should_validate_idp_callback_urls()
+
 
 @override_settings(**OVERRIDE_SETTINGS)
 @modify_settings(**MODIFY_SETTINGS)