Merge pull request #3590 from open-formulieren/upgrade/automatically-…

…create-csp-form-action-config

Set up automatic CSP form-action directive generation

src/openforms/config/management/commands/create_csp_form_action_directives_from_config.py

@@ -0,0 +1,29 @@
+from django.core.management import BaseCommand
+
+from digid_eherkenning.models import DigidConfiguration, EherkenningConfiguration
+
+from openforms.payments.contrib.ogone.models import OgoneMerchant
+
+
+class Command(BaseCommand):
+    help = (
+        "Introspect the runtime configuration and generate the relevant CSP "
+        "form-action directives"
+    )
+
+    def handle(self, **options):
+        ogone_merchants = OgoneMerchant.objects.exclude(
+            endpoint_preset="",
+            endpoint_custom="",
+        )
+        for ogone_merchant in ogone_merchants:
+            ogone_merchant.save()  # triggers CSPSetting record creation
+
+        for model_cls in (DigidConfiguration, EherkenningConfiguration):
+            configured_instance = model_cls.objects.exclude(
+                idp_metadata_file=""
+            ).first()
+            if configured_instance is None:
+                continue
+
+            configured_instance.save()  # triggers CSPSetting record creation

src/openforms/config/migrations/0060_create_csp_form_action_configs.py

@@ -0,0 +1,21 @@
+# Generated by Django 3.2.21 on 2023-11-09 08:58
+
+from django.core.management import call_command
+from django.db import migrations
+
+
+def call_management_command(apps, schema_editor):
+    call_command("create_csp_form_action_directives_from_config")
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ("config", "0059_convert_button_design_tokens"),
+        ("payments_ogone", "0002_auto_20210902_2120"),
+        ("digid_eherkenning", "0006_digidconfiguration_metadata_file_source_and_more"),
+    ]
+
+    operations = [
+        migrations.RunPython(call_management_command, migrations.RunPython.noop),
+    ]

src/openforms/config/tests/test_csp_update.py

@@ -1,5 +1,19 @@
+from io import StringIO
+
+from django.core.files.base import ContentFile
+from django.core.management import call_command
 from django.test import TestCase, override_settings
 
+from digid_eherkenning.models import DigidConfiguration, EherkenningConfiguration
+
+from openforms.contrib.digid_eherkenning.tests.test_csp_update import (
+    DIGID_METADATA_POST,
+    EHERKENNING_METADATA_POST,
+)
+from openforms.payments.contrib.ogone.models import OgoneMerchant
+from openforms.payments.contrib.ogone.tests.factories import OgoneMerchantFactory
+from openforms.utils.tests.cache import clear_caches
+
 from ..models import CSPSetting
 
 
@@ -27,3 +41,95 @@ def test_middleware_applies_cspsetting_models(self):
         self.assertIn("http://foo.bar", csp_policy["img-src"])
         self.assertIn("http://bazz.bar", csp_policy["img-src"])
         self.assertIn("http://buzz.bazz", csp_policy["default-src"])
+
+
+class CreateCSPFormActionFromConfigTests(TestCase):
+    @classmethod
+    def setUpTestData(cls):
+        super().setUpTestData()
+
+        # looks like there may be left over (migration) data from non-transaction testcases
+        DigidConfiguration.objects.all().delete()
+        EherkenningConfiguration.objects.all().delete()
+
+    def setUp(self):
+        super().setUp()
+
+        self.addCleanup(clear_caches)
+
+    def test_no_ogone_nor_digid_eherkenning_config(self):
+        assert not OgoneMerchant.objects.exists()
+        assert not DigidConfiguration.objects.exists()
+        assert not EherkenningConfiguration.objects.exists()
+
+        call_command(
+            "create_csp_form_action_directives_from_config",
+            stdout=StringIO(),
+            stderr=StringIO(),
+        )
+
+        self.assertFalse(CSPSetting.objects.exists())
+
+    def test_config_records_exist_but_are_incomplete(self):
+        OgoneMerchant.objects.create(
+            endpoint_preset="",  # not realistic, but the DB allows it
+            endpoint_custom="",
+        )
+        digid_configuration = DigidConfiguration.objects.create()
+        assert not digid_configuration.idp_metadata_file
+        eherkenning_configuration = EherkenningConfiguration.objects.create()
+        assert not eherkenning_configuration.idp_metadata_file
+        CSPSetting.objects.all().delete()  # delete creates from test data setup
+
+        call_command(
+            "create_csp_form_action_directives_from_config",
+            stdout=StringIO(),
+            stderr=StringIO(),
+        )
+
+        self.assertFalse(CSPSetting.objects.exists())
+
+    def test_config_created_when_ogone_merchants_with_url_exist(self):
+        ogone_merchant = OgoneMerchantFactory.create()
+        CSPSetting.objects.all().delete()  # delete creates from test data setup
+
+        call_command(
+            "create_csp_form_action_directives_from_config",
+            stdout=StringIO(),
+            stderr=StringIO(),
+        )
+
+        csp_setting = CSPSetting.objects.get()
+        self.assertEqual(csp_setting.content_object, ogone_merchant)
+
+    def test_digid_eherkenning_config_creates_cspsetting_records(self):
+        digid_metadata = ContentFile(
+            DIGID_METADATA_POST.read_bytes(), name="digid_metadata.xml"
+        )
+        digid_configuration = DigidConfiguration.objects.create(
+            idp_metadata_file=digid_metadata
+        )
+        assert digid_configuration.idp_metadata_file
+        eherkenning_metadata = ContentFile(
+            EHERKENNING_METADATA_POST.read_bytes(), name="eh_metadata.xml"
+        )
+        eherkenning_configuration = EherkenningConfiguration.objects.create(
+            idp_metadata_file=eherkenning_metadata
+        )
+        assert eherkenning_configuration.idp_metadata_file
+        CSPSetting.objects.all().delete()  # delete creates from test data setup
+
+        call_command(
+            "create_csp_form_action_directives_from_config",
+            stdout=StringIO(),
+            stderr=StringIO(),
+        )
+
+        csp_settings = CSPSetting.objects.all()
+        self.assertEqual(len(csp_settings), 2)
+
+        by_content_object = {
+            csp_setting.content_object: csp_setting for csp_setting in csp_settings
+        }
+        self.assertIn(digid_configuration, by_content_object)
+        self.assertIn(eherkenning_configuration, by_content_object)

