diff --git a/src/openforms/conf/base.py b/src/openforms/conf/base.py index 0e8e6a6e28..f2fa530911 100644 --- a/src/openforms/conf/base.py +++ b/src/openforms/conf/base.py @@ -1022,13 +1022,17 @@ "'self'", ] + config("CSP_EXTRA_DEFAULT_SRC", default=[], split=True) -# CORS_ALLOWED_ORIGINS is included because we (likely) need to redirect back to those -# third party domains that are embedding the SDK after login. Chrome in particular -# validates the entire redirect chain, see: https://stackoverflow.com/a/69439102 +# Allow any 'https:' host, as we don't know in advance which target is used by eHerkenning. +# Behavior is also different between browsers regarding redirects, see: +# https://stackoverflow.com/a/69439102 / https://github.com/w3c/webappsec-csp/issues/8 CSP_FORM_ACTION = ( - ["'self'"] + config( + "CSP_FORM_ACTION", + default=["\"'self'\"", "https:"] + + config("CSP_EXTRA_FORM_ACTION", default=[], split=True), + split=True, + ) + CORS_ALLOWED_ORIGINS - + config("CSP_EXTRA_FORM_ACTION", default=[], split=True) ) # * service.pdok.nl serves the tiles for the Leaflet maps (PNGs) and must be whitelisted @@ -1060,7 +1064,6 @@ CSP_FRAME_ANCESTORS = ["'none'"] # equivalent to X-Frame-Options: deny CSP_FRAME_SRC = ["'self'"] # CSP_NAVIGATE_TO = ["'self'"] # this will break all outgoing links etc # too much & tricky, see note on MDN -# CSP_FORM_ACTION = ["'self'"] # forms, possibly problematic with payments # CSP_SANDBOX # too much CSP_UPGRADE_INSECURE_REQUESTS = False # TODO enable on production? diff --git a/src/openforms/config/tests/test_admin.py b/src/openforms/config/tests/test_admin.py index bed4bf97c6..cc924ed832 100644 --- a/src/openforms/config/tests/test_admin.py +++ b/src/openforms/config/tests/test_admin.py @@ -1,38 +1,13 @@ -from django.contrib.admin.sites import AdminSite -from django.test import TestCase from django.urls import reverse from django_webtest import WebTest from maykin_2fa.test import disable_admin_mfa from openforms.accounts.tests.factories import SuperUserFactory -from openforms.config.models import CSPSetting -from openforms.payments.contrib.ogone.tests.factories import OgoneMerchantFactory -from ..admin import CSPSettingAdmin from .factories import RichTextColorFactory -class TestCSPAdmin(TestCase): - def test_content_type_link(self): - OgoneMerchantFactory() - - csp = CSPSetting.objects.get() - - admin_site = AdminSite() - admin = CSPSettingAdmin(CSPSetting, admin_site) - - expected_url = reverse( - "admin:payments_ogone_ogonemerchant_change", - kwargs={"object_id": str(csp.object_id)}, - ) - expected_link = f'{str(csp.content_object)}' - - link = admin.content_type_link(csp) - - self.assertEqual(link, expected_link) - - @disable_admin_mfa() class ColorAdminTests(WebTest): def test_color_changelist(self): diff --git a/src/openforms/contrib/digid_eherkenning/apps.py b/src/openforms/contrib/digid_eherkenning/apps.py index 370d446507..5d171cf5e2 100644 --- a/src/openforms/contrib/digid_eherkenning/apps.py +++ b/src/openforms/contrib/digid_eherkenning/apps.py @@ -6,7 +6,3 @@ class DigidEherkenningApp(AppConfig): name = "openforms.contrib.digid_eherkenning" label = "contrib_digid_eherkenning" verbose_name = _("DigiD/Eherkenning utilities") - - def ready(self): - # register the signals - from .signals import trigger_csp_update # noqa diff --git a/src/openforms/contrib/digid_eherkenning/constants.py b/src/openforms/contrib/digid_eherkenning/constants.py deleted file mode 100644 index fa9e7999ba..0000000000 --- a/src/openforms/contrib/digid_eherkenning/constants.py +++ /dev/null @@ -1,6 +0,0 @@ -from digid_eherkenning.models import DigidConfiguration, EherkenningConfiguration - -ADDITIONAL_CSP_VALUES = { - DigidConfiguration: "https://digid.nl https://*.digid.nl", - EherkenningConfiguration: "https://*.eherkenning.nl", -} diff --git a/src/openforms/contrib/digid_eherkenning/signals.py b/src/openforms/contrib/digid_eherkenning/signals.py deleted file mode 100644 index 079fd8c447..0000000000 --- a/src/openforms/contrib/digid_eherkenning/signals.py +++ /dev/null @@ -1,14 +0,0 @@ -from django.db.models.signals import post_save -from django.dispatch import receiver - -from digid_eherkenning.models import DigidConfiguration, EherkenningConfiguration - -from .utils import create_digid_eherkenning_csp_settings - - -@receiver(post_save, sender=DigidConfiguration) -@receiver(post_save, sender=EherkenningConfiguration) -def trigger_csp_update( - sender, instance: DigidConfiguration | EherkenningConfiguration, **kwargs -): - create_digid_eherkenning_csp_settings(instance) diff --git a/src/openforms/contrib/digid_eherkenning/tests/__init__.py b/src/openforms/contrib/digid_eherkenning/tests/__init__.py deleted file mode 100644 index e69de29bb2..0000000000 diff --git a/src/openforms/contrib/digid_eherkenning/tests/data/eherkenning-metadata.xml b/src/openforms/contrib/digid_eherkenning/tests/data/eherkenning-metadata.xml deleted file mode 100644 index 34b0a18cea..0000000000 --- a/src/openforms/contrib/digid_eherkenning/tests/data/eherkenning-metadata.xml +++ /dev/null @@ -1,97 +0,0 @@ - - - - - - - - - - -mTSuC+50WptA9sO0wg/eFIyQtWofMlkbEYm5Zoj/GHo= - - - - - - - - - - - - - - urn:etoegang:core:assurance-class:loa4 - - - - - - - Test key 0 - - - - - - - - Test key 1 - - - - - - - - - - - urn:etoegang:1.9:EntityConcernedID:KvKnr - urn:etoegang:1.9:IntermediateEntityID:KvKnr - urn:etoegang:1.9:EntityConcernedID:Pseudo - urn:etoegang:1.9:EntityConcernedID:RSIN - urn:etoegang:1.9:IntermediateEntityID:RSIN - urn:etoegang:1.11:EntityConcernedID:eIDASLegalIdentifier - urn:etoegang:1.12:EntityConcernedID:BSN - urn:etoegang:1.12:EntityConcernedID:PseudoID - - - - - - - - Test key 2 - - - - - - - - Test key 3 - - - - - - - - - - - - - - - Test-iWelcome - Test-iWelcome - www.test-iwelcome.nl - - - support@test-iwelcome.nl - 000 222 111 - - diff --git a/src/openforms/contrib/digid_eherkenning/tests/data/metadata_POST_bindings.xml b/src/openforms/contrib/digid_eherkenning/tests/data/metadata_POST_bindings.xml deleted file mode 100644 index 6ccc6a4ad8..0000000000 --- a/src/openforms/contrib/digid_eherkenning/tests/data/metadata_POST_bindings.xml +++ /dev/null @@ -1,69 +0,0 @@ - - - - - - - - - - - - - - - zIJH9lctgfbY1SLzbOZhOo2FN/qSqDi20MTd2OYN+qs= - - - - - - - Test key 0 - - - - - - Test key 1 - - - - - - - - - - Test key 2 - - - - - - - - - - - - - - \ No newline at end of file diff --git a/src/openforms/contrib/digid_eherkenning/tests/test_csp_update.py b/src/openforms/contrib/digid_eherkenning/tests/test_csp_update.py deleted file mode 100644 index 9847d3b15e..0000000000 --- a/src/openforms/contrib/digid_eherkenning/tests/test_csp_update.py +++ /dev/null @@ -1,240 +0,0 @@ -from pathlib import Path -from unittest.mock import patch - -from django.test import TestCase, override_settings -from django.urls import reverse - -from digid_eherkenning.models import DigidConfiguration, EherkenningConfiguration -from privates.test import temp_private_root -from simple_certmanager.test.factories import CertificateFactory - -from openforms.config.constants import CSPDirective -from openforms.config.models import CSPSetting -from openforms.forms.tests.factories import ( - FormDefinitionFactory, - FormFactory, - FormStepFactory, -) -from openforms.utils.tests.cache import clear_caches - -TEST_FILES = Path(__file__).parent / "data" -DIGID_METADATA_POST = TEST_FILES / "metadata_POST_bindings.xml" -EHERKENNING_METADATA_POST = TEST_FILES / "eherkenning-metadata.xml" - - -@temp_private_root() -@override_settings(CORS_ALLOW_ALL_ORIGINS=True, IS_HTTPS=True) -class DigidCSPUpdateTests(TestCase): - @classmethod - def setUpTestData(cls): - super().setUpTestData() - - cert = CertificateFactory.create(label="DigiD", with_private_key=True) - - cls.config = DigidConfiguration.get_solo() - - cls.config.certificate = cert - cls.config.idp_service_entity_id = "https://test-digid.nl" - cls.config.want_assertions_signed = False - cls.config.entity_id = "https://test-sp.nl" - cls.config.base_url = "https://test-sp.nl" - cls.config.service_name = "Test" - cls.config.service_description = "Test description" - cls.config.slo = False - cls.config.save() - - def setUp(self): - super().setUp() - - clear_caches() - self.addCleanup(clear_caches) - - @patch( - "onelogin.saml2.idp_metadata_parser.OneLogin_Saml2_IdPMetadataParser.get_metadata" - ) - def test_csp_updates_for_digid(self, get_metadata): - # assert no csp entries exist with initial solo model - self.assertTrue(CSPSetting.objects.none) - - # assert new csp entry is added after adding metadata url - metadata_content = DIGID_METADATA_POST.read_text("utf-8") - get_metadata.return_value = metadata_content - self.config.metadata_file_source = "https://test-digid.nl" - self.config.save() - - csp_added = CSPSetting.objects.get() - - self.assertEqual(csp_added.content_object, self.config) - self.assertEqual(csp_added.directive, CSPDirective.FORM_ACTION) - self.assertEqual( - csp_added.value, - "https://test-digid.nl/saml/idp/request_authentication " - "https://test-digid.nl/saml/idp/request_logout " - "https://digid.nl " - "https://*.digid.nl", - ) - - # assert new csp entry is added and the old one is deleted after url update - self.config.metadata_file_source = "https://test-digid-post-bindings.nl" - self.config.save() - - csp_updated = CSPSetting.objects.get() - - self.assertEqual(get_metadata.call_count, 2) - self.assertFalse(CSPSetting.objects.filter(id=csp_added.id)) - self.assertEqual(csp_updated.content_object, self.config) - self.assertEqual(csp_updated.directive, CSPDirective.FORM_ACTION) - self.assertEqual( - csp_updated.value, - "https://test-digid.nl/saml/idp/request_authentication " - "https://test-digid.nl/saml/idp/request_logout " - "https://digid.nl " - "https://*.digid.nl", - ) - - @patch( - "onelogin.saml2.idp_metadata_parser.OneLogin_Saml2_IdPMetadataParser.get_metadata" - ) - @override_settings(CSP_FORM_ACTION=["'self'"]) - def test_response_headers_contain_form_action_values_in_digid(self, get_metadata): - form = FormFactory.create(authentication_backends=["digid"]) - form_definition = FormDefinitionFactory.create(login_required=True) - FormStepFactory.create(form_definition=form_definition, form=form) - - metadata_content = DIGID_METADATA_POST.read_text("utf-8") - get_metadata.return_value = metadata_content - self.config.metadata_file_source = "https://test-digid.nl" - self.config.save() - - login_url = reverse( - "authentication:start", kwargs={"slug": form.slug, "plugin_id": "digid"} - ) - form_path = reverse("core:form-detail", kwargs={"slug": form.slug}) - form_url = f"http://testserver{form_path}?_start=1" - - # redirect_to_digid_login - response = self.client.get(login_url, {"next": form_url}, follow=True) - - self.assertEqual(get_metadata.call_count, 1) - self.assertIn( - "form-action " - "'self' " - "https://test-digid.nl/saml/idp/request_authentication " - "https://test-digid.nl/saml/idp/request_logout " - "https://digid.nl " - "https://*.digid.nl;", - response.headers["Content-Security-Policy"], - ) - - -@temp_private_root() -class EherkenningCSPUpdateTests(TestCase): - @classmethod - def setUpTestData(cls): - super().setUpTestData() - - cert = CertificateFactory.create(label="eHerkenning", with_private_key=True) - - cls.config = EherkenningConfiguration.get_solo() - - cls.config.certificate = cert - cls.config.idp_service_entity_id = ( - "urn:etoegang:DV:00000001111111111000:entities:9000" - ) - cls.config.want_assertions_signed = False - cls.config.entity_id = "https://test-sp.nl" - cls.config.base_url = "https://test-sp.nl" - cls.config.service_name = "Test" - cls.config.service_description = "Test" - cls.config.loa = "urn:etoegang:core:assurance-class:loa3" - cls.config.oin = "00000001111111111000" - cls.config.no_eidas = True - cls.config.privacy_policy = "https://test-sp.nl/privacy_policy" - cls.config.makelaar_id = "00000002222222222000" - cls.config.organization_name = "Test Organisation" - cls.config.save() - - def setUp(self): - super().setUp() - - clear_caches() - self.addCleanup(clear_caches) - - @patch( - "onelogin.saml2.idp_metadata_parser.OneLogin_Saml2_IdPMetadataParser.get_metadata" - ) - def test_csp_updates_for_eherkenning(self, get_metadata): - # assert no csp entries exist with initial solo model - self.assertTrue(CSPSetting.objects.none) - - # assert new csp entry is added after adding metadata url - - metadata_content = EHERKENNING_METADATA_POST.read_text("utf-8") - get_metadata.return_value = metadata_content - self.config.metadata_file_source = "https://test-sp.nl" - self.config.save() - - csp_added = CSPSetting.objects.get() - - self.assertEqual(csp_added.directive, CSPDirective.FORM_ACTION) - self.assertEqual( - csp_added.value, - "https://test-iwelcome.nl/broker/sso/1.13 " - "https://ehm01.iwelcome.nl/broker/slo/1.13 " - "https://*.eherkenning.nl", - ) - - # assert new csp entry is added and old one is deleted after url update - self.config.metadata_file_source = "https://updated-test-sp.nl" - self.config.save() - - csp_updated = CSPSetting.objects.get() - - self.assertEqual(get_metadata.call_count, 2) - self.assertFalse(CSPSetting.objects.filter(id=csp_added.id)) - self.assertEqual(csp_added.directive, CSPDirective.FORM_ACTION) - self.assertEqual( - csp_updated.value, - "https://test-iwelcome.nl/broker/sso/1.13 " - "https://ehm01.iwelcome.nl/broker/slo/1.13 " - "https://*.eherkenning.nl", - ) - - @patch( - "onelogin.saml2.idp_metadata_parser.OneLogin_Saml2_IdPMetadataParser.get_metadata" - ) - @override_settings(CSP_FORM_ACTION=["'self'"]) - def test_response_headers_contain_form_action_values_in_eherkenning( - self, get_metadata - ): - form = FormFactory.create( - authentication_backends=["eherkenning"], - generate_minimal_setup=True, - formstep__form_definition__login_required=True, - ) - metadata_content = EHERKENNING_METADATA_POST.read_text("utf-8") - get_metadata.return_value = metadata_content - self.config.metadata_file_source = ( - "urn:etoegang:DV:00000001111111111000:entities:9000" - ) - self.config.save() - - login_url = reverse( - "authentication:start", - kwargs={"slug": form.slug, "plugin_id": "eherkenning"}, - ) - form_path = reverse("core:form-detail", kwargs={"slug": form.slug}) - form_url = f"http://testserver{form_path}" - - # redirect_to_eherkenning_login - response = self.client.get(login_url, {"next": form_url}, follow=True) - - self.assertEqual(get_metadata.call_count, 1) - self.assertIn( - "form-action " - "'self' " - "https://test-iwelcome.nl/broker/sso/1.13 " - "https://ehm01.iwelcome.nl/broker/slo/1.13 " - "https://*.eherkenning.nl;", - response.headers["Content-Security-Policy"], - ) diff --git a/src/openforms/contrib/digid_eherkenning/utils.py b/src/openforms/contrib/digid_eherkenning/utils.py index ef14f33ad2..fae71b3a89 100644 --- a/src/openforms/contrib/digid_eherkenning/utils.py +++ b/src/openforms/contrib/digid_eherkenning/utils.py @@ -1,14 +1,6 @@ from django.templatetags.static import static -from digid_eherkenning.models import DigidConfiguration, EherkenningConfiguration -from onelogin.saml2.constants import OneLogin_Saml2_Constants -from onelogin.saml2.idp_metadata_parser import OneLogin_Saml2_IdPMetadataParser - from openforms.authentication.constants import LogoAppearance -from openforms.config.constants import CSPDirective -from openforms.config.models import CSPSetting - -from .constants import ADDITIONAL_CSP_VALUES def get_digid_logo(request) -> dict[str, str]: @@ -25,51 +17,3 @@ def get_eherkenning_logo(request) -> dict[str, str]: "href": "https://www.eherkenning.nl/", "appearance": LogoAppearance.light, } - - -def get_form_action_urls( - config: DigidConfiguration | EherkenningConfiguration, -) -> list[str] | None: - """ - Extract the
post action URLs from the configuration IDP metadata. - - This is a slightly altered version of the config.process_metadata_from_xml_source - method which takes into account existing metadata without configured source. - """ - if not config.idp_metadata_file: - return None - - with config.idp_metadata_file.open("r") as infile: - xml_metadata = infile.read() - - parsed_idp_metadata = OneLogin_Saml2_IdPMetadataParser.parse( - xml_metadata, - required_sso_binding=OneLogin_Saml2_Constants.BINDING_HTTP_POST, - required_slo_binding=OneLogin_Saml2_Constants.BINDING_HTTP_POST, - ) - if not (idp := parsed_idp_metadata.get("idp")): - return None - - urls = [] - for key in ("singleSignOnService", "singleLogoutService"): - if url := idp.get(key, {}).get("url"): - urls.append(url) - - return urls - - -def create_digid_eherkenning_csp_settings( - config: DigidConfiguration | EherkenningConfiguration, -) -> None: - # create the new directives based on the POST bindings of the metadata XML and - # the additional constants - - urls = get_form_action_urls(config) - if not urls: - return - - if additional_csp_values := ADDITIONAL_CSP_VALUES.get(type(config), ""): - urls.append(additional_csp_values) - - form_action_urls = " ".join(urls) - CSPSetting.objects.set_for(config, [(CSPDirective.FORM_ACTION, form_action_urls)]) diff --git a/src/openforms/payments/contrib/ogone/models.py b/src/openforms/payments/contrib/ogone/models.py index 31d394f4f5..c20f37d055 100644 --- a/src/openforms/payments/contrib/ogone/models.py +++ b/src/openforms/payments/contrib/ogone/models.py @@ -2,8 +2,6 @@ from django.db import models from django.utils.translation import gettext_lazy as _ -from openforms.config.constants import CSPDirective - from .constants import HashAlgorithm, OgoneEndpoints @@ -63,10 +61,3 @@ def clean(self): def __str__(self): return self.label - - def save(self, *args, **kwargs): - super().save(*args, **kwargs) - - from openforms.config.models import CSPSetting - - CSPSetting.objects.set_for(self, [(CSPDirective.FORM_ACTION, self.endpoint)]) diff --git a/src/openforms/payments/contrib/ogone/tests/test_models.py b/src/openforms/payments/contrib/ogone/tests/test_models.py deleted file mode 100644 index 37d54f2724..0000000000 --- a/src/openforms/payments/contrib/ogone/tests/test_models.py +++ /dev/null @@ -1,81 +0,0 @@ -from django.test import TestCase - -from openforms.config.constants import CSPDirective -from openforms.config.models import CSPSetting - -from ..constants import OgoneEndpoints -from .factories import OgoneMerchantFactory - - -class CSPUpdateTests(TestCase): - def test_csp_is_saved_for_new_merchant_and_url_preset(self): - self.assertTrue(CSPSetting.objects.none) - - merchant = OgoneMerchantFactory() - - csp = CSPSetting.objects.get() - - self.assertEqual(csp.content_object, merchant) - self.assertEqual(csp.directive, CSPDirective.FORM_ACTION) - self.assertEqual(csp.value, OgoneEndpoints.test) - - def test_csp_is_saved_for_new_merchant_and_custom_url(self): - self.assertTrue(CSPSetting.objects.none) - - merchant = OgoneMerchantFactory(endpoint_custom="http://example.com") - - csp = CSPSetting.objects.get() - - self.assertEqual(csp.content_object, merchant) - self.assertEqual(csp.directive, CSPDirective.FORM_ACTION) - self.assertEqual(csp.value, "http://example.com") - - def test_csp_is_updated_for_existing_merchant_and_url_preset(self): - self.assertTrue(CSPSetting.objects.none) - - # initial values - merchant = OgoneMerchantFactory() - - csp = CSPSetting.objects.get() - - self.assertEqual(csp.content_object, merchant) - self.assertEqual(csp.directive, CSPDirective.FORM_ACTION) - self.assertEqual(csp.value, OgoneEndpoints.test) - - # updated values - merchant.endpoint_preset = OgoneEndpoints.live - merchant.save() - - # assert the original value has been deleted - self.assertEqual(CSPSetting.objects.count(), 1) - - csp = CSPSetting.objects.get() - - self.assertEqual(csp.content_object, merchant) - self.assertEqual(csp.directive, CSPDirective.FORM_ACTION) - self.assertEqual(csp.value, OgoneEndpoints.live) - - def test_csp_is_updated_for_existing_merchant_and_custom_url(self): - self.assertTrue(CSPSetting.objects.none) - - # initial values - merchant = OgoneMerchantFactory(endpoint_custom="http://example.com") - - csp = CSPSetting.objects.get() - - self.assertEqual(csp.content_object, merchant) - self.assertEqual(csp.directive, CSPDirective.FORM_ACTION) - self.assertEqual(csp.value, "http://example.com") - - # updated values - merchant.endpoint_preset = OgoneEndpoints.live - merchant.save() - - # assert the original value has been deleted - self.assertEqual(CSPSetting.objects.count(), 1) - - csp = CSPSetting.objects.get() - - self.assertEqual(csp.content_object, merchant) - self.assertEqual(csp.directive, CSPDirective.FORM_ACTION) - self.assertEqual(csp.value, "http://example.com") diff --git a/src/openforms/payments/contrib/ogone/tests/test_plugin.py b/src/openforms/payments/contrib/ogone/tests/test_plugin.py index 25aec5bc04..57966381b4 100644 --- a/src/openforms/payments/contrib/ogone/tests/test_plugin.py +++ b/src/openforms/payments/contrib/ogone/tests/test_plugin.py @@ -10,7 +10,7 @@ from ....registry import register from ....tests.factories import SubmissionPaymentFactory -from ..constants import OgoneEndpoints, OgoneStatus, PaymentStatus +from ..constants import OgoneStatus, PaymentStatus from ..plugin import RETURN_ACTION_PARAM from ..signing import calculate_sha_out from .factories import OgoneMerchantFactory @@ -174,12 +174,6 @@ def test_webhook(self): self.assertEqual(response.status_code, 200) - # assert created csp header is in the response as well - self.assertIn( - f"form-action 'self' {OgoneEndpoints.test};", - response.headers["Content-Security-Policy"], - ) - submission.refresh_from_db() payment.refresh_from_db() self.assertEqual(payment.status, PaymentStatus.completed)