From 62e0a9d8300d18284c0dfc4e68abaa6b62ed93ad Mon Sep 17 00:00:00 2001
From: Viicos <65306057+Viicos@users.noreply.github.com>
Date: Mon, 29 Apr 2024 15:46:08 +0200
Subject: [PATCH] [#4205] Allow any `https:` scheme for CSP `form-action`
directive
Backport-of: #4223
---
src/openforms/conf/base.py | 15 +-
src/openforms/config/tests/test_admin.py | 25 --
.../contrib/digid_eherkenning/apps.py | 4 -
.../contrib/digid_eherkenning/constants.py | 6 -
.../contrib/digid_eherkenning/signals.py | 14 -
.../digid_eherkenning/tests/__init__.py | 0
.../tests/data/eherkenning-metadata.xml | 97 -------
.../tests/data/metadata_POST_bindings.xml | 69 -----
.../tests/test_csp_update.py | 240 ------------------
.../contrib/digid_eherkenning/utils.py | 56 ----
.../payments/contrib/ogone/models.py | 9 -
.../contrib/ogone/tests/test_models.py | 81 ------
.../contrib/ogone/tests/test_plugin.py | 8 +-
13 files changed, 10 insertions(+), 614 deletions(-)
delete mode 100644 src/openforms/contrib/digid_eherkenning/constants.py
delete mode 100644 src/openforms/contrib/digid_eherkenning/signals.py
delete mode 100644 src/openforms/contrib/digid_eherkenning/tests/__init__.py
delete mode 100644 src/openforms/contrib/digid_eherkenning/tests/data/eherkenning-metadata.xml
delete mode 100644 src/openforms/contrib/digid_eherkenning/tests/data/metadata_POST_bindings.xml
delete mode 100644 src/openforms/contrib/digid_eherkenning/tests/test_csp_update.py
delete mode 100644 src/openforms/payments/contrib/ogone/tests/test_models.py
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