diff --git a/digid_eherkenning/mock/conf.py b/digid_eherkenning/mock/conf.py index 71d68eb..0a69b21 100644 --- a/digid_eherkenning/mock/conf.py +++ b/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 diff --git a/digid_eherkenning/mock/idp/views/digid.py b/digid_eherkenning/mock/idp/views/digid.py index dc3701f..cdc7910 100644 --- a/digid_eherkenning/mock/idp/views/digid.py +++ b/digid_eherkenning/mock/idp/views/digid.py @@ -1,9 +1,11 @@ import logging from urllib.parse import urlencode -from django.http import HttpResponseBadRequest +from digid_eherkenning.views.base import get_redirect_url, is_relative_path +from django.http import HttpRequest, HttpResponseBadRequest from django.urls import reverse from django.views.generic import FormView, TemplateView +from django.utils.http import url_has_allowed_host_and_scheme from furl import furl @@ -16,6 +18,19 @@ 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,19 @@ def dispatch(self, request, *args, **kwargs): logger.debug("missing 'cancel' parameter") return HttpResponseBadRequest("missing 'cancel' parameter") + # The principal use-case of the mock IDP is to redirect from and back to the + # same service (simulating th real-world case where the IDP lives elsewhere). + # 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. + 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): diff --git a/digid_eherkenning/views/base.py b/digid_eherkenning/views/base.py index 3021fd4..ea0032e 100644 --- a/digid_eherkenning/views/base.py +++ b/digid_eherkenning/views/base.py @@ -1,3 +1,4 @@ +from urllib.parse import urlparse from django.utils.http import url_has_allowed_host_and_scheme @@ -18,3 +19,9 @@ def get_redirect_url(request, redirect_to, require_https=True): return redirect_to return "" + + +def is_relative_path(uri): + """Check if URI is a relative path by verifying it has no scheme or netloc""" + parsed = urlparse(uri) + return not (parsed.scheme or parsed.netloc) diff --git a/tests/test_mock_views.py b/tests/test_mock_views.py index 548b235..1b894b3 100644 --- a/tests/test_mock_views.py +++ b/tests/test_mock_views.py @@ -71,6 +71,40 @@ 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_must_be_safe(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_must_be_safe(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(**OVERRIDE_SETTINGS) @modify_settings(**MODIFY_SETTINGS)