From 51c46fa4a68fb5262c49874c32826e598f9b8398 Mon Sep 17 00:00:00 2001 From: vasileios Date: Wed, 25 Oct 2023 09:43:23 +0200 Subject: [PATCH 1/6] [#3477] Added csp settings for form-action to digid,eherkenning,ogone Modified CSPSetting model in order to connect it with each related instance. For now, digid, eherkenning and ogone are using it. --- requirements/base.txt | 2 +- requirements/ci.txt | 2 +- requirements/dev.txt | 2 +- requirements/extensions.txt | 2 +- .../authentication/contrib/digid/apps.py | 3 + .../authentication/contrib/digid/signals.py | 11 ++ .../tests/data/metadata_POST_bindings.xml | 69 ++++++++++ .../contrib/digid/tests/test_csp_update.py | 125 ++++++++++++++++++ .../contrib/eherkenning/apps.py | 3 + .../contrib/eherkenning/signals.py | 11 ++ .../eherkenning/tests/test_csp_update.py | 123 +++++++++++++++++ src/openforms/conf/base.py | 2 + src/openforms/config/admin.py | 12 ++ src/openforms/config/constants.py | 5 + .../migrations/0058_auto_20231024_1110.py | 39 ++++++ src/openforms/config/models.py | 18 ++- src/openforms/config/tests/test_admin.py | 28 ++++ src/openforms/config/utils.py | 46 +++++++ .../payments/contrib/ogone/models.py | 9 ++ .../contrib/ogone/tests/test_models.py | 81 ++++++++++++ .../contrib/ogone/tests/test_plugin.py | 8 +- 21 files changed, 595 insertions(+), 6 deletions(-) create mode 100644 src/openforms/authentication/contrib/digid/signals.py create mode 100644 src/openforms/authentication/contrib/digid/tests/data/metadata_POST_bindings.xml create mode 100644 src/openforms/authentication/contrib/digid/tests/test_csp_update.py create mode 100644 src/openforms/authentication/contrib/eherkenning/signals.py create mode 100644 src/openforms/authentication/contrib/eherkenning/tests/test_csp_update.py create mode 100644 src/openforms/config/migrations/0058_auto_20231024_1110.py create mode 100644 src/openforms/config/tests/test_admin.py create mode 100644 src/openforms/payments/contrib/ogone/tests/test_models.py diff --git a/requirements/base.txt b/requirements/base.txt index 676805278f..07cc98b6d6 100644 --- a/requirements/base.txt +++ b/requirements/base.txt @@ -163,7 +163,7 @@ django-csp-reports==1.8.1 # via -r requirements/base.in django-decorator-include==3.0 # via -r requirements/base.in -django-digid-eherkenning==0.8.2 +django-digid-eherkenning==0.9.0 # via -r requirements/base.in django-filter==23.2 # via -r requirements/base.in diff --git a/requirements/ci.txt b/requirements/ci.txt index 6cfbc69950..1f8def20fa 100644 --- a/requirements/ci.txt +++ b/requirements/ci.txt @@ -264,7 +264,7 @@ django-decorator-include==3.0 # via # -c requirements/base.txt # -r requirements/base.txt -django-digid-eherkenning==0.8.2 +django-digid-eherkenning==0.9.0 # via # -c requirements/base.txt # -r requirements/base.txt diff --git a/requirements/dev.txt b/requirements/dev.txt index 541af6f679..7cae3c8042 100644 --- a/requirements/dev.txt +++ b/requirements/dev.txt @@ -295,7 +295,7 @@ django-decorator-include==3.0 # via # -c requirements/ci.txt # -r requirements/ci.txt -django-digid-eherkenning==0.8.2 +django-digid-eherkenning==0.9.0 # via # -c requirements/ci.txt # -r requirements/ci.txt diff --git a/requirements/extensions.txt b/requirements/extensions.txt index c30ffabd8e..e2c97f48de 100644 --- a/requirements/extensions.txt +++ b/requirements/extensions.txt @@ -230,7 +230,7 @@ django-decorator-include==3.0 # via # -c requirements/base.in # -r requirements/base.txt -django-digid-eherkenning==0.8.2 +django-digid-eherkenning==0.9.0 # via # -c requirements/base.in # -r requirements/base.txt diff --git a/src/openforms/authentication/contrib/digid/apps.py b/src/openforms/authentication/contrib/digid/apps.py index 441c3aa3e1..dadc987992 100644 --- a/src/openforms/authentication/contrib/digid/apps.py +++ b/src/openforms/authentication/contrib/digid/apps.py @@ -10,3 +10,6 @@ class DigidApp(AppConfig): def ready(self): # register the plugin from . import plugin # noqa + + # register the signals + from .signals import trigger_csp_update # noqa diff --git a/src/openforms/authentication/contrib/digid/signals.py b/src/openforms/authentication/contrib/digid/signals.py new file mode 100644 index 0000000000..712efad6e4 --- /dev/null +++ b/src/openforms/authentication/contrib/digid/signals.py @@ -0,0 +1,11 @@ +from django.db.models.signals import post_save +from django.dispatch import receiver + +from digid_eherkenning.models.digid import DigidConfiguration + +from openforms.config.utils import create_digid_eherkenning_csp_settings + + +@receiver(post_save, sender=DigidConfiguration) +def trigger_csp_update(sender, instance, **kwargs): + create_digid_eherkenning_csp_settings(instance, "digid") diff --git a/src/openforms/authentication/contrib/digid/tests/data/metadata_POST_bindings.xml b/src/openforms/authentication/contrib/digid/tests/data/metadata_POST_bindings.xml new file mode 100644 index 0000000000..6ccc6a4ad8 --- /dev/null +++ b/src/openforms/authentication/contrib/digid/tests/data/metadata_POST_bindings.xml @@ -0,0 +1,69 @@ + + + + + + + + + + + + + + + zIJH9lctgfbY1SLzbOZhOo2FN/qSqDi20MTd2OYN+qs= + + + + + + + Test key 0 + + + + + + Test key 1 + + + + + + + + + + Test key 2 + + + + + + + + + + + + + + \ No newline at end of file diff --git a/src/openforms/authentication/contrib/digid/tests/test_csp_update.py b/src/openforms/authentication/contrib/digid/tests/test_csp_update.py new file mode 100644 index 0000000000..3313d4af79 --- /dev/null +++ b/src/openforms/authentication/contrib/digid/tests/test_csp_update.py @@ -0,0 +1,125 @@ +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.digid import DigidConfiguration +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" +METADATA_POST = TEST_FILES / "metadata_POST_bindings.xml" + + +@temp_private_root() +@override_settings(CORS_ALLOW_ALL_ORIGINS=True, IS_HTTPS=True) +class CSPUpdateTests(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(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 + with METADATA_POST.open("rb") as md_file: + metadata_content = md_file.read().decode("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://digid.nl " + "https://*.digid.nl " + "https://test-digid.nl/saml/idp/request_authentication " + "https://test-digid.nl/saml/idp/request_logout", + ) + + # 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.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://digid.nl " + "https://*.digid.nl " + "https://test-digid.nl/saml/idp/request_authentication " + "https://test-digid.nl/saml/idp/request_logout", + ) + + @patch( + "onelogin.saml2.idp_metadata_parser.OneLogin_Saml2_IdPMetadataParser.get_metadata" + ) + def test_response_headers_contain_form_action_values(self, get_metadata): + form = FormFactory.create(authentication_backends=["digid"]) + form_definition = FormDefinitionFactory.create(login_required=True) + FormStepFactory.create(form_definition=form_definition, form=form) + + with METADATA_POST.open("rb") as md_file: + metadata_content = md_file.read().decode("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(f"{login_url}?next={form_url}", follow=True) + + self.assertIn( + "form-action " + "'self' " + "https://digid.nl " + "https://*.digid.nl " + "https://test-digid.nl/saml/idp/request_authentication " + "https://test-digid.nl/saml/idp/request_logout;", + response.headers["Content-Security-Policy"], + ) diff --git a/src/openforms/authentication/contrib/eherkenning/apps.py b/src/openforms/authentication/contrib/eherkenning/apps.py index 20fa4b3e92..a53e681ab2 100644 --- a/src/openforms/authentication/contrib/eherkenning/apps.py +++ b/src/openforms/authentication/contrib/eherkenning/apps.py @@ -10,3 +10,6 @@ class EHerkenningApp(AppConfig): def ready(self): # register the plugin from . import plugin # noqa + + # register the signals + from .signals import trigger_csp_update # noqa diff --git a/src/openforms/authentication/contrib/eherkenning/signals.py b/src/openforms/authentication/contrib/eherkenning/signals.py new file mode 100644 index 0000000000..01b429f39d --- /dev/null +++ b/src/openforms/authentication/contrib/eherkenning/signals.py @@ -0,0 +1,11 @@ +from django.db.models.signals import post_save +from django.dispatch import receiver + +from digid_eherkenning.models.eherkenning import EherkenningConfiguration + +from openforms.config.utils import create_digid_eherkenning_csp_settings + + +@receiver(post_save, sender=EherkenningConfiguration) +def trigger_csp_update(sender, instance, **kwargs): + create_digid_eherkenning_csp_settings(instance, "eherkenning") diff --git a/src/openforms/authentication/contrib/eherkenning/tests/test_csp_update.py b/src/openforms/authentication/contrib/eherkenning/tests/test_csp_update.py new file mode 100644 index 0000000000..6a051a37c3 --- /dev/null +++ b/src/openforms/authentication/contrib/eherkenning/tests/test_csp_update.py @@ -0,0 +1,123 @@ +from pathlib import Path +from unittest.mock import patch + +from django.test import TestCase +from django.urls import reverse + +from digid_eherkenning.models.eherkenning import 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 FormFactory +from openforms.utils.tests.cache import clear_caches + +TEST_FILES = Path(__file__).parent / "data" +METADATA = TEST_FILES / "eherkenning-metadata.xml" + + +@temp_private_root() +class CSPUpdateTests(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(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 + with METADATA.open("rb") as md_file: + metadata_content = md_file.read().decode("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", + ) + + # 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.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", + ) + + @patch( + "onelogin.saml2.idp_metadata_parser.OneLogin_Saml2_IdPMetadataParser.get_metadata" + ) + def test_response_headers_contain_form_action_values(self, get_metadata): + form = FormFactory.create( + authentication_backends=["eherkenning"], + generate_minimal_setup=True, + formstep__form_definition__login_required=True, + ) + with METADATA.open("rb") as md_file: + metadata_content = md_file.read().decode("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.assertIn( + "form-action " + "'self' " + "https://test-iwelcome.nl/broker/sso/1.13 " + "https://ehm01.iwelcome.nl/broker/slo/1.13;", + response.headers["Content-Security-Policy"], + ) diff --git a/src/openforms/conf/base.py b/src/openforms/conf/base.py index b02b4f3836..4f412e7a48 100644 --- a/src/openforms/conf/base.py +++ b/src/openforms/conf/base.py @@ -1099,6 +1099,8 @@ "'self'", ] + config("CSP_EXTRA_DEFAULT_SRC", default=[], split=True) +CSP_FORM_ACTION = ("'self'",) + # * service.pdok.nl serves the tiles for the Leaflet maps (PNGs) and must be whitelisted # * the data: URIs are used by Leaflet (invisible pixel for memory management/image unloading) # and the signature component which saves the image drawn on the canvas as data: URI diff --git a/src/openforms/config/admin.py b/src/openforms/config/admin.py index 368b396683..ca461601c1 100644 --- a/src/openforms/config/admin.py +++ b/src/openforms/config/admin.py @@ -1,4 +1,6 @@ from django.contrib import admin +from django.urls import reverse +from django.utils.html import format_html from django.utils.translation import ugettext_lazy as _ from django_better_admin_arrayfield.admin.mixins import DynamicArrayMixin @@ -182,9 +184,11 @@ class RichTextColorAdmin(admin.ModelAdmin): @admin.register(CSPSetting) class CSPSettingAdmin(admin.ModelAdmin): + readonly_fields = ("content_type_link",) fields = [ "directive", "value", + "content_type_link", ] list_display = [ "directive", @@ -197,3 +201,11 @@ class CSPSettingAdmin(admin.ModelAdmin): "directive", "value", ] + + def content_type_link(self, obj): + ct = obj.content_type + url = reverse(f"admin:{ct.app_label}_{ct.model}_change", args=(obj.object_id,)) + link = format_html('{t}', u=url, t=str(obj.content_object)) + return link + + content_type_link.short_description = _("Content type") diff --git a/src/openforms/config/constants.py b/src/openforms/config/constants.py index c128f52b7b..a127de943c 100644 --- a/src/openforms/config/constants.py +++ b/src/openforms/config/constants.py @@ -1,6 +1,11 @@ from django.db import models from django.utils.translation import gettext_lazy as _ +ADDITIONAL_CSP_VALUES = { + "digid": "https://digid.nl https://*.digid.nl", + "eherkenning": "", +} + class CSPDirective(models.TextChoices): # via https://django-csp.readthedocs.io/en/latest/configuration.html diff --git a/src/openforms/config/migrations/0058_auto_20231024_1110.py b/src/openforms/config/migrations/0058_auto_20231024_1110.py new file mode 100644 index 0000000000..65b9e130e4 --- /dev/null +++ b/src/openforms/config/migrations/0058_auto_20231024_1110.py @@ -0,0 +1,39 @@ +# Generated by Django 3.2.21 on 2023-10-24 09:10 + +from django.db import migrations, models +import django.db.models.deletion + + +class Migration(migrations.Migration): + dependencies = [ + ("contenttypes", "0002_remove_content_type_name"), + ("config", "0057_globalconfiguration_recipients_email_digest"), + ] + + operations = [ + migrations.AddField( + model_name="cspsetting", + name="content_type", + field=models.ForeignKey( + blank=True, + null=True, + on_delete=django.db.models.deletion.SET_NULL, + to="contenttypes.contenttype", + verbose_name="content type", + ), + ), + migrations.AddField( + model_name="cspsetting", + name="object_id", + field=models.TextField( + blank=True, db_index=True, null=True, verbose_name="object id" + ), + ), + migrations.AlterField( + model_name="cspsetting", + name="value", + field=models.CharField( + help_text="CSP header value", max_length=255, verbose_name="value" + ), + ), + ] diff --git a/src/openforms/config/models.py b/src/openforms/config/models.py index d081043a48..ed071d709e 100644 --- a/src/openforms/config/models.py +++ b/src/openforms/config/models.py @@ -1,6 +1,8 @@ from collections import defaultdict from functools import partial +from django.contrib.contenttypes.fields import GenericForeignKey +from django.contrib.contenttypes.models import ContentType from django.core.exceptions import ValidationError from django.core.validators import ( FileExtensionValidator, @@ -658,9 +660,23 @@ class CSPSetting(models.Model): ) value = models.CharField( _("value"), - max_length=128, + max_length=255, help_text=_("CSP header value"), ) + content_type = models.ForeignKey( + ContentType, + verbose_name=_("content type"), + on_delete=models.SET_NULL, + blank=True, + null=True, + ) + object_id = models.TextField( + verbose_name=_("object id"), + blank=True, + null=True, + db_index=True, + ) + content_object = GenericForeignKey("content_type", "object_id") objects = CSPSettingQuerySet.as_manager() diff --git a/src/openforms/config/tests/test_admin.py b/src/openforms/config/tests/test_admin.py new file mode 100644 index 0000000000..428f6c889e --- /dev/null +++ b/src/openforms/config/tests/test_admin.py @@ -0,0 +1,28 @@ +from django.contrib.admin.sites import AdminSite +from django.test import TestCase +from django.urls import reverse + +from openforms.config.models import CSPSetting +from openforms.payments.contrib.ogone.tests.factories import OgoneMerchantFactory + +from ..admin import CSPSettingAdmin + + +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) diff --git a/src/openforms/config/utils.py b/src/openforms/config/utils.py index 69af7bf139..a864dc64a6 100644 --- a/src/openforms/config/utils.py +++ b/src/openforms/config/utils.py @@ -1,7 +1,18 @@ from dataclasses import dataclass +from typing import TYPE_CHECKING, Union + +from django.contrib.admin.options import get_content_type_for_model +from django.db.models import Model import clamd +from openforms.config.constants import ADDITIONAL_CSP_VALUES, CSPDirective +from openforms.payments.contrib.ogone.models import OgoneMerchant + +if TYPE_CHECKING: # pragma: nocover + from digid_eherkenning.models.digid import DigidConfiguration + from digid_eherkenning.models.eherkenning import EherkenningConfiguration + @dataclass class ClamAVStatus: @@ -24,3 +35,38 @@ def verify_clamav_connection(host: str, port: int, timeout: int) -> "ClamAVStatu return ClamAVStatus(can_connect=True) return ClamAVStatus(can_connect=False, error=result) + + +def _create_csp_settings(obj: Model, directive: str, value: str) -> None: + from .models import CSPSetting + + CSPSetting.objects.filter( + content_type=get_content_type_for_model(obj), object_id=str(obj.id) + ).delete() + + CSPSetting.objects.create(content_object=obj, directive=directive, value=value) + + +def create_digid_eherkenning_csp_settings( + config: Union["DigidConfiguration", "EherkenningConfiguration"], config_type: str +) -> 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() + if ADDITIONAL_CSP_VALUES[config_type]: + urls = ( + f"{ADDITIONAL_CSP_VALUES[config_type]} {urls['sso_url']} {urls['slo_url']}" + ) + else: + urls = f"{urls['sso_url']} {urls['slo_url']}" + + _create_csp_settings(config, CSPDirective.FORM_ACTION, urls) + + +def create_ogone_csp_settings( + merchant: OgoneMerchant, directive: str, value: str +) -> None: + _create_csp_settings(merchant, directive, value) diff --git a/src/openforms/payments/contrib/ogone/models.py b/src/openforms/payments/contrib/ogone/models.py index c20f37d055..e4b59e7d28 100644 --- a/src/openforms/payments/contrib/ogone/models.py +++ b/src/openforms/payments/contrib/ogone/models.py @@ -2,6 +2,8 @@ from django.db import models from django.utils.translation import gettext_lazy as _ +from openforms.config.constants import CSPDirective + from .constants import HashAlgorithm, OgoneEndpoints @@ -61,3 +63,10 @@ def clean(self): def __str__(self): return self.label + + def save(self, *args, **kwargs): + super().save(*args, **kwargs) + + from openforms.config.utils import create_ogone_csp_settings + + create_ogone_csp_settings(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 new file mode 100644 index 0000000000..37d54f2724 --- /dev/null +++ b/src/openforms/payments/contrib/ogone/tests/test_models.py @@ -0,0 +1,81 @@ +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 a1be7a892c..75b3d705c8 100644 --- a/src/openforms/payments/contrib/ogone/tests/test_plugin.py +++ b/src/openforms/payments/contrib/ogone/tests/test_plugin.py @@ -8,7 +8,7 @@ from ....registry import register from ....tests.factories import SubmissionPaymentFactory -from ..constants import OgoneStatus, PaymentStatus +from ..constants import OgoneEndpoints, OgoneStatus, PaymentStatus from ..plugin import RETURN_ACTION_PARAM from ..signing import calculate_sha_out from .factories import OgoneMerchantFactory @@ -158,6 +158,12 @@ 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) From 29f63398b2f2c99d100c395ab8cfa8c92293e597 Mon Sep 17 00:00:00 2001 From: vasileios Date: Wed, 25 Oct 2023 09:56:47 +0200 Subject: [PATCH 2/6] [#3477] Added draft info to the changelog concerning the update in digid-eherkenning configs --- CHANGELOG.rst | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/CHANGELOG.rst b/CHANGELOG.rst index 96f3a6814c..753f25e132 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -7,6 +7,16 @@ Changelog .. note:: These release notes are in development! +**DigiD - Eherkenning Configuration** + +The configuration concerning the digid and eherkenning has been updated according to the +new version (0.9) of the django-digid-eherkenning library. The XML metadata file is now +automatically retrieved so you have to edit the configuration via: +**Admin** > **Configuratie** > **DigiD-configuratie** +**Admin** > **Configuratie** > **EHerkenning/eIDAS-configuratie** +and add the XML metadata url. The identity provider and the metadata file fields will be +automatically populated if the url is valid. + Upgrade procedure ----------------- From 373d8e3184b164185ad0a787442cfa6810291b85 Mon Sep 17 00:00:00 2001 From: vasileios Date: Thu, 26 Oct 2023 12:07:34 +0200 Subject: [PATCH 3/6] [#3477] PR feedback --- .../authentication/contrib/digid/apps.py | 3 - .../authentication/contrib/digid/signals.py | 11 - .../contrib/digid/tests/test_csp_update.py | 125 ---------- .../contrib/eherkenning/apps.py | 3 - .../contrib/eherkenning/signals.py | 11 - .../eherkenning/tests/test_csp_update.py | 123 --------- src/openforms/conf/base.py | 1 + src/openforms/config/constants.py | 5 - ...024_1110.py => 0058_auto_20231026_1133.py} | 2 +- src/openforms/config/models.py | 34 ++- src/openforms/config/utils.py | 46 ---- .../contrib/digid_eherkenning/apps.py | 12 + .../contrib/digid_eherkenning/constants.py | 12 + .../contrib/digid_eherkenning/signals.py | 19 ++ .../digid_eherkenning/tests/__init__.py | 0 .../tests/data/eherkenning-metadata.xml | 97 +++++++ .../tests/data/metadata_POST_bindings.xml | 0 .../tests/test_csp_update.py | 236 ++++++++++++++++++ .../contrib/digid_eherkenning/utils.py | 26 ++ .../payments/contrib/ogone/models.py | 4 +- 20 files changed, 439 insertions(+), 331 deletions(-) delete mode 100644 src/openforms/authentication/contrib/digid/signals.py delete mode 100644 src/openforms/authentication/contrib/digid/tests/test_csp_update.py delete mode 100644 src/openforms/authentication/contrib/eherkenning/signals.py delete mode 100644 src/openforms/authentication/contrib/eherkenning/tests/test_csp_update.py rename src/openforms/config/migrations/{0058_auto_20231024_1110.py => 0058_auto_20231026_1133.py} (95%) create mode 100644 src/openforms/contrib/digid_eherkenning/apps.py create mode 100644 src/openforms/contrib/digid_eherkenning/constants.py create mode 100644 src/openforms/contrib/digid_eherkenning/signals.py create mode 100644 src/openforms/contrib/digid_eherkenning/tests/__init__.py create mode 100644 src/openforms/contrib/digid_eherkenning/tests/data/eherkenning-metadata.xml rename src/openforms/{authentication/contrib/digid => contrib/digid_eherkenning}/tests/data/metadata_POST_bindings.xml (100%) create mode 100644 src/openforms/contrib/digid_eherkenning/tests/test_csp_update.py diff --git a/src/openforms/authentication/contrib/digid/apps.py b/src/openforms/authentication/contrib/digid/apps.py index dadc987992..441c3aa3e1 100644 --- a/src/openforms/authentication/contrib/digid/apps.py +++ b/src/openforms/authentication/contrib/digid/apps.py @@ -10,6 +10,3 @@ class DigidApp(AppConfig): def ready(self): # register the plugin from . import plugin # noqa - - # register the signals - from .signals import trigger_csp_update # noqa diff --git a/src/openforms/authentication/contrib/digid/signals.py b/src/openforms/authentication/contrib/digid/signals.py deleted file mode 100644 index 712efad6e4..0000000000 --- a/src/openforms/authentication/contrib/digid/signals.py +++ /dev/null @@ -1,11 +0,0 @@ -from django.db.models.signals import post_save -from django.dispatch import receiver - -from digid_eherkenning.models.digid import DigidConfiguration - -from openforms.config.utils import create_digid_eherkenning_csp_settings - - -@receiver(post_save, sender=DigidConfiguration) -def trigger_csp_update(sender, instance, **kwargs): - create_digid_eherkenning_csp_settings(instance, "digid") diff --git a/src/openforms/authentication/contrib/digid/tests/test_csp_update.py b/src/openforms/authentication/contrib/digid/tests/test_csp_update.py deleted file mode 100644 index 3313d4af79..0000000000 --- a/src/openforms/authentication/contrib/digid/tests/test_csp_update.py +++ /dev/null @@ -1,125 +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.digid import DigidConfiguration -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" -METADATA_POST = TEST_FILES / "metadata_POST_bindings.xml" - - -@temp_private_root() -@override_settings(CORS_ALLOW_ALL_ORIGINS=True, IS_HTTPS=True) -class CSPUpdateTests(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(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 - with METADATA_POST.open("rb") as md_file: - metadata_content = md_file.read().decode("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://digid.nl " - "https://*.digid.nl " - "https://test-digid.nl/saml/idp/request_authentication " - "https://test-digid.nl/saml/idp/request_logout", - ) - - # 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.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://digid.nl " - "https://*.digid.nl " - "https://test-digid.nl/saml/idp/request_authentication " - "https://test-digid.nl/saml/idp/request_logout", - ) - - @patch( - "onelogin.saml2.idp_metadata_parser.OneLogin_Saml2_IdPMetadataParser.get_metadata" - ) - def test_response_headers_contain_form_action_values(self, get_metadata): - form = FormFactory.create(authentication_backends=["digid"]) - form_definition = FormDefinitionFactory.create(login_required=True) - FormStepFactory.create(form_definition=form_definition, form=form) - - with METADATA_POST.open("rb") as md_file: - metadata_content = md_file.read().decode("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(f"{login_url}?next={form_url}", follow=True) - - self.assertIn( - "form-action " - "'self' " - "https://digid.nl " - "https://*.digid.nl " - "https://test-digid.nl/saml/idp/request_authentication " - "https://test-digid.nl/saml/idp/request_logout;", - response.headers["Content-Security-Policy"], - ) diff --git a/src/openforms/authentication/contrib/eherkenning/apps.py b/src/openforms/authentication/contrib/eherkenning/apps.py index a53e681ab2..20fa4b3e92 100644 --- a/src/openforms/authentication/contrib/eherkenning/apps.py +++ b/src/openforms/authentication/contrib/eherkenning/apps.py @@ -10,6 +10,3 @@ class EHerkenningApp(AppConfig): def ready(self): # register the plugin from . import plugin # noqa - - # register the signals - from .signals import trigger_csp_update # noqa diff --git a/src/openforms/authentication/contrib/eherkenning/signals.py b/src/openforms/authentication/contrib/eherkenning/signals.py deleted file mode 100644 index 01b429f39d..0000000000 --- a/src/openforms/authentication/contrib/eherkenning/signals.py +++ /dev/null @@ -1,11 +0,0 @@ -from django.db.models.signals import post_save -from django.dispatch import receiver - -from digid_eherkenning.models.eherkenning import EherkenningConfiguration - -from openforms.config.utils import create_digid_eherkenning_csp_settings - - -@receiver(post_save, sender=EherkenningConfiguration) -def trigger_csp_update(sender, instance, **kwargs): - create_digid_eherkenning_csp_settings(instance, "eherkenning") diff --git a/src/openforms/authentication/contrib/eherkenning/tests/test_csp_update.py b/src/openforms/authentication/contrib/eherkenning/tests/test_csp_update.py deleted file mode 100644 index 6a051a37c3..0000000000 --- a/src/openforms/authentication/contrib/eherkenning/tests/test_csp_update.py +++ /dev/null @@ -1,123 +0,0 @@ -from pathlib import Path -from unittest.mock import patch - -from django.test import TestCase -from django.urls import reverse - -from digid_eherkenning.models.eherkenning import 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 FormFactory -from openforms.utils.tests.cache import clear_caches - -TEST_FILES = Path(__file__).parent / "data" -METADATA = TEST_FILES / "eherkenning-metadata.xml" - - -@temp_private_root() -class CSPUpdateTests(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(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 - with METADATA.open("rb") as md_file: - metadata_content = md_file.read().decode("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", - ) - - # 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.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", - ) - - @patch( - "onelogin.saml2.idp_metadata_parser.OneLogin_Saml2_IdPMetadataParser.get_metadata" - ) - def test_response_headers_contain_form_action_values(self, get_metadata): - form = FormFactory.create( - authentication_backends=["eherkenning"], - generate_minimal_setup=True, - formstep__form_definition__login_required=True, - ) - with METADATA.open("rb") as md_file: - metadata_content = md_file.read().decode("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.assertIn( - "form-action " - "'self' " - "https://test-iwelcome.nl/broker/sso/1.13 " - "https://ehm01.iwelcome.nl/broker/slo/1.13;", - response.headers["Content-Security-Policy"], - ) diff --git a/src/openforms/conf/base.py b/src/openforms/conf/base.py index 4f412e7a48..0af273f12e 100644 --- a/src/openforms/conf/base.py +++ b/src/openforms/conf/base.py @@ -210,6 +210,7 @@ "openforms.logging.apps.LoggingAppConfig", "openforms.contrib.bag.apps.BAGConfig", # TODO: remove once 2.4.0 is released "openforms.contrib.brp", + "openforms.contrib.digid_eherkenning", "openforms.contrib.haal_centraal", "openforms.contrib.kadaster", "openforms.contrib.kvk", diff --git a/src/openforms/config/constants.py b/src/openforms/config/constants.py index a127de943c..c128f52b7b 100644 --- a/src/openforms/config/constants.py +++ b/src/openforms/config/constants.py @@ -1,11 +1,6 @@ from django.db import models from django.utils.translation import gettext_lazy as _ -ADDITIONAL_CSP_VALUES = { - "digid": "https://digid.nl https://*.digid.nl", - "eherkenning": "", -} - class CSPDirective(models.TextChoices): # via https://django-csp.readthedocs.io/en/latest/configuration.html diff --git a/src/openforms/config/migrations/0058_auto_20231024_1110.py b/src/openforms/config/migrations/0058_auto_20231026_1133.py similarity index 95% rename from src/openforms/config/migrations/0058_auto_20231024_1110.py rename to src/openforms/config/migrations/0058_auto_20231026_1133.py index 65b9e130e4..33e36d30db 100644 --- a/src/openforms/config/migrations/0058_auto_20231024_1110.py +++ b/src/openforms/config/migrations/0058_auto_20231026_1133.py @@ -1,4 +1,4 @@ -# Generated by Django 3.2.21 on 2023-10-24 09:10 +# Generated by Django 3.2.21 on 2023-10-26 09:33 from django.db import migrations, models import django.db.models.deletion diff --git a/src/openforms/config/models.py b/src/openforms/config/models.py index ed071d709e..48b0936909 100644 --- a/src/openforms/config/models.py +++ b/src/openforms/config/models.py @@ -1,6 +1,9 @@ +import logging from collections import defaultdict from functools import partial +from typing import Tuple +from django.contrib.admin.options import get_content_type_for_model from django.contrib.contenttypes.fields import GenericForeignKey from django.contrib.contenttypes.models import ContentType from django.core.exceptions import ValidationError @@ -34,6 +37,8 @@ from .constants import CSPDirective, UploadFileType from .utils import verify_clamav_connection +logger = logging.getLogger(__name__) + @ensure_default_language() def _render(filename): @@ -651,6 +656,33 @@ def as_dict(self): return {k: list(v) for k, v in ret.items()} +class CSPSettingManager(models.Manager.from_queryset(CSPSettingQuerySet)): + def set_for(self, obj: models.Model, settings: list[Tuple[str, str]]) -> None: + """ + Deletes all the connected csp settings and creates new ones based on the new provided data. + """ + instances = [] + for setting in settings: + directive, value = setting + if not directive in CSPDirective.values: + logger.error( + "Could not create csp setting for model '%s'. '%s' is not a valid directive.", + obj, + directive, + ) + return + + instances.append( + CSPSetting(content_object=obj, directive=directive, value=value) + ) + + CSPSetting.objects.filter( + content_type=get_content_type_for_model(obj), object_id=str(obj.id) + ).delete() + + self.bulk_create(instances) + + class CSPSetting(models.Model): directive = models.CharField( _("directive"), @@ -678,7 +710,7 @@ class CSPSetting(models.Model): ) content_object = GenericForeignKey("content_type", "object_id") - objects = CSPSettingQuerySet.as_manager() + objects = CSPSettingManager() class Meta: ordering = ("directive", "value") diff --git a/src/openforms/config/utils.py b/src/openforms/config/utils.py index a864dc64a6..69af7bf139 100644 --- a/src/openforms/config/utils.py +++ b/src/openforms/config/utils.py @@ -1,18 +1,7 @@ from dataclasses import dataclass -from typing import TYPE_CHECKING, Union - -from django.contrib.admin.options import get_content_type_for_model -from django.db.models import Model import clamd -from openforms.config.constants import ADDITIONAL_CSP_VALUES, CSPDirective -from openforms.payments.contrib.ogone.models import OgoneMerchant - -if TYPE_CHECKING: # pragma: nocover - from digid_eherkenning.models.digid import DigidConfiguration - from digid_eherkenning.models.eherkenning import EherkenningConfiguration - @dataclass class ClamAVStatus: @@ -35,38 +24,3 @@ def verify_clamav_connection(host: str, port: int, timeout: int) -> "ClamAVStatu return ClamAVStatus(can_connect=True) return ClamAVStatus(can_connect=False, error=result) - - -def _create_csp_settings(obj: Model, directive: str, value: str) -> None: - from .models import CSPSetting - - CSPSetting.objects.filter( - content_type=get_content_type_for_model(obj), object_id=str(obj.id) - ).delete() - - CSPSetting.objects.create(content_object=obj, directive=directive, value=value) - - -def create_digid_eherkenning_csp_settings( - config: Union["DigidConfiguration", "EherkenningConfiguration"], config_type: str -) -> 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() - if ADDITIONAL_CSP_VALUES[config_type]: - urls = ( - f"{ADDITIONAL_CSP_VALUES[config_type]} {urls['sso_url']} {urls['slo_url']}" - ) - else: - urls = f"{urls['sso_url']} {urls['slo_url']}" - - _create_csp_settings(config, CSPDirective.FORM_ACTION, urls) - - -def create_ogone_csp_settings( - merchant: OgoneMerchant, directive: str, value: str -) -> None: - _create_csp_settings(merchant, directive, value) diff --git a/src/openforms/contrib/digid_eherkenning/apps.py b/src/openforms/contrib/digid_eherkenning/apps.py new file mode 100644 index 0000000000..60c2b7cb3a --- /dev/null +++ b/src/openforms/contrib/digid_eherkenning/apps.py @@ -0,0 +1,12 @@ +from django.apps import AppConfig +from django.utils.translation import gettext_lazy as _ + + +class DigidEherkenningApp(AppConfig): + name = "openforms.contrib.digid_eherkenning" + label = "contrib_digid_eherkenning" + verbose_name = _("DigiD Eherkenning Contrib") + + 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 new file mode 100644 index 0000000000..9e77ad51f9 --- /dev/null +++ b/src/openforms/contrib/digid_eherkenning/constants.py @@ -0,0 +1,12 @@ +from digid_eherkenning.models.digid import DigidConfiguration +from digid_eherkenning.models.eherkenning import EherkenningConfiguration + +ADDITIONAL_CSP_VALUES = { + "digid": "https://digid.nl https://*.digid.nl", + "eherkenning": "", +} + +CONFIG_TYPES = { + DigidConfiguration: "digid", + EherkenningConfiguration: "eherkenning", +} diff --git a/src/openforms/contrib/digid_eherkenning/signals.py b/src/openforms/contrib/digid_eherkenning/signals.py new file mode 100644 index 0000000000..8eae8e86c3 --- /dev/null +++ b/src/openforms/contrib/digid_eherkenning/signals.py @@ -0,0 +1,19 @@ +from django.db.models.signals import post_save +from django.dispatch import receiver + +from digid_eherkenning.models.digid import DigidConfiguration +from digid_eherkenning.models.eherkenning import EherkenningConfiguration + +from openforms.contrib.digid_eherkenning.utils import ( + create_digid_eherkenning_csp_settings, +) + +from .constants import CONFIG_TYPES + + +@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, CONFIG_TYPES[type(instance)]) diff --git a/src/openforms/contrib/digid_eherkenning/tests/__init__.py b/src/openforms/contrib/digid_eherkenning/tests/__init__.py new file mode 100644 index 0000000000..e69de29bb2 diff --git a/src/openforms/contrib/digid_eherkenning/tests/data/eherkenning-metadata.xml b/src/openforms/contrib/digid_eherkenning/tests/data/eherkenning-metadata.xml new file mode 100644 index 0000000000..34b0a18cea --- /dev/null +++ b/src/openforms/contrib/digid_eherkenning/tests/data/eherkenning-metadata.xml @@ -0,0 +1,97 @@ + + + + + + + + + + +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/authentication/contrib/digid/tests/data/metadata_POST_bindings.xml b/src/openforms/contrib/digid_eherkenning/tests/data/metadata_POST_bindings.xml similarity index 100% rename from src/openforms/authentication/contrib/digid/tests/data/metadata_POST_bindings.xml rename to src/openforms/contrib/digid_eherkenning/tests/data/metadata_POST_bindings.xml diff --git a/src/openforms/contrib/digid_eherkenning/tests/test_csp_update.py b/src/openforms/contrib/digid_eherkenning/tests/test_csp_update.py new file mode 100644 index 0000000000..dad5d87810 --- /dev/null +++ b/src/openforms/contrib/digid_eherkenning/tests/test_csp_update.py @@ -0,0 +1,236 @@ +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.digid import DigidConfiguration +from digid_eherkenning.models.eherkenning import 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://digid.nl " + "https://*.digid.nl " + "https://test-digid.nl/saml/idp/request_authentication " + "https://test-digid.nl/saml/idp/request_logout", + ) + + # 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, 4) + 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://digid.nl " + "https://*.digid.nl " + "https://test-digid.nl/saml/idp/request_authentication " + "https://test-digid.nl/saml/idp/request_logout", + ) + + @patch( + "onelogin.saml2.idp_metadata_parser.OneLogin_Saml2_IdPMetadataParser.get_metadata" + ) + 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, 2) + self.assertIn( + "form-action " + "'self' " + "https://digid.nl " + "https://*.digid.nl " + "https://test-digid.nl/saml/idp/request_authentication " + "https://test-digid.nl/saml/idp/request_logout;", + 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", + ) + + # 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, 4) + 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", + ) + + @patch( + "onelogin.saml2.idp_metadata_parser.OneLogin_Saml2_IdPMetadataParser.get_metadata" + ) + 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, 2) + self.assertIn( + "form-action " + "'self' " + "https://test-iwelcome.nl/broker/sso/1.13 " + "https://ehm01.iwelcome.nl/broker/slo/1.13;", + response.headers["Content-Security-Policy"], + ) diff --git a/src/openforms/contrib/digid_eherkenning/utils.py b/src/openforms/contrib/digid_eherkenning/utils.py index 42c3d403db..7507125620 100644 --- a/src/openforms/contrib/digid_eherkenning/utils.py +++ b/src/openforms/contrib/digid_eherkenning/utils.py @@ -2,7 +2,14 @@ from django.templatetags.static import static +from digid_eherkenning.models.digid import DigidConfiguration +from digid_eherkenning.models.eherkenning import EherkenningConfiguration + 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]: @@ -19,3 +26,22 @@ def get_eherkenning_logo(request) -> Dict[str, str]: "href": "https://www.eherkenning.nl/", "appearance": LogoAppearance.light, } + + +def create_digid_eherkenning_csp_settings( + config: DigidConfiguration | EherkenningConfiguration, config_type: str +) -> 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() + if ADDITIONAL_CSP_VALUES[config_type]: + urls = ( + f"{ADDITIONAL_CSP_VALUES[config_type]} {urls['sso_url']} {urls['slo_url']}" + ) + else: + urls = f"{urls['sso_url']} {urls['slo_url']}" + + CSPSetting.objects.set_for(config, [(CSPDirective.FORM_ACTION, urls)]) diff --git a/src/openforms/payments/contrib/ogone/models.py b/src/openforms/payments/contrib/ogone/models.py index e4b59e7d28..31d394f4f5 100644 --- a/src/openforms/payments/contrib/ogone/models.py +++ b/src/openforms/payments/contrib/ogone/models.py @@ -67,6 +67,6 @@ def __str__(self): def save(self, *args, **kwargs): super().save(*args, **kwargs) - from openforms.config.utils import create_ogone_csp_settings + from openforms.config.models import CSPSetting - create_ogone_csp_settings(self, CSPDirective.FORM_ACTION, self.endpoint) + CSPSetting.objects.set_for(self, [(CSPDirective.FORM_ACTION, self.endpoint)]) From fc68e8fd5f7722600272c4c3dbcfafe3d67ef02e Mon Sep 17 00:00:00 2001 From: vasileios Date: Thu, 26 Oct 2023 12:11:13 +0200 Subject: [PATCH 4/6] [#3477] Fixed flake8 error --- src/openforms/config/models.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/openforms/config/models.py b/src/openforms/config/models.py index 48b0936909..8d85fcc51c 100644 --- a/src/openforms/config/models.py +++ b/src/openforms/config/models.py @@ -664,7 +664,7 @@ def set_for(self, obj: models.Model, settings: list[Tuple[str, str]]) -> None: instances = [] for setting in settings: directive, value = setting - if not directive in CSPDirective.values: + if directive not in CSPDirective.values: logger.error( "Could not create csp setting for model '%s'. '%s' is not a valid directive.", obj, From 96d7aedaf8d35c5f39384035b4cc6390cc2a0e3a Mon Sep 17 00:00:00 2001 From: vasileios Date: Thu, 26 Oct 2023 15:02:24 +0200 Subject: [PATCH 5/6] [#3477] Added missing test --- src/openforms/config/models.py | 3 +- src/openforms/config/tests/test_models.py | 34 +++++++++++++++++++++++ 2 files changed, 35 insertions(+), 2 deletions(-) create mode 100644 src/openforms/config/tests/test_models.py diff --git a/src/openforms/config/models.py b/src/openforms/config/models.py index 8d85fcc51c..a5de33f633 100644 --- a/src/openforms/config/models.py +++ b/src/openforms/config/models.py @@ -1,7 +1,6 @@ import logging from collections import defaultdict from functools import partial -from typing import Tuple from django.contrib.admin.options import get_content_type_for_model from django.contrib.contenttypes.fields import GenericForeignKey @@ -657,7 +656,7 @@ def as_dict(self): class CSPSettingManager(models.Manager.from_queryset(CSPSettingQuerySet)): - def set_for(self, obj: models.Model, settings: list[Tuple[str, str]]) -> None: + def set_for(self, obj: models.Model, settings: list[tuple[str, str]]) -> None: """ Deletes all the connected csp settings and creates new ones based on the new provided data. """ diff --git a/src/openforms/config/tests/test_models.py b/src/openforms/config/tests/test_models.py new file mode 100644 index 0000000000..89186131c7 --- /dev/null +++ b/src/openforms/config/tests/test_models.py @@ -0,0 +1,34 @@ +from django.test import TestCase + +from openforms.config.constants import CSPDirective +from openforms.config.models import CSPSetting +from openforms.payments.contrib.ogone.tests.factories import OgoneMerchantFactory + + +class CSPSettingManagerTests(TestCase): + def test_csp_setting_is_set(self): + merchant = OgoneMerchantFactory() + + CSPSetting.objects.set_for( + merchant, [(CSPDirective.FORM_ACTION, "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_setting_is_not_set_with_wrong_directive(self): + merchant = OgoneMerchantFactory() + + with self.assertLogs() as logs: + CSPSetting.objects.set_for( + merchant, [("wrong-directive", "http://example.com")] + ) + message = logs.records[0].getMessage() + + self.assertEqual( + message, + f"Could not create csp setting for model '{merchant.__str__()}'. 'wrong-directive' is not a valid directive.", + ) + self.assertTrue(CSPSetting.objects.none) From a378c38ccb0eca62bdddc31b85fb9738346dc500 Mon Sep 17 00:00:00 2001 From: vasileios Date: Thu, 26 Oct 2023 16:19:46 +0200 Subject: [PATCH 6/6] [#3477] New PR feedback --- ...026_1133.py => 0058_auto_20231026_1525.py} | 7 ++-- src/openforms/config/models.py | 26 +++++--------- src/openforms/config/tests/test_models.py | 34 ------------------- .../contrib/digid_eherkenning/apps.py | 2 +- .../contrib/digid_eherkenning/constants.py | 12 ++----- .../contrib/digid_eherkenning/signals.py | 11 ++---- .../tests/test_csp_update.py | 21 ++++++------ .../contrib/digid_eherkenning/utils.py | 19 +++++------ 8 files changed, 37 insertions(+), 95 deletions(-) rename src/openforms/config/migrations/{0058_auto_20231026_1133.py => 0058_auto_20231026_1525.py} (85%) delete mode 100644 src/openforms/config/tests/test_models.py diff --git a/src/openforms/config/migrations/0058_auto_20231026_1133.py b/src/openforms/config/migrations/0058_auto_20231026_1525.py similarity index 85% rename from src/openforms/config/migrations/0058_auto_20231026_1133.py rename to src/openforms/config/migrations/0058_auto_20231026_1525.py index 33e36d30db..22fb1191bb 100644 --- a/src/openforms/config/migrations/0058_auto_20231026_1133.py +++ b/src/openforms/config/migrations/0058_auto_20231026_1525.py @@ -1,10 +1,11 @@ -# Generated by Django 3.2.21 on 2023-10-26 09:33 +# Generated by Django 3.2.21 on 2023-10-26 13:25 from django.db import migrations, models import django.db.models.deletion class Migration(migrations.Migration): + dependencies = [ ("contenttypes", "0002_remove_content_type_name"), ("config", "0057_globalconfiguration_recipients_email_digest"), @@ -25,9 +26,7 @@ class Migration(migrations.Migration): migrations.AddField( model_name="cspsetting", name="object_id", - field=models.TextField( - blank=True, db_index=True, null=True, verbose_name="object id" - ), + field=models.TextField(blank=True, db_index=True, verbose_name="object id"), ), migrations.AlterField( model_name="cspsetting", diff --git a/src/openforms/config/models.py b/src/openforms/config/models.py index a5de33f633..b7d0775230 100644 --- a/src/openforms/config/models.py +++ b/src/openforms/config/models.py @@ -12,7 +12,7 @@ MinValueValidator, RegexValidator, ) -from django.db import models +from django.db import models, transaction from django.template.loader import render_to_string from django.utils.encoding import force_str from django.utils.safestring import mark_safe @@ -656,24 +656,17 @@ def as_dict(self): class CSPSettingManager(models.Manager.from_queryset(CSPSettingQuerySet)): - def set_for(self, obj: models.Model, settings: list[tuple[str, str]]) -> None: + @transaction.atomic + def set_for( + self, obj: models.Model, settings: list[tuple[CSPDirective, str]] + ) -> None: """ Deletes all the connected csp settings and creates new ones based on the new provided data. """ - instances = [] - for setting in settings: - directive, value = setting - if directive not in CSPDirective.values: - logger.error( - "Could not create csp setting for model '%s'. '%s' is not a valid directive.", - obj, - directive, - ) - return - - instances.append( - CSPSetting(content_object=obj, directive=directive, value=value) - ) + instances = [ + CSPSetting(content_object=obj, directive=directive, value=value) + for directive, value in settings + ] CSPSetting.objects.filter( content_type=get_content_type_for_model(obj), object_id=str(obj.id) @@ -704,7 +697,6 @@ class CSPSetting(models.Model): object_id = models.TextField( verbose_name=_("object id"), blank=True, - null=True, db_index=True, ) content_object = GenericForeignKey("content_type", "object_id") diff --git a/src/openforms/config/tests/test_models.py b/src/openforms/config/tests/test_models.py deleted file mode 100644 index 89186131c7..0000000000 --- a/src/openforms/config/tests/test_models.py +++ /dev/null @@ -1,34 +0,0 @@ -from django.test import TestCase - -from openforms.config.constants import CSPDirective -from openforms.config.models import CSPSetting -from openforms.payments.contrib.ogone.tests.factories import OgoneMerchantFactory - - -class CSPSettingManagerTests(TestCase): - def test_csp_setting_is_set(self): - merchant = OgoneMerchantFactory() - - CSPSetting.objects.set_for( - merchant, [(CSPDirective.FORM_ACTION, "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_setting_is_not_set_with_wrong_directive(self): - merchant = OgoneMerchantFactory() - - with self.assertLogs() as logs: - CSPSetting.objects.set_for( - merchant, [("wrong-directive", "http://example.com")] - ) - message = logs.records[0].getMessage() - - self.assertEqual( - message, - f"Could not create csp setting for model '{merchant.__str__()}'. 'wrong-directive' is not a valid directive.", - ) - self.assertTrue(CSPSetting.objects.none) diff --git a/src/openforms/contrib/digid_eherkenning/apps.py b/src/openforms/contrib/digid_eherkenning/apps.py index 60c2b7cb3a..370d446507 100644 --- a/src/openforms/contrib/digid_eherkenning/apps.py +++ b/src/openforms/contrib/digid_eherkenning/apps.py @@ -5,7 +5,7 @@ class DigidEherkenningApp(AppConfig): name = "openforms.contrib.digid_eherkenning" label = "contrib_digid_eherkenning" - verbose_name = _("DigiD Eherkenning Contrib") + verbose_name = _("DigiD/Eherkenning utilities") def ready(self): # register the signals diff --git a/src/openforms/contrib/digid_eherkenning/constants.py b/src/openforms/contrib/digid_eherkenning/constants.py index 9e77ad51f9..d3840d966f 100644 --- a/src/openforms/contrib/digid_eherkenning/constants.py +++ b/src/openforms/contrib/digid_eherkenning/constants.py @@ -1,12 +1,6 @@ -from digid_eherkenning.models.digid import DigidConfiguration -from digid_eherkenning.models.eherkenning import EherkenningConfiguration +from digid_eherkenning.models import DigidConfiguration, EherkenningConfiguration ADDITIONAL_CSP_VALUES = { - "digid": "https://digid.nl https://*.digid.nl", - "eherkenning": "", -} - -CONFIG_TYPES = { - DigidConfiguration: "digid", - EherkenningConfiguration: "eherkenning", + DigidConfiguration: "https://digid.nl https://*.digid.nl", + EherkenningConfiguration: "", } diff --git a/src/openforms/contrib/digid_eherkenning/signals.py b/src/openforms/contrib/digid_eherkenning/signals.py index 8eae8e86c3..079fd8c447 100644 --- a/src/openforms/contrib/digid_eherkenning/signals.py +++ b/src/openforms/contrib/digid_eherkenning/signals.py @@ -1,14 +1,9 @@ from django.db.models.signals import post_save from django.dispatch import receiver -from digid_eherkenning.models.digid import DigidConfiguration -from digid_eherkenning.models.eherkenning import EherkenningConfiguration +from digid_eherkenning.models import DigidConfiguration, EherkenningConfiguration -from openforms.contrib.digid_eherkenning.utils import ( - create_digid_eherkenning_csp_settings, -) - -from .constants import CONFIG_TYPES +from .utils import create_digid_eherkenning_csp_settings @receiver(post_save, sender=DigidConfiguration) @@ -16,4 +11,4 @@ def trigger_csp_update( sender, instance: DigidConfiguration | EherkenningConfiguration, **kwargs ): - create_digid_eherkenning_csp_settings(instance, CONFIG_TYPES[type(instance)]) + create_digid_eherkenning_csp_settings(instance) diff --git a/src/openforms/contrib/digid_eherkenning/tests/test_csp_update.py b/src/openforms/contrib/digid_eherkenning/tests/test_csp_update.py index dad5d87810..c368a707b6 100644 --- a/src/openforms/contrib/digid_eherkenning/tests/test_csp_update.py +++ b/src/openforms/contrib/digid_eherkenning/tests/test_csp_update.py @@ -4,8 +4,7 @@ from django.test import TestCase, override_settings from django.urls import reverse -from digid_eherkenning.models.digid import DigidConfiguration -from digid_eherkenning.models.eherkenning import EherkenningConfiguration +from digid_eherkenning.models import DigidConfiguration, EherkenningConfiguration from privates.test import temp_private_root from simple_certmanager.test.factories import CertificateFactory @@ -69,10 +68,10 @@ def test_csp_updates_for_digid(self, get_metadata): self.assertEqual(csp_added.directive, CSPDirective.FORM_ACTION) self.assertEqual( csp_added.value, - "https://digid.nl " - "https://*.digid.nl " "https://test-digid.nl/saml/idp/request_authentication " - "https://test-digid.nl/saml/idp/request_logout", + "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 @@ -87,10 +86,10 @@ def test_csp_updates_for_digid(self, get_metadata): self.assertEqual(csp_updated.directive, CSPDirective.FORM_ACTION) self.assertEqual( csp_updated.value, - "https://digid.nl " - "https://*.digid.nl " "https://test-digid.nl/saml/idp/request_authentication " - "https://test-digid.nl/saml/idp/request_logout", + "https://test-digid.nl/saml/idp/request_logout " + "https://digid.nl " + "https://*.digid.nl", ) @patch( @@ -119,10 +118,10 @@ def test_response_headers_contain_form_action_values_in_digid(self, get_metadata self.assertIn( "form-action " "'self' " - "https://digid.nl " - "https://*.digid.nl " "https://test-digid.nl/saml/idp/request_authentication " - "https://test-digid.nl/saml/idp/request_logout;", + "https://test-digid.nl/saml/idp/request_logout " + "https://digid.nl " + "https://*.digid.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 7507125620..809ffc0c5d 100644 --- a/src/openforms/contrib/digid_eherkenning/utils.py +++ b/src/openforms/contrib/digid_eherkenning/utils.py @@ -2,8 +2,7 @@ from django.templatetags.static import static -from digid_eherkenning.models.digid import DigidConfiguration -from digid_eherkenning.models.eherkenning import EherkenningConfiguration +from digid_eherkenning.models import DigidConfiguration, EherkenningConfiguration from openforms.authentication.constants import LogoAppearance from openforms.config.constants import CSPDirective @@ -29,7 +28,7 @@ def get_eherkenning_logo(request) -> Dict[str, str]: def create_digid_eherkenning_csp_settings( - config: DigidConfiguration | EherkenningConfiguration, config_type: str + config: DigidConfiguration | EherkenningConfiguration, ) -> None: if not config.metadata_file_source: return @@ -37,11 +36,9 @@ def create_digid_eherkenning_csp_settings( # create the new directives based on the POST bindings of the metadata XML and # the additional constants urls, _ = config.process_metadata_from_xml_source() - if ADDITIONAL_CSP_VALUES[config_type]: - urls = ( - f"{ADDITIONAL_CSP_VALUES[config_type]} {urls['sso_url']} {urls['slo_url']}" - ) - else: - urls = f"{urls['sso_url']} {urls['slo_url']}" - - CSPSetting.objects.set_for(config, [(CSPDirective.FORM_ACTION, urls)]) + 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) + CSPSetting.objects.set_for(config, [(CSPDirective.FORM_ACTION, form_action_urls)])