src/openforms/contrib/digid_eherkenning/tests/test_csp_update.py

@@ -80,7 +80,7 @@ def test_csp_updates_for_digid(self, get_metadata):
 
         csp_updated = CSPSetting.objects.get()
 
-        self.assertEqual(get_metadata.call_count, 4)
+        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)
@@ -95,6 +95,7 @@ def test_csp_updates_for_digid(self, get_metadata):
     @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)
@@ -114,7 +115,7 @@ def test_response_headers_contain_form_action_values_in_digid(self, get_metadata
         # redirect_to_digid_login
         response = self.client.get(login_url, {"next": form_url}, follow=True)
 
-        self.assertEqual(get_metadata.call_count, 2)
+        self.assertEqual(get_metadata.call_count, 1)
         self.assertIn(
             "form-action "
             "'self' "
@@ -188,7 +189,7 @@ def test_csp_updates_for_eherkenning(self, get_metadata):
 
         csp_updated = CSPSetting.objects.get()
 
-        self.assertEqual(get_metadata.call_count, 4)
+        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(
@@ -200,6 +201,7 @@ def test_csp_updates_for_eherkenning(self, get_metadata):
     @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
     ):
@@ -225,7 +227,7 @@ def test_response_headers_contain_form_action_values_in_eherkenning(
         # redirect_to_eherkenning_login
         response = self.client.get(login_url, {"next": form_url}, follow=True)
 
-        self.assertEqual(get_metadata.call_count, 2)
+        self.assertEqual(get_metadata.call_count, 1)
         self.assertIn(
             "form-action "
             "'self' "

src/openforms/contrib/digid_eherkenning/utils.py

@@ -3,6 +3,8 @@
 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
@@ -27,18 +29,49 @@ def get_eherkenning_logo(request) -> Dict[str, str]:
     }
 
 
+def get_form_action_urls(
+    config: DigidConfiguration | EherkenningConfiguration,
+) -> list[str] | None:
+    """
+    Extract the <form> 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:
-    if not config.metadata_file_source:
-        return
-
     # create the new directives based on the POST bindings of the metadata XML and
     # the additional constants
-    urls, _ = config.process_metadata_from_xml_source()
-    csp_values = [urls["sso_url"], urls["slo_url"]]
-    if additional_csp_values := ADDITIONAL_CSP_VALUES.get(type(config), []):
-        csp_values.append(additional_csp_values)
 
-    form_action_urls = " ".join(csp_values)
+    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)])

src/openforms/payments/contrib/ogone/tests/test_plugin.py

@@ -15,7 +15,9 @@
 
 
 @override_settings(
-    CORS_ALLOW_ALL_ORIGINS=False, CORS_ALLOWED_ORIGINS=["http://foo.bar"]
+    CORS_ALLOW_ALL_ORIGINS=False,
+    CORS_ALLOWED_ORIGINS=["http://foo.bar"],
+    CSP_FORM_ACTION=["'self'"],
 )
 class OgoneTests(TestCase):
     maxDiff = None